Re: [PATCH v2 3/5] iio:light:stk3310: add more error handling

[Jonathan Cameron <jic23@kernel.org>]

On 19/07/15 14:57, Jonathan Cameron wrote:
> On 09/07/15 22:51, Hartmut Knaack wrote:
>> Check for the following error cases:
>>   * lower boundary for val in _write_event
>>   * return value of regmap_(field_)read
>>   * possible values for chan->type
>>   * return value of stk3310_gpio_probe
>>
>> Also add an error path in _probe to put the sensor back into stand-by mode
>> in case of serious errors.
>>
>> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
> looks good.  Will have to hold it until the fixes have worked there way
> through though.
Applied.
>> ---
>>  drivers/iio/light/stk3310.c | 59 ++++++++++++++++++++++++++++++++++-----------
>>  1 file changed, 45 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c
>> index 11a027adc204..d6df40220007 100644
>> --- a/drivers/iio/light/stk3310.c
>> +++ b/drivers/iio/light/stk3310.c
>> @@ -241,8 +241,11 @@ static int stk3310_write_event(struct iio_dev *indio_dev,
>>  	struct stk3310_data *data = iio_priv(indio_dev);
>>  	struct i2c_client *client = data->client;
>>  
>> -	regmap_field_read(data->reg_ps_gain, &index);
>> -	if (val > stk3310_ps_max[index])
>> +	ret = regmap_field_read(data->reg_ps_gain, &index);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (val < 0 || val > stk3310_ps_max[index])
>>  		return -EINVAL;
>>  
>>  	if (dir == IIO_EV_DIR_RISING)
>> @@ -266,9 +269,12 @@ static int stk3310_read_event_config(struct iio_dev *indio_dev,
>>  				     enum iio_event_direction dir)
>>  {
>>  	unsigned int event_val;
>> +	int ret;
>>  	struct stk3310_data *data = iio_priv(indio_dev);
>>  
>> -	regmap_field_read(data->reg_int_ps, &event_val);
>> +	ret = regmap_field_read(data->reg_int_ps, &event_val);
>> +	if (ret < 0)
>> +		return ret;
>>  
>>  	return event_val;
>>  }
>> @@ -307,14 +313,16 @@ static int stk3310_read_raw(struct iio_dev *indio_dev,
>>  	struct stk3310_data *data = iio_priv(indio_dev);
>>  	struct i2c_client *client = data->client;
>>  
>> +	if (chan->type != IIO_LIGHT && chan->type != IIO_PROXIMITY)
>> +		return -EINVAL;
>> +
>>  	switch (mask) {
>>  	case IIO_CHAN_INFO_RAW:
>>  		if (chan->type == IIO_LIGHT)
>>  			reg = STK3310_REG_ALS_DATA_MSB;
>> -		else if (chan->type == IIO_PROXIMITY)
>> -			reg = STK3310_REG_PS_DATA_MSB;
>>  		else
>> -			return -EINVAL;
>> +			reg = STK3310_REG_PS_DATA_MSB;
>> +
>>  		mutex_lock(&data->lock);
>>  		ret = regmap_bulk_read(data->regmap, reg, &buf, 2);
>>  		if (ret < 0) {
>> @@ -327,17 +335,23 @@ static int stk3310_read_raw(struct iio_dev *indio_dev,
>>  		return IIO_VAL_INT;
>>  	case IIO_CHAN_INFO_INT_TIME:
>>  		if (chan->type == IIO_LIGHT)
>> -			regmap_field_read(data->reg_als_it, &index);
>> +			ret = regmap_field_read(data->reg_als_it, &index);
>>  		else
>> -			regmap_field_read(data->reg_ps_it, &index);
>> +			ret = regmap_field_read(data->reg_ps_it, &index);
>> +		if (ret < 0)
>> +			return ret;
>> +
>>  		*val = stk3310_it_table[index][0];
>>  		*val2 = stk3310_it_table[index][1];
>>  		return IIO_VAL_INT_PLUS_MICRO;
>>  	case IIO_CHAN_INFO_SCALE:
>>  		if (chan->type == IIO_LIGHT)
>> -			regmap_field_read(data->reg_als_gain, &index);
>> +			ret = regmap_field_read(data->reg_als_gain, &index);
>>  		else
>> -			regmap_field_read(data->reg_ps_gain, &index);
>> +			ret = regmap_field_read(data->reg_ps_gain, &index);
>> +		if (ret < 0)
>> +			return ret;
>> +
>>  		*val = stk3310_scale_table[index][0];
>>  		*val2 = stk3310_scale_table[index][1];
>>  		return IIO_VAL_INT_PLUS_MICRO;
>> @@ -354,6 +368,9 @@ static int stk3310_write_raw(struct iio_dev *indio_dev,
>>  	int index;
>>  	struct stk3310_data *data = iio_priv(indio_dev);
>>  
>> +	if (chan->type != IIO_LIGHT && chan->type != IIO_PROXIMITY)
>> +		return -EINVAL;
>> +
>>  	switch (mask) {
>>  	case IIO_CHAN_INFO_INT_TIME:
>>  		index = stk3310_get_index(stk3310_it_table,
>> @@ -435,7 +452,10 @@ static int stk3310_init(struct iio_dev *indio_dev)
>>  	struct stk3310_data *data = iio_priv(indio_dev);
>>  	struct i2c_client *client = data->client;
>>  
>> -	regmap_read(data->regmap, STK3310_REG_ID, &chipid);
>> +	ret = regmap_read(data->regmap, STK3310_REG_ID, &chipid);
>> +	if (ret < 0)
>> +		return ret;
>> +
>>  	if (chipid != STK3310_CHIP_ID_VAL &&
>>  	    chipid != STK3311_CHIP_ID_VAL) {
>>  		dev_err(&client->dev, "invalid chip id: 0x%x\n", chipid);
>> @@ -608,8 +628,13 @@ static int stk3310_probe(struct i2c_client *client,
>>  	if (ret < 0)
>>  		return ret;
>>  
>> -	if (client->irq < 0)
>> +	if (client->irq < 0) {
>>  		client->irq = stk3310_gpio_probe(client);
>> +		if (client->irq < 0) {
>> +			ret = client->irq;
>> +			goto err_standby;
>> +		}
>> +	}
>>  
>>  	if (client->irq >= 0) {
>>  		ret = devm_request_threaded_irq(&client->dev, client->irq,
>> @@ -618,17 +643,23 @@ static int stk3310_probe(struct i2c_client *client,
>>  						IRQF_TRIGGER_FALLING |
>>  						IRQF_ONESHOT,
>>  						STK3310_EVENT, indio_dev);
>> -		if (ret < 0)
>> +		if (ret < 0) {
>>  			dev_err(&client->dev, "request irq %d failed\n",
>>  					client->irq);
>> +			goto err_standby;
>> +		}
>>  	}
>>  
>>  	ret = iio_device_register(indio_dev);
>>  	if (ret < 0) {
>>  		dev_err(&client->dev, "device_register failed\n");
>> -		stk3310_set_state(data, STK3310_STATE_STANDBY);
>> +		goto err_standby;
>>  	}
>>  
>> +	return 0;
>> +
>> +err_standby:
>> +	stk3310_set_state(data, STK3310_STATE_STANDBY);
>>  	return ret;
>>  }
>>  
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>