devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 1/4] dt-bindings: pps: capture-clear addition
       [not found] <20181122094602.5156-1-tom@aussec.com>
@ 2018-11-22  9:45 ` Tom Burkart
       [not found]   ` <20181122094602.5156-3-tom@aussec.com>
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Burkart @ 2018-11-22  9:45 UTC (permalink / raw)
  To: Linux kernel mailing list; +Cc: Tom Burkart, devicetree

It adds documentation for the device tree capture-clear option.

Signed-off-by: Tom Burkart <tom@aussec.com>
---
 Documentation/devicetree/bindings/pps/pps-gpio.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.txt b/Documentation/devicetree/bindings/pps/pps-gpio.txt
index 3683874832ae..1155d49c2699 100644
--- a/Documentation/devicetree/bindings/pps/pps-gpio.txt
+++ b/Documentation/devicetree/bindings/pps/pps-gpio.txt
@@ -10,6 +10,7 @@ Required properties:
 Optional properties:
 - assert-falling-edge: when present, assert is indicated by a falling edge
                        (instead of by a rising edge)
+- capture-clear: when present, also capture the PPS clear event
 
 Example:
 	pps {
@@ -18,6 +19,7 @@ Example:
 
 		gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
 		assert-falling-edge;
+		capture-clear;
 
 		compatible = "pps-gpio";
 	};
-- 
2.12.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v9 3/4] dt-bindings: pps: pps-gpio PPS ECHO implementation
       [not found]   ` <20181122094602.5156-3-tom@aussec.com>
@ 2018-11-22  9:46     ` Tom Burkart
  2018-11-26 19:42       ` Rob Herring
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Burkart @ 2018-11-22  9:46 UTC (permalink / raw)
  To: Linux kernel mailing list; +Cc: Tom Burkart, devicetree, Lukas Senger

This patch implements the device tree changes required for the pps
echo functionality for pps-gpio, that sysfs claims is available
already.

This patch was originally written by Lukas Senger as part of a masters
thesis project and modified for inclusion into the linux kernel by Tom
Burkart.

Signed-off-by: Lukas Senger <lukas@fridolin.com>
Signed-off-by: Tom Burkart <tom@aussec.com>
---
 Documentation/devicetree/bindings/pps/pps-gpio.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.txt b/Documentation/devicetree/bindings/pps/pps-gpio.txt
index 1155d49c2699..e09f6f2405c5 100644
--- a/Documentation/devicetree/bindings/pps/pps-gpio.txt
+++ b/Documentation/devicetree/bindings/pps/pps-gpio.txt
@@ -7,10 +7,15 @@ Required properties:
 - compatible: should be "pps-gpio"
 - gpios: one PPS GPIO in the format described by ../gpio/gpio.txt
 
+Additional required properties for the PPS ECHO functionality:
+- echo-gpios: one PPS ECHO GPIO in the format described by ../gpio/gpio.txt
+- echo-active-ms: duration in ms of the active portion of the echo pulse
+
 Optional properties:
 - assert-falling-edge: when present, assert is indicated by a falling edge
                        (instead of by a rising edge)
 - capture-clear: when present, also capture the PPS clear event
+- invert-pps-echo: when present, invert the PPS ECHO pulse
 
 Example:
 	pps {
@@ -21,5 +26,9 @@ Example:
 		assert-falling-edge;
 		capture-clear;
 
+		echo-gpios = <&gpio1 27 GPIO_ACTIVE_HIGH>;
+		echo-active-ms = <100>;
+		invert-pps-echo;
+
 		compatible = "pps-gpio";
 	};
-- 
2.12.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v9 3/4] dt-bindings: pps: pps-gpio PPS ECHO implementation
  2018-11-22  9:46     ` [PATCH v9 3/4] dt-bindings: pps: pps-gpio PPS ECHO implementation Tom Burkart
