Re: [PATCH 13/17] staging: wfx: fix endianness of the field 'len'

2020-05-12 Thread Geert Uytterhoeven 
Hi Jerome,

On Mon, May 11, 2020 at 5:53 PM Jerome Pouiller
 wrote:
> 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

Thanks for your patch!

> 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.

Which means sparse can no longer do any checking on the field,
and new bugs may/will creep in in the future, unnoticed.

> --- a/drivers/staging/wfx/hif_api_general.h
> +++ b/drivers/staging/wfx/hif_api_general.h
> @@ -23,7 +23,10 @@
>  #define HIF_COUNTER_MAX   7
>
>  struct hif_msg {
> -   __le16 len;
> +   // len is in fact little endian. However, it is widely used in the
> +   // driver, so we declare it in native byte order and we reorder just
> +   // before/after send/receive it (see bh.c).
> +   u16len;

While there's a small penalty associated with always doing the conversion
on big-endian platforms, it will probably be lost in the noise anyway.

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH RFC] drivers: most: add USB adapter driver

2020-05-12 Thread Greg KH 
On Mon, May 11, 2020 at 02:46:58PM +, christian.gr...@microchip.com wrote:
> 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.

No need to do that, this is a new layout :)

> > > +#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.

pr_* calls show almost nothing when it comes to the actual device/driver
being affected.  That's why the dev_*() functions are there, please use
them.

