Re: [PATCH] arm: mvebu: x240: Use i2c-gpio instead of built in controller

2023-07-24 Thread Stefan Roese

Hi Chris,

On 7/20/23 23:01, Chris Packham wrote:

Hi Me,

On Thu, Jul 20, 2023 at 3:03 PM Chris Packham  wrote:


There is an Errata with the built-in I2C controller where various I2C
hardware errors cause a complete lockup of the CPU (which eventually
results in an watchdog reset).

Put the I2C MPP pins into GPIO mode and use the i2c-gpio driver instead.
This uses a bit-banged implementation of an I2C controller and avoids
triggering the Errata.

Signed-off-by: Chris Packham 
---
This is dependent on a bug-fix for the i2c-gpio driver I just sent
out[1] (sorry I should have sent them as a series but I thought this
would take me longer to test than it did).

[1] - 
https://lore.kernel.org/u-boot/20230720023107.1184147-1-judge.pack...@gmail.com/

  arch/arm/dts/ac5-98dx35xx-atl-x240.dts | 30 ++
  configs/x240_defconfig |  1 +
  2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/arch/arm/dts/ac5-98dx35xx-atl-x240.dts 
b/arch/arm/dts/ac5-98dx35xx-atl-x240.dts
index 820ec18b4355..d47220520b9e 100644
--- a/arch/arm/dts/ac5-98dx35xx-atl-x240.dts
+++ b/arch/arm/dts/ac5-98dx35xx-atl-x240.dts
@@ -71,8 +71,25 @@
  };

   {
-   status = "okay";
+   status = "disabled";
+};


I'll remove this section completely. I did want to make it clear that
we're using the same pins just as GPIOs instead of the dedicated I2C
mode which has issues but that can be explained better in the commit
message.



+&{/} {


I should probably move this part up to the root node rather than
addressing it here.


All these comments sound like a "good think" to do.

Feel free to add my:

Reviewed-by: Stefan Roese 

in the next version.

Thanks,
Stefan


+   i2cgpio: i2c-gpio-0 {
+   compatible = "i2c-gpio";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   pinctrl-names = "default";
+   pinctrl-0 =  <_gpio>;
+   scl-gpios = < 26 (GPIO_ACTIVE_HIGH | 
GPIO_OPEN_DRAIN)>;
+   sda-gpios = < 27 (GPIO_ACTIVE_HIGH | 
GPIO_OPEN_DRAIN)>;
+   i2c-gpio,delay-us = <2>;
+   status = "okay";
+};
+};
+
+ {
 mux@71 {
 #address-cells = <1>;
 #size-cells = <0>;
@@ -177,8 +194,8 @@
  * LED_OE_N  [23]
  * USB_PWR_FLT_N [24]
  * SFP_INT_N [25]
-* I2C0_SCL  [26]
-* I2C0_SDA  [27]
+* I2C0_SCL  [26] (GPIO)
+* I2C0_SDA  [27] (GPIO)
  * USB_EN[28]
  * MONITOR_INT_N [29]
  * XM1_MDC   [30]
@@ -201,7 +218,7 @@
 /*   0123456789 */
 pin-func = < 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
  0xff 0xff 11110xff 0xff 00
-0000001100
+0000000xff 0xff 00
  1111000000
  000111>;

@@ -209,4 +226,9 @@
 marvell,pins = <0 1 2 3 4 5 6 7 8 9 10 11 16 17>;
 marvell,function = <2>;
 };
+
+   i2c0_gpio: i2c0-gpio-pins {
+   marvell,pins = <26 27>;
+   marvell,function = <0>;
+   };
  };
diff --git a/configs/x240_defconfig b/configs/x240_defconfig
index 6d25c5ae3fcf..e8589d636081 100644
--- a/configs/x240_defconfig
+++ b/configs/x240_defconfig
@@ -43,6 +43,7 @@ CONFIG_CLK_MVEBU=y
  CONFIG_GPIO_HOG=y
  CONFIG_DM_PCA953X=y
  CONFIG_DM_I2C=y
