[net-next PATCH] net: phy: aquantia: add AQR111 and AQR111B0 PHY ID

[net-next PATCH] net: phy: aquantia: add AQR111 and AQR111B0 PHY ID

[Christian Marangi]

Add Aquantia AQR111 and AQR111B0 PHY ID. These PHY advertise 10G speed
but actually supports up to 5G speed, hence some manual fixup is needed.

The Aquantia AQR111B0 PHY is just a variant of the AQR111 with smaller
chip size.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/net/phy/aquantia/aquantia_main.c | 52 ++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index 97a2fafa15ca..656ea77265a2 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -22,6 +22,8 @@
 #define PHY_ID_AQR107	0x03a1b4e0
 #define PHY_ID_AQCS109	0x03a1b5c2
 #define PHY_ID_AQR405	0x03a1b4b0
+#define PHY_ID_AQR111	0x03a1b610
+#define PHY_ID_AQR111B0	0x03a1b612
 #define PHY_ID_AQR112	0x03a1b662
 #define PHY_ID_AQR412	0x03a1b712
 #define PHY_ID_AQR113C	0x31c31c12
@@ -746,6 +748,16 @@ static int aqr107_probe(struct phy_device *phydev)
 	return aqr_hwmon_probe(phydev);
 }
 
+static int aqr111_config_init(struct phy_device *phydev)
+{
+	/* AQR111 reports supporting speed up to 10G,
+	 * however only speeds up to 5G are supported.
+	 */
+	phy_set_max_speed(phydev, SPEED_5000);
+
+	return aqr107_config_init(phydev);
+}
+
 static struct phy_driver aqr_driver[] = {
 {
 	PHY_ID_MATCH_MODEL(PHY_ID_AQ1202),
@@ -819,6 +831,44 @@ static struct phy_driver aqr_driver[] = {
 	.get_stats	= aqr107_get_stats,
 	.link_change_notify = aqr107_link_change_notify,
 },
+{
+	PHY_ID_MATCH_MODEL(PHY_ID_AQR111),
+	.name		= "Aquantia AQR111",
+	.probe		= aqr107_probe,
+	.get_rate_matching = aqr107_get_rate_matching,
+	.config_init	= aqr111_config_init,
+	.config_aneg    = aqr_config_aneg,
+	.config_intr	= aqr_config_intr,
+	.handle_interrupt = aqr_handle_interrupt,
+	.read_status	= aqr107_read_status,
+	.get_tunable    = aqr107_get_tunable,
+	.set_tunable    = aqr107_set_tunable,
+	.suspend	= aqr107_suspend,
+	.resume		= aqr107_resume,
+	.get_sset_count	= aqr107_get_sset_count,
+	.get_strings	= aqr107_get_strings,
+	.get_stats	= aqr107_get_stats,
+	.link_change_notify = aqr107_link_change_notify,
+},
+{
+	PHY_ID_MATCH_MODEL(PHY_ID_AQR111B0),
+	.name		= "Aquantia AQR111B0",
+	.probe		= aqr107_probe,
+	.get_rate_matching = aqr107_get_rate_matching,
+	.config_init	= aqr111_config_init,
+	.config_aneg    = aqr_config_aneg,
+	.config_intr	= aqr_config_intr,
+	.handle_interrupt = aqr_handle_interrupt,
+	.read_status	= aqr107_read_status,
+	.get_tunable    = aqr107_get_tunable,
+	.set_tunable    = aqr107_set_tunable,
+	.suspend	= aqr107_suspend,
+	.resume		= aqr107_resume,
+	.get_sset_count	= aqr107_get_sset_count,
+	.get_strings	= aqr107_get_strings,
+	.get_stats	= aqr107_get_stats,
+	.link_change_notify = aqr107_link_change_notify,
+},
 {
 	PHY_ID_MATCH_MODEL(PHY_ID_AQR405),
 	.name		= "Aquantia AQR405",
@@ -894,6 +944,8 @@ static struct mdio_device_id __maybe_unused aqr_tbl[] = {
 	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR107) },
 	{ PHY_ID_MATCH_MODEL(PHY_ID_AQCS109) },
 	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR405) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR111) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR111B0) },
 	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR112) },
 	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR412) },
 	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR113C) },
-- 
2.43.0

Re: [net-next PATCH] net: phy: aquantia: add AQR111 and AQR111B0 PHY ID

