Re: [PATCH v4 1/2] staging: iio: ad7780: update voltage on read
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> 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
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
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
> 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
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
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
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
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
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
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
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 '+'
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
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
> -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.
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.
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
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
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