Re: [PATCH v4 1/2] staging: iio: ad7780: update voltage on read

2018-11-06 Thread Ardelean, Alexandru
On Mon, 2018-11-05 at 17:14 -0200, Renato Lui Geh wrote:
> The ad7780 driver previously did not read the correct device output, as
> it read an outdated value set at initialization. It now updates its
> voltage on read.
> 

Looks good from my side.

Alex

> Signed-off-by: Renato Lui Geh 
> ---
> Changes in v3:
>   - removed initialization (int voltage_uv = 0)
>   - returns error when voltage_uv is null
> Changes in v4:
>   - returns error when voltage_uv is negative
> 
>  drivers/staging/iio/adc/ad7780.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7780.c
> b/drivers/staging/iio/adc/ad7780.c
> index b67412db0318..c7cb05cedbbc 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -87,12 +87,16 @@ static int ad7780_read_raw(struct iio_dev *indio_dev,
>  long m)
>  {
>   struct ad7780_state *st = iio_priv(indio_dev);
> + int voltage_uv;
>  
>   switch (m) {
>   case IIO_CHAN_INFO_RAW:
>   return ad_sigma_delta_single_conversion(indio_dev, chan,
> val);
>   case IIO_CHAN_INFO_SCALE:
> - *val = st->int_vref_mv * st->gain;
> + voltage_uv = regulator_get_voltage(st->reg);
> + if (voltage_uv < 0)
> + return voltage_uv;
> + *val = (voltage_uv / 1000) * st->gain;
>   *val2 = chan->scan_type.realbits - 1;
>   return IIO_VAL_FRACTIONAL_LOG2;
>   case IIO_CHAN_INFO_OFFSET:
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] davinci_vpfe: add a missing break

2018-11-06 Thread Mauro Carvalho Chehab
As warned by gcc:

drivers/staging/media/davinci_vpfe/dm365_ipipeif.c: In function 
'ipipeif_hw_setup':
drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:298:3: warning: this 
statement may fall through [-Wimplicit-fallthrough=]
   switch (isif_port_if) {
   ^~
drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:314:2: note: here
  case IPIPEIF_SDRAM_YUV:
  ^~~~

There is a missing break for the raw format.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c 
b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
index a53231b08d30..975272bcf8ca 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
@@ -310,6 +310,7 @@ static int ipipeif_hw_setup(struct v4l2_subdev *sd)
ipipeif_write(val, ipipeif_base_addr, IPIPEIF_CFG2);
break;
}
+   break;
 
case IPIPEIF_SDRAM_YUV:
/* Set clock divider */
-- 
2.19.1

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


Re: [PATCH] davinci_vpfe: add a missing break

2018-11-06 Thread Hans Verkuil
On 11/06/18 11:15, Mauro Carvalho Chehab wrote:
> As warned by gcc:
> 
> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c: In function 
> 'ipipeif_hw_setup':
> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:298:3: warning: this 
> statement may fall through [-Wimplicit-fallthrough=]
>switch (isif_port_if) {
>^~
> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:314:2: note: here
>   case IPIPEIF_SDRAM_YUV:
>   ^~~~
> 
> There is a missing break for the raw format.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Nacked-by: Hans Verkuil 

It really should fall through: see this comment:

/* fall through for SDRAM YUV mode */
/* configure CFG2 */
val = ipipeif_read(ipipeif_base_addr, IPIPEIF_CFG2);
switch (isif_port_if) {
case MEDIA_BUS_FMT_YUYV8_1X16:
case MEDIA_BUS_FMT_UYVY8_2X8:
case MEDIA_BUS_FMT_Y8_1X8:
RESETBIT(val, IPIPEIF_CFG2_YUV8_SHIFT);
SETBIT(val, IPIPEIF_CFG2_YUV16_SHIFT);
ipipeif_write(val, ipipeif_base_addr, IPIPEIF_CFG2);
break;

default:
RESETBIT(val, IPIPEIF_CFG2_YUV8_SHIFT);
RESETBIT(val, IPIPEIF_CFG2_YUV16_SHIFT);
ipipeif_write(val, ipipeif_base_addr, IPIPEIF_CFG2);
break;
}

case IPIPEIF_SDRAM_YUV:

So we need a proper /* fall through */ comment instead of a break.

In the SDRAM_YUV case the SDRAM clock divider is configured, and that needs
to be done for both SDRAM_* cases.

Regards,

Hans


> ---
>  drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c 
> b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> index a53231b08d30..975272bcf8ca 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> @@ -310,6 +310,7 @@ static int ipipeif_hw_setup(struct v4l2_subdev *sd)
>   ipipeif_write(val, ipipeif_base_addr, IPIPEIF_CFG2);
>   break;
>   }
> + break;
>  
>   case IPIPEIF_SDRAM_YUV:
>   /* Set clock divider */
> 

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


[PATCH] media: dm365_ipipeif: better annotate a fall though

2018-11-06 Thread Mauro Carvalho Chehab
Shut up this warning:

drivers/staging/media/davinci_vpfe/dm365_ipipeif.c: In function 
'ipipeif_hw_setup':
drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:298:3: warning: this 
statement may fall through [-Wimplicit-fallthrough=]
   switch (isif_port_if) {
   ^~
drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:314:2: note: here
  case IPIPEIF_SDRAM_YUV:
  ^~~~

By annotating a fall though case at the right place.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c 
b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
index a53231b08d30..e3425bf082ae 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
@@ -310,6 +310,7 @@ static int ipipeif_hw_setup(struct v4l2_subdev *sd)
ipipeif_write(val, ipipeif_base_addr, IPIPEIF_CFG2);
break;
}
+   /* fall through */
 
case IPIPEIF_SDRAM_YUV:
/* Set clock divider */
-- 
2.19.1

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


Re: [PATCH v4 2/2] staging: iio: ad7780: remove unnecessary stashed voltage value

2018-11-06 Thread Ardelean, Alexandru
On Mon, 2018-11-05 at 17:16 -0200, Renato Lui Geh wrote:
> This patch removes the unnecessary field int_vref_mv in ad7780_state
> referring to the device's voltage.
> 

Looks good from my side.

Alex

> Signed-off-by: Renato Lui Geh 
> ---
> Changes in v3:
>   - removed unnecessary int_vref_mv from ad7780_state
> Changes in v4:
>   - removed voltage reading on probe
> 
>  drivers/staging/iio/adc/ad7780.c | 9 +
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7780.c
> b/drivers/staging/iio/adc/ad7780.c
> index c7cb05cedbbc..52a914360574 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -42,7 +42,6 @@ struct ad7780_state {
>   struct regulator*reg;
>   struct gpio_desc*powerdown_gpio;
>   unsigned intgain;
> - u16 int_vref_mv;
>  
>   struct ad_sigma_delta sd;
>  };
> @@ -165,7 +164,7 @@ static int ad7780_probe(struct spi_device *spi)
>  {
>   struct ad7780_state *st;
>   struct iio_dev *indio_dev;
> - int ret, voltage_uv = 0;
> + int ret;
>  
>   indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
>   if (!indio_dev)
> @@ -185,16 +184,10 @@ static int ad7780_probe(struct spi_device *spi)
>   dev_err(&spi->dev, "Failed to enable specified AVdd
> supply\n");
>   return ret;
>   }
> - voltage_uv = regulator_get_voltage(st->reg);
>  
>   st->chip_info =
>   &ad7780_chip_info_tbl[spi_get_device_id(spi)->driver_data];
>  
> - if (voltage_uv)
> - st->int_vref_mv = voltage_uv / 1000;
> - else
> - dev_warn(&spi->dev, "Reference voltage unspecified\n");
> -
>   spi_set_drvdata(spi, indio_dev);
>  
>   indio_dev->dev.parent = &spi->dev;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] cedrus: check if kzalloc() fails

2018-11-06 Thread Mauro Carvalho Chehab
As warned by static code analizer checkers:
drivers/staging/media/sunxi/cedrus/cedrus.c: 
drivers/staging/media/sunxi/cedrus/cedrus.c:93 cedrus_init_ctrls() error: 
potential null dereference 'ctx->ctrls'.  (kzalloc returns null)

The problem is that it assumes that kzalloc() will always
succeed.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/media/sunxi/cedrus/cedrus.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c 
b/drivers/staging/media/sunxi/cedrus/cedrus.c
index dd121f66fa2d..6a73a7841303 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -72,6 +72,8 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct 
cedrus_ctx *ctx)
ctrl_size = sizeof(ctrl) * CEDRUS_CONTROLS_COUNT + 1;
 
