| Message ID | 20220204133635.296974-2-enguerrand.de-ribaucourt@savoirfairelinux.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: micrel: add Microchip KSZ 9897 Switch PHY support | expand |
On Fri, Feb 04, 2022 at 02:36:34PM +0100, Enguerrand de Ribaucourt wrote: > Adding Microchip 9897 Phy included in KSZ9897 Switch. > The KSZ9897 shares the same prefix as the KSZ8081. The phy_id_mask was > updated to allow the KSZ9897 to be matched. > > Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com> > --- > drivers/net/phy/micrel.c | 15 +++++++++++++-- > include/linux/micrel_phy.h | 1 + > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c > index 44a24b99c894..9b2047e26449 100644 > --- a/drivers/net/phy/micrel.c > +++ b/drivers/net/phy/micrel.c > @@ -1726,7 +1726,7 @@ static struct phy_driver ksphy_driver[] = { > }, { > .phy_id = PHY_ID_KSZ8081, > .name = "Micrel KSZ8081 or KSZ8091", > - .phy_id_mask = MICREL_PHY_ID_MASK, > + .phy_id_mask = 0x00ffffff, You can probably use PHY_ID_MATCH_EXACT(). > .flags = PHY_POLL_CABLE_TEST, > /* PHY_BASIC_FEATURES */ > .driver_data = &ksz8081_type, > @@ -1869,6 +1869,16 @@ static struct phy_driver ksphy_driver[] = { > .config_init = kszphy_config_init, > .suspend = genphy_suspend, > .resume = genphy_resume, > +}, { > + .phy_id = PHY_ID_KSZ9897, > + .phy_id_mask = 0x00ffffff, Here as well. > + .name = "Microchip KSZ9897", > + /* PHY_BASIC_FEATURES */ > + .config_init = kszphy_config_init, > + .config_aneg = ksz8873mll_config_aneg, > + .read_status = ksz8873mll_read_status, > + .suspend = genphy_suspend, > + .resume = genphy_resume, > } }; > > module_phy_driver(ksphy_driver); > @@ -1888,11 +1898,12 @@ static struct mdio_device_id __maybe_unused micrel_tbl[] = { > { PHY_ID_KSZ8041, MICREL_PHY_ID_MASK }, > { PHY_ID_KSZ8051, MICREL_PHY_ID_MASK }, > { PHY_ID_KSZ8061, MICREL_PHY_ID_MASK }, > - { PHY_ID_KSZ8081, MICREL_PHY_ID_MASK }, > + { PHY_ID_KSZ8081, 0x00ffffff }, And here. > { PHY_ID_KSZ8873MLL, MICREL_PHY_ID_MASK }, > { PHY_ID_KSZ886X, MICREL_PHY_ID_MASK }, > { PHY_ID_LAN8814, MICREL_PHY_ID_MASK }, > { PHY_ID_LAN8804, MICREL_PHY_ID_MASK }, > + { PHY_ID_KSZ9897, 0x00ffffff }, etc. Andrew
----- Original Message ----- > From: "Andrew Lunn" <andrew@lunn.ch> > To: "Enguerrand de Ribaucourt" <enguerrand.de-ribaucourt@savoirfairelinux.com> > Cc: netdev@vger.kernel.org, hkallweit1@gmail.com, linux@armlinux.org.uk > Sent: Friday, February 4, 2022 3:05:05 PM > Subject: Re: [PATCH 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support > On Fri, Feb 04, 2022 at 02:36:34PM +0100, Enguerrand de Ribaucourt wrote: > > Adding Microchip 9897 Phy included in KSZ9897 Switch. > > The KSZ9897 shares the same prefix as the KSZ8081. The phy_id_mask was > > updated to allow the KSZ9897 to be matched. >> Signed-off-by: Enguerrand de Ribaucourt > > <enguerrand.de-ribaucourt@savoirfairelinux.com> > > --- > > drivers/net/phy/micrel.c | 15 +++++++++++++-- > > include/linux/micrel_phy.h | 1 + > > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c > > index 44a24b99c894..9b2047e26449 100644 > > --- a/drivers/net/phy/micrel.c > > +++ b/drivers/net/phy/micrel.c > > @@ -1726,7 +1726,7 @@ static struct phy_driver ksphy_driver[] = { > > }, { > > .phy_id = PHY_ID_KSZ8081, > > .name = "Micrel KSZ8081 or KSZ8091", > > - .phy_id_mask = MICREL_PHY_ID_MASK, > > + .phy_id_mask = 0x00ffffff, > You can probably use PHY_ID_MATCH_EXACT(). Thank you for your feedback! The rest of the driver always uses this style instead of PHY_ID_MATCH_EXACT(). Shouldn't I stick to it for consistency in micrel.c ? Example: .phy_id = PHY_ID_KSZ8031, .phy_id_mask = 0x00ffffff, .name = "Micrel KSZ8031", > > .flags = PHY_POLL_CABLE_TEST, > > /* PHY_BASIC_FEATURES */ > > .driver_data = &ksz8081_type, > > @@ -1869,6 +1869,16 @@ static struct phy_driver ksphy_driver[] = { > > .config_init = kszphy_config_init, > > .suspend = genphy_suspend, > > .resume = genphy_resume, > > +}, { > > + .phy_id = PHY_ID_KSZ9897, > > + .phy_id_mask = 0x00ffffff, > Here as well. > > + .name = "Microchip KSZ9897", > > + /* PHY_BASIC_FEATURES */ > > + .config_init = kszphy_config_init, > > + .config_aneg = ksz8873mll_config_aneg, > > + .read_status = ksz8873mll_read_status, > > + .suspend = genphy_suspend, > > + .resume = genphy_resume, > > } }; > > module_phy_driver(ksphy_driver); >> @@ -1888,11 +1898,12 @@ static struct mdio_device_id __maybe_unused micrel_tbl[] > > = { > > { PHY_ID_KSZ8041, MICREL_PHY_ID_MASK }, > > { PHY_ID_KSZ8051, MICREL_PHY_ID_MASK }, > > { PHY_ID_KSZ8061, MICREL_PHY_ID_MASK }, > > - { PHY_ID_KSZ8081, MICREL_PHY_ID_MASK }, > > + { PHY_ID_KSZ8081, 0x00ffffff }, > And here. > > { PHY_ID_KSZ8873MLL, MICREL_PHY_ID_MASK }, > > { PHY_ID_KSZ886X, MICREL_PHY_ID_MASK }, > > { PHY_ID_LAN8814, MICREL_PHY_ID_MASK }, > > { PHY_ID_LAN8804, MICREL_PHY_ID_MASK }, > > + { PHY_ID_KSZ9897, 0x00ffffff }, > etc. > Andrew
> > You can probably use PHY_ID_MATCH_EXACT(). > > Thank you for your feedback! The rest of the driver always uses > this style instead of PHY_ID_MATCH_EXACT(). You could add another patch converting them. It looks like some could also use PHY_ID_MATCH_MODEL(). Up to you, depending on how much time you want to spend on this. Andrew
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index 44a24b99c894..9b2047e26449 100644 --- a/drivers/net/phy/micrel.c +++ b/drivers/net/phy/micrel.c @@ -1726,7 +1726,7 @@ static struct phy_driver ksphy_driver[] = { }, { .phy_id = PHY_ID_KSZ8081, .name = "Micrel KSZ8081 or KSZ8091", - .phy_id_mask = MICREL_PHY_ID_MASK, + .phy_id_mask = 0x00ffffff, .flags = PHY_POLL_CABLE_TEST, /* PHY_BASIC_FEATURES */ .driver_data = &ksz8081_type, @@ -1869,6 +1869,16 @@ static struct phy_driver ksphy_driver[] = { .config_init = kszphy_config_init, .suspend = genphy_suspend, .resume = genphy_resume, +}, { + .phy_id = PHY_ID_KSZ9897, + .phy_id_mask = 0x00ffffff, + .name = "Microchip KSZ9897", + /* PHY_BASIC_FEATURES */ + .config_init = kszphy_config_init, + .config_aneg = ksz8873mll_config_aneg, + .read_status = ksz8873mll_read_status, + .suspend = genphy_suspend, + .resume = genphy_resume, } }; module_phy_driver(ksphy_driver); @@ -1888,11 +1898,12 @@ static struct mdio_device_id __maybe_unused micrel_tbl[] = { { PHY_ID_KSZ8041, MICREL_PHY_ID_MASK }, { PHY_ID_KSZ8051, MICREL_PHY_ID_MASK }, { PHY_ID_KSZ8061, MICREL_PHY_ID_MASK }, - { PHY_ID_KSZ8081, MICREL_PHY_ID_MASK }, + { PHY_ID_KSZ8081, 0x00ffffff }, { PHY_ID_KSZ8873MLL, MICREL_PHY_ID_MASK }, { PHY_ID_KSZ886X, MICREL_PHY_ID_MASK }, { PHY_ID_LAN8814, MICREL_PHY_ID_MASK }, { PHY_ID_LAN8804, MICREL_PHY_ID_MASK }, + { PHY_ID_KSZ9897, 0x00ffffff }, { } }; diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h index 1f7c33b2f5a3..8d09a732ddf3 100644 --- a/include/linux/micrel_phy.h +++ b/include/linux/micrel_phy.h @@ -36,6 +36,7 @@ #define PHY_ID_KSZ87XX 0x00221550 #define PHY_ID_KSZ9477 0x00221631 +#define PHY_ID_KSZ9897 0x00221561 /* struct phy_device dev_flags definitions */ #define MICREL_PHY_50MHZ_CLK 0x00000001
Adding Microchip 9897 Phy included in KSZ9897 Switch. The KSZ9897 shares the same prefix as the KSZ8081. The phy_id_mask was updated to allow the KSZ9897 to be matched. Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com> --- drivers/net/phy/micrel.c | 15 +++++++++++++-- include/linux/micrel_phy.h | 1 + 2 files changed, 14 insertions(+), 2 deletions(-)