linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/2] pwm: Add GPIO PWM driver
@ 2024-06-04 21:00 Linus Walleij
  2024-06-04 21:00 ` [PATCH v7 1/2] dt-bindings: pwm: Add pwm-gpio Linus Walleij
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Linus Walleij @ 2024-06-04 21:00 UTC (permalink / raw)
  To: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, andy.shevchenko, Philip Howard, Sean Young,
	Chris Morgan, Stefan Wahren, linux-gpio, linux-pwm, devicetree
  Cc: Linus Walleij, Nicola Di Lieto, Krzysztof Kozlowski, Dhruva Gole,
	Vincent Whitchurch, Andy Shevchenko

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@linaro.org>
---
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@linaro.org

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@axis.com/T/
[2] - https://lore.kernel.org/all/20201205214353.xapax46tt5snzd2v@einstein.dilieto.eu/
[3] - https://lore.kernel.org/linux-pwm/CACRpkdary+kDrTJ=u4VbSTv7wXGLQj9_fy7mv0w-Zg+eDvGXVQ@mail.gmail.com/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@linaro.org>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v7 1/2] dt-bindings: pwm: Add pwm-gpio
  2024-06-04 21:00 [PATCH v7 0/2] pwm: Add GPIO PWM driver Linus Walleij
@ 2024-06-04 21:00 ` Linus Walleij
  2024-06-04 21:00 ` [PATCH v7 2/2] pwm: Add GPIO PWM driver Linus Walleij
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Linus Walleij @ 2024-06-04 21:00 UTC (permalink / raw)
  To: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, andy.shevchenko, Philip Howard, Sean Young,
	Chris Morgan, Stefan Wahren, linux-gpio, linux-pwm, devicetree
  Cc: Linus Walleij, Nicola Di Lieto, Krzysztof Kozlowski, Dhruva Gole

From: Nicola Di Lieto <nicola.dilieto@gmail.com>

Add bindings for PWM modulated by GPIO.

Signed-off-by: Nicola Di Lieto <nicola.dilieto@gmail.com>
Co-developed-by: Stefan Wahren <wahrenst@gmx.net>
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Dhruva Gole <d-gole@ti.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../devicetree/bindings/pwm/pwm-gpio.yaml          | 46 ++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-gpio.yaml b/Documentation/devicetree/bindings/pwm/pwm-gpio.yaml
new file mode 100644
index 000000000000..1576c193f2ab
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-gpio.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/pwm-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic software PWM for modulating GPIOs
+
+maintainers:
+  - Stefan Wahren <wahrenst@gmx.net>
+
+allOf:
+  - $ref: pwm.yaml#
+
+properties:
+  compatible:
+    const: pwm-gpio
+
+  "#pwm-cells":
+    const: 3
+    description:
+      See pwm.yaml in this directory for a description of the cells format.
+      The first cell which represents the PWM instance number must always
+      be zero.
+
+  gpios:
+    description:
+      GPIO to be modulated
+    maxItems: 1
+
+required:
+  - compatible
+  - "#pwm-cells"
+  - gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    pwm {
+        #pwm-cells = <3>;
+        compatible = "pwm-gpio";
+        gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
+    };

-- 
2.45.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v7 2/2] pwm: Add GPIO PWM driver
  2024-06-04 21:00 [PATCH v7 0/2] pwm: Add GPIO PWM driver Linus Walleij
  2024-06-04 21:00 ` [PATCH v7 1/2] dt-bindings: pwm: Add pwm-gpio Linus Walleij
@ 2024-06-04 21:00 ` Linus Walleij
  2024-06-05  5:52   ` Dhruva Gole
  2024-06-18 11:51 ` [PATCH v7 0/2] " Phil Howard
  2024-06-27  6:52 ` Uwe Kleine-König
  3 siblings, 1 reply; 7+ messages in thread
From: Linus Walleij @ 2024-06-04 21:00 UTC (permalink / raw)
  To: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, andy.shevchenko, Philip Howard, Sean Young,
	Chris Morgan, Stefan Wahren, linux-gpio, linux-pwm, devicetree
  Cc: Linus Walleij, Vincent Whitchurch, Andy Shevchenko

From: Vincent Whitchurch <vincent.whitchurch@axis.com>

Add a software PWM which toggles a GPIO from a high-resolution timer.

