Re: [PATCH v3 02/11] arm64: dts: sc7180: Add minimal dts/dtsi files for SC7180 soc

[Rajendra Nayak <rnayak@codeaurora.org>]

On 10/29/2019 10:19 PM, Stephen Boyd wrote:
> Quoting Rajendra Nayak (2019-10-23 02:02:10)
>> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> new file mode 100644
>> index 000000000000..084854341ddd
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> @@ -0,0 +1,300 @@
>> +// SPDX-License-Identifier: BSD-3-Clause
>> +/*
>> + * SC7180 SoC device tree source
>> + *
>> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#include <dt-bindings/clock/qcom,gcc-sc7180.h>
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +/ {
>> +       interrupt-parent = <&intc>;
>> +
>> +       #address-cells = <2>;
>> +       #size-cells = <2>;
>> +
>> +       chosen { };
>> +
>> +       clocks {
>> +               xo_board: xo-board {
>> +                       compatible = "fixed-clock";
>> +                       clock-frequency = <38400000>;
>> +                       clock-output-names = "xo_board";
> 
> Can you drop the output names property? I think we don't care that the
> name is "xo-board" instead of "xo_board" now.

sure, will do.

> 
>> +                       #clock-cells = <0>;
>> +               };
>> +
>> +               sleep_clk: sleep-clk {
>> +                       compatible = "fixed-clock";
>> +                       clock-frequency = <32764>;
>> +                       clock-output-names = "sleep_clk";
>> +                       #clock-cells = <0>;
>> +               };
>> +       };
>> +
> [...]
>> +
>> +       soc: soc {
>> +               #address-cells = <2>;
>> +               #size-cells = <2>;
>> +               ranges = <0 0 0 0 0x10 0>;
>> +               dma-ranges = <0 0 0 0  0x10 0>;
> 
> Why the extra space here               ^ ?

typo, will fix.

> 
>> +               compatible = "simple-bus";
>> +
>> +               gcc: clock-controller@100000 {
>> +                       compatible = "qcom,gcc-sc7180";
>> +                       reg = <0 0x00100000 0 0x1f0000>;
>> +                       #clock-cells = <1>;
>> +                       #reset-cells = <1>;
>> +                       #power-domain-cells = <1>;
>> +               };
>> +
>> +               qupv3_id_1: geniqup@ac0000 {
>> +                       compatible = "qcom,geni-se-qup";
>> +                       reg = <0 0x00ac0000 0 0x6000>;
>> +                       clock-names = "m-ahb", "s-ahb";
>> +                       clocks = <&gcc GCC_QUPV3_WRAP_1_M_AHB_CLK>,
>> +                                <&gcc GCC_QUPV3_WRAP_1_S_AHB_CLK>;
>> +                       #address-cells = <2>;
>> +                       #size-cells = <2>;
>> +                       ranges;
>> +                       status = "disabled";
>> +
>> +                       uart10: serial@a88000 {
>> +                               compatible = "qcom,geni-debug-uart";
>> +                               reg = <0 0x00a88000 0 0x4000>;
>> +                               clock-names = "se";
>> +                               clocks = <&gcc GCC_QUPV3_WRAP1_S2_CLK>;
>> +                               pinctrl-names = "default";
>> +                               pinctrl-0 = <&qup_uart10_default>;
>> +                               interrupts = <GIC_SPI 355 IRQ_TYPE_LEVEL_HIGH>;
>> +                               status = "disabled";
>> +                       };
> 
> Can we not add all the i2c/spi/uart cores here?

I see that these nodes are posted now [1].
Will pull it in as part of this series so it can be reviewed together.

[1] https://lkml.org/lkml/2019/10/31/63

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation