PHY firmware update method

PHY firmware update method

[Michael Walle]

Hi,

There are PHYs whose firmware can be updated. Usually, they have
an internal ROM and you can add patches on top of that, or there
might be an external flash device which can have a more recent
firmware version installed which can be programmed in-place
through the PHY.

The firmware update for a PHY is usually quite simple, but there
seems to be no infrastructure in the kernel for that. There is the
ETHTOOL_FLASHDEV ioctl for upgrading the firmware of a NIC it seems.
Other than that I haven't found anything. And before going in a wrong
directions I'd like to hear your thoughts on how to do it. I.e. how
should the interface to the userspace look like.

Also I think the PHY should be taken offline, similar to the cable
test.

-michael

Re: PHY firmware update method

[Andrew Lunn]

On Wed, Sep 28, 2022 at 01:27:13PM +0200, Michael Walle wrote:
> Hi,
> 
> There are PHYs whose firmware can be updated. Usually, they have
> an internal ROM and you can add patches on top of that, or there
> might be an external flash device which can have a more recent
> firmware version installed which can be programmed in-place
> through the PHY.
> 
> The firmware update for a PHY is usually quite simple, but there
> seems to be no infrastructure in the kernel for that. There is the
> ETHTOOL_FLASHDEV ioctl for upgrading the firmware of a NIC it seems.
> Other than that I haven't found anything. And before going in a wrong
> directions I'd like to hear your thoughts on how to do it. I.e. how
> should the interface to the userspace look like.
> 
> Also I think the PHY should be taken offline, similar to the cable
> test.

I've seen a few different ways of doing this.

One is to load the firmware from disk every boot using
request_firmware(). Then parse the header, determine if it is newer
than what the PHY is already using, and if so, upgrade the PHY. If you
do this during probe, it should be transparent, no user interaction
required.

I've also seen the FLASH made available as just another mtd
device. User space can then write to it, and then do a {cold} boot.

devlink has become the standard way for upgrading firmware on complex
network devices, like NICs and TOR switches. That is probably a good
solution here. The problem is, what devlink instance to use. Only a
few MAC drivers are using devlink, so it is unlikely the MAC driver
the PHY is attached to has a devlink instance. Do we create a devlink
instance for the PHY?

You might want to talk to Jiri about this.

The other issue is actually getting the firmware. Many manufactures
seem reluctant to allow redistribution as required by linux-firmware.
There is no point adding firmware upgrade if you cannot redistribute
the firmware.

    Andrew

Re: PHY firmware update method

[Jiri Pirko]

Wed, Sep 28, 2022 at 02:28:27PM CEST, andrew@lunn.ch wrote:
>On Wed, Sep 28, 2022 at 01:27:13PM +0200, Michael Walle wrote:
>> Hi,
>> 
>> There are PHYs whose firmware can be updated. Usually, they have
>> an internal ROM and you can add patches on top of that, or there
>> might be an external flash device which can have a more recent
>> firmware version installed which can be programmed in-place
>> through the PHY.
>> 
>> The firmware update for a PHY is usually quite simple, but there
>> seems to be no infrastructure in the kernel for that. There is the
>> ETHTOOL_FLASHDEV ioctl for upgrading the firmware of a NIC it seems.
>> Other than that I haven't found anything. And before going in a wrong
>> directions I'd like to hear your thoughts on how to do it. I.e. how
>> should the interface to the userspace look like.
>> 
>> Also I think the PHY should be taken offline, similar to the cable
>> test.
>
>I've seen a few different ways of doing this.
>
>One is to load the firmware from disk every boot using
>request_firmware(). Then parse the header, determine if it is newer
>than what the PHY is already using, and if so, upgrade the PHY. If you
>do this during probe, it should be transparent, no user interaction
>required.
>
>I've also seen the FLASH made available as just another mtd
>device. User space can then write to it, and then do a {cold} boot.
>
>devlink has become the standard way for upgrading firmware on complex
>network devices, like NICs and TOR switches. That is probably a good
>solution here. The problem is, what devlink instance to use. Only a
>few MAC drivers are using devlink, so it is unlikely the MAC driver
>the PHY is attached to has a devlink instance. Do we create a devlink
>instance for the PHY?

Ccing Jakub. I don't think it is good idea to create a devlink instance
per-PHY. However, on the other hand, we have a devlink instance per
devlink linecard now. The devlink linecard however has devlink
representation, which PHY does not have.

Perhaps now is the time to dust-off my devlink components implementation
and use it for PHYs? IDF. Jakub, WDYT.


>
>You might want to talk to Jiri about this.
>
>The other issue is actually getting the firmware. Many manufactures
>seem reluctant to allow redistribution as required by linux-firmware.
>There is no point adding firmware upgrade if you cannot redistribute
>the firmware.
>
>    Andrew

Re: PHY firmware update method

[Andrew Lunn]

> >devlink has become the standard way for upgrading firmware on complex
> >network devices, like NICs and TOR switches. That is probably a good
> >solution here. The problem is, what devlink instance to use. Only a
> >few MAC drivers are using devlink, so it is unlikely the MAC driver
> >the PHY is attached to has a devlink instance. Do we create a devlink
> >instance for the PHY?
> 
> Ccing Jakub. I don't think it is good idea to create a devlink instance
> per-PHY. However, on the other hand, we have a devlink instance per
> devlink linecard now. The devlink linecard however has devlink
> representation, which PHY does not have.
> 
> Perhaps now is the time to dust-off my devlink components implementation
> and use it for PHYs? IDF. Jakub, WDYT.

If we want to make the PHY a component of an existing devlink for a
MAC, we somehow have to find that devlink instance. A PHY is probably
a property of a port, so we can call netdev_to_devlink_port(), which
gives us a way into devlink.

However, the majority of MAC drivers don't have a devlink
instance. What do we do then? Have phylib create the devlink instance
for the MAC driver? That seems very wrong.

Which is why i was thinking the PHY should have its own devlink
instance.

Or we do firmware upgrade some other way.

	Andrew

Re: PHY firmware update method

[Jakub Kicinski]

On Thu, 29 Sep 2022 14:28:13 +0200 Andrew Lunn wrote:
> If we want to make the PHY a component of an existing devlink for a
> MAC, we somehow have to find that devlink instance. A PHY is probably
> a property of a port, so we can call netdev_to_devlink_port(), which
> gives us a way into devlink.
> 
> However, the majority of MAC drivers don't have a devlink
> instance. What do we do then? Have phylib create the devlink instance
> for the MAC driver? That seems very wrong.
> 
> Which is why i was thinking the PHY should have its own devlink
> instance.

Tricky stuff, how would you expose the topology of the system to 
the user? My initial direction would also be component. Although 
it may be weird if MAC has a way to flash "all" components in one go,
and that did not flash the PHY :S

Either way I don't think we can avoid MACs having a devlink instance
because there needs to be some form of topology formed.

Re: PHY firmware update method

[Andrew Lunn]

On Thu, Sep 29, 2022 at 07:12:09AM -0700, Jakub Kicinski wrote:
> On Thu, 29 Sep 2022 14:28:13 +0200 Andrew Lunn wrote:
> > If we want to make the PHY a component of an existing devlink for a
> > MAC, we somehow have to find that devlink instance. A PHY is probably
> > a property of a port, so we can call netdev_to_devlink_port(), which
> > gives us a way into devlink.
> > 
> > However, the majority of MAC drivers don't have a devlink
> > instance. What do we do then? Have phylib create the devlink instance
> > for the MAC driver? That seems very wrong.
> > 
> > Which is why i was thinking the PHY should have its own devlink
> > instance.
> 
> Tricky stuff, how would you expose the topology of the system to 
> the user? My initial direction would also be component. Although 
> it may be weird if MAC has a way to flash "all" components in one go,
> and that did not flash the PHY :S

~/linux/drivers/net$ grep -r PHYLIB | wc
    114     394    4791

~/linux/drivers/net$ grep -r NET_DEVLINK | wc
     20      60     945

And, of those 20 using DEVLINK, only 4 appear to use PHYLIB.

> Either way I don't think we can avoid MACs having a devlink instance
> because there needs to be some form of topology formed.

In the past, we have tried to make PHYLIB features just work, without
MAC changes. It does not scale otherwise. Cable testing just works if
the PHY supports it, without the MAC driver being changed. PHY stats
work without MAC changes. PHY based PTP works without MAC changes, SFP
module EEPROM dumping works without MAC changes. Why should PHY
firmware upgrade need MAC changes? The MAC does not even care. All it
should see is that the link is down while the upgrade happens.

Maybe devlink is the wrong interface, if it is going to force us to
make MAC changes for most devices to actual make use of this PHY
feature.

     Andrew