@ 2018-11-26 19:42       ` Rob Herring
  2018-11-27  4:06         ` tom burkart
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2018-11-26 19:42 UTC (permalink / raw)
  To: Tom Burkart; +Cc: linux-kernel@vger.kernel.org, devicetree, lukas

On Thu, Nov 22, 2018 at 3:49 AM Tom Burkart <tom@aussec.com> wrote:
>
> This patch implements the device tree changes required for the pps
> echo functionality for pps-gpio, that sysfs claims is available
> already.
>
> This patch was originally written by Lukas Senger as part of a masters
> thesis project and modified for inclusion into the linux kernel by Tom
> Burkart.
>
> Signed-off-by: Lukas Senger <lukas@fridolin.com>
> Signed-off-by: Tom Burkart <tom@aussec.com>
> ---
>  Documentation/devicetree/bindings/pps/pps-gpio.txt | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.txt b/Documentation/devicetree/bindings/pps/pps-gpio.txt
> index 1155d49c2699..e09f6f2405c5 100644
> --- a/Documentation/devicetree/bindings/pps/pps-gpio.txt
> +++ b/Documentation/devicetree/bindings/pps/pps-gpio.txt
> @@ -7,10 +7,15 @@ Required properties:
>  - compatible: should be "pps-gpio"
>  - gpios: one PPS GPIO in the format described by ../gpio/gpio.txt
>
> +Additional required properties for the PPS ECHO functionality:
> +- echo-gpios: one PPS ECHO GPIO in the format described by ../gpio/gpio.txt
> +- echo-active-ms: duration in ms of the active portion of the echo pulse
> +
>  Optional properties:
>  - assert-falling-edge: when present, assert is indicated by a falling edge
>                         (instead of by a rising edge)
>  - capture-clear: when present, also capture the PPS clear event
> +- invert-pps-echo: when present, invert the PPS ECHO pulse

Why do you need this? Can't you just make the echo gpio GPIO_ACTIVE_LOW?

BTW, using the flag probably should have been done for
'assert-falling-edge' as well.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v9 3/4] dt-bindings: pps: pps-gpio PPS ECHO implementation
  2018-11-26 19:42       ` Rob Herring
@ 2018-11-27  4:06         ` tom burkart
  2018-12-07 17:06           ` Rob Herring
  0 siblings, 1 reply; 9+ messages in thread
From: tom burkart @ 2018-11-27  4:06 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-kernel@vger.kernel.org, devicetree, lukas

Hi Rob,

Quoting Rob Herring <robh@kernel.org>:

> On Thu, Nov 22, 2018 at 3:49 AM Tom Burkart <tom@aussec.com> wrote:
>>
>> This patch implements the device tree changes required for the pps
>> echo functionality for pps-gpio, that sysfs claims is available
>> already.
>>
>> This patch was originally written by Lukas Senger as part of a masters
>> thesis project and modified for inclusion into the linux kernel by Tom
>> Burkart.
>>
>> Signed-off-by: Lukas Senger <lukas@fridolin.com>
>> Signed-off-by: Tom Burkart <tom@aussec.com>
>> ---
>>  Documentation/devicetree/bindings/pps/pps-gpio.txt | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.txt  
>> b/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> index 1155d49c2699..e09f6f2405c5 100644
>> --- a/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> +++ b/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> @@ -7,10 +7,15 @@ Required properties:
>>  - compatible: should be "pps-gpio"
>>  - gpios: one PPS GPIO in the format described by ../gpio/gpio.txt
>>
>> +Additional required properties for the PPS ECHO functionality:
>> +- echo-gpios: one PPS ECHO GPIO in the format described by ../gpio/gpio.txt
>> +- echo-active-ms: duration in ms of the active portion of the echo pulse
>> +
>>  Optional properties:
>>  - assert-falling-edge: when present, assert is indicated by a falling edge
>>                         (instead of by a rising edge)
>>  - capture-clear: when present, also capture the PPS clear event
>> +- invert-pps-echo: when present, invert the PPS ECHO pulse
>
> Why do you need this? Can't you just make the echo gpio GPIO_ACTIVE_LOW?
>
> BTW, using the flag probably should have been done for
> 'assert-falling-edge' as well.

The hardware I use expects a positive-going echo pulse, however, it  
was really easy to give users the option to have it inverted in case  
they use different hardware that expects a negative-going edge.

I may well be needing this option myself if I find that the GPS I am  
using produces better results for negative-going edges than  
positive-going edges.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v9 3/4] dt-bindings: pps: pps-gpio PPS ECHO implementation
  2018-11-27  4:06         ` tom burkart
