Re: [PATCH 04/11] Staging: iio: accel: Rename few macro definitions

[Jonathan Cameron]

On Mon,  5 Mar 2018 13:19:23 +0530
Himanshu Jha  wrote:

> Rename few macros with appropriate names specifying their usage/function.
Most of these are obviously good, but for one I didn't understand your
reasoning.

Given there are only a few of them, I'd put some more description here
to explain why you changed each one.

Thanks,

Jonathan
> 
> Signed-off-by: Himanshu Jha 
> ---
>  drivers/staging/iio/accel/adis16201.c | 17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 59c1166..445cb56 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -20,9 +20,8 @@
>  #include 
>  #include 
>  
> -#define ADIS16201_STARTUP_DELAY  220
> -
> -#define ADIS16201_FLASH_CNT  0x00
> +#define ADIS16201_STARTUP_DELAY_MS   220
> +#define ADIS16201_FLASH_CNT  0x00
>  
>  /* Data Output Register Information */
>  #define ADIS16201_SUPPLY_OUT 0x02
> @@ -69,7 +68,7 @@
>  
>  #define ADIS16201_MSC_CTRL_DATA_RDY_EN   BIT(2)
>  
> -#define ADIS16201_MSC_CTRL_ACTIVE_HIGH   BIT(1)
> +#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH  BIT(1)
>  
>  #define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
>  
> @@ -80,14 +79,14 @@
>  
>  #define ADIS16201_DIAG_STAT_SPI_FAIL_BIT   3
>  
> -#define ADIS16201_DIAG_STAT_FLASH_UPT_BIT  2
> +#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT  2
>  
>  #define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
>  
>  #define ADIS16201_DIAG_STAT_POWER_LOW_BIT  0
>  
>  #define ADIS16201_GLOB_CMD_SW_RESET  BIT(7)
> -#define ADIS16201_GLOB_CMD_FACTORY_CAL   BIT(1)
> +#define ADIS16201_GLOB_CMD_FACTORY   BIT(1)
What was the logic behind this particular change?

