[PATCH net-next 0/8] net: support 2.5G ethernet in dwmac-qcom-ethqos

[PATCH net-next 0/8] net: support 2.5G ethernet in dwmac-qcom-ethqos

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

The following series introduces various related changes that allow
supporting 2.5G ethernet on the sa8775p-ride board.

First two patches add support for the new SGMII mode in PHY core and the
dwmac-qcom-ethqos driver. Next three introduce support for a new PHY
model to the aquantia driver (while at it: fix two issues I noticed).

Final three provide a way to work around a DMA reset issue on the
sa8775p-ride board where RX clocks from the PHY are not available during
the reset.

Bartosz Golaszewski (8):
  net: phy: add support for overclocked SGMII
  net: stmmac: qcom-ethqos: add support for 2.5G overlocked SGMII mode
  net: phy: aquantia: add missing include guards
  net: phy: aquantia: add support for aqr115c
  net: phy: aquantia: wait for FW reset before checking the vendor ID
  net: stmmac: provide the link_up() callback
  net: stmmac: provide the open() callback
  net: stmmac: qcom-ethqos: add a DMA-reset quirk for sa8775p-ride-r3

 .../stmicro/stmmac/dwmac-qcom-ethqos.c        | 44 +++++++++++++++++
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |  6 +++
 drivers/net/phy/aquantia/aquantia.h           |  6 +++
 drivers/net/phy/aquantia/aquantia_firmware.c  |  4 ++
 drivers/net/phy/aquantia/aquantia_main.c      | 47 +++++++++++++++++--
 drivers/net/phy/phy-core.c                    |  1 +
 drivers/net/phy/phylink.c                     | 13 ++++-
 include/linux/phy.h                           |  4 ++
 include/linux/stmmac.h                        |  2 +
 9 files changed, 121 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH net-next 1/8] net: phy: add support for overclocked SGMII

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

The Aquantia AQR115C PHY supports the Overlocked SGMII mode. In order to
support it in the driver, extend the PHY core with the new mode bits and
pieces.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/net/phy/phy-core.c |  1 +
 drivers/net/phy/phylink.c  | 13 ++++++++++++-
 include/linux/phy.h        |  4 ++++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 15f349e5995a..7cf87cae11f0 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -138,6 +138,7 @@ int phy_interface_num_ports(phy_interface_t interface)
 	case PHY_INTERFACE_MODE_RXAUI:
 	case PHY_INTERFACE_MODE_XAUI:
 	case PHY_INTERFACE_MODE_1000BASEKX:
+	case PHY_INTERFACE_MODE_OCSGMII:
 		return 1;
 	case PHY_INTERFACE_MODE_QSGMII:
 	case PHY_INTERFACE_MODE_QUSGMII:
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 02427378acfd..ce07d41a233f 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -128,6 +128,7 @@ static const phy_interface_t phylink_sfp_interface_preference[] = {
 	PHY_INTERFACE_MODE_5GBASER,
 	PHY_INTERFACE_MODE_2500BASEX,
 	PHY_INTERFACE_MODE_SGMII,
+	PHY_INTERFACE_MODE_OCSGMII,
 	PHY_INTERFACE_MODE_1000BASEX,
 	PHY_INTERFACE_MODE_100BASEX,
 };