Re: PHY firmware update method

[Jiri Pirko]

Thu, Sep 29, 2022 at 04:53:11PM CEST, andrew@lunn.ch wrote:
>On Thu, Sep 29, 2022 at 07:12:09AM -0700, Jakub Kicinski wrote:
>> On Thu, 29 Sep 2022 14:28:13 +0200 Andrew Lunn wrote:
>> > If we want to make the PHY a component of an existing devlink for a
>> > MAC, we somehow have to find that devlink instance. A PHY is probably
>> > a property of a port, so we can call netdev_to_devlink_port(), which
>> > gives us a way into devlink.
>> > 
>> > However, the majority of MAC drivers don't have a devlink
>> > instance. What do we do then? Have phylib create the devlink instance
>> > for the MAC driver? That seems very wrong.
>> > 
>> > Which is why i was thinking the PHY should have its own devlink
>> > instance.
>> 
>> Tricky stuff, how would you expose the topology of the system to 
>> the user? My initial direction would also be component. Although 
>> it may be weird if MAC has a way to flash "all" components in one go,
>> and that did not flash the PHY :S
>
>~/linux/drivers/net$ grep -r PHYLIB | wc
>    114     394    4791
>
>~/linux/drivers/net$ grep -r NET_DEVLINK | wc
>     20      60     945
>
>And, of those 20 using DEVLINK, only 4 appear to use PHYLIB.
>
>> Either way I don't think we can avoid MACs having a devlink instance
>> because there needs to be some form of topology formed.
>
>In the past, we have tried to make PHYLIB features just work, without
>MAC changes. It does not scale otherwise. Cable testing just works if
>the PHY supports it, without the MAC driver being changed. PHY stats
>work without MAC changes. PHY based PTP works without MAC changes, SFP
>module EEPROM dumping works without MAC changes. Why should PHY
>firmware upgrade need MAC changes? The MAC does not even care. All it
>should see is that the link is down while the upgrade happens.
>
>Maybe devlink is the wrong interface, if it is going to force us to
>make MAC changes for most devices to actual make use of this PHY
>feature.

Yeah, I tend to agree here. I believe that phylib should probably find a
separate way to to the flash.

But perhaps it could be a non-user-facing flash. I mean, what if phylib
has internal routine to:
1) do query phy fw version
2) load a fw bin related for this phy (easy phy driver may provide the
				       path/name of the file)
3) flash if there is a newer version available

I'm not saying this is complete solution, but it could be first step and
perhaps sufficient. For example, this pattern works for ordinary mlxsw
user who just updates the kernel/linux-firmware parkages - the driver
does flashing implicitly during init when never version is found.

Re: PHY firmware update method

[Andrew Lunn]

> Yeah, I tend to agree here. I believe that phylib should probably find a
> separate way to to the flash.
> 
> But perhaps it could be a non-user-facing flash. I mean, what if phylib
> has internal routine to:
> 1) do query phy fw version
> 2) load a fw bin related for this phy (easy phy driver may provide the
> 				       path/name of the file)
> 3) flash if there is a newer version available

That was my first suggestion. One problem is getting the version from
the binary blob firmware. But this seems like a generic problem for
linux-firmware, so maybe somebody has worked on a standardised header
which can be preppended with this meta data?

      Andrew

Re: PHY firmware update method

[Jakub Kicinski]

On Fri, 30 Sep 2022 14:36:47 +0200 Andrew Lunn wrote:
> > Yeah, I tend to agree here. I believe that phylib should probably find a
> > separate way to to the flash.
> > 
> > But perhaps it could be a non-user-facing flash. I mean, what if phylib
> > has internal routine to:
> > 1) do query phy fw version
> > 2) load a fw bin related for this phy (easy phy driver may provide the
> > 				       path/name of the file)
> > 3) flash if there is a newer version available  
> 
> That was my first suggestion. One problem is getting the version from
> the binary blob firmware. But this seems like a generic problem for
> linux-firmware, so maybe somebody has worked on a standardised header
> which can be preppended with this meta data?

Not that I know, perhaps the folks that do laptop FW upgrade have some
thoughts https://fwupd.org/ 

Actually maybe there's something in DMTF, does PLDM have standard image
format? Adding Jake. Not sure if PHYs would use it tho :S 

What's the interface that the PHY FW exposes? Ben H was of the opinion
that we should just expose the raw mtd devices.. just saying..

