On Sun, May 8, 2016 at 12:29 PM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > On 05/05/16 09:17, Matt Ranostay wrote: >> On Wed, May 4, 2016 at 3:39 AM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote: >>> On 24/04/16 21:43, Matt Ranostay wrote: >>> Give us a bit more description - what is it, what is it for, what interface is >>> supplied. Err what does EC standard for? >>>> Signed-off-by: Matt Ranostay <mranostay@xxxxxxxxx> >>> I know next to nothing about these sensors and don't have the datasheet to hand. >>> >>> This is measuring 3 different types of concentration (I think) and reporting them >>> simple as channels 0-2. What are they are should we not be distinguishing between >>> them in some more informative fashion? >> >> Should I add some .extend_name for each channel? > probably not. What are these 3 channels actually measuring? > As I understand it from search the datasheet for tds... > We have > > electric conductivity (that's not a concentration channel to my mind)... > Perhaps have it as a new channel type in of itself... Ok no problem of doing that. Just that I was using a known conversion value of EC (salts) to ppm. I have no issue with adding yet another weird iio chan type :) Would a IIO_EC make more sense than IIO_US_PER_CM (microseimens per centimeter)? The latter could be used by some meters in the future. > > total dissolved solids (TDS) which kind of sounds concentration like to me... > However, we might want to consider an appropriate modifier to say 'of what?'. > > PSS is salinity? Looks like this is a temperature adjusted concentration scale? > (I'm reading wikipedia and not really following it... Yeah that is correct.. like the pH sensor temperature affects the reading greatly. > > It might make sense to define a whole new channel type for this... I'm not keen. on having modifiers effect the units so channel type may be the only way to do it > coherently.. > > I have this nasty feeling chemical measurements are going to introduce all sorts > of weird ways of measuring 'concentrations' according to what actually happens > to work for a given chemical.. > > Ah well. 27 channel types so far - bit of room left before we discover allowing > 256 in the event descriptors wasn't thinking forwards enough... > >> Horribly I have to admit my pH sensor based board doesn't give valid >> values for this sensor till I resign the ground plane under the IC :/. >> But I can confirm it does report some "values"... > :) >> >>> >>> Code itself is mostly fine... >>> >>> Jonathan >>>> --- >>>> .../bindings/iio/chemical/atlas,ec-sm.txt | 22 ++++ >>>> drivers/iio/chemical/Kconfig | 8 +- >>>> drivers/iio/chemical/atlas-ph-sensor.c | 113 ++++++++++++++++++++- >>>> 3 files changed, 138 insertions(+), 5 deletions(-) >>>> create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>>> new file mode 100644 >>>> index 0000000..2962bd9 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>>> @@ -0,0 +1,22 @@ >>>> +* Atlas Scientific EC-SM OEM sensor >>>> + >>>> +http://www.atlas-scientific.com/_files/_datasheets/_oem/EC_oem_datasheet.pdf >>>> + >>>> +Required properties: >>>> + >>>> + - compatible: must be "atlas,ec-sm" >>>> + - reg: the I2C address of the sensor >>>> + - interrupt-parent: should be the phandle for the interrupt controller >>>> + - interrupts: the sole interrupt generated by the device >>>> + >>>> + Refer to interrupt-controller/interrupts.txt for generic interrupt client >>>> + node bindings. >>>> + >>>> +Example: >>>> + >>>> +atlas@64 { >>>> + compatible = "atlas,ec-sm"; >>>> + reg = <0x64>; >>>> + interrupt-parent = <&gpio1>; >>>> + interrupts = <16 2>; >>>> +}; >>>> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig >>>> index f73290f..4bcc025 100644 >>>> --- a/drivers/iio/chemical/Kconfig >>>> +++ b/drivers/iio/chemical/Kconfig >>>> @@ -5,15 +5,17 @@ >>>> menu "Chemical Sensors" >>>> >>>> config ATLAS_PH_SENSOR >>>> - tristate "Atlas Scientific OEM pH-SM sensor" >>>> + tristate "Atlas Scientific OEM SM sensors" >>>> depends on I2C >>>> select REGMAP_I2C >>>> select IIO_BUFFER >>>> select IIO_TRIGGERED_BUFFER >>>> select IRQ_WORK >>>> help >>>> - Say Y here to build I2C interface support for the Atlas >>>> - Scientific OEM pH-SM sensor. >>>> + Say Y here to build I2C interface support for the following >>>> + Atlas Scientific OEM SM sensors: >>>> + * pH SM sensor >>>> + * EC SM sensor >>>> >>>> To compile this driver as module, choose M here: the >>>> module will be called atlas-ph-sensor. >>>> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c >>>> index c57f9c2..6dddd75 100644 >>>> --- a/drivers/iio/chemical/atlas-ph-sensor.c >>>> +++ b/drivers/iio/chemical/atlas-ph-sensor.c >>>> @@ -50,13 +50,28 @@ >>>> #define ATLAS_REG_PH_CALIB_STATUS_MID BIT(1) >>>> #define ATLAS_REG_PH_CALIB_STATUS_HIGH BIT(2) >>>> >>>> +#define ATLAS_REG_EC_CALIB_STATUS 0x0f >>>> +#define ATLAS_REG_EC_CALIB_STATUS_MASK 0x0f >>>> +#define ATLAS_REG_EC_CALIB_STATUS_DRY BIT(0) >>>> +#define ATLAS_REG_EC_CALIB_STATUS_SINGLE BIT(1) >>>> +#define ATLAS_REG_EC_CALIB_STATUS_LOW BIT(2) >>>> +#define ATLAS_REG_EC_CALIB_STATUS_HIGH BIT(3) >>>> + >>>> #define ATLAS_REG_PH_TEMP_DATA 0x0e >>>> #define ATLAS_REG_PH_DATA 0x16 >>>> >>>> +#define ATLAS_REG_EC_PROBE 0x08 >>>> +#define ATLAS_REG_EC_TEMP_DATA 0x10 >>>> +#define ATLAS_REG_EC_DATA 0x18 >>>> +#define ATLAS_REG_TDS_DATA 0x1c >>>> +#define ATLAS_REG_PSS_DATA 0x20 >>>> + >>>> #define ATLAS_PH_INT_TIME_IN_US 450000 >>>> +#define ATLAS_EC_INT_TIME_IN_US 650000 >>>> >>>> enum { >>>> ATLAS_PH_SM, >>>> + ATLAS_EC_SM, >>>> }; >>>> >>>> struct atlas_data { >>>> @@ -66,12 +81,13 @@ struct atlas_data { >>>> struct regmap *regmap; >>>> struct irq_work work; >>>> >>>> - __be32 buffer[4]; /* 32-bit pH data + 32-bit pad + 64-bit timestamp */ >>>> + __be32 buffer[6]; /* 96-bit data + 32-bit pad + 64-bit timestamp */ >>>> }; >>>> >>>> static const struct regmap_range atlas_volatile_ranges[] = { >>>> regmap_reg_range(ATLAS_REG_INT_CONTROL, ATLAS_REG_INT_CONTROL), >>>> regmap_reg_range(ATLAS_REG_PH_DATA, ATLAS_REG_PH_DATA + 4), >>>> + regmap_reg_range(ATLAS_REG_EC_DATA, ATLAS_REG_PSS_DATA + 4), >>>> }; >>>> >>>> static const struct regmap_access_table atlas_volatile_table = { >>>> @@ -86,7 +102,7 @@ static const struct regmap_config atlas_regmap_config = { >>>> .val_bits = 8, >>>> >>>> .volatile_table = &atlas_volatile_table, >>>> - .max_register = ATLAS_REG_PH_DATA + 4, >>>> + .max_register = ATLAS_REG_PSS_DATA + 4, >>>> .cache_type = REGCACHE_RBTREE, >>>> }; >>>> >>>> @@ -115,6 +131,38 @@ static const struct iio_chan_spec atlas_ph_channels[] = { >>>> }, >>>> }; >>>> >>>> +#define ATLAS_EC_CHANNEL(idx, addr) \ >>>> + {\ >>>> + .type = IIO_CONCENTRATION, \ >>>> + .indexed = 1, \ >>>> + .channel = idx, \ >>>> + .address = addr, \ >>>> + .info_mask_separate = \ >>>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), \ >>>> + .scan_index = idx, \ >>>> + .scan_type = { \ >>>> + .sign = 'u', \ >>>> + .realbits = 32, \ >>>> + .storagebits = 32, \ >>>> + .endianness = IIO_BE, \ >>>> + }, \ >>>> + } >>>> + >>>> +static const struct iio_chan_spec atlas_ec_channels[] = { >>>> + ATLAS_EC_CHANNEL(0, ATLAS_REG_EC_DATA), >>>> + ATLAS_EC_CHANNEL(1, ATLAS_REG_TDS_DATA), >>>> + ATLAS_EC_CHANNEL(2, ATLAS_REG_PSS_DATA), >>> This needs some explanation. 3 concentration channels of different types? >>> What are they... >>>> + IIO_CHAN_SOFT_TIMESTAMP(3), >>>> + { >>>> + .type = IIO_TEMP, >>>> + .address = ATLAS_REG_EC_TEMP_DATA, >>>> + .info_mask_separate = >>>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), >>>> + .output = 1, >>>> + .scan_index = -1 >>>> + }, >>>> +}; >>>> + >>>> static int atlas_check_ph_calibration(struct atlas_data *data) >>>> { >>>> struct device *dev = &data->client->dev; >>>> @@ -142,6 +190,44 @@ static int atlas_check_ph_calibration(struct atlas_data *data) >>>> return 0; >>>> } >>>> >>>> +static int atlas_check_ec_calibration(struct atlas_data *data) >>>> +{ >>>> + struct device *dev = &data->client->dev; >>>> + int ret; >>>> + unsigned int val; >>>> + >>>> + ret = regmap_bulk_read(data->regmap, ATLAS_REG_EC_PROBE, &val, 2); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + dev_info(dev, "probe set to K = %d.%.2d", be16_to_cpu(val) / 100, >>>> + be16_to_cpu(val) % 100); >>>> + >>>> + ret = regmap_read(data->regmap, ATLAS_REG_EC_CALIB_STATUS, &val); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_MASK)) { >>>> + dev_warn(dev, "device has not been calibrated\n"); >>>> + return 0; >>>> + } >>>> + >>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_DRY)) >>>> + dev_warn(dev, "device missing dry point calibration\n"); >>>> + >>>> + if (val & ATLAS_REG_EC_CALIB_STATUS_SINGLE) { >>>> + dev_warn(dev, "device using single point calibration\n"); >>>> + } else { >>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_LOW)) >>>> + dev_warn(dev, "device missing low point calibration\n"); >>>> + >>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_HIGH)) >>>> + dev_warn(dev, "device missing high point calibration\n"); >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> struct atlas_device { >>>> const struct iio_chan_spec *channels; >>>> int num_channels; >>>> @@ -159,6 +245,14 @@ static struct atlas_device atlas_devices[] = { >>>> .calibration = &atlas_check_ph_calibration, >>>> .delay = ATLAS_PH_INT_TIME_IN_US, >>>> }, >>>> + [ATLAS_EC_SM] = { >>>> + .channels = atlas_ec_channels, >>>> + .num_channels = 5, >>> Use array_size - obviously correct that way so I don't have to go count... >>>> + .data_reg = ATLAS_REG_EC_DATA, >>>> + .calibration = &atlas_check_ec_calibration, >>>> + .delay = ATLAS_EC_INT_TIME_IN_US, >>>> + }, >>>> + >>>> }; >>>> >>>> static int atlas_set_powermode(struct atlas_data *data, int on) >>>> @@ -294,6 +388,7 @@ static int atlas_read_raw(struct iio_dev *indio_dev, >>>> (u8 *) ®, sizeof(reg)); >>> I guess it's not complex enough to justify splitting read_raw for the two >>> devices. Close though so you may want to if you support a third type in here. >>> >>>> break; >>>> case IIO_PH: >>>> + case IIO_CONCENTRATION: >>>> mutex_lock(&indio_dev->mlock); >>>> >>>> if (iio_buffer_enabled(indio_dev)) >>>> @@ -324,6 +419,18 @@ static int atlas_read_raw(struct iio_dev *indio_dev, >>>> *val = 1; /* 0.001 */ >>>> *val2 = 1000; >>>> break; >>>> + case IIO_CONCENTRATION: >>>> + switch (chan->address) { >>>> + case ATLAS_REG_EC_DATA: >>>> + *val = 0; /* 0.00000000064 */ >>>> + *val2 = 640; >>>> + return IIO_VAL_INT_PLUS_NANO; >>>> + default: >>>> + *val = 0; /* 0.000000001 */ >>>> + *val2 = 1000; >>>> + return IIO_VAL_INT_PLUS_NANO; >>>> + } >>>> + break; >>>> default: >>>> return -EINVAL; >>>> } >>>> @@ -517,12 +624,14 @@ static const struct dev_pm_ops atlas_pm_ops = { >>>> >>>> static const struct i2c_device_id atlas_id[] = { >>>> { "atlas-ph-sm", ATLAS_PH_SM}, >>>> + { "atlas-ec-sm", ATLAS_EC_SM}, >>>> {} >>>> }; >>>> MODULE_DEVICE_TABLE(i2c, atlas_id); >>>> >>>> static const struct of_device_id atlas_dt_ids[] = { >>>> { .compatible = "atlas,ph-sm", .data = (void *)ATLAS_PH_SM, }, >>>> + { .compatible = "atlas,ec-sm", .data = (void *)ATLAS_EC_SM, }, >>>> { } >>>> }; >>>> MODULE_DEVICE_TABLE(of, atlas_dt_ids); >>>> >>> >> -- >> 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 >> > -- 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