@@ -180,6 +181,7 @@ static unsigned int phylink_interface_signal_rate(phy_interface_t interface)
 	switch (interface) {
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_1000BASEX: /* 1.25Mbd */
+	case PHY_INTERFACE_MODE_OCSGMII:
 		return 1250;
 	case PHY_INTERFACE_MODE_2500BASEX: /* 3.125Mbd */
 		return 3125;
@@ -231,6 +233,7 @@ static int phylink_interface_max_speed(phy_interface_t interface)
 		return SPEED_1000;
 
 	case PHY_INTERFACE_MODE_2500BASEX:
+	case PHY_INTERFACE_MODE_OCSGMII:
 		return SPEED_2500;
 
 	case PHY_INTERFACE_MODE_5GBASER:
@@ -515,6 +518,10 @@ static unsigned long phylink_get_capabilities(phy_interface_t interface,
 		caps |= MAC_1000HD | MAC_1000FD;
 		fallthrough;
 
+	case PHY_INTERFACE_MODE_OCSGMII:
+		caps |= MAC_2500FD;
+		fallthrough;
+
 	case PHY_INTERFACE_MODE_REVRMII:
 	case PHY_INTERFACE_MODE_RMII:
 	case PHY_INTERFACE_MODE_SMII:
@@ -929,6 +936,7 @@ static int phylink_parse_mode(struct phylink *pl,
 		case PHY_INTERFACE_MODE_10GKR:
 		case PHY_INTERFACE_MODE_10GBASER:
 		case PHY_INTERFACE_MODE_XLGMII:
+		case PHY_INTERFACE_MODE_OCSGMII:
 			caps = ~(MAC_SYM_PAUSE | MAC_ASYM_PAUSE);
 			caps = phylink_get_capabilities(pl->link_config.interface, caps,
 							RATE_MATCH_NONE);
@@ -1357,7 +1365,8 @@ static void phylink_mac_initial_config(struct phylink *pl, bool force_restart)
 
 	case MLO_AN_INBAND:
 		link_state = pl->link_config;
-		if (link_state.interface == PHY_INTERFACE_MODE_SGMII)
+		if (link_state.interface == PHY_INTERFACE_MODE_SGMII ||
+		    link_state.interface == PHY_INTERFACE_MODE_OCSGMII)
 			link_state.pause = MLO_PAUSE_NONE;
 		break;
 
@@ -3640,6 +3649,7 @@ void phylink_mii_c22_pcs_decode_state(struct phylink_link_state *state,
 		break;
 
 	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_OCSGMII:
 	case PHY_INTERFACE_MODE_QSGMII:
 		phylink_decode_sgmii_word(state, lpa);
 		break;
@@ -3715,6 +3725,7 @@ int phylink_mii_c22_pcs_encode_advertisement(phy_interface_t interface,
 			adv |= ADVERTISE_1000XPSE_ASYM;
 		return adv;
 	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_OCSGMII:
 	case PHY_INTERFACE_MODE_QSGMII:
 		return 0x0001;
 	default:
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e6e83304558e..73da0983d631 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -128,6 +128,7 @@ extern const int phy_10gbit_features_array[1];
  * @PHY_INTERFACE_MODE_10GKR: 10GBASE-KR - with Clause 73 AN
  * @PHY_INTERFACE_MODE_QUSGMII: Quad Universal SGMII
  * @PHY_INTERFACE_MODE_1000BASEKX: 1000Base-KX - with Clause 73 AN
+ * @PHY_INTERFACE_MODE_OCSGMII: Overclocked SGMII
  * @PHY_INTERFACE_MODE_MAX: Book keeping
  *
  * Describes the interface between the MAC and PHY.
@@ -168,6 +169,7 @@ typedef enum {
 	PHY_INTERFACE_MODE_10GKR,
 	PHY_INTERFACE_MODE_QUSGMII,
 	PHY_INTERFACE_MODE_1000BASEKX,
+	PHY_INTERFACE_MODE_OCSGMII,
 	PHY_INTERFACE_MODE_MAX,
 } phy_interface_t;
 
@@ -289,6 +291,8 @@ static inline const char *phy_modes(phy_interface_t interface)
 		return "100base-x";
 	case PHY_INTERFACE_MODE_QUSGMII:
 		return "qusgmii";
+	case PHY_INTERFACE_MODE_OCSGMII:
+		return "ocsgmii";
 	default:
 		return "unknown";
 	}
-- 
2.43.0

Re: [PATCH net-next 1/8] net: phy: add support for overclocked SGMII

[Andrew Lunn]

On Wed, Jun 19, 2024 at 08:45:42PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> The Aquantia AQR115C PHY supports the Overlocked SGMII mode. In order to
> support it in the driver, extend the PHY core with the new mode bits and
> pieces.

Here we go again....

Is this 2500BaseX but without inband signalling, since SGMII inband
signalling makes no sense at 2.5G?

	Andrew

Re: [PATCH net-next 1/8] net: phy: add support for overclocked SGMII

[Bartosz Golaszewski]

On Wed, Jun 19, 2024 at 9:09 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Wed, Jun 19, 2024 at 08:45:42PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > The Aquantia AQR115C PHY supports the Overlocked SGMII mode. In order to
> > support it in the driver, extend the PHY core with the new mode bits and
> > pieces.
>
> Here we go again....
>

Admittedly I don't post to net very often and I assume there's a story
to this comment? Care to elaborate?

Bart

> Is this 2500BaseX but without inband signalling, since SGMII inband
> signalling makes no sense at 2.5G?
>
>         Andrew

Re: [PATCH net-next 1/8] net: phy: add support for overclocked SGMII

[Andrew Lunn]

On Wed, Jun 19, 2024 at 09:29:03PM +0200, Bartosz Golaszewski wrote:
> On Wed, Jun 19, 2024 at 9:09 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > On Wed, Jun 19, 2024 at 08:45:42PM +0200, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > The Aquantia AQR115C PHY supports the Overlocked SGMII mode. In order to
> > > support it in the driver, extend the PHY core with the new mode bits and
> > > pieces.
> >
> > Here we go again....
> >
> 
> Admittedly I don't post to net very often and I assume there's a story
> to this comment? Care to elaborate?

2.5G is a mess because vendors implemented it before the standard came
out, in the form of 2500BaseX. They often did just what this seems to
suggest, they overclocked CISCO SGMII.  But the in-band signalling
SGMII uses cannot work at 2.5G, it makes no sense. So vendors disable
the in-band signalling.

What you likely end up with, is 2500BaseX, but without in-band
signalling.

Now, some real 2500BaseX devices require the peer to perform in-band
signalling. Some will listen for the signalling a while, and if they
hear nothing will go into some sort of fallback mode. Others can be
told the peer does not support inband signalling, and so don't expect
it.

And then we have those which are overclocked SGMII which don't expect
any signalling because SGMII signalling makes no sense at 2.5G.

phylib supports out of band signalling, which is enough to make this
work, so long as two peers will actually establish a link because they
are sufficiently tolerant of what the other end is doing. Sometimes
they need a hint. Russell King has been working on this mess, and i'm
sure he will be along soon.

What i expect will happen is you keep calling this 2500BaseX, without
in band signalling. You can look back in the netdev mailling list for
more details and those that have been here before you. It is always
good to search the history, otherwise you are just going to repeat it.

   Andrew

Re: [PATCH net-next 1/8] net: phy: add support for overclocked SGMII

[Russell King (Oracle)]

On Wed, Jun 19, 2024 at 09:51:12PM +0200, Andrew Lunn wrote:
> phylib supports out of band signalling, which is enough to make this
> work, so long as two peers will actually establish a link because they
> are sufficiently tolerant of what the other end is doing. Sometimes
> they need a hint. Russell King has been working on this mess, and i'm
> sure he will be along soon.

... and I'm rolling my eyes, wondering whether I will get time to
finish the code that I started any time soon. I'll note that the more
hacky code we end up merging, the harder it will become to solve this
problem (and we already have several differing behaviours merged with
2500base-X already.)

> What i expect will happen is you keep calling this 2500BaseX, without
> in band signalling. You can look back in the netdev mailling list for
> more details and those that have been here before you. It is always
> good to search the history, otherwise you are just going to repeat it.

That's where things start getting sticky, because at the moment,
phylink expects 2500base-X to be like 1000base-X, and be a media
interface mode rather than a MAC-to-PHY interface mode. This is partly
what my patches will address if I can get around to finishing them -
but at this point I really do not know when that will be.

I still have the high priority work problem that I'm actively involved
with. I may have three weeks holiday at the start of July (and I really
need it right now!) Then, there's possibly quite a lot of down time in
August because I'm having early cataract ops which will substantially
change my eye sight. There's two possible outcomes from that. The best
case is that in just over two weeks after the first op, I'll be able to
read the screen without glasses. The worst case is that I have to wait
a further two to three weeks to see my optometrist (assuming he has
availability), and then wait for replacement lenses to be made up,
fitted and the new glasses sent.

So, I'm only finding the occasional time to be able to look at
mainline stuff, and I don't see that changing very much until maybe
September.

At this point, I think we may as well give up and let people do
whatever they want to do with 2500base-X (which is basically what we're
already doing), and when they have compatibility problems... well...
really not much we can do about that, and it will be way too late to
try and sort the mess out.

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

Re: [PATCH net-next 1/8] net: phy: add support for overclocked SGMII

[Andrew Halaney]

On Wed, Jun 19, 2024 at 10:07:21PM GMT, Russell King (Oracle) wrote:
> On Wed, Jun 19, 2024 at 09:51:12PM +0200, Andrew Lunn wrote:
> > phylib supports out of band signalling, which is enough to make this
> > work, so long as two peers will actually establish a link because they
> > are sufficiently tolerant of what the other end is doing. Sometimes
> > they need a hint. Russell King has been working on this mess, and i'm
> > sure he will be along soon.
> 
> ... and I'm rolling my eyes, wondering whether I will get time to
> finish the code that I started any time soon. I'll note that the more
> hacky code we end up merging, the harder it will become to solve this
> problem (and we already have several differing behaviours merged with
> 2500base-X already.)
> 
> > What i expect will happen is you keep calling this 2500BaseX, without
> > in band signalling. You can look back in the netdev mailling list for
> > more details and those that have been here before you. It is always
> > good to search the history, otherwise you are just going to repeat it.
> 
> That's where things start getting sticky, because at the moment,
> phylink expects 2500base-X to be like 1000base-X, and be a media
> interface mode rather than a MAC-to-PHY interface mode. This is partly
> what my patches will address if I can get around to finishing them -
> but at this point I really do not know when that will be.
> 
> I still have the high priority work problem that I'm actively involved
> with. I may have three weeks holiday at the start of July (and I really
> need it right now!) Then, there's possibly quite a lot of down time in
> August because I'm having early cataract ops which will substantially
> change my eye sight. There's two possible outcomes from that. The best
> case is that in just over two weeks after the first op, I'll be able to
> read the screen without glasses. The worst case is that I have to wait
> a further two to three weeks to see my optometrist (assuming he has
> availability), and then wait for replacement lenses to be made up,
> fitted and the new glasses sent.
> 
> So, I'm only finding the occasional time to be able to look at
> mainline stuff, and I don't see that changing very much until maybe
> September.
> 
> At this point, I think we may as well give up and let people do
> whatever they want to do with 2500base-X (which is basically what we're
> already doing), and when they have compatibility problems... well...
> really not much we can do about that, and it will be way too late to
> try and sort the mess out.

I hope your holiday and operation go well Russell.

Pardon my ignorance, but I know of quite a few things you have in flight
and because of that I'm not entirely sure what specific patches you're
referring to above. Have those hit the list? I know you're cleaning
up stmmac's phylink/pcs usage, but I'm thinking that this is outside of
that series. Thanks in advance for helping me understand all that's in
progress around this mess of a topic!

Re: [PATCH net-next 1/8] net: phy: add support for overclocked SGMII

[Andrew Halaney]

On Thu, Jun 20, 2024 at 02:42:41PM GMT, Andrew Halaney wrote:
> On Wed, Jun 19, 2024 at 10:07:21PM GMT, Russell King (Oracle) wrote:
> > On Wed, Jun 19, 2024 at 09:51:12PM +0200, Andrew Lunn wrote:
> > > phylib supports out of band signalling, which is enough to make this
> > > work, so long as two peers will actually establish a link because they
> > > are sufficiently tolerant of what the other end is doing. Sometimes
> > > they need a hint. Russell King has been working on this mess, and i'm
> > > sure he will be along soon.
> > 
> > ... and I'm rolling my eyes, wondering whether I will get time to
> > finish the code that I started any time soon. I'll note that the more
> > hacky code we end up merging, the harder it will become to solve this
> > problem (and we already have several differing behaviours merged with
> > 2500base-X already.)
> > 
> > > What i expect will happen is you keep calling this 2500BaseX, without
> > > in band signalling. You can look back in the netdev mailling list for
> > > more details and those that have been here before you. It is always
> > > good to search the history, otherwise you are just going to repeat it.
> > 
> > That's where things start getting sticky, because at the moment,
> > phylink expects 2500base-X to be like 1000base-X, and be a media
> > interface mode rather than a MAC-to-PHY interface mode. This is partly
> > what my patches will address if I can get around to finishing them -
> > but at this point I really do not know when that will be.
> > 
> > I still have the high priority work problem that I'm actively involved
> > with. I may have three weeks holiday at the start of July (and I really
> > need it right now!) Then, there's possibly quite a lot of down time in
> > August because I'm having early cataract ops which will substantially
> > change my eye sight. There's two possible outcomes from that. The best
> > case is that in just over two weeks after the first op, I'll be able to
> > read the screen without glasses. The worst case is that I have to wait
> > a further two to three weeks to see my optometrist (assuming he has
> > availability), and then wait for replacement lenses to be made up,
> > fitted and the new glasses sent.
> > 
> > So, I'm only finding the occasional time to be able to look at
> > mainline stuff, and I don't see that changing very much until maybe
> > September.
> > 
> > At this point, I think we may as well give up and let people do
> > whatever they want to do with 2500base-X (which is basically what we're
> > already doing), and when they have compatibility problems... well...
> > really not much we can do about that, and it will be way too late to
> > try and sort the mess out.
> 
> I hope your holiday and operation go well Russell.
> 
> Pardon my ignorance, but I know of quite a few things you have in flight
> and because of that I'm not entirely sure what specific patches you're
> referring to above. Have those hit the list? I know you're cleaning
> up stmmac's phylink/pcs usage, but I'm thinking that this is outside of
> that series. Thanks in advance for helping me understand all that's in
> progress around this mess of a topic!

Nevermind my question, I was talking a little about this today with respect to a
Renesas board as well (can't escape it it seems) and in going through
our convos I found: https://lore.kernel.org/netdev/ZlNi11AsdDpKM6AM@shell.armlinux.org.uk/

    """
    I do have some work-in-progress patches that attempt to sort this out
    in phylink and identify incompatible situations.

    See http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=net-queue

    commits (I think)...

    net: phylink: clean up phylink_resolve()

    to:

    net: phylink: switch to MLO_AN_PHY when PCS uses outband

    and since I'm converting stmmac's hacky PCS that bypasses phylink to
    a real phylink_pcs, the ethqos code as it stands presents a blocker
    because of this issue. So, I'm intending to post a series in the next
    few days (after the bank holiday) and will definitely need to be
    tested on ethqos hardware.
    """

Thanks,
Andrew

[PATCH net-next 2/8] net: stmmac: qcom-ethqos: add support for 2.5G overlocked SGMII mode

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Add support for 2.5G speed in Overclocked SGMII mode to the QCom ethqos
driver.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 .../net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c   | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index 80eb72bc6311..dac91bc72070 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -665,6 +665,14 @@ static int ethqos_configure_sgmii(struct qcom_ethqos *ethqos)
 	return val;
 }
 
+static void qcom_ethqos_speed_mode_2500(struct net_device *ndev, void *data)
+{
+	struct stmmac_priv *priv = netdev_priv(ndev);
+
+	priv->plat->max_speed = 2500;
+	priv->plat->phy_interface = PHY_INTERFACE_MODE_OCSGMII;
+}
+
 static int ethqos_configure(struct qcom_ethqos *ethqos)
 {
 	return ethqos->configure_func(ethqos);
@@ -787,6 +795,9 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 		ethqos->configure_func = ethqos_configure_rgmii;
 		break;
+	case PHY_INTERFACE_MODE_OCSGMII:
+		plat_dat->speed_mode_2500 = qcom_ethqos_speed_mode_2500;
+		fallthrough;
 	case PHY_INTERFACE_MODE_SGMII:
 		ethqos->configure_func = ethqos_configure_sgmii;
 		break;
-- 
2.43.0

[PATCH net-next 3/8] net: phy: aquantia: add missing include guards

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

The header is missing the include guards so add them.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/net/phy/aquantia/aquantia.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/phy/aquantia/aquantia.h b/drivers/net/phy/aquantia/aquantia.h
index c0e1fd9d7152..b8502793962e 100644
--- a/drivers/net/phy/aquantia/aquantia.h
+++ b/drivers/net/phy/aquantia/aquantia.h
@@ -6,6 +6,9 @@
  * Author: Heiner Kallweit <hkallweit1@gmail.com>
  */
 
+#ifndef AQUANTIA_H
+#define AQUANTIA_H
+
 #include <linux/device.h>
 #include <linux/phy.h>
 
@@ -198,3 +201,5 @@ int aqr_phy_led_hw_control_set(struct phy_device *phydev, u8 index,
 int aqr_phy_led_active_low_set(struct phy_device *phydev, int index, bool enable);
 int aqr_phy_led_polarity_set(struct phy_device *phydev, int index,
 			     unsigned long modes);
+
+#endif /* AQUANTIA_H */
-- 
2.43.0

Re: [PATCH net-next 3/8] net: phy: aquantia: add missing include guards

[Andrew Lunn]

On Wed, Jun 19, 2024 at 08:45:44PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> The header is missing the include guards so add them.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

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

    Andrew

[PATCH net-next 4/8] net: phy: aquantia: add support for aqr115c

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Add support for a new model to the Aquantia driver. This PHY supports
Overlocked SGMII mode with 2.5G speeds.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/net/phy/aquantia/aquantia_main.c | 41 ++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index 6c14355744b7..11da460698b0 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -29,6 +29,7 @@
 #define PHY_ID_AQR113	0x31c31c40
 #define PHY_ID_AQR113C	0x31c31c12
 #define PHY_ID_AQR114C	0x31c31c22
+#define PHY_ID_AQR115C	0x31c31c33
 #define PHY_ID_AQR813	0x31c31cb2
 
 #define MDIO_PHYXS_VEND_IF_STATUS		0xe812
@@ -111,7 +112,6 @@ static u64 aqr107_get_stat(struct phy_device *phydev, int index)
 	int len_h = stat->size - len_l;
 	u64 ret;
 	int val;
-
 	val = phy_read_mmd(phydev, MDIO_MMD_C22EXT, stat->reg);
 	if (val < 0)
 		return U64_MAX;
@@ -368,7 +368,7 @@ static int aqr107_read_status(struct phy_device *phydev)
 		phydev->interface = PHY_INTERFACE_MODE_RXAUI;
 		break;
 	case MDIO_PHYXS_VEND_IF_STATUS_TYPE_OCSGMII:
-		phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
+		phydev->interface = PHY_INTERFACE_MODE_OCSGMII;
 		break;
 	default:
 		phydev->interface = PHY_INTERFACE_MODE_NA;
@@ -721,6 +721,18 @@ static int aqr113c_config_init(struct phy_device *phydev)
 	return aqr107_fill_interface_modes(phydev);
 }
 
+static int aqr115c_config_init(struct phy_device *phydev)
+{
+	/* Check that the PHY interface type is compatible */
+	if (phydev->interface != PHY_INTERFACE_MODE_SGMII &&
+	    phydev->interface != PHY_INTERFACE_MODE_OCSGMII)
+		return -ENODEV;
+
+	phy_set_max_speed(phydev, SPEED_2500);
+
+	return 0;
+}
+
 static int aqr107_probe(struct phy_device *phydev)
 {
 	int ret;
@@ -999,6 +1011,30 @@ static struct phy_driver aqr_driver[] = {
 	.led_hw_control_get = aqr_phy_led_hw_control_get,
 	.led_polarity_set = aqr_phy_led_polarity_set,
 },
+{
+	PHY_ID_MATCH_MODEL(PHY_ID_AQR115C),
+	.name           = "Aquantia AQR115C",
+	.probe          = aqr107_probe,
+	.get_rate_matching = aqr107_get_rate_matching,
+	.config_init    = aqr115c_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,
+	.led_brightness_set = aqr_phy_led_brightness_set,
+	.led_hw_is_supported = aqr_phy_led_hw_is_supported,
+	.led_hw_control_set = aqr_phy_led_hw_control_set,
+	.led_hw_control_get = aqr_phy_led_hw_control_get,
+	.led_polarity_set = aqr_phy_led_polarity_set,
+},
 {
 	PHY_ID_MATCH_MODEL(PHY_ID_AQR813),
 	.name		= "Aquantia AQR813",
@@ -1042,6 +1078,7 @@ static struct mdio_device_id __maybe_unused aqr_tbl[] = {
 	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR113) },
 	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR113C) },
 	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR114C) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR115C) },
 	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR813) },
 	{ }
 };
