On Tue, Jun 16, 2026 at 07:13:17PM +0800, Keguang Zhang via B4 Relay wrote: > From: Keguang Zhang > > The Loongson PWM register at offset 0x4 is documented as the Low > Pulse Buffer Register, which stores the low pulse width rather than > the duty cycle. > > However, this register was incorrectly defined and treated as a > duty-cycle register. As a result, the duty cycle and low pulse cycle > are swapped in the generated PWM waveform. > > Program the low pulse (period - duty) into the register and > adjust pwm_loongson_get_state() accordingly when reconstructing the > duty cycle. > > Also return -ERANGE when the requested period exceeds the hardware > 32-bit limit instead of silently truncating the value. This is the intended behaviour. > Signed-off-by: Keguang Zhang > --- > drivers/pwm/pwm-loongson.c | 29 ++++++++++++++--------------- > 1 file changed, 14 insertions(+), 15 deletions(-) > > diff --git a/drivers/pwm/pwm-loongson.c b/drivers/pwm/pwm-loongson.c > index 31a57edecfd0..dc77f82fd888 100644 > --- a/drivers/pwm/pwm-loongson.c > +++ b/drivers/pwm/pwm-loongson.c > @@ -22,6 +22,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -33,10 +34,12 @@ > #include > > /* Loongson PWM registers */ > -#define LOONGSON_PWM_REG_DUTY 0x4 /* Low Pulse Buffer Register */ > +#define LOONGSON_PWM_REG_LOW 0x4 /* Low Pulse Buffer Register */ > #define LOONGSON_PWM_REG_PERIOD 0x8 /* Pulse Period Buffer Register */ > #define LOONGSON_PWM_REG_CTRL 0xc /* Control Register */ > > +#define LOONGSON_PWM_MAX_PERIOD GENMASK(31, 0) > + > /* Control register bits */ > #define LOONGSON_PWM_CTRL_REG_EN BIT(0) /* Counter Enable Bit */ > #define LOONGSON_PWM_CTRL_REG_OE BIT(3) /* Pulse Output Enable Control Bit, Valid Low */ > @@ -118,20 +121,16 @@ static int pwm_loongson_enable(struct pwm_chip *chip, struct pwm_device *pwm) > static int pwm_loongson_config(struct pwm_chip *chip, struct pwm_device *pwm, > u64 duty_ns, u64 period_ns) > { > - u64 duty, period; > + u64 low, period; > struct pwm_loongson_ddata *ddata = to_pwm_loongson_ddata(chip); > > - /* duty = duty_ns * ddata->clk_rate / NSEC_PER_SEC */ > - duty = mul_u64_u64_div_u64(duty_ns, ddata->clk_rate, NSEC_PER_SEC); > - if (duty > U32_MAX) > - duty = U32_MAX; > - > - /* period = period_ns * ddata->clk_rate / NSEC_PER_SEC */ > period = mul_u64_u64_div_u64(period_ns, ddata->clk_rate, NSEC_PER_SEC); > - if (period > U32_MAX) > - period = U32_MAX; > + if ((!FIELD_FIT(LOONGSON_PWM_MAX_PERIOD, period))) > + return -ERANGE; As noted above, this is wrong. If period is too big, you're supposed to pick the biggest possible period and not return an error. > + > + low = mul_u64_u64_div_u64(period_ns - duty_ns, ddata->clk_rate, NSEC_PER_SEC); this is also wrong. You're supposed to pick a configuration where the duty is the biggest not bigger than the requested value. However as mul_u64_u64_div_u64 rounds down, you're rounding in the wrong direction. The register layout suggests that the period starts with the inactive part, did you reverify that? Best regards Uwe