Hello Vincent,

On 03.05.21 09:26, Vincent Chen wrote:

The pwm_sifive_set_config() and pwm_sifive_set_enable() cannot work properly due to the wrong implementations. It will cause the u-boot PWM command to not work as expected. The bugs will be resolved in this patch.

Signed-off-by: Vincent Chen vincent.chen@sifive.com

drivers/pwm/pwm-sifive.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c index 01212d6..b9813a3 100644 --- a/drivers/pwm/pwm-sifive.c +++ b/drivers/pwm/pwm-sifive.c @@ -38,6 +38,9 @@ #define PWM_SIFIVE_SIZE_PWMCMP 4 #define PWM_SIFIVE_CMPWIDTH 16

+#define PWM_SIFIVE_CHANNEL_ENABLE_VAL 0 +#define PWM_SIFIVE_CHANNEL_DISABLE_VAL 0xffff

DECLARE_GLOBAL_DATA_PTR;

struct pwm_sifive_regs { @@ -77,7 +80,7 @@ static int pwm_sifive_set_config(struct udevice *dev, uint channel, */ scale_pow = lldiv((uint64_t)priv->freq * period_ns, 1000000000); scale = clamp(ilog2(scale_pow) - PWM_SIFIVE_CMPWIDTH, 0, 0xf);

• val |= FIELD_PREP(PWM_SIFIVE_PWMCFG_SCALE, scale);

• val |= (FIELD_PREP(PWM_SIFIVE_PWMCFG_SCALE, scale) | PWM_SIFIVE_PWMCFG_EN_ALWAYS);

Ok, for this as it seems the same as in linux:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...

/* * The problem of output producing mixed setting as mentioned at top, @@ -88,6 +91,7 @@ static int pwm_sifive_set_config(struct udevice *dev, uint channel, num = (u64)duty_ns * (1U << PWM_SIFIVE_CMPWIDTH); frac = DIV_ROUND_CLOSEST_ULL(num, period_ns); frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1);

• frac = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac;

I just looked into linux code, and current code is the same as in linux, see:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...

May you can describe what problem you exactly fix? May this part is not needed?

I wonder, why this is not also a problem than in linux?

Thanks!

bye, Heiko

writel(val, priv->base + regs->cfg); writel(frac, priv->base + regs->cmp0 + channel * @@ -100,18 +104,15 @@ static int pwm_sifive_set_enable(struct udevice *dev, uint channel, bool enable) { struct pwm_sifive_priv *priv = dev_get_priv(dev); const struct pwm_sifive_regs *regs = &priv->data->regs;

• u32 val;

  debug("%s: Enable '%s'\n", __func__, dev->name);

• if (enable) {

• val = readl(priv->base + regs->cfg);
  

• val |= PWM_SIFIVE_PWMCFG_EN_ALWAYS;
  

• writel(val, priv->base + regs->cfg);
  

• } else {

• writel(0, priv->base + regs->cmp0 + channel *
  

•        PWM_SIFIVE_SIZE_PWMCMP);
  

• }

• if (enable)

• writel(PWM_SIFIVE_CHANNEL_ENABLE_VAL, priv->base +
  

•        regs->cmp0 + channel * PWM_SIFIVE_SIZE_PWMCMP);
  

• else

• writel(PWM_SIFIVE_CHANNEL_DISABLE_VAL, priv->base +
  

•        regs->cmp0 + channel * PWM_SIFIVE_SIZE_PWMCMP);
  
  

  return 0;

}