[PATCH RFC] drivers: most: add USB adapter driver
This patch adds the MOST USB adapter driver to the stable branch. This is a follow-up to commit . Signed-off-by: Christian Gromm --- drivers/most/Kconfig |6 + drivers/most/Makefile |2 + drivers/most/usb/Kconfig | 14 + drivers/most/usb/Makefile |4 + drivers/most/usb/usb.c| 1262 + drivers/staging/most/Kconfig |2 - drivers/staging/most/Makefile |1 - 7 files changed, 1288 insertions(+), 3 deletions(-) create mode 100644 drivers/most/usb/Kconfig create mode 100644 drivers/most/usb/Makefile create mode 100644 drivers/most/usb/usb.c diff --git a/drivers/most/Kconfig b/drivers/most/Kconfig index 58d7999..400ed52 100644 --- a/drivers/most/Kconfig +++ b/drivers/most/Kconfig @@ -13,3 +13,9 @@ menuconfig MOST module will be called most_core. If in doubt, say N here. + +if MOST + +source "drivers/most/usb/Kconfig" + +endif diff --git a/drivers/most/Makefile b/drivers/most/Makefile index e810cd3..4daa370 100644 --- a/drivers/most/Makefile +++ b/drivers/most/Makefile @@ -2,3 +2,5 @@ obj-$(CONFIG_MOST) += most_core.o most_core-y := core.o \ configfs.o + +obj-$(CONFIG_MOST_USB) += usb/ diff --git a/drivers/most/usb/Kconfig b/drivers/most/usb/Kconfig new file mode 100644 index 000..a86f1f6 --- /dev/null +++ b/drivers/most/usb/Kconfig @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# MOST USB configuration +# + +config MOST_USB + tristate "USB" + depends on USB && NET + help + Say Y here if you want to connect via USB to network tranceiver. + This device driver depends on the networking AIM. + + To compile this driver as a module, choose M here: the + module will be called most_usb. diff --git a/drivers/most/usb/Makefile b/drivers/most/usb/Makefile new file mode 100644 index 000..c2b2073 --- /dev/null +++ b/drivers/most/usb/Makefile @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0 +obj-$(CONFIG_MOST_USB) += most_usb.o + +most_usb-objs := usb.o diff --git a/drivers/most/usb/usb.c b/drivers/most/usb/usb.c new file mode 100644 index 000..daa5e4b --- /dev/null +++ b/drivers/most/usb/usb.c @@ -0,0 +1,1262 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * usb.c - Hardware dependent module for USB + * + * Copyright (C) 2013-2015 Microchip Technology Germany II GmbH & Co. KG + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define USB_MTU512 +#define NO_ISOCHRONOUS_URB 0 +#define AV_PACKETS_PER_XACT2 +#define BUF_CHAIN_SIZE 0x +#define MAX_NUM_ENDPOINTS 30 +#define MAX_SUFFIX_LEN 10 +#define MAX_STRING_LEN 80 +#define MAX_BUF_SIZE 0x + +#define USB_VENDOR_ID_SMSC 0x0424 /* VID: SMSC */ +#define USB_DEV_ID_BRDG0xC001 /* PID: USB Bridge */ +#define USB_DEV_ID_OS81118 0xCF18 /* PID: USB OS81118 */ +#define USB_DEV_ID_OS81119 0xCF19 /* PID: USB OS81119 */ +#define USB_DEV_ID_OS81210 0xCF30 /* PID: USB OS81210 */ +/* DRCI Addresses */ +#define DRCI_REG_NI_STATE 0x0100 +#define DRCI_REG_PACKET_BW 0x0101 +#define DRCI_REG_NODE_ADDR 0x0102 +#define DRCI_REG_NODE_POS 0x0103 +#define DRCI_REG_MEP_FILTER0x0140 +#define DRCI_REG_HASH_TBL0 0x0141 +#define DRCI_REG_HASH_TBL1 0x0142 +#define DRCI_REG_HASH_TBL2 0x0143 +#define DRCI_REG_HASH_TBL3 0x0144 +#define DRCI_REG_HW_ADDR_HI0x0145 +#define DRCI_REG_HW_ADDR_MI0x0146 +#define DRCI_REG_HW_ADDR_LO0x0147 +#define DRCI_REG_BASE 0x1100 +#define DRCI_COMMAND 0x02 +#define DRCI_READ_REQ 0xA0 +#define DRCI_WRITE_REQ 0xA1 + +/** + * struct most_dci_obj - Direct Communication Interface + * @kobj:position in sysfs + * @usb_device: pointer to the usb device + * @reg_addr: register address for arbitrary DCI access + */ +struct most_dci_obj { + struct device dev; + struct usb_device *usb_device; + u16 reg_addr; +}; + +#define to_dci_obj(p) container_of(p, struct most_dci_obj, dev) + +struct most_dev; + +struct clear_hold_work { + struct work_struct ws; + struct most_dev *mdev; + unsigned int channel; + int pipe; +}; + +#define to_clear_hold_work(w) container_of(w, struct clear_hold_work, ws) + +/** + * struct most_dev - holds all usb interface specific stuff + * @usb_device: pointer to usb device + * @iface: hardware interface + * @cap: channel capabilities + * @conf: channel configuration + * @dci: direct communication interface of hardware + * @ep_address: endpoint address table + * @description: device description + * @suffix: suffix for channel name + * @channel_lock: synchronize channel access + * @padding_active: indicates channel uses padding
[PATCH -next] media: tegra: Make tegra210_video_formats static
Fix the following sparse warning: drivers/staging/media/tegra-video/tegra210.c:589:33: warning: symbol 'tegra210_video_formats' was not declared. The tegra210_video_formats has only call site within tegra210.c It should be static Fixes: 423d10a99b30 ("media: tegra: Add Tegra210 Video input driver") Reported-by: Hulk Robot Signed-off-by: Samuel Zou --- drivers/staging/media/tegra-video/tegra210.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/tegra-video/tegra210.c b/drivers/staging/media/tegra-video/tegra210.c index 2045609..3baa4e3 100644 --- a/drivers/staging/media/tegra-video/tegra210.c +++ b/drivers/staging/media/tegra-video/tegra210.c @@ -586,7 +586,7 @@ enum tegra210_image_format { } /* Tegra210 supported video formats */ -const struct tegra_video_format tegra210_video_formats[] = { +static const struct tegra_video_format tegra210_video_formats[] = { /* RAW 8 */ TEGRA210_VIDEO_FMT(RAW8, 8, SRGGB8_1X8, 1, T_L8, SRGGB8), TEGRA210_VIDEO_FMT(RAW8, 8, SGRBG8_1X8, 1, T_L8, SGRBG8), -- 2.6.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC] drivers: most: add USB adapter driver
On Mon, May 11, 2020 at 11:51:15AM +0200, Christian Gromm wrote: > This patch adds the MOST USB adapter driver to the stable branch. This is > a follow-up to commit . I do not understand the "a follow-up..." sentance. Always use the format of: b27652753918 ("staging: most: move core files out of the staging area") when writing kernel commits in changelogs. Also, that commit doesn't really mean anything here, this is a stand-alone driver for the most subsystem. This changelog needs work. > > Signed-off-by: Christian Gromm > --- > drivers/most/Kconfig |6 + > drivers/most/Makefile |2 + > drivers/most/usb/Kconfig | 14 + > drivers/most/usb/Makefile |4 + > drivers/most/usb/usb.c| 1262 > + Why not just call this file most-usb.c so you don't have to do the 2-step Makefile work. Also, why a whole subdir for a single .c file? > drivers/staging/most/Kconfig |2 - > drivers/staging/most/Makefile |1 - Why touch the staging directory for this patch? We can delete the old driver after this one is merged, no need for that here. > diff --git a/drivers/most/usb/usb.c b/drivers/most/usb/usb.c > new file mode 100644 > index 000..daa5e4b > --- /dev/null > +++ b/drivers/most/usb/usb.c > @@ -0,0 +1,1262 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * usb.c - Hardware dependent module for USB > + * > + * Copyright (C) 2013-2015 Microchip Technology Germany II GmbH & Co. KG > + */ > + > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt You shouldn't need any pr_*() calls because this is a driver and you always have access to the struct device * it controls. So drop this and fix up the remaining pr_*() calls to be dev_*() instead. > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define USB_MTU 512 > +#define NO_ISOCHRONOUS_URB 0 > +#define AV_PACKETS_PER_XACT 2 > +#define BUF_CHAIN_SIZE 0x > +#define MAX_NUM_ENDPOINTS30 > +#define MAX_SUFFIX_LEN 10 > +#define MAX_STRING_LEN 80 > +#define MAX_BUF_SIZE 0x > + > +#define USB_VENDOR_ID_SMSC 0x0424 /* VID: SMSC */ > +#define USB_DEV_ID_BRDG 0xC001 /* PID: USB Bridge */ > +#define USB_DEV_ID_OS81118 0xCF18 /* PID: USB OS81118 */ > +#define USB_DEV_ID_OS81119 0xCF19 /* PID: USB OS81119 */ > +#define USB_DEV_ID_OS81210 0xCF30 /* PID: USB OS81210 */ > +/* DRCI Addresses */ > +#define DRCI_REG_NI_STATE0x0100 > +#define DRCI_REG_PACKET_BW 0x0101 > +#define DRCI_REG_NODE_ADDR 0x0102 > +#define DRCI_REG_NODE_POS0x0103 > +#define DRCI_REG_MEP_FILTER 0x0140 > +#define DRCI_REG_HASH_TBL0 0x0141 > +#define DRCI_REG_HASH_TBL1 0x0142 > +#define DRCI_REG_HASH_TBL2 0x0143 > +#define DRCI_REG_HASH_TBL3 0x0144 > +#define DRCI_REG_HW_ADDR_HI 0x0145 > +#define DRCI_REG_HW_ADDR_MI 0x0146 > +#define DRCI_REG_HW_ADDR_LO 0x0147 > +#define DRCI_REG_BASE0x1100 > +#define DRCI_COMMAND 0x02 > +#define DRCI_READ_REQ0xA0 > +#define DRCI_WRITE_REQ 0xA1 > + > +/** > + * struct most_dci_obj - Direct Communication Interface > + * @kobj:position in sysfs > + * @usb_device: pointer to the usb device > + * @reg_addr: register address for arbitrary DCI access > + */ > +struct most_dci_obj { > + struct device dev; Wait, why is a USB driver creating something with a separate struct device embedded in it? Shouldn't the most core handle stuff like this? > + struct usb_device *usb_device; > + u16 reg_addr; > +}; > + > +#define to_dci_obj(p) container_of(p, struct most_dci_obj, dev) > + > +struct most_dev; Don't you already have this in the most.h file? > + > +struct clear_hold_work { > + struct work_struct ws; > + struct most_dev *mdev; > + unsigned int channel; > + int pipe; > +}; > + > +#define to_clear_hold_work(w) container_of(w, struct clear_hold_work, ws) > + > +/** > + * struct most_dev - holds all usb interface specific stuff > + * @usb_device: pointer to usb device > + * @iface: hardware interface > + * @cap: channel capabilities > + * @conf: channel configuration > + * @dci: direct communication interface of hardware > + * @ep_address: endpoint address table > + * @description: device description > + * @suffix: suffix for channel name > + * @channel_lock: synchronize channel access > + * @padding_active: indicates channel uses padding > + * @is_channel_healthy: health status table of each channel > + * @busy_urbs: list of anchored items > + * @io_mutex: synchronize I/O with disconnect > + * @link_stat_timer: timer for link status reports > + * @poll_work_obj: work for polling link status > + */ > +struct most_dev { > + struct device dev; > + struct usb_device *usb_device;
Re: [PATCH RFC] drivers: most: add USB adapter driver
On 5/11/20 2:51 AM, Christian Gromm wrote: > diff --git a/drivers/most/usb/Kconfig b/drivers/most/usb/Kconfig > new file mode 100644 > index 000..a86f1f6 > --- /dev/null > +++ b/drivers/most/usb/Kconfig > @@ -0,0 +1,14 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# > +# MOST USB configuration > +# > + > +config MOST_USB > + tristate "USB" > + depends on USB && NET > + help > + Say Y here if you want to connect via USB to network tranceiver. transceiver. > + This device driver depends on the networking AIM. > + > + To compile this driver as a module, choose M here: the > + module will be called most_usb. -- ~Randy ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC] drivers: most: add USB adapter driver
On Mon, 2020-05-11 at 13:47 +0200, Greg KH wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On Mon, May 11, 2020 at 11:51:15AM +0200, Christian Gromm wrote: > > This patch adds the MOST USB adapter driver to the stable branch. > > This is > > a follow-up to commit . > > I do not understand the "a follow-up..." sentance. Always use the > format of: > b27652753918 ("staging: most: move core files out of the > staging area") > when writing kernel commits in changelogs. > > Also, that commit doesn't really mean anything here, this is a > stand-alone driver for the most subsystem. This changelog needs > work. Purpose was sharing the information that this is patch is only one part of moving the complete driver stack. That a first step has alread been done and others are to follow. But you're probably right and nobody realy needs to know. I'll skip this. > > > Signed-off-by: Christian Gromm > > --- > > drivers/most/Kconfig |6 + > > drivers/most/Makefile |2 + > > drivers/most/usb/Kconfig | 14 + > > drivers/most/usb/Makefile |4 + > > drivers/most/usb/usb.c| 1262 > > + > > Why not just call this file most-usb.c so you don't have to do the > 2-step Makefile work. Also, why a whole subdir for a single .c file? To keep the staging layout. > > > drivers/staging/most/Kconfig |2 - > > drivers/staging/most/Makefile |1 - > > Why touch the staging directory for this patch? We can delete the > old > driver after this one is merged, no need for that here. > > > diff --git a/drivers/most/usb/usb.c b/drivers/most/usb/usb.c > > new file mode 100644 > > index 000..daa5e4b > > --- /dev/null > > +++ b/drivers/most/usb/usb.c > > @@ -0,0 +1,1262 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * usb.c - Hardware dependent module for USB > > + * > > + * Copyright (C) 2013-2015 Microchip Technology Germany II GmbH & > > Co. KG > > + */ > > + > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > You shouldn't need any pr_*() calls because this is a driver and you > always have access to the struct device * it controls. So drop this > and > fix up the remaining pr_*() calls to be dev_*() instead. There are helper functions that actually don't have access to the struct device and it felt like an overhead to pass the device pointer just for logging purposes. > > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define USB_MTU 512 > > +#define NO_ISOCHRONOUS_URB 0 > > +#define AV_PACKETS_PER_XACT 2 > > +#define BUF_CHAIN_SIZE 0x > > +#define MAX_NUM_ENDPOINTS30 > > +#define MAX_SUFFIX_LEN 10 > > +#define MAX_STRING_LEN 80 > > +#define MAX_BUF_SIZE 0x > > + > > +#define USB_VENDOR_ID_SMSC 0x0424 /* VID: SMSC */ > > +#define USB_DEV_ID_BRDG 0xC001 /* PID: USB Bridge */ > > +#define USB_DEV_ID_OS81118 0xCF18 /* PID: USB OS81118 */ > > +#define USB_DEV_ID_OS81119 0xCF19 /* PID: USB OS81119 */ > > +#define USB_DEV_ID_OS81210 0xCF30 /* PID: USB OS81210 */ > > +/* DRCI Addresses */ > > +#define DRCI_REG_NI_STATE0x0100 > > +#define DRCI_REG_PACKET_BW 0x0101 > > +#define DRCI_REG_NODE_ADDR 0x0102 > > +#define DRCI_REG_NODE_POS0x0103 > > +#define DRCI_REG_MEP_FILTER 0x0140 > > +#define DRCI_REG_HASH_TBL0 0x0141 > > +#define DRCI_REG_HASH_TBL1 0x0142 > > +#define DRCI_REG_HASH_TBL2 0x0143 > > +#define DRCI_REG_HASH_TBL3 0x0144 > > +#define DRCI_REG_HW_ADDR_HI 0x0145 > > +#define DRCI_REG_HW_ADDR_MI 0x0146 > > +#define DRCI_REG_HW_ADDR_LO 0x0147 > > +#define DRCI_REG_BASE0x1100 > > +#define DRCI_COMMAND 0x02 > > +#define DRCI_READ_REQ0xA0 > > +#define DRCI_WRITE_REQ 0xA1 > > + > > +/** > > + * struct most_dci_obj - Direct Communication Interface > > + * @kobj:position in sysfs > > + * @usb_device: pointer to the usb device > > + * @reg_addr: register address for arbitrary DCI access > > + */ > > +struct most_dci_obj { > > + struct device dev; > > Wait, why is a USB driver creating something with a separate struct > device embedded in it? Shouldn't the most core handle stuff like > this? The driver adds an ABI interface that belongs to USB only. This keeps the core generic. > > > + struct usb_device *usb_device; > > + u16 reg_addr; > > +}; > > + > > +#define to_dci_obj(p) container_of(p, struct most_dci_obj, dev) > > + > > +struct most_dev; > > Don't you already have this in the most.h file? No. This belongs to USB only and must not be published with the most.h header. > > > + > > +struct clear_hold_work { > > + struc
[driver-core:driver-core-testing] BUILD SUCCESS c8be6af9ef16cf44d690fc227a0d2dd7a526ef05
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git driver-core-testing branch HEAD: c8be6af9ef16cf44d690fc227a0d2dd7a526ef05 Merge v5.7-rc5 into driver-core-next elapsed time: 485m configs tested: 100 configs skipped: 1 The following configs have been built successfully. More configs may be tested in the coming days. arm defconfig arm allyesconfig arm allmodconfig arm allnoconfig arm64allyesconfig arm64 defconfig arm64allmodconfig arm64 allnoconfig sparcallyesconfig m68k allyesconfig i386 allyesconfig i386defconfig i386 debian-10.3 i386 allnoconfig ia64 allmodconfig ia64defconfig ia64 allnoconfig ia64 allyesconfig m68k allmodconfig m68k allnoconfig m68k sun3_defconfig m68kdefconfig nios2 defconfig nios2allyesconfig openriscdefconfig c6x allyesconfig c6x allnoconfig openrisc allyesconfig nds32 defconfig nds32 allnoconfig csky allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig h8300allmodconfig xtensa defconfig arc defconfig arc allyesconfig sh allmodconfig shallnoconfig microblazeallnoconfig mips allyesconfig mips allnoconfig mips allmodconfig pariscallnoconfig parisc defconfig parisc allyesconfig parisc allmodconfig powerpc defconfig powerpc allyesconfig powerpc rhel-kconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a006-20200511 i386 randconfig-a005-20200511 i386 randconfig-a003-20200511 i386 randconfig-a001-20200511 i386 randconfig-a004-20200511 i386 randconfig-a002-20200511 x86_64 randconfig-a016-20200511 x86_64 randconfig-a012-20200511 x86_64 randconfig-a014-20200511 i386 randconfig-a012-20200511 i386 randconfig-a016-20200511 i386 randconfig-a014-20200511 i386 randconfig-a011-20200511 i386 randconfig-a013-20200511 i386 randconfig-a015-20200511 x86_64 randconfig-a005-20200511 x86_64 randconfig-a003-20200511 x86_64 randconfig-a006-20200511 x86_64 randconfig-a004-20200511 x86_64 randconfig-a001-20200511 x86_64 randconfig-a002-20200511 riscvallyesconfig riscv allnoconfig riscv defconfig riscvallmodconfig s390 allyesconfig s390 allnoconfig s390 allmodconfig s390defconfig sparc defconfig sparc64 defconfig sparc64 allnoconfig sparc64 allyesconfig sparc64 allmodconfig um allmodconfig umallnoconfig um allyesconfig um defconfig x86_64 rhel x86_64 rhel-7.6 x86_64rhel-7.6-kselftests x86_64 rhel-7.2-clear x86_64lkp x86_64 fedora-25 x86_64 kexec --- 0-DAY CI Kernel Test Service
[staging:staging-next] BUILD SUCCESS ae73e7784871ebe2c43da619b4a1e2c9ff81508d
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git staging-next branch HEAD: ae73e7784871ebe2c43da619b4a1e2c9ff81508d Merge 5.7-rc5 into staging-next elapsed time: 486m configs tested: 100 configs skipped: 1 The following configs have been built successfully. More configs may be tested in the coming days. arm defconfig arm allyesconfig arm allmodconfig arm allnoconfig arm64allyesconfig arm64 defconfig arm64allmodconfig arm64 allnoconfig sparcallyesconfig m68k allyesconfig parisc allyesconfig i386 allnoconfig i386 allyesconfig i386defconfig i386 debian-10.3 ia64 allmodconfig ia64defconfig ia64 allnoconfig ia64 allyesconfig m68k allmodconfig m68k allnoconfig m68k sun3_defconfig m68kdefconfig nios2 defconfig nios2allyesconfig openriscdefconfig c6x allyesconfig c6x allnoconfig openrisc allyesconfig nds32 defconfig nds32 allnoconfig csky allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig h8300allmodconfig xtensa defconfig arc defconfig arc allyesconfig sh allmodconfig shallnoconfig microblazeallnoconfig mips allyesconfig mips allnoconfig mips allmodconfig pariscallnoconfig parisc defconfig parisc allmodconfig powerpc defconfig powerpc allyesconfig powerpc rhel-kconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a006-20200511 i386 randconfig-a005-20200511 i386 randconfig-a003-20200511 i386 randconfig-a001-20200511 i386 randconfig-a004-20200511 i386 randconfig-a002-20200511 x86_64 randconfig-a016-20200511 x86_64 randconfig-a012-20200511 x86_64 randconfig-a014-20200511 x86_64 randconfig-a005-20200511 x86_64 randconfig-a003-20200511 x86_64 randconfig-a006-20200511 x86_64 randconfig-a004-20200511 x86_64 randconfig-a001-20200511 x86_64 randconfig-a002-20200511 i386 randconfig-a012-20200511 i386 randconfig-a016-20200511 i386 randconfig-a014-20200511 i386 randconfig-a011-20200511 i386 randconfig-a013-20200511 i386 randconfig-a015-20200511 riscvallyesconfig riscv allnoconfig riscv defconfig riscvallmodconfig s390 allyesconfig s390 allnoconfig s390 allmodconfig s390defconfig sparc defconfig sparc64 allnoconfig sparc64 allmodconfig sparc64 defconfig sparc64 allyesconfig um allmodconfig umallnoconfig um allyesconfig um defconfig x86_64 rhel x86_64 rhel-7.6 x86_64rhel-7.6-kselftests x86_64 rhel-7.2-clear x86_64lkp x86_64 fedora-25 x86_64 kexec --- 0-DAY CI Kernel Test Service, Intel Corporation
[PATCH 01/17] staging: wfx: fix use of cpu_to_le32 instead of le32_to_cpu
From: Jérôme Pouiller Sparse detected that le32_to_cpu should be used instead of cpu_to_le32. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/hwio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/wfx/hwio.c b/drivers/staging/wfx/hwio.c index d878cb3e84fc..777217cdf9a7 100644 --- a/drivers/staging/wfx/hwio.c +++ b/drivers/staging/wfx/hwio.c @@ -205,7 +205,7 @@ static int indirect_read32_locked(struct wfx_dev *wdev, int reg, return -ENOMEM; wdev->hwbus_ops->lock(wdev->hwbus_priv); ret = indirect_read(wdev, reg, addr, tmp, sizeof(u32)); - *val = cpu_to_le32(*tmp); + *val = le32_to_cpu(*tmp); _trace_io_ind_read32(reg, addr, *val); wdev->hwbus_ops->unlock(wdev->hwbus_priv); kfree(tmp); -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/17] staging: wfx: take advantage of le32_to_cpup()
From: Jérôme Pouiller le32_to_cpu(*x) can be advantageously converted in le32_to_cpup(x). Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/hif_rx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c index ac4ec4f30496..83c3fdbb10fa 100644 --- a/drivers/staging/wfx/hif_rx.c +++ b/drivers/staging/wfx/hif_rx.c @@ -22,7 +22,7 @@ static int hif_generic_confirm(struct wfx_dev *wdev, const struct hif_msg *hif, const void *buf) { // All confirm messages start with status - int status = le32_to_cpu(*((__le32 *) buf)); + int status = le32_to_cpup((__le32 *)buf); int cmd = hif->id; int len = hif->len - 4; // drop header -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 00/17] staging: wfx: fix support for big-endian hosts
From: Jérôme Pouiller Hello, As already discussed here[1], this series improves support for big endian hosts. All warnings raised by sparse are now fixed. Note, this series aims to be applied on top of PR named "staging: wfx: fix Out-Of-Band IRQ" [1] https://lore.kernel.org/lkml/2019202852.gx26...@zeniv.linux.org.uk Jérôme Pouiller (17): staging: wfx: fix use of cpu_to_le32 instead of le32_to_cpu staging: wfx: take advantage of le32_to_cpup() staging: wfx: fix cast operator staging: wfx: fix wrong bytes order staging: wfx: fix output of rx_stats on big endian hosts staging: wfx: fix endianness of fields media_delay and tx_queue_delay staging: wfx: fix endianness of hif_req_read_mib fields staging: wfx: fix access to le32 attribute 'ps_mode_error' staging: wfx: fix access to le32 attribute 'event_id' staging: wfx: fix access to le32 attribute 'indication_type' staging: wfx: declare the field 'packet_id' with native byte order staging: wfx: fix endianness of the struct hif_ind_startup staging: wfx: fix endianness of the field 'len' staging: wfx: fix endianness of the field 'status' staging: wfx: fix endianness of the field 'num_tx_confs' staging: wfx: fix endianness of the field 'channel_number' staging: wfx: update TODO drivers/staging/wfx/TODO | 19 - drivers/staging/wfx/bh.c | 11 +++--- drivers/staging/wfx/data_rx.c | 4 +- drivers/staging/wfx/data_tx.c | 9 +++-- drivers/staging/wfx/debug.c | 11 -- drivers/staging/wfx/hif_api_cmd.h | 42 +--- drivers/staging/wfx/hif_api_general.h | 55 +-- drivers/staging/wfx/hif_rx.c | 32 drivers/staging/wfx/hif_tx.c | 20 +- drivers/staging/wfx/hif_tx_mib.c | 2 +- drivers/staging/wfx/hwio.c| 2 +- drivers/staging/wfx/main.c| 2 +- drivers/staging/wfx/traces.h | 8 ++-- 13 files changed, 105 insertions(+), 112 deletions(-) -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/17] staging: wfx: fix output of rx_stats on big endian hosts
From: Jérôme Pouiller The struct hif_rx_stats contains only little endian values. Thus, it is necessary to fix byte ordering before to use them. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/debug.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/staging/wfx/debug.c b/drivers/staging/wfx/debug.c index 2fae6c913b01..846a0b29f8c9 100644 --- a/drivers/staging/wfx/debug.c +++ b/drivers/staging/wfx/debug.c @@ -155,7 +155,7 @@ static int wfx_rx_stats_show(struct seq_file *seq, void *v) mutex_lock(&wdev->rx_stats_lock); seq_printf(seq, "Timestamp: %dus\n", st->date); seq_printf(seq, "Low power clock: frequency %uHz, external %s\n", - st->pwr_clk_freq, + le32_to_cpu(st->pwr_clk_freq), st->is_ext_pwr_clk ? "yes" : "no"); seq_printf(seq, "Num. of frames: %d, PER (x10e4): %d, Throughput: %dKbps/s\n", @@ -165,9 +165,12 @@ static int wfx_rx_stats_show(struct seq_file *seq, void *v) for (i = 0; i < ARRAY_SIZE(channel_names); i++) { if (channel_names[i]) seq_printf(seq, "%5s %8d %8d %8d %8d %8d\n", - channel_names[i], st->nb_rx_by_rate[i], - st->per[i], st->rssi[i] / 100, - st->snr[i] / 100, st->cfo[i]); + channel_names[i], + le32_to_cpu(st->nb_rx_by_rate[i]), + le16_to_cpu(st->per[i]), + (s16)le16_to_cpu(st->rssi[i]) / 100, + (s16)le16_to_cpu(st->snr[i]) / 100, + (s16)le16_to_cpu(st->cfo[i])); } mutex_unlock(&wdev->rx_stats_lock); -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 06/17] staging: wfx: fix endianness of fields media_delay and tx_queue_delay
From: Jérôme Pouiller The struct hif_cnf_tx contains only little endian values. Thus, it is necessary to fix byte ordering before to use them. Especially, sparse detected wrong access to fields media_delay and tx_queue_delay. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/data_tx.c | 3 ++- drivers/staging/wfx/traces.h | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c index f64149ab0484..014fa36c8f78 100644 --- a/drivers/staging/wfx/data_tx.c +++ b/drivers/staging/wfx/data_tx.c @@ -562,7 +562,8 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg) if (!arg->status) { tx_info->status.tx_time = - arg->media_delay - arg->tx_queue_delay; + le32_to_cpu(arg->media_delay) - + le32_to_cpu(arg->tx_queue_delay); if (tx_info->flags & IEEE80211_TX_CTL_NO_ACK) tx_info->flags |= IEEE80211_TX_STAT_NOACK_TRANSMITTED; else diff --git a/drivers/staging/wfx/traces.h b/drivers/staging/wfx/traces.h index c78c46b1c990..959a0d31bf4e 100644 --- a/drivers/staging/wfx/traces.h +++ b/drivers/staging/wfx/traces.h @@ -387,8 +387,8 @@ TRACE_EVENT(tx_stats, int i; __entry->pkt_id = tx_cnf->packet_id; - __entry->delay_media = tx_cnf->media_delay; - __entry->delay_queue = tx_cnf->tx_queue_delay; + __entry->delay_media = le32_to_cpu(tx_cnf->media_delay); + __entry->delay_queue = le32_to_cpu(tx_cnf->tx_queue_delay); __entry->delay_fw = delay; __entry->ack_failures = tx_cnf->ack_failures; if (!tx_cnf->status || __entry->ack_failures) -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 04/17] staging: wfx: fix wrong bytes order
From: Jérôme Pouiller The field wakeup_period_max from struct hif_mib_beacon_wake_up_period is a u8. So, assigning it a __le16 produces a nasty bug on big-endian architectures. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/hif_tx_mib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/wfx/hif_tx_mib.c b/drivers/staging/wfx/hif_tx_mib.c index 65381b22437e..567c61d1fe2e 100644 --- a/drivers/staging/wfx/hif_tx_mib.c +++ b/drivers/staging/wfx/hif_tx_mib.c @@ -32,7 +32,7 @@ int hif_set_beacon_wakeup_period(struct wfx_vif *wvif, struct hif_mib_beacon_wake_up_period val = { .wakeup_period_min = dtim_interval, .receive_dtim = 0, - .wakeup_period_max = cpu_to_le16(listen_interval), + .wakeup_period_max = listen_interval, }; if (dtim_interval > 0xFF || listen_interval > 0x) -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 09/17] staging: wfx: fix access to le32 attribute 'event_id'
From: Jérôme Pouiller The attribute event_id is little-endian. We have to take to the endianness when we access it. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/hif_rx.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c index 87d5107a7757..966315edbab8 100644 --- a/drivers/staging/wfx/hif_rx.c +++ b/drivers/staging/wfx/hif_rx.c @@ -158,6 +158,7 @@ static int hif_event_indication(struct wfx_dev *wdev, { struct wfx_vif *wvif = wdev_to_wvif(wdev, hif->interface); const struct hif_ind_event *body = buf; + int type = le32_to_cpu(body->event_id); int cause; if (!wvif) { @@ -165,7 +166,7 @@ static int hif_event_indication(struct wfx_dev *wdev, return 0; } - switch (body->event_id) { + switch (type) { case HIF_EVENT_IND_RCPI_RSSI: wfx_event_report_rssi(wvif, body->event_data.rcpi_rssi); break; @@ -187,7 +188,7 @@ static int hif_event_indication(struct wfx_dev *wdev, break; default: dev_warn(wdev->dev, "unhandled event indication: %.2x\n", -body->event_id); +type); break; } return 0; -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 03/17] staging: wfx: fix cast operator
From: Jérôme Pouiller Sparse detects that le16_to_cpup() expects a __le16 * as argument. Change the cast operator to be compliant with sparse. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/bh.c | 2 +- drivers/staging/wfx/traces.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c index 2572fbcf1a33..55724e4295c4 100644 --- a/drivers/staging/wfx/bh.c +++ b/drivers/staging/wfx/bh.c @@ -70,7 +70,7 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf) if (wfx_data_read(wdev, skb->data, alloc_len)) goto err; - piggyback = le16_to_cpup((u16 *)(skb->data + alloc_len - 2)); + piggyback = le16_to_cpup((__le16 *)(skb->data + alloc_len - 2)); _trace_piggyback(piggyback, false); hif = (struct hif_msg *)skb->data; diff --git a/drivers/staging/wfx/traces.h b/drivers/staging/wfx/traces.h index bb9f7e9e7d21..c78c46b1c990 100644 --- a/drivers/staging/wfx/traces.h +++ b/drivers/staging/wfx/traces.h @@ -184,7 +184,7 @@ DECLARE_EVENT_CLASS(hif_data, if (!is_recv && (__entry->msg_id == HIF_REQ_ID_READ_MIB || __entry->msg_id == HIF_REQ_ID_WRITE_MIB)) { - __entry->mib = le16_to_cpup((u16 *) hif->body); + __entry->mib = le16_to_cpup((__le16 *)hif->body); header_len = 4; } else { __entry->mib = -1; -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 10/17] staging: wfx: fix access to le32 attribute 'indication_type'
From: Jérôme Pouiller The attribute indication_type is little-endian. We have to take to the endianness when we access it. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/hif_rx.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c index 966315edbab8..fca9df620ad9 100644 --- a/drivers/staging/wfx/hif_rx.c +++ b/drivers/staging/wfx/hif_rx.c @@ -259,8 +259,9 @@ static int hif_generic_indication(struct wfx_dev *wdev, const struct hif_msg *hif, const void *buf) { const struct hif_ind_generic *body = buf; + int type = le32_to_cpu(body->indication_type); - switch (body->indication_type) { + switch (type) { case HIF_GENERIC_INDICATION_TYPE_RAW: return 0; case HIF_GENERIC_INDICATION_TYPE_STRING: @@ -278,9 +279,8 @@ static int hif_generic_indication(struct wfx_dev *wdev, mutex_unlock(&wdev->rx_stats_lock); return 0; default: - dev_err(wdev->dev, - "generic_indication: unknown indication type: %#.8x\n", - body->indication_type); + dev_err(wdev->dev, "generic_indication: unknown indication type: %#.8x\n", + type); return -EIO; } } -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 16/17] staging: wfx: fix endianness of the field 'channel_number'
From: Jérôme Pouiller The field 'channel_number' from the structs hif_ind_rx and hif_req_start is a __le32. Sparse complains this field is not always correctly accessed: drivers/staging/wfx/data_rx.c:95:55: warning: incorrect type in argument 1 (different base types) drivers/staging/wfx/data_rx.c:95:55:expected int chan drivers/staging/wfx/data_rx.c:95:55:got restricted __le16 const [usertype] channel_number However, the value of channel_number cannot be greater than 14 (this device only support 2.4Ghz band). So, we only have to access to the least significant byte. It is finally easier to declare it as an array of bytes and only access to the first one. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/hif_api_cmd.h | 15 +-- drivers/staging/wfx/hif_tx.c | 4 ++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h index 8c48477e8797..21cde19cff75 100644 --- a/drivers/staging/wfx/hif_api_cmd.h +++ b/drivers/staging/wfx/hif_api_cmd.h @@ -321,7 +321,8 @@ struct hif_rx_flags { struct hif_ind_rx { __le32 status; - __le16 channel_number; + u8 channel_number; + u8 reserved; u8 rxed_rate; u8 rcpi_rssi; struct hif_rx_flags rx_flags; @@ -356,7 +357,8 @@ struct hif_req_join { u8 infrastructure_bss_mode:1; u8 reserved1:7; u8 band; - __le16 channel_number; + u8 channel_number; + u8 reserved; u8 bssid[ETH_ALEN]; __le16 atim_window; u8 short_preamble:1; @@ -421,13 +423,14 @@ struct hif_ind_set_pm_mode_cmpl { struct hif_req_start { u8 mode; u8 band; - __le16 channel_number; - __le32 reserved1; + u8 channel_number; + u8 reserved1; + __le32 reserved2; __le32 beacon_interval; u8 dtim_period; u8 short_preamble:1; - u8 reserved2:7; - u8 reserved3; + u8 reserved3:7; + u8 reserved4; u8 ssid_length; u8 ssid[HIF_API_SSID_SIZE]; __le32 basic_rate_set; diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c index e653ebbe5067..ecc99b765335 100644 --- a/drivers/staging/wfx/hif_tx.c +++ b/drivers/staging/wfx/hif_tx.c @@ -309,7 +309,7 @@ int hif_join(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf, body->probe_for_join = 0; else body->probe_for_join = 1; - body->channel_number = cpu_to_le16(channel->hw_value); + body->channel_number = channel->hw_value; body->beacon_interval = cpu_to_le32(conf->beacon_int); body->basic_rate_set = cpu_to_le32(wfx_rate_mask_to_hw(wvif->wdev, conf->basic_rates)); @@ -435,7 +435,7 @@ int hif_start(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf, WARN_ON(!conf->beacon_int); body->dtim_period = conf->dtim_period; body->short_preamble = conf->use_short_preamble; - body->channel_number = cpu_to_le16(channel->hw_value); + body->channel_number = channel->hw_value; body->beacon_interval = cpu_to_le32(conf->beacon_int); body->basic_rate_set = cpu_to_le32(wfx_rate_mask_to_hw(wvif->wdev, conf->basic_rates)); -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 07/17] staging: wfx: fix endianness of hif_req_read_mib fields
From: Jérôme Pouiller The structs hif_{req,cnf}_read_mib contain only little endian values. Thus, it is necessary to fix byte ordering before to use them. Especially, sparse detected wrong accesses to fields mib_id and length. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/hif_tx.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c index 58013c019192..490a9de54faf 100644 --- a/drivers/staging/wfx/hif_tx.c +++ b/drivers/staging/wfx/hif_tx.c @@ -189,17 +189,17 @@ int hif_read_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id, wfx_fill_header(hif, vif_id, HIF_REQ_ID_READ_MIB, sizeof(*body)); ret = wfx_cmd_send(wdev, hif, reply, buf_len, false); - if (!ret && mib_id != reply->mib_id) { + if (!ret && mib_id != le16_to_cpu(reply->mib_id)) { dev_warn(wdev->dev, "%s: confirmation mismatch request\n", __func__); ret = -EIO; } if (ret == -ENOMEM) - dev_err(wdev->dev, - "buffer is too small to receive %s (%zu < %d)\n", - get_mib_name(mib_id), val_len, reply->length); + dev_err(wdev->dev, "buffer is too small to receive %s (%zu < %d)\n", + get_mib_name(mib_id), val_len, + le16_to_cpu(reply->length)); if (!ret) - memcpy(val, &reply->mib_data, reply->length); + memcpy(val, &reply->mib_data, le16_to_cpu(reply->length)); else memset(val, 0xFF, val_len); kfree(hif); -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 15/17] staging: wfx: fix endianness of the field 'num_tx_confs'
From: Jérôme Pouiller The field 'num_tx_confs' from the struct hif_cnf_multi_transmit is a __le32. Sparse complains this field is not always correctly accessed: drivers/staging/wfx/hif_rx.c:82:9: warning: restricted __le32 degrades to integer drivers/staging/wfx/hif_rx.c:87:29: warning: restricted __le32 degrades to integer However, the value of num_tx_confs cannot be greater than 15. So, we only have to access to the least significant byte. It is finally easier to declare it as an array of bytes and only access to the first one. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/bh.c | 2 +- drivers/staging/wfx/hif_api_cmd.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c index 0355b1a1c4bb..574b1f553af3 100644 --- a/drivers/staging/wfx/bh.c +++ b/drivers/staging/wfx/bh.c @@ -103,7 +103,7 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf) if (!(hif->id & HIF_ID_IS_INDICATION)) { (*is_cnf)++; if (hif->id == HIF_CNF_ID_MULTI_TRANSMIT) - release_count = le32_to_cpu(((struct hif_cnf_multi_transmit *)hif->body)->num_tx_confs); + release_count = ((struct hif_cnf_multi_transmit *)hif->body)->num_tx_confs; else release_count = 1; WARN(wdev->hif.tx_buffers_used < release_count, "corrupted buffer counter"); diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h index d76722bff7ee..8c48477e8797 100644 --- a/drivers/staging/wfx/hif_api_cmd.h +++ b/drivers/staging/wfx/hif_api_cmd.h @@ -280,7 +280,8 @@ struct hif_cnf_tx { } __packed; struct hif_cnf_multi_transmit { - __le32 num_tx_confs; + u8 num_tx_confs; + u8 reserved[3]; struct hif_cnf_tx tx_conf_payload[]; } __packed; -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 11/17] staging: wfx: declare the field 'packet_id' with native byte order
From: Jérôme Pouiller The field packet_id is not interpreted by the device. It is only used as identifier for the device answer. So it is not necessary to declare it little endian. It fixes some warnings raised by Sparse without complexifying the code. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/hif_api_cmd.h | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h index 6f70801949bb..bb8c57291f74 100644 --- a/drivers/staging/wfx/hif_api_cmd.h +++ b/drivers/staging/wfx/hif_api_cmd.h @@ -254,7 +254,9 @@ struct hif_ht_tx_parameters { } __packed; struct hif_req_tx { - __le32 packet_id; + // packet_id is not interpreted by the device, so it is not necessary to + // declare it little endian + u32packet_id; u8 max_tx_rate; struct hif_queue queue_id; struct hif_data_flags data_flags; @@ -283,7 +285,9 @@ struct hif_tx_result_flags { struct hif_cnf_tx { __le32 status; - __le32 packet_id; + // packet_id is copied from struct hif_req_tx without been interpreted + // by the device, so it is not necessary to declare it little endian + u32packet_id; u8 txed_rate; u8 ack_failures; struct hif_tx_result_flags tx_result_flags; -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 13/17] staging: wfx: fix endianness of the field 'len'
From: Jérôme Pouiller The struct hif_msg is received from the hardware. So, it declared as little endian. However, it is also accessed from many places in the driver. Sparse complains about that: drivers/staging/wfx/bh.c:88:32: warning: restricted __le16 degrades to integer drivers/staging/wfx/bh.c:88:32: warning: restricted __le16 degrades to integer drivers/staging/wfx/bh.c:93:32: warning: restricted __le16 degrades to integer drivers/staging/wfx/bh.c:93:32: warning: cast to restricted __le16 drivers/staging/wfx/bh.c:93:32: warning: restricted __le16 degrades to integer drivers/staging/wfx/bh.c:121:25: warning: incorrect type in argument 2 (different base types) drivers/staging/wfx/bh.c:121:25:expected unsigned int len drivers/staging/wfx/bh.c:121:25:got restricted __le16 [usertype] len drivers/staging/wfx/hif_rx.c:27:22: warning: restricted __le16 degrades to integer drivers/staging/wfx/hif_rx.c:347:39: warning: incorrect type in argument 7 (different base types) drivers/staging/wfx/hif_rx.c:347:39:expected unsigned int [usertype] len drivers/staging/wfx/hif_rx.c:347:39:got restricted __le16 const [usertype] len drivers/staging/wfx/hif_rx.c:365:39: warning: incorrect type in argument 7 (different base types) drivers/staging/wfx/hif_rx.c:365:39:expected unsigned int [usertype] len drivers/staging/wfx/hif_rx.c:365:39:got restricted __le16 const [usertype] len drivers/staging/wfx/./traces.h:195:1: warning: incorrect type in assignment (different base types) drivers/staging/wfx/./traces.h:195:1:expected int msg_len drivers/staging/wfx/./traces.h:195:1:got restricted __le16 const [usertype] len drivers/staging/wfx/./traces.h:195:1: warning: incorrect type in assignment (different base types) drivers/staging/wfx/./traces.h:195:1:expected int msg_len drivers/staging/wfx/./traces.h:195:1:got restricted __le16 const [usertype] len drivers/staging/wfx/debug.c:319:20: warning: restricted __le16 degrades to integer drivers/staging/wfx/secure_link.c:85:27: warning: restricted __le16 degrades to integer drivers/staging/wfx/secure_link.c:85:27: warning: restricted __le16 degrades to integer In order to make Sparse happy and to keep access from the driver easy, this patch declare 'len' with native endianness. On reception of hardware data, this patch takes care to do byte-swap and keep Sparse happy. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/bh.c | 7 --- drivers/staging/wfx/data_tx.c | 2 +- drivers/staging/wfx/hif_api_general.h | 8 ++-- drivers/staging/wfx/hif_tx.c | 2 +- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c index 55724e4295c4..0355b1a1c4bb 100644 --- a/drivers/staging/wfx/bh.c +++ b/drivers/staging/wfx/bh.c @@ -74,6 +74,7 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf) _trace_piggyback(piggyback, false); hif = (struct hif_msg *)skb->data; + le16_to_cpus((__le16 *)&hif->len); WARN(hif->encrypted & 0x1, "unsupported encryption type"); if (hif->encrypted == 0x2) { if (wfx_sl_decode(wdev, (void *)hif)) { @@ -84,12 +85,11 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf) // piggyback is probably correct. return piggyback; } - le16_to_cpus(&hif->len); + le16_to_cpus((__le16 *)&hif->len); computed_len = round_up(hif->len - sizeof(hif->len), 16) + sizeof(struct hif_sl_msg) + sizeof(struct hif_sl_tag); } else { - le16_to_cpus(&hif->len); computed_len = round_up(hif->len, 2); } if (computed_len != read_len) { @@ -172,7 +172,7 @@ static void tx_helper(struct wfx_dev *wdev, struct hif_msg *hif) int ret; void *data; bool is_encrypted = false; - size_t len = le16_to_cpu(hif->len); + size_t len = hif->len; WARN(len < sizeof(*hif), "try to send corrupted data"); @@ -199,6 +199,7 @@ static void tx_helper(struct wfx_dev *wdev, struct hif_msg *hif) WARN(len > wdev->hw_caps.size_inp_ch_buf, "%s: request exceed WFx capability: %zu > %d\n", __func__, len, wdev->hw_caps.size_inp_ch_buf); + cpu_to_le16s(((struct hif_msg *)data)->len); len = wdev->hwbus_ops->align_size(wdev->hwbus_priv, len); ret = wfx_data_write(wdev, data, len); if (ret) diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c index 014fa36c8f78..84656d1a6278 100644 --- a/drivers/staging/wfx/data_tx.c +++ b/drivers/staging/wfx/data_tx.c @@ -384,7 +384,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *st
[PATCH 12/17] staging: wfx: fix endianness of the struct hif_ind_startup
From: Jérôme Pouiller The struct hif_ind_startup is received from the hardware. So it is declared as little endian. However, it is also stored in the main driver structure and used on different places in the driver. Sparse complains about that: drivers/staging/wfx/data_tx.c:388:43: warning: restricted __le16 degrades to integer drivers/staging/wfx/bh.c:199:9: warning: restricted __le16 degrades to integer drivers/staging/wfx/bh.c:221:62: warning: restricted __le16 degrades to integer In order to make Sparse happy and to keep access from the driver easy, this patch declare hif_ind_startup with native endianness. On reception of this struct, this patch takes care to do byte-swap and keep Sparse happy. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/hif_api_general.h | 11 +++ drivers/staging/wfx/hif_rx.c | 8 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/staging/wfx/hif_api_general.h b/drivers/staging/wfx/hif_api_general.h index f0135d27120c..995752b9f168 100644 --- a/drivers/staging/wfx/hif_api_general.h +++ b/drivers/staging/wfx/hif_api_general.h @@ -136,12 +136,15 @@ struct hif_otp_phy_info { } __packed; struct hif_ind_startup { + // As the others, this struct is interpreted as little endian by the + // device. However, this struct is also used by the driver. We prefer to + // declare it in native order and doing byte swap on reception. __le32 status; - __le16 hardware_id; + u16hardware_id; u8 opn[14]; u8 uid[8]; - __le16 num_inp_ch_bufs; - __le16 size_inp_ch_buf; + u16num_inp_ch_bufs; + u16size_inp_ch_buf; u8 num_links_ap; u8 num_interfaces; u8 mac_addr[2][ETH_ALEN]; @@ -155,7 +158,7 @@ struct hif_ind_startup { u8 disabled_channel_list[2]; struct hif_otp_regul_sel_mode_info regul_sel_mode_info; struct hif_otp_phy_info otp_phy_info; - __le32 supported_rate_mask; + u32supported_rate_mask; u8 firmware_label[128]; } __packed; diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c index fca9df620ad9..9b4f0c4ba745 100644 --- a/drivers/staging/wfx/hif_rx.c +++ b/drivers/staging/wfx/hif_rx.c @@ -100,10 +100,10 @@ static int hif_startup_indication(struct wfx_dev *wdev, return -EINVAL; } memcpy(&wdev->hw_caps, body, sizeof(struct hif_ind_startup)); - le32_to_cpus(&wdev->hw_caps.status); - le16_to_cpus(&wdev->hw_caps.hardware_id); - le16_to_cpus(&wdev->hw_caps.num_inp_ch_bufs); - le16_to_cpus(&wdev->hw_caps.size_inp_ch_buf); + le16_to_cpus((__le16 *)&wdev->hw_caps.hardware_id); + le16_to_cpus((__le16 *)&wdev->hw_caps.num_inp_ch_bufs); + le16_to_cpus((__le16 *)&wdev->hw_caps.size_inp_ch_buf); + le32_to_cpus((__le32 *)&wdev->hw_caps.supported_rate_mask); complete(&wdev->firmware_ready); return 0; -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 14/17] staging: wfx: fix endianness of the field 'status'
From: Jérôme Pouiller The field 'status' appears in most of structs returned by the hardware. This field is encoded as little endian. Sparse complains this field is not always correctly accessed: drivers/staging/wfx/data_rx.c:53:16: warning: restricted __le32 degrades to integer drivers/staging/wfx/data_rx.c:84:16: warning: restricted __le32 degrades to integer drivers/staging/wfx/data_tx.c:526:24: warning: restricted __le32 degrades to integer drivers/staging/wfx/data_tx.c:569:23: warning: restricted __le32 degrades to integer drivers/staging/wfx/hif_rx.c:128:33: warning: restricted __le32 degrades to integer drivers/staging/wfx/./traces.h:401:1: warning: restricted __le32 degrades to integer drivers/staging/wfx/./traces.h:401:1: warning: restricted __le32 degrades to integer In most of cases, this field is only compared with HIF_STATUS values. Finally, it is more convenient to solve the problem by defining the HIF_STATUS values directly in little endian. It is also the right time to make some clean up in the HIF_STATUS names. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/data_rx.c | 4 +-- drivers/staging/wfx/data_tx.c | 4 +-- drivers/staging/wfx/hif_api_cmd.h | 16 drivers/staging/wfx/hif_api_general.h | 36 --- drivers/staging/wfx/hif_rx.c | 2 +- drivers/staging/wfx/hif_tx.c | 4 +-- drivers/staging/wfx/main.c| 2 +- drivers/staging/wfx/traces.h | 2 +- 8 files changed, 30 insertions(+), 40 deletions(-) diff --git a/drivers/staging/wfx/data_rx.c b/drivers/staging/wfx/data_rx.c index c3b3edae3420..0e959ebc38b5 100644 --- a/drivers/staging/wfx/data_rx.c +++ b/drivers/staging/wfx/data_rx.c @@ -49,7 +49,7 @@ static int wfx_drop_encrypt_data(struct wfx_dev *wdev, } /* Firmware strips ICV in case of MIC failure. */ - if (arg->status == HIF_STATUS_MICFAILURE) + if (arg->status == HIF_STATUS_RX_FAIL_MIC) icv_len = 0; if (skb->len < hdrlen + iv_len + icv_len) { @@ -79,7 +79,7 @@ void wfx_rx_cb(struct wfx_vif *wvif, ieee80211_is_beacon(frame->frame_control))) goto drop; - if (arg->status == HIF_STATUS_MICFAILURE) + if (arg->status == HIF_STATUS_RX_FAIL_MIC) hdr->flag |= RX_FLAG_MMIC_ERROR; else if (arg->status) goto drop; diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c index 84656d1a6278..a256eed33381 100644 --- a/drivers/staging/wfx/data_tx.c +++ b/drivers/staging/wfx/data_tx.c @@ -528,7 +528,7 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg) if (rate->idx < 0) break; if (tx_count < rate->count && - arg->status == HIF_STATUS_RETRY_EXCEEDED && + arg->status == HIF_STATUS_TX_FAIL_RETRIES && arg->ack_failures) dev_dbg(wvif->wdev->dev, "all retries were not consumed: %d != %d\n", @@ -568,7 +568,7 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg) tx_info->flags |= IEEE80211_TX_STAT_NOACK_TRANSMITTED; else tx_info->flags |= IEEE80211_TX_STAT_ACK; - } else if (arg->status == HIF_REQUEUE) { + } else if (arg->status == HIF_STATUS_TX_FAIL_REQUEUE) { WARN(!arg->tx_result_flags.requeue, "incoherent status and result_flags"); if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) { diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h index bb8c57291f74..d76722bff7ee 100644 --- a/drivers/staging/wfx/hif_api_cmd.h +++ b/drivers/staging/wfx/hif_api_cmd.h @@ -66,22 +66,6 @@ union hif_commands_ids { enum hif_indications_ids indication; }; -enum hif_status { - HIF_STATUS_SUCCESS = 0x0, - HIF_STATUS_FAILURE = 0x1, - HIF_INVALID_PARAMETER = 0x2, - HIF_STATUS_WARNING = 0x3, - HIF_ERROR_UNSUPPORTED_MSG_ID= 0x4, - HIF_STATUS_DECRYPTFAILURE = 0x10, - HIF_STATUS_MICFAILURE = 0x11, - HIF_STATUS_NO_KEY_FOUND = 0x12, - HIF_STATUS_RETRY_EXCEEDED = 0x13, - HIF_STATUS_TX_LIFETIME_EXCEEDED = 0x14, - HIF_REQUEUE = 0x15, - HIF_STATUS_REFUSED = 0x16, - HIF_STATUS_BUSY = 0x17 -}; - struct hif_reset_flags { u8 reset_stat:1; u8 reset_all_int:1; diff --git a/drivers/staging/wfx/hif_api_general.h b/drivers/staging/wfx/hif_api_general.h index a359ae76511a..2b0cdfdd46d3 100644 --- a/drivers/staging/wfx/hif_api_general.h +++ b/drivers/staging/wfx/hif_api_general.h @@ -70,21 +70,27 @@ enum hif_general_indications_ids
[PATCH 17/17] staging: wfx: update TODO
From: Jérôme Pouiller Update the TODO list associated to the wfx driver with the last progresses. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/TODO | 19 --- 1 file changed, 19 deletions(-) diff --git a/drivers/staging/wfx/TODO b/drivers/staging/wfx/TODO index fca3332e42ce..42bf36d43970 100644 --- a/drivers/staging/wfx/TODO +++ b/drivers/staging/wfx/TODO @@ -3,32 +3,13 @@ staging directory. - The HIF API is not yet clean enough. - - Fix support for big endian architectures. See: - https://lore.kernel.org/lkml/2019202852.gx26...@zeniv.linux.org.uk - - - The pointers returned by allocation functions are always checked. - - The code that check the corectness of received message (in rx_helper()) can be improved. See: https://lore.kernel.org/driverdev-devel/2302785.6C7ODC2LYm@pc-42/ - - Support for SDIO with external IRQ is broken. - - As suggested by Felix, rate control could be improved following this idea: https://lore.kernel.org/lkml/3099559.gv3Q75KnN1@pc-42/ - - When driver is about to loose BSS, it forge its own Null Func request (see -wfx_cqm_bssloss_sm()). It should use mechanism provided by mac80211. - - - Monitoring mode is not implemented despite being mandatory by mac80211. - - - The "state" field from wfx_vif should be replaced by "vif->type". - - - It seems that wfx_upload_keys() is useless. - - - "event_queue" from wfx_vif seems overkill. These event are rare and they - probably could be handled in a simpler fashion. - - Feature called "secure link" should be either developed (using kernel crypto API) or dropped. -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 08/17] staging: wfx: fix access to le32 attribute 'ps_mode_error'
From: Jérôme Pouiller The attribute ps_mode_error is little-endian. We have to take to the endianness when we access it. Signed-off-by: Jérôme Pouiller --- drivers/staging/wfx/hif_rx.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c index 83c3fdbb10fa..87d5107a7757 100644 --- a/drivers/staging/wfx/hif_rx.c +++ b/drivers/staging/wfx/hif_rx.c @@ -158,6 +158,7 @@ static int hif_event_indication(struct wfx_dev *wdev, { struct wfx_vif *wvif = wdev_to_wvif(wdev, hif->interface); const struct hif_ind_event *body = buf; + int cause; if (!wvif) { dev_warn(wdev->dev, "received event for non-existent vif\n"); @@ -176,10 +177,10 @@ static int hif_event_indication(struct wfx_dev *wdev, dev_dbg(wdev->dev, "ignore BSSREGAINED indication\n"); break; case HIF_EVENT_IND_PS_MODE_ERROR: + cause = le32_to_cpu(body->event_data.ps_mode_error); dev_warn(wdev->dev, "error while processing power save request: %d\n", -body->event_data.ps_mode_error); - if (body->event_data.ps_mode_error == - HIF_PS_ERROR_AP_NOT_RESP_TO_POLL) { +cause); + if (cause == HIF_PS_ERROR_AP_NOT_RESP_TO_POLL) { wvif->bss_not_support_ps_poll = true; schedule_work(&wvif->update_pm_work); } -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH -next] media: tegra: Make tegra210_video_formats static
On 5/11/20 4:20 AM, Samuel Zou wrote: Fix the following sparse warning: drivers/staging/media/tegra-video/tegra210.c:589:33: warning: symbol 'tegra210_video_formats' was not declared. The tegra210_video_formats has only call site within tegra210.c It should be static Fixes: 423d10a99b30 ("media: tegra: Add Tegra210 Video input driver") Reported-by: Hulk Robot Signed-off-by: Samuel Zou --- drivers/staging/media/tegra-video/tegra210.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: Sowjanya Komatineni ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 13/17] staging: wfx: fix endianness of the field 'len'
Hi Jerome, I love your patch! Perhaps something to improve: [auto build test WARNING on staging/staging-testing] [also build test WARNING on next-20200511] [cannot apply to v5.7-rc5] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Jerome-Pouiller/staging-wfx-fix-support-for-big-endian-hosts/20200512-031750 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git ae73e7784871ebe2c43da619b4a1e2c9ff81508d config: m68k-allmodconfig (attached as .config) compiler: m68k-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=m68k If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot All warnings (new ones prefixed by >>): In file included from include/linux/byteorder/big_endian.h:5, from arch/m68k/include/uapi/asm/byteorder.h:5, from include/asm-generic/bitops/le.h:6, from arch/m68k/include/asm/bitops.h:528, from include/linux/bitops.h:29, from include/linux/kernel.h:12, from include/asm-generic/bug.h:19, from arch/m68k/include/asm/bug.h:32, from include/linux/bug.h:5, from include/linux/gpio/consumer.h:6, from drivers/staging/wfx/bh.c:8: drivers/staging/wfx/bh.c: In function 'tx_helper': >> drivers/staging/wfx/bh.c:202:39: warning: passing argument 1 of '__swab16s' >> makes pointer from integer without a cast [-Wint-conversion] 202 | cpu_to_le16s(((struct hif_msg *)data)->len); include/uapi/linux/byteorder/big_endian.h:96:38: note: in definition of macro '__cpu_to_le16s' 96 | #define __cpu_to_le16s(x) __swab16s((x)) | ^ >> drivers/staging/wfx/bh.c:202:2: note: in expansion of macro 'cpu_to_le16s' 202 | cpu_to_le16s(((struct hif_msg *)data)->len); | ^~~~ In file included from include/linux/swab.h:5, from include/uapi/linux/byteorder/big_endian.h:13, from include/linux/byteorder/big_endian.h:5, from arch/m68k/include/uapi/asm/byteorder.h:5, from include/asm-generic/bitops/le.h:6, from arch/m68k/include/asm/bitops.h:528, from include/linux/bitops.h:29, from include/linux/kernel.h:12, from include/asm-generic/bug.h:19, from arch/m68k/include/asm/bug.h:32, from include/linux/bug.h:5, from include/linux/gpio/consumer.h:6, from drivers/staging/wfx/bh.c:8: include/uapi/linux/swab.h:240:37: note: expected '__u16 *' {aka 'short unsigned int *'} but argument is of type 'u16' {aka 'short unsigned int'} 240 | static inline void __swab16s(__u16 *p) | ~~~^ vim +/__swab16s +202 drivers/staging/wfx/bh.c 169 170 static void tx_helper(struct wfx_dev *wdev, struct hif_msg *hif) 171 { 172 int ret; 173 void *data; 174 bool is_encrypted = false; 175 size_t len = hif->len; 176 177 WARN(len < sizeof(*hif), "try to send corrupted data"); 178 179 hif->seqnum = wdev->hif.tx_seqnum; 180 wdev->hif.tx_seqnum = (wdev->hif.tx_seqnum + 1) % (HIF_COUNTER_MAX + 1); 181 182 if (wfx_is_secure_command(wdev, hif->id)) { 183 len = round_up(len - sizeof(hif->len), 16) + sizeof(hif->len) + 184 sizeof(struct hif_sl_msg_hdr) + 185 sizeof(struct hif_sl_tag); 186 // AES support encryption in-place. However, mac80211 access to 187 // 802.11 header after frame was sent (to get MAC addresses). 188 // So, keep origin buffer clear. 189 data = kmalloc(len, GFP_KERNEL); 190 if (!data) 191 goto end; 192 is_encrypted = true; 193 ret = wfx_sl_encode(wdev, hif, data); 194 if (ret) 195 goto end; 196 } else { 197 data = hif; 198 } 19