Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220620150225.1307946-1-mw@semihalf.com> <20220620150225.1307946-11-mw@semihalf.com>
 <YrC9KpEuYCgHv14l@lunn.ch>
In-Reply-To: <YrC9KpEuYCgHv14l@lunn.ch>
From:   Marcin Wojtas <mw@semihalf.com>
Date:   Tue, 21 Jun 2022 01:31:17 +0200
Message-ID: <CAPv3WKeHpB0v1udzSAjm3QnRoCaVTUqiOa7f_MvAbgMOWDDEAg@mail.gmail.com>
Subject: Re: [net-next: PATCH 10/12] net: dsa: add ACPI support
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
        netdev <netdev@vger.kernel.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Len Brown <lenb@kernel.org>, vivien.didelot@gmail.com,
        Florian Fainelli <f.fainelli@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>, pabeni@redhat.com,
        Russell King - ARM Linux <linux@armlinux.org.uk>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Grzegorz Bernacki <gjb@semihalf.com>,
        Grzegorz Jaszczyk <jaz@semihalf.com>,
        Tomasz Nowicki <tn@semihalf.com>,
        Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>,
        upstream@semihalf.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

pon., 20 cze 2022 o 20:32 Andrew Lunn <andrew@lunn.ch> napisa=C5=82(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 po=
rt #%d\n",
> > +                      dp->index);
> > +             dp->type =3D 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