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@xxxxxx> > 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@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html