@ 2018-12-07 17:06           ` Rob Herring
  2018-12-09  3:07             ` tom burkart
  2018-12-11  2:17             ` tom burkart
  0 siblings, 2 replies; 9+ messages in thread
From: Rob Herring @ 2018-12-07 17:06 UTC (permalink / raw)
  To: Tom Burkart; +Cc: linux-kernel@vger.kernel.org, devicetree, lukas

On Mon, Nov 26, 2018 at 10:06 PM tom burkart <tom@aussec.com> wrote:
>
> Hi Rob,
>
> Quoting Rob Herring <robh@kernel.org>:
>
> > On Thu, Nov 22, 2018 at 3:49 AM Tom Burkart <tom@aussec.com> wrote:
> >>
> >> This patch implements the device tree changes required for the pps
> >> echo functionality for pps-gpio, that sysfs claims is available
> >> already.
> >>
> >> This patch was originally written by Lukas Senger as part of a masters
> >> thesis project and modified for inclusion into the linux kernel by Tom
> >> Burkart.
> >>
> >> Signed-off-by: Lukas Senger <lukas@fridolin.com>
> >> Signed-off-by: Tom Burkart <tom@aussec.com>
> >> ---
> >>  Documentation/devicetree/bindings/pps/pps-gpio.txt | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.txt
> >> b/Documentation/devicetree/bindings/pps/pps-gpio.txt
> >> index 1155d49c2699..e09f6f2405c5 100644
> >> --- a/Documentation/devicetree/bindings/pps/pps-gpio.txt
> >> +++ b/Documentation/devicetree/bindings/pps/pps-gpio.txt
> >> @@ -7,10 +7,15 @@ Required properties:
> >>  - compatible: should be "pps-gpio"
> >>  - gpios: one PPS GPIO in the format described by ../gpio/gpio.txt
> >>
> >> +Additional required properties for the PPS ECHO functionality:
> >> +- echo-gpios: one PPS ECHO GPIO in the format described by ../gpio/gpio.txt
> >> +- echo-active-ms: duration in ms of the active portion of the echo pulse
> >> +
> >>  Optional properties:
> >>  - assert-falling-edge: when present, assert is indicated by a falling edge
> >>                         (instead of by a rising edge)
> >>  - capture-clear: when present, also capture the PPS clear event
> >> +- invert-pps-echo: when present, invert the PPS ECHO pulse
> >
> > Why do you need this? Can't you just make the echo gpio GPIO_ACTIVE_LOW?
> >
> > BTW, using the flag probably should have been done for
> > 'assert-falling-edge' as well.
>
> The hardware I use expects a positive-going echo pulse, however, it
> was really easy to give users the option to have it inverted in case
> they use different hardware that expects a negative-going edge.

It will be even easier to implement if you use GPIO_ACTIVE_LOW or
GPIO_ACTIVE_HIGH as appropriate. If the flag is set appropriately,
then gpiod_set_value(gpio, 1) asserts the pulse and
gpiod_set_value(gpio, 0) deasserts it no matter which way the h/w is
wired. You can then get rid of invert_pps_echo in the driver.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v9 3/4] dt-bindings: pps: pps-gpio PPS ECHO implementation
  2018-12-07 17:06           ` Rob Herring
@ 2018-12-09  3:07             ` tom burkart
  2018-12-11  2:17             ` tom burkart
  1 sibling, 0 replies; 9+ messages in thread
From: tom burkart @ 2018-12-09  3:07 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-kernel@vger.kernel.org, devicetree, lukas

Quoting Rob Herring <robh@kernel.org>:

> On Mon, Nov 26, 2018 at 10:06 PM tom burkart <tom@aussec.com> wrote:
>>
>> Hi Rob,
>>
>> Quoting Rob Herring <robh@kernel.org>:
>>
>> > On Thu, Nov 22, 2018 at 3:49 AM Tom Burkart <tom@aussec.com> wrote:
>> >>
>> >> This patch implements the device tree changes required for the pps
>> >> echo functionality for pps-gpio, that sysfs claims is available
>> >> already.
>> >>
>> >> This patch was originally written by Lukas Senger as part of a masters
>> >> thesis project and modified for inclusion into the linux kernel by Tom
>> >> Burkart.
>> >>
>> >> Signed-off-by: Lukas Senger <lukas@fridolin.com>
>> >> Signed-off-by: Tom Burkart <tom@aussec.com>
>> >> ---
>> >>  Documentation/devicetree/bindings/pps/pps-gpio.txt | 9 +++++++++
>> >>  1 file changed, 9 insertions(+)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> >> b/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> >> index 1155d49c2699..e09f6f2405c5 100644
>> >> --- a/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> >> +++ b/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> >> @@ -7,10 +7,15 @@ Required properties:
>> >>  - compatible: should be "pps-gpio"
>> >>  - gpios: one PPS GPIO in the format described by ../gpio/gpio.txt
>> >>
>> >> +Additional required properties for the PPS ECHO functionality:
>> >> +- echo-gpios: one PPS ECHO GPIO in the format described by  
>> ../gpio/gpio.txt
>> >> +- echo-active-ms: duration in ms of the active portion of the echo pulse
>> >> +
>> >>  Optional properties:
>> >>  - assert-falling-edge: when present, assert is indicated by a  
>> falling edge
>> >>                         (instead of by a rising edge)
>> >>  - capture-clear: when present, also capture the PPS clear event
>> >> +- invert-pps-echo: when present, invert the PPS ECHO pulse
>> >
>> > Why do you need this? Can't you just make the echo gpio GPIO_ACTIVE_LOW?
>> >
>> > BTW, using the flag probably should have been done for
>> > 'assert-falling-edge' as well.
>>
>> The hardware I use expects a positive-going echo pulse, however, it
>> was really easy to give users the option to have it inverted in case
>> they use different hardware that expects a negative-going edge.
>
> It will be even easier to implement if you use GPIO_ACTIVE_LOW or
> GPIO_ACTIVE_HIGH as appropriate. If the flag is set appropriately,
> then gpiod_set_value(gpio, 1) asserts the pulse and
> gpiod_set_value(gpio, 0) deasserts it no matter which way the h/w is
> wired. You can then get rid of invert_pps_echo in the driver.

Hi Rob,
sure we can do that, however, a hardware change will need a recompile  
of the kernel, rather than setting a flag.
You still need something to help decide whether to use GPIO_ACTIVE_LOW  
or GPIO_ACTIVE_HIGH depending on what hardware is currently connected  
to the system.

I am happy to delete that code as well, if it makes you happy.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v9 3/4] dt-bindings: pps: pps-gpio PPS ECHO implementation
  2018-12-07 17:06           ` Rob Herring
  2018-12-09  3:07             ` tom burkart
@ 2018-12-11  2:17             ` tom burkart
  2018-12-11 15:29               ` Rob Herring
  1 sibling, 1 reply; 9+ messages in thread
From: tom burkart @ 2018-12-11  2:17 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-kernel@vger.kernel.org, devicetree, lukas

Quoting Rob Herring <robh@kernel.org>:

