[2/5] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply

Message ID 20190208160944.13281-3-gregory.clement@bootlin.com
State Not Applicable
Headers show
Series
  • Adding scale support to the lpc32xx ADC driver
Related show

Checks

Context Check Description
robh/checkpatch success

Commit Message

Gregory CLEMENT Feb. 8, 2019, 4:09 p.m.
As most of the other ADC the lpc32xx one use a vref-supply property:
document it.

Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Jonathan Cameron Feb. 9, 2019, 5:09 p.m. | #1
On Fri,  8 Feb 2019 17:09:41 +0100
Gregory CLEMENT <gregory.clement@bootlin.com> wrote:

> As most of the other ADC the lpc32xx one use a vref-supply property:
> document it.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
Hmm. This is indeed an oddity as you document. Normally we would
insist on it, but we can't because of legacy and as it is actually
queries, we can't even use the fact a stub regulator will be provided
to get around it.

I'll have some comments on the patch implementing it anyway, but
good to let this sit for a while given it's slightly unusual nature.

Thanks,

Jonathan

> ---
>  Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
> index b3629d3a9adf..3a1bc669bd51 100644
> --- a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
> @@ -6,6 +6,10 @@ Required properties:
>    region.
>  - interrupts: The ADC interrupt
>  
> +Optional:
> + - vref-supply: The regulator supply ADC reference voltage, optional
> +   for legacy reason, but highly encouraging to us in new device tree
> +
>  Example:
>  
>  	adc@40048000 {
> @@ -13,4 +17,5 @@ Example:
>  		reg = <0x40048000 0x1000>;
>  		interrupt-parent = <&mic>;
>  		interrupts = <39 0>;
> +		vref-supply = <&vcc>;
>  	};
Vladimir Zapolskiy Feb. 9, 2019, 6:07 p.m. | #2
Hi Gregory,

On 02/08/2019 06:09 PM, Gregory CLEMENT wrote:
> As most of the other ADC the lpc32xx one use a vref-supply property:
> document it.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> ---
>  Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
> index b3629d3a9adf..3a1bc669bd51 100644
> --- a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
> @@ -6,6 +6,10 @@ Required properties:
>    region.
>  - interrupts: The ADC interrupt
>  
> +Optional:
> + - vref-supply: The regulator supply ADC reference voltage, optional
> +   for legacy reason, but highly encouraging to us in new device tree
> +

here vref-supply voltage is 3.3v only, I think that might be a reason why
the property is omitted.

My concern is that the documentation shall not contain sections with
description of properties found in the past for 'legacy reasons', if you
like to support the backward compatibility in the driver, please update
the documentation to the expected state and add a stub to care about
legacy DTBs in the driver only.

FWIW I personally don't mind about breaking the backward compatibility
for NXP LPC32xx, it might be a concern when CONFIG_ARM_ATAG_DTB_COMPAT=y
is removed from the lpc32xx_defconfig though.

>  Example:
>  
>  	adc@40048000 {
> @@ -13,4 +17,5 @@ Example:
>  		reg = <0x40048000 0x1000>;
>  		interrupt-parent = <&mic>;
>  		interrupts = <39 0>;
> +		vref-supply = <&vcc>;
>  	};
> 

I kindly ask you to include me to To: list, if you send another changes for
NXP LPC32xx platform.

Thank you.

--
Best wishes,
Vladimir
Gregory CLEMENT Feb. 15, 2019, 4:07 p.m. | #3
Hi Vladimir,
 
 On sam., févr. 09 2019, Vladimir Zapolskiy <vz@mleia.com> wrote:

> Hi Gregory,
>
> On 02/08/2019 06:09 PM, Gregory CLEMENT wrote:
>> As most of the other ADC the lpc32xx one use a vref-supply property:
>> document it.
>> 
>> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>> ---
>>  Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt | 5 +++++
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
>> index b3629d3a9adf..3a1bc669bd51 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
>> +++ b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
>> @@ -6,6 +6,10 @@ Required properties:
>>    region.
>>  - interrupts: The ADC interrupt
>>  
>> +Optional:
>> + - vref-supply: The regulator supply ADC reference voltage, optional
>> +   for legacy reason, but highly encouraging to us in new device tree
>> +
>
> here vref-supply voltage is 3.3v only, I think that might be a reason why
> the property is omitted.

VDD_AD is an external pin so in theroy it would be possible to use a
different voltage, but indeed the datasheet describe this pin as the
3.3V supply for ADC.

>
> My concern is that the documentation shall not contain sections with
> description of properties found in the past for 'legacy reasons', if you
> like to support the backward compatibility in the driver, please update
> the documentation to the expected state and add a stub to care about
> legacy DTBs in the driver only.

I saw a lot of reference about legacy properties in the binding, and I
found it quite usefull when I read old device trees.

>
> FWIW I personally don't mind about breaking the backward compatibility
> for NXP LPC32xx, it might be a concern when CONFIG_ARM_ATAG_DTB_COMPAT=y
> is removed from the lpc32xx_defconfig though.
>
>>  Example:
>>  
>>  	adc@40048000 {
>> @@ -13,4 +17,5 @@ Example:
>>  		reg = <0x40048000 0x1000>;
>>  		interrupt-parent = <&mic>;
>>  		interrupts = <39 0>;
>> +		vref-supply = <&vcc>;
>>  	};
>> 
>
> I kindly ask you to include me to To: list, if you send another changes for
> NXP LPC32xx platform.

Yes sure! I wrongly only use the first lines provided by
./scripts/get_maintainer.pl, I will include you in the next version.


Thanks,

Gregory



>
> Thank you.
>
> --
> Best wishes,
> Vladimir
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Sylvain Lemieux Feb. 20, 2019, 12:11 p.m. | #4
Hi Gregory,

On Fri, Feb 15, 2019 at 11:07 AM Gregory CLEMENT
<gregory.clement@bootlin.com> wrote:
>
> Hi Vladimir,
>
>  On sam., févr. 09 2019, Vladimir Zapolskiy <vz@mleia.com> wrote:
>
> > Hi Gregory,
> >
> > On 02/08/2019 06:09 PM, Gregory CLEMENT wrote:
> >> As most of the other ADC the lpc32xx one use a vref-supply property:
> >> document it.
> >>
> >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> >> ---
> >>  Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
[...]
> >>
> >
> > I kindly ask you to include me to To: list, if you send another changes for
> > NXP LPC32xx platform.
>
> Yes sure! I wrongly only use the first lines provided by
> ./scripts/get_maintainer.pl, I will include you in the next version.
>

Please also include myself in the next LPC32xx related patches.

Regards,
Sylvain
>
> Thanks,
>
> Gregory
>
>
>
> >
> > Thank you.
> >
> > --
> > Best wishes,
> > Vladimir
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
> --
> Gregory Clement, Bootlin
> Embedded Linux and Kernel engineering
> http://bootlin.com
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Rob Herring Feb. 25, 2019, 11:32 p.m. | #5
On Fri,  8 Feb 2019 17:09:41 +0100, Gregory CLEMENT wrote:
> As most of the other ADC the lpc32xx one use a vref-supply property:
> document it.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> ---
>  Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Patch

diff --git a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
index b3629d3a9adf..3a1bc669bd51 100644
--- a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
@@ -6,6 +6,10 @@  Required properties:
   region.
 - interrupts: The ADC interrupt
 
+Optional:
+ - vref-supply: The regulator supply ADC reference voltage, optional
+   for legacy reason, but highly encouraging to us in new device tree
+
 Example:
 
 	adc@40048000 {
@@ -13,4 +17,5 @@  Example:
 		reg = <0x40048000 0x1000>;
 		interrupt-parent = <&mic>;
 		interrupts = <39 0>;
+		vref-supply = <&vcc>;
 	};