[RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

[RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

[Vladimir Oltean]

This reverts commit 3ac8eed62596387214869319379c1fcba264d8c6.

I am not actually sure I follow the patch author's logic, because the
change does more than it says on the box, but this patch breaks
suspend/resume on NXP LS1028A and probably on any other systems which
have PHY devices with no driver bound, because the patch has removed the
"!phydev->drv" check without actually explaining why that is fine.

Examples why that is not fine:

- There is an MDIO bus with a PHY device that doesn't have a specific
  PHY driver loaded, because mdiobus_register() automatically creates a
  PHY device for it but there is no specific PHY driver in the system.
  Normally under those circumstances, the generic PHY driver will be
  bound lazily to it (at phy_attach_direct time), but since no one has
  attempted to connect to that PHY device (yet), it will not have a
  driver.

- There is an internal MDIO bus with PCS devices on it, for serial links
  such as SGMII. If the driver does not set bus->phy_mask = ~0 to
  prevent auto-scanning, PHY devices will get created for those PCSes
  too, and although those PHY devices are not used, they do not bother
  anybody either. PCS devices are usually managed in Linux as raw MDIO
  devices. Nonetheless, they do not have a PHY driver, nor does anybody
  attempt to connect to them (because they are not a PHY), and therefore
  this patch breaks that.

The stack trace is:

Unable to handle kernel NULL pointer dereference at virtual address 00000000000000e8
pc : mdio_bus_phy_suspend+0xd8/0xec
lr : dpm_run_callback+0x38/0x90
Call trace:
 mdio_bus_phy_suspend+0xd8/0xec
 dpm_run_callback+0x38/0x90
 __device_suspend+0x108/0x3cc
 dpm_suspend+0x140/0x210
 dpm_suspend_start+0x7c/0xa0
 suspend_devices_and_enter+0x13c/0x540
 pm_suspend+0x2a4/0x330

Fixes: 3ac8eed62596 ("net: phy: Uniform PHY driver access")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/phy/phy_device.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9e2891d8e8dd..ba5ad86ec826 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -233,9 +233,11 @@ static DEFINE_MUTEX(phy_fixup_lock);
 
 static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
 {
+	struct device_driver *drv = phydev->mdio.dev.driver;
+	struct phy_driver *phydrv = to_phy_driver(drv);
 	struct net_device *netdev = phydev->attached_dev;
 
-	if (!phydev->drv->suspend)
+	if (!drv || !phydrv->suspend)
 		return false;
 
 	/* PHY not attached? May suspend if the PHY has not already been
-- 
2.25.1

Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

[Gerhard Engleder]

> This reverts commit 3ac8eed62596387214869319379c1fcba264d8c6.
>
> I am not actually sure I follow the patch author's logic, because the
> change does more than it says on the box, but this patch breaks
> suspend/resume on NXP LS1028A and probably on any other systems which
> have PHY devices with no driver bound, because the patch has removed the
> "!phydev->drv" check without actually explaining why that is fine.

The wrong assumption was that the driver is set for every device during probe
before suspend. Intention of the patch was only clean up of
to_phy_driver() usage.

>  static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
>  {
> +       struct device_driver *drv = phydev->mdio.dev.driver;
> +       struct phy_driver *phydrv = to_phy_driver(drv);
>         struct net_device *netdev = phydev->attached_dev;
>
> -       if (!phydev->drv->suspend)
> +       if (!drv || !phydrv->suspend)
>                 return false;
>
>         /* PHY not attached? May suspend if the PHY has not already been

I suggest to add the "!phydev->drv" check, but others may know it
better than me.

Gerhard

Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

[Vladimir Oltean]

On Sun, Sep 12, 2021 at 10:49:25PM +0200, Gerhard Engleder wrote:
> > This reverts commit 3ac8eed62596387214869319379c1fcba264d8c6.
> >
> > I am not actually sure I follow the patch author's logic, because the
> > change does more than it says on the box, but this patch breaks
> > suspend/resume on NXP LS1028A and probably on any other systems which
> > have PHY devices with no driver bound, because the patch has removed the
> > "!phydev->drv" check without actually explaining why that is fine.
> 
> The wrong assumption was that the driver is set for every device during probe
> before suspend. Intention of the patch was only clean up of
> to_phy_driver() usage.

I am not sure why "to_phy_driver" needs cleanup. Au contraire, I think
the PHY library's usage of struct phy_device :: drv is what is strange
and potentially buggy, it is the only subsystem I know of that keeps its
own driver pointer rather than looking at struct device :: driver.
I think this is largely for historical reasons (it has done this since
the first commit), but it looks to me like to_phy_driver could be used
as part of a larger macro called something like phydev_get_drv which
retrieves the phy_driver from the phydev->mdio.dev.driver.

I say it is buggy because when probing fails ("fails" includes things
like -EPROBE_DEFER) it does not even bother to clear phydev->drv back to
NULL, even though the device will not have a driver pointer. There are
also other things which it does not clean up on probe failure, btw, each
with its own interesting side effects.

> >  static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
> >  {
> > +       struct device_driver *drv = phydev->mdio.dev.driver;
> > +       struct phy_driver *phydrv = to_phy_driver(drv);
> >         struct net_device *netdev = phydev->attached_dev;
> >
> > -       if (!phydev->drv->suspend)
> > +       if (!drv || !phydrv->suspend)
> >                 return false;
> >
> >         /* PHY not attached? May suspend if the PHY has not already been
> 
> I suggest to add the "!phydev->drv" check, but others may know it
> better than me.

So in this case, the difference will be that with your change, a
phy_probe that returns an error code like -EPROBE_DEFER will have the
phydev->drv set, and it will not return false ("may not suspend") quickly,
while the code in its original form will not attempt to suspend that PHY.

The implication is that we may call the ->suspend method of a PHY that
is deferring probe, _before_ the probe has actually succeeded.

To me, making that change and moving the code in yet a third state is
way outside of the scope, which was to restore it to a known working
condition (aka bug fix). If you want to make that change, feel free, I will not.

Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

[Andrew Lunn]

> I am not sure why "to_phy_driver" needs cleanup. Au contraire, I think
> the PHY library's usage of struct phy_device :: drv is what is strange
> and potentially buggy, it is the only subsystem I know of that keeps its
> own driver pointer rather than looking at struct device :: driver.

There is one odd driver in the mix. Take a look at xilinx_gmii2rgmii.c.

It probably could be done a better way, but that is what we have.

   Andrew

Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

[Vladimir Oltean]

On Mon, Sep 13, 2021 at 08:49:19PM +0200, Andrew Lunn wrote:
> > I am not sure why "to_phy_driver" needs cleanup. Au contraire, I think
> > the PHY library's usage of struct phy_device :: drv is what is strange
> > and potentially buggy, it is the only subsystem I know of that keeps its
> > own driver pointer rather than looking at struct device :: driver.
> 
> There is one odd driver in the mix. Take a look at xilinx_gmii2rgmii.c.
> 
> It probably could be done a better way, but that is what we have.

Interesting, to say the least. Also, is there any connection between
that and the revert I'm proposing?

So compared to other vendors, where the RGMII gasket is part of the MAC
device, with Xilinx Zynq it is accessible via MDIO?

This is not all that different from dpaa2-eth and dpaa2-mac which are
different devices on the bus, with different drivers, and the phy-handle
is present on the dpaa2-mac OF node, but the net_device is registered by
the dpaa2-eth device, is it? It seems to be even simpler in fact,
because the dpaa2-mac and dpaa2-eth can even connect/disconnect from
each other at runtime, something which does not appear possible with the
Xilinx MAC and its RGMII gasket.

What was done in that case was that all drivers which could possibly
connect to the DPMAC would need to call dpaa2_mac_connect(), this is
done currently from dpaa2-eth and dpaa2-switch.

It looks like it is said that this GMII2RGMII converter can be placed in
front of any GMII MAC. Nice that there are zero in-tree users of
"xlnx,gmii-to-rgmii-1.0" so that I could figure out exactly how that
plays out in practice. If it is only a few drivers that use the
GMII2RGMII converter, maybe something like that (export a few symbols
from this driver, make all MAC drivers call them) could take less of a
toll on the overall PHY library architecture.

Note that the usage of priv->phy_dev, priv->phy_drv, priv->conv_phy_drv
beats me. Why is "phy_dev" kept inside "priv" even though it is accessed
only inside xgmiitorgmii_probe? Why does xgmiitorgmii_configure() need to
be called from xgmiitorgmii_read_status() which in turn hooks into the
attached PHY driver's phy_read_status()? Why does xgmiitorgmii_configure
not get exported and called from an .adjust_link method or the phylink
equivalent, like any other MAC-side hardware linked with the PHY library
in the kernel?

Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

[Andrew Lunn]

On Tue, Sep 14, 2021 at 12:06:18PM +0000, Vladimir Oltean wrote:
> On Mon, Sep 13, 2021 at 08:49:19PM +0200, Andrew Lunn wrote:
> > > I am not sure why "to_phy_driver" needs cleanup. Au contraire, I think
> > > the PHY library's usage of struct phy_device :: drv is what is strange
> > > and potentially buggy, it is the only subsystem I know of that keeps its
> > > own driver pointer rather than looking at struct device :: driver.
> > 
> > There is one odd driver in the mix. Take a look at xilinx_gmii2rgmii.c.
> > 
> > It probably could be done a better way, but that is what we have.
> 
> Interesting, to say the least. Also, is there any connection between
> that and the revert I'm proposing?

If i remember correctly, Gerhard Engleder is actually using this, and
ran into a problem because the wrong driver structure was used.

> So compared to other vendors, where the RGMII gasket is part of the MAC
> device, with Xilinx Zynq it is accessible via MDIO?

Yes. Its control plane sits on the MDIO bus. Unfortunately, it does
not have any ID registers, so it does not directly appear as a PHY. So
it does interesting things it put itself in the control path to the
real PHY.

> It looks like it is said that this GMII2RGMII converter can be placed in
> front of any GMII MAC. Nice that there are zero in-tree users of
> "xlnx,gmii-to-rgmii-1.0" so that I could figure out exactly how that
> plays out in practice.

If you look back at the thread for that patch, i think Gerhard posted
a DT fragment he is using. Hopefully it will get submitted as a full
board description at some point.

> Note that the usage of priv->phy_dev, priv->phy_drv, priv->conv_phy_drv
> beats me. Why is "phy_dev" kept inside "priv" even though it is accessed
> only inside xgmiitorgmii_probe? Why does xgmiitorgmii_configure() need to
> be called from xgmiitorgmii_read_status() which in turn hooks into the
> attached PHY driver's phy_read_status()? Why does xgmiitorgmii_configure
> not get exported and called from an .adjust_link method or the phylink
> equivalent, like any other MAC-side hardware linked with the PHY library
> in the kernel?

I was never happy with this driver. It got submitted before i went on
vacation, i had a few rounds trying to get the submitter to refactor
it and was mostly ignored. I left on vacation with lots of open review
points, and when i got back it had been merged. And the original
submitters never responded to my requests for improvements.

	Andrew

Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

[Vladimir Oltean]

On Tue, Sep 14, 2021 at 04:33:25PM +0200, Andrew Lunn wrote:
> On Tue, Sep 14, 2021 at 12:06:18PM +0000, Vladimir Oltean wrote:
> > On Mon, Sep 13, 2021 at 08:49:19PM +0200, Andrew Lunn wrote:
> > > > I am not sure why "to_phy_driver" needs cleanup. Au contraire, I think
> > > > the PHY library's usage of struct phy_device :: drv is what is strange
> > > > and potentially buggy, it is the only subsystem I know of that keeps its
> > > > own driver pointer rather than looking at struct device :: driver.
> > >
> > > There is one odd driver in the mix. Take a look at xilinx_gmii2rgmii.c.
> > >
> > > It probably could be done a better way, but that is what we have.
> >
> > Interesting, to say the least. Also, is there any connection between
> > that and the revert I'm proposing?
>
> If i remember correctly, Gerhard Engleder is actually using this, and
> ran into a problem because the wrong driver structure was used.
>
> > So compared to other vendors, where the RGMII gasket is part of the MAC
> > device, with Xilinx Zynq it is accessible via MDIO?
>
> Yes. Its control plane sits on the MDIO bus. Unfortunately, it does
> not have any ID registers, so it does not directly appear as a PHY. So
> it does interesting things it put itself in the control path to the
> real PHY.
>
> > It looks like it is said that this GMII2RGMII converter can be placed in
> > front of any GMII MAC. Nice that there are zero in-tree users of
> > "xlnx,gmii-to-rgmii-1.0" so that I could figure out exactly how that
> > plays out in practice.
>
> If you look back at the thread for that patch, i think Gerhard posted
> a DT fragment he is using. Hopefully it will get submitted as a full
> board description at some point.
>
> > Note that the usage of priv->phy_dev, priv->phy_drv, priv->conv_phy_drv
> > beats me. Why is "phy_dev" kept inside "priv" even though it is accessed
> > only inside xgmiitorgmii_probe? Why does xgmiitorgmii_configure() need to
> > be called from xgmiitorgmii_read_status() which in turn hooks into the
> > attached PHY driver's phy_read_status()? Why does xgmiitorgmii_configure
> > not get exported and called from an .adjust_link method or the phylink
> > equivalent, like any other MAC-side hardware linked with the PHY library
> > in the kernel?
>
> I was never happy with this driver. It got submitted before i went on
> vacation, i had a few rounds trying to get the submitter to refactor
> it and was mostly ignored. I left on vacation with lots of open review
> points, and when i got back it had been merged. And the original
> submitters never responded to my requests for improvements.

Sorry, this is a rabbit hole I really don't want to go into. Allowing it
to override PHY driver functions in order to 'automagically' configure
itself when the PHY driver does stuff is probably where the bad decision
was, everything from there is just the resulting fallout.

Why don't all MAC drivers just hook themselves into the PHY driver's
->read_status method and configure themselves from there?! Why do we
even need adjust_link, phylink, any of that? It's just a small
pointer/driver override, the PHY library supports it.

I have dug up this discussion where your stance seemed to be that
"you want the MAC phy-handle to point to the gmii_to_rgmii 'PHY'"
https://lore.kernel.org/netdev/20190309161912.GD9000@lunn.ch/#t

I am not really sure if that particular reply went towards making this
driver's design any saner than it is. As explained by Harini Katakam in
his reply to you, the GMII2RGMII converter is not a PHY, and should
therefore not be treated like one. It is an RGMII gasket for the MAC.
Treating it as a satellite device of the MAC, which happens by chance to
sit on an MDIO bus, but having otherwise nothing to do with the PHY
library, sounds like a more normal approach (please note that it is
quite likely I am oversimplifying some things since I just learned about
this).

Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

[Gerhard Engleder]

On Tue, Sep 14, 2021 at 5:15 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> On Tue, Sep 14, 2021 at 04:33:25PM +0200, Andrew Lunn wrote:
> > On Tue, Sep 14, 2021 at 12:06:18PM +0000, Vladimir Oltean wrote:
> > > On Mon, Sep 13, 2021 at 08:49:19PM +0200, Andrew Lunn wrote:
> > > > > I am not sure why "to_phy_driver" needs cleanup. Au contraire, I think
> > > > > the PHY library's usage of struct phy_device :: drv is what is strange
> > > > > and potentially buggy, it is the only subsystem I know of that keeps its
> > > > > own driver pointer rather than looking at struct device :: driver.
> > > >
> > > > There is one odd driver in the mix. Take a look at xilinx_gmii2rgmii.c.
> > > >
> > > > It probably could be done a better way, but that is what we have.
> > >
> > > Interesting, to say the least. Also, is there any connection between
> > > that and the revert I'm proposing?
> >
> > If i remember correctly, Gerhard Engleder is actually using this, and
> > ran into a problem because the wrong driver structure was used.

Yes, but that was about phy_loopback and was fixed in the commit before.
With this commit I tried to fix the remaining similar problems like the wrong
driver structure use in phy_loopback. But as explained by Vladimir I failed.
So it is totally ok to revert this commit, no functionality is lost.

> > > So compared to other vendors, where the RGMII gasket is part of the MAC
> > > device, with Xilinx Zynq it is accessible via MDIO?
> >
> > Yes. Its control plane sits on the MDIO bus. Unfortunately, it does
> > not have any ID registers, so it does not directly appear as a PHY. So
> > it does interesting things it put itself in the control path to the
> > real PHY.
> >
> > > It looks like it is said that this GMII2RGMII converter can be placed in
> > > front of any GMII MAC. Nice that there are zero in-tree users of
> > > "xlnx,gmii-to-rgmii-1.0" so that I could figure out exactly how that
> > > plays out in practice.
> >
> > If you look back at the thread for that patch, i think Gerhard posted
> > a DT fragment he is using. Hopefully it will get submitted as a full
> > board description at some point.

I submitted it, but Michal Simek argumented that dts files of FPGA logic shall
not be part of mainline. I suggested that at least one reference
platform for every
FPGA based IP core should be allowed, but he said that no one is able
to test it.
So it seems that you will never see any dts file which contains FPGA logic in
mainline. I will try to submit it again if anyone will support me?

> > > Note that th                       e usage of priv->phy_dev, priv->phy_drv, priv->conv_phy_drv
> > > beats me. Why is "phy_dev" kept inside "priv" even though it is accessed
> > > only inside xgmiitorgmii_probe? Why does xgmiitorgmii_configure() need to
> > > be called from xgmiitorgmii_read_status() which in turn hooks into the
> > > attached PHY driver's phy_read_status()? Why does xgmiitorgmii_configure
> > > not get exported and called from an .adjust_link method or the phylink
> > > equivalent, like any other MAC-side hardware linked with the PHY library
> > > in the kernel?
> >
> > I was never happy with this driver. It got submitted before i went on
> > vacation, i had a few rounds trying to get the submitter to refactor
> > it and was mostly ignored. I left on vacation with lots of open review
> > points, and when i got back it had been merged. And the original
> > submitters never responded to my requests for improvements.
>
> Sorry, this is a rabbit hole I really don't want to go into. Allowing it
> to override PHY driver functions in order to 'automagically' configure
> itself when the PHY driver does stuff is probably where the bad decision
> was, everything from there is just the resulting fallout.
>
> Why don't all MAC drivers just hook themselves into the PHY driver's
> ->read_status method and configure themselves from there?! Why do we
> even need adjust_link, phylink, any of that? It's just a small
> pointer/driver override, the PHY library supports it.
>
> I have dug up this discussion where your stance seemed to be that
> "you want the MAC phy-handle to point to the gmii_to_rgmii 'PHY'"
> https://lore.kernel.org/netdev/20190309161912.GD9000@lunn.ch/#t
>
> I am not really sure if that particular reply went towards making this
> driver's design any saner than it is. As explained by Harini Katakam in
> his reply to you, the GMII2RGMII converter is not a PHY, and should
> therefore not be treated like one. It is an RGMII gasket for the MAC.
> Treating it as a satellite device of the MAC, which happens by chance to
> sit on an MDIO bus, but having otherwise nothing to do with the PHY
> library, sounds like a more normal approach (please note that it is
> quite likely I am oversimplifying some things since I just learned about
> this).

Just for information dts with working GMII2RGMII looks like this:

       tnsep0: ethernet@a0000000 {
                       compatible = "engleder,tsnep";
                       reg = <0x0 0xa0000000 0x0 0x10000>;
                       interrupts = <0 89 1>;
                       interrupt-parent = <&gic>;
                       local-mac-address = [00 00 00 00 00 00];
                       phy-mode = "rgmii";
                       phy-handle = <&phy1>;
                       mdio {
                               #address-cells = <1>;
                               #size-cells = <0>;
                               phy1: ethernet-phy@1 {
                                       reg = <1>;
                                       rxc-skew-ps = <1080>;
                               };
                               gmiitorgmii@8 {
                                       compatible = "xlnx,gmii-to-rgmii-1.0";
                                       reg = <8>;
                                       phy-handle = <&phy1>;
                               };
                       };
               };

Gerhard

Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

[Andrew Lunn]

> I submitted it, but Michal Simek argumented that dts files of FPGA
> logic shall not be part of mainline. I suggested that at least one
> reference platform for every FPGA based IP core should be allowed,
> but he said that no one is able to test it.  So it seems that you
> will never see any dts file which contains FPGA logic in mainline. I
> will try to submit it again if anyone will support me?

My opinion: If there is a real product out in the field using this,
the DT for the product can be in mainline.

Reference Design Kits for ASICs are well supported in mainline. So the
question is, is an FPGA sufficiently different to an ASIC that is
should be treated differently? Do you have an off the shelf platform
or something custom? How easy is it to get the platform which is used
as an RDK? Can you make a bitstream available for anybody to use?

	Andrew

Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

[Gerhard Engleder]

> > I submitted it, but Michal Simek argumented that dts files of FPGA
> > logic shall not be part of mainline. I suggested that at least one
> > reference platform for every FPGA based IP core should be allowed,
> > but he said that no one is able to test it.  So it seems that you
> > will never see any dts file which contains FPGA logic in mainline. I
> > will try to submit it again if anyone will support me?
>
> My opinion: If there is a real product out in the field using this,
> the DT for the product can be in mainline.
>
> Reference Design Kits for ASICs are well supported in mainline. So the
> question is, is an FPGA sufficiently different to an ASIC that is
> should be treated differently? Do you have an off the shelf platform
> or something custom? How easy is it to get the platform which is used
> as an RDK? Can you make a bitstream available for anybody to use?

At least in combination with the board I can see no difference between ASIC
and FPGA. Usually a FPGA bitstream targets a specific board, so the devices
within the FPGA can be treated like devices on the board.

The reference platform is based on off the shelf stuff (Xilinx ZCU104 and Avnet
AES-FMC-NETW1-G). At least I had no problem buying the boards.

Yes, I can provide a bitstream for everybody.

Gerhard

Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

[Vladimir Oltean]

On Tue, Sep 14, 2021 at 07:14:12PM +0200, Gerhard Engleder wrote:
> > > I submitted it, but Michal Simek argumented that dts files of FPGA
> > > logic shall not be part of mainline. I suggested that at least one
> > > reference platform for every FPGA based IP core should be allowed,
> > > but he said that no one is able to test it.  So it seems that you
> > > will never see any dts file which contains FPGA logic in mainline. I
> > > will try to submit it again if anyone will support me?
> >
> > My opinion: If there is a real product out in the field using this,
> > the DT for the product can be in mainline.
> >
> > Reference Design Kits for ASICs are well supported in mainline. So the
> > question is, is an FPGA sufficiently different to an ASIC that is
> > should be treated differently? Do you have an off the shelf platform
> > or something custom? How easy is it to get the platform which is used
> > as an RDK? Can you make a bitstream available for anybody to use?
>
> At least in combination with the board I can see no difference between ASIC
> and FPGA. Usually a FPGA bitstream targets a specific board, so the devices
> within the FPGA can be treated like devices on the board.
>
> The reference platform is based on off the shelf stuff (Xilinx ZCU104 and Avnet
> AES-FMC-NETW1-G). At least I had no problem buying the boards.
>
> Yes, I can provide a bitstream for everybody.

My opinion is that Linux has gotten into the position of maintaining the
central repository of device tree blobs by some sort of strange accident,
and these blobs do not really have to describe "a real product out in
the field" in order to have a place in that central repository, no
matter where that might be hosted. On some platforms it is not even
possible to change the device tree (easily) since it is provided by the
firmware, nonetheless it is still valuable to be able to look at it for reference.

So I think anyone should be able to post their toy TSN driver running on
their toy bit stream described by their toy device tree, and not be too
concerned that they are littering the kernel. I would leave it upon the
device tree maintainers to figure out the scalability concern, after all
Linux took it upon itself to manage the central reference of device trees.
But I do agree that the hardware setup needs at least to be reasonably
reproducible by somebody non-you.

I think the implication of not welcoming this kind of work is marginalizing
hardware vendors such as Xilinx, and their users ending up in a worse
place than if the device trees had a place in the mainline kernel.
I am not a Xilinx engineer nor a Xilinx customer, but I am dreading each
time I get a support request for an NXP switch attached to a Zynq SoC,
just because I am always told that the person I'm helping is trapped
with some sort of odd and old SDK kernel with modifications and it is
not possible for them to move to mainline. So I am really happy to see
people getting past that barrier and submitting drivers developed on
Xilinx SoCs, it would be even nicer if they had an unimpeded path to
make forward progress with their work.

Does this invalidate my point about the GMII2RGMII converter, where I
said that it would be better for all MAC drivers to treat it like a
satellite device, instead of stowing it inside the PHY library with all
the hacks associated with that? After all, Gerhard's TSN endpoint driver
has nothing to do with Xilinx per se, it is only by chance that it runs
on Xilinx hardware, so it may seem reasonable for his driver to not need
to explicitly manage the platform's RGMII gasket. His TSN endpoint might
be ported to a different FPGA manufacturer with no such oddity. Having
this device hidden in the PHY library makes his life easier (at least
apparently, until he hits things that don't work as they should).

So while that is a valid point, there might be other places to put that
converter, which are not in the direct path of the attached PHY's driver.
For example, phylink is the melting pot of a lot of devices, on-board
PHYs, SFP modules with and without PHYs, PCSes, it may even gain support
for retimers, this was brought up a while ago. So maybe it would happily
deal with another off-label device, a standalone RGMII gasket. It could
have a structure with generic ops such as what is in place for struct
phylink_pcs, and it ensures that this will be programmed to the right
speed, put in loopback, etc etc, automatically. MAC driver uses phylink,
the device tree has a phandle to the rgmii-gasket, and it works.
Odd, and may or may not be worth it depending on how much demand there
is, but at least it's an option worth considering.