From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753105AbdF2Mbq convert rfc822-to-8bit (ORCPT ); Thu, 29 Jun 2017 08:31:46 -0400 Received: from mx42-out3.antispamcloud.com ([138.201.61.179]:59796 "EHLO mx42-out3.antispamcloud.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752899AbdF2Mbh (ORCPT ); Thu, 29 Jun 2017 08:31:37 -0400 X-Greylist: delayed 1384 seconds by postgrey-1.27 at vger.kernel.org; Thu, 29 Jun 2017 08:31:36 EDT Subject: Re: [PATCH] hwmon: Add LTC2471/LTC2473 driver To: Frans Klaver CC: , "linux-kernel@vger.kernel.org" , Guenter Roeck , References: <1498733135-4729-1-git-send-email-mike.looijmans@topic.nl> From: Mike Looijmans Organization: TOPIC Message-ID: Date: Thu, 29 Jun 2017 14:08:17 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: nl Content-Transfer-Encoding: 8BIT X-Originating-IP: [192.168.80.121] X-EXCLAIMER-MD-CONFIG: 9833cda7-5b21-4d34-9a38-8d025ddc3664 X-EXCLAIMER-MD-BIFURCATION-INSTANCE: 0 X-Originating-IP: 37.74.225.130 X-SpamExperts-Domain: topic.nl X-SpamExperts-Username: 37.74.225.128/28 Authentication-Results: antispamcloud.com; auth=pass smtp.auth=37.74.225.128/28@topic.nl X-SpamExperts-Outgoing-Class: ham X-SpamExperts-Outgoing-Evidence: SB/global_tokens (0.000309816273143) X-Recommended-Action: accept X-Filter-ID: PqwsvolAWURa0gwxuN3S5YEa3T7JuZT23fGO2rGt3ZgWMD8gia7lRmdoHJ3n0cm6Vtfp9X1ZBchZ wvx8a7dTUQWF0kx98HH3GX+iQK7NiP6LBDMrD7q/cJogwbqzsuokIXMJ1U4MKKnQ5SAJ1V9dkP9x bu57BDazLmklZgT7xzZw7Hw48UxWpghM5863ZD8ki0nNC/O7A3rDsgAugZZCoBvH2nBKMbkQqol4 tb+drTg8CVsONrMJuGzuoGnKTKcygLJjfy2/hUPxxGqITycuLMdYaphGvYopQ24KJ9kgRDN3pbXn aMbbC1QSUBxihEsjBWOBqoptsYkcljagCMIDypReAQCFQo+KUtaSCrVTlxOfVsst/QEVbupaaYfx KYRJ8ar4sO9kYmr7/nmchPR9GwS0RTbIegwn2yn+XE4z9zMbAT/bTGM3eqpSRQBxZnoh6wB8/5OG 6NX6624gTVMPxnxlXOrp8+AEqj1HlCbH4usZQGxcFxcrkq+IKIlinlm7VgW9/bktU41htiJ8fk7N kPOW6bvL9DC431WlF1LxckxMBAJkcsAhDD4OTTRkzMU8KuR9h4rJ1IhRjQc5xQiShPHcddFAPKLn BT/JZzTASedFRFR4qXE7Cn+3zlGq5KdMR0tMwLh/UfgIMsW8UawravVdyPOcJwZf+mGEJuzRRI/I bdND4vqFwExA0NnYCqbG5QNcEtGovWtscnF17r8cbe+VjUUaGVSupUlW8pBQgN4HmFDqewO9xyOq CYO8P1aHD+EfksOZG6EJmr72jxD5Fckvp3AwOznETyhcf7tZ+vbdB2PFz3p6kuvO+YMomQq9iz+M vz3TgGnVjlPKUWu31bJplnAJKxa1DiBAS6sMnUw1l8M9Q2KQmm9hMajvTmH4U2Itm39BdCc4FEP6 OrUewmT7+aOz0/pb38Vwm4cDBYQAoAVuNNggAyx5P9TXQW8lidMlhCno/QsYaTlhyywkGQIUGl+M KOR7qlxRiFgi0Ku/vW4P9it5XBmhB4n4jP0fh8aqMWy7FjLg+egfmAyy1v820wTapR6nKqVwMWjs Ad8= X-Report-Abuse-To: spam@quarantine2.antispamcloud.com Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 29-06-17 13:38, Frans Klaver wrote: > On Thu, Jun 29, 2017 at 1:30 PM, Mike Looijmans wrote: >> On 29-06-17 13:18, Frans Klaver wrote: >>> >>> On Thu, Jun 29, 2017 at 12:45 PM, Mike Looijmans >>> 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 >>>> --- >>>> 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 >>>> + * >>>> + * License: GPLv2 >>>> + */ >>>> + >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> + >>>> +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@topicproducts.com Website: www.topicproducts.com Please consider the environment before printing this e-mail