Re: [PATCH v2 1/2] staging: gs_fpgaboot: add buffer overflow checks
On Mon, Jul 17, 2017 at 08:47:25PM -0400, Jacob von Chorus wrote: > Four fields in struct fpgaimage are char arrays of length MAX_STR (256). > The amount of data read into these buffers is controlled by a length > field in the bitstream file read from userspace. If a corrupt or > malicious firmware file was supplied, kernel data beyond these buffers > can be overwritten arbitrarily. > > This patch adds a check of the bitstream's length value to ensure it > fits within the bounds of the allocated buffers. An error condition is > returned from gs_read_bitstream if any of the reads fail. > > Signed-off-by: Jacob von Chorus > --- > drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 48 > --- > 1 file changed, 37 insertions(+), 11 deletions(-) What changed from v1? Always list that below the --- line. Please fix up and resend... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] Staging: iio: light: tsl2x7x_core.c: Replace symbolic permission with octal permission
Replace symbolic permissions with their octect representation to fix checkpatch warnings. Signed-off-by: Jaya Durga --- drivers/staging/iio/light/tsl2x7x_core.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/staging/iio/light/tsl2x7x_core.c b/drivers/staging/iio/light/tsl2x7x_core.c index af3910b..c63fe6a 100644 --- a/drivers/staging/iio/light/tsl2x7x_core.c +++ b/drivers/staging/iio/light/tsl2x7x_core.c @@ -1498,34 +1498,34 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev, return 0; } -static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR, +static DEVICE_ATTR(power_state, 0644, tsl2x7x_power_state_show, tsl2x7x_power_state_store); -static DEVICE_ATTR(in_proximity0_calibscale_available, S_IRUGO, +static DEVICE_ATTR(in_proximity0_calibscale_available, 0444, tsl2x7x_prox_gain_available_show, NULL); -static DEVICE_ATTR(in_illuminance0_calibscale_available, S_IRUGO, +static DEVICE_ATTR(in_illuminance0_calibscale_available, 0444, tsl2x7x_gain_available_show, NULL); -static DEVICE_ATTR(in_illuminance0_integration_time, S_IRUGO | S_IWUSR, +static DEVICE_ATTR(in_illuminance0_integration_time, 0644, tsl2x7x_als_time_show, tsl2x7x_als_time_store); -static DEVICE_ATTR(in_illuminance0_target_input, S_IRUGO | S_IWUSR, +static DEVICE_ATTR(in_illuminance0_target_input, 0644, tsl2x7x_als_cal_target_show, tsl2x7x_als_cal_target_store); -static DEVICE_ATTR(in_illuminance0_calibrate, S_IWUSR, NULL, +static DEVICE_ATTR(in_illuminance0_calibrate, 0200, NULL, tsl2x7x_do_calibrate); -static DEVICE_ATTR(in_proximity0_calibrate, S_IWUSR, NULL, +static DEVICE_ATTR(in_proximity0_calibrate, 0200, NULL, tsl2x7x_do_prox_calibrate); -static DEVICE_ATTR(in_illuminance0_lux_table, S_IRUGO | S_IWUSR, +static DEVICE_ATTR(in_illuminance0_lux_table, 0644, tsl2x7x_luxtable_show, tsl2x7x_luxtable_store); -static DEVICE_ATTR(in_intensity0_thresh_period, S_IRUGO | S_IWUSR, +static DEVICE_ATTR(in_intensity0_thresh_period, 0644, tsl2x7x_als_persistence_show, tsl2x7x_als_persistence_store); -static DEVICE_ATTR(in_proximity0_thresh_period, S_IRUGO | S_IWUSR, +static DEVICE_ATTR(in_proximity0_thresh_period, 0644, tsl2x7x_prox_persistence_show, tsl2x7x_prox_persistence_store); /* Use the default register values to identify the Taos device */ -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] staging: unisys: visorbus: Constify attribute_group structures.
attribute_group are not supposed to change at runtime. All functions working with attribute_group provided by work with const attribute_group. So mark the non-const structs as const. Signed-off-by: Arvind Yadav --- drivers/staging/unisys/visorbus/visorbus_main.c | 4 ++-- drivers/staging/unisys/visorbus/visorchipset.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c index 9012cc1..ff80ab2 100644 --- a/drivers/staging/unisys/visorbus/visorbus_main.c +++ b/drivers/staging/unisys/visorbus/visorbus_main.c @@ -294,7 +294,7 @@ static ssize_t typename_show(struct device *dev, struct device_attribute *attr, NULL }; -static struct attribute_group channel_attr_grp = { +static const struct attribute_group channel_attr_grp = { .name = "channel", .attrs = channel_attrs, }; @@ -391,7 +391,7 @@ static ssize_t channel_id_show(struct device *dev, NULL }; -static struct attribute_group dev_attr_grp = { +static const struct attribute_group dev_attr_grp = { .attrs = dev_attrs, }; diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c index 87ea852..6d4498f 100644 --- a/drivers/staging/unisys/visorbus/visorchipset.c +++ b/drivers/staging/unisys/visorbus/visorchipset.c @@ -1138,7 +1138,7 @@ static ssize_t deviceenabled_store(struct device *dev, NULL }; -static struct attribute_group visorchipset_parahotplug_group = { +static const struct attribute_group visorchipset_parahotplug_group = { .name = "parahotplug", .attrs = visorchipset_parahotplug_attrs }; -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: gs_fpgaboot: add buffer overflow checks
On Mon, Jul 17, 2017 at 08:21:20PM -0400, Jacob von Chorus wrote: > On Mon, Jul 17, 2017 at 10:53:25PM +0300, Dan Carpenter wrote: > > > + if (len + 1 > n) { > > > > It's more idiomatic to say "if (len >= n)". Plus that's a good habbit > > My reasoning behind using "((len + 1) > n)" is that len represents the length > of > the string without null-termination. "buf" is required to store a > null-terminator on top of len. Using "len + 1" shows this requirement > more clearly; I will add brackets around "len + 1" for emphasis. > Don't get into the habbit of saying len + 1 because you will end up introducing integer overflows. Also don't add useless parenthesis. Everyone who programs in C is used to NUL terminators, so it's not a new concept which has to be explained. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: unisys: visorbus: Constify attribute_group structures.
On Tue, Jul 18, 2017 at 01:24:42PM +0530, Arvind Yadav wrote: > attribute_group are not supposed to change at runtime. All functions > working with attribute_group provided by work > with const attribute_group. So mark the non-const structs as const. > > Signed-off-by: Arvind Yadav > --- > drivers/staging/unisys/visorbus/visorbus_main.c | 4 ++-- > drivers/staging/unisys/visorbus/visorchipset.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) What changed from the previous patches? Always include that below the --- line as maintainers have no short-term memory at all. v4? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: unisys: visorbus: Constify attribute_group structures.
Hi Greg, On Tuesday 18 July 2017 01:30 PM, Greg KH wrote: On Tue, Jul 18, 2017 at 01:24:42PM +0530, Arvind Yadav wrote: attribute_group are not supposed to change at runtime. All functions working with attribute_group provided by work with const attribute_group. So mark the non-const structs as const. Signed-off-by: Arvind Yadav --- drivers/staging/unisys/visorbus/visorbus_main.c | 4 ++-- drivers/staging/unisys/visorbus/visorchipset.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) What changed from the previous patches? Always include that below the --- line as maintainers have no short-term memory at all. v4? Sorry, I have missed it. :( thanks, greg k-h ~arvind ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4] staging: unisys: visorbus: Constify attribute_group structures.
attribute_group are not supposed to change at runtime. All functions working with attribute_group provided by work with const attribute_group. So mark the non-const structs as const. Signed-off-by: Arvind Yadav --- Changes in v2: Change log typo was not correct. Changes in v3: merge conflict. Changes in v4: version change description was missing. drivers/staging/unisys/visorbus/visorbus_main.c | 4 ++-- drivers/staging/unisys/visorbus/visorchipset.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c index 9012cc1..ff80ab2 100644 --- a/drivers/staging/unisys/visorbus/visorbus_main.c +++ b/drivers/staging/unisys/visorbus/visorbus_main.c @@ -294,7 +294,7 @@ static ssize_t typename_show(struct device *dev, struct device_attribute *attr, NULL }; -static struct attribute_group channel_attr_grp = { +static const struct attribute_group channel_attr_grp = { .name = "channel", .attrs = channel_attrs, }; @@ -391,7 +391,7 @@ static ssize_t channel_id_show(struct device *dev, NULL }; -static struct attribute_group dev_attr_grp = { +static const struct attribute_group dev_attr_grp = { .attrs = dev_attrs, }; diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c index 87ea852..6d4498f 100644 --- a/drivers/staging/unisys/visorbus/visorchipset.c +++ b/drivers/staging/unisys/visorbus/visorchipset.c @@ -1138,7 +1138,7 @@ static ssize_t deviceenabled_store(struct device *dev, NULL }; -static struct attribute_group visorchipset_parahotplug_group = { +static const struct attribute_group visorchipset_parahotplug_group = { .name = "parahotplug", .attrs = visorchipset_parahotplug_attrs }; -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 05/42] staging: unisys: visorinput: visorinput.c: Adjust whitespace usage
> @@ -324,11 +322,10 @@ setup_client_keyboard(void *devdata, /* opaque on > purpose */ > static struct input_dev * > setup_client_mouse(void *devdata /* opaque on purpose */) > { > - struct input_dev *visorinput_dev = NULL; > int xres, yres; > struct fb_info *fb0; > + struct input_dev *visorinput_dev = input_allocate_device(); > > - visorinput_dev = input_allocate_device(); > if (!visorinput_dev) > return NULL; > So far these patches are pretty annoying. You seem to be using Christmas tree ordering: short medium loong Instead of reverse Christmas tree: loong medium short Nobody else does that... People either use reverse Christmas tree or they don't care. Also putting alloc functions in the declarations is a common source of bugs. Empirically, from reviewing thousands of static checker bugs, I can say that declarations get reviewed worse than normal code. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 29/42] staging: unisys: visorchipset: remove local_addr in handle_command
On Mon, Jul 17, 2017 at 04:17:09PM -0400, David Kershner wrote: > + mapping = memremap(addr, bytes, MEMREMAP_WB); > + if (!mapping) > + goto err_finish_ctx; This goto is wrong. It calls parser_done(ctx) when it should just call kfree(ctx). The difference is that parser_done() looks like this: 289 static void parser_done(struct parser_context *ctx) 290 { 291 chipset_dev->controlvm_payload_bytes_buffered -= ctx->param_bytes; ^ We never incremented this so we shouldn't be decrementing it. 292 kfree(ctx); 293 } regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 05/42] staging: unisys: visorinput: visorinput.c: Adjust whitespace usage
I perhaps judged to early and harshly. This patchset is fine. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2] staging: gs_fpgaboot: add buffer overflow checks
On Mon, Jul 17, 2017 at 08:47:25PM -0400, Jacob von Chorus wrote: > -static void readinfo_bitstream(char *bitdata, char *buf, int *offset) > +static int readinfo_bitstream(char *bitdata, char *buf, int size, int > *offset) > { > char tbuf[64]; > s32 len; > @@ -59,9 +59,15 @@ static void readinfo_bitstream(char *bitdata, char *buf, > int *offset) > read_bitstream(bitdata, tbuf, offset, 2); > > len = tbuf[0] << 8 | tbuf[1]; > + if ((len + 1) > size) { > + pr_err("error: readinfo buffer too small\n"); > + return -ETOOSMALL; Probably the correct error code is -EINVAL. I should have just said that in the first email. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: fbtft: array underflow in fbtft_request_gpios_match()
"val" can be negative, so we'd write before the start of the par->gpio.db[] array. Fixes: c296d5f9957c ("staging: fbtft: core support") Signed-off-by: Dan Carpenter diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index b742ee786615..6d0363deba61 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -84,7 +84,7 @@ static unsigned long fbtft_request_gpios_match(struct fbtft_par *par, const struct fbtft_gpio *gpio) { int ret; - long val; + unsigned int val; fbtft_par_dbg(DEBUG_REQUEST_GPIOS_MATCH, par, "%s('%s')\n", __func__, gpio->name); @@ -108,7 +108,7 @@ static unsigned long fbtft_request_gpios_match(struct fbtft_par *par, par->gpio.latch = gpio->gpio; return GPIOF_OUT_INIT_LOW; } else if (gpio->name[0] == 'd' && gpio->name[1] == 'b') { - ret = kstrtol(&gpio->name[2], 10, &val); + ret = kstrtouint(&gpio->name[2], 10, &val); if (ret == 0 && val < 16) { par->gpio.db[val] = gpio->gpio; return GPIOF_OUT_INIT_LOW; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] Staging: iio: adc: ad7280a.c: Fixed Macro argument reuse
CHECK: Macro argument reuse 'addr' - possible side-effects? convert AD7280A_DEVADDR to inline function to fix checkpath check Signed-off-by: Jaya Durga --- drivers/staging/iio/adc/ad7280a.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c index d5ab83f..cb94b7f 100644 --- a/drivers/staging/iio/adc/ad7280a.c +++ b/drivers/staging/iio/adc/ad7280a.c @@ -99,9 +99,12 @@ #define AD7280A_DEVADDR_MASTER 0 #define AD7280A_DEVADDR_ALL0x1F /* 5-bit device address is sent LSB first */ -#define AD7280A_DEVADDR(addr) (((addr & 0x1) << 4) | ((addr & 0x2) << 3) | \ - (addr & 0x4) | ((addr & 0x8) >> 3) | \ - ((addr & 0x10) >> 4)) +static inline unsigned int AD7280A_DEVADDR(unsigned int addr) +{ + return addr & 0x1) << 4) | ((addr & 0x2) << 3) | + (addr & 0x4) | ((addr & 0x8) >> 3) | + ((addr & 0x10) >> 4))); +} /* During a read a valid write is mandatory. * So writing to the highest available address (Address 0x1F) -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] Staging: iio: adc: ad7280a.c: Fixed Macro argument reuse
On Tue, Jul 18, 2017 at 03:17:52PM +0530, Jaya Durga wrote: > diff --git a/drivers/staging/iio/adc/ad7280a.c > b/drivers/staging/iio/adc/ad7280a.c > index d5ab83f..cb94b7f 100644 > --- a/drivers/staging/iio/adc/ad7280a.c > +++ b/drivers/staging/iio/adc/ad7280a.c > @@ -99,9 +99,12 @@ > #define AD7280A_DEVADDR_MASTER 0 > #define AD7280A_DEVADDR_ALL 0x1F > /* 5-bit device address is sent LSB first */ > -#define AD7280A_DEVADDR(addr)(((addr & 0x1) << 4) | ((addr & 0x2) << > 3) | \ > - (addr & 0x4) | ((addr & 0x8) >> 3) | \ > - ((addr & 0x10) >> 4)) > +static inline unsigned int AD7280A_DEVADDR(unsigned int addr) Don't make this inline. GCC is going to ignore the inline anyway, and make up its own mind about what to do. Change the name to not be all caps. Generally, all caps means it's a macro. > +{ > + return addr & 0x1) << 4) | ((addr & 0x2) << 3) | > + (addr & 0x4) | ((addr & 0x8) >> 3) | > + ((addr & 0x10) >> 4))); We don't need all the parens. return ((addr & 0x1) << 4) | ((addr & 0x2) << 3) | (addr & 0x4) | ((addr & 0x8) >> 3) | ((addr & 0x10) >> 4); regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: greybus: loopback: Enclose multiple macro statements in do-while loop
From: Karthik Tummala Enclosed multiple macro statements in a do - while loop as per kernel coding standard, pointed by checkpatch. Signed-off-by: Karthik Tummala --- drivers/staging/greybus/loopback.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 08e2558..c616555 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -176,10 +176,12 @@ static DEVICE_ATTR_RO(name##_##field) } \ static DEVICE_ATTR_RO(name##_avg) -#define gb_loopback_stats_attrs(field) \ - gb_loopback_ro_stats_attr(field, min, u); \ - gb_loopback_ro_stats_attr(field, max, u); \ - gb_loopback_ro_avg_attr(field) +#define gb_loopback_stats_attrs(field) \ +do { \ + gb_loopback_ro_stats_attr(field, min, u); \ + gb_loopback_ro_stats_attr(field, max, u); \ + gb_loopback_ro_avg_attr(field); \ +} while (0) #define gb_loopback_attr(field, type) \ static ssize_t field##_show(struct device *dev, \ -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/3] staging: ccree: Convert to devm_ioremap_resource for map, unmap
From: Suniel Mahesh It is recommended to use managed function devm_ioremap_resource(), which simplifies driver cleanup paths and driver code. This patch does the following: (a) replace request_mem_region(), ioremap() and corresponding error handling with devm_ioremap_resource(). (b) remove struct resource pointer(res_mem) in struct ssi_drvdata as it seems redundant, use struct resource pointer which is defined locally and adjust return value of platform_get_resource() accordingly. (c) release_mem_region() and iounmap() are dropped, since devm_ioremap_ resource() releases and unmaps mem region on driver detach. (d) adjust log messages accordingly and remove any blank lines. Signed-off-by: Suniel Mahesh --- Changes for v2: - format specifiers changed in log messages. - Rebased on top of next-20170718. --- Note: - Patch was tested and built(ARCH=arm) on next-20170718. No build issues reported, however it was not tested on real hardware. --- drivers/staging/ccree/ssi_driver.c | 60 ++ drivers/staging/ccree/ssi_driver.h | 1 - 2 files changed, 15 insertions(+), 46 deletions(-) diff --git a/drivers/staging/ccree/ssi_driver.c b/drivers/staging/ccree/ssi_driver.c index e918cf4..36b7c92 100644 --- a/drivers/staging/ccree/ssi_driver.c +++ b/drivers/staging/ccree/ssi_driver.c @@ -246,35 +246,21 @@ static int init_cc_resources(struct platform_device *plat_dev) dev_set_drvdata(&plat_dev->dev, new_drvdata); /* Get device resources */ /* First CC registers space */ - new_drvdata->res_mem = platform_get_resource(plat_dev, IORESOURCE_MEM, 0); - if (unlikely(!new_drvdata->res_mem)) { - SSI_LOG_ERR("Failed getting IO memory resource\n"); - rc = -ENODEV; - goto init_cc_res_err; - } - SSI_LOG_DEBUG("Got MEM resource (%s): start=%pad end=%pad\n", - new_drvdata->res_mem->name, - new_drvdata->res_mem->start, - new_drvdata->res_mem->end); + req_mem_cc_regs = platform_get_resource(plat_dev, IORESOURCE_MEM, 0); /* Map registers space */ - req_mem_cc_regs = request_mem_region(new_drvdata->res_mem->start, resource_size(new_drvdata->res_mem), "arm_cc7x_regs"); - if (unlikely(!req_mem_cc_regs)) { - SSI_LOG_ERR("Couldn't allocate registers memory region at " -"0x%08X\n", (unsigned int)new_drvdata->res_mem->start); - rc = -EBUSY; - goto init_cc_res_err; - } - cc_base = ioremap(new_drvdata->res_mem->start, resource_size(new_drvdata->res_mem)); - if (unlikely(!cc_base)) { - SSI_LOG_ERR("ioremap[CC](0x%08X,0x%08X) failed\n", - (unsigned int)new_drvdata->res_mem->start, - (unsigned int)resource_size(new_drvdata->res_mem)); - rc = -ENOMEM; + new_drvdata->cc_base = devm_ioremap_resource(&plat_dev->dev, +req_mem_cc_regs); + if (IS_ERR(new_drvdata->cc_base)) { + rc = PTR_ERR(new_drvdata->cc_base); goto init_cc_res_err; } - SSI_LOG_DEBUG("CC registers mapped from %pa to 0x%p\n", &new_drvdata->res_mem->start, cc_base); - new_drvdata->cc_base = cc_base; - + SSI_LOG_DEBUG("Got MEM resource (%s): start=%pad end=%pad\n", + req_mem_cc_regs->name, + req_mem_cc_regs->start, + req_mem_cc_regs->end); + SSI_LOG_DEBUG("CC registers mapped from %pa to 0x%p\n", + &req_mem_cc_regs->start, new_drvdata->cc_base); + cc_base = new_drvdata->cc_base; /* Then IRQ */ new_drvdata->res_irq = platform_get_resource(plat_dev, IORESOURCE_IRQ, 0); if (unlikely(!new_drvdata->res_irq)) { @@ -424,17 +410,9 @@ static int init_cc_resources(struct platform_device *plat_dev) #ifdef ENABLE_CC_SYSFS ssi_sysfs_fini(); #endif - - if (req_mem_cc_regs) { - if (irq_registered) { - free_irq(new_drvdata->res_irq->start, new_drvdata); - new_drvdata->res_irq = NULL; - iounmap(cc_base); - new_drvdata->cc_base = NULL; - } - release_mem_region(new_drvdata->res_mem->start, - resource_size(new_drvdata->res_mem)); - new_drvdata->res_mem = NULL; + if (irq_registered) { + free_irq(new_drvdata->res_irq->start, new_drvdata
[PATCH v2 1/3] staging: ccree: Replace kzalloc with devm_kzalloc
From: Suniel Mahesh It is recommended to use managed function devm_kzalloc, which simplifies driver cleanup paths and driver code. This patch does the following: (a) replace kzalloc with devm_kzalloc. (b) drop kfree(), because memory allocated with devm_kzalloc() is automatically freed on driver detach, otherwise it leads to a double free. (c) remove unnecessary blank lines. Signed-off-by: Suniel Mahesh --- Changes for v2: - Changes done as suggested by Greg-KH. - Rebased on top of next-20170718. --- Note: - Patch was tested and built(ARCH=arm) on next-20170718. No build issues reported, however it was not tested on real hardware. --- drivers/staging/ccree/ssi_driver.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/staging/ccree/ssi_driver.c b/drivers/staging/ccree/ssi_driver.c index d7b9a63..e918cf4 100644 --- a/drivers/staging/ccree/ssi_driver.c +++ b/drivers/staging/ccree/ssi_driver.c @@ -223,13 +223,15 @@ static int init_cc_resources(struct platform_device *plat_dev) struct resource *req_mem_cc_regs = NULL; void __iomem *cc_base = NULL; bool irq_registered = false; - struct ssi_drvdata *new_drvdata = kzalloc(sizeof(struct ssi_drvdata), GFP_KERNEL); + struct ssi_drvdata *new_drvdata; struct device *dev = &plat_dev->dev; struct device_node *np = dev->of_node; u32 signature_val; int rc = 0; - if (unlikely(!new_drvdata)) { + new_drvdata = devm_kzalloc(&plat_dev->dev, sizeof(*new_drvdata), + GFP_KERNEL); + if (!new_drvdata) { SSI_LOG_ERR("Failed to allocate drvdata"); rc = -ENOMEM; goto init_cc_res_err; @@ -434,10 +436,8 @@ static int init_cc_resources(struct platform_device *plat_dev) resource_size(new_drvdata->res_mem)); new_drvdata->res_mem = NULL; } - kfree(new_drvdata); dev_set_drvdata(&plat_dev->dev, NULL); } - return rc; } @@ -478,8 +478,6 @@ static void cleanup_cc_resources(struct platform_device *plat_dev) drvdata->cc_base = NULL; drvdata->res_mem = NULL; } - - kfree(drvdata); dev_set_drvdata(&plat_dev->dev, NULL); } -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/3] staging: ccree: Use platform_get_irq and devm_request_irq
From: Suniel Mahesh It is recommended to use managed function devm_request_irq(), which simplifies driver cleanup paths and driver code. This patch does the following: (a) replace platform_get_resource(), request_irq() and corresponding error handling with platform_get_irq() and devm_request_irq(). (b) remove struct resource pointer(res_irq) in struct ssi_drvdata as it seems redundant. (c) change type of member irq in struct ssi_drvdata from unsigned int to int, as return type of platform_get_irq is int and can be used in error handling. (d) remove irq_registered variable from driver probe as it seems redundant. (e) free_irq is not required any more, devm_request_irq() free's it on driver detach. (f) adjust log messages accordingly and remove any blank lines. Signed-off-by: Suniel Mahesh --- Changes for v2: - Rebased on top of next-20170718. --- Note: - Patch was tested and built(ARCH=arm) on next-20170718. No build issues reported, however it was not tested on real hardware. --- drivers/staging/ccree/ssi_driver.c | 30 +- drivers/staging/ccree/ssi_driver.h | 3 +-- 2 files changed, 10 insertions(+), 23 deletions(-) diff --git a/drivers/staging/ccree/ssi_driver.c b/drivers/staging/ccree/ssi_driver.c index 36b7c92..bac27d4 100644 --- a/drivers/staging/ccree/ssi_driver.c +++ b/drivers/staging/ccree/ssi_driver.c @@ -222,7 +222,6 @@ static int init_cc_resources(struct platform_device *plat_dev) { struct resource *req_mem_cc_regs = NULL; void __iomem *cc_base = NULL; - bool irq_registered = false; struct ssi_drvdata *new_drvdata; struct device *dev = &plat_dev->dev; struct device_node *np = dev->of_node; @@ -262,26 +261,22 @@ static int init_cc_resources(struct platform_device *plat_dev) &req_mem_cc_regs->start, new_drvdata->cc_base); cc_base = new_drvdata->cc_base; /* Then IRQ */ - new_drvdata->res_irq = platform_get_resource(plat_dev, IORESOURCE_IRQ, 0); - if (unlikely(!new_drvdata->res_irq)) { + new_drvdata->irq = platform_get_irq(plat_dev, 0); + if (new_drvdata->irq < 0) { SSI_LOG_ERR("Failed getting IRQ resource\n"); - rc = -ENODEV; + rc = new_drvdata->irq; goto init_cc_res_err; } - rc = request_irq(new_drvdata->res_irq->start, cc_isr, -IRQF_SHARED, "arm_cc7x", new_drvdata); - if (unlikely(rc != 0)) { - SSI_LOG_ERR("Could not register to interrupt %llu\n", - (unsigned long long)new_drvdata->res_irq->start); + rc = devm_request_irq(&plat_dev->dev, new_drvdata->irq, cc_isr, + IRQF_SHARED, "arm_cc7x", new_drvdata); + if (rc) { + SSI_LOG_ERR("Could not register to interrupt %d\n", + new_drvdata->irq); goto init_cc_res_err; } init_completion(&new_drvdata->icache_setup_completion); - irq_registered = true; - SSI_LOG_DEBUG("Registered to IRQ (%s) %llu\n", - new_drvdata->res_irq->name, - (unsigned long long)new_drvdata->res_irq->start); - + SSI_LOG_DEBUG("Registered to IRQ: %d\n", new_drvdata->irq); new_drvdata->plat_dev = plat_dev; rc = cc_clk_on(new_drvdata); @@ -410,10 +405,6 @@ static int init_cc_resources(struct platform_device *plat_dev) #ifdef ENABLE_CC_SYSFS ssi_sysfs_fini(); #endif - if (irq_registered) { - free_irq(new_drvdata->res_irq->start, new_drvdata); - new_drvdata->res_irq = NULL; - } dev_set_drvdata(&plat_dev->dev, NULL); } return rc; @@ -443,11 +434,8 @@ static void cleanup_cc_resources(struct platform_device *plat_dev) #ifdef ENABLE_CC_SYSFS ssi_sysfs_fini(); #endif - fini_cc_regs(drvdata); cc_clk_off(drvdata); - free_irq(drvdata->res_irq->start, drvdata); - drvdata->res_irq = NULL; dev_set_drvdata(&plat_dev->dev, NULL); } diff --git a/drivers/staging/ccree/ssi_driver.h b/drivers/staging/ccree/ssi_driver.h index 518c0bf..88ef370 100644 --- a/drivers/staging/ccree/ssi_driver.h +++ b/drivers/staging/ccree/ssi_driver.h @@ -128,9 +128,8 @@ struct ssi_crypto_req { * @fw_ver:SeP loaded firmware version */ struct ssi_drvdata { - struct resource *res_irq; void __iomem *cc_base; - unsigned int irq; + int irq; u32 irq_mask; u32 fw_ver; /* Calibration time of start/stop -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: loopback: Enclose multiple macro statements in do-while loop
On Tue, Jul 18, 2017 at 04:26:40PM +0530, kart...@techveda.org wrote: > From: Karthik Tummala > > Enclosed multiple macro statements in a do - while loop as per kernel > coding standard, pointed by checkpatch. \ > static DEVICE_ATTR_RO(name##_avg) > > -#define gb_loopback_stats_attrs(field) \ > - gb_loopback_ro_stats_attr(field, min, u); \ > - gb_loopback_ro_stats_attr(field, max, u); \ > - gb_loopback_ro_avg_attr(field) > +#define gb_loopback_stats_attrs(field) > \ > +do { \ > + gb_loopback_ro_stats_attr(field, min, u); \ > + gb_loopback_ro_stats_attr(field, max, u); \ > + gb_loopback_ro_avg_attr(field); \ > +} while (0) You obviously did not even compile-test this before submitting it, which needless to say, is not ok. Johan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/7] staging: fsl-mc: rewrite mc command send/receive to work on 32-bits
On 07/17/2017 06:00 PM, Arnd Bergmann wrote: > On Mon, Jul 17, 2017 at 4:27 PM, Laurentiu Tudor > wrote: >> Hi Arnd, >> >> On 07/17/2017 04:45 PM, Arnd Bergmann wrote: >>> On Mon, Jul 17, 2017 at 3:26 PM, wrote: From: Laurentiu Tudor Split the 64-bit accesses in 32-bit accesses because there's no real constrain in MC to do only atomic 64-bit. There's only one place where ordering is important: when writing the MC command header the first 32-bit part of the header must be written last. We do this switch in order to allow compiling the driver on 32-bit. Signed-off-by: Laurentiu Tudor --- drivers/staging/fsl-mc/bus/mc-sys.c | 31 --- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c b/drivers/staging/fsl-mc/bus/mc-sys.c index 195d9f3..dd2828e 100644 --- a/drivers/staging/fsl-mc/bus/mc-sys.c +++ b/drivers/staging/fsl-mc/bus/mc-sys.c @@ -124,14 +124,15 @@ static inline void mc_write_command(struct mc_command __iomem *portal, { int i; - /* copy command parameters into the portal */ - for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++) - __raw_writeq(cmd->params[i], &portal->params[i]); - /* ensure command params are committed before submitting it */ - wmb(); - - /* submit the command by writing the header */ - __raw_writeq(cmd->header, &portal->header); + /* +* copy command parameters into the portal. Final write +* triggers the submission of the command. +*/ + for (i = sizeof(struct mc_command) / sizeof(u32) - 1; i >= 0; i--) { + __raw_writel(((u32 *)cmd)[i], &((u32 *)portal)[i]); + /* ensure command params are committed before submitting it */ + wmb(); + } } >>> >>> What is the byte order requirement on this buffer? >> >> Endianness is handled by the callers so this function must leave >> the binary blob intact. > > Got it, the endianess looks correct indeed. > >>> If this is a byte string >>> rather than individual registers, you should probably just use >>> memcpy_toio() >> >> It's a header followed by an opaque command. The protocol for queueing a >> command says that the first 32-bit half of the header must be written >> last, this triggering the command handling in the MC. > > Strictly speaking the __raw_writel() won't guarantee that the > data is written as a single word, the compiler might decide to > split it up into byte-sized writes if it believes the destination pointer > is unaligned and the CPU has no efficient writes. > > I think this cannot happen on arm or powerpc, as we go through > inline assembly for the store, but it's not completely portable. Should i worry about portability? Slim changes that this driver will ever run on anything else other than ARM & ARM64. My current goal was just to make it compile on other arches. > Before your patch, both the compiler and the CPU could also > reorder the stores in theory (but normally won't), but the wmb() > will prevent that now. Ok, thanks for the info. >>> but if these are separate registers, then using the >>> __raw_* accessors is still wrong, at least on kernels that have a >>> different byteorder from the hardware. >> >> As mentioned above, endianness is handled by the caller. This function >> takes raw data and must leave it unchanged. >> >>> Also, are you sure that adding those six extra barriers has no >>> performance impact? >> >> This is a slow interface used in slow paths, so i don't think those >> extra barriers will have any performance impact. > > Ok. > > One other problem remains: most developers looking at this > code like Robin and me will immediately think there might be > an endianess bug here. As this is a slow path, how about > using an explicit conversion using > > writeq(le64_to_cpup(buffer), pointer); Sure, sounds perfect. I'll do that in the next respin. --- Thanks & Best Regards, Laurentiu ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: loopback: Enclose multiple macro statements in do-while loop
On 18/07/17 11:56, kart...@techveda.org wrote: From: Karthik Tummala Enclosed multiple macro statements in a do - while loop as per kernel coding standard, pointed by checkpatch. Signed-off-by: Karthik Tummala --- drivers/staging/greybus/loopback.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 08e2558..c616555 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -176,10 +176,12 @@ static DEVICE_ATTR_RO(name##_##field) } \ static DEVICE_ATTR_RO(name##_avg) -#define gb_loopback_stats_attrs(field)\ - gb_loopback_ro_stats_attr(field, min, u); \ - gb_loopback_ro_stats_attr(field, max, u); \ - gb_loopback_ro_avg_attr(field) +#define gb_loopback_stats_attrs(field) \ +do { \ + gb_loopback_ro_stats_attr(field, min, u); \ + gb_loopback_ro_stats_attr(field, max, u); \ + gb_loopback_ro_avg_attr(field); \ +} while (0) #define gb_loopback_attr(field, type) \ static ssize_t field##_show(struct device *dev, \ Macros with multiple statements should be enclosed in a do - while block: .. code-block:: c #define macrofun(a, b, c) \ do {\ if (a == 5) \ do_this(b, c); \ } while (0) I don't think it really applies. We're declaring functions not inlining executable code.. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [BUG] drivers: staging: comedi: do not call blocking ops when !TASK_RUNNING; state=1
[Top-posting fixed. Please don't top-post!] On 17/07/17 19:14, Piotr Gregor wrote: On Mon, Jul 17, 2017 at 05:02:45PM +0100, Ian Abbott wrote: On 17/07/17 15:48, Piotr Gregor wrote: Calling blocking operations from wrong context. Kernel: Linux piotrpc 4.4.70-rt83 #1 SMP PREEMPT RT Thu Jul 13 08:42:02 BST 2017 x86_64 GNU/Linux [ 80.542018] NOHZ: local_softirq_pending 80 [ 125.175471] [ cut here ] [ 125.175491] WARNING: CPU: 0 PID: 1497 at kernel/sched/core.c:7833 __might_sleep+0x9f/0xb0() [ 125.175728] do not call blocking ops when !TASK_RUNNING; state=1 set at [] comedi_read+0x1a1/0x610 [comedi] [snip] [ 125.175926] CPU: 0 PID: 1497 Comm: txrx Tainted: GWC 4.4.70-rt83 #1 [ 125.175928] Hardware name: NOVATECH LTD PC-XB04472/H110M-C, BIOS 3019 01/06/2017 [ 125.175935] 8802205b3cb0 81387160 8802205b3cf8 [ 125.175940] 0009 8802205b3ce8 81089766 c057ca60 [ 125.175944] 02dc 0001 0001 [ 125.175945] Call Trace: [ 125.175955] [] dump_stack+0x85/0xc5 [ 125.175963] [] warn_slowpath_common+0x86/0xe0 [ 125.175971] [] warn_slowpath_fmt+0x4c/0x50 [ 125.175980] [] ? trace_preempt_on+0x1a7/0x2b0 [ 125.175985] [] ? schedule+0x55/0xe0 [ 125.175994] [] ? comedi_read+0x1a1/0x610 [comedi] [ 125.176001] [] ? comedi_read+0x1a1/0x610 [comedi] [ 125.176005] [] __might_sleep+0x9f/0xb0 [ 125.176012] [] __might_fault+0x3b/0xb0 [ 125.176020] [] comedi_read+0x3df/0x610 [comedi] [ 125.176034] [] ? wake_up_process+0x20/0x20 [ 125.176044] [] __vfs_read+0x28/0xe0 [ 125.176053] [] ? security_file_permission+0xa6/0xc0 [ 125.176060] [] ? rw_verify_area+0x53/0xf0 [ 125.176066] [] vfs_read+0x89/0x130 [ 125.176074] [] SyS_read+0x49/0xb0 [ 125.176084] [] entry_SYSCALL_64_fastpath+0x16/0x7a [ 125.176095] ---[ end trace 0003 ]--- cheers, Piotr Thanks for the bug report. I think comedi_read() in "drivers/staging/comedi/comedi_fops.c" needs a call to set_current_state(TASK_RUNNING) just before the call to copy_to_user(...). Similarly, comedi_write() needs a call to set_current_task(TASK_RUNNING) just before the call to copy_from_user(...), and another call to set_current_task(TASK_RUNNING) before the call to mutex_lock(). I'll work on a patch. > Hi Ian, > > I am afraid I have more problems with running Comedi on rt-patched kernel than > only that. I am still trying to figure out the specific reason - hopefully it is > my fault and I scrued up my own code, but so far it seems pcie215 > support doesn't work at least in the same way on rt-patched kernel > as it was on non-rt. I will dig it a little bit harder, likely coming with another > bug reports shoud they be observed. > > Do you know about other experience other people may have using Comedi > for pcie215 on rt-patched kernels, specifically 4.4.70? Not in particular. People have used Comedi with RTAI in the past, but that's a different animal to the RT patch-set. > Has Comedi been tested on rt kernels by yourself? Not up until now. Initial testing with the "comedi_test" module suggests there may be some issues to be resolved (e.g. it locks up when running an asynchronous command), although "comedi_test" isn't a typical Comedi driver as it makes heavy use of kernel timers. I do have a pcie215, but haven't tried it on an RT kernel yet. -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/3] Staging: iio: adc: ad7280a.c: Fixed Macro argument reuse
CHECK: Macro argument reuse 'addr' - possible side-effects? convert AD7280A_DEVADDR to ad7280a_devaddr static function to fix checkpath check Signed-off-by: Jaya Durga --- drivers/staging/iio/adc/ad7280a.c | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c index d5ab83f..f543b42 100644 --- a/drivers/staging/iio/adc/ad7280a.c +++ b/drivers/staging/iio/adc/ad7280a.c @@ -99,9 +99,14 @@ #define AD7280A_DEVADDR_MASTER 0 #define AD7280A_DEVADDR_ALL0x1F /* 5-bit device address is sent LSB first */ -#define AD7280A_DEVADDR(addr) (((addr & 0x1) << 4) | ((addr & 0x2) << 3) | \ - (addr & 0x4) | ((addr & 0x8) >> 3) | \ - ((addr & 0x10) >> 4)) +static unsigned int ad7280a_devaddr(unsigned int addr) +{ + return ((addr & 0x1) << 4) | + ((addr & 0x2) << 3) | + (addr & 0x4) | + ((addr & 0x8) >> 3) | + ((addr & 0x10) >> 4); +} /* During a read a valid write is mandatory. * So writing to the highest available address (Address 0x1F) @@ -372,7 +377,7 @@ static int ad7280_chain_setup(struct ad7280_state *st) if (ad7280_check_crc(st, val)) return -EIO; - if (n != AD7280A_DEVADDR(val >> 27)) + if (n != ad7280a_devaddr(val >> 27)) return -EIO; } @@ -511,7 +516,7 @@ static int ad7280_channel_init(struct ad7280_state *st) st->channels[cnt].info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE); st->channels[cnt].address = - AD7280A_DEVADDR(dev) << 8 | ch; + ad7280a_devaddr(dev) << 8 | ch; st->channels[cnt].scan_index = cnt; st->channels[cnt].scan_type.sign = 'u'; st->channels[cnt].scan_type.realbits = 12; @@ -558,7 +563,7 @@ static int ad7280_attr_init(struct ad7280_state *st) for (ch = AD7280A_CELL_VOLTAGE_1; ch <= AD7280A_CELL_VOLTAGE_6; ch++, cnt++) { st->iio_attr[cnt].address = - AD7280A_DEVADDR(dev) << 8 | ch; + ad7280a_devaddr(dev) << 8 | ch; st->iio_attr[cnt].dev_attr.attr.mode = 0644; st->iio_attr[cnt].dev_attr.show = @@ -574,7 +579,7 @@ static int ad7280_attr_init(struct ad7280_state *st) &st->iio_attr[cnt].dev_attr.attr; cnt++; st->iio_attr[cnt].address = - AD7280A_DEVADDR(dev) << 8 | + ad7280a_devaddr(dev) << 8 | (AD7280A_CB1_TIMER + ch); st->iio_attr[cnt].dev_attr.attr.mode = 0644; @@ -918,7 +923,7 @@ static int ad7280_probe(struct spi_device *spi) if (ret) goto error_unregister; - ret = ad7280_write(st, AD7280A_DEVADDR(st->slave_num), + ret = ad7280_write(st, ad7280a_devaddr(st->slave_num), AD7280A_ALERT, 0, AD7280A_ALERT_GEN_STATIC_HIGH | (pdata->chain_last_alert_ignore & 0xF)); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 3/3] Staging: iio: adc: ad7280a.c: Fixed Macro argument reuse
On Tue, Jul 18, 2017 at 04:45:04PM +0530, Jaya Durga wrote: > CHECK: Macro argument reuse 'addr' - possible side-effects? > > convert AD7280A_DEVADDR to ad7280a_devaddr static function > to fix checkpath check > > Signed-off-by: Jaya Durga > --- Put a little changelog here: v2: small style changes > drivers/staging/iio/adc/ad7280a.c | 21 + > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/drivers/staging/iio/adc/ad7280a.c > b/drivers/staging/iio/adc/ad7280a.c > index d5ab83f..f543b42 100644 > --- a/drivers/staging/iio/adc/ad7280a.c > +++ b/drivers/staging/iio/adc/ad7280a.c > @@ -99,9 +99,14 @@ > #define AD7280A_DEVADDR_MASTER 0 > #define AD7280A_DEVADDR_ALL 0x1F > /* 5-bit device address is sent LSB first */ > -#define AD7280A_DEVADDR(addr)(((addr & 0x1) << 4) | ((addr & 0x2) << > 3) | \ > - (addr & 0x4) | ((addr & 0x8) >> 3) | \ > - ((addr & 0x10) >> 4)) > +static unsigned int ad7280a_devaddr(unsigned int addr) > +{ > + return ((addr & 0x1) << 4) | > + ((addr & 0x2) << 3) | > + (addr & 0x4) | > + ((addr & 0x8) >> 3) | > + ((addr & 0x10) >> 4); This indenting makes no sense though. You're almost but not quite making the '|' characters line up. When I did it, I added space characters to make it work: return ((addr & 0x1) << 4) | ((addr & 0x2) << 3) | <- 1 tab and 7 spaces (addr & 0x4) | <- 2 tabs ((addr & 0x8) >> 3) | ((addr & 0x10) >> 4); regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export
On Mon, Jul 17, 2017 at 11:04:38PM +0100, Alan Cox wrote: > > > Sure. I can fix the tty->count mismatch based on Alan's suggestion. However > > I don't understand why the exclusivity flag should belong to tty_port and > > not tty_struct. It will be good to know why. > > We are trying to move all the flags that we can and structs into the > tty_port, except any that are used internally within the struct tty > level code. The main reason for this is to make the object lifetimes and > locking simpler - because the tty_port lasts for the time the hardware is > present. I see. That does make sense. I have appended a version of the patch which adds the flag to tty_port and uses that inside tty_kopen. >From readability point of view however, I think adding the flag to tty_struct looks more intuitive. At least for now - until we move other things from tty_struct to tty_port. The patch is untested but I can work on this if that fits in with the plans for tty. Thanks, Okash --- drivers/tty/tty_io.c | 67 +++ include/linux/tty.h | 17 2 files changed, 79 insertions(+), 5 deletions(-) --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -280,7 +280,7 @@ static int check_tty_count(struct tty_st { #ifdef CHECK_TTY_COUNT struct list_head *p; - int count = 0; + int count = 0, kopen_count = 0; spin_lock(&tty->files_lock); list_for_each(p, &tty->tty_files) { @@ -291,10 +291,12 @@ static int check_tty_count(struct tty_st tty->driver->subtype == PTY_TYPE_SLAVE && tty->link && tty->link->count) count++; - if (tty->count != count) { - tty_warn(tty, "%s: tty->count(%d) != #fd's(%d)\n", -routine, tty->count, count); - return count; + if (tty_port_kopened(tty->port)) + kopen_count++; + if (tty->count != (count + kopen_count)) { + tty_warn(tty, "%s: tty->count(%d) != (#fd's(%d) + #kopen's(%d))\n", +routine, tty->count, count, kopen_count); + return (count + kopen_count); } #endif return 0; @@ -1522,6 +1524,7 @@ static int tty_release_checks(struct tty */ void tty_release_struct(struct tty_struct *tty, int idx) { + // TODO: reset TTY_PORT_KOPENED on tty->port /* * Ask the line discipline code to release its structures */ @@ -1786,6 +1789,54 @@ static struct tty_driver *tty_lookup_dri } /** + * tty_kopen - open a tty device for kernel + * @device: dev_t of device to open + * + * Opens tty exclusively for kernel. Performs the driver lookup, + * makes sure it's not already opened and performs the first-time + * tty initialization. + * + * Returns the locked initialized &tty_struct + * + * Claims the global tty_mutex to serialize: + * - concurrent first-time tty initialization + * - concurrent tty driver removal w/ lookup + * - concurrent tty removal from driver table + */ +struct tty_struct *tty_kopen(dev_t device) +{ + struct tty_struct *tty; + struct tty_driver *driver = NULL; + int index = -1; + + mutex_lock(&tty_mutex); + driver = tty_lookup_driver(device, NULL, &index); + if (IS_ERR(driver)) { + mutex_unlock(&tty_mutex); + return ERR_CAST(driver); + } + + /* check whether we're reopening an existing tty */ + tty = tty_driver_lookup_tty(driver, NULL, index); + if (IS_ERR(tty)) + goto out; + + if (tty) { + /* drop kref from tty_driver_lookup_tty() */ + tty_kref_put(tty); + tty = ERR_PTR(-EBUSY); + } else { /* tty_init_dev returns tty with the tty_lock held */ + tty = tty_init_dev(driver, index); + tty_port_set_kopened(tty->port, 1); + } +out: + mutex_unlock(&tty_mutex); + tty_driver_kref_put(driver); + return tty; +} +EXPORT_SYMBOL_GPL(tty_kopen); + +/** * tty_open_by_driver - open a tty device * @device: dev_t of device to open * @inode: inode of device file @@ -1824,6 +1875,12 @@ struct tty_struct *tty_open_by_driver(de } if (tty) { + if (tty_port_kopened(tty->port)) { + tty_kref_put(tty); + mutex_unlock(&tty_mutex); + tty = ERR_PTR(-EBUSY); + goto out; + } mutex_unlock(&tty_mutex); retval = tty_lock_interruptible(tty); tty_kref_put(tty); /* drop kref from tty_driver_lookup_tty() */ --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -261,6 +261,7 @@ struct tty_port { */ #define TTY_PORT_CTS_FLOW 3 /* h/w flow control enabled */ #define TTY_PORT_CHECK_CD 4 /* carrier detect enabled */ +
Re: [PATCH 6/7] staging: fsl-mc: rewrite mc command send/receive to work on 32-bits
On Tue, Jul 18, 2017 at 1:08 PM, Laurentiu Tudor wrote: > On 07/17/2017 06:00 PM, Arnd Bergmann wrote: >> Strictly speaking the __raw_writel() won't guarantee that the >> data is written as a single word, the compiler might decide to >> split it up into byte-sized writes if it believes the destination pointer >> is unaligned and the CPU has no efficient writes. >> >> I think this cannot happen on arm or powerpc, as we go through >> inline assembly for the store, but it's not completely portable. > > Should i worry about portability? Slim changes that this driver > will ever run on anything else other than ARM & ARM64. > My current goal was just to make it compile on other arches. I always recommend writing any driver in the most portable way out of principle, since you never know who looks at it for reference when writing another driver. I wouldn't expect the driver itself to be used on other architectures, but of course you never know what CPU becomes fashionable 10 years from now. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export
On Tue, Jul 18, 2017 at 12:29:52PM +0100, Okash Khawaja wrote: > +struct tty_struct *tty_kopen(dev_t device) > +{ > + struct tty_struct *tty; > + struct tty_driver *driver = NULL; > + int index = -1; > + > + mutex_lock(&tty_mutex); > + driver = tty_lookup_driver(device, NULL, &index); > + if (IS_ERR(driver)) { > + mutex_unlock(&tty_mutex); > + return ERR_CAST(driver); > + } > + > + /* check whether we're reopening an existing tty */ > + tty = tty_driver_lookup_tty(driver, NULL, index); > + if (IS_ERR(tty)) > + goto out; > + > + if (tty) { > + /* drop kref from tty_driver_lookup_tty() */ > + tty_kref_put(tty); > + tty = ERR_PTR(-EBUSY); > + } else { /* tty_init_dev returns tty with the tty_lock held */ > + tty = tty_init_dev(driver, index); > + tty_port_set_kopened(tty->port, 1); ^ tty_init_dev() can fail leading to an error pointer dereference here. > + } > +out: > + mutex_unlock(&tty_mutex); > + tty_driver_kref_put(driver); > + return tty; > +} > +EXPORT_SYMBOL_GPL(tty_kopen); regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 3/3] Staging: iio: adc: ad7280a.c: Fixed Macro argument reuse
CHECK: Macro argument reuse 'addr' - possible side-effects? convert AD7280A_DEVADDR to ad7280a_devaddr static function to fix checkpath check v3: small style changes Signed-off-by: Jaya Durga --- drivers/staging/iio/adc/ad7280a.c | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c index d5ab83f..f85dde9 100644 --- a/drivers/staging/iio/adc/ad7280a.c +++ b/drivers/staging/iio/adc/ad7280a.c @@ -99,9 +99,14 @@ #define AD7280A_DEVADDR_MASTER 0 #define AD7280A_DEVADDR_ALL0x1F /* 5-bit device address is sent LSB first */ -#define AD7280A_DEVADDR(addr) (((addr & 0x1) << 4) | ((addr & 0x2) << 3) | \ - (addr & 0x4) | ((addr & 0x8) >> 3) | \ - ((addr & 0x10) >> 4)) +static unsigned int ad7280a_devaddr(unsigned int addr) +{ + return ((addr & 0x1) << 4) | + ((addr & 0x2) << 3) | + (addr & 0x4) | + ((addr & 0x8) >> 3) | + ((addr & 0x10) >> 4); +} /* During a read a valid write is mandatory. * So writing to the highest available address (Address 0x1F) @@ -372,7 +377,7 @@ static int ad7280_chain_setup(struct ad7280_state *st) if (ad7280_check_crc(st, val)) return -EIO; - if (n != AD7280A_DEVADDR(val >> 27)) + if (n != ad7280a_devaddr(val >> 27)) return -EIO; } @@ -511,7 +516,7 @@ static int ad7280_channel_init(struct ad7280_state *st) st->channels[cnt].info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE); st->channels[cnt].address = - AD7280A_DEVADDR(dev) << 8 | ch; + ad7280a_devaddr(dev) << 8 | ch; st->channels[cnt].scan_index = cnt; st->channels[cnt].scan_type.sign = 'u'; st->channels[cnt].scan_type.realbits = 12; @@ -558,7 +563,7 @@ static int ad7280_attr_init(struct ad7280_state *st) for (ch = AD7280A_CELL_VOLTAGE_1; ch <= AD7280A_CELL_VOLTAGE_6; ch++, cnt++) { st->iio_attr[cnt].address = - AD7280A_DEVADDR(dev) << 8 | ch; + ad7280a_devaddr(dev) << 8 | ch; st->iio_attr[cnt].dev_attr.attr.mode = 0644; st->iio_attr[cnt].dev_attr.show = @@ -574,7 +579,7 @@ static int ad7280_attr_init(struct ad7280_state *st) &st->iio_attr[cnt].dev_attr.attr; cnt++; st->iio_attr[cnt].address = - AD7280A_DEVADDR(dev) << 8 | + ad7280a_devaddr(dev) << 8 | (AD7280A_CB1_TIMER + ch); st->iio_attr[cnt].dev_attr.attr.mode = 0644; @@ -918,7 +923,7 @@ static int ad7280_probe(struct spi_device *spi) if (ret) goto error_unregister; - ret = ad7280_write(st, AD7280A_DEVADDR(st->slave_num), + ret = ad7280_write(st, ad7280a_devaddr(st->slave_num), AD7280A_ALERT, 0, AD7280A_ALERT_GEN_STATIC_HIGH | (pdata->chain_last_alert_ignore & 0xF)); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: pi433: Coding style fixes
Fixes if-statement related warning and errors reported by checkpatch.pl Signed-off-by: Simo Koskinen --- drivers/staging/pi433/pi433_if.c | 253 +-- drivers/staging/pi433/rf69.c | 69 ++- 2 files changed, 148 insertions(+), 174 deletions(-) diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 1bc478a..3e4be2e 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -131,19 +131,14 @@ DIO0_irq_handler(unsigned int irq, void *dev_id, struct pt_regs *regs) { struct pi433_device *device = dev_id; - if (device->irq_state[DIO0] == DIO_PacketSent) - { + if (device->irq_state[DIO0] == DIO_PacketSent) { device->free_in_fifo = FIFO_SIZE; printk("DIO0 irq: Packet sent\n"); // TODO: printk() should include KERN_ facility level wake_up_interruptible(&device->fifo_wait_queue); - } - else if (device->irq_state[DIO0] == DIO_Rssi_DIO0) - { + } else if (device->irq_state[DIO0] == DIO_Rssi_DIO0) { printk("DIO0 irq: RSSI level over threshold\n"); wake_up_interruptible(&device->rx_wait_queue); - } - else if (device->irq_state[DIO0] == DIO_PayloadReady) - { + } else if (device->irq_state[DIO0] == DIO_PayloadReady) { printk("DIO0 irq: PayloadReady\n"); device->free_in_fifo = 0; wake_up_interruptible(&device->fifo_wait_queue); @@ -157,15 +152,15 @@ DIO1_irq_handler(unsigned int irq, void *dev_id, struct pt_regs *regs) { struct pi433_device *device = dev_id; - if (device->irq_state[DIO1] == DIO_FifoNotEmpty_DIO1) - { + if (device->irq_state[DIO1] == DIO_FifoNotEmpty_DIO1) { device->free_in_fifo = FIFO_SIZE; + } else if (device->irq_state[DIO1] == DIO_FifoLevel) { + if (device->rx_active) + device->free_in_fifo = FIFO_THRESHOLD - 1; + else + device->free_in_fifo = FIFO_SIZE - FIFO_THRESHOLD - 1; } - else if (device->irq_state[DIO1] == DIO_FifoLevel) - { - if (device->rx_active) device->free_in_fifo = FIFO_THRESHOLD - 1; - elsedevice->free_in_fifo = FIFO_SIZE - FIFO_THRESHOLD - 1; - } + printk("DIO1 irq: %d bytes free in fifo\n", device->free_in_fifo); // TODO: printk() should include KERN_ facility level wake_up_interruptible(&device->fifo_wait_queue); @@ -200,43 +195,38 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg) /* packet config */ /* enable */ SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync)); - if (rx_cfg->enable_sync == optionOn) - { + if (rx_cfg->enable_sync == optionOn) { SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, afterSyncInterrupt)); - } - else - { + } else { SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always)); } + SET_CHECKED(rf69_set_packet_format (dev->spi, rx_cfg->enable_length_byte)); SET_CHECKED(rf69_set_adressFiltering(dev->spi, rx_cfg->enable_address_filtering)); SET_CHECKED(rf69_set_crc_enable (dev->spi, rx_cfg->enable_crc)); /* lengths */ SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length)); - if (rx_cfg->enable_length_byte == optionOn) - { + if (rx_cfg->enable_length_byte == optionOn) { SET_CHECKED(rf69_set_payload_length(dev->spi, 0xff)); - } - else if (rx_cfg->fixed_message_length != 0) - { + } else if (rx_cfg->fixed_message_length != 0) { payload_length = rx_cfg->fixed_message_length; - if (rx_cfg->enable_length_byte == optionOn) payload_length++; - if (rx_cfg->enable_address_filtering != filteringOff) payload_length++; + if (rx_cfg->enable_length_byte == optionOn) + payload_length++; + + if (rx_cfg->enable_address_filtering != filteringOff) + payload_length++; + SET_CHECKED(rf69_set_payload_length(dev->spi, payload_length)); - } - else - { + } else { SET_CHECKED(rf69_set_payload_length(dev->spi, 0)); } /* values */ if (rx_cfg->enable_sync == optionOn) - { SET_CHECKED(rf69_set_sync_values(dev->spi, rx_cfg->sync_pattern)); - } - if (rx_cfg->enable_address_filtering != filteringOff) - { + + if (rx_cfg->enable_address_filtering != filteringOff) { SET_CHECKED(rf69_set_node_address (dev->spi, rx_cfg->node_address)); SET_CHECKED(rf69_set_broadcast_address(dev->spi, rx_cfg->broadcast_address));
[PATCH 1/3][staging-next] staging: pi433: Make a couple of functions static
From: Colin Ian King The functions pi433_receive and pi433_tx_thread are local to the source and do not need to be in global scope, so make them static Cleans up sparse warnings: symbol 'pi433_receive' was not declared. Should it be static? symbol 'pi433_tx_thread' was not declared. Should it be static? Signed-off-by: Colin Ian King --- drivers/staging/pi433/pi433_if.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index d9328ce5ec1d..95930a192de4 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -313,7 +313,7 @@ pi433_start_rx(struct pi433_device *dev) /*-*/ -int +static int pi433_receive(void *data) { struct pi433_device *dev = data; @@ -463,7 +463,7 @@ pi433_receive(void *data) return bytes_total; } -int +static int pi433_tx_thread(void *data) { struct pi433_device *device = data; -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3][staging-next] staging: pi433: fix thread_run failure check
From: Colin Ian King The check for thread_run failure is incorrect, use IS_ERR instead. Cleans up sparse error message: "error: incompatible types for operation (<)" Fixes: 874bcba65f9a ("staging: pi433: New driver") Signed-off-by: Colin Ian King --- drivers/staging/pi433/pi433_if.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 95930a192de4..266e572ae66e 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -1152,8 +1152,7 @@ static int pi433_probe(struct spi_device *spi) device->tx_task_struct = kthread_run(pi433_tx_thread, device, "pi433_tx_task"); - if (device->tx_task_struct < 0) - { + if (IS_ERR(device->tx_task_struct)) { dev_dbg(device->dev, "start of send thread failed"); goto send_thread_failed; } -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3][staging-next] staging: pi433: Make functions rf69_set_bandwidth_intern static
From: Colin Ian King The function rf69_set_bandwidth_intern is local to the source and do not need to be in global scope, so make it static. Also break overly wide line. Cleans up sparse warning: symbol 'update_share_count' was not declared. Should it be static? Signed-off-by: Colin Ian King --- drivers/staging/pi433/rf69.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c index e391ce777bc7..04af906476e3 100644 --- a/drivers/staging/pi433/rf69.c +++ b/drivers/staging/pi433/rf69.c @@ -433,7 +433,8 @@ int rf69_set_dc_cut_off_frequency_during_afc(struct spi_device *spi, enum dccPer return rf69_set_dc_cut_off_frequency_intern(spi, REG_AFCBW, dccPercent); } -int rf69_set_bandwidth_intern(struct spi_device *spi, u8 reg, enum mantisse mantisse, u8 exponent) +static int rf69_set_bandwidth_intern(struct spi_device *spi, u8 reg, +enum mantisse mantisse, u8 exponent) { u8 newValue; -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] staging: wlan-ng: prism2sta: Use endian types in 'comm_tallies_{16, 32}' structs fields
Fields from structures 'hfa384x_comm_tallies_16' and 'hfa384x_comm_tallies_32' represent little-endian integers (16-bit and 32-bit, respectively), so declare them as such in the containing structure and access them as such in the 'prism2sta_inf_tallies()' function. Fixes the following sparse warnings being raised when the structures' fields are accessed via 'le16_to_cpu()' and 'le32_to_cpu()' functions: prism2sta.c:1010:33: warning: cast to restricted __le32 prism2sta.c:1015:33: warning: cast to restricted __le16 Signed-off-by: Ricardo Silva --- drivers/staging/wlan-ng/hfa384x.h | 84 ++--- drivers/staging/wlan-ng/prism2sta.c | 8 ++-- 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h index 9837a591e7e3..0fe316b5ab5d 100644 --- a/drivers/staging/wlan-ng/hfa384x.h +++ b/drivers/staging/wlan-ng/hfa384x.h @@ -598,51 +598,51 @@ struct hfa384x_rx_frame { /*-- Inquiry Frame, Diagnose: Communication Tallies --*/ struct hfa384x_comm_tallies_16 { - u16 txunicastframes; - u16 txmulticastframes; - u16 txfragments; - u16 txunicastoctets; - u16 txmulticastoctets; - u16 txdeferredtrans; - u16 txsingleretryframes; - u16 txmultipleretryframes; - u16 txretrylimitexceeded; - u16 txdiscards; - u16 rxunicastframes; - u16 rxmulticastframes; - u16 rxfragments; - u16 rxunicastoctets; - u16 rxmulticastoctets; - u16 rxfcserrors; - u16 rxdiscardsnobuffer; - u16 txdiscardswrongsa; - u16 rxdiscardswepundecr; - u16 rxmsginmsgfrag; - u16 rxmsginbadmsgfrag; + __le16 txunicastframes; + __le16 txmulticastframes; + __le16 txfragments; + __le16 txunicastoctets; + __le16 txmulticastoctets; + __le16 txdeferredtrans; + __le16 txsingleretryframes; + __le16 txmultipleretryframes; + __le16 txretrylimitexceeded; + __le16 txdiscards; + __le16 rxunicastframes; + __le16 rxmulticastframes; + __le16 rxfragments; + __le16 rxunicastoctets; + __le16 rxmulticastoctets; + __le16 rxfcserrors; + __le16 rxdiscardsnobuffer; + __le16 txdiscardswrongsa; + __le16 rxdiscardswepundecr; + __le16 rxmsginmsgfrag; + __le16 rxmsginbadmsgfrag; } __packed; struct hfa384x_comm_tallies_32 { - u32 txunicastframes; - u32 txmulticastframes; - u32 txfragments; - u32 txunicastoctets; - u32 txmulticastoctets; - u32 txdeferredtrans; - u32 txsingleretryframes; - u32 txmultipleretryframes; - u32 txretrylimitexceeded; - u32 txdiscards; - u32 rxunicastframes; - u32 rxmulticastframes; - u32 rxfragments; - u32 rxunicastoctets; - u32 rxmulticastoctets; - u32 rxfcserrors; - u32 rxdiscardsnobuffer; - u32 txdiscardswrongsa; - u32 rxdiscardswepundecr; - u32 rxmsginmsgfrag; - u32 rxmsginbadmsgfrag; + __le32 txunicastframes; + __le32 txmulticastframes; + __le32 txfragments; + __le32 txunicastoctets; + __le32 txmulticastoctets; + __le32 txdeferredtrans; + __le32 txsingleretryframes; + __le32 txmultipleretryframes; + __le32 txretrylimitexceeded; + __le32 txdiscards; + __le32 rxunicastframes; + __le32 rxmulticastframes; + __le32 rxfragments; + __le32 rxunicastoctets; + __le32 rxmulticastoctets; + __le32 rxfcserrors; + __le32 rxdiscardsnobuffer; + __le32 txdiscardswrongsa; + __le32 rxdiscardswepundecr; + __le32 rxmsginmsgfrag; + __le32 rxmsginbadmsgfrag; } __packed; /*-- Inquiry Frame, Diagnose: Scan Results & Subfields--*/ diff --git a/drivers/staging/wlan-ng/prism2sta.c b/drivers/staging/wlan-ng/prism2sta.c index 250af0de9c3e..8c347128e810 100644 --- a/drivers/staging/wlan-ng/prism2sta.c +++ b/drivers/staging/wlan-ng/prism2sta.c @@ -991,9 +991,9 @@ static void prism2sta_inf_tallies(struct wlandevice *wlandev, struct hfa384x_inf_frame *inf) { struct hfa384x *hw = wlandev->priv; - u16 *src16; + __le16 *src16; u32 *dst; - u32 *src32; + __le32 *src32; int i; int cnt; @@ -1005,12 +1005,12 @@ static void prism2sta_inf_tallies(struct wlandevice *wlandev, cnt = sizeof(struct hfa384x_comm_tallies_32) / sizeof(u32); if (inf->framelen > 22) { dst = (u32 *)&hw->tallies; - src32 = (u32 *)&inf->info.commtallies32; + src32 = (__le32 *)&inf->info.commtallies32; for (i = 0; i < cnt; i++, dst++, src32++) *dst += le32_to_cpu(*src32); } else { dst = (u32 *)&hw->tallies; - src16 = (u16 *)&inf->info.commtallies16; + src16 = (__le16 *)&inf->i
[PATCH 0/4] staging: wlan-ng: prism2sta: Fix sparse warnings (endian)
This series intends to fix all sparse warnings left in 'prism2sta.c' from the 'wlan-ng' staging driver. Patch 1 fixes the fields declarations of 'struct hfa384x_comm_tallies_{16.32}' and the way they are accessed from 'prism2sta_inf_tallies()'. Patches 2 and 3 each fix a different struct field declaration to have endianess indication. Patch 4 fixes the remaining endian related sparse warnings in function 'prism2sta_inf_chinforesults()' by using 'in situ' conversions, required due to the same structures being shared for storing both the little-endian values and the converted ones. With this series, no sparse reported issues are left in the 'wlan-ng' driver. Ricardo Silva (4): staging: wlan-ng: prism2sta: Use endian types in 'comm_tallies_{16,32}' structs fields staging: wlan-ng: prism2sta: Use endian type in 'hfa384x_link_status' struct staging: wlan-ng: prism2sta: Use endian type in 'hfa384x_ps_user_count' struct staging: wlan-ng: prism2sta: Use 'in situ' endian conversions in 'prism2sta_inf_chinforesults()' drivers/staging/wlan-ng/hfa384x.h | 88 ++--- drivers/staging/wlan-ng/prism2sta.c | 25 +++ 2 files changed, 60 insertions(+), 53 deletions(-) -- 2.13.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] staging: wlan-ng: prism2sta: Use endian type in 'hfa384x_ps_user_count' struct
The 'usercnt' field from struct 'hfa384x_ps_user_count' represents a 16-bit little-endian integer, so declare it as such to avoid the following sparse warning when accessing it through the 'le16_to_cpu()' function: prism2sta.c:1727:27: warning: cast to restricted __le16 Signed-off-by: Ricardo Silva --- drivers/staging/wlan-ng/hfa384x.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h index b61ace1d1e66..f5a3a1ce21ce 100644 --- a/drivers/staging/wlan-ng/hfa384x.h +++ b/drivers/staging/wlan-ng/hfa384x.h @@ -739,7 +739,7 @@ struct hfa384x_auth_request { /*-- Unsolicited Frame, MAC Mgmt: PSUserCount (AP Only) --*/ struct hfa384x_ps_user_count { - u16 usercnt; + __le16 usercnt; } __packed; struct hfa384x_key_id_changed { -- 2.13.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] staging: wlan-ng: prism2sta: Use endian type in 'hfa384x_link_status' struct
The 'linkstatus' field from struct 'hfa384x_link_status' represents a 16-bit little-endian integer, so declare it as such to avoid the following sparse warning when accessing it through the 'le16_to_cpu()' function: prism2sta.c:1450:31: warning: cast to restricted __le16 Signed-off-by: Ricardo Silva --- drivers/staging/wlan-ng/hfa384x.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h index 0fe316b5ab5d..b61ace1d1e66 100644 --- a/drivers/staging/wlan-ng/hfa384x.h +++ b/drivers/staging/wlan-ng/hfa384x.h @@ -711,7 +711,7 @@ struct hfa384x_hscan_result { #define HFA384x_LINK_ASSOCFAIL ((u16)6) struct hfa384x_link_status { - u16 linkstatus; + __le16 linkstatus; } __packed; /*-- Unsolicited Frame, MAC Mgmt: AssociationStatus (--*/ -- 2.13.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] staging: wlan-ng: prism2sta: Use 'in situ' endian conversions in 'prism2sta_inf_chinforesults()'
Function 'prism2sta_inf_chinforesults()' decodes some little-endian 16-bit values from a received Channel Info Results info frame and stores them on the 'struct hfa384x' associated instance. To note that the structs and fields for storing both the little-endian (in info frame struct) and CPU byte order (in instance struct) values are the same ('struct hfa384x_ch_info_result'). Some sparse warnings are being triggered when accessing the little-endian values using 'le16_to_cpu()' from within 'prism2sta_inf_chinforesults()': prism2sta.c:1139:13: warning: cast to restricted __le16 prism2sta.c:1150:24: warning: cast to restricted __le16 prism2sta.c:1157:37: warning: cast to restricted __le16 prism2sta.c:1158:37: warning: cast to restricted __le16 prism2sta.c:1159:40: warning: cast to restricted __le16 Changing the relevant fields declarations to '__le16' won't fix the issue, because the same struct and fields are also used for storing the values in CPU byte order (which must be 'u16'). This way, use 'in situ' conversion on the info frame fields and then assign the converted values to the associated fields on the instance data side. Add a local 'le16_to_cpus_ret()' function as a helper for this process, doing the 'in situ' conversion and returning the converted value. The 'in situ' conversions on the info frame side fields is safe, in the sense that there are no posterior accesses to the (now already converted) values, otherwise any posterior accesses must consider that the conversion was already done. Signed-off-by: Ricardo Silva --- drivers/staging/wlan-ng/prism2sta.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/staging/wlan-ng/prism2sta.c b/drivers/staging/wlan-ng/prism2sta.c index 8c347128e810..a549ef0e2ff4 100644 --- a/drivers/staging/wlan-ng/prism2sta.c +++ b/drivers/staging/wlan-ng/prism2sta.c @@ -132,6 +132,13 @@ static void prism2sta_inf_authreq_defer(struct wlandevice *wlandev, static void prism2sta_inf_psusercnt(struct wlandevice *wlandev, struct hfa384x_inf_frame *inf); +/* Convert from __le16 to u16 in situ and return converted value. */ +static inline u16 le16_to_cpus_ret(u16 *var) +{ + le16_to_cpus(var); + return *var; +} + /* * prism2sta_open * @@ -1136,7 +1143,7 @@ static void prism2sta_inf_chinforesults(struct wlandevice *wlandev, unsigned int i, n; hw->channel_info.results.scanchannels = - le16_to_cpu(inf->info.chinforesult.scanchannels); + le16_to_cpus_ret(&inf->info.chinforesult.scanchannels); for (i = 0, n = 0; i < HFA384x_CHINFORESULT_MAX; i++) { struct hfa384x_ch_info_result_sub *result; @@ -1147,16 +1154,16 @@ static void prism2sta_inf_chinforesults(struct wlandevice *wlandev, continue; result = &inf->info.chinforesult.result[n]; - chan = le16_to_cpu(result->chid) - 1; + chan = le16_to_cpus_ret(&result->chid) - 1; if (chan < 0 || chan >= HFA384x_CHINFORESULT_MAX) continue; chinforesult = &hw->channel_info.results.result[chan]; chinforesult->chid = chan; - chinforesult->anl = le16_to_cpu(result->anl); - chinforesult->pnl = le16_to_cpu(result->pnl); - chinforesult->active = le16_to_cpu(result->active); + chinforesult->anl = le16_to_cpus_ret(&result->anl); + chinforesult->pnl = le16_to_cpus_ret(&result->pnl); + chinforesult->active = le16_to_cpus_ret(&result->active); pr_debug("chinfo: channel %d, %s level (avg/peak)=%d/%d dB, pcf %d\n", chan + 1, -- 2.13.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: pi433: Coding style fixes
On Tue, Jul 18, 2017 at 02:41:33PM +0200, Simo Koskinen wrote: > Fixes if-statement related warning and errors reported > by checkpatch.pl > > Signed-off-by: Simo Koskinen > --- > drivers/staging/pi433/pi433_if.c | 253 > +-- > drivers/staging/pi433/rf69.c | 69 ++- > 2 files changed, 148 insertions(+), 174 deletions(-) > > diff --git a/drivers/staging/pi433/pi433_if.c > b/drivers/staging/pi433/pi433_if.c > index 1bc478a..3e4be2e 100644 > --- a/drivers/staging/pi433/pi433_if.c > +++ b/drivers/staging/pi433/pi433_if.c > @@ -131,19 +131,14 @@ DIO0_irq_handler(unsigned int irq, void *dev_id, struct > pt_regs *regs) > { > struct pi433_device *device = dev_id; > > - if (device->irq_state[DIO0] == DIO_PacketSent) > - { > + if (device->irq_state[DIO0] == DIO_PacketSent) { > device->free_in_fifo = FIFO_SIZE; > printk("DIO0 irq: Packet sent\n"); // TODO: printk() should > include KERN_ facility level > wake_up_interruptible(&device->fifo_wait_queue); > - } > - else if (device->irq_state[DIO0] == DIO_Rssi_DIO0) > - { > + } else if (device->irq_state[DIO0] == DIO_Rssi_DIO0) { > printk("DIO0 irq: RSSI level over threshold\n"); > wake_up_interruptible(&device->rx_wait_queue); > - } > - else if (device->irq_state[DIO0] == DIO_PayloadReady) > - { > + } else if (device->irq_state[DIO0] == DIO_PayloadReady) { > printk("DIO0 irq: PayloadReady\n"); > device->free_in_fifo = 0; > wake_up_interruptible(&device->fifo_wait_queue); > @@ -157,15 +152,15 @@ DIO1_irq_handler(unsigned int irq, void *dev_id, struct > pt_regs *regs) > { > struct pi433_device *device = dev_id; > > - if (device->irq_state[DIO1] == DIO_FifoNotEmpty_DIO1) > - { > + if (device->irq_state[DIO1] == DIO_FifoNotEmpty_DIO1) { > device->free_in_fifo = FIFO_SIZE; > + } else if (device->irq_state[DIO1] == DIO_FifoLevel) { > + if (device->rx_active) > + device->free_in_fifo = FIFO_THRESHOLD - 1; > + else > + device->free_in_fifo = FIFO_SIZE - FIFO_THRESHOLD - 1; > } > - else if (device->irq_state[DIO1] == DIO_FifoLevel) > - { > - if (device->rx_active) device->free_in_fifo = FIFO_THRESHOLD - > 1; > - elsedevice->free_in_fifo = FIFO_SIZE - > FIFO_THRESHOLD - 1; > - } > + > printk("DIO1 irq: %d bytes free in fifo\n", device->free_in_fifo); // > TODO: printk() should include KERN_ facility level > wake_up_interruptible(&device->fifo_wait_queue); > > @@ -200,43 +195,38 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct > pi433_rx_cfg *rx_cfg) > /* packet config */ > /* enable */ > SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync)); > - if (rx_cfg->enable_sync == optionOn) > - { > + if (rx_cfg->enable_sync == optionOn) { > SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, > afterSyncInterrupt)); > - } > - else > - { > + } else { > SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always)); > } > + > SET_CHECKED(rf69_set_packet_format (dev->spi, > rx_cfg->enable_length_byte)); > SET_CHECKED(rf69_set_adressFiltering(dev->spi, > rx_cfg->enable_address_filtering)); > SET_CHECKED(rf69_set_crc_enable (dev->spi, rx_cfg->enable_crc)); > > /* lengths */ > SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length)); > - if (rx_cfg->enable_length_byte == optionOn) > - { > + if (rx_cfg->enable_length_byte == optionOn) { > SET_CHECKED(rf69_set_payload_length(dev->spi, 0xff)); > - } > - else if (rx_cfg->fixed_message_length != 0) > - { > + } else if (rx_cfg->fixed_message_length != 0) { > payload_length = rx_cfg->fixed_message_length; > - if (rx_cfg->enable_length_byte == optionOn) payload_length++; > - if (rx_cfg->enable_address_filtering != filteringOff) > payload_length++; > + if (rx_cfg->enable_length_byte == optionOn) > + payload_length++; > + Don't add new coding style issues :( Always be careful, and ideally, you would show the before and after results are the same for such a large patch... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/7] staging: fsl-mc: make the driver compile on other architectures
From: Laurentiu Tudor Apart from a small change (first patch) which adds a missing comment, this series make the bus driver compile on other architectures, as per GregKH comment [1]. Compiled tested on: - booke powerpc (corenet{32,64}_smp_defconfig) with this ppc patch [2] - x86 (i386_defconfig, x86_64_defconfig, needs CONFIG_OF) - arm64 (defconfig) [1] https://www.spinics.net/lists/linux-driver-devel/msg100585.html [2] https://patchwork.ozlabs.org/patch/789474/ version 2 changes - use writeq() / writeq_relaxed() / readq_relaxed() instead of raw versions (Robin) - use linux/io-64-nonatomic-hi-lo.h to make the driver compile on 32-bit platforms (Robin) - add extra LE <-> CPU so that standard device io may be used (Arnd) Laurentiu Tudor (7): staging: fsl-mc: add missing fsl_mc comment in struct msi_desc staging: fsl-mc: use generic memory barriers staging: fsl-mc: drop useless gic v3 related #include staging: fsl-mc: fix compilation with non-generic msi domain ops staging: fsl-mc: fix formating of phys_addr_t on 32 bits staging: fsl-mc: rewrite mc command send/receive to work on 32-bits staging: fsl-mc: allow the driver compile multi-arch drivers/staging/fsl-dpaa2/Kconfig | 2 +- drivers/staging/fsl-mc/bus/Kconfig | 4 +- drivers/staging/fsl-mc/bus/fsl-mc-msi.c| 5 ++- .../staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 3 +- drivers/staging/fsl-mc/bus/mc-io.c | 8 ++-- drivers/staging/fsl-mc/bus/mc-sys.c| 45 +- include/linux/msi.h| 1 + 7 files changed, 32 insertions(+), 36 deletions(-) -- 2.9.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/8] staging: fsl-mc: add missing fsl_mc comment in struct msi_desc
From: Laurentiu Tudor The mc-bus specific field, fsl_mc in struct msi_desc is missing its comment so add it. Signed-off-by: Laurentiu Tudor --- Notes: -v2 -no changes include/linux/msi.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/msi.h b/include/linux/msi.h index df6d592..80e3b56 100644 --- a/include/linux/msi.h +++ b/include/linux/msi.h @@ -66,6 +66,7 @@ struct fsl_mc_msi_desc { * @mask_pos: [PCI MSI] Mask register position * @mask_base: [PCI MSI-X] Mask register base address * @platform: [platform] Platform device specific msi descriptor data + * @fsl_mc:[fsl-mc]FSL MC device specific msi descriptor data */ struct msi_desc { /* Shared device/bus type independent data */ -- 2.9.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 6/8] staging: fsl-mc: don't use raw device io functions
From: Laurentiu Tudor As raw device io functions are not portable and don't handle byte-order (triggering suspicion that endianness isn't handled well) switch to using the standard api. Since MC expects LE byte-order and the upper layers already take care of that, we need to trick the device io api by doing a LE -> CPU conversion just before calling it. This way, the CPU -> LE conversion done in the api puts the data back in the right byte-order. Obviously, for reads the extra step is mirrored: there's a CPU -> LE conversion following the API call. Signed-off-by: Laurentiu Tudor --- Notes: -v2 -new patch replacing https://lkml.org/lkml/2017/7/17/419 drivers/staging/fsl-mc/bus/mc-sys.c | 21 +++-- 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c b/drivers/staging/fsl-mc/bus/mc-sys.c index 195d9f3..8a6dc47 100644 --- a/drivers/staging/fsl-mc/bus/mc-sys.c +++ b/drivers/staging/fsl-mc/bus/mc-sys.c @@ -126,12 +126,15 @@ static inline void mc_write_command(struct mc_command __iomem *portal, /* copy command parameters into the portal */ for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++) - __raw_writeq(cmd->params[i], &portal->params[i]); - /* ensure command params are committed before submitting it */ - wmb(); + /* +* Data is already in the expected LE byte-order. Do an +* extra LE -> CPU conversion so that the CPU -> LE done in +* the device io write api puts it back in the right order. +*/ + writeq_relaxed(le64_to_cpu(cmd->params[i]), &portal->params[i]); /* submit the command by writing the header */ - __raw_writeq(cmd->header, &portal->header); + writeq(le64_to_cpu(cmd->header), &portal->header); } /** @@ -151,14 +154,20 @@ static inline enum mc_cmd_status mc_read_response(struct mc_command __iomem * enum mc_cmd_status status; /* Copy command response header from MC portal: */ - resp->header = __raw_readq(&portal->header); + resp->header = cpu_to_le64(readq_relaxed(&portal->header)); status = mc_cmd_hdr_read_status(resp); if (status != MC_CMD_STATUS_OK) return status; /* Copy command response data from MC portal: */ for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++) - resp->params[i] = __raw_readq(&portal->params[i]); + /* +* Data is expected to be in LE byte-order. Do an +* extra CPU -> LE to revert the LE -> CPU done in +* the device io read api. +*/ + resp->params[i] = + cpu_to_le64(readq_relaxed(&portal->params[i])); return status; } -- 2.9.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 5/8] staging: fsl-mc: fix formating of phys_addr_t on 32 bits
From: Laurentiu Tudor Use correct format specifier for phys_addr_t variables (%pa) instead of %llx. This fixes these warnings on 32 bit targets: "format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'phys_addr_t' [-Wformat=]" Signed-off-by: Laurentiu Tudor --- Notes: -v2 -no changes drivers/staging/fsl-mc/bus/mc-io.c | 8 drivers/staging/fsl-mc/bus/mc-sys.c | 12 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/mc-io.c b/drivers/staging/fsl-mc/bus/mc-io.c index 35221a17..4e6f99a 100644 --- a/drivers/staging/fsl-mc/bus/mc-io.c +++ b/drivers/staging/fsl-mc/bus/mc-io.c @@ -129,8 +129,8 @@ int __must_check fsl_create_mc_io(struct device *dev, "mc_portal"); if (!res) { dev_err(dev, - "devm_request_mem_region failed for MC portal %#llx\n", - mc_portal_phys_addr); + "devm_request_mem_region failed for MC portal %pa\n", + &mc_portal_phys_addr); return -EBUSY; } @@ -139,8 +139,8 @@ int __must_check fsl_create_mc_io(struct device *dev, mc_portal_size); if (!mc_portal_virt_addr) { dev_err(dev, - "devm_ioremap_nocache failed for MC portal %#llx\n", - mc_portal_phys_addr); + "devm_ioremap_nocache failed for MC portal %pa\n", + &mc_portal_phys_addr); return -ENXIO; } diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c b/drivers/staging/fsl-mc/bus/mc-sys.c index 012abd5..195d9f3 100644 --- a/drivers/staging/fsl-mc/bus/mc-sys.c +++ b/drivers/staging/fsl-mc/bus/mc-sys.c @@ -196,8 +196,8 @@ static int mc_polling_wait_preemptible(struct fsl_mc_io *mc_io, if (time_after_eq(jiffies, jiffies_until_timeout)) { dev_dbg(mc_io->dev, - "MC command timed out (portal: %#llx, dprc handle: %#x, command: %#x)\n", -mc_io->portal_phys_addr, + "MC command timed out (portal: %pa, dprc handle: %#x, command: %#x)\n", +&mc_io->portal_phys_addr, (unsigned int)mc_cmd_hdr_read_token(cmd), (unsigned int)mc_cmd_hdr_read_cmdid(cmd)); @@ -236,8 +236,8 @@ static int mc_polling_wait_atomic(struct fsl_mc_io *mc_io, timeout_usecs -= MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS; if (timeout_usecs == 0) { dev_dbg(mc_io->dev, - "MC command timed out (portal: %#llx, dprc handle: %#x, command: %#x)\n", -mc_io->portal_phys_addr, + "MC command timed out (portal: %pa, dprc handle: %#x, command: %#x)\n", +&mc_io->portal_phys_addr, (unsigned int)mc_cmd_hdr_read_token(cmd), (unsigned int)mc_cmd_hdr_read_cmdid(cmd)); @@ -290,8 +290,8 @@ int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd) if (status != MC_CMD_STATUS_OK) { dev_dbg(mc_io->dev, - "MC command failed: portal: %#llx, dprc handle: %#x, command: %#x, status: %s (%#x)\n", -mc_io->portal_phys_addr, + "MC command failed: portal: %pa, dprc handle: %#x, command: %#x, status: %s (%#x)\n", +&mc_io->portal_phys_addr, (unsigned int)mc_cmd_hdr_read_token(cmd), (unsigned int)mc_cmd_hdr_read_cmdid(cmd), mc_status_to_string(status), -- 2.9.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/8] staging: fsl-mc: use generic memory barriers
From: Laurentiu Tudor No need to use arch-specific memory barriers; switch to using generic ones. The rmb()s were useless so drop them. Signed-off-by: Laurentiu Tudor --- Notes: -v2 -no changes drivers/staging/fsl-mc/bus/mc-sys.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c b/drivers/staging/fsl-mc/bus/mc-sys.c index a1704c3..012abd5 100644 --- a/drivers/staging/fsl-mc/bus/mc-sys.c +++ b/drivers/staging/fsl-mc/bus/mc-sys.c @@ -127,7 +127,8 @@ static inline void mc_write_command(struct mc_command __iomem *portal, /* copy command parameters into the portal */ for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++) __raw_writeq(cmd->params[i], &portal->params[i]); - __iowmb(); + /* ensure command params are committed before submitting it */ + wmb(); /* submit the command by writing the header */ __raw_writeq(cmd->header, &portal->header); @@ -150,9 +151,7 @@ static inline enum mc_cmd_status mc_read_response(struct mc_command __iomem * enum mc_cmd_status status; /* Copy command response header from MC portal: */ - __iormb(); resp->header = __raw_readq(&portal->header); - __iormb(); status = mc_cmd_hdr_read_status(resp); if (status != MC_CMD_STATUS_OK) return status; @@ -160,7 +159,6 @@ static inline enum mc_cmd_status mc_read_response(struct mc_command __iomem * /* Copy command response data from MC portal: */ for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++) resp->params[i] = __raw_readq(&portal->params[i]); - __iormb(); return status; } -- 2.9.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/8] staging: fsl-mc: drop useless gic v3 related #include
From: Laurentiu Tudor Nothing from linux/irqchip/arm-gic-v3.h is used, so the #include can be safely dropped. Signed-off-by: Laurentiu Tudor --- Notes: -v2 -no changes drivers/staging/fsl-mc/bus/fsl-mc-msi.c| 1 - drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 1 - 2 files changed, 2 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/fsl-mc-msi.c index c04a2f2..ee6e3b7 100644 --- a/drivers/staging/fsl-mc/bus/fsl-mc-msi.c +++ b/drivers/staging/fsl-mc/bus/fsl-mc-msi.c @@ -11,7 +11,6 @@ #include #include -#include #include #include #include diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c index 865d385..e798ea4 100644 --- a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c +++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c @@ -11,7 +11,6 @@ #include #include -#include #include #include #include -- 2.9.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 7/8] staging: fsl-mc: make the driver compile on 32-bit
From: Laurentiu Tudor Since there's no real constrain in MC to do only atomic 64-bit we can enable this driver on 32-bit platforms too. Include linux/io-64-nonatomic-hi-lo.h to make quad device io apis used in the driver available on 32-bit platforms. Signed-off-by: Laurentiu Tudor --- Notes: -v2 -new patch replacing https://lkml.org/lkml/2017/7/17/419 drivers/staging/fsl-mc/bus/mc-sys.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c b/drivers/staging/fsl-mc/bus/mc-sys.c index 8a6dc47..7ce105b 100644 --- a/drivers/staging/fsl-mc/bus/mc-sys.c +++ b/drivers/staging/fsl-mc/bus/mc-sys.c @@ -37,6 +37,7 @@ #include #include #include +#include #include "../include/mc.h" #include "dpmcp.h" -- 2.9.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/8] staging: fsl-mc: fix compilation with non-generic msi domain ops
From: Laurentiu Tudor The bus driver relies on generic msi domain ops. Fix compilation for architectures that don't provide it (e.g. x86_64). Signed-off-by: Laurentiu Tudor --- Notes: -v2 -no changes drivers/staging/fsl-mc/bus/fsl-mc-msi.c| 4 drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 2 ++ 2 files changed, 6 insertions(+) diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/fsl-mc-msi.c index ee6e3b7..18774ee 100644 --- a/drivers/staging/fsl-mc/bus/fsl-mc-msi.c +++ b/drivers/staging/fsl-mc/bus/fsl-mc-msi.c @@ -17,6 +17,7 @@ #include #include "fsl-mc-private.h" +#ifdef GENERIC_MSI_DOMAIN_OPS /* * Generate a unique ID identifying the interrupt (only used within the MSI * irqdomain. Combine the icid with the interrupt index. @@ -38,6 +39,9 @@ static void fsl_mc_msi_set_desc(msi_alloc_info_t *arg, arg->hwirq = fsl_mc_domain_calc_hwirq(to_fsl_mc_device(desc->dev), desc); } +#else +#define fsl_mc_msi_set_desc NULL +#endif static void fsl_mc_msi_update_dom_ops(struct msi_domain_info *info) { diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c index e798ea4..cd73c58 100644 --- a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c +++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c @@ -45,7 +45,9 @@ static int its_fsl_mc_msi_prepare(struct irq_domain *msi_domain, * NOTE: This device id corresponds to the IOMMU stream ID * associated with the DPRC object (ICID). */ +#ifdef GENERIC_MSI_DOMAIN_OPS info->scratchpad[0].ul = mc_bus_dev->icid; +#endif msi_info = msi_get_domain_info(msi_domain->parent); return msi_info->ops->msi_prepare(msi_domain->parent, dev, nvec, info); } -- 2.9.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 8/8] staging: fsl-mc: allow the driver compile multi-arch
From: Laurentiu Tudor Drop dependency on ARCH_LAYERSCAPE (which in turn depends on ARM64), thus leaving this driver compile on other architectures. Also, other drivers depending on the bus are updated to depend on ARCH_LAYERSCAPE until they'll also be made multi-arch. This was compiled tested on: - booke powerpc (corenet{32,64}_smp_defconfig) - x86 (i386_defconfig, x86_64_defconfig, needs CONFIG_OF) - arm64 (defconfig) Signed-off-by: Laurentiu Tudor --- Notes: -v2 -no changes drivers/staging/fsl-dpaa2/Kconfig | 2 +- drivers/staging/fsl-mc/bus/Kconfig | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/fsl-dpaa2/Kconfig b/drivers/staging/fsl-dpaa2/Kconfig index 730fd6d..dfff675 100644 --- a/drivers/staging/fsl-dpaa2/Kconfig +++ b/drivers/staging/fsl-dpaa2/Kconfig @@ -4,7 +4,7 @@ config FSL_DPAA2 bool "Freescale DPAA2 devices" - depends on FSL_MC_BUS + depends on FSL_MC_BUS && ARCH_LAYERSCAPE ---help--- Build drivers for Freescale DataPath Acceleration Architecture (DPAA2) family of SoCs. diff --git a/drivers/staging/fsl-mc/bus/Kconfig b/drivers/staging/fsl-mc/bus/Kconfig index a10aaf0..c0acc97 100644 --- a/drivers/staging/fsl-mc/bus/Kconfig +++ b/drivers/staging/fsl-mc/bus/Kconfig @@ -8,7 +8,7 @@ config FSL_MC_BUS bool "QorIQ DPAA2 fsl-mc bus driver" - depends on OF && ARCH_LAYERSCAPE + depends on OF select GENERIC_MSI_IRQ_DOMAIN help Driver to enable the bus infrastructure for the QorIQ DPAA2 @@ -18,7 +18,7 @@ config FSL_MC_BUS config FSL_MC_DPIO tristate "QorIQ DPAA2 DPIO driver" -depends on FSL_MC_BUS +depends on FSL_MC_BUS && ARCH_LAYERSCAPE help Driver for the DPAA2 DPIO object. A DPIO provides queue and buffer management facilities for software to interact with -- 2.9.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export
> The patch is untested but I can work on this if that fits in with the > plans for tty. I think this is the right direction. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] staging: wlan-ng: prism2sta: Use 'in situ' endian conversions in 'prism2sta_inf_chinforesults()'
No no no no no no... Don't change them in place for no reason just because it doesn't cause a bug. It's better to just cast away the warnings except we're going to automatically NAK that patch as well... But this is even worse. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 6/8] staging: fsl-mc: don't use raw device io functions
On Tue, Jul 18, 2017 at 3:37 PM, wrote: > From: Laurentiu Tudor > > As raw device io functions are not portable and don't handle byte-order > (triggering suspicion that endianness isn't handled well) switch to > using the standard api. > Since MC expects LE byte-order and the upper layers already take care > of that, we need to trick the device io api by doing a LE -> CPU > conversion just before calling it. This way, the CPU -> LE conversion > done in the api puts the data back in the right byte-order. Obviously, > for reads the extra step is mirrored: there's a CPU -> LE conversion > following the API call. > > Signed-off-by: Laurentiu Tudor > --- > Notes: > -v2 > -new patch replacing https://lkml.org/lkml/2017/7/17/419 > > drivers/staging/fsl-mc/bus/mc-sys.c | 21 +++-- > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c > b/drivers/staging/fsl-mc/bus/mc-sys.c > index 195d9f3..8a6dc47 100644 > --- a/drivers/staging/fsl-mc/bus/mc-sys.c > +++ b/drivers/staging/fsl-mc/bus/mc-sys.c > @@ -126,12 +126,15 @@ static inline void mc_write_command(struct mc_command > __iomem *portal, > > /* copy command parameters into the portal */ > for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++) > - __raw_writeq(cmd->params[i], &portal->params[i]); > - /* ensure command params are committed before submitting it */ > - wmb(); > + /* > +* Data is already in the expected LE byte-order. Do an > +* extra LE -> CPU conversion so that the CPU -> LE done in > +* the device io write api puts it back in the right order. > +*/ > + writeq_relaxed(le64_to_cpu(cmd->params[i]), > &portal->params[i]); > > /* submit the command by writing the header */ > - __raw_writeq(cmd->header, &portal->header); > + writeq(le64_to_cpu(cmd->header), &portal->header); > } Looks good, but just to be sure this is what you intended: On 32-bit systems, this will now write val>>32 to cmd->header+4, followed by writing val&0x to cmd->header. You said earlier that the command is triggered when the final four bytes are written, but it looks like the order is wrong now. Should you use io-64-nonatomic-lo-hi.h instead of io-64-nonatomic-hi-lo.h then? Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 8/8] staging: fsl-mc: allow the driver compile multi-arch
On Tue, Jul 18, 2017 at 3:37 PM, wrote: > --- a/drivers/staging/fsl-dpaa2/Kconfig > +++ b/drivers/staging/fsl-dpaa2/Kconfig > @@ -4,7 +4,7 @@ > > config FSL_DPAA2 > bool "Freescale DPAA2 devices" > - depends on FSL_MC_BUS > + depends on FSL_MC_BUS && ARCH_LAYERSCAPE > ---help--- > Build drivers for Freescale DataPath Acceleration > Architecture (DPAA2) family of SoCs. I would probably leave the dependency in there conditionally, like depends on ARCH_LAYERSCAPE || COMPILE_TEST That way, we can build the driver on all architectures with "make allmodconfig" or "make randconfig", but regular users that disable COMPILE_TEST won't be bothered by the extra config options unless they have the right hardware. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/7] staging: fsl-mc: make the driver compile on other architectures
On Tue, Jul 18, 2017 at 3:37 PM, wrote: > From: Laurentiu Tudor > > Apart from a small change (first patch) which adds a missing comment, > this series make the bus driver compile on other architectures, as per > GregKH comment [1]. > Compiled tested on: > - booke powerpc (corenet{32,64}_smp_defconfig) with this ppc patch [2] > - x86 (i386_defconfig, x86_64_defconfig, needs CONFIG_OF) > - arm64 (defconfig) > > [1] https://www.spinics.net/lists/linux-driver-devel/msg100585.html > [2] https://patchwork.ozlabs.org/patch/789474/ Looks good overall. With the two minor questions addressed Acked-by: Arnd Bergmann ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 6/8] staging: fsl-mc: don't use raw device io functions
On Tue, Jul 18, 2017 at 4:26 PM, Laurentiu Tudor wrote: > > Maybe i made an error in my previous emails, but the hi-lo variant is > the correct one. The command execution is triggered when the _first_ > 32-bit half of the header (header&0x) is written, so that's why > it must be written last. I'm pretty sure I just misremembered it then. Thanks for the clarification. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 8/8] staging: fsl-mc: allow the driver compile multi-arch
On Tue, Jul 18, 2017 at 4:36 PM, Laurentiu Tudor wrote: > Good point, I'll take care of it. But don't you mean COMPILE_TEST be > added on the actual MC_BUS config, like so: > > config FSL_MC_BUS > bool "QorIQ DPAA2 fsl-mc bus driver" > - depends on OF && ARCH_LAYERSCAPE > + depends on OF && (ARCH_LAYERSCAPE || COMPILE_TEST) > select GENERIC_MSI_IRQ_DOMAIN > ? > > The other drivers that depend on the MC_BUS won't compile on other > architectures. I was thinking of adding it to all three, but you are right we only needed it for the bus. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 6/8] staging: fsl-mc: don't use raw device io functions
Hi Arnd, On 07/18/2017 05:18 PM, Arnd Bergmann wrote: > On Tue, Jul 18, 2017 at 3:37 PM, wrote: >> From: Laurentiu Tudor >> >> As raw device io functions are not portable and don't handle byte-order >> (triggering suspicion that endianness isn't handled well) switch to >> using the standard api. >> Since MC expects LE byte-order and the upper layers already take care >> of that, we need to trick the device io api by doing a LE -> CPU >> conversion just before calling it. This way, the CPU -> LE conversion >> done in the api puts the data back in the right byte-order. Obviously, >> for reads the extra step is mirrored: there's a CPU -> LE conversion >> following the API call. >> >> Signed-off-by: Laurentiu Tudor >> --- >> Notes: >> -v2 >>-new patch replacing >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2017%2F7%2F17%2F419&data=01%7C01%7Claurentiu.tudor%40nxp.com%7C77381272b4914c9ac64708d4cde7d94e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=FTVLKox6T4i9OFmb%2B5BkSEDQrDrafXznY6nsJ0dgFSk%3D&reserved=0 >> >> drivers/staging/fsl-mc/bus/mc-sys.c | 21 +++-- >> 1 file changed, 15 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c >> b/drivers/staging/fsl-mc/bus/mc-sys.c >> index 195d9f3..8a6dc47 100644 >> --- a/drivers/staging/fsl-mc/bus/mc-sys.c >> +++ b/drivers/staging/fsl-mc/bus/mc-sys.c >> @@ -126,12 +126,15 @@ static inline void mc_write_command(struct mc_command >> __iomem *portal, >> >> /* copy command parameters into the portal */ >> for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++) >> - __raw_writeq(cmd->params[i], &portal->params[i]); >> - /* ensure command params are committed before submitting it */ >> - wmb(); >> + /* >> +* Data is already in the expected LE byte-order. Do an >> +* extra LE -> CPU conversion so that the CPU -> LE done in >> +* the device io write api puts it back in the right order. >> +*/ >> + writeq_relaxed(le64_to_cpu(cmd->params[i]), >> &portal->params[i]); >> >> /* submit the command by writing the header */ >> - __raw_writeq(cmd->header, &portal->header); >> + writeq(le64_to_cpu(cmd->header), &portal->header); >> } > > Looks good, but just to be sure this is what you intended: > > On 32-bit systems, this will now write val>>32 to cmd->header+4, > followed by writing val&0x to cmd->header. Right. That's how it should happen. > You said earlier that the command is triggered when the final > four bytes are written, but it looks like the order is wrong now. > > Should you use io-64-nonatomic-lo-hi.h instead of > io-64-nonatomic-hi-lo.h then? > Maybe i made an error in my previous emails, but the hi-lo variant is the correct one. The command execution is triggered when the _first_ 32-bit half of the header (header&0x) is written, so that's why it must be written last. --- Thanks & Best Regards, Laurentiu ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: goldfish: Use __func__ instead of function name
Replace hard-coded function names in strings with "%s", __func__ in the goldfish_nand.c file. Issue found by checkpatch.pl. Signed-off-by: Chris Coffey --- drivers/staging/goldfish/goldfish_nand.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/staging/goldfish/goldfish_nand.c b/drivers/staging/goldfish/goldfish_nand.c index 8f92ff4ba4..e742b92bb1 100644 --- a/drivers/staging/goldfish/goldfish_nand.c +++ b/drivers/staging/goldfish/goldfish_nand.c @@ -157,8 +157,8 @@ static int goldfish_nand_read_oob(struct mtd_info *mtd, loff_t ofs, return 0; invalid_arg: - pr_err("goldfish_nand_read_oob: invalid read, start %llx, len %zx, ooblen %zx, dev_size %llx, write_size %x\n", - ofs, ops->len, ops->ooblen, mtd->size, mtd->writesize); + pr_err("%s: invalid read, start %llx, len %zx, ooblen %zx, dev_size %llx, write_size %x\n", + __func__, ofs, ops->len, ops->ooblen, mtd->size, mtd->writesize); return -EINVAL; } @@ -189,8 +189,8 @@ static int goldfish_nand_write_oob(struct mtd_info *mtd, loff_t ofs, return 0; invalid_arg: - pr_err("goldfish_nand_write_oob: invalid write, start %llx, len %zx, ooblen %zx, dev_size %llx, write_size %x\n", - ofs, ops->len, ops->ooblen, mtd->size, mtd->writesize); + pr_err("%s: invalid write, start %llx, len %zx, ooblen %zx, dev_size %llx, write_size %x\n", + __func__, ofs, ops->len, ops->ooblen, mtd->size, mtd->writesize); return -EINVAL; } @@ -211,8 +211,8 @@ static int goldfish_nand_read(struct mtd_info *mtd, loff_t from, size_t len, return 0; invalid_arg: - pr_err("goldfish_nand_read: invalid read, start %llx, len %zx, dev_size %llx, write_size %x\n", - from, len, mtd->size, mtd->writesize); + pr_err("%s: invalid read, start %llx, len %zx, dev_size %llx, write_size %x\n", + __func__, from, len, mtd->size, mtd->writesize); return -EINVAL; } @@ -233,8 +233,8 @@ static int goldfish_nand_write(struct mtd_info *mtd, loff_t to, size_t len, return 0; invalid_arg: - pr_err("goldfish_nand_write: invalid write, start %llx, len %zx, dev_size %llx, write_size %x\n", - to, len, mtd->size, mtd->writesize); + pr_err("%s: invalid write, start %llx, len %zx, dev_size %llx, write_size %x\n", + __func__, to, len, mtd->size, mtd->writesize); return -EINVAL; } @@ -340,8 +340,8 @@ static int goldfish_nand_init_device(struct platform_device *pdev, name); if (result != name_len) { dev_err(&pdev->dev, - "goldfish_nand_init_device failed to get dev name %d != %d\n", - result, name_len); + "%s: failed to get dev name %d != %d\n", + __func__, result, name_len); return -ENODEV; } ((char *)mtd->name)[name_len] = '\0'; -- 2.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] staging: wlan-ng: prism2sta: Use 'in situ' endian conversions in 'prism2sta_inf_chinforesults()'
Hello Dan, On 07/18/2017 02:52 PM, Dan Carpenter wrote: > No no no no no no... Don't change them in place for no reason just > because it doesn't cause a bug. It's better to just cast away the > warnings except we're going to automatically NAK that patch as well... > But this is even worse. > Got it. Thanks for reviewing. Another possibility would be to use different (although twin) structs for the info frame data (using __le16) and the associated instance data (using u16), but that would mean having to maintain the 2 structs in sync with each other in case some change is needed. Would this be acceptable? Or, shall I just leave the warnings there? Regards, Ricardo Silva > regards, > dan carpenter > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 8/8] staging: fsl-mc: allow the driver compile multi-arch
Hi Arnd, On 07/18/2017 05:25 PM, Arnd Bergmann wrote: > On Tue, Jul 18, 2017 at 3:37 PM, wrote: > >> --- a/drivers/staging/fsl-dpaa2/Kconfig >> +++ b/drivers/staging/fsl-dpaa2/Kconfig >> @@ -4,7 +4,7 @@ >> >> config FSL_DPAA2 >> bool "Freescale DPAA2 devices" >> - depends on FSL_MC_BUS >> + depends on FSL_MC_BUS && ARCH_LAYERSCAPE >> ---help--- >>Build drivers for Freescale DataPath Acceleration >>Architecture (DPAA2) family of SoCs. > > I would probably leave the dependency in there conditionally, like > > depends on ARCH_LAYERSCAPE || COMPILE_TEST > > That way, we can build the driver on all architectures with "make > allmodconfig" > or "make randconfig", but regular users that disable COMPILE_TEST > won't be bothered by the extra config options unless they have the > right hardware. > Good point, I'll take care of it. But don't you mean COMPILE_TEST be added on the actual MC_BUS config, like so: config FSL_MC_BUS bool "QorIQ DPAA2 fsl-mc bus driver" - depends on OF && ARCH_LAYERSCAPE + depends on OF && (ARCH_LAYERSCAPE || COMPILE_TEST) select GENERIC_MSI_IRQ_DOMAIN ? The other drivers that depend on the MC_BUS won't compile on other architectures. --- Thanks & Best Regards, Laurentiu ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 1/1] Staging: android/ion: declare function static
On Tue, Jul 18, 2017 at 08:58:22AM +0200, Greg KH wrote: > On Sat, Jul 15, 2017 at 11:43:05AM +, Joseph Wright wrote: > > Declare private function static to fix sparse warning: > > > > ion_cma_heap.c:109:5: warning: symbol '__ion_add_cma_heaps' \ > > was not declared. Should it be static? > > > > Signed-off-by: Joseph Wright > > --- > > Changes in v3: > > - Make subject clearer > > > > drivers/staging/android/ion/ion_cma_heap.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Sorry, does not apply to my tree as I think someone else already did > this before you did :( Okay, thanks anyway. Joseph ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723bs: rtw_efuse: Fix a misspell
Fix a comment misspell Signed-off-by: Xaralampos Mainas --- drivers/staging/rtl8723bs/core/rtw_efuse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8723bs/core/rtw_efuse.c b/drivers/staging/rtl8723bs/core/rtw_efuse.c index 8e29802..44b92ef 100644 --- a/drivers/staging/rtl8723bs/core/rtw_efuse.c +++ b/drivers/staging/rtl8723bs/core/rtw_efuse.c @@ -163,7 +163,7 @@ Efuse_CalculateWordCnts(u8 word_en) /* Description: */ /* 1. Execute E-Fuse read byte operation according as map offset and */ /* save to E-Fuse table. */ -/* 2. Refered from SD1 Richard. */ +/* 2. Referred from SD1 Richard. */ /* */ /* Assumption: */ /* 1. Boot from E-Fuse and successfully auto-load. */ -- 2.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/7] staging: fsl-mc: make the driver compile on other architectures
Hi Arnd, On 07/18/2017 05:26 PM, Arnd Bergmann wrote: > On Tue, Jul 18, 2017 at 3:37 PM, wrote: >> From: Laurentiu Tudor >> >> Apart from a small change (first patch) which adds a missing comment, >> this series make the bus driver compile on other architectures, as per >> GregKH comment [1]. >> Compiled tested on: >> - booke powerpc (corenet{32,64}_smp_defconfig) with this ppc patch [2] >> - x86 (i386_defconfig, x86_64_defconfig, needs CONFIG_OF) >> - arm64 (defconfig) >> >> [1] >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Flinux-driver-devel%2Fmsg100585.html&data=01%7C01%7Claurentiu.tudor%40nxp.com%7C2564747d26964ed5b3f408d4cde8f442%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=Dz8vvLzQESel2UTXrhQ3JZyNhx5VhUdj6%2BE6TDLnXmc%3D&reserved=0 >> [2] >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.ozlabs.org%2Fpatch%2F789474%2F&data=01%7C01%7Claurentiu.tudor%40nxp.com%7C2564747d26964ed5b3f408d4cde8f442%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=%2FiGSaa4j60INeTWEbT926iEIAtya6tqIiIoQN1yFbmA%3D&reserved=0 > > Looks good overall. With the two minor questions addressed > > Acked-by: Arnd Bergmann > Thanks for the ack. I'll mention it the next version. --- Thanks & Best Regards, Laurentiu ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: wlan-ng: Fix endianness warnings
These variables were set to u16 and u32 although they always hold a little endian value. This patch fixes multiple sparse warnings like: drivers/staging/wlan-ng/prism2sta.c:375:46: warning: cast to restricted __le16 Signed-off-by: Alfonso Lima Astor --- v2: Resubmit patch that wouldn't apply correcty drivers/staging/wlan-ng/hfa384x.h | 48 ++--- drivers/staging/wlan-ng/prism2sta.c | 8 +++ 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h index 018db22..aaa8327 100644 --- a/drivers/staging/wlan-ng/hfa384x.h +++ b/drivers/staging/wlan-ng/hfa384x.h @@ -445,9 +445,9 @@ struct hfa384x_downloadbuffer { /*-- Information Record: commsquality --*/ struct hfa384x_commsquality { - u16 cq_curr_bss; - u16 asl_curr_bss; - u16 anl_curr_fc; + __le16 cq_curr_bss; + __le16 asl_curr_bss; + __le16 anl_curr_fc; } __packed; /*-- Information Record: dmbcommsquality --*/ @@ -598,27 +598,27 @@ struct hfa384x_rx_frame { /*-- Inquiry Frame, Diagnose: Communication Tallies --*/ struct hfa384x_comm_tallies_16 { - u16 txunicastframes; - u16 txmulticastframes; - u16 txfragments; - u16 txunicastoctets; - u16 txmulticastoctets; - u16 txdeferredtrans; - u16 txsingleretryframes; - u16 txmultipleretryframes; - u16 txretrylimitexceeded; - u16 txdiscards; - u16 rxunicastframes; - u16 rxmulticastframes; - u16 rxfragments; - u16 rxunicastoctets; - u16 rxmulticastoctets; - u16 rxfcserrors; - u16 rxdiscardsnobuffer; - u16 txdiscardswrongsa; - u16 rxdiscardswepundecr; - u16 rxmsginmsgfrag; - u16 rxmsginbadmsgfrag; + __le16 txunicastframes; + __le16 txmulticastframes; + __le16 txfragments; + __le16 txunicastoctets; + __le16 txmulticastoctets; + __le16 txdeferredtrans; + __le16 txsingleretryframes; + __le16 txmultipleretryframes; + __le16 txretrylimitexceeded; + __le16 txdiscards; + __le16 rxunicastframes; + __le16 rxmulticastframes; + __le16 rxfragments; + __le16 rxunicastoctets; + __le16 rxmulticastoctets; + __le16 rxfcserrors; + __le16 rxdiscardsnobuffer; + __le16 txdiscardswrongsa; + __le16 rxdiscardswepundecr; + __le16 rxmsginmsgfrag; + __le16 rxmsginbadmsgfrag; } __packed; struct hfa384x_comm_tallies_32 { diff --git a/drivers/staging/wlan-ng/prism2sta.c b/drivers/staging/wlan-ng/prism2sta.c index e16da34..5d0495e 100644 --- a/drivers/staging/wlan-ng/prism2sta.c +++ b/drivers/staging/wlan-ng/prism2sta.c @@ -991,9 +991,9 @@ static void prism2sta_inf_tallies(struct wlandevice *wlandev, struct hfa384x_inf_frame *inf) { struct hfa384x *hw = wlandev->priv; - u16 *src16; + __le16 *src16; u32 *dst; - u32 *src32; + __le32 *src32; int i; int cnt; @@ -1005,12 +1005,12 @@ static void prism2sta_inf_tallies(struct wlandevice *wlandev, cnt = sizeof(struct hfa384x_comm_tallies_32) / sizeof(u32); if (inf->framelen > 22) { dst = (u32 *)&hw->tallies; - src32 = (u32 *)&inf->info.commtallies32; + src32 = (__le32 *)&inf->info.commtallies32; for (i = 0; i < cnt; i++, dst++, src32++) *dst += le32_to_cpu(*src32); } else { dst = (u32 *)&hw->tallies; - src16 = (u16 *)&inf->info.commtallies16; + src16 = (__le16 *)&inf->info.commtallies16; for (i = 0; i < cnt; i++, dst++, src16++) *dst += le16_to_cpu(*src16); } -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export
On Tue, Jul 18, 2017 at 03:26:37PM +0300, Dan Carpenter wrote: > > + if (tty) { > > + /* drop kref from tty_driver_lookup_tty() */ > > + tty_kref_put(tty); > > + tty = ERR_PTR(-EBUSY); > > + } else { /* tty_init_dev returns tty with the tty_lock held */ > > + tty = tty_init_dev(driver, index); > > + tty_port_set_kopened(tty->port, 1); >^ > > tty_init_dev() can fail leading to an error pointer dereference here. Thanks very much. I will check for error pointer here. Okash ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/6] Fix additional alignment issues in staging/ccree
Here are a few more patches that fixes alignment issues in staging/ccree. Includes the patches that I sent previously which could not be applied plus a few more fixes for issues that I found. These patches should fix all remaining alignment warnings reported by checkpatch.pl in staging/ccree. - Simon --- Simon Sandström (6): staging: ccree: Fix alignment issues in ssi_aead.c staging: ccree: Fix alignment issues in ssi_hash.c staging: ccree: Fix alignment issues in ssi_buffer_mgr.c staging: ccree: Fix alignment issues in ssi_cipher.c staging: ccree: Fix alignment issues in ssi_ivgen.c staging: ccree: Fix alignment issues in ssi_request_mgr.c drivers/staging/ccree/ssi_aead.c| 47 +++--- drivers/staging/ccree/ssi_buffer_mgr.c | 40 +++- drivers/staging/ccree/ssi_cipher.c | 3 +- drivers/staging/ccree/ssi_hash.c| 105 +--- drivers/staging/ccree/ssi_ivgen.c | 3 +- drivers/staging/ccree/ssi_request_mgr.c | 4 +- 6 files changed, 112 insertions(+), 90 deletions(-) -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/6] staging: ccree: Fix alignment issues in ssi_aead.c
Fixes checkpatch.pl alignment warnings. Signed-off-by: Simon Sandström --- drivers/staging/ccree/ssi_aead.c | 47 +--- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/drivers/staging/ccree/ssi_aead.c b/drivers/staging/ccree/ssi_aead.c index ea29b8a1a71d..ad53126d6705 100644 --- a/drivers/staging/ccree/ssi_aead.c +++ b/drivers/staging/ccree/ssi_aead.c @@ -96,7 +96,7 @@ static void ssi_aead_exit(struct crypto_aead *tfm) struct ssi_aead_ctx *ctx = crypto_aead_ctx(tfm); SSI_LOG_DEBUG("Clearing context @%p for %s\n", - crypto_aead_ctx(tfm), crypto_tfm_alg_name(&tfm->base)); + crypto_aead_ctx(tfm), crypto_tfm_alg_name(&tfm->base)); dev = &ctx->drvdata->plat_dev->dev; /* Unmap enckey buffer */ @@ -163,7 +163,7 @@ static int ssi_aead_init(struct crypto_aead *tfm) /* Allocate key buffer, cache line aligned */ ctx->enckey = dma_alloc_coherent(dev, AES_MAX_KEY_SIZE, - &ctx->enckey_dma_addr, GFP_KERNEL); +&ctx->enckey_dma_addr, GFP_KERNEL); if (!ctx->enckey) { SSI_LOG_ERR("Failed allocating key buffer\n"); goto init_failed; @@ -239,7 +239,7 @@ static void ssi_aead_complete(struct device *dev, void *ssi_req, void __iomem *c if (areq_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT) { if (memcmp(areq_ctx->mac_buf, areq_ctx->icv_virt_addr, - ctx->authsize) != 0) { + ctx->authsize) != 0) { SSI_LOG_DEBUG("Payload authentication failure, " "(auth-size=%d, cipher=%d).\n", ctx->authsize, ctx->cipher_mode); @@ -378,7 +378,7 @@ static int hmac_setkey(struct cc_hw_desc *desc, struct ssi_aead_ctx *ctx) static int validate_keys_sizes(struct ssi_aead_ctx *ctx) { SSI_LOG_DEBUG("enc_keylen=%u authkeylen=%u\n", - ctx->enc_keylen, ctx->auth_keylen); + ctx->enc_keylen, ctx->auth_keylen); switch (ctx->auth_mode) { case DRV_HASH_SHA1: @@ -402,7 +402,7 @@ static int validate_keys_sizes(struct ssi_aead_ctx *ctx) if (unlikely(ctx->flow_mode == S_DIN_to_DES)) { if (ctx->enc_keylen != DES3_EDE_KEY_SIZE) { SSI_LOG_ERR("Invalid cipher(3DES) key size: %u\n", - ctx->enc_keylen); + ctx->enc_keylen); return -EINVAL; } } else { /* Default assumed to be AES ciphers */ @@ -410,7 +410,7 @@ static int validate_keys_sizes(struct ssi_aead_ctx *ctx) (ctx->enc_keylen != AES_KEYSIZE_192) && (ctx->enc_keylen != AES_KEYSIZE_256)) { SSI_LOG_ERR("Invalid cipher(AES) key size: %u\n", - ctx->enc_keylen); + ctx->enc_keylen); return -EINVAL; } } @@ -553,7 +553,8 @@ ssi_aead_setkey(struct crypto_aead *tfm, const u8 *key, unsigned int keylen) int seq_len = 0, rc = -EINVAL; SSI_LOG_DEBUG("Setting key in context @%p for %s. key=%p keylen=%u\n", - ctx, crypto_tfm_alg_name(crypto_aead_tfm(tfm)), key, keylen); + ctx, crypto_tfm_alg_name(crypto_aead_tfm(tfm)), + key, keylen); /* STAT_PHASE_0: Init and sanity checks */ @@ -684,7 +685,7 @@ static int ssi_aead_setauthsize( #if SSI_CC_HAS_AES_CCM static int ssi_rfc4309_ccm_setauthsize(struct crypto_aead *authenc, - unsigned int authsize) + unsigned int authsize) { switch (authsize) { case 8: @@ -699,7 +700,7 @@ static int ssi_rfc4309_ccm_setauthsize(struct crypto_aead *authenc, } static int ssi_ccm_setauthsize(struct crypto_aead *authenc, - unsigned int authsize) + unsigned int authsize) { switch (authsize) { case 4: @@ -1183,8 +1184,8 @@ static inline void ssi_aead_load_mlli_to_sram( (req_ctx->data_buff_type == SSI_DMA_BUF_MLLI) || !req_ctx->is_single_pass)) { SSI_LOG_DEBUG("Copy-to-sram: mlli_dma=%08x, mlli_size=%u\n", - (unsigned int)ctx->drvdata->mlli_sram_addr, - req_ctx->mlli_params.mlli_len); + (unsigned int)ctx->drvdata->mlli_sram_addr, + req_ctx->mlli_params.mlli_len); /* Copy MLLI table host-to-sram */ hw_desc_init(&desc[*seq_size]); set_din_type(&desc[*seq_size], DMA_DLLI, @@ -1328,7 +1329,8 @@ ssi_aead_xcbc_authenc( } static int validate_data_size(struct ssi_aead_ctx *ctx,
[PATCH 3/6] staging: ccree: Fix alignment issues in ssi_buffer_mgr.c
Fixes checkpatch.pl alignment warnings. Signed-off-by: Simon Sandström --- drivers/staging/ccree/ssi_buffer_mgr.c | 40 -- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/drivers/staging/ccree/ssi_buffer_mgr.c b/drivers/staging/ccree/ssi_buffer_mgr.c index 6579a54f9dc4..63936091d524 100644 --- a/drivers/staging/ccree/ssi_buffer_mgr.c +++ b/drivers/staging/ccree/ssi_buffer_mgr.c @@ -371,7 +371,7 @@ static int ssi_buffer_mgr_map_scatterlist( *mapped_nents = 1; } else { /*sg_is_last*/ *nents = ssi_buffer_mgr_get_sgl_nents(sg, nbytes, lbytes, -&is_chained); + &is_chained); if (*nents > max_sg_nents) { *nents = 0; SSI_LOG_ERR("Too many fragments. current %d max %d\n", @@ -393,9 +393,9 @@ static int ssi_buffer_mgr_map_scatterlist( * must have the same nents before and after map */ *mapped_nents = ssi_buffer_mgr_dma_map_sg(dev, -sg, -*nents, -direction); + sg, + *nents, + direction); if (unlikely(*mapped_nents != *nents)) { *nents = *mapped_nents; SSI_LOG_ERR("dma_map_sg() sg buffer failed\n"); @@ -783,8 +783,8 @@ static inline int ssi_buffer_mgr_aead_chain_iv( goto chain_iv_exit; } - areq_ctx->gen_ctx.iv_dma_addr = dma_map_single(dev, req->iv, - hw_iv_size, DMA_BIDIRECTIONAL); + areq_ctx->gen_ctx.iv_dma_addr = dma_map_single(dev, req->iv, hw_iv_size, + DMA_BIDIRECTIONAL); if (unlikely(dma_mapping_error(dev, areq_ctx->gen_ctx.iv_dma_addr))) { SSI_LOG_ERR("Mapping iv %u B at va=%pK for DMA failed\n", hw_iv_size, req->iv); @@ -1323,8 +1323,9 @@ int ssi_buffer_mgr_map_aead_request( req->cryptlen : (req->cryptlen - authsize); - areq_ctx->mac_buf_dma_addr = dma_map_single(dev, - areq_ctx->mac_buf, MAX_MAC_SIZE, DMA_BIDIRECTIONAL); + areq_ctx->mac_buf_dma_addr = dma_map_single(dev, areq_ctx->mac_buf, + MAX_MAC_SIZE, + DMA_BIDIRECTIONAL); if (unlikely(dma_mapping_error(dev, areq_ctx->mac_buf_dma_addr))) { SSI_LOG_ERR("Mapping mac_buf %u B at va=%pK for DMA failed\n", MAX_MAC_SIZE, areq_ctx->mac_buf); @@ -1334,8 +1335,9 @@ int ssi_buffer_mgr_map_aead_request( if (areq_ctx->ccm_hdr_size != ccm_header_size_null) { areq_ctx->ccm_iv0_dma_addr = dma_map_single(dev, - (areq_ctx->ccm_config + CCM_CTR_COUNT_0_OFFSET), - AES_BLOCK_SIZE, DMA_TO_DEVICE); + (areq_ctx->ccm_config + CCM_CTR_COUNT_0_OFFSET), + AES_BLOCK_SIZE, + DMA_TO_DEVICE); if (unlikely(dma_mapping_error(dev, areq_ctx->ccm_iv0_dma_addr))) { SSI_LOG_ERR("Mapping mac_buf %u B at va=%pK " @@ -1356,7 +1358,9 @@ int ssi_buffer_mgr_map_aead_request( #if SSI_CC_HAS_AES_GCM if (areq_ctx->cipher_mode == DRV_CIPHER_GCTR) { areq_ctx->hkey_dma_addr = dma_map_single(dev, - areq_ctx->hkey, AES_BLOCK_SIZE, DMA_BIDIRECTIONAL); +areq_ctx->hkey, +AES_BLOCK_SIZE, +DMA_BIDIRECTIONAL); if (unlikely(dma_mapping_error(dev, areq_ctx->hkey_dma_addr))) { SSI_LOG_ERR("Mapping hkey %u B at va=%pK for DMA failed\n", AES_BLOCK_SIZE, areq_ctx->hkey); @@ -1365,7 +1369,9 @@ int ssi_buffer_mgr_map_aead_request( } areq_ctx->gcm_block_len_dma_addr = dma_map_single(dev, - &areq_ctx->gcm_len_block, AES_BLOCK_SIZE, DMA_TO_DEVICE); + &areq_ctx->gcm_len_block, + AES_BLO
[PATCH 4/6] staging: ccree: Fix alignment issues in ssi_cipher.c
Fixes checkpatch.pl alignment warnings. Signed-off-by: Simon Sandström --- drivers/staging/ccree/ssi_cipher.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/ccree/ssi_cipher.c b/drivers/staging/ccree/ssi_cipher.c index bfe9b1ccbf37..aec7c1480336 100644 --- a/drivers/staging/ccree/ssi_cipher.c +++ b/drivers/staging/ccree/ssi_cipher.c @@ -203,7 +203,8 @@ static int ssi_blkcipher_init(struct crypto_tfm *tfm) /* Map key buffer */ ctx_p->user.key_dma_addr = dma_map_single(dev, (void *)ctx_p->user.key, -max_key_buf_size, DMA_TO_DEVICE); + max_key_buf_size, + DMA_TO_DEVICE); if (dma_mapping_error(dev, ctx_p->user.key_dma_addr)) { SSI_LOG_ERR("Mapping Key %u B at va=%pK for DMA failed\n", max_key_buf_size, ctx_p->user.key); -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/6] staging: ccree: Fix alignment issues in ssi_hash.c
Fixes checkpatch.pl alignment warnings. Signed-off-by: Simon Sandström --- drivers/staging/ccree/ssi_hash.c | 105 +-- 1 file changed, 56 insertions(+), 49 deletions(-) diff --git a/drivers/staging/ccree/ssi_hash.c b/drivers/staging/ccree/ssi_hash.c index fba0643e78fa..a5b3e9bebd95 100644 --- a/drivers/staging/ccree/ssi_hash.c +++ b/drivers/staging/ccree/ssi_hash.c @@ -70,8 +70,8 @@ static void ssi_hash_create_xcbc_setup( unsigned int *seq_size); static void ssi_hash_create_cmac_setup(struct ahash_request *areq, - struct cc_hw_desc desc[], - unsigned int *seq_size); + struct cc_hw_desc desc[], + unsigned int *seq_size); struct ssi_hash_alg { struct list_head entry; @@ -117,8 +117,8 @@ static void ssi_hash_create_data_desc( static inline void ssi_set_hash_endianity(u32 mode, struct cc_hw_desc *desc) { if (unlikely((mode == DRV_HASH_MD5) || - (mode == DRV_HASH_SHA384) || - (mode == DRV_HASH_SHA512))) { +(mode == DRV_HASH_SHA384) || +(mode == DRV_HASH_SHA512))) { set_bytes_swap(desc, 1); } else { set_cipher_config0(desc, HASH_DIGEST_RESULT_LITTLE_ENDIAN); @@ -135,7 +135,7 @@ static int ssi_hash_map_result(struct device *dev, DMA_BIDIRECTIONAL); if (unlikely(dma_mapping_error(dev, state->digest_result_dma_addr))) { SSI_LOG_ERR("Mapping digest result buffer %u B for DMA failed\n", - digestsize); + digestsize); return -ENOMEM; } SSI_LOG_DEBUG("Mapped digest result buffer %u B " @@ -200,12 +200,12 @@ static int ssi_hash_map_request(struct device *dev, state->digest_buff_dma_addr = dma_map_single(dev, (void *)state->digest_buff, ctx->inter_digestsize, DMA_BIDIRECTIONAL); if (dma_mapping_error(dev, state->digest_buff_dma_addr)) { SSI_LOG_ERR("Mapping digest len %d B at va=%pK for DMA failed\n", - ctx->inter_digestsize, state->digest_buff); + ctx->inter_digestsize, state->digest_buff); goto fail3; } SSI_LOG_DEBUG("Mapped digest %d B at va=%pK to dma=%pad\n", - ctx->inter_digestsize, state->digest_buff, - state->digest_buff_dma_addr); + ctx->inter_digestsize, state->digest_buff, + state->digest_buff_dma_addr); if (is_hmac) { dma_sync_single_for_cpu(dev, ctx->digest_buff_dma_addr, ctx->inter_digestsize, DMA_BIDIRECTIONAL); @@ -249,12 +249,12 @@ static int ssi_hash_map_request(struct device *dev, state->digest_bytes_len_dma_addr = dma_map_single(dev, (void *)state->digest_bytes_len, HASH_LEN_SIZE, DMA_BIDIRECTIONAL); if (dma_mapping_error(dev, state->digest_bytes_len_dma_addr)) { SSI_LOG_ERR("Mapping digest len %u B at va=%pK for DMA failed\n", - HASH_LEN_SIZE, state->digest_bytes_len); + HASH_LEN_SIZE, state->digest_bytes_len); goto fail4; } SSI_LOG_DEBUG("Mapped digest len %u B at va=%pK to dma=%pad\n", - HASH_LEN_SIZE, state->digest_bytes_len, - state->digest_bytes_len_dma_addr); + HASH_LEN_SIZE, state->digest_bytes_len, + state->digest_bytes_len_dma_addr); } else { state->digest_bytes_len_dma_addr = 0; } @@ -263,12 +263,13 @@ static int ssi_hash_map_request(struct device *dev, state->opad_digest_dma_addr = dma_map_single(dev, (void *)state->opad_digest_buff, ctx->inter_digestsize, DMA_BIDIRECTIONAL); if (dma_mapping_error(dev, state->opad_digest_dma_addr)) { SSI_LOG_ERR("Mapping opad digest %d B at va=%pK for DMA failed\n", - ctx->inter_digestsize, state->opad_digest_buff); + ctx->inter_digestsize, + state->opad_digest_buff); goto fail5; } SSI_LOG_DEBUG("Mapped opad digest %d B at va=%pK to dma=%pad\n", - ctx->inter_digestsize, state->opad_digest_buff, - state->opad_digest_dma_addr); + ctx->inter_digestsize, state->opad_digest_buff, + state->opad_digest_dma_addr); } else { state->opad_digest_dma_addr = 0; } @@ -602,7 +603,7 @@ static int ssi_hash_update(struct ahash_req_ctx *state, if (unlikely(rc)) { if (rc == 1) {
[PATCH 5/6] staging: ccree: Fix alignment issues in ssi_ivgen.c
Fixes checkpatch.pl alignment warnings. Signed-off-by: Simon Sandström --- drivers/staging/ccree/ssi_ivgen.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/ccree/ssi_ivgen.c b/drivers/staging/ccree/ssi_ivgen.c index f140dbc5195c..86364f81acab 100644 --- a/drivers/staging/ccree/ssi_ivgen.c +++ b/drivers/staging/ccree/ssi_ivgen.c @@ -202,7 +202,8 @@ int ssi_ivgen_init(struct ssi_drvdata *drvdata) /* Allocate pool's header for intial enc. key/IV */ ivgen_ctx->pool_meta = dma_alloc_coherent(device, SSI_IVPOOL_META_SIZE, - &ivgen_ctx->pool_meta_dma, GFP_KERNEL); + &ivgen_ctx->pool_meta_dma, + GFP_KERNEL); if (!ivgen_ctx->pool_meta) { SSI_LOG_ERR("Not enough memory to allocate DMA of pool_meta " "(%u B)\n", SSI_IVPOOL_META_SIZE); -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/6] staging: ccree: Fix alignment issues in ssi_request_mgr.c
Fixes checkpatch.pl alignment warnings. Signed-off-by: Simon Sandström --- drivers/staging/ccree/ssi_request_mgr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/ccree/ssi_request_mgr.c b/drivers/staging/ccree/ssi_request_mgr.c index 3f39150cda4f..2eda82f317d2 100644 --- a/drivers/staging/ccree/ssi_request_mgr.c +++ b/drivers/staging/ccree/ssi_request_mgr.c @@ -136,7 +136,9 @@ int request_mgr_init(struct ssi_drvdata *drvdata) /* Allocate DMA word for "dummy" completion descriptor use */ req_mgr_h->dummy_comp_buff = dma_alloc_coherent(&drvdata->plat_dev->dev, - sizeof(u32), &req_mgr_h->dummy_comp_buff_dma, GFP_KERNEL); + sizeof(u32), + &req_mgr_h->dummy_comp_buff_dma, + GFP_KERNEL); if (!req_mgr_h->dummy_comp_buff) { SSI_LOG_ERR("Not enough memory to allocate DMA (%zu) dropped " "buffer\n", sizeof(u32)); -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: fsl-mc: Convert to using %pOF instead of full_name
Now that we have a custom printf format specifier, convert users of full_name to use %pOF instead. This is preparation to remove storing of the full path string for each node. Signed-off-by: Rob Herring Cc: Stuart Yoder Cc: Laurentiu Tudor Cc: Greg Kroah-Hartman Cc: de...@driverdev.osuosl.org --- drivers/staging/fsl-mc/bus/fsl-mc-bus.c| 4 ++-- drivers/staging/fsl-mc/bus/fsl-mc-msi.c| 4 ++-- drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 8 +++- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c index 19606e8d25dd..409f2b9e70ff 100644 --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c @@ -757,8 +757,8 @@ static int fsl_mc_bus_probe(struct platform_device *pdev) error = of_address_to_resource(pdev->dev.of_node, 0, &res); if (error < 0) { dev_err(&pdev->dev, - "of_address_to_resource() failed for %s\n", - pdev->dev.of_node->full_name); + "of_address_to_resource() failed for %pOF\n", + pdev->dev.of_node); return error; } diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/fsl-mc-msi.c index c04a2f2b3409..49f95123b5c7 100644 --- a/drivers/staging/fsl-mc/bus/fsl-mc-msi.c +++ b/drivers/staging/fsl-mc/bus/fsl-mc-msi.c @@ -183,8 +183,8 @@ int fsl_mc_find_msi_domain(struct device *mc_platform_dev, msi_domain = of_msi_get_domain(mc_platform_dev, mc_of_node, DOMAIN_BUS_FSL_MC_MSI); if (!msi_domain) { - pr_err("Unable to find fsl-mc MSI domain for %s\n", - mc_of_node->full_name); + pr_err("Unable to find fsl-mc MSI domain for %pOF\n", + mc_of_node); return -ENOENT; } diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c index 865d38517508..e49dcde480fc 100644 --- a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c +++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c @@ -79,8 +79,7 @@ int __init its_fsl_mc_msi_init(void) parent = irq_find_matching_host(np, DOMAIN_BUS_NEXUS); if (!parent || !msi_get_domain_info(parent)) { - pr_err("%s: unable to locate ITS domain\n", - np->full_name); + pr_err("%pOF: unable to locate ITS domain\n", np); continue; } @@ -89,15 +88,14 @@ int __init its_fsl_mc_msi_init(void) &its_fsl_mc_msi_domain_info, parent); if (!mc_msi_domain) { - pr_err("%s: unable to create fsl-mc domain\n", - np->full_name); + pr_err("%pOF: unable to create fsl-mc domain\n", np); continue; } WARN_ON(mc_msi_domain->host_data != &its_fsl_mc_msi_domain_info); - pr_info("fsl-mc MSI: %s domain created\n", np->full_name); + pr_info("fsl-mc MSI: %pOF domain created\n", np); } return 0; -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/3] staging: gs_fpgaboot: add buffer overflow checks
Four fields in struct fpgaimage are char arrays of length MAX_STR (256). The amount of data read into these buffers is controlled by a length field in the bitstream file read from userspace. If a corrupt or malicious firmware file was supplied, kernel data beyond these buffers can be overwritten arbitrarily. This patch adds a check of the bitstream's length value to ensure it fits within the bounds of the allocated buffers. An error condition is returned from gs_read_bitstream if any of the reads fail. Signed-off-by: Jacob von Chorus v3: - use >= to prevent an integer overflow in the comparison - use get_unaligned_be functions to interpret length fields - fix remainder of file to use valid error codes v2: - char arrays converted to u8 arrays - replace error return value with proper error code in gs_read_bitstream --- drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 54 +++ 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c index 19b550fff0..a49019af60 100644 --- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c +++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c @@ -23,6 +23,7 @@ #include #include #include +#include #include "gs_fpgaboot.h" #include "io.h" @@ -47,7 +48,7 @@ static void read_bitstream(char *bitdata, char *buf, int *offset, int rdsize) *offset += rdsize; } -static void readinfo_bitstream(char *bitdata, char *buf, int *offset) +static int readinfo_bitstream(char *bitdata, char *buf, int size, int *offset) { char tbuf[64]; s32 len; @@ -58,10 +59,16 @@ static void readinfo_bitstream(char *bitdata, char *buf, int *offset) /* read length */ read_bitstream(bitdata, tbuf, offset, 2); - len = tbuf[0] << 8 | tbuf[1]; + len = get_unaligned_be16(tbuf); + if (len >= size) { + pr_err("error: readinfo buffer too small\n"); + return -EINVAL; + } read_bitstream(bitdata, buf, offset, len); buf[len] = '\0'; + + return 0; } /* @@ -83,8 +90,7 @@ static int readlength_bitstream(char *bitdata, int *lendata, int *offset) /* read 4bytes length */ read_bitstream(bitdata, tbuf, offset, 4); - *lendata = tbuf[0] << 24 | tbuf[1] << 16 | - tbuf[2] << 8 | tbuf[3]; + *lendata = get_unaligned_be32(tbuf); return 0; } @@ -113,7 +119,7 @@ static int readmagic_bitstream(char *bitdata, int *offset) /* * NOTE: supports only bitstream format */ -static enum fmt_image get_imageformat(struct fpgaimage *fimage) +static enum fmt_image get_imageformat(void) { return f_bit; } @@ -127,34 +133,54 @@ static void gs_print_header(struct fpgaimage *fimage) pr_info("lendata: %d\n", fimage->lendata); } -static void gs_read_bitstream(struct fpgaimage *fimage) +static int gs_read_bitstream(struct fpgaimage *fimage) { char *bitdata; int offset; + int err; offset = 0; bitdata = (char *)fimage->fw_entry->data; - readmagic_bitstream(bitdata, &offset); - readinfo_bitstream(bitdata, fimage->filename, &offset); - readinfo_bitstream(bitdata, fimage->part, &offset); - readinfo_bitstream(bitdata, fimage->date, &offset); - readinfo_bitstream(bitdata, fimage->time, &offset); - readlength_bitstream(bitdata, &fimage->lendata, &offset); + err = readmagic_bitstream(bitdata, &offset); + if (err) + return err; + + err = readinfo_bitstream(bitdata, fimage->filename, MAX_STR, &offset); + if (err) + return err; + err = readinfo_bitstream(bitdata, fimage->part, MAX_STR, &offset); + if (err) + return err; + err = readinfo_bitstream(bitdata, fimage->date, MAX_STR, &offset); + if (err) + return err; + err = readinfo_bitstream(bitdata, fimage->time, MAX_STR, &offset); + if (err) + return err; + + err = readlength_bitstream(bitdata, &fimage->lendata, &offset); + if (err) + return err; fimage->fpgadata = bitdata + offset; + + return 0; } static int gs_read_image(struct fpgaimage *fimage) { int img_fmt; + int err; - img_fmt = get_imageformat(fimage); + img_fmt = get_imageformat(); switch (img_fmt) { case f_bit: pr_info("image is bitstream format\n"); - gs_read_bitstream(fimage); + err = gs_read_bitstream(fimage); + if (err) + return err; break; default: pr_err("unsupported fpga image format\n"); -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 2/3] staging: gs_fpgaboot: change char to u8
The bitstream storage variables were changed from char to u8 arrays to prevent issues such as negative lengths. This change makes the code compatible with the "data" field in "struct firmware" which is of type u8. Signed-off-by: Jacob von Chorus v3: - reduce temporary buffer size in bitstream reading functions --- drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 24 drivers/staging/gs_fpgaboot/gs_fpgaboot.h | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c index a49019af60..ff59708792 100644 --- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c +++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c @@ -42,16 +42,16 @@ static char *file = "xlinx_fpga_firmware.bit"; module_param(file, charp, 0444); MODULE_PARM_DESC(file, "Xilinx FPGA firmware file."); -static void read_bitstream(char *bitdata, char *buf, int *offset, int rdsize) +static void read_bitstream(u8 *bitdata, u8 *buf, int *offset, int rdsize) { memcpy(buf, bitdata + *offset, rdsize); *offset += rdsize; } -static int readinfo_bitstream(char *bitdata, char *buf, int size, int *offset) +static int readinfo_bitstream(u8 *bitdata, u8 *buf, int size, int *offset) { - char tbuf[64]; - s32 len; + u8 tbuf[2]; + u16 len; /* read section char */ read_bitstream(bitdata, tbuf, offset, 1); @@ -74,9 +74,9 @@ static int readinfo_bitstream(char *bitdata, char *buf, int size, int *offset) /* * read bitdata length */ -static int readlength_bitstream(char *bitdata, int *lendata, int *offset) +static int readlength_bitstream(u8 *bitdata, int *lendata, int *offset) { - char tbuf[64]; + u8 tbuf[4]; /* read section char */ read_bitstream(bitdata, tbuf, offset, 1); @@ -98,9 +98,9 @@ static int readlength_bitstream(char *bitdata, int *lendata, int *offset) /* * read first 13 bytes to check bitstream magic number */ -static int readmagic_bitstream(char *bitdata, int *offset) +static int readmagic_bitstream(u8 *bitdata, int *offset) { - char buf[13]; + u8 buf[13]; int r; read_bitstream(bitdata, buf, offset, 13); @@ -135,12 +135,12 @@ static void gs_print_header(struct fpgaimage *fimage) static int gs_read_bitstream(struct fpgaimage *fimage) { - char *bitdata; + u8 *bitdata; int offset; int err; offset = 0; - bitdata = (char *)fimage->fw_entry->data; + bitdata = (u8 *)fimage->fw_entry->data; err = readmagic_bitstream(bitdata, &offset); if (err) @@ -209,11 +209,11 @@ static int gs_load_image(struct fpgaimage *fimage, char *fw_file) static int gs_download_image(struct fpgaimage *fimage, enum wbus bus_bytes) { - char *bitdata; + u8 *bitdata; int size, i, cnt; cnt = 0; - bitdata = (char *)fimage->fpgadata; + bitdata = (u8 *)fimage->fpgadata; size = fimage->lendata; #ifdef DEBUG_FPGA diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h index cd1eb2c4c9..986e841f6b 100644 --- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h +++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h @@ -47,5 +47,5 @@ struct fpgaimage { chardate[MAX_STR]; chartime[MAX_STR]; int lendata; - char*fpgadata; + u8 *fpgadata; }; -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 3/3] staging: gs_fpgaboot: return valid error codes
The return values on error are modified to be valid error codes. Theses error codes are propagated back to the init function's return. Signed-off-by: Jacob von Chorus --- drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c index ff59708792..bcbdc7340b 100644 --- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c +++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c @@ -84,7 +84,7 @@ static int readlength_bitstream(u8 *bitdata, int *lendata, int *offset) /* make sure it is section 'e' */ if (tbuf[0] != 'e') { pr_err("error: length section is not 'e', but %c\n", tbuf[0]); - return -1; + return -EINVAL; } /* read 4bytes length */ @@ -107,7 +107,7 @@ static int readmagic_bitstream(u8 *bitdata, int *offset) r = memcmp(buf, bits_magic, 13); if (r) { pr_err("error: corrupted header"); - return -1; + return -EINVAL; } pr_info("bitstream file magic number Ok\n"); @@ -184,7 +184,7 @@ static int gs_read_image(struct fpgaimage *fimage) break; default: pr_err("unsupported fpga image format\n"); - return -1; + return -EINVAL; } gs_print_header(fimage); @@ -223,7 +223,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum wbus bus_bytes) if (!xl_supported_prog_bus_width(bus_bytes)) { pr_err("unsupported program bus width %d\n", bus_bytes); - return -1; + return -EINVAL; } /* Bring csi_b, rdwr_b Low and program_b High */ @@ -250,7 +250,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum wbus bus_bytes) /* Check INIT_B */ if (xl_get_init_b() == 0) { pr_err("init_b 0\n"); - return -1; + return -EIO; } while (xl_get_done_b() == 0) { @@ -262,7 +262,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum wbus bus_bytes) if (cnt > MAX_WAIT_DONE) { pr_err("fpga download fail\n"); - return -1; + return -EIO; } pr_info("download fpgaimage\n"); @@ -351,7 +351,7 @@ static int gs_fpgaboot(void) err_out1: kfree(fimage); - return -1; + return err; } static int __init gs_fpgaboot_init(void) -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] Staging: pi433: declare functions static
Declare functions static to fix sparse warnings: warning: symbol 'pi433_receive' was not declared. Should it be static? warning: symbol 'pi433_tx_thread' was not declared. Should it be static? Signed-off-by: Joseph Wright --- drivers/staging/pi433/pi433_if.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 1bc478a..46461b4 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -313,7 +313,7 @@ pi433_start_rx(struct pi433_device *dev) /*-*/ -int +static int pi433_receive(void *data) { struct pi433_device *dev = data; @@ -463,7 +463,7 @@ pi433_receive(void *data) return bytes_total; } -int +static int pi433_tx_thread(void *data) { struct pi433_device *device = data; -- 2.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/2] Staging: pi433: fix sparse warnings
Two patches included to fix warnings found with sparse. Joseph Wright (2): Staging: pi433: declare functions static Staging: pi433: check error after kthread_run() drivers/staging/pi433/pi433_if.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- 2.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] Staging: pi433: check error after kthread_run()
Error should be checked with IS_ERR after calling kthread_run() instead of comparing the returned pointer to an int. Found by sparse warning: incompatible types for operation (<) left side has type struct task_struct *tx_task_struct right side has type int Signed-off-by: Joseph Wright --- drivers/staging/pi433/pi433_if.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 46461b4..4f724a5 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -1152,7 +1152,7 @@ static int pi433_probe(struct spi_device *spi) device->tx_task_struct = kthread_run(pi433_tx_thread, device, "pi433_tx_task"); - if (device->tx_task_struct < 0) + if (IS_ERR(device->tx_task_struct)) { dev_dbg(device->dev, "start of send thread failed"); goto send_thread_failed; -- 2.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
CONGRATULATIONS
CONGRATULATIONS YOUR MOBILE NUMBER WON 500,000 CASH FROM SAMSUNG PROMO CLAIMS SEND YOUR. NAME MOBILE NO AGE SEX ADDS REPLY TO: deptsamsung06@hotmail.com ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: loopback: Enclose multiple macro statements in do-while loop
On Tuesday 18 July 2017 04:41 PM, Bryan O'Donoghue wrote: On 18/07/17 11:56, kart...@techveda.org wrote: From: Karthik Tummala Enclosed multiple macro statements in a do - while loop as per kernel coding standard, pointed by checkpatch. Signed-off-by: Karthik Tummala --- drivers/staging/greybus/loopback.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 08e2558..c616555 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -176,10 +176,12 @@ static DEVICE_ATTR_RO(name##_##field) }\ static DEVICE_ATTR_RO(name##_avg) -#define gb_loopback_stats_attrs(field)\ -gb_loopback_ro_stats_attr(field, min, u);\ -gb_loopback_ro_stats_attr(field, max, u);\ -gb_loopback_ro_avg_attr(field) +#define gb_loopback_stats_attrs(field)\ +do {\ +gb_loopback_ro_stats_attr(field, min, u);\ +gb_loopback_ro_stats_attr(field, max, u);\ +gb_loopback_ro_avg_attr(field);\ +} while (0) #define gb_loopback_attr(field, type)\ static ssize_t field##_show(struct device *dev,\ Macros with multiple statements should be enclosed in a do - while block: .. code-block:: c #define macrofun(a, b, c) \ do {\ if (a == 5) \ do_this(b, c); \ } while (0) I don't think it really applies. We're declaring functions not inlining executable code.Hi, It makes sense, thanks for the insight. Please drop this patch Thanks ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel