[PATCH v3 101/108] drm/bridge: ti-sn65dsi86: Make use of devm_pwmchip_alloc() function

[Laurent Pinchart]

Hi Uwe,

(CC'ing Bartosz)

Thank you for the patch.

On Tue, Nov 21, 2023 at 02:50:43PM +0100, Uwe Kleine-König wrote:
> This prepares the pwm driver of the ti-sn65dsi86 to further changes of
> the pwm core outlined in the commit introducing devm_pwmchip_alloc().
> There is no intended semantical change and the driver should behave as
> before.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig at pengutronix.de>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index c45c07840f64..cd40530ffd71 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -197,7 +197,7 @@ struct ti_sn65dsi86 {
>  	DECLARE_BITMAP(gchip_output, SN_NUM_GPIOS);
>  #endif
>  #if defined(CONFIG_PWM)
> -	struct pwm_chip			pchip;
> +	struct pwm_chip			*pchip;

Dynamic allocation with devm_*() isn't the right solution for lifetime
issues related to cdev. See my talk at LPC 2022
(https://www.youtube.com/watch?v=kW8LHWlJPTU, slides at
https://lpc.events/event/16/contributions/1227/attachments/1103/2115/20220914-lpc-devm_kzalloc.pdf),
and Bartosz's talk at LPC 2023
(https://lpc.events/event/17/contributions/1627/attachments/1258/2725/Linux%20Plumbers%20Conference%202023.pdf).

>  	bool				pwm_enabled;
>  	atomic_t			pwm_pin_busy;
>  #endif
> @@ -1372,7 +1372,8 @@ static void ti_sn_pwm_pin_release(struct ti_sn65dsi86 *pdata)
>  
>  static struct ti_sn65dsi86 *pwm_chip_to_ti_sn_bridge(struct pwm_chip *chip)
>  {
> -	return container_of(chip, struct ti_sn65dsi86, pchip);
> +	struct ti_sn65dsi86 **pdata = pwmchip_priv(chip);
> +	return *pdata;
>  }
>  
>  static int ti_sn_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> @@ -1585,22 +1586,28 @@ static const struct pwm_ops ti_sn_pwm_ops = {
>  static int ti_sn_pwm_probe(struct auxiliary_device *adev,
>  			   const struct auxiliary_device_id *id)
>  {
> +	struct pwm_chip *chip;
>  	struct ti_sn65dsi86 *pdata = dev_get_drvdata(adev->dev.parent);
>  
> -	pdata->pchip.dev = pdata->dev;
> -	pdata->pchip.ops = &ti_sn_pwm_ops;
> -	pdata->pchip.npwm = 1;
> -	pdata->pchip.of_xlate = of_pwm_single_xlate;
> -	pdata->pchip.of_pwm_n_cells = 1;
> +	/* XXX: should this better use adev->dev instead of pdata->dev? */
> +	pdata->pchip = chip = devm_pwmchip_alloc(pdata->dev, 1, sizeof(&pdata));
> +	if (IS_ERR(chip))
> +		return PTR_ERR(chip);
>  
> -	return pwmchip_add(&pdata->pchip);
> +	*(struct ti_sn65dsi86 **)pwmchip_priv(chip) = pdata;
> +
> +	chip->ops = &ti_sn_pwm_ops;
> +	chip->of_xlate = of_pwm_single_xlate;
> +	chip->of_pwm_n_cells = 1;
> +
> +	return pwmchip_add(chip);
>  }
>  
>  static void ti_sn_pwm_remove(struct auxiliary_device *adev)
>  {
>  	struct ti_sn65dsi86 *pdata = dev_get_drvdata(adev->dev.parent);
>  
> -	pwmchip_remove(&pdata->pchip);
> +	pwmchip_remove(pdata->pchip);
>  
>  	if (pdata->pwm_enabled)
>  		pm_runtime_put_sync(pdata->dev);

-- 
Regards,

Laurent Pinchart