* [PATCH v4 3/7] [media] ad5820: DT new optional field enable-gpios [not found] <20180920204751.29117-1-ricardo.ribalda@gmail.com> @ 2018-09-20 20:47 ` Ricardo Ribalda Delgado 2018-09-27 18:23 ` Rob Herring 2018-09-20 20:47 ` [PATCH v4 7/7] [media] ad5820: DT new compatible devices Ricardo Ribalda Delgado 1 sibling, 1 reply; 10+ messages in thread From: Ricardo Ribalda Delgado @ 2018-09-20 20:47 UTC (permalink / raw) To: Pavel Machek, Sakari Ailus, Mauro Carvalho Chehab, linux-media, linux-kernel, Hans Verkuil, Laurent Pinchart Cc: Ricardo Ribalda Delgado, devicetree 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.18.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/7] [media] ad5820: DT new optional field enable-gpios 2018-09-20 20:47 ` [PATCH v4 3/7] [media] ad5820: DT new optional field enable-gpios Ricardo Ribalda Delgado @ 2018-09-27 18:23 ` Rob Herring 2018-10-01 8:20 ` Ricardo Ribalda Delgado 0 siblings, 1 reply; 10+ messages in thread From: Rob Herring @ 2018-09-27 18:23 UTC (permalink / raw) To: Ricardo Ribalda Delgado Cc: Pavel Machek, Sakari Ailus, Mauro Carvalho Chehab, linux-media, linux-kernel, Hans Verkuil, Laurent Pinchart, devicetree On Thu, Sep 20, 2018 at 10:47:47PM +0200, Ricardo Ribalda Delgado wrote: > Document new enable-gpio field. It can be used to disable the part enable-gpios > 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). shutdown-gpios is also standard and seems like it would make more sense here. Yes, it is a bit redundant to have both, but things just evolved that way and we don't want to totally abandon the hardware names (just all the variants). Rob ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/7] [media] ad5820: DT new optional field enable-gpios 2018-09-27 18:23 ` Rob Herring @ 2018-10-01 8:20 ` Ricardo Ribalda Delgado 2018-10-01 12:36 ` Rob Herring 0 siblings, 1 reply; 10+ messages in thread From: Ricardo Ribalda Delgado @ 2018-10-01 8:20 UTC (permalink / raw) To: Rob Herring Cc: Pavel Machek, Sakari Ailus, Mauro Carvalho Chehab, linux-media, LKML, Hans Verkuil, Laurent Pinchart, devicetree Hi Rob On Thu, Sep 27, 2018 at 8:23 PM Rob Herring <robh@kernel.org> wrote: > > On Thu, Sep 20, 2018 at 10:47:47PM +0200, Ricardo Ribalda Delgado wrote: > > Document new enable-gpio field. It can be used to disable the part > > enable-gpios > > > 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). > > shutdown-gpios is also standard and seems like it would make more sense > here. Yes, it is a bit redundant to have both, but things just evolved > that way and we don't want to totally abandon the hardware names (just > all the variants). > Sorry to insist The pin is called xshutdown, not shutdown and is inverse logic, Wouldnt it make more sense to use the name enable-gpios? Regards > Rob -- Ricardo Ribalda ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/7] [media] ad5820: DT new optional field enable-gpios 2018-10-01 8:20 ` Ricardo Ribalda Delgado @ 2018-10-01 12:36 ` Rob Herring 2018-10-01 12:40 ` Ricardo Ribalda Delgado 0 siblings, 1 reply; 10+ messages in thread From: Rob Herring @ 2018-10-01 12:36 UTC (permalink / raw) To: Ricardo Ribalda Delgado Cc: Pavel Machek, Sakari Ailus, Mauro Carvalho Chehab, Linux Media Mailing List, linux-kernel@vger.kernel.org, Hans Verkuil, Laurent Pinchart, devicetree On Mon, Oct 1, 2018 at 3:20 AM Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote: > > Hi Rob > On Thu, Sep 27, 2018 at 8:23 PM Rob Herring <robh@kernel.org> wrote: > > > > On Thu, Sep 20, 2018 at 10:47:47PM +0200, Ricardo Ribalda Delgado wrote: > > > Document new enable-gpio field. It can be used to disable the part > > > > enable-gpios > > > > > 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). > > > > shutdown-gpios is also standard and seems like it would make more sense > > here. Yes, it is a bit redundant to have both, but things just evolved > > that way and we don't want to totally abandon the hardware names (just > > all the variants). > > > > Sorry to insist > > The pin is called xshutdown, not shutdown and is inverse logic, > Wouldnt it make more sense to use the name > enable-gpios? Inverse of what? shutdown-gpios is the inverse of enable-gpios. By using shutdown-gpios you can just get rid of "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)." This looks to me like a case of just standardizing the name so for example we just have "reset" instead of many flavors like rst, RSTb, RESETb, RESETn, nRESET, etc. Rob ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/7] [media] ad5820: DT new optional field enable-gpios 2018-10-01 12:36 ` Rob Herring @ 2018-10-01 12:40 ` Ricardo Ribalda Delgado 2018-10-01 15:01 ` Rob Herring 0 siblings, 1 reply; 10+ messages in thread From: Ricardo Ribalda Delgado @ 2018-10-01 12:40 UTC (permalink / raw) To: Rob Herring Cc: Pavel Machek, Sakari Ailus, Mauro Carvalho Chehab, linux-media, LKML, Hans Verkuil, Laurent Pinchart, devicetree Hi On Mon, Oct 1, 2018 at 2:36 PM Rob Herring <robh@kernel.org> wrote: > > On Mon, Oct 1, 2018 at 3:20 AM Ricardo Ribalda Delgado > <ricardo.ribalda@gmail.com> wrote: > > > > Hi Rob > > On Thu, Sep 27, 2018 at 8:23 PM Rob Herring <robh@kernel.org> wrote: > > > > > > On Thu, Sep 20, 2018 at 10:47:47PM +0200, Ricardo Ribalda Delgado wrote: > > > > Document new enable-gpio field. It can be used to disable the part > > > > > > enable-gpios > > > > > > > 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). > > > > > > shutdown-gpios is also standard and seems like it would make more sense > > > here. Yes, it is a bit redundant to have both, but things just evolved > > > that way and we don't want to totally abandon the hardware names (just > > > all the variants). > > > > > > > Sorry to insist > > > > The pin is called xshutdown, not shutdown and is inverse logic, > > Wouldnt it make more sense to use the name > > enable-gpios? > > Inverse of what? shutdown-gpios is the inverse of enable-gpios. By > using shutdown-gpios you can just get rid of "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)." The pin is called XSHUTDOWN 0V means shutdown 3.3V means enable This is why I think is more clear to use enable as name in the device tree. > > This looks to me like a case of just standardizing the name so for > example we just have "reset" instead of many flavors like rst, RSTb, > RESETb, RESETn, nRESET, etc. > > Rob -- Ricardo Ribalda ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/7] [media] ad5820: DT new optional field enable-gpios 2018-10-01 12:40 ` Ricardo Ribalda Delgado @ 2018-10-01 15:01 ` Rob Herring 2018-10-01 15:55 ` Laurent Pinchart 0 siblings, 1 reply; 10+ messages in thread From: Rob Herring @ 2018-10-01 15:01 UTC (permalink / raw) To: Ricardo Ribalda Delgado Cc: Pavel Machek, Sakari Ailus, Mauro Carvalho Chehab, Linux Media Mailing List, linux-kernel@vger.kernel.org, Hans Verkuil, Laurent Pinchart, devicetree On Mon, Oct 1, 2018 at 7:40 AM Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote: > > Hi > On Mon, Oct 1, 2018 at 2:36 PM Rob Herring <robh@kernel.org> wrote: > > > > On Mon, Oct 1, 2018 at 3:20 AM Ricardo Ribalda Delgado > > <ricardo.ribalda@gmail.com> wrote: > > > > > > Hi Rob > > > On Thu, Sep 27, 2018 at 8:23 PM Rob Herring <robh@kernel.org> wrote: > > > > > > > > On Thu, Sep 20, 2018 at 10:47:47PM +0200, Ricardo Ribalda Delgado wrote: > > > > > Document new enable-gpio field. It can be used to disable the part > > > > > > > > enable-gpios > > > > > > > > > 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). > > > > > > > > shutdown-gpios is also standard and seems like it would make more sense > > > > here. Yes, it is a bit redundant to have both, but things just evolved > > > > that way and we don't want to totally abandon the hardware names (just > > > > all the variants). > > > > > > > > > > Sorry to insist > > > > > > The pin is called xshutdown, not shutdown and is inverse logic, > > > Wouldnt it make more sense to use the name > > > enable-gpios? > > > > Inverse of what? shutdown-gpios is the inverse of enable-gpios. By > > using shutdown-gpios you can just get rid of "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)." > > The pin is called XSHUTDOWN > > 0V means shutdown > > 3.3V means enable > > This is why I think is more clear to use enable as name in the device tree. Neither enable-gpios nor shutdown-gpios have a defined polarity. The polarity is part of the flags cell in the specifier. Rob ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/7] [media] ad5820: DT new optional field enable-gpios 2018-10-01 15:01 ` Rob Herring @ 2018-10-01 15:55 ` Laurent Pinchart 2018-10-02 7:18 ` Ricardo Ribalda Delgado 0 siblings, 1 reply; 10+ messages in thread From: Laurent Pinchart @ 2018-10-01 15:55 UTC (permalink / raw) To: Rob Herring Cc: Ricardo Ribalda Delgado, Pavel Machek, Sakari Ailus, Mauro Carvalho Chehab, Linux Media Mailing List, linux-kernel@vger.kernel.org, Hans Verkuil, devicetree Hello, On Monday, 1 October 2018 18:01:42 EEST Rob Herring wrote: > On Mon, Oct 1, 2018 at 7:40 AM Ricardo Ribalda Delgado wrote: > > On Mon, Oct 1, 2018 at 2:36 PM Rob Herring wrote: > >> On Mon, Oct 1, 2018 at 3:20 AM Ricardo Ribalda Delgado wrote: > >>> On Thu, Sep 27, 2018 at 8:23 PM Rob Herring wrote: > >>>> On Thu, Sep 20, 2018 at 10:47:47PM +0200, Ricardo Ribalda Delgado wrote: > >>>>> Document new enable-gpio field. It can be used to disable the part > >>>>> enable-gpios 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). > >>>> > >>>> shutdown-gpios is also standard and seems like it would make more > >>>> sense here. Yes, it is a bit redundant to have both, but things just > >>>> evolved that way and we don't want to totally abandon the hardware > >>>> names (just all the variants). > >>> > >>> Sorry to insist > >>> > >>> The pin is called xshutdown, not shutdown and is inverse logic, > >>> Wouldnt it make more sense to use the name enable-gpios? > >> > >> Inverse of what? shutdown-gpios is the inverse of enable-gpios. By > >> using shutdown-gpios you can just get rid of "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)." > > > > The pin is called XSHUTDOWN > > > > 0V means shutdown > > > > 3.3V means enable > > > > This is why I think is more clear to use enable as name in the device > > tree. > > Neither enable-gpios nor shutdown-gpios have a defined polarity. The > polarity is part of the flags cell in the specifier. To be precise, the polarity is the relationship between the logical level (low or high) *on the GPIO side* and the logical state (asserted or deasserted) of the signal *on the device side*. This is important in order to take all components on the signal path into account, such as inverters on the board. The polarity does tell what level to output on the GPIO in order to achieve a given effect. The polarity, however, doesn't dictate what effect is expected. This is defined by the DT bindings (together with the documentation of the device). For instance an enable-gpios property in DT implies that an asserted logical state will enable the device. The GPIO polarity flags thus take into account a possible inverter at the device input (as in the difference between the ENABLE and nENABLE signals), but stops there. In this case, we have an XSHUTDOWN pin that will shut the device down when driven to 0V. If we call the related DT property shutdown, its logical level will be the inverse of XSHUTDOWN: the signal will need to be driven low to assert the shutdown effect. The GPIO flags will thus need to take this into account, and documenting it in DT could be useful to avoid errors. On the other hand, if we call the related DT property enable its logical level will the the same as XSHUTDOWN: the signal will need to be driven high to assert the enable effect. On the driver side we would have to deassert shutdown or assert enable to enable the device. I don't mind which option is selected, as long as the DT bindings are clear (without any inverter in the signal path beside the one inside the ad5820, the polarity would need to be high for the enable case and low for the shutdown case). -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/7] [media] ad5820: DT new optional field enable-gpios 2018-10-01 15:55 ` Laurent Pinchart @ 2018-10-02 7:18 ` Ricardo Ribalda Delgado 0 siblings, 0 replies; 10+ messages in thread From: Ricardo Ribalda Delgado @ 2018-10-02 7:18 UTC (permalink / raw) To: Laurent Pinchart Cc: Rob Herring, Pavel Machek, Sakari Ailus, Mauro Carvalho Chehab, linux-media, LKML, Hans Verkuil, devicetree Hi Laurent On Mon, Oct 1, 2018 at 5:55 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hello, > > On Monday, 1 October 2018 18:01:42 EEST Rob Herring wrote: > > On Mon, Oct 1, 2018 at 7:40 AM Ricardo Ribalda Delgado wrote: > > > On Mon, Oct 1, 2018 at 2:36 PM Rob Herring wrote: > > >> On Mon, Oct 1, 2018 at 3:20 AM Ricardo Ribalda Delgado wrote: > > >>> On Thu, Sep 27, 2018 at 8:23 PM Rob Herring wrote: > > >>>> On Thu, Sep 20, 2018 at 10:47:47PM +0200, Ricardo Ribalda Delgado > wrote: > > >>>>> Document new enable-gpio field. It can be used to disable the part > > >>>>> enable-gpios 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). > > >>>> > > >>>> shutdown-gpios is also standard and seems like it would make more > > >>>> sense here. Yes, it is a bit redundant to have both, but things just > > >>>> evolved that way and we don't want to totally abandon the hardware > > >>>> names (just all the variants). > > >>> > > >>> Sorry to insist > > >>> > > >>> The pin is called xshutdown, not shutdown and is inverse logic, > > >>> Wouldnt it make more sense to use the name enable-gpios? > > >> > > >> Inverse of what? shutdown-gpios is the inverse of enable-gpios. By > > >> using shutdown-gpios you can just get rid of "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)." > > > > > > The pin is called XSHUTDOWN > > > > > > 0V means shutdown > > > > > > 3.3V means enable > > > > > > This is why I think is more clear to use enable as name in the device > > > tree. > > > > Neither enable-gpios nor shutdown-gpios have a defined polarity. The > > polarity is part of the flags cell in the specifier. > > To be precise, the polarity is the relationship between the logical level (low > or high) *on the GPIO side* and the logical state (asserted or deasserted) of > the signal *on the device side*. This is important in order to take all > components on the signal path into account, such as inverters on the board. > The polarity does tell what level to output on the GPIO in order to achieve a > given effect. > > The polarity, however, doesn't dictate what effect is expected. This is > defined by the DT bindings (together with the documentation of the device). > For instance an enable-gpios property in DT implies that an asserted logical > state will enable the device. The GPIO polarity flags thus take into account a > possible inverter at the device input (as in the difference between the ENABLE > and nENABLE signals), but stops there. > > In this case, we have an XSHUTDOWN pin that will shut the device down when > driven to 0V. If we call the related DT property shutdown, its logical level > will be the inverse of XSHUTDOWN: the signal will need to be driven low to > assert the shutdown effect. The GPIO flags will thus need to take this into > account, and documenting it in DT could be useful to avoid errors. > > On the other hand, if we call the related DT property enable its logical level > will the the same as XSHUTDOWN: the signal will need to be driven high to > assert the enable effect. > > On the driver side we would have to deassert shutdown or assert enable to > enable the device. > > I don't mind which option is selected, as long as the DT bindings are clear > (without any inverter in the signal path beside the one inside the ad5820, the > polarity would need to be high for the enable case and low for the shutdown > case). Thanks for the clarification. I definitely prefer the name enable, so if there is no strong opposition against it I will send it with that name. Best regards! > > -- > Regards, > > Laurent Pinchart > > > -- Ricardo Ribalda ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 7/7] [media] ad5820: DT new compatible devices [not found] <20180920204751.29117-1-ricardo.ribalda@gmail.com> 2018-09-20 20:47 ` [PATCH v4 3/7] [media] ad5820: DT new optional field enable-gpios Ricardo Ribalda Delgado @ 2018-09-20 20:47 ` Ricardo Ribalda Delgado 2018-09-27 18:23 ` Rob Herring 1 sibling, 1 reply; 10+ messages in thread From: Ricardo Ribalda Delgado @ 2018-09-20 20:47 UTC (permalink / raw) To: Pavel Machek, Sakari Ailus, Mauro Carvalho Chehab, linux-media, linux-kernel, Hans Verkuil, Laurent Pinchart Cc: Ricardo Ribalda Delgado, devicetree Document new compatible devices. Cc: devicetree@vger.kernel.org Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> --- 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.18.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 7/7] [media] ad5820: DT new compatible devices 2018-09-20 20:47 ` [PATCH v4 7/7] [media] ad5820: DT new compatible devices Ricardo Ribalda Delgado @ 2018-09-27 18:23 ` Rob Herring 0 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2018-09-27 18:23 UTC (permalink / raw) Cc: Pavel Machek, Sakari Ailus, Mauro Carvalho Chehab, linux-media, linux-kernel, Hans Verkuil, Laurent Pinchart, Ricardo Ribalda Delgado, devicetree On Thu, 20 Sep 2018 22:47:51 +0200, Ricardo Ribalda Delgado wrote: > Document new compatible devices. > > Cc: devicetree@vger.kernel.org > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> > --- > Documentation/devicetree/bindings/media/i2c/ad5820.txt | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-10-02 7:18 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20180920204751.29117-1-ricardo.ribalda@gmail.com>
2018-09-20 20:47 ` [PATCH v4 3/7] [media] ad5820: DT new optional field enable-gpios Ricardo Ribalda Delgado
2018-09-27 18:23 ` Rob Herring
2018-10-01 8:20 ` Ricardo Ribalda Delgado
2018-10-01 12:36 ` Rob Herring
2018-10-01 12:40 ` Ricardo Ribalda Delgado
2018-10-01 15:01 ` Rob Herring
2018-10-01 15:55 ` Laurent Pinchart
2018-10-02 7:18 ` Ricardo Ribalda Delgado
2018-09-20 20:47 ` [PATCH v4 7/7] [media] ad5820: DT new compatible devices Ricardo Ribalda Delgado
2018-09-27 18:23 ` Rob Herring
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).