| Message ID | 20180421064730.11358-1-matt.ranostay@konsulko.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 20 Apr 2018 23:47:30 -0700 Matt Ranostay <matt.ranostay@konsulko.com> wrote: > Lookup table was highly dependent on using the LMP91000EVM which had an > 16-bit TI ADC161S626 but it is mistake to assume all applications will > be using that ADC part. Any processing should be done in the respective > userspace application. > > Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com> Hohum. The problem here is that now, any users that are out there will have code based on the conversion done in this table. It should at least be linear (I think) off the back of this table? If so we should probably have reported it as raw (as there is an unknown scale still to apply) but the table is otherwise valid (I think!) Jonathan > --- > drivers/iio/potentiostat/lmp91000.c | 38 ++++--------------------------------- > 1 file changed, 4 insertions(+), 34 deletions(-) > > diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c > index 85714055cc74..e464f72b144b 100644 > --- a/drivers/iio/potentiostat/lmp91000.c > +++ b/drivers/iio/potentiostat/lmp91000.c > @@ -41,21 +41,6 @@ static const int lmp91000_rload[] = { 10, 33, 50, 100 }; > > #define LMP91000_TEMP_BASE -40 > > -static const u16 lmp91000_temp_lut[] = { > - 1875, 1867, 1860, 1852, 1844, 1836, 1828, 1821, 1813, 1805, > - 1797, 1789, 1782, 1774, 1766, 1758, 1750, 1742, 1734, 1727, > - 1719, 1711, 1703, 1695, 1687, 1679, 1671, 1663, 1656, 1648, > - 1640, 1632, 1624, 1616, 1608, 1600, 1592, 1584, 1576, 1568, > - 1560, 1552, 1544, 1536, 1528, 1520, 1512, 1504, 1496, 1488, > - 1480, 1472, 1464, 1456, 1448, 1440, 1432, 1424, 1415, 1407, > - 1399, 1391, 1383, 1375, 1367, 1359, 1351, 1342, 1334, 1326, > - 1318, 1310, 1302, 1293, 1285, 1277, 1269, 1261, 1253, 1244, > - 1236, 1228, 1220, 1212, 1203, 1195, 1187, 1179, 1170, 1162, > - 1154, 1146, 1137, 1129, 1121, 1112, 1104, 1096, 1087, 1079, > - 1071, 1063, 1054, 1046, 1038, 1029, 1021, 1012, 1004, 996, > - 987, 979, 971, 962, 954, 945, 937, 929, 920, 912, > - 903, 895, 886, 878, 870, 861 }; > - > static const struct regmap_config lmp91000_regmap_config = { > .reg_bits = 8, > .val_bits = 8, > @@ -95,7 +80,7 @@ static const struct iio_chan_spec lmp91000_channels[] = { > .type = IIO_TEMP, > .channel = 1, > .address = LMP91000_REG_MODECN_TEMP, > - .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > .scan_index = -1, > }, > }; > @@ -157,11 +142,11 @@ static int lmp91000_read_raw(struct iio_dev *indio_dev, > int *val, int *val2, long mask) > { > struct lmp91000_data *data = iio_priv(indio_dev); > + int ret = -EINVAL; > > switch (mask) { > case IIO_CHAN_INFO_RAW: > - case IIO_CHAN_INFO_PROCESSED: { > - int ret = iio_channel_start_all_cb(data->cb_buffer); > + ret = iio_channel_start_all_cb(data->cb_buffer); > > if (ret) > return ret; > @@ -173,29 +158,14 @@ static int lmp91000_read_raw(struct iio_dev *indio_dev, > if (ret) > return ret; > > - if (mask == IIO_CHAN_INFO_PROCESSED) { > - int tmp, i; > - > - ret = iio_convert_raw_to_processed(data->adc_chan, > - *val, &tmp, 1); > - if (ret) > - return ret; > - > - for (i = 0; i < ARRAY_SIZE(lmp91000_temp_lut); i++) > - if (lmp91000_temp_lut[i] < tmp) > - break; > - > - *val = (LMP91000_TEMP_BASE + i) * 1000; > - } > return IIO_VAL_INT; > - } > case IIO_CHAN_INFO_OFFSET: > return iio_read_channel_offset(data->adc_chan, val, val2); > case IIO_CHAN_INFO_SCALE: > return iio_read_channel_scale(data->adc_chan, val, val2); > } > > - return -EINVAL; > + return ret; > } > > static const struct iio_info lmp91000_info = { -- 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
On Sat, Apr 21, 2018 at 9:41 AM, Jonathan Cameron <jic23@kernel.org> wrote: > On Fri, 20 Apr 2018 23:47:30 -0700 > Matt Ranostay <matt.ranostay@konsulko.com> wrote: > >> Lookup table was highly dependent on using the LMP91000EVM which had an >> 16-bit TI ADC161S626 but it is mistake to assume all applications will >> be using that ADC part. Any processing should be done in the respective >> userspace application. >> >> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com> > Hohum. The problem here is that now, any users that are out there will > have code based on the conversion done in this table. Yeah I had a feeling you or someone would comment on that :). > It should at least be linear (I think) off the back of this table? > If so we should probably have reported it as raw (as there is an unknown > scale still to apply) but the table is otherwise valid (I think!) > Yeah should have always been reported as raw. I'll have to think of a better solution, and it may not even be an issue because we are using iio_convert_raw_to_processed() on the channel data and it should error out if it can't do it. - Matt > Jonathan > >> --- >> drivers/iio/potentiostat/lmp91000.c | 38 ++++--------------------------------- >> 1 file changed, 4 insertions(+), 34 deletions(-) >> >> diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c >> index 85714055cc74..e464f72b144b 100644 >> --- a/drivers/iio/potentiostat/lmp91000.c >> +++ b/drivers/iio/potentiostat/lmp91000.c >> @@ -41,21 +41,6 @@ static const int lmp91000_rload[] = { 10, 33, 50, 100 }; >> >> #define LMP91000_TEMP_BASE -40 >> >> -static const u16 lmp91000_temp_lut[] = { >> - 1875, 1867, 1860, 1852, 1844, 1836, 1828, 1821, 1813, 1805, >> - 1797, 1789, 1782, 1774, 1766, 1758, 1750, 1742, 1734, 1727, >> - 1719, 1711, 1703, 1695, 1687, 1679, 1671, 1663, 1656, 1648, >> - 1640, 1632, 1624, 1616, 1608, 1600, 1592, 1584, 1576, 1568, >> - 1560, 1552, 1544, 1536, 1528, 1520, 1512, 1504, 1496, 1488, >> - 1480, 1472, 1464, 1456, 1448, 1440, 1432, 1424, 1415, 1407, >> - 1399, 1391, 1383, 1375, 1367, 1359, 1351, 1342, 1334, 1326, >> - 1318, 1310, 1302, 1293, 1285, 1277, 1269, 1261, 1253, 1244, >> - 1236, 1228, 1220, 1212, 1203, 1195, 1187, 1179, 1170, 1162, >> - 1154, 1146, 1137, 1129, 1121, 1112, 1104, 1096, 1087, 1079, >> - 1071, 1063, 1054, 1046, 1038, 1029, 1021, 1012, 1004, 996, >> - 987, 979, 971, 962, 954, 945, 937, 929, 920, 912, >> - 903, 895, 886, 878, 870, 861 }; >> - >> static const struct regmap_config lmp91000_regmap_config = { >> .reg_bits = 8, >> .val_bits = 8, >> @@ -95,7 +80,7 @@ static const struct iio_chan_spec lmp91000_channels[] = { >> .type = IIO_TEMP, >> .channel = 1, >> .address = LMP91000_REG_MODECN_TEMP, >> - .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), >> .scan_index = -1, >> }, >> }; >> @@ -157,11 +142,11 @@ static int lmp91000_read_raw(struct iio_dev *indio_dev, >> int *val, int *val2, long mask) >> { >> struct lmp91000_data *data = iio_priv(indio_dev); >> + int ret = -EINVAL; >> >> switch (mask) { >> case IIO_CHAN_INFO_RAW: >> - case IIO_CHAN_INFO_PROCESSED: { >> - int ret = iio_channel_start_all_cb(data->cb_buffer); >> + ret = iio_channel_start_all_cb(data->cb_buffer); >> >> if (ret) >> return ret; >> @@ -173,29 +158,14 @@ static int lmp91000_read_raw(struct iio_dev *indio_dev, >> if (ret) >> return ret; >> >> - if (mask == IIO_CHAN_INFO_PROCESSED) { >> - int tmp, i; >> - >> - ret = iio_convert_raw_to_processed(data->adc_chan, >> - *val, &tmp, 1); >> - if (ret) >> - return ret; >> - >> - for (i = 0; i < ARRAY_SIZE(lmp91000_temp_lut); i++) >> - if (lmp91000_temp_lut[i] < tmp) >> - break; >> - >> - *val = (LMP91000_TEMP_BASE + i) * 1000; >> - } >> return IIO_VAL_INT; >> - } >> case IIO_CHAN_INFO_OFFSET: >> return iio_read_channel_offset(data->adc_chan, val, val2); >> case IIO_CHAN_INFO_SCALE: >> return iio_read_channel_scale(data->adc_chan, val, val2); >> } >> >> - return -EINVAL; >> + return ret; >> } >> >> static const struct iio_info lmp91000_info = { > -- 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
diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c index 85714055cc74..e464f72b144b 100644 --- a/drivers/iio/potentiostat/lmp91000.c +++ b/drivers/iio/potentiostat/lmp91000.c @@ -41,21 +41,6 @@ static const int lmp91000_rload[] = { 10, 33, 50, 100 }; #define LMP91000_TEMP_BASE -40 -static const u16 lmp91000_temp_lut[] = { - 1875, 1867, 1860, 1852, 1844, 1836, 1828, 1821, 1813, 1805, - 1797, 1789, 1782, 1774, 1766, 1758, 1750, 1742, 1734, 1727, - 1719, 1711, 1703, 1695, 1687, 1679, 1671, 1663, 1656, 1648, - 1640, 1632, 1624, 1616, 1608, 1600, 1592, 1584, 1576, 1568, - 1560, 1552, 1544, 1536, 1528, 1520, 1512, 1504, 1496, 1488, - 1480, 1472, 1464, 1456, 1448, 1440, 1432, 1424, 1415, 1407, - 1399, 1391, 1383, 1375, 1367, 1359, 1351, 1342, 1334, 1326, - 1318, 1310, 1302, 1293, 1285, 1277, 1269, 1261, 1253, 1244, - 1236, 1228, 1220, 1212, 1203, 1195, 1187, 1179, 1170, 1162, - 1154, 1146, 1137, 1129, 1121, 1112, 1104, 1096, 1087, 1079, - 1071, 1063, 1054, 1046, 1038, 1029, 1021, 1012, 1004, 996, - 987, 979, 971, 962, 954, 945, 937, 929, 920, 912, - 903, 895, 886, 878, 870, 861 }; - static const struct regmap_config lmp91000_regmap_config = { .reg_bits = 8, .val_bits = 8, @@ -95,7 +80,7 @@ static const struct iio_chan_spec lmp91000_channels[] = { .type = IIO_TEMP, .channel = 1, .address = LMP91000_REG_MODECN_TEMP, - .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), .scan_index = -1, }, }; @@ -157,11 +142,11 @@ static int lmp91000_read_raw(struct iio_dev *indio_dev, int *val, int *val2, long mask) { struct lmp91000_data *data = iio_priv(indio_dev); + int ret = -EINVAL; switch (mask) { case IIO_CHAN_INFO_RAW: - case IIO_CHAN_INFO_PROCESSED: { - int ret = iio_channel_start_all_cb(data->cb_buffer); + ret = iio_channel_start_all_cb(data->cb_buffer); if (ret) return ret; @@ -173,29 +158,14 @@ static int lmp91000_read_raw(struct iio_dev *indio_dev, if (ret) return ret; - if (mask == IIO_CHAN_INFO_PROCESSED) { - int tmp, i; - - ret = iio_convert_raw_to_processed(data->adc_chan, - *val, &tmp, 1); - if (ret) - return ret; - - for (i = 0; i < ARRAY_SIZE(lmp91000_temp_lut); i++) - if (lmp91000_temp_lut[i] < tmp) - break; - - *val = (LMP91000_TEMP_BASE + i) * 1000; - } return IIO_VAL_INT; - } case IIO_CHAN_INFO_OFFSET: return iio_read_channel_offset(data->adc_chan, val, val2); case IIO_CHAN_INFO_SCALE: return iio_read_channel_scale(data->adc_chan, val, val2); } - return -EINVAL; + return ret; } static const struct iio_info lmp91000_info = {
Lookup table was highly dependent on using the LMP91000EVM which had an 16-bit TI ADC161S626 but it is mistake to assume all applications will be using that ADC part. Any processing should be done in the respective userspace application. Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com> --- drivers/iio/potentiostat/lmp91000.c | 38 ++++--------------------------------- 1 file changed, 4 insertions(+), 34 deletions(-)