[PATCH] staging: iio: adc: add missing spaces around minus sign

2018-10-01 Thread Slawomir Stepien
The checkpatch.pl tool detected coding style problem:

CHECK: spaces preferred around that '-' (ctx:VxV)

in two files inside the adc directory. This patch will remove this
problem.

Signed-off-by: Slawomir Stepien 
---
 drivers/staging/iio/adc/ad7192.c  | 2 +-
 drivers/staging/iio/adc/ad7280a.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index acdbc07fd259..7c632cf1932b 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -355,7 +355,7 @@ ad7192_show_scale_available(struct device *dev,
 }
 
 static IIO_DEVICE_ATTR_NAMED(in_v_m_v_scale_available,
-in_voltage-voltage_scale_available,
+in_voltage - voltage_scale_available,
 0444, ad7192_show_scale_available, NULL, 0);
 
 static IIO_DEVICE_ATTR(in_voltage_scale_available, 0444,
diff --git a/drivers/staging/iio/adc/ad7280a.c 
b/drivers/staging/iio/adc/ad7280a.c
index 58420dcb406d..a4b4f8678c56 100644
--- a/drivers/staging/iio/adc/ad7280a.c
+++ b/drivers/staging/iio/adc/ad7280a.c
@@ -750,14 +750,14 @@ static irqreturn_t ad7280_event_handler(int irq, void 
*private)
 }
 
 static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
-   in_voltage-voltage_thresh_low_value,
+   in_voltage - voltage_thresh_low_value,
0644,
ad7280_read_channel_config,
ad7280_write_channel_config,
AD7280A_CELL_UNDERVOLTAGE);
 
 static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
-   in_voltage-voltage_thresh_high_value,
+   in_voltage - voltage_thresh_high_value,
0644,
ad7280_read_channel_config,
ad7280_write_channel_config,
-- 
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: iio: adc: add missing spaces around minus sign

2018-10-01 Thread Slawomir Stepien
On paź 01, 2018 14:51, Peter Meerwald-Stadler wrote:
> 
> > The checkpatch.pl tool detected coding style problem:
> > 
> > CHECK: spaces preferred around that '-' (ctx:VxV)
> 
> oh no, not again
> please see e.g. https://lore.kernel.org/patchwork/patch/635994/
> for some discussion why this patch doesn't make sense

Oh...my mistake. Thank you for clarification. I should be more careful. Sorry!

> > in two files inside the adc directory. This patch will remove this
> > problem.
> > 
> > Signed-off-by: Slawomir Stepien 
> > ---
> >  drivers/staging/iio/adc/ad7192.c  | 2 +-
> >  drivers/staging/iio/adc/ad7280a.c | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/adc/ad7192.c 
> > b/drivers/staging/iio/adc/ad7192.c
> > index acdbc07fd259..7c632cf1932b 100644
> > --- a/drivers/staging/iio/adc/ad7192.c
> > +++ b/drivers/staging/iio/adc/ad7192.c
> > @@ -355,7 +355,7 @@ ad7192_show_scale_available(struct device *dev,
> >  }
> >  
> 
> >  static IIO_DEVICE_ATTR_NAMED(in_v_m_v_scale_available,
> > -in_voltage-voltage_scale_available,
> > +in_voltage - voltage_scale_available,
> >  0444, ad7192_show_scale_available, NULL, 0);
> >  
> >  static IIO_DEVICE_ATTR(in_voltage_scale_available, 0444,
> > diff --git a/drivers/staging/iio/adc/ad7280a.c 
> > b/drivers/staging/iio/adc/ad7280a.c
> > index 58420dcb406d..a4b4f8678c56 100644
> > --- a/drivers/staging/iio/adc/ad7280a.c
> > +++ b/drivers/staging/iio/adc/ad7280a.c
> > @@ -750,14 +750,14 @@ static irqreturn_t ad7280_event_handler(int irq, void 
> > *private)
> >  }
> >  
> >  static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > -   in_voltage-voltage_thresh_low_value,
> > +   in_voltage - voltage_thresh_low_value,
> > 0644,
> > ad7280_read_channel_config,
> > ad7280_write_channel_config,
> > AD7280A_CELL_UNDERVOLTAGE);
> >  
> >  static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> > -   in_voltage-voltage_thresh_high_value,
> > +   in_voltage - voltage_thresh_high_value,
> > 0644,
> > ad7280_read_channel_config,
> > ad7280_write_channel_config,
> > 
> 

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


[PATCH 0/2] Fix style problems in ad7150 driver

2018-10-01 Thread Slawomir Stepien
This series will fix simple style problem inside the ad7150 driver.

Slawomir Stepien (2):
  staging: iio: cdc: ad7150: use pointer to shorten the line length
  staging: iio: cdc: ad7150: fix misaligned lines

 drivers/staging/iio/cdc/ad7150.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

-- 
2.19.0

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


[PATCH 1/2] staging: iio: cdc: ad7150: use pointer to shorten the line length

2018-10-01 Thread Slawomir Stepien
By using the pointer to channel attribute, we can now make the lines
short enough to eliminate the checkpatch.pl problem:

CHECK: Alignment should match open parenthesis

