linux-kernel - Re: [PATCH 3/3] pwm: axi-pwmgen: add support for external clock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <zdltaexty6pzbqesoluuyluygyt6w7nq7r2wccmtfktppwuw3e@qb36fsu3jq4k>
Date: Wed, 21 May 2025 11:22:07 +0200
From: Uwe Kleine-König <ukleinek@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Michael Hennerich <michael.hennerich@...log.com>, 
	Nuno Sá <nuno.sa@...log.com>, Trevor Gamblin <tgamblin@...libre.com>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, linux-pwm@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] pwm: axi-pwmgen: add support for external clock

On Tue, May 20, 2025 at 04:00:46PM -0500, David Lechner wrote:
> Add support for external clock to the AXI PWM generator driver.
> 
> In most cases, there is a separate external clock that drives the PWM
> output separate from the peripheral clock. This allows enabling both
> clocks.
> 
> Signed-off-by: David Lechner <dlechner@...libre.com>
> ---
>  drivers/pwm/pwm-axi-pwmgen.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-axi-pwmgen.c b/drivers/pwm/pwm-axi-pwmgen.c
> index 4337c8f5acf055fc87dc134f2a70b99b0cb5ede6..67992a7561ec0440b1c1fa327f844a0602872771 100644
> --- a/drivers/pwm/pwm-axi-pwmgen.c
> +++ b/drivers/pwm/pwm-axi-pwmgen.c
> @@ -280,9 +280,26 @@ static int axi_pwmgen_probe(struct platform_device *pdev)
>  	ddata = pwmchip_get_drvdata(chip);
>  	ddata->regmap = regmap;
>  
> -	clk = devm_clk_get_enabled(dev, NULL);
> -	if (IS_ERR(clk))
> -		return dev_err_probe(dev, PTR_ERR(clk), "failed to get clock\n");
> +	/* When clock-names is present, there is a separate ext clock. */
> +	if (device_property_present(dev, "clock-names")) {
> +		struct clk *axi_clk;
> +
> +		axi_clk = devm_clk_get_enabled(dev, "axi");
> +		if (IS_ERR(axi_clk))
> +			return dev_err_probe(dev, PTR_ERR(axi_clk),
> +					     "failed to get axi clock\n");
> +
> +		clk = devm_clk_get_enabled(dev, "ext");
> +		if (IS_ERR(clk))
> +			return dev_err_probe(dev, PTR_ERR(clk),
> +					     "failed to get ext clock\n");
> +	} else {
> +		/* Otherwise, a single clock does everything. */
> +		clk = devm_clk_get_enabled(dev, NULL);
> +		if (IS_ERR(clk))
> +			return dev_err_probe(dev, PTR_ERR(clk),
> +					     "failed to get clock\n");
> +	}

Can you achieve the same effect with the (IMHO slightly nicer but
hand-crafted) following patch:

 	ddata = pwmchip_get_drvdata(chip);
 	ddata->regmap = regmap;
 
-	clk = devm_clk_get_enabled(dev, NULL);
-	if (IS_ERR(clk))
-		return dev_err_probe(dev, PTR_ERR(clk), "failed to get clock\n");
+	axi_clk = devm_clk_get_enabled(dev, "axi");
+	if (IS_ERR(axi_clk))
+		return dev_err_probe(dev, PTR_ERR(axi_clk), "failed to get axi clock\n");

+	clk = devm_clk_get_enabled_optional(dev, "ext");
+	if (IS_ERR(clk))
+		return dev_err_probe(dev, PTR_ERR(clk), "failed to get ext clock\n");
+	}

 	ret = devm_clk_rate_exclusive_get(dev, clk);
 	if (ret)

with the only side effect that for machines with a single clk we get
axi_clk == clk and it's enabled twice.

Another upside is that

	clock-names = "axi";
	clocks = <...>;

still works.

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)