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

[Andrew Lunn <andrew@lunn.ch>]

>  #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