This will naturally not be as accurate or as efficient as a hardware
PWM, but it is useful in some cases.  I have for example used it for
evaluating LED brightness handling (via leds-pwm) on a board where the
LED was just hooked up to a GPIO, and for a simple verification of the
timer frequency on another platform.

Since high-resolution timers are used, sleeping GPIO chips are not
supported and are rejected in the probe function.

Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
Co-developed-by: Stefan Wahren <wahrenst@gmx.net>
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Co-developed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 Documentation/driver-api/gpio/drivers-on-gpio.rst |   7 +-
 drivers/pwm/Kconfig                               |  11 +
 drivers/pwm/Makefile                              |   1 +
 drivers/pwm/pwm-gpio.c                            | 241 ++++++++++++++++++++++
 4 files changed, 259 insertions(+), 1 deletion(-)

diff --git a/Documentation/driver-api/gpio/drivers-on-gpio.rst b/Documentation/driver-api/gpio/drivers-on-gpio.rst
index af632d764ac6..95572d2a94ce 100644
--- a/Documentation/driver-api/gpio/drivers-on-gpio.rst
+++ b/Documentation/driver-api/gpio/drivers-on-gpio.rst
@@ -27,7 +27,12 @@ hardware descriptions such as device tree or ACPI:
   to the lines for a more permanent solution of this type.
 
 - gpio-beeper: drivers/input/misc/gpio-beeper.c is used to provide a beep from
-  an external speaker connected to a GPIO line.
+  an external speaker connected to a GPIO line. (If the beep is controlled by
+  off/on, for an actual PWM waveform, see pwm-gpio below.)
+
+- pwm-gpio: drivers/pwm/pwm-gpio.c is used to toggle a GPIO with a high
+  resolution timer producing a PWM waveform on the GPIO line, as well as
+  Linux high resolution timers can do.
 
 - extcon-gpio: drivers/extcon/extcon-gpio.c is used when you need to read an
   external connector status, such as a headset line for an audio driver or an
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 1dd7921194f5..68ba28d52c4c 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -223,6 +223,17 @@ config PWM_FSL_FTM
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-fsl-ftm.
 
+config PWM_GPIO
+	tristate "GPIO PWM support"
+	depends on GPIOLIB
+	depends on HIGH_RES_TIMERS
+	help
+	  Generic PWM framework driver for software PWM toggling a GPIO pin
+	  from kernel high-resolution timers.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-gpio.
+
 config PWM_HIBVT
 	tristate "HiSilicon BVT PWM support"
 	depends on ARCH_HISI || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 90913519f11a..65d62cc41a8f 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_PWM_DWC_CORE)	+= pwm-dwc-core.o
 obj-$(CONFIG_PWM_DWC)		+= pwm-dwc.o
 obj-$(CONFIG_PWM_EP93XX)	+= pwm-ep93xx.o
 obj-$(CONFIG_PWM_FSL_FTM)	+= pwm-fsl-ftm.o
+obj-$(CONFIG_PWM_GPIO)		+= pwm-gpio.o
 obj-$(CONFIG_PWM_HIBVT)		+= pwm-hibvt.o
 obj-$(CONFIG_PWM_IMG)		+= pwm-img.o
 obj-$(CONFIG_PWM_IMX1)		+= pwm-imx1.o
