netdev - Re: [PATCH net-next 1/2] net: bcmgenet: add support for new GENET PHY revision scheme

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:	Wed, 03 Dec 2014 09:54:22 -0800
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
	netdev@...r.kernel.org
CC:	davem@...emloft.net
Subject: Re: [PATCH net-next 1/2] net: bcmgenet: add support for new GENET
 PHY revision scheme

On 03/12/14 03:23, Sergei Shtylyov wrote:
> Hello.
> 
> On 12/3/2014 2:28 AM, Florian Fainelli wrote:
> 
>> Starting with GPHY revision G0, the GENET register layout has changed to
>> use the same numbering scheme as the Starfighter 2 switch. This means
>> that GPHY major revision is in bits 15:12, minor in bits 11:8 and patch
>> level is in bits 7:4.
> 
>> Introduce a small heuristic which checks for the old scheme first, tests
>> for the new scheme and finally attempts to catch reserved values and
>> aborts.
> 
>> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
>> ---
>>   drivers/net/ethernet/broadcom/genet/bcmgenet.c | 24
>> +++++++++++++++++++++++-
>>   1 file changed, 23 insertions(+), 1 deletion(-)
> 
>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> index f2fadb053d52..23e283174c4e 100644
>> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> [...]
>> @@ -2551,8 +2552,29 @@ static void bcmgenet_set_hw_params(struct
>> bcmgenet_priv *priv)
>>        * to pass this information to the PHY driver. The PHY driver
>> expects
>>        * to find the PHY major revision in bits 15:8 while the GENET
>> register
>>        * stores that information in bits 7:0, account for that.
>> +     *
>> +     * On newer chips, starting with PHY revision G0, a new scheme is
>> +     * deployed similar to the Starfighter 2 switch with GPHY major
>> +     * revision in bits 15:8 and patch level in bits 7:0. Major
>> revision 0
>> +     * is reserved as well as special value 0x01ff, we have a small
>> +     * heuristic to check for the new GPHY revision and re-arrange
>> things
>> +     * so the GPHY driver is happy.
>>        */
>> -    priv->gphy_rev = (reg & 0xffff) << 8;
>> +    gphy_rev = (reg & 0xffff);
> 
>    Parens not needed anymore.
> 
>> +
>> +    /* This the good old scheme, just GPHY major, no minor nor patch */
> 
>    Missing "is" after "This"?

Alright, I will resubmit...

> 
>> +    if ((gphy_rev & 0xf0) != 0)
>> +        priv->gphy_rev = gphy_rev << 8;
>> +
>> +    /* This is the new scheme, GPHY major rolls over with 0x10 = rev
>> G0 */
>> +    else if ((gphy_rev & 0xff00) != 0)
>> +        priv->gphy_rev = gphy_rev;
>> +
>> +    /* This is reserved so should require special treatment */
>> +    else if (gphy_rev == 0 || gphy_rev == 0x01ff) {
>> +        pr_warn("Invalid GPHY revision detected: 0x%04x\n", gphy_rev);
>> +        return;
>> +    }
> 
>    Hm, {} are needed on all *if* branches.

checkpatch.pl did not complain about that.

> 
> [...]
> 
> WBR, Sergei
> 

--
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