From mboxrd@z Thu Jan 1 00:00:00 1970 From: jacopo mondi Subject: Re: [PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver Date: Sun, 15 Jan 2017 15:41:35 +0100 Message-ID: References: <1484301038-16386-1-git-send-email-jacopo+renesas@jmondi.org> <502b12a0-6011-c0eb-06b4-a40245d1d8db@jmondi.org> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-iio-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Jonathan Cameron , Jacopo Mondi , wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org, magnus.damm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, knaack.h-Mmb7MZpHnFY@public.gmane.org, lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org, pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org, marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, geert-Td1EMuHUCqxL1ZNQvxDV9g@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-renesas-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: devicetree@vger.kernel.org Hi Jonathan, On 15/01/2017 15:31, Jonathan Cameron wrote: > On 15/01/17 14:13, jacopo mondi wrote: >> Hi Jonathan, >> thanks for review, >> >> On 13/01/2017 10:50, Jacopo Mondi wrote: >>> Hello, >>> sending out v4 splitting device tree bindings documentation and actual ADC >>> driver. >>> No changes in driver code since v3. >>> >>> Same question for iio maintainers here: >>> I would like to have clarified the measure unit returned by read_raw(). >>> Currently (value_raw * value_scale) return the ADC input value in mV. > Good. >>> While testing the patch I've been questioned if that should not actually >>> be in uV. This is easily achievable making _scale return a value in uV. >>> I have found no mention of this in the ABI documentation as it speaks of >>> generic voltage. Can we have a final word on this? >> >> I see you have reviewed the driver without complaining for the >> read_raw() measure unit, so I assume this replies to the above >> question as well... >> > The units are specified in Documentation/ABI/testing/sysfs-bus-iio. > >> What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_raw >> What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw >> What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw >> What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_q_raw >> KernelVersion: 2.6.35 >> Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org >> Description: >> Raw (unscaled no bias removal etc.) voltage measurement from >> channel Y. In special cases where the channel does not >> correspond to externally available input one of the named >> versions may be used. The number must always be specified and >> unique to allow association with event codes. Units after >> application of scale and offset are millivolts. > > So millivolts. This comes from a, perhaps ill judged, decision to match > hwmon units were we could. > Thank you! The "generic voltage" I said is mentioned in the ABI documentation is in _scale attributes description. I assume then the measure unit of the value returned by _scale is not relevant as long as the _raw one with scale applied returns millivolts. Thanks, as soon as I have feedbacks on DTS binding docs, I'll send v5 and we can hopefully close this one ;) Thanks j > Jonathan > >> Thanks j >> >>> >>> Thanks Marek for having tested this. >>> >>> v1 -> v2: >>> - incorporated pmeerw's review comments >>> - retrieve vref from dts and use that to convert read_raw result >>> to mV >>> - add device tree bindings documentation >>> >>> v2 -> v3: >>> - add _SCALE bit of read_raw function and change _RAW bit accordingly >>> - call regulator_get_voltage when accessing the _SCALE part of read_raw >>> and not during probe >>> - add back remove function as regulator has to be disabled when detaching >>> the module. Do not use devm_ version of iio_register/unregister functions >>> anymore but do unregister in the remove. >>> - remove mutex as access to SPI bus is protected by SPI core. Thanks marex >>> >>> v3 -> v4: >>> - split device tree binding documentation and actual ADC driver >>> - add "reg" to the list of required properties and use a better >>> namimg for the adc device node in bindings documentation as suggested >>> by Geert. >>> >>> Jacopo Mondi (2): >>> iio: adc: Add Maxim MAX11100 driver >>> dt-bindings: iio: document MAX11100 ADC >>> >>> .../devicetree/bindings/iio/adc/max11100.txt | 19 +++ >>> drivers/iio/adc/Kconfig | 9 + >>> drivers/iio/adc/Makefile | 1 + >>> drivers/iio/adc/max11100.c | 187 +++++++++++++++++++++ >>> 4 files changed, 216 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt >>> create mode 100644 drivers/iio/adc/max11100.c >>> >> > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay3-d.mail.gandi.net ([217.70.183.195]:47990 "EHLO relay3-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750956AbdAOOlq (ORCPT ); Sun, 15 Jan 2017 09:41:46 -0500 Subject: Re: [PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver To: Jonathan Cameron , Jacopo Mondi , wsa+renesas@sang-engineering.com, magnus.damm@gmail.com, knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net, marek.vasut@gmail.com, geert@linux-m68k.org, robh+dt@kernel.org, mark.rutland@arm.com References: <1484301038-16386-1-git-send-email-jacopo+renesas@jmondi.org> <502b12a0-6011-c0eb-06b4-a40245d1d8db@jmondi.org> Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org From: jacopo mondi Message-ID: Date: Sun, 15 Jan 2017 15:41:35 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: Hi Jonathan, On 15/01/2017 15:31, Jonathan Cameron wrote: > On 15/01/17 14:13, jacopo mondi wrote: >> Hi Jonathan, >> thanks for review, >> >> On 13/01/2017 10:50, Jacopo Mondi wrote: >>> Hello, >>> sending out v4 splitting device tree bindings documentation and actual ADC >>> driver. >>> No changes in driver code since v3. >>> >>> Same question for iio maintainers here: >>> I would like to have clarified the measure unit returned by read_raw(). >>> Currently (value_raw * value_scale) return the ADC input value in mV. > Good. >>> While testing the patch I've been questioned if that should not actually >>> be in uV. This is easily achievable making _scale return a value in uV. >>> I have found no mention of this in the ABI documentation as it speaks of >>> generic voltage. Can we have a final word on this? >> >> I see you have reviewed the driver without complaining for the >> read_raw() measure unit, so I assume this replies to the above >> question as well... >> > The units are specified in Documentation/ABI/testing/sysfs-bus-iio. > >> What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_raw >> What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw >> What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw >> What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_q_raw >> KernelVersion: 2.6.35 >> Contact: linux-iio@vger.kernel.org >> Description: >> Raw (unscaled no bias removal etc.) voltage measurement from >> channel Y. In special cases where the channel does not >> correspond to externally available input one of the named >> versions may be used. The number must always be specified and >> unique to allow association with event codes. Units after >> application of scale and offset are millivolts. > > So millivolts. This comes from a, perhaps ill judged, decision to match > hwmon units were we could. > Thank you! The "generic voltage" I said is mentioned in the ABI documentation is in _scale attributes description. I assume then the measure unit of the value returned by _scale is not relevant as long as the _raw one with scale applied returns millivolts. Thanks, as soon as I have feedbacks on DTS binding docs, I'll send v5 and we can hopefully close this one ;) Thanks j > Jonathan > >> Thanks j >> >>> >>> Thanks Marek for having tested this. >>> >>> v1 -> v2: >>> - incorporated pmeerw's review comments >>> - retrieve vref from dts and use that to convert read_raw result >>> to mV >>> - add device tree bindings documentation >>> >>> v2 -> v3: >>> - add _SCALE bit of read_raw function and change _RAW bit accordingly >>> - call regulator_get_voltage when accessing the _SCALE part of read_raw >>> and not during probe >>> - add back remove function as regulator has to be disabled when detaching >>> the module. Do not use devm_ version of iio_register/unregister functions >>> anymore but do unregister in the remove. >>> - remove mutex as access to SPI bus is protected by SPI core. Thanks marex >>> >>> v3 -> v4: >>> - split device tree binding documentation and actual ADC driver >>> - add "reg" to the list of required properties and use a better >>> namimg for the adc device node in bindings documentation as suggested >>> by Geert. >>> >>> Jacopo Mondi (2): >>> iio: adc: Add Maxim MAX11100 driver >>> dt-bindings: iio: document MAX11100 ADC >>> >>> .../devicetree/bindings/iio/adc/max11100.txt | 19 +++ >>> drivers/iio/adc/Kconfig | 9 + >>> drivers/iio/adc/Makefile | 1 + >>> drivers/iio/adc/max11100.c | 187 +++++++++++++++++++++ >>> 4 files changed, 216 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt >>> create mode 100644 drivers/iio/adc/max11100.c >>> >> >