Re: [PATCH v2] PWM: imx: pwm-imx-tpm: Add driver support

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Michael,

On 28.01.25 16:14, Michael Grzeschik wrote:
> +	for (i = 0; i < npwm; i++) {
> +		struct imx_tpm_pwm_chip *tpmc = &tpm->pwm[i];
> +
> +		chip = &tpmc->chip;
> +
> +		tpmc->tpm = tpm;
> +		tpmc->tpm->base = base;
> +		tpmc->tpm->clk = clk;
> +
> +		chip->ops = &imx_tpm_pwm_ops;
> +		chip->devname = xasprintf("pwm%d", i);

You're not registering different PWM controllers here, but channels
of the same PWM controller. If another PWM controller is enabled
and probed, it will try to claim pwm0 again for itself and the
later probe would fail.

Until we have PWM channel support in the core, you will need to
take care of this here yourself by using e.g. pwmXchY as name.
See drivers/pwm/pwm-stm32.c for an example. You'll likely want
to drop the + 1 there though, because TPM channels start counting
from zero (as seen in imx93-pinfunc.h)

Thanks,
Ahmad

> +		chip->id = i;
> +		chip->dev = dev;
> +
> +		ret = pwmchip_add(chip);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "failed to add PWM chip\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id imx_tpm_pwm_dt_ids[] = {
> +	{ .compatible = "fsl,imx7ulp-pwm", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx_tpm_pwm_dt_ids);
> +
> +static struct driver imx_tpm_pwm_driver = {
> +	.name = "imx7ulp-tpm-pwm",
> +	.probe	= pwm_imx_tpm_probe,
> +	.of_compatible = imx_tpm_pwm_dt_ids,
> +};
> +coredevice_platform_driver(imx_tpm_pwm_driver);
> +
> +MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>");
> +MODULE_DESCRIPTION("i.MX TPM PWM Driver");
> +MODULE_LICENSE("GPL v2");


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |