Re: [PATCH 2/5] dt-bindings: pps: pps-gpio: introduce capture-clear property

[Farber, Eliav]

On 6/25/2023 6:46 PM, Krzysztof Kozlowski wrote:
> On 25/06/2023 16:21, Eliav Farber wrote:
> > Add a new optional "capture-clear" boolean property.
> > When present, PPS clear events shall also be reported.
> >
> > Signed-off-by: Eliav Farber <farbere@xxxxxxxxxx>
> > ---
> >  Documentation/devicetree/bindings/pps/pps-gpio.txt | 4 ++++
>
> Please convert the bindings to DT schema first.
I will convert to DT schema first, if I indeed end up modifying this file.

> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.txt 
> > b/Documentation/devicetree/bindings/pps/pps-gpio.txt
> > index 9012a2a02e14..8d588e38c44e 100644
> > --- a/Documentation/devicetree/bindings/pps/pps-gpio.txt
> > +++ b/Documentation/devicetree/bindings/pps/pps-gpio.txt
> > @@ -14,6 +14,10 @@ Additional required properties for the PPS ECHO 
> > functionality:
> >  Optional properties:
> >  - assert-falling-edge: when present, assert is indicated by a 
> > falling edge
> >                         (instead of by a rising edge)
> > +- capture-clear: when present, report also PPS clear events, which 
> > is the
> > +                 opposite of the assert edge (if assert is 
> > rising-edge then
> > +                 clear is falling-edge and if assert is falling-edge 
> > then
> > +                 clear is rising-edge).
>
> Why this is board-dependant? Falling edge is happening anyway, so it is
> in the hardware all the time. DT describes the hardware, not Linux
> driver behavior.
Falling edge of the pulse is happening all the time, but the falling
edge event is currently never reported by the pps-gpio driver.

It is because there is no place in the pps-gpio driver that sets
info->capture_clear, so
  pps_event(info->pps, &ts, PPS_CAPTURECLEAR, data);
will never be called.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pps/clients/pps-gpio.c?h=v6.4#n59

There was an option in the past to set info->capture_clear, but that
option was removed in commit ee89646619ba ("pps: clients: gpio: Get rid
of legacy platform data").

This node in the DT isn't a pure HW device, but rather a SW driver.
The patch I shared allows to set capture-clear from device-tree same as
setting of assert-falling-edge is done.

Do you suggest I enable capture_clear in a different way?
Perhaps module-param?

> What's more, your property name sounds a lot like a driver property, so
> even if this is suitable for DT, you would need to name it properly to
> match hardware feature/characteristic.
I chose capture-clear as a name since it is aligned with the driver's
terminology. It sets the capture_clear parameter, just like
assert-falling-edge in DT sets assert_falling_edge parameter.

---
Regards, Eliav