Re: [PATCH v4 10/10] iio: adc: ti-ads1015: Switch to read_avail

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/28/22 18:52, Jonathan Cameron wrote:
On Mon, 28 Mar 2022 02:31:32 +0200
Marek Vasut <marex@xxxxxxx> wrote:

On 3/27/22 17:18, Jonathan Cameron wrote:
On Tue, 22 Mar 2022 23:02:10 +0100
Marek Vasut <marex@xxxxxxx> wrote:
Replace sysfs attributes with read_avail() callback. This also permits
removal of ads1115_info, since the scale attribute tables are now part
of chip data.

Signed-off-by: Marek Vasut <marex@xxxxxxx>
Cc: Andy Shevchenko <andy@xxxxxxxxxx>
Cc: Daniel Baluta <daniel.baluta@xxxxxxx>
Cc: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>

Sorry, I didn't catch your question on v3 about why I was advocating
signed.

You are passing pointers to those arrays as signed in the
read_avail.

Obviously you can 'get away with it' because the values are small
positive numbers and hence in 2's complement the data representation
will be the same.  Not pretty though so my inclination would
be to keep them signed everywhere.

If you are fine with that change I can change it whilst applying if
nothing else comes up in review.

I'm fine with it, although I did switch them all to unsigned int in this
V4 (unless I'm missing something still).

You switched to unsigned int, but...

+static int ads1015_read_avail(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      const int **vals, int *type, int *length,
+			      long mask)
+{
+	struct ads1015_data *data = iio_priv(indio_dev);
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*type = IIO_VAL_FRACTIONAL_LOG2;
+		*vals =  data->chip->scale;

This then uses them as signed integers.  Which as mentioned is
technically not a bug, but that's only because the numbers
are small..  So better to go signed throughout.

Ah, I see.



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux