lkml.org 
[lkml]   [2019]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v5 13/13] arm64: dts: sc7180: Add qupv3_0 and qupv3_1
    From
    Date


    On 12/6/2019 5:55 PM, Doug Anderson wrote:
    > 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.

    sure, I'll add it.

    >
    >
    >> + 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?

    so looks like these qup instances (qup2/4/7/9) can only be configured to be used as i2c or uart
    and not spi since we have only 2 pins for them and spi needs 4.

    >
    >> + 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.

    yes, i will fix all of them to remove the additional chip select muxes.

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

    \
     
     \ /
      Last update: 2019-12-10 11:34    [W:11.444 / U:0.156 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site