linux-kernel - Re: [PATCH v2 2/2] serial: 8250_of: add support for an optional bus 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: <20250409214345-GYA19066@gentoo>
Date: Wed, 9 Apr 2025 21:43:45 +0000
From: Yixun Lan <dlan@...too.org>
To: Alex Elder <elder@...cstar.com>
Cc: gregkh@...uxfoundation.org, jirislaby@...nel.org, robh@...nel.org,
	krzk+dt@...nel.org, conor+dt@...nel.org,
	benjamin.larsson@...exis.eu, bastien.curutchet@...tlin.com,
	andriy.shevchenko@...ux.intel.com, u.kleine-koenig@...libre.com,
	lkundrak@...sk, devicetree@...r.kernel.org,
	linux-serial@...r.kernel.org, spacemit@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] serial: 8250_of: add support for an optional bus
 clock

Hi Alex,

On 14:22 Wed 09 Apr     , Alex Elder wrote:
> The SpacemiT UART requires a bus clock to be enabled, in addition to
> it's "normal" core clock.  Look up the optional bus clock by name,
> and if that's found, look up the core clock using the name "core".
> 
> Supplying a bus clock is optional.  If no bus clock is needed, the
> the first/only clock is used for the core clock.
> 
> Signed-off-by: Alex Elder <elder@...cstar.com>
> ---
> v2: Update logic to more check for the optional bus clock first
> 
>  drivers/tty/serial/8250/8250_of.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
> index 11c860ea80f60..a90a5462aa72a 100644
> --- a/drivers/tty/serial/8250/8250_of.c
> +++ b/drivers/tty/serial/8250/8250_of.c
> @@ -123,7 +123,16 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
>  
>  	/* Get clk rate through clk driver if present */
>  	if (!port->uartclk) {
> -		info->clk = devm_clk_get_enabled(dev, NULL);
> +		struct clk *bus_clk;
we also need to handle clk in suspend/resume procedure, so
I think you need to put bus_clk inside struct of_serial_info..

> +
> +		bus_clk = devm_clk_get_optional_enabled(dev, "bus");
for the 'optional', we can interpret it's optional for other vendor 
UART, but a must required clk for SpacemiT's k1 UART controller

would it better to guard this inside a compatible test or even introduce
a flag in compatible data?

	if (of_device_is_compatible(pdev->dev.of_node, "spacemit,k1-uart")) {
		bus_clk = devm_clk_get_enabled(dev, "bus");
		..
	}

}
> +		if (IS_ERR(bus_clk)) {
> +			ret = dev_err_probe(dev, PTR_ERR(bus_clk), "failed to get bus clock\n");
> +			goto err_pmruntime;
> +		}
> +
> +		/* If the bus clock is required, core clock must be named */
> +		info->clk = devm_clk_get_enabled(dev, bus_clk ? "core" : NULL);
>  		if (IS_ERR(info->clk)) {
>  			ret = dev_err_probe(dev, PTR_ERR(info->clk), "failed to get clock\n");
>  			goto err_pmruntime;
> -- 
> 2.45.2
> 

-- 
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55