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

On Mon, Mar 30, 2026 at 11:24:09AM +0900, Mikko Perttunen wrote:

On 2026-03-26 10:47 +0100, Thierry Reding wrote:

quoted

On Wed, Mar 25, 2026 at 07:17:02PM +0900, Mikko Perttunen wrote:

quoted

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 <redacted>
Signed-off-by: Yi-Wei Wang <redacted>
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

quoted

 
 	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