[Andrew Lunn]

On Tue, Feb 13, 2024 at 02:35:51PM +0100, Christian Marangi wrote:
> Add Aquantia AQR111 and AQR111B0 PHY ID. These PHY advertise 10G speed
> but actually supports up to 5G speed, hence some manual fixup is needed.

Any chance this is a "golden screwdriver" situation? The chip really
can do 10G, but the firmware is supposed to limit it to 5G? This is
just a firmware "bug"?

     Andrew

Re: [net-next PATCH] net: phy: aquantia: add AQR111 and AQR111B0 PHY ID

[Christian Marangi]

On Tue, Feb 13, 2024 at 03:09:57PM +0100, Andrew Lunn wrote:
> On Tue, Feb 13, 2024 at 02:35:51PM +0100, Christian Marangi wrote:
> > Add Aquantia AQR111 and AQR111B0 PHY ID. These PHY advertise 10G speed
> > but actually supports up to 5G speed, hence some manual fixup is needed.
> 
> Any chance this is a "golden screwdriver" situation? The chip really
> can do 10G, but the firmware is supposed to limit it to 5G? This is
> just a firmware "bug"?
>

From [1] the PHY can support up to 5G so yes it is a firmware bug. I can
try searching for some regs to fix the wrong provision values if really
needed.

[1] https://www.marvell.com/content/dam/marvell/en/public-collateral/transceivers/marvell-phys-transceivers-aqrate-gen3-product-brief-2019-09.pdf

-- 
	Ansuel

Re: [net-next PATCH] net: phy: aquantia: add AQR111 and AQR111B0 PHY ID

[Andrew Lunn]

On Tue, Feb 13, 2024 at 03:18:09PM +0100, Christian Marangi wrote:
> On Tue, Feb 13, 2024 at 03:09:57PM +0100, Andrew Lunn wrote:
> > On Tue, Feb 13, 2024 at 02:35:51PM +0100, Christian Marangi wrote:
> > > Add Aquantia AQR111 and AQR111B0 PHY ID. These PHY advertise 10G speed
> > > but actually supports up to 5G speed, hence some manual fixup is needed.
> > 
> > Any chance this is a "golden screwdriver" situation? The chip really
> > can do 10G, but the firmware is supposed to limit it to 5G? This is
> > just a firmware "bug"?
> >
> 
> >From [1] the PHY can support up to 5G so yes it is a firmware bug. I can
> try searching for some regs to fix the wrong provision values if really
> needed.
> 
> [1] https://www.marvell.com/content/dam/marvell/en/public-collateral/transceivers/marvell-phys-transceivers-aqrate-gen3-product-brief-2019-09.pdf

I think you missed the meaning of golden screwdriver.

https://www.urbandictionary.com/define.php?term=Golden%20Screwdriver

It could be that the silicon can do 10G, but marvell are selling it as
a 5G device, with firmware limiting it to 5G. And that firmware
limitation has a bug, so some of the 10G functionality is leaking
through.

Anyway, you change looks O.K.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [net-next PATCH] net: phy: aquantia: add AQR111 and AQR111B0 PHY ID

[Christian Marangi]

On Tue, Feb 13, 2024 at 04:10:01PM +0100, Andrew Lunn wrote:
> On Tue, Feb 13, 2024 at 03:18:09PM +0100, Christian Marangi wrote:
> > On Tue, Feb 13, 2024 at 03:09:57PM +0100, Andrew Lunn wrote:
> > > On Tue, Feb 13, 2024 at 02:35:51PM +0100, Christian Marangi wrote:
> > > > Add Aquantia AQR111 and AQR111B0 PHY ID. These PHY advertise 10G speed
> > > > but actually supports up to 5G speed, hence some manual fixup is needed.
> > > 
> > > Any chance this is a "golden screwdriver" situation? The chip really
> > > can do 10G, but the firmware is supposed to limit it to 5G? This is
> > > just a firmware "bug"?
> > >
> > 
> > >From [1] the PHY can support up to 5G so yes it is a firmware bug. I can
> > try searching for some regs to fix the wrong provision values if really
> > needed.
> > 
> > [1] https://www.marvell.com/content/dam/marvell/en/public-collateral/transceivers/marvell-phys-transceivers-aqrate-gen3-product-brief-2019-09.pdf
> 
> I think you missed the meaning of golden screwdriver.
> 
> https://www.urbandictionary.com/define.php?term=Golden%20Screwdriver
> 
> It could be that the silicon can do 10G, but marvell are selling it as
> a 5G device, with firmware limiting it to 5G. And that firmware
> limitation has a bug, so some of the 10G functionality is leaking
> through.

