RE: [PATCH] net: stmmac: fix stmmac_pci_probe failed when CONFIG_HAVE_CLK is selected

-----Original Message-----
From: Giuseppe CAVALLARO [mailto:peppe.cavallaro@st.com]
Sent: Thursday, September 18, 2014 10:50 PM
On 9/18/2014 2:34 PM, Kweh Hock Leong wrote:

quoted

From: "Kweh, Hock Leong" <redacted>

Hmm I am not sure this is the right fix. The driver has to fail if the main clock is
not found. Indeed dev_warn has to be changed in dev_err.

Take a look at Documentation/networking/stmmac.txt but I will post some
patch to improve the documentation adding further detail for clocks too.

The the logic behind the code is that the CSR clock will be set at runtime if in
case of priv->plat->clk_csr ==0 or it will be forced to a fixed value if passed
from the platform instead of.
IIRC This was required on some platforms time ago.
For sure the driver is designed to fail in case of no main clock is found.

Peppe

Hi Peppe,

I understand your point from the code below (at file stmmac_main.c line 2784):

/* 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
  * set the MDC clock dynamically according to the csr actual
  * clock input.
  */
if (!priv->plat->clk_csr)
        stmmac_clk_csr_set(priv);
else
        priv->clk_csr = priv->plat->clk_csr;


I did search through the whole stmmac_main.c file and found that only stmmac_clk_csr_set()
function is leveraging the priv->stmmac_clk params for it calculation. By the logic point of
view, I do not need priv->stmmac_clk when I got priv->plat->clk_csr. With this thinking,
I propose this fix as when the probe get priv->plat->clk_csr, it shouldn't fail if priv->stmmac_clk
has the error value.


Regards,
Wilson