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 <b...@kernel.crashing.org>
> ---
>
> 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 <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;
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