>  
>  #define ADIS16201_ERROR_ACTIVE  BIT(14)
>  
> @@ -231,7 +230,7 @@ static const struct iio_info adis16201_info = {
>  
>  static const char * const adis16201_status_error_msgs[] = {
>   [ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
> - [ADIS16201_DIAG_STAT_FLASH_UPT_BIT] = "Flash update failed",
> + [ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
>   [ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
>   [ADIS16201_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V",
>  };
> @@ -244,11 +243,11 @@ static const struct adis_data adis16201_data = {
>  
>   .self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
>   .self_test_no_autoclear = true,
> - .startup_delay = ADIS16201_STARTUP_DELAY,
> + .startup_delay = ADIS16201_STARTUP_DELAY_MS,
>  
>   .status_error_msgs = adis16201_status_error_msgs,
>   .status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
> - BIT(ADIS16201_DIAG_STAT_FLASH_UPT_BIT) |
> + BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
>   BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
>   BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
>  };

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

Re: [PATCH 10/11] Staging: iio: accel: Add comments about units in data read function

[Jonathan Cameron]

On Mon,  5 Mar 2018 13:19:29 +0530
Himanshu Jha  wrote:

> Clarify the conversion and formation of resultant data in the
> adis16201_read_raw() with sufficient comments.
> 
> Signed-off-by: Himanshu Jha 
This is fine but it needs to be in the original comment changing patch
rather than removing the comments first then a few patches later putting
back a different version.

So good change but in the wrong place in the series.  Learning to reorder
a series and merge down multiple patches into one is very useful when
working with git.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/accel/adis16201.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 8d795e2..946c7b1 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -130,6 +130,11 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
>   *val2 = 0;
>   return IIO_VAL_INT_PLUS_MICRO;
>   case IIO_ACCEL:
> +/*
> + * IIO base unit for sensitivity of accelerometer
> + * is milli g.
> + * 1 LSB represents 0.244 mg.
> + */
>   *val = 0;
>   *val2 = IIO_G_TO_M_S_2(462400);
>   return IIO_VAL_INT_PLUS_NANO;
> @@ -142,6 +147,11 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
>   }
>   break;
>   case IIO_CHAN_INFO_OFFSET:
> +/*
> + * The raw ADC value is 0x4FE when the temperature
> + * is 25 degrees and the scale factor per milli
> + * degree celcius is -470.
> + */
>   *val = 25000 / -470 - 1278;
>   return IIO_VAL_INT;
>   case IIO_CHAN_INFO_CALIBBIAS:

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

Re: [PATCH v3 1/4] Staging: iio: adis16209: Remove and add some comments and group the definitions

[Jonathan Cameron]

On Sun, 04 Mar 2018 18:37:04 +0530
Shreeya Patel  wrote:

> On Sun, 2018-03-04 at 18:26 +0530, Himanshu Jha wrote:
> > Hi Shreeya,
> > 
> > On Sun, Mar 04, 2018 at 06:06:22PM +0530, Shreeya Patel wrote:  
> > > 
> > > Remove some unnecessay comments and group the control
> > > register and register field macros together.
> > > Some of the register names does not make it's puporse
> > > very clear and hence, add some comments for more
> > > information.
> > > Also there are certain unit based comments which are not
> > > providing sufficient information, so expand those comments.
> > > 
> > > Signed-off-by: Shreeya Patel 
> > > ---
> > > 
> > > Changes in v3
> > >   -This patch is the combination of two patches from the
> > > previous series. Also add some more comments.
> > > 
> > > 
> > >  drivers/staging/iio/accel/adis16209.c | 132 ++--
> > > --
> > >  1 file changed, 39 insertions(+), 93 deletions(-)
> > > 
> > > diff --git a/drivers/staging/iio/accel/adis16209.c
> > > b/drivers/staging/iio/accel/adis16209.c
> > > index 151120f..d8aef9c 100644
> > > --- a/drivers/staging/iio/accel/adis16209.c
> > > +++ b/drivers/staging/iio/accel/adis16209.c
> > > @@ -21,135 +21,70 @@
> > >  #include 
> > >  
> > >  #define ADIS16209_STARTUP_DELAY_MS   220
> > > -
> > > -/* Flash memory write count */
> > >  #define ADIS16209_FLASH_CNT_REG  0x00
> > >  
> > > -/* Output, power supply */
> > > +/* Data Output Register Definitions */
> > >  #define ADIS16209_SUPPLY_OUT_REG 0x02
> > > -
> > > -/* Output, x-axis accelerometer */
> > >  #define ADIS16209_XACCL_OUT_REG  0x04
> > > -
> > > -/* Output, y-axis accelerometer */
> > >  #define ADIS16209_YACCL_OUT_REG  0x06
> > > -
> > >  /* Output, auxiliary ADC input */
> > >  #define ADIS16209_AUX_ADC_REG0x08
> > > -
> > >  /* Output, temperature */
> > >  #define ADIS16209_TEMP_OUT_REG   0x0A
> > > -
> > > -/* Output, x-axis inclination */
> > > +/* Output, +/- 90 degrees X-axis inclination */
> > >  #define ADIS16209_XINCL_OUT_REG  0x0C
> > > -
> > > -/* Output, y-axis inclination */
> > >  #define ADIS16209_YINCL_OUT_REG  0x0E
> > > -
> > >  /* Output, +/-180 vertical rotational position */
> > >  #define ADIS16209_ROT_OUT_REG0x10
> > >  
> > > -/* Calibration, x-axis acceleration offset null */
> > > +/*
> > > + * Calibration Register Definitions.
> > > + * Acceleration, inclination or rotation offset null.
> > > + */
> > >  #define ADIS16209_XACCL_NULL_REG 0x12
> > > -
> > > -/* Calibration, y-axis acceleration offset null */
> > >  #define ADIS16209_YACCL_NULL_REG 0x14
> > > -
> > > -/* Calibration, x-axis inclination offset null */
> > >  #define ADIS16209_XINCL_NULL_REG 0x16
> > > -
> > > -/* Calibration, y-axis inclination offset null */
> > >  #define ADIS16209_YINCL_NULL_REG 0x18
> > > -
> > > -/* Calibration, vertical rotation offset null */
> > >  #define ADIS16209_ROT_NULL_REG   0x1A
> > >  
> > > -/* Alarm 1 amplitude threshold */
> > > +/* Alarm Register Definitions */
> > >  #define ADIS16209_ALM_MAG1_REG   0x20
> > > -
> > > -/* Alarm 2 amplitude threshold */
> > >  #define ADIS16209_ALM_MAG2_REG   0x22
> > > -
> > > -/* Alarm 1, sample period */
> > >  #define ADIS16209_ALM_SMPL1_REG  0x24
> > > -
> > > -/* Alarm 2, sample period */
> > >  #define ADIS16209_ALM_SMPL2_REG  0x26
> > > -
> > > -/* Alarm control */
> > >  #define ADIS16209_ALM_CTRL_REG   0x28
> > >  
> > > -/* Auxiliary DAC data */
> > >  #define ADIS16209_AUX_DAC_REG0x30
> > > -
> > > -/* General-purpose digital input/output control */
> > >  #define ADIS16209_GPIO_CTRL_REG  0x32
> > > -
> > > -/* Miscellaneous control */
> > > -#define ADIS16209_MSC_CTRL_REG   0x34
> > > -
> > > -/* Internal sample period (rate) control */
> > >  #define ADIS16209_SMPL_PRD_REG   0x36
> > > -
> > > -/* Operation, filter configuration */
> > >  #define ADIS16209_AVG_CNT_REG0x38
> > > -
> > > -/* Operation, sleep mode control */
> > >  #define ADIS16209_SLP_CNT_REG0x3A
> > >  
> > > -/* Diagnostics, system status register */
> > > -#define ADIS16209_DIAG_STAT_REG  0x3C
> > > -
> > > -/* Operation, system command register */
> > > -#define ADIS16209_GLOB_CMD_REG   0x3E
> > > -
> > > -/* MSC_CTRL */
> > > -
> > > -/* Self-test at power-on: 1 = disabled, 0 = enabled */
> > > -#define ADIS16209_MSC_CTRL_PWRUP_SELF_TEST   BIT(10)
> > > -
> > > -/* Self-test enable */
> > > -#define ADIS16209_MSC_CTRL_SELF_TEST_EN  BIT(8)
> > > -
> > > -/* Data-ready enable: 1 = enabled, 0 = disabled */
> > > -#define ADIS16209_MSC_CTRL_DATA_RDY_EN   BIT(2)
> > > -
> > > +#define ADIS16209_MSC_CTRL_REG   0x34
> > > +#define  ADIS16209_MSC_CTRL_PWRUP_SELF_TEST  BIT(10)
> > > +#define  ADIS16209_MSC_CTRL_SELF_TEST_EN 

Re: [PATCH v2 2/3] staging:iio:meter: Remove unused macro IIO_DEV_ATTR_CH_OFF

[Jonathan Cameron]

On Tue, 6 Mar 2018 21:44:07 -0300
Rodrigo Siqueira  wrote:

> This patch removes the macro IIO_DEV_ATTR_CH_OFF. The macro
> IIO_DEV_ATTR_CH_OFF is not required, due to the replace of it by the
> direct use of IIO_DEVICE_ATTR in files staging/iio/meter/ade7759.c and
> staging/iio/meter/ade7753.c.
> 
> Signed-off-by: Rodrigo Siqueira 
Doh! I should have read to the next patch.

Personally I wouldn't have split this in to, but it is a small thing
so doesn't matter.

Applied to the togreg branch of iio.git (having reverted my tweak
to the previous patch) and pushed out as testing.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/meter/meter.h | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/meter.h 
> b/drivers/staging/iio/meter/meter.h
> index edf26302fa57..5ed59bf30a25 100644
> --- a/drivers/staging/iio/meter/meter.h
> +++ b/drivers/staging/iio/meter/meter.h
> @@ -348,9 +348,6 @@
>  #define IIO_DEV_ATTR_VPERIOD(_mode, _show, _store, _addr)\
>   IIO_DEVICE_ATTR(vperiod, _mode, _show, _store, _addr)
>  
> -#define IIO_DEV_ATTR_CH_OFF(_num, _mode, _show, _store, _addr)   
> \
> - IIO_DEVICE_ATTR(choff_##_num, _mode, _show, _store, _addr)
> -
>  /* active energy register, AENERGY, is more than half full */
>  #define IIO_EVENT_ATTR_AENERGY_HALF_FULL(_evlist, _show, _store, _mask) \
>   IIO_EVENT_ATTR_SH(aenergy_half_full, _evlist, _show, _store, _mask)

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

Re: [PATCH 03/11] Staging: iio: accel: Remove unnecessary comments

[Jonathan Cameron]

On Mon,  5 Mar 2018 13:19:22 +0530
Himanshu Jha  wrote:

> Remove unnecessary comments since the definitions are pretty clear
> with their macro names.
> 
> Signed-off-by: Himanshu Jha 

Hi,

The art of commenting (and indeed removing comments) is never a clear one.
I agree with the vast majority of what you have here, but as with other
drivers in this set, I think there are a couple of places comments are useful
and even in one case where the comment should be expanded to become useful.

I'd also like to see the patches reordered so the renames occur before this
one.  That will make some of the changes you have made in here a lot more
obviously good!

Thanks,

Jonathan.
> ---
>  drivers/staging/iio/accel/adis16201.c | 82 
> +--
>  1 file changed, 10 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 0fae8aa..59c1166 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -20,87 +20,42 @@
>  #include 
>  #include 
>  
> -#define ADIS16201_STARTUP_DELAY  220 /* ms */
> +#define ADIS16201_STARTUP_DELAY  220
As below, this would have made a lot more sense if you had
the rename of the macro patch (I assume the unit will be added
to the name) before this one.

>  
> -/* Flash memory write count */
>  #define ADIS16201_FLASH_CNT  0x00
>  
> -/* Output, power supply */
> +/* Data Output Register Information */
>  #define ADIS16201_SUPPLY_OUT 0x02
> -
> -/* Output, x-axis accelerometer */
>  #define ADIS16201_XACCL_OUT  0x04
> -
> -/* Output, y-axis accelerometer */
>  #define ADIS16201_YACCL_OUT  0x06
> -
> -/* Output, auxiliary ADC input */
>  #define ADIS16201_AUX_ADC0x08
> -
> -/* Output, temperature */
>  #define ADIS16201_TEMP_OUT   0x0A
> -
> -/* Output, x-axis inclination */
>  #define ADIS16201_XINCL_OUT  0x0C
> -
> -/* Output, y-axis inclination */
>  #define ADIS16201_YINCL_OUT  0x0E
>  
> -/* Calibration, x-axis acceleration offset */
> +/* Calibration Register Definition */
>  #define ADIS16201_XACCL_OFFS 0x10
> -
> -/* Calibration, y-axis acceleration offset */
>  #define ADIS16201_YACCL_OFFS 0x12
> -
> -/* x-axis acceleration scale factor */
>  #define ADIS16201_XACCL_SCALE0x14
> -
> -/* y-axis acceleration scale factor */
>  #define ADIS16201_YACCL_SCALE0x16
> -
> -/* Calibration, x-axis inclination offset */
>  #define ADIS16201_XINCL_OFFS 0x18
> -
> -/* Calibration, y-axis inclination offset */
>  #define ADIS16201_YINCL_OFFS 0x1A
> -
> -/* x-axis inclination scale factor */
>  #define ADIS16201_XINCL_SCALE0x1C
> -
> -/* y-axis inclination scale factor */
>  #define ADIS16201_YINCL_SCALE0x1E
>  
> -/* Alarm 1 amplitude threshold */
> +/* Alarm Register Definition */
>  #define ADIS16201_ALM_MAG1   0x20
> -
> -/* Alarm 2 amplitude threshold */
>  #define ADIS16201_ALM_MAG2   0x22
> -
> -/* Alarm 1, sample period */
>  #define ADIS16201_ALM_SMPL1  0x24
> -
> -/* Alarm 2, sample period */
>  #define ADIS16201_ALM_SMPL2  0x26
> -
> -/* Alarm control */
>  #define ADIS16201_ALM_CTRL   0x28
>  
> -/* Auxiliary DAC data */
>  #define ADIS16201_AUX_DAC0x30
> -
> -/* General-purpose digital input/output control */
>  #define ADIS16201_GPIO_CTRL  0x32
> -
> -/* Miscellaneous control */
>  #define ADIS16201_MSC_CTRL   0x34
>  
> -/* Internal sample period (rate) control */
>  #define ADIS16201_SMPL_PRD   0x36
> -
> -/* Operation, filter configuration */
>  #define ADIS16201_AVG_CNT0x38
> -
> -/* Operation, sleep mode control */
>  #define ADIS16201_SLP_CNT0x3A
>  
>  /* Diagnostics, system status register */
> @@ -109,42 +64,28 @@
>  /* Operation, system command register */
>  #define ADIS16201_GLOB_CMD   0x3E
>  
> -/* MSC_CTRL */
>  
> -/* Self-test enable */
>  #define ADIS16201_MSC_CTRL_SELF_TEST_EN  BIT(8)
>  
> -/* Data-ready enable: 1 = enabled, 0 = disabled */
>  #define ADIS16201_MSC_CTRL_DATA_RDY_EN   BIT(2)
>  
> -/* Data-ready polarity: 1 = active high, 0 = active low */
>  #define ADIS16201_MSC_CTRL_ACTIVE_HIGH   BIT(1)
>  
> -/* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
>  #define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
>  
> -/* DIAG_STAT */
>  
> -/* Alarm 2 status: 1 = alarm active, 0 = alarm inactive */
>  #define ADIS16201_DIAG_STAT_ALARM2BIT(9)
>  
> -/* Alarm 1 status: 1 = alarm active, 0 = alarm inactive */
>  #define ADIS16201_DIAG_STAT_ALARM1BIT(8)
>  
> -/* SPI communications failure */
>  #define ADIS16201_DIAG_STAT_SPI_FAIL_BIT   3
>  
> -/* Flash update failure */
>  #define ADIS16201_DIAG_STAT_FLASH_UPT_BIT  2
I would have preferred this and the next patch to be
in the opposite order as then it would have been obvious when
reading this patch on why the 

Re: [PATCH] staging: iio: Remove unnecessary cast on void pointer

[Jonathan Cameron]

On Wed, 7 Mar 2018 18:47:17 +0530
Arushi Singhal  wrote:

> The following Coccinelle script was used to detect this:
> @r@
> expression x;
> void* e;
> type T;
> identifier f;
> @@
> (
>   *((T *)e)
> |
>   ((T *)x)[...]
> |
>   ((T*)x)->f
> |
> 
> - (T*)
>   e
> )
> 
> Signed-off-by: Arushi Singhal 
Applied to the togreg branch of iio.git and pushed out as testing.git
for the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/adc/ad7816.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7816.c 
> b/drivers/staging/iio/adc/ad7816.c
> index bfe180a..bf76a86 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -254,7 +254,7 @@ static const struct attribute_group 
> ad7816_attribute_group = {
>  static irqreturn_t ad7816_event_handler(int irq, void *private)
>  {
>   iio_push_event(private, IIO_EVENT_CODE_AD7816_OTI,
> -iio_get_time_ns((struct iio_dev *)private));
> +iio_get_time_ns(private));
>   return IRQ_HANDLED;
>  }
>  

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

Re: [PATCH v2 3/3] staging:iio:meter: Aligns open parenthesis

[Jonathan Cameron]

On Tue, 6 Mar 2018 21:44:26 -0300
Rodrigo Siqueira  wrote:

> This patch fixes the checkpatch.pl checks:
> 
> staging/iio/meter/ade7854-spi.c:19: CHECK: Alignment should match open
> parenthesis
> staging/iio/meter/ade7854-spi.c:44: CHECK: Alignment should match open
> parenthesis
> staging/iio/meter/ade7854-spi.c:70: CHECK: Alignment should match open
> parenthesis
> staging/iio/meter/ade7854-spi.c:97: CHECK: Alignment should match open
> parenthesis
> staging/iio/meter/ade7854-spi.c:125: CHECK: Alignment should match open
> parenthesis
> 
> ...
> 
> Signed-off-by: Rodrigo Siqueira 
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/meter/ade7758_trigger.c |  6 ++---
>  drivers/staging/iio/meter/ade7854-spi.c | 40 
> ++---
>  2 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7758_trigger.c 
> b/drivers/staging/iio/meter/ade7758_trigger.c
> index 1f0d1a0cf889..3314aa298d99 100644
> --- a/drivers/staging/iio/meter/ade7758_trigger.c
> +++ b/drivers/staging/iio/meter/ade7758_trigger.c
> @@ -30,7 +30,7 @@ static irqreturn_t ade7758_data_rdy_trig_poll(int irq, void 
> *private)
>   * ade7758_data_rdy_trigger_set_state() set datardy interrupt state
>   **/
>  static int ade7758_data_rdy_trigger_set_state(struct iio_trigger *trig,
> - bool state)
> +   bool state)
>  {
>   struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
>  
> @@ -63,8 +63,8 @@ int ade7758_probe_trigger(struct iio_dev *indio_dev)
>   int ret;
>  
>   st->trig = iio_trigger_alloc("%s-dev%d",
> - spi_get_device_id(st->us)->name,
> - indio_dev->id);
> +  spi_get_device_id(st->us)->name,
> +  indio_dev->id);
>   if (!st->trig) {
>   ret = -ENOMEM;
>   goto error_ret;
> diff --git a/drivers/staging/iio/meter/ade7854-spi.c 
> b/drivers/staging/iio/meter/ade7854-spi.c
> index 72eddfec21f7..4419b8f06197 100644
> --- a/drivers/staging/iio/meter/ade7854-spi.c
> +++ b/drivers/staging/iio/meter/ade7854-spi.c
> @@ -16,8 +16,8 @@
>  #include "ade7854.h"
>  
>  static int ade7854_spi_write_reg_8(struct device *dev,
> - u16 reg_address,
> - u8 val)
> +u16 reg_address,
> +u8 val)
>  {
>   int ret;
>   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> @@ -41,8 +41,8 @@ static int ade7854_spi_write_reg_8(struct device *dev,
>  }
>  
>  static int ade7854_spi_write_reg_16(struct device *dev,
> - u16 reg_address,
> - u16 val)
> + u16 reg_address,
> + u16 val)
>  {
>   int ret;
>   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> @@ -67,8 +67,8 @@ static int ade7854_spi_write_reg_16(struct device *dev,
>  }
>  
>  static int ade7854_spi_write_reg_24(struct device *dev,
> - u16 reg_address,
> - u32 val)
> + u16 reg_address,
> + u32 val)
>  {
>   int ret;
>   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> @@ -94,8 +94,8 @@ static int ade7854_spi_write_reg_24(struct device *dev,
>  }
>  
>  static int ade7854_spi_write_reg_32(struct device *dev,
> - u16 reg_address,
> - u32 val)
> + u16 reg_address,
> + u32 val)
>  {
>   int ret;
>   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> @@ -122,8 +122,8 @@ static int ade7854_spi_write_reg_32(struct device *dev,
>  }
>  
>  static int ade7854_spi_read_reg_8(struct device *dev,
> - u16 reg_address,
> - u8 *val)
> +   u16 reg_address,
> +   u8 *val)
>  {
>   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>   struct ade7854_state *st = iio_priv(indio_dev);
> @@ -149,7 +149,7 @@ static int ade7854_spi_read_reg_8(struct device *dev,
>   ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
>   if (ret) {
>   dev_err(>spi->dev, "problem when reading 8 bit register 
> 0x%02X",
> - reg_address);
> + reg_address);
>   goto error_ret;
>   }
>   *val = st->rx[0];
> @@ -160,8 +160,8 @@ static int ade7854_spi_read_reg_8(struct device *dev,
>  }
>  
>  static int ade7854_spi_read_reg_16(struct device *dev,
> - u16 reg_address,
> - u16 *val)
> +u16 reg_address,
> +u16 *val)
>  {
>   

Re: [PATCH 05/11] Staging: iio: accel: Add _REG suffix to registers

[Jonathan Cameron]

On Mon,  5 Mar 2018 13:19:24 +0530
Himanshu Jha  wrote:

> Addition of _REG suffix to the register definitions allows a distinction
> between registers and register fields. The various registers and its field
> bits are grouped together to improve readability and easy indentification.
> 
> Signed-off-by: Himanshu Jha 
This one looks good. I will pick it up once you've tweaked the
predecessor patches.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/accel/adis16201.c | 133 
> --
>  1 file changed, 61 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 445cb56..476b1c3 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -20,75 +20,64 @@
>  #include 
>  #include 
>  
> -#define ADIS16201_STARTUP_DELAY_MS   220
> -#define ADIS16201_FLASH_CNT  0x00
> +#define ADIS16201_STARTUP_DELAY_MS   220
> +#define ADIS16201_FLASH_CNT  0x00
>  
>  /* Data Output Register Information */
> -#define ADIS16201_SUPPLY_OUT 0x02
> -#define ADIS16201_XACCL_OUT  0x04
> -#define ADIS16201_YACCL_OUT  0x06
> -#define ADIS16201_AUX_ADC0x08
> -#define ADIS16201_TEMP_OUT   0x0A
> -#define ADIS16201_XINCL_OUT  0x0C
> -#define ADIS16201_YINCL_OUT  0x0E
> +#define ADIS16201_SUPPLY_OUT_REG 0x02
> +#define ADIS16201_XACCL_OUT_REG  0x04
> +#define ADIS16201_YACCL_OUT_REG  0x06
> +#define ADIS16201_AUX_ADC_REG0x08
> +#define ADIS16201_TEMP_OUT_REG   0x0A
> +#define ADIS16201_XINCL_OUT_REG  0x0C
> +#define ADIS16201_YINCL_OUT_REG  0x0E
>  
>  /* Calibration Register Definition */
> -#define ADIS16201_XACCL_OFFS 0x10
> -#define ADIS16201_YACCL_OFFS 0x12
> -#define ADIS16201_XACCL_SCALE0x14
> -#define ADIS16201_YACCL_SCALE0x16
> -#define ADIS16201_XINCL_OFFS 0x18
> -#define ADIS16201_YINCL_OFFS 0x1A
> -#define ADIS16201_XINCL_SCALE0x1C
> -#define ADIS16201_YINCL_SCALE0x1E
> +#define ADIS16201_XACCL_OFFS_REG 0x10
> +#define ADIS16201_YACCL_OFFS_REG 0x12
> +#define ADIS16201_XACCL_SCALE_REG0x14
> +#define ADIS16201_YACCL_SCALE_REG0x16
> +#define ADIS16201_XINCL_OFFS_REG 0x18
> +#define ADIS16201_YINCL_OFFS_REG 0x1A
> +#define ADIS16201_XINCL_SCALE_REG0x1C
> +#define ADIS16201_YINCL_SCALE_REG0x1E
>  
>  /* Alarm Register Definition */
> -#define ADIS16201_ALM_MAG1   0x20
> -#define ADIS16201_ALM_MAG2   0x22
> -#define ADIS16201_ALM_SMPL1  0x24
> -#define ADIS16201_ALM_SMPL2  0x26
> -#define ADIS16201_ALM_CTRL   0x28
> -
> -#define ADIS16201_AUX_DAC0x30
> -#define ADIS16201_GPIO_CTRL  0x32
> -#define ADIS16201_MSC_CTRL   0x34
> -
> -#define ADIS16201_SMPL_PRD   0x36
> -#define ADIS16201_AVG_CNT0x38
> -#define ADIS16201_SLP_CNT0x3A
> +#define ADIS16201_ALM_MAG1_REG   0x20
> +#define ADIS16201_ALM_MAG2_REG   0x22
> +#define ADIS16201_ALM_SMPL1_REG  0x24
> +#define ADIS16201_ALM_SMPL2_REG  0x26
> +#define ADIS16201_ALM_CTRL_REG   0x28
> +
> +#define ADIS16201_AUX_DAC_REG0x30
> +#define ADIS16201_GPIO_CTRL_REG  0x32
> +#define ADIS16201_MSC_CTRL_REG   0x34
> +#define ADIS16201_SMPL_PRD_REG   0x36
> +#define ADIS16201_AVG_CNT_REG0x38
> +#define ADIS16201_SLP_CNT_REG0x3A
> +
> +/* Miscellaneous Control Register Definition */
> +#define ADIS16201_MSC_CTRL_REG   0x34
> +#define  ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
> +#define  ADIS16201_MSC_CTRL_DATA_RDY_EN  BIT(2)
> +#define  ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
> +#define  ADIS16201_MSC_CTRL_DATA_RDY_DIO1BIT(0)
>  
>  /* Diagnostics, system status register */
> -#define ADIS16201_DIAG_STAT  0x3C
> +#define ADIS16201_DIAG_STAT_REG  0x3C
> +#define  ADIS16201_DIAG_STAT_ALARM2  BIT(9)
> +#define  ADIS16201_DIAG_STAT_ALARM1  BIT(8)
> 

[PATCH v2] staging: lustre: Remove VLA usage

[Kees Cook]

The kernel would like to have all stack VLA usage removed[1]. This switches
to a simple kasprintf() instead, and in the process fixes an off-by-one
between the allocation and the sprintf (allocation did not include NULL
byte in calculation).

[1] https://lkml.org/lkml/2018/3/7/621

Signed-off-by: Kees Cook 
Reviewed-by: Rasmus Villemoes 
---
 drivers/staging/lustre/lustre/llite/xattr.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/xattr.c 
b/drivers/staging/lustre/lustre/llite/xattr.c
index 532384c91447..ff6fe81a4ddb 100644
--- a/drivers/staging/lustre/lustre/llite/xattr.c
+++ b/drivers/staging/lustre/lustre/llite/xattr.c
@@ -87,10 +87,10 @@ ll_xattr_set_common(const struct xattr_handler *handler,
const char *name, const void *value, size_t size,
int flags)
 {
-   char fullname[strlen(handler->prefix) + strlen(name) + 1];
struct ll_sb_info *sbi = ll_i2sbi(inode);
struct ptlrpc_request *req = NULL;
const char *pv = value;
+   char *fullname;
__u64 valid;
int rc;
 
@@ -141,10 +141,13 @@ ll_xattr_set_common(const struct xattr_handler *handler,
return -EPERM;
}
 
-   sprintf(fullname, "%s%s\n", handler->prefix, name);
+   fullname = kasprintf(GFP_KERNEL, "%s%s\n", handler->prefix, name);
+   if (!fullname)
+   return -ENOMEM;
rc = md_setxattr(sbi->ll_md_exp, ll_inode2fid(inode),
 valid, fullname, pv, size, 0, flags,
 ll_i2suppgid(inode), );
+   kfree(fullname);
if (rc) {
if (rc == -EOPNOTSUPP && handler->flags == XATTR_USER_T) {
LCONSOLE_INFO("Disabling user_xattr feature because it 
is not supported on the server\n");
@@ -364,11 +367,11 @@ static int ll_xattr_get_common(const struct xattr_handler 
*handler,
   struct dentry *dentry, struct inode *inode,
   const char *name, void *buffer, size_t size)
 {
-   char fullname[strlen(handler->prefix) + strlen(name) + 1];
struct ll_sb_info *sbi = ll_i2sbi(inode);
 #ifdef CONFIG_FS_POSIX_ACL
struct ll_inode_info *lli = ll_i2info(inode);
 #endif
+   char *fullname;
int rc;
 
CDEBUG(D_VFSTRACE, "VFS Op:inode=" DFID "(%p)\n",
@@ -411,9 +414,13 @@ static int ll_xattr_get_common(const struct xattr_handler 
*handler,
if (handler->flags == XATTR_ACL_DEFAULT_T && !S_ISDIR(inode->i_mode))
return -ENODATA;
 #endif
-   sprintf(fullname, "%s%s\n", handler->prefix, name);
-   return ll_xattr_list(inode, fullname, handler->flags, buffer, size,
-OBD_MD_FLXATTR);
+   fullname = kasprintf(GFP_KERNEL, "%s%s\n", handler->prefix, name);
+   if (!fullname)
+   return -ENOMEM;
+   rc = ll_xattr_list(inode, fullname, handler->flags, buffer, size,
+  OBD_MD_FLXATTR);
+   kfree(fullname);
+   return rc;
 }
 
 static ssize_t ll_getxattr_lov(struct inode *inode, void *buf, size_t buf_size)
-- 
2.7.4


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

Re: [PATCH v3 4/4] Staging: iio: adis16209: Use sign_extend32 function

[Jonathan Cameron]

On Sun,  4 Mar 2018 18:15:06 +0530
Shreeya Patel  wrote:

> Use sign_extend32 function instead of manually coding
> it.
> 
> Signed-off-by: Shreeya Patel 
Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to play with it.

Thanks,

Jonathan

> ---
> 
> Changes in v3
>   -After split patch.
> 
>  drivers/staging/iio/accel/adis16209.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16209.c 
> b/drivers/staging/iio/accel/adis16209.c
> index 9cb1ce0..a8453bf 100644
> --- a/drivers/staging/iio/accel/adis16209.c
> +++ b/drivers/staging/iio/accel/adis16209.c
> @@ -212,9 +212,8 @@ static int adis16209_read_raw(struct iio_dev *indio_dev,
>   ret = adis_read_reg_16(st, addr, );
>   if (ret)
>   return ret;
> - val16 &= (1 << bits) - 1;
> - val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
> - *val = val16;
> +
> + *val = sign_extend32(val16, bits - 1);
>   return IIO_VAL_INT;
>   }
>   return -EINVAL;

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

Re: [PATCH 02/11] Staging: iio: accel: Add a blank space before returns

[Jonathan Cameron]

On Mon,  5 Mar 2018 13:19:21 +0530
Himanshu Jha  wrote:

> Adding a blank space before/after some returns improves readability.
> 
> Signed-off-by: Himanshu Jha 
Applied - patch title adjusted.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/accel/adis16201.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 0f6a204..0fae8aa 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -232,6 +232,7 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
>   *val = val16;
>   return IIO_VAL_INT;
>   }
> +
>   return -EINVAL;
>  }
>  
> @@ -262,6 +263,7 @@ static int adis16201_write_raw(struct iio_dev *indio_dev,
>   addr = adis16201_addresses[chan->scan_index];
>   return adis_write_reg_16(st, addr, val16);
>   }
> +
>   return -EINVAL;
>  }
>  
> @@ -336,6 +338,7 @@ static int adis16201_probe(struct spi_device *spi)
>   ret = adis_init(st, indio_dev, spi, _data);
>   if (ret)
>   return ret;
> +
>   ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
>   if (ret)
>   return ret;
> @@ -348,6 +351,7 @@ static int adis16201_probe(struct spi_device *spi)
>   ret = iio_device_register(indio_dev);
>   if (ret < 0)
>   goto error_cleanup_buffer_trigger;
> +
>   return 0;
>  
>  error_cleanup_buffer_trigger:

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

Re: [PATCH 11/11] Staging: iio: accel: Move adis16201 driver out of staging subsystem

[Jonathan Cameron]

On Mon,  5 Mar 2018 13:19:30 +0530
Himanshu Jha  wrote:

> Move the adis16201 driver out of staging directory and merge to the
> mainline IIO subsystem.
> 
> Signed-off-by: Himanshu Jha 
One comment inline (that I should have noticed in one of your earlier patches 
but
missed).

Michael, do you have the time to look through these at the moment?

The changes are not likely to break anything so if you are too busy I'm happy to
move them out of staging if you are fine with that.

Thanks,

Jonathan

> ---
>  drivers/iio/accel/Kconfig |  12 ++
>  drivers/iio/accel/Makefile|   1 +
>  drivers/iio/accel/adis16201.c | 323 
> ++
>  drivers/staging/iio/accel/Kconfig |  12 --
>  drivers/staging/iio/accel/Makefile|   1 -
>  drivers/staging/iio/accel/adis16201.c | 323 
> --
>  6 files changed, 336 insertions(+), 336 deletions(-)
>  create mode 100644 drivers/iio/accel/adis16201.c
>  delete mode 100644 drivers/staging/iio/accel/adis16201.c
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index c6d9517..9416c6f 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -5,6 +5,18 @@
>  
>  menu "Accelerometers"
>  
> +config ADIS16201
> +tristate "Analog Devices ADIS16201 Dual-Axis Digital Inclinometer 
> and Accelerometer"
> +depends on SPI
> +select IIO_ADIS_LIB
> +select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
> +help
> +  Say Y here to build support for Analog Devices adis16201 dual-axis
> +  digital inclinometer and accelerometer.
> +
> +  To compile this driver as a module, say M here: the module will
> +  be called adis16201.
> +
>  config ADXL345
>   tristate
>  
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 368aedb..7832ec9 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -4,6 +4,7 @@
>  #
>  
>  # When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_ADIS16201) += adis16201.o
>  obj-$(CONFIG_ADXL345) += adxl345_core.o
>  obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
>  obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
> diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
> new file mode 100644
> index 000..946c7b1
> --- /dev/null
> +++ b/drivers/iio/accel/adis16201.c
> @@ -0,0 +1,323 @@
> +/*
> + * ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer
> + *
> + * Copyright 2010 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define ADIS16201_STARTUP_DELAY_MS   220
> +#define ADIS16201_FLASH_CNT  0x00
> +
> +/* Data Output Register Information */
> +#define ADIS16201_SUPPLY_OUT_REG 0x02
> +#define ADIS16201_XACCL_OUT_REG  0x04
> +#define ADIS16201_YACCL_OUT_REG  0x06
> +#define ADIS16201_AUX_ADC_REG0x08
> +#define ADIS16201_TEMP_OUT_REG   0x0A
> +#define ADIS16201_XINCL_OUT_REG  0x0C
> +#define ADIS16201_YINCL_OUT_REG  0x0E
> +
> +/* Calibration Register Definition */
> +#define ADIS16201_XACCL_OFFS_REG 0x10
> +#define ADIS16201_YACCL_OFFS_REG 0x12
> +#define ADIS16201_XACCL_SCALE_REG0x14
> +#define ADIS16201_YACCL_SCALE_REG0x16
> +#define ADIS16201_XINCL_OFFS_REG 0x18
> +#define ADIS16201_YINCL_OFFS_REG 0x1A
> +#define ADIS16201_XINCL_SCALE_REG0x1C
> +#define ADIS16201_YINCL_SCALE_REG0x1E
> +
> +/* Alarm Register Definition */
> +#define ADIS16201_ALM_MAG1_REG   0x20
> +#define ADIS16201_ALM_MAG2_REG   0x22
> +#define ADIS16201_ALM_SMPL1_REG  0x24
> +#define ADIS16201_ALM_SMPL2_REG  0x26
> +#define ADIS16201_ALM_CTRL_REG   0x28
> +
> +#define ADIS16201_AUX_DAC_REG0x30
> +#define ADIS16201_GPIO_CTRL_REG  0x32
> +#define ADIS16201_MSC_CTRL_REG   0x34
> +#define ADIS16201_SMPL_PRD_REG   0x36
> +#define ADIS16201_AVG_CNT_REG0x38
> +#define ADIS16201_SLP_CNT_REG  

Re: [PATCH v3 2/4] Staging: iio: adis16209: Change some macro names

[Jonathan Cameron]

On Sun,  4 Mar 2018 18:11:17 +0530
Shreeya Patel  wrote:

> Make some of the macro names according to the names
> given in the datasheet of the adis16209 driver.
> 
> Signed-off-by: Shreeya Patel 
A small comment inline which we should clear up in a follow up patch.
> ---
> 
> Changes in v3
>   -Introduce this new patch for v3 of the series.
> 
>  drivers/staging/iio/accel/adis16209.c | 48 
> +--
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16209.c 
> b/drivers/staging/iio/accel/adis16209.c
> index d8aef9c..eb5c878 100644
> --- a/drivers/staging/iio/accel/adis16209.c
> +++ b/drivers/staging/iio/accel/adis16209.c
> @@ -68,21 +68,21 @@
>  #define  ADIS16209_MSC_CTRL_ACTIVE_HIGH  BIT(1)
>  #define  ADIS16209_MSC_CTRL_DATA_RDY_DIO2BIT(0)
>  
> -#define ADIS16209_DIAG_STAT_REG  0x3C
> -#define  ADIS16209_DIAG_STAT_ALARM2  BIT(9)
> -#define  ADIS16209_DIAG_STAT_ALARM1  BIT(8)
> -#define ADIS16209_DIAG_STAT_SELFTEST_FAIL_BIT5
> -#define ADIS16209_DIAG_STAT_SPI_FAIL_BIT 3
> -#define ADIS16209_DIAG_STAT_FLASH_UPT_BIT2
> +#define ADIS16209_STAT_REG   0x3C
> +#define  ADIS16209_STAT_ALARM2   BIT(9)
> +#define  ADIS16209_STAT_ALARM1   BIT(8)
> +#define ADIS16209_STAT_SELFTEST_FAIL_BIT 5
> +#define ADIS16209_STAT_SPI_FAIL_BIT  3
> +#define ADIS16209_STAT_FLASH_UPT_FAIL_BIT2
Given these are also fields of the STAT_REG, be it defined in a different
fashion I think they should also have the small additional indent.


>  /* Power supply above 3.625 V */
> -#define ADIS16209_DIAG_STAT_POWER_HIGH_BIT   1
> +#define ADIS16209_STAT_POWER_HIGH_BIT1
>  /* Power supply below 3.15 V */
> -#define ADIS16209_DIAG_STAT_POWER_LOW_BIT0
> +#define ADIS16209_STAT_POWER_LOW_BIT 0
>  
> -#define ADIS16209_GLOB_CMD_REG   0x3E
> -#define  ADIS16209_GLOB_CMD_SW_RESET BIT(7)
> -#define  ADIS16209_GLOB_CMD_CLEAR_STAT   BIT(4)
> -#define  ADIS16209_GLOB_CMD_FACTORY_CAL  BIT(1)
> +#define ADIS16209_CMD_REG0x3E
> +#define  ADIS16209_CMD_SW_RESET  BIT(7)
> +#define  ADIS16209_CMD_CLEAR_STATBIT(4)
> +#define  ADIS16209_CMD_FACTORY_CAL   BIT(1)
>  
>  #define ADIS16209_ERROR_ACTIVE   BIT(14)
>  
> @@ -238,29 +238,29 @@ static const struct iio_info adis16209_info = {
>  };
>  
>  static const char * const adis16209_status_error_msgs[] = {
> - [ADIS16209_DIAG_STAT_SELFTEST_FAIL_BIT] = "Self test failure",
> - [ADIS16209_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
> - [ADIS16209_DIAG_STAT_FLASH_UPT_BIT] = "Flash update failed",
> - [ADIS16209_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
> - [ADIS16209_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V",
> + [ADIS16209_STAT_SELFTEST_FAIL_BIT] = "Self test failure",
> + [ADIS16209_STAT_SPI_FAIL_BIT] = "SPI failure",
> + [ADIS16209_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
> + [ADIS16209_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
> + [ADIS16209_STAT_POWER_LOW_BIT] = "Power supply below 3.15V",
>  };
>  
>  static const struct adis_data adis16209_data = {
>   .read_delay = 30,
>   .msc_ctrl_reg = ADIS16209_MSC_CTRL_REG,
> - .glob_cmd_reg = ADIS16209_GLOB_CMD_REG,
> - .diag_stat_reg = ADIS16209_DIAG_STAT_REG,
> + .glob_cmd_reg = ADIS16209_CMD_REG,
> + .diag_stat_reg = ADIS16209_STAT_REG,
>  
>   .self_test_mask = ADIS16209_MSC_CTRL_SELF_TEST_EN,
>   .self_test_no_autoclear = true,
>   .startup_delay = ADIS16209_STARTUP_DELAY_MS,
>  
>   .status_error_msgs = adis16209_status_error_msgs,
> - .status_error_mask = BIT(ADIS16209_DIAG_STAT_SELFTEST_FAIL_BIT) |
> - BIT(ADIS16209_DIAG_STAT_SPI_FAIL_BIT) |
> - BIT(ADIS16209_DIAG_STAT_FLASH_UPT_BIT) |
> - BIT(ADIS16209_DIAG_STAT_POWER_HIGH_BIT) |
> - BIT(ADIS16209_DIAG_STAT_POWER_LOW_BIT),
> + .status_error_mask = BIT(ADIS16209_STAT_SELFTEST_FAIL_BIT) |
> + BIT(ADIS16209_STAT_SPI_FAIL_BIT) |
> + BIT(ADIS16209_STAT_FLASH_UPT_FAIL_BIT) |
> + BIT(ADIS16209_STAT_POWER_HIGH_BIT) |
> + BIT(ADIS16209_STAT_POWER_LOW_BIT),
>  };
>  
>  static int adis16209_probe(struct spi_device *spi)

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

Re: [PATCH] staging: iio: meter: Remove reduntant __func__ from debug print

[Jonathan Cameron]

On Wed,  7 Mar 2018 11:08:04 +0530
hariprasath.ela...@gmail.com wrote:

> From: HariPrasath Elango 
> 
> dev_dbg includes the function name & line number by default when dynamic
> debugging is enabled. Hence__func__ is reduntant here and removed.
> 
> Signed-off-by: HariPrasath Elango 
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Please note that, after discussions with Michael, I'm planning to send
out an email stating these meter drivers will be dropped next cycle
unless we have someone come forward who can test them.

We can't realistically do the work needed to move them out of testing
without a high risk of breaking them and they are 'not suitable
for new designs'.

Ah well, we'll see - someone might care and be able to help

Thanks

Jonathan

> ---
>  drivers/staging/iio/meter/ade7758_trigger.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7758_trigger.c 
> b/drivers/staging/iio/meter/ade7758_trigger.c
> index 1f0d1a0..da489ae 100644
> --- a/drivers/staging/iio/meter/ade7758_trigger.c
> +++ b/drivers/staging/iio/meter/ade7758_trigger.c
> @@ -34,7 +34,7 @@ static int ade7758_data_rdy_trigger_set_state(struct 
> iio_trigger *trig,
>  {
>   struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
>  
> - dev_dbg(_dev->dev, "%s (%d)\n", __func__, state);
> + dev_dbg(_dev->dev, "(%d)\n", state);
>   return ade7758_set_irq(_dev->dev, state);
>  }
>  

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

Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR

[Jonathan Cameron]

On Tue, 6 Mar 2018 21:43:47 -0300
Rodrigo Siqueira  wrote:

> The macro IIO_DEV_ATTR_CH_OFF is a wrapper for IIO_DEVICE_ATTR, with a
> tiny change in the name definition. This extra macro does not improve
> the readability and also creates some checkpatch errors.
> 
> This patch fixes the checkpatch.pl errors:
> 
> staging/iio/meter/ade7753.c:391: ERROR: Use 4 digit octal (0777) not
> decimal permissions
> staging/iio/meter/ade7753.c:395: ERROR: Use 4 digit octal (0777) not
> decimal permissions
> staging/iio/meter/ade7759.c:331: ERROR: Use 4 digit octal (0777) not
> decimal permissions
> staging/iio/meter/ade7759.c:335: ERROR: Use 4 digit octal (0777) not
> decimal permissions
> 
> Signed-off-by: Rodrigo Siqueira 

Hmm. I wondered a bit about this one. It's a correct patch in of
itself but the interface in question doesn't even vaguely conform
to any of defined IIO ABI.  Anyhow, it's still and improvement so
I'll take it.

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

I also added the removal of the header define which is no
longer used.

Please note, following discussions with Michael, I am going to send
an email announcing an intent to drop these meter drivers next
cycle unless someone can provide testing for any attempt to
move them out of staging.  I'm still taking patches on the basis
that 'might' happen - but I wouldn't focus on these until we
have some certainty on whether they will be around long term!

Jonathan

> ---
>  drivers/staging/iio/meter/ade7753.c | 18 ++
>  drivers/staging/iio/meter/ade7759.c | 18 ++
>  2 files changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7753.c 
> b/drivers/staging/iio/meter/ade7753.c
> index c44eb577dc35..275e8dfff836 100644
> --- a/drivers/staging/iio/meter/ade7753.c
> +++ b/drivers/staging/iio/meter/ade7753.c
> @@ -388,14 +388,16 @@ static IIO_DEV_ATTR_VPERIOD(0444,
>   ade7753_read_16bit,
>   NULL,
>   ADE7753_PERIOD);
> -static IIO_DEV_ATTR_CH_OFF(1, 0644,
> - ade7753_read_8bit,
> - ade7753_write_8bit,
> - ADE7753_CH1OS);
> -static IIO_DEV_ATTR_CH_OFF(2, 0644,
> - ade7753_read_8bit,
> - ade7753_write_8bit,
> - ADE7753_CH2OS);
> +
> +static IIO_DEVICE_ATTR(choff_1, 0644,
> + ade7753_read_8bit,
> + ade7753_write_8bit,
> + ADE7753_CH1OS);
> +
> +static IIO_DEVICE_ATTR(choff_2, 0644,
> + ade7753_read_8bit,
> + ade7753_write_8bit,
> + ADE7753_CH2OS);
>  
>  static int ade7753_set_irq(struct device *dev, bool enable)
>  {
> diff --git a/drivers/staging/iio/meter/ade7759.c 
> b/drivers/staging/iio/meter/ade7759.c
> index 1decb2b8afab..c078b770fa53 100644
> --- a/drivers/staging/iio/meter/ade7759.c
> +++ b/drivers/staging/iio/meter/ade7759.c
> @@ -328,14 +328,16 @@ static IIO_DEV_ATTR_ACTIVE_POWER_GAIN(0644,
>   ade7759_read_16bit,
>   ade7759_write_16bit,
>   ADE7759_APGAIN);
> -static IIO_DEV_ATTR_CH_OFF(1, 0644,
> - ade7759_read_8bit,
> - ade7759_write_8bit,
> - ADE7759_CH1OS);
> -static IIO_DEV_ATTR_CH_OFF(2, 0644,
> - ade7759_read_8bit,
> - ade7759_write_8bit,
> - ADE7759_CH2OS);
> +
> +static IIO_DEVICE_ATTR(choff_1, 0644,
> + ade7759_read_8bit,
> + ade7759_write_8bit,
> + ADE7759_CH1OS);
> +
> +static IIO_DEVICE_ATTR(choff_2, 0644,
> + ade7759_read_8bit,
> + ade7759_write_8bit,
> + ADE7759_CH2OS);
>  
>  static int ade7759_set_irq(struct device *dev, bool enable)
>  {

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

Re: [PATCH 01/11] Staging: iio: accel: Prefer alphabetical sequence of header files

[Jonathan Cameron]

On Mon,  5 Mar 2018 13:19:20 +0530
Himanshu Jha  wrote:

> Arrange header files in alphabetical sequence to improve readability.
> 
> Signed-off-by: Himanshu Jha 
One general comment - when naming a patch it will form the description
in the git short log or on the various web interfaces etc. It's 
really useful to actually say which driver it is.

I've fixed that up by adding adis16201: in the appropriate place.

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/accel/adis16201.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 2ebd275..0f6a204 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -7,13 +7,13 @@
>   */
>  
>  #include 
> -#include 
>  #include 
>  #include 
> -#include 
> +#include 
> +#include 
>  #include 
> +#include 
>  #include 
> -#include 
>  
>  #include 
>  #include 

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

Re: [PATCH 08/11] Staging: iio: accel: Use switch statement than if-else

[Jonathan Cameron]

On Mon,  5 Mar 2018 13:19:27 +0530
Himanshu Jha  wrote:

> Use switch statement instead of if-else pair to explicitly match
> the only two channels present.
> 
> Signed-off-by: Himanshu Jha 
I think this is going to generate some warnings in the various static
analysers as they will point out there are lots of values channel can
take that aren't handled by the switch statement..  You should have
a default.

(This is what made Dan less than convinced of whether this was a good
change when I originally suggested it)  I still think it's a marginal
improvement in making it explicit that we only have two valid choices
though - and Dan didn't care strongly about it.

Jonathan
> ---
>  drivers/staging/iio/accel/adis16201.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 1737708..307d4ab 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -114,12 +114,15 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
>   case IIO_CHAN_INFO_SCALE:
>   switch (chan->type) {
>   case IIO_VOLTAGE:
> - if (chan->channel == 0) {
> + switch (chan->channel) {
> + case 0:
>   *val = 1;
>   *val2 = 22;
> - } else {
> + break;
> + case 1:
>   *val = 0;
>   *val2 = 61;
> + break;
>   }
>   return IIO_VAL_INT_PLUS_MICRO;
>   case IIO_TEMP:

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

Re: [PATCH] staging: lustre: Remove VLA usage

[Rasmus Villemoes]

On Wed, Mar 07 2018, Kees Cook  wrote:

> On Wed, Mar 7, 2018 at 5:10 AM, Rasmus Villemoes
>  wrote:
>> On 2018-03-07 06:46, Kees Cook wrote:
>>> The kernel would like to remove all VLA usage. This switches to a
>>> simple kasprintf() instead.
>>>
>>
>> It's probably worth pointing out that this actually fixes an
>> unconditional buffer overflow: fullname only has room for the two
>> strings and the '\n', but vsnprintf() is told that the buffer has
>> infinite size (well, INT_MAX), so there should be plenty of room to
>> append the '\0' after the '\n'.
>>
>
> Oh yes, hah. I didn't even see the \n in the string. :P
>
> So, both a VLA fix and a buffer over-run fix. Can I add your "Reviewed-by"? :)

Sure,

Reviewed-by: Rasmus Villemoes 

A nit, if you're resending anyway: can you move the "char *fullname"
declarations down a bit, to between pv,valid, and lli,rc, respectively?
That keeps the initialized and uninitialized variables nicely together
and ends up looking better.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 3/4] Staging: iio: adis16209: Adjust a switch statement

[Jonathan Cameron]

On Sun,  4 Mar 2018 18:13:12 +0530
Shreeya Patel  wrote:

> Adjust a switch block to explicitly match channels and
> return -EINVAL as default case which makes the code
> semantically more clear.
> 
> Signed-off-by: Shreeya Patel 
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan.
> ---
> 
> Changes in v3
>   -After split patch.
> 
>  drivers/staging/iio/accel/adis16209.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16209.c 
> b/drivers/staging/iio/accel/adis16209.c
> index eb5c878..9cb1ce0 100644
> --- a/drivers/staging/iio/accel/adis16209.c
> +++ b/drivers/staging/iio/accel/adis16209.c
> @@ -155,10 +155,16 @@ static int adis16209_read_raw(struct iio_dev *indio_dev,
>   switch (chan->type) {
>   case IIO_VOLTAGE:
>   *val = 0;
> - if (chan->channel == 0)
> + switch (chan->channel) {
> + case 0:
>   *val2 = 305180; /* 0.30518 mV */
> - else
> + break;
> + case 1:
>   *val2 = 610500; /* 0.6105 mV */
> + break;
> + default:
> + return -EINVAL;
> + }
>   return IIO_VAL_INT_PLUS_MICRO;
>   case IIO_TEMP:
>   *val = -470;

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

[PATCH] staging: android: ashmem: Remove deadlock

[Paul Lawrence]

Regression introduced in commit ce8a3a9e76d0193e2e8d74a06d275b3c324ca652
("staging: android: ashmem: Fix a race condition in pin ioctls")
causing deadlock.

No need to hold ashmem_mutex while copying from user

Stacks are:

ashmem_mmap+0x53/0x400 drivers/staging/android/ashmem.c:379
mmap_region+0x7dd/0xfd0 mm/mmap.c:1694
do_mmap+0x57b/0xbe0 mm/mmap.c:1473

and

lock_acquire+0x12e/0x410 kernel/locking/lockdep.c:3756
__might_fault+0x14a/0x1d0 mm/memory.c:4014
copy_from_user arch/x86/include/asm/uaccess.h:705 [inline]
ashmem_pin_unpin drivers/staging/android/ashmem.c:719 [inline]

Signed-off-by: Paul Lawrence 
Cc:  # 4.9.x
Cc:  # 4.4.x
Cc:  # 3.18.x: ce8a3a9e76d01
Cc:  # 3.18.x
Cc: Ben Hutchings 
---
 drivers/staging/android/ashmem.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 6dbba5aff191..8c55706c2cfa 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -702,16 +702,14 @@ static int ashmem_pin_unpin(struct ashmem_area *asma, 
unsigned long cmd,
size_t pgstart, pgend;
int ret = -EINVAL;
 
+   if (unlikely(copy_from_user(, p, sizeof(pin
+   return -EFAULT;
+
mutex_lock(_mutex);
 
if (unlikely(!asma->file))
goto out_unlock;
 
-   if (unlikely(copy_from_user(, p, sizeof(pin {
-   ret = -EFAULT;
-   goto out_unlock;
-   }
-
/* per custom, you can pass zero for len to mean "everything onward" */
if (!pin.len)
pin.len = PAGE_ALIGN(asma->size) - pin.offset;
-- 
2.16.2.395.g2e18187dfd-goog

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

RE: [PATCH v3 6/6] PCI: hv: fix 2 hang issues in hv_compose_msi_msg()

[Dexuan Cui]

> From: Lorenzo Pieralisi 
> Sent: Wednesday, March 7, 2018 04:35
> On Tue, Mar 06, 2018 at 06:21:56PM +, Dexuan Cui wrote:
> > 1. With the patch "x86/vector/msi: Switch to global reservation mode"
> > (4900be8360), the recent v4.15 and newer kernels always hang for 1-vCPU
> > Hyper-V VM with SR-IOV. This is because when we reach
> hv_compose_msi_msg()
> > by request_irq()  -> request_threaded_irq() -> __setup_irq()->irq_startup()
> >  -> __irq_startup() -> irq_domain_activate_irq() -> ... ->
> > msi_domain_activate() -> ... -> hv_compose_msi_msg(), local irq is
> > disabled in __setup_irq().
> >
> > Fix this by polling the channel.
> >
> > 2. If the host is ejecting the VF device before we reach
> > hv_compose_msi_msg(), in a UP VM, we can hang in hv_compose_msi_msg()
> > forever, because at this time the host doesn't respond to the
> > CREATE_INTERRUPT request. This issue also happens to old kernels like
> > v4.14, v4.13, etc.
>
> If you are fixing a problem you should report what commit you are fixing
> with a Fixes: tag and add a CC: sta...@vger.kernel.org to the commit log
> to send it to stable kernels to which it should be applied; mentioning
> kernel versions in the commit log is useless and should be omitted.

Hi Lorenzo,
Thanks for your comments!
This patch does have a "Cc: sta...@vger.kernel.org" in the sign-off area. :-)

Here the patch is made to resolve 2 issues:
#1 is triggered by the x86 global reservation mode (4900be8360) patch.
4900be8360 in itself is good. It's just that drivers/pci/host/pci-hyperv.c
should be fixed.

#2 is a longstanding issue since the first day the pci-hyperv driver was
accepted into the kernel.

So IMO actually we don't really need to add a Fixes: tag, which is usually
used to specify a specific commit that introduces a bug that is being fixed.

> Side note: you should not have sta...@vger.kernel.org in the email
> addresses CC list you are sending the patches to (you mark patches for
> stable by adding an appropriate CC tag in the commit log).

Sorry, I didn't know this, but actually I didn't add sta...@vger.kernel.org
manually. Instead I used "git send-email" to send this patchset, and it told
me "The Cc list above has been expanded by additional addresses found
in the patch commit message."

I didn't find a way to disable this behavior of "git send-email" by checking
its manual and googling it. This is strange.

> Here:
>
> git.kernel.org/.../Documentation/process/stable-kernel-rules.rst
>
> Last but not least, most of the patches in this series do not justify
> sending them to stable kernels at all so you should remove the
> corresponding tag from the patches.

I hope at least these 2 patches can go into the stable kernels:
[PATCH v3 3/6] PCI: hv: serialize the present/eject work items
[PATCH v3 6/6] PCI: hv: fix 2 hang issues in hv_compose_msi_msg()
Especially the second one, which fixes a real hang issue for UP virtual
machines running v4.15 and newer.
And,  IMO the patches are small enough (<100 lines) , but definitely
the maintainers make the final call.

>
> Thanks,
> Lorenzo
>
> > Fix this by polling the channel for the PCI_EJECT message and
> > hpdev->state, and by checking the PCI vendor ID.
> >
> > Note: actually the above issues also happen to a SMP VM, if
> > "hbus->hdev->channel->target_cpu == smp_processor_id()" is true.
> >
> > Signed-off-by: Dexuan Cui 
> > Tested-by: Adrian Suhov 
> > Tested-by: Chris Valean 
> > Cc: sta...@vger.kernel.org
> > Cc: Stephen Hemminger 
> > Cc: K. Y. Srinivasan 
> > Cc: Vitaly Kuznetsov 
> > Cc: Jack Morgenstein 
> > ---
> >  drivers/pci/host/pci-hyperv.c | 58


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

Re: [PATCH] staging: android: ashmem: Remove deadlock

[Nathan Chancellor]

On Wed, Mar 07, 2018 at 01:40:30PM -0800, Paul Lawrence wrote:
> Regression introduced in commit ce8a3a9e76d0193e2e8d74a06d275b3c324ca652
> ("staging: android: ashmem: Fix a race condition in pin ioctls")
> causing deadlock.
> 
> No need to hold ashmem_mutex while copying from user
> 
> Stacks are:
> 
> ashmem_mmap+0x53/0x400 drivers/staging/android/ashmem.c:379
> mmap_region+0x7dd/0xfd0 mm/mmap.c:1694
> do_mmap+0x57b/0xbe0 mm/mmap.c:1473
> 
> and
> 
> lock_acquire+0x12e/0x410 kernel/locking/lockdep.c:3756
> __might_fault+0x14a/0x1d0 mm/memory.c:4014
> copy_from_user arch/x86/include/asm/uaccess.h:705 [inline]
> ashmem_pin_unpin drivers/staging/android/ashmem.c:719 [inline]
> 
> Signed-off-by: Paul Lawrence 
> Cc:  # 4.9.x
> Cc:  # 4.4.x
> Cc:  # 3.18.x: ce8a3a9e76d01
> Cc:  # 3.18.x
> Cc: Ben Hutchings 
> ---
>  drivers/staging/android/ashmem.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/android/ashmem.c 
> b/drivers/staging/android/ashmem.c
> index 6dbba5aff191..8c55706c2cfa 100644
> --- a/drivers/staging/android/ashmem.c
> +++ b/drivers/staging/android/ashmem.c
> @@ -702,16 +702,14 @@ static int ashmem_pin_unpin(struct ashmem_area *asma, 
> unsigned long cmd,
>   size_t pgstart, pgend;
>   int ret = -EINVAL;
>  
> + if (unlikely(copy_from_user(, p, sizeof(pin
> + return -EFAULT;
> +
>   mutex_lock(_mutex);
>  
>   if (unlikely(!asma->file))
>   goto out_unlock;
>  
> - if (unlikely(copy_from_user(, p, sizeof(pin {
> - ret = -EFAULT;
> - goto out_unlock;
> - }
> -
>   /* per custom, you can pass zero for len to mean "everything onward" */
>   if (!pin.len)
>   pin.len = PAGE_ALIGN(asma->size) - pin.offset;
> -- 
> 2.16.2.395.g2e18187dfd-goog
>

Hey Paul,

Looks like this same patch is already in Greg's tree:
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?id=740a5759bf222332fbb5eda42f89aa25ba38f9b2

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

[PATCH PATCH net v2 0/4] hv_netvsc: fix multicast flags and sync

[Stephen Hemminger]

This set of patches deals with the handling of multicast flags
and addresses in transparent VF mode. The recent set of patches
(in linux-net) had a couple of bugs.

Stephen Hemminger (4):
  hv_netvsc: fix filter flags
  hv_netvsc: avoid repeated updates of packet filter
  hv_netvsc: fix locking for rx_mode
  hv_netvsc: fix locking during VF setup

 drivers/net/hyperv/hyperv_net.h   |  1 +
 drivers/net/hyperv/netvsc_drv.c   | 15 ---
 drivers/net/hyperv/rndis_filter.c | 12 
 3 files changed, 21 insertions(+), 7 deletions(-)

-- 
2.16.1

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

[PATCH] staging: fsl-mc/dpio: Add missing argument identifier

[Roy Pledge]

When running checkpatch over the DPIO code the following warning
is reported:

WARNING: function definition argument 'struct dpaa2_io_notification_ctx *' 
should also have an identifier name

Add the missing identifier.

Signed-off-by: Roy Pledge 
---
 drivers/staging/fsl-mc/include/dpaa2-io.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/fsl-mc/include/dpaa2-io.h 
b/drivers/staging/fsl-mc/include/dpaa2-io.h
index 9cb1eec..f71227d 100644
--- a/drivers/staging/fsl-mc/include/dpaa2-io.h
+++ b/drivers/staging/fsl-mc/include/dpaa2-io.h
@@ -80,7 +80,7 @@ struct dpaa2_io *dpaa2_io_service_select(int cpu);
  * Used when a FQDAN/CDAN registration is made by drivers.
  */
 struct dpaa2_io_notification_ctx {
-   void (*cb)(struct dpaa2_io_notification_ctx *);
+   void (*cb)(struct dpaa2_io_notification_ctx *ctx);
int is_cdan;
u32 id;
int desired_cpu;
-- 
2.7.4

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

Re: [PATCH] staging: android: ashmem: Remove deadlock

[Greg Kroah-Hartman]

On Wed, Mar 07, 2018 at 02:02:13PM -0800, Paul Lawrence wrote:
> Great! We need to make sure this gets backported to 4.4 and 4.9, and to
> 3.18 with the original dependency, please.

That will happen when it lands in Linus's tree, which should be later
this week if all goes well.

thanks,

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

ACKNOWLEDGE RECEIPT 3/7/2018

[MINT SA]

Good Day,

This is a letter of Intent for Investment Project. See attached for full 
details.

Indicate your interest to my personal email: batud...@yandex.com

Yours sincerely.

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

[PATCH PATCH net v2 3/4] hv_netvsc: fix locking for rx_mode

[Stephen Hemminger]

The rx_mode operation handler is different than other callbacks
in that is not always called with rtnl held. Therefore use
RCU to ensure that references are valid.

Fixes: bee9d41b37ea ("hv_netvsc: propagate rx filters to VF")
Signed-off-by: Stephen Hemminger 
---
 drivers/net/hyperv/netvsc_drv.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index cdb78eefab67..48d9fa7a66c2 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -89,15 +89,20 @@ static void netvsc_change_rx_flags(struct net_device *net, 
int change)
 static void netvsc_set_rx_mode(struct net_device *net)
 {
struct net_device_context *ndev_ctx = netdev_priv(net);
-   struct net_device *vf_netdev = rtnl_dereference(ndev_ctx->vf_netdev);
-   struct netvsc_device *nvdev = rtnl_dereference(ndev_ctx->nvdev);
+   struct net_device *vf_netdev;
+   struct netvsc_device *nvdev;
 
+   rcu_read_lock();
+   vf_netdev = rcu_dereference(ndev_ctx->vf_netdev);
if (vf_netdev) {
dev_uc_sync(vf_netdev, net);
dev_mc_sync(vf_netdev, net);
}
 
-   rndis_filter_update(nvdev);
+   nvdev = rcu_dereference(ndev_ctx->nvdev);
+   if (nvdev)
+   rndis_filter_update(nvdev);
+   rcu_read_unlock();
 }
 
 static int netvsc_open(struct net_device *net)
-- 
2.16.1

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

[PATCH PATCH net v2 4/4] hv_netvsc: fix locking during VF setup

[Stephen Hemminger]

The dev_uc/mc_sync calls need to have the device address list
locked. This was spotted by running with lockdep enabled.

Fixes: bee9d41b37ea ("hv_netvsc: propagate rx filters to VF")
Signed-off-by: Stephen Hemminger 
---
 drivers/net/hyperv/netvsc_drv.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 48d9fa7a66c2..faea0be18924 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1851,8 +1851,12 @@ static void __netvsc_vf_setup(struct net_device *ndev,
 
/* set multicast etc flags on VF */
dev_change_flags(vf_netdev, ndev->flags | IFF_SLAVE);
+
+   /* sync address list from ndev to VF */
+   netif_addr_lock_bh(ndev);
dev_uc_sync(vf_netdev, ndev);
dev_mc_sync(vf_netdev, ndev);
+   netif_addr_unlock_bh(ndev);
 
if (netif_running(ndev)) {
ret = dev_open(vf_netdev);
-- 
2.16.1

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

[PATCH PATCH net v2 2/4] hv_netvsc: avoid repeated updates of packet filter

[Stephen Hemminger]

The netvsc driver can get repeated calls to netvsc_rx_mode during
network setup; each of these calls ends up scheduling the lower
layers to update tha packet filter. This update requires an
request/response to the host. So avoid doing this if we already
know that the correct packet filter value is set.

Signed-off-by: Stephen Hemminger 
---
 drivers/net/hyperv/hyperv_net.h   | 1 +
 drivers/net/hyperv/rndis_filter.c | 8 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 0db3bd1ea06f..cd538d5a7986 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -173,6 +173,7 @@ struct rndis_device {
struct list_head req_list;
 
struct work_struct mcast_work;
+   u32 filter;
 
bool link_state;/* 0 - link up, 1 - link down */
 
diff --git a/drivers/net/hyperv/rndis_filter.c 
b/drivers/net/hyperv/rndis_filter.c
index 411a3aee39b2..00ec80c23fe5 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -825,13 +825,15 @@ static int rndis_filter_set_packet_filter(struct 
rndis_device *dev,
struct rndis_set_request *set;
int ret;
 
+   if (dev->filter == new_filter)
+   return 0;
+
request = get_rndis_request(dev, RNDIS_MSG_SET,
RNDIS_MESSAGE_SIZE(struct rndis_set_request) +
sizeof(u32));
if (!request)
return -ENOMEM;
 
-
/* Setup the rndis set */
set = >request_msg.msg.set_req;
set->oid = RNDIS_OID_GEN_CURRENT_PACKET_FILTER;
@@ -842,8 +844,10 @@ static int rndis_filter_set_packet_filter(struct 
rndis_device *dev,
   _filter, sizeof(u32));
 
ret = rndis_filter_send_request(dev, request);
-   if (ret == 0)
+   if (ret == 0) {
wait_for_completion(>wait_event);
+   dev->filter = new_filter;
+   }
 
put_rndis_request(dev, request);
 
-- 
2.16.1

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

[PATCH PATCH net v2 1/4] hv_netvsc: fix filter flags

[Stephen Hemminger]

The recent change to nto always enable all multicast and broadcast
was broken; meant to set filter, not change flags.

Fixes: 009f766ca238 ("hv_netvsc: filter multicast/broadcast")
Signed-off-by: Stephen Hemminger 
---
 drivers/net/hyperv/rndis_filter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hyperv/rndis_filter.c 
b/drivers/net/hyperv/rndis_filter.c
index 8927c483c217..411a3aee39b2 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -861,9 +861,9 @@ static void rndis_set_multicast(struct work_struct *w)
filter = NDIS_PACKET_TYPE_PROMISCUOUS;
} else {
if (flags & IFF_ALLMULTI)
-   flags |= NDIS_PACKET_TYPE_ALL_MULTICAST;
+   filter |= NDIS_PACKET_TYPE_ALL_MULTICAST;
if (flags & IFF_BROADCAST)
-   flags |= NDIS_PACKET_TYPE_BROADCAST;
+   filter |= NDIS_PACKET_TYPE_BROADCAST;
}
 
rndis_filter_set_packet_filter(rdev, filter);
-- 
2.16.1

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

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

[Laura Abbott]

On 02/28/2018 09:18 PM, Liam Mark wrote:

The issue:

Currently in ION if you allocate uncached memory it is possible that there
are still dirty lines in the cache.  And often these dirty lines in the
cache are the zeros which were meant to clear out any sensitive kernel
data.

What this means is that if you allocate uncached memory from ION, and then
subsequently write to that buffer (using the uncached mapping you are
provided by ION) then the data you have written could be corrupted at some
point in the future if a dirty line is evicted from the cache.

Also this means there is a potential security issue.  If an un-privileged
userspace user allocated uncached memory (for example from the system heap)
and then if they were to read from that buffer (through the un-cached
mapping they are provided by ION), and if some of the zeros which were
written to that memory are still in the cache then this un-privileged
userspace user could read potentially sensitive kernel data.


For the use case you are describing we don't actually need the
memory to be non-cached until it comes time to do the dma mapping.
Here's a proposal to shoot holes in:

- Before any dma_buf attach happens, all mmap mappings are cached
- At the time attach happens, we shoot down any existing userspace
mappings, do the dma_map with appropriate flags to clean the pages
and then allow remapping to userspace as uncached. Really this
looks like a variation on the old Ion faulting code which I removed
except it's for uncached buffers instead of cached buffers.

Potential problems:
- I'm not 100% about the behavior here if the attaching device
is already dma_coherent. I also consider uncached mappings
enough of a device specific optimization that you shouldn't
do them unless you know it's needed.
- The locking/sequencing with userspace could be tricky
since userspace may not like us ripping mappings out from
underneath if it's trying to access.

Thoughts?

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

,Your urgent confirmation

[James Williams]

Attn: Beneficiary,

We have contacted the Federal Ministry of Finance on your Behalf and
they have brought a solution to your problem by coordinating your
payment in total (10,000,000.00) Ten Million Dollars in an atm card
which you can use to withdraw money from any ATM MACHINE CENTER
anywhere in the world with a maximum of 1 Dollars daily. You now
have the lawful right to claim your fund in an atm card. Since the
Federal Bureau of Investigation is involved in this transaction, you
have to be rest assured for this is 100% risk free it is our duty to
protect the American Citizens, European Citizens, Asian Citizen. All I
want you to do is to contact the atm card CENTER Via email or call the
office telephone number one of the Consultant will assist you for
their requirements to proceed and procure your Approval Slip on your
behalf.

CONTACT INFORMATION
NAME: James  Williams
EMAIL: paymasterofficed...@gmail.com


Do contact us with your details:

Full name//
Address// Age//
 Telephone Numbers//
Occupation//
 Your Country//
Bank Details//

So your files would be updated after which the Delivery of your
atm card will be affected to your designated home Address without any
further delay and the bank will transfer your funds in total
(10,000,000.00) Ten Million Dollars to your Bank account. We
will reply you with the secret code (1600 atm card). We advice you get
back to the payment office after you have contacted the ATM SWIFT CARD
CENTER and we do await your response so we can move on with our
Investigation and make sure your ATM SWIFT CARD gets to you.


Best Regards
James Williams
Paymaster General
Federal Republic Of Nigeri
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 10/11] Staging: iio: accel: Add comments about units in data read function

[Himanshu Jha]

On Wed, Mar 07, 2018 at 08:50:30PM +, Jonathan Cameron wrote:
> On Mon,  5 Mar 2018 13:19:29 +0530
> Himanshu Jha  wrote:
> 
> > Clarify the conversion and formation of resultant data in the
> > adis16201_read_raw() with sufficient comments.
> > 
> > Signed-off-by: Himanshu Jha 
> This is fine but it needs to be in the original comment changing patch
> rather than removing the comments first then a few patches later putting
> back a different version.
> 
> So good change but in the wrong place in the series.  Learning to reorder
> a series and merge down multiple patches into one is very useful when
> working with git.

I will send v2 with all the updates!

What about sign_extend32 & reverse christmas ordering patch ? Will you
pick that up or shall I send them again ?

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

Re: [PATCH] staging: lustre: Remove VLA usage

[Kees Cook]

On Wed, Mar 7, 2018 at 5:10 AM, Rasmus Villemoes
 wrote:
> On 2018-03-07 06:46, Kees Cook wrote:
>> The kernel would like to remove all VLA usage. This switches to a
>> simple kasprintf() instead.
>>
>> Signed-off-by: Kees Cook 
>> ---
>>  drivers/staging/lustre/lustre/llite/xattr.c | 19 +--
>>  1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/llite/xattr.c 
>> b/drivers/staging/lustre/lustre/llite/xattr.c
>> index 532384c91447..aab4eab64289 100644
>> --- a/drivers/staging/lustre/lustre/llite/xattr.c
>> +++ b/drivers/staging/lustre/lustre/llite/xattr.c
>> @@ -87,7 +87,7 @@ ll_xattr_set_common(const struct xattr_handler *handler,
>>   const char *name, const void *value, size_t size,
>>   int flags)
>>  {
>> - char fullname[strlen(handler->prefix) + strlen(name) + 1];
>> + char *fullname;
>>   struct ll_sb_info *sbi = ll_i2sbi(inode);
>>   struct ptlrpc_request *req = NULL;
>>   const char *pv = value;
>> @@ -141,10 +141,13 @@ ll_xattr_set_common(const struct xattr_handler 
>> *handler,
>>   return -EPERM;
>>   }
>>
>> - sprintf(fullname, "%s%s\n", handler->prefix, name);
>
> It's probably worth pointing out that this actually fixes an
> unconditional buffer overflow: fullname only has room for the two
> strings and the '\n', but vsnprintf() is told that the buffer has
> infinite size (well, INT_MAX), so there should be plenty of room to
> append the '\0' after the '\n'.
>
>> + fullname = kasprintf(GFP_KERNEL, "%s%s\n", handler->prefix, name);
>> + if (!fullname)
>> + return -ENOMEM;
>>   rc = md_setxattr(sbi->ll_md_exp, ll_inode2fid(inode),
>>valid, fullname, pv, size, 0, flags,
>>ll_i2suppgid(inode), );
>> + kfree(fullname);
>>   if (rc) {
>>   if (rc == -EOPNOTSUPP && handler->flags == XATTR_USER_T) {
>>   LCONSOLE_INFO("Disabling user_xattr feature because it 
>> is not supported on the server\n");
>> @@ -364,7 +367,7 @@ static int ll_xattr_get_common(const struct 
>> xattr_handler *handler,
>>  struct dentry *dentry, struct inode *inode,
>>  const char *name, void *buffer, size_t size)
>>  {
>> - char fullname[strlen(handler->prefix) + strlen(name) + 1];
>> + char *fullname;
>>   struct ll_sb_info *sbi = ll_i2sbi(inode);
>>  #ifdef CONFIG_FS_POSIX_ACL
>>   struct ll_inode_info *lli = ll_i2info(inode);
>> @@ -411,9 +414,13 @@ static int ll_xattr_get_common(const struct 
>> xattr_handler *handler,
>>   if (handler->flags == XATTR_ACL_DEFAULT_T && !S_ISDIR(inode->i_mode))
>>   return -ENODATA;
>>  #endif
>> - sprintf(fullname, "%s%s\n", handler->prefix, name);
>
> Same here.
>
> I'm a little surprised this hasn't been caugt by static analysis, I
> thought gcc/coverity/smatch/whatnot had gotten pretty good at computing
> the size of the output generated by a given format string with "known"
> arguments and comparing to the size of the output buffer. Though of
> course it does require the tool to be able to do symbolic manipulations,
> in this case realizing that
>
> outsize == strlen(x)+strlen(y)+1+1 > bufsize == strlen(x)+strlen(y)+1
>
> Rasmus

Oh yes, hah. I didn't even see the \n in the string. :P

So, both a VLA fix and a buffer over-run fix. Can I add your "Reviewed-by"? :)

-Kees

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

Re: [PATCH v3] staging: rtl8192u: Replace printk() with more standardize output format.

[Dan Carpenter]

On Wed, Mar 07, 2018 at 04:09:09PM +0530, Arushi Singhal wrote:
> @@ -276,22 +276,22 @@ static int ieee80211_ccmp_decrypt(struct sk_buff *skb, 
> int hdr_len, void *priv)
>   keyidx = pos[3];
>   if (!(keyidx & (1 << 5))) {
>   if (net_ratelimit()) {
> - printk(KERN_DEBUG "CCMP: received packet without ExtIV 
> flag from %pM\n",
> - hdr->addr2);
> + netdev_dbg(skb->dev, "CCMP: received packet without 
> ExtIV flag from %pM\n",
> +hdr->addr2);
>   }

I'm sorry.  I really hate to do this to you...  But the right thing here
is to use net_dbg_ratelimited() but get rid of the if (net_ratelimit())
check.  So it looks like this:

if (!(keyidx & (1 << 5))) {
net_dbg_ratelimited(skb->dev,
"CCMP: received packet without ExtIV flag 
from %pM\n",
hdr->addr2);
}

Sorry again, when I looked at it before I just replied what my fast
thinking lizard brain said instead of taking time to look at what I was
saying.


regards,
dan carpenter

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

Re: [Outreachy kernel] Re: [PATCH v3] staging: rtl8192u: Replace printk() with more standardize output format.

[Julia Lawall]

On Wed, 7 Mar 2018, Dan Carpenter wrote:

> On Wed, Mar 07, 2018 at 04:09:09PM +0530, Arushi Singhal wrote:
> > @@ -276,22 +276,22 @@ static int ieee80211_ccmp_decrypt(struct sk_buff 
> > *skb, int hdr_len, void *priv)
> > keyidx = pos[3];
> > if (!(keyidx & (1 << 5))) {
> > if (net_ratelimit()) {
> > -   printk(KERN_DEBUG "CCMP: received packet without ExtIV 
> > flag from %pM\n",
> > -   hdr->addr2);
> > +   netdev_dbg(skb->dev, "CCMP: received packet without 
> > ExtIV flag from %pM\n",
> > +  hdr->addr2);
> > }
>
> I'm sorry.  I really hate to do this to you...  But the right thing here
> is to use net_dbg_ratelimited() but get rid of the if (net_ratelimit())
> check.  So it looks like this:
>
>   if (!(keyidx & (1 << 5))) {
>   net_dbg_ratelimited(skb->dev,
>   "CCMP: received packet without ExtIV flag 
> from %pM\n",
>   hdr->addr2);
>   }
>
> Sorry again, when I looked at it before I just replied what my fast
> thinking lizard brain said instead of taking time to look at what I was
> saying.

I don't think that net_dbg_ratelimited wants a skb->dev argument.  The
various definitions in include/linux/net.h just have fmt as the first
argument.

julia

>
>
> regards,
> dan carpenter
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20180307104741.mdy2wa7hplfwxmb3%40mwanda.
> For more options, visit https://groups.google.com/d/optout.
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: most: Remove unnecessary usage of BUG_ON().

[Christian Gromm]

On 07.03.2018 02:31, Quytelda Kahja wrote:

There is no need for the calls to BUG_ON() in this driver, which are
used to check if mbo or mbo->context are NULL; mbo is never NULL, and
if mbo->context is NULL it would have already been dereferenced and
oopsed before reaching the BUG_ON().

Signed-off-by: Quytelda Kahja 

Acked-by: Christian Gromm 

---
  drivers/staging/most/core.c | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index 0ab2de5ecf18..3afc25a61643 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -915,7 +915,6 @@ static void arm_mbo(struct mbo *mbo)
unsigned long flags;
struct most_channel *c;
  
-	BUG_ON((!mbo) || (!mbo->context));

c = mbo->context;
  
  	if (c->is_poisoned) {

@@ -1018,8 +1017,6 @@ static void most_write_completion(struct mbo *mbo)
  {
struct most_channel *c;
  
-	BUG_ON((!mbo) || (!mbo->context));

-
c = mbo->context;
if (mbo->status == MBO_E_INVAL)
pr_info("WARN: Tx MBO status: invalid\n");



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

[PATCH v3] staging: rtl8192u: Replace printk() with more standardize output format.

[Arushi Singhal]

printk() is the raw way to print output and should be avoided.

For drivers with defined "struct device object", dev_*macro() is
prefer and for "struct netdevice object", netdev_*macro() is prefer over
dev_*macro() to standardize the output format within the subsystem.

If no "struct device object" is defined prefer pr_*macro() over printk().

This patch Replace printk having a log level with the appropriate output
format according to the order of preference.

Signed-off-by: Arushi Singhal 
---
Changes in v3
*In version2 net_*macro_ratelimited was used which is not helping in
standardizing the output format.

Changes in v2
*change the subject line, driver name was wrong.

 .../rtl8192u/ieee80211/ieee80211_crypt_ccmp.c  | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_ccmp.c 
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_ccmp.c
index e6648f7..a4b4042 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_ccmp.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_ccmp.c
@@ -73,7 +73,7 @@ static void *ieee80211_ccmp_init(int key_idx)
 
priv->tfm = (void *)crypto_alloc_cipher("aes", 0, CRYPTO_ALG_ASYNC);
if (IS_ERR(priv->tfm)) {
-   printk(KERN_DEBUG "ieee80211_crypt_ccmp: could not allocate 
crypto API aes\n");
+   pr_debug("ieee80211_crypt_ccmp: could not allocate crypto API 
aes\n");
priv->tfm = NULL;
goto fail;
}
@@ -276,22 +276,22 @@ static int ieee80211_ccmp_decrypt(struct sk_buff *skb, 
int hdr_len, void *priv)
keyidx = pos[3];
if (!(keyidx & (1 << 5))) {
if (net_ratelimit()) {
-   printk(KERN_DEBUG "CCMP: received packet without ExtIV 
flag from %pM\n",
-   hdr->addr2);
+   netdev_dbg(skb->dev, "CCMP: received packet without 
ExtIV flag from %pM\n",
+  hdr->addr2);
}
key->dot11RSNAStatsCCMPFormatErrors++;
return -2;
}
keyidx >>= 6;
if (key->key_idx != keyidx) {
-   printk(KERN_DEBUG "CCMP: RX tkey->key_idx=%d frame keyidx=%d 
priv=%p\n",
-   key->key_idx, keyidx, priv);
+   netdev_dbg(skb->dev, "CCMP: RX tkey->key_idx=%d frame keyidx=%d 
priv=%p\n",
+  key->key_idx, keyidx, priv);
return -6;
}
if (!key->key_set) {
if (net_ratelimit()) {
-   printk(KERN_DEBUG "CCMP: received packet from %pM with 
keyid=%d that does not have a configured key\n",
-   hdr->addr2, keyidx);
+   netdev_dbg(skb->dev, "CCMP: received packet from %pM 
with keyid=%d that does not have a configured key\n",
+  hdr->addr2, keyidx);
}
return -3;
}
@@ -306,8 +306,8 @@ static int ieee80211_ccmp_decrypt(struct sk_buff *skb, int 
hdr_len, void *priv)
 
if (memcmp(pn, key->rx_pn, CCMP_PN_LEN) <= 0) {
if (net_ratelimit()) {
-   printk(KERN_DEBUG "CCMP: replay detected: STA=%pM 
previous PN %pm received PN %pm\n",
-  hdr->addr2, key->rx_pn, pn);
+   netdev_dbg(skb->dev, "CCMP: replay detected: STA=%pM 
previous PN %pm received PN %pm\n",
+  hdr->addr2, key->rx_pn, pn);
}
key->dot11RSNAStatsCCMPReplays++;
return -4;
@@ -341,8 +341,8 @@ static int ieee80211_ccmp_decrypt(struct sk_buff *skb, int 
hdr_len, void *priv)
 
if (memcmp(mic, a, CCMP_MIC_LEN) != 0) {
if (net_ratelimit()) {
-   printk(KERN_DEBUG "CCMP: decrypt failed: 
STA=%pM\n",
-   hdr->addr2);
+   netdev_dbg(skb->dev, "CCMP: decrypt failed: 
STA=%pM\n",
+  hdr->addr2);
}
key->dot11RSNAStatsCCMPDecryptErrors++;
return -5;
-- 
2.7.4

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

[PATCH] staging: iio: Remove unnecessary cast on void pointer

[Arushi Singhal]

The following Coccinelle script was used to detect this:
@r@
expression x;
void* e;
type T;
identifier f;
@@
(
  *((T *)e)
|
  ((T *)x)[...]
|
  ((T*)x)->f
|

- (T*)
  e
)

Signed-off-by: Arushi Singhal 
---
 drivers/staging/iio/adc/ad7816.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index bfe180a..bf76a86 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -254,7 +254,7 @@ static const struct attribute_group ad7816_attribute_group 
= {
 static irqreturn_t ad7816_event_handler(int irq, void *private)
 {
iio_push_event(private, IIO_EVENT_CODE_AD7816_OTI,
-  iio_get_time_ns((struct iio_dev *)private));
+  iio_get_time_ns(private));
return IRQ_HANDLED;
 }
 
-- 
2.7.4

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

Re: [Outreachy kernel] Re: [PATCH v3] staging: rtl8192u: Replace printk() with more standardize output format.

[Dan Carpenter]

On Wed, Mar 07, 2018 at 11:52:16AM +0100, Julia Lawall wrote:
> 
> 
> On Wed, 7 Mar 2018, Dan Carpenter wrote:
> 
> > On Wed, Mar 07, 2018 at 04:09:09PM +0530, Arushi Singhal wrote:
> > > @@ -276,22 +276,22 @@ static int ieee80211_ccmp_decrypt(struct sk_buff 
> > > *skb, int hdr_len, void *priv)
> > >   keyidx = pos[3];
> > >   if (!(keyidx & (1 << 5))) {
> > >   if (net_ratelimit()) {
> > > - printk(KERN_DEBUG "CCMP: received packet without ExtIV 
> > > flag from %pM\n",
> > > - hdr->addr2);
> > > + netdev_dbg(skb->dev, "CCMP: received packet without 
> > > ExtIV flag from %pM\n",
> > > +hdr->addr2);
> > >   }
> >
> > I'm sorry.  I really hate to do this to you...  But the right thing here
> > is to use net_dbg_ratelimited() but get rid of the if (net_ratelimit())
> > check.  So it looks like this:
> >
> > if (!(keyidx & (1 << 5))) {
> > net_dbg_ratelimited(skb->dev,
> > "CCMP: received packet without ExtIV flag 
> > from %pM\n",
> > hdr->addr2);
> > }
> >
> > Sorry again, when I looked at it before I just replied what my fast
> > thinking lizard brain said instead of taking time to look at what I was
> > saying.
> 
> I don't think that net_dbg_ratelimited wants a skb->dev argument.  The
> various definitions in include/linux/net.h just have fmt as the first
> argument.
>

Ah  That's fine then.  Don't worry about it.

regards,
dan carpenter

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

Re: [PATCH v3 6/6] PCI: hv: fix 2 hang issues in hv_compose_msi_msg()

[Lorenzo Pieralisi]

On Tue, Mar 06, 2018 at 06:21:56PM +, Dexuan Cui wrote:
> 1. With the patch "x86/vector/msi: Switch to global reservation mode"
> (4900be8360), the recent v4.15 and newer kernels always hang for 1-vCPU
> Hyper-V VM with SR-IOV. This is because when we reach hv_compose_msi_msg()
> by request_irq()  -> request_threaded_irq() -> __setup_irq()->irq_startup()
>  -> __irq_startup() -> irq_domain_activate_irq() -> ... ->
> msi_domain_activate() -> ... -> hv_compose_msi_msg(), local irq is
> disabled in __setup_irq().
> 
> Fix this by polling the channel.
> 
> 2. If the host is ejecting the VF device before we reach
> hv_compose_msi_msg(), in a UP VM, we can hang in hv_compose_msi_msg()
> forever, because at this time the host doesn't respond to the
> CREATE_INTERRUPT request. This issue also happens to old kernels like
> v4.14, v4.13, etc.

If you are fixing a problem you should report what commit you are fixing
with a Fixes: tag and add a CC: sta...@vger.kernel.org to the commit log
to send it to stable kernels to which it should be applied; mentioning
kernel versions in the commit log is useless and should be omitted.

Side note: you should not have sta...@vger.kernel.org in the email
addresses CC list you are sending the patches to (you mark patches for
stable by adding an appropriate CC tag in the commit log).

Here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v4.16-rc4

Last but not least, most of the patches in this series do not justify
sending them to stable kernels at all so you should remove the
corresponding tag from the patches.

Thanks,
Lorenzo

> Fix this by polling the channel for the PCI_EJECT message and
> hpdev->state, and by checking the PCI vendor ID.
> 
> Note: actually the above issues also happen to a SMP VM, if
> "hbus->hdev->channel->target_cpu == smp_processor_id()" is true.
> 
> Signed-off-by: Dexuan Cui 
> Tested-by: Adrian Suhov 
> Tested-by: Chris Valean 
> Cc: sta...@vger.kernel.org
> Cc: Stephen Hemminger 
> Cc: K. Y. Srinivasan 
> Cc: Vitaly Kuznetsov 
> Cc: Jack Morgenstein 
> ---
>  drivers/pci/host/pci-hyperv.c | 58 
> ++-
>  1 file changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 265ba11e53e2..50cdefe3f6d3 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -521,6 +521,8 @@ struct hv_pci_compl {
>   s32 completion_status;
>  };
>  
> +static void hv_pci_onchannelcallback(void *context);
> +
>  /**
>   * hv_pci_generic_compl() - Invoked for a completion packet
>   * @context: Set up by the sender of the packet.
> @@ -665,6 +667,31 @@ static void _hv_pcifront_read_config(struct hv_pci_dev 
> *hpdev, int where,
>   }
>  }
>  
> +static u16 hv_pcifront_get_vendor_id(struct hv_pci_dev *hpdev)
> +{
> + u16 ret;
> + unsigned long flags;
> + void __iomem *addr = hpdev->hbus->cfg_addr + CFG_PAGE_OFFSET +
> +  PCI_VENDOR_ID;
> +
> + spin_lock_irqsave(>hbus->config_lock, flags);
> +
> + /* Choose the function to be read. (See comment above) */
> + writel(hpdev->desc.win_slot.slot, hpdev->hbus->cfg_addr);
> + /* Make sure the function was chosen before we start reading. */
> + mb();
> + /* Read from that function's config space. */
> + ret = readw(addr);
> + /*
> +  * mb() is not required here, because the spin_unlock_irqrestore()
> +  * is a barrier.
> +  */
> +
> + spin_unlock_irqrestore(>hbus->config_lock, flags);
> +
> + return ret;
> +}
> +
>  /**
>   * _hv_pcifront_write_config() - Internal PCI config write
>   * @hpdev:   The PCI driver's representation of the device
> @@ -1107,8 +1134,37 @@ static void hv_compose_msi_msg(struct irq_data *data, 
> struct msi_msg *msg)
>* Since this function is called with IRQ locks held, can't
>* do normal wait for completion; instead poll.
>*/
> - while (!try_wait_for_completion(_pkt.host_event))
> + while (!try_wait_for_completion(_pkt.host_event)) {
> + /* 0x means an invalid PCI VENDOR ID. */
> + if (hv_pcifront_get_vendor_id(hpdev) == 0x) {
> + dev_err_once(>hdev->device,
> +  "the device has gone\n");
> + goto free_int_desc;
> + }
> +
> + /*
> +  * When the higher level interrupt code calls us with
> +  * interrupt disabled, we must poll the channel by calling
> +  * the channel callback directly when channel->target_cpu is
> +  * the current CPU. When the higher level interrupt code
> +  * calls us with interrupt enabled, let's add the
> + 

Re: [PATCH] staging: lustre: Remove VLA usage

[Rasmus Villemoes]

On 2018-03-07 06:46, Kees Cook wrote:
> The kernel would like to remove all VLA usage. This switches to a
> simple kasprintf() instead.
> 
> Signed-off-by: Kees Cook 
> ---
>  drivers/staging/lustre/lustre/llite/xattr.c | 19 +--
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/llite/xattr.c 
> b/drivers/staging/lustre/lustre/llite/xattr.c
> index 532384c91447..aab4eab64289 100644
> --- a/drivers/staging/lustre/lustre/llite/xattr.c
> +++ b/drivers/staging/lustre/lustre/llite/xattr.c
> @@ -87,7 +87,7 @@ ll_xattr_set_common(const struct xattr_handler *handler,
>   const char *name, const void *value, size_t size,
>   int flags)
>  {
> - char fullname[strlen(handler->prefix) + strlen(name) + 1];
> + char *fullname;
>   struct ll_sb_info *sbi = ll_i2sbi(inode);
>   struct ptlrpc_request *req = NULL;
>   const char *pv = value;
> @@ -141,10 +141,13 @@ ll_xattr_set_common(const struct xattr_handler *handler,
>   return -EPERM;
>   }
>  
> - sprintf(fullname, "%s%s\n", handler->prefix, name);

It's probably worth pointing out that this actually fixes an
unconditional buffer overflow: fullname only has room for the two
strings and the '\n', but vsnprintf() is told that the buffer has
infinite size (well, INT_MAX), so there should be plenty of room to
append the '\0' after the '\n'.

> + fullname = kasprintf(GFP_KERNEL, "%s%s\n", handler->prefix, name);
> + if (!fullname)
> + return -ENOMEM;
>   rc = md_setxattr(sbi->ll_md_exp, ll_inode2fid(inode),
>valid, fullname, pv, size, 0, flags,
>ll_i2suppgid(inode), );
> + kfree(fullname);
>   if (rc) {
>   if (rc == -EOPNOTSUPP && handler->flags == XATTR_USER_T) {
>   LCONSOLE_INFO("Disabling user_xattr feature because it 
> is not supported on the server\n");
> @@ -364,7 +367,7 @@ static int ll_xattr_get_common(const struct xattr_handler 
> *handler,
>  struct dentry *dentry, struct inode *inode,
>  const char *name, void *buffer, size_t size)
>  {
> - char fullname[strlen(handler->prefix) + strlen(name) + 1];
> + char *fullname;
>   struct ll_sb_info *sbi = ll_i2sbi(inode);
>  #ifdef CONFIG_FS_POSIX_ACL
>   struct ll_inode_info *lli = ll_i2info(inode);
> @@ -411,9 +414,13 @@ static int ll_xattr_get_common(const struct 
> xattr_handler *handler,
>   if (handler->flags == XATTR_ACL_DEFAULT_T && !S_ISDIR(inode->i_mode))
>   return -ENODATA;
>  #endif
> - sprintf(fullname, "%s%s\n", handler->prefix, name);

Same here.

I'm a little surprised this hasn't been caugt by static analysis, I
thought gcc/coverity/smatch/whatnot had gotten pretty good at computing
the size of the output generated by a given format string with "known"
arguments and comparing to the size of the output buffer. Though of
course it does require the tool to be able to do symbolic manipulations,
in this case realizing that

outsize == strlen(x)+strlen(y)+1+1 > bufsize == strlen(x)+strlen(y)+1

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

Re: [PATCH] staging: lustre: Remove VLA usage

[Tobin C. Harding]

On Tue, Mar 06, 2018 at 09:46:08PM -0800, Kees Cook wrote:
> The kernel would like to remove all VLA usage. This switches to a
> simple kasprintf() instead.
> 
> Signed-off-by: Kees Cook 
> ---
>  drivers/staging/lustre/lustre/llite/xattr.c | 19 +--
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/llite/xattr.c 
> b/drivers/staging/lustre/lustre/llite/xattr.c
> index 532384c91447..aab4eab64289 100644
> --- a/drivers/staging/lustre/lustre/llite/xattr.c
> +++ b/drivers/staging/lustre/lustre/llite/xattr.c
> @@ -87,7 +87,7 @@ ll_xattr_set_common(const struct xattr_handler *handler,
>   const char *name, const void *value, size_t size,
>   int flags)
>  {
> - char fullname[strlen(handler->prefix) + strlen(name) + 1];
> + char *fullname;
>   struct ll_sb_info *sbi = ll_i2sbi(inode);
>   struct ptlrpc_request *req = NULL;
>   const char *pv = value;
> @@ -141,10 +141,13 @@ ll_xattr_set_common(const struct xattr_handler *handler,
>   return -EPERM;
>   }
>  
> - sprintf(fullname, "%s%s\n", handler->prefix, name);
> + fullname = kasprintf(GFP_KERNEL, "%s%s\n", handler->prefix, name);
> + if (!fullname)
> + return -ENOMEM;
>   rc = md_setxattr(sbi->ll_md_exp, ll_inode2fid(inode),
>valid, fullname, pv, size, 0, flags,
>ll_i2suppgid(inode), );
> + kfree(fullname);

This is cool.  We've had kasprintf() since 2007, who knew?!

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