Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1535567 > unrolled thread

[PATCH 00/19] staging: iio: isl29028: staging cleanups

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/19] staging: iio: isl29028: staging cleanups Brian Masney <masneyb@onstation.org> - 2016-12-04 03:30 +0100
    [PATCH 03/19] staging: iio: isl29028: remove chip test and defaults from isl29028_chip_init() Brian Masney <masneyb@onstation.org> - 2016-12-04 03:30 +0100
    [PATCH 15/19] staging: iio: isl29028: made error messages consistent Brian Masney <masneyb@onstation.org> - 2016-12-04 03:30 +0100
    [PATCH 19/19] staging: iio: isl29028: remove legacy device tree binding Brian Masney <masneyb@onstation.org> - 2016-12-04 03:30 +0100
    [PATCH 09/19] staging: iio: isl29028: change newlines to improve readability Brian Masney <masneyb@onstation.org> - 2016-12-04 03:30 +0100
    [PATCH 12/19] staging: iio: isl29028: fix comparison between signed and unsigned integers Brian Masney <masneyb@onstation.org> - 2016-12-04 03:30 +0100
      Re: [PATCH 12/19] staging: iio: isl29028: fix comparison between  signed and unsigned integers Dan Carpenter <dan.carpenter@oracle.com> - 2016-12-05 22:00 +0100
        Re: [PATCH 12/19] staging: iio: isl29028: fix comparison between  signed and unsigned integers Brian Masney <masneyb@onstation.org> - 2016-12-06 01:20 +0100
          Re: [PATCH 12/19] staging: iio: isl29028: fix comparison between  signed and unsigned integers Dan Carpenter <dan.carpenter@oracle.com> - 2016-12-06 11:30 +0100
    [PATCH 10/19] staging: iio: isl29028: remove unused define ISL29028_DEV_ATTR Brian Masney <masneyb@onstation.org> - 2016-12-04 03:30 +0100
    [PATCH 04/19] staging: iio: isl29028: add power management support Brian Masney <masneyb@onstation.org> - 2016-12-04 03:30 +0100
    [PATCH 16/19] staging: iio: isl29028: remove unnecessary error logging in isl29028_chip_init_and_power_on() Brian Masney <masneyb@onstation.org> - 2016-12-04 03:30 +0100

#1535567 — [PATCH 00/19] staging: iio: isl29028: staging cleanups

This is my first round of cleanups to the isl29028 light driver in
preparation for moving the driver out of staging. The main feature
introduced by this patch set is support for power management. The
rest of the patches are minor improvements, mostly style.

I have one of these devices from
https://www.digikey.com/product-detail/en/intersil/ISL29028AIROZ-T7/ISL29028AIROZ-T7CT-ND/2639604
mounted on a breakout board hooked up to a Raspberry Pi 2. The IR and
light sensor are functioning, however I can't get the proximity sensor
to work. I hooked an oscilloscope up to the IRDR pin and +3V to verify
that the pin state begins pulsing when proximity mode is enabled on the
chip. According to page 9 of the datasheet
http://www.intersil.com/content/dam/Intersil/documents/isl2/isl29028.pdf,
the chip has a noise reduction feature where it will take an IR reading
with the IR LED off to get the baseline, drive the IR LED for 0.1ms, take
another IR reading, and subtract the two readings. My hunch about the
cause of my issue is the 5mm IR LED that I am using is still emitting IR
light during the brief time that the IRDR has no power. The LED is huge
size wise compared to the size of the chip. Replacing the IR LED with a
visible light LED causes the visible light LED to become illuminated
dimly when the IRDR pin begins pulsing.

Any suggestions are welcome. Ideally, I'd like to verify that the
proximity sensing works before submitting a patch moving the driver out
of staging. Looking through the data sheet, and the git commits since
the driver was first introduced into staging, everything in the driver
looks correct so I think this is operator error on my part.

Brian Masney (19):
  staging: iio: isl29028: remove nested if statements
  staging: iio: isl29028: remove enable flag from
    isl29028_enable_proximity()
  staging: iio: isl29028: remove chip test and defaults from
    isl29028_chip_init()
  staging: iio: isl29028: add power management support
  staging: iio: isl29028: made alignment of #defines consistent
  staging: iio: isl29028: made alignment of variables in struct
    isl29028_chip consistent
  staging: iio: isl29028: fix alignment of function arguments
  staging: iio: isl29028: combine isl29028_proxim_get() and
    isl29028_read_proxim()
  staging: iio: isl29028: change newlines to improve readability
  staging: iio: isl29028: remove unused define ISL29028_DEV_ATTR
  staging: iio: isl29028: made column alignment in isl29028_channels
    consistent
  staging: iio: isl29028: fix comparison between signed and unsigned
    integers
  staging: iio: isl29028: move failure logging into
    isl29028_set_proxim_sampling()
  staging: iio: isl29028: move failure logging into
    isl29028_set_als_scale()
  staging: iio: isl29028: made error messages consistent
  staging: iio: isl29028: remove unnecessary error logging in
    isl29028_chip_init_and_power_on()
  staging: iio: isl29028: remove out of memory log message
  staging: iio: isl29028: remove unnecessary parenthesis
  staging: iio: isl29028: remove legacy device tree binding

 drivers/staging/iio/light/isl29028.c | 358 +++++++++++++++++++++--------------
 1 file changed, 215 insertions(+), 143 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1535568 — [PATCH 03/19] staging: iio: isl29028: remove chip test and defaults from isl29028_chip_init()

isl29028_chip_init() contains the device driver defaults and
two I2C calls that detect the presence of the chip. This patch
moves these into isl29028_probe() so that this function can
be used by the power management runtinme in a followup patch. This
patch also renames isl29028_chip_init() to
isl29028_chip_init_and_power_on().

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/isl29028.c | 42 +++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c b/drivers/staging/iio/light/isl29028.c
index c8e234d..4f819a8 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -416,29 +416,11 @@ static const struct iio_info isl29028_info = {
 	.write_raw = isl29028_write_raw,
 };
 
-static int isl29028_chip_init(struct isl29028_chip *chip)
+static int isl29028_chip_init_and_power_on(struct isl29028_chip *chip)
 {
 	struct device *dev = regmap_get_device(chip->regmap);
 	int ret;
 
-	chip->enable_prox  = false;
-	chip->prox_sampling = 20;
-	chip->lux_scale = 2000;
-	chip->als_ir_mode = ISL29028_MODE_NONE;
-
-	ret = regmap_write(chip->regmap, ISL29028_REG_TEST1_MODE, 0x0);
-	if (ret < 0) {
-		dev_err(dev, "%s(): write to reg %d failed, err = %d\n",
-			__func__, ISL29028_REG_TEST1_MODE, ret);
-		return ret;
-	}
-	ret = regmap_write(chip->regmap, ISL29028_REG_TEST2_MODE, 0x0);
-	if (ret < 0) {
-		dev_err(dev, "%s(): write to reg %d failed, err = %d\n",
-			__func__, ISL29028_REG_TEST2_MODE, ret);
-		return ret;
-	}
-
 	ret = regmap_write(chip->regmap, ISL29028_REG_CONFIGURE, 0x0);
 	if (ret < 0) {
 		dev_err(dev, "%s(): write to reg %d failed, err = %d\n",
@@ -506,7 +488,27 @@ static int isl29028_probe(struct i2c_client *client,
 		return ret;
 	}
 
-	ret = isl29028_chip_init(chip);
+	chip->enable_prox  = false;
+	chip->prox_sampling = 20;
+	chip->lux_scale = 2000;
+	chip->als_ir_mode = ISL29028_MODE_NONE;
+
+	ret = regmap_write(chip->regmap, ISL29028_REG_TEST1_MODE, 0x0);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"%s(): write to reg %d failed, err = %d\n", __func__,
+			ISL29028_REG_TEST1_MODE, ret);
+		return ret;
+	}
+	ret = regmap_write(chip->regmap, ISL29028_REG_TEST2_MODE, 0x0);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"%s(): write to reg %d failed, err = %d\n", __func__,
+			ISL29028_REG_TEST2_MODE, ret);
+		return ret;
+	}
+
+	ret = isl29028_chip_init_and_power_on(chip);
 	if (ret < 0) {
 		dev_err(&client->dev, "chip initialization failed: %d\n", ret);
 		return ret;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1535569 — [PATCH 15/19] staging: iio: isl29028: made error messages consistent

The wording and style of the different error messages was not
consistent. This patches makes the wording and style consistent
throughout the driver.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/isl29028.c | 56 ++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c b/drivers/staging/iio/light/isl29028.c
index 11e1673..a45d57c 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -197,14 +197,16 @@ static int isl29028_read_als_ir(struct isl29028_chip *chip, int *als_ir)
 	ret = regmap_read(chip->regmap, ISL29028_REG_ALSIR_L, &lsb);
 	if (ret < 0) {
 		dev_err(dev,
-			"Error in reading register ALSIR_L err %d\n", ret);
+			"%s(): Error %d reading register ALSIR_L\n",
+			__func__, ret);
 		return ret;
 	}
 
 	ret = regmap_read(chip->regmap, ISL29028_REG_ALSIR_U, &msb);
 	if (ret < 0) {
 		dev_err(dev,
-			"Error in reading register ALSIR_U err %d\n", ret);
+			"%s(): Error %d reading register ALSIR_U\n",
+			__func__, ret);
 		return ret;
 	}
 
