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

[Uwe Kleine-König]

Hello Doug,

On Thu, Jan 25, 2024 at 09:48:04AM -0800, Doug Anderson wrote:
> On Thu, Jan 25, 2024 at 4:11 AM Uwe Kleine-König
> <u.kleine-koenig at pengutronix.de> wrote:
> > @@ -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?

The upside of keeping the thin wrapper is that it returns the right
type, so I tend to keep it. Probably subjective, but even if it the
function should be dropped, I'd suggest to do that in a separate change
to keep the changes easier to review.

> 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 at chromium.org>

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20240125/be37ba20/attachment.sig>