| Message ID | 20120213202846.631.30806.stgit@speedy5.asicdesigners.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 2012-02-13 at 12:28 -0800, Divy Le Ray wrote:
> This patch updates the firmware version cxgb3 is requesting to 7.12.0.
I'm confused. Your patch doesn't change anything in the driver, except
the firmware version. So why is it a "major" change?
If it's a *compatible* change then we wouldn't normally change the
soname of a library; why do we change the filename of the firmware
(which is the equivalent operation)?
On Monday, February 13, 2012 12:43:37 PM, David Woodhouse wrote: > > On Mon, 2012-02-13 at 12:28 -0800, Divy Le Ray wrote: >> >> This patch updates the firmware version cxgb3 is requesting to 7.12.0. > > > I'm confused. Your patch doesn't change anything in the driver, except > the firmware version. So why is it a "major" change? Hi David, cxgb3 embeds the firmware file name it will request at load time. It uses the FW_VERSION_* defs to construct the firmware name FW_FNAME before calling request_firmware(). Hence the need to update the firmware minor version. On the other hand, the driver version update is not a strict requirement here. I would prefer keeping it though. Cheers, Divy -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2012-02-13 at 13:39 -0800, Divy Le Ray wrote: > On Monday, February 13, 2012 12:43:37 PM, David Woodhouse wrote: > > > > On Mon, 2012-02-13 at 12:28 -0800, Divy Le Ray wrote: > >> > >> This patch updates the firmware version cxgb3 is requesting to 7.12.0. > > > > > > I'm confused. Your patch doesn't change anything in the driver, except > > the firmware version. So why is it a "major" change? > > Hi David, > > cxgb3 embeds the firmware file name it will request at load time. > It uses the FW_VERSION_* defs to construct the firmware name FW_FNAME > before calling request_firmware(). > Hence the need to update the firmware minor version. > > On the other hand, the driver version update is not a strict requirement > here. > I would prefer keeping it though. The problems with this are: 1. Older kernel versions don't benefit from the firmware update. 2. The old firmware has to be kept in linux-firmware to support those old kernel versions, and distributions may have to package more versions. To avoid this, the firmware filename should only be changed if you make incompatible changes in the driver/firmware interface. Ben.
On Monday, February 13, 2012 5:59:14 PM, Ben Hutchings wrote: > > On Mon, 2012-02-13 at 13:39 -0800, Divy Le Ray wrote: >> >> On Monday, February 13, 2012 12:43:37 PM, David Woodhouse wrote: >>> >>> >>> On Mon, 2012-02-13 at 12:28 -0800, Divy Le Ray wrote: >>>> >>>> >>>> This patch updates the firmware version cxgb3 is requesting to 7.12.0. >>> >>> >>> >>> I'm confused. Your patch doesn't change anything in the driver, except >>> the firmware version. So why is it a "major" change? >> >> >> Hi David, >> >> cxgb3 embeds the firmware file name it will request at load time. >> It uses the FW_VERSION_* defs to construct the firmware name FW_FNAME >> before calling request_firmware(). >> Hence the need to update the firmware minor version. >> >> On the other hand, the driver version update is not a strict requirement >> here. >> I would prefer keeping it though. > > > The problems with this are: > 1. Older kernel versions don't benefit from the firmware update. > 2. The old firmware has to be kept in linux-firmware to support those > old kernel versions, and distributions may have to package more > versions. > > To avoid this, the firmware filename should only be changed if you make > incompatible changes in the driver/firmware interface. Hi Ben, The scheme for cxgb3 firmware update was designed in coordination with OEMs in the early stages of the driver existence back in 2007. Committing FW 7.12.0 under the name of t3fw-7.10.0.bin would be confusing, in my mind. Some distributions have explicitly requested the FW rev number to be appended to the firmware file name. Cheers, Divy -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2012-02-13 at 20:49 -0800, Divy Le Ray wrote: > On Monday, February 13, 2012 5:59:14 PM, Ben Hutchings wrote: > > > > On Mon, 2012-02-13 at 13:39 -0800, Divy Le Ray wrote: > >> > >> On Monday, February 13, 2012 12:43:37 PM, David Woodhouse wrote: > >>> > >>> > >>> On Mon, 2012-02-13 at 12:28 -0800, Divy Le Ray wrote: > >>>> > >>>> > >>>> This patch updates the firmware version cxgb3 is requesting to 7.12.0. > >>> > >>> > >>> > >>> I'm confused. Your patch doesn't change anything in the driver, except > >>> the firmware version. So why is it a "major" change? > >> > >> > >> Hi David, > >> > >> cxgb3 embeds the firmware file name it will request at load time. > >> It uses the FW_VERSION_* defs to construct the firmware name FW_FNAME > >> before calling request_firmware(). > >> Hence the need to update the firmware minor version. > >> > >> On the other hand, the driver version update is not a strict requirement > >> here. > >> I would prefer keeping it though. > > > > > > The problems with this are: > > 1. Older kernel versions don't benefit from the firmware update. > > 2. The old firmware has to be kept in linux-firmware to support those > > old kernel versions, and distributions may have to package more > > versions. > > > > To avoid this, the firmware filename should only be changed if you make > > incompatible changes in the driver/firmware interface. > > Hi Ben, > > The scheme for cxgb3 firmware update was designed in coordination with > OEMs in the early stages of the driver existence back in 2007. I'm aware that some OEMs have specifications for flash firmware updates and version numbers. I don't recall seeing anything about naming firmware files that are loaded by the driver at boot. In any case, whatever your OEMs want in out-of-tree driver packages is not necessarily relevant to mainline Linux. > Committing FW 7.12.0 under the name of t3fw-7.10.0.bin would be > confusing, in my mind. Indeed it would, but I was speaking in general terms - that it would be preferable to use something like t3fw-<major>.bin. I assume that the firmware blob has a version embedded in it, so that the driver would still be able to report the exact version through ethtool_ops::get_drvinfo. > Some distributions have explicitly requested the FW rev number to be > appended to the firmware file name. Did they really want the full version there or did they want you to distinguish incompatible versions? Did they assume that the file would be installed by a package (so that the package carries a version number) or not? Ben.
On 2/13/2012 9:08 PM, Ben Hutchings wrote: > On Mon, 2012-02-13 at 20:49 -0800, Divy Le Ray wrote: >> On Monday, February 13, 2012 5:59:14 PM, Ben Hutchings wrote: >>> On Mon, 2012-02-13 at 13:39 -0800, Divy Le Ray wrote: >>>> On Monday, February 13, 2012 12:43:37 PM, David Woodhouse wrote: >>>>> >>>>> On Mon, 2012-02-13 at 12:28 -0800, Divy Le Ray wrote: >>>>>> >>>>>> This patch updates the firmware version cxgb3 is requesting to 7.12.0. >>>>> >>>>> >>>>> I'm confused. Your patch doesn't change anything in the driver, except >>>>> the firmware version. So why is it a "major" change? >>>> >>>> Hi David, >>>> >>>> cxgb3 embeds the firmware file name it will request at load time. >>>> It uses the FW_VERSION_* defs to construct the firmware name FW_FNAME >>>> before calling request_firmware(). >>>> Hence the need to update the firmware minor version. >>>> >>>> On the other hand, the driver version update is not a strict requirement >>>> here. >>>> I would prefer keeping it though. >>> >>> The problems with this are: >>> 1. Older kernel versions don't benefit from the firmware update. >>> 2. The old firmware has to be kept in linux-firmware to support those >>> old kernel versions, and distributions may have to package more >>> versions. >>> >>> To avoid this, the firmware filename should only be changed if you make >>> incompatible changes in the driver/firmware interface. >> Hi Ben, >> >> The scheme for cxgb3 firmware update was designed in coordination with >> OEMs in the early stages of the driver existence back in 2007. > I'm aware that some OEMs have specifications for flash firmware updates > and version numbers. I don't recall seeing anything about naming > firmware files that are loaded by the driver at boot. > > In any case, whatever your OEMs want in out-of-tree driver packages is > not necessarily relevant to mainline Linux. Ben, it was about the driver committed to mainline linux. >> Committing FW 7.12.0 under the name of t3fw-7.10.0.bin would be >> confusing, in my mind. > Indeed it would, but I was speaking in general terms - that it would be > preferable to use something like t3fw-<major>.bin. Okay, I understand now. We'd have to discuss it with the original OEMS and distributions. I'd rather stick to the current scheme for the time being and update it once we have agreement from all parties. > I assume that the firmware blob has a version embedded in it, so that > the driver would still be able to report the exact version through > ethtool_ops::get_drvinfo. > >> Some distributions have explicitly requested the FW rev number to be >> appended to the firmware file name. > Did they really want the full version there or did they want you to > distinguish incompatible versions? Did they assume that the file would > be installed by a package (so that the package carries a version number) > or not? Our orginal approach for cxgb4 was to commit its firmware under a constant name. Following minor FW updates overwrite the existing firmware file, and it raised some negative feedback. The cxgb3 scheme had not raised any concern until today, it seemed natural to get back to a similar approach for cxgb4, at least partially. Cheers, Divy -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What does an end-user/customer see and do? When they are told that version the firmware, specified down to the fix level, fixes a problem they are having, how will they know that firmware blob filename drvfw-X.bin on a download/archive site has that fix? Yes, the support folks can say "It is the drcfw-X.bin file which is 12345 bytes in size with the MD5sum of <blah> but the principle of the telephone game dictates the size and/or MD5sum information will drop as "the word" passes from one person to another. And one cannot run ethtool_ops::get_drvinfo against a website. Two versions may indeed be "compatible" in terms of interfaces, but they won't be identical in terms of desirability. As such, for them to be contained in files with identical names just seems like asking for confusion. Firmware is something of a tiny kernel no? Would we have everyone name vmlinux files with just a major number? rick jones -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 14, 2012 at 10:17:27AM -0800, Rick Jones wrote: > What does an end-user/customer see and do? When they are told that > version the firmware, specified down to the fix level, fixes a > problem they are having, how will they know that firmware blob > filename drvfw-X.bin on a download/archive site has that fix? [...] Customers currently can't get bug fixes by updating the firmware. Think about it. Ben.
On 02/14/2012 10:21 AM, Ben Hutchings wrote: > On Tue, Feb 14, 2012 at 10:17:27AM -0800, Rick Jones wrote: >> What does an end-user/customer see and do? When they are told that >> version the firmware, specified down to the fix level, fixes a >> problem they are having, how will they know that firmware blob >> filename drvfw-X.bin on a download/archive site has that fix? > [...] > > Customers currently can't get bug fixes by updating the firmware. > Think about it. Because in this case at least the driver is requesting a specific filename down to the minor and micro level and a change in the filename requires a change in the driver? #define FW_VERSION __stringify(FW_VERSION_MAJOR) "." \ __stringify(FW_VERSION_MINOR) "." __stringify(FW_VERSION_MICRO) #define FW_FNAME "cxgb3/t3fw-" FW_VERSION ".bin" rather than the driver having logic to make it willing to accept later minor/micro changes by being more sophisticated in its specification of firmware file. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 14, 2012 at 12:02:26PM -0800, Rick Jones wrote: > On 02/14/2012 10:21 AM, Ben Hutchings wrote: > >On Tue, Feb 14, 2012 at 10:17:27AM -0800, Rick Jones wrote: > >>What does an end-user/customer see and do? When they are told that > >>version the firmware, specified down to the fix level, fixes a > >>problem they are having, how will they know that firmware blob > >>filename drvfw-X.bin on a download/archive site has that fix? > >[...] > > > >Customers currently can't get bug fixes by updating the firmware. > >Think about it. > > Because in this case at least the driver is requesting a specific > filename down to the minor and micro level and a change in the > filename requires a change in the driver? > > #define FW_VERSION __stringify(FW_VERSION_MAJOR) "." \ > __stringify(FW_VERSION_MINOR) "." __stringify(FW_VERSION_MICRO) > #define FW_FNAME "cxgb3/t3fw-" FW_VERSION ".bin" > > rather than the driver having logic to make it willing to accept > later minor/micro changes by being more sophisticated in its > specification of firmware file. Well, you can't really be very sophisticated as there is no way to list the available firmware files. The Intel wireless drivers try iterating from the newest to oldest supported API version. But that API version is changed only rarely. If you were to iterate over some wide range of potential future versions it could take a substantial time to find the file that's actualy installed. Ben.
From: Divy Le Ray <divy@chelsio.com> Date: Mon, 13 Feb 2012 12:28:46 -0800 > This patch updates the firmware version cxgb3 is requesting to 7.12.0. > > Signed-off-by: Divy Le Ray <divy@chelsio.com> While I agree with Ben Hutchings that how this is being handled is terrible, it's the current status-quo so I'll apply this. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/chelsio/cxgb3/version.h b/drivers/net/ethernet/chelsio/cxgb3/version.h index 8bda06e..165bfb9 100644 --- a/drivers/net/ethernet/chelsio/cxgb3/version.h +++ b/drivers/net/ethernet/chelsio/cxgb3/version.h @@ -35,10 +35,10 @@ #define DRV_DESC "Chelsio T3 Network Driver" #define DRV_NAME "cxgb3" /* Driver version */ -#define DRV_VERSION "1.1.4-ko" +#define DRV_VERSION "1.1.5-ko" /* Firmware version */ #define FW_VERSION_MAJOR 7 -#define FW_VERSION_MINOR 10 +#define FW_VERSION_MINOR 12 #define FW_VERSION_MICRO 0 #endif /* __CHELSIO_VERSION_H */