@@ -229,8 +231,8 @@ static int isl29028_read_proxim(struct isl29028_chip *chip, int *prox)
 
 	ret = regmap_read(chip->regmap, ISL29028_REG_PROX_DATA, &data);
 	if (ret < 0) {
-		dev_err(dev, "Error in reading register %d, error %d\n",
-			ISL29028_REG_PROX_DATA, ret);
+		dev_err(dev, "%s(): Error %d reading register PROX_DATA\n",
+			__func__, ret);
 		return ret;
 	}
 
@@ -247,7 +249,8 @@ static int isl29028_als_get(struct isl29028_chip *chip, int *als_data)
 
 	ret = isl29028_set_als_ir_mode(chip, ISL29028_MODE_ALS);
 	if (ret < 0) {
-		dev_err(dev, "Error in enabling ALS mode err %d\n", ret);
+		dev_err(dev, "%s(): Error %d enabling ALS mode\n", __func__,
+			ret);
 		return ret;
 	}
 
@@ -277,7 +280,8 @@ static int isl29028_ir_get(struct isl29028_chip *chip, int *ir_data)
 
 	ret = isl29028_set_als_ir_mode(chip, ISL29028_MODE_IR);
 	if (ret < 0) {
-		dev_err(dev, "Error in enabling IR mode err %d\n", ret);
+		dev_err(dev, "%s(): Error %d enabling IR mode\n", __func__,
+			ret);
 		return ret;
 	}
 
@@ -304,14 +308,15 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 	case IIO_PROXIMITY:
 		if (mask != IIO_CHAN_INFO_SAMP_FREQ) {
 			dev_err(dev,
-				"proximity: mask value 0x%08lx not supported\n",
-				mask);
+				"%s(): proximity: Mask value 0x%08lx is not supported\n",
+				__func__, mask);
 			break;
 		}
 
 		if (val < 1 || val > 100) {
 			dev_err(dev,
-				"Samp_freq %d is not in range[1:100]\n", val);
+				"%s(): proximity: Sampling frequency %d is not in the range [1:100]\n",
+				__func__, val);
 			break;
 		}
 
@@ -320,21 +325,23 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 	case IIO_LIGHT:
 		if (mask != IIO_CHAN_INFO_SCALE) {
 			dev_err(dev,
-				"light: mask value 0x%08lx not supported\n",
-				mask);
+				"%s(): light: Mask value 0x%08lx is not supported\n",
+				__func__, mask);
 			break;
 		}
 
 		if ((val != 125) && (val != 2000)) {
 			dev_err(dev,
-				"lux scale %d is invalid [125, 2000]\n", val);
+				"%s(): light: Lux scale %d is not in the set {125, 2000}\n",
+				__func__, val);
 			break;
 		}
 
 		ret = isl29028_set_als_scale(chip, val);
 		break;
 	default:
