Re: [PATCH v7 1/2] Driver for TI HDC20x0 humidity and temperature sensors

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 27 Apr 2020 09:57:49 +0200
Eugene Zaikonnikov <eugene.zaikonnikov@norphonic.com> wrote:

> Hi Peter,
> 
> 
> On 27.04.2020 00:35, Peter Meerwald-Stadler wrote:
> >> +{
> >> +	struct i2c_client *client = data->client;
> >> +	s32 ret;
> >> +
> >> +	ret = i2c_smbus_read_byte_data(client,
> >> +			hdc2010_reg_translation[chan->address].peak);
> >> +
> >> +	if (ret < 0)
> >> +		dev_err(&client->dev, "Could not read sensor data\n");
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static int hdc2010_get_heater_status(struct hdc2010_data *data)  
> > return value should be bool?  
> 
> No idea. It is an int in hdc100x hdc heater sysfs and I try to stick to existing practices.
> 
> Should those be bools?

Locally it would make sense, but then we write them into an int where
they are used, so little point. Probably best to leave them as they are.

> 
> >  
> >> +	case IIO_CHAN_INFO_PEAK: {
> >> +		int ret;
> >> +
> >> +		ret = iio_device_claim_direct_mode(indio_dev);
> >> +		if (ret)
> >> +			return ret;
> >> +		mutex_lock(&data->lock);
> >> +		ret = hdc2010_get_peak_measurement_byte(data, chan);
> >> +		mutex_unlock(&data->lock);
> >> +		iio_device_release_direct_mode(indio_dev);
> >> +		if (ret < 0)
> >> +			return ret;
> >> +		/* Scaling up the value so we can use same offset as RAW */
> >> +		*val = ret * 256;  
> > I'd rather have different _SCALEs for peak and raw  
> 
> They are made shared per Jonathan's suggestion early on, as the offsets for the channels don't match.

Yup. There is no standard ABI to deal with different scales for peak and
raw and I wasn't keen that we introduce it for this relatively rare
corner case.

Jonathan

> 
> 
> Regards,
> 
>    Eugene.
>