[v3,0/6] Support mt7531 on BPI-R2 Pro

From: Frank Wunderlich <frank-w@public-files.de>

This Series add Support for the mt7531 switch on Bananapi R2 Pro board.

This board uses port5 of the switch to conect to the gmac0 of the
rk3568 SoC.

Currently CPU-Port is hardcoded in the mt7530 driver to port 6.

Compared to v1 the reset-Patch was dropped as it was not needed and
CPU-Port-changes are completely rewriten based on suggestions/code from
Vladimir Oltean (many thanks to this).
In DTS Patch i only dropped the status-property that was not
needed/ignored by driver.

Due to the Changes i also made a regression Test on mt7623 bpi-r2 using
mt7530 with cpu-port 6. Tests were done directly (ipv4 config on dsa user
port) and with vlan-aware bridge including vlan that was tagged outgoing
on dsa user port.

Main change is that i now include the binding-changes into the patchset which
i posted separately.

Frank Wunderlich (6):
  dt-bindings: net: dsa: convert binding for mediatek switches
  net: dsa: mt7530: rework mt7530_hw_vlan_{add,del}
  net: dsa: mt7530: rework mt753[01]_setup
  net: dsa: mt7530: get cpu-port via dp->cpu_dp instead of constant
  dt-bindings: net: dsa: make reset optional and add rgmii-mode to
    mt7531
  arm64: dts: rockchip: Add mt7531 dsa node to BPI-R2-Pro board

 .../bindings/net/dsa/mediatek,mt7530.yaml     | 421 ++++++++++++++++++
 .../devicetree/bindings/net/dsa/mt7530.txt    | 327 --------------
 .../boot/dts/rockchip/rk3568-bpi-r2-pro.dts   |  48 ++
 drivers/net/dsa/mt7530.c                      |  82 ++--
 drivers/net/dsa/mt7530.h                      |   1 -
 5 files changed, 522 insertions(+), 357 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/dsa/mt7530.txt

Am Sonntag, 8. Mai 2022, 08:24:37 CEST schrieb Frank Wunderlich:
> Am 7. Mai 2022 22:01:22 MESZ schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
> >On 07/05/2022 19:04, Frank Wunderlich wrote:
> >> From: Frank Wunderlich <frank-w@public-files.de>
> >> 
> >> Make reset optional as driver already supports it, 
> >
> >I do not see the connection between hardware needing or not needing a
> >reset GPIO and a driver supporting it or not... What does it mean?
> 
> My board has a shared gpio-reset between gmac and switch, so both will resetted if it is asserted. Currently it is set to the gmac and is aquired exclusive. Adding it to switch results in 2 problems:
> 
> - due to exclusive and already mapped to gmac, switch driver exits as it cannot get the reset-gpio again.
> - if i drop the reset from gmac and add to switch, it resets the gmac and this takes too long for switch to get up. Of course i can increase the wait time after reset,but dropping reset here was the easier way.
> 
> Using reset only on gmac side brings the switch up.

I think the issue is more for the description itself.

Devicetree is only meant to describe the hardware and does in general don't
care how any firmware (Linux-kernel, *BSD, etc) handles it. So going with
"the kernel does it this way" is not a valid reason for a binding change ;-) .

Instead in general want to reason that there are boards without this reset
facility and thus make it optional for those.

Heiko

> >> allow port 5 as
> >> cpu-port 
> >
> >How do you allow it here?
> 
> Argh, seems i accidentally removed this part and have not recognized while checking :(
> 
> It should only change description of reg for ports to:
> 
> "Port address described must be 5 or 6 for CPU port and from 0 to 5 for user ports."
> 
> regards Frank
>

On Sun, May 8, 2022 at 8:12 AM Frank Wunderlich <frank-w@public-files.de> wrote:
>
> Hi Heiko
>
> > Gesendet: Sonntag, 08. Mai 2022 um 11:41 Uhr
> > Von: "Heiko Stuebner" <heiko@sntech.de>
> > Am Sonntag, 8. Mai 2022, 08:24:37 CEST schrieb Frank Wunderlich:
> > > Am 7. Mai 2022 22:01:22 MESZ schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
> > > >On 07/05/2022 19:04, Frank Wunderlich wrote:
> > > >> From: Frank Wunderlich <frank-w@public-files.de>
> > > >>
> > > >> Make reset optional as driver already supports it,
> > > >
> > > >I do not see the connection between hardware needing or not needing a
> > > >reset GPIO and a driver supporting it or not... What does it mean?
> > >
> > > My board has a shared gpio-reset between gmac and switch, so both will resetted if it
> > > is asserted. Currently it is set to the gmac and is aquired exclusive. Adding it to switch results in 2 problems:
> > >
> > > - due to exclusive and already mapped to gmac, switch driver exits as it cannot get the reset-gpio again.
> > > - if i drop the reset from gmac and add to switch, it resets the gmac and this takes too long for switch
> > > to get up. Of course i can increase the wait time after reset,but dropping reset here was the easier way.
> > >
> > > Using reset only on gmac side brings the switch up.
> >
> > I think the issue is more for the description itself.
> >
> > Devicetree is only meant to describe the hardware and does in general don't
> > care how any firmware (Linux-kernel, *BSD, etc) handles it. So going with
> > "the kernel does it this way" is not a valid reason for a binding change ;-) .
> >
> > Instead in general want to reason that there are boards without this reset
> > facility and thus make it optional for those.
>
> if only the wording is the problem i try to rephrase it from hardware PoV.
>
> maybe something like this?
>
> https://github.com/frank-w/BPI-R2-4.14/commits/5.18-mt7531-mainline2/Documentation/devicetree/bindings/net/dsa/mediatek%2Cmt7530.yaml
>
> Another way is maybe increasing the delay after the reset (to give more time all
> come up again), but imho it is no good idea resetting the gmac/mdio-bus from the
> child device.
>
> have not looked into the gmac driver if this always  does the initial reset to
> have a "clean state". In this initial reset the switch will be resetted too
> and does not need an additional one which needs the gmac/mdio initialization
> to be done again.

For clarification, the reset gpio line is purely to reset the phy.
If having the switch driver own the reset gpio instead of the gmac
breaks initialization that means there's a bug in the gmac driver
handling phy init.
In testing I've seen issues moving the reset line to the mdio node
with other phys and the stmmac gmac driver, so I do believe this is
the case.

>
> > > >> allow port 5 as
> > > >> cpu-port
> > > >
> > > >How do you allow it here?
> > >
> > > Argh, seems i accidentally removed this part and have not recognized while checking :(
> > >
> > > It should only change description of reg for ports to:
> > >
> > > "Port address described must be 5 or 6 for CPU port and from 0 to 5 for user ports."
>
> noticed that the target-phase is not removed but squashed in the first bindings-patch.
> This was a rebasing error and not intented...will fix in next version.
>
> regards Frank

just a gentle ping, is anything missing/wrong?

regards Frank

On Wed, 8 Jun 2022 19:38:30 +0200 Frank Wunderlich wrote:
> just a gentle ping, is anything missing/wrong?

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#how-can-i-tell-the-status-of-a-patch-i-ve-sent

In your case:

https://patchwork.kernel.org/project/netdevbpf/list/?series=639420&state=*

I haven't double checked but even if the feedback you received was
waved the patch didn't apply when it was posted, so you gotta repost.