[PATCH v2 11/12] arm64: dts: exynos: gs101: define USI8 with I2C configuration

Tudor Ambarus posted 12 patches 1 year, 12 months ago
There is a newer version of this series
[PATCH v2 11/12] arm64: dts: exynos: gs101: define USI8 with I2C configuration
Posted by Tudor Ambarus 1 year, 12 months ago
USI8 I2C is used to communicate with an eeprom found on the battery
connector. Define USI8 in I2C configuration.

USI8 CONFIG register comes with a 0x0 reset value, meaning that USI8
doesn't have a default protocol (I2C, SPI, UART) at reset. Thus the
selection of the protocol is intentionally left for the board dts file.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
v2:
- identify and use gate clocks instead of dividers
- move cells and pinctrl properties from dts to dtsi
- move IRQ type constant on the previous line

 arch/arm64/boot/dts/exynos/google/gs101.dtsi | 29 ++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
index 0e5b1b490b0b..c6ae33016992 100644
--- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
+++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
@@ -354,6 +354,35 @@ pinctrl_peric0: pinctrl@10840000 {
 			interrupts = <GIC_SPI 625 IRQ_TYPE_LEVEL_HIGH 0>;
 		};
 
+		usi8: usi@109700c0 {
+			compatible = "google,gs101-usi",
+				     "samsung,exynos850-usi";
+			reg = <0x109700c0 0x20>;
+			ranges;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			clocks = <&cmu_peric0 CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7>,
+				 <&cmu_peric0 CLK_GOUT_PERIC0_CLK_PERIC0_USI8_USI_CLK>;
+			clock-names = "pclk", "ipclk";
+			samsung,sysreg = <&sysreg_peric0 0x101c>;
+			status = "disabled";
+
+			hsi2c_8: i2c@10970000 {
+				compatible = "google,gs101-hsi2c",
+					     "samsung,exynosautov9-hsi2c";
+				reg = <0x10970000 0xc0>;
+				interrupts = <GIC_SPI 642 IRQ_TYPE_LEVEL_HIGH 0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				pinctrl-names = "default";
+				pinctrl-0 = <&hsi2c8_bus>;
+				clocks = <&cmu_peric0 CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7>,
+					 <&cmu_peric0 CLK_GOUT_PERIC0_CLK_PERIC0_USI8_USI_CLK>;
+				clock-names = "hsi2c", "hsi2c_pclk";
+				status = "disabled";
+			};
+		};
+
 		usi_uart: usi@10a000c0 {
 			compatible = "google,gs101-usi",
 				     "samsung,exynos850-usi";
-- 
2.43.0.472.g3155946c3a-goog
Re: [PATCH v2 11/12] arm64: dts: exynos: gs101: define USI8 with I2C configuration
Posted by André Draszik 1 year, 12 months ago
Hi Tudor,

On Thu, 2023-12-28 at 12:58 +0000, Tudor Ambarus wrote:
> [...]
> 
> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> index 0e5b1b490b0b..c6ae33016992 100644
> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> @@ -354,6 +354,35 @@ pinctrl_peric0: pinctrl@10840000 {
>  			interrupts = <GIC_SPI 625 IRQ_TYPE_LEVEL_HIGH 0>;
>  		};
>  
> +		usi8: usi@109700c0 {
> +			compatible = "google,gs101-usi",
> +				     "samsung,exynos850-usi";
> +			reg = <0x109700c0 0x20>;
> +			ranges;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			clocks = <&cmu_peric0 CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7>,
> +				 <&cmu_peric0 CLK_GOUT_PERIC0_CLK_PERIC0_USI8_USI_CLK>;
> +			clock-names = "pclk", "ipclk";

Given the clock-names, shouldn't the clock indices be the other way around? Also see below.

> +			samsung,sysreg = <&sysreg_peric0 0x101c>;
> +			status = "disabled";
> +
> +			hsi2c_8: i2c@10970000 {
> +				compatible = "google,gs101-hsi2c",
> +					     "samsung,exynosautov9-hsi2c";
> +				reg = <0x10970000 0xc0>;
> +				interrupts = <GIC_SPI 642 IRQ_TYPE_LEVEL_HIGH 0>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&hsi2c8_bus>;
> +				clocks = <&cmu_peric0 CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7>,
> +					 <&cmu_peric0 CLK_GOUT_PERIC0_CLK_PERIC0_USI8_USI_CLK>;
> +				clock-names = "hsi2c", "hsi2c_pclk";

Here, pclk == CLK_GOUT_PERIC0_CLK_PERIC0_USI8_USI_CLK (which is correct, I believe), whereas
above pclk == CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7

Cheers,
A.
Re: [PATCH v2 11/12] arm64: dts: exynos: gs101: define USI8 with I2C configuration
Posted by Tudor Ambarus 1 year, 12 months ago

On 12/28/23 14:04, André Draszik wrote:
> Hi Tudor,

Hi!

> 
> On Thu, 2023-12-28 at 12:58 +0000, Tudor Ambarus wrote:
>> [...]
>>
>> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>> index 0e5b1b490b0b..c6ae33016992 100644
>> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>> @@ -354,6 +354,35 @@ pinctrl_peric0: pinctrl@10840000 {
>>  			interrupts = <GIC_SPI 625 IRQ_TYPE_LEVEL_HIGH 0>;
>>  		};
>>  
>> +		usi8: usi@109700c0 {
>> +			compatible = "google,gs101-usi",
>> +				     "samsung,exynos850-usi";
>> +			reg = <0x109700c0 0x20>;
>> +			ranges;
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			clocks = <&cmu_peric0 CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7>,
>> +				 <&cmu_peric0 CLK_GOUT_PERIC0_CLK_PERIC0_USI8_USI_CLK>;
>> +			clock-names = "pclk", "ipclk";
> 
> Given the clock-names, shouldn't the clock indices be the other way around? Also see below.

You're right, they should have been the other way around! Didn't make
any difference at testing because the usi driver uses
clk_bulk_prepare_enable(), what matters is the order of clocks in the
i2c node, and those are fine.

> 
>> +			samsung,sysreg = <&sysreg_peric0 0x101c>;
>> +			status = "disabled";
>> +
>> +			hsi2c_8: i2c@10970000 {
>> +				compatible = "google,gs101-hsi2c",
>> +					     "samsung,exynosautov9-hsi2c";
>> +				reg = <0x10970000 0xc0>;
>> +				interrupts = <GIC_SPI 642 IRQ_TYPE_LEVEL_HIGH 0>;
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +				pinctrl-names = "default";
>> +				pinctrl-0 = <&hsi2c8_bus>;
>> +				clocks = <&cmu_peric0 CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7>,
>> +					 <&cmu_peric0 CLK_GOUT_PERIC0_CLK_PERIC0_USI8_USI_CLK>;
>> +				clock-names = "hsi2c", "hsi2c_pclk";
> 
> Here, pclk == CLK_GOUT_PERIC0_CLK_PERIC0_USI8_USI_CLK (which is correct, I believe), whereas
> above pclk == CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7
> 

Indeed, I'll reverse the order for the USI clocks and do some more
testing. Thanks!
ta
Re: [PATCH v2 11/12] arm64: dts: exynos: gs101: define USI8 with I2C configuration
Posted by Tudor Ambarus 1 year, 12 months ago

On 12/29/23 08:04, Tudor Ambarus wrote:
> 
> 
> On 12/28/23 14:04, André Draszik wrote:
>> Hi Tudor,
> 
> Hi!
> 
>>
>> On Thu, 2023-12-28 at 12:58 +0000, Tudor Ambarus wrote:
>>> [...]
>>>
>>> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>> index 0e5b1b490b0b..c6ae33016992 100644
>>> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>> @@ -354,6 +354,35 @@ pinctrl_peric0: pinctrl@10840000 {
>>>  			interrupts = <GIC_SPI 625 IRQ_TYPE_LEVEL_HIGH 0>;
>>>  		};
>>>  
>>> +		usi8: usi@109700c0 {
>>> +			compatible = "google,gs101-usi",
>>> +				     "samsung,exynos850-usi";
>>> +			reg = <0x109700c0 0x20>;
>>> +			ranges;
>>> +			#address-cells = <1>;
>>> +			#size-cells = <1>;
>>> +			clocks = <&cmu_peric0 CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7>,
>>> +				 <&cmu_peric0 CLK_GOUT_PERIC0_CLK_PERIC0_USI8_USI_CLK>;
>>> +			clock-names = "pclk", "ipclk";
>>
>> Given the clock-names, shouldn't the clock indices be the other way around? Also see below.
> 
> You're right, they should have been the other way around! Didn't make
> any difference at testing because the usi driver uses
> clk_bulk_prepare_enable(), what matters is the order of clocks in the
> i2c node, and those are fine.
> 
>>
>>> +			samsung,sysreg = <&sysreg_peric0 0x101c>;
>>> +			status = "disabled";
>>> +
>>> +			hsi2c_8: i2c@10970000 {
>>> +				compatible = "google,gs101-hsi2c",
>>> +					     "samsung,exynosautov9-hsi2c";
>>> +				reg = <0x10970000 0xc0>;
>>> +				interrupts = <GIC_SPI 642 IRQ_TYPE_LEVEL_HIGH 0>;
>>> +				#address-cells = <1>;
>>> +				#size-cells = <0>;
>>> +				pinctrl-names = "default";
>>> +				pinctrl-0 = <&hsi2c8_bus>;
>>> +				clocks = <&cmu_peric0 CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7>,
>>> +					 <&cmu_peric0 CLK_GOUT_PERIC0_CLK_PERIC0_USI8_USI_CLK>;
>>> +				clock-names = "hsi2c", "hsi2c_pclk";
>>
>> Here, pclk == CLK_GOUT_PERIC0_CLK_PERIC0_USI8_USI_CLK (which is correct, I believe), whereas
>> above pclk == CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7
>>
> 
> Indeed, I'll reverse the order for the USI clocks and do some more
> testing. Thanks!

FYI, I reversed the order of the USI clocks, tested again with the
eeprom at 100 KHz and 10KHz, everything went fine. I'll wait for some
other feedback and probably submit a v3 next week.

Cheers,
ta