netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY
@ 2022-02-07 17:45 Enguerrand de Ribaucourt
  2022-02-07 17:45 ` [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support Enguerrand de Ribaucourt
  2022-02-07 17:45 ` [PATCH v2 2/2] net: phy: micrel: add Microchip KSZ 9477 to the device table Enguerrand de Ribaucourt
  0 siblings, 2 replies; 11+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-02-07 17:45 UTC (permalink / raw)
  To: netdev; +Cc: andrew, hkallweit1, linux

v2: I was on my way to convert the .phy_id_mask to PHY_ID_MATCH_EXACT()
following reviews, but I discovered that the datasheet actually contained PHY id
registers but they do not correspond to RMII ports. I rechecked the phy_id value
on my KSZ9897 and confirmed that I did not make this up. But this lead me to
find that the KSZ8081 revision A2's datasheet shared the exact same phy_id.
Hence, in order not to break compatibility with this model, I wrote a new way to
differenciate them with the default LED MODE configuration instead of
PHY_ID_MATCH_EXACT() by mimicking ksz8051_ksz8795_match_phy_device().

I abstained from converting MICREL_PHY_ID_MASK to PHY_ID_MATCH_MODEL() because
it is used in other drivers, and would take a very long time to check all the
datasheets of these models.

Thanks again to Andrew Lunn for your reviews and suggestions.

Original patch v1 discussion:
https://lore.kernel.org/all/20220204133635.296974-1-enguerrand.de-ribaucourt@savoirfairelinux.com/

---

Hi,
I've recently used a KSZ9897 DSA switch that was connected to an i.MX6 CPU
through SPI for the DSA control, and RMII as the data cpu-port. The SPI/DSA was
well supported in drivers/net/dsa/microchip/ksz9477.c, but the RMII connection
was not working. I would like to upstream the patch I developped to add support
for the KSZ9897 RMII bus. This is required for the cpu-port capability of
the DSA switch and have a complete support of this DSA switch.

Since PHY_ID_KSZ9897 and PHY_ID_KSZ8081 are very close, I had to modify the mask
used for the latter. I don't have this one, so it would be very appreciated if
someone could test this patch with the KSZ8081 or KSZ8091. In particular, I'd
like to know the exact phy_id used by those models to check that the new mask is
valid, and that they don't collide with the KSZ9897. The phy_ids cannot be found
in the datasheet, so I couldn't verify that myself.

My definition of the struct phy_driver was copied from the similar
PHY_ID_KSZ8873MLL and proved to work on a 5.4 kernel. However, my patch may not
support the Gigabit Ethernet but works reliably otherwise.

