Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> 於 2024年3月19日 週二 上午2:16寫道： > > 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. I believe the statement is true, but I don't know which part the driver header file refers to. > > > 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? got it > > > mutex_lock(&ddata->lock); > > if (state->period != ddata->approx_period) { > Thank you for taking the time to help me review my implementation. Nylon > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ |
