| Message ID | 1437583153-31359-1-git-send-email-christian@paral.in |
|---|---|
| State | Superseded |
| Headers | show |
Dear Christian Stewart, On Wed, 22 Jul 2015 09:39:13 -0700, Christian Stewart wrote: > Adding a kernel module based USB wifi driver. There is no official > version of this driver that works properly on ARM and with newer USB > WiFi cards. This driver version builds module 8821au and is based on a > kernel module release from ASUS with various fixes integrated in. > > [Thomas, Yann, Luca: > - Using the new kernel-module setup > - Compacted endian flag define to one line] This last paragraph has nothing to do in the commit log itself. The changelog between the different versions of the patch should go... > > Signed-off-by: Christian Stewart <christian@paral.in> > --- here, i.e after the "---" line. > diff --git a/package/rtl8821au/Config.in b/package/rtl8821au/Config.in > new file mode 100644 > index 0000000..0a8cd74 > --- /dev/null > +++ b/package/rtl8821au/Config.in > @@ -0,0 +1,7 @@ > +config BR2_PACKAGE_RTL8821AU > + bool "rtl8821au" > + depends on BR2_LINUX_KERNEL > + help > + rtl8821au wifi driver > + > + https://github.com/paralin/rtl8821au Indentation for the help text is one tab + two spaces. Also, it is customary to add a comment like: comment "rtl8821au needs a Linux kernel to be built" depends on !BR2_LINUX_KERNEL > diff --git a/package/rtl8821au/rtl8821au.mk b/package/rtl8821au/rtl8821au.mk > new file mode 100644 > index 0000000..9c3d286 > --- /dev/null > +++ b/package/rtl8821au/rtl8821au.mk > @@ -0,0 +1,21 @@ > +################################################################################ > +# > +# rtl8821au > +# > +################################################################################ > + > +RTL8821AU_VERSION = 4.3.14 > +RTL8821AU_SITE = $(call github,paralin,rtl8821au,v$(RTL8821AU_VERSION)) > +RTL8821AU_CFLAGS_ENDIAN = -DCONFIG_$(call qstrip,$(BR2_ENDIAN))_ENDIAN > + > +define RTL8821AU_CONFIGURE_CMDS > + $(SED) "s/CONFIG_PLATFORM_I386_PC = y/CONFIG_PLATFORM_I386_PC = n/g" $(@D)/Makefile > +endef I think this $(SED) is probably unnecessary, you can probably just pass CONFIG_PLATFORM_I386_PC=n on the command line, i.e > + > +RTL8821AU_MODULE_MAKE_OPTS = \ > + CONFIG_RTL8821AU=m \ > + KVER=$(LINUX_VERSION_PROBED) \ > + USER_EXTRA_CFLAGS=$(RTL8821AU_CFLAGS_ENDIAN) CONFIG_PLATFORM_I386_PC=n However, this driver doesn't build for me. First because the usage of __DATE__ and __TIME__ is triggering the following build error: /home/thomas/projets/buildroot/output/build/rtl8821au-4.3.14/./core/rtw_debug.c:70:64: error: macro "__DATE__" might prevent reproducible builds [-Werror=date-time] DBG_871X_SEL_NL(sel, "build time: %s %s\n", __DATE__, __TIME__); ^ Then, after patching the code to remove this useless debugging line, the build fails with: /home/thomas/projets/buildroot/output/build/rtl8821au-4.3.14/./os_dep/linux/rtw_android.c: In function ‘rtw_android_cmdstr_to_num’: /home/thomas/projets/buildroot/output/build/rtl8821au-4.3.14/./os_dep/linux/rtw_android.c:346:3: error: implicit declaration of function ‘strnicmp’ [-Werror=implicit-function-declaration] if(0 == strnicmp(cmdstr , android_wifi_cmd_str[cmd_num], strlen(android_wifi_cmd_str[cmd_num])) ) ^ Here is the defconfig to reproduce the problem: BR2_arm=y BR2_TOOLCHAIN_EXTERNAL=y BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-2015.05-496-g85945aa.tar.bz2" BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_1=y BR2_TOOLCHAIN_EXTERNAL_LOCALE=y # BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y BR2_TOOLCHAIN_EXTERNAL_CXX=y BR2_INIT_NONE=y BR2_SYSTEM_BIN_SH_NONE=y BR2_LINUX_KERNEL=y BR2_LINUX_KERNEL_CUSTOM_VERSION=y BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.0" BR2_LINUX_KERNEL_DEFCONFIG="sama5" BR2_LINUX_KERNEL_ZIMAGE=y # BR2_PACKAGE_BUSYBOX is not set BR2_PACKAGE_RTL8821AU=y # BR2_TARGET_ROOTFS_TAR is not set Can you rework your patch to fix those issues? Thanks! Thomas
Hi Thomas, I've re-submitted with the fixes. I've just realized I moved the differences comment below the Signed-off note, but not below the ---. I have seen these notes in the git history before so I assumed they were part of the commit message. Is this not the case? I'll fix it if so. Christian On Wed, Jul 22, 2015 at 2:20 PM Thomas Petazzoni < thomas.petazzoni@free-electrons.com> wrote: > Dear Christian Stewart, > > On Wed, 22 Jul 2015 09:39:13 -0700, Christian Stewart wrote: > > Adding a kernel module based USB wifi driver. There is no official > > version of this driver that works properly on ARM and with newer USB > > WiFi cards. This driver version builds module 8821au and is based on a > > kernel module release from ASUS with various fixes integrated in. > > > > [Thomas, Yann, Luca: > > - Using the new kernel-module setup > > - Compacted endian flag define to one line] > > This last paragraph has nothing to do in the commit log itself. The > changelog between the different versions of the patch should go... > > > > > Signed-off-by: Christian Stewart <christian@paral.in> > > --- > > here, i.e after the "---" line. > > > > diff --git a/package/rtl8821au/Config.in b/package/rtl8821au/Config.in > > new file mode 100644 > > index 0000000..0a8cd74 > > --- /dev/null > > +++ b/package/rtl8821au/Config.in > > @@ -0,0 +1,7 @@ > > +config BR2_PACKAGE_RTL8821AU > > + bool "rtl8821au" > > + depends on BR2_LINUX_KERNEL > > + help > > + rtl8821au wifi driver > > + > > + https://github.com/paralin/rtl8821au > > Indentation for the help text is one tab + two spaces. Also, it is > customary to add a comment like: > > comment "rtl8821au needs a Linux kernel to be built" > depends on !BR2_LINUX_KERNEL > > > diff --git a/package/rtl8821au/rtl8821au.mk b/package/rtl8821au/ > rtl8821au.mk > > new file mode 100644 > > index 0000000..9c3d286 > > --- /dev/null > > +++ b/package/rtl8821au/rtl8821au.mk > > @@ -0,0 +1,21 @@ > > > +################################################################################ > > +# > > +# rtl8821au > > +# > > > +################################################################################ > > + > > +RTL8821AU_VERSION = 4.3.14 > > +RTL8821AU_SITE = $(call github,paralin,rtl8821au,v$(RTL8821AU_VERSION)) > > +RTL8821AU_CFLAGS_ENDIAN = -DCONFIG_$(call qstrip,$(BR2_ENDIAN))_ENDIAN > > + > > +define RTL8821AU_CONFIGURE_CMDS > > + $(SED) "s/CONFIG_PLATFORM_I386_PC = y/CONFIG_PLATFORM_I386_PC = > n/g" $(@D)/Makefile > > +endef > > I think this $(SED) is probably unnecessary, you can probably just pass > CONFIG_PLATFORM_I386_PC=n on the command line, i.e > > > + > > +RTL8821AU_MODULE_MAKE_OPTS = \ > > + CONFIG_RTL8821AU=m \ > > + KVER=$(LINUX_VERSION_PROBED) \ > > + USER_EXTRA_CFLAGS=$(RTL8821AU_CFLAGS_ENDIAN) > > CONFIG_PLATFORM_I386_PC=n > > However, this driver doesn't build for me. > > First because the usage of __DATE__ and __TIME__ is triggering the > following build error: > > /home/thomas/projets/buildroot/output/build/rtl8821au-4.3.14/./core/rtw_debug.c:70:64: > error: macro "__DATE__" might prevent reproducible builds > [-Werror=date-time] > DBG_871X_SEL_NL(sel, "build time: %s %s\n", __DATE__, __TIME__); > ^ > Then, after patching the code to remove this useless debugging line, > the build fails with: > > /home/thomas/projets/buildroot/output/build/rtl8821au-4.3.14/./os_dep/linux/rtw_android.c: > In function ‘rtw_android_cmdstr_to_num’: > /home/thomas/projets/buildroot/output/build/rtl8821au-4.3.14/./os_dep/linux/rtw_android.c:346:3: > error: implicit declaration of function ‘strnicmp’ > [-Werror=implicit-function-declaration] > if(0 == strnicmp(cmdstr , android_wifi_cmd_str[cmd_num], > strlen(android_wifi_cmd_str[cmd_num])) ) > ^ > > Here is the defconfig to reproduce the problem: > > BR2_arm=y > BR2_TOOLCHAIN_EXTERNAL=y > BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y > BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y > BR2_TOOLCHAIN_EXTERNAL_URL=" > http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-2015.05-496-g85945aa.tar.bz2 > " > BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_1=y > BR2_TOOLCHAIN_EXTERNAL_LOCALE=y > # BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set > BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y > BR2_TOOLCHAIN_EXTERNAL_CXX=y > BR2_INIT_NONE=y > BR2_SYSTEM_BIN_SH_NONE=y > BR2_LINUX_KERNEL=y > BR2_LINUX_KERNEL_CUSTOM_VERSION=y > BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.0" > BR2_LINUX_KERNEL_DEFCONFIG="sama5" > BR2_LINUX_KERNEL_ZIMAGE=y > # BR2_PACKAGE_BUSYBOX is not set > BR2_PACKAGE_RTL8821AU=y > # BR2_TARGET_ROOTFS_TAR is not set > > Can you rework your patch to fix those issues? > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com >
Dear Christian Stewart, On Wed, 22 Jul 2015 22:27:54 +0000, Christian Stewart wrote: > I've just realized I moved the differences comment below the Signed-off > note, but not below the ---. I have seen these notes in the git history > before so I assumed they were part of the commit message. Is this not the > case? I'll fix it if so. The notes that we keep in the commit message are the changes that we do when applying the patch, if any, so that it is clear what was the original thing done by the submitter and which things were changed by the person applying the patch. But for the changelog of changes done by the submitter, it is not necessary to keep it part of the commit log. Best regards, Thomas
Hey Thomas, I've respun it without the notes, looking forward to your comments. Sorry for the confusion and patch spam. Best, Christian On Wed, Jul 22, 2015, 11:46 PM Thomas Petazzoni < thomas.petazzoni@free-electrons.com> wrote: > Dear Christian Stewart, > > On Wed, 22 Jul 2015 22:27:54 +0000, Christian Stewart wrote: > > > I've just realized I moved the differences comment below the Signed-off > > note, but not below the ---. I have seen these notes in the git history > > before so I assumed they were part of the commit message. Is this not the > > case? I'll fix it if so. > > The notes that we keep in the commit message are the changes that we do > when applying the patch, if any, so that it is clear what was the > original thing done by the submitter and which things were changed by > the person applying the patch. > > But for the changelog of changes done by the submitter, it is not > necessary to keep it part of the commit log. > > Best regards, > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com >
diff --git a/package/Config.in b/package/Config.in index 9942e3a..04f2136 100644 --- a/package/Config.in +++ b/package/Config.in @@ -404,6 +404,7 @@ endif source "package/read-edid/Config.in" source "package/rng-tools/Config.in" source "package/rpi-userland/Config.in" + source "package/rtl8821au/Config.in" source "package/sane-backends/Config.in" source "package/sdparm/Config.in" source "package/setserial/Config.in" diff --git a/package/rtl8821au/Config.in b/package/rtl8821au/Config.in new file mode 100644 index 0000000..0a8cd74 --- /dev/null +++ b/package/rtl8821au/Config.in @@ -0,0 +1,7 @@ +config BR2_PACKAGE_RTL8821AU + bool "rtl8821au" + depends on BR2_LINUX_KERNEL + help + rtl8821au wifi driver + + https://github.com/paralin/rtl8821au diff --git a/package/rtl8821au/rtl8821au.hash b/package/rtl8821au/rtl8821au.hash new file mode 100644 index 0000000..21f47a2 --- /dev/null +++ b/package/rtl8821au/rtl8821au.hash @@ -0,0 +1 @@ +sha256 7ab9aae237ff562d5c40675115ffb9e36a4774490df0a6a8a4c7746dfd567e14 rtl8821au-4.3.14.tar.gz diff --git a/package/rtl8821au/rtl8821au.mk b/package/rtl8821au/rtl8821au.mk new file mode 100644 index 0000000..9c3d286 --- /dev/null +++ b/package/rtl8821au/rtl8821au.mk @@ -0,0 +1,21 @@ +################################################################################ +# +# rtl8821au +# +################################################################################ + +RTL8821AU_VERSION = 4.3.14 +RTL8821AU_SITE = $(call github,paralin,rtl8821au,v$(RTL8821AU_VERSION)) +RTL8821AU_CFLAGS_ENDIAN = -DCONFIG_$(call qstrip,$(BR2_ENDIAN))_ENDIAN + +define RTL8821AU_CONFIGURE_CMDS + $(SED) "s/CONFIG_PLATFORM_I386_PC = y/CONFIG_PLATFORM_I386_PC = n/g" $(@D)/Makefile +endef + +RTL8821AU_MODULE_MAKE_OPTS = \ + CONFIG_RTL8821AU=m \ + KVER=$(LINUX_VERSION_PROBED) \ + USER_EXTRA_CFLAGS=$(RTL8821AU_CFLAGS_ENDIAN) + +$(eval $(kernel-module)) +$(eval $(generic-package))
Adding a kernel module based USB wifi driver. There is no official version of this driver that works properly on ARM and with newer USB WiFi cards. This driver version builds module 8821au and is based on a kernel module release from ASUS with various fixes integrated in. [Thomas, Yann, Luca: - Using the new kernel-module setup - Compacted endian flag define to one line] Signed-off-by: Christian Stewart <christian@paral.in> --- package/Config.in | 1 + package/rtl8821au/Config.in | 7 +++++++ package/rtl8821au/rtl8821au.hash | 1 + package/rtl8821au/rtl8821au.mk | 21 +++++++++++++++++++++ 4 files changed, 30 insertions(+) create mode 100644 package/rtl8821au/Config.in create mode 100644 package/rtl8821au/rtl8821au.hash create mode 100644 package/rtl8821au/rtl8821au.mk