[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <0101016eef5f3e13-a3071a8d-10d8-41fc-b635-9a2d2dcc8a68-000000@us-west-2.amazonses.com>
Date: Tue, 10 Dec 2019 10:33:46 +0000
From: Rajendra Nayak <rnayak@...eaurora.org>
To: Doug Anderson <dianders@...omium.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
On 12/6/2019 5:55 PM, Doug Anderson wrote:
> 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.
sure, I'll add it.
>
>
>> + 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?
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
Powered by blists - more mailing lists