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

From: Ido Schimmel <idosch@idosch.org>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Jakub Kicinski <kuba@kernel.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 23:50:04 +0300	[thread overview]
Message-ID: <YRLmfNkCD5hjsTuk@shredder> (raw)
In-Reply-To: <YRLTSC90Lu/3IyJa@lunn.ch>

On Tue, Aug 10, 2021 at 09:28:08PM +0200, Andrew Lunn wrote:
> > 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...

Thanks Andrew and Jakub. I will look into these suggestions more closely
when I start working on modules firmware update.