| Message ID | 20220811185901.3992631-1-tharvey@gateworks.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | board: gateworks: venice: poll I2C lines to wait for GSC firmware | expand |
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@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@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@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@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;
On Thu, Aug 11, 2022 at 5:39 PM Peng Fan <peng.fan@oss.nxp.com> wrote: > > > > 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@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@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 > Peng, Thanks for catching this! I will fix this and the others in this patch and send a new version. Best Regards, Tim
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@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; + }; + 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@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; + }; + 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@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; + }; + 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;
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@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(-)