| Message ID | 33b13f17d7135923d08e2ff40c867803e19609b9.1784030076.git.ukleinek@kernel.org |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | pwm: tegra: Cleanups and .get_state() | expand |
On Tuesday, July 14, 2026 9:02 PM Uwe Kleine-König wrote: > The registers of the PWM IP are readable. Use that to implement the > .get_state() callback. I swear I was going to implement this after the Tegra264 series was accepted, but I don't mind this way either.. :) > > Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org> > --- > drivers/pwm/pwm-tegra.c | 48 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c > index 8e5e7e37f4ff..79bfc7589db8 100644 > --- a/drivers/pwm/pwm-tegra.c > +++ b/drivers/pwm/pwm-tegra.c > @@ -309,8 +309,56 @@ static int tegra_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, > return err; > } > > +static int tegra_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,(((1 << pc->soc->scale_width) - 1)) > + struct pwm_state *state) > +{ > + struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip); > + int rc; > + u32 val; > + > + rc = pm_runtime_resume_and_get(pwmchip_parent(chip)); > + if (rc) > + return rc; > + > + val = tegra_pwm_readl(pwm, pc->soc->enable_reg); > + if (val & TEGRA_PWM_ENABLE) { > + u32 scale, pwm0; > + > + if (pc->soc->enable_reg != TEGRA_PWM_CSR_0) > + val = tegra_pwm_readl(pwm, TEGRA_PWM_CSR_0); > + > + scale = (val >> TEGRA_PWM_SCALE_SHIFT) & (((1 << pc->soc->scale_width) - 1)); There's one unnecessary pair of parentheses. > + pwm0 = (val >> TEGRA_PWM_DUTY_SHIFT) & (2 * TEGRA_PWM_DEPTH - 1); > + > + if (pwm0 > TEGRA_PWM_DEPTH) > + pwm0 = TEGRA_PWM_DEPTH; > + > + /* > + * scale + 1 is at most 1 << 17, TEGRA_PWM_DEPTH is 256, so the > + * multiplication for .period doesn't overflow a u64. With > + * pwm0 ≤ TEGRA_PWM_DEPTH, .duty_cycle is also fine. > + */ > + *state = (struct pwm_state){ > + .period = DIV64_U64_ROUND_UP((u64)(scale + 1) * TEGRA_PWM_DEPTH * NSEC_PER_SEC, pc->clk_rate), > + .duty_cycle = DIV64_U64_ROUND_UP((u64)(scale + 1) * pwm0 * NSEC_PER_SEC, pc->clk_rate), > + .polarity = PWM_POLARITY_NORMAL, > + .enabled = true, > + }; > + > + } else { > + *state = (struct pwm_state){ > + .enabled = false, > + }; > + } > + > + pm_runtime_put(pwmchip_parent(chip)); > + > + return 0; > +} > + > static const struct pwm_ops tegra_pwm_ops = { > .apply = tegra_pwm_apply, > + .get_state = tegra_pwm_get_state, > }; > > static int tegra_pwm_probe(struct platform_device *pdev) > -- > 2.55.0.11.g153666a7d9bb > > Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
Hello Uwe, On Tue, Jul 14, 2026 at 02:02:40PM +0200, Uwe Kleine-König wrote: > Subject: [PATCH v1 6/6] pwm: tegra: Implement .get_state() I tested this on a Jetson Orin NX (Tegra234), the board from my divider truncation report, driving a fan on the 32a0000 PWM instance. The board runs NVIDIA's L4T 5.15 kernel, so I backported the patch onto that tree: .get_state() returns void there, pwmchip_parent(chip) becomes the driver's device pointer, and the per-SoC enable_reg / scale_width indirection collapses to the fixed CSR layout (that tree predates the Tegra264 restructure). The decode logic is unchanged from your patch. The tree also carries my divider change from the other thread, which is why the achieved periods in the table differ from what your base would program. Procedure: apply a state through the pwm sysfs interface, read the CSR register and the clock rate independently (/dev/mem and clk_summary), then unexport/re-export the channel so pwm_device_request() invokes .get_state(), and compare the reported state against values computed from the raw register: applied period/duty/enabled readback computed from CSR+clk 45334/20000/1 46432/20496/1 46432/20496/1 500000/250000/1 481883/240942/1 481883/240942/1 5000/2500/1 5020/2510/1 5020/2510/1 45334/45334/1 (100% duty) 46432/46432/1 46432/46432/1 45334/20000/0 enabled=0 enabled=0 The clock rate varied across the cases (3.19, 11.03 and 102 MHz), so the scale field was exercised at several values, and the 100% duty case reads the full 9-bit duty field (pwm0 = 256). The readback consistently reports the achieved hardware state rather than the requested one. One limitation of my backport, not your code: in the disabled case I only set state->enabled = false instead of zeroing the whole struct, so I did not verify the period/duty values your version reports for a disabled channel. Tested-by: Ola Chr. Vaage <ola.christoffer.vage@scoutdi.com> Best regards Ola
diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c index 8e5e7e37f4ff..79bfc7589db8 100644 --- a/drivers/pwm/pwm-tegra.c +++ b/drivers/pwm/pwm-tegra.c @@ -309,8 +309,56 @@ static int tegra_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, return err; } +static int tegra_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, + struct pwm_state *state) +{ + struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip); + int rc; + u32 val; + + rc = pm_runtime_resume_and_get(pwmchip_parent(chip)); + if (rc) + return rc; + + val = tegra_pwm_readl(pwm, pc->soc->enable_reg); + if (val & TEGRA_PWM_ENABLE) { + u32 scale, pwm0; + + if (pc->soc->enable_reg != TEGRA_PWM_CSR_0) + val = tegra_pwm_readl(pwm, TEGRA_PWM_CSR_0); + + scale = (val >> TEGRA_PWM_SCALE_SHIFT) & (((1 << pc->soc->scale_width) - 1)); + pwm0 = (val >> TEGRA_PWM_DUTY_SHIFT) & (2 * TEGRA_PWM_DEPTH - 1); + + if (pwm0 > TEGRA_PWM_DEPTH) + pwm0 = TEGRA_PWM_DEPTH; + + /* + * scale + 1 is at most 1 << 17, TEGRA_PWM_DEPTH is 256, so the + * multiplication for .period doesn't overflow a u64. With + * pwm0 ≤ TEGRA_PWM_DEPTH, .duty_cycle is also fine. + */ + *state = (struct pwm_state){ + .period = DIV64_U64_ROUND_UP((u64)(scale + 1) * TEGRA_PWM_DEPTH * NSEC_PER_SEC, pc->clk_rate), + .duty_cycle = DIV64_U64_ROUND_UP((u64)(scale + 1) * pwm0 * NSEC_PER_SEC, pc->clk_rate), + .polarity = PWM_POLARITY_NORMAL, + .enabled = true, + }; + + } else { + *state = (struct pwm_state){ + .enabled = false, + }; + } + + pm_runtime_put(pwmchip_parent(chip)); + + return 0; +} + static const struct pwm_ops tegra_pwm_ops = { .apply = tegra_pwm_apply, + .get_state = tegra_pwm_get_state, }; static int tegra_pwm_probe(struct platform_device *pdev)
The registers of the PWM IP are readable. Use that to implement the .get_state() callback. Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org> --- drivers/pwm/pwm-tegra.c | 48 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)