Skip to contentSkip to conversation

Staging: speakup: speakup_keypc.c: usleep_range is preferred over udelay

PATCH In review Jaya Durga 4 participants 7 messages Jul 19, 2017 – Jul 22, 2017

Subsystems (4)
Staging IIO Analog Devices IIO drivers Staging - industrial io
lore
Share & tools
lore

Jaya Durga

author
#0 lore raw

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

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

Signed-off-by Jaya Durga <[email protected]>
---
 drivers/staging/speakup/speakup_keypc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/speakup/speakup_keypc.c +2 -2
index d3203f8..1ba4cfc 100644
@@ -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;
Signature · 1 line
1.9.1
author
#1 lore raw

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

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

Signed-off-by Jaya Durga <[email protected]>
---
 drivers/staging/iio/light/tsl2x7x_core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
drivers/staging/iio/light/tsl2x7x_core.c +10 -10
index af3910b..c63fe6a 100644
@@ -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 */
Signature · 1 line
1.9.1
maintainer
#2 lore raw

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

Quoted 4 lines · On Wed, 19 Jul 2017 17:55:56 +0530 Jaya Durga <[email protected]> wrote:

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

Signed-off-by Jaya Durga <[email protected]>

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

Quoted 53 lines
---
 drivers/staging/iio/light/tsl2x7x_core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
drivers/staging/iio/light/tsl2x7x_core.c +10 -10
index af3910b..c63fe6a 100644
@@ -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 */
author
#3 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/staging/iio/adc/ad7280a.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
drivers/staging/iio/adc/ad7280a.c +13 -8
index d5ab83f..f85dde9 100644
@@ -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));
Signature · 1 line
1.9.1
maintainer
#4 lore raw

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

Quoted 8 lines · On Wed, 19 Jul 2017 17:55:57 +0530 Jaya Durga <[email protected]> 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 <[email protected]>

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

Quoted 71 lines
---
 drivers/staging/iio/adc/ad7280a.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
drivers/staging/iio/adc/ad7280a.c +13 -8
index d5ab83f..f85dde9 100644
@@ -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));
#5 lore raw
In reply to Jaya Durga
Quoted 1 line · Jaya Durga

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

[]

drivers/staging/speakup/speakup_keypc.c

Quoted 1 line
drivers/staging/speakup/speakup_keypc.c +0 -0

[]

drivers/staging/speakup/speakup_keypc.c line 31

Quoted 6 lines
drivers/staging/speakup/speakup_keypc.c +1 -1
@@ -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.

maintainer
#6 lore raw
Quoted 7 lines · Joe Perches
Quoted 1 line · Jaya Durga

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

[]

drivers/staging/speakup/speakup_keypc.c

Quoted 1 line
drivers/staging/speakup/speakup_keypc.c +0 -0

[]

Quoted 2 lines
drivers/staging/speakup/speakup_keypc.c +0 -0
@@ -28,7 +28,7 @@
Quoted 8 lines

drivers/staging/speakup/speakup_keypc.c line 31

Quoted 4 lines
drivers/staging/speakup/speakup_keypc.c +1 -1
 #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...