Re: [PATCH v5 3/3] Documentation: add the binding file for Freescale vf610 ADC driver

Re: [PATCH v5 3/3] Documentation: add the binding file for Freescale vf610 ADC driver

[Jonathan Cameron]

On 26/01/14 05:39, Fugang Duan wrote:
> The patch adds the binding file for Freescale vf610 ADC driver.
>
> CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> CC: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> CC: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> CC: Otavio Salvador <otavio-fKevB0iiKLMBZ+LybsDmbA@public.gmane.org>
> CC: Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
> CC: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
> Signed-off-by: Fugang Duan <B38611-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
It's been near enough 3 weeks without a comment from the Device Tree maintainers.
(admittedly it wasn't cc'd to the devicetree list).  I'm going to take this through
the IIO tree.

Applied to the togreg branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
though will go out briefly as the testing branch to let the autobuilders play.

> ---
>   .../devicetree/bindings/iio/adc/vf610-adc.txt      |   22 ++++++++++++++++++++
>   1 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> new file mode 100644
> index 0000000..dcebff1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> @@ -0,0 +1,22 @@
> +Freescale vf610 Analog to Digital Converter bindings
> +
> +The devicetree bindings are for the new ADC driver written for
> +vf610/i.MX6slx and upward SoCs from Freescale.
> +
> +Required properties:
> +- compatible: Should contain "fsl,vf610-adc"
> +- reg: Offset and length of the register set for the device
> +- interrupts: Should contain the interrupt for the device
> +- clocks: The clock is needed by the ADC controller, ADC clock source is ipg clock.
> +- clock-names: Must contain "adc", matching entry in the clocks property.
> +- vref-supply: The regulator supply ADC refrence voltage.
> +
> +Example:
> +adc0: adc@4003b000 {
> +	compatible = "fsl,vf610-adc";
> +	reg = <0x4003b000 0x1000>;
> +	interrupts = <0 53 0x04>;
> +	clocks = <&clks VF610_CLK_ADC0>;
> +	clock-names = "adc";
> +	vref-supply = <&reg_vcc_3v3_mcu>;
> +};
>

Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Shawn Guo]

Copy more DT folks and lists, as I want to make sure everyone agrees on
how the fixed regulators should organized in the device tree sources,
before I apply the patch.

On Sun, Jan 26, 2014 at 01:39:31PM +0800, Fugang Duan wrote:
> vf610 has two ADC controllers, and vf610-twr board ADC0_SE5 pin connect
> to sliding rheostat for ADC test, other ADC pins connect to connectors for
> future use.
> 
> Add support for ADC0_SE5.
> 
> CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> CC: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> CC: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> CC: Otavio Salvador <otavio-fKevB0iiKLMBZ+LybsDmbA@public.gmane.org>
> CC: Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
> CC: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
> Signed-off-by: Fugang Duan <B38611-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  arch/arm/boot/dts/vf610-twr.dts |   21 +++++++++++++++++++++
>  arch/arm/boot/dts/vf610.dtsi    |   26 ++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
> index c8047ca..d867be3 100644
> --- a/arch/arm/boot/dts/vf610-twr.dts
> +++ b/arch/arm/boot/dts/vf610-twr.dts
> @@ -34,6 +34,27 @@
>  		};
>  	};
>  
> +	regulators {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		reg_vcc_3v3_mcu: regulator@0 {
> +			compatible = "regulator-fixed";
> +			reg = <0>;
> +			regulator-name = "vcc_3v3_mcu";
> +			regulator-min-microvolt = <3300000>;
> +			regulator-max-microvolt = <3300000>;
> +		};
> +	};

Per discussion [1], Mark Rutland suggests that instead of organizing
the fixed regulator nodes in a simple-bus container, it should be put
under root node directly like below.

/ {
	reg_vcc_3v3_mcu: regulator_0 {
		compatible = "regulator-fixed";
		regulator-name = "vcc_3v3_mcu";
		regulator-min-microvolt = <3300000>;
		regulator-max-microvolt = <3300000>;
	};
};

Is this what all DT folks agree on?  At least the node name should be
'regulator-0' since it's more idiomatic to use '-' than '_' in node
name?

Shawn

[1] http://thread.gmane.org/gmane.linux.drivers.devicetree/61467/focus=300895

> +
> +};
> +
> +&adc0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_adc0_ad5>;
> +	vref-supply = <&reg_vcc_3v3_mcu>;
> +	status = "okay";
>  };
>  
>  &dspi0 {
> diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
> index d31ce1b..b5b21ea 100644
> --- a/arch/arm/boot/dts/vf610.dtsi
> +++ b/arch/arm/boot/dts/vf610.dtsi
> @@ -152,6 +152,15 @@
>  				clock-names = "pit";
>  			};
>  
> +			adc0: adc@4003b000 {
> +				compatible = "fsl,vf610-adc";
> +				reg = <0x4003b000 0x1000>;
> +				interrupts = <0 53 0x04>;
> +				clocks = <&clks VF610_CLK_ADC0>;
> +				clock-names = "adc";
> +				status = "disabled";
> +			};
> +
>  			wdog@4003e000 {
>  				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";
>  				reg = <0x4003e000 0x1000>;
> @@ -178,6 +187,14 @@
>  
>  				/* functions and groups pins */
>  
> +				adc0 {
> +					pinctrl_adc0_ad5: adc0_ad5 {
> +						fsl,pins = <
> +						VF610_PAD_PTC30__ADC0_SE5	0xa1
> +						>;
> +					};
> +				};
> +
>  				dcu0 {
>  					pinctrl_dcu0_1: dcu0grp_1 {
>  						fsl,pins = <
> @@ -450,6 +467,15 @@
>  				status = "disabled";
>  			};
>  
> +			adc1: adc@400bb000 {
> +				compatible = "fsl,vf610-adc";
> +				reg = <0x400bb000 0x1000>;
> +				interrupts = <0 54 0x04>;
> +				clocks = <&clks VF610_CLK_ADC1>;
> +				clock-names = "adc";
> +				status = "disabled";
> +			};
> +
>  			fec0: ethernet@400d0000 {
>  				compatible = "fsl,mvf600-fec";
>  				reg = <0x400d0000 0x1000>;
> -- 
> 1.7.2.rc3
> 
> 

Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Shawn Guo]

On Sun, Feb 16, 2014 at 03:48:53PM +0800, Shawn Guo wrote:
> Copy more DT folks and lists, as I want to make sure everyone agrees on
> how the fixed regulators should organized in the device tree sources,
> before I apply the patch.
> 
> On Sun, Jan 26, 2014 at 01:39:31PM +0800, Fugang Duan wrote:
> > vf610 has two ADC controllers, and vf610-twr board ADC0_SE5 pin connect
> > to sliding rheostat for ADC test, other ADC pins connect to connectors for
> > future use.
> > 
> > Add support for ADC0_SE5.
> > 
> > CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > CC: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > CC: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> > CC: Otavio Salvador <otavio-fKevB0iiKLMBZ+LybsDmbA@public.gmane.org>
> > CC: Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
> > CC: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
> > Signed-off-by: Fugang Duan <B38611-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/vf610-twr.dts |   21 +++++++++++++++++++++
> >  arch/arm/boot/dts/vf610.dtsi    |   26 ++++++++++++++++++++++++++
> >  2 files changed, 47 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
> > index c8047ca..d867be3 100644
> > --- a/arch/arm/boot/dts/vf610-twr.dts
> > +++ b/arch/arm/boot/dts/vf610-twr.dts
> > @@ -34,6 +34,27 @@
> >  		};
> >  	};
> >  
> > +	regulators {
> > +		compatible = "simple-bus";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		reg_vcc_3v3_mcu: regulator@0 {
> > +			compatible = "regulator-fixed";
> > +			reg = <0>;
> > +			regulator-name = "vcc_3v3_mcu";
> > +			regulator-min-microvolt = <3300000>;
> > +			regulator-max-microvolt = <3300000>;
> > +		};
> > +	};
> 
> Per discussion [1], Mark Rutland suggests that instead of organizing
> the fixed regulator nodes in a simple-bus container, it should be put
> under root node directly like below.
> 
> / {
> 	reg_vcc_3v3_mcu: regulator_0 {
> 		compatible = "regulator-fixed";
> 		regulator-name = "vcc_3v3_mcu";
> 		regulator-min-microvolt = <3300000>;
> 		regulator-max-microvolt = <3300000>;
> 	};
> };
> 
> Is this what all DT folks agree on?  At least the node name should be
> 'regulator-0' since it's more idiomatic to use '-' than '_' in node
> name?

It looks that Mark is giving up [1].  And we're fine with the original
code then.

Shawn

[1] http://thread.gmane.org/gmane.linux.drivers.devicetree/61894/focus=62405

RE: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[fugang.duan-KZfg59tc24xl57MIdRCFDg]

From: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Data: Wednesday, February 19, 2014 1:07 PM + 0800

