netdev - RE: [PATCH] net: stmmac: fix stmmac_pci_probe failed when CONFIG_HAVE

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] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 19 Sep 2014 09:52:21 +0000
From:	"Kweh, Hock Leong" <hock.leong.kweh@...el.com>
To:	Giuseppe CAVALLARO <peppe.cavallaro@...com>,
	"David S. Miller" <davem@...emloft.net>,
	"rayagond@...avyalabs.com" <rayagond@...avyalabs.com>
CC:	Vince Bridgers <vbridgers2013@...il.com>,
	Chen-Yu Tsai <wens@...e.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	"Ong, Boon Leong" <boon.leong.ong@...el.com>,
	Tobias Klausmann <tobias.johannes.klausmann@....thm.de>
Subject: RE: [PATCH] net: stmmac: fix stmmac_pci_probe failed when
 CONFIG_HAVE_CLK is selected

> -----Original Message-----
> From: Giuseppe CAVALLARO [mailto:peppe.cavallaro@...com]
> Sent: Thursday, September 18, 2014 10:50 PM
> On 9/18/2014 2:34 PM, Kweh Hock Leong wrote:
> > From: "Kweh, Hock Leong" <hock.leong.kweh@...el.com>
> 
> 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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html