| Message ID | 20200428210854.28088-4-michael@walle.cc |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next,1/4] net: phy: bcm54140: use genphy_soft_reset() | expand |
On Tue, Apr 28, 2020 at 11:08:54PM +0200, Michael Walle wrote: > This PHY have to PHY IDs depending on its mode. Adjust the mask so that > it includes both IDs. Hi Michael I don't have a strong opinion, but maybe list it as two different PHYs? I do sometimes grep for PHY IDs, and that would not work due to the odd mask. Andrew
Hi Andrew, Am 2020-04-28 23:29, schrieb Andrew Lunn: > On Tue, Apr 28, 2020 at 11:08:54PM +0200, Michael Walle wrote: >> This PHY have to PHY IDs depending on its mode. Adjust the mask so >> that >> it includes both IDs. > > Hi Michael > > I don't have a strong opinion, but maybe list it as two different > PHYs? I do sometimes grep for PHY IDs, and that would not work due to > the odd mask. Me neither. I just looked odd to have actually the same PHY listed twice with just another id. That makes me wonder if it is possible to have the same PHY driver name twice. IIRC it is at leased used somewhere in the sysfs. If that is true, I'd prefer to just have one PHY "BCM54140" instead of a "BCM54140 (QSGMII)" and "BCM54140 (4x SGMII)". Because it is actually the same PHY but only another interface towards the MAC is used. -michael
On 4/28/20 2:08 PM, Michael Walle wrote: > This PHY have to PHY IDs depending on its mode. Adjust the mask so that > it includes both IDs. > > Also move the PHY_ID_BCM54140 out of the brcmphy.h into the driver to > have the mask and the ID in one place. > > Signed-off-by: Michael Walle <michael@walle.cc> > --- [snip] Just one request below. > diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h > index 8be150e69c7c..897b69309964 100644 > --- a/include/linux/brcmphy.h > +++ b/include/linux/brcmphy.h > @@ -25,7 +25,6 @@ > #define PHY_ID_BCM5461 0x002060c0 > #define PHY_ID_BCM54612E 0x03625e60 > #define PHY_ID_BCM54616S 0x03625d10 > -#define PHY_ID_BCM54140 0xae025019 I would prefer if you could leave the entry here since this the file that centralizes all Broadcom PHY OUIs. Thanks!
diff --git a/drivers/net/phy/bcm54140.c b/drivers/net/phy/bcm54140.c index d73cbddbc69b..6c54f067c7fb 100644 --- a/drivers/net/phy/bcm54140.c +++ b/drivers/net/phy/bcm54140.c @@ -115,6 +115,14 @@ #define BCM54140_HWMON_IN_ALARM_BIT(ch) ((ch) ? BCM54140_RDB_MON_ISR_3V3 \ : BCM54140_RDB_MON_ISR_1V0) +/* This PHY has two different PHY IDs depening on its MODE_SEL pin. This + * pin choses between 4x SGMII and QSGMII mode: + * AE02_5009 4x SGMII + * AE02_5019 QSGMII + */ +#define PHY_ID_BCM54140 0xae025009 +#define BCM54140_PHY_ID_MASK 0xffffffe8 + #define BCM54140_PHY_ID_REV(phy_id) ((phy_id) & 0x7) #define BCM54140_REV_B0 1 @@ -857,7 +865,7 @@ static int bcm54140_set_tunable(struct phy_device *phydev, static struct phy_driver bcm54140_drivers[] = { { .phy_id = PHY_ID_BCM54140, - .phy_id_mask = 0xfffffff8, + .phy_id_mask = BCM54140_PHY_ID_MASK, .name = "Broadcom BCM54140", .features = PHY_GBIT_FEATURES, .config_init = bcm54140_config_init, @@ -875,7 +883,7 @@ static struct phy_driver bcm54140_drivers[] = { module_phy_driver(bcm54140_drivers); static struct mdio_device_id __maybe_unused bcm54140_tbl[] = { - { PHY_ID_BCM54140, 0xfffffff8 }, + { PHY_ID_BCM54140, BCM54140_PHY_ID_MASK }, { } }; diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h index 8be150e69c7c..897b69309964 100644 --- a/include/linux/brcmphy.h +++ b/include/linux/brcmphy.h @@ -25,7 +25,6 @@ #define PHY_ID_BCM5461 0x002060c0 #define PHY_ID_BCM54612E 0x03625e60 #define PHY_ID_BCM54616S 0x03625d10 -#define PHY_ID_BCM54140 0xae025019 #define PHY_ID_BCM57780 0x03625d90 #define PHY_ID_BCM89610 0x03625cd0
This PHY have to PHY IDs depending on its mode. Adjust the mask so that it includes both IDs. Also move the PHY_ID_BCM54140 out of the brcmphy.h into the driver to have the mask and the ID in one place. Signed-off-by: Michael Walle <michael@walle.cc> --- drivers/net/phy/bcm54140.c | 12 ++++++++++-- include/linux/brcmphy.h | 1 - 2 files changed, 10 insertions(+), 3 deletions(-)