[PATCH] net: phy: realtek: add support for RTL8366S Gigabit PHY

[PATCH] net: phy: realtek: add support for RTL8366S Gigabit PHY

[Mark Mentovai]

The PHY built in to the Realtek RTL8366S switch controller was
previously supported by genphy_driver. This PHY does not implement MMD
operations. Since 9b01c885be36 (2023-02-13, in 6.3), MMD register reads
have been made during phy_probe to determine EEE support. For
genphy_driver, these reads are transformed into 802.3 annex 22D clause
45-over-clause 22 mmd_phy_indirect operations that perform MII register
writes to MII_MMD_CTRL and MII_MMD_DATA. This overwrites those two MII
registers, which on this PHY are reserved and have another function,
rendering the PHY unusable while so configured.

Proper support for this PHY is restored by providing a phy_driver that
declares MMD operations as unsupported by using the helper functions
provided for that purpose, while remaining otherwise identical to
genphy_driver.

Fixes: 9b01c885be36 ("net: phy: c22: migrate to genphy_c45_write_eee_adv()")
Fixes: https://github.com/openwrt/openwrt/issues/15981
Link: https://github.com/openwrt/openwrt/issues/15739
Reported-by: Russell Senior <russell@personaltelco.net>
Signed-off-by: Mark Mentovai <mark@mentovai.com>
---
 drivers/net/phy/realtek.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index bed839237fb5..87865918dab6 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -1465,6 +1465,13 @@ static struct phy_driver realtek_drvs[] = {
 		.handle_interrupt = genphy_handle_interrupt_no_ack,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
+	}, {
+		PHY_ID_MATCH_EXACT(0x001cc960),
+		.name		= "RTL8366S Gigabit Ethernet",
+		.suspend	= genphy_suspend,
+		.resume		= genphy_resume,
+		.read_mmd	= genphy_read_mmd_unsupported,
+		.write_mmd	= genphy_write_mmd_unsupported,
 	},
 };
 
-- 
2.45.2

Re: [PATCH] net: phy: realtek: add support for RTL8366S Gigabit PHY

[Simon Horman]

On Thu, Jul 25, 2024 at 01:05:19PM -0400, Mark Mentovai wrote:
> The PHY built in to the Realtek RTL8366S switch controller was
> previously supported by genphy_driver. This PHY does not implement MMD
> operations. Since 9b01c885be36 (2023-02-13, in 6.3), MMD register reads
> have been made during phy_probe to determine EEE support. For
> genphy_driver, these reads are transformed into 802.3 annex 22D clause
> 45-over-clause 22 mmd_phy_indirect operations that perform MII register
> writes to MII_MMD_CTRL and MII_MMD_DATA. This overwrites those two MII
> registers, which on this PHY are reserved and have another function,
> rendering the PHY unusable while so configured.
> 
> Proper support for this PHY is restored by providing a phy_driver that
> declares MMD operations as unsupported by using the helper functions
> provided for that purpose, while remaining otherwise identical to
> genphy_driver.
> 
> Fixes: 9b01c885be36 ("net: phy: c22: migrate to genphy_c45_write_eee_adv()")
> Fixes: https://github.com/openwrt/openwrt/issues/15981

nit: AFAIK, the line immediately above is not a correct use of the Fixes
     tag. I think Link or Closes would be appropriate instead.

> Link: https://github.com/openwrt/openwrt/issues/15739
> Reported-by: Russell Senior <russell@personaltelco.net>
> Signed-off-by: Mark Mentovai <mark@mentovai.com>

Also, as a fix, this should be targeted at the net tree.

	Subject: [PATCH net] ...

Please see https://docs.kernel.org/process/maintainer-netdev.html

Re: [PATCH] net: phy: realtek: add support for RTL8366S Gigabit PHY

[Mark Mentovai]

Simon Horman wrote:
> On Thu, Jul 25, 2024 at 01:05:19PM -0400, Mark Mentovai wrote:
>> The PHY built in to the Realtek RTL8366S switch controller was
>> previously supported by genphy_driver. This PHY does not implement MMD
>> operations. Since 9b01c885be36 (2023-02-13, in 6.3), MMD register reads
>> have been made during phy_probe to determine EEE support. For
>> genphy_driver, these reads are transformed into 802.3 annex 22D clause
>> 45-over-clause 22 mmd_phy_indirect operations that perform MII register
>> writes to MII_MMD_CTRL and MII_MMD_DATA. This overwrites those two MII
>> registers, which on this PHY are reserved and have another function,
>> rendering the PHY unusable while so configured.
>>
>> Proper support for this PHY is restored by providing a phy_driver that
>> declares MMD operations as unsupported by using the helper functions
>> provided for that purpose, while remaining otherwise identical to
>> genphy_driver.
>>
>> Fixes: 9b01c885be36 ("net: phy: c22: migrate to genphy_c45_write_eee_adv()")
>> Fixes: https://github.com/openwrt/openwrt/issues/15981
>
> nit: AFAIK, the line immediately above is not a correct use of the Fixes
>     tag. I think Link or Closes would be appropriate instead.
>
>> Link: https://github.com/openwrt/openwrt/issues/15739
>> Reported-by: Russell Senior <russell@personaltelco.net>
>> Signed-off-by: Mark Mentovai <mark@mentovai.com>
>
> Also, as a fix, this should be targeted at the net tree.
>
> 	Subject: [PATCH net] ...
>
> Please see https://docs.kernel.org/process/maintainer-netdev.html

Thanks for your feedback, Simon. I've sent an updated (v2) patch, now at 
https://lore.kernel.org/netdev/20240725204147.69730-1-mark@mentovai.com/.