linux-kernel - Re: [PATCH] stmmac: CSR clock configuration fix

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]

Message-ID: <db749688-5b91-a7c6-4220-3440a8a689b5@synopsys.com>
Date:   Thu, 22 Dec 2016 12:23:20 +0000
From:   Joao Pinto <Joao.Pinto@...opsys.com>
To:     David Miller <davem@...emloft.net>, <Joao.Pinto@...opsys.com>
CC:     <peppe.cavallaro@...com>, <hock.leong.kweh@...el.com>,
        <niklas.cassel@...s.com>, <pavel@....cz>,
        <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH] stmmac: CSR clock configuration fix


Hi David,

Às 10:15 AM de 12/22/2016, Joao Pinto escreveu:
> Às 6:21 PM de 12/21/2016, David Miller escreveu:
>> From: Joao Pinto <Joao.Pinto@...opsys.com>
>> Date: Tue, 20 Dec 2016 11:21:47 +0000
>>
>>> When testing stmmac with my QoS reference design I checked a problem in the
>>> CSR clock configuration that was impossibilitating the phy discovery, since
>>> every read operation returned 0x0000ffff. This patch fixes the issue.
>>>
>>> Signed-off-by: Joao Pinto <jpinto@...opsys.com>
>>
>> This isn't enough.
>>
>> It looks like various parts of this driver set the mask field
>> differently.
>>
>> dwmac1000_core.c and dwmac100_core.c set the mask to be the low bits.
>>
>> But dwmac4_core.c uses GENMASK(11, 8) which means the mask is a value
>> which is shifted up already.
>>
>> So your patch will break chips driven by dwmac4_core.c.
> 
> I am using a GMAC4 reference design to test the patches. The clock configuration
> as is, does not work, resulting in the phy discovery failure. By applying this
> patch I am able to set the clock value properly.
> 
> I am going to check in the Databook of GMAC4 and older versions in order to
> justify better.

So from the 4.20 Databook:

Bits
11:8 CR parameter
0000: CSR clock = 60-100 MHz; MDC clock = CSR
0001: CSR clock = 100-150 MHz; MDC clock = CSR
0010: CSR clock = 20-35 MHz; MDC clock = CSR
0011: CSR clock = 35-60 MHz; MDC clock = CSR
0100: CSR clock = 150-250 MHz; MDC clock = CSR
0101: CSR clock = 250-300 MHz; MDC clock = CSR
0110, 0111: Reserved

For example, if you want configure the CSR clock to 250-300MHZ (my case), you
will set the parameter CR to 0x5. The current mechanism simply messes the value.
With this fix, the 0x5 is shifted to 11:8 like the databook specifies and
applies a GENMASK(11:8) on top, causing the reset of every bit outside the 11:8
which is an assurance.

For older cores like 4.10 and 4.00 the logic is the same. The information was
confirmed by R&D.

Thanks.

> 
>>
>> In order for your change to be correct you must consolidate all of
>> these various pieces to use the same convention.
>>
> 
> Thanks.
>