diff --git a/drivers/pwm/pwm-gpio.c b/drivers/pwm/pwm-gpio.c
new file mode 100644
index 000000000000..9f8884ac7504
--- /dev/null
+++ b/drivers/pwm/pwm-gpio.c
@@ -0,0 +1,241 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generic software PWM for modulating GPIOs
+ *
+ * Copyright (C) 2020 Axis Communications AB
+ * Copyright (C) 2020 Nicola Di Lieto
+ * Copyright (C) 2024 Stefan Wahren
+ * Copyright (C) 2024 Linus Walleij
+ */
+
+#include <linux/cleanup.h>
+#include <linux/container_of.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/hrtimer.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/pwm.h>
+#include <linux/spinlock.h>
+#include <linux/time.h>
+#include <linux/types.h>
+
+struct pwm_gpio {
+	struct hrtimer gpio_timer;
+	struct gpio_desc *gpio;
+	struct pwm_state state;
+	struct pwm_state next_state;
+
+	/* Protect internal state between pwm_ops and hrtimer */
+	spinlock_t lock;
+
+	bool changing;
+	bool running;
+	bool level;
+};
+
+static void pwm_gpio_round(struct pwm_state *dest, const struct pwm_state *src)
+{
+	u64 dividend;
+	u32 remainder;
+
+	*dest = *src;
+
+	/* Round down to hrtimer resolution */
+	dividend = dest->period;
+	remainder = do_div(dividend, hrtimer_resolution);
+	dest->period -= remainder;
+
+	dividend = dest->duty_cycle;
+	remainder = do_div(dividend, hrtimer_resolution);
+	dest->duty_cycle -= remainder;
+}
+
+static u64 pwm_gpio_toggle(struct pwm_gpio *gpwm, bool level)
+{
+	const struct pwm_state *state = &gpwm->state;
+	bool invert = state->polarity == PWM_POLARITY_INVERSED;
+
+	gpwm->level = level;
+	gpiod_set_value(gpwm->gpio, gpwm->level ^ invert);
+
+	if (!state->duty_cycle || state->duty_cycle == state->period) {
+		gpwm->running = false;
+		return 0;
+	}
+
+	gpwm->running = true;
+	return level ? state->duty_cycle : state->period - state->duty_cycle;
+}
+
+static enum hrtimer_restart pwm_gpio_timer(struct hrtimer *gpio_timer)
+{
+	struct pwm_gpio *gpwm = container_of(gpio_timer, struct pwm_gpio,
+					     gpio_timer);
+	u64 next_toggle;
+	bool new_level;
+
+	guard(spinlock_irqsave)(&gpwm->lock);
+
+	/* Apply new state at end of current period */
+	if (!gpwm->level && gpwm->changing) {
+		gpwm->changing = false;
+		gpwm->state = gpwm->next_state;
+		new_level = !!gpwm->state.duty_cycle;
+	} else {
+		new_level = !gpwm->level;
+	}
+
+	next_toggle = pwm_gpio_toggle(gpwm, new_level);
+	if (next_toggle)
+		hrtimer_forward(gpio_timer, hrtimer_get_expires(gpio_timer),
+				ns_to_ktime(next_toggle));
+
+	return next_toggle ? HRTIMER_RESTART : HRTIMER_NORESTART;
+}
+
+static int pwm_gpio_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			  const struct pwm_state *state)
+{
+	struct pwm_gpio *gpwm = pwmchip_get_drvdata(chip);
+	bool invert = state->polarity == PWM_POLARITY_INVERSED;
+
+	if (state->duty_cycle && state->duty_cycle < hrtimer_resolution)
+		return -EINVAL;
+
+	if (state->duty_cycle != state->period &&
+	    (state->period - state->duty_cycle < hrtimer_resolution))
+		return -EINVAL;
+
+	if (!state->enabled) {
+		hrtimer_cancel(&gpwm->gpio_timer);
+	} else if (!gpwm->running) {
+		int ret;
+
+		/*
+		 * This just enables the output, but pwm_gpio_toggle()
+		 * really starts the duty cycle.
+		 */
+		ret = gpiod_direction_output(gpwm->gpio, invert);
+		if (ret)
+			return ret;
+	}
+
+	guard(spinlock_irqsave)(&gpwm->lock);
+
+	if (!state->enabled) {
+		pwm_gpio_round(&gpwm->state, state);
+		gpwm->running = false;
+		gpwm->changing = false;
+
+		gpiod_set_value(gpwm->gpio, invert);
+	} else if (gpwm->running) {
+		pwm_gpio_round(&gpwm->next_state, state);
+		gpwm->changing = true;
+	} else {
+		unsigned long next_toggle;
+
+		pwm_gpio_round(&gpwm->state, state);
+		gpwm->changing = false;
+
+		next_toggle = pwm_gpio_toggle(gpwm, !!state->duty_cycle);
+		if (next_toggle)
+			hrtimer_start(&gpwm->gpio_timer, next_toggle,
+				      HRTIMER_MODE_REL);
+	}
+
+	return 0;
+}
+
+static int pwm_gpio_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+			       struct pwm_state *state)
+{
+	struct pwm_gpio *gpwm = pwmchip_get_drvdata(chip);
+
+	guard(spinlock_irqsave)(&gpwm->lock);
+
+	if (gpwm->changing)
+		*state = gpwm->next_state;
+	else
+		*state = gpwm->state;
+
+	return 0;
+}
+
+static const struct pwm_ops pwm_gpio_ops = {
+	.apply = pwm_gpio_apply,
+	.get_state = pwm_gpio_get_state,
+};
+
+static void pwm_gpio_disable_hrtimer(void *data)
+{
+	struct pwm_gpio *gpwm = data;
+
+	hrtimer_cancel(&gpwm->gpio_timer);
+}
+
+static int pwm_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct pwm_chip *chip;
+	struct pwm_gpio *gpwm;
+	int ret;
+
+	chip = devm_pwmchip_alloc(dev, 1, sizeof(*gpwm));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+
+	gpwm = pwmchip_get_drvdata(chip);
+
+	spin_lock_init(&gpwm->lock);
+
+	gpwm->gpio = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
+	if (IS_ERR(gpwm->gpio))
+		return dev_err_probe(dev, PTR_ERR(gpwm->gpio),
+				     "%pfw: could not get gpio\n",
+				     dev_fwnode(dev));
+
+	if (gpiod_cansleep(gpwm->gpio))
+		return dev_err_probe(dev, -EINVAL,
+				     "%pfw: sleeping GPIO not supported\n",
+				     dev_fwnode(dev));
+
+	chip->ops = &pwm_gpio_ops;
+	chip->atomic = true;
+
+	hrtimer_init(&gpwm->gpio_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	ret = devm_add_action_or_reset(dev, pwm_gpio_disable_hrtimer, gpwm);
+	if (ret)
+		return ret;
+
+	gpwm->gpio_timer.function = pwm_gpio_timer;
+
+	ret = pwmchip_add(chip);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "could not add pwmchip\n");
+
+	return 0;
+}
+
+static const struct of_device_id pwm_gpio_dt_ids[] = {
+	{ .compatible = "pwm-gpio" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, pwm_gpio_dt_ids);
+
+static struct platform_driver pwm_gpio_driver = {
+	.driver = {
+		.name = "pwm-gpio",
+		.of_match_table = pwm_gpio_dt_ids,
+	},
+	.probe = pwm_gpio_probe,
+};
+module_platform_driver(pwm_gpio_driver);
+
+MODULE_DESCRIPTION("PWM GPIO driver");
+MODULE_AUTHOR("Vincent Whitchurch");
+MODULE_LICENSE("GPL");

-- 
2.45.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 2/2] pwm: Add GPIO PWM driver
  2024-06-04 21:00 ` [PATCH v7 2/2] pwm: Add GPIO PWM driver Linus Walleij
@ 2024-06-05  5:52   ` Dhruva Gole
  0 siblings, 0 replies; 7+ messages in thread
