Re: [RFC PATCH net-next v4 01/28] net: mdio: ipq8064: clean whitespaces in define

On Sat, May 15, 2021 at 07:08:57PM +0100, Jonathan McDowell wrote:

On Sat, May 15, 2021 at 07:30:26PM +0200, Ansuel Smith wrote:

quoted

On Sat, May 15, 2021 at 06:00:46PM +0100, Jonathan McDowell wrote:

quoted

On Sat, May 08, 2021 at 08:05:58PM +0200, Ansuel Smith wrote:

quoted

On Sat, May 08, 2021 at 08:02:33PM +0200, Andrew Lunn wrote:

quoted

On Sat, May 08, 2021 at 02:28:51AM +0200, Ansuel Smith wrote:

quoted

Fix mixed whitespace and tab for define spacing.

Please add a patch [0/28] which describes the big picture of what
these changes are doing.

Also, this series is getting big. You might want to split it into two,
One containing the cleanup, and the second adding support for the new
switch.

	Andrew

There is a 0/28. With all the changes. Could be that I messed the cc?
I agree think it's better to split this for the mdio part, the cleanup
and the changes needed for the internal phy.

FWIW I didn't see the 0/28 mail either. I tried these out on my RB3011
today. I currently use the GPIO MDIO driver because I saw issues with
the IPQ8064 driver in the past, and sticking with the GPIO driver I see
both QCA8337 devices and traffic flows as expected, so no obvious
regressions from your changes.

I also tried switching to the IPQ8064 MDIO driver for my first device
(which is on the MDIO0 bus), but it's still not happy:

qca8k 37000000.mdio-mii:10: Switch id detected 0 but expected 13

Can you try the v6 version of this series?

Both the v6 qca8k series and the separate ipq806x mdio series, yes?

quoted

Anyway the fact that 0 is produced instead of a wrong value let me
think that there is some problem with the mdio read. (on other device
there was a problem of wrong id read but nerver 0). Could be that the
bootloader on your board set the mdio MASTER disabled. I experienced
this kind of problem when switching from the dsa driver and the legacy
swconfig driver. On remove of the dsa driver, the swconfig didn't work
as the bit was never cleared by the dsa driver and resulted in your
case. (id 0 read from the mdio)

Do you want to try a quick patch so we can check if this is the case?
(about the cover letter... sorry will check why i'm pushing this
wrong)

There's definitely something odd going on here. I went back to mainline
to see what the situation is there. With the GPIO MDIO driver both
switches work (expected, as this is what I run with). I changed switch0
over to use the IPQ MDIO driver and it wasn't detected (but switch1
still on the GPIO MDIO driver was fine).

I then tried putting both switches onto the IPQ MDIO driver and in that
instance switch0 came up fine, while switch1 wasn't detected.

Oh wait, your board have 2 different switch? So they both use the master
bit when used... Mhhh I need to think about this if there is a clean way
to handle this. The idea would be that one of the 2 dsa switch should
use the already defined mdio bus.

The problem here is that to use the internal mdio bus, a bit must be
set or 0 is read on every value (as the bit actually disable the internal
mdio). This is good if one dsa driver is used but when 2 or more are
used I think this clash and only one of them work. The gpio mdio path is
not affected by this. Will check if I can find some way to address this.

If there's a simple patch that might give more debug I can try it out.

J.

-- 
   "Reality Or Nothing!" -- Cold   |  .''`.  Debian GNU/Linux Developer
              Lazarus              | : :' :  Happy to accept PGP signed
                                   | `. `'   or encrypted mail - RSA
|   `-    key on the keyservers.