| Message ID | 20210617154206.GA17555@plvision.eu (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [GIT,PULL] linux-firmware: mrvl: prestera: Update Marvell Prestera Switchdev v3.0 with policer support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Thu, Jun 17, 2021 at 06:42:06PM +0300, Vadym Kochan wrote: > The following changes since commit 0f66b74b6267fce66395316308d88b0535aa3df2: > > cypress: update firmware for cyw54591 pcie (2021-06-09 07:12:02 -0400) > > are available in the Git repository at: > > https://github.com/PLVision/linux-firmware.git mrvl-prestera > > for you to fetch changes up to a43d95a48b8e8167e21fb72429d860c7961c2e32: > > mrvl: prestera: Update Marvell Prestera Switchdev v3.0 with policer support (2021-06-17 18:22:57 +0300) > > ---------------------------------------------------------------- > Vadym Kochan (1): > mrvl: prestera: Update Marvell Prestera Switchdev v3.0 with policer support > > mrvl/prestera/mvsw_prestera_fw-v3.0.img | Bin 13721584 -> 13721676 bytes > 1 file changed, 0 insertions(+), 0 deletions(-) Hi Vadym You keep the version the same, but add new features? So what does the version number actually mean? How does the driver know if should not use the policer if it cannot tell old version 3.0 from new version 3.0? How is a user supposed to know if they have old version 3.0 rather than new 3.0, when policer fails? Andrew
Hi Andrew, On Thu, Jun 17, 2021 at 06:45:14PM +0200, Andrew Lunn wrote: > On Thu, Jun 17, 2021 at 06:42:06PM +0300, Vadym Kochan wrote: > > The following changes since commit 0f66b74b6267fce66395316308d88b0535aa3df2: > > > > cypress: update firmware for cyw54591 pcie (2021-06-09 07:12:02 -0400) > > > > are available in the Git repository at: > > > > https://github.com/PLVision/linux-firmware.git mrvl-prestera > > > > for you to fetch changes up to a43d95a48b8e8167e21fb72429d860c7961c2e32: > > > > mrvl: prestera: Update Marvell Prestera Switchdev v3.0 with policer support (2021-06-17 18:22:57 +0300) > > > > ---------------------------------------------------------------- > > Vadym Kochan (1): > > mrvl: prestera: Update Marvell Prestera Switchdev v3.0 with policer support > > > > mrvl/prestera/mvsw_prestera_fw-v3.0.img | Bin 13721584 -> 13721676 bytes > > 1 file changed, 0 insertions(+), 0 deletions(-) > > Hi Vadym > > You keep the version the same, but add new features? So what does the > version number actually mean? How does the driver know if should not > use the policer if it cannot tell old version 3.0 from new version > 3.0? How is a user supposed to know if they have old version 3.0 > rather than new 3.0, when policer fails? > > Andrew So the last 'sub' x.x.1 version will be showed in dmesg output and via: $ ethtool -i $PORT ... firmware-version: 3.0.1 ... The older driver version will not use the policer feature. I am not sure if I need to add a check for 3.0.1 in the newer one which will have the policer changes. So may be the only case which can be handled (in new driver changes with policer code) is to check if the FW is not older than 3.0.1 during the validation of police rules. For this reason I decided to do not increase the supported version MIN.MAJ but only the SUB (last one).
On Thu, Jun 17, 2021 at 07:58:24PM +0300, Vadym Kochan wrote: > Hi Andrew, > > On Thu, Jun 17, 2021 at 06:45:14PM +0200, Andrew Lunn wrote: > > On Thu, Jun 17, 2021 at 06:42:06PM +0300, Vadym Kochan wrote: > > > The following changes since commit 0f66b74b6267fce66395316308d88b0535aa3df2: > > > > > > cypress: update firmware for cyw54591 pcie (2021-06-09 07:12:02 -0400) > > > > > > are available in the Git repository at: > > > > > > https://github.com/PLVision/linux-firmware.git mrvl-prestera > > > > > > for you to fetch changes up to a43d95a48b8e8167e21fb72429d860c7961c2e32: > > > > > > mrvl: prestera: Update Marvell Prestera Switchdev v3.0 with policer support (2021-06-17 18:22:57 +0300) > > > > > > ---------------------------------------------------------------- > > > Vadym Kochan (1): > > > mrvl: prestera: Update Marvell Prestera Switchdev v3.0 with policer support > > > > > > mrvl/prestera/mvsw_prestera_fw-v3.0.img | Bin 13721584 -> 13721676 bytes > > > 1 file changed, 0 insertions(+), 0 deletions(-) > > > > Hi Vadym > > > > You keep the version the same, but add new features? So what does the > > version number actually mean? How does the driver know if should not > > use the policer if it cannot tell old version 3.0 from new version > > 3.0? How is a user supposed to know if they have old version 3.0 > > rather than new 3.0, when policer fails? > > > > Andrew > > So the last 'sub' x.x.1 version will be showed in dmesg output and via: > > $ ethtool -i $PORT > > ... > firmware-version: 3.0.1 That is pretty unfriendly, the filename saying one thing, the kernel another. If you look back in the git history, are there other firmware blobs which get updated while retaining the same version? If this is very unusual, you probably should not be doing it. If it is common practice, then i will be surprised, and it is probably acceptable. I suppose you could consider another alternative: Make mrvl/prestera/mvsw_prestera_fw-v3.0.img a symbolic link, and it would point to mrvl/prestera/mvsw_prestera_fw-v3.0.1.img. Andrew
Hi Andrew, On Fri, Jun 18, 2021 at 03:18:16AM +0200, Andrew Lunn wrote: > On Thu, Jun 17, 2021 at 07:58:24PM +0300, Vadym Kochan wrote: > > Hi Andrew, > > > > On Thu, Jun 17, 2021 at 06:45:14PM +0200, Andrew Lunn wrote: > > > On Thu, Jun 17, 2021 at 06:42:06PM +0300, Vadym Kochan wrote: > > > > The following changes since commit 0f66b74b6267fce66395316308d88b0535aa3df2: > > > > > > > > cypress: update firmware for cyw54591 pcie (2021-06-09 07:12:02 -0400) > > > > > > > > are available in the Git repository at: > > > > > > > > https://github.com/PLVision/linux-firmware.git mrvl-prestera > > > > > > > > for you to fetch changes up to a43d95a48b8e8167e21fb72429d860c7961c2e32: > > > > > > > > mrvl: prestera: Update Marvell Prestera Switchdev v3.0 with policer support (2021-06-17 18:22:57 +0300) > > > > > > > > ---------------------------------------------------------------- > > > > Vadym Kochan (1): > > > > mrvl: prestera: Update Marvell Prestera Switchdev v3.0 with policer support > > > > > > > > mrvl/prestera/mvsw_prestera_fw-v3.0.img | Bin 13721584 -> 13721676 bytes > > > > 1 file changed, 0 insertions(+), 0 deletions(-) > > > > > > Hi Vadym > > > > > > You keep the version the same, but add new features? So what does the > > > version number actually mean? How does the driver know if should not > > > use the policer if it cannot tell old version 3.0 from new version > > > 3.0? How is a user supposed to know if they have old version 3.0 > > > rather than new 3.0, when policer fails? > > > > > > Andrew > > > > So the last 'sub' x.x.1 version will be showed in dmesg output and via: > > > > $ ethtool -i $PORT > > > > ... > > firmware-version: 3.0.1 > > That is pretty unfriendly, the filename saying one thing, the kernel > another. > > If you look back in the git history, are there other firmware blobs > which get updated while retaining the same version? If this is very > unusual, you probably should not be doing it. If it is common > practice, then i will be surprised, and it is probably acceptable. > > I suppose you could consider another alternative: Make > mrvl/prestera/mvsw_prestera_fw-v3.0.img a symbolic link, and it would > point to mrvl/prestera/mvsw_prestera_fw-v3.0.1.img. > > Andrew I just picked some from the git log: 48237834129d ("QCA: Update Bluetooth firmware for QCA6174") this just updates the binary and description says that it updates to v26. Not sure if it is good example. But anyway, I agree with you that better if new changes also reflects the FW binary name (version) so it will be easy to find out which FW binary have or not particular features. So I think better to add new FW 3.1 binary ? Thank you,
> I just picked some from the git log: > > 48237834129d ("QCA: Update Bluetooth firmware for QCA6174") > > this just updates the binary and description says that it updates > to v26. > > Not sure if it is good example. The filename is qca/rampatch_usb_00000302.bin. If you look at drivers/bluetooth/btusb.c you can see that 00000302 is the version of the ROM in the device which is being patched. So there is no expectation of knowing the firmware version from the firmware filename. > But anyway, I agree with you that better if new changes also reflects > the FW binary name (version) so it will be easy to find out which FW binary > have or not particular features. > > So I think better to add new FW 3.1 binary ? Probably. But please consider your whole upgrade story. You are changing the firmware version quite frequently. How do end users cope with this? Is the driver going to support 3.1, 3.0 and 2.0? Or just 3.1 and 2.0? Do you have more features in firmware 3.1 you need to add driver support for? Or can we expect a 3.2 in a few weeks time? What are your users expectations at the moment? It could be, you don't consider the driver has enough features at the moment that anybody other than early adopters playing with it would consider using it. That you don't expect real use of it for another six months, or a year. If that is true, you probably can be a bit more disruptive at the moment. But when you have a production ready driver, you really do need to consider how your users deal with upgrades, keeping the firmware version stable for a longer period of time. Andrew
Hi Andrew, On Fri, Jun 18, 2021 at 03:41:39PM +0200, Andrew Lunn wrote: > > I just picked some from the git log: > > > > 48237834129d ("QCA: Update Bluetooth firmware for QCA6174") > > > > this just updates the binary and description says that it updates > > to v26. > > > > Not sure if it is good example. > > The filename is qca/rampatch_usb_00000302.bin. If you look at > drivers/bluetooth/btusb.c you can see that 00000302 is the version of > the ROM in the device which is being patched. So there is no > expectation of knowing the firmware version from the firmware > filename. > > > But anyway, I agree with you that better if new changes also reflects > > the FW binary name (version) so it will be easy to find out which FW binary > > have or not particular features. > > > > So I think better to add new FW 3.1 binary ? > > Probably. But please consider your whole upgrade story. You are > changing the firmware version quite frequently. How do end users cope > with this? Is the driver going to support 3.1, 3.0 and 2.0? Or just > 3.1 and 2.0? I think 3.1, 3.0 (still need to check about this) and 2.0, and the driver need to be changed to use array of versions rather the previous hardcoded value. > > Do you have more features in firmware 3.1 you need to add driver > support for? Or can we expect a 3.2 in a few weeks time? What are your > users expectations at the moment? It could be, you don't consider the > driver has enough features at the moment that anybody other than early > adopters playing with it would consider using it. That you don't > expect real use of it for another six months, or a year. If that is > true, you probably can be a bit more disruptive at the moment. But > when you have a production ready driver, you really do need to > consider how your users deal with upgrades, keeping the firmware > version stable for a longer period of time. Thats, actually true, at this stage the FW can be updated frequently because of bringing new offloading support. And you are right that currently this driver is mostly in the sandbox mode. But even for the earlier adopters looks like it is good to keep the version consistency. For this particular update it has missing small changes for policer and routing offloading (so it is kind of fix for 3.0 version). So, for easiness it would be good to just replace the current 3.0 version, but for consistency - better to use some extra version. > > Andrew
The following changes since commit 0f66b74b6267fce66395316308d88b0535aa3df2: cypress: update firmware for cyw54591 pcie (2021-06-09 07:12:02 -0400) are available in the Git repository at: https://github.com/PLVision/linux-firmware.git mrvl-prestera for you to fetch changes up to a43d95a48b8e8167e21fb72429d860c7961c2e32: mrvl: prestera: Update Marvell Prestera Switchdev v3.0 with policer support (2021-06-17 18:22:57 +0300) ---------------------------------------------------------------- Vadym Kochan (1): mrvl: prestera: Update Marvell Prestera Switchdev v3.0 with policer support mrvl/prestera/mvsw_prestera_fw-v3.0.img | Bin 13721584 -> 13721676 bytes 1 file changed, 0 insertions(+), 0 deletions(-)