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: <183329b4-b99c-862b-02fe-651e5f68707e@linaro.org>
Date:   Tue, 13 Dec 2022 17:13:13 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Rob Herring <robh@...nel.org>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Prakash Ranjan <saiprakash.ranjan@...eaurora.org>,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, linux-watchdog@...r.kernel.org
Subject: Re: [PATCH 06/12] dt-bindings: watchdog: qcom-wdt: merge MSM timer

On 13/12/2022 16:40, Rob Herring wrote:
> On Mon, Dec 12, 2022 at 05:35:26PM +0100, Krzysztof Kozlowski wrote:
>> Merge Qualcomm MSM timer bindings into watchdog, because the timer
>> compatibles are already included here and the hardware is quite similar.
>>
>> While converting the MSM timer bindings, adjust clock-frequency
>> property to take only one frequency, instead of two, because:
>> 1. DT schema does not allow to frequencies,
>> 2. The Linux timer driver reads only first frequency.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
>> ---
>>  .../bindings/timer/qcom,msm-timer.txt         | 47 ------------------
>>  .../bindings/watchdog/qcom-wdt.yaml           | 49 +++++++++++++++++++
>>  2 files changed, 49 insertions(+), 47 deletions(-)
>>  delete mode 100644 Documentation/devicetree/bindings/timer/qcom,msm-timer.txt
>>
>> diff --git a/Documentation/devicetree/bindings/timer/qcom,msm-timer.txt b/Documentation/devicetree/bindings/timer/qcom,msm-timer.txt
>> deleted file mode 100644
>> index 5e10c345548f..000000000000
>> --- a/Documentation/devicetree/bindings/timer/qcom,msm-timer.txt
>> +++ /dev/null
>> @@ -1,47 +0,0 @@
>> -* MSM Timer
>> -
>> -Properties:
>> -
>> -- compatible : Should at least contain "qcom,msm-timer". More specific
>> -               properties specify which subsystem the timers are paired with.
>> -
>> -               "qcom,kpss-timer" - krait subsystem
>> -               "qcom,scss-timer" - scorpion subsystem
>> -
>> -- interrupts : Interrupts for the debug timer, the first general purpose
>> -               timer, and optionally a second general purpose timer, and
>> -               optionally as well, 2 watchdog interrupts, in that order.
>> -
>> -- reg : Specifies the base address of the timer registers.
>> -
>> -- clocks: Reference to the parent clocks, one per output clock. The parents
>> -          must appear in the same order as the clock names.
>> -
>> -- clock-names: The name of the clocks as free-form strings. They should be in
>> -               the same order as the clocks.
>> -
>> -- clock-frequency : The frequency of the debug timer and the general purpose
>> -                    timer(s) in Hz in that order.
>> -
>> -Optional:
>> -
>> -- cpu-offset : per-cpu offset used when the timer is accessed without the
>> -               CPU remapping facilities. The offset is
>> -               cpu-offset + (0x10000 * cpu-nr).
>> -
>> -Example:
>> -
>> -       timer@...a000 {
>> -               compatible = "qcom,scss-timer", "qcom,msm-timer";
>> -               interrupts = <1 1 0x301>,
>> -                            <1 2 0x301>,
>> -                            <1 3 0x301>,
>> -                            <1 4 0x301>,
>> -                            <1 5 0x301>;
>> -               reg = <0x0200a000 0x100>;
>> -               clock-frequency = <19200000>,
>> -                                 <32768>;
>> -               clocks = <&sleep_clk>;
>> -               clock-names = "sleep";
>> -               cpu-offset = <0x40000>;
>> -       };
>> diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>> index b7fc57f4800e..697caf1937cc 100644
>> --- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>> +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>> @@ -10,6 +10,9 @@ maintainers:
>>    - Sai Prakash Ranjan <saiprakash.ranjan@...eaurora.org>
>>  
>>  properties:
>> +  $nodename:
>> +    pattern: "^(watchdog|timer)@[0-9a-f]+$"
>> +
>>    compatible:
>>      oneOf:
>>        - items:
>> @@ -48,6 +51,20 @@ properties:
>>    clocks:
>>      maxItems: 1
>>  
>> +  clock-names:
>> +    items:
>> +      - const: sleep
>> +
>> +  clock-frequency:
>> +    description:
>> +      The frequency of the general purpose timer in Hz in that order.
> 
> 'in that order'?

Left over, as I initial binding had here two frequencies. I'll remove it.

> 
>> +
>> +  cpu-offset:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      Per-CPU offset used when the timer is accessed without the CPU remapping
>> +      facilities. The offset is cpu-offset + (0x10000 * cpu-nr).
>> +
>>    interrupts:
>>      minItems: 1
>>      maxItems: 5
>> @@ -67,12 +84,27 @@ allOf:
>>              const: qcom,kpss-wdt
>>      then:
>>        properties:
>> +        clock-frequency: false
>> +        cpu-offset: false
>>          interrupts:
>>            minItems: 1
>>            items:
>>              - description: Bark
>>              - description: Bite
>>  
>> +    else:
>> +      properties:
>> +        interrupts:
>> +          minItems: 3
>> +          items:
>> +            - description: Debug
>> +            - description: First general purpose timer
>> +            - description: Second general purpose timer
>> +            - description: First watchdog
>> +            - description: Second watchdog
> 
> Okay, here they are. No need to set it to 2 then change to 5 in the 
> series.

My previous patch where you noticed the difference sets it to maxItems:5
in top-level and only specific variant has two described interrupts.

So if I understand your comment correctly, it is okay and I will just
fix the clock-frequency description ("in that order").

Best regards,
Krzysztof

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