Re: [PATCH v5 6/7] pwm: tegra: Add support for Tegra264

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver assumes a fixed PWM depth of 256 but fails to initialize the new configurable DEPTH register field on Tegra264, risking incorrect PWM frequency and duty cycle.
--

commit a215267521de0fbfc0019ea87d4d0d3a5d3250ea
Author: Mikko Perttunen <mperttunen@nvidia.com>

pwm: tegra: Add support for Tegra264

This patch adds support for the Tegra264 PWM controller. It introduces
an enable_reg field to handle the relocated PWM_ENABLE bit in the new
CSR_1 register for Tegra264 devices.

> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index 7c7b884d44363..50e72139cbc34 100644
> --- a/drivers/pwm/pwm-tegra.c
> +++ b/drivers/pwm/pwm-tegra.c
[ ... ]
> @@ -215,6 +257,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;
>  
> @@ -222,20 +265,21 @@ static int tegra_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>  	if (rc)
>  		return rc;
>  
> -	val = tegra_pwm_readl(pwm, TEGRA_PWM_CSR_0);
> +	val = tegra_pwm_readl(pwm, pc->soc->enable_reg);
>  	val |= TEGRA_PWM_ENABLE;
> -	tegra_pwm_writel(pwm, TEGRA_PWM_CSR_0, val);
> +	tegra_pwm_writel(pwm, pc->soc->enable_reg, val);

[Severity: High]
Could this cause a regression where the PWM output is left in a broken state
if the hardware resets the DEPTH field to 0?

The commit message notes that configurable depth will be supported in a
later patch, and tegra_pwm_config() continues to use a hardcoded
TEGRA_PWM_DEPTH of 256 for duty cycle and clock rate calculations.

Since this read-modify-write only updates the ENB bit, if the CSR_1.DEPTH
field defaults to 0 after a device reset, wouldn't the hardware divide by 1
instead of 256?

This seems like it would cause the PWM to operate at 256x the intended
frequency, and any calculated duty cycle value would exceed the hardware's
maximum depth. Should CSR_1.DEPTH be explicitly initialized to 255 here to
safely match the software's TEGRA_PWM_DEPTH assumption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529-t264-pwm-v5-0-7bf9e405a96a@nvidia.com?part=6