RE: PHY firmware update method

[Keller, Jacob E]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Friday, September 30, 2022 7:46 AM
> To: Andrew Lunn <andrew@lunn.ch>; Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: Jiri Pirko <jiri@resnulli.us>; Michael Walle <michael@walle.cc>; Heiner
> Kallweit <hkallweit1@gmail.com>; Russell King <linux@armlinux.org.uk>;
> netdev@vger.kernel.org
> Subject: Re: PHY firmware update method
> 
> On Fri, 30 Sep 2022 14:36:47 +0200 Andrew Lunn wrote:
> > > Yeah, I tend to agree here. I believe that phylib should probably find a
> > > separate way to to the flash.
> > >
> > > But perhaps it could be a non-user-facing flash. I mean, what if phylib
> > > has internal routine to:
> > > 1) do query phy fw version
> > > 2) load a fw bin related for this phy (easy phy driver may provide the
> > > 				       path/name of the file)
> > > 3) flash if there is a newer version available
> >
> > That was my first suggestion. One problem is getting the version from
> > the binary blob firmware. But this seems like a generic problem for
> > linux-firmware, so maybe somebody has worked on a standardised header
> > which can be preppended with this meta data?
> 
> Not that I know, perhaps the folks that do laptop FW upgrade have some
> thoughts https://fwupd.org/
> 
> Actually maybe there's something in DMTF, does PLDM have standard image
> format? Adding Jake. Not sure if PHYs would use it tho :S
> 

PLDM for Firmware does indeed specify a standardized header for binary images and could be used for PHY firmware. The PLDM header itself describes things as components, where each component gets a record which indicates the version of that component and where in the binary that component exists. Then there are a series of records which determine which sets of components go together and for which devices. I don't think the ice firmware files use the full standard as they always have only a single device record, though the lib/pldmfw codes does try to allow the more expressive ability to support multiple device types from a single binary. Specifying a PHY component would be possible, and you could fairly easily prepend a PLDM firmware header on top of an arbitrary binary.

Thanks,
Jake

> What's the interface that the PHY FW exposes? Ben H was of the opinion
> that we should just expose the raw mtd devices.. just saying..

Re: PHY firmware update method

[Russell King (Oracle)]

On Fri, Sep 30, 2022 at 07:45:46AM -0700, Jakub Kicinski wrote:
> Actually maybe there's something in DMTF, does PLDM have standard image
> format? Adding Jake. Not sure if PHYs would use it tho :S 

DMTF? PLDM?

> What's the interface that the PHY FW exposes? Ben H was of the opinion
> that we should just expose the raw mtd devices.. just saying..

Not all PHYs provide raw access to the firmware memory to the host; in
some cases, firmware memory access needs the PHY to be shutdown, and
programs loaded into the PHY to provide a "bridge" to program the
external firmware memory. I'm thinking about 88x3310 here - effectively
there it's write-only access via an intermediate program on the PHY,
and there's things like checksums etc.

So, exposing everything as a MTD sounds like a solution, but not all
PHYs provide such access. IMHO, if we say "everything must provide a
MTD" then we're boxing ourselves into a corner.

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

Re: PHY firmware update method

[Jakub Kicinski]

On Mon, 3 Oct 2022 13:18:51 +0100 Russell King (Oracle) wrote:
> On Fri, Sep 30, 2022 at 07:45:46AM -0700, Jakub Kicinski wrote:
> > Actually maybe there's something in DMTF, does PLDM have standard image
> > format? Adding Jake. Not sure if PHYs would use it tho :S   
> 
> DMTF? PLDM?

Based on google search + pattern matching the name I think this is 
the spec:

https://www.dmtf.org/sites/default/files/standards/documents/DSP0267_1.0.0.pdf

RE: PHY firmware update method

[Keller, Jacob E]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Monday, October 03, 2022 7:42 AM
> To: Russell King (Oracle) <linux@armlinux.org.uk>
> Cc: Andrew Lunn <andrew@lunn.ch>; Keller, Jacob E <jacob.e.keller@intel.com>;
> Jiri Pirko <jiri@resnulli.us>; Michael Walle <michael@walle.cc>; Heiner Kallweit
> <hkallweit1@gmail.com>; netdev@vger.kernel.org
> Subject: Re: PHY firmware update method
> 
> On Mon, 3 Oct 2022 13:18:51 +0100 Russell King (Oracle) wrote:
> > On Fri, Sep 30, 2022 at 07:45:46AM -0700, Jakub Kicinski wrote:
> > > Actually maybe there's something in DMTF, does PLDM have standard image
> > > format? Adding Jake. Not sure if PHYs would use it tho :S
> >
> > DMTF? PLDM?
> 
> Based on google search + pattern matching the name I think this is
> the spec:
> 
> https://www.dmtf.org/sites/default/files/standards/documents/DSP0267_1.0.0.
> pdf