Oh! Yep I didn't know the meaning of Golden Screwdriver.

With the amount of things we are noticing on these PHY it can be
anything from Marvell itself, from OEM messing up with the Provision to
a buf in the FW itself...

(example that thing that Asus made on the other patch where they HW
disable the port by default, that is against any default spec of the
documentation)

(or also a patch that I still have to submit where some manual fixup are
needed on aqr112 since the FW mess with the SERDES startup regs (again
probably OEM not correctly provisioning the FW))

> 
> Anyway, you change looks O.K.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 

Thanks for the Reviewed-by tag!

-- 
	Ansuel

Re: [net-next PATCH] net: phy: aquantia: add AQR111 and AQR111B0 PHY ID

[Andrew Lunn]

> With the amount of things we are noticing on these PHY it can be
> anything from Marvell itself, from OEM messing up with the Provision to
> a buf in the FW itself...

Ideally, we want to get Marvell to release one universal firmware for
each PHY. Get the OEM out of the picture. This is something i said to
then Aquantia years ago, that provisioning is going to make driver
support a real problem. Seems like i predicted it correctly :-(

	Andrew

Re: [net-next PATCH] net: phy: aquantia: add AQR111 and AQR111B0 PHY ID

[Christian Marangi]

On Tue, Feb 13, 2024 at 04:46:47PM +0100, Andrew Lunn wrote:
> > With the amount of things we are noticing on these PHY it can be
> > anything from Marvell itself, from OEM messing up with the Provision to
> > a buf in the FW itself...
> 
> Ideally, we want to get Marvell to release one universal firmware for
> each PHY. Get the OEM out of the picture. This is something i said to
> then Aquantia years ago, that provisioning is going to make driver
> support a real problem. Seems like i predicted it correctly :-(
>

I can totally see them answer to this request with:
OEM should just attach a SPI to the PHY if major modification are
needed, making us (Marvell) releasing an universal firmware irrelevant
since it should be handled internally by the Aquantia PHY.

Reality is that on Router or more Consumer devices where OEM try to cut
cost everywhere, FW are getting placed on NAND partition or even part of
the OEM FW and then userspace tools are used to setup and load the
Aquantia PHY FW.

This cause the side effect of OEM building one Aquantia PHY FW and then
fixing the Provision data at runtime making all the idea of Marvell to
include these configuration values in the FW broken. (example OEM build
one Aquantia PHY FW for all kind of PHY ID and then apply specific
fixup to setup 10base-r or uxsgmii based on what is actually present on
the board on the MAC side) (yes the PHY can support both and there is a
way to tweak this... this is actually required on AQR112 and other
version that we currently don't support, ideally it should be handled by
the FW internally but reality is that we have case where the FW
provision one mode and the other is actually present in the system)

-- 
	Ansuel

Re: [net-next PATCH] net: phy: aquantia: add AQR111 and AQR111B0 PHY ID

[Andrew Lunn]

On Tue, Feb 13, 2024 at 02:35:51PM +0100, Christian Marangi wrote:
> Add Aquantia AQR111 and AQR111B0 PHY ID. These PHY advertise 10G speed
> but actually supports up to 5G speed, hence some manual fixup is needed.
> 
> The Aquantia AQR111B0 PHY is just a variant of the AQR111 with smaller
> chip size.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

The discussion around provisioning should not prevent this being
merged.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [net-next PATCH] net: phy: aquantia: add AQR111 and AQR111B0 PHY ID

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 13 Feb 2024 14:35:51 +0100 you wrote:
> Add Aquantia AQR111 and AQR111B0 PHY ID. These PHY advertise 10G speed
> but actually supports up to 5G speed, hence some manual fixup is needed.
> 
> The Aquantia AQR111B0 PHY is just a variant of the AQR111 with smaller
> chip size.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> 
> [...]

Here is the summary with links:
  - [net-next] net: phy: aquantia: add AQR111 and AQR111B0 PHY ID
    https://git.kernel.org/netdev/net-next/c/038ba1dc4e54

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html