[{"id":3673794,"web_url":"http://patchwork.ozlabs.org/comment/3673794/","msgid":"<41d54477e46354664360931ffcbeda11@trvn.ru>","list_archive_url":null,"date":"2026-04-06T16:20:16","subject":"Re: [PATCH 2/2] pwm: clk-pwm: add GPIO and pinctrl support for\n constant output levels","submitter":{"id":82201,"url":"http://patchwork.ozlabs.org/api/people/82201/","name":"Nikita Travkin","email":"nikita@trvn.ru"},"content":"Xilin Wu писал(а) 06.04.2026 20:50:\n> The clk-pwm driver cannot guarantee a defined output level when the\n> PWM is disabled or when 0%/100% duty cycle is requested, because the\n> pin state when the clock is stopped is hardware-dependent.\n> \n> Add optional GPIO and pinctrl support: when a GPIO descriptor and\n> pinctrl states (\"default\" for clock mux, \"gpio\" for GPIO mode) are\n> provided in the device tree, the driver switches the pin to GPIO mode\n> and drives the appropriate level for disabled/0%/100% states. For\n> normal PWM output, the pin is switched back to its clock function mux.\n> \n> If no GPIO is provided, the driver falls back to the original\n> clock-only behavior.\n> \n> Signed-off-by: Xilin Wu <sophon@radxa.com>\n> ---\n>  drivers/pwm/pwm-clk.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++-----\n>  1 file changed, 66 insertions(+), 6 deletions(-)\n> \n> diff --git a/drivers/pwm/pwm-clk.c b/drivers/pwm/pwm-clk.c\n> index f8f5af57acba..99821fae54e7 100644\n> --- a/drivers/pwm/pwm-clk.c\n> +++ b/drivers/pwm/pwm-clk.c\n> @@ -10,12 +10,15 @@\n>   * Limitations:\n>   * - Due to the fact that exact behavior depends on the underlying\n>   *   clock driver, various limitations are possible.\n> - * - Underlying clock may not be able to give 0% or 100% duty cycle\n> - *   (constant off or on), exact behavior will depend on the clock.\n> - * - When the PWM is disabled, the clock will be disabled as well,\n> - *   line state will depend on the clock.\n\nnit: I think those limitations would still stand for existing\nusers, perhaps we could just add \"... unless gpio pinctrl state\nis supplied\" to these two?\n\n>   * - The clk API doesn't expose the necessary calls to implement\n>   *   .get_state().\n> + *\n> + * Optionally, a GPIO descriptor and pinctrl states (\"default\" and\n> + * \"gpio\") can be provided. When a constant output level is needed\n> + * (0% duty, 100% duty, or disabled), the driver switches the pin to\n> + * GPIO mode and drives the appropriate level. For normal PWM output\n> + * the pin is switched back to its clock function mux. If no GPIO is\n> + * provided, the driver falls back to the original clock-only behavior.\n>   */\n>  \n>  #include <linux/kernel.h>\n> @@ -25,11 +28,17 @@\n>  #include <linux/of.h>\n>  #include <linux/platform_device.h>\n>  #include <linux/clk.h>\n> +#include <linux/gpio/consumer.h>\n> +#include <linux/pinctrl/consumer.h>\n>  #include <linux/pwm.h>\n>  \n>  struct pwm_clk_chip {\n>  \tstruct clk *clk;\n>  \tbool clk_enabled;\n> +\tstruct pinctrl *pinctrl;\n> +\tstruct pinctrl_state *pins_default;  /* clock function mux */\n> +\tstruct pinctrl_state *pins_gpio;     /* GPIO mode */\n> +\tstruct gpio_desc *gpiod;\n>  };\n>  \n>  static inline struct pwm_clk_chip *to_pwm_clk_chip(struct pwm_chip *chip)\n> @@ -45,14 +54,36 @@ static int pwm_clk_apply(struct pwm_chip *chip, struct pwm_device *pwm,\n>  \tu32 rate;\n>  \tu64 period = state->period;\n>  \tu64 duty_cycle = state->duty_cycle;\n> +\tbool constant_level = false;\n> +\tint gpio_value = 0;\n>  \n>  \tif (!state->enabled) {\n> -\t\tif (pwm->state.enabled) {\n> +\t\tconstant_level = true;\n> +\t\tgpio_value = 0;\n> +\t} else if (state->duty_cycle == 0) {\n> +\t\tconstant_level = true;\n> +\t\tgpio_value = (state->polarity == PWM_POLARITY_INVERSED) ? 1 : 0;\n> +\t} else if (state->duty_cycle >= state->period) {\n> +\t\tconstant_level = true;\n> +\t\tgpio_value = (state->polarity == PWM_POLARITY_INVERSED) ? 0 : 1;\n> +\t}\n> +\n> +\tif (constant_level) {\n> +\t\tif (pcchip->gpiod) {\n> +\t\t\tpinctrl_select_state(pcchip->pinctrl, pcchip->pins_gpio);\n> +\t\t\tgpiod_direction_output(pcchip->gpiod, gpio_value);\n\nIs this the same case as below where gpio state has to be set\nbefore we can control it, or can we swap those so we first\nput gpio into a known state and only then mux it to the pad?\n\n\nThanks for improving this driver,\nNikita\n\n> +\t\t}\n> +\t\tif (pcchip->clk_enabled) {\n>  \t\t\tclk_disable(pcchip->clk);\n>  \t\t\tpcchip->clk_enabled = false;\n>  \t\t}\n>  \t\treturn 0;\n> -\t} else if (!pwm->state.enabled) {\n> +\t}\n> +\n> +\tif (pcchip->gpiod)\n> +\t\tpinctrl_select_state(pcchip->pinctrl, pcchip->pins_default);\n> +\n> +\tif (!pcchip->clk_enabled) {\n>  \t\tret = clk_enable(pcchip->clk);\n>  \t\tif (ret)\n>  \t\t\treturn ret;\n> @@ -97,6 +128,35 @@ static int pwm_clk_probe(struct platform_device *pdev)\n>  \t\treturn dev_err_probe(&pdev->dev, PTR_ERR(pcchip->clk),\n>  \t\t\t\t     \"Failed to get clock\\n\");\n>  \n> +\tpcchip->pinctrl = devm_pinctrl_get(&pdev->dev);\n> +\tif (IS_ERR(pcchip->pinctrl)) {\n> +\t\tret = PTR_ERR(pcchip->pinctrl);\n> +\t\tpcchip->pinctrl = NULL;\n> +\t\tif (ret == -EPROBE_DEFER)\n> +\t\t\treturn ret;\n> +\t} else {\n> +\t\tpcchip->pins_default = pinctrl_lookup_state(pcchip->pinctrl,\n> +\t\t\t\t\t\t\t    PINCTRL_STATE_DEFAULT);\n> +\t\tpcchip->pins_gpio = pinctrl_lookup_state(pcchip->pinctrl,\n> +\t\t\t\t\t\t\t \"gpio\");\n> +\t\tif (IS_ERR(pcchip->pins_default) || IS_ERR(pcchip->pins_gpio))\n> +\t\t\tpcchip->pinctrl = NULL;\n> +\t}\n> +\n> +\t/*\n> +\t * Switch to GPIO pinctrl state before requesting the GPIO.\n> +\t * The driver core has already applied the \"default\" state, which\n> +\t * muxes the pin to the clock function and claims it.  We must\n> +\t * release that claim first so that gpiolib can request the pin.\n> +\t */\n> +\tif (pcchip->pinctrl)\n> +\t\tpinctrl_select_state(pcchip->pinctrl, pcchip->pins_gpio);\n> +\n> +\tpcchip->gpiod = devm_gpiod_get_optional(&pdev->dev, NULL, GPIOD_ASIS);\n> +\tif (IS_ERR(pcchip->gpiod))\n> +\t\treturn dev_err_probe(&pdev->dev, PTR_ERR(pcchip->gpiod),\n> +\t\t\t\t     \"Failed to get gpio\\n\");\n> +\n>  \tchip->ops = &pwm_clk_ops;\n>  \n>  \tret = pwmchip_add(chip);","headers":{"Return-Path":"\n <linux-pwm+bounces-8498-incoming=patchwork.ozlabs.org@vger.kernel.org>","X-Original-To":["incoming@patchwork.ozlabs.org","linux-pwm@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=trvn.ru header.i=@trvn.ru header.a=rsa-sha256\n header.s=mail header.b=2pDhEJYT;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c15:e001:75::12fc:5321; helo=sin.lore.kernel.org;\n envelope-from=linux-pwm+bounces-8498-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=trvn.ru header.i=@trvn.ru\n header.b=\"2pDhEJYT\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=45.141.101.25","smtp.subspace.kernel.org;\n dmarc=pass (p=quarantine dis=none) header.from=trvn.ru","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=trvn.ru"],"Received":["from sin.lore.kernel.org (sin.lore.kernel.org\n [IPv6:2600:3c15:e001:75::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fqF9r5nmCz1xy1\n\tfor <incoming@patchwork.ozlabs.org>; Tue, 07 Apr 2026 02:28:00 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sin.lore.kernel.org (Postfix) with ESMTP id 565A930072BC\n\tfor <incoming@patchwork.ozlabs.org>; Mon,  6 Apr 2026 16:27:57 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 2B1B438AC7D;\n\tMon,  6 Apr 2026 16:27:56 +0000 (UTC)","from box.trvn.ru (box.trvn.ru [45.141.101.25])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id A149930EF9B;\n\tMon,  6 Apr 2026 16:27:54 +0000 (UTC)","from authenticated-user (box.trvn.ru [45.141.101.25])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest\n SHA256)\n\t(No client certificate requested)\n\tby box.trvn.ru (Postfix) with ESMTPSA id DB5C871601;\n\tMon,  6 Apr 2026 21:20:16 +0500 (+05)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1775492876; cv=none;\n b=L1vjHk8q/ov0hC2DZ8yzidgO35SHBGS9htYP7kRI9XMD3gToSsxTKqy5AcTVrzM40rE5N5SEfcY4aW0C35wDev+XPdjAdAdP2zccP7HpUluS1nkcnbQSYtxyuJlJk+mMEIiTKH2lKOjAQg3azjEiq/RD+QHhuw1t3KsZGOQ9Drg=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1775492876; c=relaxed/simple;\n\tbh=RVnjW7CD2ecBRU87/pB8wigS1EWW6WYzVGDWe2uBPl0=;\n\th=MIME-Version:Date:From:To:Cc:Subject:In-Reply-To:References:\n\t Message-ID:Content-Type;\n b=YIXpicoJD6nkKPcmftNoFbfZwzKZqWzwbBYJA8p3jl/GxjTXkSpZL7mGV91BZxXjTCyGxWtdKXibfDmImIVDA9lNBCT1r7EaOfsycFzfqKoMA7qnIrBLJL0gOsRCzKIapOCnBClwzU2UZMdECbPFGAjfOTveinWNXVjv7B2hs4M=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=quarantine dis=none) header.from=trvn.ru;\n spf=pass smtp.mailfrom=trvn.ru;\n dkim=pass (2048-bit key) header.d=trvn.ru header.i=@trvn.ru\n header.b=2pDhEJYT; arc=none smtp.client-ip=45.141.101.25","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=trvn.ru; s=mail;\n\tt=1775492416; bh=RVnjW7CD2ecBRU87/pB8wigS1EWW6WYzVGDWe2uBPl0=;\n\th=Date:From:To:Cc:Subject:In-Reply-To:References:From;\n\tb=2pDhEJYTGguKAHIryfNB3cWmhxeJK+qm9BERexNa8oWwmgCed72Ul3dn0XnaHKv7S\n\t QQbjRu+ADwCj14p5f9Fna2OLxFc/oHhm2u2fwStLHCZubKp1/T2l7iwWA+//V64+C0\n\t 2GXoK1kLZ5Ua6p4w9b1fUhU8CUaa9kNUssOvXa+ygkqQirrwRueUsPLooObVeWi75D\n\t n5YeiYUHh8bX81HTwTy1ojxQZ9PuS0ufkXsCOcvK3YKpPIAJ/epK0L0wlHtLYiTvHH\n\t ssdD0LfBD6E4w8Tk8rwgs5K9kMGM/i7dqEOp1TYy9Pu0XfvjC42U0T1hwit997PIK5\n\t HF84HSJ+dK/sg==","Precedence":"bulk","X-Mailing-List":"linux-pwm@vger.kernel.org","List-Id":"<linux-pwm.vger.kernel.org>","List-Subscribe":"<mailto:linux-pwm+subscribe@vger.kernel.org>","List-Unsubscribe":"<mailto:linux-pwm+unsubscribe@vger.kernel.org>","MIME-Version":"1.0","Date":"Mon, 06 Apr 2026 21:20:16 +0500","From":"Nikita Travkin <nikita@trvn.ru>","To":"Xilin Wu <sophon@radxa.com>","Cc":"=?utf-8?q?Uwe_Kleine-K=C3=B6nig?= <ukleinek@kernel.org>,\n Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzk+dt@kernel.org>,\n Conor Dooley <conor+dt@kernel.org>, linux-pwm@vger.kernel.org,\n devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,\n linux-arm-msm@vger.kernel.org","Subject":"Re: [PATCH 2/2] pwm: clk-pwm: add GPIO and pinctrl support for\n constant output levels","In-Reply-To":"<20260406-clk-pwm-gpio-v1-2-40d2f3a20aff@radxa.com>","References":"<20260406-clk-pwm-gpio-v1-0-40d2f3a20aff@radxa.com>\n <20260406-clk-pwm-gpio-v1-2-40d2f3a20aff@radxa.com>","Message-ID":"<41d54477e46354664360931ffcbeda11@trvn.ru>","X-Sender":"nikita@trvn.ru","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit"}}]