Signed-off-by: Slawomir Stepien 
---
 drivers/staging/iio/cdc/ad7150.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index d16084d7068c..5b5b766a0405 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -102,18 +102,19 @@ static int ad7150_read_raw(struct iio_dev *indio_dev,
 {
int ret;
struct ad7150_chip_info *chip = iio_priv(indio_dev);
+   const int *channel = &chan->channel;
 
switch (mask) {
case IIO_CHAN_INFO_RAW:
ret = i2c_smbus_read_word_data(chip->client,
-   ad7150_addresses[chan->channel][0]);
+  ad7150_addresses[*channel][0]);
if (ret < 0)
return ret;
*val = swab16(ret);
return IIO_VAL_INT;
case IIO_CHAN_INFO_AVERAGE_RAW:
ret = i2c_smbus_read_word_data(chip->client,
-   ad7150_addresses[chan->channel][1]);
+  ad7150_addresses[*channel][1]);
if (ret < 0)
return ret;
*val = swab16(ret);
-- 
2.19.0

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


[PATCH 2/2] staging: iio: cdc: ad7150: fix misaligned lines

2018-10-01 Thread Slawomir Stepien
These lines were misaligned, but the checkpatch.pl didn't indicate them
as such.

Signed-off-by: Slawomir Stepien 
---
 drivers/staging/iio/cdc/ad7150.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 5b5b766a0405..bbbf05ca2624 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -183,8 +183,8 @@ static int ad7150_write_event_params(struct iio_dev 
*indio_dev,
case IIO_EV_TYPE_THRESH:
value = chip->threshold[rising][chan];
return i2c_smbus_write_word_data(chip->client,
-   ad7150_addresses[chan][3],
-   swab16(value));
+ad7150_addresses[chan][3],
+swab16(value));
case IIO_EV_TYPE_MAG_ADAPTIVE:
sens = chip->mag_sensitivity[rising][chan];
timeout = chip->mag_timeout[rising][chan];
-- 
2.19.0

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


Re: [PATCH 1/2] staging: iio: cdc: ad7150: use pointer to shorten the line length

2018-10-04 Thread Slawomir Stepien
On paź 04, 2018 17:21, Lars-Peter Clausen wrote:
> On 10/01/2018 09:33 PM, Slawomir Stepien wrote:
> >  drivers/staging/iio/cdc/ad7150.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/cdc/ad7150.c 
> > b/drivers/staging/iio/cdc/ad7150.c
> > index d16084d7068c..5b5b766a0405 100644
> > --- a/drivers/staging/iio/cdc/ad7150.c
> > +++ b/drivers/staging/iio/cdc/ad7150.c
> > @@ -102,18 +102,19 @@ static int ad7150_read_raw(struct iio_dev *indio_dev,
> >  {
> > int ret;
> > struct ad7150_chip_info *chip = iio_priv(indio_dev);
> > +   const int *channel = &chan->channel;
> 
> Instead of taking a pointer, just take the value of the channel index and
> use that. The generated code should be very similar but its a bit nicer to 
> read.

OK. I've been thinking about that for a moment, but in the end decided to go
with the pointer. I don't mind the value. I'll prepare v2. Thank you.

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


[PATCH v2 0/2] Fix style problems in ad7150 driver

2018-10-05 Thread Slawomir Stepien
This series will fix simple style problems inside the ad7150 driver.

---
Changes since v1:
- Use channel value copy rather than pointer - for better readability
- Change "problem" -> "problems" in cover letter description
---
Slawomir Stepien (2):
  staging: iio: cdc: ad7150: use value copy to shorten the line length
  staging: iio: cdc: ad7150: fix misaligned lines

 drivers/staging/iio/cdc/ad7150.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

-- 
2.19.0

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


[PATCH v2 1/2] staging: iio: cdc: ad7150: use value copy to shorten the line length

2018-10-05 Thread Slawomir Stepien
By using the copy of channel attribute, we can now make the lines short
enough to eliminate the checkpatch.pl problem:

CHECK: Alignment should match open parenthesis

Signed-off-by: Slawomir Stepien 
---
 drivers/staging/iio/cdc/ad7150.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index d16084d7068c..e2c70063fa0f 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -102,18 +102,19 @@ static int ad7150_read_raw(struct iio_dev *indio_dev,
 {
int ret;
struct ad7150_chip_info *chip = iio_priv(indio_dev);
+   int channel = chan->channel;
 
switch (mask) {
case IIO_CHAN_INFO_RAW:
ret = i2c_smbus_read_word_data(chip->client,
-   ad7150_addresses[chan->channel][0]);
+  ad7150_addresses[channel][0]);
if (ret < 0)
return ret;
*val = swab16(ret);
return IIO_VAL_INT;
case IIO_CHAN_INFO_AVERAGE_RAW:
ret = i2c_smbus_read_word_data(chip->client,
-   ad7150_addresses[chan->channel][1]);
+  ad7150_addresses[channel][1]);
if (ret < 0)
return ret;
*val = swab16(ret);
-- 
2.19.0

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


[PATCH v2 2/2] staging: iio: cdc: ad7150: fix misaligned lines

2018-10-05 Thread Slawomir Stepien
These lines were misaligned, but the checkpatch.pl didn't indicate them
as such.

Signed-off-by: Slawomir Stepien 
---
 drivers/staging/iio/cdc/ad7150.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index e2c70063fa0f..24f74ce60f80 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -183,8 +183,8 @@ static int ad7150_write_event_params(struct iio_dev 
*indio_dev,
case IIO_EV_TYPE_THRESH:
value = chip->threshold[rising][chan];
return i2c_smbus_write_word_data(chip->client,
-   ad7150_addresses[chan][3],
-   swab16(value));
+ad7150_addresses[chan][3],
+swab16(value));
case IIO_EV_TYPE_MAG_ADAPTIVE:
sens = chip->mag_sensitivity[rising][chan];
timeout = chip->mag_timeout[rising][chan];
-- 
2.19.0

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


Re: [PATCH] staging: iio: adc: ad7280a: fix 2 checks

2018-10-06 Thread Slawomir Stepien
On paź 06, 2018 13:27, Gabriel Capella wrote:
> This patch does not change the logic, it only
> corrects the checkpatch checks.
> 
> The patch fixes 2 checks of type:
> "CHECK: spaces preferred around that '-'"

I've made the same mistake few days ago. This change is incorrect.
Please see: https://lore.kernel.org/patchwork/patch/635994/.

> Signed-off-by: Gabriel Capella 
> ---
>  drivers/staging/iio/adc/ad7280a.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7280a.c 
> b/drivers/staging/iio/adc/ad7280a.c
> index 58420dcb406d..a4b4f8678c56 100644
> --- a/drivers/staging/iio/adc/ad7280a.c
> +++ b/drivers/staging/iio/adc/ad7280a.c
> @@ -750,14 +750,14 @@ static irqreturn_t ad7280_event_handler(int irq, void 
> *private)
>  }
>  
>  static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> - in_voltage-voltage_thresh_low_value,
> + in_voltage - voltage_thresh_low_value,
>   0644,
>   ad7280_read_channel_config,
>   ad7280_write_channel_config,
>   AD7280A_CELL_UNDERVOLTAGE);
>  
>  static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> - in_voltage-voltage_thresh_high_value,
> + in_voltage - voltage_thresh_high_value,
>       0644,
>   ad7280_read_channel_config,
>   ad7280_write_channel_config,

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


Re: [PATCH] staging: iio: adc: ad7280a: fix 2 checks

2018-10-11 Thread Slawomir Stepien
On paź 11, 2018 10:32, Dan Carpenter wrote:
> On Sat, Oct 06, 2018 at 10:25:42PM +0200, Slawomir Stepien wrote:
> > On paź 06, 2018 13:27, Gabriel Capella wrote:
> > > This patch does not change the logic, it only
> > > corrects the checkpatch checks.
> > > 
> > > The patch fixes 2 checks of type:
> > > "CHECK: spaces preferred around that '-'"
> > 
> > I've made the same mistake few days ago. This change is incorrect.
> > Please see: https://lore.kernel.org/patchwork/patch/635994/.
> >
> 
> You could add a comment.  /* do not put spaces around the hyphen.  
> #checkpatch.pl */
> 
> These are the only places which use this style and a lot of people
> bump into it.

Gabriel go ahead if you want to. If not then I would be happy to prepare this
patch.

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


Re: [PATCH] staging: iio: adc: Add comments to prevent checks corrections

2018-10-16 Thread Slawomir Stepien
On paź 15, 2018 18:25, Gabriel Capella wrote:
> This patch adds 3 comments in 2 different files.
> These comments warn to don't correct the checks of type:
> "CHECK: spaces preferred around that '-'"
> 
> Signed-off-by: Gabriel Capella 
> ---
>  drivers/staging/iio/adc/ad7192.c  | 1 +
>  drivers/staging/iio/adc/ad7280a.c | 2 ++
>  2 files changed, 3 insertions(+)

I have this simpler solution...let's focus our efforts on moving the two drivers
out of staging. This will prevent the CHK to appear:

Cut from checkpatch.pl:

if ($realfile =~ m@^(?:drivers/net/|net/|drivers/staging/)@) {
$check = 1;

Existing driver out of staging, with this "problem":
$ ./scripts/checkpatch.pl --types SPACING --file drivers/iio/adc/ad7793.c   
 
total: 0 errors, 0 warnings, 827 lines checked

drivers/iio/adc/ad7793.c has no obvious style problems and is ready for 
submission.

NOTE: Used message types: SPACING

In my opinion it would stop this incorrect patches.

I have also this changes to checkpatch.pl:
https://github.com/s-stepien/linux-1/commit/c976a31b392393fb417f2d9e2ec9639bc226ad0b
and usage:
https://github.com/s-stepien/linux-1/commit/1adc0428b496f44f6a931637084bb619ddd9992d
but I'm not that sure it is the best way to go.

What do you all think?

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


Re: [PATCH] staging: iio: ad2s1210: Switch to the gpio descriptor interface

2018-10-21 Thread Slawomir Stepien

>  {
> - unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
> - struct gpio ad2s1210_gpios[] = {
> - { st->pdata->sample, GPIOF_DIR_IN, "sample" },
> - { st->pdata->a[0], flags, "a0" },
> - { st->pdata->a[1], flags, "a1" },
> - { st->pdata->res[0], flags, "res0" },
> - { st->pdata->res[0], flags, "res1" },
> - };
> + int ret = 0;
> + unsigned long flags = st->pdata->gpioin ? GPIOD_IN : GPIOD_OUT_LOW;
> +
> + st->sample = devm_gpiod_get(&spi->dev, "sample", GPIOD_IN);
> + if (IS_ERR(st->sample)) {
> + ret = PTR_ERR(st->sample);
> + dev_err(&spi->dev, "Failed to request sample GPIO: %d\n",
> + ret);
> + return ret;
> + }
> + st->a0 = devm_gpiod_get(&spi->dev, "a0", flags);
> + if (IS_ERR(st->a0)) {
> + ret = PTR_ERR(st->a0);
> + dev_err(&spi->dev, "Failed to request a0 GPIO: %d\n",
> + ret);
> + return ret;
> + }
> + st->a1 = devm_gpiod_get(&spi->dev, "a1", flags);
> + if (IS_ERR(st->a1)) {
> + ret = PTR_ERR(st->a1);
> + dev_err(&spi->dev, "Failed to request a1 GPIO: %d\n",
> + ret);
> + return ret;
> + }
> + st->res0 = devm_gpiod_get(&spi->dev, "res0", flags);
> + if (IS_ERR(st->res0)) {
> + ret = PTR_ERR(st->res0);
> + dev_err(&spi->dev, "Failed to request res0 GPIO: %d\n",
> + ret);
> + return ret;
> + }
> + st->res1 = devm_gpiod_get(&spi->dev, "res1", flags);
> + if (IS_ERR(st->res1)) {
> + ret = PTR_ERR(st->res1);
> + dev_err(&spi->dev, "Failed to request res1 GPIO: %d\n",
> + ret);
> + return ret;
> + }

To reduce the redundant code here, create an array of structs. The struct will
have a pointer to gpio_desc, name of the gpio and the gpios' flags. Then you can
use for loop to get all the needed gpios:

for (l = 0; l < ARRAY_SIZE(str); l++) {
str[l].ptr = devm_gpiod_get(&spi->dev, str[l].name, str[l].flags);
if (IS_ERR(str[l].res1)) {
...
}
}

> - return gpio_request_array(ad2s1210_gpios, ARRAY_SIZE(ad2s1210_gpios));
> + return ret;
>  }
>  
> -static void ad2s1210_free_gpios(struct ad2s1210_state *st)
> +static void ad2s1210_free_gpios(struct spi_device *spi,
> + struct ad2s1210_state *st)
>  {
> - unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
> - struct gpio ad2s1210_gpios[] = {
> - { st->pdata->sample, GPIOF_DIR_IN, "sample" },
> - { st->pdata->a[0], flags, "a0" },
> - { st->pdata->a[1], flags, "a1" },
> - { st->pdata->res[0], flags, "res0" },
> - { st->pdata->res[0], flags, "res1" },
> - };
> -
> - gpio_free_array(ad2s1210_gpios, ARRAY_SIZE(ad2s1210_gpios));
> + devm_gpiod_put(&spi->dev, st->sample);
> + devm_gpiod_put(&spi->dev, st->a0);
> + devm_gpiod_put(&spi->dev, st->a1);
> + devm_gpiod_put(&spi->dev, st->res0);
> + devm_gpiod_put(&spi->dev, st->res1);
>  }

This whole function ad2s1210_free_gpios is not needed anymore because you are
using the devm API (it's called from probe and remove, so you are safe).

>  static int ad2s1210_probe(struct spi_device *spi)
> @@ -670,7 +702,7 @@ static int ad2s1210_probe(struct spi_device *spi)
>   return -ENOMEM;
>   st = iio_priv(indio_dev);
>   st->pdata = spi->dev.platform_data;
> - ret = ad2s1210_setup_gpios(st);
> + ret = ad2s1210_setup_gpios(spi, st);
>   if (ret < 0)
>   return ret;
>  
> @@ -702,7 +734,7 @@ static int ad2s1210_probe(struct spi_device *spi)
>   return 0;
>  
>  error_free_gpios:
> - ad2s1210_free_gpios(st);
> + ad2s1210_free_gpios(spi, st);
>   return ret;
>  }
>  
> @@ -711,7 +743,7 @@ static int ad2s1210_remove(struct spi_device *spi)
>   struct iio_dev *indio_dev = spi_get_drvdata(spi);
>  
>   iio_device_unregister(indio_dev);
> - ad2s1210_free_gpios(iio_priv(indio_dev));
> + ad2s1210_free_gpios(spi, iio_priv(indio_dev));
>  
>   return 0;
>  }
> diff --git a/drivers/staging/iio/resolver/ad2s1210.h 
> b/drivers/staging/iio/resolver/ad2s1210.h
> index e9b2147701fc..63d479b20a6c 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.h
> +++ b/drivers/staging/iio/resolver/ad2s1210.h
> @@ -12,9 +12,6 @@
>  #define _AD2S1210_H
>  
>  struct ad2s1210_platform_data {
> - unsigned intsample;
> - unsigned inta[2];
> - unsigned intres[2];
>   boolgpioin;
>  };
>  #endif /* _AD2S1210_H */

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


Re: [PATCH] staging: iio: ad2s1210: Switch to the gpio descriptor interface

2018-10-21 Thread Slawomir Stepien
On paź 21, 2018 10:31, Slawomir Stepien wrote:
> On paź 21, 2018 11:49, Nishad Kamdar wrote:
> > -static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
> > +static int ad2s1210_setup_gpios(struct spi_device *spi,
> > +   struct ad2s1210_state *st)
> 
> This change is not needed. The st has the spi_device inside. Use 
> container_of()
> macro here.

Wait...what? I mean use direct access using st->sdev.

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


Re: [PATCH v2] staging: iio: ad2s1210: Switch to the gpio descriptor interface

2018-10-23 Thread Slawomir Stepien
-512,7 +523,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
>   }
>  
>  error_ret:
> - gpio_set_value(st->pdata->sample, 1);
> + gpiod_set_value(st->sample, 1);
>   /* delay (2 * tck + 20) nano seconds */
>   udelay(1);
>   mutex_unlock(&st->lock);
> @@ -630,30 +641,30 @@ static const struct iio_info ad2s1210_info = {
>  
>  static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
>  {
> - unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
> - struct gpio ad2s1210_gpios[] = {
> - { st->pdata->sample, GPIOF_DIR_IN, "sample" },
> - { st->pdata->a[0], flags, "a0" },
> - { st->pdata->a[1], flags, "a1" },
> - { st->pdata->res[0], flags, "res0" },
> - { st->pdata->res[0], flags, "res1" },
> + int ret = 0, i = 0;

You do not need to init that i with 0.
Also the ret do not have to be 0. If you return just 0 at the on this function.

> + struct spi_device *spi = st->sdev;
> + unsigned long flags = st->pdata->gpioin ? GPIOD_IN : GPIOD_OUT_LOW;
> +
> + struct ad2s1210_gpio gpios[] = {
> + {st->sample, "sample", GPIOD_IN},
> + {st->a0, "a0", flags},
> + {st->a1, "a1", flags},
> + {st->res0, "res0", flags},
> + {st->res1, "res1", flags},

To change a pointer, you need to pass an address of that pointer.
I think you should you . notation when initializing the struct.

>   };
>  
> - return gpio_request_array(ad2s1210_gpios, ARRAY_SIZE(ad2s1210_gpios));
> -}
> -
> -static void ad2s1210_free_gpios(struct ad2s1210_state *st)
> -{
> - unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
> - struct gpio ad2s1210_gpios[] = {
> - { st->pdata->sample, GPIOF_DIR_IN, "sample" },
> - { st->pdata->a[0], flags, "a0" },
> - { st->pdata->a[1], flags, "a1" },
> - { st->pdata->res[0], flags, "res0" },
> - { st->pdata->res[0], flags, "res1" },
> - };
> + for (i = 0; i < ARRAY_SIZE(gpios); i++) {

I do not know if that is a common practise, but you can set a pointer to gpio[i]
as the first instruction in this for, so you would not have to write this whole
gpio[i]:

pin = &gpios[i]

and then: pin->ptr, etc.

> + gpios[i].ptr = devm_gpiod_get(&spi->dev, gpios[i].name,
> +   gpios[i].flags);
> + if (IS_ERR(gpios[i].ptr)) {
> + ret = PTR_ERR(gpios[i].ptr);
> + dev_err(&spi->dev, "Failed to request %s GPIO: %d\n",
> + gpios[i].name, ret);
> + return ret;
> + }
> + }
>  
> - gpio_free_array(ad2s1210_gpios, ARRAY_SIZE(ad2s1210_gpios));
> + return ret;
>  }
>  
>  static int ad2s1210_probe(struct spi_device *spi)
> @@ -692,18 +703,13 @@ static int ad2s1210_probe(struct spi_device *spi)
>  
>   ret = iio_device_register(indio_dev);
>   if (ret)
> - goto error_free_gpios;
> -

Removing a empty line here, that is here to make it more readable.

> + return ret;
>   st->fclkin = spi->max_speed_hz;
>   spi->mode = SPI_MODE_3;
>   spi_setup(spi);
>   ad2s1210_initial(st);
>  
>   return 0;
> -
> -error_free_gpios:
> - ad2s1210_free_gpios(st);
> - return ret;
>  }
>  
>  static int ad2s1210_remove(struct spi_device *spi)
> @@ -711,7 +717,6 @@ static int ad2s1210_remove(struct spi_device *spi)
>   struct iio_dev *indio_dev = spi_get_drvdata(spi);
>  
>   iio_device_unregister(indio_dev);
> - ad2s1210_free_gpios(iio_priv(indio_dev));
>  
>   return 0;
>  }
> diff --git a/drivers/staging/iio/resolver/ad2s1210.h 
> b/drivers/staging/iio/resolver/ad2s1210.h
> index e9b2147701fc..63d479b20a6c 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.h
> +++ b/drivers/staging/iio/resolver/ad2s1210.h
> @@ -12,9 +12,6 @@
>  #define _AD2S1210_H
>  
>  struct ad2s1210_platform_data {
> - unsigned intsample;
> - unsigned inta[2];
> - unsigned intres[2];
>   boolgpioin;
>  };
>  #endif /* _AD2S1210_H */

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


Re: [PATCH v3] staging: iio: ad2s1210: Switch to the gpio descriptor interface

2018-10-23 Thread Slawomir Stepien
gt;sample, 0);
>   /* delay (6 * tck + 20) nano seconds */
>   udelay(1);
>  
> @@ -512,7 +523,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
>   }
>  
>  error_ret:
> - gpio_set_value(st->pdata->sample, 1);
> + gpiod_set_value(st->sample, 1);
>   /* delay (2 * tck + 20) nano seconds */
>   udelay(1);
>   mutex_unlock(&st->lock);
> @@ -630,30 +641,32 @@ static const struct iio_info ad2s1210_info = {
>  
>  static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
>  {
> - unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
> - struct gpio ad2s1210_gpios[] = {
> - { st->pdata->sample, GPIOF_DIR_IN, "sample" },
> - { st->pdata->a[0], flags, "a0" },
> - { st->pdata->a[1], flags, "a1" },
> - { st->pdata->res[0], flags, "res0" },
> - { st->pdata->res[0], flags, "res1" },
> + int ret, i;
> + struct spi_device *spi = st->sdev;
> + struct ad2s1210_gpio *pin;
> + unsigned long flags = st->pdata->gpioin ? GPIOD_IN : GPIOD_OUT_LOW;
> +
> + struct ad2s1210_gpio gpios[] = {
> + {.ptr = &st->sample, .name = "sample", .flags = GPIOD_IN},
> + {.ptr = &st->a0, .name = "a0", .flags = flags},
> + {.ptr = &st->a1, .name = "a1", .flags = flags},
> + {.ptr = &st->res0, .name = "res0", .flags = flags},
> + {.ptr = &st->res1, .name = "res1", .flags = flags},
>   };

I think that you should add spaces after { and before }. It's the file style
from what I see.

>  
> - return gpio_request_array(ad2s1210_gpios, ARRAY_SIZE(ad2s1210_gpios));
> -}
> -
> -static void ad2s1210_free_gpios(struct ad2s1210_state *st)
> -{
> - unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
> - struct gpio ad2s1210_gpios[] = {
> - { st->pdata->sample, GPIOF_DIR_IN, "sample" },
> - { st->pdata->a[0], flags, "a0" },
> - { st->pdata->a[1], flags, "a1" },
> - { st->pdata->res[0], flags, "res0" },
> - { st->pdata->res[0], flags, "res1" },
> - };
> + for (i = 0; i < ARRAY_SIZE(gpios); i++) {
> + pin = &gpios[i];
> + *pin->ptr = devm_gpiod_get(&spi->dev, pin->name,
> +pin->flags);

This can now fit into one line.

> + if (IS_ERR(pin->ptr)) {

Here you are checking the wrong pointer. Also notice the line below.

> + ret = PTR_ERR(pin->ptr);
> + dev_err(&spi->dev, "Failed to request %s GPIO: %d\n",
> + pin->name, ret);

Please notice how other dev_err looks like in this code. Align the message to
the existing format.

> + return ret;
> + }
> + }
>  
> - gpio_free_array(ad2s1210_gpios, ARRAY_SIZE(ad2s1210_gpios));
> + return 0;
>  }
>  
>  static int ad2s1210_probe(struct spi_device *spi)
> @@ -692,7 +705,7 @@ static int ad2s1210_probe(struct spi_device *spi)
>  
>   ret = iio_device_register(indio_dev);
>   if (ret)
> - goto error_free_gpios;
> + return ret;
>  
>   st->fclkin = spi->max_speed_hz;
>   spi->mode = SPI_MODE_3;
> @@ -700,10 +713,6 @@ static int ad2s1210_probe(struct spi_device *spi)
>   ad2s1210_initial(st);
>  
>   return 0;
> -
> -error_free_gpios:
> - ad2s1210_free_gpios(st);
> - return ret;
>  }
>  
>  static int ad2s1210_remove(struct spi_device *spi)
> @@ -711,7 +720,6 @@ static int ad2s1210_remove(struct spi_device *spi)
>   struct iio_dev *indio_dev = spi_get_drvdata(spi);
>  
>   iio_device_unregister(indio_dev);
> - ad2s1210_free_gpios(iio_priv(indio_dev));
>  
>   return 0;
>  }
> diff --git a/drivers/staging/iio/resolver/ad2s1210.h 
> b/drivers/staging/iio/resolver/ad2s1210.h
> index e9b2147701fc..63d479b20a6c 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.h
> +++ b/drivers/staging/iio/resolver/ad2s1210.h
> @@ -12,9 +12,6 @@
>  #define _AD2S1210_H
>  
>  struct ad2s1210_platform_data {
> - unsigned intsample;
> - unsigned inta[2];
> - unsigned intres[2];
>   boolgpioin;
>  };
>  #endif /* _AD2S1210_H */

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


