Re: [PATCH v2 4/7] pwm: tegra: Parametrize enable register offset

[Mikko Perttunen]

On 2026-03-26 10:47 +0100, Thierry Reding wrote:
> On Wed, Mar 25, 2026 at 07:17:02PM +0900, Mikko Perttunen wrote:
> > On Tegra264, the PWM enablement bit is not located at the base address
> > of the PWM controller. Hence, introduce an enablement offset field in
> > the tegra_pwm_soc structure to describe the offset of the register.
> > 
> > Co-developed-by: Yi-Wei Wang <yiweiw@xxxxxxxxxx>
> > Signed-off-by: Yi-Wei Wang <yiweiw@xxxxxxxxxx>
> > Signed-off-by: Mikko Perttunen <mperttunen@xxxxxxxxxx>
> > ---
> >  drivers/pwm/pwm-tegra.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> > index cf54f75d92a5..22d709986e8c 100644
> > --- a/drivers/pwm/pwm-tegra.c
> > +++ b/drivers/pwm/pwm-tegra.c
> > @@ -61,6 +61,7 @@
> >  
> >  struct tegra_pwm_soc {
> >  	unsigned int num_channels;
> > +	unsigned int enable_reg;
> >  };
> >  
> >  struct tegra_pwm_chip {
> > @@ -197,8 +198,9 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> >  		err = pm_runtime_resume_and_get(pwmchip_parent(chip));
> >  		if (err)
> >  			return err;
> > -	} else
> > +	} else if (pc->soc->enable_reg == PWM_CSR_0) {
> >  		val |= PWM_ENABLE;
> > +	}
> 
> This looks incomplete for the Tegra264 case where
> 
> 	pc->soc->enable_reg == PWM_CSR_1
> 
> >  
> >  	pwm_writel(pwm, PWM_CSR_0, val);
> 
> I think we need another write for PWM_CSR_1 here to properly toggle the
> PWM_ENABLE bit on Tegra264.
> 
> Or am I missing something?

This check is here just so we don't change the value of PWM_ENABLE when
writing the CSR_0 register. The function doesn't write to CSR_1 so
nothing needs to be done on Tegra264.

I agree it's not the clearest, but it'll get cleaned up when adding
support for configurable depth, as at that point we will need to write
both registers on Tegra264.

> 
> Thierry