From mboxrd@z Thu Jan 1 00:00:00 1970 From: Giuseppe CAVALLARO Subject: Re: [PATCH] net: stmmac: fix stmmac_pci_probe failed when CONFIG_HAVE_CLK is selected Date: Thu, 18 Sep 2014 16:49:58 +0200 Message-ID: <541AF116.9010004@st.com> References: <1411043650-31712-1-git-send-email-hock.leong.kweh@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Cc: Vince Bridgers , Chen-Yu Tsai , , LKML , Ong Boon Leong , Tobias Klausmann To: Kweh Hock Leong , "David S. Miller" , Return-path: In-Reply-To: <1411043650-31712-1-git-send-email-hock.leong.kweh@intel.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 9/18/2014 2:34 PM, Kweh Hock Leong wrote: > From: "Kweh, Hock Leong" > > When the CONFIG_HAVE_CLK is selected for the system, the stmmac_pci_probe > will fail with dmesg: > [ 2.167225] stmmaceth 0000:00:14.6: enabling device (0000 -> 0002) > [ 2.178267] stmmaceth 0000:00:14.6: enabling bus mastering > [ 2.178436] stmmaceth 0000:00:14.6: irq 24 for MSI/MSI-X > [ 2.178703] stmmaceth 0000:00:14.6: stmmac_dvr_probe: warning: cannot > get CSR clock > [ 2.186503] stmmac_pci_probe: main driver probe failed > [ 2.194003] stmmaceth 0000:00:14.6: disabling bus mastering > [ 2.196473] stmmaceth: probe of 0000:00:14.6 failed with error -2 > > This patch fix the issue by breaking the dependency to devm_clk_get() > as the CSR clock can be obtained at priv->plat->clk_csr from pci driver. > > Reported-by: Tobias Klausmann > Signed-off-by: Kweh, Hock Leong > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 08addd6..ea3859a 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -2714,10 +2714,15 @@ struct stmmac_priv *stmmac_dvr_probe(struct device *device, > > priv->stmmac_clk = devm_clk_get(priv->device, STMMAC_RESOURCE_NAME); > if (IS_ERR(priv->stmmac_clk)) { > - dev_warn(priv->device, "%s: warning: cannot get CSR clock\n", 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 > - __func__); > - ret = PTR_ERR(priv->stmmac_clk); > - goto error_clk_get; > + if (!priv->plat->clk_csr) { > + dev_warn(priv->device, > + "%s: warning: cannot get CSR clock\n", > + __func__); > + ret = PTR_ERR(priv->stmmac_clk); > + goto error_clk_get; > + } else { > + priv->stmmac_clk = NULL; > + } > } > clk_prepare_enable(priv->stmmac_clk); > >