[PATCH] staging: media: make use of devm_platform_ioremap_resource

2019-10-07 Thread hariprasad 
From: Hariprasad Kelam 

fix below issue reported by coccicheck
drivers/staging//media/omap4iss/iss.c:915:1-15: WARNING: Use
devm_platform_ioremap_resource for iss -> regs [ res ]

Signed-off-by: Hariprasad Kelam 
---
 drivers/staging/media/omap4iss/iss.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/media/omap4iss/iss.c 
b/drivers/staging/media/omap4iss/iss.c
index 1a966cb..6fb60b5 100644
--- a/drivers/staging/media/omap4iss/iss.c
+++ b/drivers/staging/media/omap4iss/iss.c
@@ -908,11 +908,7 @@ static int iss_map_mem_resource(struct platform_device 
*pdev,
struct iss_device *iss,
enum iss_mem_resources res)
 {
-   struct resource *mem;
-
-   mem = platform_get_resource(pdev, IORESOURCE_MEM, res);
-
-   iss->regs[res] = devm_ioremap_resource(iss->dev, mem);
+   iss->regs[res] = devm_platform_ioremap_resource(pdev, res);
 
return PTR_ERR_OR_ZERO(iss->regs[res]);
 }
-- 
2.7.4

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

[PATCH] staging: emxx_udc: make use of devm_platform_ioremap_resource

2019-10-07 Thread hariprasad 
From: Hariprasad Kelam 

fix below issue reported by coccicheck
drivers/staging//emxx_udc/emxx_udc.c:3092:1-10: WARNING: Use
devm_platform_ioremap_resource for mmio_base

Signed-off-by: Hariprasad Kelam 
---
 drivers/staging/emxx_udc/emxx_udc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c 
b/drivers/staging/emxx_udc/emxx_udc.c
index 147481b..9e0c19e 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -3078,7 +3078,6 @@ static int nbu2ss_drv_probe(struct platform_device *pdev)
 {
int status = -ENODEV;
struct nbu2ss_udc   *udc;
-   struct resource *r;
int irq;
void __iomem *mmio_base;
 
@@ -3088,8 +3087,7 @@ static int nbu2ss_drv_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, udc);
 
/* require I/O memory and IRQ to be provided as resources */
-   r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   mmio_base = devm_ioremap_resource(&pdev->dev, r);
+   mmio_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(mmio_base))
return PTR_ERR(mmio_base);
 
-- 
2.7.4

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

[PATCH] staging: ralink-gdma: make use of devm_platform_ioremap_resource

2019-10-07 Thread hariprasad 
From: Hariprasad Kelam 

fix below issue reported by coccicheck
drivers/staging//ralink-gdma/ralink-gdma.c:822:1-5: WARNING: Use
devm_platform_ioremap_resource for base

Signed-off-by: Hariprasad Kelam 
---
 drivers/staging/ralink-gdma/ralink-gdma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c 
b/drivers/staging/ralink-gdma/ralink-gdma.c
index 900424d..eabf109 100644
--- a/drivers/staging/ralink-gdma/ralink-gdma.c
+++ b/drivers/staging/ralink-gdma/ralink-gdma.c
@@ -796,7 +796,6 @@ static int gdma_dma_probe(struct platform_device *pdev)
struct gdma_dma_dev *dma_dev;
struct dma_device *dd;
unsigned int i;
-   struct resource *res;
int ret;
int irq;
void __iomem *base;
@@ -818,8 +817,7 @@ static int gdma_dma_probe(struct platform_device *pdev)
return -EINVAL;
dma_dev->data = data;
 
-   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   base = devm_ioremap_resource(&pdev->dev, res);
+   base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(base))
return PTR_ERR(base);
dma_dev->base = base;
-- 
2.7.4

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

[PATCH] staging: netlogic: make use of devm_platform_ioremap_resource

2019-10-07 Thread hariprasad 
From: Hariprasad Kelam 

fix below issue reported by coccicheck
drivers/staging//netlogic/xlr_net.c:980:2-17: WARNING: Use
devm_platform_ioremap_resource for priv -> base_addr

Signed-off-by: Hariprasad Kelam 
---
 drivers/staging/netlogic/xlr_net.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/netlogic/xlr_net.c 
b/drivers/staging/netlogic/xlr_net.c
index 05079f7..204fcdf 100644
--- a/drivers/staging/netlogic/xlr_net.c
+++ b/drivers/staging/netlogic/xlr_net.c
@@ -976,8 +976,7 @@ static int xlr_net_probe(struct platform_device *pdev)
priv->ndev = ndev;
priv->port_id = (pdev->id * 4) + port;
priv->nd = (struct xlr_net_data *)pdev->dev.platform_data;
-   res = platform_get_resource(pdev, IORESOURCE_MEM, port);
-   priv->base_addr = devm_ioremap_resource(&pdev->dev, res);
+   priv->base_addr = devm_platform_ioremap_resource(pdev, port);
if (IS_ERR(priv->base_addr)) {
err = PTR_ERR(priv->base_addr);
goto err_gmac;
-- 
2.7.4

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

[PATCH] staging: mt7621-dma: make use of devm_platform_ioremap_resource

2019-10-07 Thread hariprasad 
From: Hariprasad Kelam 

fix below issue reported by coccicheck
rivers/staging//mt7621-dma/mtk-hsdma.c:671:1-5: WARNING: Use
devm_platform_ioremap_resource for base

Signed-off-by: Hariprasad Kelam 
---
 drivers/staging/mt7621-dma/mtk-hsdma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/mt7621-dma/mtk-hsdma.c 
b/drivers/staging/mt7621-dma/mtk-hsdma.c
index d964642..03c8937 100644
--- a/drivers/staging/mt7621-dma/mtk-hsdma.c
+++ b/drivers/staging/mt7621-dma/mtk-hsdma.c
@@ -650,7 +650,6 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
struct mtk_hsdma_chan *chan;
struct mtk_hsdam_engine *hsdma;
struct dma_device *dd;
-   struct resource *res;
int ret;
int irq;
void __iomem *base;
@@ -667,8 +666,7 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
if (!hsdma)
return -EINVAL;
 
-   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   base = devm_ioremap_resource(&pdev->dev, res);
+   base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(base))
return PTR_ERR(base);
hsdma->base = base + HSDMA_BASE_OFFSET;
-- 
2.7.4

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

[PATCH] staging: media: imx: make use devm_platform_ioremap_resource

2019-10-07 Thread hariprasad 
From: Hariprasad Kelam 

fix below issue reported by coccicheck
drivers/staging//media/imx/imx7-mipi-csis.c:973:1-12: WARNING: Use
devm_platform_ioremap_resource for state -> regs

Signed-off-by: Hariprasad Kelam 
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
b/drivers/staging/media/imx/imx7-mipi-csis.c
index 73d8354..bf21db3 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -947,7 +947,6 @@ static void mipi_csis_debugfs_exit(struct csi_state *state)
 static int mipi_csis_probe(struct platform_device *pdev)
 {
struct device *dev = &pdev->dev;
-   struct resource *mem_res;
struct csi_state *state;
int ret;
 
@@ -969,8 +968,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
mipi_csis_phy_init(state);
mipi_csis_phy_reset(state);
 
-   mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   state->regs = devm_ioremap_resource(dev, mem_res);
+   state->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(state->regs))
return PTR_ERR(state->regs);
 
-- 
2.7.4

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

Re: [PATCH] staging: wlan-ng: p80211wep.c: use lib/crc32

2019-10-07 Thread Dan Carpenter 
On Mon, Oct 07, 2019 at 06:15:23PM +0200, Thomas Meyer wrote:
> Dan Carpenter  writes:
> 
> > On Sun, Oct 06, 2019 at 04:07:45PM +0200, Thomas Meyer wrote:
> >> Use lib/crc32 instead of another implementation.
> >> 
> >> Signed-off-by: Thomas Meyer 
> >
> > I always get annoyed whenever anyone asks if people have tested their
> > patches, but have you tested this?
> 
> no annoynence on my side, but... :-)
> 
> Good question. I tell you what I did and then you tell me if I did test!
> 
> So I did this: I did write a small C program that does contain a small
> byte buffer and the extracted CRC32 logic from the wlan driver.
> The program does calculate the CRC32 sum with the extracted logic and by
> calling crc32_le function. but values are the same.
> 
> But as I don't own the hardware I couldn't do a real test with WEP (as
> far as I understand only WEP on this hardware would be affected.)
> 
> So a better test would be to find someone which actually owns the
> hardware and could test the change.
> 
> so... what do you think?

Could you send your test program?

Anyway your testing sounds reasonable so I'm fine with this change.

regards,
dan carpenter

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

Re: [PATCH] staging: rtl8712: Add comment to lock declaration