-		dev_err(dev, "Unsupported channel type\n");
+		dev_err(dev, "%s(): Unsupported channel type %x\n",
+			__func__, chan->type);
 		break;
 	}
 
@@ -395,7 +402,8 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
 		ret = IIO_VAL_INT;
 		break;
 	default:
-		dev_err(dev, "mask value 0x%08lx not supported\n", mask);
+		dev_err(dev, "%s(): mask value 0x%08lx is not supported\n",
+			__func__, mask);
 		break;
 	}
 
@@ -451,8 +459,8 @@ static int isl29028_chip_init_and_power_on(struct isl29028_chip *chip)
 
 	ret = regmap_write(chip->regmap, ISL29028_REG_CONFIGURE, 0x0);
 	if (ret < 0) {
-		dev_err(dev, "%s(): write to reg %d failed, err = %d\n",
-			__func__, ISL29028_REG_CONFIGURE, ret);
+		dev_err(dev, "%s(): Error %d clearing the CONFIGURE register\n",
+			__func__, ret);
 		return ret;
 	}
 
@@ -513,8 +521,8 @@ static int isl29028_probe(struct i2c_client *client,
 	chip->regmap = devm_regmap_init_i2c(client, &isl29028_regmap_config);
 	if (IS_ERR(chip->regmap)) {
 		ret = PTR_ERR(chip->regmap);
-		dev_err(&client->dev, "regmap initialization failed: %d\n",
-			ret);
+		dev_err(&client->dev, "%s: Error %d initializing regmap\n",
+			__func__, ret);
 		return ret;
 	}
 
@@ -526,16 +534,16 @@ static int isl29028_probe(struct i2c_client *client,
 	ret = regmap_write(chip->regmap, ISL29028_REG_TEST1_MODE, 0x0);
 	if (ret < 0) {
 		dev_err(&client->dev,
-			"%s(): write to reg %d failed, err = %d\n", __func__,
-			ISL29028_REG_TEST1_MODE, ret);
+			"%s(): Error %d writing to TEST1_MODE register\n",
+			__func__, ret);
 		return ret;
 	}
 
 	ret = regmap_write(chip->regmap, ISL29028_REG_TEST2_MODE, 0x0);
 	if (ret < 0) {
 		dev_err(&client->dev,
-			"%s(): write to reg %d failed, err = %d\n", __func__,
-			ISL29028_REG_TEST2_MODE, ret);
+			"%s(): Error %d writing to TEST2_MODE register\n",
+			__func__, ret);
 		return ret;
 	}
 
@@ -555,8 +563,8 @@ static int isl29028_probe(struct i2c_client *client,
 	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
 	if (ret < 0) {
 		dev_err(&client->dev,
-			"iio registration fails with error %d\n",
-			ret);
+			"%s(): iio registration failed with error %d\n",
+			__func__, ret);
 		return ret;
 	}
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1535570 — [PATCH 19/19] staging: iio: isl29028: remove legacy device tree binding

The isl29028 staging driver contains a legacy binding of "isl,isl29028"
that is marked as legacy and to not use. There are no in tree references
to that binding. This patch removes the legacy binding in preparation
for moving the driver out of staging. Otherwise, we will have to support
this binding indefinitely.

Signed-off-by: Brian Masney <masneyb@onstation.org>
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <Mark.Rutland@arm.com>
---
 drivers/staging/iio/light/isl29028.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/iio/light/isl29028.c b/drivers/staging/iio/light/isl29028.c
