On Thu, Feb 22, 2024 at 04:12:30PM +0800, Nylon Chen wrote: > The `frac` variable represents the pulse inactive time, and the result > of this algorithm is the pulse active time. > Therefore, we must reverse the result. > > The reference is SiFive FU740-C000 Manual[0] > > Link: https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf [0] > > Co-developed-by: Zong Li <zong.li@xxxxxxxxxx> > Signed-off-by: Zong Li <zong.li@xxxxxxxxxx> > Co-developed-by: Vincent Chen <vincent.chen@xxxxxxxxxx> > Signed-off-by: Vincent Chen <vincent.chen@xxxxxxxxxx> > Signed-off-by: Nylon Chen <nylon.chen@xxxxxxxxxx> > --- > drivers/pwm/pwm-sifive.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c > index eabddb7c7820..a586cfe4191b 100644 > --- a/drivers/pwm/pwm-sifive.c > +++ b/drivers/pwm/pwm-sifive.c > @@ -110,9 +110,10 @@ static int pwm_sifive_get_state(struct pwm_chip *chip, struct pwm_device *pwm, > struct pwm_state *state) > { > struct pwm_sifive_ddata *ddata = pwm_sifive_chip_to_ddata(chip); > - u32 duty, val; > + u32 duty, val, inactive; > > - duty = readl(ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm)); > + inactive = readl(ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm)); > + duty = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - inactive; > > state->enabled = duty > 0; > > @@ -123,7 +124,7 @@ static int pwm_sifive_get_state(struct pwm_chip *chip, struct pwm_device *pwm, > state->period = ddata->real_period; > state->duty_cycle = > (u64)duty * ddata->real_period >> PWM_SIFIVE_CMPWIDTH; > - state->polarity = PWM_POLARITY_INVERSED; > + state->polarity = PWM_POLARITY_NORMAL; > > return 0; > } > @@ -139,7 +140,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, > int ret = 0; > u32 frac; > > - if (state->polarity != PWM_POLARITY_INVERSED) > + if (state->polarity != PWM_POLARITY_NORMAL) > return -EINVAL; > > cur_state = pwm->state; > @@ -159,6 +160,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, > frac = DIV64_U64_ROUND_CLOSEST(num, state->period); > /* The hardware cannot generate a 100% duty cycle */ Is this still true now that we know that PWM_SIFIVE_PWMCMP is the inactive time in a period? If you fix that, the same claim in the header of the driver needs adaption, too. > frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1); > + frac = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac; I like the additional variable in pwm_sifive_get_state(). Can you please add one here, too? > mutex_lock(&ddata->lock); > if (state->period != ddata->approx_period) { Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
