linux-firmware: Allow selection of bnx2x fw version
diff mbox

Message ID 1456461093-10202-1-git-send-email-joel@jms.id.au
State Rejected
Headers show

Commit Message

Joel Stanley Feb. 26, 2016, 4:31 a.m. UTC
Linux kernel 4.2 moved the driver to a newer firmware version.

Instead of hard coding for 4.2+'s behaviour, introduce a selection
mechanism where the user specifies which version to use based on
the kernel they have.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 package/linux-firmware/Config.in         | 22 ++++++++++++++++++++++
 package/linux-firmware/linux-firmware.mk | 11 ++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

Comments

Thomas Petazzoni Feb. 26, 2016, 8:39 a.m. UTC | #1
Dear Joel Stanley,

On Fri, 26 Feb 2016 15:01:33 +1030, Joel Stanley wrote:
> Linux kernel 4.2 moved the driver to a newer firmware version.
> 
> Instead of hard coding for 4.2+'s behaviour, introduce a selection
> mechanism where the user specifies which version to use based on
> the kernel they have.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  package/linux-firmware/Config.in         | 22 ++++++++++++++++++++++
>  package/linux-firmware/linux-firmware.mk | 11 ++++++++++-
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/package/linux-firmware/Config.in b/package/linux-firmware/Config.in
> index d8389864adaa..b28064b9e45c 100644
> --- a/package/linux-firmware/Config.in
> +++ b/package/linux-firmware/Config.in
> @@ -243,6 +243,28 @@ config BR2_PACKAGE_LINUX_FIRMWARE_BNX2X
>  	help
>  	  Firmware files for Broadcom NetXtreme 10Gb ethernet cards (bnx2x)
>  
> +if BR2_PACKAGE_LINUX_FIRMWARE_BNX2X
> +
> +choice
> +	bool "Broadcom bnx2x revision to use"
> +	help
> +	  Use revision 7.12.30.0 for kernel 4.2 onward.
> +	  Use revision 7.10.51.0 for kernel 3.16 to 4.1.
> +
> +config BR2_PACKAGE_LINUX_FIRMWARE_BNX2X_REV_7_12_30_0
> +	prompt "revision 7.12.30.0"
> +	help
> +	  Use revision 7.12.30.0 for kernel 4.2 onwards.
> +
> +config BR2_PACKAGE_LINUX_FIRMWARE_BNX2X_REV_7_10_51_0
> +	prompt "revision 7.10.51.0"
> +	help
> +	  Use revision 7.10.51.0 for kernel 3.16 to 4.1.
> +
> +endchoice

I think adding new config to chose between different versions of a
given firmware is going a bit too far. If we were to do that for all
firmwares in linux-firmware, it would really increase the number of
Config.in options too much.

Shall I suggest to install both versions of the firmware? The firmware
files are not that large, and if filesystem size is really a strong
issue, it is always possible to clean up the non-required firmware
files in a post-build script.

What do you think?

Thanks!

Thomas
Joel Stanley Feb. 29, 2016, 1:45 a.m. UTC | #2
On Fri, Feb 26, 2016 at 7:09 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> I think adding new config to chose between different versions of a
> given firmware is going a bit too far. If we were to do that for all
> firmwares in linux-firmware, it would really increase the number of
> Config.in options too much.

Yeah, I do agree with this concern. It's unfortunate we don't have a
variable we can test to see what the kernel version is.

> Shall I suggest to install both versions of the firmware? The firmware
> files are not that large, and if filesystem size is really a strong
> issue, it is always possible to clean up the non-required firmware
> files in a post-build script.

In our case the extra ~640kB would be worth cleaning up. I like
buildroot for the clean tiny images it produces; I think that's worth
preserving.

The driver was broken for the 2015.11 release, so given no one has
noticed since then, perhaps we should track the correct firmware
versions for BR2_LINUX_KERNEL_LATEST_VERSION?

Cheers,

