[PATCH v4 089/115] pwm: sifive: Make use of devm_pwmchip_alloc() function

[Emil Renner Berthing]

Uwe Kleine-König wrote:
> Hello Emil,
>
> On Fri, Dec 08, 2023 at 04:30:41AM -0500, Emil Renner Berthing wrote:
> > Uwe Kleine-König wrote:
> > > This prepares the pwm-sifive driver 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/pwm/pwm-sifive.c | 28 ++++++++++++++--------------
> > >  1 file changed, 14 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
> > > index c1b412905d9e..cf3fcffe7b3a 100644
> > > --- a/drivers/pwm/pwm-sifive.c
> > > +++ b/drivers/pwm/pwm-sifive.c
> > > @@ -41,7 +41,7 @@
> > >  #define PWM_SIFIVE_DEFAULT_PERIOD	10000000
> > >
> > >  struct pwm_sifive_ddata {
> > > -	struct pwm_chip	chip;
> > > +	struct pwm_chip *chip;
> >
> > In patch 43 you do ddata = (void *)chip + sizeof(*chip). Shouldn't we
> > be able to get
> > the chip pointer from ddata using chip = (void *)ddata - sizeof(*chip)?
>
> That would work, but I don't want to use that implementation detail in
> lowlevel drivers. Also it's a bit obscure because not all drivers use
> the driver data located after the pwmchip. Another difficulty is that
> starting with patch #111 the memory layout changes and you can only
> determine the chip from the driver data if you know the value of npwm.
> (So you need information from the chip to get access to the chip. huch)

Ah, yeah it would of course need wrappers so drivers won't need to do the
calculation, but the last past definitely makes sense.

>
> > >  	struct mutex lock; /* lock to protect user_count and approx_period */
> > >  	struct notifier_block notifier;
> > >  	struct clk *clk;
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | https://www.pengutronix.de/ |