Regression: supported_interfaces filling enforcement

Regression: supported_interfaces filling enforcement

[Sergei Antonov]

Hello!
This commit seems to break the mv88e6060 dsa driver:
de5c9bf40c4582729f64f66d9cf4920d50beb897    "net: phylink: require
supported_interfaces to be filled"

The driver does not fill 'supported_interfaces'. What is the proper
way to fix it? I managed to fix it by the following quick code.
Comments? Recommendations?

+static void mv88e6060_get_caps(struct dsa_switch *ds, int port,
+                              struct phylink_config *config)
+{
+       __set_bit(PHY_INTERFACE_MODE_INTERNAL, config->supported_interfaces);
+       __set_bit(PHY_INTERFACE_MODE_GMII, config->supported_interfaces);
+}
+
 static const struct dsa_switch_ops mv88e6060_switch_ops = {
        .get_tag_protocol = mv88e6060_get_tag_protocol,
        .setup          = mv88e6060_setup,
        .phy_read       = mv88e6060_phy_read,
        .phy_write      = mv88e6060_phy_write,
+       .phylink_get_caps = mv88e6060_get_caps
 };

Re: Regression: supported_interfaces filling enforcement

[Linux regression tracking (Thorsten Leemhuis)]

[CCing the regression list, as it should be in the loop for regressions:
https://docs.kernel.org/admin-guide/reporting-regressions.html]

[TLDR: I'm adding this report to the list of tracked Linux kernel
regressions; the text you find below is based on a few templates
paragraphs you might have encountered already in similar form.
See link in footer if these mails annoy you.]

On 04.07.23 16:28, Sergei Antonov wrote:
> Hello!
> This commit seems to break the mv88e6060 dsa driver:
> de5c9bf40c4582729f64f66d9cf4920d50beb897    "net: phylink: require
> supported_interfaces to be filled"
> 
> The driver does not fill 'supported_interfaces'. What is the proper
> way to fix it? I managed to fix it by the following quick code.
> Comments? Recommendations?
> 
> +static void mv88e6060_get_caps(struct dsa_switch *ds, int port,
> +                              struct phylink_config *config)
> +{
> +       __set_bit(PHY_INTERFACE_MODE_INTERNAL, config->supported_interfaces);
> +       __set_bit(PHY_INTERFACE_MODE_GMII, config->supported_interfaces);
> +}
> +
>  static const struct dsa_switch_ops mv88e6060_switch_ops = {
>         .get_tag_protocol = mv88e6060_get_tag_protocol,
>         .setup          = mv88e6060_setup,
>         .phy_read       = mv88e6060_phy_read,
>         .phy_write      = mv88e6060_phy_write,
> +       .phylink_get_caps = mv88e6060_get_caps
>  };

Thanks for the report. To be sure the issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
tracking bot:

#regzbot ^introduced de5c9bf40c4582729f6
#regzbot title net: phylink: mv88e6060 dsa driver broken
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply and tell me -- ideally
while also telling regzbot about it, as explained by the page listed in
the footer of this mail.

Developers: When fixing the issue, remember to add 'Link:' tags pointing
to the report (the parent of this mail). See page linked in footer for
details.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.

Re: Regression: supported_interfaces filling enforcement

[Linux regression tracking #update (Thorsten Leemhuis)]

On 08.07.23 16:01, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 04.07.23 16:28, Sergei Antonov wrote:
>> Hello!
>> This commit seems to break the mv88e6060 dsa driver:
>> de5c9bf40c4582729f64f66d9cf4920d50beb897    "net: phylink: require
>> supported_interfaces to be filled"
>>
>> The driver does not fill 'supported_interfaces'. What is the proper
>> way to fix it? I managed to fix it by the following quick code.
>> Comments? Recommendations?

#regzbot monitor:
https://lore.kernel.org/all/E1qOflM-001AEz-D3@rmk-PC.armlinux.org.uk/
#regzbot fix: 9945c1fb03a3
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.

Re: Regression: supported_interfaces filling enforcement

[Vladimir Oltean]

On Tue, Jul 04, 2023 at 05:28:47PM +0300, Sergei Antonov wrote:
> Hello!
> This commit seems to break the mv88e6060 dsa driver:
> de5c9bf40c4582729f64f66d9cf4920d50beb897    "net: phylink: require
> supported_interfaces to be filled"
> 
> The driver does not fill 'supported_interfaces'. What is the proper
> way to fix it? I managed to fix it by the following quick code.
> Comments? Recommendations?

Ok, it seems that commit de5c9bf40c45 ("net: phylink: require
supported_interfaces to be filled") was based on a miscalculation.

> 
> +static void mv88e6060_get_caps(struct dsa_switch *ds, int port,
> +                              struct phylink_config *config)
> +{
> +       __set_bit(PHY_INTERFACE_MODE_INTERNAL, config->supported_interfaces);
> +       __set_bit(PHY_INTERFACE_MODE_GMII, config->supported_interfaces);

This is enough to fix phylink generic validation on the front-facing
ports with internal PHYs. But it is possible (and encouraged) to use
phylink on the CPU port too (rev-mii, rev-rmii); currently that's not
enforced for mv88e6060 because it's in the dsa_switches_apply_workarounds[]
array.

Could you please modify your device tree to add a fixed-link and
phy-mode property on your CPU port so that phylink does get used, and
populate supported_interfaces and mac_capabilities properly on the MII
ports (4 and 5) as well (so that it doesn't fail validation)?

> +}
> +
>  static const struct dsa_switch_ops mv88e6060_switch_ops = {
>         .get_tag_protocol = mv88e6060_get_tag_protocol,
>         .setup          = mv88e6060_setup,
>         .phy_read       = mv88e6060_phy_read,
>         .phy_write      = mv88e6060_phy_write,
> +       .phylink_get_caps = mv88e6060_get_caps

Comma at the end, please.

>  };

Re: Regression: supported_interfaces filling enforcement

[Sergei Antonov]

On Mon, 10 Jul 2023 at 15:35, Vladimir Oltean <olteanv@gmail.com> wrote:
> > +static void mv88e6060_get_caps(struct dsa_switch *ds, int port,
> > +                              struct phylink_config *config)
> > +{
> > +       __set_bit(PHY_INTERFACE_MODE_INTERNAL, config->supported_interfaces);
> > +       __set_bit(PHY_INTERFACE_MODE_GMII, config->supported_interfaces);
>
> This is enough to fix phylink generic validation on the front-facing
> ports with internal PHYs. But it is possible (and encouraged) to use
> phylink on the CPU port too (rev-mii, rev-rmii); currently that's not
> enforced for mv88e6060 because it's in the dsa_switches_apply_workarounds[]
> array.
>
> Could you please modify your device tree to add a fixed-link and
> phy-mode property on your CPU port so that phylink does get used

I already have fixed-link and phy-mode on CPU port. See below.

> , and
> populate supported_interfaces and mac_capabilities properly on the MII
> ports (4 and 5) as well (so that it doesn't fail validation)?

By setting bits in .phylink_get_caps function?

Should I remove mv88e6060 from dsa_switches_apply_workarounds too?

&mdio1 {
        status = "okay";

        #address-cells = <1>;
        #size-cells = <0>;

        switch@10 {
                compatible = "marvell,mv88e6060";
                reg = <0x10>;

                ports {
                        #address-cells = <1>;
                        #size-cells = <0>;

                        port@0 {
                                reg = <0>;
                                label = "lan2";
                        };

                        port@1 {
                                reg = <1>;
                                label = "lan3";
                        };

                        port@2 {
                                reg = <2>;
                                label = "lan1";
                        };

                        port@5 {
                                reg = <5>;
                                label = "cpu";
                                ethernet = <&mac1>;
                                phy-mode = "mii";

                                fixed-link {
                                        speed = <100>;
                                        full-duplex;
                                };
                        };
                };
        };
};

Re: Regression: supported_interfaces filling enforcement

[Vladimir Oltean]

On Mon, Jul 10, 2023 at 05:35:35PM +0300, Sergei Antonov wrote:
> On Mon, 10 Jul 2023 at 15:35, Vladimir Oltean <olteanv@gmail.com> wrote:
> > > +static void mv88e6060_get_caps(struct dsa_switch *ds, int port,
> > > +                              struct phylink_config *config)
> > > +{
> > > +       __set_bit(PHY_INTERFACE_MODE_INTERNAL, config->supported_interfaces);
> > > +       __set_bit(PHY_INTERFACE_MODE_GMII, config->supported_interfaces);
> >
> > This is enough to fix phylink generic validation on the front-facing
> > ports with internal PHYs. But it is possible (and encouraged) to use
> > phylink on the CPU port too (rev-mii, rev-rmii); currently that's not
> > enforced for mv88e6060 because it's in the dsa_switches_apply_workarounds[]
> > array.
> >
> > Could you please modify your device tree to add a fixed-link and
> > phy-mode property on your CPU port so that phylink does get used
> 
> I already have fixed-link and phy-mode on CPU port. See below.

And with PHY_INTERFACE_MODE_MII missing from supported_interfaces, how
did phylink_generic_validate() not fail for the CPU port?

... goes to check dsa_port_phylink_validate() ...

ah, the call to phylink_generic_validate() is skipped because you're not
setting config->mac_capabilities in your mini implementation.
Please also set that - take other drivers as a model. But for
config->legacy_pre_march2020, I guess you don't have to explicitly set
that to false, because dsa_port_phylink_create() doesn't set it to true,
either.

> > , and
> > populate supported_interfaces and mac_capabilities properly on the MII
> > ports (4 and 5) as well (so that it doesn't fail validation)?
> 
> By setting bits in .phylink_get_caps function?

Yes, depending on which port it is and how it is configured to be used.

> Should I remove mv88e6060 from dsa_switches_apply_workarounds too?

If the device tree doesn't have fixed-link and phy-mode on the CPU port,
a phylink instance will fail to be created there, and the switch will
fail to probe. The presence in dsa_switches_apply_workarounds[] allows
phylink to be skipped for the CPU port.

I won't oppose to mv88e6060 getting removed from that array, but if
there exist production device trees with the above characteristics,
it would be unwise to break them.

That being said, given the kind of bugs I've seen uncovered in this
driver recently, I'd say it would be ridiculous to play pretend - you're
probably one of its only users. You can probably be a bit aggressive,
remove support for incomplete device trees, see if anyone complains, and
they do, revert the removal.

> &mdio1 {
>         status = "okay";
> 
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         switch@10 {
>                 compatible = "marvell,mv88e6060";
>                 reg = <0x10>;
> 
>                 ports {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> 
>                         port@0 {
>                                 reg = <0>;
>                                 label = "lan2";
>                         };
> 
>                         port@1 {
>                                 reg = <1>;
>                                 label = "lan3";
>                         };
> 
>                         port@2 {
>                                 reg = <2>;
>                                 label = "lan1";
>                         };
> 
>                         port@5 {
>                                 reg = <5>;
>                                 label = "cpu";
>                                 ethernet = <&mac1>;
>                                 phy-mode = "mii";

I think we have phy-mode = "rev-mii" for "MAC acting as MII PHY", rather
than "mii" which is "MAC acting as MII MAC".

> 
>                                 fixed-link {
>                                         speed = <100>;
>                                         full-duplex;
>                                 };
>                         };
>                 };
>         };
> };

Re: Regression: supported_interfaces filling enforcement

[Sergei Antonov]

On Mon, 10 Jul 2023 at 18:38, Vladimir Oltean <olteanv@gmail.com> wrote:

> That being said, given the kind of bugs I've seen uncovered in this
> driver recently, I'd say it would be ridiculous to play pretend - you're
> probably one of its only users. You can probably be a bit aggressive,
> remove support for incomplete device trees, see if anyone complains, and
> they do, revert the removal.

Can mv88e6060 functionality be transferred to mv88e6xxx?

Re: Regression: supported_interfaces filling enforcement

[Vladimir Oltean]

On Mon, Jul 10, 2023 at 09:09:48PM +0300, Sergei Antonov wrote:
> On Mon, 10 Jul 2023 at 18:38, Vladimir Oltean <olteanv@gmail.com> wrote:
> 
> > That being said, given the kind of bugs I've seen uncovered in this
> > driver recently, I'd say it would be ridiculous to play pretend - you're
> > probably one of its only users. You can probably be a bit aggressive,
> > remove support for incomplete device trees, see if anyone complains, and
> > they do, revert the removal.
> 
> Can mv88e6060 functionality be transferred to mv88e6xxx?

Honestly, I don't know.

Re: Regression: supported_interfaces filling enforcement

[Andrew Lunn]

On Wed, Jul 12, 2023 at 12:58:48AM +0300, Vladimir Oltean wrote:
> On Mon, Jul 10, 2023 at 09:09:48PM +0300, Sergei Antonov wrote:
> > On Mon, 10 Jul 2023 at 18:38, Vladimir Oltean <olteanv@gmail.com> wrote:
> > 
> > > That being said, given the kind of bugs I've seen uncovered in this
> > > driver recently, I'd say it would be ridiculous to play pretend - you're
> > > probably one of its only users. You can probably be a bit aggressive,
> > > remove support for incomplete device trees, see if anyone complains, and
> > > they do, revert the removal.
> > 
> > Can mv88e6060 functionality be transferred to mv88e6xxx?
> 
> Honestly, I don't know.

I think Vivien looked at that once, but decided against it. I don't
remember why. Maybe because of lack of hardware, or anybody to test
it?

	Andrew

Re: Regression: supported_interfaces filling enforcement

[Russell King (Oracle)]

On Mon, Jul 10, 2023 at 05:35:35PM +0300, Sergei Antonov wrote:
> &mdio1 {
>         status = "okay";
> 
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         switch@10 {
>                 compatible = "marvell,mv88e6060";
>                 reg = <0x10>;
> 
>                 ports {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> 
>                         port@0 {
>                                 reg = <0>;
>                                 label = "lan2";
>                         };
> 
>                         port@1 {
>                                 reg = <1>;
>                                 label = "lan3";
>                         };
> 
>                         port@2 {
>                                 reg = <2>;
>                                 label = "lan1";
>                         };
> 
>                         port@5 {
>                                 reg = <5>;
>                                 label = "cpu";
>                                 ethernet = <&mac1>;
>                                 phy-mode = "mii";

While looking at the datasheet for this switch, it supports SNI,
MII MAC (mii), MII PHY (revmii), and RMII PHY (revrmii) modes.

Is your port 5 actually configured for "revmii" rather than "mii"?

Thanks.

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

Re: Regression: supported_interfaces filling enforcement

[Russell King (Oracle)]

On Mon, Jul 10, 2023 at 03:35:56PM +0300, Vladimir Oltean wrote:
> On Tue, Jul 04, 2023 at 05:28:47PM +0300, Sergei Antonov wrote:
> > Hello!
> > This commit seems to break the mv88e6060 dsa driver:
> > de5c9bf40c4582729f64f66d9cf4920d50beb897    "net: phylink: require
> > supported_interfaces to be filled"
> > 
> > The driver does not fill 'supported_interfaces'. What is the proper
> > way to fix it? I managed to fix it by the following quick code.
> > Comments? Recommendations?
> 
> Ok, it seems that commit de5c9bf40c45 ("net: phylink: require
> supported_interfaces to be filled") was based on a miscalculation.

Yes, it seems so. I'm not great with dealing with legacy stuff - which
is something I've stated time and time again when drivers fall behind
with phylink development. There's only so much that I can hold in my
head, and I can't runtime test the legacy stuff.

I suspect two other DSA drivers are also broken by this:

drivers/net/dsa/dsa_loop.c
drivers/net/dsa/realtek/rtl8366rb.c

based upon:

$ grep -lr dsa_switch_ops drivers/net/dsa | xargs grep -L '\.phylink_get_caps.*=' | xargs grep -L '\.adjust_link'

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

Re: Regression: supported_interfaces filling enforcement

[Linux regression tracking (Thorsten Leemhuis)]

[CCing the regression list, as it should be in the loop for regressions:
https://docs.kernel.org/admin-guide/reporting-regressions.html]

On 13.07.23 17:08, Russell King (Oracle) wrote:
> On Mon, Jul 10, 2023 at 03:35:56PM +0300, Vladimir Oltean wrote:
>> On Tue, Jul 04, 2023 at 05:28:47PM +0300, Sergei Antonov wrote:
>>> This commit seems to break the mv88e6060 dsa driver:
>>> de5c9bf40c4582729f64f66d9cf4920d50beb897    "net: phylink: require
>>> supported_interfaces to be filled"
>>>
>>> The driver does not fill 'supported_interfaces'. What is the proper
>>> way to fix it? I managed to fix it by the following quick code.
>>> Comments? Recommendations?
>>
>> Ok, it seems that commit de5c9bf40c45 ("net: phylink: require
>> supported_interfaces to be filled") was based on a miscalculation.
> 
> Yes, it seems so. I'm not great with dealing with legacy stuff - which
> is something I've stated time and time again when drivers fall behind
> with phylink development. There's only so much that I can hold in my
> head, and I can't runtime test the legacy stuff.
> 
> I suspect two other DSA drivers are also broken by this:
> 
> drivers/net/dsa/dsa_loop.c
> drivers/net/dsa/realtek/rtl8366rb.c
> 
> based upon:
> 
> $ grep -lr dsa_switch_ops drivers/net/dsa | xargs grep -L '\.phylink_get_caps.*=' | xargs grep -L '\.adjust_link'

What happened to this regression? From here it looks like things
stalled, but I might have missed something, hence allow me to ask:

Is this still happening? Is anyone still working on fixing this?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

Re: Regression: supported_interfaces filling enforcement

[Russell King (Oracle)]

On Tue, Jul 25, 2023 at 12:58:31PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> [CCing the regression list, as it should be in the loop for regressions:
> https://docs.kernel.org/admin-guide/reporting-regressions.html]
> 
> On 13.07.23 17:08, Russell King (Oracle) wrote:
> > On Mon, Jul 10, 2023 at 03:35:56PM +0300, Vladimir Oltean wrote:
> >> On Tue, Jul 04, 2023 at 05:28:47PM +0300, Sergei Antonov wrote:
> >>> This commit seems to break the mv88e6060 dsa driver:
> >>> de5c9bf40c4582729f64f66d9cf4920d50beb897    "net: phylink: require
> >>> supported_interfaces to be filled"
> >>>
> >>> The driver does not fill 'supported_interfaces'. What is the proper
> >>> way to fix it? I managed to fix it by the following quick code.
> >>> Comments? Recommendations?
> >>
> >> Ok, it seems that commit de5c9bf40c45 ("net: phylink: require
> >> supported_interfaces to be filled") was based on a miscalculation.
> > 
> > Yes, it seems so. I'm not great with dealing with legacy stuff - which
> > is something I've stated time and time again when drivers fall behind
> > with phylink development. There's only so much that I can hold in my
> > head, and I can't runtime test the legacy stuff.
> > 
> > I suspect two other DSA drivers are also broken by this:
> > 
> > drivers/net/dsa/dsa_loop.c
> > drivers/net/dsa/realtek/rtl8366rb.c
> > 
> > based upon:
> > 
> > $ grep -lr dsa_switch_ops drivers/net/dsa | xargs grep -L '\.phylink_get_caps.*=' | xargs grep -L '\.adjust_link'
> 
> What happened to this regression? From here it looks like things
> stalled, but I might have missed something, hence allow me to ask:
> 
> Is this still happening? Is anyone still working on fixing this?

I think the discussion got side-tracked into whether mv88e6060 should
be merged into mv88e6xxx, and then just petered out with no further
patch(es) - plus I was on holiday so obviously wasn't paying much
attention.

I suppose the sane thing to do would be to fix all drivers in one
go - maybe something like this:

-	if (ds->ops->phylink_get_caps)
+	if (ds->ops->phylink_get_caps) {
 		ds->ops->phylink_get_caps(ds, dp->index, &dp->pl_config);
+	} else {
+		/* For legacy drivers */
+		__set_bit(PHY_INTERFACE_MODE_INTERNAL,
+			  &dp->pl_config.supported_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_GMII,
+			  &dp->pl_config.supported_interfaces);
+	}

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

Re: Regression: supported_interfaces filling enforcement

[Vladimir Oltean]

On Tue, Jul 25, 2023 at 12:26:00PM +0100, Russell King (Oracle) wrote:
> On Tue, Jul 25, 2023 at 12:58:31PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> > [CCing the regression list, as it should be in the loop for regressions:
> > https://docs.kernel.org/admin-guide/reporting-regressions.html]
> > 
> > On 13.07.23 17:08, Russell King (Oracle) wrote:
> > > On Mon, Jul 10, 2023 at 03:35:56PM +0300, Vladimir Oltean wrote:
> > >> On Tue, Jul 04, 2023 at 05:28:47PM +0300, Sergei Antonov wrote:
> > >>> This commit seems to break the mv88e6060 dsa driver:
> > >>> de5c9bf40c4582729f64f66d9cf4920d50beb897    "net: phylink: require
> > >>> supported_interfaces to be filled"
> > >>>
> > >>> The driver does not fill 'supported_interfaces'. What is the proper
> > >>> way to fix it? I managed to fix it by the following quick code.
> > >>> Comments? Recommendations?
> > >>
> > >> Ok, it seems that commit de5c9bf40c45 ("net: phylink: require
> > >> supported_interfaces to be filled") was based on a miscalculation.
> > > 
> > > Yes, it seems so. I'm not great with dealing with legacy stuff - which
> > > is something I've stated time and time again when drivers fall behind
> > > with phylink development. There's only so much that I can hold in my
> > > head, and I can't runtime test the legacy stuff.
> > > 
> > > I suspect two other DSA drivers are also broken by this:
> > > 
> > > drivers/net/dsa/dsa_loop.c
> > > drivers/net/dsa/realtek/rtl8366rb.c
> > > 
> > > based upon:
> > > 
> > > $ grep -lr dsa_switch_ops drivers/net/dsa | xargs grep -L '\.phylink_get_caps.*=' | xargs grep -L '\.adjust_link'
> > 
> > What happened to this regression? From here it looks like things
> > stalled, but I might have missed something, hence allow me to ask:
> > 
> > Is this still happening? Is anyone still working on fixing this?
> 
> I think the discussion got side-tracked into whether mv88e6060 should
> be merged into mv88e6xxx, and then just petered out with no further
> patch(es) - plus I was on holiday so obviously wasn't paying much
> attention.
> 
> I suppose the sane thing to do would be to fix all drivers in one
> go - maybe something like this:
> 
> -	if (ds->ops->phylink_get_caps)
> +	if (ds->ops->phylink_get_caps) {
>  		ds->ops->phylink_get_caps(ds, dp->index, &dp->pl_config);
> +	} else {
> +		/* For legacy drivers */
> +		__set_bit(PHY_INTERFACE_MODE_INTERNAL,
> +			  &dp->pl_config.supported_interfaces);
> +		__set_bit(PHY_INTERFACE_MODE_GMII,
> +			  &dp->pl_config.supported_interfaces);
> +	}
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Yup, dsa_loop is broken too, I've just tested it by enabling
CONFIG_NET_DSA_LOOP=y and booting on a board which has an existing
interface named eth0 (this will be used as fake DSA master):

[    4.936197] dsa-loop fixed-0:1f: skipping link registration for CPU port 5
[    4.944316]  (null): phylink: error: empty supported_interfaces
[    4.950551] error creating PHYLINK: -22
[    4.954467] dsa-loop fixed-0:1f lan1 (uninitialized): error -22 setting up PHY for tree 0, switch 0, port 0
[    4.964869]  (null): phylink: error: empty supported_interfaces
[    4.970932] error creating PHYLINK: -22
[    4.974833] dsa-loop fixed-0:1f lan2 (uninitialized): error -22 setting up PHY for tree 0, switch 0, port 1
[    4.985131]  (null): phylink: error: empty supported_interfaces
[    4.991120] error creating PHYLINK: -22
[    4.995002] dsa-loop fixed-0:1f lan3 (uninitialized): error -22 setting up PHY for tree 0, switch 0, port 2
[    5.005277]  (null): phylink: error: empty supported_interfaces
[    5.011267] error creating PHYLINK: -22
[    5.015148] dsa-loop fixed-0:1f lan4 (uninitialized): error -22 setting up PHY for tree 0, switch 0, port 3
[    5.026292] DSA: tree 0 setup
[    5.029282] dsa-loop fixed-0:1f: DSA mockup driver: 0x1f

I've also tested that your patch (fixed so that it doesn't take the "&"
address of supported_interfaces, which is already an array type)
restores the functionality:

[    4.949944] dsa-loop fixed-0:1f: skipping link registration for CPU port 5
[    4.957757] dsa-loop fixed-0:1f lan1 (uninitialized): PHY [dsa-0.0:00] driver [Generic PHY] (irq=POLL)
[    4.973085] dsa-loop fixed-0:1f lan2 (uninitialized): PHY [dsa-0.0:01] driver [Generic PHY] (irq=POLL)
[    4.986189] dsa-loop fixed-0:1f lan3 (uninitialized): PHY [dsa-0.0:02] driver [Generic PHY] (irq=POLL)
[    4.998763] dsa-loop fixed-0:1f lan4 (uninitialized): PHY [dsa-0.0:03] driver [Generic PHY] (irq=POLL)
[    5.012044] DSA: tree 0 setup
[    5.015040] dsa-loop fixed-0:1f: DSA mockup driver: 0x1f

Russell, can you please go ahead and turn the short-term fix into a
formal patch?

Once I get rid of some of the pending stuff that's keeping me busy,
I've made a note to not let the short-term workarounds (this and
dsa_port_phylink_validate() avoiding phylink_generic_validate() if
mac_capabilities isn't provided) turn into long-term workarounds that
new drivers, too, might start relying on.