On Wed, Feb 13, 2019 at 11:34:59AM +0100, Uwe Kleine-König wrote:
> On Wed, Feb 13, 2019 at 02:56:18PM +0530, Yash Shah wrote:
[...]
> > + unsigned long scale_pow =
> > + div64_ul(pwm->real_period * (u64)rate, NSEC_PER_SEC);
> > + int scale = clamp(ilog2(scale_pow) - PWM_SIFIVE_CMPWIDTH, 0, 0xf);
> > +
> > + writel((1 << PWM_SIFIVE_PWMCFG_EN_ALWAYS) | (scale <<
> > + PWM_SIFIVE_PWMCFG_SCALE), pwm->regs + PWM_SIFIVE_PWMCFG);
>
> I think this would be better readable with the newline after the |. With
> my editor's configuration when broken like this, the 2nd line would be
> intended with the opening ( after the |.
>
> > +
> > + /* As scale <= 15 the shift operation cannot overflow. */
> > + pwm->real_period = div64_ul(1000000000ULL << (PWM_SIFIVE_CMPWIDTH +
> > + scale), rate);
>
> ditto. Maybe break after the =?
>
> > + dev_dbg(pwm->chip.dev, "New real_period = %u ns\n", pwm->real_period);
> > +}
> > +
> > +static void pwm_sifive_get_state(struct pwm_chip *chip, struct pwm_device *dev,
> > + struct pwm_state *state)
> > +{
> > + struct pwm_sifive_ddata *pwm = pwm_sifive_chip_to_ddata(chip);
> > + u32 duty;
> > + int val;
> > +
> > + duty = readl(pwm->regs + PWM_SIFIVE_PWMCMP0 + dev->hwpwm *
> > + PWM_SIFIVE_SIZE_PWMCMP);
> > +
> > + val = readl(pwm->regs + PWM_SIFIVE_PWMCFG);
> > + state->enabled = (val & BIT(PWM_SIFIVE_PWMCFG_EN_ALWAYS)) > 0;
> > +
> > + val &= 0x0F;
> > + pwm->real_period = div64_ul(1000000000ULL << (PWM_SIFIVE_CMPWIDTH +
> > + val), clk_get_rate(pwm->clk));
>
> Another bad line break.
Maybe just split all of these very long expressions up by introducing
temporary variables to make things more palatable?
Thierry
Attachment:
signature.asc
Description: PGP signature
