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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2e37f014-d9f5-4a09-93b2-81543399d2c4@savoirfairelinux.com>
Date: Fri, 7 Jun 2024 10:11:02 +0200
From: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@...oirfairelinux.com>
To: Woojung.Huh@...rochip.com
Cc: andrew@...n.ch, hkallweit1@...il.com, linux@...linux.org.uk,
 UNGLinuxDriver@...rochip.com, horms@...nel.org, Tristram.Ha@...rochip.com,
 Arun.Ramadoss@...rochip.com, netdev@...r.kernel.org
Subject: Re: [PATCH net v5 1/4] net: phy: micrel: add Microchip KSZ 9897
 Switch PHY support


Hello,

The exact hardware is a Phycore-i.MX6ULL. ENET2 is directly the 
i.MX6ULL's FEC that connects to port 6 of the KSZ9897R (GMAC6) in RMII:

  - X_ENET2_TX_CLK -- TX_CLK6
  - X_ENET2_TX_EN  -- TX_CTL6
  - X_ENET2_TX_D1  -- TXD6_1
  - X_ENET2_TX_D0  -- TXD6_0
  - X_ENET2_RX_EN  -- RX_CTL6
  - X_ENET2_RX_ER  -- RX_ER6
  - X_ENET2_RX_D1  -- RXD6_1
  - X_ENET2_RX_D0  -- RXD6_0

The DSA control is using SPI, but not involved in reading the phy_id in 
my case.

This is materialized in my device tree:

```c
ethernet@...4000 {
	compatible = "fsl,imx6ul-fec\0fsl,imx6q-fec";
	...
	phy-mode = "rmii";
	phy-handle = <0x15>;
	fixed-link {
		speed = <0x64>;
		full-duplex;
	};
};

// MDIO bus is only defined on eth1 but shared with eth2
ethernet@...8000 {
         ...
	mdio {
                 ...
		ksz9897port5@1 {
			compatible = "ethernet-phy-ieee802.3-c22";
			...
			clock-names = "rmii-ref";
			phandle = <0x15>;
		};
};

spi@...0000 {
         ...
	ksz9897@0 {
		compatible = "microchip,ksz9897";
		...
		ports {
			...
			// GMAC6
			port@5 {
				reg = <0x05>;
				label = "cpu";
				ethernet = <0x0c>;
				phy-mode = "rmii";
				rx-internal-delay-ps = <0x5dc>;
				fixed-link {
					speed = <0x64>;
					full-duplex;
				};
			};
		};
	};
};
```

Before I implemented the pseudo phy_id, it was read in the generic IEEE 
clause 22 PHY registers, through the compatible 
"ethernet-phy-ieee802.3-c22". That would be implemented in 
get_phy_c22_id() in MII_PHYSID1/2 registers at 0x2 of the MDIO device.

It is not read through SPI registers 0x6104-0x6105 which are not defined 
in the datasheet for port 6/7 (section 5.2.2.3):
	Address: 0xN104, Size: 16 bits, Port N: 1-5

Do you have other suggestions to read the phy_id?

Thanks for your support,
Enguerrand de Ribaucourt


