[PATCH 01/11] iio: sensors-core: st: Check st_sensors_set_drdy_int_pin()'s return value

[PATCH 01/11] iio: sensors-core: st: Check st_sensors_set_drdy_int_pin()'s return value

[Lee Jones]

Value from st_sensors_set_drdy_int_pin() is assigned to err here,
but that stored value is not used before it is overwritten.  To fix
this we're enforcing a check on st_sensors_set_drdy_int_pin()'s
return value and if it's an error, we're returning right away.

Cc: jic23 at kernel.org
Cc: linux-iio at vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/iio/common/st_sensors/st_sensors_core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
index 8a4ec00..24cfe4e 100644
--- a/drivers/iio/common/st_sensors/st_sensors_core.c
+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
@@ -306,8 +306,11 @@ int st_sensors_init_sensor(struct iio_dev *indio_dev,
 	if (of_pdata)
 		pdata = of_pdata;
 
-	if (pdata)
+	if (pdata) {
 		err = st_sensors_set_drdy_int_pin(indio_dev, pdata);
+		if (err < 0)
+			return err;
+	}
 
 	err = st_sensors_set_enable(indio_dev, false);
 	if (err < 0)
-- 
1.9.1

[PATCH 02/11] misc: st_core: Protect unsigned value against becoming negative

[Lee Jones]

Coverity reported:
  This less-than-zero comparison of an unsigned value is never true.

In answer to that, we only ever decrement if protos_registered is
positive.  We can subsequently remove the paranoid checking during
unregister.

Cc: gregkh at linuxfoundation.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/misc/ti-st/st_core.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/misc/ti-st/st_core.c b/drivers/misc/ti-st/st_core.c
index 1972d57a..54be83d 100644
--- a/drivers/misc/ti-st/st_core.c
+++ b/drivers/misc/ti-st/st_core.c
@@ -153,8 +153,9 @@ static void st_reg_complete(struct st_data_s *st_gdata, char err)
 				(st_gdata->list[i]->priv_data, err);
 			pr_info("protocol %d's cb sent %d\n", i, err);
 			if (err) { /* cleanup registered protocol */
-				st_gdata->protos_registered--;
 				st_gdata->is_registered[i] = false;
+				if (st_gdata->protos_registered)
+					st_gdata->protos_registered--;
 			}
 		}
 	}
@@ -639,14 +640,12 @@ long st_unregister(struct st_proto_s *proto)
 		return -EPROTONOSUPPORT;
 	}
 
-	st_gdata->protos_registered--;
+	if (st_gdata->protos_registered)
+		st_gdata->protos_registered--;
+
 	remove_channel_from_table(st_gdata, proto);
 	spin_unlock_irqrestore(&st_gdata->lock, flags);
 
