netdev - Re: [PATCH v2 net-next 2/3] net: axienet: set mdio clock according to bus-frequency

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: <Y2G8UMKSOadW8/5P@lunn.ch>
Date:   Wed, 2 Nov 2022 01:39:44 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Andy Chiu <andy.chiu@...ive.com>
Cc:     davem@...emloft.net, kuba@...nel.org, michal.simek@...inx.com,
        radhey.shyam.pandey@...inx.com, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        krzysztof.kozlowski+dt@...aro.org, robh+dt@...nel.org,
        pabeni@...hat.com, edumazet@...gle.com, greentime.hu@...ive.com
Subject: Re: [PATCH v2 net-next 2/3] net: axienet: set mdio clock according
 to bus-frequency

>  #include "xilinx_axienet.h"
>  
>  #define MAX_MDIO_FREQ		2500000 /* 2.5 MHz */
> +#define MDIO_CLK_DIV_MASK	0x3f /* bits[5:0] */

XAE_MDIO_MC_CLOCK_DIVIDE_MAX ??

> +static int axienet_mdio_enable(struct axienet_local *lp, struct device_node *np)
>  {
> +	u32 clk_div;
>  	u32 host_clock;
> +	u32 mdio_freq = MAX_MDIO_FREQ;

Reverse Christmas tree.

>  
>  	lp->mii_clk_div = 0;
>  
> @@ -184,6 +188,12 @@ static int axienet_mdio_enable(struct axienet_local *lp)
>  			    host_clock);
>  	}
>  
> +	if (np)
> +		of_property_read_u32(np, "clock-frequency", &mdio_freq);
> +	if (mdio_freq != MAX_MDIO_FREQ)
> +		netdev_info(lp->ndev, "Setting non-standard mdio bus frequency to %u Hz\n",
> +			    mdio_freq);
> +
>  	/* clk_div can be calculated by deriving it from the equation:
>  	 * fMDIO = fHOST / ((1 + clk_div) * 2)
>  	 *
> @@ -209,13 +219,20 @@ static int axienet_mdio_enable(struct axienet_local *lp)
>  	 * "clock-frequency" from the CPU
>  	 */
>  
> -	lp->mii_clk_div = (host_clock / (MAX_MDIO_FREQ * 2)) - 1;
> +	clk_div = (host_clock / (mdio_freq * 2)) - 1;
>  	/* If there is any remainder from the division of
> -	 * fHOST / (MAX_MDIO_FREQ * 2), then we need to add
> +	 * fHOST / (mdio_freq * 2), then we need to add
>  	 * 1 to the clock divisor or we will surely be above 2.5 MHz
>  	 */
> -	if (host_clock % (MAX_MDIO_FREQ * 2))
> -		lp->mii_clk_div++;
> +	if (host_clock % (mdio_freq * 2))
> +		clk_div++;
> +
> +	/* Check for overflow of mii_clk_div */
> +	if (clk_div & ~MDIO_CLK_DIV_MASK) {
> +		netdev_dbg(lp->ndev, "MDIO clock divisor overflow, setting to maximum value\n");
> +		clk_div = MDIO_CLK_DIV_MASK;

It would be better to return -EINVAL. netdev_dbg() is not going to be
seen, and it could be the hardware does not work for no obvious
reason. It is better the driver fails to probe, which is much more
obvious.

	Andrew