On 01/07/2014 11:27 PM, Peter Meerwald wrote:
> +static int ltr501_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct ltr501_data *data = iio_priv(indio_dev);
> + __le16 buf[2];
> + int ret, i;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (iio_buffer_enabled(indio_dev))
> + return -EBUSY;
> +
> + switch (chan->type) {
> + case IIO_INTENSITY:
> + ret = ltr501_drdy(data, LTR501_STATUS_ALS_RDY);
> + if (ret < 0)
> + return ret;
> + /* always read both ALS channels in given order */
> + ret = i2c_smbus_read_i2c_block_data(data->client,
> + LTR501_ALS_DATA1, sizeof(buf), (u8 *) buf);
> + if (ret < 0)
> + return ret;
> + *val = le16_to_cpu(chan->address == LTR501_ALS_DATA1 ?
> + buf[0] : buf[1]);
> + return IIO_VAL_INT;
> + case IIO_PROXIMITY:
> + ret = ltr501_drdy(data, LTR501_STATUS_PS_RDY);
> + if (ret < 0)
> + return ret;
> + ret = i2c_smbus_read_word_data(data->client,
> + chan->address);
> + if (ret < 0)
> + return ret;
> + *val = ret & LTR501_PS_DATA_MASK;
> + return IIO_VAL_INT;
Will anything bad happen if two processes concurrently read from the device?
> + }
[...]
> +
> +static int ltr501_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct ltr501_data *data;
> + struct iio_dev *indio_dev;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + data = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> + data->client = client;
> +
> + ret = i2c_smbus_read_byte_data(data->client, LTR501_PART_ID);
> + if (ret < 0)
> + return ret;
> +
> + dev_info(&client->dev, "LTR501 Ambient light/proximity sensor, Part number %02x, Rev: %02x\n",
> + ret >> 4, ret & 0xf);
I'm not a fan of those probe() dev_info()s. It becomes pretty useless if all
drivers start doing this as you'll end up with a endless streams of "Hello
here I am, look at me!!!".
> +
> + indio_dev->dev.parent = &client->dev;
> + indio_dev->info = <r501_info;
> + indio_dev->channels = ltr501_channels;
> + indio_dev->num_channels = ARRAY_SIZE(ltr501_channels);
> + indio_dev->name = LTR501_DRV_NAME;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + ret = ltr501_init(data);
> + if (ret < 0)
> + return ret;
> +
> + ret = iio_triggered_buffer_setup(indio_dev, NULL,
> + ltr501_trigger_handler, NULL);
> + if (ret)
> + return ret;
> +
> + ret = devm_iio_device_register(&client->dev, indio_dev);
This is one of the cases where we can not use devm_iio_device_register since
it changes the cleanup order. If you uss managed registration
iio_device_unregister will run after iio_triggered_buffer_cleanup and
ltr501_powerdown. Which means there is a chance of a race as the device will
still be accessible until iio_device_unregister has been called.
> + if (ret)
> + goto error_unreg_buffer;
> +
> + return 0;
> +
> +error_unreg_buffer:
> + iio_triggered_buffer_cleanup(indio_dev);
> + 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
