On Thu, Nov 23, 2023 at 11:10:18AM +0100, Uwe Kleine-König wrote:
> Hello Laurent,
>
> On Thu, Nov 23, 2023 at 11:46:52AM +0200, Laurent Pinchart wrote:
> > (CC'ing Bartosz)
>
> I'm already in discussion with Bart :-)
>
> > 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@xxxxxxxxxxxxxx>
> > > ---
> > > 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).
>
> Once the series is completely applied, the pwm_chip isn't allocated
> using devm_kzalloc any more. You're only looking at an intermediate
> state where I push the broken lifetime tracking from all drivers into a
> single function in the core that is then fixed after all drivers are
> converted to it.
Indeed, I missed that devm_pwm_alloc() got changed later in the series
to not call devm_kzalloc(). The naming is quite unfortunate, a
devm_*_alloc() function really gives a strong hint that the
corresponding cleanup at device remove time will be a free(), not a
put() :-S That's pretty confusing for the readers.
> If you find issues with the complete series applied, please tell me.
One thing I still dislike is forcing drivers to dynamically allocate the
pwm_chip series.
--
Regards,
Laurent Pinchart