> On Mon, Nov 26, 2018 at 10:06 PM tom burkart <tom@aussec.com> wrote:
>>
>> Quoting Rob Herring <robh@kernel.org>:
>>
>> > On Thu, Nov 22, 2018 at 3:49 AM Tom Burkart <tom@aussec.com> wrote:
>> >>
>> >> This patch implements the device tree changes required for the pps
>> >> echo functionality for pps-gpio, that sysfs claims is available
>> >> already.
>> >>
>> >> This patch was originally written by Lukas Senger as part of a masters
>> >> thesis project and modified for inclusion into the linux kernel by Tom
>> >> Burkart.
>> >>
>> >> Signed-off-by: Lukas Senger <lukas@fridolin.com>
>> >> Signed-off-by: Tom Burkart <tom@aussec.com>
>> >> ---
>> >>  Documentation/devicetree/bindings/pps/pps-gpio.txt | 9 +++++++++
>> >>  1 file changed, 9 insertions(+)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> >> b/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> >> index 1155d49c2699..e09f6f2405c5 100644
>> >> --- a/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> >> +++ b/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> >> @@ -7,10 +7,15 @@ Required properties:
>> >>  - compatible: should be "pps-gpio"
>> >>  - gpios: one PPS GPIO in the format described by ../gpio/gpio.txt
>> >>
>> >> +Additional required properties for the PPS ECHO functionality:
>> >> +- echo-gpios: one PPS ECHO GPIO in the format described by  
>> ../gpio/gpio.txt
>> >> +- echo-active-ms: duration in ms of the active portion of the echo pulse
>> >> +
>> >>  Optional properties:
>> >>  - assert-falling-edge: when present, assert is indicated by a  
>> falling edge
>> >>                         (instead of by a rising edge)
>> >>  - capture-clear: when present, also capture the PPS clear event
>> >> +- invert-pps-echo: when present, invert the PPS ECHO pulse
>> >
>> > Why do you need this? Can't you just make the echo gpio GPIO_ACTIVE_LOW?
>> >
>> > BTW, using the flag probably should have been done for
>> > 'assert-falling-edge' as well.
>>
>> The hardware I use expects a positive-going echo pulse, however, it
>> was really easy to give users the option to have it inverted in case
>> they use different hardware that expects a negative-going edge.
>
> It will be even easier to implement if you use GPIO_ACTIVE_LOW or
> GPIO_ACTIVE_HIGH as appropriate. If the flag is set appropriately,
> then gpiod_set_value(gpio, 1) asserts the pulse and
> gpiod_set_value(gpio, 0) deasserts it no matter which way the h/w is
> wired. You can then get rid of invert_pps_echo in the driver.

Hi Rob,
I have looked at the appropriate changes to my code to implement the  
above.  However, there is no GPIO_ACTIVE_* as part of the gpiod_flags  
enum (include/linux/gpio/consumer.h).

