lkml.org 
[lkml]   [2020]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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@linaro.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@ad00000 {
> + 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@784000 {
> 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@ac4a000 {
> + 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@ae00000 {
> compatible = "qcom,sdm845-mdss";
> reg = <0 0x0ae00000 0 0x1000>;
> --
> 2.20.1
>

\
 
 \ /
  Last update: 2020-03-12 06:13    [W:0.058 / U:43.472 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site