net: ethernet: ibm-emac: Add 5482 PHY init for OpenBlocks 600
Subsystems (2)
PATCH 1/1 net: ethernet: ibm-emac: Add 5482 PHY init for OpenBlocks 600
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 :-)
---
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.
(Some of the HW could prove hard to find ...)
--- drivers/net/ethernet/ibm/emac/phy.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
drivers/net/ethernet/ibm/emac/phy.c +30 -0
@@ -24,6 +24,7 @@#include <linux/mii.h> #include <linux/ethtool.h> #include <linux/delay.h> +#include <linux/of.h> #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 ; + +/* Magic inits from vendor original patches */ +phy_write (phy,0x1c ,0xa410 ); +phy_write (phy,0x1c ,0x8804 ); + +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,
Signature · 1 line
2.13.5
Quoted 10 lines · Benjamin Herrenschmidt
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 :-)
Quoted 7 lines
Signed-off-by Benjamin Herrenschmidt <[email protected]>---
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.
Quoted 19 lines
(Some of the HW could prove hard to find ...)
--- drivers/net/ethernet/ibm/emac/phy.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)drivers/net/ethernet/ibm/emac/phy.c +1
-0@@ -24,6 +24,7 @@#include <linux/mii.h> #include <linux/ethtool.h> #include <linux/delay.h> +#include <linux/of.h> #include "emac.h" #include "phy.h"
drivers/net/ethernet/ibm/emac/phy.c static struct mii_phy_def bcm5248_phy_def = { · line 370
Quoted 7 lines
drivers/net/ethernet/ibm/emac/phy.c +4
-0@@ -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
drivers/net/ethernet/ibm/emac/phy.c static struct mii_phy_def bcm5248_phy_def = { · line 373
Quoted 3 lines
drivers/net/ethernet/ibm/emac/phy.c +3
-0+ +/* 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));
drivers/net/ethernet/ibm/emac/phy.c static struct mii_phy_def bcm5248_phy_def = { · line 374
Quoted 1 line
drivers/net/ethernet/ibm/emac/phy.c +1
-0+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...
Quoted 32 lines
drivers/net/ethernet/ibm/emac/phy.c +21
-0+ +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,
Signature · 1 line
Florian
Quoted 14 lines · Florian Fainelli
Quoted 13 lines · Benjamin Herrenschmidt
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 <[email protected]>---
Quoted 7 lines
Quoted 4 lines
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.
I think there's some common ancestry :-)
That said, I'm weary of doing it without proper testing, especially those old cell blades which I'm not sure I still have a functional one, and whatever is using gpcs...
Cheers,
Ben.
Quoted 23 lines
Quoted 23 lines
(Some of the HW could prove hard to find ...)
--- drivers/net/ethernet/ibm/emac/phy.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)drivers/net/ethernet/ibm/emac/phy.c +1
-0@@ -24,6 +24,7 @@#include <linux/mii.h> #include <linux/ethtool.h> #include <linux/delay.h> +#include <linux/of.h> #include "emac.h" #include "phy.h" @@ -363,6 +364,34 @@ static struct mii_phy_def bcm5248_phy_def = { .ops = &generic_phy_ops };
Quoted 7 lines
drivers/net/ethernet/ibm/emac/phy.c static struct mii_phy_def bcm5248_phy_def = { · line 370
Quoted 4 lines
drivers/net/ethernet/ibm/emac/phy.c +4
-0+static int bcm5482_init (struct mii_phy *phy) +{ +if (!of_machine_is_compatible ("plathome,obs600" )) +return 0 ;You can probably include
brcmphy.hand pull the definition for at least0x1c:MII_BCM54XX_SHD
Yup.
Quoted 8 lines
drivers/net/ethernet/ibm/emac/phy.c static struct mii_phy_def bcm5248_phy_def = { · line 373
Quoted 3 lines
drivers/net/ethernet/ibm/emac/phy.c +3
-0+ +/* 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:
Ok so I really don't *need* that in fact.
Quoted 9 lines
phy_write(phy, MII_BCM54XX_SHD, MII_BCM54XX_SHD_WRITE | MII_BCM54XX_SHD_VAL(9) | MII_BCM54XX_SHD_DATA(BIT(4));drivers/net/ethernet/ibm/emac/phy.c static struct mii_phy_def bcm5248_phy_def = { · line 374
Quoted 1 line
drivers/net/ethernet/ibm/emac/phy.c +1
-0+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:
Quoted 5 lines
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...
I wonder if I need to bother at all then. I was worried it was related to actual function of the device, but if it's just LEDs, I think I may as well just drop it.
Quoted 34 lines
Quoted 32 lines
drivers/net/ethernet/ibm/emac/phy.c +21
-0+ +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,