netdev - Re: [PATCH net-next] net: dsa: sja1105: use detected device id instead of DT one on mismatch

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: <20200805205938.2d2bhrkyeu3lgioe@skbuf>
Date:   Wed, 5 Aug 2020 23:59:38 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        andrew@...n.ch, vivien.didelot@...il.com
Subject: Re: [PATCH net-next] net: dsa: sja1105: use detected device id
 instead of DT one on mismatch

On Tue, Aug 04, 2020 at 08:01:46PM -0700, Florian Fainelli wrote:
> On 8/4/2020 3:59 PM, David Miller wrote:
> > From: Vladimir Oltean <olteanv@...il.com>
> > Date: Mon,  3 Aug 2020 19:48:23 +0300
> > 
> >> Although we can detect the chip revision 100% at runtime, it is useful
> >> to specify it in the device tree compatible string too, because
> >> otherwise there would be no way to assess the correctness of device tree
> >> bindings statically, without booting a board (only some switch versions
> >> have internal RGMII delays and/or an SGMII port).
> >>
> >> But for testing the P/Q/R/S support, what I have is a reworked board
> >> with the SJA1105T replaced by a pin-compatible SJA1105Q, and I don't
> >> want to keep a separate device tree blob just for this one-off board.
> >> Since just the chip has been replaced, its RGMII delay setup is
> >> inherently the same (meaning: delays added by the PHY on the slave
> >> ports, and by PCB traces on the fixed-link CPU port).
> >>
> >> For this board, I'd rather have the driver shout at me, but go ahead and
> >> use what it found even if it doesn't match what it's been told is there.
> >>
> >> [    2.970826] sja1105 spi0.1: Device tree specifies chip SJA1105T but found SJA1105Q, please fix it!
> >> [    2.980010] sja1105 spi0.1: Probed switch chip: SJA1105Q
> >> [    3.005082] sja1105 spi0.1: Enabled switch tagging
> >>
> >> Signed-off-by: Vladimir Oltean <olteanv@...il.com>
> > 
> > Andrew/Florian, do we really want to set a precedence for doing this
> > kind of fallback in our drivers?
> 
> Not a big fan of it, and the justification is a little bit weak IMHO,
> especially since one could argue that the boot agent providing the FDT
> could do that check and present an appropriate compatible string to the
> kernel.

I'll admit I'm not a huge fan either, but what you're suggesting is
basically to move the whole problem one level lower (somebody would
still have to be aware about the device id mismatch). I _was_ going to
eventually patch the U-Boot driver to adapt to the real device id too,
but only for its own use of networking. I am an even smaller fan of
having to do a fdt fixup from U-Boot, then I'd have to rely on that
always being there to do its job properly.

I've been using this board with a local fdt blob for more than one year
now, but it's inconvenient for me to have custom tftp commands for this
one board only. I hope I'm not setting for a behavior that might be
abused, tbh I don't really see how. At the end of the day though, I
don't see why the driver would have to be as punishing as to refuse to
probe when it can, warning is more than enough.

Thanks,
-Vladimir