netdev - RE: [PATCH net v4 1/5] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 31 May 2024 19:39:12 +0000
From: <Tristram.Ha@...rochip.com>
To: <enguerrand.de-ribaucourt@...oirfairelinux.com>
CC: <andrew@...n.ch>, <hkallweit1@...il.com>, <linux@...linux.org.uk>,
	<Woojung.Huh@...rochip.com>, <UNGLinuxDriver@...rochip.com>,
	<netdev@...r.kernel.org>
Subject: RE: [PATCH net v4 1/5] net: phy: micrel: add Microchip KSZ 9897
 Switch PHY support



> -----Original Message-----
> From: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@...oirfairelinux.com>
> Sent: Friday, May 31, 2024 7:24 AM
> To: netdev@...r.kernel.org
> Cc: andrew@...n.ch; hkallweit1@...il.com; linux@...linux.org.uk; Woojung Huh -
> C21699 <Woojung.Huh@...rochip.com>; UNGLinuxDriver
> <UNGLinuxDriver@...rochip.com>; Enguerrand de Ribaucourt <enguerrand.de-
> ribaucourt@...oirfairelinux.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@...oirfairelinux.com/
> 
> Fixes: b987e98e50ab ("dsa: add DSA switch driver for Microchip KSZ9477")
> Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-
> ribaucourt@...oirfairelinux.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@...oirfairelinux.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.