Joel
Stewart Smith Feb. 29, 2016, 2:51 a.m. UTC | #3
Joel Stanley <joel@jms.id.au> writes:
> On Fri, Feb 26, 2016 at 7:09 PM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
>> I think adding new config to chose between different versions of a
>> given firmware is going a bit too far. If we were to do that for all
>> firmwares in linux-firmware, it would really increase the number of
>> Config.in options too much.
>
> Yeah, I do agree with this concern. It's unfortunate we don't have a
> variable we can test to see what the kernel version is.
>
>> Shall I suggest to install both versions of the firmware? The firmware
>> files are not that large, and if filesystem size is really a strong
>> issue, it is always possible to clean up the non-required firmware
>> files in a post-build script.
>
> In our case the extra ~640kB would be worth cleaning up. I like
> buildroot for the clean tiny images it produces; I think that's worth
> preserving.

Also, at some point we run out of room on flash.

Patch
diff mbox

diff --git a/package/linux-firmware/Config.in b/package/linux-firmware/Config.in
index d8389864adaa..b28064b9e45c 100644
--- a/package/linux-firmware/Config.in
+++ b/package/linux-firmware/Config.in
@@ -243,6 +243,28 @@  config BR2_PACKAGE_LINUX_FIRMWARE_BNX2X
 	help
 	  Firmware files for Broadcom NetXtreme 10Gb ethernet cards (bnx2x)
 
+if BR2_PACKAGE_LINUX_FIRMWARE_BNX2X
+
+choice
+	bool "Broadcom bnx2x revision to use"
+	help
+	  Use revision 7.12.30.0 for kernel 4.2 onward.
+	  Use revision 7.10.51.0 for kernel 3.16 to 4.1.
+
+config BR2_PACKAGE_LINUX_FIRMWARE_BNX2X_REV_7_12_30_0
+	prompt "revision 7.12.30.0"
+	help
+	  Use revision 7.12.30.0 for kernel 4.2 onwards.
+
+config BR2_PACKAGE_LINUX_FIRMWARE_BNX2X_REV_7_10_51_0
+	prompt "revision 7.10.51.0"
+	help
+	  Use revision 7.10.51.0 for kernel 3.16 to 4.1.
+
+endchoice
+
+endif
+
 config BR2_PACKAGE_LINUX_FIRMWARE_CXGB4_T4
 	bool "Chelsio T4"
 	help
diff --git a/package/linux-firmware/linux-firmware.mk b/package/linux-firmware/linux-firmware.mk
index 083a381ea00a..a73cbf9d6209 100644
--- a/package/linux-firmware/linux-firmware.mk
+++ b/package/linux-firmware/linux-firmware.mk
@@ -251,7 +251,7 @@  LINUX_FIRMWARE_FILES += iwlwifi-7265-$(BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_REV).u
 LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENCE.iwlwifi_firmware
 endif
 
-ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_BNX2X),y)
+ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_BNX2X_REV_7_10_51_0),y)
 LINUX_FIRMWARE_FILES += \
 	bnx2x/bnx2x-e1-7.10.51.0.fw \
 	bnx2x/bnx2x-e1h-7.10.51.0.fw \
@@ -260,6 +260,15 @@  LINUX_FIRMWARE_FILES += \
 # which is installed unconditionally
 endif
 
+ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_BNX2X_REV_7_12_30_0),y)
+LINUX_FIRMWARE_FILES += \
+	bnx2x/bnx2x-e1-7.12.30.0.fw \
+	bnx2x/bnx2x-e1h-7.12.30.0.fw \
+	bnx2x/bnx2x-e2-7.12.30.0.fw
+# No license file; the license is in the file WHENCE
+# which is installed unconditionally
+endif
+
 ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_CXGB4_T4),y)
 # cxgb4/t4fw.bin is a symlink to cxgb4/t4fw-1.14.4.0.bin
 LINUX_FIRMWARE_FILES += cxgb4/t4fw-1.14.4.0.bin cxgb4/t4fw.bin