linux-kernel - Re: [PATCH 6/7] arm64: dts: msm: add PDC device bindings for sdm845

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]

Date:   Thu, 20 Dec 2018 10:14:25 -0800
From:   Doug Anderson <dianders@...omium.org>
To:     Lina Iyer <ilina@...eaurora.org>
Cc:     Stephen Boyd <sboyd@...nel.org>, Evan Green <evgreen@...omium.org>,
        Marc Zyngier <marc.zyngier@....com>,
        LKML <linux-kernel@...r.kernel.org>,
        Raju P L S S S N <rplsssn@...eaurora.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>
Subject: Re: [PATCH 6/7] arm64: dts: msm: add PDC device bindings for sdm845

Hi,

On Wed, Dec 19, 2018 at 2:11 PM Lina Iyer <ilina@...eaurora.org> wrote:
>
> Add PDC interrupt controller device bindings for SDM845.
>
> Signed-off-by: Lina Iyer <ilina@...eaurora.org>
> ---
> Changes in v3:
>         - Fix PDC map, use GIC SPI port number for hwirq
> Changes in v2:
>         - Order by address
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)

nit: ${SUBJECT} makes it sounds like you're adding something into the
"Documentation/devicetree/bindings" folder, but you're not.  Also you
probably want the prefix "qcom", not "msm" since it ends up in the
"qcom" dir.  Also, subject should say that this is the interrupt
controller.  How about:

arm64: dts: qcom: add PDC interrupt controller for sdm845

> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index b72bdb0a31a5..8e15392a6f64 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -1278,6 +1278,15 @@
>                         #reset-cells = <1>;
>                 };

nit: the above node looks like the tail end of pdc reset controller.
That has a unit address of b2e0000.  Your unit address is smaller than
the the pdc reset controller so you should be above it, not below it.

> +               pdc: interrupt-controller@...0000 {

nit: Maybe the label should be "pdc_intc" not just "pdc".  This is
just the node for the interrupt controller, not the whole pdc, right?

> +                       compatible = "qcom,sdm845-pdc";
> +                       reg = <0xb220000 0x30000>;

nit: apparently common practice for Quaclomm dts is to pad the address
in the "reg" field to all 8 digits.  So the above should be:

reg = <0x0b220000 0x30000>;

NOTE: it's important to _not_ pad the unit address in the node name
(so you got that right).  Only update the "reg".  For context:

https://lkml.kernel.org/r/CAD=FV=WrvRH6QpaQ67yw2MFz8RP59ozkSfQC4+OAM_8fAbGZuw@mail.gmail.com

-Doug