linux-kernel - Re: [PATCH net] net: mdio: mdio-bcm-unimac: Correct rate fallback logic

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] [day] [month] [year] [list]

Message-ID: <20250730143300.GI1877762@horms.kernel.org>
Date: Wed, 30 Jul 2025 15:33:00 +0100
From: Simon Horman <horms@...nel.org>
To: Florian Fainelli <florian.fainelli@...adcom.com>
Cc: netdev@...r.kernel.org, Doug Berger <opendmb@...il.com>,
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
	Andrew Lunn <andrew@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Jacob Keller <jacob.e.keller@...el.com>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net] net: mdio: mdio-bcm-unimac: Correct rate fallback
 logic

On Tue, Jul 29, 2025 at 02:31:48PM -0700, Florian Fainelli wrote:
> In case the rate for the parent clock is zero, make sure that we still
> fallback to using a fixed rate for the divider calculation, otherwise we
> simply ignore the desired MDIO bus clock frequency which can prevent us
> from interfacing with Ethernet PHYs properly.
> 
> Fixes: ee975351cf0c ("net: mdio: mdio-bcm-unimac: Manage clock around I/O accesses")
> Signed-off-by: Florian Fainelli <florian.fainelli@...adcom.com>
> ---
>  drivers/net/mdio/mdio-bcm-unimac.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/mdio/mdio-bcm-unimac.c b/drivers/net/mdio/mdio-bcm-unimac.c
> index b6e30bdf5325..9c0a11316cfd 100644
> --- a/drivers/net/mdio/mdio-bcm-unimac.c
> +++ b/drivers/net/mdio/mdio-bcm-unimac.c
> @@ -209,10 +209,9 @@ static int unimac_mdio_clk_set(struct unimac_mdio_priv *priv)
>  	if (ret)
>  		return ret;
>  
> -	if (!priv->clk)
> +	rate = clk_get_rate(priv->clk);
> +	if (!priv->clk || !rate)

If priv->clk is NULL then drivers/clk/clk.c:clk_get_rate()
(are there other relevant implementations?) will return 0.
So I think the condition above is tautological, and could be expressed as:

	if (!rate)

>  		rate = 250000000;
> -	else
> -		rate = clk_get_rate(priv->clk);
>  
>  	div = (rate / (2 * priv->clk_freq)) - 1;
>  	if (div & ~MDIO_CLK_DIV_MASK) {
> -- 
> 2.34.1
> 
>