Re: [PATCH v3 2/6] Add tsys01 meas-spec driver support

[Jonathan Cameron <jic23@kernel.org>]

On 29/09/15 10:36, ludovic.tancerel@maplehightech.com wrote:
> 
> Thank you for reviewing
> Please have a look at comments below,
> 
> Regards,
> Ludovic
> 
> Le 27 sept. 2015 à 18:55, Jonathan Cameron <jic23@kernel.org> a écrit :
> 
>> On 25/09/15 14:56, Ludovic Tancerel wrote:
>>> Support for TSYS01 temperature sensor
>>>
>>> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
>> This is fine as far as i am concerned, though I would like to leave it
>> on the list for a little while for others to have a chance to comment.
>>> ---
>>> drivers/iio/temperature/Kconfig  |  11 ++
>>> drivers/iio/temperature/Makefile |   1 +
>>> drivers/iio/temperature/tsys01.c | 231 +++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 243 insertions(+)
>>> create mode 100644 drivers/iio/temperature/tsys01.c
>>>
>>>
> …
> 
>>> +static int tsys01_i2c_probe(struct i2c_client *client,
>>> +			    const struct i2c_device_id *id)
>>> +{
>>> +	struct tsys01_dev *dev_data;
>>> +	struct iio_dev *indio_dev;
>>> +
>>> +	if (!i2c_check_functionality(client->adapter,
>>> +				     I2C_FUNC_SMBUS_WORD_DATA |
>>> +				     I2C_FUNC_SMBUS_WRITE_BYTE |
>>> +				     I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
>>> +		dev_err(&client->dev,
>>> +			"Adapter does not support some i2c transaction\n");
>>> +		return -ENODEV;
>>> +	}
>>> +
>>> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*dev_data));
>>> +	if (!indio_dev)
>>> +		return -ENOMEM;
>>> +
>>> +	dev_data = iio_priv(indio_dev);
>>> +	dev_data->client = client;
>>> +	dev_data->reset = ms_sensors_i2c_reset;
>>> +	dev_data->read_prom_word = ms_sensors_i2c_read_prom_word;
>>> +	dev_data->convert_and_read = ms_sensors_i2c_convert_and_read;
>>> +
>>> +	i2c_set_clientdata(client, indio_dev);
>>> +
>> This separation into i2c probe and main probe is something one would
>> generally only introduce at the point of adding support for another bus.
>> It just adds complexity here for little gain.  If there is intent
>> to add spi support shortly then fair enough, leave it as it is.
> 
> I don’t plan on doing the SPI stuff myself,
> so if you feel I should simplify as there is no plan to do it, please let me know.
> 
> I prefer to keep it as I made this change especially to comply a
> request to have this similar to other existing meas-spec drivers
> (ms5611).
It's not something I feel strongly about so leave it as it is.

> 
>>> +	return tsys01_probe(indio_dev, &client->dev);
>>> +}
>>> +
>>> +static const struct i2c_device_id tsys01_id[] = {
>>> +	{"tsys01", 0},
>>> +	{}
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, tsys01_id);
>>> +
>>> +static struct i2c_driver tsys01_driver = {
>>> +	.probe = tsys01_i2c_probe,
>>> +	.id_table = tsys01_id,
>>> +	.driver = {
>>> +		   .name = "tsys01",
>>> +		   },
>>> +};
>>> +
>>> +module_i2c_driver(tsys01_driver);
>>> +
>>> +MODULE_DESCRIPTION("Measurement-Specialties tsys01 temperature driver");
>>> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
>>> +MODULE_AUTHOR("Ludovic Tancerel <ludovic.tancerel@maplehightech.com>");
>>> +MODULE_LICENSE("GPL v2");
>>>
>>
>> --
>> 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
> 
> --
> 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
>