[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <547F4E4E.5030906@gmail.com>
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
Powered by blists - more mailing lists