[PATCH] board: gateworks: venice: poll I2C lines to wait for GSC firmware

[Peng Fan]

On 8/12/2022 2:59 AM, Tim Harvey wrote:
> In some situations the GSC firmware where the EEPROM containing the
> model and DRAM configuration may not be ready by the time the SoC
> is ready to talk to it over I2C.
> 
> Instead of a hard delay, poll the I2C lines to wait until they are
> released to avoid the I2C drivers 'Arbitation lost' error message.
> 
> Signed-off-by: Tim Harvey <tharvey at gateworks.com>
> ---
>   arch/arm/dts/imx8mm-venice.dts | 13 +++++++++++-
>   arch/arm/dts/imx8mn-venice.dts | 13 +++++++++++-
>   arch/arm/dts/imx8mp-venice.dts | 13 +++++++++++-
>   board/gateworks/venice/spl.c   | 39 ++++++++++++++++++++++------------
>   4 files changed, 61 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm/dts/imx8mm-venice.dts b/arch/arm/dts/imx8mm-venice.dts
> index 39b030691e53..a1f292fbd9d3 100644
> --- a/arch/arm/dts/imx8mm-venice.dts
> +++ b/arch/arm/dts/imx8mm-venice.dts
> @@ -23,8 +23,11 @@
>   
>   &i2c1 {
>   	clock-frequency = <100000>;
> -	pinctrl-names = "default";
> +	pinctrl-names = "default", "gpio";
>   	pinctrl-0 = <&pinctrl_i2c1>;
> +	pinctrl-1 = <&pinctrl_i2c1_gpio>;
> +	scl-gpios = <&gpio5 14 GPIO_ACTIVE_HIGH>;
> +	sda-gpios = <&gpio5 15 GPIO_ACTIVE_HIGH>;
>   	status = "okay";
>   
>   	gsc: gsc at 20 {
> @@ -89,6 +92,14 @@
>   		>;
>   	};
>   
> +	pinctrl_i2c1_gpio: i2c1grp-gpio-grp {
> +		fsl,pins = <
> +			MX8MM_IOMUXC_I2C1_SCL_GPIO5_IO14	0x400001c3
> +			MX8MM_IOMUXC_I2C1_SDA_GPIO5_IO15	0x400001c3
> +		>;
> +		u-boot,dm-spl;
u-boot,dm-spl should be in xx-u-boot.dtsi

> +	};
> +
>   	pinctrl_i2c2: i2c2grp {
>   		fsl,pins = <
>   			MX8MM_IOMUXC_I2C2_SCL_I2C2_SCL		0x400001c3
> diff --git a/arch/arm/dts/imx8mn-venice.dts b/arch/arm/dts/imx8mn-venice.dts
> index eeae225632d7..8480a2e368aa 100644
> --- a/arch/arm/dts/imx8mn-venice.dts
> +++ b/arch/arm/dts/imx8mn-venice.dts
> @@ -23,8 +23,11 @@
>   
>   &i2c1 {
>   	clock-frequency = <100000>;
> -	pinctrl-names = "default";
> +	pinctrl-names = "default", "gpio";
>   	pinctrl-0 = <&pinctrl_i2c1>;
> +	pinctrl-1 = <&pinctrl_i2c1_gpio>;
> +	scl-gpios = <&gpio5 14 GPIO_ACTIVE_HIGH>;
> +	sda-gpios = <&gpio5 15 GPIO_ACTIVE_HIGH>;
>   	status = "okay";
>   
>   	gsc: gsc at 20 {
> @@ -89,6 +92,14 @@
>   		>;
>   	};
>   
> +	pinctrl_i2c1_gpio: i2c1grp-gpio-grp {
> +		fsl,pins = <
> +			MX8MN_IOMUXC_I2C1_SCL_GPIO5_IO14	0x400001c3
> +			MX8MN_IOMUXC_I2C1_SDA_GPIO5_IO15	0x400001c3
> +		>;
> +		u-boot,dm-spl;
Ditto.

> +	};
> +
>   	pinctrl_i2c2: i2c2grp {
>   		fsl,pins = <
>   			MX8MN_IOMUXC_I2C2_SCL_I2C2_SCL		0x400001c3
> diff --git a/arch/arm/dts/imx8mp-venice.dts b/arch/arm/dts/imx8mp-venice.dts
> index 6b1a7f1a89d4..93145b37049b 100644
> --- a/arch/arm/dts/imx8mp-venice.dts
> +++ b/arch/arm/dts/imx8mp-venice.dts
> @@ -23,8 +23,11 @@
>   
>   &i2c1 {
>   	clock-frequency = <100000>;
> -	pinctrl-names = "default";
> +	pinctrl-names = "default", "gpio";
>   	pinctrl-0 = <&pinctrl_i2c1>;
> +	pinctrl-1 = <&pinctrl_i2c1_gpio>;
> +	scl-gpios = <&gpio5 14 GPIO_ACTIVE_HIGH>;
> +	sda-gpios = <&gpio5 15 GPIO_ACTIVE_HIGH>;
>   	status = "okay";
>   
>   	gsc: gsc at 20 {
> @@ -89,6 +92,14 @@
>   		>;
>   	};
>   
> +	pinctrl_i2c1_gpio: i2c1grp-gpio-grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_I2C1_SCL__GPIO5_IO14	0x400001c3
> +			MX8MP_IOMUXC_I2C1_SDA__GPIO5_IO15	0x400001c3
> +		>;
> +		u-boot,dm-spl;

Ditto.

Regards,
Peng.

> +	};
> +
>   	pinctrl_i2c2: i2c2grp {
>   		fsl,pins = <
>   			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL		0x400001c3
> diff --git a/board/gateworks/venice/spl.c b/board/gateworks/venice/spl.c
> index 914a56a96f52..ae460296cb99 100644
> --- a/board/gateworks/venice/spl.c
> +++ b/board/gateworks/venice/spl.c
> @@ -16,10 +16,12 @@
>   #include <asm/arch/imx8mp_pins.h>
>   #include <asm/arch/sys_proto.h>
>   #include <asm/mach-imx/boot_mode.h>
> +#include <asm/mach-imx/mxc_i2c.h>
>   #include <asm/arch/ddr.h>
>   #include <asm-generic/gpio.h>
>   #include <dm/uclass.h>
>   #include <dm/device.h>
> +#include <dm/pinctrl.h>
>   #include <linux/delay.h>
>   #include <power/bd71837.h>
>   #include <power/mp5416.h>
> @@ -219,8 +221,8 @@ static int power_init_board(void)
>   
>   void board_init_f(ulong dummy)
>   {
> -	struct udevice *dev;
> -	int ret;
> +	struct udevice *bus, *dev;
> +	int i, ret;
>   	int dram_sz;
>   
>   	arch_cpu_init();
> @@ -251,19 +253,28 @@ void board_init_f(ulong dummy)
>   	 *
>   	 * On a board with a missing/depleted backup battery for GSC, the
>   	 * board may be ready to probe the GSC before its firmware is
> -	 * running. We will wait here indefinately for the GSC EEPROM.
> -	 */
> -#ifdef CONFIG_IMX8MN
> -	/*
> -	 * IMX8MN boots quicker than IMX8MM and exposes issue
> -	 * where because GSC I2C state machine isn't running and its
> -	 * SCL/SDA are driven low the I2C driver spams 'Arbitration lost'
> -	 * I2C errors.
> -	 *
> -	 * TODO: Put a loop here that somehow waits for I2C CLK/DAT to be high
> +	 * running. Wait here for 50ms for the GSC firmware to let go of
> +	 * the SCL/SDA lines to avoid the i2c driver spamming
> +	 * 'Arbitration lost' I2C errors
>   	 */
> -	mdelay(50);
> -#endif
> +	if (!uclass_get_device_by_seq(UCLASS_I2C, 0, &bus)) {
> +		if (!pinctrl_select_state(bus, "gpio")) {
> +			struct mxc_i2c_bus *i2c_bus = dev_get_priv(bus);
> +			struct gpio_desc *scl_gpio = &i2c_bus->scl_gpio;
> +			struct gpio_desc *sda_gpio = &i2c_bus->sda_gpio;
> +
> +			dm_gpio_set_dir_flags(scl_gpio, GPIOD_IS_IN);
> +			dm_gpio_set_dir_flags(sda_gpio, GPIOD_IS_IN);
> +			for (i = 0; i < 5; i++) {
> +				if (dm_gpio_get_value(scl_gpio) &&
> +				    dm_gpio_get_value(sda_gpio))
> +					break;
> +				mdelay(10);
> +			}
> +			pinctrl_select_state(bus, "default");
> +		}
> +	}
> +	/* Wait indefiniately until the GSC probes */
>   	while (1) {
>   		if (!uclass_get_device_by_driver(UCLASS_MISC, DM_DRIVER_GET(gsc), &dev))
>   			break;