| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e23ac8fb-e43e-39be-aced-d83cf8e31d87@topic.nl>
Date: Thu, 29 Jun 2017 14:08:17 +0200
From: Mike Looijmans <mike.looijmans@...ic.nl>
To: Frans Klaver <fransklaver@...il.com>
CC: <linux-hwmon@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Guenter Roeck <linux@...ck-us.net>, <jdelvare@...e.com>
Subject: Re: [PATCH] hwmon: Add LTC2471/LTC2473 driver
On 29-06-17 13:38, Frans Klaver wrote:
> On Thu, Jun 29, 2017 at 1:30 PM, Mike Looijmans <mike.looijmans@...ic.nl> wrote:
>> On 29-06-17 13:18, Frans Klaver wrote:
>>>
>>> On Thu, Jun 29, 2017 at 12:45 PM, Mike Looijmans
>>> <mike.looijmans@...ic.nl> wrote:
>>>>
>>>> The LTC2741 and LTC2473 are single voltage monitoring chips. The LTC2473
>>>> is similar to the LTC2471 but outputs a signed differential value.
>>>>
>>>> Datasheet:
>>>> http://cds.linear.com/docs/en/datasheet/24713fb.pdf
>>>>
>>>> Signed-off-by: Mike Looijmans <mike.looijmans@...ic.nl>
>>>> ---
>>>> drivers/hwmon/Kconfig | 10 ++++
>>>> drivers/hwmon/Makefile | 1 +
>>>> drivers/hwmon/ltc2471.c | 127
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 138 insertions(+)
>>>> create mode 100644 drivers/hwmon/ltc2471.c
>>>>
>>>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>>>> index fbde226..c9a2a87 100644
>>>> --- a/drivers/hwmon/Kconfig
>>>> +++ b/drivers/hwmon/Kconfig
>>>> @@ -673,6 +673,16 @@ config SENSORS_LINEAGE
>>>> This driver can also be built as a module. If so, the module
>>>> will be called lineage-pem.
>>>>
>>>> +config SENSORS_LTC2471
>>>> + tristate "Linear Technology LTC2471 and LTC2473"
>>>> + depends on I2C
>>>> + help
>>>> + If you say yes here you get support for Linear Technology
>>>> LTC2471
>>>> + and LTC2473 voltage monitors.
>>>> +
>>>> + This driver can also be built as a module. If so, the module
>>>> will
>>>> + be called ltc2471.
>>>> +
>>>> config SENSORS_LTC2945
>>>> tristate "Linear Technology LTC2945"
>>>> depends on I2C
>>>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>>>> index 58cc3ac..6f60fe7 100644
>>>> --- a/drivers/hwmon/Makefile
>>>> +++ b/drivers/hwmon/Makefile
>>>> @@ -99,6 +99,7 @@ obj-$(CONFIG_SENSORS_LM93) += lm93.o
>>>> obj-$(CONFIG_SENSORS_LM95234) += lm95234.o
>>>> obj-$(CONFIG_SENSORS_LM95241) += lm95241.o
>>>> obj-$(CONFIG_SENSORS_LM95245) += lm95245.o
>>>> +obj-$(CONFIG_SENSORS_LTC2471) += ltc2471.o
>>>> obj-$(CONFIG_SENSORS_LTC2945) += ltc2945.o
>>>> obj-$(CONFIG_SENSORS_LTC2990) += ltc2990.o
>>>> obj-$(CONFIG_SENSORS_LTC4151) += ltc4151.o
>>>> diff --git a/drivers/hwmon/ltc2471.c b/drivers/hwmon/ltc2471.c
>>>> new file mode 100644
>>>> index 0000000..17eaad8
>>>> --- /dev/null
>>>> +++ b/drivers/hwmon/ltc2471.c
>>>> @@ -0,0 +1,127 @@
>>>> +/*
>>>> + * Driver for Linear Technology LTC2471 and LTC2473 voltage monitors
>>>> + * The LTC2473 is identical to the 2471, but reports a differential
>>>> signal.
>>>> + *
>>>> + * Copyright (C) 2017 Topic Embedded Products
>>>> + * Author: Mike Looijmans <mike.looijmans@...ic.nl>
>>>> + *
>>>> + * License: GPLv2
>>>> + */
>>>> +
>>>> +#include <linux/err.h>
>>>> +#include <linux/hwmon.h>
>>>> +#include <linux/hwmon-sysfs.h>
>>>> +#include <linux/i2c.h>
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/module.h>
>>>> +
>>>> +enum chips {
>>>> + ltc2471,
>>>> + ltc2473
>>>> +};
>>>> +
>>>> +struct ltc2471_data {
>>>> + struct i2c_client *i2c;
>>>> + bool differential;
>>>> +};
>>>
>>>
>>> I haven't checked if there are similar drivers, but how about
>>> ltc247x_data? It's a bit odd to tie all naming in this driver to one
>>> chip and then support another one as well.
>>
>>
>> Did a query on the linear site, found the LTC2470 and LTC2472 which appear
>> to be the SPI versions of the same chip. So yes, using "ltc247x" may cause
>> confusion when other drivers arrive. Linear solves this by naming the
>> datasheet "ltc24712" but I think that'd be even more confusing.
>
> OK.
>
>
>>>
>>>> +
>>>> +/* Reference voltage is 1.25V */
>>>> +#define LTC2471_VREF 1250
>>>> +
>>>> +/* Read two bytes from the I2C bus to obtain the ADC result */
>>>> +static int ltc2471_get_value(struct i2c_client *i2c)
>>>> +{
>>>> + int ret;
>>>> + __be16 buf;
>>>> +
>>>> + ret = i2c_master_recv(i2c, (char *)&buf, 2);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> + if (ret != 2)
>>>> + return -EIO;
>>>> +
>>>> + /* MSB first */
>>>> + return be16_to_cpu(buf);
>>>> +}
>>>> +
>>>> +static ssize_t ltc2471_show_value(struct device *dev,
>>>> + struct device_attribute *da, char *buf)
>>>> +{
>>>> + struct ltc2471_data *data = dev_get_drvdata(dev);
>>>> + int value;
>>>> +
>>>> + value = ltc2471_get_value(data->i2c);
>>>> + if (unlikely(value < 0))
>>>> + return value;
>>>> +
>>>> + if (data->differential)
>>>> + /* Ranges from -VREF to +VREF with "0" at 0x8000 */
>>>> + value = ((s32)LTC2471_VREF * (s32)(value - 0x8000)) >>
>>>> 15;
>>>> + else
>>>> + /* Ranges from 0 to +VREF */
>>>> + value = ((u32)LTC2471_VREF * (u32)value) >> 16;
>>>> +
>>>> + return snprintf(buf, PAGE_SIZE, "%d\n", value);
>>>> +}
>>>> +
>>>> +static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, ltc2471_show_value, NULL,
>>>> 0);
>>>
>>>
>>> Octal permissions are preferred over S_IRUGO and such.
>>
>>
>> Uh, can elaborate on that, or give an example on what you mean by that
>> remark?
>>
>> A quick grep reveils that most (if not all) hwmon devices are currently
>> using "S_IRUGO" and such.
>
> I am aware of the fact that lots of drivers use these .Linus filed an
> executive decision that octals are preferred over symbols when it
> comes to permissions. checkpatch.pl should warn about it nowadays.
rebased on current master and checkpatch now indeed suggests to use "0444"
instead. I'll submit a v2 with changes as suggested.
Kind regards,
Mike Looijmans
System Expert
TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@...icproducts.com
Website: www.topicproducts.com
Please consider the environment before printing this e-mail
Powered by blists - more mailing lists