Re: [PATCH v4] staging: iio: ad2s1210: Switch to the gpio descriptor interface

2018-10-24 Thread Slawomir Stepien
On paź 24, 2018 20:20, Nishad Kamdar wrote:
> Use the gpiod interface instead of the deprecated old non-descriptor
> interface.
> 
> Signed-off-by: Nishad Kamdar 
> ---
> Changes in v4:
>  - Add spaces after { and before } in gpios[]
>initialization.
>  - Check the correct pointer for error.
>  - Align the dev_err msg to existing format in the code.
> Changes in v3:
>  - Use a pointer to pointer for gpio_desc in
>struct ad2s1210_gpio as it will be used to
>modify a pointer.
>  - Use dot notation to initialize the structure.
>  - Use a pointer variable to avoid writing gpios[i].
> Changes in v2:
>  - Use the spi_device struct embedded in st instead
>of passing it as an argument to ad2s1210_setup_gpios().
>  - Use an array of structs to reduce redundant code in
>in ad2s1210_setup_gpios().
>  - Remove ad2s1210_free_gpios() as devm API is being used.
> ---
>  drivers/staging/iio/resolver/ad2s1210.c | 92 ++---
>  drivers/staging/iio/resolver/ad2s1210.h |  3 -
>  2 files changed, 50 insertions(+), 45 deletions(-)

