On 15/04/16 16:36, Tiberiu Breana wrote: > Add basic support for the Aosong AM2315 ambient temperature > and humidity sensor. > Includes support for raw readings and ACPI detection. > > Datasheet: > http://www.aosong.com/asp_bin/Products/en/AM2315.pdf > > Signed-off-by: Tiberiu Breana <tiberiu.a.breana@xxxxxxxxx> I think I'd prefer this in drivers/iio/humidty/. If people just want a temperature sensor they'll not use the much more complex device needed to measure humidity, whereas to measure humidity you have to know what the temperature is. Hence I'd argue that here the humidity side of the device in the primary one. 'Interestingly' temperature and humidity seems to sound better to me in English than humidity and temperature... hohum. I suppose we could roll these two directories together if people think that is sensible? Various little bits inline.. Jonathan > --- > drivers/iio/temperature/Kconfig | 10 ++ > drivers/iio/temperature/Makefile | 1 + > drivers/iio/temperature/am2315.c | 216 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 227 insertions(+) > create mode 100644 drivers/iio/temperature/am2315.c > > diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig > index c4664e5..79e4409 100644 > --- a/drivers/iio/temperature/Kconfig > +++ b/drivers/iio/temperature/Kconfig > @@ -3,6 +3,16 @@ > # > menu "Temperature sensors" > > +config AM2315 > + tristate "Aosong AM2315 temperature and humidity sensor" > + depends on I2C > + help > + If you say yes here you get support for the Aosong AM2315 > + ambient temperature and humidity sensor. > + > + This driver can also be built as a module. If so, the module will > + be called am2315. > + > config MLX90614 > tristate "MLX90614 contact-less infrared sensor" > depends on I2C > diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile > index 02bc79d..2e7521b 100644 > --- a/drivers/iio/temperature/Makefile > +++ b/drivers/iio/temperature/Makefile > @@ -2,6 +2,7 @@ > # Makefile for industrial I/O temperature drivers > # > > +obj-$(CONFIG_AM2315) += am2315.o > obj-$(CONFIG_MLX90614) += mlx90614.o > obj-$(CONFIG_TMP006) += tmp006.o > obj-$(CONFIG_TSYS01) += tsys01.o > diff --git a/drivers/iio/temperature/am2315.c b/drivers/iio/temperature/am2315.c > new file mode 100644 > index 0000000..2642bae > --- /dev/null > +++ b/drivers/iio/temperature/am2315.c > @@ -0,0 +1,216 @@ > +/** > + * Aosong AM2315 ambient temperature & humidity sensor driver > + * > + * Copyright (c) 2016, Intel Corporation. > + * > + * This file is subject to the terms and conditions of version 2 of > + * the GNU General Public License. See the file COPYING in the main > + * directory of this archive for more details. > + * > + * 7-bit I2C address: 0x5C. > + */ > + > +#include <linux/acpi.h> > +#include <linux/delay.h> > +#include <linux/i2c.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > + > +#define AM2315_REG_HUM_MSB 0x00 > +#define AM2315_REG_HUM_LSB 0x01 > +#define AM2315_REG_TEMP_MSB 0x02 > +#define AM2315_REG_TEMP_LSB 0x03 > + > +#define AM2315_FUNCTION_READ 0x03 > +#define AM2315_HUM_OFFSET 0 > +#define AM2315_TEMP_OFFSET 2 > + > +#define AM2315_DRIVER_NAME "am2315" > + > +struct am2315_data { > + struct i2c_client *client; > +}; > + > +static const struct iio_chan_spec am2315_channels[] = { > + { > + .type = IIO_HUMIDITYRELATIVE, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_SCALE) > + }, > + { > + .type = IIO_TEMP, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_SCALE) > + }, > +}; > + > +/* CRC calculation algorithm, as specified in the datasheet (page 13). */ > +static u16 am2315_crc(u8 *data, u8 nr_bytes) I think this 'might' be reverse CRC16-IBM according to wikipedia... Which is obscure enough that it probably doesn't have a generic implementation already in the kernel... There is one other user of this polynomial that I can fine, not enough to justify a common implementation at this point I guess. Note that 16bit polynomials tend to get implemented as lookup tables if generalized. Speed isn't that important here though I guess. > +{ > + int i; > + u16 crc = 0xffff; > + > + while (nr_bytes--) { > + crc ^= *data++; > + for (i = 0; i < 8; i++) { > + if (crc & 0x01) { > + crc >>= 1; > + crc ^= 0xA001; > + } else { > + crc >>= 1; > + } > + } > + } > + > + return crc; > +} > + > +/* Simple function that sends a few bytes to the device to wake it up. */ We always like novel hardware corners <groans> > +static inline void am2315_ping(struct i2c_client *client) > +{ > + i2c_smbus_read_byte_data(client, AM2315_REG_HUM_MSB); > +} > + > +static int am2315_read_data(struct i2c_client *client, u8 *rx_buf) > +{ > + int ret; > + /* tx_buf format: <function code> <start addr> <nr of regs to read> */ > + u8 tx_buf[3] = { AM2315_FUNCTION_READ, AM2315_REG_HUM_MSB, 4 }; > + u16 crc; > + > + /* First wake up the device. */ > + am2315_ping(client); > + > + ret = i2c_master_send(client, tx_buf, sizeof(tx_buf)); > + if (ret < 0) { > + dev_err(&client->dev, "failed to send read request\n"); > + return ret; > + } > + /* Wait 2-3 ms, then read back the data sent by the device. */ > + usleep_range(2000, 3000); > + > + ret = i2c_master_recv(client, rx_buf, sizeof(rx_buf)); > + if (ret < 0) { > + dev_err(&client->dev, "failed to read sensor data\n"); > + return ret; > + } > + /* > + * Do a CRC check on the data and compare it to the value > + * calculated by the device. > + */ > + crc = am2315_crc(rx_buf, sizeof(rx_buf) - 2); > + if ((crc & 0xff) != rx_buf[6] || (crc >> 8) != rx_buf[7]) { > + dev_err(&client->dev, "failed to verify sensor data\n"); > + return -1; Provide a more informative error code... -EIO maybe... > + } > + > + return ret; > +} > + > +static int am2315_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + int ret; > + s16 value; > + u8 rx_buf[8]; > + u8 offset; > + struct am2315_data *data = iio_priv(indio_dev); > + struct i2c_client *client = data->client; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + offset = (chan->type == IIO_HUMIDITYRELATIVE) ? > + AM2315_HUM_OFFSET : AM2315_TEMP_OFFSET; > + /* Do a bulk data read, then pick out what we need. */ > + ret = am2315_read_data(client, rx_buf); > + if (ret < 0) > + return ret; > + /* > + * rx_buf format: > + * <function code> <number of registers read> > + * <humidity MSB> <humidity LSB> <temp MSB> <temp LSB> > + * <CRC LSB> <CRC MSB> > + */ > + value = (rx_buf[offset + 2] << 8) | rx_buf[offset + 3]; > + *val = value / 10; > + *val2 = (value % 10) * 100000; So for temperature this is outputing a value in degrees? Give you are outputting as raw, don't divide by 10 and just return IIO_VAL_INT; Leave the maths to userspace. > + return IIO_VAL_INT_PLUS_MICRO; > + case IIO_CHAN_INFO_SCALE: > + *val = 0; > + *val2 = 100000; For temp to be presented in standard base units (milli degrees C) this should be *val2 = 1000 I think... Same for humidity - obviously this will change if you don't do the divide by 10 in the raw read out. > + return IIO_VAL_INT_PLUS_MICRO; > + } > + > + return -EINVAL; > +} > + > +static const struct iio_info am2315_info = { > + .driver_module = THIS_MODULE, > + .read_raw = am2315_read_raw, > +}; > + > +static int am2315_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct iio_dev *indio_dev; > + struct am2315_data *data; > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > + if (!indio_dev) { > + dev_err(&client->dev, "iio allocation failed!\n"); > + return -ENOMEM; > + } > + > + data = iio_priv(indio_dev); > + data->client = client; > + i2c_set_clientdata(client, indio_dev); > + > + indio_dev->dev.parent = &client->dev; > + indio_dev->info = &am2315_info; > + indio_dev->name = AM2315_DRIVER_NAME; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->channels = am2315_channels; > + indio_dev->num_channels = ARRAY_SIZE(am2315_channels); > + > + return iio_device_register(indio_dev); > +} > + > +static int am2315_remove(struct i2c_client *client) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(client); > + If all you have in remove is the iio_device_unregister, you are safe to use devm_iio_device_unregister in probe and drop the remove entirely. (It's relatively rare that there is nothing to do in remove, but it is to simiplify that rare case that we have the devm_iio_device_register call) > + iio_device_unregister(indio_dev); > + > + return 0; > +} > + > +static const struct i2c_device_id am2315_i2c_id[] = { > + {"am2315", 0}, > + {} > +}; > + > +static const struct acpi_device_id am2315_acpi_id[] = { > + {"AOS2315", 0}, > + {} > +}; > + > +MODULE_DEVICE_TABLE(acpi, am2315_acpi_id); > + > +static struct i2c_driver am2315_driver = { > + .driver = { > + .name = "am2315", > + .acpi_match_table = ACPI_PTR(am2315_acpi_id), > + }, > + .probe = am2315_probe, > + .remove = am2315_remove, > + .id_table = am2315_i2c_id, > +}; > + > +module_i2c_driver(am2315_driver); > + > +MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@xxxxxxxxx>"); > +MODULE_DESCRIPTION("Aosong AM2315 ambient temperature & humidity sensor driver"); > +MODULE_LICENSE("GPL v2"); > -- 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