-	/* paranoid check */
-	if (st_gdata->protos_registered < ST_EMPTY)
-		st_gdata->protos_registered = ST_EMPTY;
-
 	if ((st_gdata->protos_registered == ST_EMPTY) &&
 	    (!test_bit(ST_REG_PENDING, &st_gdata->st_state))) {
 		pr_info(" all chnl_ids unregistered ");
-- 
1.9.1

[PATCH 03/11] misc: st_kim: Increase size of dev_name buffer to incorporate termination

[Lee Jones]

Calling strncpy with a maximum size argument of 32 bytes on destination
array kim_gdata->dev_name of size 32 bytes might leave the destination
string unterminated.

Cc: gregkh at linuxfoundation.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 include/linux/ti_wilink_st.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/ti_wilink_st.h b/include/linux/ti_wilink_st.h
index 932b7639..884d626 100644
--- a/include/linux/ti_wilink_st.h
+++ b/include/linux/ti_wilink_st.h
@@ -268,7 +268,7 @@ struct kim_data_s {
 	struct st_data_s *core_data;
 	struct chip_version version;
 	unsigned char ldisc_install;
-	unsigned char dev_name[UART_DEV_NAME_LEN];
+	unsigned char dev_name[UART_DEV_NAME_LEN + 1];
 	unsigned char flow_cntrl;
 	unsigned long baud_rate;
 };
-- 
1.9.1

[PATCH 04/11] mfd: htc-i2cpld: container_of() cannot return NULL

[Lee Jones]

Logically dead code (DEADCODE)

dead_error_line: Execution cannot reach this statement: return;

Remove the check for NULL.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/htc-i2cpld.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/htc-i2cpld.c b/drivers/mfd/htc-i2cpld.c
index b44f020..8e3ea07 100644
--- a/drivers/mfd/htc-i2cpld.c
+++ b/drivers/mfd/htc-i2cpld.c
@@ -227,13 +227,10 @@ static irqreturn_t htcpld_handler(int irq, void *dev)
 static void htcpld_chip_set(struct gpio_chip *chip, unsigned offset, int val)
 {
 	struct i2c_client *client;
-	struct htcpld_chip *chip_data;
+	struct htcpld_chip *chip_data =
+		container_of(chip, struct htcpld_chip, chip_out);
 	unsigned long flags;
 
-	chip_data = container_of(chip, struct htcpld_chip, chip_out);
-	if (!chip_data)
-		return;
-
 	client = chip_data->client;
 	if (client == NULL)
 		return;
-- 
1.9.1

[PATCH 05/11] mfd: htc-i2cpld: Use '!variable' instead of 'variable == NULL'

[Lee Jones]

This aids in consistency, as the rest of the checks in the file use this
notation.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/htc-i2cpld.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/htc-i2cpld.c b/drivers/mfd/htc-i2cpld.c
index 8e3ea07..073e180 100644
--- a/drivers/mfd/htc-i2cpld.c
+++ b/drivers/mfd/htc-i2cpld.c
@@ -232,7 +232,7 @@ static void htcpld_chip_set(struct gpio_chip *chip, unsigned offset, int val)
 	unsigned long flags;
 
 	client = chip_data->client;
-	if (client == NULL)
+	if (!client)
 		return;
 
 	spin_lock_irqsave(&chip_data->lock, flags);
@@ -373,7 +373,7 @@ static int htcpld_register_chip_i2c(
 	plat_chip_data = &pdata->chip[chip_index];
 
 	adapter = i2c_get_adapter(pdata->i2c_adapter_id);
-	if (adapter == NULL) {
+	if (!adapter) {
 		/* Eek, no such I2C adapter!  Bail out. */
 		dev_warn(dev, "Chip at i2c address 0x%x: Invalid i2c adapter %d\n",
 			 plat_chip_data->addr, pdata->i2c_adapter_id);
-- 
1.9.1

[PATCH 06/11] mfd: htc-i2cpld: Rectify pointer offset error

[Lee Jones]

Checking the result of container_of() against NULL will always result to
false.  Using the offset of member 'chip_out' to find the start of 'struct
htcpld_chip' will result in an offset error when .get_chip() is attempting
to obtain 'htcpld-in'.  Instead, we'll use the correct member based on a
previously the set chip label.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/htc-i2cpld.c | 33 ++++++++++-----------------------
 1 file changed, 10 insertions(+), 23 deletions(-)

diff --git a/drivers/mfd/htc-i2cpld.c b/drivers/mfd/htc-i2cpld.c
index 073e180..99b362e 100644
--- a/drivers/mfd/htc-i2cpld.c
+++ b/drivers/mfd/htc-i2cpld.c
@@ -258,31 +258,18 @@ static void htcpld_chip_set_ni(struct work_struct *work)
 static int htcpld_chip_get(struct gpio_chip *chip, unsigned offset)
 {
 	struct htcpld_chip *chip_data;
-	int val = 0;
-	int is_input = 0;
-
-	/* Try out first */
-	chip_data = container_of(chip, struct htcpld_chip, chip_out);
-	if (!chip_data) {
-		/* Try in */
-		is_input = 1;
-		chip_data = container_of(chip, struct htcpld_chip, chip_in);
-		if (!chip_data)
-			return -EINVAL;
-	}
+	u8 cache;
 
-	/* Determine if this is an input or output GPIO */
-	if (!is_input)
-		/* Use the output cache */
-		val = (chip_data->cache_out >> offset) & 1;
-	else
-		/* Use the input cache */
-		val = (chip_data->cache_in >> offset) & 1;
+	if (!strncmp(chip->label, "htcpld-out", 10)) {
+		chip_data = container_of(chip, struct htcpld_chip, chip_out);
+		cache = chip_data->cache_out;
+	} else if (!strncmp(chip->label, "htcpld-in", 9)) {
+		chip_data = container_of(chip, struct htcpld_chip, chip_in);
+		cache = chip_data->cache_in;
+	} else
+		return -EINVAL;
 
-	if (val)
-		return 1;
-	else
-		return 0;
+	return (cache >> offset) & 1;
 }
 
 static int htcpld_direction_output(struct gpio_chip *chip,
-- 
1.9.1

[PATCH 07/11] mfd: wm5102: Insert missing break in case statement

[Lee Jones]

Chip identifier wm5102_reva_patch is always overwritten with
wm5102_revb_patch, even when the code is run on a Rev-A chip.  Place
in the missing break to force the code into doing something sensible
instead.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/wm5102-tables.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/wm5102-tables.c b/drivers/mfd/wm5102-tables.c
index fb4d4bb..a219888 100644
--- a/drivers/mfd/wm5102-tables.c
+++ b/drivers/mfd/wm5102-tables.c
@@ -87,6 +87,7 @@ int wm5102_patch(struct arizona *arizona)
 	case 0:
 		wm5102_patch = wm5102_reva_patch;
 		patch_size = ARRAY_SIZE(wm5102_reva_patch);
+		break;
 	default:
 		wm5102_patch = wm5102_revb_patch;
 		patch_size = ARRAY_SIZE(wm5102_revb_patch);
-- 
1.9.1

[PATCH 08/11] mfd: max77686: Remove check which is always true

[Lee Jones]

As 'reg' is unsigned, it can't be less than 0, so checking if it is
greater than or equal to 0 will always result in a true condition.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/max77686.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
index 86e5523..929795e 100644
--- a/drivers/mfd/max77686.c
+++ b/drivers/mfd/max77686.c
@@ -52,7 +52,7 @@ static const struct mfd_cell max77802_devs[] = {
 static bool max77802_pmic_is_accessible_reg(struct device *dev,
 					    unsigned int reg)
 {
-	return (reg >= MAX77802_REG_DEVICE_ID && reg < MAX77802_REG_PMIC_END);
+	return reg < MAX77802_REG_PMIC_END;
 }
 
 static bool max77802_rtc_is_accessible_reg(struct device *dev,
-- 
1.9.1

[PATCH 09/11] mfd: pcf50633: Check return value of platform_device_add()

[Lee Jones]

The return value of platform_device_add() is checked after every
other use throughout the kernel.

We're also sliding in another cheeky dev_err() => dev_warn() change
as we're not actually erroring out here, rather reporting the fact
that something's gone wrong, but carrying on regardless.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/pcf50633-core.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/mfd/pcf50633-core.c b/drivers/mfd/pcf50633-core.c
index c87f7a0..e15c060 100644
--- a/drivers/mfd/pcf50633-core.c
+++ b/drivers/mfd/pcf50633-core.c
@@ -195,8 +195,9 @@ static int pcf50633_probe(struct i2c_client *client,
 				const struct i2c_device_id *ids)
 {
 	struct pcf50633 *pcf;
+	struct platform_device *pdev;
 	struct pcf50633_platform_data *pdata = dev_get_platdata(&client->dev);
-	int i, ret;
+	int i, j, ret;
 	int version, variant;
 
 	if (!client->irq) {
@@ -243,9 +244,6 @@ static int pcf50633_probe(struct i2c_client *client,
 
 
 	for (i = 0; i < PCF50633_NUM_REGULATORS; i++) {
-		struct platform_device *pdev;
-		int j;
-
 		pdev = platform_device_alloc("pcf50633-regulator", i);
 		if (!pdev)
 			return -ENOMEM;
@@ -253,25 +251,31 @@ static int pcf50633_probe(struct i2c_client *client,
 		pdev->dev.parent = pcf->dev;
 		ret = platform_device_add_data(pdev, &pdata->reg_init_data[i],
 					       sizeof(pdata->reg_init_data[i]));
-		if (ret) {
-			platform_device_put(pdev);
-			for (j = 0; j < i; j++)
-				platform_device_put(pcf->regulator_pdev[j]);
-			return ret;
-		}
-		pcf->regulator_pdev[i] = pdev;
+		if (ret)
+			goto err;
+
+		ret = platform_device_add(pdev);
+		if (ret)
+			goto err;
 
-		platform_device_add(pdev);
+		pcf->regulator_pdev[i] = pdev;
 	}
 
 	ret = sysfs_create_group(&client->dev.kobj, &pcf_attr_group);
 	if (ret)
-		dev_err(pcf->dev, "error creating sysfs entries\n");
+		dev_warn(pcf->dev, "error creating sysfs entries\n");
 
 	if (pdata->probe_done)
 		pdata->probe_done(pcf);
 
 	return 0;
+
+err:
+	platform_device_put(pdev);
+	for (j = 0; j < i; j++)
+		platform_device_put(pcf->regulator_pdev[j]);
+
+	return ret;
 }
 
 static int pcf50633_remove(struct i2c_client *client)
-- 
1.9.1

[PATCH 10/11] mfd: twl4030-irq: Check return value from twl_i2c_write() - warn() on failure

[Lee Jones]

In the original code a return value variable was provided, but it
was never checked and the user was never informed of failure.  Now
it is and they are.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/twl4030-irq.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index b1dabba..1b772ef 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -396,13 +396,17 @@ static int twl4030_init_sih_modules(unsigned line)
 			status = twl_i2c_read(sih->module, rxbuf,
 				sih->mask[line].isr_offset, sih->bytes_ixr);
 			if (status < 0)
-				pr_err("twl4030: err %d initializing %s %s\n",
+				pr_warn("twl4030: err %d initializing %s %s\n",
 					status, sih->name, "ISR");
 
-			if (!sih->set_cor)
+			if (!sih->set_cor) {
 				status = twl_i2c_write(sih->module, buf,
 					sih->mask[line].isr_offset,
 					sih->bytes_ixr);
+				if (status < 0)
+					pr_warn("twl4030: write failed: %d\n",
+						status);
+			}
 			/*
 			 * else COR=1 means read sufficed.
 			 * (for most SIH modules...)
-- 
1.9.1

[PATCH 11/11] mfd: sm501: Avoid implicit sign extension

[Lee Jones]

Suspicious implicit sign extension where 'reg' (unsigned char) is
promoted in reg << clksrc to int, then sign-extended to unsigned
long. If reg << clksrc is greater than 0x7FFFFFFF, the upper bits
of the result will all be 1.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/sm501.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c
index 81e6d09..1f9233b 100644
--- a/drivers/mfd/sm501.c
+++ b/drivers/mfd/sm501.c
@@ -514,9 +514,9 @@ unsigned long sm501_set_clock(struct device *dev,
 	unsigned long mode = smc501_readl(sm->regs + SM501_POWER_MODE_CONTROL);
 	unsigned long gate = smc501_readl(sm->regs + SM501_CURRENT_GATE);
 	unsigned long clock = smc501_readl(sm->regs + SM501_CURRENT_CLOCK);
-	unsigned char reg;
 	unsigned int pll_reg = 0;
 	unsigned long sm501_freq; /* the actual frequency achieved */
+	u64 reg;
 
 	struct sm501_clock to;
 
-- 
1.9.1

[PATCH 01/11] iio: sensors-core: st: Check st_sensors_set_drdy_int_pin()'s return value

[Jonathan Cameron]

On 28/08/14 14:14, Lee Jones wrote:
> Value from st_sensors_set_drdy_int_pin() is assigned to err here,
> but that stored value is not used before it is overwritten.  To fix
> this we're enforcing a check on st_sensors_set_drdy_int_pin()'s
> return value and if it's an error, we're returning right away.
> 
> Cc: jic23 at kernel.org
> Cc: linux-iio at vger.kernel.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
Applied to the togreg branch of iio.git

Thanks,

Jonathan
> ---
>  drivers/iio/common/st_sensors/st_sensors_core.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> index 8a4ec00..24cfe4e 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -306,8 +306,11 @@ int st_sensors_init_sensor(struct iio_dev *indio_dev,
>  	if (of_pdata)
>  		pdata = of_pdata;
>  
> -	if (pdata)
> +	if (pdata) {
>  		err = st_sensors_set_drdy_int_pin(indio_dev, pdata);
> +		if (err < 0)
> +			return err;
> +	}
>  
>  	err = st_sensors_set_enable(indio_dev, false);
>  	if (err < 0)
>