From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from h1.radempa.de ([176.9.142.194]:35585 "EHLO mail.cosmopool.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755546AbcECLW6 (ORCPT ); Tue, 3 May 2016 07:22:58 -0400 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Date: Tue, 03 May 2016 13:22:55 +0200 From: Harald Geyer To: Stefan Wahren Cc: Jonathan Cameron , , , Shawn Guo , Sascha Hauer , Marek Vasut , Subject: Re: [PATCH 1/3] iio: mxs-lradc: Add regulators for current sources In-Reply-To: <57288674.9050601@i2se.com> References: <1461333147-11873-1-git-send-email-harald@ccbib.org> <1461333147-11873-2-git-send-email-harald@ccbib.org> <57288674.9050601@i2se.com> Message-ID: <6cdd4f858dacf72e192a292f29c12feb@ccbib.org> Sender: linux-iio-owner@vger.kernel.org List-Id: linux-iio@vger.kernel.org Hi Stefan! On 03.05.2016 13:07, Stefan Wahren wrote: > Am 22.04.2016 um 15:52 schrieb Harald Geyer: >> The hardware has two current sources ISRC0 and ISRC1 to allow >> measuring >> resistors without additional circuitry. This commit makes them >> available >> as regulators. >> >> Tested on an imx233-olinuxino board. >> >> Signed-off-by: Harald Geyer >> --- >> The current regulator API doesn't fit this type of device very well: >> Typically >> consumers will want to set a defined current, ie. min_uA == max_uA, >> but they >> can't without help from configuration data, because the valid values >> aren't >> reported by the API for current regulators. I have been thinking >> about >> extending the API, but currently AFAIK no such consumers exist and >> most >> users, like myself, will force the regulator to a defined value in >> devicetree anyway. >> >> .../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ >> drivers/iio/adc/Kconfig | 1 + >> drivers/iio/adc/mxs-lradc.c | 152 >> +++++++++++++++++++++ >> 3 files changed, 182 insertions(+) >> >> diff --git >> a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt >> b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt >> index 555fb11..983952c 100644 >> --- >> a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt >> +++ >> b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt >> @@ -19,6 +19,15 @@ Optional properties: >> - fsl,settling: delay between plate switch to next sample. Allowed >> value is >> 1 ... 2047. It counts at 2 kHz and its default is >> 10 (= 5 ms) >> +- ISRC0: A node describing the regulator of internal current source >> 0 >> +- ISRC1: A node describing the regulator of internal current source >> 1 >> + >> +Required properties for the ISRCx sub-nodes: >> +- regulator-max-microamp: See standard regulator binding >> documentation. >> + Valid values are from 0 to 300 in steps >> of 20. >> + >> +Optional properties for the ISRCx sub-nodes: >> +Any standard regulator properties that apply to current regulators. >> >> Example for i.MX23 SoC: >> >> @@ -31,6 +40,16 @@ Example for i.MX23 SoC: >> fsl,ave-ctrl = <4>; >> fsl,ave-delay = <2>; >> fsl,settling = <10>; >> + >> + isrc_lradc0: ISRC0 { >> + regulator-max-microamp = <300>; >> + }; >> + >> + isrc_lradc1: ISRC1 { >> + regulator-max-microamp = <120>; >> + regulator-min-microamp = <120>; >> + regulator-always-on; >> + }; > > IMHO the binding should represent the direct connection between the > current source and the physical channel. I think a node label isn't > enough. Unfortunately i don't have an idea to do it the "right way". I strictly followed the names of the data sheet here: imx23 and imx28 reference manuals both call the current sources ISRC0 and ISRC1 even if they are attached to different channels of the ADC. I think inventing some ISRC6 for imx28 would do more harm then good. >> }; >> >> Example for i.MX28 SoC: >> @@ -44,4 +63,14 @@ Example for i.MX28 SoC: >> fsl,ave-ctrl = <4>; >> fsl,ave-delay = <2>; >> fsl,settling = <10>; >> + >> + isrc_lradc0: ISRC0 { >> + regulator-max-microamp = <300>; >> + }; >> + >> + isrc_lradc6: ISRC1 { >> + regulator-max-microamp = <120>; >> + regulator-min-microamp = <120>; >> + regulator-always-on; >> + }; >> }; >> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig >> index 5937030..1968d1c 100644 >> --- a/drivers/iio/adc/Kconfig >> +++ b/drivers/iio/adc/Kconfig >> @@ -319,6 +319,7 @@ config MXS_LRADC >> tristate "Freescale i.MX23/i.MX28 LRADC" >> depends on (ARCH_MXS || COMPILE_TEST) && HAS_IOMEM >> depends on INPUT >> + depends on REGULATOR > > Any chance to avoid this dependency for an optional feature? I haven't looked into it yet, but I expect that that the mfd conversion will avoid this neatly. Thanks, Harald > Stefan > >> select STMP_DEVICE >> select IIO_BUFFER >> select IIO_TRIGGERED_BUFFER -- If you want to support my work: see http://friends.ccbib.org/harald/supporting/ or donate via peercoin to P98LRdhit3gZbHDBe7ta5jtXrMJUms4p7w or bitcoin 1FUtd8T9jRN1rFz63vZz7s2fDtB6d6A7aS