Yep, this is the standard I implemented in lib/pldmfw and which is used by the binaries for the main firmware in the ice hardware.

Thanks,
Jake

Re: PHY firmware update method

[Jacob Keller]

On 10/3/2022 5:18 AM, Russell King (Oracle) wrote:
> On Fri, Sep 30, 2022 at 07:45:46AM -0700, Jakub Kicinski wrote:
>> Actually maybe there's something in DMTF, does PLDM have standard image
>> format? Adding Jake. Not sure if PHYs would use it tho :S 
> 
> DMTF? PLDM?
> 

Jakub already linked to the standard we were referring to, but for
clarification:

DMTF refers to the group which was the Distributed Management Task
Force, and describes themselves as:

DMTF (formerly known as the Distributed Management Task Force) creates
open manageability standards spanning diverse emerging and traditional
IT infrastructures including cloud, virtualization, network, servers and
storage. Member companies and alliance partners worldwide collaborate on
standards to improve the interoperable management of information
technologies.

PLDM refers to "PLDM - Platform Level Data Model Including Firmware
Update, Redfish Device Enablement (RDE)"

The "PLDM for Firmware Update" is a standard which describes a file
format and associated specifications for how to interpret that file when
performing an update.

I implemented lib/pldmfw.c which is use in the ice driver as a way to
read the binary file to extract information about where each component
of firmware is in the binary, which could then be written to the device
to perform an update.

Re: PHY firmware update method

[Michael Walle]

[sorry for the very late response, but I was working on getting an
acceptable license for the PHY binary in the meantime]

>> Yeah, I tend to agree here. I believe that phylib should probably find 
>> a
>> separate way to to the flash.
>> 
>> But perhaps it could be a non-user-facing flash. I mean, what if 
>> phylib
>> has internal routine to:
>> 1) do query phy fw version
>> 2) load a fw bin related for this phy (easy phy driver may provide the
>> 				       path/name of the file)
>> 3) flash if there is a newer version available
> 
> That was my first suggestion. One problem is getting the version from
> the binary blob firmware. But this seems like a generic problem for
> linux-firmware, so maybe somebody has worked on a standardised header
> which can be preppended with this meta data?

In my case, the firmware binary blob has some static offset to get
firmware version (I need to double check that one with the vendor).
Of course we could put that PLDM thingy around it. But it seems we are
mangling with the binary provided by the vendor before putting it into
linux-firmware.git. If I understand Jacob correct, Intel will already
provide the binaries in PLDM format.

Another problem I see is how we can determine if a firmware update
is possible at all - or if we just try it anyway if that is possible.
In my case there is already an integrated firmware and the external
flash is optional. The PHY will try to boot from external flash and
fall back to the internal one. AFAIK you can read out where the PHY
was booted from. If the external flash is empty, you cannot detect if
you could update the firmware.

So if you'd do this during the PHY probe, it might try to update the
firmware on every boot and fail. Would that be acceptable?

How long could can a firmware update during probe run? Do we need
to do it in the background with the PHY being offline. Sounds like
not something we want.

-michael

Re: PHY firmware update method

[Michael Walle]

Am 2022-10-03 14:18, schrieb Russell King (Oracle):
> On Fri, Sep 30, 2022 at 07:45:46AM -0700, Jakub Kicinski wrote:
>> Actually maybe there's something in DMTF, does PLDM have standard 
>> image
>> format? Adding Jake. Not sure if PHYs would use it tho :S
> 
> DMTF? PLDM?
> 
>> What's the interface that the PHY FW exposes? Ben H was of the opinion
>> that we should just expose the raw mtd devices.. just saying..
> 
> Not all PHYs provide raw access to the firmware memory to the host; in
> some cases, firmware memory access needs the PHY to be shutdown, and
> programs loaded into the PHY to provide a "bridge" to program the
> external firmware memory. I'm thinking about 88x3310 here - effectively
> there it's write-only access via an intermediate program on the PHY,
> and there's things like checksums etc.
> 
> So, exposing everything as a MTD sounds like a solution, but not all
> PHYs provide such access. IMHO, if we say "everything must provide a
> MTD" then we're boxing ourselves into a corner.

