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? 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
