| 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
| ||
|
Message-ID: <20200312051239.GV264362@yoga>
Date: Wed, 11 Mar 2020 22:12:39 -0700
From: Bjorn Andersson <bjorn.andersson@...aro.org>
To: Robert Foss <robert.foss@...aro.org>
Cc: agross@...nel.org, robh+dt@...nel.org, mark.rutland@....com,
catalin.marinas@....com, will@...nel.org, shawnguo@...nel.org,
olof@...om.net, Anson.Huang@....com, maxime@...no.tech,
leonard.crestez@....com, dinguyen@...nel.org,
marcin.juszkiewicz@...aro.org, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
Loic Poulain <loic.poulain@...aro.org>
Subject: Re: [v1 3/6] arm64: dts: sdm845: Add i2c-qcom-cci node
On Wed 11 Mar 05:34 PDT 2020, Robert Foss wrote:
> The sdm845 SOC ships with a CCI controller, which
> has two CCI/I2C buses.
>
> Signed-off-by: Robert Foss <robert.foss@...aro.org>
> ---
> arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 4 +
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 110 +++++++++++++++++++++
> 2 files changed, 114 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> index eb77aaa6a819..a6b6837c3d68 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> @@ -583,3 +583,7 @@
> bias-pull-up;
> };
> };
> +
> +&cci {
> + status = "ok";
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index d42302b8889b..b7f5c0b0f6af 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -5,6 +5,7 @@
> * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> */
>
> +#include <dt-bindings/clock/qcom,camcc-sdm845.h>
> #include <dt-bindings/clock/qcom,dispcc-sdm845.h>
> #include <dt-bindings/clock/qcom,gcc-sdm845.h>
> #include <dt-bindings/clock/qcom,gpucc-sdm845.h>
> @@ -717,6 +718,14 @@
> #power-domain-cells = <1>;
> };
>
> + clock_camcc: clock-controller@...0000 {
> + compatible = "qcom,sdm845-camcc";
> + reg = <0 0xad00000 0 0x10000>;
Please pad address (i.e. the second cell) to 8 digits and maintain sort
order by address.
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + #power-domain-cells = <1>;
> + };
> +
> qfprom@...000 {
> compatible = "qcom,qfprom";
> reg = <0 0x00784000 0 0x8ff>;
> @@ -1451,6 +1460,60 @@
> gpio-ranges = <&tlmm 0 0 150>;
> wakeup-parent = <&pdc_intc>;
>
> + cci0_default: cci0_default {
No _ in the node name (i.e you can do cci0_default: cci0-default).
> + /* SDA, SCL */
> + pinmux {
You no longer need this intermediate node, instead you can write this
as:
cci0_default: cci0-default {
pins = "gpio17", "gpio18";
function = "cci_i2c";
bias-pull-up;
drive-strength = <2>;
};
Or alternatively if you would like to group things in subnodes, do so by
pin (to allow different pinconf per pin in a nice way), i.e:
cci0_default: cci0-default {
sda {
pins = "gpio17";
function = "cci_i2c";
bias-pull-up;
drive-strength = <2>;
};
scl {
pins = "gpio18";
function = "cci_i2c";
bias-pull-up;
drive-strength = <2>;
};
};
> + function = "cci_i2c";
> + pins = "gpio17", "gpio18";
> + };
> + pinconf {
> + pins = "gpio17", "gpio18";
> + bias-pull-up;
> + drive-strength = <2>; /* 2 mA */
> + };
> + };
> +
> + cci0_sleep: cci0_sleep {
> + /* SDA, SCL */
> + mux {
> + pins = "gpio17", "gpio18";
> + function = "cci_i2c";
> + };
> +
> + config {
> + pins = "gpio17", "gpio18";
> + drive-strength = <2>; /* 2 mA */
> + bias-pull-down;
> + };
> + };
> +
> + cci1_default: cci1_default {
> + /* SDA, SCL */
> + pinmux {
> + function = "cci_i2c";
> + pins = "gpio19", "gpio20";
> + };
> + pinconf {
> + pins = "gpio19", "gpio20";
> + bias-pull-up;
> + drive-strength = <2>; /* 2 mA */
> + };
> + };
> +
> + cci1_sleep: cci1_sleep {
> + /* SDA, SCL */
> + mux {
> + pins = "gpio19", "gpio20";
> + function = "cci_i2c";
> + };
> +
> + config {
> + pins = "gpio19", "gpio20";
> + drive-strength = <2>; /* 2 mA */
> + bias-pull-down;
> + };
> + };
> +
> qspi_clk: qspi-clk {
> pinmux {
> pins = "gpio95";
> @@ -2608,6 +2671,53 @@
> #reset-cells = <1>;
> };
>
> + cci: cci@...a000 {
> + compatible = "qcom,sdm845-cci";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + reg = <0 0xac4a000 0 0x4000>;
Please pad 0xac4a000 to 8 digits.
> + interrupts = <GIC_SPI 460 IRQ_TYPE_EDGE_RISING>;
> + power-domains = <&clock_camcc TITAN_TOP_GDSC>;
> +
> + clocks = <&clock_camcc CAM_CC_CAMNOC_AXI_CLK>,
> + <&clock_camcc CAM_CC_SOC_AHB_CLK>,
> + <&clock_camcc CAM_CC_SLOW_AHB_CLK_SRC>,
> + <&clock_camcc CAM_CC_CPAS_AHB_CLK>,
> + <&clock_camcc CAM_CC_CCI_CLK>,
> + <&clock_camcc CAM_CC_CCI_CLK_SRC>;
> + clock-names = "camnoc_axi_clk",
> + "soc_ahb_clk",
> + "slow_ahb_src_clk",
> + "cpas_ahb_clk",
> + "cci",
> + "cci_clk_src";
Please drop the "_clk" suffix from these (iirc, these strings aren't
significant to the binding anyways).
> +
> + assigned-clocks = <&clock_camcc CAM_CC_CAMNOC_AXI_CLK>,
> + <&clock_camcc CAM_CC_CCI_CLK>;
> + assigned-clock-rates = <80000000>, <37500000>;
> +
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&cci0_default &cci1_default>;
> + pinctrl-1 = <&cci0_sleep &cci1_sleep>;
> +
> + status = "disabled";
> +
> + i2c-bus@0 {
Please give these labels, to make it easy to reference each bus in the
board dts and to add children.
Regards,
Bjorn
> + reg = <0>;
> + clock-frequency = <1000000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + i2c-bus@1 {
> + reg = <1>;
> + clock-frequency = <1000000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> +
> mdss: mdss@...0000 {
> compatible = "qcom,sdm845-mdss";
> reg = <0 0x0ae00000 0 0x1000>;
> --
> 2.20.1
>
Powered by blists - more mailing lists