Re: [RFC PATCH net-next 2/8] ethtool: Add ability to reset transceiver modules

From: Andrew Lunn <andrew@lunn.ch>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Ido Schimmel <idosch@idosch.org>,
	netdev@vger.kernel.org, davem@davemloft.net, mkubecek@suse.cz,
	pali@kernel.org, vadimp@nvidia.com, mlxsw@nvidia.com,
	Ido Schimmel <idosch@nvidia.com>
Subject: Re: [RFC PATCH net-next 2/8] ethtool: Add ability to reset transceiver modules
Date: Tue, 10 Aug 2021 21:28:08 +0200	[thread overview]
Message-ID: <YRLTSC90Lu/3IyJa@lunn.ch> (raw)
In-Reply-To: <20210810120030.5092ec22@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

> Hm, flashing is harder than reset. We can't unbind the driver while
> it's in progress. I don't have any ready solution in mind, but I'd 
> like to make sure the locking is clear and hard to get wrong. Maybe 
> we could have a mix of ops, one called for "preparing" the flashing
> called under rtnl and another for "commit" with "unlocked" in the name.
> Drivers which don't want to deal with dropping rtnl lock can just do
> everything in the first stage? Perhaps Andrew has better ideas, I'm
> just spit-balling. Presumably there are already locks at play, locks
> we would have to take in the case where Linux manages the PHY. Maybe
> they dictate an architecture?

I don't think the way linux manages PHYs dictates the
architecture. PHY cable test requires that the link is
administratively up, so the PHY state machine is in play. It
transitions into a testing state when cable test is started, and when
the test is finished, it resets the PHY to put it back into running
state. If you down the interface while the cable test is running, it
aborts the cable test, and then downs the PHY.

Flashing firmware is a bit different. You need to ensure the interface
is down. And i guess that gets interesting with split modules. You
really should not abort an upgrade because the user wants to up the
interface. So -EBUSY to open() seems like the best option, based on
the state of the SFP state machine.

I suspect you are going to need a kernel thread to do the real
work. So your "prepare" netlink op would pass the name of the firmware
file. Some basic validation would be performed, that all the needed
interfaces are down etc, and then the netlink OP would return. The
thread then uses request_firmware() to get access to the firmware, and
program it. Once complete, or on error, it can async notify user space
that it is sorry, your module is toast, or firmware upgrade was
successful.

This is just throwing out ideas...

     Andrew