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]
Date:   Fri, 6 Dec 2019 20:25:20 +0800
From:   Doug Anderson <dianders@...omium.org>
To:     Rajendra Nayak <rnayak@...eaurora.org>
Cc:     Andy Gross <agross@...nel.org>, Rob Herring <robh+dt@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        Stephen Boyd <swboyd@...omium.org>,
        Roja Rani Yarubandi <rojay@...eaurora.org>
Subject: Re: [PATCH v5 13/13] arm64: dts: sc7180: Add qupv3_0 and qupv3_1

Hi,

On Fri, Nov 8, 2019 at 5:29 PM Rajendra Nayak <rnayak@...eaurora.org> wrote:
>
> From: Roja Rani Yarubandi <rojay@...eaurora.org>
>
> Add QUP SE instances configuration for sc7180.
>
> Signed-off-by: Roja Rani Yarubandi <rojay@...eaurora.org>
> Signed-off-by: Rajendra Nayak <rnayak@...eaurora.org>
> Reviewed-by: Stephen Boyd <swboyd@...omium.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7180-idp.dts | 146 +++++
>  arch/arm64/boot/dts/qcom/sc7180.dtsi    | 675 ++++++++++++++++++++++++
>  2 files changed, 821 insertions(+)

Comments below could be done in a follow-up patch if it makes more sense.


> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index e1d6278d85f7..666e9b92c7ad 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi

At the top of this file, please add aliases for all i2c and spi
devices (like sdm845 did).  Right now trying to use command line i2c
tools is super confusing because busses are super jumbled.


> +                       i2c2: i2c@...000 {
> +                               compatible = "qcom,geni-i2c";
> +                               reg = <0 0x00888000 0 0x4000>;
> +                               clock-names = "se";
> +                               clocks = <&gcc GCC_QUPV3_WRAP0_S2_CLK>;
> +                               pinctrl-names = "default";
> +                               pinctrl-0 = <&qup_i2c2_default>;
> +                               interrupts = <GIC_SPI 603 IRQ_TYPE_LEVEL_HIGH>;
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +                               status = "disabled";
> +                       };

Where is spi2?


> +                       i2c4: i2c@...000 {
> +                               compatible = "qcom,geni-i2c";
> +                               reg = <0 0x00890000 0 0x4000>;
> +                               clock-names = "se";
> +                               clocks = <&gcc GCC_QUPV3_WRAP0_S4_CLK>;
> +                               pinctrl-names = "default";
> +                               pinctrl-0 = <&qup_i2c4_default>;
> +                               interrupts = <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH>;
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +                               status = "disabled";
> +                       };

Where is spi4?


> +                       i2c7: i2c@...000 {
> +                               compatible = "qcom,geni-i2c";
> +                               reg = <0 0x00a84000 0 0x4000>;
> +                               clock-names = "se";
> +                               clocks = <&gcc GCC_QUPV3_WRAP1_S1_CLK>;
> +                               pinctrl-names = "default";
> +                               pinctrl-0 = <&qup_i2c7_default>;
> +                               interrupts = <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>;
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +                               status = "disabled";
> +                       };

Where is spi7?


> +                       i2c9: i2c@...000 {
> +                               compatible = "qcom,geni-i2c";
> +                               reg = <0 0x00a8c000 0 0x4000>;
> +                               clock-names = "se";
> +                               clocks = <&gcc GCC_QUPV3_WRAP1_S3_CLK>;
> +                               pinctrl-names = "default";
> +                               pinctrl-0 = <&qup_i2c9_default>;
> +                               interrupts = <GIC_SPI 356 IRQ_TYPE_LEVEL_HIGH>;
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +                               status = "disabled";
> +                       };

Where is spi9?


> +                       qup_spi1_default: qup-spi1-default {
> +                               pinmux {
> +                                       pins = "gpio0", "gpio1",
> +                                              "gpio2", "gpio3",
> +                                              "gpio12", "gpio94";

Please just mux one of the chip selects by default.  It seems like it
would be _much_ more common to have a single SPI device on the bus and
then every board doesn't have to override this.


> +                       qup_spi6_default: qup-spi6-default {
> +                               pinmux {
> +                                       pins = "gpio59", "gpio60",
> +                                              "gpio61", "gpio62",
> +                                              "gpio68", "gpio72";

Please just mux one of the chip selects by default.  It seems like it
would be _much_ more common to have a single SPI device on the bus and
then every board doesn't have to override this.


> +                       qup_spi10_default: qup-spi10-default {
> +                               pinmux {
> +                                       pins = "gpio86", "gpio87",
> +                                              "gpio88", "gpio89",
> +                                              "gpio90", "gpio91";

Please just mux one of the chip selects by default.  It seems like it
would be _much_ more common to have a single SPI device on the bus and
then every board doesn't have to override this.


-Doug

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