Date:   Mon, 13 Feb 2023 15:36:22 +0200
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Richard van Schagen <richard@routerhints.com>
Cc:     "arinc9.unal@gmail.com" <arinc9.unal@gmail.com>,
        Sean Wang <sean.wang@mediatek.com>,
        Landen Chao <Landen.Chao@mediatek.com>,
        DENG Qingfang <dqfext@gmail.com>, Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        =?utf-8?B?QXLEsW7DpyDDnE5BTA==?= <arinc.unal@arinc9.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-mediatek@lists.infradead.org" 
        <linux-mediatek@lists.infradead.org>,
        "erkin.bozoglu@xeront.com" <erkin.bozoglu@xeront.com>
Subject: Re: [PATCH net] net: dsa: mt7530: fix CPU flooding and do not set
 CPU association
Message-ID: <20230213133622.vatoblghocxlq4lo@skbuf>
References: <20230210172822.12960-1-richard@routerhints.com>
 <20230210172822.12960-1-richard@routerhints.com>
 <20230210184409.e6ueolfdsmhqfph5@skbuf>
 <829A471E-D1FB-4DC0-95FF-481A73E6E8E7@routerhints.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <829A471E-D1FB-4DC0-95FF-481A73E6E8E7@routerhints.com>
Precedence: bulk

On Sat, Feb 11, 2023 at 08:52:20PM +0000, Richard van Schagen wrote:
> > FWIW, the problem can also be solved similar to 8d5f7954b7c8 ("net: dsa:
> > felix: break at first CPU port during init and teardown"), and both CPU
> > ports could be added to the flooding mask only as part of the "multiple
> > CPU ports" feature. When a multiple CPU ports device tree is used with a
> > kernel capable of a single CPU port, your patch enables flooding towards
> > the second CPU port, which will never be used (or up). Not sure if you
> > want that.
> 
> So basically that means the wrong DTS with a kernel? Isn’t that similar to the wrong DTS 
> for a device?

"that" meaning what? A device tree specifying that there are two CPU
ports, used with a kernel that only sets up the first CPU port?
Why would that be the same as a wrong DTS? What's wrong about it?

FYI, there exists an Arm certification program called SystemReady IR,
where the goal is for one firmware image (U-Boot with bootefi) to boot a
number of different embedded Linux or BSD distributions, having different
kernel versions. With this boot flow, currently there is no concept to
get a device tree from the OS, so using EFI_FDT_USE_INTERNAL, U-Boot
will provide its own runtime device tree to the booted OS.

In U-Boot there are efforts for several SoCs to periodically sync up
their device trees with the latest Linux device trees, in order for the
most complete hardware description to be available to all booted
distributions. U-Boot drivers are also expected to work with the same
device trees that Linux uses.

The work of these people is made unnecessarily harder by mentalities
like this, that there's such a thing as a "wrong DTS with a kernel".
Generally, the default expectation is that at least for a time window,
kernels do something sensible when given device trees newer than them
(forward compatibility). This has always been a guideline for device
tree usage, and with SystemReady IR, it makes it possible for one
firmware image to support distros having different kernel versions.

The current DSA device tree binding implementation (in the framework)
has always been careful to ignore other CPU ports present in the device
tree and just use the first one, until it gained support for changing
the default assignment.

Drivers which have forward or backward compatibility bugs can reasonably
have those bugs fixed, those fixes included into the stable LTS kernels,
and from there, integrated into distributions.

Now, if you are certain that Mediatek SoCs are not used in this
particular way, and there is some strong reason to not make an effort to
preserve forward compatibility, that is entirely a separate discussion.

> Port 5 / GMAC1 can be used as <ethphy0>, <ethphy4>, external phy on port 5.
> e.g. SPF port on port 5, or used as second CPU port.

And one would expect this information to be accurately described in the
device tree. Is there any reason to not trust the correctness of the
device tree?

> Not sure how we could prevent that?

Prevent what? Flooding to an unused CPU port?