2019-10-07 Thread Dan Carpenter 
On Mon, Oct 07, 2019 at 09:52:48PM +0100, Jules Irenge wrote:
> Add comment to spinlock declaration to fix warning issued by checkpatch.pl
> "CHECK: spinlock_t definition without comment".
> 
> Signed-off-by: Jules Irenge 
> ---
>  drivers/staging/rtl8712/drv_types.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8712/drv_types.h 
> b/drivers/staging/rtl8712/drv_types.h
> index 0c4325073c63..960d8709aada 100644
> --- a/drivers/staging/rtl8712/drv_types.h
> +++ b/drivers/staging/rtl8712/drv_types.h
> @@ -160,7 +160,7 @@ struct _adapter {
>   int pid; /*process id from UI*/
>   struct work_struct wk_filter_rx_ff0;
>   u8 blnEnableRxFF0Filter;
> - spinlock_t lock_rx_ff0_filter;
> + spinlock_t lock_rx_ff0_filter; /*spinlock to protect interrupt request*/

This spinlock seems to be nonsense.  It's only used one time:

drivers/staging/rtl8712/xmit_linux.c
94  void r8712_SetFilter(struct work_struct *work)
95  {
96  struct _adapter *adapter = container_of(work, struct _adapter,
97  wk_filter_rx_ff0);
98  u8  oldvalue = 0x00, newvalue = 0x00;
99  unsigned long irqL;
   100  
   101  oldvalue = r8712_read8(adapter, 0x117);
   102  newvalue = oldvalue & 0xfe;
   103  r8712_write8(adapter, 0x117, newvalue);
   104  
   105  spin_lock_irqsave(&adapter->lock_rx_ff0_filter, irqL);
   106  adapter->blnEnableRxFF0Filter = 1;

It only protects writing to ->blnEnableRxFF0Filter but it doesn't
protect reading so it can't possibly work.

   107  spin_unlock_irqrestore(&adapter->lock_rx_ff0_filter, irqL);
   108  do {
   109  msleep(100);
   110  } while (adapter->blnEnableRxFF0Filter == 1);
   111  r8712_write8(adapter, 0x117, oldvalue);
   112  }

Also put a space after /* and before */ so the comment looks like:
/* spinlock to protect interrupt request */

But in this case, the comment isn't correct so please just leave it
as-is until someone can fix the locking.

regards,
dan carpenter

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

[PATCH] staging: fieldbus: make use of devm_platform_ioremap_resource

2019-10-07 Thread hariprasad 
From: Hariprasad Kelam 

fix below issues reported by coccicheck
drivers/staging//fieldbus/anybuss/arcx-anybus.c:135:1-5: WARNING: Use
devm_platform_ioremap_resource for base
drivers/staging//fieldbus/anybuss/arcx-anybus.c:248:1-14: WARNING: Use
devm_platform_ioremap_resource for cd -> cpld_base

Signed-off-by: Hariprasad Kelam 
---
 drivers/staging/fieldbus/anybuss/arcx-anybus.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/fieldbus/anybuss/arcx-anybus.c 
b/drivers/staging/fieldbus/anybuss/arcx-anybus.c
index 2ecffa4..5b8d0ba 100644
--- a/drivers/staging/fieldbus/anybuss/arcx-anybus.c
+++ b/drivers/staging/fieldbus/anybuss/arcx-anybus.c
@@ -127,12 +127,10 @@ static const struct regmap_config arcx_regmap_cfg = {
 static struct regmap *create_parallel_regmap(struct platform_device *pdev,
 int idx)
 {
-   struct resource *res;
void __iomem *base;
struct device *dev = &pdev->dev;
 
-   res = platform_get_resource(pdev, IORESOURCE_MEM, idx + 1);
-   base = devm_ioremap_resource(dev, res);
+   base = devm_platform_ioremap_resource(pdev, idx + 1);
if (IS_ERR(base))
return ERR_CAST(base);
return devm_regmap_init_mmio(dev, base, &arcx_regmap_cfg);
@@ -230,7 +228,6 @@ static int controller_probe(struct platform_device *pdev)
struct regulator_config config = { };
struct regulator_dev *regulator;
int err, id;
-   struct resource *res;
struct anybuss_host *host;
u8 status1, cap;
 
@@ -244,8 +241,7 @@ static int controller_probe(struct platform_device *pdev)
return PTR_ERR(cd->reset_gpiod);
 
/* CPLD control memory, sits at index 0 */
-   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   cd->cpld_base = devm_ioremap_resource(dev, res);
+   cd->cpld_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(cd->cpld_base)) {
dev_err(dev,
"failed to map cpld base address\n");
-- 
2.7.4

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

Re: [PATCH 2/6] staging: sm750fb: Remove set but not used variable 'uiActualPixelClk'

2019-10-07 Thread Dan Carpenter 
On Tue, Oct 08, 2019 at 09:06:11AM +0300, Dan Carpenter wrote:
> On Tue, Oct 08, 2019 at 01:38:58PM +0800, zhengbin wrote:
> > diff --git a/drivers/staging/sm750fb/ddk750_mode.c 
> > b/drivers/staging/sm750fb/ddk750_mode.c
> > index 9722692..e0230f4 100644
> > --- a/drivers/staging/sm750fb/ddk750_mode.c
> > +++ b/drivers/staging/sm750fb/ddk750_mode.c
> > @@ -209,12 +209,11 @@ static int programModeRegisters(struct mode_parameter 
> > *pModeParam,
> >  int ddk750_setModeTiming(struct mode_parameter *parm, enum clock_type 
> > clock)
> >  {
> > struct pll_value pll;
> > -   unsigned int uiActualPixelClk;
> > 
> > pll.input_freq = DEFAULT_INPUT_CLOCK;
> > pll.clock_type = clock;
> > 
> > -   uiActualPixelClk = sm750_calc_pll_value(parm->pixel_clock, &pll);
> > +   sm750_calc_pll_value(parm->pixel_clock, &pll);
> 
> Get rid of this function as well.

Oops.  I'm wrong.  Sorry for the noise.  Leave the function.

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

Re: [PATCH 2/6] staging: sm750fb: Remove set but not used variable 'uiActualPixelClk'

2019-10-07 Thread Dan Carpenter 
On Tue, Oct 08, 2019 at 01:38:58PM +0800, zhengbin wrote:
> diff --git a/drivers/staging/sm750fb/ddk750_mode.c 
> b/drivers/staging/sm750fb/ddk750_mode.c
> index 9722692..e0230f4 100644
> --- a/drivers/staging/sm750fb/ddk750_mode.c
> +++ b/drivers/staging/sm750fb/ddk750_mode.c
> @@ -209,12 +209,11 @@ static int programModeRegisters(struct mode_parameter 
> *pModeParam,
>  int ddk750_setModeTiming(struct mode_parameter *parm, enum clock_type clock)
>  {
>   struct pll_value pll;
> - unsigned int uiActualPixelClk;
> 
>   pll.input_freq = DEFAULT_INPUT_CLOCK;
>   pll.clock_type = clock;
> 
> - uiActualPixelClk = sm750_calc_pll_value(parm->pixel_clock, &pll);
> + sm750_calc_pll_value(parm->pixel_clock, &pll);

Get rid of this function as well.

regards,
dan carpenter


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

Re: [PATCH 1/6] staging: bcm2835-audio: Remove set but not used variable 'status'

2019-10-07 Thread Dan Carpenter 
On Tue, Oct 08, 2019 at 01:38:57PM +0800, zhengbin wrote:
> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c 
> b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> index c6f9cf1..8a94c5b 100644
> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> @@ -96,13 +96,12 @@ static void audio_vchi_callback(void *param,
>   struct bcm2835_audio_instance *instance = param;
>   struct vc_audio_msg m;
>   int msg_len;
> - int status;
> 
>   if (reason != VCHI_CALLBACK_MSG_AVAILABLE)
>   return;
> 
> - status = vchi_msg_dequeue(instance->vchi_handle,
> -   &m, sizeof(m), &msg_len, VCHI_FLAGS_NONE);
> + vchi_msg_dequeue(instance->vchi_handle,
> +  &m, sizeof(m), &msg_len, VCHI_FLAGS_NONE);

This looks like a bug in the code.  flags is VCHI_FLAGS_NONE so it can
return -1 and we should check for that.

>   if (m.type == VC_AUDIO_MSG_TYPE_RESULT) {

regards,
dan carpenter

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

Re: [Outreachy kernel] [PATCH v2] staging: vc04_services: Avoid NULL comparison

2019-10-07 Thread Julia Lawall 



On Mon, 7 Oct 2019, Nachammai Karuppiah wrote:

> Remove NULL comparison. Issue found using checkpatch.pl

This introduces compiler warnings, which you should try very hard not to
do.

julia

>
> Signed-off-by: Nachammai Karuppiah 
>
> ---
>
> Changes in V2
>- Remove all NULL comparisons in vc04_services/interface directory.
> ---
>  .../interface/vchiq_arm/vchiq_2835_arm.c   |  4 ++--
>  .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 28 
> +++---
>  .../vc04_services/interface/vchiq_arm/vchiq_core.c |  4 ++--
>  .../vc04_services/interface/vchiq_arm/vchiq_shim.c |  2 +-
>  4 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git 
> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c 
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> index 8dc730c..7cdb21e 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> @@ -526,11 +526,11 @@ create_pagelist(char __user *buf, size_t count, 
> unsigned short type)
>   return NULL;
>   }
>
> - WARN_ON(g_free_fragments == NULL);
> + WARN_ON(!g_free_fragments);
>
>   down(&g_free_fragments_mutex);
>   fragments = g_free_fragments;
> - WARN_ON(fragments == NULL);
> + WARN_ON(!fragments);
>   g_free_fragments = *(char **) g_free_fragments;
>   up(&g_free_fragments_mutex);
>   pagelist->type = PAGELIST_READ_WITH_FRAGMENTS +
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c 
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index b1595b1..b930148 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -826,8 +826,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned 
> long arg)
>
>   /* Remove all services */
>   i = 0;
> - while ((service = next_service_by_instance(instance->state,
> - instance, &i)) != NULL) {
> + while (service = next_service_by_instance(instance->state,
> + instance, &i)) {
>   status = vchiq_remove_service(service->handle);
>   unlock_service(service);
>   if (status != VCHIQ_SUCCESS)
> @@ -907,7 +907,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned 
> long arg)
>   &args.params, srvstate,
>   instance, user_service_free);
>
> - if (service != NULL) {
> + if (service) {
>   user_service->service = service;
>   user_service->userdata = userdata;
>   user_service->instance = instance;
> @@ -988,7 +988,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned 
> long arg)
>   VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
>
>   service = find_service_for_instance(instance, handle);
> - if (service != NULL) {
> + if (service) {
>   status = (cmd == VCHIQ_IOC_USE_SERVICE) ?
>   vchiq_use_service_internal(service) :
>   vchiq_release_service_internal(service);
> @@ -1021,7 +1021,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, 
> unsigned long arg)
>
>   service = find_service_for_instance(instance, args.handle);
>
> - if ((service != NULL) && (args.count <= MAX_ELEMENTS)) {
> + if (service && (args.count <= MAX_ELEMENTS)) {
>   /* Copy elements into kernel space */
>   struct vchiq_element elements[MAX_ELEMENTS];
>
> @@ -1343,11 +1343,11 @@ vchiq_ioctl(struct file *file, unsigned int cmd, 
> unsigned long arg)
>   spin_unlock(&msg_queue_spinlock);
>
>   complete(&user_service->remove_event);
> - if (header == NULL)
> + if (!header)
>   ret = -ENOTCONN;
>   else if (header->size <= args.bufsize) {
>   /* Copy to user space if msgbuf is not NULL */
> - if ((args.buf == NULL) ||
> + if (!args.buf ||
>   (copy_to_user((void __user *)args.buf,
>   header->data,
>   header->size) == 0)) {
> @@ -1426,7 +1426,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, 
> unsigned long arg)
>   VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
>
>   service = find_closed_service_for_instance(instance, handle);
> - if (service != NULL) {
> + if (service) {
>   struct user_service *user_ser

[PATCH] staging: clocking-wizard: make use of devm_platform_ioremap_resource

2019-10-07 Thread hariprasad 
From: Hariprasad Kelam 

Fix below issue reported by coccicheck
drivers/staging//clocking-wizard/clk-xlnx-clock-wizard.c:147:1-15:
WARNING: Use devm_platform_ioremap_resource for clk_wzrd -> base

Signed-off-by: Hariprasad Kelam 
---
 drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c 
b/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
index 15b7a82..e52a64b 100644
--- a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
+++ b/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
@@ -135,7 +135,6 @@ static int clk_wzrd_probe(struct platform_device *pdev)
unsigned long rate;
const char *clk_name;
struct clk_wzrd *clk_wzrd;
-   struct resource *mem;
struct device_node *np = pdev->dev.of_node;
 
clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
@@ -143,8 +142,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
return -ENOMEM;
platform_set_drvdata(pdev, clk_wzrd);
 
-   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   clk_wzrd->base = devm_ioremap_resource(&pdev->dev, mem);
+   clk_wzrd->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(clk_wzrd->base))
return PTR_ERR(clk_wzrd->base);
 
-- 
2.7.4

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

[PATCH 2/6] staging: sm750fb: Remove set but not used variable 'uiActualPixelClk'

2019-10-07 Thread zhengbin 
Fixes gcc '-Wunused-but-set-variable' warning:

drivers/staging/sm750fb/ddk750_mode.c: In function ddk750_setModeTiming:
drivers/staging/sm750fb/ddk750_mode.c:212:15: warning: variable 
uiActualPixelClk set but not used [-Wunused-but-set-variable]

It is not used since commit 81dee67e215b ("staging:
sm750fb: add sm750 to staging")

Reported-by: Hulk Robot 
Signed-off-by: zhengbin 
---
 drivers/staging/sm750fb/ddk750_mode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_mode.c 
b/drivers/staging/sm750fb/ddk750_mode.c
index 9722692..e0230f4 100644
--- a/drivers/staging/sm750fb/ddk750_mode.c
+++ b/drivers/staging/sm750fb/ddk750_mode.c
@@ -209,12 +209,11 @@ static int programModeRegisters(struct mode_parameter 
*pModeParam,
 int ddk750_setModeTiming(struct mode_parameter *parm, enum clock_type clock)
 {
struct pll_value pll;
-   unsigned int uiActualPixelClk;

pll.input_freq = DEFAULT_INPUT_CLOCK;
pll.clock_type = clock;

-   uiActualPixelClk = sm750_calc_pll_value(parm->pixel_clock, &pll);
+   sm750_calc_pll_value(parm->pixel_clock, &pll);
if (sm750_get_chip_type() == SM750LE) {
/* set graphic mode via IO method */
outb_p(0x88, 0x3d4);
--
2.7.4

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

[PATCH 3/6] staging: sm750fb: Remove set but not used variable 'actual_mx_clk'

2019-10-07 Thread zhengbin 
Fixes gcc '-Wunused-but-set-variable' warning:

drivers/staging/sm750fb/ddk750_chip.c: In function set_chip_clock:
drivers/staging/sm750fb/ddk750_chip.c:59:15: warning: variable actual_mx_clk 
set but not used [-Wunused-but-set-variable]

It is not used since commit f0977109a577 ("staging:
sm750fb: lower case to fix camelcase checkpatch warning")

Reported-by: Hulk Robot 
Signed-off-by: zhengbin 
---
 drivers/staging/sm750fb/ddk750_chip.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_chip.c 
b/drivers/staging/sm750fb/ddk750_chip.c
index e5988813..02860d3 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -56,7 +56,6 @@ static unsigned int get_mxclk_freq(void)
 static void set_chip_clock(unsigned int frequency)
 {
struct pll_value pll;
-   unsigned int actual_mx_clk;

/* Cheok_0509: For SM750LE, the chip clock is fixed. Nothing to set. */
if (sm750_get_chip_type() == SM750LE)
@@ -76,7 +75,7 @@ static void set_chip_clock(unsigned int frequency)
 * Return value of sm750_calc_pll_value gives the actual
 * possible clock.
 */
-   actual_mx_clk = sm750_calc_pll_value(frequency, &pll);
+   sm750_calc_pll_value(frequency, &pll);

/* Master Clock Control: MXCLK_PLL */
poke32(MXCLK_PLL_CTRL, sm750_format_pll_reg(&pll));
--
2.7.4

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

[PATCH 6/6] staging: comedi: Remove set but not used variable 'aref'

2019-10-07 Thread zhengbin 
Fixes gcc '-Wunused-but-set-variable' warning:

drivers/staging/comedi/drivers/dt3000.c: In function dt3k_ai_insn_read:
drivers/staging/comedi/drivers/dt3000.c:511:27: warning: variable aref set but 
not used [-Wunused-but-set-variable]

It is not used since commit 2e310235ca8f ("staging:
comedi: dt3000: rename dt3k_ai_insn()")

Reported-by: Hulk Robot 
Signed-off-by: zhengbin 
---
 drivers/staging/comedi/drivers/dt3000.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt3000.c 
b/drivers/staging/comedi/drivers/dt3000.c
index caf4d4d..f7c365b 100644
--- a/drivers/staging/comedi/drivers/dt3000.c
+++ b/drivers/staging/comedi/drivers/dt3000.c
@@ -508,12 +508,11 @@ static int dt3k_ai_insn_read(struct comedi_device *dev,
 unsigned int *data)
 {
int i;
-   unsigned int chan, gain, aref;
+   unsigned int chan, gain;

chan = CR_CHAN(insn->chanspec);
gain = CR_RANGE(insn->chanspec);
/* XXX docs don't explain how to select aref */
-   aref = CR_AREF(insn->chanspec);

for (i = 0; i < insn->n; i++)
data[i] = dt3k_readsingle(dev, DPR_SUBSYS_AI, chan, gain);
--
2.7.4

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

[PATCH 5/6] staging: comedi: Remove set but not used variable 'hi'

2019-10-07 Thread zhengbin 
Fixes gcc '-Wunused-but-set-variable' warning:

drivers/staging/comedi/drivers/dt2815.c: In function dt2815_ao_insn:
drivers/staging/comedi/drivers/dt2815.c:91:19: warning: variable hi set but not 
used [-Wunused-but-set-variable]

It is not used since commit d6a929b7608a ("Staging:
comedi: add dt2815 driver")

Reported-by: Hulk Robot 
Signed-off-by: zhengbin 
---
 drivers/staging/comedi/drivers/dt2815.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2815.c 
b/drivers/staging/comedi/drivers/dt2815.c
index 83026ba..bcf85ec 100644
--- a/drivers/staging/comedi/drivers/dt2815.c
+++ b/drivers/staging/comedi/drivers/dt2815.c
@@ -88,12 +88,11 @@ static int dt2815_ao_insn(struct comedi_device *dev, struct 
comedi_subdevice *s,
struct dt2815_private *devpriv = dev->private;
int i;
int chan = CR_CHAN(insn->chanspec);
-   unsigned int lo, hi;
+   unsigned int lo;
int ret;

for (i = 0; i < insn->n; i++) {
lo = ((data[i] & 0x0f) << 4) | (chan << 1) | 0x01;
-   hi = (data[i] & 0xff0) >> 4;

ret = comedi_timeout(dev, s, insn, dt2815_ao_status, 0x00);
if (ret)
--
2.7.4

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

[PATCH 1/6] staging: bcm2835-audio: Remove set but not used variable 'status'

2019-10-07 Thread zhengbin 
Fixes gcc '-Wunused-but-set-variable' warning:

drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c: In function 
audio_vchi_callback:
drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c:99:6: warning: 
variable status set but not used [-Wunused-but-set-variable]

It is not used since commit 23b028c871e1 ("staging:
bcm2835-audio: initial staging submission")

Reported-by: Hulk Robot 
Signed-off-by: zhengbin 
---
 drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c 
b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index c6f9cf1..8a94c5b 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -96,13 +96,12 @@ static void audio_vchi_callback(void *param,
struct bcm2835_audio_instance *instance = param;
struct vc_audio_msg m;
int msg_len;
-   int status;

if (reason != VCHI_CALLBACK_MSG_AVAILABLE)
return;

-   status = vchi_msg_dequeue(instance->vchi_handle,
- &m, sizeof(m), &msg_len, VCHI_FLAGS_NONE);
+   vchi_msg_dequeue(instance->vchi_handle,
+&m, sizeof(m), &msg_len, VCHI_FLAGS_NONE);
if (m.type == VC_AUDIO_MSG_TYPE_RESULT) {
instance->result = m.result.success;
complete(&instance->msg_avail_comp);
--
2.7.4

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

[PATCH 4/6] staging: comedi: Remove set but not used variable 'data'

2019-10-07 Thread zhengbin 
Fixes gcc '-Wunused-but-set-variable' warning:

drivers/staging/comedi/drivers/dt2814.c: In function dt2814_interrupt:
drivers/staging/comedi/drivers/dt2814.c:193:6: warning: variable data set but 
not used [-Wunused-but-set-variable]

It is not used since commit 7806012e97ed ("staging:
comedi: refactor dt2814 driver and use module_comedi_driver")

Reported-by: Hulk Robot 
Signed-off-by: zhengbin 
---
 drivers/staging/comedi/drivers/dt2814.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2814.c 
b/drivers/staging/comedi/drivers/dt2814.c
index d2c7157..4825168 100644
--- a/drivers/staging/comedi/drivers/dt2814.c
+++ b/drivers/staging/comedi/drivers/dt2814.c
@@ -190,7 +190,6 @@ static irqreturn_t dt2814_interrupt(int irq, void *d)
struct comedi_device *dev = d;
struct dt2814_private *devpriv = dev->private;
struct comedi_subdevice *s = dev->read_subdev;
-   int data;

if (!dev->attached) {
dev_err(dev->class_dev, "spurious interrupt\n");
@@ -200,8 +199,6 @@ static irqreturn_t dt2814_interrupt(int irq, void *d)
hi = inb(dev->iobase + DT2814_DATA);
lo = inb(dev->iobase + DT2814_DATA);

-   data = (hi << 4) | (lo >> 4);
-
if (!(--devpriv->ntrig)) {
int i;

--
2.7.4

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

[PATCH 0/6] staging: Remove a bounch set of set but not used variables

2019-10-07 Thread zhengbin 
zhengbin (6):
  staging: bcm2835-audio: Remove set but not used variable 'status'
  staging: sm750fb: Remove set but not used variable 'uiActualPixelClk'
  staging: sm750fb: Remove set but not used variable 'actual_mx_clk'
  staging: comedi: Remove set but not used variable 'data'
  staging: comedi: Remove set but not used variable 'hi'
  staging: comedi: Remove set but not used variable 'aref'

 drivers/staging/comedi/drivers/dt2814.c | 3 ---
 drivers/staging/comedi/drivers/dt2815.c | 3 +--
 drivers/staging/comedi/drivers/dt3000.c | 3 +--
 drivers/staging/sm750fb/ddk750_chip.c   | 3 +--
 drivers/staging/sm750fb/ddk750_mode.c   | 3 +--
 drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 5 ++---
 6 files changed, 6 insertions(+), 14 deletions(-)

--
2.7.4

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

[staging:staging-next 50/93] drivers/staging/wfx/debug.c:112 wfx_send_hif_msg_read() warn: maybe return -EFAULT instead of the bytes remaining?

2019-10-07 Thread Dan Carpenter 
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 
staging-next
head:   e772cd8c9c9cd3d08715800aabaf50b771b395d9
commit: 4f8b7fabb15df3658564a98971fc67029be1815d [50/93] staging: wfx: allow to 
send commands to chip

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot 
Reported-by: Dan Carpenter 

smatch warnings:
drivers/staging/wfx/debug.c:112 wfx_send_hif_msg_read() warn: maybe return 
-EFAULT instead of the bytes remaining?

# 
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?id=4f8b7fabb15df3658564a98971fc67029be1815d
git remote add staging 
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
git remote update staging
git checkout 4f8b7fabb15df3658564a98971fc67029be1815d
vim +112 drivers/staging/wfx/debug.c

4f8b7fabb15df3 Jérôme Pouiller 2019-09-19   95  static ssize_t 
wfx_send_hif_msg_read(struct file *file, char __user *user_buf,
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19   96  
 size_t count, loff_t *ppos)
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19   97  {
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19   98  struct dbgfs_hif_msg 
*context = file->private_data;
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19   99  int ret;
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  100  
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  101  if (count > 
sizeof(context->reply))
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  102  return -EINVAL;
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  103  ret = 
wait_for_completion_interruptible(&context->complete);
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  104  if (ret)
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  105  return ret;
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  106  if (context->ret < 0)
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  107  return 
context->ret;
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  108  // Be carefull, write() 
is waiting for a full message while read()
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  109  // only return a payload
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  110  ret = 
copy_to_user(user_buf, context->reply, count);
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  111  if (ret)
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19 @112  return ret;

Yeah.  It should be:

if (copy_to_user(user_buf, context->reply, count))
return -EFAULT;

4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  113  
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  114  return count;
4f8b7fabb15df3 Jérôme Pouiller 2019-09-19  115  }

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: octeon: Remove typedef declaration

2019-10-07 Thread Wambui Karuga 
Fixes checkpatch.pl warning: do not add new typedefs in
drivers/staging/octeon/octeon-stubs.h:41

Signed-off-by: Wambui Karuga 
---
 drivers/staging/octeon/octeon-stubs.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/octeon/octeon-stubs.h 
b/drivers/staging/octeon/octeon-stubs.h
index a4ac3bfb62a8..773591348ef4 100644
--- a/drivers/staging/octeon/octeon-stubs.h
+++ b/drivers/staging/octeon/octeon-stubs.h
@@ -38,7 +38,7 @@
 #define CVMX_NPI_RSL_INT_BLOCKS0
 #define CVMX_POW_WQ_INT_PC 0
 
-typedef union {
+union cvmx_pip_wqe_word2 {
uint64_t u64;
struct {
uint64_t bufs:8;
@@ -114,7 +114,7 @@ typedef union {
uint64_t err_code:8;
} snoip;
 
-} cvmx_pip_wqe_word2;
+};
 
 union cvmx_pip_wqe_word0 {
struct {
@@ -183,7 +183,7 @@ union cvmx_buf_ptr {
 typedef struct {
union cvmx_wqe_word0 word0;
union cvmx_wqe_word1 word1;
-   cvmx_pip_wqe_word2 word2;
+   union cvmx_pip_wqe_word2 word2;
union cvmx_buf_ptr packet_ptr;
uint8_t packet_data[96];
 } cvmx_wqe_t;
-- 
2.23.0

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

[PATCH] staging/octeon: Use stubs for MIPS && !CAVIUM_OCTEON_SOC

2019-10-07 Thread Paul Burton 
When building for a non-Cavium MIPS system with COMPILE_TEST=y, the
Octeon ethernet driver hits a number of issues due to use of macros
provided only for CONFIG_CAVIUM_OCTEON_SOC=y configurations. For
example:

  drivers/staging/octeon/ethernet-rx.c:190:6: error:
'CONFIG_CAVIUM_OCTEON_CVMSEG_SIZE' undeclared (first use in this function)
  drivers/staging/octeon/ethernet-rx.c:472:25: error:
'OCTEON_IRQ_WORKQ0' undeclared (first use in this function)

These come from various asm/ headers that a non-Octeon build will be
using a non-Octeon version of.

Fix this by using the octeon-stubs.h header for non-Cavium MIPS builds,
and only using the real asm/octeon/ headers when building a Cavium
Octeon kernel configuration.

This requires that octeon-stubs.h doesn't redefine XKPHYS_TO_PHYS, which
is defined for MIPS by asm/addrspace.h which is pulled in by many other
common asm/ headers.

Signed-off-by: Paul Burton 
Reported-by: Geert Uytterhoeven 
URL: 
https://lore.kernel.org/linux-mips/CAMuHMdXvu+BppwzsU9imNWVKea_hoLcRt9N+a29Q-QsjW=i...@mail.gmail.com/
Fixes: 171a9bae68c7 ("staging/octeon: Allow test build on !MIPS")
Cc: Matthew Wilcox (Oracle) 
Cc: Greg Kroah-Hartman 
Cc: David S. Miller 

---

 drivers/staging/octeon/octeon-ethernet.h | 2 +-
 drivers/staging/octeon/octeon-stubs.h| 5 -
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/octeon/octeon-ethernet.h 
b/drivers/staging/octeon/octeon-ethernet.h
index a8a864b40913..042220d86d33 100644
--- a/drivers/staging/octeon/octeon-ethernet.h
+++ b/drivers/staging/octeon/octeon-ethernet.h
@@ -14,7 +14,7 @@
 #include 
 #include 
 
-#ifdef CONFIG_MIPS
+#ifdef CONFIG_CAVIUM_OCTEON_SOC
 
 #include 
 
diff --git a/drivers/staging/octeon/octeon-stubs.h 
b/drivers/staging/octeon/octeon-stubs.h
index a4ac3bfb62a8..c7ff90207f8a 100644
--- a/drivers/staging/octeon/octeon-stubs.h
+++ b/drivers/staging/octeon/octeon-stubs.h
@@ -1,5 +1,8 @@
 #define CONFIG_CAVIUM_OCTEON_CVMSEG_SIZE   512
-#define XKPHYS_TO_PHYS(p)  (p)
+
+#ifndef XKPHYS_TO_PHYS
+# define XKPHYS_TO_PHYS(p) (p)
+#endif
 
 #define OCTEON_IRQ_WORKQ0 0
 #define OCTEON_IRQ_RML 0
-- 
2.23.0

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

[PATCH v2] staging: rtl8723bs: Remove set but not used variable 'i'

2019-10-07 Thread zhengbin 
Fixes gcc '-Wunused-but-set-variable' warning:

drivers/staging/rtl8723bs/core/rtw_xmit.c: In function update_attrib:
drivers/staging/rtl8723bs/core/rtw_xmit.c:680:7: warning: variable i set but 
not used [-Wunused-but-set-variable]

It is not used since commit 554c0a3abf21 ("staging:
Add rtl8723bs sdio wifi driver")

Reported-by: Hulk Robot 
Signed-off-by: zhengbin 
---
 drivers/staging/rtl8723bs/core/rtw_xmit.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c 
b/drivers/staging/rtl8723bs/core/rtw_xmit.c
index b5dcb78..c50d05e 100644
--- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
+++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
@@ -675,7 +675,6 @@ static void set_qos(struct pkt_file *ppktfile, struct 
pkt_attrib *pattrib)

 static s32 update_attrib(struct adapter *padapter, _pkt *pkt, struct 
pkt_attrib *pattrib)
 {
-   uint i;
struct pkt_file pktfile;
struct sta_info *psta = NULL;
struct ethhdr etherhdr;
@@ -689,7 +688,7 @@ static s32 update_attrib(struct adapter *padapter, _pkt 
*pkt, struct pkt_attrib
DBG_COUNTER(padapter->tx_logs.core_tx_upd_attrib);

_rtw_open_pktfile(pkt, &pktfile);
-   i = _rtw_pktfile_read(&pktfile, (u8 *)ðerhdr, ETH_HLEN);
+   _rtw_pktfile_read(&pktfile, (u8 *)ðerhdr, ETH_HLEN);

pattrib->ether_type = ntohs(etherhdr.h_proto);

--
2.7.4

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

[PATCH v2] staging: vc04_services: Avoid NULL comparison

2019-10-07 Thread Nachammai Karuppiah 
Remove NULL comparison. Issue found using checkpatch.pl

Signed-off-by: Nachammai Karuppiah 

---

Changes in V2
   - Remove all NULL comparisons in vc04_services/interface directory.
---
 .../interface/vchiq_arm/vchiq_2835_arm.c   |  4 ++--
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 28 +++---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c |  4 ++--
 .../vc04_services/interface/vchiq_arm/vchiq_shim.c |  2 +-
 4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 8dc730c..7cdb21e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -526,11 +526,11 @@ create_pagelist(char __user *buf, size_t count, unsigned 
short type)
return NULL;
}
 
-   WARN_ON(g_free_fragments == NULL);
+   WARN_ON(!g_free_fragments);
 
down(&g_free_fragments_mutex);
fragments = g_free_fragments;
-   WARN_ON(fragments == NULL);
+   WARN_ON(!fragments);
g_free_fragments = *(char **) g_free_fragments;
up(&g_free_fragments_mutex);
pagelist->type = PAGELIST_READ_WITH_FRAGMENTS +
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index b1595b1..b930148 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -826,8 +826,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned 
long arg)
 
/* Remove all services */
i = 0;
-   while ((service = next_service_by_instance(instance->state,
-   instance, &i)) != NULL) {
+   while (service = next_service_by_instance(instance->state,
+   instance, &i)) {
status = vchiq_remove_service(service->handle);
unlock_service(service);
if (status != VCHIQ_SUCCESS)
@@ -907,7 +907,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned 
long arg)
&args.params, srvstate,
instance, user_service_free);
 
-   if (service != NULL) {
+   if (service) {
user_service->service = service;
user_service->userdata = userdata;
user_service->instance = instance;
@@ -988,7 +988,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned 
long arg)
VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
 
service = find_service_for_instance(instance, handle);
-   if (service != NULL) {
+   if (service) {
status = (cmd == VCHIQ_IOC_USE_SERVICE) ?
vchiq_use_service_internal(service) :
vchiq_release_service_internal(service);
@@ -1021,7 +1021,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned 
long arg)
 
service = find_service_for_instance(instance, args.handle);
 
-   if ((service != NULL) && (args.count <= MAX_ELEMENTS)) {
+   if (service && (args.count <= MAX_ELEMENTS)) {
/* Copy elements into kernel space */
struct vchiq_element elements[MAX_ELEMENTS];
 
@@ -1343,11 +1343,11 @@ vchiq_ioctl(struct file *file, unsigned int cmd, 
unsigned long arg)
spin_unlock(&msg_queue_spinlock);
 
complete(&user_service->remove_event);
-   if (header == NULL)
+   if (!header)
ret = -ENOTCONN;
else if (header->size <= args.bufsize) {
/* Copy to user space if msgbuf is not NULL */
-   if ((args.buf == NULL) ||
+   if (!args.buf ||
(copy_to_user((void __user *)args.buf,
header->data,
header->size) == 0)) {
@@ -1426,7 +1426,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned 
long arg)
VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
 
service = find_closed_service_for_instance(instance, handle);
-   if (service != NULL) {
+   if (service) {
struct user_service *user_service =
(struct user_service *)service->base.userdata;
close_delivered(user_service);
@@ -2223,13 +2223,13 @@ struct vchiq_state *
 vchiq_get_state(void)
 {
 
-

Re: UAF read in print_binder_transaction_log_entry() on ANDROID_BINDERFS kernels

2019-10-07 Thread Hridya Valsaraju 
Thank you for letting us know about the issue Jann. I will work on a
fix and send out the same for review once ready.

Regards,
Hridya


On Mon, Oct 7, 2019 at 2:04 PM Todd Kjos  wrote:
>
> +Hridya Valsaraju
>
>
> On Mon, Oct 7, 2019 at 1:50 PM Jann Horn  wrote:
> >
> > Hi!
> >
> > There is a use-after-free read in print_binder_transaction_log_entry()
> > on ANDROID_BINDERFS kernels because
> > print_binder_transaction_log_entry() prints the char* e->context_name
> > as string, and if the transaction occurred on a binder device from
> > binderfs, e->context_name belongs to the binder device and is freed
> > when the inode disappears.
> >
> > Luckily this shouldn't have security implications, since:
> >
> > a) reading the binder transaction log is already a pretty privileged 
> > operation
> > b) I can't find any actual users of ANDROID_BINDERFS
> >
> > I guess there are three ways to fix it:
> > 1) Create a new shared global spinlock for binderfs_evict_inode() and
> > binder_transaction_log_show(), and let binderfs_evict_inode() scan the
> > transaction log for pointers to its name and replace them with
> > pointers to a statically-allocated string "{DELETED}" or something
> > like that.
> > 2) Let the transaction log contain non-reusable device identifiers
> > instead of name pointers, and let print_binder_transaction_log_entry()
> > look them up in something like a hashtable.
> > 3) Just copy the name into the transaction log every time.
> >
> > I'm not sure which one is better, or whether there's a nicer fourth
> > option, so I'm leaving writing a patch for this to y'all.
> >
> >
> > Trigger instructions (requires you to have some helpers that can
> > register a context manager and send some transaction to it):
> > ==
> > root@test:/home/user# mkdir /tmp/binder
> > root@test:/home/user# mount -t binder -o stats=global /dev/null /tmp/binder
> > root@test:/home/user# ls -l /tmp/binder
> > total 0
> > crw--- 1 root root 248, 1 Oct  7 20:34 binder
> > crw--- 1 root root 248, 0 Oct  7 20:34 binder-control
> > drwxr-xr-x 3 root root  0 Oct  7 20:34 binder_logs
> > crw--- 1 root root 248, 2 Oct  7 20:34 hwbinder
> > crw--- 1 root root 248, 3 Oct  7 20:34 vndbinder
> > root@test:/home/user# ln -s /tmp/binder/binder /dev/binder
> > [run some simple binder demo code to temporarily register a context
> > manager and send a binder transaction]
> > root@test:/home/user# rm /tmp/binder/binder
> > root@test:/home/user# cat /tmp/binder/binder_logs/transaction_log
> > 2: call  from 2277:2277 to 2273:0 context @��� node 1 handle 0
> > size 24:8 ret 0/0 l=0
> > 5: call  from 2273:2273 to 2277:2277 context @��� node 3 handle 1
> > size 0:0 ret 0/0 l=0
> > 6: reply from 2277:2277 to 2273:2273 context @��� node 0 handle 0
> > size 4:0 ret 0/0 l=0
> > 7: reply from 2273:2273 to 2277:2277 context @��� node 0 handle 0
> > size 4:0 ret 0/0 l=0
> > root@test:/home/user#
> > ==
> >
> > ASAN splat:
> > [  333.300753] 
> > ==
> > [  333.303197] BUG: KASAN: use-after-free in string_nocheck+0x9d/0x160
> > [  333.305081] Read of size 1 at addr 8880b0981258 by task cat/2279
> >
> > [  333.307415] CPU: 1 PID: 2279 Comm: cat Not tainted 5.4.0-rc1+ #513
> > [  333.309304] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> > BIOS 1.12.0-1 04/01/2014
> > [  333.310987] Call Trace:
> > [  333.312032]  dump_stack+0x7c/0xc0
> > [  333.312581]  ? string_nocheck+0x9d/0x160
> > [  333.313157]  print_address_description.constprop.7+0x36/0x50
> > [  333.314030]  ? string_nocheck+0x9d/0x160
> > [  333.314603]  ? string_nocheck+0x9d/0x160
> > [  333.315236]  __kasan_report.cold.10+0x1a/0x35
> > [  333.315972]  ? string_nocheck+0x9d/0x160
> > [  333.316545]  kasan_report+0xe/0x20
> > [  333.317104]  string_nocheck+0x9d/0x160
> > [  333.317652]  ? widen_string+0x160/0x160
> > [  333.318270]  ? string_nocheck+0x160/0x160
> > [  333.318857]  ? unwind_get_return_address+0x2a/0x40
> > [  333.319636]  ? profile_setup.cold.9+0x96/0x96
> > [  333.320359]  string+0xb6/0xc0
> > [  333.320800]  ? hex_string+0x280/0x280
> > [  333.321398]  vsnprintf+0x20c/0x780
> > [  333.321898]  ? num_to_str+0x180/0x180
> > [  333.322503]  ? __kasan_kmalloc.constprop.6+0xc1/0xd0
> > [  333.323235]  ? vfs_read+0xbc/0x1e0
> > [  333.323814]  ? ksys_read+0xb5/0x150
> > [  333.324323]  ? do_syscall_64+0xb9/0x3b0
> > [  333.324948]  ? entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > [  333.325756]  seq_vprintf+0x78/0xb0
> > [  333.326253]  seq_printf+0x96/0xc0
> > [  333.327132]  ? seq_vprintf+0xb0/0xb0
> > [  333.327678]  ? match_held_lock+0x2e/0x240
> > [  333.328450]  binder_transaction_log_show+0x237/0x2d0
> > [  333.329163]  seq_read+0x266/0x690
> > [  333.329705]  vfs_read+0xbc/0x1e0
> > [  333.330178]  ksys_read+0xb5/0x150
> > [  333.330724]  ? kernel_write+0xb0/0xb0
> > [  333.331257]  ? trace_hardirqs_off_caller+0x57/0x130
> > [  333.332045]  ? ma

Re: UAF read in print_binder_transaction_log_entry() on ANDROID_BINDERFS kernels

2019-10-07 Thread Christian Brauner 
On Mon, Oct 07, 2019 at 10:49:57PM +0200, Jann Horn wrote:
> Hi!
> 
> There is a use-after-free read in print_binder_transaction_log_entry()
> on ANDROID_BINDERFS kernels because
> print_binder_transaction_log_entry() prints the char* e->context_name
> as string, and if the transaction occurred on a binder device from
> binderfs, e->context_name belongs to the binder device and is freed
> when the inode disappears.
> 
> Luckily this shouldn't have security implications, since:
> 
> a) reading the binder transaction log is already a pretty privileged operation
> b) I can't find any actual users of ANDROID_BINDERFS
> 
> I guess there are three ways to fix it:
> 1) Create a new shared global spinlock for binderfs_evict_inode() and
> binder_transaction_log_show(), and let binderfs_evict_inode() scan the
> transaction log for pointers to its name and replace them with
> pointers to a statically-allocated string "{DELETED}" or something
> like that.
> 2) Let the transaction log contain non-reusable device identifiers
> instead of name pointers, and let print_binder_transaction_log_entry()
> look them up in something like a hashtable.
> 3) Just copy the name into the transaction log every time.
> 
> I'm not sure which one is better, or whether there's a nicer fourth
> option, so I'm leaving writing a patch for this to y'all.

Moin,

Thanks for the report.
Android binderfs is enabled by default on Ubuntu and - iirc - Debian
kernels. It is actively used by Anbox to run Android in containers.

The codepath you're referring to is specific to the stats=global mount
option. This was contributed by the Android team for the 5.4 cycle (cf.
[1]). That means there is no released kernel that supports the
stats=global mount option. So all current users cannot be affected by
this bug.

I'll take a look at this tomorrow and see what makes the most sense. I
agree that this is not a security issue. Thanks for catching this early.

If you already have a script that trivially reproduces the bug it'd be
nice if you could paste it. Otherwise we can just add a reproducer based
on your snippet from below. I want to add a regression test for this.

[1]: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f00834518ed3194b866f5f3d63b71e0ed7f6bc00
 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0e13e452dafc009049a9a5a4153e2f9e51b23915
 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=03e2e07e38147917482d595ad3cf193212ded8ac
 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4feb80faf428a02d407a9ea1952004af01308765

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

Re: UAF read in print_binder_transaction_log_entry() on ANDROID_BINDERFS kernels

2019-10-07 Thread Todd Kjos 
+Hridya Valsaraju


On Mon, Oct 7, 2019 at 1:50 PM Jann Horn  wrote:
>
> Hi!
>
> There is a use-after-free read in print_binder_transaction_log_entry()
> on ANDROID_BINDERFS kernels because
> print_binder_transaction_log_entry() prints the char* e->context_name
> as string, and if the transaction occurred on a binder device from
> binderfs, e->context_name belongs to the binder device and is freed
> when the inode disappears.
>
> Luckily this shouldn't have security implications, since:
>
> a) reading the binder transaction log is already a pretty privileged operation
> b) I can't find any actual users of ANDROID_BINDERFS
>
> I guess there are three ways to fix it:
> 1) Create a new shared global spinlock for binderfs_evict_inode() and
> binder_transaction_log_show(), and let binderfs_evict_inode() scan the
> transaction log for pointers to its name and replace them with
> pointers to a statically-allocated string "{DELETED}" or something
> like that.
> 2) Let the transaction log contain non-reusable device identifiers
> instead of name pointers, and let print_binder_transaction_log_entry()
> look them up in something like a hashtable.
> 3) Just copy the name into the transaction log every time.
>
> I'm not sure which one is better, or whether there's a nicer fourth
> option, so I'm leaving writing a patch for this to y'all.
>
>
> Trigger instructions (requires you to have some helpers that can
> register a context manager and send some transaction to it):
> ==
> root@test:/home/user# mkdir /tmp/binder
> root@test:/home/user# mount -t binder -o stats=global /dev/null /tmp/binder
> root@test:/home/user# ls -l /tmp/binder
> total 0
> crw--- 1 root root 248, 1 Oct  7 20:34 binder
> crw--- 1 root root 248, 0 Oct  7 20:34 binder-control
> drwxr-xr-x 3 root root  0 Oct  7 20:34 binder_logs
> crw--- 1 root root 248, 2 Oct  7 20:34 hwbinder
> crw--- 1 root root 248, 3 Oct  7 20:34 vndbinder
> root@test:/home/user# ln -s /tmp/binder/binder /dev/binder
> [run some simple binder demo code to temporarily register a context
> manager and send a binder transaction]
> root@test:/home/user# rm /tmp/binder/binder
> root@test:/home/user# cat /tmp/binder/binder_logs/transaction_log
> 2: call  from 2277:2277 to 2273:0 context @��� node 1 handle 0
> size 24:8 ret 0/0 l=0
> 5: call  from 2273:2273 to 2277:2277 context @��� node 3 handle 1
> size 0:0 ret 0/0 l=0
> 6: reply from 2277:2277 to 2273:2273 context @��� node 0 handle 0
> size 4:0 ret 0/0 l=0
> 7: reply from 2273:2273 to 2277:2277 context @��� node 0 handle 0
> size 4:0 ret 0/0 l=0
> root@test:/home/user#
> ==
>
> ASAN splat:
> [  333.300753] 
> ==
> [  333.303197] BUG: KASAN: use-after-free in string_nocheck+0x9d/0x160
> [  333.305081] Read of size 1 at addr 8880b0981258 by task cat/2279
>
> [  333.307415] CPU: 1 PID: 2279 Comm: cat Not tainted 5.4.0-rc1+ #513
> [  333.309304] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.12.0-1 04/01/2014
> [  333.310987] Call Trace:
> [  333.312032]  dump_stack+0x7c/0xc0
> [  333.312581]  ? string_nocheck+0x9d/0x160
> [  333.313157]  print_address_description.constprop.7+0x36/0x50
> [  333.314030]  ? string_nocheck+0x9d/0x160
> [  333.314603]  ? string_nocheck+0x9d/0x160
> [  333.315236]  __kasan_report.cold.10+0x1a/0x35
> [  333.315972]  ? string_nocheck+0x9d/0x160
> [  333.316545]  kasan_report+0xe/0x20
> [  333.317104]  string_nocheck+0x9d/0x160
> [  333.317652]  ? widen_string+0x160/0x160
> [  333.318270]  ? string_nocheck+0x160/0x160
> [  333.318857]  ? unwind_get_return_address+0x2a/0x40
> [  333.319636]  ? profile_setup.cold.9+0x96/0x96
> [  333.320359]  string+0xb6/0xc0
> [  333.320800]  ? hex_string+0x280/0x280
> [  333.321398]  vsnprintf+0x20c/0x780
> [  333.321898]  ? num_to_str+0x180/0x180
> [  333.322503]  ? __kasan_kmalloc.constprop.6+0xc1/0xd0
> [  333.323235]  ? vfs_read+0xbc/0x1e0
> [  333.323814]  ? ksys_read+0xb5/0x150
> [  333.324323]  ? do_syscall_64+0xb9/0x3b0
> [  333.324948]  ? entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [  333.325756]  seq_vprintf+0x78/0xb0
> [  333.326253]  seq_printf+0x96/0xc0
> [  333.327132]  ? seq_vprintf+0xb0/0xb0
> [  333.327678]  ? match_held_lock+0x2e/0x240
> [  333.328450]  binder_transaction_log_show+0x237/0x2d0
> [  333.329163]  seq_read+0x266/0x690
> [  333.329705]  vfs_read+0xbc/0x1e0
> [  333.330178]  ksys_read+0xb5/0x150
> [  333.330724]  ? kernel_write+0xb0/0xb0
> [  333.331257]  ? trace_hardirqs_off_caller+0x57/0x130
> [  333.332045]  ? mark_held_locks+0x29/0xa0
> [  333.332678]  ? do_syscall_64+0x6b/0x3b0
> [  333.333235]  do_syscall_64+0xb9/0x3b0
> [  333.333856]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [  333.334635] RIP: 0033:0x7fbbb95d4461
> [  333.335153] Code: fe ff ff 50 48 8d 3d fe d0 09 00 e8 e9 03 02 00
> 66 0f 1f 84 00 00 00 00 00 48 8d 05 99 62 0d 00 8b 00 85 c0 75 13 31
> c0 0f 05 <48> 3d 00 f0 ff ff 77 57 c3 66 0f

Re: [Outreachy kernel] [PATCH] staging: rtl8712: Add comment to lock declaration

2019-10-07 Thread Julia Lawall 



On Mon, 7 Oct 2019, Jules Irenge wrote:

> Add comment to spinlock declaration to fix warning issued by checkpatch.pl
> "CHECK: spinlock_t definition without comment".

Since it is not apparent from the shown code, it would be helpful to
describe what evidence you used to choose this comment.

thanks,
julia

>
> Signed-off-by: Jules Irenge 
> ---
>  drivers/staging/rtl8712/drv_types.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8712/drv_types.h 
> b/drivers/staging/rtl8712/drv_types.h
> index 0c4325073c63..960d8709aada 100644
> --- a/drivers/staging/rtl8712/drv_types.h
> +++ b/drivers/staging/rtl8712/drv_types.h
> @@ -160,7 +160,7 @@ struct _adapter {
>   int pid; /*process id from UI*/
>   struct work_struct wk_filter_rx_ff0;
>   u8 blnEnableRxFF0Filter;
> - spinlock_t lock_rx_ff0_filter;
> + spinlock_t lock_rx_ff0_filter; /*spinlock to protect interrupt request*/
>   const struct firmware *fw;
>   struct usb_interface *pusb_intf;
>   struct mutex mutex_start;
> --
> 2.21.0
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20191007205248.24466-1-jbi.octave%40gmail.com.
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: rtl8712: Add comment to lock declaration

2019-10-07 Thread Jules Irenge 
Add comment to spinlock declaration to fix warning issued by checkpatch.pl
"CHECK: spinlock_t definition without comment".

Signed-off-by: Jules Irenge 
---
 drivers/staging/rtl8712/drv_types.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8712/drv_types.h 
b/drivers/staging/rtl8712/drv_types.h
index 0c4325073c63..960d8709aada 100644
--- a/drivers/staging/rtl8712/drv_types.h
+++ b/drivers/staging/rtl8712/drv_types.h
@@ -160,7 +160,7 @@ struct _adapter {
int pid; /*process id from UI*/
struct work_struct wk_filter_rx_ff0;
u8 blnEnableRxFF0Filter;
-   spinlock_t lock_rx_ff0_filter;
+   spinlock_t lock_rx_ff0_filter; /*spinlock to protect interrupt request*/
const struct firmware *fw;
struct usb_interface *pusb_intf;
struct mutex mutex_start;
-- 
2.21.0

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

UAF read in print_binder_transaction_log_entry() on ANDROID_BINDERFS kernels

2019-10-07 Thread Jann Horn 
Hi!

There is a use-after-free read in print_binder_transaction_log_entry()
on ANDROID_BINDERFS kernels because
print_binder_transaction_log_entry() prints the char* e->context_name
as string, and if the transaction occurred on a binder device from
binderfs, e->context_name belongs to the binder device and is freed
when the inode disappears.

Luckily this shouldn't have security implications, since:

a) reading the binder transaction log is already a pretty privileged operation
b) I can't find any actual users of ANDROID_BINDERFS

I guess there are three ways to fix it:
1) Create a new shared global spinlock for binderfs_evict_inode() and
binder_transaction_log_show(), and let binderfs_evict_inode() scan the
transaction log for pointers to its name and replace them with
pointers to a statically-allocated string "{DELETED}" or something
like that.
2) Let the transaction log contain non-reusable device identifiers
instead of name pointers, and let print_binder_transaction_log_entry()
look them up in something like a hashtable.
3) Just copy the name into the transaction log every time.

I'm not sure which one is better, or whether there's a nicer fourth
option, so I'm leaving writing a patch for this to y'all.


Trigger instructions (requires you to have some helpers that can
register a context manager and send some transaction to it):
==
root@test:/home/user# mkdir /tmp/binder
root@test:/home/user# mount -t binder -o stats=global /dev/null /tmp/binder
root@test:/home/user# ls -l /tmp/binder
total 0
crw--- 1 root root 248, 1 Oct  7 20:34 binder
crw--- 1 root root 248, 0 Oct  7 20:34 binder-control
drwxr-xr-x 3 root root  0 Oct  7 20:34 binder_logs
crw--- 1 root root 248, 2 Oct  7 20:34 hwbinder
crw--- 1 root root 248, 3 Oct  7 20:34 vndbinder
root@test:/home/user# ln -s /tmp/binder/binder /dev/binder
[run some simple binder demo code to temporarily register a context
manager and send a binder transaction]
root@test:/home/user# rm /tmp/binder/binder
root@test:/home/user# cat /tmp/binder/binder_logs/transaction_log
2: call  from 2277:2277 to 2273:0 context @��� node 1 handle 0
size 24:8 ret 0/0 l=0
5: call  from 2273:2273 to 2277:2277 context @��� node 3 handle 1
size 0:0 ret 0/0 l=0
6: reply from 2277:2277 to 2273:2273 context @��� node 0 handle 0
size 4:0 ret 0/0 l=0
7: reply from 2273:2273 to 2277:2277 context @��� node 0 handle 0
size 4:0 ret 0/0 l=0
root@test:/home/user#
==

ASAN splat:
[  333.300753] 
==
[  333.303197] BUG: KASAN: use-after-free in string_nocheck+0x9d/0x160
[  333.305081] Read of size 1 at addr 8880b0981258 by task cat/2279

[  333.307415] CPU: 1 PID: 2279 Comm: cat Not tainted 5.4.0-rc1+ #513
[  333.309304] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.12.0-1 04/01/2014
[  333.310987] Call Trace:
[  333.312032]  dump_stack+0x7c/0xc0
[  333.312581]  ? string_nocheck+0x9d/0x160
[  333.313157]  print_address_description.constprop.7+0x36/0x50
[  333.314030]  ? string_nocheck+0x9d/0x160
[  333.314603]  ? string_nocheck+0x9d/0x160
[  333.315236]  __kasan_report.cold.10+0x1a/0x35
[  333.315972]  ? string_nocheck+0x9d/0x160
[  333.316545]  kasan_report+0xe/0x20
[  333.317104]  string_nocheck+0x9d/0x160
[  333.317652]  ? widen_string+0x160/0x160
[  333.318270]  ? string_nocheck+0x160/0x160
[  333.318857]  ? unwind_get_return_address+0x2a/0x40
[  333.319636]  ? profile_setup.cold.9+0x96/0x96
[  333.320359]  string+0xb6/0xc0
[  333.320800]  ? hex_string+0x280/0x280
[  333.321398]  vsnprintf+0x20c/0x780
[  333.321898]  ? num_to_str+0x180/0x180
[  333.322503]  ? __kasan_kmalloc.constprop.6+0xc1/0xd0
[  333.323235]  ? vfs_read+0xbc/0x1e0
[  333.323814]  ? ksys_read+0xb5/0x150
[  333.324323]  ? do_syscall_64+0xb9/0x3b0
[  333.324948]  ? entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  333.325756]  seq_vprintf+0x78/0xb0
[  333.326253]  seq_printf+0x96/0xc0
[  333.327132]  ? seq_vprintf+0xb0/0xb0
[  333.327678]  ? match_held_lock+0x2e/0x240
[  333.328450]  binder_transaction_log_show+0x237/0x2d0
[  333.329163]  seq_read+0x266/0x690
[  333.329705]  vfs_read+0xbc/0x1e0
[  333.330178]  ksys_read+0xb5/0x150
[  333.330724]  ? kernel_write+0xb0/0xb0
[  333.331257]  ? trace_hardirqs_off_caller+0x57/0x130
[  333.332045]  ? mark_held_locks+0x29/0xa0
[  333.332678]  ? do_syscall_64+0x6b/0x3b0
[  333.333235]  do_syscall_64+0xb9/0x3b0
[  333.333856]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  333.334635] RIP: 0033:0x7fbbb95d4461
[  333.335153] Code: fe ff ff 50 48 8d 3d fe d0 09 00 e8 e9 03 02 00
66 0f 1f 84 00 00 00 00 00 48 8d 05 99 62 0d 00 8b 00 85 c0 75 13 31
c0 0f 05 <48> 3d 00 f0 ff ff 77 57 c3 66 0f 1f 44 00 00 41 54 49 89 d4
55 48
[  333.337950] RSP: 002b:7ffcbe6438e8 EFLAGS: 0246 ORIG_RAX:

[  333.339072] RAX: ffda RBX: 0002 RCX: 7fbbb95d4461
[  333.340157] RDX: 0002 RSI: 7fbbb9324000 RDI:

Re: [PATCH v2 0/6] media: cedrus: h264: Support multi-slice frames

2019-10-07 Thread Jernej Škrabec 
Dne ponedeljek, 07. oktober 2019 ob 12:44:24 CEST je Hans Verkuil napisal(a):
> Hi Jernej,
> 
> On 9/29/19 10:00 PM, Jernej Skrabec wrote:
> > This series adds support for decoding multi-slice H264 frames along with
> > support for V4L2_DEC_CMD_FLUSH and V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF.
> > 
> > Code was tested by modified ffmpeg, which can be found here:
> > https://github.com/jernejsk/FFmpeg, branch mainline-test
> > It has to be configured with at least following options:
> > --enable-v4l2-request --enable-libudev --enable-libdrm
> > 
> > Samples used for testing:
> > http://jernej.libreelec.tv/videos/h264/BA1_FT_C.mp4
> > http://jernej.libreelec.tv/videos/h264/h264.mp4
> > 
> > Command line used for testing:
> > ffmpeg -hwaccel drm -hwaccel_device /dev/dri/card0 -i h264.mp4 -pix_fmt
> > bgra -f fbdev /dev/fb0
> > 
> > Please note that V4L2_DEC_CMD_FLUSH was not tested because I'm
> > not sure how. ffmpeg follows exactly which slice is last in frame
> > and sets hold flag accordingly. Improper usage of hold flag would
> > corrupt ffmpeg assumptions and it would probably crash. Any ideas
> > how to test this are welcome!
> > 
> > Thanks to Jonas for adjusting ffmpeg.
> > 
> > Please let me know what you think.
> > 
> > Best regards,
> > Jernej
> > 
> > Changes from v1:
> > - added Rb tags
> > - updated V4L2_DEC_CMD_FLUSH documentation
> > - updated first slice detection in Cedrus
> > - hold capture buffer flag is set according to source format
> > - added v4l m2m stateless_(try_)decoder_cmd ioctl helpers
> > 
> > Hans Verkuil (2):
> >   vb2: add V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF
> >   videodev2.h: add V4L2_DEC_CMD_FLUSH
> > 
> > Jernej Skrabec (4):
> >   media: v4l2-mem2mem: add stateless_(try_)decoder_cmd ioctl helpers
> >   media: cedrus: Detect first slice of a frame
> >   media: cedrus: h264: Support multiple slices per frame
> >   media: cedrus: Add support for holding capture buffer
> >  
> >  Documentation/media/uapi/v4l/buffer.rst   | 13 ++
> >  .../media/uapi/v4l/vidioc-decoder-cmd.rst | 10 +++-
> >  .../media/uapi/v4l/vidioc-reqbufs.rst |  6 +++
> >  .../media/videodev2.h.rst.exceptions  |  1 +
> >  .../media/common/videobuf2/videobuf2-v4l2.c   |  8 +++-
> >  drivers/media/v4l2-core/v4l2-mem2mem.c| 35 ++
> >  drivers/staging/media/sunxi/cedrus/cedrus.h   |  1 +
> >  .../staging/media/sunxi/cedrus/cedrus_dec.c   | 11 +
> >  .../staging/media/sunxi/cedrus/cedrus_h264.c  | 11 -
> >  .../staging/media/sunxi/cedrus/cedrus_hw.c|  8 ++--
> >  .../staging/media/sunxi/cedrus/cedrus_video.c | 14 ++
> >  include/media/v4l2-mem2mem.h  | 46 +++
> >  include/media/videobuf2-core.h|  3 ++
> >  include/media/videobuf2-v4l2.h|  5 ++
> >  include/uapi/linux/videodev2.h| 14 --
> >  15 files changed, 175 insertions(+), 11 deletions(-)
> 
> I didn't want to make a v3 of this series, instead I hacked this patch that
> will hopefully do all the locking right.
> 
> Basically I moved all the 'held' related code into v4l2-mem2mem under
> job_spinlock. This simplifies the driver code as well.
> 
> But this is a hack that sits on top of this series. If your ffmpeg tests are
> now successful, then I'll turn this into a proper series with correct
> documentation (a lot of the comments are now wrong with this patch, so just
> ignore that).

Thanks for looking into this! With small fix mentioned below, it works! Note 
that both scenarios I tested (flushing during decoding and flushing after 
decoding is finished) are focused on capture queue. In order to trigger output 
queue flush, ffmpeg would need to queue multiple jobs and call flush before 
they 
are all processed. This is not something I can do at this time. Maybe Jonas 
can help with modifying ffmpeg appropriately. However, code for case seems 
correct to me.

> 
> Regards,
> 
>   Hans
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c
> b/drivers/media/v4l2-core/v4l2-mem2mem.c index 2677a07e4c9b..f81a8f2465ab
> 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -412,25 +412,24 @@ static void v4l2_m2m_cancel_job(struct v4l2_m2m_ctx
> *m2m_ctx) }
>  }
> 
> -void v4l2_m2m_job_finish(struct v4l2_m2m_dev *m2m_dev,
> -  struct v4l2_m2m_ctx *m2m_ctx)
> +static bool _v4l2_m2m_job_finish(struct v4l2_m2m_dev *m2m_dev,
> +   struct v4l2_m2m_ctx *m2m_ctx)
>  {
> - unsigned long flags;
> -
> - spin_lock_irqsave(&m2m_dev->job_spinlock, flags);
>   if (!m2m_dev->curr_ctx || m2m_dev->curr_ctx != m2m_ctx) {
> - spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
>   dprintk("Called by an instance not currently 
running\n");
> - return;
> + return false;
>   }
> 
>   list_del(&m2m_dev->curr_ctx->queue);
>   m2m_dev->curr_ctx->job_flags &= ~(TRANS_QUEUED | TRAN

RE: [PATCH v2] PCI: PM: Move to D0 before calling pci_legacy_resume_early()

2019-10-07 Thread Dexuan Cui 
> -Original Message-
> From: Bjorn Helgaas 
> Sent: Monday, October 7, 2019 6:24 AM
> To: Dexuan Cui 
> Cc: lorenzo.pieral...@arm.com; linux-...@vger.kernel.org; Michael Kelley
> ; linux-hyp...@vger.kernel.org;
> linux-ker...@vger.kernel.org; driverdev-devel@linuxdriverproject.org; Sasha
> Levin ; Haiyang Zhang
> ; KY Srinivasan ;
> o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; vkuznets
> ; marcelo.ce...@canonical.com; Stephen Hemminger
> ; ja...@mellanox.com
> Subject: Re: [PATCH v2] PCI: PM: Move to D0 before calling
> pci_legacy_resume_early()
> 
> On Wed, Aug 14, 2019 at 01:06:55AM +, Dexuan Cui wrote:
> >
> > In pci_legacy_suspend_late(), the device state is moved to PCI_UNKNOWN.
> >
> > In pci_pm_thaw_noirq(), the state is supposed to be moved back to PCI_D0,
> > but the current code misses the pci_legacy_resume_early() path, so the
> > state remains in PCI_UNKNOWN in that path. As a result, in the resume
> > phase of hibernation, this causes an error for the Mellanox VF driver,
> > which fails to enable MSI-X because pci_msi_supported() is false due
> > to dev->current_state != PCI_D0:
> >
> > mlx4_core a6d1:00:02.0: Detected virtual function - running in slave mode
> > mlx4_core a6d1:00:02.0: Sending reset
> > mlx4_core a6d1:00:02.0: Sending vhcr0
> > mlx4_core a6d1:00:02.0: HCA minimum page size:512
> > mlx4_core a6d1:00:02.0: Timestamping is not supported in slave mode
> > mlx4_core a6d1:00:02.0: INTx is not supported in multi-function mode,
> aborting
> > PM: dpm_run_callback(): pci_pm_thaw+0x0/0xd7 returns -95
> > PM: Device a6d1:00:02.0 failed to thaw: error -95
> >
> > To be more accurate, the "resume" phase means the "thaw" callbacks which
> > run before the system enters hibernation: when the user runs the command
> > "echo disk > /sys/power/state" for hibernation, first the kernel "freezes"
> > all the devices and creates a hibernation image, then the kernel "thaws"
> > the devices including the disk/NIC, writes the memory to the disk, and
> > powers down. This patch fixes the error message for the Mellanox VF driver
> > in this phase.
> >
> > When the system starts again, a fresh kernel starts to run, and when the
> > kernel detects that a hibernation image was saved, the kernel "quiesces"
> > the devices, and then "restores" the devices from the saved image. In this
> > path:
> > device_resume_noirq() -> ... ->
> >   pci_pm_restore_noirq() ->
> > pci_pm_default_resume_early() ->
> >   pci_power_up() moves the device states back to PCI_D0. This path is
> > not broken and doesn't need my patch.
> >
> > Signed-off-by: Dexuan Cui 
> 
> This looks like a bugfix for 5839ee7389e8 ("PCI / PM: Force devices to
> D0 in pci_pm_thaw_noirq()") so maybe it should be marked for stable as
> 5839ee7389e8 was?
> 
> Rafael, could you confirm?
> 
> > ---
> >
> > changes in v2:
> > Updated the changelog with more details.
> >
> >  drivers/pci/pci-driver.c | 7 ---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> > index 36dbe960306b..27dfc68db9e7 100644
> > --- a/drivers/pci/pci-driver.c
> > +++ b/drivers/pci/pci-driver.c
> > @@ -1074,15 +1074,16 @@ static int pci_pm_thaw_noirq(struct device
> *dev)
> > return error;
> > }
> >
> > -   if (pci_has_legacy_pm_support(pci_dev))
> > -   return pci_legacy_resume_early(dev);
> > -
> > /*
> >  * pci_restore_state() requires the device to be in D0 (because of MSI
> >  * restoration among other things), so force it into D0 in case the
> >  * driver's "freeze" callbacks put it into a low-power state directly.
> >  */
> > pci_set_power_state(pci_dev, PCI_D0);
> > +
> > +   if (pci_has_legacy_pm_support(pci_dev))
> > +   return pci_legacy_resume_early(dev);
> > +
> > pci_restore_state(pci_dev);
> >
> > if (drv && drv->pm && drv->pm->thaw_noirq)
> > --
> > 2.19.1
> >

Added Rafael to "To".

Thanks,
-- Dexuan

[PATCH] staging: rtl8723bs: fix typo of "mechanism" in comment

2019-10-07 Thread Antonio Borneo 
Fix typo s/mechansim/mechanism/

Signed-off-by: Antonio Borneo 
---
 drivers/staging/rtl8723bs/hal/hal_btcoex.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/hal/hal_btcoex.c 
b/drivers/staging/rtl8723bs/hal/hal_btcoex.c
index 6e4a1fcb8790..d5793e4614bf 100644
--- a/drivers/staging/rtl8723bs/hal/hal_btcoex.c
+++ b/drivers/staging/rtl8723bs/hal/hal_btcoex.c
@@ -1315,7 +1315,7 @@ void EXhalbtcoutsrc_DisplayBtCoexInfo(PBTC_COEXIST 
pBtCoexist)
 
 /*
  * Description:
- *Run BT-Coexist mechansim or not
+ *Run BT-Coexist mechanism or not
  *
  */
 void hal_btcoex_SetBTCoexist(struct adapter *padapter, u8 bBtExist)
-- 
2.23.0

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

Re: [PATCH] staging: wlan-ng: p80211wep.c: use lib/crc32

2019-10-07 Thread Thomas Meyer 
Dan Carpenter  writes:

> On Sun, Oct 06, 2019 at 04:07:45PM +0200, Thomas Meyer wrote:
>> Use lib/crc32 instead of another implementation.
>> 
>> Signed-off-by: Thomas Meyer 
>
> I always get annoyed whenever anyone asks if people have tested their
> patches, but have you tested this?

no annoynence on my side, but... :-)

Good question. I tell you what I did and then you tell me if I did test!

So I did this: I did write a small C program that does contain a small
byte buffer and the extracted CRC32 logic from the wlan driver.
The program does calculate the CRC32 sum with the extracted logic and by
calling crc32_le function. but values are the same.

But as I don't own the hardware I couldn't do a real test with WEP (as
far as I understand only WEP on this hardware would be affected.)

So a better test would be to find someone which actually owns the
hardware and could test the change.

so... what do you think?

with kind regards
thomas

>It's hard for me to review it
> because I don't have the relevant background and because I'm a little
> bit stupid.
>
> regards,
> dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8712: align arguments with open parenthesis in file rtl8712_led.c

2019-10-07 Thread Dan Carpenter 
On Sun, Oct 06, 2019 at 09:39:02PM -0300, Gabriela Bittencourt wrote:
> Cleans up checks of "Alignment should match open parenthesis"
> 
> Signed-off-by: Gabriela Bittencourt 
> ---
>  drivers/staging/rtl8712/rtl8712_led.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8712/rtl8712_led.c 
> b/drivers/staging/rtl8712/rtl8712_led.c
> index db99129d3169..5901026949f2 100644
> --- a/drivers/staging/rtl8712/rtl8712_led.c
> +++ b/drivers/staging/rtl8712/rtl8712_led.c
> @@ -75,7 +75,7 @@ static void BlinkWorkItemCallback(struct work_struct *work);
>   *   Initialize an LED_871x object.
>   */
>  static void InitLed871x(struct _adapter *padapter, struct LED_871x *pLed,
> -  enum LED_PIN_871x  LedPin)
> + enum LED_PIN_871x   LedPin)
 ^^
Delete these extra spaces.

regards,
dan carpenter

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

Re: [PATCH] staging: wlan-ng: p80211wep.c: use lib/crc32

2019-10-07 Thread Dan Carpenter 
On Sun, Oct 06, 2019 at 04:07:45PM +0200, Thomas Meyer wrote:
> Use lib/crc32 instead of another implementation.
> 
> Signed-off-by: Thomas Meyer 

I always get annoyed whenever anyone asks if people have tested their
patches, but have you tested this?  It's hard for me to review it
because I don't have the relevant background and because I'm a little
bit stupid.

regards,
dan carpenter

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

Re: [PATCH 1/5] staging: rtl8723bs: Remove set but not used variable 'i'

2019-10-07 Thread Dan Carpenter 
On Sun, Oct 06, 2019 at 05:09:55PM +0800, zhengbin wrote:
> @@ -689,7 +688,7 @@ static s32 update_attrib(struct adapter *padapter, _pkt 
> *pkt, struct pkt_attrib
>   DBG_COUNTER(padapter->tx_logs.core_tx_upd_attrib);
> 
>   _rtw_open_pktfile(pkt, &pktfile);
> - i = _rtw_pktfile_read(&pktfile, (u8 *)ðerhdr, ETH_HLEN);
> + (void)_rtw_pktfile_read(&pktfile, (u8 *)ðerhdr, ETH_HLEN);

Don't add this "(void)" here.  There is no need and it looks ugly.

_rtw_pktfile_read(&pktfile, (u8 *)ðerhdr, ETH_HLEN);

regards,
dan carpenter

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

Re: [PATCH v2] PCI: PM: Move to D0 before calling pci_legacy_resume_early()

2019-10-07 Thread Bjorn Helgaas 
On Wed, Aug 14, 2019 at 01:06:55AM +, Dexuan Cui wrote:
> 
> In pci_legacy_suspend_late(), the device state is moved to PCI_UNKNOWN.
> 
> In pci_pm_thaw_noirq(), the state is supposed to be moved back to PCI_D0,
> but the current code misses the pci_legacy_resume_early() path, so the
> state remains in PCI_UNKNOWN in that path. As a result, in the resume
> phase of hibernation, this causes an error for the Mellanox VF driver,
> which fails to enable MSI-X because pci_msi_supported() is false due
> to dev->current_state != PCI_D0:
> 
> mlx4_core a6d1:00:02.0: Detected virtual function - running in slave mode
> mlx4_core a6d1:00:02.0: Sending reset
> mlx4_core a6d1:00:02.0: Sending vhcr0
> mlx4_core a6d1:00:02.0: HCA minimum page size:512
> mlx4_core a6d1:00:02.0: Timestamping is not supported in slave mode
> mlx4_core a6d1:00:02.0: INTx is not supported in multi-function mode, aborting
> PM: dpm_run_callback(): pci_pm_thaw+0x0/0xd7 returns -95
> PM: Device a6d1:00:02.0 failed to thaw: error -95
> 
> To be more accurate, the "resume" phase means the "thaw" callbacks which
> run before the system enters hibernation: when the user runs the command
> "echo disk > /sys/power/state" for hibernation, first the kernel "freezes"
> all the devices and creates a hibernation image, then the kernel "thaws"
> the devices including the disk/NIC, writes the memory to the disk, and
> powers down. This patch fixes the error message for the Mellanox VF driver
> in this phase.
> 
> When the system starts again, a fresh kernel starts to run, and when the
> kernel detects that a hibernation image was saved, the kernel "quiesces"
> the devices, and then "restores" the devices from the saved image. In this
> path:
> device_resume_noirq() -> ... ->
>   pci_pm_restore_noirq() ->
> pci_pm_default_resume_early() ->
>   pci_power_up() moves the device states back to PCI_D0. This path is
> not broken and doesn't need my patch.
> 
> Signed-off-by: Dexuan Cui 

This looks like a bugfix for 5839ee7389e8 ("PCI / PM: Force devices to
D0 in pci_pm_thaw_noirq()") so maybe it should be marked for stable as
5839ee7389e8 was?

Rafael, could you confirm?

> ---
> 
> changes in v2:
>   Updated the changelog with more details.
> 
>  drivers/pci/pci-driver.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 36dbe960306b..27dfc68db9e7 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1074,15 +1074,16 @@ static int pci_pm_thaw_noirq(struct device *dev)
>   return error;
>   }
>  
> - if (pci_has_legacy_pm_support(pci_dev))
> - return pci_legacy_resume_early(dev);
> -
>   /*
>* pci_restore_state() requires the device to be in D0 (because of MSI
>* restoration among other things), so force it into D0 in case the
>* driver's "freeze" callbacks put it into a low-power state directly.
>*/
>   pci_set_power_state(pci_dev, PCI_D0);
> +
> + if (pci_has_legacy_pm_support(pci_dev))
> + return pci_legacy_resume_early(dev);
> +
>   pci_restore_state(pci_dev);
>  
>   if (drv && drv->pm && drv->pm->thaw_noirq)
> -- 
> 2.19.1
>

Re: [PATCH v2 0/6] media: cedrus: h264: Support multi-slice frames

2019-10-07 Thread Hans Verkuil 
Hi Jernej,

On 9/29/19 10:00 PM, Jernej Skrabec wrote:
> This series adds support for decoding multi-slice H264 frames along with
> support for V4L2_DEC_CMD_FLUSH and V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF.
> 
> Code was tested by modified ffmpeg, which can be found here:
> https://github.com/jernejsk/FFmpeg, branch mainline-test
> It has to be configured with at least following options:
> --enable-v4l2-request --enable-libudev --enable-libdrm
> 
> Samples used for testing:
> http://jernej.libreelec.tv/videos/h264/BA1_FT_C.mp4
> http://jernej.libreelec.tv/videos/h264/h264.mp4
> 
> Command line used for testing:
> ffmpeg -hwaccel drm -hwaccel_device /dev/dri/card0 -i h264.mp4 -pix_fmt bgra 
> -f fbdev /dev/fb0
> 
> Please note that V4L2_DEC_CMD_FLUSH was not tested because I'm
> not sure how. ffmpeg follows exactly which slice is last in frame
> and sets hold flag accordingly. Improper usage of hold flag would
> corrupt ffmpeg assumptions and it would probably crash. Any ideas
> how to test this are welcome!
> 
> Thanks to Jonas for adjusting ffmpeg.
> 
> Please let me know what you think.
> 
> Best regards,
> Jernej
> 
> Changes from v1:
> - added Rb tags
> - updated V4L2_DEC_CMD_FLUSH documentation
> - updated first slice detection in Cedrus
> - hold capture buffer flag is set according to source format
> - added v4l m2m stateless_(try_)decoder_cmd ioctl helpers
> 
> Hans Verkuil (2):
>   vb2: add V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF
>   videodev2.h: add V4L2_DEC_CMD_FLUSH
> 
> Jernej Skrabec (4):
>   media: v4l2-mem2mem: add stateless_(try_)decoder_cmd ioctl helpers
>   media: cedrus: Detect first slice of a frame
>   media: cedrus: h264: Support multiple slices per frame
>   media: cedrus: Add support for holding capture buffer
> 
>  Documentation/media/uapi/v4l/buffer.rst   | 13 ++
>  .../media/uapi/v4l/vidioc-decoder-cmd.rst | 10 +++-
>  .../media/uapi/v4l/vidioc-reqbufs.rst |  6 +++
>  .../media/videodev2.h.rst.exceptions  |  1 +
>  .../media/common/videobuf2/videobuf2-v4l2.c   |  8 +++-
>  drivers/media/v4l2-core/v4l2-mem2mem.c| 35 ++
>  drivers/staging/media/sunxi/cedrus/cedrus.h   |  1 +
>  .../staging/media/sunxi/cedrus/cedrus_dec.c   | 11 +
>  .../staging/media/sunxi/cedrus/cedrus_h264.c  | 11 -
>  .../staging/media/sunxi/cedrus/cedrus_hw.c|  8 ++--
>  .../staging/media/sunxi/cedrus/cedrus_video.c | 14 ++
>  include/media/v4l2-mem2mem.h  | 46 +++
>  include/media/videobuf2-core.h|  3 ++
>  include/media/videobuf2-v4l2.h|  5 ++
>  include/uapi/linux/videodev2.h| 14 --
>  15 files changed, 175 insertions(+), 11 deletions(-)
> 

I didn't want to make a v3 of this series, instead I hacked this patch that will
hopefully do all the locking right.

Basically I moved all the 'held' related code into v4l2-mem2mem under 
job_spinlock.
This simplifies the driver code as well.

But this is a hack that sits on top of this series. If your ffmpeg tests are now
successful, then I'll turn this into a proper series with correct documentation
(a lot of the comments are now wrong with this patch, so just ignore that).

Regards,

Hans

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 2677a07e4c9b..f81a8f2465ab 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -412,25 +412,24 @@ static void v4l2_m2m_cancel_job(struct v4l2_m2m_ctx 
*m2m_ctx)
}
 }

-void v4l2_m2m_job_finish(struct v4l2_m2m_dev *m2m_dev,
-struct v4l2_m2m_ctx *m2m_ctx)
+static bool _v4l2_m2m_job_finish(struct v4l2_m2m_dev *m2m_dev,
+ struct v4l2_m2m_ctx *m2m_ctx)
 {
-   unsigned long flags;
-
-   spin_lock_irqsave(&m2m_dev->job_spinlock, flags);
if (!m2m_dev->curr_ctx || m2m_dev->curr_ctx != m2m_ctx) {
-   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
dprintk("Called by an instance not currently running\n");
-   return;
+   return false;
}

list_del(&m2m_dev->curr_ctx->queue);
m2m_dev->curr_ctx->job_flags &= ~(TRANS_QUEUED | TRANS_RUNNING);
wake_up(&m2m_dev->curr_ctx->finished);
m2m_dev->curr_ctx = NULL;
+   return true;
+}

-   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
-
+static void v4l2_m2m_job_next(struct v4l2_m2m_dev *m2m_dev,
+  struct v4l2_m2m_ctx *m2m_ctx)
+{
/* This instance might have more buffers ready, but since we do not
 * allow more than one job on the job_queue per instance, each has
 * to be scheduled separately after the previous one finishes. */
@@ -441,8 +440,113 @@ void v4l2_m2m_job_finish(struct v4l2_m2m_dev *m2m_dev,
 */
schedule_work(&m2m_dev->job_work);
 }
+
+void v4l2_m2m_job_finish(struct v4l2_m2m_dev *m2m_dev,
+

Re: [PATCH 1/5] staging: rtl8723bs: Remove set but not used variable 'i'

2019-10-07 Thread Greg KH 
On Sun, Oct 06, 2019 at 05:09:55PM +0800, zhengbin wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/staging/rtl8723bs/core/rtw_xmit.c: In function update_attrib:
> drivers/staging/rtl8723bs/core/rtw_xmit.c:680:7: warning: variable i set but 
> not used [-Wunused-but-set-variable]
> 
> It is not used since commit 554c0a3abf21 ("staging:
> Add rtl8723bs sdio wifi driver")
> 
> Reported-by: Hulk Robot 
> Signed-off-by: zhengbin 
> ---
>  drivers/staging/rtl8723bs/core/rtw_xmit.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c 
> b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> index b5dcb78..c24b524 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> @@ -675,7 +675,6 @@ static void set_qos(struct pkt_file *ppktfile, struct 
> pkt_attrib *pattrib)
> 
>  static s32 update_attrib(struct adapter *padapter, _pkt *pkt, struct 
> pkt_attrib *pattrib)
>  {
> - uint i;
>   struct pkt_file pktfile;
>   struct sta_info *psta = NULL;
>   struct ethhdr etherhdr;
> @@ -689,7 +688,7 @@ static s32 update_attrib(struct adapter *padapter, _pkt 
> *pkt, struct pkt_attrib
>   DBG_COUNTER(padapter->tx_logs.core_tx_upd_attrib);
> 
>   _rtw_open_pktfile(pkt, &pktfile);
> - i = _rtw_pktfile_read(&pktfile, (u8 *)ðerhdr, ETH_HLEN);
> + (void)_rtw_pktfile_read(&pktfile, (u8 *)ðerhdr, ETH_HLEN);

No need for the (void) marking here.  If you really are supposed to do
something with the return value, then do something with it, don't ignore
it.

thanks,

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

Re: [PATCH] staging: rtl8712: align arguments with open parenthesis

2019-10-07 Thread Greg KH 
On Sun, Oct 06, 2019 at 07:20:15PM -0300, Gabriela Bittencourt wrote:
> Cleans up checks of: "Alignment should match open parenthesis"
> 
> Signed-off-by: Gabriela Bittencourt 
> ---
>  drivers/staging/rtl8712/osdep_service.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

You sent 2 different patches that do different things, yet have the same
exact subject line :(

Please fix this up and send a patch series, so I know what order in
which to apply your patches.

thanks,

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

Re: [PATCH] staging: vt6656: align arguments with open parenthesis

2019-10-07 Thread Greg KH 
On Sun, Oct 06, 2019 at 04:10:20PM -0300, Gabriela Bittencourt wrote:
> Cleans up CHECKs of "Alignment should match open parenthesis"
> 
> Signed-off-by: Gabriela Bittencourt 
> ---
>  drivers/staging/vt6656/rxtx.c | 63 +++
>  1 file changed, 41 insertions(+), 22 deletions(-)

This patch does not apply to my staging-next branch either :(

Please rebase and resend it.

thanks,

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

Re: [PATCH] staging: rtl8712: align block comments

2019-10-07 Thread Greg KH 
On Sun, Oct 06, 2019 at 05:34:20PM -0300, Gabriela Bittencourt wrote:
> Cleans up warnings of "Block comments should align the * on each line"
> 
> Signed-off-by: Gabriela Bittencourt 
> ---
>  drivers/staging/rtl8712/recv_linux.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8712/recv_linux.c 
> b/drivers/staging/rtl8712/recv_linux.c
> index 84c4c8580f9a..70a4dcd4a1e5 100644
> --- a/drivers/staging/rtl8712/recv_linux.c
> +++ b/drivers/staging/rtl8712/recv_linux.c
> @@ -115,8 +115,8 @@ void r8712_recv_indicatepkt(struct _adapter *adapter,
>   skb->protocol = eth_type_trans(skb, adapter->pnetdev);
>   netif_rx(skb);
>   recvframe->u.hdr.pkt = NULL; /* pointers to NULL before
> - * r8712_free_recvframe()
> - */
> +   * r8712_free_recvframe()
> +   */
>   r8712_free_recvframe(recvframe, free_recv_queue);
>   return;
>  _recv_indicatepkt_drop:

This patch did not apply to my tree at all.  Be sure you are working
against the staging-next branch of the staging.git tree, or off of
linux-next which includes this branch as well.

thanks,

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

Re: [PATCH] staging: vt6656: remove duplicated blank line

2019-10-07 Thread Quentin Deslandes 
On Sun, Oct 06, 2019 at 04:58:54PM -0300, Gabriela Bittencourt wrote:
> Cleans up checks of "don't use multiple blank line"
> 
> Signed-off-by: Gabriela Bittencourt 
> ---
>  drivers/staging/vt6656/main_usb.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/staging/vt6656/main_usb.c 
> b/drivers/staging/vt6656/main_usb.c
> index 856ba97aec4f..a1884b5cc915 100644
> --- a/drivers/staging/vt6656/main_usb.c
> +++ b/drivers/staging/vt6656/main_usb.c
> @@ -362,7 +362,6 @@ static int vnt_init_registers(struct vnt_private *priv)
>   goto end;
>   }
>  
> -
>   ret = vnt_mac_set_led(priv, LEDSTS_TMLEN, 0x38);
>   if (ret)
>   goto end;
> -- 
> 2.20.1
> 

Reviewed-by: Quentin Deslandes 

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

Re: [PATCH v2 3/6] media: v4l2-mem2mem: add stateless_(try_)decoder_cmd ioctl helpers

2019-10-07 Thread Hans Verkuil 
On 10/7/19 10:32 AM, Hans Verkuil wrote:
> On 10/7/19 8:02 AM, Jernej Škrabec wrote:
>> Dne petek, 04. oktober 2019 ob 11:21:12 CEST je Hans Verkuil napisal(a):
>>> On 9/29/19 10:00 PM, Jernej Skrabec wrote:
 These helpers are used by stateless codecs when they support multiple
 slices per frame and hold capture buffer flag is set. It's expected that
 all such codecs will use this code.

 Signed-off-by: Jernej Skrabec 
 ---

  drivers/media/v4l2-core/v4l2-mem2mem.c | 35 ++
  include/media/v4l2-mem2mem.h   |  4 +++
  2 files changed, 39 insertions(+)

 diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c
 b/drivers/media/v4l2-core/v4l2-mem2mem.c index 19937dd3c6f6..2677a07e4c9b
 100644
 --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
 +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
 @@ -1154,6 +1154,41 @@ int v4l2_m2m_ioctl_try_decoder_cmd(struct file
 *file, void *fh,> 
  }
  EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_try_decoder_cmd);

 +int v4l2_m2m_ioctl_stateless_try_decoder_cmd(struct file *file, void *fh,
 +   struct 
>> v4l2_decoder_cmd *dc)
 +{
 +  if (dc->cmd != V4L2_DEC_CMD_FLUSH)
 +  return -EINVAL;
 +
 +  dc->flags = 0;
 +
 +  return 0;
 +}
 +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_stateless_try_decoder_cmd);
 +
 +int v4l2_m2m_ioctl_stateless_decoder_cmd(struct file *file, void *priv,
 +   struct 
>> v4l2_decoder_cmd *dc)
 +{
 +  struct v4l2_fh *fh = file->private_data;
 +  struct vb2_v4l2_buffer *out_vb, *cap_vb;
 +  int ret;
 +
 +  ret = v4l2_m2m_ioctl_stateless_try_decoder_cmd(file, priv, dc);
 +  if (ret < 0)
 +  return ret;
 +
 +  out_vb = v4l2_m2m_last_src_buf(fh->m2m_ctx);
 +  cap_vb = v4l2_m2m_last_dst_buf(fh->m2m_ctx);
>>>
>>> I think this should be v4l2_m2m_next_dst_buf. If multiple capture buffers
>>> were queued up, then it can only be the first capture buffer that can be
>>> 'HELD'.
>>
>> I'm pretty sure v4l2_m2m_last_dst_buf() is correct. We want to affect last 
>> job 
>> in the queue, all jobs before are already properly handled by comparing 
>> timestamps.
> 
> You're absolutely right.
> 
>>
>>>
>>> This might solve the race condition you saw with ffmpeg.
>>
>> This actually doesn't change anything. ffmpeg currently queues only one job 
>> and 
>> then waits until it's executed. In this case it actually doesn't matter if 
>> "last" or "next" variant is used.
> 
> Can you debug this a bit further? I don't want to merge this unless we know 
> what's
> going wrong with ffmpeg.
> 
> I suspect it is a race condition. I'll reply to patch 6/6 with more info.

I've decided to make a v3 of this series. There are major locking issues with 
this
and this will require a bit of rework.

Regards,

Hans

> 
> Regards,
> 
>   Hans
> 
>>
>> Best regards,
>> Jernej
>>
>>>
>>> Regards,
>>>
>>> Hans
>>>
 +
 +  if (out_vb)
 +  out_vb->flags &= ~V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF;
 +  else if (cap_vb && cap_vb->is_held)
 +  v4l2_m2m_buf_done(cap_vb, VB2_BUF_STATE_DONE);
 +
 +  return 0;
 +}
 +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_stateless_decoder_cmd);
 +

  /*
  
   * v4l2_file_operations helpers. It is assumed here same lock is used
   * for the output and the capture buffer queue.

 diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
 index c9fa96c8eed1..8ae2f56c7fa3 100644
 --- a/include/media/v4l2-mem2mem.h
 +++ b/include/media/v4l2-mem2mem.h
 @@ -714,6 +714,10 @@ int v4l2_m2m_ioctl_try_encoder_cmd(struct file *file,
 void *fh,> 
   struct v4l2_encoder_cmd *ec);
  
  int v4l2_m2m_ioctl_try_decoder_cmd(struct file *file, void *fh,
  
   struct v4l2_decoder_cmd *dc);

 +int v4l2_m2m_ioctl_stateless_try_decoder_cmd(struct file *file, void *fh,
 +   struct 
>> v4l2_decoder_cmd *dc);
 +int v4l2_m2m_ioctl_stateless_decoder_cmd(struct file *file, void *priv,
 +   struct 
>> v4l2_decoder_cmd *dc);

  int v4l2_m2m_fop_mmap(struct file *file, struct vm_area_struct *vma);
  __poll_t v4l2_m2m_fop_poll(struct file *file, poll_table *wait);
>>
>>
>>
>>
> 

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

Re: [staging:staging-testing 41/59] drivers/staging/wfx/main.c:47:14-21: ERROR: PTR_ERR applied after initialization to constant on line 42

2019-10-07 Thread Greg Kroah-Hartman 
On Mon, Oct 07, 2019 at 08:36:22AM +, Jerome Pouiller wrote:
> On Friday 4 October 2019 12:48:32 CEST kbuild test robot wrote:
> [...]
> > >> drivers/staging/wfx/main.c:47:14-21: ERROR: PTR_ERR applied after 
> > >> initialization to constant on line 42
> > 
> > vim +47 drivers/staging/wfx/main.c
> > 
> > 30
> > 31  struct gpio_desc *wfx_get_gpio(struct device *dev, int override, 
> > const char *label)
> > 32  {
> > 33  struct gpio_desc *ret;
> > 34  char label_buf[256];
> > 35
> > 36  if (override >= 0) {
> > 37  snprintf(label_buf, sizeof(label_buf), "wfx_%s", 
> > label);
> > 38  ret = ERR_PTR(devm_gpio_request_one(dev, override, 
> > GPIOF_OUT_INIT_LOW, label_buf));
> > 39  if (!ret)
> > 40  ret = gpio_to_desc(override);
> > 41  } else if (override == -1) {
> >   > 42  ret = NULL;
> > 43  } else {
> > 44  ret = devm_gpiod_get(dev, label, GPIOD_OUT_LOW);
> > 45  }
> > 46  if (IS_ERR(ret) || !ret) {
> >   > 47  if (!ret || PTR_ERR(ret) == -ENOENT)
> > 48  dev_warn(dev, "gpio %s is not defined\n", 
> > label);
> > 49  else
> > 50  dev_warn(dev, "error while requesting gpio 
> > %s\n", label);
> > 51  ret = NULL;
> > 52  } else {
> > 53  dev_dbg(dev, "using gpio %d for %s\n", 
> > desc_to_gpio(ret), label);
> > 54  }
> > 55  return ret;
> > 56  }
> > 57
> 
> I think that this report is a false positive or I missed something?

No idea, but I really can not understand that code at all, so it does
need to be simplified no matter what :)

thanks,

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

Re: [staging:staging-testing 41/59] drivers/staging/wfx/main.c:47:14-21: ERROR: PTR_ERR applied after initialization to constant on line 42

2019-10-07 Thread Jerome Pouiller 
On Friday 4 October 2019 12:48:32 CEST kbuild test robot wrote:
[...]
> >> drivers/staging/wfx/main.c:47:14-21: ERROR: PTR_ERR applied after 
> >> initialization to constant on line 42
> 
> vim +47 drivers/staging/wfx/main.c
> 
> 30
> 31  struct gpio_desc *wfx_get_gpio(struct device *dev, int override, 
> const char *label)
> 32  {
> 33  struct gpio_desc *ret;
> 34  char label_buf[256];
> 35
> 36  if (override >= 0) {
> 37  snprintf(label_buf, sizeof(label_buf), "wfx_%s", 
> label);
> 38  ret = ERR_PTR(devm_gpio_request_one(dev, override, 
> GPIOF_OUT_INIT_LOW, label_buf));
> 39  if (!ret)
> 40  ret = gpio_to_desc(override);
> 41  } else if (override == -1) {
>   > 42  ret = NULL;
> 43  } else {
> 44  ret = devm_gpiod_get(dev, label, GPIOD_OUT_LOW);
> 45  }
> 46  if (IS_ERR(ret) || !ret) {
>   > 47  if (!ret || PTR_ERR(ret) == -ENOENT)
> 48  dev_warn(dev, "gpio %s is not defined\n", 
> label);
> 49  else
> 50  dev_warn(dev, "error while requesting gpio 
> %s\n", label);
> 51  ret = NULL;
> 52  } else {
> 53  dev_dbg(dev, "using gpio %d for %s\n", 
> desc_to_gpio(ret), label);
> 54  }
> 55  return ret;
> 56  }
> 57

I think that this report is a false positive or I missed something?

-- 
Jérôme Pouiller
-- 
Jérôme Pouiller

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

Re: [PATCH v2 3/6] media: v4l2-mem2mem: add stateless_(try_)decoder_cmd ioctl helpers

2019-10-07 Thread Hans Verkuil 
On 10/7/19 8:02 AM, Jernej Škrabec wrote:
> Dne petek, 04. oktober 2019 ob 11:21:12 CEST je Hans Verkuil napisal(a):
>> On 9/29/19 10:00 PM, Jernej Skrabec wrote:
>>> These helpers are used by stateless codecs when they support multiple
>>> slices per frame and hold capture buffer flag is set. It's expected that
>>> all such codecs will use this code.
>>>
>>> Signed-off-by: Jernej Skrabec 
>>> ---
>>>
>>>  drivers/media/v4l2-core/v4l2-mem2mem.c | 35 ++
>>>  include/media/v4l2-mem2mem.h   |  4 +++
>>>  2 files changed, 39 insertions(+)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c
>>> b/drivers/media/v4l2-core/v4l2-mem2mem.c index 19937dd3c6f6..2677a07e4c9b
>>> 100644
>>> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
>>> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
>>> @@ -1154,6 +1154,41 @@ int v4l2_m2m_ioctl_try_decoder_cmd(struct file
>>> *file, void *fh,> 
>>>  }
>>>  EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_try_decoder_cmd);
>>>
>>> +int v4l2_m2m_ioctl_stateless_try_decoder_cmd(struct file *file, void *fh,
>>> +struct 
> v4l2_decoder_cmd *dc)
>>> +{
>>> +   if (dc->cmd != V4L2_DEC_CMD_FLUSH)
>>> +   return -EINVAL;
>>> +
>>> +   dc->flags = 0;
>>> +
>>> +   return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_stateless_try_decoder_cmd);
>>> +
>>> +int v4l2_m2m_ioctl_stateless_decoder_cmd(struct file *file, void *priv,
>>> +struct 
> v4l2_decoder_cmd *dc)
>>> +{
>>> +   struct v4l2_fh *fh = file->private_data;
>>> +   struct vb2_v4l2_buffer *out_vb, *cap_vb;
>>> +   int ret;
>>> +
>>> +   ret = v4l2_m2m_ioctl_stateless_try_decoder_cmd(file, priv, dc);
>>> +   if (ret < 0)
>>> +   return ret;
>>> +
>>> +   out_vb = v4l2_m2m_last_src_buf(fh->m2m_ctx);
>>> +   cap_vb = v4l2_m2m_last_dst_buf(fh->m2m_ctx);
>>
>> I think this should be v4l2_m2m_next_dst_buf. If multiple capture buffers
>> were queued up, then it can only be the first capture buffer that can be
>> 'HELD'.
> 
> I'm pretty sure v4l2_m2m_last_dst_buf() is correct. We want to affect last 
> job 
> in the queue, all jobs before are already properly handled by comparing 
> timestamps.

You're absolutely right.

> 
>>
>> This might solve the race condition you saw with ffmpeg.
> 
> This actually doesn't change anything. ffmpeg currently queues only one job 
> and 
> then waits until it's executed. In this case it actually doesn't matter if 
> "last" or "next" variant is used.

Can you debug this a bit further? I don't want to merge this unless we know 
what's
going wrong with ffmpeg.

I suspect it is a race condition. I'll reply to patch 6/6 with more info.

Regards,

Hans

> 
> Best regards,
> Jernej
> 
>>
>> Regards,
>>
>>  Hans
>>
>>> +
>>> +   if (out_vb)
>>> +   out_vb->flags &= ~V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF;
>>> +   else if (cap_vb && cap_vb->is_held)
>>> +   v4l2_m2m_buf_done(cap_vb, VB2_BUF_STATE_DONE);
>>> +
>>> +   return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_stateless_decoder_cmd);
>>> +
>>>
>>>  /*
>>>  
>>>   * v4l2_file_operations helpers. It is assumed here same lock is used
>>>   * for the output and the capture buffer queue.
>>>
>>> diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
>>> index c9fa96c8eed1..8ae2f56c7fa3 100644
>>> --- a/include/media/v4l2-mem2mem.h
>>> +++ b/include/media/v4l2-mem2mem.h
>>> @@ -714,6 +714,10 @@ int v4l2_m2m_ioctl_try_encoder_cmd(struct file *file,
>>> void *fh,> 
>>>struct v4l2_encoder_cmd *ec);
>>>  
>>>  int v4l2_m2m_ioctl_try_decoder_cmd(struct file *file, void *fh,
>>>  
>>>struct v4l2_decoder_cmd *dc);
>>>
>>> +int v4l2_m2m_ioctl_stateless_try_decoder_cmd(struct file *file, void *fh,
>>> +struct 
> v4l2_decoder_cmd *dc);
>>> +int v4l2_m2m_ioctl_stateless_decoder_cmd(struct file *file, void *priv,
>>> +struct 
> v4l2_decoder_cmd *dc);
>>>
>>>  int v4l2_m2m_fop_mmap(struct file *file, struct vm_area_struct *vma);
>>>  __poll_t v4l2_m2m_fop_poll(struct file *file, poll_table *wait);
> 
> 
> 
> 

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

Re: [RESEND TRIVIAL 3/3] treewide: arch: Fix Kconfig indentation

2019-10-07 Thread Geert Uytterhoeven 
On Fri, Oct 4, 2019 at 4:57 PM Krzysztof Kozlowski  wrote:
> Adjust indentation from spaces to tab (+optional two spaces) as in
> coding style with command like:
> $ sed -e 's/^/\t/' -i */Kconfig
>
> Signed-off-by: Krzysztof Kozlowski 

>  arch/m68k/Kconfig.bus  |  2 +-
>  arch/m68k/Kconfig.debug| 16 
>  arch/m68k/Kconfig.machine  |  8 

For m68k:
Acked-by: Geert Uytterhoeven 

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