Message ID | 20200907213802.26745-1-michael@walle.cc |
---|---|
Headers | show |
Series | Add support for Kontron sl28cpld | expand |
On Mon, 07 Sep 2020, Michael Walle wrote: > The Kontron sl28cpld is a board management chip providing gpio, pwm, fan > monitoring and an interrupt controller. For now this controller is used on > the Kontron SMARC-sAL28 board. But because of its flexible nature, it > might also be used on other boards in the future. The individual blocks > (like gpio, pwm, etc) are kept intentionally small. The MFD core driver > then instantiates different (or multiple of the same) blocks. It also > provides the register layout so it might be updated in the future without a > device tree change; and support other boards with a different layout or > functionalities. > > See also [1] for more information. > > This is my first take of a MFD driver. I don't know whether the subsystem > maintainers should only be CCed on the patches which affect the subsystem > or on all patches for this series. I've chosen the latter so you can get a > more complete picture. > > [1] https://lore.kernel.org/linux-devicetree/0e3e8204ab992d75aa07fc36af7e4ab2@walle.cc/ I'll take all of the non-ARM patches later this week.
Hello, just a bit of nitpicking left. If Lee is going to apply, I can care for a followup patch if need be. On Mon, Sep 07, 2020 at 11:37:55PM +0200, Michael Walle wrote: > [..] > +config PWM_SL28CPLD > + tristate "Kontron sl28cpld PWM support" > + depends on MFD_SL28CPLD || COMPILE_TEST s/ / / (@Lee, maybe fixup during application?) > + help > + Generic PWM framework driver for board management controller > + found on the Kontron sl28 CPLD. > [...] > +#define SL28CPLD_PWM_CLK 32000 /* 32 kHz */ > +#define SL28CPLD_PWM_MAX_DUTY_CYCLE(prescaler) (1 << (7 - (prescaler))) > +#define SL28CPLD_PWM_PERIOD(prescaler) \ > + (NSEC_PER_SEC / SL28CPLD_PWM_CLK * SL28CPLD_PWM_MAX_DUTY_CYCLE(prescaler)) > + > +/* > + * We calculate the duty cycle like this: > + * duty_cycle_ns = pwm_cycle_reg * max_period_ns / max_duty_cycle > + * > + * With > + * max_period_ns = 1 << (7 - prescaler) / pwm_clk * NSEC_PER_SEC > + * max_duty_cycle = 1 << (7 - prescaler) > + * this then simplifies to: > + * duty_cycle_ns = pwm_cycle_reg / pwm_clk * NSEC_PER_SEC > + * > + * NSEC_PER_SEC is a multiple of SL28CPLD_PWM_CLK, therefore we're not losing > + * precision by doing the divison first. The division you're talking about is NSEC_PER_SEC / pwm_clk which isn't obvious in the formula in the line above. Maybe: ... this then simplifies to: duty_cycle_ns = NSEC_PER_SEC / SL28CPLD_PWM_CLK * pwm_cycle_reg NSEC_PER_SEC is a multiple of SL28CPLD_PWM_CLK, therefor ... to make it easier to understand the comment. > + */ > +#define SL28CPLD_PWM_TO_DUTY_CYCLE(reg) \ > + (NSEC_PER_SEC / SL28CPLD_PWM_CLK * (reg)) > +#define SL28CPLD_PWM_FROM_DUTY_CYCLE(duty_cycle) \ > + (DIV_ROUND_DOWN_ULL((duty_cycle), NSEC_PER_SEC / SL28CPLD_PWM_CLK)) > [...] > + /* > + * To avoid glitches when we switch the prescaler, we have to make sure > + * we have a valid duty cycle for the new mode. > + * > + * Take the current prescaler (or the current period length) into > + * account to decide whether we have to write the duty cycle or the new > + * prescaler first. If the period length is decreasing we have to > + * write the duty cycle first. > + */ > + write_duty_cycle_first = pwm->state.period > state->period; > + > + if (write_duty_cycle_first) { > + ret = sl28cpld_pwm_write(priv, SL28CPLD_PWM_CYCLE, cycle); > + if (ret) > + return ret; > + } > + > + ret = sl28cpld_pwm_write(priv, SL28CPLD_PWM_CTRL, ctrl); > + if (ret) > + return ret; > + > + if (!write_duty_cycle_first) { > + ret = sl28cpld_pwm_write(priv, SL28CPLD_PWM_CYCLE, cycle); > + if (ret) > + return ret; > + } Nice! I didn't spend the necessary brain cycles to confirm this algorithm, but it seems you did :-) > + > + return 0; > +} > [...] Best regards Uwe
On Mon 2020-09-07 23:38:00, Michael Walle wrote: > Now that we have support for GPIO lines of the SMARC connector, enable > LED support on the KBox A-230-LS. There are two LEDs without fixed > functions, one is yellow and one is green. Unfortunately, it is just one > multi-color LED, thus while it is possible to enable both at the same > time it is hard to tell the difference between "yellow only" and "yellow > and green". > > Signed-off-by: Michael Walle <michael@walle.cc> Acked-by: Pavel Machek <pavel@ucw.cz>
On Mon 2020-09-07 23:38:00, Michael Walle wrote: > Now that we have support for GPIO lines of the SMARC connector, enable > LED support on the KBox A-230-LS. There are two LEDs without fixed > functions, one is yellow and one is green. Unfortunately, it is just one > multi-color LED, thus while it is possible to enable both at the same > time it is hard to tell the difference between "yellow only" and "yellow > and green". > > Signed-off-by: Michael Walle <michael@walle.cc> Acked-by: Pavel Machek <pavel@ucw.cz>