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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7d2fa941-8c3e-d99a-f556-ac9c11b500d8@amazon.com>
Date:   Wed, 28 Jun 2023 11:47:43 +0300
From:   "Farber, Eliav" <farbere@...zon.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        <giometti@...eenne.com>, <robh+dt@...nel.org>,
        <krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC:     <ronenk@...zon.com>, <talel@...zon.com>, <hhhawa@...zon.com>,
        <jonnyc@...zon.com>, <itamark@...zon.com>, <shellykz@...zon.com>,
        <amitlavi@...zon.com>, <almogbs@...zon.com>
Subject: Re: [PATCH 2/5] dt-bindings: pps: pps-gpio: introduce capture-clear property

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@...zon.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