* [PATCH 3/4] [media] ad5820: DT new optional field enable-gpios [not found] <20180920161912.17063-1-ricardo.ribalda@gmail.com> @ 2018-09-20 16:19 ` Ricardo Ribalda Delgado 2018-09-20 20:21 ` Laurent Pinchart 0 siblings, 1 reply; 5+ messages in thread From: Ricardo Ribalda Delgado @ 2018-09-20 16:19 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> --- Documentation/devicetree/bindings/media/i2c/ad5820.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt b/Documentation/devicetree/bindings/media/i2c/ad5820.txt index 5940ca11c021..07d577bb37f7 100644 --- a/Documentation/devicetree/bindings/media/i2c/ad5820.txt +++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt @@ -8,6 +8,11 @@ Required Properties: - VANA-supply: supply of voltage for VANA pin +Optional properties: + + - enable-gpios : GPIO spec for the XSHUTDOWN pin. If specified, it will be + asserted when VANA-supply is enabled. + Example: ad5820: coil@c { @@ -15,5 +20,6 @@ Example: reg = <0x0c>; VANA-supply = <&vaux4>; + enable-gpios = <&msmgpio 26 GPIO_ACTIVE_HIGH>; }; -- 2.18.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 3/4] [media] ad5820: DT new optional field enable-gpios 2018-09-20 16:19 ` [PATCH 3/4] [media] ad5820: DT new optional field enable-gpios Ricardo Ribalda Delgado @ 2018-09-20 20:21 ` Laurent Pinchart 2018-09-20 20:23 ` Laurent Pinchart 0 siblings, 1 reply; 5+ messages in thread From: Laurent Pinchart @ 2018-09-20 20:21 UTC (permalink / raw) To: Ricardo Ribalda Delgado Cc: Pavel Machek, Sakari Ailus, Mauro Carvalho Chehab, linux-media, linux-kernel, Hans Verkuil, devicetree Hi Ricardo, Thank you for the patch. On Thursday, 20 September 2018 19:19:11 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> > --- > Documentation/devicetree/bindings/media/i2c/ad5820.txt | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt > b/Documentation/devicetree/bindings/media/i2c/ad5820.txt index > 5940ca11c021..07d577bb37f7 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ad5820.txt > +++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt > @@ -8,6 +8,11 @@ Required Properties: > > - VANA-supply: supply of voltage for VANA pin > > +Optional properties: > + > + - enable-gpios : GPIO spec for the XSHUTDOWN pin. xshutdown is active-low, so enable is active-high. Should this be documented explicitly, to avoid polarity errors ? Maybe something along the lines of - 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). > If specified, it will be > + asserted when VANA-supply is enabled. That documents a driver behaviour, is it needed in DT ? > Example: > > ad5820: coil@c { > @@ -15,5 +20,6 @@ Example: > reg = <0x0c>; > > VANA-supply = <&vaux4>; > + enable-gpios = <&msmgpio 26 GPIO_ACTIVE_HIGH>; > }; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/4] [media] ad5820: DT new optional field enable-gpios 2018-09-20 20:21 ` Laurent Pinchart @ 2018-09-20 20:23 ` Laurent Pinchart 2018-09-20 20:25 ` Ricardo Ribalda Delgado 0 siblings, 1 reply; 5+ messages in thread From: Laurent Pinchart @ 2018-09-20 20:23 UTC (permalink / raw) To: Ricardo Ribalda Delgado Cc: Pavel Machek, Sakari Ailus, Mauro Carvalho Chehab, linux-media, linux-kernel, Hans Verkuil, devicetree On Thursday, 20 September 2018 23:21:28 EEST Laurent Pinchart wrote: > Hi Ricardo, > > Thank you for the patch. > > On Thursday, 20 September 2018 19:19:11 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> > > --- > > > > Documentation/devicetree/bindings/media/i2c/ad5820.txt | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt > > b/Documentation/devicetree/bindings/media/i2c/ad5820.txt index > > 5940ca11c021..07d577bb37f7 100644 > > --- a/Documentation/devicetree/bindings/media/i2c/ad5820.txt > > +++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt > > > > @@ -8,6 +8,11 @@ Required Properties: > > - VANA-supply: supply of voltage for VANA pin > > > > +Optional properties: > > + > > + - enable-gpios : GPIO spec for the XSHUTDOWN pin. > > xshutdown is active-low, so enable is active-high. Should this be documented > explicitly, to avoid polarity errors ? Maybe something along the lines of > > - 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). Or alternatively you could name the property xshutdown-gpios, as explained in my (incorrect) review of 2/4. > > If specified, it will be > > + asserted when VANA-supply is enabled. > > That documents a driver behaviour, is it needed in DT ? > > > Example: > > ad5820: coil@c { > > > > @@ -15,5 +20,6 @@ Example: > > reg = <0x0c>; > > VANA-supply = <&vaux4>; > > > > + enable-gpios = <&msmgpio 26 GPIO_ACTIVE_HIGH>; > > }; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/4] [media] ad5820: DT new optional field enable-gpios 2018-09-20 20:23 ` Laurent Pinchart @ 2018-09-20 20:25 ` Ricardo Ribalda Delgado 2018-09-20 20:28 ` Pavel Machek 0 siblings, 1 reply; 5+ messages in thread From: Ricardo Ribalda Delgado @ 2018-09-20 20:25 UTC (permalink / raw) To: Laurent Pinchart Cc: Pavel Machek, Sakari Ailus, Mauro Carvalho Chehab, linux-media, LKML, Hans Verkuil, devicetree Hi On Thu, Sep 20, 2018 at 10:23 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > On Thursday, 20 September 2018 23:21:28 EEST Laurent Pinchart wrote: > > Hi Ricardo, > > > > Thank you for the patch. > > > > On Thursday, 20 September 2018 19:19:11 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> > > > --- > > > > > > Documentation/devicetree/bindings/media/i2c/ad5820.txt | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt > > > b/Documentation/devicetree/bindings/media/i2c/ad5820.txt index > > > 5940ca11c021..07d577bb37f7 100644 > > > --- a/Documentation/devicetree/bindings/media/i2c/ad5820.txt > > > +++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt > > > > > > @@ -8,6 +8,11 @@ Required Properties: > > > - VANA-supply: supply of voltage for VANA pin > > > > > > +Optional properties: > > > + > > > + - enable-gpios : GPIO spec for the XSHUTDOWN pin. > > > > xshutdown is active-low, so enable is active-high. Should this be documented > > explicitly, to avoid polarity errors ? Maybe something along the lines of > > > > - 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). Agreed > > Or alternatively you could name the property xshutdown-gpios, as explained in > my (incorrect) review of 2/4. I have double negatives :). If there is no other option I will rename it xshutdown, but I want to give it a try to enable. > > > > If specified, it will be > > > + asserted when VANA-supply is enabled. > > > > That documents a driver behaviour, is it needed in DT ? > > > > > Example: > > > ad5820: coil@c { > > > > > > @@ -15,5 +20,6 @@ Example: > > > reg = <0x0c>; > > > VANA-supply = <&vaux4>; > > > > > > + enable-gpios = <&msmgpio 26 GPIO_ACTIVE_HIGH>; > > > }; > > > -- > Regards, > > Laurent Pinchart > > > -- Ricardo Ribalda ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/4] [media] ad5820: DT new optional field enable-gpios 2018-09-20 20:25 ` Ricardo Ribalda Delgado @ 2018-09-20 20:28 ` Pavel Machek 0 siblings, 0 replies; 5+ messages in thread From: Pavel Machek @ 2018-09-20 20:28 UTC (permalink / raw) To: Ricardo Ribalda Delgado Cc: Laurent Pinchart, Sakari Ailus, Mauro Carvalho Chehab, linux-media, LKML, Hans Verkuil, devicetree [-- Attachment #1: Type: text/plain, Size: 2210 bytes --] On Thu 2018-09-20 22:25:54, Ricardo Ribalda Delgado wrote: > Hi > On Thu, Sep 20, 2018 at 10:23 PM Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > > > > On Thursday, 20 September 2018 23:21:28 EEST Laurent Pinchart wrote: > > > Hi Ricardo, > > > > > > Thank you for the patch. > > > > > > On Thursday, 20 September 2018 19:19:11 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> > > > > --- > > > > > > > > Documentation/devicetree/bindings/media/i2c/ad5820.txt | 6 ++++++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt > > > > b/Documentation/devicetree/bindings/media/i2c/ad5820.txt index > > > > 5940ca11c021..07d577bb37f7 100644 > > > > --- a/Documentation/devicetree/bindings/media/i2c/ad5820.txt > > > > +++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt > > > > > > > > @@ -8,6 +8,11 @@ Required Properties: > > > > - VANA-supply: supply of voltage for VANA pin > > > > > > > > +Optional properties: > > > > + > > > > + - enable-gpios : GPIO spec for the XSHUTDOWN pin. > > > > > > xshutdown is active-low, so enable is active-high. Should this be documented > > > explicitly, to avoid polarity errors ? Maybe something along the lines of > > > > > > - 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). > > Agreed > > > > > Or alternatively you could name the property xshutdown-gpios, as explained in > > my (incorrect) review of 2/4. > > I have double negatives :). If there is no other option I will rename > it xshutdown, but I want to give it a try to enable. I think enable is fine. Pavel -- (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 --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-09-20 20:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20180920161912.17063-1-ricardo.ribalda@gmail.com>
2018-09-20 16:19 ` [PATCH 3/4] [media] ad5820: DT new optional field enable-gpios Ricardo Ribalda Delgado
2018-09-20 20:21 ` Laurent Pinchart
2018-09-20 20:23 ` Laurent Pinchart
2018-09-20 20:25 ` Ricardo Ribalda Delgado
2018-09-20 20:28 ` Pavel Machek
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).