linux-kernel - 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:	Tue, 23 Sep 2014 08:10:09 +0200
From:	Giuseppe CAVALLARO <peppe.cavallaro@...com>
To:	"Kweh, Hock Leong" <hock.leong.kweh@...el.com>,
	David Miller <davem@...emloft.net>
Cc:	"rayagond@...avyalabs.com" <rayagond@...avyalabs.com>,
	"vbridgers2013@...il.com" <vbridgers2013@...il.com>,
	"wens@...e.org" <wens@...e.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Ong, Boon Leong" <boon.leong.ong@...el.com>,
	"tobias.johannes.klausmann@....thm.de" 
	<tobias.johannes.klausmann@....thm.de>
Subject: Re: [PATCH] net: stmmac: fix stmmac_pci_probe failed when CONFIG_HAVE_CLK
 is selected

On 9/23/2014 3:16 AM, Kweh, Hock Leong wrote:
>> -----Original Message-----
>> From: David Miller [mailto:davem@...emloft.net]
>> Sent: Tuesday, September 23, 2014 2:19 AM
>> From: Kweh Hock Leong <hock.leong.kweh@...el.com>
>> Date: Thu, 18 Sep 2014 20:34:10 +0800
>>
>> Giuseppe, Kweh, where are we with this patch?
>
> We are discussing whether this is the correct fix to the issue. Below is the discussion log:
>
>>> 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.
>
> Hi Peppe,
>
> Are you trying to tell that if there is a fix CSR clock value, but driver cannot obtain the clk from devm_clk_get()
> (even it is not in use), the driver should fail the probe?
>
> You have a case with this condition:
> HW allow SW to discover the STMMAC controller but HW/SW configures not to supply the CLOCK to STMMAC controller?
>
> My understanding to these priv->plat->clk_csr and priv->stmmac_clk is that 1st one is fixed and 2nd one is dynamic.
> When fixed value occurs, dynamic one would not be use. Do I understand this correctly?

the logic is: the priv->stmmac_clk must be always provided from the
platform then we have two cases:

1) if priv->plat->clk_csr is also passed then it will be adopt in the
    mdio functions to program the Reg4[5:2]
    This was required in the past IIRC on SPEAr platforms.

2) if priv->plat->clk_csr is not passed from the platform then the
    priv->clk_csr will be set according to the priv->stmmac_clk
    and always used in the mdio part.

So IIUC now you are asking for not passing the priv->stmmac_clk
and warning this event w/o failing. Why you cannot pass this clock?

peppe


>
> Thanks.
>
>
> Regards,
> Wilson
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/