| Message ID | 20230625142134.33690-3-farbere@amazon.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add PPS pulse-width support | expand |
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@amazon.com> > --- > Documentation/devicetree/bindings/pps/pps-gpio.txt | 4 ++++ Please convert the bindings to DT schema first. > 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. 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. Best regards, Krzysztof
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@amazon.com> >> --- >> 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
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). Example: pps {
Add a new optional "capture-clear" boolean property. When present, PPS clear events shall also be reported. Signed-off-by: Eliav Farber <farbere@amazon.com> --- Documentation/devicetree/bindings/pps/pps-gpio.txt | 4 ++++ 1 file changed, 4 insertions(+)