[PATCH 01/10] net: stmmac: Enable stmmac main clock when probing hardware

[Maxime Ripard]

Hi,

On Sat, Dec 07, 2013 at 01:29:34AM +0800, Chen-Yu Tsai wrote:
> Signed-off-by: Chen-Yu Tsai <wens at csie.org>
> ---
> 
> Guiseppe previously stated that the "stmmaceth" clock is the
> main clock that drives the IP. The stmmac driver does not
> enable this clock during the probe phase. When the driver is
> built in to the kernel, this is fine because the clock maybe
> on by default, or the boot loader had enabled it.
> 
> If stmmac is built as a module, when the module is loaded,
> the clock may be found unused and disabled by the kernel.

This should be your commit log.

And this is actually not related to the fact that we are building it
as a module or not. You'd face the same issue with a statically built
kernel if the bootloader didn't enable it.

> 
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 24 +++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 8d4ccd3..7da71ed 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2688,10 +2688,17 @@ struct stmmac_priv *stmmac_dvr_probe(struct device *device,
>  	if ((phyaddr >= 0) && (phyaddr <= 31))
>  		priv->plat->phy_addr = phyaddr;
>  
> +	priv->stmmac_clk = clk_get(priv->device, STMMAC_RESOURCE_NAME);

You can probably use devm_clk_get to make the exit path smaller.

> +	if (IS_ERR(priv->stmmac_clk)) {
> +		pr_warn("%s: warning: cannot get CSR clock\n", __func__);

And dev_warn here.

> +		goto error_clk_get;
> +	}
> +	clk_prepare_enable(priv->stmmac_clk);
> +
>  	/* Init MAC and get the capabilities */
>  	ret = stmmac_hw_init(priv);
>  	if (ret)
> -		goto error_free_netdev;
> +		goto error_hw_init;
>  	ndev->netdev_ops = &stmmac_netdev_ops;
>  
> @@ -2729,12 +2736,6 @@ struct stmmac_priv *stmmac_dvr_probe(struct device *device,
>  		goto error_netdev_register;
>  	}
>  
> -	priv->stmmac_clk = clk_get(priv->device, STMMAC_RESOURCE_NAME);
> -	if (IS_ERR(priv->stmmac_clk)) {
> -		pr_warn("%s: warning: cannot get CSR clock\n", __func__);
> -		goto error_clk_get;
> -	}
> -
>  	/* If a specific clk_csr value is passed from the platform
>  	 * this means that the CSR Clock Range selection cannot be
>  	 * changed at run-time and it is fixed. Viceversa the driver'll try to
> @@ -2759,15 +2760,18 @@ struct stmmac_priv *stmmac_dvr_probe(struct device *device,
>  		}
>  	}
>  
> +	clk_disable_unprepare(priv->stmmac_clk);
> +

Hu? Why do you disable the clock? don't you need it afterwards?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131207/2f937ed5/attachment.sig>