index ae6896f..6be9aa7 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -611,7 +611,6 @@ static const struct i2c_device_id isl29028_id[] = {
 MODULE_DEVICE_TABLE(i2c, isl29028_id);
 
 static const struct of_device_id isl29028_of_match[] = {
-	{ .compatible = "isl,isl29028", }, /* for backward compat., don't use */
 	{ .compatible = "isil,isl29028", },
 	{ },
 };
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1535571 — [PATCH 09/19] staging: iio: isl29028: change newlines to improve readability

Add and remove newlines to improve code readability in preparation for
moving the driver out of staging.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/isl29028.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c b/drivers/staging/iio/light/isl29028.c
index fa2b283..8f9295f 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -69,10 +69,8 @@ enum isl29028_als_ir_mode {
 struct isl29028_chip {
 	struct mutex			lock;
 	struct regmap			*regmap;
-
 	unsigned int			prox_sampling;
 	bool				enable_prox;
-
 	int				lux_scale;
 	enum isl29028_als_ir_mode	als_ir_mode;
 	bool				suspended;
@@ -89,6 +87,7 @@ static int isl29028_set_proxim_sampling(struct isl29028_chip *chip,
 		if (period >= prox_period[sel])
 			break;
 	}
+
 	return regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
 				  ISL29028_CONF_PROX_SLP_MASK,
 				  sel << ISL29028_CONF_PROX_SLP_SH);
@@ -106,6 +105,7 @@ static int isl29028_enable_proximity(struct isl29028_chip *chip)
 
 	/* Wait for conversion to be complete for first sample */
 	mdelay(DIV_ROUND_UP(1000, chip->prox_sampling));
+
 	return 0;
 }
 
@@ -138,13 +138,11 @@ static int isl29028_set_als_ir_mode(struct isl29028_chip *chip,
 					 ISL29028_CONF_ALS_RANGE_MASK,
 					 ISL29028_CONF_ALS_RANGE_HIGH_LUX);
 		break;
-
 	case ISL29028_MODE_IR:
 		ret = regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
 					 ISL29028_CONF_ALS_IR_MODE_MASK,
 					 ISL29028_CONF_ALS_IR_MODE_IR);
 		break;
-
 	case ISL29028_MODE_NONE:
 		return regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
 					  ISL29028_CONF_ALS_EN_MASK,
@@ -191,6 +189,7 @@ static int isl29028_read_als_ir(struct isl29028_chip *chip, int *als_ir)
 	}
 
 	*als_ir = ((msb & 0xF) << 8) | (lsb & 0xFF);
+
 	return 0;
 }
 
@@ -204,6 +203,7 @@ static int isl29028_read_proxim(struct isl29028_chip *chip, int *prox)
 		ret = isl29028_enable_proximity(chip);
 		if (ret < 0)
 			return ret;
+
 		chip->enable_prox = true;
 	}
 
@@ -213,7 +213,9 @@ static int isl29028_read_proxim(struct isl29028_chip *chip, int *prox)
 			ISL29028_REG_PROX_DATA, ret);
 		return ret;
 	}
+
 	*prox = data;
+
 	return 0;
 }
 
@@ -244,6 +246,7 @@ static int isl29028_als_get(struct isl29028_chip *chip, int *als_data)
 		als_ir_data = (als_ir_data * 49) / 100;
 
 	*als_data = als_ir_data;
+
 	return 0;
 }
 
@@ -257,6 +260,7 @@ static int isl29028_ir_get(struct isl29028_chip *chip, int *ir_data)
 		dev_err(dev, "Error in enabling IR mode err %d\n", ret);
 		return ret;
 	}
+
 	return isl29028_read_als_ir(chip, ir_data);
 }
 
@@ -284,11 +288,13 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 				mask);
 			break;
 		}
+
 		if (val < 1 || val > 100) {
 			dev_err(dev,
 				"Samp_freq %d is not in range[1:100]\n", val);
 			break;
 		}
+
 		ret = isl29028_set_proxim_sampling(chip, val);
 		if (ret < 0) {
 			dev_err(dev,
@@ -296,9 +302,9 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 				ret);
 			break;
 		}
+
 		chip->prox_sampling = val;
 		break;
-
 	case IIO_LIGHT:
 		if (mask != IIO_CHAN_INFO_SCALE) {
 			dev_err(dev,
@@ -306,20 +312,22 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 				mask);
 			break;
 		}
+
 		if ((val != 125) && (val != 2000)) {
 			dev_err(dev,
 				"lux scale %d is invalid [125, 2000]\n", val);
 			break;
 		}
