| Message ID | 1422453245-11725-1-git-send-email-antoine.tenart@free-electrons.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Dear Antoine Tenart, The preferred title for a new package is: <package name>: new package On Wed, 28 Jan 2015 14:54:05 +0100, Antoine Tenart wrote: > Adds support for the BlueGiga WF111 WiFi driver and the binary utilities > distributed alongside the driver. An account is required to download the > sources from the BlueGiga website, which can be created freely. The > driver is available for armv5, arvmv7a and i386. Typo on armv7. > It is not possible to automatically retrieve the sources, because of the Missing "Since" at the beginning of the sentence? > required user account needed on the BlueGiga website, an option is added > to let the Buildroot user specify the directory were the driver's were -> where "the driver's tarball" -> the driver tarball. > tarball was downloaded. > > Finally, two options must be selected in the Linux kernel configuration: > CONFIG_WIRELESS_EXT and CONFIG_WEXT_PRIV. Please explain that those options are blind options, so they cannot be enabled by a change in linux/linux.mk. And maybe you should explain how the user is supposed to enable such blind options: either patch the kernel to make them non-blind, or enable some other random WiFi driver that selects them. > diff --git a/package/wf111/Config.in b/package/wf111/Config.in > new file mode 100644 > index 000000000000..4806a6958476 > --- /dev/null > +++ b/package/wf111/Config.in > @@ -0,0 +1,25 @@ > +config BR2_PACKAGE_WF111 > + bool "wf111" > + depends on BR2_LINUX_KERNEL > + depends on BR2_ARM_CPU_ARMV5 || BR2_ARM_CPU_ARMV7A || BR2_i386 > + # Binary tools are distributed alongside the driver, and are > + # dynamically linked against the glibc. > + depends on BR2_TOOLCHAIN_USES_GLIBC > + help > + BlueGiga WF111 WiFi driver and utilities. > + > + Warning: CONFIG_WIRELESS_EXT and CONFIG_WEXT_PRIV must be > + selected in the Linux kernel configuration. See above: please give more details about this. Also, the upstream URL should be in the Config.in help text of the main option, i.e here. > + > +if BR2_PACKAGE_WF111 > + > +config BR2_PACKAGE_WF111_TARBALL_PATH > + string "WF111 tarball directory location" No need to repeat "WF111" in the prompt, since this option will appear as a sub-option of the previous one, so: string "local tarball location" or something like that. > + help > + The WF111 tarball can be retrieve on the BlueGiga website after retrieve -> retrieved. > + registration. This options specify the path were the tarball is options -> option specify -> specifies were -> where > + locally saved. > + > + http://www.bluegiga.com/en-US/products/wifi-modules/wf111-wifi-module/ > + > +endif Please add: comment "wf111 needs an (e)glibc toolchain" depends on BR2_LINUX_KERNEL depends on BR2_ARM_CPU_ARMV5 || BR2_ARM_CPU_ARMV7A || BR2_i386 depends on !BR2_TOOLCHAIN_USES_GLIBC > diff --git a/package/wf111/wf111.mk b/package/wf111/wf111.mk > new file mode 100644 > index 000000000000..a9642e5d724a > --- /dev/null > +++ b/package/wf111/wf111.mk > @@ -0,0 +1,32 @@ > +################################################################################ > +# > +# wf111 > +# > +################################################################################ > + > +WF111_VERSION = 5.2.2 > +WF111_SITE_METHOD = file > +WF111_SITE = $(BR2_PACKAGE_WF111_TARBALL_PATH) Maybe: WF111_SITE = $(call qstrip,$(BR2_PACKAGE_WF111_TARBALL_PATH)) to strip the double quotes that appear in the value of string options. > +WF111_DEPENDENCIES = linux > + > +ifeq ($(BR2_ARM_CPU_ARMV7A),y) > +WF111_SOURCE = wf111-linux-driver_5.2.2-r1_armv7-a.tar.gz > +else ifeq ($(BR2_ARM_CPU_ARMV5),y) > +WF111_SOURCE = wf111-linux-driver_5.2.2-r1_armv5t.tar.gz > +else ifeq ($(BR2_i386),y) > +WF111_SOURCE = wf111-linux-driver_5.2.2-r1_x86.tar.gz > +endif > + > +define WF111_BUILD_CMDS > + make \ $(MAKE) > + -C $(@D) PWD=$(@D) ARCH=arm \ ARCH=arm, really ? :-) Please use $(LINUX_MAKE_FLAGS) instead, which already contains ARCH= correctly. > + CC=$(TARGET_CC) LD=$(TARGET_LD) \ With $(LINUX_MAKE_FLAGS) containing CROSS_COMPILE, this should probably become unnecessary. > + KDIR=$(LINUX_DIR) \ > + install_static > +endef > + > +define WF111_INSTALL_TARGET_CMDS > + rsync -a $(@D)/output/ $(TARGET_DIR) We typically don't use rsync for such things. What does $(@D)/output contains exactly? We would more typically use: cp -dpfr $(@D)/output/* $(TARGET_DIR) Thanks, Thomas
Thomas, On Wed, Jan 28, 2015 at 04:17:36PM +0100, Thomas Petazzoni wrote: > > > > Finally, two options must be selected in the Linux kernel configuration: > > CONFIG_WIRELESS_EXT and CONFIG_WEXT_PRIV. > > Please explain that those options are blind options, so they cannot be > enabled by a change in linux/linux.mk. And maybe you should explain how > the user is supposed to enable such blind options: either patch the > kernel to make them non-blind, or enable some other random WiFi driver > that selects them. Will do. > > Please add: > > comment "wf111 needs an (e)glibc toolchain" > depends on BR2_LINUX_KERNEL > depends on BR2_ARM_CPU_ARMV5 || BR2_ARM_CPU_ARMV7A || BR2_i386 > depends on !BR2_TOOLCHAIN_USES_GLIBC Sure. > > > + -C $(@D) PWD=$(@D) ARCH=arm \ > > ARCH=arm, really ? :-) Oops :) > > Please use $(LINUX_MAKE_FLAGS) instead, which already contains ARCH= > correctly. > > > + CC=$(TARGET_CC) LD=$(TARGET_LD) \ > > With $(LINUX_MAKE_FLAGS) containing CROSS_COMPILE, this should probably > become unnecessary. I'll update in v2. > > > + KDIR=$(LINUX_DIR) \ > > + install_static > > +endef > > + > > +define WF111_INSTALL_TARGET_CMDS > > + rsync -a $(@D)/output/ $(TARGET_DIR) > > We typically don't use rsync for such things. What does $(@D)/output > contains exactly? We would more typically use: > > cp -dpfr $(@D)/output/* $(TARGET_DIR) Utility binaries, the firmware and the kernel module, all in theirs respective directories. `cp -dpfr` should be fine, I'll update. I'll also fix all the typos, and I'll add a check to be sure BR2_PACKAGE_WF111_TARBALL_PATH is correctly filled. Thanks! Antoine
diff --git a/package/Config.in b/package/Config.in index d65de7057ff4..dc0b0bc15e06 100644 --- a/package/Config.in +++ b/package/Config.in @@ -387,6 +387,7 @@ endif source "package/usbmount/Config.in" source "package/usbutils/Config.in" source "package/w_scan/Config.in" + source "package/wf111/Config.in" source "package/wipe/Config.in" source "package/xorriso/Config.in" endmenu diff --git a/package/wf111/Config.in b/package/wf111/Config.in new file mode 100644 index 000000000000..4806a6958476 --- /dev/null +++ b/package/wf111/Config.in @@ -0,0 +1,25 @@ +config BR2_PACKAGE_WF111 + bool "wf111" + depends on BR2_LINUX_KERNEL + depends on BR2_ARM_CPU_ARMV5 || BR2_ARM_CPU_ARMV7A || BR2_i386 + # Binary tools are distributed alongside the driver, and are + # dynamically linked against the glibc. + depends on BR2_TOOLCHAIN_USES_GLIBC + help + BlueGiga WF111 WiFi driver and utilities. + + Warning: CONFIG_WIRELESS_EXT and CONFIG_WEXT_PRIV must be + selected in the Linux kernel configuration. + +if BR2_PACKAGE_WF111 + +config BR2_PACKAGE_WF111_TARBALL_PATH + string "WF111 tarball directory location" + help + The WF111 tarball can be retrieve on the BlueGiga website after + registration. This options specify the path were the tarball is + locally saved. + + http://www.bluegiga.com/en-US/products/wifi-modules/wf111-wifi-module/ + +endif diff --git a/package/wf111/wf111.mk b/package/wf111/wf111.mk new file mode 100644 index 000000000000..a9642e5d724a --- /dev/null +++ b/package/wf111/wf111.mk @@ -0,0 +1,32 @@ +################################################################################ +# +# wf111 +# +################################################################################ + +WF111_VERSION = 5.2.2 +WF111_SITE_METHOD = file +WF111_SITE = $(BR2_PACKAGE_WF111_TARBALL_PATH) +WF111_DEPENDENCIES = linux + +ifeq ($(BR2_ARM_CPU_ARMV7A),y) +WF111_SOURCE = wf111-linux-driver_5.2.2-r1_armv7-a.tar.gz +else ifeq ($(BR2_ARM_CPU_ARMV5),y) +WF111_SOURCE = wf111-linux-driver_5.2.2-r1_armv5t.tar.gz +else ifeq ($(BR2_i386),y) +WF111_SOURCE = wf111-linux-driver_5.2.2-r1_x86.tar.gz +endif + +define WF111_BUILD_CMDS + make \ + -C $(@D) PWD=$(@D) ARCH=arm \ + CC=$(TARGET_CC) LD=$(TARGET_LD) \ + KDIR=$(LINUX_DIR) \ + install_static +endef + +define WF111_INSTALL_TARGET_CMDS + rsync -a $(@D)/output/ $(TARGET_DIR) +endef + +$(eval $(generic-package))
Adds support for the BlueGiga WF111 WiFi driver and the binary utilities distributed alongside the driver. An account is required to download the sources from the BlueGiga website, which can be created freely. The driver is available for armv5, arvmv7a and i386. It is not possible to automatically retrieve the sources, because of the required user account needed on the BlueGiga website, an option is added to let the Buildroot user specify the directory were the driver's tarball was downloaded. Finally, two options must be selected in the Linux kernel configuration: CONFIG_WIRELESS_EXT and CONFIG_WEXT_PRIV. Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> --- package/Config.in | 1 + package/wf111/Config.in | 25 +++++++++++++++++++++++++ package/wf111/wf111.mk | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 package/wf111/Config.in create mode 100644 package/wf111/wf111.mk