linux-kernel - Re: [PATCH 1/3] arm64: dts: qcom: sc7180: Add APSS watchdog node

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAD=FV=X3Akg07hetQOgd0P_wTVWs3QpuCNQ8O6qQ5LK2ZeWSaQ@mail.gmail.com>
Date:   Wed, 11 Dec 2019 11:25:23 -0800
From:   Doug Anderson <dianders@...omium.org>
To:     Sai Prakash Ranjan <saiprakash.ranjan@...eaurora.org>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Stephen Boyd <swboyd@...omium.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        Guenter Roeck <groeck@...omium.org>,
        Rob Herring <robh+dt@...nel.org>
Subject: Re: [PATCH 1/3] arm64: dts: qcom: sc7180: Add APSS watchdog node

Sai,

On Tue, Dec 10, 2019 at 8:30 PM Sai Prakash Ranjan
<saiprakash.ranjan@...eaurora.org> wrote:
>
> Add APSS (Application Processor Subsystem) watchdog
> DT node for SC7180 SoC.
>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@...eaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index 666e9b92c7ad..a6773ad3738b 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -1038,6 +1038,12 @@
>                         };
>                 };
>
> +               watchdog@...10000 {
> +                       compatible = "qcom,apss-wdt-sc7180", "qcom,kpss-wdt";

If you haven't already done it (I couldn't find it), can you please
add this to "Documentation/devicetree/bindings/watchdog/qcom-wdt.txt"?
 Presumably at the same time it would be good to change the format of
that file to .yaml.

Unrelated to sc7180, but it also feels like something is awfully
screwy here in terms of the various Qualcomm device tree files
referring to watchdog timers.  It feels wrong, but perhaps you can
educate me on how it works and I'll see the light.  Specifically:

1. It seems like the same node is used for two things on other Qualcomm SoCs

If I grep the bindings for "qcom,kpss-timer" or "qcom,scss-timer", I
get two hits:

Documentation/devicetree/bindings/timer/qcom,msm-timer.txt
Documentation/devicetree/bindings/watchdog/qcom-wdt.txt

...and, in fact, there appear to be two drivers claiming compatibility here:

drivers/clocksource/timer-qcom.c
drivers/watchdog/qcom-wdt.c

That seems super odd to me.  Is that really right?  We have two
drivers probing against the same device tree nodes?  ...and that's OK?
 If so, why does only one of the bindings list the SoC-specific
bindings names?

2. The actual nodes look really wonky.  A few examples below:

2a) arch/arm/boot/dts/qcom-apq8064.dtsi:
compatible = "qcom,kpss-timer", "qcom,kpss-wdt-apq8064", "qcom,msm-timer";

...why is the SoC-specific compatible string in the middle?  The
SoC-specific one should be first.

2b) arch/arm/boot/dts/qcom-ipq4019.dtsi:
compatible = "qcom,kpss-wdt", "qcom,kpss-wdt-ipq4019";

...same question, but in this case there is no "msm-timer" at the end?

2c) arch/arm64/boot/dts/qcom/qcs404.dtsi
compatible = "qcom,kpss-wdt";

...no SoC-specific string at all?

Thanks!

-Doug