What am I missing?

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v9 3/4] dt-bindings: pps: pps-gpio PPS ECHO implementation
  2018-12-11  2:17             ` tom burkart
@ 2018-12-11 15:29               ` Rob Herring
  2018-12-12  2:25                 ` tom burkart
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2018-12-11 15:29 UTC (permalink / raw)
  To: Tom Burkart; +Cc: linux-kernel@vger.kernel.org, devicetree, lukas

On Mon, Dec 10, 2018 at 8:17 PM tom burkart <tom@aussec.com> wrote:
>
> Quoting Rob Herring <robh@kernel.org>:
>
> > On Mon, Nov 26, 2018 at 10:06 PM tom burkart <tom@aussec.com> wrote:
> >>
> >> Quoting Rob Herring <robh@kernel.org>:
> >>
> >> > On Thu, Nov 22, 2018 at 3:49 AM Tom Burkart <tom@aussec.com> wrote:
> >> >>
> >> >> This patch implements the device tree changes required for the pps
> >> >> echo functionality for pps-gpio, that sysfs claims is available
> >> >> already.
> >> >>
> >> >> This patch was originally written by Lukas Senger as part of a masters
> >> >> thesis project and modified for inclusion into the linux kernel by Tom
> >> >> Burkart.
> >> >>
> >> >> Signed-off-by: Lukas Senger <lukas@fridolin.com>
> >> >> Signed-off-by: Tom Burkart <tom@aussec.com>
> >> >> ---
> >> >>  Documentation/devicetree/bindings/pps/pps-gpio.txt | 9 +++++++++
> >> >>  1 file changed, 9 insertions(+)
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.txt
> >> >> b/Documentation/devicetree/bindings/pps/pps-gpio.txt
> >> >> index 1155d49c2699..e09f6f2405c5 100644
> >> >> --- a/Documentation/devicetree/bindings/pps/pps-gpio.txt
> >> >> +++ b/Documentation/devicetree/bindings/pps/pps-gpio.txt
> >> >> @@ -7,10 +7,15 @@ Required properties:
> >> >>  - compatible: should be "pps-gpio"
> >> >>  - gpios: one PPS GPIO in the format described by ../gpio/gpio.txt
> >> >>
> >> >> +Additional required properties for the PPS ECHO functionality:
> >> >> +- echo-gpios: one PPS ECHO GPIO in the format described by
> >> ../gpio/gpio.txt
> >> >> +- echo-active-ms: duration in ms of the active portion of the echo pulse
> >> >> +
> >> >>  Optional properties:
> >> >>  - assert-falling-edge: when present, assert is indicated by a
> >> falling edge
> >> >>                         (instead of by a rising edge)
> >> >>  - capture-clear: when present, also capture the PPS clear event
> >> >> +- invert-pps-echo: when present, invert the PPS ECHO pulse
> >> >
> >> > Why do you need this? Can't you just make the echo gpio GPIO_ACTIVE_LOW?
> >> >
> >> > BTW, using the flag probably should have been done for
> >> > 'assert-falling-edge' as well.
> >>
> >> The hardware I use expects a positive-going echo pulse, however, it
> >> was really easy to give users the option to have it inverted in case
> >> they use different hardware that expects a negative-going edge.
> >
> > It will be even easier to implement if you use GPIO_ACTIVE_LOW or
> > GPIO_ACTIVE_HIGH as appropriate. If the flag is set appropriately,
> > then gpiod_set_value(gpio, 1) asserts the pulse and
> > gpiod_set_value(gpio, 0) deasserts it no matter which way the h/w is
> > wired. You can then get rid of invert_pps_echo in the driver.
>
> Hi Rob,
> I have looked at the appropriate changes to my code to implement the
> above.  However, there is no GPIO_ACTIVE_* as part of the gpiod_flags
> enum (include/linux/gpio/consumer.h).
>
> What am I missing?

You don't need to know in the driver. You set the gpio state to 1 for
active, 0 for inactive and that does the right thing based on the flag
in the DT. IOW, if the DT defines the GPIO as active low, then setting
the gpio state to 1 will result in the GPIO being 0V. It's a bit
annoying and confusing at first until you realize the driver can just
handle either polarity transparently.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v9 3/4] dt-bindings: pps: pps-gpio PPS ECHO implementation
  2018-12-11 15:29               ` Rob Herring
