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;
}