Re: DSA: request for your help with your DSA driver

From: Linus Walleij <linus.walleij@linaro.org>
To: "Marek Behún" <kabel@kernel.org>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	 Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Alvin __ipraga <alsi@bang-olufsen.dk>,
	 Claudiu Manoil <claudiu.manoil@nxp.com>,
	"David S. Miller" <davem@davemloft.net>,
	 DENG Qingfang <dqfext@gmail.com>,
	Eric Dumazet <edumazet@google.com>,
	 Florian Fainelli <f.fainelli@gmail.com>,
	George McCollister <george.mccollister@gmail.com>,
	 Hauke Mehrtens <hauke@hauke-m.de>,
	Jakub Kicinski <kuba@kernel.org>,
	Kurt Kanzenbach <kurt@linutronix.de>,
	 Landen Chao <Landen.Chao@mediatek.com>,
	linux-arm-kernel@lists.infradead.org,
	 linux-mediatek@lists.infradead.org,
	Matthias Brugger <matthias.bgg@gmail.com>,
	 Paolo Abeni <pabeni@redhat.com>,
	Sean Wang <sean.wang@mediatek.com>,
	UNGLinuxDriver@microchip.com,
	 Vivien Didelot <vivien.didelot@gmail.com>,
	Vladimir Oltean <olteanv@gmail.com>,
	 Woojung Huh <woojung.huh@microchip.com>,
	 "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>
Subject: Re: DSA: request for your help with your DSA driver
Date: Tue, 5 Jul 2022 15:32:50 +0200	[thread overview]
Message-ID: <CACRpkdb1Z3jWy2VO2EiZ3JO_JtHz7xt24_bTcxCwGkLekea3AA@mail.gmail.com> (raw)
In-Reply-To: <20220705142236.1fab0033@thinkpad>

On Tue, Jul 5, 2022 at 2:22 PM Marek Behún <kabel@kernel.org> wrote:

> It is very probable that this will break your drivers, and so I ask you
> to look at the RFC series, maybe test it, and give your comments or
> additional patches that make it work.

I've seen Russell's patches and looked at the impact on the drivers
I maintain:

drivers/net/dsa/realtek/rtl8366rb.c
Uses
phylink_mac_link_up -> rtl8366rb_mac_link_up()

This will simply (possibly erroneously WRT the API) force the
CPU port link into 1GB mode with no autonegotiation and then
enable the CPU port.

phylink_mac_link_down -> rtl8366rb_mac_link_down()

This will disable the CPU port.

I suspect this is maybe not ideal. I look at the (a bit horrible) vendor
driver and I see it supports forcing:

- Link Up/Down
- Link speed (10M, 100M, 1G)
- Duplex (full/half)
- Optional txPause
- Optional rxPause

I suspect I should be handling all this properly with the
.phylink_get_caps and augment .phylib_mac_link_up to properly
force the requested features.

I'm a bit uncertain about .phylink_mac_config callbacks?

It seems if I should fix up this incomplete implementation I'd best do
that on top of Russell's patches? I can also test with the patches
applied but the way this driver (ab-)uses the API I think it probably
us a no-op.

The other switch chip I comaintain is drivers/net/dsa/vitesse-vsc73xx-core.c
which isn't using any phylink callbacks at all, just .adjust_link.
It might need some good patching as well but I am uncertain
where to start with that one.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel