[PATCH v5 2/6] [media] ad5820: DT new optional field enable-gpios

[PATCH v5 2/6] [media] ad5820: DT new optional field enable-gpios

[Ricardo Ribalda Delgado]

Document new enable-gpio field. It can be used to disable the part
without turning down its regulator.

Cc: devicetree@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 Documentation/devicetree/bindings/media/i2c/ad5820.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
index 5940ca11c021..9ccd96d3d5f0 100644
--- a/Documentation/devicetree/bindings/media/i2c/ad5820.txt
+++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
@@ -8,6 +8,12 @@ Required Properties:
 
   - VANA-supply: supply of voltage for VANA pin
 
+Optional properties:
+
+   - enable-gpios : GPIO spec for the XSHUTDOWN pin. Note that the polarity of
+the enable GPIO is the opposite of the XSHUTDOWN pin (asserting the enable
+GPIO deasserts the XSHUTDOWN signal and vice versa).
+
 Example:
 
        ad5820: coil@c {
@@ -15,5 +21,6 @@ Example:
                reg = <0x0c>;
 
                VANA-supply = <&vaux4>;
+               enable-gpios = <&msmgpio 26 GPIO_ACTIVE_HIGH>;
        };
 
-- 
2.19.0

[PATCH v5 5/6] [media] ad5820: DT new compatible devices

[Ricardo Ribalda Delgado]

Document new compatible devices.

Cc: devicetree@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/media/i2c/ad5820.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
index 9ccd96d3d5f0..cc7b10fe0368 100644
--- a/Documentation/devicetree/bindings/media/i2c/ad5820.txt
+++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
@@ -2,7 +2,10 @@
 
 Required Properties:
 
-  - compatible: Must contain "adi,ad5820"
+  - compatible: Must contain one of:
+		- "adi,ad5820"
+		- "adi,ad5821"
+		- "adi,ad5823"
 
   - reg: I2C slave address
 
-- 
2.19.0

Re: [PATCH v5 2/6] [media] ad5820: DT new optional field enable-gpios

[Laurent Pinchart]

Hi Ricardo,

Thank you for the patch.

On Tuesday, 2 October 2018 10:32:18 EEST Ricardo Ribalda Delgado wrote:
> Document new enable-gpio field. It can be used to disable the part
> without turning down its regulator.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
>  Documentation/devicetree/bindings/media/i2c/ad5820.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> b/Documentation/devicetree/bindings/media/i2c/ad5820.txt index
> 5940ca11c021..9ccd96d3d5f0 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> @@ -8,6 +8,12 @@ Required Properties:
> 
>    - VANA-supply: supply of voltage for VANA pin
> 
> +Optional properties:
> +
> +   - enable-gpios : GPIO spec for the XSHUTDOWN pin. Note that the polarity
> of +the enable GPIO is the opposite of the XSHUTDOWN pin (asserting the
> enable +GPIO deasserts the XSHUTDOWN signal and vice versa).

After reading this one more time, I think the text is at the very least 
confusing. The logic level of the enable GPIO is the same as the logic level 
of the XSHUTDOWN pin. The latter being active low, asserting "enable" will 
deassert "shutdown", but talking about "desserting XSHUTDOWN" is confusing.

>  Example:
> 
>         ad5820: coil@c {
> @@ -15,5 +21,6 @@ Example:
>                 reg = <0x0c>;
> 
>                 VANA-supply = <&vaux4>;
> +               enable-gpios = <&msmgpio 26 GPIO_ACTIVE_HIGH>;
>         };


-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 2/6] [media] ad5820: DT new optional field enable-gpios

[Ricardo Ribalda Delgado]

Hi Laurent
On Tue, Oct 2, 2018 at 12:35 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
>
> On Tuesday, 2 October 2018 10:32:18 EEST Ricardo Ribalda Delgado wrote:
> > Document new enable-gpio field. It can be used to disable the part
> > without turning down its regulator.
> >
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> > Acked-by: Pavel Machek <pavel@ucw.cz>
> > ---
> >  Documentation/devicetree/bindings/media/i2c/ad5820.txt | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> > b/Documentation/devicetree/bindings/media/i2c/ad5820.txt index
> > 5940ca11c021..9ccd96d3d5f0 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> > +++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> > @@ -8,6 +8,12 @@ Required Properties:
> >
> >    - VANA-supply: supply of voltage for VANA pin
> >
> > +Optional properties:
> > +
> > +   - enable-gpios : GPIO spec for the XSHUTDOWN pin. Note that the polarity
> > of +the enable GPIO is the opposite of the XSHUTDOWN pin (asserting the
> > enable +GPIO deasserts the XSHUTDOWN signal and vice versa).
>
> After reading this one more time, I think the text is at the very least
> confusing. The logic level of the enable GPIO is the same as the logic level
> of the XSHUTDOWN pin. The latter being active low, asserting "enable" will
> deassert "shutdown", but talking about "desserting XSHUTDOWN" is confusing.
>

what about:

- enable-gpios : GPIO spec for the XSHUTDOWN pin. When the XSHUTDOWN pin
is asserted the device is enabled.

> > of +the enable GPIO is the opposite of the XSHUTDOWN pin (asserting the
> > enable +GPIO deasserts the XSHUTDOWN signal and vice versa).

> >  Example:
> >
> >         ad5820: coil@c {
> > @@ -15,5 +21,6 @@ Example:
> >                 reg = <0x0c>;
> >
> >                 VANA-supply = <&vaux4>;
> > +               enable-gpios = <&msmgpio 26 GPIO_ACTIVE_HIGH>;
> >         };
>
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>


-- 
Ricardo Ribalda

Re: [PATCH v5 2/6] [media] ad5820: DT new optional field enable-gpios

[Laurent Pinchart]

Hi Ricardo,

On Tuesday, 2 October 2018 13:57:22 EEST Ricardo Ribalda Delgado wrote:
> On Tue, Oct 2, 2018 at 12:35 PM Laurent Pinchart wrote:
> > On Tuesday, 2 October 2018 10:32:18 EEST Ricardo Ribalda Delgado wrote:
> > > Document new enable-gpio field. It can be used to disable the part
> > > without turning down its regulator.
> > > 
> > > Cc: devicetree@vger.kernel.org
> > > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> > > Acked-by: Pavel Machek <pavel@ucw.cz>
> > > ---
> > > 
> > >  Documentation/devicetree/bindings/media/i2c/ad5820.txt | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> > > b/Documentation/devicetree/bindings/media/i2c/ad5820.txt index
> > > 5940ca11c021..9ccd96d3d5f0 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> > > +++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> > > 
> > > @@ -8,6 +8,12 @@ Required Properties:
> > >    - VANA-supply: supply of voltage for VANA pin
> > > 
> > > +Optional properties:
> > > +
> > > +   - enable-gpios : GPIO spec for the XSHUTDOWN pin. Note that the
> > > polarity of +the enable GPIO is the opposite of the XSHUTDOWN pin
> > > (asserting the enable +GPIO deasserts the XSHUTDOWN signal and vice
> > > versa).
> > 
> > After reading this one more time, I think the text is at the very least
> > confusing. The logic level of the enable GPIO is the same as the logic
> > level of the XSHUTDOWN pin. The latter being active low, asserting
> > "enable" will deassert "shutdown", but talking about "desserting
> > XSHUTDOWN" is confusing.
> 
> what about:
> 
> - enable-gpios : GPIO spec for the XSHUTDOWN pin. When the XSHUTDOWN pin
> is asserted the device is enabled.

For some reason "asserting a pin" doesn't seem right to me. How about

- enable-gpios: GPIO spec for the XSHUTDOWN pin. The XSHUTDOWN signal is 
active low, a high level on the pin enables the device.

> > > of +the enable GPIO is the opposite of the XSHUTDOWN pin (asserting the
> > > enable +GPIO deasserts the XSHUTDOWN signal and vice versa).
> > > 
> > >  Example:
> > >         ad5820: coil@c {
> > > 
> > > @@ -15,5 +21,6 @@ Example:
> > >                 reg = <0x0c>;
> > >                 
> > >                 VANA-supply = <&vaux4>;
> > > 
> > > +               enable-gpios = <&msmgpio 26 GPIO_ACTIVE_HIGH>;
> > > 
> > >         };
> > 
> > --
> > Regards,
> > 
> > Laurent Pinchart


-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 5/6] [media] ad5820: DT new compatible devices

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 979 bytes --]

On Tue 2018-10-02 09:32:21, Ricardo Ribalda Delgado wrote:
> Document new compatible devices.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Acked-by: Pavel Machek <pavel@ucw.cz>

> diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> index 9ccd96d3d5f0..cc7b10fe0368 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> @@ -2,7 +2,10 @@
>  
>  Required Properties:
>  
> -  - compatible: Must contain "adi,ad5820"
> +  - compatible: Must contain one of:
> +		- "adi,ad5820"
> +		- "adi,ad5821"
> +		- "adi,ad5823"
>  
>    - reg: I2C slave address
>  

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]