lkml.org 
[lkml]   [2019]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [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@codeaurora.org> wrote:
    >
    > From: Roja Rani Yarubandi <rojay@codeaurora.org>
    >
    > Add QUP SE instances configuration for sc7180.
    >
    > Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org>
    > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
    > Reviewed-by: Stephen Boyd <swboyd@chromium.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@888000 {
    > + 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@890000 {
    > + 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@a84000 {
    > + 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@a8c000 {
    > + 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

    \
     
     \ /
      Last update: 2019-12-06 13:26    [W:8.196 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site