ctx->ctrls = kzalloc(ctrl_size, GFP_KERNEL);
+   if (!ctx->ctrls)
+   return -ENOMEM;
memset(ctx->ctrls, 0, ctrl_size);
 
for (i = 0; i < CEDRUS_CONTROLS_COUNT; i++) {
-- 
2.19.1

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


[PATCH] media: platform: fix platform_no_drv_owner.cocci warnings

2018-11-06 Thread kbuild test robot
From: kbuild test robot 

drivers/staging/media/sunxi/cedrus/cedrus.c:421:3-8: No need to set .owner 
here. The core will do it.

 Remove .owner field if calls are used which set it automatically

Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci

Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
CC: Paul Kocialkowski 
Signed-off-by: kbuild test robot 
---

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   163c8d54a997153ee1a1e07fcac087492ad85b37
commit: 50e761516f2b8c0cdeb31a8c6ca1b4ef98cd13f1 media: platform: Add Cedrus 
VPU decoder driver

 cedrus.c |1 -
 1 file changed, 1 deletion(-)

--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -418,7 +418,6 @@ static struct platform_driver cedrus_dri
.remove = cedrus_remove,
.driver = {
.name   = CEDRUS_NAME,
-   .owner  = THIS_MODULE,
.of_match_table = of_match_ptr(cedrus_dt_match),
},
 };
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] media: dm365_ipipeif: better annotate a fall though

2018-11-06 Thread Hans Verkuil
On 11/06/18 11:55, Mauro Carvalho Chehab wrote:
> Shut up this warning:
> 
>   drivers/staging/media/davinci_vpfe/dm365_ipipeif.c: In function 
> 'ipipeif_hw_setup':
>   drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:298:3: warning: this 
> statement may fall through [-Wimplicit-fallthrough=]
>  switch (isif_port_if) {
>  ^~
>   drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:314:2: note: here
> case IPIPEIF_SDRAM_YUV:
> ^~~~
> 
> By annotating a fall though case at the right place.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

Thanks!

Hans

> ---
>  drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c 
> b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> index a53231b08d30..e3425bf082ae 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> @@ -310,6 +310,7 @@ static int ipipeif_hw_setup(struct v4l2_subdev *sd)
>   ipipeif_write(val, ipipeif_base_addr, IPIPEIF_CFG2);
>   break;
>   }
> + /* fall through */
>  
>   case IPIPEIF_SDRAM_YUV:
>   /* Set clock divider */
> 

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


Re: [PATCH] cedrus: check if kzalloc() fails

2018-11-06 Thread Maxime Ripard
On Tue, Nov 06, 2018 at 06:21:29AM -0500, Mauro Carvalho Chehab wrote:
> As warned by static code analizer checkers:
>   drivers/staging/media/sunxi/cedrus/cedrus.c: 
> drivers/staging/media/sunxi/cedrus/cedrus.c:93 cedrus_init_ctrls() error: 
> potential null dereference 'ctx->ctrls'.  (kzalloc returns null)
> 
> The problem is that it assumes that kzalloc() will always
> succeed.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Maxime Ripard 

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


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


Re: [PATCH] media: platform: fix platform_no_drv_owner.cocci warnings

2018-11-06 Thread Maxime Ripard
On Tue, Nov 06, 2018 at 07:33:19PM +0800, kbuild test robot wrote:
> From: kbuild test robot 
> 
> drivers/staging/media/sunxi/cedrus/cedrus.c:421:3-8: No need to set .owner 
> here. The core will do it.
> 
>  Remove .owner field if calls are used which set it automatically
> 
> Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci
> 
> Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
> CC: Paul Kocialkowski 
> Signed-off-by: kbuild test robot 

Acked-by: Maxime Ripard 

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


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


Re: [PATCH] staging: Remove the mt29f_spinand driver

2018-11-06 Thread Miquel Raynal
Hi Greg,

Greg Kroah-Hartman  wrote on Mon, 5 Nov
2018 15:34:33 +0100:

> On Mon, Nov 05, 2018 at 03:27:34PM +0100, Miquel Raynal wrote:
> > Hi Greg,
> > 
> > Greg Kroah-Hartman  wrote on Mon, 5 Nov
> > 2018 14:29:49 +0100:
> >   
> > > On Mon, Nov 05, 2018 at 11:12:27AM +0100, Miquel Raynal wrote:  
> > > > Hi Greg,
> > > > 
> > > > Boris Brezillon  wrote on Mon, 22 Oct 2018
> > > > 22:10:59 +0200:
> > > > 
> > > > > A new SPI NAND subsystem has been added in drivers/mtd/nand/spi/ and
> > > > > Micron's MT29F devices are now supported in
> > > > > drivers/mtd/nand/spi/micron.c.
> > > > > 
> > > > > Remove the old driver.
> > > > > 
> > > > > Signed-off-by: Boris Brezillon 
> > > > > ---
> > > > > Hello,
> > > > > 
> > > > > If anything is missing in drivers/mtd/nand/spi/micron.c to properly
> > > > > support the devices supported by the mt29f_spinand driver, please let
> > > > > me know.
> > > > > I might accept to delay removal of this driver if I have some 
> > > > > guarantees
> > > > > that existing users will actually switch to the new driver at some
> > > > > point.
> > > > > 
> > > > > Regards,
> > > > > 
> > > > > Boris
> > > > > ---
> > > > 
> > > > I plan to apply this patch but I would like your approval first.
> > > > 
> > > > As a summary, the mt29f_spinand driver is a Micron SPI NAND chip
> > > > driver interfacing with the raw NAND API (which is 'wrong').
> > > > 
> > > > Boris has recently contributed a SPI NAND framework supporting SPI
> > > > NAND chips from several vendors, including Micron, that is supposed to
> > > > take over this driver.
> > > > 
> > > > Do you see anything that should prevent us to remove it now?
> > > 
> > > Not at all, I was going to add this patch to my tree right now, as I
> > > couldn't do anything until after 4.20-rc1 was out.  Any objection from
> > > me just taking it that way and getting it into 4.20-final?  
> > 
> > I'm fine with you taking it but I have changes in the pipe that are
> > impacted by this removal so it would be great if this could happen
> > pretty early in the 4.20 release cycle (4.20-rc2?), so I will still be
> > able to base nand/next on top of it.  
> 
> If you need to work on top of this, please take it through your tree and
> feel free to add:
> 
> Acked-by: Greg Kroah-Hartman 
> 
> That way I'm not holding up any work from anyone else.

Sure, patch applied (nand/next).

Thanks,
Miquèl
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] x86/hyper-v: fix a commit description style in the comments

