Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <fe670133-f9e0-c653-8b13-8abc847b1fc6@linaro.org>
Date:   Thu, 15 Dec 2022 13:57:14 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.0
Subject: Re: [PATCH v2 6/7] arm64: dts: qcom: sa8540p-ride: add i2c nodes
Content-Language: en-US
To:     Brian Masney <bmasney@redhat.com>, andersson@kernel.org,
        krzysztof.kozlowski+dt@linaro.org
Cc:     quic_shazhuss@quicinc.com, robh+dt@kernel.org,
        johan+linaro@kernel.org, linux-arm-msm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        ahalaney@redhat.com, echanude@redhat.com
References: <20221214171145.2913557-1-bmasney@redhat.com>
 <20221214171145.2913557-7-bmasney@redhat.com>
From:   Konrad Dybcio <konrad.dybcio@linaro.org>
In-Reply-To: <20221214171145.2913557-7-bmasney@redhat.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 14.12.2022 18:11, Brian Masney wrote:
> Add the necessary nodes in order to get i2c0, i2c1, i2c12, i2c15, and
> i2c18 functioning on the automotive board and exposed to userspace.
> 
> This work was derived from various patches that Qualcomm delivered
> to Red Hat in a downstream kernel. This change was validated by using
> i2c-tools 4.3.3 on CentOS Stream 9:
> 
> [root@localhost ~]# i2cdetect -l
> i2c-0  i2c             Geni-I2C                                I2C adapter
> i2c-1  i2c             Geni-I2C                                I2C adapter
> i2c-12 i2c             Geni-I2C                                I2C adapter
> i2c-15 i2c             Geni-I2C                                I2C adapter
> i2c-18 i2c             Geni-I2C                                I2C adapter
> 
> [root@localhost ~]# i2cdetect -a -y 15
> Warning: Can't use SMBus Quick Write command, will skip some addresses
>      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
> 00:
> 10:
> 20:
> 30: -- -- -- -- -- -- -- --
> 40:
> 50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
> 60:
> 70:
> 
> Signed-off-by: Brian Masney <bmasney@redhat.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
> Changes since v1:
> - Dropped qupX_ prefix from labels. (Johan)
> - Reordered nodes based on new name.
> - Added i2c buses 0, 1, and 12 (Shazad)
> - Drop mux/config-pins and have the pin properties live directly
>   under the i2cX-default-state node. (Konrad)
> - Use decimal notation for drive strength (Johan)
> 
> A few things to note with this series applied on top of linux-next:
> - Reading from i2c-0 using 'i2cdetect -y -a 0' gives the following error
>   when reading from the ranges 0x30-0x37 and 0x50-0x5F.
>       geni_i2c 980000.i2c: Timeout abort_m_cmd
> - i2c-1 and i2c-2 successfully read using i2cdetect, however it takes
>   several seconds.
> - i2cdetect runs fast within a small fraction of a second for i2c-15
>   and i2c18.
> - 'i2cdetect -y -a $BUSNUM' shows the same address ranges 0x30-0x37
>   and 0x50-0x5F in use on all 5 buses.
> 
>  arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 83 +++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts
> index b6e0db5508c7..ccd2ea3c9d04 100644
> --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts
> +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts
> @@ -17,6 +17,11 @@ / {
>  	compatible = "qcom,sa8540p-ride", "qcom,sa8540p";
>  
>  	aliases {
> +		i2c0 = &i2c0;
> +		i2c1 = &i2c1;
> +		i2c12 = &i2c12;
> +		i2c15 = &i2c15;
> +		i2c18 = &i2c18;
>  		serial0 = &uart17;
>  	};
>  
> @@ -146,6 +151,41 @@ vreg_l8g: ldo8 {
>  	};
>  };
>  
> +&i2c0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c0_default>;
> +
> +	status = "okay";
> +};
> +
> +&i2c1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c1_default>;
> +
> +	status = "okay";
> +};
> +
> +&i2c12 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c12_default>;
> +
> +	status = "okay";
> +};
> +
> +&i2c15 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c15_default>;
> +
> +	status = "okay";
> +};
> +
> +&i2c18 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c18_default>;
> +
> +	status = "okay";
> +};
> +
>  &pcie2a {
>  	ranges = <0x01000000 0x0 0x3c200000 0x0 0x3c200000 0x0 0x100000>,
>  		 <0x02000000 0x0 0x3c300000 0x0 0x3c300000 0x0 0x1d00000>,
> @@ -188,6 +228,14 @@ &pcie3a_phy {
>  	status = "okay";
>  };
>  
> +&qup0 {
> +	status = "okay";
> +};
> +
> +&qup1 {
> +	status = "okay";
> +};
> +
>  &qup2 {
>  	status = "okay";
>  };
> @@ -268,6 +316,41 @@ &xo_board_clk {
>  /* PINCTRL */
>  
>  &tlmm {
> +	i2c0_default: i2c0-default-state {
> +		pins = "gpio135", "gpio136";
> +		function = "qup15";
> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};
> +
> +	i2c1_default: i2c1-default-state {
> +		pins = "gpio158", "gpio159";
> +		function = "qup15";
> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};
> +
> +	i2c12_default: i2c12-default-state {
> +		pins = "gpio0", "gpio1";
> +		function = "qup15";
> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};
> +
> +	i2c15_default: i2c15-default-state {
> +		pins = "gpio36", "gpio37";
> +		function = "qup15";
> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};
> +
> +	i2c18_default: i2c18-default-state {
> +		pins = "gpio66", "gpio67";
> +		function = "qup18";
> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};
> +
>  	pcie2a_default: pcie2a-default-state {
>  		perst-pins {
>  			pins = "gpio143";