I agree, the PHY I'm looking at right now, can't read back the
firmware binary. And there is also no random access, you basically
have to stream the whole binary in one go to the PHY.

-michael

Re: PHY firmware update method

[Andrew Lunn]

> So if you'd do this during the PHY probe, it might try to update the
> firmware on every boot and fail. Would that be acceptable?

Do you have a feeling how long that takes?

Also, is it possible to put the firmware into RAM and run it from
there, rather than put it into the EEPROM?

> How long could can a firmware update during probe run? Do we need
> to do it in the background with the PHY being offline. Sounds like
> not something we want.

One device being slow to probe will slow down the probe of that
bus. But probe of other busses should be unaffected. I _guess_ it
might have a global affect on EPROBE_DEFER, the next cycle could be
delayed?  Probably a question for GregKH, or reading the code.

If it going to be really slow, then i would suggest making use of
devlink and it being a user initiated operation.

	Andrew

Re: PHY firmware update method

[Jacob Keller]

On 1/24/2023 9:11 AM, Michael Walle wrote:
> [sorry for the very late response, but I was working on getting an
> acceptable license for the PHY binary in the meantime]
> 
>>> Yeah, I tend to agree here. I believe that phylib should probably find 
>>> a
>>> separate way to to the flash.
>>>
>>> But perhaps it could be a non-user-facing flash. I mean, what if 
>>> phylib
>>> has internal routine to:
>>> 1) do query phy fw version
>>> 2) load a fw bin related for this phy (easy phy driver may provide the
>>> 				       path/name of the file)
>>> 3) flash if there is a newer version available
>>
>> That was my first suggestion. One problem is getting the version from
>> the binary blob firmware. But this seems like a generic problem for
>> linux-firmware, so maybe somebody has worked on a standardised header
>> which can be preppended with this meta data?
> 
> In my case, the firmware binary blob has some static offset to get
> firmware version (I need to double check that one with the vendor).
> Of course we could put that PLDM thingy around it. But it seems we are
> mangling with the binary provided by the vendor before putting it into
> linux-firmware.git. If I understand Jacob correct, Intel will already
> provide the binaries in PLDM format.

Correct, we ship the firmware binaries already in PLDM format, but it
*is* more or less just a header that gets added ahead of the 3 binaries.
(the header points to where in the full binary file each actual firmware
binary data is located, so we can combine all of our images, i.e. the
main NVM firmware, the UEFI firmware, and our netlist firmware)

> 
> Another problem I see is how we can determine if a firmware update
> is possible at all - or if we just try it anyway if that is possible.
> In my case there is already an integrated firmware and the external
> flash is optional. The PHY will try to boot from external flash and
> fall back to the internal one. AFAIK you can read out where the PHY
> was booted from. If the external flash is empty, you cannot detect if
> you could update the firmware.
> 
> So if you'd do this during the PHY probe, it might try to update the
> firmware on every boot and fail. Would that be acceptable?
> 

Wouldn't you just want to update when user requests and have info report
something if it can't figure out what version?

> How long could can a firmware update during probe run? Do we need
> to do it in the background with the PHY being offline. Sounds like
> not something we want.
> 
> -michael

I'm not sure. Since the firmware is loaded from flash I would not expect
to need to download it every probe. I would only expect that if the
firmware had to be loaded each time and wasn't maintained across resets.

For example the ice hardware has some data we load that does not get
saved to flash and we must load that after every probe or reset.

Re: PHY firmware update method

[Michael Walle]

Am 2023-01-24 21:42, schrieb Andrew Lunn:
>> So if you'd do this during the PHY probe, it might try to update the
>> firmware on every boot and fail. Would that be acceptable?
> 
> Do you have a feeling how long that takes?

As a ballpark, with my PoC it takes about 11s. The binary itself
is around 128k and the MDC has a frequency of 2.5MHz. Although,
I didn't do any testing for now with slower or faster clock
frequency. I'm polling for a ready bit most of the time.

> Also, is it possible to put the firmware into RAM and run it from
> there, rather than put it into the EEPROM?

Not that I'm aware of.