> > > +/**
> > > + * 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.

So this same type of thing is also needed in the other bus controllers
(serial, i2c, etc.)?

Creating a new device implies it lives on a bus, and almost always the
bus code for creating/managing that code lives in a single place, not in
the individual drivers.  Why doesn't the most core handle this?  What
does the most core do?  :)


> > > +static DEVICE_ATTR(arb_address, 0644, value_show, value_store);
> > > +static DEVICE_ATTR(arb_value, 0644, value_show, value_store);
> > 
> > Loads of sysfs files with no documentation for them?
> > 
> 
> see driver/staging/most/Documentation

Add it as part of this patch series, as you are moving these sysfs files
into the "real" part of the kernel and belong out of drivers/staging/

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: wfx: unlock on error path

2020-05-12 Thread Dan Carpenter 
We need to release the tx_lock on the error path before returning.

Fixes: d1c015b4ef6f ("staging: wfx: rewrite wfx_hw_scan()")
Signed-off-by: Dan Carpenter 
---
>From static analysis.  Not tested.

 drivers/staging/wfx/scan.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/wfx/scan.c b/drivers/staging/wfx/scan.c
index 76761e4960dd7..eff1be9fb28f8 100644
--- a/drivers/staging/wfx/scan.c
+++ b/drivers/staging/wfx/scan.c
@@ -57,8 +57,10 @@ static int send_scan_req(struct wfx_vif *wvif,
wvif->scan_abort = false;
reinit_completion(&wvif->scan_complete);
timeout = hif_scan(wvif, req, start_idx, i - start_idx);
-   if (timeout < 0)
+   if (timeout < 0) {
+   wfx_tx_unlock(wvif->wdev);
return timeout;
+   }
ret = wait_for_completion_timeout(&wvif->scan_complete, timeout);
if (req->channels[start_idx]->max_power != wvif->vif->bss_conf.txpower)
hif_set_output_power(wvif, wvif->vif->bss_conf.txpower);
-- 
2.26.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 31/38] staging: ion: remove dead code

2020-05-12 Thread Marek Szyprowski 
ion_heap_pages_zero() function is not used at all, so remove it to
simplify the ion_heap_sglist_zero() function later.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprow...@samsung.com/T/
---
 drivers/staging/android/ion/ion.h  | 1 -
 drivers/staging/android/ion/ion_heap.c | 9 -
 2 files changed, 10 deletions(-)

diff --git a/drivers/staging/android/ion/ion.h 
b/drivers/staging/android/ion/ion.h
index 74914a2..c199e88 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -177,7 +177,6 @@ struct ion_heap {
 int ion_heap_map_user(struct ion_heap *heap, struct ion_buffer *buffer,
  struct vm_area_struct *vma);
 int ion_heap_buffer_zero(struct ion_buffer *buffer);
-int ion_heap_pages_zero(struct page *page, size_t size, pgprot_t pgprot);
 
 /**
  * ion_heap_init_shrinker
diff --git a/drivers/staging/android/ion/ion_heap.c 
b/drivers/staging/android/ion/ion_heap.c
index 0755b11..9c23b23 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -145,15 +145,6 @@ int ion_heap_buffer_zero(struct ion_buffer *buffer)
return ion_heap_sglist_zero(table->sgl, table->nents, pgprot);
 }
 
-int ion_heap_pages_zero(struct page *page, size_t size, pgprot_t pgprot)
-{
-   struct scatterlist sg;
-
-   sg_init_table(&sg, 1);
-   sg_set_page(&sg, page, size, 0);
-   return ion_heap_sglist_zero(&sg, 1, pgprot);
-}
-
 void ion_heap_freelist_add(struct ion_heap *heap, struct ion_buffer *buffer)
 {
spin_lock(&heap->free_lock);
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 33/38] staging: tegra-vde: fix common struct sg_table related issues

2020-05-12 Thread Marek Szyprowski 
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().

struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).

It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.

To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprow...@samsung.com/T/
---
 drivers/staging/media/tegra-vde/iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/iommu.c 
b/drivers/staging/media/tegra-vde/iommu.c
index 6af863d..adf8dc7 100644
--- a/drivers/staging/media/tegra-vde/iommu.c
+++ b/drivers/staging/media/tegra-vde/iommu.c
@@ -36,8 +36,8 @@ int tegra_vde_iommu_map(struct tegra_vde *vde,
 
addr = iova_dma_addr(&vde->iova, iova);
 
-   size = iommu_map_sg(vde->domain, addr, sgt->sgl, sgt->nents,
-   IOMMU_READ | IOMMU_WRITE);
+   size = iommu_map_sgtable(vde->domain, addr, sgt,
+IOMMU_READ | IOMMU_WRITE);
if (!size) {
__free_iova(&vde->iova, iova);
return -ENXIO;
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 32/38] staging: ion: fix common struct sg_table related issues

2020-05-12 Thread Marek Szyprowski 
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().

struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).

It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.

To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprow...@samsung.com/T/
---
 drivers/staging/android/ion/ion.c | 25 +++
 drivers/staging/android/ion/ion_heap.c| 44 ---
 drivers/staging/android/ion/ion_system_heap.c |  2 +-
 3 files changed, 25 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 38b51ea..2db041d 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -147,14 +147,14 @@ static struct sg_table *dup_sg_table(struct sg_table 
*table)
if (!new_table)
return ERR_PTR(-ENOMEM);
 
-   ret = sg_alloc_table(new_table, table->nents, GFP_KERNEL);
+   ret = sg_alloc_table(new_table, table->orig_nents, GFP_KERNEL);
if (ret) {
kfree(new_table);
return ERR_PTR(-ENOMEM);
}
 
new_sg = new_table->sgl;
-   for_each_sg(table->sgl, sg, table->nents, i) {
+   for_each_sgtable_sg(table, sg, i) {
memcpy(new_sg, sg, sizeof(*sg));
new_sg->dma_address = 0;
new_sg = sg_next(new_sg);
@@ -224,12 +224,13 @@ static struct sg_table *ion_map_dma_buf(struct 
dma_buf_attachment *attachment,
 {
struct ion_dma_buf_attachment *a = attachment->priv;
struct sg_table *table;
+   int ret;
 
table = a->table;
 
-   if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
-   direction))
-   return ERR_PTR(-ENOMEM);
+   ret = dma_map_sgtable(attachment->dev, table, direction);
+   if (ret)
+   return ERR_PTR(ret);
 
return table;
 }
@@ -238,7 +239,7 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment 
*attachment,
  struct sg_table *table,
  enum dma_data_direction direction)
 {
-   dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
+   dma_unmap_sgtable(attachment->dev, table, direction);
 }
 
 static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
@@ -296,10 +297,8 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf 
*dmabuf,
}
 
mutex_lock(&buffer->lock);
-   list_for_each_entry(a, &buffer->attachments, list) {
-   dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
-   direction);
-   }
+   list_for_each_entry(a, &buffer->attachments, list)
+   dma_sync_sgtable_for_cpu(a->dev, a->table, direction);
 
 unlock:
mutex_unlock(&buffer->lock);
@@ -319,10 +318,8 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf 
*dmabuf,
}
 
mutex_lock(&buffer->lock);
-   list_for_each_entry(a, &buffer->attachments, list) {
-   dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents,
-  direction);
-   }
+   list_for_each_entry(a, &buffer->attachments, list)
+   dma_sync_sgtable_for_device(a->dev, a->table, direction);
mutex_unlock(&buffer->lock);
 
return 0;
diff --git a/drivers/staging/android/ion/ion_heap.c 
b/drivers/staging/android/ion/ion_heap.c
index 9c23b23..79f2794 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -20,8 +20,7 @@
 void *ion_heap_map_kernel(struct ion_heap *heap,
  struct ion_buffer *buffer)
 {
-   struct scatterlist *sg;
-   int i, j;
+   struct sg_page_iter piter;
void *vaddr;
pgprot_t pgprot;
struct sg_table *table

Re: [PATCH] staging: wfx: unlock on error path

2020-05-12 Thread Jérôme Pouiller 
On Tuesday 12 May 2020 10:36:56 CEST Dan Carpenter wrote:
> 
> We need to release the tx_lock on the error path before returning.
> 
> Fixes: d1c015b4ef6f ("staging: wfx: rewrite wfx_hw_scan()")
> Signed-off-by: Dan Carpenter 
> ---
> From static analysis.  Not tested.
> 
>  drivers/staging/wfx/scan.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/wfx/scan.c b/drivers/staging/wfx/scan.c
> index 76761e4960dd7..eff1be9fb28f8 100644
> --- a/drivers/staging/wfx/scan.c
> +++ b/drivers/staging/wfx/scan.c
> @@ -57,8 +57,10 @@ static int send_scan_req(struct wfx_vif *wvif,
> wvif->scan_abort = false;
> reinit_completion(&wvif->scan_complete);
> timeout = hif_scan(wvif, req, start_idx, i - start_idx);
> -   if (timeout < 0)
> +   if (timeout < 0) {
> +   wfx_tx_unlock(wvif->wdev);
> return timeout;
> +   }
> ret = wait_for_completion_timeout(&wvif->scan_complete, timeout);
> if (req->channels[start_idx]->max_power != 
> wvif->vif->bss_conf.txpower)
> hif_set_output_power(wvif, wvif->vif->bss_conf.txpower);
> --
> 2.26.2
> 
> 

Good catch!

Reviewed-by: Jérôme Pouiller 

-- 
Jérôme Pouiller


___
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'

2020-05-12 Thread Jérôme Pouiller 
On Tuesday 12 May 2020 09:43:34 CEST Geert Uytterhoeven wrote:
> Hi Jerome,
> 
> On Mon, May 11, 2020 at 5:53 PM Jerome Pouiller
>  wrote:
> > 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
> 
> Thanks for your patch!
> 
> > 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.
> 
> Which means sparse can no longer do any checking on the field,
> and new bugs may/will creep in in the future, unnoticed.
> 
> > --- a/drivers/staging/wfx/hif_api_general.h
> > +++ b/drivers/staging/wfx/hif_api_general.h
> > @@ -23,7 +23,10 @@
> >  #define HIF_COUNTER_MAX   7
> >
> >  struct hif_msg {
> > -   __le16 len;
> > +   // len is in fact little endian. However, it is widely used in the
> > +   // driver, so we declare it in native byte order and we reorder just
> > +   // before/after send/receive it (see bh.c).
> > +   u16len;
> 
> While there's a small penalty associated with always doing the conversion
> on big-endian platforms, it will probably be lost in the noise anyway.

I have made the changes to show you that the code is far more complicated
with a le16... and the result was not as complicated as I expected...

I am going to post a v2.


-- 
Jérôme Pouiller


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH RFC] drivers: most: add USB adapter driver

2020-05-12 Thread Christian.Gromm 
On Mon, 2020-05-11 at 18:33 +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 02:46:58PM +, 
> christian.gr...@microchip.com wrote:
> > 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.
> 
> No need to do that, this is a new layout :)
> 
> > > > +#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.
> 
> pr_* calls show almost nothing when it comes to the actual
> device/driver
> being affected.  That's why the dev_*() functions are there, please
> use
> them.
> 
> > > > +/**
> > > > + * 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.
> 
> So this same type of thing is also needed in the other bus
> controllers
> (serial, i2c, etc.)?
> 
> Creating a new device implies it lives on a bus, and almost always
> the
> bus code for creating/managing that code lives in a single place, not
> in
> the individual drivers.  Why doesn't the most core handle this?  What
> does the most core do?  :)

The core module manages the buffers, routing, configuration,
sysfs/configfs and user space interface (via its component modules)
for common communication channels. The DCI interface is only available
when the hardware is connected via USB. Other connections do not
provide this.

That's why I want the module that actually introduces such an
interface (and has all the necessary information about it) to be in
charge. This keeps the core code simpler, as USB isn't always used.

Also, a new device is needed to create the desired sysfs layout,
in which the dci interface is represented with a new sub directory.


thanks,
Chris
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 09/17] staging: wfx: fix access to le32 attribute 'event_id'

2020-05-12 Thread Jerome Pouiller 
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 v2 06/17] staging: wfx: fix endianness of fields media_delay and tx_queue_delay

2020-05-12 Thread Jerome Pouiller 
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 v2 07/17] staging: wfx: fix endianness of hif_req_read_mib fields

2020-05-12 Thread Jerome Pouiller 
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 v2 08/17] staging: wfx: fix access to le32 attribute 'ps_mode_error'

2020-05-12 Thread Jerome Pouiller 
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

[PATCH v2 14/17] staging: wfx: fix endianness of the field 'status'

2020-05-12 Thread Jerome Pouiller 
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 014fa36c8f78..4a2910897b6f 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 995752b9f168..f5abd8174706 100644
--- a/drivers/staging/wfx/hif_api_general.h
+++ b/drivers/staging/wfx/hif_api_general.h
@@ -67,21 +67,27 @@ enum hif_general_indications_ids

[PATCH v2 03/17] staging: wfx: fix cast operator

2020-05-12 Thread Jerome Pouiller 
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 v2 12/17] staging: wfx: fix endianness of the struct hif_ind_startup

2020-05-12 Thread Jerome Pouiller 
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 v2 04/17] staging: wfx: fix wrong bytes order

2020-05-12 Thread Jerome Pouiller 
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 v2 16/17] staging: wfx: fix endianness of the field 'channel_number'

2020-05-12 Thread Jerome Pouiller 
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 bb776ee6689c..7f459719e7b4 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 v2 02/17] staging: wfx: take advantage of le32_to_cpup()

2020-05-12 Thread Jerome Pouiller 
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 v2 17/17] staging: wfx: update TODO

2020-05-12 Thread Jerome Pouiller 
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 v2 05/17] staging: wfx: fix output of rx_stats on big endian hosts

2020-05-12 Thread Jerome Pouiller 
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 v2 01/17] staging: wfx: fix use of cpu_to_le32 instead of le32_to_cpu

2020-05-12 Thread Jerome Pouiller 
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 v2 10/17] staging: wfx: fix access to le32 attribute 'indication_type'

2020-05-12 Thread Jerome Pouiller 
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 v2 11/17] staging: wfx: declare the field 'packet_id' with native byte order

2020-05-12 Thread Jerome Pouiller 
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 v2 15/17] staging: wfx: fix endianness of the field 'num_tx_confs'

2020-05-12 Thread Jerome Pouiller 
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 6c6e29cb7dcf..1cbaf8bb4fa3 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -102,7 +102,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 v2 00/17] staging: wfx: fix support for big-endian hosts

2020-05-12 Thread Jerome Pouiller 
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

v2:
  Rewrite patch 13: keep the endianness of the field 'len' as-is and fix
  the accesses.

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 access to le32 attribute '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  | 17 +-
 drivers/staging/wfx/data_rx.c |  4 +--
 drivers/staging/wfx/data_tx.c |  7 ++--
 drivers/staging/wfx/debug.c   | 13 +---
 drivers/staging/wfx/hif_api_cmd.h | 42 ++--
 drivers/staging/wfx/hif_api_general.h | 47 ---
 drivers/staging/wfx/hif_rx.c  | 38 --
 drivers/staging/wfx/hif_tx.c  | 18 +-
 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  | 10 +++---
 13 files changed, 104 insertions(+), 117 deletions(-)

-- 
2.26.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 13/17] staging: wfx: fix access to le32 attribute 'len'

2020-05-12 Thread Jerome Pouiller 
From: Jérôme Pouiller 

Sparse complains about the accesses to the field 'len' from struct hif_msg:

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

Indeed, the attribute len is little-endian. We have to take to the
endianness when we access it.

Signed-off-by: Jérôme Pouiller 
---
 drivers/staging/wfx/bh.c | 13 ++---
 drivers/staging/wfx/debug.c  |  2 +-
 drivers/staging/wfx/hif_rx.c |  6 +++---
 drivers/staging/wfx/traces.h |  2 +-
 4 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index 55724e4295c4..6c6e29cb7dcf 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -84,13 +84,12 @@ 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);
-   computed_len = round_up(hif->len - sizeof(hif->len), 16)
-  + sizeof(struct hif_sl_msg)
-  + sizeof(struct hif_sl_tag);
+   computed_len =
+   round_up(le16_to_cpu(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);
+   computed_len = round_up(le16_to_cpu(hif->len), 2);
}
if (computed_len != read_len) {
dev_err(wdev->dev, "inconsistent message length: %zu != %zu\n",
@@ -118,7 +117,7 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, 
int *is_cnf)
wdev->hif.rx_seqnum = (hif->seqnum + 1) % (HIF_COUNTER_MAX + 1);
}
 
-   skb_put(skb, hif->len);
+   skb_put(skb, le16_to_cpu(hif->len));
// wfx_handle_rx takes care on SKB livetime
wfx_handle_rx(wdev, skb);
if (!wdev->hif.tx_buffers_used)
diff --git a/drivers/staging/wfx/debug.c b/drivers/staging/wfx/debug.c
index 846a0b29f8c9..f52e7cf885cb 100644
--- a/drivers/staging/wfx/debug.c
+++ b/drivers/staging/wfx/debug.c
@@ -250,7 +250,7 @@ static ssize_t wfx_send_hif_msg_write(struct file *file,
request = memdup_user(user_buf, count);
if (IS_ERR(request))
return PTR_ERR(request);
-   if (request->len != count) {
+   if (le16_to_cpu(request->len) != count) {
kfree(request);
return -EINVAL;
}
diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c
index 9b4f0c4ba745..36132909a6ae 100644
--- a/drivers/staging/wfx/hif_rx.c
+++ b/drivers/staging/wfx/hif_rx.c
@@ -24,7 +24,7 @@ static int hif_generic_confirm(struct wfx_dev *wdev,
// All confirm messages start with status
int status = le32_to_cpup((__le32 *)buf);
int cmd = hif->id;
-   int len = hif->len -

Re: [PATCH -next] media: tegra: Make tegra210_video_formats static

2020-05-12 Thread Thierry Reding 
On Mon, May 11, 2020 at 07:20:15PM +0800, 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(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel