[PATCH v3 1/3] Staging: speakup: speakup_keypc.c: usleep_range is preferred over udelay

[PATCH v3 1/3] Staging: speakup: speakup_keypc.c: usleep_range is preferred over udelay

[Jaya Durga]

Fix checkpatch issue: CHECK: usleep_range is preferred over udelay;

Signed-off-by: Jaya Durga <rjdurga@gmail.com>
---
 drivers/staging/speakup/speakup_keypc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/speakup/speakup_keypc.c b/drivers/staging/speakup/speakup_keypc.c
index d3203f8..1ba4cfc 100644
--- a/drivers/staging/speakup/speakup_keypc.c
+++ b/drivers/staging/speakup/speakup_keypc.c
@@ -28,7 +28,7 @@
 
 #define DRV_VERSION "2.10"
 #define SYNTH_IO_EXTENT	0x04
-#define SWAIT udelay(70)
+#define SWAIT usleep_range(70, 150)
 #define PROCSPEECH 0x1f
 #define SYNTH_CLEAR 0x03
 
@@ -164,7 +164,7 @@ static const char *synth_immediate(struct spk_synth *synth, const char *buf)
 			if (--timeout <= 0)
 				return oops();
 		outb_p(ch, synth_port);
-		udelay(70);
+		usleep_range(70, 150);
 		buf++;
 	}
 	return NULL;
-- 
1.9.1

[PATCH v3 2/3] Staging: iio: light: tsl2x7x_core.c: Replace symbolic permission with octal permission

[Jaya Durga]

Replace symbolic permissions with their
octect representation to fix checkpatch warnings.

Signed-off-by: Jaya Durga <rjdurga@gmail.com>
---
 drivers/staging/iio/light/tsl2x7x_core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x_core.c b/drivers/staging/iio/light/tsl2x7x_core.c
index af3910b..c63fe6a 100644
--- a/drivers/staging/iio/light/tsl2x7x_core.c
+++ b/drivers/staging/iio/light/tsl2x7x_core.c
@@ -1498,34 +1498,34 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
 	return 0;
 }
 
-static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(power_state, 0644,
 		tsl2x7x_power_state_show, tsl2x7x_power_state_store);
 
-static DEVICE_ATTR(in_proximity0_calibscale_available, S_IRUGO,
+static DEVICE_ATTR(in_proximity0_calibscale_available, 0444,
 		tsl2x7x_prox_gain_available_show, NULL);
 
-static DEVICE_ATTR(in_illuminance0_calibscale_available, S_IRUGO,
+static DEVICE_ATTR(in_illuminance0_calibscale_available, 0444,
 		tsl2x7x_gain_available_show, NULL);
 
-static DEVICE_ATTR(in_illuminance0_integration_time, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_illuminance0_integration_time, 0644,
 		tsl2x7x_als_time_show, tsl2x7x_als_time_store);
 
-static DEVICE_ATTR(in_illuminance0_target_input, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_illuminance0_target_input, 0644,
 		tsl2x7x_als_cal_target_show, tsl2x7x_als_cal_target_store);
 
-static DEVICE_ATTR(in_illuminance0_calibrate, S_IWUSR, NULL,
+static DEVICE_ATTR(in_illuminance0_calibrate, 0200, NULL,
 		tsl2x7x_do_calibrate);
 
-static DEVICE_ATTR(in_proximity0_calibrate, S_IWUSR, NULL,
+static DEVICE_ATTR(in_proximity0_calibrate, 0200, NULL,
 		tsl2x7x_do_prox_calibrate);
 
-static DEVICE_ATTR(in_illuminance0_lux_table, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_illuminance0_lux_table, 0644,
 		tsl2x7x_luxtable_show, tsl2x7x_luxtable_store);
 
-static DEVICE_ATTR(in_intensity0_thresh_period, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_intensity0_thresh_period, 0644,
 		tsl2x7x_als_persistence_show, tsl2x7x_als_persistence_store);
 
-static DEVICE_ATTR(in_proximity0_thresh_period, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_proximity0_thresh_period, 0644,
 		tsl2x7x_prox_persistence_show, tsl2x7x_prox_persistence_store);
 
 /* Use the default register values to identify the Taos device */
-- 
1.9.1

Re: [PATCH v3 2/3] Staging: iio: light: tsl2x7x_core.c: Replace symbolic permission with octal permission

[Jonathan Cameron]

On Wed, 19 Jul 2017 17:55:56 +0530
Jaya Durga <rjdurga@gmail.com> wrote:

> Replace symbolic permissions with their
> octect representation to fix checkpatch warnings.
> 
> Signed-off-by: Jaya Durga <rjdurga@gmail.com>
The IIO drivers in staging are handled via the iio
tree on git.kernel.org from which Greg pulls a few
times a cycle (when I get enough queued up to make
it worth a pull request).

This code has changed substantially and these symbolic
permissions aren't there any more.

I'll probably be sending Greg a pull request sometime
this week and some time after that these will show
up in staging tree.

Thanks and unfortunate you were working on a driver
that is seeing a lot of changes at the moment as 
Brian tries to clean it up entirely for a move out of
staging.

Jonathan
> ---
>  drivers/staging/iio/light/tsl2x7x_core.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x_core.c b/drivers/staging/iio/light/tsl2x7x_core.c
> index af3910b..c63fe6a 100644
> --- a/drivers/staging/iio/light/tsl2x7x_core.c
> +++ b/drivers/staging/iio/light/tsl2x7x_core.c
> @@ -1498,34 +1498,34 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
>  	return 0;
>  }
>  
> -static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
> +static DEVICE_ATTR(power_state, 0644,
>  		tsl2x7x_power_state_show, tsl2x7x_power_state_store);
>  
> -static DEVICE_ATTR(in_proximity0_calibscale_available, S_IRUGO,
> +static DEVICE_ATTR(in_proximity0_calibscale_available, 0444,
>  		tsl2x7x_prox_gain_available_show, NULL);
>  
> -static DEVICE_ATTR(in_illuminance0_calibscale_available, S_IRUGO,
> +static DEVICE_ATTR(in_illuminance0_calibscale_available, 0444,
>  		tsl2x7x_gain_available_show, NULL);
>  
> -static DEVICE_ATTR(in_illuminance0_integration_time, S_IRUGO | S_IWUSR,
> +static DEVICE_ATTR(in_illuminance0_integration_time, 0644,
>  		tsl2x7x_als_time_show, tsl2x7x_als_time_store);
>  
> -static DEVICE_ATTR(in_illuminance0_target_input, S_IRUGO | S_IWUSR,
> +static DEVICE_ATTR(in_illuminance0_target_input, 0644,
>  		tsl2x7x_als_cal_target_show, tsl2x7x_als_cal_target_store);
>  
> -static DEVICE_ATTR(in_illuminance0_calibrate, S_IWUSR, NULL,
> +static DEVICE_ATTR(in_illuminance0_calibrate, 0200, NULL,
>  		tsl2x7x_do_calibrate);
>  
> -static DEVICE_ATTR(in_proximity0_calibrate, S_IWUSR, NULL,
> +static DEVICE_ATTR(in_proximity0_calibrate, 0200, NULL,
>  		tsl2x7x_do_prox_calibrate);
>  
> -static DEVICE_ATTR(in_illuminance0_lux_table, S_IRUGO | S_IWUSR,
> +static DEVICE_ATTR(in_illuminance0_lux_table, 0644,
>  		tsl2x7x_luxtable_show, tsl2x7x_luxtable_store);
>  
> -static DEVICE_ATTR(in_intensity0_thresh_period, S_IRUGO | S_IWUSR,
> +static DEVICE_ATTR(in_intensity0_thresh_period, 0644,
>  		tsl2x7x_als_persistence_show, tsl2x7x_als_persistence_store);
>  
> -static DEVICE_ATTR(in_proximity0_thresh_period, S_IRUGO | S_IWUSR,
> +static DEVICE_ATTR(in_proximity0_thresh_period, 0644,
>  		tsl2x7x_prox_persistence_show, tsl2x7x_prox_persistence_store);
>  
>  /* Use the default register values to identify the Taos device */

[PATCH v3 3/3] Staging: iio: adc: ad7280a.c: Fixed Macro argument reuse

[Jaya Durga]

CHECK: Macro argument reuse 'addr' - possible side-effects?

convert AD7280A_DEVADDR to ad7280a_devaddr static function
to fix checkpath check

v3: small style changes

Signed-off-by: Jaya Durga <rjdurga@gmail.com>
---
 drivers/staging/iio/adc/ad7280a.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
index d5ab83f..f85dde9 100644
--- a/drivers/staging/iio/adc/ad7280a.c
+++ b/drivers/staging/iio/adc/ad7280a.c
@@ -99,9 +99,14 @@
 #define AD7280A_DEVADDR_MASTER		0
 #define AD7280A_DEVADDR_ALL		0x1F
 /* 5-bit device address is sent LSB first */
-#define AD7280A_DEVADDR(addr)	(((addr & 0x1) << 4) | ((addr & 0x2) << 3) | \
-				(addr & 0x4) | ((addr & 0x8) >> 3) | \
-				((addr & 0x10) >> 4))
+static unsigned int ad7280a_devaddr(unsigned int addr)
+{
+	return ((addr & 0x1) << 4) |
+	       ((addr & 0x2) << 3) |
+	       (addr & 0x4) |
+	       ((addr & 0x8) >> 3) |
+	       ((addr & 0x10) >> 4);
+}
 
 /* During a read a valid write is mandatory.
  * So writing to the highest available address (Address 0x1F)
@@ -372,7 +377,7 @@ static int ad7280_chain_setup(struct ad7280_state *st)
 		if (ad7280_check_crc(st, val))
 			return -EIO;
 
-		if (n != AD7280A_DEVADDR(val >> 27))
+		if (n != ad7280a_devaddr(val >> 27))
 			return -EIO;
 	}
 
@@ -511,7 +516,7 @@ static int ad7280_channel_init(struct ad7280_state *st)
 			st->channels[cnt].info_mask_shared_by_type =
 				BIT(IIO_CHAN_INFO_SCALE);
 			st->channels[cnt].address =
-				AD7280A_DEVADDR(dev) << 8 | ch;
+				ad7280a_devaddr(dev) << 8 | ch;
 			st->channels[cnt].scan_index = cnt;
 			st->channels[cnt].scan_type.sign = 'u';
 			st->channels[cnt].scan_type.realbits = 12;
@@ -558,7 +563,7 @@ static int ad7280_attr_init(struct ad7280_state *st)
 		for (ch = AD7280A_CELL_VOLTAGE_1; ch <= AD7280A_CELL_VOLTAGE_6;
 			ch++, cnt++) {
 			st->iio_attr[cnt].address =
-				AD7280A_DEVADDR(dev) << 8 | ch;
+				ad7280a_devaddr(dev) << 8 | ch;
 			st->iio_attr[cnt].dev_attr.attr.mode =
 				0644;
 			st->iio_attr[cnt].dev_attr.show =
@@ -574,7 +579,7 @@ static int ad7280_attr_init(struct ad7280_state *st)
 				&st->iio_attr[cnt].dev_attr.attr;
 			cnt++;
 			st->iio_attr[cnt].address =
-				AD7280A_DEVADDR(dev) << 8 |
+				ad7280a_devaddr(dev) << 8 |
 				(AD7280A_CB1_TIMER + ch);
 			st->iio_attr[cnt].dev_attr.attr.mode =
 				0644;
@@ -918,7 +923,7 @@ static int ad7280_probe(struct spi_device *spi)
 		if (ret)
 			goto error_unregister;
 
-		ret = ad7280_write(st, AD7280A_DEVADDR(st->slave_num),
+		ret = ad7280_write(st, ad7280a_devaddr(st->slave_num),
 				   AD7280A_ALERT, 0,
 				   AD7280A_ALERT_GEN_STATIC_HIGH |
 				   (pdata->chain_last_alert_ignore & 0xF));
-- 
1.9.1

Re: [PATCH v3 3/3] Staging: iio: adc: ad7280a.c: Fixed Macro argument reuse

[Jonathan Cameron]

On Wed, 19 Jul 2017 17:55:57 +0530
Jaya Durga <rjdurga@gmail.com> wrote:

> CHECK: Macro argument reuse 'addr' - possible side-effects?
> 
> convert AD7280A_DEVADDR to ad7280a_devaddr static function
> to fix checkpath check
> 
> v3: small style changes
> 
> Signed-off-by: Jaya Durga <rjdurga@gmail.com>

This patch is fine so I've applied it to the togreg branch
of iio.git which will be initially pushed out as testing
for the autobuilders to play with it.

Just as an aside there was no connection between the 3 patches
in this series.  As such, what was the point in making them
a series?  If you aren't either working on one driver,
or applying the same change to a set of drivers, please don't
combine patches into a series of random unconnected patches.

Jonathan
> ---
>  drivers/staging/iio/adc/ad7280a.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
> index d5ab83f..f85dde9 100644
> --- a/drivers/staging/iio/adc/ad7280a.c
> +++ b/drivers/staging/iio/adc/ad7280a.c
> @@ -99,9 +99,14 @@
>  #define AD7280A_DEVADDR_MASTER		0
>  #define AD7280A_DEVADDR_ALL		0x1F
>  /* 5-bit device address is sent LSB first */
> -#define AD7280A_DEVADDR(addr)	(((addr & 0x1) << 4) | ((addr & 0x2) << 3) | \
> -				(addr & 0x4) | ((addr & 0x8) >> 3) | \
> -				((addr & 0x10) >> 4))
> +static unsigned int ad7280a_devaddr(unsigned int addr)
> +{
> +	return ((addr & 0x1) << 4) |
> +	       ((addr & 0x2) << 3) |
> +	       (addr & 0x4) |
> +	       ((addr & 0x8) >> 3) |
> +	       ((addr & 0x10) >> 4);
> +}
>  
>  /* During a read a valid write is mandatory.
>   * So writing to the highest available address (Address 0x1F)
> @@ -372,7 +377,7 @@ static int ad7280_chain_setup(struct ad7280_state *st)
>  		if (ad7280_check_crc(st, val))
>  			return -EIO;
>  
> -		if (n != AD7280A_DEVADDR(val >> 27))
> +		if (n != ad7280a_devaddr(val >> 27))
>  			return -EIO;
>  	}
>  
> @@ -511,7 +516,7 @@ static int ad7280_channel_init(struct ad7280_state *st)
>  			st->channels[cnt].info_mask_shared_by_type =
>  				BIT(IIO_CHAN_INFO_SCALE);
>  			st->channels[cnt].address =
> -				AD7280A_DEVADDR(dev) << 8 | ch;
> +				ad7280a_devaddr(dev) << 8 | ch;
>  			st->channels[cnt].scan_index = cnt;
>  			st->channels[cnt].scan_type.sign = 'u';
>  			st->channels[cnt].scan_type.realbits = 12;
> @@ -558,7 +563,7 @@ static int ad7280_attr_init(struct ad7280_state *st)
>  		for (ch = AD7280A_CELL_VOLTAGE_1; ch <= AD7280A_CELL_VOLTAGE_6;
>  			ch++, cnt++) {
>  			st->iio_attr[cnt].address =
> -				AD7280A_DEVADDR(dev) << 8 | ch;
> +				ad7280a_devaddr(dev) << 8 | ch;
>  			st->iio_attr[cnt].dev_attr.attr.mode =
>  				0644;
>  			st->iio_attr[cnt].dev_attr.show =
> @@ -574,7 +579,7 @@ static int ad7280_attr_init(struct ad7280_state *st)
>  				&st->iio_attr[cnt].dev_attr.attr;
>  			cnt++;
>  			st->iio_attr[cnt].address =
> -				AD7280A_DEVADDR(dev) << 8 |
> +				ad7280a_devaddr(dev) << 8 |
>  				(AD7280A_CB1_TIMER + ch);
>  			st->iio_attr[cnt].dev_attr.attr.mode =
>  				0644;
> @@ -918,7 +923,7 @@ static int ad7280_probe(struct spi_device *spi)
>  		if (ret)
>  			goto error_unregister;
>  
> -		ret = ad7280_write(st, AD7280A_DEVADDR(st->slave_num),
> +		ret = ad7280_write(st, ad7280a_devaddr(st->slave_num),
>  				   AD7280A_ALERT, 0,
>  				   AD7280A_ALERT_GEN_STATIC_HIGH |
>  				   (pdata->chain_last_alert_ignore & 0xF));

Re: [PATCH v3 1/3] Staging: speakup: speakup_keypc.c: usleep_range is preferred over udelay

[Joe Perches]

On Wed, 2017-07-19 at 17:55 +0530, Jaya Durga wrote:
> Fix checkpatch issue: CHECK: usleep_range is preferred over udelay;
[]
> diff --git a/drivers/staging/speakup/speakup_keypc.c b/drivers/staging/speakup/speakup_keypc.c
[]
> @@ -28,7 +28,7 @@
>  
>  #define DRV_VERSION "2.10"
>  #define SYNTH_IO_EXTENT	0x04
> -#define SWAIT udelay(70)
> +#define SWAIT usleep_range(70, 150)

It'd probably be more sensible to remove this define as
SWAIT is used just once and use usleep_range directly.

Re: [PATCH v3 1/3] Staging: speakup: speakup_keypc.c: usleep_range is preferred over udelay

[Greg KH]

On Wed, Jul 19, 2017 at 05:36:28AM -0700, Joe Perches wrote:
> On Wed, 2017-07-19 at 17:55 +0530, Jaya Durga wrote:
> > Fix checkpatch issue: CHECK: usleep_range is preferred over udelay;
> []
> > diff --git a/drivers/staging/speakup/speakup_keypc.c b/drivers/staging/speakup/speakup_keypc.c
> []
> > @@ -28,7 +28,7 @@
> >  
> >  #define DRV_VERSION "2.10"
> >  #define SYNTH_IO_EXTENT	0x04
> > -#define SWAIT udelay(70)
> > +#define SWAIT usleep_range(70, 150)
> 
> It'd probably be more sensible to remove this define as
> SWAIT is used just once and use usleep_range directly.
> 

I asked for that last time :(

I guess I'll just reject this again...