Looks good to me.

Reviewed-by: Slawomir Stepien 

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


Re: [PATCH v5 2/2] staging: iio: ad2s1210: Add device tree support.

2018-10-27 Thread Slawomir Stepien
Hi

On paź 26, 2018 18:55, Nishad Kamdar wrote:
> Add device tree table for matching vendor ID
> and support for retrieving platform data
> from device tree.

So maybe you should make 2 commits?

> Signed-off-by: Nishad Kamdar 
> ---
>  drivers/staging/iio/resolver/ad2s1210.c | 43 -
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c 
> b/drivers/staging/iio/resolver/ad2s1210.c
> index 93c3c70ce62e..9fd5461c4ed0 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -669,6 +670,27 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state 
> *st)
>   return 0;
>  }
>  
> +#ifdef CONFIG_OF
> +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device *dev)
> +{
> + struct device_node *np = dev->of_node;
> + struct ad2s1210_platform_data *pdata;
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return NULL;
> +
> + pdata->gpioin = of_property_read_bool(np, "adi,gpioin");

Why here you are adding "adi", but you are not adding it to the gpios in prev
commit?

I've also seen this: https://patchwork.kernel.org/patch/10355839/. However I do
not understand why adding vendor id to props is needed...

> +
> + return pdata;
> +}
> +#else
> +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device *dev)
> +{
> + return NULL;
> +}
> +#endif
> +
>  static int ad2s1210_probe(struct spi_device *spi)
>  {
>   struct iio_dev *indio_dev;
> @@ -682,7 +704,19 @@ static int ad2s1210_probe(struct spi_device *spi)
>   if (!indio_dev)
>   return -ENOMEM;
>   st = iio_priv(indio_dev);
> - st->pdata = spi->dev.platform_data;
> + if (spi->dev.of_node) {
> + st->pdata = ad2s1210_parse_dt(&spi->dev);
> + if (!st->pdata)
> + return -EINVAL;
> + } else {
> + st->pdata = spi->dev.platform_data;
> + }
> +
> + if (!st->pdata) {
> + dev_err(&spi->dev, "ad2s1210: no platform data supplied\n");
> + return -EINVAL;
> + }
> +

Why not just use only device-tree here? The ad2s1210_platform_data has now only
one entry... In that case remember to add "depends on OF" in Kconfig.

>   ret = ad2s1210_setup_gpios(st);
>   if (ret < 0)
>   return ret;
> @@ -724,6 +758,12 @@ static int ad2s1210_remove(struct spi_device *spi)
>   return 0;
>  }
>  
> +static const struct of_device_id ad2s1210_of_match[] = {
> + { .compatible = "adi,ad2s1210", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ad2s1210_of_match);
> +
>  static const struct spi_device_id ad2s1210_id[] = {
>   { "ad2s1210" },
>   {}
> @@ -733,6 +773,7 @@ MODULE_DEVICE_TABLE(spi, ad2s1210_id);
>  static struct spi_driver ad2s1210_driver = {
>   .driver = {
>   .name = DRV_NAME,
> + .of_match_table = of_match_ptr(ad2s1210_of_match),
>   },
>   .probe = ad2s1210_probe,
>   .remove = ad2s1210_remove,

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


Re: [PATCH] staging: iio: ad5933: finalized device-tree support

2018-11-24 Thread Slawomir Stepien
On lis 23, 2018 21:51, Marcelo Schmitt wrote:
> Added a of_device_id struct variable and subsequent call to
> MODULE_DEVICE_TABLE macro to complete device-tree support for this
> driver.
> 
> Signed-off-by: Marcelo Schmitt 
> ---
>  drivers/staging/iio/impedance-analyzer/ad5933.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c 
> b/drivers/staging/iio/impedance-analyzer/ad5933.c
> index edb8b540bbf1..19e8b6d2865c 100644
> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> @@ -771,9 +771,18 @@ static const struct i2c_device_id ad5933_id[] = {
>  
>  MODULE_DEVICE_TABLE(i2c, ad5933_id);
>  
> +static const struct of_device_id ad5933_of_match[] = {
> + { .compatible = "analog-devices,ad5933" },
> + { .compatible = "analog-devices,ad5934" },

Why "analog-devices", rather than "adi"? My understanding is that analog devices
have "adi" as a manufacture part in compatible.

> + { },
> +};
> +
> +MODULE_DEVICE_TABLE(of, ad5933_of_match);
> +
>  static struct i2c_driver ad5933_driver = {
>   .driver = {
>   .name = "ad5933",
> + .of_match_table = ad5933_of_match,
>   },
>   .probe = ad5933_probe,
>   .remove = ad5933_remove,

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


[RFC] Resolving "false positive" error message from checkpatch.pl

2017-02-03 Thread Slawomir Stepien
Hi all

There is a "false positive" error reported by checkpatch.pl:

ERROR: Use 4 digit octal (0777) not decimal permissions
#272: FILE: drivers/staging/iio/meter/ade7759.c:272:
+static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO,
+   ade7759_read_8bit,
+   ade7759_write_8bit,
+   ADE7759_CH1OS);

ERROR: Use 4 digit octal (0777) not decimal permissions
#276: FILE: drivers/staging/iio/meter/ade7759.c:276:
+static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO,
+   ade7759_read_8bit,
+   ade7759_write_8bit,
+   ADE7759_CH2OS);

The same for the file drivers/staging/iio/meter/ade7753.c.

We can see that this macro is matched by the pattern "IIO_DEV_ATTR_[A-Z_]+" from
@mode_permission_funcs in checkpatch.pl.

My question is: how this should be fixed?

>From what I can say, it's a false positive so the correct way to fix it is to
change the matching pattern: "IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+".

Or maybe the fix should be done in iio code - argument position inside this
macro?

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


Re: [RFC] Resolving "false positive" error message from checkpatch.pl

2017-02-03 Thread Slawomir Stepien
On Feb 03, 2017 11:27, Greg KH wrote:
> On Fri, Feb 03, 2017 at 11:11:35AM +0100, Slawomir Stepien wrote:
> > Hi all
> > 
> > There is a "false positive" error reported by checkpatch.pl:
> > 
> > ERROR: Use 4 digit octal (0777) not decimal permissions
> > #272: FILE: drivers/staging/iio/meter/ade7759.c:272:
> > +static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO,
> > +   ade7759_read_8bit,
> > +   ade7759_write_8bit,
> > +   ADE7759_CH1OS);
> > 
> > ERROR: Use 4 digit octal (0777) not decimal permissions
> > #276: FILE: drivers/staging/iio/meter/ade7759.c:276:
> > +static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO,
> > +   ade7759_read_8bit,
> > +   ade7759_write_8bit,
> > +   ADE7759_CH2OS);
> > 
> > The same for the file drivers/staging/iio/meter/ade7753.c.
> > 
> > We can see that this macro is matched by the pattern "IIO_DEV_ATTR_[A-Z_]+" 
> > from
> > @mode_permission_funcs in checkpatch.pl.
> > 
> > My question is: how this should be fixed?
> 
> Why do you think this is a false-positive?

Because the 1st arg of that macro is not supposed to be a permissions flags.

> > From what I can say, it's a false positive so the correct way to fix it is 
> > to
> > change the matching pattern: "IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+".
> 
> Huh?
> 
> Provide a patch to show what you are suggesting please.

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8e96af53611c..0a1c6ec86caa 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -525,7 +525,7 @@ our @mode_permission_funcs = (

["debugfs_create_(?:file|u8|u16|u32|u64|x8|x16|x32|x64|size_t|atomic_t|bool|blob|regset32|u32_array)",
 2],
["proc_create(?:_data|)", 2],
["(?:CLASS|DEVICE|SENSOR|SENSOR_DEVICE|IIO_DEVICE)_ATTR", 2],
-   ["IIO_DEV_ATTR_[A-Z_]+", 1],
+   ["IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+", 1],
["SENSOR_(?:DEVICE_|)ATTR_2", 2],
["SENSOR_TEMPLATE(?:_2|)", 3],
["__ATTR", 2],
-- 
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC] Resolving "false positive" error message from checkpatch.pl

2017-02-03 Thread Slawomir Stepien
On Feb 03, 2017 04:39, Joe Perches wrote:
> On Fri, 2017-02-03 at 13:10 +0100, Slawomir Stepien wrote:
> > On Feb 03, 2017 11:27, Greg KH wrote:
> > > On Fri, Feb 03, 2017 at 11:11:35AM +0100, Slawomir Stepien wrote:
> > > > Hi all
> > > > 
> > > > There is a "false positive" error reported by checkpatch.pl:
> > > > 
> > > > ERROR: Use 4 digit octal (0777) not decimal permissions
> > > > #272: FILE: drivers/staging/iio/meter/ade7759.c:272:
> > > > +static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO,
> > > > +   ade7759_read_8bit,
> > > > +   ade7759_write_8bit,
> > > > +   ADE7759_CH1OS);
> > > > 
> > > > ERROR: Use 4 digit octal (0777) not decimal permissions
> > > > #276: FILE: drivers/staging/iio/meter/ade7759.c:276:
> > > > +static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO,
> > > > +   ade7759_read_8bit,
> > > > +   ade7759_write_8bit,
> > > > +   ADE7759_CH2OS);
> > > > 
> > > > The same for the file drivers/staging/iio/meter/ade7753.c.
> > > > 
> > > > We can see that this macro is matched by the pattern 
> > > > "IIO_DEV_ATTR_[A-Z_]+" from
> > > > @mode_permission_funcs in checkpatch.pl.
> > > > 
> > > > My question is: how this should be fixed?
> > > 
> > > Why do you think this is a false-positive?
> > 
> > Because the 1st arg of that macro is not supposed to be a permissions flags.
> > 
> > > > From what I can say, it's a false positive so the correct way to fix it 
> > > > is to
> > > > change the matching pattern: "IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+".
> > > 
> > > Huh?
> > > 
> > > Provide a patch to show what you are suggesting please.
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 8e96af53611c..0a1c6ec86caa 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -525,7 +525,7 @@ our @mode_permission_funcs = (
> > 
> > ["debugfs_create_(?:file|u8|u16|u32|u64|x8|x16|x32|x64|size_t|atomic_t|bool|blob|regset32|u32_array)",
> >  2],
> > ["proc_create(?:_data|)", 2],
> > ["(?:CLASS|DEVICE|SENSOR|SENSOR_DEVICE|IIO_DEVICE)_ATTR", 2],
> > -   ["IIO_DEV_ATTR_[A-Z_]+", 1],
> > +   ["IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+", 1],
> > ["SENSOR_(?:DEVICE_|)ATTR_2", 2],
> > ["SENSOR_TEMPLATE(?:_2|)", 3],
> > ["__ATTR", 2],
> 
> There are many local MODE_DEV_ATTR_ functions
> that don't have permission in as the first argument.

Yeah...I just found the ones in the drivers/staging/iio/frequency that you
pointed out.

> $ grep -rPoh --include=*.[ch] "\bIIO_DEV_ATTR_[A-Z_]+\b" *|sort|uniq|wc -l
> 123
> 
> Maybe it'd be better to list the ones that do
> or to change the argument order of the defines
> and uses so mode _is_ the first argument.

That is my original question. What would be better here (especially that no we
can see that there are few more places where the error will pop)?

> $ grep -rPoh --include=*.[ch] "\bIIO_DEV_ATTR_[A-Z_]+\b" *|sort|uniq| \
>   while read line ; do git grep -w $line | grep -w define ; done|grep _mode | 
> grep -v "(_mode"
> drivers/staging/iio/meter/meter.h:#define IIO_DEV_ATTR_CH_OFF(_num, _mode, 
> _show, _store, _addr)  \
> drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_FREQ(_channel, _num, 
> _mode, _show, _store, _addr)\
> drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_FREQSYMBOL(_channel, 
> _mode, _show, _store, _addr)\
> drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_OUT_ENABLE(_channel, 
> _mode, _show, _store, _addr)\
> drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_PHASE(_channel, 
> _num, _mode, _show, _store, _addr)   \
> drivers/staging/iio/frequency/dds.h:#define 
> IIO_DEV_ATTR_PHASESYMBOL(_channel, _mode, _show, _store, _addr)   \
> drivers/staging/iio/frequency/dds.h:#define 
> IIO_DEV_ATTR_PINCONTROL_EN(_channel, _mode, _show, _store, _addr)\
> drivers/staging/iio/frequency/dds.h:#define 
> IIO_DEV_ATTR_PINCONTROL_FREQ_EN(_channel, _mode, _show, _store, _addr)\
> drivers/staging/iio/frequency/dds.h:#define 
> IIO_DEV_ATTR_PINCONTROL_PHASE_EN(_channel, _mode, _show, _store, _addr)\

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