diff mbox series

[v2,2/3] arm64: dts: marvell: AC5: use I2C unstuck function

Message ID 20231006003321.2100016-3-chris.packham@alliedtelesis.co.nz
State New
Headers
Series i2c: mv64xxx: Support for I2C unstuck |

Commit Message

Chris Packham Oct. 6, 2023, 12:33 a.m. UTC 
  The AC5 SoC supports using a controller based I2C unstuck function for
recovery. Use this instead of the generic GPIO recovery.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

Comments

Gregory CLEMENT Oct. 19, 2023, 2:40 p.m. UTC | #1 
Hello Chris,

> The AC5 SoC supports using a controller based I2C unstuck function for
> recovery. Use this instead of the generic GPIO recovery.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
> index c9ce1010c415..e52d3c3496d5 100644
> --- a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
> @@ -137,7 +137,7 @@ mdio: mdio@22004 {
>  
>  			i2c0: i2c@11000{
>  				compatible = "marvell,mv78230-i2c";
> -				reg = <0x11000 0x20>;
> +				reg = <0x11000 0x20>, <0x110a0 0x4>;
>  				#address-cells = <1>;
>  				#size-cells = <0>;
>  
> @@ -146,17 +146,14 @@ i2c0: i2c@11000{
>  				interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
>  				clock-frequency=<100000>;
>  
> -				pinctrl-names = "default", "gpio";
> +				pinctrl-names = "default";
>  				pinctrl-0 = <&i2c0_pins>;
> -				pinctrl-1 = <&i2c0_gpio>;
> -				scl-gpios = <&gpio0 26 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> -				sda-gpios = <&gpio0 27 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;

By doing this then older kernel won't be able to do recovery, while if
you keep it, the new kernels will still use new way to support recovery
thanks to the new reg filed added and old kernels will continue to work.

However, what we try to maintain is running new kernel on old dtb not
the opposite which is just a nice to have. At the end it is up to you,
if you really want to remove this chunk I will apply it once the driver
part of the series will be accepted.

Gregory


>  				status = "disabled";
>  			};
>  
>  			i2c1: i2c@11100{
>  				compatible = "marvell,mv78230-i2c";
> -				reg = <0x11100 0x20>;
> +				reg = <0x11100 0x20>, <0x110a4 0x4>;
>  				#address-cells = <1>;
>  				#size-cells = <0>;
>  
> @@ -165,11 +162,8 @@ i2c1: i2c@11100{
>  				interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
>  				clock-frequency=<100000>;
>  
> -				pinctrl-names = "default", "gpio";
> +				pinctrl-names = "default";
>  				pinctrl-0 = <&i2c1_pins>;
> -				pinctrl-1 = <&i2c1_gpio>;
> -				scl-gpios = <&gpio0 20 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> -				sda-gpios = <&gpio0 21 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>  				status = "disabled";
>  			};
>  
> -- 
> 2.42.0
>
Chris Packham Oct. 19, 2023, 7:41 p.m. UTC | #2 
On 20/10/23 03:40, Gregory CLEMENT wrote:
> Hello Chris,
>
>> The AC5 SoC supports using a controller based I2C unstuck function for
>> recovery. Use this instead of the generic GPIO recovery.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>   arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 14 ++++----------
>>   1 file changed, 4 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
>> index c9ce1010c415..e52d3c3496d5 100644
>> --- a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
>> +++ b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
>> @@ -137,7 +137,7 @@ mdio: mdio@22004 {
>>   
>>   			i2c0: i2c@11000{
>>   				compatible = "marvell,mv78230-i2c";
>> -				reg = <0x11000 0x20>;
>> +				reg = <0x11000 0x20>, <0x110a0 0x4>;
>>   				#address-cells = <1>;
>>   				#size-cells = <0>;
>>   
>> @@ -146,17 +146,14 @@ i2c0: i2c@11000{
>>   				interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
>>   				clock-frequency=<100000>;
>>   
>> -				pinctrl-names = "default", "gpio";
>> +				pinctrl-names = "default";
>>   				pinctrl-0 = <&i2c0_pins>;
>> -				pinctrl-1 = <&i2c0_gpio>;
>> -				scl-gpios = <&gpio0 26 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> -				sda-gpios = <&gpio0 27 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> By doing this then older kernel won't be able to do recovery, while if
> you keep it, the new kernels will still use new way to support recovery
> thanks to the new reg filed added and old kernels will continue to work.
>
> However, what we try to maintain is running new kernel on old dtb not
> the opposite which is just a nice to have. At the end it is up to you,
> if you really want to remove this chunk I will apply it once the driver
> part of the series will be accepted.

The GPIO recovery triggers an Erratum where the SoC locks up so I'd 
prefer to see it gone (basically a version of that offload Erratum from 
the early Armada-XPs).

I think it's all academic because I'm pretty sure I'm the only one 
actually running an upstream kernel on the AC5X. Marvell still ship a 
horribly out of date fork in their official SDK.

>
> Gregory
>
>
>>   				status = "disabled";
>>   			};
>>   
>>   			i2c1: i2c@11100{
>>   				compatible = "marvell,mv78230-i2c";
>> -				reg = <0x11100 0x20>;
>> +				reg = <0x11100 0x20>, <0x110a4 0x4>;
>>   				#address-cells = <1>;
>>   				#size-cells = <0>;
>>   
>> @@ -165,11 +162,8 @@ i2c1: i2c@11100{
>>   				interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
>>   				clock-frequency=<100000>;
>>   
>> -				pinctrl-names = "default", "gpio";
>> +				pinctrl-names = "default";
>>   				pinctrl-0 = <&i2c1_pins>;
>> -				pinctrl-1 = <&i2c1_gpio>;
>> -				scl-gpios = <&gpio0 20 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> -				sda-gpios = <&gpio0 21 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>>   				status = "disabled";
>>   			};
>>   
>> -- 
>> 2.42.0
>>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
index c9ce1010c415..e52d3c3496d5 100644
--- a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
@@ -137,7 +137,7 @@  mdio: mdio@22004 {
 
 			i2c0: i2c@11000{
 				compatible = "marvell,mv78230-i2c";
-				reg = <0x11000 0x20>;
+				reg = <0x11000 0x20>, <0x110a0 0x4>;
 				#address-cells = <1>;
 				#size-cells = <0>;
 
@@ -146,17 +146,14 @@  i2c0: i2c@11000{
 				interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
 				clock-frequency=<100000>;
 
-				pinctrl-names = "default", "gpio";
+				pinctrl-names = "default";
 				pinctrl-0 = <&i2c0_pins>;
-				pinctrl-1 = <&i2c0_gpio>;
-				scl-gpios = <&gpio0 26 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
-				sda-gpios = <&gpio0 27 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
 				status = "disabled";
 			};
 
 			i2c1: i2c@11100{
 				compatible = "marvell,mv78230-i2c";
-				reg = <0x11100 0x20>;
+				reg = <0x11100 0x20>, <0x110a4 0x4>;
 				#address-cells = <1>;
 				#size-cells = <0>;
 
@@ -165,11 +162,8 @@  i2c1: i2c@11100{
 				interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
 				clock-frequency=<100000>;
 
-				pinctrl-names = "default", "gpio";
+				pinctrl-names = "default";
 				pinctrl-0 = <&i2c1_pins>;
-				pinctrl-1 = <&i2c1_gpio>;
-				scl-gpios = <&gpio0 20 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
-				sda-gpios = <&gpio0 21 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
 				status = "disabled";
 			};