netdev - DSA mv88e6xxx

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

Date:   Thu, 2 Feb 2023 15:18:37 +0000
From:   "Valek, Andrej" <andrej.valek@...mens.com>
To:     "vivien.didelot@...il.com" <vivien.didelot@...il.com>,
        "andrew@...n.ch" <andrew@...n.ch>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: DSA mv88e6xxx_probe

Hello everyone!

I have a switch mv88e6085 which is connected via MDIO bus to iMX.8 SoC.

Switch is not being detected during booting because the address is
different (due to uninitialized PINs from DTB). The problem is, that
switch has to be reset during boot phase, but it isn't.

So I would like to ask you maybe a generic question about
devm_gpiod_get_optional function inside mv88e6xxx_probe.

Is this "chip->reset = devm_gpiod_get_optional(dev, "reset",
GPIOD_OUT_LOW);" line really do the reset? Because from the lines below
looks like, but the reset pulse hasn't been made. Measured with scope.

> chip->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> if (IS_ERR(chip->reset))
>	goto out;
>
> if (chip->reset)
>	usleep_range(1000, 2000);

So it should wait, but for what?

I see an other lines down bellow
> mv88e6xxx_reg_lock(chip);
> err = mv88e6xxx_switch_reset(chip);
> mv88e6xxx_reg_unlock(chip);

but they are deeper after the "mv88e6xxx_detect" which failed, because
it can't find the switch.
> [3.229659] mv88e6085: probe of 5b040000.ethernet-1:10 failed with
error -110

So I "hard-coded" the real reset there:
> chip->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> if (IS_ERR(chip->reset))
>	goto out;
>
> if (chip->reset) {
> 	gpiod_set_value_cansleep(chip->reset, 1);
> 	usleep_range(10000, 20000);
> 	gpiod_set_value_cansleep(chip->reset, 0);
> 	usleep_range(10000, 20000);

and switch was correctly founded:
> [    4.022175] mv88e6085 5b040000.ethernet-1:10: switch 0x3100
detected: Marvell 88E6321, revision 2
> [    4.210834] mv88e6085 5b040000.ethernet-1:10: configuring for
fixed/ link mode
> [    4.218587] mv88e6085 5b040000.ethernet-1:10: Link is Up -
1Gbps/Full - flow control off
...

So my question is, how the reset really works, or there is some kind of
potential bug?

Many thanks for your explanation.
Andrej