LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Robert Foss <robert.foss@linaro.org>
Cc: agross@kernel.org, robh+dt@kernel.org, mark.rutland@arm.com,
	catalin.marinas@arm.com, will@kernel.org, shawnguo@kernel.org,
	olof@lixom.net, Anson.Huang@nxp.com, maxime@cerno.tech,
	leonard.crestez@nxp.com, dinguyen@kernel.org,
	marcin.juszkiewicz@linaro.org, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Loic Poulain <loic.poulain@linaro.org>
Subject: Re: [v1 3/6] arm64: dts: sdm845: Add i2c-qcom-cci node
Date: Wed, 11 Mar 2020 22:12:39 -0700	[thread overview]
Message-ID: <20200312051239.GV264362@yoga> (raw)
In-Reply-To: <20200311123501.18202-4-robert.foss@linaro.org>

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
> 

  reply	other threads:[~2020-03-12  5:12 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-11 12:34 [v1 0/6] Qualcomm CCI & Camera for db410c & db845c Robert Foss
2020-03-11 12:34 ` [v1 1/6] arm64: dts: msm8916: Add i2c-qcom-cci node Robert Foss
2020-03-12  4:50   ` Bjorn Andersson
2020-03-11 12:34 ` [v1 2/6] arm64: dts: apq8016-sbc: Add CCI/Sensor nodes Robert Foss
2020-03-12  5:03   ` Bjorn Andersson
2020-03-11 12:34 ` [v1 3/6] arm64: dts: sdm845: Add i2c-qcom-cci node Robert Foss
2020-03-12  5:12   ` Bjorn Andersson [this message]
2020-03-11 12:34 ` [v1 4/6] arm64: dts: sdm845-db845c: Add pm_8998 gpio names Robert Foss
2020-03-12  5:13   ` Bjorn Andersson
2020-03-11 12:35 ` [v1 5/6] arm64: dts: sdm845-db845c: Add ov8856 & ov7251 camera nodes Robert Foss
2020-03-12  5:34   ` Bjorn Andersson
2020-03-11 12:35 ` [v1 6/6] arm64: defconfig: Enable QCOM CAMCC, CAMSS and CCI drivers Robert Foss
2020-03-12  5:35   ` Bjorn Andersson
2020-03-17 13:54     ` Robert Foss

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200312051239.GV264362@yoga \
    --to=bjorn.andersson@linaro.org \
    --cc=Anson.Huang@nxp.com \
    --cc=agross@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dinguyen@kernel.org \
    --cc=leonard.crestez@nxp.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=loic.poulain@linaro.org \
    --cc=marcin.juszkiewicz@linaro.org \
    --cc=mark.rutland@arm.com \
    --cc=maxime@cerno.tech \
    --cc=olof@lixom.net \
    --cc=robert.foss@linaro.org \
    --cc=robh+dt@kernel.org \
    --cc=shawnguo@kernel.org \
    --cc=will@kernel.org \
    --subject='Re: [v1 3/6] arm64: dts: sdm845: Add i2c-qcom-cci node' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).