2018-11-06 Thread Peng Hao


From: Peng Hao 

Use commit description style 'commit <12+ chars of sha1> ("")'.

Signed-off-by: Peng Hao 
---
 arch/x86/include/asm/mshyperv.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index f377044..b56b77c 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -386,7 +386,8 @@ static inline u64 hv_read_tsc_page_tsc(const struct 
ms_hyperv_tsc_page *tsc_pg,
 *   A special '0' value indicates the time source is unreliable and we
 *   need to use something else. The currently published specification
 *   versions (up to 4.0b) contain a mistake and wrongly claim '-1'
-*   instead of '0' as the special value, see commit c35b82ef0294.
+*   instead of '0' as the special value, see commit c35b82ef0294 (
+*   "drivers/hv: correct tsc page sequence invalid value").
 * - ReferenceTime =
 *((RDTSC() * ReferenceTscScale) >> 64) + ReferenceTscOffset
 * - READ ReferenceTscSequence again. In case its value has changed
-- 
1.8.3.1


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


Re: [PATCH RFC 09/18] staging: vchiq_core: do not initialize semaphores twice

2018-11-06 Thread Nicolas Saenz Julienne
Hi Stefan,
thanks for spending the time reviewing the code. I took note of the
rest of comments.

On Sun, 2018-10-28 at 21:45 +0100, Stefan Wahren wrote:
> Hi Nicolas,
> 
> > Nicolas Saenz Julienne  hat am 26. Oktober
> > 2018 um 15:48 geschrieben:
> > 
> > 
> > vchiq_init_state() initialises a series of semaphores to then call
> > remote_event_create() on the same semaphores, which initializes
> > them
> > again.
> 
> i would prefer to have all init stuff at one place in
> vchiq_init_state() and drop this ugliness from remote_event_create()
> instead. Is this possible?

As I'm sure you're aware of, REMOTE_EVENT_T is shared between the CPU
and VC4, which can't be expanded. And since storing a pointer is out of
question because of arm64, I can only think of storing an index to an
array of completions in the shared structure instead of the pointer
magic implemented right now. It would be a little more explicit. Then
we could completely decouple both initializations. I'm not sure if it's
similar to what you had in mind. 

On a semi-related topic, I'm curious to know why these shared
structures aren't set with the "__packed" preprocessor macro. Any
ideas? As fas as I've been told, in general, the compiler may reorder
or add unexpected padding to any structure. Which would be very bad in
this case.

Regards,
Nicolas


signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH RFC 09/18] staging: vchiq_core: do not initialize semaphores twice

2018-11-06 Thread Stefan Wahren
Am 06.11.18 um 16:41 schrieb Nicolas Saenz Julienne:
> Hi Stefan,
> thanks for spending the time reviewing the code. I took note of the
> rest of comments.
>
> On Sun, 2018-10-28 at 21:45 +0100, Stefan Wahren wrote:
>> Hi Nicolas,
>>
>>> Nicolas Saenz Julienne  hat am 26. Oktober
>>> 2018 um 15:48 geschrieben:
>>>
>>>
>>> vchiq_init_state() initialises a series of semaphores to then call
>>> remote_event_create() on the same semaphores, which initializes
>>> them
>>> again.
>> i would prefer to have all init stuff at one place in
>> vchiq_init_state() and drop this ugliness from remote_event_create()
>> instead. Is this possible?
> As I'm sure you're aware of, REMOTE_EVENT_T is shared between the CPU
> and VC4, which can't be expanded. And since storing a pointer is out of
> question because of arm64, I can only think of storing an index to an
> array of completions in the shared structure instead of the pointer
> magic implemented right now. It would be a little more explicit. Then
> we could completely decouple both initializations. I'm not sure if it's
> similar to what you had in mind. 

I don't think so, this was my intention:

 static inline void
 remote_event_create(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event)
 {
    event->armed = 0;
    /* Don't clear the 'fired' flag because it may already have been set
    ** by the other side. */
-    sema_init((struct semaphore *)((char *)state + event->event), 0);
 }


>
> On a semi-related topic, I'm curious to know why these shared
> structures aren't set with the "__packed" preprocessor macro. Any
> ideas? As fas as I've been told, in general, the compiler may reorder
> or add unexpected padding to any structure. Which would be very bad in
> this case.

