Re: [PATCH 2/2] pwm: clk-pwm: add GPIO and pinctrl support for constant output levels

Nikita Travkin
2026-04-06T16:27:54Z

Xilin Wu писал(а) 06.04.2026 20:50:
> The clk-pwm driver cannot guarantee a defined output level when the
> PWM is disabled or when 0%/100% duty cycle is requested, because the
> pin state when the clock is stopped is hardware-dependent.
>
> Add optional GPIO and pinctrl support: when a GPIO descriptor and
> pinctrl states ("default" for clock mux, "gpio" for GPIO mode) are
> provided in the device tree, the driver switches the pin to GPIO mode
> and drives the appropriate level for disabled/0%/100% states. For
> normal PWM output, the pin is switched back to its clock function mux.
>
> If no GPIO is provided, the driver falls back to the original
> clock-only behavior.
>
> Signed-off-by: Xilin Wu <sophon@radxa.com>
> ---
> drivers/pwm/pwm-clk.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 66 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pwm/pwm-clk.c b/drivers/pwm/pwm-clk.c
> index f8f5af57acba..99821fae54e7 100644
> --- a/drivers/pwm/pwm-clk.c
> +++ b/drivers/pwm/pwm-clk.c
> @@ -10,12 +10,15 @@
> * Limitations:
> * - Due to the fact that exact behavior depends on the underlying
> * clock driver, various limitations are possible.
> - * - Underlying clock may not be able to give 0% or 100% duty cycle
> - * (constant off or on), exact behavior will depend on the clock.
> - * - When the PWM is disabled, the clock will be disabled as well,
> - * line state will depend on the clock.

nit: I think those limitations would still stand for existing
users, perhaps we could just add "... unless gpio pinctrl state
is supplied" to these two?

> * - The clk API doesn't expose the necessary calls to implement
> * .get_state().
> + *
> + * Optionally, a GPIO descriptor and pinctrl states ("default" and
> + * "gpio") can be provided. When a constant output level is needed
> + * (0% duty, 100% duty, or disabled), the driver switches the pin to
> + * GPIO mode and drives the appropriate level. For normal PWM output
> + * the pin is switched back to its clock function mux. If no GPIO is
> + * provided, the driver falls back to the original clock-only behavior.
> */
>
> #include <linux/kernel.h>
> @@ -25,11 +28,17 @@
> #include <linux/of.h>
> #include <linux/platform_device.h>
> #include <linux/clk.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/pinctrl/consumer.h>
> #include <linux/pwm.h>
>
> struct pwm_clk_chip {
> struct clk *clk;
> bool clk_enabled;
> + struct pinctrl *pinctrl;
> + struct pinctrl_state *pins_default; /* clock function mux */
> + struct pinctrl_state *pins_gpio; /* GPIO mode */
> + struct gpio_desc *gpiod;
> };
>
> static inline struct pwm_clk_chip *to_pwm_clk_chip(struct pwm_chip *chip)
> @@ -45,14 +54,36 @@ static int pwm_clk_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> u32 rate;
> u64 period = state->period;
> u64 duty_cycle = state->duty_cycle;
> + bool constant_level = false;
> + int gpio_value = 0;
>
> if (!state->enabled) {
> - if (pwm->state.enabled) {
> + constant_level = true;
> + gpio_value = 0;
> + } else if (state->duty_cycle == 0) {
> + constant_level = true;
> + gpio_value = (state->polarity == PWM_POLARITY_INVERSED) ? 1 : 0;
> + } else if (state->duty_cycle >= state->period) {
> + constant_level = true;
> + gpio_value = (state->polarity == PWM_POLARITY_INVERSED) ? 0 : 1;
> + }
> +
> + if (constant_level) {
> + if (pcchip->gpiod) {
> + pinctrl_select_state(pcchip->pinctrl, pcchip->pins_gpio);
> + gpiod_direction_output(pcchip->gpiod, gpio_value);

Is this the same case as below where gpio state has to be set
before we can control it, or can we swap those so we first
put gpio into a known state and only then mux it to the pad?

Thanks for improving this driver,
Nikita

> + }
> + if (pcchip->clk_enabled) {
> clk_disable(pcchip->clk);
> pcchip->clk_enabled = false;
> }
> return 0;
> - } else if (!pwm->state.enabled) {
> + }
> +
> + if (pcchip->gpiod)
> + pinctrl_select_state(pcchip->pinctrl, pcchip->pins_default);
> +
> + if (!pcchip->clk_enabled) {
> ret = clk_enable(pcchip->clk);
> if (ret)
> return ret;
> @@ -97,6 +128,35 @@ static int pwm_clk_probe(struct platform_device *pdev)
> return dev_err_probe(&pdev->dev, PTR_ERR(pcchip->clk),
> "Failed to get clock\n");
>
> + pcchip->pinctrl = devm_pinctrl_get(&pdev->dev);
> + if (IS_ERR(pcchip->pinctrl)) {
> + ret = PTR_ERR(pcchip->pinctrl);
> + pcchip->pinctrl = NULL;
> + if (ret == -EPROBE_DEFER)
> + return ret;
> + } else {
> + pcchip->pins_default = pinctrl_lookup_state(pcchip->pinctrl,
> + PINCTRL_STATE_DEFAULT);
> + pcchip->pins_gpio = pinctrl_lookup_state(pcchip->pinctrl,
> + "gpio");
> + if (IS_ERR(pcchip->pins_default) || IS_ERR(pcchip->pins_gpio))
> + pcchip->pinctrl = NULL;
> + }
> +
> + /*
> + * Switch to GPIO pinctrl state before requesting the GPIO.
> + * The driver core has already applied the "default" state, which
> + * muxes the pin to the clock function and claims it. We must
> + * release that claim first so that gpiolib can request the pin.
> + */
> + if (pcchip->pinctrl)
> + pinctrl_select_state(pcchip->pinctrl, pcchip->pins_gpio);
> +
> + pcchip->gpiod = devm_gpiod_get_optional(&pdev->dev, NULL, GPIOD_ASIS);
> + if (IS_ERR(pcchip->gpiod))
> + return dev_err_probe(&pdev->dev, PTR_ERR(pcchip->gpiod),
> + "Failed to get gpio\n");
> +
> chip->ops = &pwm_clk_ops;
>
> ret = pwmchip_add(chip);