+
 		ret = isl29028_set_als_scale(chip, val);
 		if (ret < 0) {
 			dev_err(dev,
 				"Setting lux scale fail with error %d\n", ret);
 			break;
 		}
+
 		chip->lux_scale = val;
 		break;
-
 	default:
 		dev_err(dev, "Unsupported channel type\n");
 		break;
@@ -327,6 +335,7 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 
 write_done:
 	mutex_unlock(&chip->lock);
+
 	return ret;
 }
 
@@ -361,25 +370,25 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
 		default:
 			break;
 		}
+
 		if (ret < 0)
 			break;
+
 		ret = IIO_VAL_INT;
 		break;
-
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		if (chan->type != IIO_PROXIMITY)
 			break;
+
 		*val = chip->prox_sampling;
 		ret = IIO_VAL_INT;
 		break;
-
 	case IIO_CHAN_INFO_SCALE:
 		if (chan->type != IIO_LIGHT)
 			break;
 		*val = chip->lux_scale;
 		ret = IIO_VAL_INT;
 		break;
-
 	default:
 		dev_err(dev, "mask value 0x%08lx not supported\n", mask);
 		break;
@@ -387,6 +396,7 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
 
 read_done:
 	mutex_unlock(&chip->lock);
+
 	return ret;
 }
 
@@ -516,6 +526,7 @@ static int isl29028_probe(struct i2c_client *client,
 			ISL29028_REG_TEST1_MODE, ret);
 		return ret;
 	}
