Re: [PATCH v7 0/2] pwm: Add GPIO PWM driver

[Phil Howard <phil@xxxxxxxxxxxxx>]

On Tue, 4 Jun 2024 at 22:00, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>
> Add a software PWM which toggles a GPIO from a high-resolution timer.
>
> Recent discussions in the Raspberry Pi community revealt that a lot
> of users still use MMIO userspace tools for GPIO access. One argument
> for this approach is the lack of a GPIO PWM kernel driver. So this
> series tries to fill this gap.
>
> This continues the work of Vincent Whitchurch [1], which is easier
> to read and more consequent by rejecting sleeping GPIOs than Nicola's
> approach [2]. It further takes over where Stefan Wahren left off.
>
> I have not looked into the interrupt storm problem mentioned in [3]
> but instead focused on some real-life tests:
>
> The IXP4xx NSLU2 has a speaker connected directly to a GPIO, and I
> wanted to use this patch to provide a proper beeper for the machine
> and not have to rely on custom hacks.
>
> I added a DTS patch like this:
>
> gpio_pwm: pwm {
>         #pwm-cells = <3>;
>         compatible = "pwm-gpio";
>         gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>;
> };
>
> beeper {
>         compatible = "pwm-beeper";
>         pwms = <&gpio_pwm 0 1 0>;
>         beeper-hz = <1000>;
> };
>
> Then activated the necessary drivers with input evdev and they
> both probe fine. Then I use this test program:
> https://gist.github.com/licheegh/3dbdc8918325e8af8d4d13805cd0c350
> with a few different beep frequencies, such as "beep 400"
> for an OK sounding 400 Hz signal.
>
> Above ~10000 Hz the sound becomes bad and mostly sound like
> different kinds of noise. But this is not so bad for the NSLU2
> which is a 266 MHz ARM XScale machine, and we do not need any better
> on this machine so mission accomplished. Pushing the playback
> to higher and higher rates does not crash the machine, the
> sample program always terminates.
>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> Changes in v7:
> - Drop platform_set_drvdata() as nothing use this anymore.
> - Add a small blurb documenting the pwm-cells in the binding.
> - Link to v6: https://lore.kernel.org/r/20240602-pwm-gpio-v6-0-e8f6ec9cc783@xxxxxxxxxx
>
> v6:
> - Add a small blurb to drivers-on-gpio.rst
> - Add missing headers to driver
> - Use guarded spinlocks from <linux/cleanup.h>
> - Drop minor surplus whitespace
> - Use dev pointer everywhere in probe()
> - Drop dev_info() chatter at end of probe()
> - Use devm_add_action_or_reset() for a hook to disable the hrtimer and
>   get rid of the .remove() callback altogether.
>
> V5:
>  - add Reviewed-by's for dt-binding patch
>  - rebase on top of v6.10-rc1
>  - print hr resolution at driver probe
>  - fix grammar in Kconfig
>  - fix return type of pwm_gpio_toggle
>  - implement hrtimer resolution rounding as noted by Uwe
>  - use firmware node path instead of GPIO numbers as suggested by Andy
>  - adjust some header includes and style issues as noted by Andy
>
> V4:
>  - address DT bindings comments from Conor Dooley
>  - drop unnecessary MODULE_ALIAS() as suggested by Krzysztof Kozlowski
>  - add range checks to apply which consider the hrtimer resolution
>    (idea comes from Sean Young)
>  - mark driver as atomic as suggested by Sean Young
>
> V3:
>  - rebase on top of v6.8-pwm-next
>  - cherry-pick improvements from Nicola's series
>  - try to address Uwe's, Linus' and Andy's comments
>  - try to avoid GPIO glitches during probe
>  - fix pwm_gpio_remove()
>  - some code clean up's and comments
>
> V2:
>  - Rename gpio to gpios in binding
>  - Calculate next expiry from expected current expiry rather than "now"
>  - Only change configuration after current period ends
>  - Implement get_state()
>  - Add error message for probe failures
>  - Stop PWM before unregister
>
> [1] - https://lore.kernel.org/all/20200915135445.al75xmjxudj2rgcp@xxxxxxxx/T/
> [2] - https://lore.kernel.org/all/20201205214353.xapax46tt5snzd2v@xxxxxxxxxxxxxxxxxxx/
> [3] - https://lore.kernel.org/linux-pwm/CACRpkdary+kDrTJ=u4VbSTv7wXGLQj9_fy7mv0w-Zg+eDvGXVQ@xxxxxxxxxxxxxx/T/#m513f255daa9f30c325d11999cacd086411591bf9
>
> ---
> Nicola Di Lieto (1):
>       dt-bindings: pwm: Add pwm-gpio
>
> Vincent Whitchurch (1):
>       pwm: Add GPIO PWM driver
>
>  .../devicetree/bindings/pwm/pwm-gpio.yaml          |  46 ++++
>  Documentation/driver-api/gpio/drivers-on-gpio.rst  |   7 +-
>  drivers/pwm/Kconfig                                |  11 +
>  drivers/pwm/Makefile                               |   1 +
>  drivers/pwm/pwm-gpio.c                             | 241 +++++++++++++++++++++
>  5 files changed, 305 insertions(+), 1 deletion(-)
> ---
> base-commit: cc97b8f37b097a1598410154ca472964a1d9b255
> change-id: 20240530-pwm-gpio-e9133a1806ec
>
> Best regards,
> --
> Linus Walleij <linus.walleij@xxxxxxxxxx>
>

Thank you for picking this up, it's reassuring to see it making progress.

I will give it a shot today.

- Phil