Re: [PATCH v5 104/111] drm/bridge: ti-sn65dsi86: Make use of devm_pwmchip_alloc() function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Thu, Jan 25, 2024 at 4:11 AM Uwe Kleine-König
<u.kleine-koenig@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index f1fffbef3324..7fbc307cc025 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;
>         bool                            pwm_enabled;
>         atomic_t                        pwm_pin_busy;
>  #endif
> @@ -1374,7 +1374,7 @@ 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);
> +       return pwmchip_get_drvdata(chip);
>  }

nit: given Linux conventions that I'm aware of, a reader of the code
would see the name "pwm_chip_to_ti_sn_bridge" and assume it's doing a
container_of operation. It no longer is, so the name doesn't make as
much sense. ...and, in fact, the function itself doesn't make as much
sense. Maybe just have all callers call pwmchip_get_drvdata()
directly?

In any case, this seems fine to me. I haven't done lots to analyze
your full plans to fix lifetime issues, but this patch itself looks
benign and I wouldn't object to it landing. Thus I'm OK with:

Acked-by: Douglas Anderson <dianders@xxxxxxxxxxxx>

Similar to the other ti-sn65dsi86 patch in this series, unless someone
more senior in the drm-misc community contradicts me I think it's safe
to assume you could land this through your tree.


-Doug




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux