Re: [net-next: PATCH 10/12] net: dsa: add ACPI support

[Marcin Wojtas]

pon., 20 cze 2022 o 20:32 Andrew Lunn <andrew@xxxxxxx> napisał(a):
>
> >  static int dsa_port_parse_dsa(struct dsa_port *dp)
> >  {
> > +     /* Cascade switch connection is not supported in ACPI world. */
> > +     if (is_acpi_node(dp->fwnode)) {
> > +             dev_warn(dp->ds->dev,
> > +                      "DSA type is not supported with ACPI, disable port #%d\n",
> > +                      dp->index);
> > +             dp->type = DSA_PORT_TYPE_UNUSED;
> > +             return 0;
> > +     }
> > +
>
> Did you try this? I'm not sure it will work correctly. When a switch
> registers with the DSA core, the core will poke around in DT and fill
> in various bits of information, including the DSA links. Once that has
> completed, the core will look at all the switches registered so far
> and try to determine if it has a complete set, i.e, it has both ends
> of all DSA links. If it does have a complete set, it then calls the
> setup methods on each switch, and gets them configured. If it finds it
> does not have a complete setup, it does nothing, waiting for the next
> switch to register.
>
> So if somebody passed an ACPI description with multiple switches, it
> is likely to call the setup methods as soon as the first switch is
> registered. And it might call those same setup methods a second time,
> when the second switch registers, on both switches. And when the third
> switch registers, it will probably call the setup methods yet again on
> all the switches....
>
> You will have a much safer system if you return -EINVAL if you find a
> DSA link in ACPI. That should abort the switch probe.
>

I only set a single port to "dsa" label to check if this condition is
entered. I see 2 devices in the arm64 tree (fsl-lx2160a-bluebox3.dts
and armada-3720-turris-mox.dts) that support cascade switches via
"link" property. I don't have access to real life setup (and those
seem to not support ACPI anyway...).

In case this temporarily would remain as unsupported feature, I agree
-EINVAL is a safer solution.

Thanks,
Marcin