From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli Subject: Re: [PATCH] net: ethernet: ibm-emac: Add 5482 PHY init for OpenBlocks 600 Date: Fri, 1 Sep 2017 17:35:37 -0700 Message-ID: <816adc32-9df6-3433-b30c-79490a6328d1@gmail.com> References: <1504241089.4974.67.camel@kernel.crashing.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit To: Benjamin Herrenschmidt , netdev@vger.kernel.org Return-path: Received: from mail-qk0-f193.google.com ([209.85.220.193]:36734 "EHLO mail-qk0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752642AbdIBAfo (ORCPT ); Fri, 1 Sep 2017 20:35:44 -0400 Received: by mail-qk0-f193.google.com with SMTP id l65so1237511qkc.3 for ; Fri, 01 Sep 2017 17:35:43 -0700 (PDT) In-Reply-To: <1504241089.4974.67.camel@kernel.crashing.org> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 08/31/2017 09:44 PM, Benjamin Herrenschmidt wrote: > The vendor patches initialize those registers to get the > PHY working properly. > > Sadly I don't have that PHY spec and whatever Broadcom PHY > code we already have don't seem to document these two shadow > registers (unless I miscalculated the address) so I'm keeping > this as "vendor magic for that board". The vendor has long > abandoned that product, but I find it handy to test ppc405 > kernels and so would like to keep it alive upstream :-) > > Signed-off-by: Benjamin Herrenschmidt > --- > > Note: Ideally, the whole driver should switch over to the > generic PHY layer. However this is a much bigger undertaking > which requires access to a bunch of HW to test, and for which > I have neither the time nor the HW available these days. Yes it sure does and the function names are so close, it is almost irresistible not to do it. > > (Some of the HW could prove hard to find ...) > --- > drivers/net/ethernet/ibm/emac/phy.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/net/ethernet/ibm/emac/phy.c b/drivers/net/ethernet/ibm/emac/phy.c > index 35865d05fccd..daa10de542fb 100644 > --- a/drivers/net/ethernet/ibm/emac/phy.c > +++ b/drivers/net/ethernet/ibm/emac/phy.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > > #include "emac.h" > #include "phy.h" > @@ -363,6 +364,34 @@ static struct mii_phy_def bcm5248_phy_def = { > .ops = &generic_phy_ops > }; > > +static int bcm5482_init(struct mii_phy *phy) > +{ > + if (!of_machine_is_compatible("plathome,obs600")) > + return 0; You can probably include brcmphy.h and pull the definition for at least 0x1c: MII_BCM54XX_SHD > + > + /* Magic inits from vendor original patches */ > + phy_write(phy, 0x1c, 0xa410); What you are doing here is write to shadow register 9 (9 << 10) which is the LED control register, and making the activity LED be driven on activity/link as opposed to just activity. So this can probably be written as: phy_write(phy, MII_BCM54XX_SHD, MII_BCM54XX_SHD_WRITE | MII_BCM54XX_SHD_VAL(9) | MII_BCM54XX_SHD_DATA(BIT(4)); > + phy_write(phy, 0x1c, 0x8804); And here you are writing to the spare control 1 register and setting bit 2 (which appears reserved but this is not clear) which would be enabling the activity LED for 10BaseT or no link which can be written as: phy_write(phy, MII_BCM54XX_SHD, MII_BCM54XX_SHD_WRITE | MII_BCM54XX_SHD_VAL(2) | MII_BCM4XX_SHD_DATA(BIT(2)); So basically you are touching registers that only affect LED configuration and should not be doing anything else... > + > + return 0; > +} > + > +static const struct mii_phy_ops bcm5482_phy_ops = { > + .init = bcm5482_init, > + .setup_aneg = genmii_setup_aneg, > + .setup_forced = genmii_setup_forced, > + .poll_link = genmii_poll_link, > + .read_link = genmii_read_link > +}; > + > +static struct mii_phy_def bcm5482_phy_def = { > + > + .phy_id = 0x0143bcb0, > + .phy_id_mask = 0x0ffffff0, > + .name = "BCM5482 Gigabit Ethernet", > + .ops = &bcm5482_phy_ops > +}; > + > static int m88e1111_init(struct mii_phy *phy) > { > pr_debug("%s: Marvell 88E1111 Ethernet\n", __func__); > @@ -499,6 +528,7 @@ static struct mii_phy_def *mii_phy_table[] = { > &et1011c_phy_def, > &cis8201_phy_def, > &bcm5248_phy_def, > + &bcm5482_phy_def, > &m88e1111_phy_def, > &m88e1112_phy_def, > &ar8035_phy_def, > -- Florian