The second patch fixes an issue with the KSZ9477 declaration I noticed. I
couldn't find PHY_ID_KSZ9477, or an equivalent mask in the MODULE_DEVICE_TABLE
declaration. I fear the driver is not initialized properly with this PHY. I
don't have this model either so it would be great if someone could test this.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support
  2022-02-07 17:45 [PATCH v2 0/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY Enguerrand de Ribaucourt
@ 2022-02-07 17:45 ` Enguerrand de Ribaucourt
  2022-02-07 23:28   ` Andrew Lunn
  2022-02-07 17:45 ` [PATCH v2 2/2] net: phy: micrel: add Microchip KSZ 9477 to the device table Enguerrand de Ribaucourt
  1 sibling, 1 reply; 11+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-02-07 17:45 UTC (permalink / raw)
  To: netdev; +Cc: andrew, hkallweit1, linux, Enguerrand de Ribaucourt

Adding Microchip 9897 Phy included in KSZ9897 Switch.
The KSZ9897 shares the same phy_id as some revisions of the KSZ8081.
match_phy_device functions were added to distinguish them.

Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>
---
 drivers/net/phy/micrel.c   | 45 ++++++++++++++++++++++++++++++++++++++
 include/linux/micrel_phy.h |  5 +++++
 2 files changed, 50 insertions(+)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 44a24b99c894..fc5c33194bdc 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -522,6 +522,34 @@ static int ksz8081_read_status(struct phy_device *phydev)
 	return genphy_read_status(phydev);
 }

+static int ksz8081_ksz9897_match_phy_device(struct phy_device *phydev,
+					    const bool ksz_8081)
+{
+	int ret;
+
+	if ((phydev->phy_id & MICREL_PHY_ID_MASK) != PHY_ID_KSZ8081)
+		return 0;
+
+	ret = phy_read(phydev, MICREL_KSZ8081_CTRL2);
+	if (ret < 0)
+		return ret;
+
+	/* KSZ8081A3/KSZ8091R1 PHY and KSZ9897 switch share the same
+	 * exact PHY ID. However, they can be told apart by the default value
+	 * of the LED mode. It is 0 for the PHY, and 1 for the switch.
+	 */
+	ret &= (MICREL_KSZ8081_CTRL2_LED_MODE0 | MICREL_KSZ8081_CTRL2_LED_MODE1);
+	if (!ksz_8081)
+		return ret;
+	else
+		return !ret;
+}
+
+static int ksz8081_match_phy_device(struct phy_device *phydev)
+{
+	return ksz8081_ksz9897_match_phy_device(phydev, true);
+}
+
 static int ksz8061_config_init(struct phy_device *phydev)
 {
 	int ret;
@@ -1561,6 +1589,11 @@ static int ksz886x_cable_test_get_status(struct phy_device *phydev,
 	return ret;
 }

+static int ksz9897_match_phy_device(struct phy_device *phydev)
+{
+	return ksz8081_ksz9897_match_phy_device(phydev, false);
+}
+
 #define LAN_EXT_PAGE_ACCESS_CONTROL			0x16
 #define LAN_EXT_PAGE_ACCESS_ADDRESS_DATA		0x17
 #define LAN_EXT_PAGE_ACCESS_CTRL_EP_FUNC		0x4000
@@ -1734,6 +1767,7 @@ static struct phy_driver ksphy_driver[] = {
 	.config_init	= ksz8081_config_init,
 	.soft_reset	= genphy_soft_reset,
 	.config_aneg	= ksz8081_config_aneg,
+	.match_phy_device = ksz8081_match_phy_device,
 	.read_status	= ksz8081_read_status,
 	.config_intr	= kszphy_config_intr,
 	.handle_interrupt = kszphy_handle_interrupt,
@@ -1869,6 +1903,17 @@ 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,
+	.match_phy_device = ksz9897_match_phy_device,
+	.read_status	= ksz8873mll_read_status,
+	.suspend	= genphy_suspend,
+	.resume		= genphy_resume,
 } };

 module_phy_driver(ksphy_driver);
diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index 1f7c33b2f5a3..05b24bf7f75f 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
@@ -62,4 +63,8 @@

 #define KSZ886X_CTRL_MDIX_STAT			BIT(4)

+#define MICREL_KSZ8081_CTRL2	0x1F
+#define MICREL_KSZ8081_CTRL2_LED_MODE0	BIT(4)
+#define MICREL_KSZ8081_CTRL2_LED_MODE1	BIT(5)
+
 #endif /* _MICREL_PHY_H */
--
2.25.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/2] net: phy: micrel: add Microchip KSZ 9477 to the device table
  2022-02-07 17:45 [PATCH v2 0/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY Enguerrand de Ribaucourt
  2022-02-07 17:45 ` [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support Enguerrand de Ribaucourt
@ 2022-02-07 17:45 ` Enguerrand de Ribaucourt
  2022-02-07 23:29   ` Andrew Lunn
  1 sibling, 1 reply; 11+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-02-07 17:45 UTC (permalink / raw)
  To: netdev; +Cc: andrew, hkallweit1, linux, Enguerrand de Ribaucourt

PHY_ID_KSZ9477 was supported but not added to the device table passed to
MODULE_DEVICE_TABLE.

Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>
---
 drivers/net/phy/micrel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index fc5c33194bdc..3112965a1fd4 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -1938,6 +1938,7 @@ static struct mdio_device_id __maybe_unused micrel_tbl[] = {
 	{ PHY_ID_KSZ886X, MICREL_PHY_ID_MASK },
 	{ PHY_ID_LAN8814, MICREL_PHY_ID_MASK },
 	{ PHY_ID_LAN8804, MICREL_PHY_ID_MASK },
+	{ PHY_ID_KSZ9477, MICREL_PHY_ID_MASK },
 	{ }
 };

--
2.25.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support
  2022-02-07 17:45 ` [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support Enguerrand de Ribaucourt
@ 2022-02-07 23:28   ` Andrew Lunn
  2022-02-08  8:38     ` Enguerrand de Ribaucourt
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2022-02-07 23:28 UTC (permalink / raw)
  To: Enguerrand de Ribaucourt; +Cc: netdev, hkallweit1, linux

> +	/* KSZ8081A3/KSZ8091R1 PHY and KSZ9897 switch share the same
> +	 * exact PHY ID. However, they can be told apart by the default value
> +	 * of the LED mode. It is 0 for the PHY, and 1 for the switch.
> +	 */
> +	ret &= (MICREL_KSZ8081_CTRL2_LED_MODE0 | MICREL_KSZ8081_CTRL2_LED_MODE1);
> +	if (!ksz_8081)
> +		return ret;
> +	else
> +		return !ret;

What exactly does MICREL_KSZ8081_CTRL2_LED_MODE0 and
MICREL_KSZ8081_CTRL2_LED_MODE1 mean? We have to be careful in that
there could be use cases which actually wants to configure the
LEDs. There have been recent discussions for two other PHYs recently
where the bootloader is configuring the LEDs, to something other than
their default value.

      Andrew

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] net: phy: micrel: add Microchip KSZ 9477 to the device table
  2022-02-07 17:45 ` [PATCH v2 2/2] net: phy: micrel: add Microchip KSZ 9477 to the device table Enguerrand de Ribaucourt
@ 2022-02-07 23:29   ` Andrew Lunn
  0 siblings, 0 replies; 11+ messages in thread
From: Andrew Lunn @ 2022-02-07 23:29 UTC (permalink / raw)
  To: Enguerrand de Ribaucourt; +Cc: netdev, hkallweit1, linux

On Mon, Feb 07, 2022 at 06:45:36PM +0100, Enguerrand de Ribaucourt wrote:
> PHY_ID_KSZ9477 was supported but not added to the device table passed to
> MODULE_DEVICE_TABLE.
> 
> Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>

I'm pretty sure i gave a reviewed-by: for this patch. Please add such
tags when you repost.

     Andrew

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support
  2022-02-07 23:28   ` Andrew Lunn
@ 2022-02-08  8:38     ` Enguerrand de Ribaucourt
  2022-02-08 13:13       ` Andrew Lunn
  0 siblings, 1 reply; 11+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-02-08  8:38 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, hkallweit1, linux

----- Original Message -----
> From: "Andrew Lunn" <andrew@lunn.ch>
> To: "Enguerrand de Ribaucourt" <enguerrand.de-ribaucourt@savoirfairelinux.com>
> Cc: "netdev" <netdev@vger.kernel.org>, "hkallweit1" <hkallweit1@gmail.com>, "linux" <linux@armlinux.org.uk>
> Sent: Tuesday, February 8, 2022 12:28:53 AM
> Subject: Re: [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support

> > + /* KSZ8081A3/KSZ8091R1 PHY and KSZ9897 switch share the same
> > + * exact PHY ID. However, they can be told apart by the default value
> > + * of the LED mode. It is 0 for the PHY, and 1 for the switch.
> > + */
> > + ret &= (MICREL_KSZ8081_CTRL2_LED_MODE0 | MICREL_KSZ8081_CTRL2_LED_MODE1);
> > + if (!ksz_8081)
> > + return ret;
> > + else
> > + return !ret;

> What exactly does MICREL_KSZ8081_CTRL2_LED_MODE0 and
> MICREL_KSZ8081_CTRL2_LED_MODE1 mean? We have to be careful in that
> there could be use cases which actually wants to configure the
> LEDs. There have been recent discussions for two other PHYs recently
> where the bootloader is configuring the LEDs, to something other than
> their default value.

Those registers configure the LED Mode according to the KSZ8081 datasheet:
[00] = LED1: Speed LED0: Link/Activity
[01] = LED1: Activity LED0: Link
[10], [11] = Reserved
default value is [00].

Indeed, if the bootloader changes them, we would match the wrong
device. However, I closely examined all the registers, and there is no
read-only bit that we can use to differentiate both models. The
LED mode bits are the only ones that have a different default value on the
KSZ8081: [00] and the KSZ9897: [01]. Also, the RMII registers are not
documented in the KSZ9897 datasheet so that value is not guaranteed to
be [01] even though that's what I observed.

Do you think we should find another way to match KSZ8081 and KSZ9897?
The good news is that I'm now confident about the phy_id emitted by
both models.

Thanks for your help.

> Andrew

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support
  2022-02-08  8:38     ` Enguerrand de Ribaucourt
@ 2022-02-08 13:13       ` Andrew Lunn
  2022-02-10 15:30         ` Prasanna.VengateshanVaradharajan
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2022-02-08 13:13 UTC (permalink / raw)
  To: Enguerrand de Ribaucourt, Prasanna Vengateshan; +Cc: netdev, hkallweit1, linux

On Tue, Feb 08, 2022 at 03:38:41AM -0500, Enguerrand de Ribaucourt wrote:
> ----- Original Message -----
> > From: "Andrew Lunn" <andrew@lunn.ch>
> > To: "Enguerrand de Ribaucourt" <enguerrand.de-ribaucourt@savoirfairelinux.com>
> > Cc: "netdev" <netdev@vger.kernel.org>, "hkallweit1" <hkallweit1@gmail.com>, "linux" <linux@armlinux.org.uk>
> > Sent: Tuesday, February 8, 2022 12:28:53 AM
> > Subject: Re: [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support
> 
> > > + /* KSZ8081A3/KSZ8091R1 PHY and KSZ9897 switch share the same
> > > + * exact PHY ID. However, they can be told apart by the default value
> > > + * of the LED mode. It is 0 for the PHY, and 1 for the switch.
> > > + */
> > > + ret &= (MICREL_KSZ8081_CTRL2_LED_MODE0 | MICREL_KSZ8081_CTRL2_LED_MODE1);
> > > + if (!ksz_8081)
> > > + return ret;
> > > + else
> > > + return !ret;
> 
> > What exactly does MICREL_KSZ8081_CTRL2_LED_MODE0 and
> > MICREL_KSZ8081_CTRL2_LED_MODE1 mean? We have to be careful in that
> > there could be use cases which actually wants to configure the
> > LEDs. There have been recent discussions for two other PHYs recently
> > where the bootloader is configuring the LEDs, to something other than
> > their default value.
> 
> Those registers configure the LED Mode according to the KSZ8081 datasheet:
> [00] = LED1: Speed LED0: Link/Activity
> [01] = LED1: Activity LED0: Link
> [10], [11] = Reserved
> default value is [00].
> 
> Indeed, if the bootloader changes them, we would match the wrong
> device. However, I closely examined all the registers, and there is no
> read-only bit that we can use to differentiate both models. The
> LED mode bits are the only ones that have a different default value on the
> KSZ8081: [00] and the KSZ9897: [01]. Also, the RMII registers are not
> documented in the KSZ9897 datasheet so that value is not guaranteed to
> be [01] even though that's what I observed.
> 
> Do you think we should find another way to match KSZ8081 and KSZ9897?
> The good news is that I'm now confident about the phy_id emitted by
> both models.

Lets try asking Prasanna Vengateshan, who is working on other
Microchip switches and PHYs at Microchip.

     Andrew

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support
  2022-02-08 13:13       ` Andrew Lunn
@ 2022-02-10 15:30         ` Prasanna.VengateshanVaradharajan
  2022-02-10 15:38           ` Prasanna.VengateshanVaradharajan
  0 siblings, 1 reply; 11+ messages in thread
From: Prasanna.VengateshanVaradharajan @ 2022-02-10 15:30 UTC (permalink / raw)
  To: enguerrand.de-ribaucourt, andrew
  Cc: Vlad.Lyalikov, Sundararaman.H, Lars.Povlsen, Yuiko.Oshino,
	Allan.Nielsen, netdev, David.Cai, Joergen.Andreasen,
	Scott.Kasten, Horatiu.Vultur, Nisar.Sayed, Arun.Ramadoss,
	Divya.Koppera, linux, Kavyasree.Kotagiri, Balaje.Rajasundaram,
	hkallweit1, Prasanna.VengateshanVaradharajan, john.oleary,
	Bryan.Whitehead, Raju.Lakkaraju, Ravi.Hegde, Woojung.Huh,
	Sukanya.Palanisami, Ronnie.Kunin, Tristram.Ha, Antoine.Sebert,
	Selvakannan.Murugesan, Steen.Hegelund, Sushovan.Ranjanroy,
	Bjarni.Jonasson, John.Haechten, Andre.Edich,
	Pandiselvan.Soundrapandian

On Tue, 2022-02-08 at 14:13 +0100, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On Tue, Feb 08, 2022 at 03:38:41AM -0500, Enguerrand de Ribaucourt wrote:
> > ----- Original Message -----
> > > From: "Andrew Lunn" <andrew@lunn.ch>
> > > To: "Enguerrand de Ribaucourt" < 
> > > enguerrand.de-ribaucourt@savoirfairelinux.com>
> > > Cc: "netdev" <netdev@vger.kernel.org>, "hkallweit1"
> > > <hkallweit1@gmail.com>, "linux" <linux@armlinux.org.uk>
> > > Sent: Tuesday, February 8, 2022 12:28:53 AM
> > > Subject: Re: [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897
> > > Switch PHY support
> > 
> > > > + /* KSZ8081A3/KSZ8091R1 PHY and KSZ9897 switch share the same
> > > > + * exact PHY ID. However, they can be told apart by the default value
> > > > + * of the LED mode. It is 0 for the PHY, and 1 for the switch.
> > > > + */
> > > > + ret &= (MICREL_KSZ8081_CTRL2_LED_MODE0 |
> > > > MICREL_KSZ8081_CTRL2_LED_MODE1);
> > > > + if (!ksz_8081)
> > > > + return ret;
> > > > + else
> > > > + return !ret;
> > 
> > > What exactly does MICREL_KSZ8081_CTRL2_LED_MODE0 and
> > > MICREL_KSZ8081_CTRL2_LED_MODE1 mean? We have to be careful in that
> > > there could be use cases which actually wants to configure the
> > > LEDs. There have been recent discussions for two other PHYs recently
> > > where the bootloader is configuring the LEDs, to something other than
> > > their default value.
> > 
> > Those registers configure the LED Mode according to the KSZ8081 datasheet:
> > [00] = LED1: Speed LED0: Link/Activity
> > [01] = LED1: Activity LED0: Link
> > [10], [11] = Reserved
> > default value is [00].
> > 
> > Indeed, if the bootloader changes them, we would match the wrong
> > device. However, I closely examined all the registers, and there is no
> > read-only bit that we can use to differentiate both models. The
> > LED mode bits are the only ones that have a different default value on the
> > KSZ8081: [00] and the KSZ9897: [01]. Also, the RMII registers are not
> > documented in the KSZ9897 datasheet so that value is not guaranteed to
> > be [01] even though that's what I observed.
> > 
> > Do you think we should find another way to match KSZ8081 and KSZ9897?
> > The good news is that I'm now confident about the phy_id emitted by
> > both models.
> 
> Lets try asking Prasanna Vengateshan, who is working on other
> Microchip switches and PHYs at Microchip.
> 
>      Andrew

I have already forwarded to the team who worked on the KSZ9897 PHY and added
here (part of UNGLinuxDriver).

Prasanna V


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support
  2022-02-10 15:30         ` Prasanna.VengateshanVaradharajan
@ 2022-02-10 15:38           ` Prasanna.VengateshanVaradharajan
  2022-04-05 14:53             ` Enguerrand de Ribaucourt
  0 siblings, 1 reply; 11+ messages in thread
From: Prasanna.VengateshanVaradharajan @ 2022-02-10 15:38 UTC (permalink / raw)
  To: enguerrand.de-ribaucourt, andrew
  Cc: linux, netdev, hkallweit1, UNGLinuxDriver

On Thu, 2022-02-10 at 21:00 +0530, Prasanna Vengateshan wrote:
> On Tue, 2022-02-08 at 14:13 +0100, Andrew Lunn wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > content is safe
> > 
> > On Tue, Feb 08, 2022 at 03:38:41AM -0500, Enguerrand de Ribaucourt wrote:
> > > ----- Original Message -----
> > > > From: "Andrew Lunn" <andrew@lunn.ch>
> > > > To: "Enguerrand de Ribaucourt" < 
> > > > enguerrand.de-ribaucourt@savoirfairelinux.com>
> > > > Cc: "netdev" <netdev@vger.kernel.org>, "hkallweit1"
> > > > <hkallweit1@gmail.com>, "linux" <linux@armlinux.org.uk>
> > > > Sent: Tuesday, February 8, 2022 12:28:53 AM
> > > > Subject: Re: [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897
> > > > Switch PHY support
> > > 
> > > > > + /* KSZ8081A3/KSZ8091R1 PHY and KSZ9897 switch share the same
> > > > > + * exact PHY ID. However, they can be told apart by the default value
> > > > > + * of the LED mode. It is 0 for the PHY, and 1 for the switch.
> > > > > + */
> > > > > + ret &= (MICREL_KSZ8081_CTRL2_LED_MODE0 |
> > > > > MICREL_KSZ8081_CTRL2_LED_MODE1);
> > > > > + if (!ksz_8081)
> > > > > + return ret;
> > > > > + else
> > > > > + return !ret;
> > > 
> > > > What exactly does MICREL_KSZ8081_CTRL2_LED_MODE0 and
> > > > MICREL_KSZ8081_CTRL2_LED_MODE1 mean? We have to be careful in that
> > > > there could be use cases which actually wants to configure the
> > > > LEDs. There have been recent discussions for two other PHYs recently
> > > > where the bootloader is configuring the LEDs, to something other than
> > > > their default value.
> > > 
> > > Those registers configure the LED Mode according to the KSZ8081 datasheet:
> > > [00] = LED1: Speed LED0: Link/Activity
> > > [01] = LED1: Activity LED0: Link
> > > [10], [11] = Reserved
> > > default value is [00].
> > > 
> > > Indeed, if the bootloader changes them, we would match the wrong
> > > device. However, I closely examined all the registers, and there is no
> > > read-only bit that we can use to differentiate both models. The
> > > LED mode bits are the only ones that have a different default value on the
> > > KSZ8081: [00] and the KSZ9897: [01]. Also, the RMII registers are not
> > > documented in the KSZ9897 datasheet so that value is not guaranteed to
> > > be [01] even though that's what I observed.
> > > 
> > > Do you think we should find another way to match KSZ8081 and KSZ9897?
> > > The good news is that I'm now confident about the phy_id emitted by
> > > both models.
> > 
> > Lets try asking Prasanna Vengateshan, who is working on other
> > Microchip switches and PHYs at Microchip.
> > 
> >      Andrew
> 
> I have already forwarded to the team who worked on the KSZ9897 PHY and added
> here (part of UNGLinuxDriver).
> 
> Prasanna V

Added right email id..

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support
  2022-02-10 15:38           ` Prasanna.VengateshanVaradharajan
@ 2022-04-05 14:53             ` Enguerrand de Ribaucourt
  2022-04-05 16:47               ` Russell King (Oracle)
  0 siblings, 1 reply; 11+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-04-05 14:53 UTC (permalink / raw)
  To: Prasanna VengateshanVaradharajan
  Cc: Andrew Lunn, linux, netdev, hkallweit1, UNGLinuxDriver

----- Original Message -----
> From: "Prasanna VengateshanVaradharajan" <Prasanna.VengateshanVaradharajan@microchip.com>
> To: "Enguerrand de Ribaucourt" <enguerrand.de-ribaucourt@savoirfairelinux.com>, "Andrew Lunn" <andrew@lunn.ch>
> Cc: "linux" <linux@armlinux.org.uk>, "netdev" <netdev@vger.kernel.org>, "hkallweit1" <hkallweit1@gmail.com>,
> UNGLinuxDriver@microchip.com
> Sent: Thursday, February 10, 2022 4:38:59 PM
> Subject: Re: [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support

> On Thu, 2022-02-10 at 21:00 +0530, Prasanna Vengateshan wrote:
> > On Tue, 2022-02-08 at 14:13 +0100, Andrew Lunn wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > > content is safe

> > > On Tue, Feb 08, 2022 at 03:38:41AM -0500, Enguerrand de Ribaucourt wrote:
> > > > ----- Original Message -----
> > > > > From: "Andrew Lunn" <andrew@lunn.ch>
> > > > > To: "Enguerrand de Ribaucourt" <
> > > > > enguerrand.de-ribaucourt@savoirfairelinux.com>
> > > > > Cc: "netdev" <netdev@vger.kernel.org>, "hkallweit1"
> > > > > <hkallweit1@gmail.com>, "linux" <linux@armlinux.org.uk>
> > > > > Sent: Tuesday, February 8, 2022 12:28:53 AM
> > > > > Subject: Re: [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897
> > > > > Switch PHY support

> > > > > > + /* KSZ8081A3/KSZ8091R1 PHY and KSZ9897 switch share the same
> > > > > > + * exact PHY ID. However, they can be told apart by the default value
> > > > > > + * of the LED mode. It is 0 for the PHY, and 1 for the switch.
> > > > > > + */
> > > > > > + ret &= (MICREL_KSZ8081_CTRL2_LED_MODE0 |
> > > > > > MICREL_KSZ8081_CTRL2_LED_MODE1);
> > > > > > + if (!ksz_8081)
> > > > > > + return ret;
> > > > > > + else
> > > > > > + return !ret;

> > > > > What exactly does MICREL_KSZ8081_CTRL2_LED_MODE0 and
> > > > > MICREL_KSZ8081_CTRL2_LED_MODE1 mean? We have to be careful in that
> > > > > there could be use cases which actually wants to configure the
> > > > > LEDs. There have been recent discussions for two other PHYs recently
> > > > > where the bootloader is configuring the LEDs, to something other than
> > > > > their default value.

> > > > Those registers configure the LED Mode according to the KSZ8081 datasheet:
> > > > [00] = LED1: Speed LED0: Link/Activity
> > > > [01] = LED1: Activity LED0: Link
> > > > [10], [11] = Reserved
> > > > default value is [00].

> > > > Indeed, if the bootloader changes them, we would match the wrong
> > > > device. However, I closely examined all the registers, and there is no
> > > > read-only bit that we can use to differentiate both models. The
> > > > LED mode bits are the only ones that have a different default value on the
> > > > KSZ8081: [00] and the KSZ9897: [01]. Also, the RMII registers are not
> > > > documented in the KSZ9897 datasheet so that value is not guaranteed to
> > > > be [01] even though that's what I observed.

> > > > Do you think we should find another way to match KSZ8081 and KSZ9897?
> > > > The good news is that I'm now confident about the phy_id emitted by
> > > > both models.

> > > Lets try asking Prasanna Vengateshan, who is working on other
> > > Microchip switches and PHYs at Microchip.

> > > Andrew

> > I have already forwarded to the team who worked on the KSZ9897 PHY and added
> > here (part of UNGLinuxDriver).

> > Prasanna V

> Added right email id..

Hello Prasanna,

Have you had any luck contacting the people working on the KSZ9897
PHY? As stated with more details in my previous emails, the RMII phy interface of
the KSZ9897 seems to share the same phy_id as the KSZ8081. However, a different
ksphy_driver struct must be used for the KSZ9897 PHY to work.

Thanks,
Enguerrand

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support
  2022-04-05 14:53             ` Enguerrand de Ribaucourt
@ 2022-04-05 16:47               ` Russell King (Oracle)
  0 siblings, 0 replies; 11+ messages in thread
From: Russell King (Oracle) @ 2022-04-05 16:47 UTC (permalink / raw)
  To: Enguerrand de Ribaucourt
  Cc: Prasanna VengateshanVaradharajan, Andrew Lunn, netdev,
	hkallweit1, UNGLinuxDriver

On Tue, Apr 05, 2022 at 10:53:57AM -0400, Enguerrand de Ribaucourt wrote:
> Hello Prasanna,
> 
> Have you had any luck contacting the people working on the KSZ9897
> PHY? As stated with more details in my previous emails, the RMII phy interface of
> the KSZ9897 seems to share the same phy_id as the KSZ8081. However, a different
> ksphy_driver struct must be used for the KSZ9897 PHY to work.

If there is some other way of detecting the phy device, the drivers can
implement the "match_phy_device" method to do whatever it needs to
differentiate between the two.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-04-05 20:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-07 17:45 [PATCH v2 0/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY Enguerrand de Ribaucourt
2022-02-07 17:45 ` [PATCH v2 1/2] net: phy: micrel: add Microchip KSZ 9897 Switch PHY support Enguerrand de Ribaucourt
2022-02-07 23:28   ` Andrew Lunn
2022-02-08  8:38     ` Enguerrand de Ribaucourt
2022-02-08 13:13       ` Andrew Lunn
2022-02-10 15:30         ` Prasanna.VengateshanVaradharajan
2022-02-10 15:38           ` Prasanna.VengateshanVaradharajan
2022-04-05 14:53             ` Enguerrand de Ribaucourt
2022-04-05 16:47               ` Russell King (Oracle)
2022-02-07 17:45 ` [PATCH v2 2/2] net: phy: micrel: add Microchip KSZ 9477 to the device table Enguerrand de Ribaucourt
2022-02-07 23:29   ` Andrew Lunn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).