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

[Phil Howard <phil@xxxxxxxxxxxxx>]

On Sun, 4 Feb 2024 at 22:09, Stefan Wahren <wahrenst@xxxxxxx> 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].
>
> The work has been tested on a Raspberry Pi 3 B+ and a cheap logic
> analyzer.

Tested on a Pi 5 with a Rigol scope. I have not done any comparative
testing to the contemporary methods of GPIO on a Pi (they're probably
not great) but it looks good at a glance.

There is jitter, as you might expect, though I would expect this to be much
better than the userspace soft PWM we're used to.

Mostly for Pi users who happen to be watching this unfold-

PWM on the Pi 5 is achieved via the PCIe bus to the RP1 peripheral and
IO controller on a Pi 5 so it's possible to saturate the PCIe bus and have
A Bad Day. I think ~200KHz is about the practical limit here, if I try and push
to 400KHz things start to get dicy. The faster you go, the more proportional
impact jitter will have on the resulting signal.

My optimistic first try in the MHz range booted me off SSH and required a
power cycle so I'm not even sure what went wrong. Less of an issue for
on-chip GPIO controllers ala <= Pi 4.

Here's my very naive overlay -

// Definitions for w1-gpio module (without external pullup)
/dts-v1/;
/plugin/;

/ {
    compatible = "brcm,bcm2835";
    fragment@0 {
        target-path = "/";
        __overlay__ {
            pwm_gpio: pwm_gpio@0 {
                compatible = "pwm-gpio";
                pinctrl-names = "default";
                pinctrl-0 = <&pwm_gpio_pins>;
                gpios = <&gpio 4 0>;
                status = "okay";
            };
        };
    };

    fragment@1 {
        target = <&gpio>;
        __overlay__ {
            pwm_gpio_pins: pwm_gpio_pins@0 {
                brcm,pins = <4>;
                brcm,function = <1>; // out
                brcm,pull = <0>; // off
            };
        };
    };
};

>
> 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/
>
> Nicola Di Lieto (1):
>   dt-bindings: pwm: Add pwm-gpio
>
> Vincent Whitchurch (1):
>   pwm: Add GPIO PWM driver
>
>  .../devicetree/bindings/pwm/pwm-gpio.yaml     |  42 ++++
>  drivers/pwm/Kconfig                           |  11 +
>  drivers/pwm/Makefile                          |   1 +
>  drivers/pwm/pwm-gpio.c                        | 228 ++++++++++++++++++
>  4 files changed, 282 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-gpio.yaml
>  create mode 100644 drivers/pwm/pwm-gpio.c
>
> --
> 2.34.1
>


-- 
Philip Howard