linux-kernel - Re: [PATCH v3 3/3] serial: 8250_of: manage bus clock in suspend/resume

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: <Z_ltyAO-OBzl0adV@surfacebook.localdomain>
Date: Fri, 11 Apr 2025 22:30:16 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Alex Elder <elder@...cstar.com>
Cc: gregkh@...uxfoundation.org, jirislaby@...nel.org, robh@...nel.org,
	krzk+dt@...nel.org, conor+dt@...nel.org, dlan@...too.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 v3 3/3] serial: 8250_of: manage bus clock in
 suspend/resume

Fri, Apr 11, 2025 at 10:44:18AM -0500, Alex Elder kirjoitti:
> Save the bus clock pointer in the of_serial_info structure, and use
> that to disable the bus clock on suspend and re-enable it on resume.

...

>  	if (!port->uartclk) {
> -		struct clk *bus_clk;
> -
> -		bus_clk = devm_clk_get_optional_enabled(dev, "bus");
> -		if (IS_ERR(bus_clk)) {
> -			ret = dev_err_probe(dev, PTR_ERR(bus_clk), "failed to get bus clock\n");
> +		info->bus_clk = devm_clk_get_optional_enabled(dev, "bus");
> +		if (IS_ERR(info->bus_clk)) {
> +			ret = dev_err_probe(dev, PTR_ERR(info->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);
> +		info->clk = devm_clk_get_enabled(dev, info->bus_clk ? "core" : NULL);
>  		if (IS_ERR(info->clk)) {
>  			ret = dev_err_probe(dev, PTR_ERR(info->clk), "failed to get clock\n");

While the first patch against this file looks okay now, this one inherits the
same problem (seems like not enought thinking about the code representation).

Instead of rewritting half of the lines you just introduced (which is also a
bad practice), add a one-liner that assigns a field to the local variable.

>  			goto err_pmruntime;


-- 
With Best Regards,
Andy Shevchenko