>To: Duan Fugang-B38611
>Cc: jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; sachin.kamat-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org;
>lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org; mark.rutland-5wv7dgnIgG8@public.gmane.org; linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Rob Herring;
>Grant Likely; Pawel Moll; Ian Campbell; Kumar Gala; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
>linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>Subject: Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support
>
>On Sun, Feb 16, 2014 at 03:48:53PM +0800, Shawn Guo wrote:
>> Copy more DT folks and lists, as I want to make sure everyone agrees
>> on how the fixed regulators should organized in the device tree
>> sources, before I apply the patch.
>>
>> On Sun, Jan 26, 2014 at 01:39:31PM +0800, Fugang Duan wrote:
>> > vf610 has two ADC controllers, and vf610-twr board ADC0_SE5 pin
>> > connect to sliding rheostat for ADC test, other ADC pins connect to
>> > connectors for future use.
>> >
>> > Add support for ADC0_SE5.
>> >
>> > CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> > CC: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> > CC: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>> > CC: Otavio Salvador <otavio-fKevB0iiKLMBZ+LybsDmbA@public.gmane.org>
>> > CC: Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
>> > CC: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
>> > Signed-off-by: Fugang Duan <B38611-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> > ---
>> >  arch/arm/boot/dts/vf610-twr.dts |   21 +++++++++++++++++++++
>> >  arch/arm/boot/dts/vf610.dtsi    |   26 ++++++++++++++++++++++++++
>> >  2 files changed, 47 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/arm/boot/dts/vf610-twr.dts
>> > b/arch/arm/boot/dts/vf610-twr.dts index c8047ca..d867be3 100644
>> > --- a/arch/arm/boot/dts/vf610-twr.dts
>> > +++ b/arch/arm/boot/dts/vf610-twr.dts
>> > @@ -34,6 +34,27 @@
>> >  		};
>> >  	};
>> >
>> > +	regulators {
>> > +		compatible = "simple-bus";
>> > +		#address-cells = <1>;
>> > +		#size-cells = <0>;
>> > +
>> > +		reg_vcc_3v3_mcu: regulator@0 {
>> > +			compatible = "regulator-fixed";
>> > +			reg = <0>;
>> > +			regulator-name = "vcc_3v3_mcu";
>> > +			regulator-min-microvolt = <3300000>;
>> > +			regulator-max-microvolt = <3300000>;
>> > +		};
>> > +	};
>>
>> Per discussion [1], Mark Rutland suggests that instead of organizing
>> the fixed regulator nodes in a simple-bus container, it should be put
>> under root node directly like below.
>>
>> / {
>> 	reg_vcc_3v3_mcu: regulator_0 {
>> 		compatible = "regulator-fixed";
>> 		regulator-name = "vcc_3v3_mcu";
>> 		regulator-min-microvolt = <3300000>;
>> 		regulator-max-microvolt = <3300000>;
>> 	};
>> };
>>
>> Is this what all DT folks agree on?  At least the node name should be
>> 'regulator-0' since it's more idiomatic to use '-' than '_' in node
>> name?
>
>It looks that Mark is giving up [1].  And we're fine with the original code
>then.
>
>Shawn

So, you apply the original patch ? Do I need to resend it again ?

Thanks,
Andy

Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Shawn Guo]

On Fri, Feb 21, 2014 at 05:17:20AM +0000, fugang.duan-KZfg59tc24xl57MIdRCFDg@public.gmane.org wrote:
> So, you apply the original patch ? Do I need to resend it again ?

You still need to rebase it against my for-next branch to resolve my
comment on pinctrl setting.

Shawn

RE: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[fugang.duan]

From: Shawn Guo <shawn.guo@linaro.org>
Data: Friday, February 21, 2014 1:39 PM + 0800

>To: Duan Fugang-B38611
>Cc: jic23@kernel.org; sachin.kamat@linaro.org; pmeerw@pmeerw.net;
>lars@metafoo.de; mark.rutland@arm.com; linux-iio@vger.kernel.org; Rob Herring;
>Grant Likely; Pawel Moll; Ian Campbell; Kumar Gala; devicetree@vger.kernel.org;
>linux-arm-kernel@lists.infradead.org
>Subject: Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support
>
>On Fri, Feb 21, 2014 at 05:17:20AM +0000, fugang.duan@freescale.com wrote:
>> So, you apply the original patch ? Do I need to resend it again ?
>
>You still need to rebase it against my for-next branch to resolve my comment on
>pinctrl setting.
>
>Shawn

I send the patch to you after moving pinctrl setting to board dts.

Thanks,
Andy