>> How long could can a firmware update during probe run? Do we need
>> to do it in the background with the PHY being offline. Sounds like
>> not something we want.
> 
> One device being slow to probe will slow down the probe of that
> bus. But probe of other busses should be unaffected. I _guess_ it
> might have a global affect on EPROBE_DEFER, the next cycle could be
> delayed?  Probably a question for GregKH, or reading the code.
> 
> If it going to be really slow, then i would suggest making use of
> devlink and it being a user initiated operation.

One concern which raised internally was that you'll always do
the update (unconditionally) if there is a newer version. You seem
to make life easier for the user, because the update just runs
automatically. OTHO, what if a user doesn't want to update (for
whatever reason) to the particular version in linux-firmware.git.
I'm undecided on that.

It's different than a firmware which is loaded into RAM and which
*needs* to be loaded anyway. In this case the update is voluntary.

-michael

Re: PHY firmware update method

[Russell King (Oracle)]

On Tue, Jan 31, 2023 at 05:10:08PM +0100, Michael Walle wrote:
> Am 2023-01-24 21:42, schrieb Andrew Lunn:
> > One device being slow to probe will slow down the probe of that
> > bus. But probe of other busses should be unaffected. I _guess_ it
> > might have a global affect on EPROBE_DEFER, the next cycle could be
> > delayed?  Probably a question for GregKH, or reading the code.
> > 
> > If it going to be really slow, then i would suggest making use of
> > devlink and it being a user initiated operation.
> 
> One concern which raised internally was that you'll always do
> the update (unconditionally) if there is a newer version. You seem
> to make life easier for the user, because the update just runs
> automatically. OTHO, what if a user doesn't want to update (for
> whatever reason) to the particular version in linux-firmware.git.
> I'm undecided on that.

On one hand, the user should always be asked whether they want to
upgrade the firmware on their systems, but there is the argument
about whether a user has sufficient information to make an informed
choice about it.

