Re: [PATCH] pps-gpio: use IRQ edge config when not capturing both edges
On 12/09/2017 17:54, Tim Harvey wrote: On Sun, Sep 10, 2017 at 3:43 AM, Rodolfo Giomettiwrote: 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 = <_pps>; gpios = < 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: giome...@hce-engineering.it GNU/Linux Solutions giome...@enneenne.com Linux Device Driver giome...@linux.it Embedded Systems phone: +39 349 2432127 UNIX programming skype: rodolfo.giometti Cosino Project - the quick prototyping embedded system - www.cosino.it Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it
Re: [PATCH] pps-gpio: use IRQ edge config when not capturing both edges
On 12/09/2017 17:54, Tim Harvey wrote: On Sun, Sep 10, 2017 at 3:43 AM, Rodolfo Giometti 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 = <_pps>; gpios = < 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: giome...@hce-engineering.it GNU/Linux Solutions giome...@enneenne.com Linux Device Driver giome...@linux.it Embedded Systems phone: +39 349 2432127 UNIX programming skype: rodolfo.giometti Cosino Project - the quick prototyping embedded system - www.cosino.it Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it
Re: [PATCH] pps-gpio: use IRQ edge config when not capturing both edges
On Sun, Sep 10, 2017 at 3:43 AM, Rodolfo Giomettiwrote: > 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 = <_pps>; > > gpios = < 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
Re: [PATCH] pps-gpio: use IRQ edge config when not capturing both edges
On Sun, Sep 10, 2017 at 3:43 AM, Rodolfo Giometti 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 = <_pps>; > > gpios = < 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
Re: [PATCH] pps-gpio: use IRQ edge config when not capturing both edges
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 = <_pps>; gpios = < 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: giome...@hce-engineering.it GNU/Linux Solutions giome...@enneenne.com Linux Device Driver giome...@linux.it Embedded Systems phone: +39 349 2432127 UNIX programming skype: rodolfo.giometti Cosino Project - the quick prototyping embedded system - www.cosino.it Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it
Re: [PATCH] pps-gpio: use IRQ edge config when not capturing both edges
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 = <_pps>; gpios = < 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: giome...@hce-engineering.it GNU/Linux Solutions giome...@enneenne.com Linux Device Driver giome...@linux.it Embedded Systems phone: +39 349 2432127 UNIX programming skype: rodolfo.giometti Cosino Project - the quick prototyping embedded system - www.cosino.it Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it
[PATCH] pps-gpio: use IRQ edge config when not capturing both edges
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--- 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, , PPS_CAPTUREASSERT, NULL); -- 2.7.4
[PATCH] pps-gpio: use IRQ edge config when not capturing both edges
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 --- 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, , PPS_CAPTUREASSERT, NULL); -- 2.7.4