+CONFIG_DM_I2C_GPIO=y
  CONFIG_SYS_I2C_MVTWSI=y
  CONFIG_I2C_MUX=y
  CONFIG_I2C_MUX_PCA954x=y
--
2.41.0



Viele Grüße,
Stefan Roese

--
DENX Software Engineering GmbH,  Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de


Re: [PATCH] arm: mvebu: x240: Use i2c-gpio instead of built in controller

2023-07-20 Thread Chris Packham
Hi Me,

On Thu, Jul 20, 2023 at 3:03 PM Chris Packham  wrote:
>
> There is an Errata with the built-in I2C controller where various I2C
> hardware errors cause a complete lockup of the CPU (which eventually
> results in an watchdog reset).
>
> Put the I2C MPP pins into GPIO mode and use the i2c-gpio driver instead.
> This uses a bit-banged implementation of an I2C controller and avoids
> triggering the Errata.
>
> Signed-off-by: Chris Packham 
> ---
> This is dependent on a bug-fix for the i2c-gpio driver I just sent
> out[1] (sorry I should have sent them as a series but I thought this
> would take me longer to test than it did).
>
> [1] - 
> https://lore.kernel.org/u-boot/20230720023107.1184147-1-judge.pack...@gmail.com/
>
>  arch/arm/dts/ac5-98dx35xx-atl-x240.dts | 30 ++
>  configs/x240_defconfig |  1 +
>  2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/dts/ac5-98dx35xx-atl-x240.dts 
> b/arch/arm/dts/ac5-98dx35xx-atl-x240.dts
> index 820ec18b4355..d47220520b9e 100644
> --- a/arch/arm/dts/ac5-98dx35xx-atl-x240.dts
> +++ b/arch/arm/dts/ac5-98dx35xx-atl-x240.dts
> @@ -71,8 +71,25 @@
>  };
>
>   {
> -   status = "okay";
> +   status = "disabled";
> +};

I'll remove this section completely. I did want to make it clear that
we're using the same pins just as GPIOs instead of the dedicated I2C
mode which has issues but that can be explained better in the commit
message.

>
> +&{/} {

I should probably move this part up to the root node rather than
addressing it here.

> +   i2cgpio: i2c-gpio-0 {
> +   compatible = "i2c-gpio";
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   pinctrl-names = "default";
> +   pinctrl-0 =  <_gpio>;
> +   scl-gpios = < 26 (GPIO_ACTIVE_HIGH | 
> GPIO_OPEN_DRAIN)>;
> +   sda-gpios = < 27 (GPIO_ACTIVE_HIGH | 
> GPIO_OPEN_DRAIN)>;
> +   i2c-gpio,delay-us = <2>;
> +   status = "okay";
> +};
> +};
> +
> + {
> mux@71 {
> #address-cells = <1>;
> #size-cells = <0>;
> @@ -177,8 +194,8 @@
>  * LED_OE_N  [23]
>  * USB_PWR_FLT_N [24]
>  * SFP_INT_N [25]
> -* I2C0_SCL  [26]
> -* I2C0_SDA  [27]
> +* I2C0_SCL  [26] (GPIO)
> +* I2C0_SDA  [27] (GPIO)
>  * USB_EN[28]
>  * MONITOR_INT_N [29]
>  * XM1_MDC   [30]
> @@ -201,7 +218,7 @@
> /*   0123456789 */
> pin-func = < 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
>  0xff 0xff 11110xff 0xff 00
> -0000001100
> +0000000xff 0xff 00
>  1111000000
>  000111>;
>
> @@ -209,4 +226,9 @@
> marvell,pins = <0 1 2 3 4 5 6 7 8 9 10 11 16 17>;
> marvell,function = <2>;
> };
> +
> +   i2c0_gpio: i2c0-gpio-pins {
> +   marvell,pins = <26 27>;
> +   marvell,function = <0>;
> +   };
>  };
> diff --git a/configs/x240_defconfig b/configs/x240_defconfig
> index 6d25c5ae3fcf..e8589d636081 100644
> --- a/configs/x240_defconfig
> +++ b/configs/x240_defconfig
> @@ -43,6 +43,7 @@ CONFIG_CLK_MVEBU=y
>  CONFIG_GPIO_HOG=y
>  CONFIG_DM_PCA953X=y
>  CONFIG_DM_I2C=y
> +CONFIG_DM_I2C_GPIO=y
>  CONFIG_SYS_I2C_MVTWSI=y
>  CONFIG_I2C_MUX=y
>  CONFIG_I2C_MUX_PCA954x=y
> --
> 2.41.0
>


