On 09/28/11 17:03, Guenter Roeck wrote:
> Hi Jonathan,
>
> On Wed, 2011-09-28 at 08:57 -0400, Jonathan Cameron wrote:
>> Currently dropped power down mode support.
>> Not tested.
>>
> Guess you don't really want the above in the commit log.
Well, I'm crossing my fingers someone might give me a tested-by ;)
>
> I requested samples for all three chips. Should be simple enough to test
> once I have also written a SPI master driver for one of my
> controllers ;).
Nice ;)
Thanks for the review and sorry for the dumb ass mistakes.
Not my best work. oops.
>
>> Signed-off-by: Jonathan Cameron <jic23@xxxxxxxxx>
>> ---
>> drivers/hwmon/Kconfig | 10 +++
>> drivers/hwmon/Makefile | 1 +
>> drivers/hwmon/ad7314.c | 181 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 192 insertions(+), 0 deletions(-)
>>
> We would also like to see
> Documentation/hwmon/ad7314
>
> Does this file or an equivalent of it exist in the iio tree ?
Nope, I'll make one up.
>
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index 0b62c3c..e5d4daa 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -68,6 +68,16 @@ config SENSORS_ABITUGURU3
>> This driver can also be built as a module. If so, the module
>> will be called abituguru3.
>>
>> +config SENSORS_AD7314
>> + tristate "Analog Devices AD7314 and compatibles"
>> + depends on SPI && EXPERIMENTAL
>> + help
>> + If you say yes here you get support for the Analog Devices
>> + AD7314, ADT7301 and ADT7302 temperature sensors.
>> +
>> + This driver can also be built as a module. If so, the module
>> + will be called ad7314.
>> +
>> config SENSORS_AD7414
>> tristate "Analog Devices AD7414"
>> depends on I2C && EXPERIMENTAL
>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> index 3c9ccef..228a4b7 100644
>> --- a/drivers/hwmon/Makefile
>> +++ b/drivers/hwmon/Makefile
>> @@ -21,6 +21,7 @@ obj-$(CONFIG_SENSORS_W83791D) += w83791d.o
>>
>> obj-$(CONFIG_SENSORS_ABITUGURU) += abituguru.o
>> obj-$(CONFIG_SENSORS_ABITUGURU3)+= abituguru3.o
>> +obj-$(CONFIG_SENSORS_AD7314) += ad7314.o
>> obj-$(CONFIG_SENSORS_AD7414) += ad7414.o
>> obj-$(CONFIG_SENSORS_AD7418) += ad7418.o
>> obj-$(CONFIG_SENSORS_ADCXX) += adcxx.o
>> diff --git a/drivers/hwmon/ad7314.c b/drivers/hwmon/ad7314.c
>> new file mode 100644
>> index 0000000..9cabe7b
>> --- /dev/null
>> +++ b/drivers/hwmon/ad7314.c
>> @@ -0,0 +1,181 @@
>> +/*
>> + * AD7314 digital temperature sensor driver for AD7314, ADT7301 and ADT7302
>> + *
>> + * Copyright 2010 Analog Devices Inc.
>> + *
>> + * Licensed under the GPL-2 or later.
>> + *
>> + * Conversion to hwmon from IIO done by Jonathan Cameron <jic23@xxxxxxxxx>
>> + */
>> +#include <linux/device.h>
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/sysfs.h>
>> +#include <linux/spi/spi.h>
>> +#include <linux/module.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/hwmon-sysfs.h>
>> +
>> +/*
>> + * AD7314 power mode
>> + */
>> +#define AD7314_PD 0x2000
>> +
>> +/*
>> + * AD7314 temperature masks
>> + */
>> +#define AD7314_TEMP_SIGN 0x200
>> +#define AD7314_TEMP_MASK 0x7FE0
>> +#define AD7314_TEMP_OFFSET 5
>> +#define AD7314_TEMP_FLOAT_OFFSET 2
>> +#define AD7314_TEMP_FLOAT_MASK 0x3
>> +
>> +/*
>> + * ADT7301 and ADT7302 temperature masks
>> + */
>> +#define ADT7301_TEMP_SIGN 0x2000
>> +#define ADT7301_TEMP_MASK 0x2FFF
>> +#define ADT7301_TEMP_FLOAT_OFFSET 5
>> +#define ADT7301_TEMP_FLOAT_MASK 0x1F
>> +
>> +enum ad7314_variant {
>> + adt7301,
>> + adt7302,
>> + ad7314,
>> +};
>> +
>> +struct ad7314_data {
>> + struct spi_device *spi_dev;
>> + struct device *hwmon_dev;
>> + u16 rx ____cacheline_aligned;
>> +};
>> +
>> +static int ad7314_spi_read(struct spi_device *spi_dev, u16 *data)
>> +{
>> + int ret = 0;
>
> No need to initialize ret.
>
>> + u16 value;
>> +
>> + ret = spi_read(spi_dev, (u8 *)&value, sizeof(value));
>
> I am having a bit of trouble here. value is not cache aligned, yet you
> use it for the spi transaction. data is cache aligned, but you only
> store the result from the spi transaction into in.
>
> Given this, either the code doesn't work, or "rx" is not needed at all
> since you don't use it for a transaction which would require or ask for
> cache alignment.
>
> Am I missing something ?
Nope, I'm just being stupid. Sometimes I assume code does what I
expect and then don't notice it isn't doing so.
Will fix.
>
>> + if (ret < 0) {
>> + dev_err(&spi_dev->dev, "SPI read error\n");
>> + return ret;
>> + }
>> +
>> + *data = be16_to_cpu((u16)value);
>
> value is defined as u16 and does not need a type cast.
>
>> +
>> + return ret;
>> +}
>> +
>> +static ssize_t ad7314_show_temperature(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct ad7314_data *chip = dev_get_drvdata(dev);
>> + s16 data;
>> + int ret;
>> +
>> + ret = ad7314_spi_read(chip->spi_dev, &chip->rx);
>> + if (ret < 0)
>> + return ret;
>> + data = chip->rx;
>> + switch (spi_get_device_id(chip->spi_dev)->driver_data) {
>> + case ad7314:
>> + data = (data & AD7314_TEMP_MASK) >> AD7314_TEMP_OFFSET;
>> + data = (data << 6) >> 6;
>> +
>> + return sprintf(buf, "%d.%.2d\n",
>> + data >> AD7314_TEMP_FLOAT_OFFSET,
>> + (data & AD7314_TEMP_FLOAT_MASK) * 25);
>
> hwmon reports temperatures in milli-degrees C, and does not know about
> decimal points. So this should be something like
>
> return sprintf(buf, "%d\n", data * 250);
Sorry, I'm half asleep today and forgot that. Will fix.
>
>> + case adt7301:
>> + case adt7302:
>> + data &= ADT7301_TEMP_MASK;
>> + data = (data << 3) >> 3;
>> +
>> + return sprintf(buf, "%d.%.5d\n",
>> + data >> ADT7301_TEMP_FLOAT_OFFSET,
>> + (data & ADT7301_TEMP_FLOAT_MASK) * 3125);
>
> Similar, this should be something like
>
> return sprintf(buf, "%d\n", (data * 1000) >> 5);
>
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
>> + ad7314_show_temperature, NULL, 0);
>> +
>> +static struct attribute *ad7314_attributes[] = {
>> + &sensor_dev_attr_temp1_input.dev_attr.attr,
>> + NULL,
>> +};
>> +
>> +static const struct attribute_group ad7314_group = {
>> + .attrs = ad7314_attributes,
>> +};
>> +
>> +static int __devinit ad7314_probe(struct spi_device *spi_dev)
>> +{
>> + int ret;
>> + struct ad7314_data *chip;
>> +
>> + chip = kzalloc(sizeof(*chip), GFP_KERNEL);
>> + if (chip == NULL) {
>> + ret = -ENOMEM;
>> + goto error_ret;
>> + }
>> + dev_set_drvdata(&spi_dev->dev, chip);
>> +
>> + ret = sysfs_create_group(&spi_dev->dev.kobj, &ad7314_group);
>> + if (ret < 0)
>> + goto error_free_chip;
>> +
>> + return 0;
>> +error_free_chip:
>> + kfree(chip);
>> +error_ret:
>> + return ret;
>> +}
>> +
>> +static int __devexit ad7314_remove(struct spi_device *spi_dev)
>> +{
>> + struct ad7314_data *chip = dev_get_drvdata(&spi_dev->dev);
>> +
>> + hwmon_device_unregister(chip->hwmon_dev);
>> + sysfs_remove_group(&spi_dev->dev.kobj, &ad7314_group);
>> + kfree(chip);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct spi_device_id ad7314_id[] = {
>> + { "adt7301", adt7301 },
>> + { "adt7302", adt7302 },
>> + { "ad7314", ad7314 },
>> + {}
>> +};
>> +
>> +static struct spi_driver ad7314_driver = {
>> + .driver = {
>> + .name = "ad7314",
>> + .bus = &spi_bus_type,
>> + .owner = THIS_MODULE,
>> + },
>> + .probe = ad7314_probe,
>> + .remove = __devexit_p(ad7314_remove),
>> + .id_table = ad7314_id,
>> +};
>> +
>> +static __init int ad7314_init(void)
>> +{
>> + return spi_register_driver(&ad7314_driver);
>> +}
>> +module_init(ad7314_init);
>> +
>> +static __exit void ad7314_exit(void)
>> +{
>> + spi_unregister_driver(&ad7314_driver);
>> +}
>> +module_exit(ad7314_exit);
>> +
>> +MODULE_AUTHOR("Sonic Zhang <sonic.zhang@xxxxxxxxxx>");
>> +MODULE_DESCRIPTION("Analog Devices AD7314, ADT7301 and ADT7302 digital"
>> + " temperature sensor driver");
>> +MODULE_LICENSE("GPL v2");
>
>
>
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
