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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