From: Dhruva Gole @ 2024-06-05  5:52 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, andy.shevchenko, Philip Howard, Sean Young,
	Chris Morgan, Stefan Wahren, linux-gpio, linux-pwm, devicetree,
	Vincent Whitchurch, Andy Shevchenko

On Jun 04, 2024 at 23:00:41 +0200, Linus Walleij wrote:
> From: Vincent Whitchurch <vincent.whitchurch@axis.com>
> 
> Add a software PWM which toggles a GPIO from a high-resolution timer.
> 
> This will naturally not be as accurate or as efficient as a hardware
> PWM, but it is useful in some cases.  I have for example used it for
> evaluating LED brightness handling (via leds-pwm) on a board where the
> LED was just hooked up to a GPIO, and for a simple verification of the
> timer frequency on another platform.
> 
> Since high-resolution timers are used, sleeping GPIO chips are not
> supported and are rejected in the probe function.
> 
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
> Co-developed-by: Stefan Wahren <wahrenst@gmx.net>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Co-developed-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  Documentation/driver-api/gpio/drivers-on-gpio.rst |   7 +-
>  drivers/pwm/Kconfig                               |  11 +
>  drivers/pwm/Makefile                              |   1 +
>  drivers/pwm/pwm-gpio.c                            | 241 ++++++++++++++++++++++
>  4 files changed, 259 insertions(+), 1 deletion(-)
> 

Reviewed-by: Dhruva Gole <d-gole@ti.com>

