netdev - Re: [PATCH v4 11/23] net: stmmac: add support for dwmac 3.72a

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAFXKEHa5A039NfyGFnXH4pm1FN9YwDuTLtoa9Xn8xzZYwTCvKg@mail.gmail.com>
Date: Sat, 2 Nov 2024 12:58:47 +0100
From: Lothar Rubusch <l.rubusch@...il.com>
To: Rob Herring <robh@...nel.org>
Cc: krzk+dt@...nel.org, a.fatoum@...gutronix.de, conor+dt@...nel.org, 
	dinguyen@...nel.org, marex@...x.de, s.trumtrar@...gutronix.de, 
	alexandre.torgue@...s.st.com, joabreu@...opsys.com, davem@...emloft.net, 
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, 
	mcoquelin.stm32@...il.com, netdev@...r.kernel.org, 
	linux-stm32@...md-mailman.stormreply.com, 
	linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 11/23] net: stmmac: add support for dwmac 3.72a

On Fri, Nov 1, 2024 at 8:35 PM Rob Herring <robh@...nel.org> wrote:
>
> On Tue, Oct 29, 2024 at 08:23:37PM +0000, Lothar Rubusch wrote:
> > The dwmac 3.72a is an ip version that can be found on Intel/Altera Arria10
> > SoCs. Going by the hardware features "snps,multicast-filter-bins" and
> > "snps,perfect-filter-entries" shall be supported. Thus add a
> > compatibility flag, and extend coverage of the driver for the 3.72a.
> >
> > Signed-off-by: Lothar Rubusch <l.rubusch@...il.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c   | 1 +
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c
> > index 598eff926..b9218c07e 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c
> > @@ -56,6 +56,7 @@ static const struct of_device_id dwmac_generic_match[] = {
> >       { .compatible = "snps,dwmac-3.610"},
> >       { .compatible = "snps,dwmac-3.70a"},
> >       { .compatible = "snps,dwmac-3.710"},
> > +     { .compatible = "snps,dwmac-3.72a"},
> >       { .compatible = "snps,dwmac-4.00"},
> >       { .compatible = "snps,dwmac-4.10a"},
> >       { .compatible = "snps,dwmac"},
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > index 54797edc9..e7e2d6c20 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > @@ -522,6 +522,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
> >       if (of_device_is_compatible(np, "st,spear600-gmac") ||
> >               of_device_is_compatible(np, "snps,dwmac-3.50a") ||
> >               of_device_is_compatible(np, "snps,dwmac-3.70a") ||
> > +             of_device_is_compatible(np, "snps,dwmac-3.72a") ||
>
> All these of_device_is_compatible() checks should really go away and all
> the settings just come from match table data. Then everything is const
> and we're not matching multiple times at run-time. That would be a bit
> of refactoring though...

I can see your point, but I have some doubts on that. As I asked
before, my current scope is actually on upstreaming my .dts/.dtsi
files. This would be nice, since I'm doing it in parallel in u-boot
and they're going to dts/upstream so we're waiting on the kernel
community here.

I'm unclear what additionally to fixing my .dts files is needed. Shall
I fix all? / some? / none? of the errors your dtbs check bot gives?
First, most of the errors come from not covered bindings. Since TXT
files for socfpga never were converted to YAML. Should I do this? I
guess this would increase coverage of dt-bindings check and reduce the
errors tremendously. But I'd see it as a different patch set if so. Is
this needed to upstream my .dts files?
Second, the maintainer of platform socfpga seems to be currently on a
sabbatical. At least I hope he's fine and not sick or burned out. But
also I would like to get some feedback on the .dts files. Please help
me if you can give me some.
Third, when I update things like the mentioned dwmac things and try to
update the driver compatible to best of my knowledge, I understand
that it would be better to refactor a redundantly checked
compatibility to just one check in the driver.
But, I only have one hardware: an Arria10 SOM which is affected here.
And, is it really anyway needed to refactor networking drivers for
upstreamining my .dts/.dtsi files at the end of the day?

Please don't missunderstand me. I can absolutely see your point here
and appreciate all type of feedback. Anyway IMHO these are all
different topics i.e. different sets of patches, or smells a bit like
scope creep. I separated the 2 patches for a different set, as Jakub
wrote.