Then there's the problem that a newer firmware might introduce a
bug, but the user wants to use an older version (which is something
I do with some WiFi setups, and it becomes a pain when linux-firmware
is maintained by the distro, but you don't want to use that provided
version.

I really don't like the idea of the kernel automatically updating
non-volatile firmware - that sounds to me like a recipe for all
sorts of disasters.

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

Re: PHY firmware update method

[Michael Walle]

Am 2023-01-31 17:29, schrieb Russell King (Oracle):
> On Tue, Jan 31, 2023 at 05:10:08PM +0100, Michael Walle wrote:
>> Am 2023-01-24 21:42, schrieb Andrew Lunn:
>> > One device being slow to probe will slow down the probe of that
>> > bus. But probe of other busses should be unaffected. I _guess_ it
>> > might have a global affect on EPROBE_DEFER, the next cycle could be
>> > delayed?  Probably a question for GregKH, or reading the code.
>> >
>> > If it going to be really slow, then i would suggest making use of
>> > devlink and it being a user initiated operation.
>> 
>> One concern which raised internally was that you'll always do
>> the update (unconditionally) if there is a newer version. You seem
>> to make life easier for the user, because the update just runs
>> automatically. OTHO, what if a user doesn't want to update (for
>> whatever reason) to the particular version in linux-firmware.git.
>> I'm undecided on that.
> 
> On one hand, the user should always be asked whether they want to
> upgrade the firmware on their systems, but there is the argument
> about whether a user has sufficient information to make an informed
> choice about it.
> 
> Then there's the problem that a newer firmware might introduce a
> bug, but the user wants to use an older version (which is something
> I do with some WiFi setups, and it becomes a pain when linux-firmware
> is maintained by the distro, but you don't want to use that provided
> version.
> 
> I really don't like the idea of the kernel automatically updating
> non-volatile firmware - that sounds to me like a recipe for all
> sorts of disasters.

I agree. That leaves us with the devlink solution, right?

Where would the firmware be stored, fwupd.org was mentioned by
Jakub, or is it the users responsibility to fetch it from the
vendor? Andrew was against adding a firmware update mechanism
without having the binaries.

-michael

Re: PHY firmware update method

[Jacob Keller]

On 1/31/2023 9:48 AM, Michael Walle wrote:
> Am 2023-01-31 17:29, schrieb Russell King (Oracle):
>> On Tue, Jan 31, 2023 at 05:10:08PM +0100, Michael Walle wrote:
>>> Am 2023-01-24 21:42, schrieb Andrew Lunn:
>>>> One device being slow to probe will slow down the probe of that
>>>> bus. But probe of other busses should be unaffected. I _guess_ it
>>>> might have a global affect on EPROBE_DEFER, the next cycle could be
>>>> delayed?  Probably a question for GregKH, or reading the code.
>>>>
>>>> If it going to be really slow, then i would suggest making use of
>>>> devlink and it being a user initiated operation.
>>>
>>> One concern which raised internally was that you'll always do
>>> the update (unconditionally) if there is a newer version. You seem
>>> to make life easier for the user, because the update just runs
>>> automatically. OTHO, what if a user doesn't want to update (for
>>> whatever reason) to the particular version in linux-firmware.git.
>>> I'm undecided on that.
>>
>> On one hand, the user should always be asked whether they want to
>> upgrade the firmware on their systems, but there is the argument
>> about whether a user has sufficient information to make an informed
>> choice about it.
>>
>> Then there's the problem that a newer firmware might introduce a
>> bug, but the user wants to use an older version (which is something
>> I do with some WiFi setups, and it becomes a pain when linux-firmware
>> is maintained by the distro, but you don't want to use that provided
>> version.
>>
>> I really don't like the idea of the kernel automatically updating
>> non-volatile firmware - that sounds to me like a recipe for all
>> sorts of disasters.
> 
> I agree. That leaves us with the devlink solution, right?
> 
> Where would the firmware be stored, fwupd.org was mentioned by
> Jakub, or is it the users responsibility to fetch it from the
> vendor? Andrew was against adding a firmware update mechanism
> without having the binaries.
> 
> -michael

Well devlink flash update is already supported, and typically assumes
you got binaries from somewhere like a vendor website if I understand?

For non volatile firmware I think its the best approach we have currently.

Thanks,
Jake

Re: PHY firmware update method

[Jakub Kicinski]

On Tue, 31 Jan 2023 16:29:04 +0000 Russell King (Oracle) wrote:
> I really don't like the idea of the kernel automatically updating
> non-volatile firmware - that sounds to me like a recipe for all
> sorts of disasters.

FWIW we had a concept of "FW load policy" in devlink but IDK if it
addresses the concern sufficiently. For mlxsw IIRC the load policy
"from disk" means "flash the device if loaded FW is older".

Re: PHY firmware update method

[Jacob Keller]

On 1/31/2023 10:41 AM, Jakub Kicinski wrote:
> On Tue, 31 Jan 2023 16:29:04 +0000 Russell King (Oracle) wrote:
>> I really don't like the idea of the kernel automatically updating
>> non-volatile firmware - that sounds to me like a recipe for all
>> sorts of disasters.
> 
> FWIW we had a concept of "FW load policy" in devlink but IDK if it
> addresses the concern sufficiently. For mlxsw IIRC the load policy
> "from disk" means "flash the device if loaded FW is older".

My initial interpretation of this parameter was that "LOAD_DISK" implied
the device could choose to load the firmware from disk but didn't
necessarily overwrite what was stored in the flash permanently.

Your interpretation also makes sense on a second review, but I'm not
sure what "driver" would mean in this context? I guess "whichever driver
prefers?"

The parameter does seem like a suitable place to allow admin to specify
the policy.

Re: PHY firmware update method

[Jakub Kicinski]

On Tue, 31 Jan 2023 11:56:26 -0800 Jacob Keller wrote:
> On 1/31/2023 10:41 AM, Jakub Kicinski wrote:
> > FWIW we had a concept of "FW load policy" in devlink but IDK if it
> > addresses the concern sufficiently. For mlxsw IIRC the load policy
> > "from disk" means "flash the device if loaded FW is older".  
> 
> My initial interpretation of this parameter was that "LOAD_DISK" implied
> the device could choose to load the firmware from disk but didn't
> necessarily overwrite what was stored in the flash permanently.
> 
> Your interpretation also makes sense on a second review, but I'm not
> sure what "driver" would mean in this context? I guess "whichever driver
> prefers?"

Ah, you're right, I dismembered. Looking at the code 'driver' means
flash to the latest supported by the driver. 'flash' means use what's
there.

For the NFP IIRC 'driver' meant load whichever is newer (but don't
write to flash if disk was newer, just load to sram).

> The parameter does seem like a suitable place to allow admin to specify
> the policy.