From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753784AbdCWLFW (ORCPT ); Thu, 23 Mar 2017 07:05:22 -0400 Received: from www381.your-server.de ([78.46.137.84]:48107 "EHLO www381.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752404AbdCWLFU (ORCPT ); Thu, 23 Mar 2017 07:05:20 -0400 Subject: Re: [PATCH] iio/adc/ltc2497: Driver for Linear Technology LTC2497 ADC To: michael.hennerich@analog.com, jic23@kernel.org, knaack.h@gmx.de, pmeerw@pmeerw.net, robh+dt@kernel.org, mark.rutland@arm.com References: <1490265310-6162-1-git-send-email-michael.hennerich@analog.com> Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org From: Lars-Peter Clausen X-Enigmail-Draft-Status: N1110 Message-ID: <6774c073-7fe4-e259-306e-3cf4621f7c69@metafoo.de> Date: Thu, 23 Mar 2017 12:05:10 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.6.0 MIME-Version: 1.0 In-Reply-To: <1490265310-6162-1-git-send-email-michael.hennerich@analog.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-Authenticated-Sender: lars@metafoo.de Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org The subject should follow the standard subsystem subject scheme. E.g. in this case "iio: adc: Add driver for ..." On 03/23/2017 11:35 AM, michael.hennerich@analog.com wrote: > From: Michael Hennerich Needs a commit message. > > Signed-off-by: Michael Hennerich > --- > .../devicetree/bindings/iio/adc/ltc2497.txt | 13 + > MAINTAINERS | 1 + > drivers/iio/adc/Kconfig | 11 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ltc2497.c | 263 +++++++++++++++++++++ > 5 files changed, 289 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/ltc2497.txt > create mode 100644 drivers/iio/adc/ltc2497.c > > diff --git a/Documentation/devicetree/bindings/iio/adc/ltc2497.txt b/Documentation/devicetree/bindings/iio/adc/ltc2497.txt > new file mode 100644 > index 0000000..c2829c19 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/ltc2497.txt > @@ -0,0 +1,13 @@ > +* Linear Technology / Analog Devices LTC2497 ADC > + > +Required properties: > + - compatible: Should be "lltc,ltc2497" Replace 'should' with 'must'. It won't work if it isn't. > + - reg: Should contain the ADC I2C address Same here. > + - vref-supply: The regulator supply for ADC reference voltage > + > +Example: > + ltc2497: adc@76 { > + compatible = "lltc,ltc2497"; > + reg = <0x76>; > + vref-supply = <<c2497_reg>; > + }; [...] > diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c > new file mode 100644 > index 0000000..0452715 > --- /dev/null > +++ b/drivers/iio/adc/ltc2497.c > @@ -0,0 +1,263 @@ > +/* > + * ltc2497.c - Driver for Linear Technology LTC2497 ADC That's the "Analog Devices LTC2497 ADC" now ;) > + * > + * Copyright (C) 2017 Analog Devices Inc. > + * > + * Licensed under the GPL-2. > + * > + * Datasheet: http://cds.linear.com/docs/en/datasheet/2497fd.pdf > + */ > + > +#include > +#include > +#include > +#include > +#include Sorting alphabetically is preferred. > + > +#include > +#include > + > +#define LTC2497_ENABLE 0xA0 > +#define LTC2497_SGL (1 << 4) I'm sure 5 minutes after the patch has been applied somebody will send a patch to replace this with BIT(4) > +#define LTC2497_DIFF (0 << 4) > +#define LTC2497_SIGN (1 << 3) > +#define LTC2497_CONFIG_DEFAULT LTC2497_ENABLE > +#define LTC2497_CONVERSION_TIME_MS 150ULL > + > +struct ltc2497_st { > + struct i2c_client *client; > + struct regulator *ref; > + ktime_t time_prev; > + u8 addr_prev; > +}; > + > +static int ltc2497_wait_conv(struct ltc2497_st *st) > +{ > + s64 time_elapsed; > + > + time_elapsed = ktime_ms_delta(ktime_get(), st->time_prev); > + > + if (time_elapsed < LTC2497_CONVERSION_TIME_MS) { > + /* delay if conversion time not passed > + * since last read or write > + */ > + msleep(LTC2497_CONVERSION_TIME_MS - time_elapsed); Considering how long this sleeps msleep_interruptible() might be the better choice. > + return 0; > + } > + > + if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) { > + /* We're in automatic mode - > + * so the last reading is stil not outdated > + */ > + return 0; > + } > + > + return -ETIMEDOUT; > +} > + > +static int ltc2497_read(struct ltc2497_st *st, u8 address, int *val) > +{ > + struct i2c_client *client = st->client; > + __be32 buf = 0; transfer buffers must not be on the stack to avoid issues if the controller should use DMA. > + int ret; > + > + ret = ltc2497_wait_conv(st); > + if (ret < 0 || st->addr_prev != address) { > + ret = i2c_smbus_write_byte(st->client, 0xA0 | address); > + if (ret < 0) > + return ret; > + st->addr_prev = address; > + msleep(LTC2497_CONVERSION_TIME_MS); > + } > + ret = i2c_master_recv(client, (char *)&buf, 3); > + if (ret < 0) { > + dev_err(&client->dev, "i2c_master_recv failed\n"); > + return ret; > + } > + st->time_prev = ktime_get(); > + *val = (be32_to_cpu(buf) >> 14) - (1 << 17); > + > + return ret; > +} [...] From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lars-Peter Clausen Subject: Re: [PATCH] iio/adc/ltc2497: Driver for Linear Technology LTC2497 ADC Date: Thu, 23 Mar 2017 12:05:10 +0100 Message-ID: <6774c073-7fe4-e259-306e-3cf4621f7c69@metafoo.de> References: <1490265310-6162-1-git-send-email-michael.hennerich@analog.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: <1490265310-6162-1-git-send-email-michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org> Sender: linux-iio-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org, jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, knaack.h-Mmb7MZpHnFY@public.gmane.org, pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, mark.rutland-5wv7dgnIgG8@public.gmane.org Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: devicetree@vger.kernel.org The subject should follow the standard subsystem subject scheme. E.g. in this case "iio: adc: Add driver for ..." On 03/23/2017 11:35 AM, michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org wrote: > From: Michael Hennerich Needs a commit message. > > Signed-off-by: Michael Hennerich > --- > .../devicetree/bindings/iio/adc/ltc2497.txt | 13 + > MAINTAINERS | 1 + > drivers/iio/adc/Kconfig | 11 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ltc2497.c | 263 +++++++++++++++++++++ > 5 files changed, 289 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/ltc2497.txt > create mode 100644 drivers/iio/adc/ltc2497.c > > diff --git a/Documentation/devicetree/bindings/iio/adc/ltc2497.txt b/Documentation/devicetree/bindings/iio/adc/ltc2497.txt > new file mode 100644 > index 0000000..c2829c19 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/ltc2497.txt > @@ -0,0 +1,13 @@ > +* Linear Technology / Analog Devices LTC2497 ADC > + > +Required properties: > + - compatible: Should be "lltc,ltc2497" Replace 'should' with 'must'. It won't work if it isn't. > + - reg: Should contain the ADC I2C address Same here. > + - vref-supply: The regulator supply for ADC reference voltage > + > +Example: > + ltc2497: adc@76 { > + compatible = "lltc,ltc2497"; > + reg = <0x76>; > + vref-supply = <<c2497_reg>; > + }; [...] > diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c > new file mode 100644 > index 0000000..0452715 > --- /dev/null > +++ b/drivers/iio/adc/ltc2497.c > @@ -0,0 +1,263 @@ > +/* > + * ltc2497.c - Driver for Linear Technology LTC2497 ADC That's the "Analog Devices LTC2497 ADC" now ;) > + * > + * Copyright (C) 2017 Analog Devices Inc. > + * > + * Licensed under the GPL-2. > + * > + * Datasheet: http://cds.linear.com/docs/en/datasheet/2497fd.pdf > + */ > + > +#include > +#include > +#include > +#include > +#include Sorting alphabetically is preferred. > + > +#include > +#include > + > +#define LTC2497_ENABLE 0xA0 > +#define LTC2497_SGL (1 << 4) I'm sure 5 minutes after the patch has been applied somebody will send a patch to replace this with BIT(4) > +#define LTC2497_DIFF (0 << 4) > +#define LTC2497_SIGN (1 << 3) > +#define LTC2497_CONFIG_DEFAULT LTC2497_ENABLE > +#define LTC2497_CONVERSION_TIME_MS 150ULL > + > +struct ltc2497_st { > + struct i2c_client *client; > + struct regulator *ref; > + ktime_t time_prev; > + u8 addr_prev; > +}; > + > +static int ltc2497_wait_conv(struct ltc2497_st *st) > +{ > + s64 time_elapsed; > + > + time_elapsed = ktime_ms_delta(ktime_get(), st->time_prev); > + > + if (time_elapsed < LTC2497_CONVERSION_TIME_MS) { > + /* delay if conversion time not passed > + * since last read or write > + */ > + msleep(LTC2497_CONVERSION_TIME_MS - time_elapsed); Considering how long this sleeps msleep_interruptible() might be the better choice. > + return 0; > + } > + > + if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) { > + /* We're in automatic mode - > + * so the last reading is stil not outdated > + */ > + return 0; > + } > + > + return -ETIMEDOUT; > +} > + > +static int ltc2497_read(struct ltc2497_st *st, u8 address, int *val) > +{ > + struct i2c_client *client = st->client; > + __be32 buf = 0; transfer buffers must not be on the stack to avoid issues if the controller should use DMA. > + int ret; > + > + ret = ltc2497_wait_conv(st); > + if (ret < 0 || st->addr_prev != address) { > + ret = i2c_smbus_write_byte(st->client, 0xA0 | address); > + if (ret < 0) > + return ret; > + st->addr_prev = address; > + msleep(LTC2497_CONVERSION_TIME_MS); > + } > + ret = i2c_master_recv(client, (char *)&buf, 3); > + if (ret < 0) { > + dev_err(&client->dev, "i2c_master_recv failed\n"); > + return ret; > + } > + st->time_prev = ktime_get(); > + *val = (be32_to_cpu(buf) >> 14) - (1 << 17); > + > + return ret; > +} [...]