netdev - Re: mt7530: dsa_switch_parse_of() fails, causes probe code to run twice

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZDnnjcG5uR9gQrUb@makrotopia.org>
Date:   Sat, 15 Apr 2023 00:53:49 +0100
From:   Daniel Golle <daniel@...rotopia.org>
To:     Arınç ÜNAL <arinc.unal@...nc9.com>
Cc:     Vladimir Oltean <olteanv@...il.com>,
        Frank Wunderlich <frank-w@...lic-files.de>,
        netdev <netdev@...r.kernel.org>, erkin.bozoglu@...ont.com
Subject: Re: mt7530: dsa_switch_parse_of() fails, causes probe code to run
 twice

On Sat, Apr 15, 2023 at 02:23:16AM +0300, Arınç ÜNAL wrote:
> On 15.04.2023 01:48, Daniel Golle wrote:
> > On Sat, Apr 15, 2023 at 01:41:07AM +0300, Arınç ÜNAL wrote:
> > > Hey there,
> > > 
> > > I've been working on the MT7530 DSA subdriver. While doing some tests, I
> > > realised mt7530_probe() runs twice. I moved enabling the regulators from
> > > mt7530_setup() to mt7530_probe(). Enabling the regulators there ends up
> > > with exception warnings on the first time. It works fine when
> > > mt7530_probe() is run again.
> > > 
> > > This should not be an expected behaviour, right? Any ideas how we can make
> > > it work the first time?
> > 
> > Can you share the patch or work-in-progress tree which will allow me
> > to reproduce this problem?
> 
> I tested this on vanilla 6.3-rc6. There's just the diff below that is
> applied. I encountered it on the standalone MT7530 on my Bananapi BPI-R2. I
> haven't tried it on MCM MT7530 on MT7621 SoC yet.
> 
> > 
> > It can of course be that regulator driver has not yet been loaded on
> > the first run and -EPROBE_DEFER is returned in that case. Knowing the
> > value of 'err' variable below would hence be valuable information.
> 
> Regardless of enabling the regulator on either mt7530_probe() or
> mt7530_setup(), dsa_switch_parse_of() always fails.

So dsa_switch_parse_of() can return -EPROBE_DEFER if the ethernet
driver responsible for the CPU port has not yet been loaded.

See net/dsa/dsa.c (inside function dsa_port_parse_of):
[...]
1232)                master = of_find_net_device_by_node(ethernet);
1233)                of_node_put(ethernet);
1234)                if (!master)
1235)                        return -EPROBE_DEFER;
[...]

Hence it would be important to include the value of 'err' in your
debugging printf output, as -EPROBE_DEFER can be an expected and
implicitely intended reality and nothing is wrong then.