| Message ID | 20240531142430.678198-2-enguerrand.de-ribaucourt@savoirfairelinux.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add Microchip KSZ 9897 Switch CPU PHY + Errata | expand |
> -----Original Message----- > From: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com> > Sent: Friday, May 31, 2024 7:24 AM > To: netdev@vger.kernel.org > Cc: andrew@lunn.ch; hkallweit1@gmail.com; linux@armlinux.org.uk; Woojung Huh - > C21699 <Woojung.Huh@microchip.com>; UNGLinuxDriver > <UNGLinuxDriver@microchip.com>; Enguerrand de Ribaucourt <enguerrand.de- > ribaucourt@savoirfairelinux.com> > Subject: [PATCH net v4 1/5] net: phy: micrel: add Microchip KSZ 9897 Switch PHY > support > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content > is safe > > There is a DSA driver for microchip,ksz9897 which can be controlled > through SPI or I2C. This patch adds support for it's CPU ports PHYs to > also allow network access to the switch's CPU port. > > The CPU ports PHYs of the KSZ9897 are not documented in the datasheet. > They weirdly use the same PHY ID as the KSZ8081, which is a different > PHY and that driver isn't compatible with KSZ9897. Before this patch, > the KSZ8081 driver was used for the CPU ports of the KSZ9897 but the > link would never come up. > > A new driver for the KSZ9897 is added, based on the compatible KSZ87XX. > I could not test if Gigabit Ethernet works, but the link comes up and > can successfully allow packets to be sent and received with DSA tags. > > To resolve the KSZ8081/KSZ9897 phy_id conflicts, I could not find any > stable register to distinguish them. Instead of a match_phy_device() , > I've declared a virtual phy_id with the highest value in Microchip's OUI > range. > > Example usage in the device tree: > compatible = "ethernet-phy-id0022.17ff"; > > A discussion to find better alternatives had been opened with the > Microchip team, with no response yet. > > See https://lore.kernel.org/all/20220207174532.362781-1-enguerrand.de- > ribaucourt@savoirfairelinux.com/ > > Fixes: b987e98e50ab ("dsa: add DSA switch driver for Microchip KSZ9477") > Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de- > ribaucourt@savoirfairelinux.com> > --- > v4: > - rebase on net/main > - add Fixes tag > - use pseudo phy_id instead of of_tree search > v3: https://lore.kernel.org/all/20240530102436.226189-2-enguerrand.de- > ribaucourt@savoirfairelinux.com/ > --- > drivers/net/phy/micrel.c | 14 +++++++++++++- > include/linux/micrel_phy.h | 4 ++++ > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c > index 2b8f8b7f1517..8a6dfaceeab3 100644 > --- a/drivers/net/phy/micrel.c > +++ b/drivers/net/phy/micrel.c > @@ -16,7 +16,7 @@ > * ksz8081, ksz8091, > * ksz8061, > * Switch : ksz8873, ksz886x > - * ksz9477, lan8804 > + * ksz9477, ksz9897, lan8804 > */ > > #include <linux/bitfield.h> > @@ -5495,6 +5495,17 @@ static struct phy_driver ksphy_driver[] = { > .suspend = genphy_suspend, > .resume = genphy_resume, > .get_features = ksz9477_get_features, > +}, { > + .phy_id = PHY_ID_KSZ9897, > + .phy_id_mask = MICREL_PHY_ID_MASK, > + .name = "Microchip KSZ9897 Switch", > + /* PHY_BASIC_FEATURES */ > + .config_init = kszphy_config_init, > + .config_aneg = ksz8873mll_config_aneg, > + .read_status = ksz8873mll_read_status, > + /* No suspend/resume callbacks because of errata DS00002330D: > + * Toggling PHY Powerdown can cause errors or link failures in adjacent PHYs > + */ > } }; > > module_phy_driver(ksphy_driver); > @@ -5520,6 +5531,7 @@ static struct mdio_device_id __maybe_unused micrel_tbl[] > = { > { PHY_ID_LAN8814, MICREL_PHY_ID_MASK }, > { PHY_ID_LAN8804, MICREL_PHY_ID_MASK }, > { PHY_ID_LAN8841, MICREL_PHY_ID_MASK }, > + { PHY_ID_KSZ9897, MICREL_PHY_ID_MASK }, > { } > }; > > diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h > index 591bf5b5e8dc..81cc16dc2ddf 100644 > --- a/include/linux/micrel_phy.h > +++ b/include/linux/micrel_phy.h > @@ -39,6 +39,10 @@ > #define PHY_ID_KSZ87XX 0x00221550 > > #define PHY_ID_KSZ9477 0x00221631 > +/* Pseudo ID to specify in compatible field of device tree. > + * Otherwise the device reports the same ID as KSZ8081 on CPU ports. > + */ > +#define PHY_ID_KSZ9897 0x002217ff > I am curious about this KSZ9897 device. Can you point out its product page on Microchip website? KSZ9897 is typically referred to the KSZ9897 switch family, which contains KSZ9897, KSZ9896, KSZ9567, KSZ8567, KSZ9477 and some others. I am not aware that KSZ9897 has MDIO access. The switch is only accessed through I2C and SPI and proprietary IBA. It seems the only function is just to report link so a fixed PHY should be adequate in this situation. MDIO only mode is present in KSZ8863/KSZ8873 switches. I do not know useful to use such mode in KSZ9897.
On Fri, May 31, 2024 at 02:24:26PM +0000, Enguerrand de Ribaucourt wrote: > There is a DSA driver for microchip,ksz9897 which can be controlled > through SPI or I2C. This patch adds support for it's CPU ports PHYs to > also allow network access to the switch's CPU port. > > The CPU ports PHYs of the KSZ9897 are not documented in the datasheet. > They weirdly use the same PHY ID as the KSZ8081, which is a different > PHY and that driver isn't compatible with KSZ9897. Before this patch, > the KSZ8081 driver was used for the CPU ports of the KSZ9897 but the > link would never come up. > > A new driver for the KSZ9897 is added, based on the compatible KSZ87XX. > I could not test if Gigabit Ethernet works, but the link comes up and > can successfully allow packets to be sent and received with DSA tags. > > To resolve the KSZ8081/KSZ9897 phy_id conflicts, I could not find any > stable register to distinguish them. Instead of a match_phy_device() , > I've declared a virtual phy_id with the highest value in Microchip's OUI > range. > > Example usage in the device tree: > compatible = "ethernet-phy-id0022.17ff"; > > A discussion to find better alternatives had been opened with the > Microchip team, with no response yet. > > See https://lore.kernel.org/all/20220207174532.362781-1-enguerrand.de-ribaucourt@savoirfairelinux.com/ > > Fixes: b987e98e50ab ("dsa: add DSA switch driver for Microchip KSZ9477") > Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com> ... > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c ... > @@ -5495,6 +5495,17 @@ static struct phy_driver ksphy_driver[] = { > .suspend = genphy_suspend, > .resume = genphy_resume, > .get_features = ksz9477_get_features, > +}, { > + .phy_id = PHY_ID_KSZ9897, > + .phy_id_mask = MICREL_PHY_ID_MASK, > + .name = "Microchip KSZ9897 Switch", > + /* PHY_BASIC_FEATURES */ > + .config_init = kszphy_config_init, > + .config_aneg = ksz8873mll_config_aneg, > + .read_status = ksz8873mll_read_status, > + /* No suspend/resume callbacks because of errata DS00002330D: > + * Toggling PHY Powerdown can cause errors or link failures in adjacent PHYs > + */ It looks like there will be another version of this patchset. If so, please line-wrap the comment above so it is 80 columns wide or less, as is preferred for Networking code. Likewise in the following patch. Flagged by checkpatch.pl --max-line-length=80 > } }; > > module_phy_driver(ksphy_driver); ...
On 31/05/2024 21:39, Tristram.Ha@microchip.com wrote: > > >> -----Original Message----- >> From: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com> >> Sent: Friday, May 31, 2024 7:24 AM >> diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h >> index 591bf5b5e8dc..81cc16dc2ddf 100644 >> --- a/include/linux/micrel_phy.h >> +++ b/include/linux/micrel_phy.h >> @@ -39,6 +39,10 @@ >> #define PHY_ID_KSZ87XX 0x00221550 >> >> #define PHY_ID_KSZ9477 0x00221631 >> +/* Pseudo ID to specify in compatible field of device tree. >> + * Otherwise the device reports the same ID as KSZ8081 on CPU ports. >> + */ >> +#define PHY_ID_KSZ9897 0x002217ff >> > > I am curious about this KSZ9897 device. Can you point out its product > page on Microchip website? > > KSZ9897 is typically referred to the KSZ9897 switch family, which > contains KSZ9897, KSZ9896, KSZ9567, KSZ8567, KSZ9477 and some others. > > I am not aware that KSZ9897 has MDIO access. The switch is only accessed > through I2C and SPI and proprietary IBA. > > It seems the only function is just to report link so a fixed PHY should > be adequate in this situation. > > MDIO only mode is present in KSZ8863/KSZ8873 switches. I do not know > useful to use such mode in KSZ9897. > I'm using the KSZ9897R from this page: - https://www.microchip.com/en-us/product/ksz9897 My CPU (i.MX6ULL) is connected to the CPU port 6 in RMII, listed in "Two Configurable External MAC Ports" with RGMII. This is for network connectivity with the switch, while I'm using SPI for DSA control. FIGURE 2-1 illustrates that architecture. However, this MDIO interface is indeed missing some documentation. For instance, it's phy_id is never listed (Section 5.2.2.3 only for ports 1-5). I use a fixed-link property in the device tree, but the link would never come up if I use the KSZ8081 PHY driver which was selected without these patches because it emits the same phy_id as port 6. The genphy and KSZ9477 PHY dirvers were no better. I found out that the KSZ8873MLL driver was compatible though, which seems to make sense given your explanation.
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index 2b8f8b7f1517..8a6dfaceeab3 100644 --- a/drivers/net/phy/micrel.c +++ b/drivers/net/phy/micrel.c @@ -16,7 +16,7 @@ * ksz8081, ksz8091, * ksz8061, * Switch : ksz8873, ksz886x - * ksz9477, lan8804 + * ksz9477, ksz9897, lan8804 */ #include <linux/bitfield.h> @@ -5495,6 +5495,17 @@ static struct phy_driver ksphy_driver[] = { .suspend = genphy_suspend, .resume = genphy_resume, .get_features = ksz9477_get_features, +}, { + .phy_id = PHY_ID_KSZ9897, + .phy_id_mask = MICREL_PHY_ID_MASK, + .name = "Microchip KSZ9897 Switch", + /* PHY_BASIC_FEATURES */ + .config_init = kszphy_config_init, + .config_aneg = ksz8873mll_config_aneg, + .read_status = ksz8873mll_read_status, + /* No suspend/resume callbacks because of errata DS00002330D: + * Toggling PHY Powerdown can cause errors or link failures in adjacent PHYs + */ } }; module_phy_driver(ksphy_driver); @@ -5520,6 +5531,7 @@ static struct mdio_device_id __maybe_unused micrel_tbl[] = { { PHY_ID_LAN8814, MICREL_PHY_ID_MASK }, { PHY_ID_LAN8804, MICREL_PHY_ID_MASK }, { PHY_ID_LAN8841, MICREL_PHY_ID_MASK }, + { PHY_ID_KSZ9897, MICREL_PHY_ID_MASK }, { } }; diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h index 591bf5b5e8dc..81cc16dc2ddf 100644 --- a/include/linux/micrel_phy.h +++ b/include/linux/micrel_phy.h @@ -39,6 +39,10 @@ #define PHY_ID_KSZ87XX 0x00221550 #define PHY_ID_KSZ9477 0x00221631 +/* Pseudo ID to specify in compatible field of device tree. + * Otherwise the device reports the same ID as KSZ8081 on CPU ports. + */ +#define PHY_ID_KSZ9897 0x002217ff /* struct phy_device dev_flags definitions */ #define MICREL_PHY_50MHZ_CLK BIT(0)
There is a DSA driver for microchip,ksz9897 which can be controlled through SPI or I2C. This patch adds support for it's CPU ports PHYs to also allow network access to the switch's CPU port. The CPU ports PHYs of the KSZ9897 are not documented in the datasheet. They weirdly use the same PHY ID as the KSZ8081, which is a different PHY and that driver isn't compatible with KSZ9897. Before this patch, the KSZ8081 driver was used for the CPU ports of the KSZ9897 but the link would never come up. A new driver for the KSZ9897 is added, based on the compatible KSZ87XX. I could not test if Gigabit Ethernet works, but the link comes up and can successfully allow packets to be sent and received with DSA tags. To resolve the KSZ8081/KSZ9897 phy_id conflicts, I could not find any stable register to distinguish them. Instead of a match_phy_device() , I've declared a virtual phy_id with the highest value in Microchip's OUI range. Example usage in the device tree: compatible = "ethernet-phy-id0022.17ff"; A discussion to find better alternatives had been opened with the Microchip team, with no response yet. See https://lore.kernel.org/all/20220207174532.362781-1-enguerrand.de-ribaucourt@savoirfairelinux.com/ Fixes: b987e98e50ab ("dsa: add DSA switch driver for Microchip KSZ9477") Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com> --- v4: - rebase on net/main - add Fixes tag - use pseudo phy_id instead of of_tree search v3: https://lore.kernel.org/all/20240530102436.226189-2-enguerrand.de-ribaucourt@savoirfairelinux.com/ --- drivers/net/phy/micrel.c | 14 +++++++++++++- include/linux/micrel_phy.h | 4 ++++ 2 files changed, 17 insertions(+), 1 deletion(-)