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]

Date:   Sun, 16 Apr 2023 12:38:03 +0300
From:   Arınç ÜNAL <arinc.unal@...nc9.com>
To:     Vladimir Oltean <olteanv@...il.com>,
        Daniel Golle <daniel@...rotopia.org>
Cc:     Frank Wunderlich <frank-w@...lic-files.de>,
        netdev <netdev@...r.kernel.org>, erkin.bozoglu@...ont.com,
        Thibaut <hacks@...shdirt.org>
Subject: Re: mt7530: dsa_switch_parse_of() fails, causes probe code to run
 twice

On 15.04.2023 17:57, Arınç ÜNAL wrote:
> On 15.04.2023 17:20, Vladimir Oltean wrote:
>> On Sat, Apr 15, 2023 at 03:02:10PM +0100, Daniel Golle wrote:
>>> As the PHYs are accessed over the MDIO bus which is exposed by the 
>>> mt7530.c
>>> DSA driver the only middle ground would possibly be to introduce a MFD
>>> driver taking care of creating the bus access regmap (MDIO vs. MDIO) and
>>> expose the mt7530-controlled MDIO bus.
>>
>> Which is something I had already mentioned as a possible way forward in
>> the other thread. One would need to take care of ensuring a reasonable
>> migration path in terms of device tree compatibility though.
>>
>>>
>>> Obviously that'd be a bit more work than just moving some things from 
>>> the
>>> switch setup function to the probe function...
>>
>> On the other hand, it would actually work reliably, and would not depend
>> on whomever wanted to reorder things just a little bit differently for
>> his system to probe faster.
> 
> Ok thanks. I will investigate how the switch would be set up with an MFD 
> driver, and how it would affect dt-bindings.
> 
> Looking back at my patch series, currently with this [0], SGMII on 
> MT7531BE's port 6 starts working, and with Daniel's addition [1], the 
> regulator warnings disappear.
> 
> I will submit the patch series as an RFC after addressing Daniel's 
> inline functions suggestion.

I've been giving this some thought. My understanding of probe in this 
context has changed drastically. The probe here is supposed to probe the 
driver, like setting up the pointers, reading from the devicetree, 
filling up the info table, and finally calling dsa_register_switch(). It 
would not necessarily do anything to the switch hardware like resetting 
and reading information from the registers. This is currently how 
mt7530-mdio and mt7530-mmio already operate. So I'm not going to move 
anything from setup to probe.

The duplicate code on mt7530_setup() and mt7531_setup() could rather be 
put on mt753x_setup() instead. But now there's ID_MT7988 also going 
through mt753x_setup, so it's not very feasible to do this anymore, too 
many ID checks there would be.

Moving forward, I will send a separate bugfix patch that makes port 6 on 
MT7531BE work. My patch series will solely be for improving the driver.

Daniel, can you confirm this patch is enough to make port 6 work on 
MT7531BE? I won't touch the PCS creation code here as it'd be an 
improvement rather than a fix, if this works.

https://github.com/arinc9/linux/commit/bb55b97b8f600cf28433e7ff494d296a15191cb3

Arınç