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

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

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.

Thierry