1. Patchew
  2. linux
  3. [PATCH v2 0/7] Tegra264 PWM support
  4. View patch

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

Mikko Perttunen posted 7 patches 1 week, 1 day ago
There is a newer version of this series
[PATCH v2 4/7] pwm: tegra: Parametrize enable register offset
Posted by Mikko Perttunen 1 week, 1 day ago 
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@nvidia.com>
Signed-off-by: Yi-Wei Wang <yiweiw@nvidia.com>
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 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;
+	}
 
 	pwm_writel(pwm, PWM_CSR_0, val);
 
@@ -213,6 +215,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 static int tegra_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
+	struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
 	int rc = 0;
 	u32 val;
 
@@ -220,20 +223,22 @@ static int tegra_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	if (rc)
 		return rc;
 
-	val = pwm_readl(pwm, PWM_CSR_0);
+
+	val = pwm_readl(pwm, pc->soc->enable_reg);
 	val |= PWM_ENABLE;
-	pwm_writel(pwm, PWM_CSR_0, val);
+	pwm_writel(pwm, pc->soc->enable_reg, val);
 
 	return 0;
 }
 
 static void tegra_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
+	struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
 	u32 val;
 
-	val = pwm_readl(pwm, PWM_CSR_0);
+	val = pwm_readl(pwm, pc->soc->enable_reg);
 	val &= ~PWM_ENABLE;
-	pwm_writel(pwm, PWM_CSR_0, val);
+	pwm_writel(pwm, pc->soc->enable_reg, val);
 
 	pm_runtime_put_sync(pwmchip_parent(chip));
 }
@@ -398,10 +403,12 @@ static int __maybe_unused tegra_pwm_runtime_resume(struct device *dev)
 
 static const struct tegra_pwm_soc tegra20_pwm_soc = {
 	.num_channels = 4,
+	.enable_reg = PWM_CSR_0,
 };
 
 static const struct tegra_pwm_soc tegra186_pwm_soc = {
 	.num_channels = 1,
+	.enable_reg = PWM_CSR_0,
 };
 
 static const struct of_device_id tegra_pwm_of_match[] = {

-- 
2.53.0
Re: [PATCH v2 4/7] pwm: tegra: Parametrize enable register offset
Posted by Thierry Reding 1 week ago 
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@nvidia.com>
> Signed-off-by: Yi-Wei Wang <yiweiw@nvidia.com>
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  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?

Thierry
Re: [PATCH v2 4/7] pwm: tegra: Parametrize enable register offset
Posted by Mikko Perttunen 3 days, 23 hours ago 
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@nvidia.com>
> > Signed-off-by: Yi-Wei Wang <yiweiw@nvidia.com>
> > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> > ---
> >  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
Re: [PATCH v2 4/7] pwm: tegra: Parametrize enable register offset
Posted by Thierry Reding 2 days, 17 hours ago 
On Mon, Mar 30, 2026 at 11:24:09AM +0900, Mikko Perttunen wrote:
> 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@nvidia.com>
> > > Signed-off-by: Yi-Wei Wang <yiweiw@nvidia.com>
> > > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> > > ---
> > >  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.

Ah... nevermind. I realize now that we're not touching PWM_CSR_1 at all
in tegra_pwm_config(), so there's no need to explicitly set PWM_ENABLE.

If moving to the new APIs, that would need to change, but for the legacy
PWM callbacks this is probably fine.

Thierry

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.