On 07/06/2024 00:57, Woojung.Huh@...rochip.com wrote:
> Hi Enguerrand,
> 
> We still can't reproduce what you observed with KSZ9897.
> 
> Just to be sure, you accessed PHY register of Port 6 which is GMAC6.
> It is directly connected to MAC of i.MX6ULL over RMII.
> I guess the PHY ID access is register 0x6104-0x6105 of KSZ9897.
> And, return value of PHY ID is 0x0022-0x1561.
> 
> Correct understanding?
> > Thanks.
> Woojung
> 
>> -----Original Message-----
>> From: Enguerrand de Ribaucourt <enguerrand.de-
>> ribaucourt@...oirfairelinux.com>
>> Sent: Wednesday, June 5, 2024 4:34 AM
>> To: Woojung Huh - C21699 <Woojung.Huh@...rochip.com>
>> Cc: andrew@...n.ch; hkallweit1@...il.com; linux@...linux.org.uk;
>> UNGLinuxDriver <UNGLinuxDriver@...rochip.com>; horms@...nel.org; Tristram Ha
>> - C24268 <Tristram.Ha@...rochip.com>; Arun Ramadoss - I17769
>> <Arun.Ramadoss@...rochip.com>; netdev@...r.kernel.org
>> Subject: Re: [PATCH net v5 1/4] 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
>>
>> Hello,
>>
>> On 04/06/2024 22:49, Woojung.Huh@...rochip.com wrote:
>>> Hi Enguerrand,
>>>
>>> Can you help me to understand your setup? I could see you are using
>>>    - Host CPU : i.MX6ULL
>>>    - DSA Switch : KSZ9897R (https://www.microchip.com/en-us/product/ksz9897)
>>>    - Host-to-KSZ interface : RGMII for data path & SPI for control
>>> Based on this, CPU port is either GMAC6 or GMAC7 (Figure 2-1 of [1])
>>>
>>> I have two questions for you.
>>> 1. PHY on CPU port
>>>      Which GMAC (or port number) is connected between Host CPU and KSZ9897R?
>>>      If CPU port is either GMAC6 or GMAC7, it is just a MAC-to-MAC
>> connection over RGMII.
>>
>> I'm using port number 6 as the CPU port for KSZ9897R. GMAC6 is directly
>> connected to the MAC of i.MX6ULL (driver is i.MX fec). I'm using RMII
>> since gigabit is not supported by the i.MX6ULL.
>>
>>> 2. PHY ID
>>>      Its PHY ID is different when checking datasheet of KSZ9897 and KSZ8081.
>>>      PHY ID of Port 1-5 of KSZ9897 is 0x0022-0x1631 per [1]
>>>      PHY ID of KSZ8081 is 0x0022-0x0156x per [2]
>> That's true for port 1-5, however, I found out that the phy_id emitted
>> by GMAC6 is 0x00221561. It is the same as KSZ8081-revA3 according to the
>> datasheet. I also studied all registers at runtime for a reliable
>> difference to implement something like ksz8051_ksz8795_match_phy_device
>> between GMAC6 and KSZ8081, but none appeared to me. Following
>> suggestions by Andrew Lunn, I added this virtual phy_id (0x002217ff) to
>> hardcode in the devicetree. I'm happy with this solution.
>>>
>>> Beside patch, you can create a ticket to Microchip site
>> (https://microchipsupport.force.com/s/supportservice)
>>> if you think it is easier to solve your problem.
>> I created a joined ticket for tracking (Case number 01457279).
>>>
>>
>> Thank you very much for your time,
>>
>> Enguerrand de Ribaucourt
>>
>>> Best regards,
>>> Woojung
>>>
>>> [1]
>> https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocume
>> nts/DataSheets/KSZ9897R-Data-Sheet-DS00002330D.pdf
>>> [2] https://www.microchip.com/en-us/product/ksz8081#document-table
>>>
>>>> -----Original Message-----
>>>> From: Enguerrand de Ribaucourt <enguerrand.de-
>>>> ribaucourt@...oirfairelinux.com>
>>>> Sent: Tuesday, June 4, 2024 5:23 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>; horms@...nel.org; Tristram Ha - C24268
>>>> <Tristram.Ha@...rochip.com>; Arun Ramadoss - I17769
>>>> <Arun.Ramadoss@...rochip.com>; Enguerrand de Ribaucourt <enguerrand.de-
>>>> ribaucourt@...oirfairelinux.com>
>>>> Subject: [PATCH net v5 1/4] 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>
>>>> ---
>>>> v5:
>>>>    - rewrap comments
>>>>    - restore suspend/resume for KSZ9897
>>>> v4: https://lore.kernel.org/all/20240531142430.678198-2-enguerrand.de-
>>>> ribaucourt@...oirfairelinux.com/
>>>>    - 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   | 13 ++++++++++++-
>>>>    include/linux/micrel_phy.h |  4 ++++
>>>>    2 files changed, 16 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
>>>> index 8c20cf937530..11e58fc628df 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>
>>>> @@ -5545,6 +5545,16 @@ static struct phy_driver ksphy_driver[] = {
>>>>           .suspend        = genphy_suspend,
>>>>           .resume         = ksz9477_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,
>>>> +       .suspend        = genphy_suspend,
>>>> +       .resume         = genphy_resume,
>>>>    } };
>>>>
>>>>    module_phy_driver(ksphy_driver);
>>>> @@ -5570,6 +5580,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)
>>>> --
>>>> 2.34.1
>>>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