mbox series

[net,v2,0/2] lantiq: GSWIP: two more fixes

Message ID 20210408183828.1907807-1-martin.blumenstingl@googlemail.com
Headers show
Series lantiq: GSWIP: two more fixes | expand

Message

Martin Blumenstingl April 8, 2021, 6:38 p.m. UTC
Hello,

after my last patch got accepted and is now in net as commit
3e6fdeb28f4c33 ("net: dsa: lantiq_gswip: Let GSWIP automatically set
the xMII clock") [0] some more people from the OpenWrt community
(many thanks to everyone involved) helped test the GSWIP driver: [1]

It turns out that the previous fix does not work for all boards.
There's no regression, but it doesn't fix as many problems as I
thought. This is why two more fixes are needed:
- the first one solves many (four known but probably there are
  a few extra hidden ones) reported bugs with the GSWIP where no
  traffic would flow. Not all circumstances are fully understood
  but testing shows that switching away from PHY auto polling
  solves all of them
- while investigating the different problems which are addressed
  by the first patch some small issues with the existing code were
  found. These are addressed by the second patch


Changes since v1 at [0]:
- Don't configure the link parameters in gswip_phylink_mac_config
  (as we're using the "modern" way in gswip_phylink_mac_link_up).
  Thanks to Andrew for the hint with the phylink documentation.
- Clarify that GSWIP_MII_CFG_RMII_CLK is ignored by the hardware in
  the description of the second patch as suggested by Hauke
- Don't set GSWIP_MII_CFG_RGMII_IBS in the second patch as we don't
  have any hardware available for testing this. The patch
  description now also reflects this.
- Added Andrew's Reviewed-by to the first patch (thank you!)


Best regards,
Martin


[0] https://patchwork.kernel.org/project/netdevbpf/cover/20210406203508.476122-1-martin.blumenstingl@googlemail.com/


Martin Blumenstingl (2):
  net: dsa: lantiq_gswip: Don't use PHY auto polling
  net: dsa: lantiq_gswip: Configure all remaining GSWIP_MII_CFG bits

 drivers/net/dsa/lantiq_gswip.c | 202 ++++++++++++++++++++++++++++-----
 1 file changed, 174 insertions(+), 28 deletions(-)

Comments

Martin Blumenstingl April 9, 2021, 7:43 p.m. UTC | #1
Hello Vladimir,

On Fri, Apr 9, 2021 at 12:46 AM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Thu, Apr 08, 2021 at 08:38:27PM +0200, Martin Blumenstingl wrote:
> > PHY auto polling on the GSWIP hardware can be used so link changes
> > (speed, link up/down, etc.) can be detected automatically. Internally
> > GSWIP reads the PHY's registers for this functionality. Based on this
> > automatic detection GSWIP can also automatically re-configure it's port
> > settings. Unfortunately this auto polling (and configuration) mechanism
> > seems to cause various issues observed by different people on different
> > devices:
> > - FritzBox 7360v2: the two Gbit/s ports (connected to the two internal
> >   PHY11G instances) are working fine but the two Fast Ethernet ports
> >   (using an AR8030 RMII PHY) are completely dead (neither RX nor TX are
> >   received). It turns out that the AR8030 PHY sets the BMSR_ESTATEN bit
> >   as well as the ESTATUS_1000_TFULL and ESTATUS_1000_XFULL bits. This
> >   makes the PHY auto polling state machine (rightfully?) think that the
> >   established link speed (when the other side is Gbit/s capable) is
> >   1Gbit/s.
>
> Why do you say "rightfully"? The PHY is gigabit capable, and it reports
> that via the Extended Status register. This is one of the reasons why
> the "advertising" and "supported" link modes are separate concepts,
> because even though you support gigabit, you don't advertise it because
> you are in RMII mode.
according to the marketing materials of the AR8030 it is a "Ultra
low-power single RMII Fast Ethernet PHY"
based on that I am referring to this PHY as "not Gbit/s capable"
(other PHYs from the AR803x series are Gbit/s capable though)

> How does turning off the auto polling feature help circumvent the
> Atheros PHY reporting "issue"? Do we even know that is the problem, or
> is it simply a guess on your part based on something that looked strange?
I have a patch in my queue (which I'll send for the next -net-next
cycle) which adds "ethtool -d" (.get_regs) support to the GSWIP
driver.
There are multiple status registers, one of them indicates that the
link speed (as result of the auto polling mechanism) is 1Gbit/s

[...]
> > Switch to software based configuration instead of PHY auto polling (and
> > letting the GSWIP hardware configure the ports automatically) for the
> > following link parameters:
> > - link up/down
> > - link speed
> > - full/half duplex
> > - flow control (RX / TX pause)
>
> What does the auto polling feature consist of, exactly? Is there some
> sort of microcontroller accessing the MDIO bus simultaneously with
> Linux?
I believe the answer is yes, but there's no clear description in the
datasheet for a newer GSWIP revision [0]
"Figure 8" on page 41 (or page 39 if you go by the numbers at the
bottom of each page) has a description of the state machine logic.
If I understood Hauke correct the "not fiber" part is only checked for
newer GSWIP revisions

Please note that the datasheet from [0] refers to part number GSW140
which is a stand-alone IC.
The GSWIP driver (currently) supports an older revision (at least two
generations older) of GSWIP which is built into Lantiq xRX200 and
xRX300 SoCs.


Best regards,
Martin


[0] https://www.maxlinear.com/document/index?id=23266&languageid=1033&type=Datasheet&partnumber=GSW140&filename=617930_GSW140_DS_Rev1.7.pdf&part=GSW140