@ 2018-12-12  2:25                 ` tom burkart
  0 siblings, 0 replies; 9+ messages in thread
From: tom burkart @ 2018-12-12  2:25 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-kernel@vger.kernel.org, devicetree, lukas

Quoting Rob Herring <robh@kernel.org>:

> On Mon, Dec 10, 2018 at 8:17 PM tom burkart <tom@aussec.com> wrote:
>>
>> Quoting Rob Herring <robh@kernel.org>:
>>
>> > On Mon, Nov 26, 2018 at 10:06 PM tom burkart <tom@aussec.com> wrote:
>> >>
>> >> Quoting Rob Herring <robh@kernel.org>:
>> >>
>> >> > On Thu, Nov 22, 2018 at 3:49 AM Tom Burkart <tom@aussec.com> wrote:
>> >> >>
>> >> >> This patch implements the device tree changes required for the pps
>> >> >> echo functionality for pps-gpio, that sysfs claims is available
>> >> >> already.
>> >> >>
>> >> >> This patch was originally written by Lukas Senger as part of a masters
>> >> >> thesis project and modified for inclusion into the linux kernel by Tom
>> >> >> Burkart.
>> >> >>
>> >> >> Signed-off-by: Lukas Senger <lukas@fridolin.com>
>> >> >> Signed-off-by: Tom Burkart <tom@aussec.com>
>> >> >> ---
>> >> >>  Documentation/devicetree/bindings/pps/pps-gpio.txt | 9 +++++++++
>> >> >>  1 file changed, 9 insertions(+)
>> >> >>
>> >> >> diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> >> >> b/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> >> >> index 1155d49c2699..e09f6f2405c5 100644
>> >> >> --- a/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> >> >> +++ b/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> >> >> @@ -7,10 +7,15 @@ Required properties:
>> >> >>  - compatible: should be "pps-gpio"
>> >> >>  - gpios: one PPS GPIO in the format described by ../gpio/gpio.txt
>> >> >>
>> >> >> +Additional required properties for the PPS ECHO functionality:
>> >> >> +- echo-gpios: one PPS ECHO GPIO in the format described by
>> >> ../gpio/gpio.txt
>> >> >> +- echo-active-ms: duration in ms of the active portion of  
>> the echo pulse
>> >> >> +
>> >> >>  Optional properties:
>> >> >>  - assert-falling-edge: when present, assert is indicated by a
>> >> falling edge
>> >> >>                         (instead of by a rising edge)
>> >> >>  - capture-clear: when present, also capture the PPS clear event
>> >> >> +- invert-pps-echo: when present, invert the PPS ECHO pulse
>> >> >
>> >> > Why do you need this? Can't you just make the echo gpio  
>> GPIO_ACTIVE_LOW?
>> >> >
>> >> > BTW, using the flag probably should have been done for
>> >> > 'assert-falling-edge' as well.
>> >>
>> >> The hardware I use expects a positive-going echo pulse, however, it
>> >> was really easy to give users the option to have it inverted in case
>> >> they use different hardware that expects a negative-going edge.
>> >
>> > It will be even easier to implement if you use GPIO_ACTIVE_LOW or
>> > GPIO_ACTIVE_HIGH as appropriate. If the flag is set appropriately,
>> > then gpiod_set_value(gpio, 1) asserts the pulse and
>> > gpiod_set_value(gpio, 0) deasserts it no matter which way the h/w is
>> > wired. You can then get rid of invert_pps_echo in the driver.
>>
>> Hi Rob,
>> I have looked at the appropriate changes to my code to implement the
>> above.  However, there is no GPIO_ACTIVE_* as part of the gpiod_flags
>> enum (include/linux/gpio/consumer.h).
>>
>> What am I missing?
>
> You don't need to know in the driver. You set the gpio state to 1 for
> active, 0 for inactive and that does the right thing based on the flag
> in the DT. IOW, if the DT defines the GPIO as active low, then setting
> the gpio state to 1 will result in the GPIO being 0V. It's a bit
> annoying and confusing at first until you realize the driver can just
> handle either polarity transparently.

Hi Rob,
thanks a lot!  The penny finally dropped.

My patch v12 will have the changes implemented (no invert-pps-echo).

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-12-12  2:25 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20181122094602.5156-1-tom@aussec.com>
2018-11-22  9:45 ` [PATCH v9 1/4] dt-bindings: pps: capture-clear addition Tom Burkart
     [not found]   ` <20181122094602.5156-3-tom@aussec.com>
2018-11-22  9:46     ` [PATCH v9 3/4] dt-bindings: pps: pps-gpio PPS ECHO implementation Tom Burkart
2018-11-26 19:42       ` Rob Herring
2018-11-27  4:06         ` tom burkart
2018-12-07 17:06           ` Rob Herring
2018-12-09  3:07             ` tom burkart
2018-12-11  2:17             ` tom burkart
2018-12-11 15:29               ` Rob Herring
2018-12-12  2:25                 ` tom burkart

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).