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: <e49c2cec-56fd-a71e-0568-c56df3df4cbe@quicinc.com>
Date:   Tue, 20 Dec 2022 18:14:09 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.0
Subject: Re: [PATCH v2 6/7] arm64: dts: qcom: sa8540p-ride: add i2c nodes
To:     Brian Masney <bmasney@redhat.com>, <andersson@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>
CC:     <konrad.dybcio@linaro.org>, <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>
Content-Language: en-US
From:   Shazad Hussain <quic_shazhuss@quicinc.com>
In-Reply-To: <20221214171145.2913557-7-bmasney@redhat.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 12/14/2022 10:41 PM, 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>

Tested-by: Shazad Hussain <quic_shazhuss@quicinc.com>

Verified on next-20221220.

> ---
> 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.

expected, as no slaves are connected.

> - i2cdetect runs fast within a small fraction of a second for i2c-15
>    and i2c18.

slaves have ack on addr:0x18 ret:1 for i2c-15 and addr:0x6a ret:1 for i2c-18

> - '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";