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

[Nikita Travkin <nikita@trvn.ru>]

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);