[PATCH] arm: mvebu: x240: Use i2c-gpio instead of built in controller

2023-07-19 Thread Chris Packham
There is an Errata with the built-in I2C controller where various I2C
hardware errors cause a complete lockup of the CPU (which eventually
results in an watchdog reset).

Put the I2C MPP pins into GPIO mode and use the i2c-gpio driver instead.
This uses a bit-banged implementation of an I2C controller and avoids
triggering the Errata.

Signed-off-by: Chris Packham 
---
This is dependent on a bug-fix for the i2c-gpio driver I just sent
out[1] (sorry I should have sent them as a series but I thought this
would take me longer to test than it did).

[1] - 
https://lore.kernel.org/u-boot/20230720023107.1184147-1-judge.pack...@gmail.com/

 arch/arm/dts/ac5-98dx35xx-atl-x240.dts | 30 ++
 configs/x240_defconfig |  1 +
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/arch/arm/dts/ac5-98dx35xx-atl-x240.dts 
b/arch/arm/dts/ac5-98dx35xx-atl-x240.dts
index 820ec18b4355..d47220520b9e 100644
--- a/arch/arm/dts/ac5-98dx35xx-atl-x240.dts
+++ b/arch/arm/dts/ac5-98dx35xx-atl-x240.dts
@@ -71,8 +71,25 @@
 };
 
  {
-   status = "okay";
+   status = "disabled";
+};
 
+&{/} {
+   i2cgpio: i2c-gpio-0 {
+   compatible = "i2c-gpio";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   pinctrl-names = "default";
+   pinctrl-0 =  <_gpio>;
+   scl-gpios = < 26 (GPIO_ACTIVE_HIGH | 
GPIO_OPEN_DRAIN)>;
+   sda-gpios = < 27 (GPIO_ACTIVE_HIGH | 
GPIO_OPEN_DRAIN)>;
+   i2c-gpio,delay-us = <2>;
+   status = "okay";
+};
+};
+
+ {
mux@71 {
#address-cells = <1>;
#size-cells = <0>;
@@ -177,8 +194,8 @@
 * LED_OE_N  [23]
 * USB_PWR_FLT_N [24]
 * SFP_INT_N [25]
-* I2C0_SCL  [26]
-* I2C0_SDA  [27]
+* I2C0_SCL  [26] (GPIO)
+* I2C0_SDA  [27] (GPIO)
 * USB_EN[28]
 * MONITOR_INT_N [29]
 * XM1_MDC   [30]
@@ -201,7 +218,7 @@
/*   0123456789 */
pin-func = < 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
 0xff 0xff 11110xff 0xff 00
-0000001100
+0000000xff 0xff 00
 1111000000
 000111>;
 
@@ -209,4 +226,9 @@
marvell,pins = <0 1 2 3 4 5 6 7 8 9 10 11 16 17>;
marvell,function = <2>;
};
+
+   i2c0_gpio: i2c0-gpio-pins {
+   marvell,pins = <26 27>;
+   marvell,function = <0>;
+   };
 };
diff --git a/configs/x240_defconfig b/configs/x240_defconfig
index 6d25c5ae3fcf..e8589d636081 100644
--- a/configs/x240_defconfig
+++ b/configs/x240_defconfig
@@ -43,6 +43,7 @@ CONFIG_CLK_MVEBU=y
 CONFIG_GPIO_HOG=y
 CONFIG_DM_PCA953X=y
 CONFIG_DM_I2C=y
+CONFIG_DM_I2C_GPIO=y
 CONFIG_SYS_I2C_MVTWSI=y
 CONFIG_I2C_MUX=y
 CONFIG_I2C_MUX_PCA954x=y
-- 
2.41.0