Nylon Chen <nylon.chen@xxxxxxxxxx> 於 2024年4月2日 週二 上午10:08寫道： > > 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); Hi Uwe, > > > /* The hardware cannot generate a 100% duty cycle */ Do you mean that if the inference is correct, this comment should be modified? > > > > 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/ |