-- 
2.43.0

Re: [PATCH net-next 4/8] net: phy: aquantia: add support for aqr115c

[Andrew Lunn]

>  	case MDIO_PHYXS_VEND_IF_STATUS_TYPE_OCSGMII:
> -		phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
> +		phydev->interface = PHY_INTERFACE_MODE_OCSGMII;

That looks interesting. I wounder what will break.

>  		break;
>  	default:
>  		phydev->interface = PHY_INTERFACE_MODE_NA;
> @@ -721,6 +721,18 @@ static int aqr113c_config_init(struct phy_device *phydev)
>  	return aqr107_fill_interface_modes(phydev);
>  }
>  
> +static int aqr115c_config_init(struct phy_device *phydev)
> +{
> +	/* Check that the PHY interface type is compatible */
> +	if (phydev->interface != PHY_INTERFACE_MODE_SGMII &&
> +	    phydev->interface != PHY_INTERFACE_MODE_OCSGMII)
> +		return -ENODEV;

Does it support 2500BaseX? 

	Andrew

Re: [PATCH net-next 4/8] net: phy: aquantia: add support for aqr115c

[Andrew Lunn]

>  	case MDIO_PHYXS_VEND_IF_STATUS_TYPE_OCSGMII:
> -		phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
> +		phydev->interface = PHY_INTERFACE_MODE_OCSGMII;
>  		break;

O.K. Given Russells reply, what happens if you skip this hunk, don't
add PHY_INTERFACE_MODE_OCSGMII but use PHY_INTERFACE_MODE_2500BASEX,
and change the MAC driver as needed. Given this here, it seems likely
somebody else is using MDIO_PHYXS_VEND_IF_STATUS_TYPE_OCSGMII as
PHY_INTERFACE_MODE_2500BASEX, so i expect it will work.

Adding PHY_INTERFACE_MODE_OCSGMII is a UAPI addition, since it becomes
possible to pass it in phy-mode in DT. That means, it is hard to
remove later, if Russell ever finds the time to finish his patches,
and PHY_INTERFACE_MODE_OCSGMII is not needed. So if we can avoid
adding it, we should.

       Andrew

[PATCH net-next 5/8] net: phy: aquantia: wait for FW reset before checking the vendor ID

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Checking the firmware register before it boots makes no sense, it will
report 0 even if FW is loaded. Always wait for FW to boot before
continuing.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/net/phy/aquantia/aquantia.h          | 1 +
 drivers/net/phy/aquantia/aquantia_firmware.c | 4 ++++
 drivers/net/phy/aquantia/aquantia_main.c     | 6 +++---
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/aquantia/aquantia.h b/drivers/net/phy/aquantia/aquantia.h
index b8502793962e..2465345081f8 100644
--- a/drivers/net/phy/aquantia/aquantia.h
+++ b/drivers/net/phy/aquantia/aquantia.h
@@ -201,5 +201,6 @@ int aqr_phy_led_hw_control_set(struct phy_device *phydev, u8 index,
 int aqr_phy_led_active_low_set(struct phy_device *phydev, int index, bool enable);
 int aqr_phy_led_polarity_set(struct phy_device *phydev, int index,
 			     unsigned long modes);
+int aqr_wait_reset_complete(struct phy_device *phydev);
 
 #endif /* AQUANTIA_H */
diff --git a/drivers/net/phy/aquantia/aquantia_firmware.c b/drivers/net/phy/aquantia/aquantia_firmware.c
index 0c9640ef153b..524627a36c6f 100644
--- a/drivers/net/phy/aquantia/aquantia_firmware.c
+++ b/drivers/net/phy/aquantia/aquantia_firmware.c
@@ -353,6 +353,10 @@ int aqr_firmware_load(struct phy_device *phydev)
 {
 	int ret;
 
+	ret = aqr_wait_reset_complete(phydev);
+	if (ret)
+		return ret;
+
 	/* Check if the firmware is not already loaded by pooling
 	 * the current version returned by the PHY. If 0 is returned,
 	 * no firmware is loaded.
diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index 11da460698b0..eab779db225c 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -441,7 +441,7 @@ static int aqr107_set_tunable(struct phy_device *phydev,
  * The chip also provides a "reset completed" bit, but it's cleared after
  * read. Therefore function would time out if called again.
  */
-static int aqr107_wait_reset_complete(struct phy_device *phydev)
+int aqr_wait_reset_complete(struct phy_device *phydev)
 {
 	int val;
 
@@ -494,7 +494,7 @@ static int aqr107_config_init(struct phy_device *phydev)
 	WARN(phydev->interface == PHY_INTERFACE_MODE_XGMII,
 	     "Your devicetree is out of date, please update it. The AQR107 family doesn't support XGMII, maybe you mean USXGMII.\n");
 
-	ret = aqr107_wait_reset_complete(phydev);
+	ret = aqr_wait_reset_complete(phydev);
 	if (!ret)
 		aqr107_chip_info(phydev);
 
@@ -522,7 +522,7 @@ static int aqcs109_config_init(struct phy_device *phydev)
 	    phydev->interface != PHY_INTERFACE_MODE_2500BASEX)
 		return -ENODEV;
 
-	ret = aqr107_wait_reset_complete(phydev);
+	ret = aqr_wait_reset_complete(phydev);
 	if (!ret)
 		aqr107_chip_info(phydev);
 
-- 
2.43.0

Re: [PATCH net-next 5/8] net: phy: aquantia: wait for FW reset before checking the vendor ID

[Andrew Lunn]

On Wed, Jun 19, 2024 at 08:45:46PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Checking the firmware register before it boots makes no sense, it will
> report 0 even if FW is loaded. Always wait for FW to boot before
> continuing.

Please split this patch up. One patch which renames the method to the
more generic aqr_ since it is used by more than aqr107. Then add the
new use of it.

Is this actually a fix? What happens to the firmware if you try to
download it while it is still booting? Or do you end up downloading
firmware when it is not actually needed? Please expand the commit
message.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next 5/8] net: phy: aquantia: wait for FW reset before checking the vendor ID

[Bartosz Golaszewski]

On Wed, Jun 19, 2024 at 9:27 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Wed, Jun 19, 2024 at 08:45:46PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Checking the firmware register before it boots makes no sense, it will
> > report 0 even if FW is loaded. Always wait for FW to boot before
> > continuing.
>
> Please split this patch up. One patch which renames the method to the
> more generic aqr_ since it is used by more than aqr107. Then add the
> new use of it.
>

Will do.

> Is this actually a fix? What happens to the firmware if you try to
> download it while it is still booting? Or do you end up downloading
> firmware when it is not actually needed? Please expand the commit
> message.
>

It says '0' and the driver tries to load it from nvmem, then the
filesystem and bails-out after these two fail. I'll extend the commit
message.

Bart

>     Andrew
>
> ---
> pw-bot: cr

[PATCH net-next 6/8] net: stmmac: provide the link_up() callback

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Provide drivers with means to react to link-up events in the form of
another platform callback.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
 include/linux/stmmac.h                            | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 5ddbb0d44373..644bc8a24661 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1006,6 +1006,9 @@ static void stmmac_mac_link_up(struct phylink_config *config,
 	struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
 	u32 old_ctrl, ctrl;
 
+	if (priv->plat->link_up)
+		priv->plat->link_up(priv->dev, priv->plat->bsp_priv);
+
 	if ((priv->plat->flags & STMMAC_FLAG_SERDES_UP_AFTER_PHY_LINKUP) &&
 	    priv->plat->serdes_powerup)
 		priv->plat->serdes_powerup(priv->dev, priv->plat->bsp_priv);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 9c54f82901a1..9dc54d6d65ae 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -271,6 +271,7 @@ struct plat_stmmacenet_data {
 	void (*dump_debug_regs)(void *priv);
 	int (*pcs_init)(struct stmmac_priv *priv);
 	void (*pcs_exit)(struct stmmac_priv *priv);
+	void (*link_up)(struct net_device *ndev, void *priv);
 	struct phylink_pcs *(*select_pcs)(struct stmmac_priv *priv,
 					  phy_interface_t interface);
 	void *bsp_priv;
-- 
2.43.0

[PATCH net-next 7/8] net: stmmac: provide the open() callback

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Provide drivers with means of injecting additional code into the
stmmac_open() function.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
 include/linux/stmmac.h                            | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 644bc8a24661..5f628176d994 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3953,6 +3953,9 @@ static int __stmmac_open(struct net_device *dev,
 	if (ret < 0)
 		return ret;
 
+	if (priv->plat->open)
+		priv->plat->open(dev, priv->plat->bsp_priv);
+
 	if ((!priv->hw->xpcs ||
 	     xpcs_get_an_mode(priv->hw->xpcs, mode) != DW_AN_C73)) {
 		ret = stmmac_init_phy(dev);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 9dc54d6d65ae..59991b38cadb 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -262,6 +262,7 @@ struct plat_stmmacenet_data {
 	void (*serdes_powerdown)(struct net_device *ndev, void *priv);
 	void (*speed_mode_2500)(struct net_device *ndev, void *priv);
 	void (*ptp_clk_freq_config)(struct stmmac_priv *priv);
+	void (*open)(struct net_device *ndev, void *priv);
 	int (*init)(struct platform_device *pdev, void *priv);
 	void (*exit)(struct platform_device *pdev, void *priv);
 	struct mac_device_info *(*setup)(void *priv);
-- 
2.43.0

[PATCH net-next 8/8] net: stmmac: qcom-ethqos: add a DMA-reset quirk for sa8775p-ride-r3

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

On sa8775p-ride the RX clocks from the AQR115C PHY are not available at
the time of the DMA reset so we need to loop TX clocks to RX and then
disable loopback after link-up. Use the provided callbacks to do it for
this board.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 .../stmicro/stmmac/dwmac-qcom-ethqos.c        | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index dac91bc72070..ec43449d0252 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -21,6 +21,7 @@
 #define RGMII_IO_MACRO_CONFIG2		0x1C
 #define RGMII_IO_MACRO_DEBUG1		0x20
 #define EMAC_SYSTEM_LOW_POWER_DEBUG	0x28
+#define EMAC_WRAPPER_SGMII_PHY_CNTRL1	0xf4
 
 /* RGMII_IO_MACRO_CONFIG fields */
 #define RGMII_CONFIG_FUNC_CLK_EN		BIT(30)
@@ -79,6 +80,9 @@
 #define ETHQOS_MAC_CTRL_SPEED_MODE		BIT(14)
 #define ETHQOS_MAC_CTRL_PORT_SEL		BIT(15)
 
+/* EMAC_WRAPPER_SGMII_PHY_CNTRL1 bits */
+#define SGMII_PHY_CNTRL1_SGMII_TX_TO_RX_LOOPBACK_EN	BIT(3)
+
 #define SGMII_10M_RX_CLK_DVDR			0x31
 
 struct ethqos_emac_por {
@@ -678,6 +682,29 @@ static int ethqos_configure(struct qcom_ethqos *ethqos)
 	return ethqos->configure_func(ethqos);
 }
 
+static void qcom_ethqos_set_serdes_loopback(struct qcom_ethqos *ethqos,
+					    bool enable)
+{
+	rgmii_updatel(ethqos,
+		      SGMII_PHY_CNTRL1_SGMII_TX_TO_RX_LOOPBACK_EN,
+		      enable ? SGMII_PHY_CNTRL1_SGMII_TX_TO_RX_LOOPBACK_EN : 0,
+		      EMAC_WRAPPER_SGMII_PHY_CNTRL1);
+}
+
+static void qcom_ethqos_open(struct net_device *pdev, void *priv)
+{
+	struct qcom_ethqos *ethqos = priv;
+
+	qcom_ethqos_set_serdes_loopback(ethqos, true);
+}
+
+static void qcom_ethqos_link_up(struct net_device *ndev, void *priv)
+{
+	struct qcom_ethqos *ethqos = priv;
+
+	qcom_ethqos_set_serdes_loopback(ethqos, false);
+}
+
 static void ethqos_fix_mac_speed(void *priv, unsigned int speed, unsigned int mode)
 {
 	struct qcom_ethqos *ethqos = priv;
@@ -861,6 +888,12 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 	if (data->dma_addr_width)
 		plat_dat->host_dma_width = data->dma_addr_width;
 
+	if (of_device_is_compatible(np, "qcom,sa8775p-ethqos") &&
+	    ethqos->phy_mode == PHY_INTERFACE_MODE_OCSGMII) {
+		plat_dat->open = qcom_ethqos_open;
+		plat_dat->link_up = qcom_ethqos_link_up;
+	}
+
 	if (ethqos->serdes_phy) {
 		plat_dat->serdes_powerup = qcom_ethqos_serdes_powerup;
 		plat_dat->serdes_powerdown  = qcom_ethqos_serdes_powerdown;
-- 
2.43.0

Re: [PATCH net-next 8/8] net: stmmac: qcom-ethqos: add a DMA-reset quirk for sa8775p-ride-r3

[Andrew Lunn]

On Wed, Jun 19, 2024 at 08:45:49PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> On sa8775p-ride the RX clocks from the AQR115C PHY are not available at
> the time of the DMA reset so we need to loop TX clocks to RX and then
> disable loopback after link-up. Use the provided callbacks to do it for
> this board.

How does this differ to ethqos_clks_config()?

	Andrew

Re: [PATCH net-next 8/8] net: stmmac: qcom-ethqos: add a DMA-reset quirk for sa8775p-ride-r3

[Bartosz Golaszewski]

On Wed, Jun 19, 2024 at 9:33 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Wed, Jun 19, 2024 at 08:45:49PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > On sa8775p-ride the RX clocks from the AQR115C PHY are not available at
> > the time of the DMA reset so we need to loop TX clocks to RX and then
> > disable loopback after link-up. Use the provided callbacks to do it for
> > this board.
>
> How does this differ to ethqos_clks_config()?
>

I'm not sure I understand the question. This function is called at
probe/remove and suspend/resume. It's not linked to the issue solved
here.

Bart

Re: [PATCH net-next 8/8] net: stmmac: qcom-ethqos: add a DMA-reset quirk for sa8775p-ride-r3

[Andrew Lunn]

On Thu, Jun 20, 2024 at 10:20:08AM +0200, Bartosz Golaszewski wrote:
> On Wed, Jun 19, 2024 at 9:33 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > On Wed, Jun 19, 2024 at 08:45:49PM +0200, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > On sa8775p-ride the RX clocks from the AQR115C PHY are not available at
> > > the time of the DMA reset so we need to loop TX clocks to RX and then
> > > disable loopback after link-up. Use the provided callbacks to do it for
> > > this board.
> >
> > How does this differ to ethqos_clks_config()?
> >
> 
> I'm not sure I understand the question. This function is called at
> probe/remove and suspend/resume. It's not linked to the issue solved
> here.

		/* Enable functional clock to prevent DMA reset to timeout due
		 * to lacking PHY clock after the hardware block has been power
		 * cycled. The actual configuration will be adjusted once
		 * ethqos_fix_mac_speed() is invoked.

It sounds similar, "DMA reset", "lacking PHY clock".

There is also

commit 58329b03a5957904fa2b33b3824ed19e7b42c9e9
Author: Romain Gantois <romain.gantois@bootlin.com>
Date:   Tue Mar 26 14:32:11 2024 +0100

    net: stmmac: Signal to PHY/PCS drivers to keep RX clock on
    
    There is a reocurring issue with stmmac controllers where the MAC fails to
    initialize its hardware if an RX clock signal isn't provided on the MAC/PHY
    link.
    
    This causes issues when PHY or PCS devices either go into suspend while
    cutting the RX clock or do not bring the clock signal up early enough for
    the MAC to initialize successfully.
    
    Set the mac_requires_rxc flag in the stmmac phylink config so that PHY/PCS
    drivers know to keep the RX clock up at all times.

It would be good to explain the big pictures, why these two changes
are not sufficient.

    Andrew

Re: [PATCH net-next 8/8] net: stmmac: qcom-ethqos: add a DMA-reset quirk for sa8775p-ride-r3

[Russell King (Oracle)]

On Wed, Jun 19, 2024 at 08:45:49PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> On sa8775p-ride the RX clocks from the AQR115C PHY are not available at
> the time of the DMA reset so we need to loop TX clocks to RX and then
> disable loopback after link-up. Use the provided callbacks to do it for
> this board.

If you're using true Cisco SGMII, there are _no_ clocks transferred
between the PHY and PCS/MAC. There are two balanced pairs of data
lines and that is all - one for transmit and one for receive. So this
explanation doesn't make sense to me.

> +static void qcom_ethqos_set_serdes_loopback(struct qcom_ethqos *ethqos,
> +					    bool enable)
> +{
> +	rgmii_updatel(ethqos,
> +		      SGMII_PHY_CNTRL1_SGMII_TX_TO_RX_LOOPBACK_EN,
> +		      enable ? SGMII_PHY_CNTRL1_SGMII_TX_TO_RX_LOOPBACK_EN : 0,
> +		      EMAC_WRAPPER_SGMII_PHY_CNTRL1);
> +}
> +
> +static void qcom_ethqos_open(struct net_device *pdev, void *priv)
> +{
> +	struct qcom_ethqos *ethqos = priv;
> +
> +	qcom_ethqos_set_serdes_loopback(ethqos, true);
> +}
> +
> +static void qcom_ethqos_link_up(struct net_device *ndev, void *priv)
> +{
> +	struct qcom_ethqos *ethqos = priv;
> +
> +	qcom_ethqos_set_serdes_loopback(ethqos, false);
> +}
> +

So you enable loopback at open time, and disable it when the link comes
up. This breaks inband signalling (should stmmac ever use that) because
enabling loopback prevents the PHY sending the SGMII result to the PCS
to indicate that the link has come up... thus phylink won't call
mac_link_up().

So no, I really hate this proposed change.

What I think would be better is if there were hooks at the appropriate
places to handle the lack of clock over _just_ the period that it needs
to be handled, rather than hacking the driver as this proposal does,
abusing platform callbacks because there's nothing better.

I don't have time to go through stmmac and make any suggestions (sorry)
so I can only to say NAK to this change.

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

Re: [PATCH net-next 8/8] net: stmmac: qcom-ethqos: add a DMA-reset quirk for sa8775p-ride-r3

[Russell King (Oracle)]

On Thu, Jun 20, 2024 at 11:57:20AM +0100, Russell King (Oracle) wrote:
> I don't have time to go through stmmac and make any suggestions (sorry)
> so I can only to say NAK to this change.

Also... where is the cover message? I don't have it, so I don't have the
context behind your patch series - and I haven't received all the
patches either.

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

Re: [PATCH net-next 8/8] net: stmmac: qcom-ethqos: add a DMA-reset quirk for sa8775p-ride-r3

[Bartosz Golaszewski]

On Thu, Jun 20, 2024 at 1:16 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Thu, Jun 20, 2024 at 11:57:20AM +0100, Russell King (Oracle) wrote:
> > I don't have time to go through stmmac and make any suggestions (sorry)
> > so I can only to say NAK to this change.
>
> Also... where is the cover message? I don't have it, so I don't have the
> context behind your patch series - and I haven't received all the
> patches either.
>

It's in lore alright:
https://lore.kernel.org/netdev/20240619184550.34524-1-brgl@bgdev.pl/

You were in TO for all the patches (as evident from lore) so maybe spam folder?

Bart