PPS signals with very short pulse-widths can be missed if their state
changes by the time the interrupt handler reads the GPIO pin state.
To avoid this in the case where we are only looking for one edge we can
use the edge configuration for the pin state but fall back to reading the
pin if both edges are being watched.
Signed-off-by: Tim Harvey <[email protected]>
---
drivers/pps/clients/pps-gpio.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
index 333ad7d..0d2b807 100644
--- a/drivers/pps/clients/pps-gpio.c
+++ b/drivers/pps/clients/pps-gpio.c
@@ -61,7 +61,16 @@ static irqreturn_t pps_gpio_irq_handler(int irq, void *data)
info = data;
- rising_edge = gpio_get_value(info->gpio_pin);
+ /*
+ * if not capturing both assert/clear events use the IRQ state
+ * otherwise read the gpio state from the pin (which could miss
+ * assertions on very small pulse-widths due to interrupt latency
+ * and CPU performance).
+ */
+ if (!info->capture_clear)
+ rising_edge = !info->assert_falling_edge;
+ else
+ rising_edge = gpio_get_value(info->gpio_pin);
if ((rising_edge && !info->assert_falling_edge) ||
(!rising_edge && info->assert_falling_edge))
pps_event(info->pps, &ts, PPS_CAPTUREASSERT, NULL);
--
2.7.4
On 08/09/2017 21:53, Tim Harvey wrote:
> PPS signals with very short pulse-widths can be missed if their state
> changes by the time the interrupt handler reads the GPIO pin state.
>
> To avoid this in the case where we are only looking for one edge we can
> use the edge configuration for the pin state but fall back to reading the
> pin if both edges are being watched.
I disagree. The "rising_edge" status should be get from the hardware and not
derived by an empirical computation. Or, at least, it should be specifically
activated by setting something like this:
pps {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pps>;
gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
Yes-I-want-get-signal-status-in-an-epirical-way;
compatible = "pps-gpio";
status = "okay";
};
This setting should also print a warning in order to be clear for the user that
he/she should know what he/she is doing.
Then the code should check also the compatibility with property
"assert-falling-edge"...
Ciao,
Rodolfo
--
HCE Engineering e-mail: [email protected]
GNU/Linux Solutions [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Cosino Project - the quick prototyping embedded system - http://www.cosino.it
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it
On Sun, Sep 10, 2017 at 3:43 AM, Rodolfo Giometti <[email protected]> wrote:
> On 08/09/2017 21:53, Tim Harvey wrote:
>>
>> PPS signals with very short pulse-widths can be missed if their state
>> changes by the time the interrupt handler reads the GPIO pin state.
>>
>> To avoid this in the case where we are only looking for one edge we can
>> use the edge configuration for the pin state but fall back to reading the
>> pin if both edges are being watched.
>
>
> I disagree. The "rising_edge" status should be get from the hardware and not
> derived by an empirical computation. Or, at least, it should be specifically
> activated by setting something like this:
>
> pps {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_pps>;
>
> gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
> Yes-I-want-get-signal-status-in-an-epirical-way;
>
> compatible = "pps-gpio";
> status = "okay";
> };
>
> This setting should also print a warning in order to be clear for the user
> that he/she should know what he/she is doing.
>
> Then the code should check also the compatibility with property
> "assert-falling-edge"...
>
Hi Rodolfo,
Do you agree with using the irq edge in general if/when it is
available to resolve the case where small pulse-widths can be caught?
I assumed because pps-gpio is the one configuring the irq based on
info->capture_clear and info->assert_falling_edge that that it made
sense to use that logic again when handling the interrupt but there is
likely a call I can make to determine the irq (edge) type based on the
irq.
Tim
On 12/09/2017 17:54, Tim Harvey wrote:
> On Sun, Sep 10, 2017 at 3:43 AM, Rodolfo Giometti <[email protected]> wrote:
>> On 08/09/2017 21:53, Tim Harvey wrote:
>>>
>>> PPS signals with very short pulse-widths can be missed if their state
>>> changes by the time the interrupt handler reads the GPIO pin state.
>>>
>>> To avoid this in the case where we are only looking for one edge we can
>>> use the edge configuration for the pin state but fall back to reading the
>>> pin if both edges are being watched.
>>
>>
>> I disagree. The "rising_edge" status should be get from the hardware and not
>> derived by an empirical computation. Or, at least, it should be specifically
>> activated by setting something like this:
>>
>> pps {
>> pinctrl-names = "default";
>> pinctrl-0 = <&pinctrl_pps>;
>>
>> gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
>> Yes-I-want-get-signal-status-in-an-epirical-way;
>>
>> compatible = "pps-gpio";
>> status = "okay";
>> };
>>
>> This setting should also print a warning in order to be clear for the user
>> that he/she should know what he/she is doing.
>>
>> Then the code should check also the compatibility with property
>> "assert-falling-edge"...
>>
>
> Hi Rodolfo,
>
> Do you agree with using the irq edge in general if/when it is
> available to resolve the case where small pulse-widths can be caught?
>
> I assumed because pps-gpio is the one configuring the irq based on
> info->capture_clear and info->assert_falling_edge that that it made
> sense to use that logic again when handling the interrupt but there is
> likely a call I can make to determine the irq (edge) type based on the
> irq.
If you get the information from the hardware it's OK for me, otherwise you
should enable this behavior by using proper DT property.
Ciao,
Rodolfo
--
HCE Engineering e-mail: [email protected]
GNU/Linux Solutions [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Cosino Project - the quick prototyping embedded system - http://www.cosino.it
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it