This would be better, but i assume the firmware side uses the same
source code. So using __packed only on ARM side could also break :-(

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


Re: [PATCH v3 0/4] Improve VCHIQ cache line size handling

2018-11-06 Thread Stefan Wahren
> Phil Elwell  hat am 17. September 2018 um 20:01 
> geschrieben:
> 
> 
> On 17/09/2018 18:51, Florian Fainelli wrote:
> > On 09/17/2018 04:47 AM, Phil Elwell wrote:
> >> Hi Stefan,
> >>
> >> On 17/09/2018 12:39, Stefan Wahren wrote:
> >>> Hi Phil,
> >>>
> >>> Am 17.09.2018 um 10:22 schrieb Phil Elwell:
>  Both sides of the VCHIQ communications mechanism need to agree on the 
>  cache
>  line size. Using an incorrect value can lead to data corruption, but 
>  having the
>  two sides using different values is usually worse.
> 
>  In the absence of an obvious convenient run-time method to determine the
>  correct value in the ARCH=arm world, the downstream Raspberry Pi trees 
>  used a
>  Device Tree property, written by the firmware, to configure the kernel 
>  driver.
>  This method was vetoed during the upstreaming process, so a fixed value 
>  of 32
>  was used instead, and some corruptions ensued. This is take 2 at 
>  arriving at
>  the correct value.
> 
>  Add a new compatible string - "brcm,bcm2836-vchiq" - to indicate an SoC 
>  with
>  a 64-byte cache line. Document the new string in the binding, and use it 
>  on
>  the appropriate platforms.
> 
>  The final patch is a (seemingly cosmetic) correction of the Device Tree 
>  "reg"
>  declaration for the device node, but it doubles as an indication to the
>  Raspberry Pi firmware that the kernel driver is running a recent kernel 
>  driver
>  that chooses the correct value. As such it would help if the DT patches 
>  are
>  not merged before the driver patch.
> 
>  v3: Builds without errors, tested on multiple Raspberry Pi models.
>  v2: Replaced ARM-specific logic used to determine cache line size with
>   a new compatible string for BCM2836 and BCM2837.
> 
>  Phil Elwell (4):
> staging/vc04_services: Use correct cache line size
> dt-bindings: soc: Document "brcm,bcm2836-vchiq"
> ARM: dts: bcm283x: Correct vchiq compatible string
> ARM: dts: bcm283x: Correct mailbox register sizes
> >>>
> >>> since my pull requests are out, would it be okay to apply patch #1 for
> >>> 4.20 and the DT stuff for 4.21 (with the assumption Rob is okay with
> >>> these patches)?
> >>
> >> Patch 4 is the only one I'd like to be delayed, but delaying 2-4 is fine 
> >> with me.
> > 
> > Humm, did you mean you would like not to be delayed? In any case Stefan,
> > you can send an additional pull request, and I will merge it and send a
> > second pull request towards ARM SoC maintainers, that's not a problem.
> 
> No, I meant what I wrote - I would prefer patch 1 to be merged before patch 4 
> (or at least
> in the same release) to avoid the need for another firmware change, hence 
> delaying patch
> 4 is good. It makes sense for the other commits to be merged in that order, 
> but the
> normal compatible-string fallback mechanism means there is no hard dependency 
> there.
> 
> Phil

Patches #2 - #4 applied to bcm2835-dt-next

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


Re: [PATCH RFC 09/18] staging: vchiq_core: do not initialize semaphores twice

2018-11-06 Thread Nicolas Saenz Julienne
On Tue, 2018-11-06 at 17:06 +0100, Stefan Wahren wrote:
> Am 06.11.18 um 16:41 schrieb Nicolas Saenz Julienne:
> > Hi Stefan,
> > thanks for spending the time reviewing the code. I took note of the
> > rest of comments.
> > 
> > On Sun, 2018-10-28 at 21:45 +0100, Stefan Wahren wrote:
> > > Hi Nicolas,
> > > 
> > > > Nicolas Saenz Julienne  hat am 26.
> > > > Oktober
> > > > 2018 um 15:48 geschrieben:
> > > > 
> > > > 
> > > > vchiq_init_state() initialises a series of semaphores to then
> > > > call
> > > > remote_event_create() on the same semaphores, which initializes
> > > > them
> > > > again.
> > > i would prefer to have all init stuff at one place in
> > > vchiq_init_state() and drop this ugliness from
> > > remote_event_create()
> > > instead. Is this possible?
> > As I'm sure you're aware of, REMOTE_EVENT_T is shared between the
> > CPU
> > and VC4, which can't be expanded. And since storing a pointer is
> > out of
> > question because of arm64, I can only think of storing an index to
> > an
> > array of completions in the shared structure instead of the pointer
> > magic implemented right now. It would be a little more explicit.
> > Then
> > we could completely decouple both initializations. I'm not sure if
> > it's
> > similar to what you had in mind. 
> 
> I don't think so, this was my intention:
> 
>  static inline void
>  remote_event_create(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event)
>  {
> event->armed = 0;
> /* Don't clear the 'fired' flag because it may already have been
> set
> ** by the other side. */
> -sema_init((struct semaphore *)((char *)state + event->event),
> 0);
>  }

Fair enough, even simpler.

> 
> 
> > On a semi-related topic, I'm curious to know why these shared
> > structures aren't set with the "__packed" preprocessor macro. Any
> > ideas? As fas as I've been told, in general, the compiler may
> > reorder
> > or add unexpected padding to any structure. Which would be very bad
> > in
> > this case.
> 
> This would be better, but i assume the firmware side uses the same
> source code. So using __packed only on ARM side could also break :-(

True. Yet in that case, aren't we still relying on the fact that both
compilers are going to behave the same way?
 
> 
> > Regards,
> > Nicolas



signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


PERSONAL FROM AFGHANISTAN

2018-11-06 Thread Mr. X



Dear Friend



I am sorry to encroach into your privacy in this manner.My name is Scott Metz 
an American soldier serving in the Military with the 25th Brigade Support 
Battalion, 1st Stryker Brigade Combat Team, 25th Infantry Division in 
Afghanistan.My reason for contacting you is that we managed to move funds 
donated by the US government for the reconstruction of the Iraqi/Afghanistan 
land but this act was prompted because as serving military officers who have 
been deployed to both Iraq and Afghanistan we know that this money was never 
going to be used appropriately by those greedy leader in both nations,this huge 
donations running into hundred of millions in American dollar is sent out here 
on a yearly basis and the money is American tax payers money and because over 
the years we have watched how donations made by the American government have 
been swindled by leaders of these countries thereby not making use of the funds 
and as such the decision to take from this national cake arises.You m
 ay condemn my deeds but sincerely i have served the Military in good faith and 
i have never
been involved in any illegal act and the shameful plight of many US Military 
vets have pushed me to secure my future in the best way that i can.Without 
mincing words,i would want to confide in you that i have in my possession the 
sum of $2,980,000.00USD (Two million Nine hundred and eighty thousand American 
Dollars)The above sum was given to me as my share and to conceal this amount of 
money became a problem for me.



I have this money stored some where with the help of a German contact working 
here who's office enjoys diplomatic immunity i was able to get it secured in a 
safe Military tagged consignment entirely out of trouble spot in Kabul waiting 
for a moment like this to put the money in good use with the help of a 
reputable and sincere person for investment purposes.This is the reason for 
contacting you i am ready to compensate you with 40% of the funds.The only 
thing i require from you is just for you to help me receive this funds once i 
move the money out of Afghanistan because i am in a war zone and also been a 
uniform man i can not parade with such an amount.I have already mapped out 
strategy to move the money out of Afghanistan to your location with the aid of 
the diplomat and the diplomat certainly does not know the real contents of the 
consignments and he believes that it belongs to a Late Franco-American soldier 
Spc.Aguila Francisco Xavier,who was attacked and killed by enemy 
 with small arms fire and before giving up he trusted me to hand over the 
package to his family.



With my present arrangement/procedure to evacuate this money out of here i am 
very positive that within two weeks it will get to your location.I shall 
provide you with the full details as soon as i receive your response.Right now 
i am very careful with the way i communicate so as to reduce any kind of risk 
until this money is finally in your custody.I will be communicating with you 
through email alone because our phone conversation might be monitored by unit 
bugs,i am doing this on trust and so i would want you to put aside any act of 
greed as we have a lot to gain in this project.Can I trust you? When you 
receive this message please kindly send me an e-mail signifying your interest.



http://www.military.com/daily-news/2017/03/25/military-members-stole-millions-in-afghan-rebuilding-effort.html




Respectfully,
Captain Scott Metz


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


Re: [alsa-devel] [PATCH v2] staging: bcm2835-audio: interpolate audio delay

2018-11-06 Thread Mike Brady

> On 5 Nov 2018, at 16:11, Takashi Iwai  wrote:
> 
> On Mon, 05 Nov 2018 16:57:07 +0100,
> Mike Brady wrote:
>> 
>>> One another thing I'd like to point out is that the value given in the
>>> patch is nothing but an estimated position, optimistically calculated
>>> via the system timer.  Mike and I had already discussion in another
>>> thread, and another possible option would be to provide the proper
>>> timestamp-vs-hwptr pair, instead of updating the timestamp always at
>>> the status read.
>> 
>> Agreed — that would give the caller the information needed to do the
>> interpolation for themselves if desired.
> 
> And now I wonder whether the problem is still present with the latest
> code.  There was a (kind of) regression in this regard when we
> introduced the fine-grained hardware timestamping, but it should have
> been addressed by the commit 20e3f985bb875fea4f86b04eba4b6cc29bfd6b71
>ALSA: pcm: update tstamp only if audio_tstamp changed
> 
> Could you double-check whether the tstamp field gets still updated
> even if no hwptr (and delay) is changed?

Yes, this could be a bit problematic. The function update_audio_tstamp in 
pcm_lib.c could include the interpolated delay in the calculation of 
audio_tstamp, and hence
could trigger the update of tstamp.

Another issue, as I see it, is that the the audio_tstamp value would depend on 
whether, and when, a snd_pcm_delay() call (which recalculates the interpolation 
and puts it into the delay field) was made immediately prior to it. By zeroing 
the delay when a GPU interrupt occurs, you could be certain that the 
interpolated delay would be less than or equal to the true delay, but this 
doesn’t seem very satisfactory — you have neither the timestamp of the last 
update nor the correctly interpolated timestamp. 

Sadly, therefore, I’m now of the view that this approach to interpolating the 
delay between GPU interrupts is not really viable. Would that be your view?

Regards
Mike

> thanks,
> 
> Takashi

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


Re: [RFC PATCH v2] android: ion: How to properly clean caches for uncached allocations

2018-11-06 Thread Liam Mark
On Fri, 2 Nov 2018, John Stultz wrote:

> On Thu, Nov 1, 2018 at 3:15 PM, Liam Mark  wrote:
> > Based on the suggestions from Laura I created a first draft for a change
> > which will attempt to ensure that uncached mappings are only applied to
> > ION memory who's cache lines have been cleaned.
> > It does this by providing cached mappings (for uncached ION allocations)
> > until the ION buffer is dma mapped and successfully cleaned, then it drops
> > the userspace mappings and when pages are accessed they are faulted back
> > in and uncached mappings are created.
> >
> > This change has the following potential disadvantages:
> > - It assumes that userpace clients won't attempt to access the buffer
> > while it is being mapped as we are removing the userpspace mappings at
> > this point (though it is okay for them to have it mapped)
> > - It assumes that kernel clients won't hold a kernel mapping to the buffer
> > (ie dma_buf_kmap) while it is being dma-mapped. What should we do if there
> > is a kernel mapping at the time of dma mapping, fail the mapping, warn?
> > - There may be a performance penalty as a result of having to fault in the
> > pages after removing the userspace mappings.
> >
> > It passes basic testing involving reading writing and reading from
> > uncached system heap allocations before and after dma mapping.
> >
> > Please let me know if this is heading in the right direction and if there
> > are any concerns.
> >
> > Signed-off-by: Liam Mark 
> 
> 
> Thanks for sending this out! I gave this a whirl on my HiKey960. Seems
> to work ok, but I'm not sure if the board's usage benefits much from
> your changes.
> 

Thanks for testing this.
I didn't expect this patch to improve performance but I was worried it
might hurt performance.

I don't know how many uncached ION allocations Hikey960 makes, or how it
uses uncached allocations.

It is possible that Hikey960 doesn't make much usage of uncached buffers,
or if it does it may not attempt to mmap them before dma mapping them,
so it is possible this change isn't getting exercised very much in the
test you ran.

I will need to look into how best to exercise this patch on Hikey960.

Liam

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [alsa-devel] [PATCH v2] staging: bcm2835-audio: interpolate audio delay

2018-11-06 Thread Takashi Iwai
On Tue, 06 Nov 2018 22:05:11 +0100,
Mike Brady wrote:
> 
> 
> > On 5 Nov 2018, at 16:11, Takashi Iwai  wrote:
> > 
> > On Mon, 05 Nov 2018 16:57:07 +0100,
> > Mike Brady wrote:
> >> 
> >>> One another thing I'd like to point out is that the value given in the
> >>> patch is nothing but an estimated position, optimistically calculated
> >>> via the system timer.  Mike and I had already discussion in another
> >>> thread, and another possible option would be to provide the proper
> >>> timestamp-vs-hwptr pair, instead of updating the timestamp always at
> >>> the status read.
> >> 
> >> Agreed — that would give the caller the information needed to do the
> >> interpolation for themselves if desired.
> > 
> > And now I wonder whether the problem is still present with the latest
> > code.  There was a (kind of) regression in this regard when we
> > introduced the fine-grained hardware timestamping, but it should have
> > been addressed by the commit 20e3f985bb875fea4f86b04eba4b6cc29bfd6b71
> >ALSA: pcm: update tstamp only if audio_tstamp changed
> > 
> > Could you double-check whether the tstamp field gets still updated
> > even if no hwptr (and delay) is changed?
> 
> Yes, this could be a bit problematic. The function update_audio_tstamp in 
> pcm_lib.c could include the interpolated delay in the calculation of 
> audio_tstamp, and hence
> could trigger the update of tstamp.

Well, my question is about the current driver as-is.
It has no runtime->delay, so far, hence audio_tstamp is calculated
only from the hwptr position.  As the corresponding tstamp gets
updated only when the audio_tstamp (i.e. hwptr) is updated, the driver
should provide the consistent pair of audio_tstamp (i.e. hwptr) vs
tstamp.

> Another issue, as I see it, is that the the audio_tstamp value would depend 
> on whether, and when, a snd_pcm_delay() call (which recalculates the 
> interpolation and puts it into the delay field) was made immediately prior to 
> it. By zeroing the delay when a GPU interrupt occurs, you could be certain 
> that the interpolated delay would be less than or equal to the true delay, 
> but this doesn’t seem very satisfactory — you have neither the timestamp of 
> the last update nor the correctly interpolated timestamp. 

No, audio_stamp field is updated at snd_pcm_period_elapsed() call as
well as tstamp field.

Basically the driver provides three things: hwptr, tstamp and
audio_tstamp.  For the default configuration (like bcm audio does),
audio_tstamp is calculated from hwptr, so it can be seen as the hwptr
represented in timespec.  OTOH, tstamp is the actual system time that
is updated only when audio_tstamp changes -- which means tstamp gets
updated *only* at snd_pcm_period_elapsed() call on bcm audio.

And, my point is that you should be able to interpolate the actual
position in user-space side based on these information; it doesn't
have to be done in the kernel at all.

> Sadly, therefore, I’m now of the view that this approach to interpolating the 
> delay between GPU interrupts is not really viable. Would that be your view?

Actually there were some bugs in the past that the tstamp was updated
at each snd_pcm_status(), but it should have been fixed in the recent
kernels.  That's why I asked to re-check the current status.


thanks,

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


[PATCH] staging: rtl8723bs: Correct errors from checkpatch

2018-11-06 Thread Josenivaldo Benito Jr
Correct following errors reported by checkpath.pl:

ERROR: space required before the open parenthesis '(' #265: FILE:
drivers/staging/rtl8723bs/hal/rtl8723bs_recv.c:265:
+   if(!precvframe) ')'

Also similar errors on line 274 and 283.

Signed-off-by: Josenivaldo Benito Jr 

---

I am new to the community, working on a study group called LKCamp.
Please provide any feedback to this patch so I can correct and learn.

Thanks
---
 drivers/staging/rtl8723bs/hal/rtl8723bs_recv.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/rtl8723bs_recv.c 
b/drivers/staging/rtl8723bs/hal/rtl8723bs_recv.c
index 8507794..23d6cb6 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723bs_recv.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723bs_recv.c
@@ -262,7 +262,7 @@ static void rtl8723bs_recv_tasklet(void *priv)

while (ptr < precvbuf->ptail) {
precvframe = try_alloc_recvframe(precvpriv, precvbuf);
-   if(!precvframe)
+   if (!precvframe)
return;

/* rx desc parsing */
@@ -271,7 +271,7 @@ static void rtl8723bs_recv_tasklet(void *priv)

pattrib = &precvframe->u.hdr.attrib;

-   if(rx_crc_err(precvpriv, p_hal_data,
+   if (rx_crc_err(precvpriv, p_hal_data,
  pattrib, precvframe))
break;

@@ -280,7 +280,7 @@ static void rtl8723bs_recv_tasklet(void *priv)
pattrib->shift_sz +
pattrib->pkt_len;

-   if(pkt_exceeds_tail(precvpriv, ptr + pkt_offset,
+   if (pkt_exceeds_tail(precvpriv, ptr + pkt_offset,
precvbuf->ptail, precvframe))
break;

--
2.7.4

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


[PATCH] binder: fix race that allows malicious free of live buffer

2018-11-06 Thread Todd Kjos
Malicious code can attempt to free buffers using the
BC_FREE_BUFFER ioctl to binder. There are protections
against a user freeing a buffer while in use by the
kernel, however there was a window where BC_FREE_BUFFER
could be used to free a recently allocated buffer that
was not completely initialized. This resulted in a
use-after-free detected by KASAN with a malicious
test program.

This window is closed by setting the buffer's
allow_user_free attribute to 0 when the buffer
is allocated or when the user has previously
freed it instead of waiting for the caller
to set it. The problem was that when the struct
buffer was recycled, allow_user_free was stale
and set to 1 allowing a free to go through.

Signed-off-by: Todd Kjos 
Acked-by: Arve Hjønnevåg 
---
 drivers/android/binder.c   | 21 -
 drivers/android/binder_alloc.c | 16 ++--
 drivers/android/binder_alloc.h |  3 +--
 3 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index cb30a524d16d8..9f1000d2a40c7 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2974,7 +2974,6 @@ static void binder_transaction(struct binder_proc *proc,
t->buffer = NULL;
goto err_binder_alloc_buf_failed;
}
-   t->buffer->allow_user_free = 0;
t->buffer->debug_id = t->debug_id;
t->buffer->transaction = t;
t->buffer->target_node = target_node;
@@ -3510,14 +3509,18 @@ static int binder_thread_write(struct binder_proc *proc,
 
buffer = binder_alloc_prepare_to_free(&proc->alloc,
  data_ptr);
-   if (buffer == NULL) {
-   binder_user_error("%d:%d BC_FREE_BUFFER 
u%016llx no match\n",
-   proc->pid, thread->pid, (u64)data_ptr);
-   break;
-   }
-   if (!buffer->allow_user_free) {
-   binder_user_error("%d:%d BC_FREE_BUFFER 
u%016llx matched unreturned buffer\n",
-   proc->pid, thread->pid, (u64)data_ptr);
+   if (IS_ERR_OR_NULL(buffer)) {
+   if (PTR_ERR(buffer) == -EPERM) {
+   binder_user_error(
+   "%d:%d BC_FREE_BUFFER u%016llx 
matched unreturned or currently freeing buffer\n",
+   proc->pid, thread->pid,
+   (u64)data_ptr);
+   } else {
+   binder_user_error(
+   "%d:%d BC_FREE_BUFFER u%016llx 
no match\n",
+   proc->pid, thread->pid,
+   (u64)data_ptr);
+   }
break;
}
binder_debug(BINDER_DEBUG_FREE_BUFFER,
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 64fd96eada31f..030c98f35cca7 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -151,16 +151,12 @@ static struct binder_buffer 
*binder_alloc_prepare_to_free_locked(
else {
/*
 * Guard against user threads attempting to
-* free the buffer twice
+* free the buffer when in use by kernel or
+* after it's already been freed.
 */
-   if (buffer->free_in_progress) {
-   binder_alloc_debug(BINDER_DEBUG_USER_ERROR,
-  "%d:%d FREE_BUFFER u%016llx 
user freed buffer twice\n",
-  alloc->pid, current->pid,
-  (u64)user_ptr);
-   return NULL;
-   }
-   buffer->free_in_progress = 1;
+   if (!buffer->allow_user_free)
+   return ERR_PTR(-EPERM);
+   buffer->allow_user_free = 0;
return buffer;
}
}
@@ -500,7 +496,7 @@ static struct binder_buffer *binder_alloc_new_buf_locked(
 
rb_erase(best_fit, &alloc->free_buffers);
buffer->free = 0;
-   buffer->free_in_progress = 0;
+   buffer->allow_user_free = 0;
binder_insert_allocated_buffer_locked(alloc, buffer);
binder_alloc_debug(BINDER_DEBUG_BUFFER_ALLOC,
 "%d: binder_alloc_buf size %zd got %pK\n",
diff --git a/drivers/android/binder_alloc.h b/drivers/android/binde

Re: [PATCH V2] binder: ipc namespace support for android binder

2018-11-06 Thread Andrew Morton
On Mon, 29 Oct 2018 06:18:11 + chouryzhou(周威)  
wrote:

>   We are working for running android in container, but we found that binder is
> not isolated by ipc namespace. Since binder is a form of IPC and therefore 
> should
> be tied to ipc namespace. With this patch, we can run more than one android
> container on one host.
>   This patch move "binder_procs" and "binder_context" into ipc_namespace,
> driver will find the context from it when opening. Althought statistics in 
> debugfs
> remain global.
> 
> ...
>
> --- a/ipc/namespace.c
> +++ b/ipc/namespace.c
> @@ -56,6 +56,9 @@ static struct ipc_namespace *create_ipc_ns(struct 
> user_namespace *user_ns,
> ns->ucounts = ucounts;
>  
> err = mq_init_ns(ns);
> +   if (err)
> +   goto fail_put;
> +   err = binder_init_ns(ns);
> if (err)
> goto fail_put;
>  

Don't we need an mq_put_mnt() if binder_init_ns() fails?

free_ipc_ns() seems to have forgotten about that too.  In which case it
must be madly leaking mounts so probably I'm wrong.  Confused.

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


[PATCH] Staging: vchi: Add license id and change int type

2018-11-06 Thread andrealmeid
From: André Almeida 

Fix the following checkpatch issues:

WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
CHECK: Prefer kernel type 's32' over 'int32_t'

Signed-off-by: André Almeida 

---

Hello! This is my first patch to Linux Kernel. Let me know any feedback
---
 drivers/staging/vc04_services/interface/vchi/vchi_mh.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h 
b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
index 198bd076b666..42fc0c693d43 100644
--- a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
+++ b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
@@ -1,5 +1,5 @@
-/**
- * Copyright (c) 2010-2012 Broadcom. All rights reserved.
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2010-2012 Broadcom. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -36,7 +36,7 @@
 
 #include 
 
-typedef int32_t VCHI_MEM_HANDLE_T;
+typedef s32 VCHI_MEM_HANDLE_T;
 #define VCHI_MEM_HANDLE_INVALID 0
 
 #endif
-- 
2.19.1

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


[PATCH] binder: fix sparse warnings on locking context

2018-11-06 Thread Todd Kjos
Add __acquire()/__release() annnotations to fix warnings
in sparse context checking

There is one case where the warning was due to a lack of
a "default:" case in a switch statement where a lock was
being released in each of the cases, so the default
case was added.

Signed-off-by: Todd Kjos 
---
 drivers/android/binder.c   | 43 +-
 drivers/android/binder_alloc.c |  1 +
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 9f1000d2a40c7..9f2059d24ae2d 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -660,6 +660,7 @@ struct binder_transaction {
 #define binder_proc_lock(proc) _binder_proc_lock(proc, __LINE__)
 static void
 _binder_proc_lock(struct binder_proc *proc, int line)
+   __acquires(&proc->outer_lock)
 {
binder_debug(BINDER_DEBUG_SPINLOCKS,
 "%s: line=%d\n", __func__, line);
@@ -675,6 +676,7 @@ _binder_proc_lock(struct binder_proc *proc, int line)
 #define binder_proc_unlock(_proc) _binder_proc_unlock(_proc, __LINE__)
 static void
 _binder_proc_unlock(struct binder_proc *proc, int line)
+   __releases(&proc->outer_lock)
 {
binder_debug(BINDER_DEBUG_SPINLOCKS,
 "%s: line=%d\n", __func__, line);
@@ -690,6 +692,7 @@ _binder_proc_unlock(struct binder_proc *proc, int line)
 #define binder_inner_proc_lock(proc) _binder_inner_proc_lock(proc, __LINE__)
 static void
 _binder_inner_proc_lock(struct binder_proc *proc, int line)
+   __acquires(&proc->inner_lock)
 {
binder_debug(BINDER_DEBUG_SPINLOCKS,
 "%s: line=%d\n", __func__, line);
@@ -705,6 +708,7 @@ _binder_inner_proc_lock(struct binder_proc *proc, int line)
 #define binder_inner_proc_unlock(proc) _binder_inner_proc_unlock(proc, 
__LINE__)
 static void
 _binder_inner_proc_unlock(struct binder_proc *proc, int line)
+   __releases(&proc->inner_lock)
 {
binder_debug(BINDER_DEBUG_SPINLOCKS,
 "%s: line=%d\n", __func__, line);
@@ -720,6 +724,7 @@ _binder_inner_proc_unlock(struct binder_proc *proc, int 
line)
 #define binder_node_lock(node) _binder_node_lock(node, __LINE__)
 static void
 _binder_node_lock(struct binder_node *node, int line)
+   __acquires(&node->lock)
 {
binder_debug(BINDER_DEBUG_SPINLOCKS,
 "%s: line=%d\n", __func__, line);
@@ -735,6 +740,7 @@ _binder_node_lock(struct binder_node *node, int line)
 #define binder_node_unlock(node) _binder_node_unlock(node, __LINE__)
 static void
 _binder_node_unlock(struct binder_node *node, int line)
+   __releases(&node->lock)
 {
binder_debug(BINDER_DEBUG_SPINLOCKS,
 "%s: line=%d\n", __func__, line);
@@ -751,12 +757,16 @@ _binder_node_unlock(struct binder_node *node, int line)
 #define binder_node_inner_lock(node) _binder_node_inner_lock(node, __LINE__)
 static void
 _binder_node_inner_lock(struct binder_node *node, int line)
+   __acquires(&node->lock) __acquires(&node->proc->inner_lock)
 {
binder_debug(BINDER_DEBUG_SPINLOCKS,
 "%s: line=%d\n", __func__, line);
spin_lock(&node->lock);
if (node->proc)
binder_inner_proc_lock(node->proc);
+   else
+   /* annotation for sparse */
+   __acquire(&node->proc->inner_lock);
 }
 
 /**
@@ -768,6 +778,7 @@ _binder_node_inner_lock(struct binder_node *node, int line)
 #define binder_node_inner_unlock(node) _binder_node_inner_unlock(node, 
__LINE__)
 static void
 _binder_node_inner_unlock(struct binder_node *node, int line)
+   __releases(&node->lock) __releases(&node->proc->inner_lock)
 {
struct binder_proc *proc = node->proc;
 
@@ -775,6 +786,9 @@ _binder_node_inner_unlock(struct binder_node *node, int 
line)
 "%s: line=%d\n", __func__, line);
if (proc)
binder_inner_proc_unlock(proc);
+   else
+   /* annotation for sparse */
+   __release(&node->proc->inner_lock);
spin_unlock(&node->lock);
 }
 
@@ -1384,10 +1398,14 @@ static void binder_dec_node_tmpref(struct binder_node 
*node)
binder_node_inner_lock(node);
if (!node->proc)
spin_lock(&binder_dead_nodes_lock);
+   else
+   __acquire(&binder_dead_nodes_lock);
node->tmp_refs--;
BUG_ON(node->tmp_refs < 0);
if (!node->proc)
spin_unlock(&binder_dead_nodes_lock);
+   else
+   __release(&binder_dead_nodes_lock);
/*
 * Call binder_dec_node() to check if all refcounts are 0
 * and cleanup is needed. Calling with strong=0 and internal=1
@@ -1890,18 +1908,22 @@ static struct binder_thread *binder_get_txn_from(
  */
 static struct binder_thread *binder_get_txn_from_and_acq_inner(
struct binder_transaction *t)
+   __acquires(&t->from->proc->inner_lock)
 {
struct binder_thread *from;
 
   

[PATCH] staging: sm750fb: Add spaces around '+'

2018-11-06 Thread Laís Pessine do Carmo
Add spaces around '+' to correct the following checkpath.pl warnings:

WARNING: line over 80 characters
+   write_dpPort(accel, *(unsigned int *)(pSrcbuf +
(j * 4)));

WARNING: line over 80 characters
+   memcpy(ajRemain, pSrcbuf+ul4BytesPerScan,
ulBytesRemain);

Signed-off-by: Laís Pessine do Carmo 

---

Hi, I am new to Linux community and I am sending this patch to learn
about the kernel development.
Please, let me know if there is anything I can improve.
Thank you!
---
 drivers/staging/sm750fb/sm750_accel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/sm750fb/sm750_accel.c 
b/drivers/staging/sm750fb/sm750_accel.c
index 1035e91e7cd3..eed840b251da 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -383,7 +383,8 @@ int sm750_hw_imageblit(struct lynx_accel *accel,
write_dpPort(accel, *(unsigned int *)(pSrcbuf + (j * 
4)));
 
if (ulBytesRemain) {
-   memcpy(ajRemain, pSrcbuf+ul4BytesPerScan, 
ulBytesRemain);
+   memcpy(ajRemain, pSrcbuf + ul4BytesPerScan,
+  ulBytesRemain);
write_dpPort(accel, *(unsigned int *)ajRemain);
}
 
-- 
2.11.0

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


[PATCH] staging: mt7621-mmc: Add blank line after declaration

2018-11-06 Thread nicolas
From: Nícolas F. R. A. Prado 

Correct the following warning from checkpatch.pl:

WARNING: Missing a blank line after declarations
+   struct msdc_host *host = mmc_priv(mmc);
+   msdc_pm(state, (void *)host);

Signed-off-by: Nícolas F. R. A. Prado 

---

Hi, this is my first commit. Let me know if there's anything I can do
better.
---
 drivers/staging/mt7621-mmc/sd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index 0379f9c96..7b66f9b0a 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -1797,6 +1797,7 @@ static void msdc_drv_pm(struct platform_device *pdev, 
pm_message_t state)
 
if (mmc) {
struct msdc_host *host = mmc_priv(mmc);
+
msdc_pm(state, (void *)host);
}
 }
-- 
2.19.1

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


RE: [PATCH V2] binder: ipc namespace support for android binder

2018-11-06 Thread 周威
> -Original Message-
> From: Andrew Morton 
> Sent: Wednesday, November 7, 2018 8:07 AM
> To: chouryzhou(周威) 
> Cc: gre...@linuxfoundation.org; a...@android.com; tk...@android.com;
> d...@stgolabs.net; de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH V2] binder: ipc namespace support for android
> binder
> 
> On Mon, 29 Oct 2018 06:18:11 + chouryzhou(周威)
>  wrote:
> 
> >   We are working for running android in container, but we found that binder
> is
> > not isolated by ipc namespace. Since binder is a form of IPC and therefore
> should
> > be tied to ipc namespace. With this patch, we can run more than one android
> > container on one host.
> >   This patch move "binder_procs" and "binder_context" into
> ipc_namespace,
> > driver will find the context from it when opening. Although statistics in
> debugfs
> > remain global.
> >
> > ...
> >
> > --- a/ipc/namespace.c
> > +++ b/ipc/namespace.c
> > @@ -56,6 +56,9 @@ static struct ipc_namespace *create_ipc_ns(struct
> user_namespace *user_ns,
> > ns->ucounts = ucounts;
> >
> > err = mq_init_ns(ns);
> > +   if (err)
> > +   goto fail_put;
> > +   err = binder_init_ns(ns);
> > if (err)
> > goto fail_put;
> >
> 
> Don't we need an mq_put_mnt() if binder_init_ns() fails?
> 
> free_ipc_ns() seems to have forgotten about that too.  In which case it
> must be madly leaking mounts so probably I'm wrong.  Confused.
> 

mq_init_ns will do clean job if it failed, and as do binder_init_ns. 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] media: staging: tegra-vde: Change from __attribute to __packed.

2018-11-06 Thread rafaelgoncalves
From: Rafael Goncalves 

Correct the following warnings from checkpatch.pl:

WARNING: __packed is preferred over __attribute__((packed))
+} __attribute__((packed));

WARNING: __packed is preferred over __attribute__((packed))
+} __attribute__((packed));

Signed-off-by: Rafael Goncalves 

---

Hi.
It's my first patch submission, please let me know if there is something
that I can improve.
---
 drivers/staging/media/tegra-vde/uapi.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/uapi.h 
b/drivers/staging/media/tegra-vde/uapi.h
index a50c7bcae057..5ffa4afa4047 100644
--- a/drivers/staging/media/tegra-vde/uapi.h
+++ b/drivers/staging/media/tegra-vde/uapi.h
@@ -29,7 +29,7 @@ struct tegra_vde_h264_frame {
__u32 flags;
 
__u32 reserved;
-} __attribute__((packed));
+} __packed;
 
 struct tegra_vde_h264_decoder_ctx {
__s32 bitstream_data_fd;
@@ -61,7 +61,7 @@ struct tegra_vde_h264_decoder_ctx {
__u8  num_ref_idx_l1_active_minus1;
 
__u32 reserved;
-} __attribute__((packed));
+} __packed;
 
 #define VDE_IOCTL_BASE ('v' + 0x20)
 
-- 
2.19.1

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


[PATCH v3] driver-staging: vsoc.c: Add sysfs support for examining the permissions of regions.

2018-11-06 Thread Jerry Lin
Add a attribute called permissions under vsoc device node for examining
current granted permissions in vsoc_device.

This file will display permissions in following format:
  begin_offset  end_offset  owner_offset  owned_value
%x  %x%x   %x

Signed-off-by: Jerry Lin 
---
 drivers/staging/android/vsoc.c | 48 +++---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/android/vsoc.c b/drivers/staging/android/vsoc.c
index 22571ab..8ce3604 100644
--- a/drivers/staging/android/vsoc.c
+++ b/drivers/staging/android/vsoc.c
@@ -128,9 +128,10 @@ struct vsoc_device {
 
 static struct vsoc_device vsoc_dev;
 
-/*
- * TODO(ghartman): Add a /sys filesystem entry that summarizes the permissions.
- */
+static ssize_t permissions_show(struct device *dev,
+   struct device_attribute *attr,
+   char *buf);
+static DEVICE_ATTR_RO(permissions);
 
 struct fd_scoped_permission_node {
struct fd_scoped_permission permission;
@@ -718,6 +719,37 @@ static ssize_t vsoc_write(struct file *filp, const char 
__user *buffer,
return len;
 }
 
+static ssize_t permissions_show(struct device *dev,
+   struct device_attribute *attr,
+   char *buffer)
+{
+   struct fd_scoped_permission_node *node;
+   char *row;
+   int ret;
+   ssize_t written = 0;
+
+   row = kzalloc(sizeof(char) * 128, GFP_KERNEL);
+   if (!row)
+   return 0;
+   mutex_lock(&vsoc_dev.mtx);
+   list_for_each_entry(node, &vsoc_dev.permissions, list) {
+   ret = snprintf(row, 128, "%x\t%x\t%x\t%x\n",
+  node->permission.begin_offset,
+  node->permission.end_offset,
+  node->permission.owner_offset,
+  node->permission.owned_value);
+   if (ret < 0)
+   goto done;
+   memcpy(buffer + written, row, ret);
+   written += ret;
+   }
+
+done:
+   mutex_unlock(&vsoc_dev.mtx);
+   kfree(row);
+   return written;
+}
+
 static irqreturn_t vsoc_interrupt(int irq, void *region_data_v)
 {
struct vsoc_region_data *region_data =
@@ -942,6 +974,15 @@ static int vsoc_probe_device(struct pci_dev *pdev,
}
vsoc_dev.regions_data[i].device_created = true;
}
+   /*
+* Create permission attribute on device node.
+*/
+   result = device_create_file(&pdev->dev, &dev_attr_permissions);
+   if (result) {
+   dev_err(&vsoc_dev.dev->dev, "device_create_file failed\n");
+   vsoc_remove_device(pdev);
+   return -EFAULT;
+   }
return 0;
 }
 
@@ -967,6 +1008,7 @@ static void vsoc_remove_device(struct pci_dev *pdev)
if (!pdev || !vsoc_dev.dev)
return;
dev_info(&pdev->dev, "remove_device\n");
+   device_remove_file(&pdev->dev, &dev_attr_permissions);
if (vsoc_dev.regions_data) {
for (i = 0; i < vsoc_dev.layout->region_count; ++i) {
if (vsoc_dev.regions_data[i].device_created) {
-- 
2.7.4

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


[PATCH] staging: rtl8723bs: Fix incorrect sense of ether_addr_equal

2018-11-06 Thread Larry Finger
In commit b37f9e1c3801 ("staging: rtl8723bs: Fix lines too long in
update_recvframe_attrib()."), the refactoring involved replacing
two memcmp() calls with ether_addr_equal() calls. What the author
missed is that memcmp() returns false when the two strings are equal,
whereas ether_addr_equal() returns true when the two addresses are
equal. One side effect of this error is that the strength of an
unassociated AP was much stronger than the same AP after association.
This bug is reported at bko#201611.

Fixes: b37f9e1c3801 ("staging: rtl8723bs: Fix lines too long in 
update_recvframe_attrib().")
Cc: Stable 
Cc: youling257 
Cc: u.srikant.patn...@gmail.com
Reported-and-tested-by: youling257 
Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/hal/rtl8723bs_recv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/rtl8723bs_recv.c 
b/drivers/staging/rtl8723bs/hal/rtl8723bs_recv.c
index 85077947b9b8..85aba8a503cd 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723bs_recv.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723bs_recv.c
@@ -109,12 +109,12 @@ static void update_recvframe_phyinfo(union recv_frame 
*precvframe,
rx_bssid = get_hdr_bssid(wlanhdr);
pkt_info.bssid_match = ((!IsFrameTypeCtrl(wlanhdr)) &&
!pattrib->icv_err && !pattrib->crc_err &&
-   !ether_addr_equal(rx_bssid, my_bssid));
+   ether_addr_equal(rx_bssid, my_bssid));
 
rx_ra = get_ra(wlanhdr);
my_hwaddr = myid(&padapter->eeprompriv);
pkt_info.to_self = pkt_info.bssid_match &&
-   !ether_addr_equal(rx_ra, my_hwaddr);
+   ether_addr_equal(rx_ra, my_hwaddr);
 
 
pkt_info.is_beacon = pkt_info.bssid_match &&
-- 
2.19.1

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


[PATCH v2] staging: wilc1000: update wilc1000 driver maintainer ids

2018-11-06 Thread Ajay.Kathat
From: Ajay Singh 

We would like to update the maintainer email id's for wilc1000 driver.

Signed-off-by: Aditya Shankar 
Signed-off-by: Ganesh Krishna 
Signed-off-by: Adham Abozaeid 
Signed-off-by: Ajay Singh 
---
Changes in v2:
- added sob of current wilc1000 maintainers
---
 MAINTAINERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index f485597..49ae03f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14132,8 +14132,8 @@ S:  Odd Fixes
 F: drivers/staging/vt665?/
 
 STAGING - WILC1000 WIFI DRIVER
-M: Aditya Shankar 
-M: Ganesh Krishna 
+M: Adham Abozaeid 
+M: Ajay Singh 
 L: linux-wirel...@vger.kernel.org
 S: Supported
 F: drivers/staging/wilc1000/
-- 
2.7.4

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