Re: [PATCH] staging: kpc2000: replace assertion with recovery code
On Sun, Dec 15, 2019 at 07:28:14PM +0100, Greg Kroah-Hartman wrote: > On Sun, Dec 15, 2019 at 12:12:37PM -0600, Aditya Pakki wrote: > > In kpc_dma_transfer, if either priv or ldev is NULL, crashing the > > process is excessive and is not needed. Instead of asserting, by > > passing the error upstream, the error can be handled. > > > > Signed-off-by: Aditya Pakki > > --- > > drivers/staging/kpc2000/kpc_dma/fileops.c | 6 -- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c > > b/drivers/staging/kpc2000/kpc_dma/fileops.c > > index cb52bd9a6d2f..1c4633267cc1 100644 > > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c > > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c > > @@ -49,9 +49,11 @@ static int kpc_dma_transfer(struct dev_private_data > > *priv, > > u64 dma_addr; > > u64 user_ctl; > > > > - BUG_ON(priv == NULL); > > + if (!priv) > > + return -EINVAL; > > How can prive ever be NULL here? Can you track back to all callers to > verify this? If so, just remove the check. > Smatch says that "priv" can't be NULL. Also if you have a NULL dereference the kernel prints a nice stack trace. Normally just doing the NULL dereference and Oopsing is better than doing a BUG_ON(). The one exception would be when the Oops leads to filesystem corruption. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 13/55] staging: wfx: avoid double warning when no more tx policy are available
On Mon, Dec 16, 2019 at 05:03:40PM +, Jérôme Pouiller wrote:
> From: Jérôme Pouiller
>
> Currently, number of available tx retry policies is checked two times.
> Only one is sufficient.
>
> Signed-off-by: Jérôme Pouiller
> ---
> drivers/staging/wfx/data_tx.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
> index 32e269becd75..c9dea627661f 100644
> --- a/drivers/staging/wfx/data_tx.c
> +++ b/drivers/staging/wfx/data_tx.c
> @@ -169,7 +169,8 @@ static int wfx_tx_policy_get(struct wfx_vif *wvif,
> wfx_tx_policy_build(wvif, &wanted, rates);
>
> spin_lock_bh(&cache->lock);
> - if (WARN_ON(list_empty(&cache->free))) {
> + if (list_empty(&cache->free)) {
> + WARN(1, "unable to get a valid Tx policy");
> spin_unlock_bh(&cache->lock);
> return WFX_INVALID_RATE_ID;
This warning is more clear than the original which is good, but that's
not what the commit message says. How does this fix a double warning?
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 25/55] staging: wfx: fix name of struct hif_req_start_scan_alt
On Mon, Dec 16, 2019 at 05:03:46PM +, Jérôme Pouiller wrote:
> From: Jérôme Pouiller
>
> The original name did not make any sense.
>
> Signed-off-by: Jérôme Pouiller
> ---
> drivers/staging/wfx/hif_api_cmd.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/wfx/hif_api_cmd.h
> b/drivers/staging/wfx/hif_api_cmd.h
> index 3e77fbe3d5ff..4ce3bb51cf04 100644
> --- a/drivers/staging/wfx/hif_api_cmd.h
> +++ b/drivers/staging/wfx/hif_api_cmd.h
> @@ -188,7 +188,7 @@ struct hif_req_start_scan {
> u8ssid_and_channel_lists[];
> } __packed;
>
> -struct hif_start_scan_req_cstnbssid_body {
> +struct hif_req_start_scan_alt {
> u8band;
> struct hif_scan_type scan_type;
> struct hif_scan_flags scan_flags;
Why not just delete this if it isn't used?
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] Staging: most: core: Fix SPDX License Identifier style issue
On Fri, Dec 27, 2019 at 03:41:55AM -0500, Matthew Hanzelik wrote: > Fixed a style issue with the SPDX License Identifier style. > > Signed-off-by: Matthew Hanzelik > --- > Changes in v2: > - Fix trailing space in patch diff > > drivers/staging/most/core.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/most/core.h b/drivers/staging/most/core.h > index 49859aef98df..1380e7586376 100644 > --- a/drivers/staging/most/core.h > +++ b/drivers/staging/most/core.h > @@ -1,4 +1,4 @@ > -// SPDX-License-Identifier: GPL-2.0 > +/* SPDX-License-Identifier: GPL-2.0 */ > /* > * most.h - API for component and adapter drivers > * > -- > 2.24.1 > This does not apply to linux-next, please rebase it if it is still needed and resend. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] staging: vt6656: set usb_set_intfdata on driver fail.
On Fri, Dec 20, 2019 at 09:15:59PM +, Malcolm Priestley wrote:
> intfdata will contain stale pointer when the device is detached after
> failed initialization when referenced in vt6656_disconnect
>
> Provide driver access to it here and NULL it.
>
> Cc: stable
> Signed-off-by: Malcolm Priestley
> ---
> drivers/staging/vt6656/device.h | 1 +
> drivers/staging/vt6656/main_usb.c | 1 +
> drivers/staging/vt6656/wcmd.c | 1 +
> 3 files changed, 3 insertions(+)
>
> diff --git a/drivers/staging/vt6656/device.h b/drivers/staging/vt6656/device.h
> index 6074ceda78bf..50e1c8918040 100644
> --- a/drivers/staging/vt6656/device.h
> +++ b/drivers/staging/vt6656/device.h
> @@ -259,6 +259,7 @@ struct vnt_private {
> u8 mac_hw;
> /* netdev */
> struct usb_device *usb;
> + struct usb_interface *intf;
>
> u64 tsf_time;
> u8 rx_rate;
> diff --git a/drivers/staging/vt6656/main_usb.c
> b/drivers/staging/vt6656/main_usb.c
> index 4a5d741f94f5..9cb924c54571 100644
> --- a/drivers/staging/vt6656/main_usb.c
> +++ b/drivers/staging/vt6656/main_usb.c
> @@ -992,6 +992,7 @@ vt6656_probe(struct usb_interface *intf, const struct
> usb_device_id *id)
> priv = hw->priv;
> priv->hw = hw;
> priv->usb = udev;
> + priv->intf = intf;
>
> vnt_set_options(priv);
>
> diff --git a/drivers/staging/vt6656/wcmd.c b/drivers/staging/vt6656/wcmd.c
> index 3eb2f11a5de1..2c5250ca2801 100644
> --- a/drivers/staging/vt6656/wcmd.c
> +++ b/drivers/staging/vt6656/wcmd.c
> @@ -99,6 +99,7 @@ void vnt_run_command(struct work_struct *work)
> if (vnt_init(priv)) {
> /* If fail all ends TODO retry */
> dev_err(&priv->usb->dev, "failed to start\n");
> + usb_set_intfdata(priv->intf, NULL);
> ieee80211_free_hw(priv->hw);
> return;
> }
You set this variable, but never reference it, so how does this change
any behavior in the driver?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] staging: vt6656: set usb_set_intfdata on driver fail.
On Fri, Jan 03, 2020 at 11:01:59AM +0100, Greg Kroah-Hartman wrote:
> On Fri, Dec 20, 2019 at 09:15:59PM +, Malcolm Priestley wrote:
> > intfdata will contain stale pointer when the device is detached after
> > failed initialization when referenced in vt6656_disconnect
> >
> > Provide driver access to it here and NULL it.
> >
> > Cc: stable
> > Signed-off-by: Malcolm Priestley
> > ---
> > drivers/staging/vt6656/device.h | 1 +
> > drivers/staging/vt6656/main_usb.c | 1 +
> > drivers/staging/vt6656/wcmd.c | 1 +
> > 3 files changed, 3 insertions(+)
> >
> > diff --git a/drivers/staging/vt6656/device.h
> > b/drivers/staging/vt6656/device.h
> > index 6074ceda78bf..50e1c8918040 100644
> > --- a/drivers/staging/vt6656/device.h
> > +++ b/drivers/staging/vt6656/device.h
> > @@ -259,6 +259,7 @@ struct vnt_private {
> > u8 mac_hw;
> > /* netdev */
> > struct usb_device *usb;
> > + struct usb_interface *intf;
> >
> > u64 tsf_time;
> > u8 rx_rate;
> > diff --git a/drivers/staging/vt6656/main_usb.c
> > b/drivers/staging/vt6656/main_usb.c
> > index 4a5d741f94f5..9cb924c54571 100644
> > --- a/drivers/staging/vt6656/main_usb.c
> > +++ b/drivers/staging/vt6656/main_usb.c
> > @@ -992,6 +992,7 @@ vt6656_probe(struct usb_interface *intf, const struct
> > usb_device_id *id)
> > priv = hw->priv;
> > priv->hw = hw;
> > priv->usb = udev;
> > + priv->intf = intf;
> >
> > vnt_set_options(priv);
> >
> > diff --git a/drivers/staging/vt6656/wcmd.c b/drivers/staging/vt6656/wcmd.c
> > index 3eb2f11a5de1..2c5250ca2801 100644
> > --- a/drivers/staging/vt6656/wcmd.c
> > +++ b/drivers/staging/vt6656/wcmd.c
> > @@ -99,6 +99,7 @@ void vnt_run_command(struct work_struct *work)
> > if (vnt_init(priv)) {
> > /* If fail all ends TODO retry */
> > dev_err(&priv->usb->dev, "failed to start\n");
> > + usb_set_intfdata(priv->intf, NULL);
> > ieee80211_free_hw(priv->hw);
> > return;
> > }
>
> You set this variable, but never reference it, so how does this change
> any behavior in the driver?
Nevermind, I've looked at the driver, it makes more sense now, sorry for
the noise.
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: exfat: add STAGING prefix to config names
On Fri, Jan 3, 2020 at 3:17 AM Namjae Jeon wrote:
>
> Add STAGING prefix to config names to avoid collsion with fs/exfat config.
>
> Signed-off-by: Namjae Jeon
> ---
> drivers/staging/Makefile | 2 +-
> drivers/staging/exfat/Kconfig| 26 +++
> drivers/staging/exfat/Makefile | 2 +-
> drivers/staging/exfat/exfat.h| 14
> drivers/staging/exfat/exfat_blkdev.c | 12 +++
> drivers/staging/exfat/exfat_core.c | 8 ++---
> drivers/staging/exfat/exfat_super.c | 50 ++--
> 7 files changed, 57 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
> index 463aef6a18ef..fdd03fd6e704 100644
> --- a/drivers/staging/Makefile
> +++ b/drivers/staging/Makefile
> @@ -48,7 +48,7 @@ obj-$(CONFIG_FIELDBUS_DEV) += fieldbus/
> obj-$(CONFIG_KPC2000) += kpc2000/
> obj-$(CONFIG_UWB) += uwb/
> obj-$(CONFIG_USB_WUSB) += wusbcore/
> -obj-$(CONFIG_EXFAT_FS) += exfat/
> +obj-$(CONFIG_STAGING_EXFAT_FS) += exfat/
> obj-$(CONFIG_QLGE) += qlge/
> obj-$(CONFIG_NET_VENDOR_HP)+= hp/
> obj-$(CONFIG_WFX) += wfx/
> diff --git a/drivers/staging/exfat/Kconfig b/drivers/staging/exfat/Kconfig
> index 0130019cbec2..292a19dfcaf5 100644
> --- a/drivers/staging/exfat/Kconfig
> +++ b/drivers/staging/exfat/Kconfig
> @@ -1,41 +1,41 @@
> # SPDX-License-Identifier: GPL-2.0
> -config EXFAT_FS
> +config STAGING_EXFAT_FS
> tristate "exFAT fs support"
> depends on BLOCK
> select NLS
> help
> This adds support for the exFAT file system.
>
> -config EXFAT_DISCARD
> +config STAGING_EXFAT_DISCARD
> bool "enable discard support"
> - depends on EXFAT_FS
> + depends on STAGING_EXFAT_FS
> default y
>
> -config EXFAT_DELAYED_SYNC
> +config STAGING_EXFAT_DELAYED_SYNC
> bool "enable delayed sync"
> - depends on EXFAT_FS
> + depends on STAGING_EXFAT_FS
> default n
>
> -config EXFAT_KERNEL_DEBUG
> +config STAGING_EXFAT_KERNEL_DEBUG
> bool "enable kernel debug features via ioctl"
> - depends on EXFAT_FS
> + depends on STAGING_EXFAT_FS
> default n
>
> -config EXFAT_DEBUG_MSG
> +config STAGING_EXFAT_DEBUG_MSG
> bool "print debug messages"
> - depends on EXFAT_FS
> + depends on STAGING_EXFAT_FS
> default n
>
> -config EXFAT_DEFAULT_CODEPAGE
> +config STAGING_EXFAT_DEFAULT_CODEPAGE
> int "Default codepage for exFAT"
> default 437
> - depends on EXFAT_FS
> + depends on STAGING_EXFAT_FS
> help
> This option should be set to the codepage of your exFAT filesystems.
>
> -config EXFAT_DEFAULT_IOCHARSET
> +config STAGING_EXFAT_DEFAULT_IOCHARSET
> string "Default iocharset for exFAT"
> default "utf8"
> - depends on EXFAT_FS
> + depends on STAGING_EXFAT_FS
> help
> Set this to the default input/output character set you'd like exFAT
> to use.
> diff --git a/drivers/staging/exfat/Makefile b/drivers/staging/exfat/Makefile
> index 6c90aec83feb..057556eeca0c 100644
> --- a/drivers/staging/exfat/Makefile
> +++ b/drivers/staging/exfat/Makefile
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0-or-later
>
> -obj-$(CONFIG_EXFAT_FS) += exfat.o
> +obj-$(CONFIG_STAGING_EXFAT_FS) += exfat.o
>
> exfat-y := exfat_core.o\
> exfat_super.o \
> diff --git a/drivers/staging/exfat/exfat.h b/drivers/staging/exfat/exfat.h
> index 51c665a924b7..3865c17027ce 100644
> --- a/drivers/staging/exfat/exfat.h
> +++ b/drivers/staging/exfat/exfat.h
> @@ -9,7 +9,7 @@
> #include
> #include
>
> -#ifdef CONFIG_EXFAT_KERNEL_DEBUG
> +#ifdef CONFIG_STAGING_EXFAT_KERNEL_DEBUG
>/* For Debugging Purpose */
> /* IOCTL code 'f' used by
> * - file systems typically #0~0x1F
> @@ -22,9 +22,9 @@
>
> #define EXFAT_DEBUGFLAGS_INVALID_UMOUNT0x01
> #define EXFAT_DEBUGFLAGS_ERROR_RW 0x02
> -#endif /* CONFIG_EXFAT_KERNEL_DEBUG */
> +#endif /* CONFIG_STAGING_EXFAT_KERNEL_DEBUG */
>
> -#ifdef CONFIG_EXFAT_DEBUG_MSG
> +#ifdef CONFIG_STAGING_EXFAT_DEBUG_MSG
> #define DEBUG 1
> #else
> #undef DEBUG
> @@ -661,10 +661,10 @@ struct exfat_mount_options {
>
> /* on error: continue, panic, remount-ro */
> unsigned char errors;
> -#ifdef CONFIG_EXFAT_DISCARD
> +#ifdef CONFIG_STAGING_EXFAT_DISCARD
> /* flag on if -o dicard specified and device support discard() */
> unsigned char discard;
> -#endif /* CONFIG_EXFAT_DISCARD */
> +#endif /* CONFIG_STAGING_EXFAT_DISCARD */
> };
>
> #define EXFAT_HASH_BITS8
> @@ -700,9 +700,9 @@ struct exfat_sb_info {
>
> spinlock_t inode_hash_lock;
> struct hlist_head inode_hashtable[EXFAT_HASH_SIZE];
> -#ifdef CONFIG_EXFAT_KERNEL_DEBUG
> +#ifdef CONFIG_STAGING_EXFAT_KERNEL_DEBUG
> long debug_flags;
> -#endif /* CONFIG_EXFAT_K
Re: [PATCH] staging: exfat: add STAGING prefix to config names
On Fri, Jan 03, 2020 at 12:08:10PM +0200, Amir Goldstein wrote:
> On Fri, Jan 3, 2020 at 3:17 AM Namjae Jeon wrote:
> >
> > Add STAGING prefix to config names to avoid collsion with fs/exfat config.
> >
> > Signed-off-by: Namjae Jeon
> > ---
> > drivers/staging/Makefile | 2 +-
> > drivers/staging/exfat/Kconfig| 26 +++
> > drivers/staging/exfat/Makefile | 2 +-
> > drivers/staging/exfat/exfat.h| 14
> > drivers/staging/exfat/exfat_blkdev.c | 12 +++
> > drivers/staging/exfat/exfat_core.c | 8 ++---
> > drivers/staging/exfat/exfat_super.c | 50 ++--
> > 7 files changed, 57 insertions(+), 57 deletions(-)
> >
> > diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
> > index 463aef6a18ef..fdd03fd6e704 100644
> > --- a/drivers/staging/Makefile
> > +++ b/drivers/staging/Makefile
> > @@ -48,7 +48,7 @@ obj-$(CONFIG_FIELDBUS_DEV) += fieldbus/
> > obj-$(CONFIG_KPC2000) += kpc2000/
> > obj-$(CONFIG_UWB) += uwb/
> > obj-$(CONFIG_USB_WUSB) += wusbcore/
> > -obj-$(CONFIG_EXFAT_FS) += exfat/
> > +obj-$(CONFIG_STAGING_EXFAT_FS) += exfat/
> > obj-$(CONFIG_QLGE) += qlge/
> > obj-$(CONFIG_NET_VENDOR_HP)+= hp/
> > obj-$(CONFIG_WFX) += wfx/
> > diff --git a/drivers/staging/exfat/Kconfig b/drivers/staging/exfat/Kconfig
> > index 0130019cbec2..292a19dfcaf5 100644
> > --- a/drivers/staging/exfat/Kconfig
> > +++ b/drivers/staging/exfat/Kconfig
> > @@ -1,41 +1,41 @@
> > # SPDX-License-Identifier: GPL-2.0
> > -config EXFAT_FS
> > +config STAGING_EXFAT_FS
> > tristate "exFAT fs support"
> > depends on BLOCK
> > select NLS
> > help
> > This adds support for the exFAT file system.
> >
> > -config EXFAT_DISCARD
> > +config STAGING_EXFAT_DISCARD
> > bool "enable discard support"
> > - depends on EXFAT_FS
> > + depends on STAGING_EXFAT_FS
> > default y
> >
> > -config EXFAT_DELAYED_SYNC
> > +config STAGING_EXFAT_DELAYED_SYNC
> > bool "enable delayed sync"
> > - depends on EXFAT_FS
> > + depends on STAGING_EXFAT_FS
> > default n
> >
> > -config EXFAT_KERNEL_DEBUG
> > +config STAGING_EXFAT_KERNEL_DEBUG
> > bool "enable kernel debug features via ioctl"
> > - depends on EXFAT_FS
> > + depends on STAGING_EXFAT_FS
> > default n
> >
> > -config EXFAT_DEBUG_MSG
> > +config STAGING_EXFAT_DEBUG_MSG
> > bool "print debug messages"
> > - depends on EXFAT_FS
> > + depends on STAGING_EXFAT_FS
> > default n
> >
> > -config EXFAT_DEFAULT_CODEPAGE
> > +config STAGING_EXFAT_DEFAULT_CODEPAGE
> > int "Default codepage for exFAT"
> > default 437
> > - depends on EXFAT_FS
> > + depends on STAGING_EXFAT_FS
> > help
> > This option should be set to the codepage of your exFAT
> > filesystems.
> >
> > -config EXFAT_DEFAULT_IOCHARSET
> > +config STAGING_EXFAT_DEFAULT_IOCHARSET
> > string "Default iocharset for exFAT"
> > default "utf8"
> > - depends on EXFAT_FS
> > + depends on STAGING_EXFAT_FS
> > help
> > Set this to the default input/output character set you'd like
> > exFAT to use.
> > diff --git a/drivers/staging/exfat/Makefile b/drivers/staging/exfat/Makefile
> > index 6c90aec83feb..057556eeca0c 100644
> > --- a/drivers/staging/exfat/Makefile
> > +++ b/drivers/staging/exfat/Makefile
> > @@ -1,6 +1,6 @@
> > # SPDX-License-Identifier: GPL-2.0-or-later
> >
> > -obj-$(CONFIG_EXFAT_FS) += exfat.o
> > +obj-$(CONFIG_STAGING_EXFAT_FS) += exfat.o
> >
> > exfat-y := exfat_core.o\
> > exfat_super.o \
> > diff --git a/drivers/staging/exfat/exfat.h b/drivers/staging/exfat/exfat.h
> > index 51c665a924b7..3865c17027ce 100644
> > --- a/drivers/staging/exfat/exfat.h
> > +++ b/drivers/staging/exfat/exfat.h
> > @@ -9,7 +9,7 @@
> > #include
> > #include
> >
> > -#ifdef CONFIG_EXFAT_KERNEL_DEBUG
> > +#ifdef CONFIG_STAGING_EXFAT_KERNEL_DEBUG
> >/* For Debugging Purpose */
> > /* IOCTL code 'f' used by
> > * - file systems typically #0~0x1F
> > @@ -22,9 +22,9 @@
> >
> > #define EXFAT_DEBUGFLAGS_INVALID_UMOUNT0x01
> > #define EXFAT_DEBUGFLAGS_ERROR_RW 0x02
> > -#endif /* CONFIG_EXFAT_KERNEL_DEBUG */
> > +#endif /* CONFIG_STAGING_EXFAT_KERNEL_DEBUG */
> >
> > -#ifdef CONFIG_EXFAT_DEBUG_MSG
> > +#ifdef CONFIG_STAGING_EXFAT_DEBUG_MSG
> > #define DEBUG 1
> > #else
> > #undef DEBUG
> > @@ -661,10 +661,10 @@ struct exfat_mount_options {
> >
> > /* on error: continue, panic, remount-ro */
> > unsigned char errors;
> > -#ifdef CONFIG_EXFAT_DISCARD
> > +#ifdef CONFIG_STAGING_EXFAT_DISCARD
> > /* flag on if -o dicard specified and device support discard() */
> > unsigned char discard;
> > -#endif /* CONFIG_EXFAT_DISCARD */
> > +#endif /* CONFIG_STAGING_EXFAT_DISCAR
Re: [PATCH 13/55] staging: wfx: avoid double warning when no more tx policy are available
On Friday 3 January 2020 10:21:16 CET Dan Carpenter wrote:
> On Mon, Dec 16, 2019 at 05:03:40PM +, Jérôme Pouiller wrote:
> > From: Jérôme Pouiller
> >
> > Currently, number of available tx retry policies is checked two times.
> > Only one is sufficient.
> >
> > Signed-off-by: Jérôme Pouiller
> > ---
> > drivers/staging/wfx/data_tx.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
> > index 32e269becd75..c9dea627661f 100644
> > --- a/drivers/staging/wfx/data_tx.c
> > +++ b/drivers/staging/wfx/data_tx.c
> > @@ -169,7 +169,8 @@ static int wfx_tx_policy_get(struct wfx_vif *wvif,
> > wfx_tx_policy_build(wvif, &wanted, rates);
> >
> > spin_lock_bh(&cache->lock);
> > - if (WARN_ON(list_empty(&cache->free))) {
> > + if (list_empty(&cache->free)) {
> > + WARN(1, "unable to get a valid Tx policy");
> > spin_unlock_bh(&cache->lock);
> > return WFX_INVALID_RATE_ID;
>
> This warning is more clear than the original which is good, but that's
> not what the commit message says. How does this fix a double warning?
Err... Indeed, it don't. From wfx_tx_get_rate_id():
rate_id = wfx_tx_policy_get(wvif, ...);
if (rate_id == WFX_INVALID_RATE_ID)
dev_warn(wvif->wdev->dev, "unable to get a valid Tx policy");
I will fix that in my next PR.
--
Jérôme Pouiller
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/5] staging: vt6656: Fix non zero logical return of, usb_control_msg
On Fri, Dec 20, 2019 at 09:14:59PM +, Malcolm Priestley wrote:
> Starting with commit 59608cb1de1856
> ("staging: vt6656: clean function's error path in usbpipe.c")
> the usb control functions have returned errors throughout driver
> with only logical variable checking.
Use the Fixes tag for this.
Fixes: 59608cb1de18 ("staging: vt6656: clean function's error path in
usbpipe.c")
12 digits to the hash. Add Quentin to the CC list.
>
> However, usb_control_msg return the amount of bytes transferred
> this means that normal operation causes errors.
>
> Correct the return function so only return zero when transfer
> is successful.
>
> Cc: stable # v5.3+
> Signed-off-by: Malcolm Priestley
> ---
> drivers/staging/vt6656/usbpipe.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/vt6656/usbpipe.c
> b/drivers/staging/vt6656/usbpipe.c
> index d3304df6bd53..488ebd98773d 100644
> --- a/drivers/staging/vt6656/usbpipe.c
> +++ b/drivers/staging/vt6656/usbpipe.c
> @@ -59,7 +59,9 @@ int vnt_control_out(struct vnt_private *priv, u8 request,
> u16 value,
>
> kfree(usb_buffer);
>
> - if (ret >= 0 && ret < (int)length)
> + if (ret == (int)length)
No need for this cast (no need in the original either).
> + ret = 0;
> + else
> ret = -EIO;
It would be better to preserve the error codes from usb_control_msg().
if (ret == length)
ret = 0;
else if (ret >= 0)
ret = -EIO;
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 25/55] staging: wfx: fix name of struct hif_req_start_scan_alt
On Friday 3 January 2020 10:27:44 CET Dan Carpenter wrote:
> On Mon, Dec 16, 2019 at 05:03:46PM +, Jérôme Pouiller wrote:
> > From: Jérôme Pouiller
> >
> > The original name did not make any sense.
> >
> > Signed-off-by: Jérôme Pouiller
> > ---
> > drivers/staging/wfx/hif_api_cmd.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/wfx/hif_api_cmd.h
> > b/drivers/staging/wfx/hif_api_cmd.h
> > index 3e77fbe3d5ff..4ce3bb51cf04 100644
> > --- a/drivers/staging/wfx/hif_api_cmd.h
> > +++ b/drivers/staging/wfx/hif_api_cmd.h
> > @@ -188,7 +188,7 @@ struct hif_req_start_scan {
> > u8ssid_and_channel_lists[];
> > } __packed;
> >
> > -struct hif_start_scan_req_cstnbssid_body {
> > +struct hif_req_start_scan_alt {
> > u8band;
> > struct hif_scan_type scan_type;
> > struct hif_scan_flags scan_flags;
>
> Why not just delete this if it isn't used?
Patch 47/55 start to use it.
However, since patch 47, struct hif_req_start_scan is no more used.
There is an item in TODO file about this:
hif_api_*.h have been imported from firmware code. Some of the structures
are never used in driver.
--
Jérôme Pouiller
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/5] staging: vt6656: correct return of vnt_init_registers.
On Fri, Dec 20, 2019 at 09:15:09PM +, Malcolm Priestley wrote:
> The driver standard error returns remove bool false conditions.
>
> Cc: stable # v5.3+
> Signed-off-by: Malcolm Priestley
Fixes: 07ba60a15843 ("staging: vt6656: clean-up registers initialization error
path")
The other part of that bug was fixed silently in commit 987d864a2363
("staging: vt6656: manage error path during device initialization").
I'm quite embarrassed that I didn't catch these during review... It's
the obvious bug right? "You have reversed the return values but not
updated any of the callers." *Egg on my face*.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/5] staging: vt6656: limit reg output to block size
On Fri, Dec 20, 2019 at 09:15:24PM +, Malcolm Priestley wrote:
> vnt_control_out appears to fail when BBREG is greater than 64 writes.
>
> Create new function that will relay an array in no larger than
> the indicated block size.
>
> It appears that this command has always failed but was ignored by
> driver until the introduction of error checking.
>
> Cc: stable # v5.3+
Please add the Fixes tag.
> Signed-off-by: Malcolm Priestley
> ---
> drivers/staging/vt6656/baseband.c | 4 ++--
> drivers/staging/vt6656/usbpipe.c | 17 +
> drivers/staging/vt6656/usbpipe.h | 5 +
> 3 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/vt6656/baseband.c
> b/drivers/staging/vt6656/baseband.c
> index 8d19ae71e7cc..4e651b698617 100644
> --- a/drivers/staging/vt6656/baseband.c
> +++ b/drivers/staging/vt6656/baseband.c
> @@ -449,8 +449,8 @@ int vnt_vt3184_init(struct vnt_private *priv)
>
> memcpy(array, addr, length);
>
> - ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, 0,
> - MESSAGE_REQUEST_BBREG, length, array);
> + ret = vnt_control_out_blocks(priv, VNT_REG_BLOCK_SIZE,
> + MESSAGE_REQUEST_BBREG, length, array);
> if (ret)
> goto end;
>
> diff --git a/drivers/staging/vt6656/usbpipe.c
> b/drivers/staging/vt6656/usbpipe.c
> index 488ebd98773d..d977d4777e4f 100644
> --- a/drivers/staging/vt6656/usbpipe.c
> +++ b/drivers/staging/vt6656/usbpipe.c
> @@ -76,6 +76,23 @@ int vnt_control_out_u8(struct vnt_private *priv, u8 reg,
> u8 reg_off, u8 data)
> reg_off, reg, sizeof(u8), &data);
> }
>
> +int vnt_control_out_blocks(struct vnt_private *priv,
> +u16 block, u8 reg, u16 length, u8 *data)
> +{
> + int ret = 0, i;
> +
> + for (i = 0; i < length; i += block) {
> + u16 len = min_t(int, length - i, block);
> +
> + ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE,
> + i, reg, len, data + i);
> + if (ret)
> + goto end;
> + }
> +end:
> + return ret;
Just do a direct return. Goto end is pointless. It hurts readability
because with direct returns we can immediately see that this returns
zero on success.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/5] staging: vt6656: remove bool from vnt_radio_power_on ret
On Fri, Dec 20, 2019 at 09:15:33PM +, Malcolm Priestley wrote:
> The driver uses logical only error checking a bool true would flag error.
>
This commit message is too vague. This is a bugfix and needs to go to
stable. Add a Fixes tag. Here is a suggested wording:
The caller expects this function to return zero or negative error codes
but it instead returns true so it's totally broken.
> Signed-off-by: Malcolm Priestley
> ---
> drivers/staging/vt6656/card.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
> index 56cd77fd9ea0..7958fc165462 100644
> --- a/drivers/staging/vt6656/card.c
> +++ b/drivers/staging/vt6656/card.c
> @@ -719,7 +719,7 @@ int vnt_radio_power_off(struct vnt_private *priv)
> */
> int vnt_radio_power_on(struct vnt_private *priv)
> {
> - int ret = true;
> + int ret = 0;
>
Get rid of the "ret" variable and return 0 directly.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH][next] staging: wfx: check for memory allocation failures from wfx_alloc_hif
On Sat, Dec 21, 2019 at 12:15:43AM +, Colin King wrote:
> From: Colin Ian King
>
> Currently calls to wfx_alloc_hif are not checking for a null return
> when a memory allocation fails and this leads to null pointer
> dereferencing issues. Fix this by adding null pointer checks and
> returning passing down -ENOMEM errors where necessary. The error
> checking in the current driver is a bit sparse, so this may need
> some extra attention later if required.
>
> Fixes: f95a29d40782 ("staging: wfx: add HIF commands helpers")
> Signed-off-by: Colin Ian King
> ---
> drivers/staging/wfx/hif_tx.c | 6 ++
> drivers/staging/wfx/sta.c| 13 +++--
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
> index 8a34a52dd5b9..d8e159670eae 100644
> --- a/drivers/staging/wfx/hif_tx.c
> +++ b/drivers/staging/wfx/hif_tx.c
> @@ -366,6 +366,9 @@ int hif_set_edca_queue_params(struct wfx_vif *wvif, u16
> queue,
> struct hif_req_edca_queue_params *body = wfx_alloc_hif(sizeof(*body),
> &hif);
>
I hate allocations in declaration block. It's way more likely to have
a bug like this where it's missing the NULL check.
> + if (!body)
> + return -ENOMEM;
> +
> WARN_ON(arg->aifs > 255);
> body->aifsn = arg->aifs;
> body->cw_min = cpu_to_le16(arg->cw_min);
> @@ -390,6 +393,9 @@ int hif_set_pm(struct wfx_vif *wvif, bool ps, int
> dynamic_ps_timeout)
> struct hif_msg *hif;
> struct hif_req_set_pm_mode *body = wfx_alloc_hif(sizeof(*body), &hif);
>
> + if (!body)
> + return -ENOMEM;
> +
> if (ps) {
> body->pm_mode.enter_psm = 1;
> // Firmware does not support more than 128ms
> diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
> index 9a61478d98f8..c08d691fe870 100644
> --- a/drivers/staging/wfx/sta.c
> +++ b/drivers/staging/wfx/sta.c
> @@ -316,6 +316,7 @@ int wfx_conf_tx(struct ieee80211_hw *hw, struct
> ieee80211_vif *vif,
> {
> struct wfx_dev *wdev = hw->priv;
> struct wfx_vif *wvif = (struct wfx_vif *) vif->drv_priv;
> + int ret = 0;
>
> WARN_ON(queue >= hw->queues);
>
> @@ -326,10 +327,10 @@ int wfx_conf_tx(struct ieee80211_hw *hw, struct
> ieee80211_vif *vif,
> if (wvif->vif->type == NL80211_IFTYPE_STATION) {
> hif_set_uapsd_info(wvif, wvif->uapsd_mask);
> if (wvif->setbssparams_done && wvif->state == WFX_STATE_STA)
> - wfx_update_pm(wvif);
> + ret = wfx_update_pm(wvif);
> }
> mutex_unlock(&wdev->conf_mutex);
> - return 0;
> + return ret;
> }
>
> int wfx_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
> @@ -1322,7 +1323,7 @@ int wfx_config(struct ieee80211_hw *hw, u32 changed)
> if (changed & IEEE80211_CONF_CHANGE_PS) {
> wvif = NULL;
> while ((wvif = wvif_iterate(wdev, wvif)) != NULL)
> - wfx_update_pm(wvif);
> + ret = wfx_update_pm(wvif);
We reset "ret" on every iteration through the loop and only use the
last value. Probably we should break out of the loop on failure.
> wvif = wdev_to_wvif(wdev, 0);
> }
>
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Fixing coding style. Signed-off-by: amritapat...@gmail.com
On Thu, Jan 02, 2020 at 12:16:53PM +0100, Greg KH wrote:
> On Thu, Jan 02, 2020 at 12:59:29PM +0530, Amrita Patole wrote:
> > From: Amrita Patole
> >
> > ---
> > drivers/staging/qlge/qlge_mpi.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/qlge/qlge_mpi.c
> > b/drivers/staging/qlge/qlge_mpi.c
> > index 9e4226ab..f63db2c79fac 100644
> > --- a/drivers/staging/qlge/qlge_mpi.c
> > +++ b/drivers/staging/qlge/qlge_mpi.c
> > @@ -136,7 +136,8 @@ static int ql_get_mb_sts(struct ql_adapter *qdev,
> > struct mbox_params *mbcp)
> > ql_read_mpi_reg(qdev, qdev->mailbox_out + i,
> > &mbcp->mbox_out[i]);
> > if (status) {
> > - netif_err(qdev, drv, qdev->ndev, "Failed mailbox
> > read.\n");
> > + netif_err(qdev, drv, qdev->ndev,
> > + "Failed mailbox read. \n");
netif_err(qdev, drv, qdev->ndev,
"Failed mailbox read. \n");
"Failed mailbox read. \n");
I'm pretty sure this will introduce a couplee new checkpatch warnings...
It should be indented:
[tab][tab][tab][tab][space][space]"Failed mailbox read.\n");
No space after the period, please.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
From: William Johnson.
Hello, Thank you for lending me your timeand as you may wish to know,I am the accounts updating officer with a leading bank here in London United Kingdom and the transaction is about an abandoned fund with us here hence I have perfected every arrangement to pull the funds totalling the sum of 29.9M British Pound Sterling into your account and it will take 10 official working days for the funds to reflect in the account you will provide for the transfer as all the arrangements has been completed for a successful remittance into your account from the day I receive your reply with your details as stated below. The payment has been in our records for a very long time as abandoned and on further investigations as the accounts officer,I discovered that the customer did not write any name as his next of kin during the account opening and on further inquiry,I was made to understand that the customer died on 17 March 2008 and the account has not been serviced since then hence the rules is that after a period of 10yrs any abandoned funds should be taken to Her Majesty's Treasury Department and it will be ten years this fund has been abandoned this November hence I want to move the funds out to your account before the end of September 2019 before the management will discover that the customer is dead and that the account has been inoperative for 10 years hence you should forward the following asap: 1. Your full name and address 2. Your telephone numbers including your mobile number 3. Your bank name and address,swift/iban number,and your account number. 4. Your International passport/Driver's license for identification. I will prepare an indemnity form on your name as the next of kin to the late account owner which the approval will begin from my department as the accounts updating officer and with the above details the funds will be released into your account within 10 working days as I expect your response with above request to proceed thank you. Sincerely, William Johnson. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
From: William Johnson.
Hello, Thank you for lending me your timeand as you may wish to know,I am the accounts updating officer with a leading bank here in London United Kingdom and the transaction is about an abandoned fund with us here hence I have perfected every arrangement to pull the funds totalling the sum of 29.9M British Pound Sterling into your account and it will take 10 official working days for the funds to reflect in the account you will provide for the transfer as all the arrangements has been completed for a successful remittance into your account from the day I receive your reply with your details as stated below. The payment has been in our records for a very long time as abandoned and on further investigations as the accounts officer,I discovered that the customer did not write any name as his next of kin during the account opening and on further inquiry,I was made to understand that the customer died on 17 March 2008 and the account has not been serviced since then hence the rules is that after a period of 10yrs any abandoned funds should be taken to Her Majesty's Treasury Department and it will be ten years this fund has been abandoned this November hence I want to move the funds out to your account before the end of September 2019 before the management will discover that the customer is dead and that the account has been inoperative for 10 years hence you should forward the following asap: 1. Your full name and address 2. Your telephone numbers including your mobile number 3. Your bank name and address,swift/iban number,and your account number. 4. Your International passport/Driver's license for identification. I will prepare an indemnity form on your name as the next of kin to the late account owner which the approval will begin from my department as the accounts updating officer and with the above details the funds will be released into your account within 10 working days as I expect your response with above request to proceed thank you. Sincerely, William Johnson. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
