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

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: "Marek Behún" <kabel@kernel.org>, "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>
Subject: Re: DSA: request for your help with your DSA driver
Date: Tue, 5 Jul 2022 15:48:29 +0100	[thread overview]
Message-ID: <YsRPPdMc31hTHH5A@shell.armlinux.org.uk> (raw)
In-Reply-To: <CACRpkdb1Z3jWy2VO2EiZ3JO_JtHz7xt24_bTcxCwGkLekea3AA@mail.gmail.com>

On Tue, Jul 05, 2022 at 03:32:50PM +0200, Linus Walleij wrote:
> 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.

It would be good to know that this series at least does not break the
driver.

> 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.

I don't see anything obvious in there which would prevent a conversion,
and I'd suggest doing it after this series has been applied, as it makes
the whole way phylink gets used in DSA much more sane (i.o.w. phylink
always gets used for every port in a phylink-converted DSA driver,
instead of only in most but not all situations.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!