On Mon, Jan 17, 2022 at 11:04:31PM +0500, Nikita Travkin wrote: > Hi, > > Uwe Kleine-König писал(а) 17.01.2022 20:58: > > Hello, > > > > On Mon, Dec 13, 2021 at 08:03:35PM +0500, Nikita Travkin wrote: > >> Some systems have clocks exposed to external devices. If the clock > >> controller supports duty-cycle configuration, such clocks can be used as > >> pwm outputs. In fact PWM and CLK subsystems are interfaced with in a > >> similar way and an "opposite" driver already exists (clk-pwm). Add a > >> driver that would enable pwm devices to be used via clk subsystem. > >> > >> Signed-off-by: Nikita Travkin <nikita@xxxxxxx> > >> -- > >> > >> Changes in v2: > >> - Address Uwe's review comments: > >> - Round set clk rate up > >> - Add a description with limitations of the driver > >> - Disable and unprepare clock before removing pwmchip > >> --- > >> drivers/pwm/Kconfig | 10 +++ > >> drivers/pwm/Makefile | 1 + > >> drivers/pwm/pwm-clk.c | 143 ++++++++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 154 insertions(+) > >> create mode 100644 drivers/pwm/pwm-clk.c > >> > >> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > >> index 21e3b05a5153..daa2491a4054 100644 > >> --- a/drivers/pwm/Kconfig > >> +++ b/drivers/pwm/Kconfig > >> @@ -140,6 +140,16 @@ config PWM_BRCMSTB > >> To compile this driver as a module, choose M Here: the module > >> will be called pwm-brcmstb.c. > >> > >> +config PWM_CLK > >> + tristate "Clock based PWM support" > >> + depends on HAVE_CLK || COMPILE_TEST > >> + help > >> + Generic PWM framework driver for outputs that can be > >> + muxed to clocks. > >> + > >> + To compile this driver as a module, choose M here: the module > >> + will be called pwm-clk. > >> + > >> config PWM_CLPS711X > >> tristate "CLPS711X PWM support" > >> depends on ARCH_CLPS711X || COMPILE_TEST > >> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > >> index 708840b7fba8..4a860103c470 100644 > >> --- a/drivers/pwm/Makefile > >> +++ b/drivers/pwm/Makefile > >> @@ -10,6 +10,7 @@ obj-$(CONFIG_PWM_BCM_KONA) += pwm-bcm-kona.o > >> obj-$(CONFIG_PWM_BCM2835) += pwm-bcm2835.o > >> obj-$(CONFIG_PWM_BERLIN) += pwm-berlin.o > >> obj-$(CONFIG_PWM_BRCMSTB) += pwm-brcmstb.o > >> +obj-$(CONFIG_PWM_CLK) += pwm-clk.o > >> obj-$(CONFIG_PWM_CLPS711X) += pwm-clps711x.o > >> obj-$(CONFIG_PWM_CRC) += pwm-crc.o > >> obj-$(CONFIG_PWM_CROS_EC) += pwm-cros-ec.o > >> diff --git a/drivers/pwm/pwm-clk.c b/drivers/pwm/pwm-clk.c > >> new file mode 100644 > >> index 000000000000..55fd320b9c19 > >> --- /dev/null > >> +++ b/drivers/pwm/pwm-clk.c > >> @@ -0,0 +1,143 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* > >> + * Clock based PWM controller > >> + * > >> + * Copyright (c) 2021 Nikita Travkin <nikita@xxxxxxx> > >> + * > >> + * This is an "adapter" driver that allows PWM consumers to use > >> + * system clocks with duty cycle control as PWM outputs. > >> + * > >> + * Limitations: > >> + * - There is no way to atomically set both clock rate and > >> + * duty-cycle so glitches are possible when new pwm state > >> + * is applied. > >> + * - Period depends on the underlying clock driver and, > >> + * in general, not guaranteed. > >> + * - Underlying clock may not be able to give 100% > >> + * duty cycle (constant on) and only set the closest > >> + * possible duty cycle. (e.g. 99.9%) > > > > What about 0%? > > You're right, this is also a problematic case that I should've > mentioned here. In fact I *did* have problems with zero written > into the duty cycle register of my clock. I decided that it > should be solved by the hardware driver so I sent a patch > with a zero check there. (As otherwise there might be a clock > that would properly support 0% and 100% cycles so making the > write like this impossible is not a job of this driver I think) > > > > > - Periods are not completed on changes in general. > > I suppose I should reword the limitation, dropping > the reference to impossible atomic operations and > just state that glitches are inevitable. > > > - Behaviour on disable depends on the underlaying clk, don't assume it > > to provide the inactive level. > > > > Hm, now thinking of it, I'm not sure if the clock line > was set to logic 0 or was left floating (which is what I assume > you mean by the undefined behavior here) on the clock I was > debugging this on with an oscilloscope. (nor am I sure > if I even can make such a conclusion by looking at that...) > > Do you think that this should be just documented in the > limitations? Like: > > - Underlying clock may not be able to give 0% or 100% > duty cycle (constant off or on) and only set the > closest possible duty cycle. (e.g. 0.1% or 99.9%) I would not bet on this. Maybe in such a case clk_set_duty_cycle might also fail. The clk API isn't (TTBOMK) well-defined enough to make promises like that. > - When the PWM is disabled, the clock will be disabled > as well. User should take care of properly pulling > the line down in case the disabled clock leaves it > floating. This isn't universally true. I'd expect that just freezing (i.e. driving either high or low depending on the state when the clk was stopped) is a very usual behaviour. So a pull isn't always a good idea. I would just keep that unspecified. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