-- 
Best regards,
Dhruva Gole <d-gole@ti.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 0/2] pwm: Add GPIO PWM driver
  2024-06-04 21:00 [PATCH v7 0/2] pwm: Add GPIO PWM driver Linus Walleij
  2024-06-04 21:00 ` [PATCH v7 1/2] dt-bindings: pwm: Add pwm-gpio Linus Walleij
  2024-06-04 21:00 ` [PATCH v7 2/2] pwm: Add GPIO PWM driver Linus Walleij
@ 2024-06-18 11:51 ` Phil Howard
  2024-06-27  6:52 ` Uwe Kleine-König
  3 siblings, 0 replies; 7+ messages in thread
From: Phil Howard @ 2024-06-18 11:51 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, andy.shevchenko, Sean Young, Chris Morgan,
	Stefan Wahren, linux-gpio, linux-pwm, devicetree, Nicola Di Lieto,
	Krzysztof Kozlowski, Dhruva Gole, Vincent Whitchurch,
	Andy Shevchenko

On Tue, 4 Jun 2024 at 22:00, Linus Walleij <linus.walleij@linaro.org> 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@linaro.org>
> ---
> 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@linaro.org
>
> 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@axis.com/T/
> [2] - https://lore.kernel.org/all/20201205214353.xapax46tt5snzd2v@einstein.dilieto.eu/
> [3] - https://lore.kernel.org/linux-pwm/CACRpkdary+kDrTJ=u4VbSTv7wXGLQj9_fy7mv0w-Zg+eDvGXVQ@mail.gmail.com/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@linaro.org>
>

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

I will give it a shot today.

- Phil

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 0/2] pwm: Add GPIO PWM driver
  2024-06-04 21:00 [PATCH v7 0/2] pwm: Add GPIO PWM driver Linus Walleij
                   ` (2 preceding siblings ...)
  2024-06-18 11:51 ` [PATCH v7 0/2] " Phil Howard
@ 2024-06-27  6:52 ` Uwe Kleine-König
  2024-06-27  9:54   ` Stefan Wahren
  3 siblings, 1 reply; 7+ messages in thread
From: Uwe Kleine-König @ 2024-06-27  6:52 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, andy.shevchenko,
	Philip Howard, Sean Young, Chris Morgan, Stefan Wahren,
	linux-gpio, linux-pwm, devicetree, Nicola Di Lieto,
	Krzysztof Kozlowski, Dhruva Gole, Vincent Whitchurch,
	Andy Shevchenko

[-- Attachment #1: Type: text/plain, Size: 1356 bytes --]

Hello,

On Tue, Jun 04, 2024 at 11:00:39PM +0200, Linus Walleij 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>;
> };

Applied both patches to 
https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next

Thanks
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 0/2] pwm: Add GPIO PWM driver
  2024-06-27  6:52 ` Uwe Kleine-König
@ 2024-06-27  9:54   ` Stefan Wahren
  0 siblings, 0 replies; 7+ messages in thread
From: Stefan Wahren @ 2024-06-27  9:54 UTC (permalink / raw)
  To: Uwe Kleine-König, Linus Walleij
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, andy.shevchenko,
	Philip Howard, Sean Young, Chris Morgan, linux-gpio, linux-pwm,
	devicetree, Nicola Di Lieto, Krzysztof Kozlowski, Dhruva Gole,
	Vincent Whitchurch, Andy Shevchenko

Am 27.06.24 um 08:52 schrieb Uwe Kleine-König:
> Hello,
>
> On Tue, Jun 04, 2024 at 11:00:39PM +0200, Linus Walleij 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>;
>> };
> Applied both patches to
> https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next
\o/

Thanks
>
> Thanks
> Uwe


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-06-27  9:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-04 21:00 [PATCH v7 0/2] pwm: Add GPIO PWM driver Linus Walleij
2024-06-04 21:00 ` [PATCH v7 1/2] dt-bindings: pwm: Add pwm-gpio Linus Walleij
2024-06-04 21:00 ` [PATCH v7 2/2] pwm: Add GPIO PWM driver Linus Walleij
2024-06-05  5:52   ` Dhruva Gole
2024-06-18 11:51 ` [PATCH v7 0/2] " Phil Howard
2024-06-27  6:52 ` Uwe Kleine-König
2024-06-27  9:54   ` Stefan Wahren

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).