+
 	ret = regmap_write(chip->regmap, ISL29028_REG_TEST2_MODE, 0x0);
 	if (ret < 0) {
 		dev_err(&client->dev,
@@ -536,6 +547,7 @@ static int isl29028_probe(struct i2c_client *client,
 	indio_dev->name = id->name;
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->modes = INDIO_DIRECT_MODE;
+
 	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
 	if (ret < 0) {
 		dev_err(&client->dev,
@@ -543,6 +555,7 @@ static int isl29028_probe(struct i2c_client *client,
 			ret);
 		return ret;
 	}
+
 	return 0;
 }
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1535572 — [PATCH 12/19] staging: iio: isl29028: fix comparison between signed and unsigned integers

Fixed warning found by make W=2 to reduce the amount of build noise:

warning: comparison between signed and unsigned integer expressions
[-Wsign-compare]

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/isl29028.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/light/isl29028.c b/drivers/staging/iio/light/isl29028.c
index 218d165..23a4db8 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -80,7 +80,7 @@ static int isl29028_set_proxim_sampling(struct isl29028_chip *chip,
 					unsigned int sampling)
 {
 	static unsigned int prox_period[] = {800, 400, 200, 100, 75, 50, 12, 0};
-	int sel;
+	unsigned int sel;
 	unsigned int period = DIV_ROUND_UP(1000, sampling);
 
 	for (sel = 0; sel < ARRAY_SIZE(prox_period); ++sel) {
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1536415 — Re: [PATCH 12/19] staging: iio: isl29028: fix comparison between signed and unsigned integers

On Sat, Dec 03, 2016 at 09:19:36PM -0500, Brian Masney wrote:
> Fixed warning found by make W=2 to reduce the amount of build noise:
> 
> warning: comparison between signed and unsigned integer expressions
> [-Wsign-compare]

Ugh...  Please don't do work arounds for nonsense warnings.  W=2 is so
stupid.  Better to just grep -v this warning instead of trying to please
a broken static analysis.  Warnings like this are why it's disabled by
default.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1536547 — Re: [PATCH 12/19] staging: iio: isl29028: fix comparison between signed and unsigned integers

On Mon, Dec 05, 2016 at 11:53:39PM +0300, Dan Carpenter wrote:
> On Sat, Dec 03, 2016 at 09:19:36PM -0500, Brian Masney wrote:
> > Fixed warning found by make W=2 to reduce the amount of build noise:
> > 
> > warning: comparison between signed and unsigned integer expressions
> > [-Wsign-compare]
> 
> Ugh...  Please don't do work arounds for nonsense warnings.  W=2 is so
> stupid.  Better to just grep -v this warning instead of trying to please
> a broken static analysis.  Warnings like this are why it's disabled by
> default.

Hi Dan,
   I would normally agree, however there could be a case where this
warning flags a legitimate issue. It is obviously not an issue in this
case. Since I'm already working on cleaning up this driver to move it
out of staging, I figured that I would make sure that it builds cleanly
with W=2. This was the only warning found in that driver. The
change is harmless in my opinion and it may eliminate a nonsense warning
for someone else down the road when doing security audits.

   This driver doesn't need much to move it out of staging. Most of the
patches in this series were trivial cleanups and not interesting at all.
Since I already have one of these devices, I figured that I'd do the
grunt work to get it out of staging. My goal with the upcoming final
patch that moves it out of staging is to reduce the amount of code churn
in the driver once it graduates from staging.

Brian

[toc] | [prev] | [next] | [standalone]

#1536832 — Re: [PATCH 12/19] staging: iio: isl29028: fix comparison between signed and unsigned integers

On Mon, Dec 05, 2016 at 07:10:45PM -0500, Brian Masney wrote:
> On Mon, Dec 05, 2016 at 11:53:39PM +0300, Dan Carpenter wrote:
> > On Sat, Dec 03, 2016 at 09:19:36PM -0500, Brian Masney wrote:
> > > Fixed warning found by make W=2 to reduce the amount of build noise:
> > > 
> > > warning: comparison between signed and unsigned integer expressions
> > > [-Wsign-compare]
> > 
> > Ugh...  Please don't do work arounds for nonsense warnings.  W=2 is so
> > stupid.  Better to just grep -v this warning instead of trying to please
> > a broken static analysis.  Warnings like this are why it's disabled by
> > default.
> 
> Hi Dan,
>    I would normally agree, however there could be a case where this
> warning flags a legitimate issue. It is obviously not an issue in this
> case. Since I'm already working on cleaning up this driver to move it
> out of staging, I figured that I would make sure that it builds cleanly
> with W=2. This was the only warning found in that driver. The
> change is harmless in my opinion and it may eliminate a nonsense warning
> for someone else down the road when doing security audits.

Iterators should be int unless there is a specific reason for a fancier
data type.  Using complicated types just makes the code more complicated
and tiring to read.

Smatch or other similar static analysis tools know that "sel" is in the
0-7 range and that ARRAY_SIZE(prox_period) is 8.  GCC almost certainly
knows this as well.  The warning messages is just printed because the
devs are lazy.  It's totally pointless.

Don't work around lazy static analysis.  It sends the wrong message to
do pointless things.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1535573 — [PATCH 10/19] staging: iio: isl29028: remove unused define ISL29028_DEV_ATTR

The #define ISL29028_DEV_ATTR was not used so this patch removes the
unnecessary code.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/isl29028.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/iio/light/isl29028.c b/drivers/staging/iio/light/isl29028.c
index 8f9295f..0e727ba 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -404,7 +404,6 @@ static IIO_CONST_ATTR(in_proximity_sampling_frequency_available,
 				"1 3 5 10 13 20 83 100");
 static IIO_CONST_ATTR(in_illuminance_scale_available, "125 2000");
 
-#define ISL29028_DEV_ATTR(name) (&iio_dev_attr_##name.dev_attr.attr)
 #define ISL29028_CONST_ATTR(name) (&iio_const_attr_##name.dev_attr.attr)
 static struct attribute *isl29028_attributes[] = {
 	ISL29028_CONST_ATTR(in_proximity_sampling_frequency_available),
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1535574 — [PATCH 04/19] staging: iio: isl29028: add power management support

This patch adds power management support to the isl29028 driver.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/isl29028.c | 59 ++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/drivers/staging/iio/light/isl29028.c b/drivers/staging/iio/light/isl29028.c
index 4f819a8..e96a8cb 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -75,6 +75,7 @@ struct isl29028_chip {
 
 	int			lux_scale;
 	enum isl29028_als_ir_mode	als_ir_mode;
+	bool				suspended;
 };
 
 static int isl29028_set_proxim_sampling(struct isl29028_chip *chip,
@@ -274,6 +275,12 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 	int ret = -EINVAL;
 
 	mutex_lock(&chip->lock);
+
+	if (chip->suspended) {
+		ret = -EBUSY;
+		goto write_done;
+	}
+
 	switch (chan->type) {
 	case IIO_PROXIMITY:
 		if (mask != IIO_CHAN_INFO_SAMP_FREQ) {
@@ -322,6 +329,8 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 		dev_err(dev, "Unsupported channel type\n");
 		break;
 	}
+
+write_done:
 	mutex_unlock(&chip->lock);
 	return ret;
 }
@@ -335,6 +344,12 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
 	int ret = -EINVAL;
 
 	mutex_lock(&chip->lock);
+
+	if (chip->suspended) {
+		ret = -EBUSY;
+		goto read_done;
+	}
+
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 	case IIO_CHAN_INFO_PROCESSED:
@@ -374,6 +389,8 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
 		dev_err(dev, "mask value 0x%08lx not supported\n", mask);
 		break;
 	}
+
+read_done:
 	mutex_unlock(&chip->lock);
 	return ret;
 }
@@ -437,6 +454,9 @@ static int isl29028_chip_init_and_power_on(struct isl29028_chip *chip)
 	ret = isl29028_set_als_scale(chip, chip->lux_scale);
 	if (ret < 0)
 		dev_err(dev, "setting als scale failed, err = %d\n", ret);
+
+	chip->suspended = false;
+
 	return ret;
 }
 
@@ -479,6 +499,7 @@ static int isl29028_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, indio_dev);
 	mutex_init(&chip->lock);
+	chip->suspended = true;
 
 	chip->regmap = devm_regmap_init_i2c(client, &isl29028_regmap_config);
 	if (IS_ERR(chip->regmap)) {
@@ -530,6 +551,43 @@ static int isl29028_probe(struct i2c_client *client,
 	return 0;
 }
 
+static int __maybe_unused isl29028_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct isl29028_chip *chip = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&chip->lock);
+
+	ret = regmap_write(chip->regmap, ISL29028_REG_CONFIGURE, 0x0);
+	if (ret < 0)
+		dev_err(dev, "%s(): Error %d turning off chip\n", __func__,
+			ret);
+
+	chip->suspended = true;
+
+	mutex_unlock(&chip->lock);
+
+	return ret;
+}
+
+static int __maybe_unused isl29028_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct isl29028_chip *chip = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&chip->lock);
+
+	ret = isl29028_chip_init_and_power_on(chip);
+
+	mutex_unlock(&chip->lock);
+
+	return ret;
+}
+
+static SIMPLE_DEV_PM_OPS(isl29028_pm_ops, isl29028_suspend, isl29028_resume);
+
 static const struct i2c_device_id isl29028_id[] = {
 	{"isl29028", 0},
 	{}
@@ -546,6 +604,7 @@ MODULE_DEVICE_TABLE(of, isl29028_of_match);
 static struct i2c_driver isl29028_driver = {
 	.driver  = {
 		.name = "isl29028",
+		.pm = &isl29028_pm_ops,
 		.of_match_table = isl29028_of_match,
 	},
 	.probe	 = isl29028_probe,
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1535575 — [PATCH 16/19] staging: iio: isl29028: remove unnecessary error logging in isl29028_chip_init_and_power_on()

If the call to isl29028_chip_init_and_power_on() in isl29028_probe()
fails, then isl29028_probe() will log an error message. All of the
error paths in that call path already have error logging in place. This
patch removes the unnecessary logging.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/isl29028.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/iio/light/isl29028.c b/drivers/staging/iio/light/isl29028.c
index a45d57c..3f01ad1 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -548,10 +548,8 @@ static int isl29028_probe(struct i2c_client *client,
 	}
 
 	ret = isl29028_chip_init_and_power_on(chip);
-	if (ret < 0) {
-		dev_err(&client->dev, "chip initialization failed: %d\n", ret);
+	if (ret < 0)
 		return ret;
-	}
 
 	indio_dev->info = &isl29028_info;
 	indio_dev->channels = isl29028_channels;
-- 
2.7.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web