diff mbox series

[OpenWrt-Devel] imx6: add I2C retries for ventana i2c1

Message ID 1590674204-26618-1-git-send-email-tharvey@gateworks.com
State Needs Review / ACK
Headers show
Series [OpenWrt-Devel] imx6: add I2C retries for ventana i2c1 | expand

Commit Message

Tim Harvey May 28, 2020, 1:56 p.m. UTC
The GSC sitting on i2c1 can NAK I2C transactions if it is busy
performing an ADC cycle. Allow enough retries to work around this.

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 ...x-add-retries-for-NAK-s-on-ventana-boards.patch | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 target/linux/imx6/patches-5.4/200-i2c-imx-add-retries-for-NAK-s-on-ventana-boards.patch

Comments

Petr Štetiar June 3, 2020, 12:24 p.m. UTC | #1
Tim Harvey <tharvey@gateworks.com> [2020-05-28 06:56:44]:

Hi,

> The GSC sitting on i2c1 can NAK I2C transactions if it is busy
> performing an ADC cycle. Allow enough retries to work around this.

this looks like you either need to keep in your own vendor tree or try to fix
properly upstream, thanks. So I'm not going to accept this as this would
become just another maintenance burden during kernel version bumps etc.

-- ynezz

> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
>  ...x-add-retries-for-NAK-s-on-ventana-boards.patch | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 target/linux/imx6/patches-5.4/200-i2c-imx-add-retries-for-NAK-s-on-ventana-boards.patch
> 
> diff --git a/target/linux/imx6/patches-5.4/200-i2c-imx-add-retries-for-NAK-s-on-ventana-boards.patch b/target/linux/imx6/patches-5.4/200-i2c-imx-add-retries-for-NAK-s-on-ventana-boards.patch
> new file mode 100644
> index 0000000..5f45342
> --- /dev/null
> +++ b/target/linux/imx6/patches-5.4/200-i2c-imx-add-retries-for-NAK-s-on-ventana-boards.patch
> @@ -0,0 +1,42 @@
> +From be183fc2af49e6020bb0b1ac8359351707383f63 Mon Sep 17 00:00:00 2001
> +From: Tim Harvey <tharvey@gateworks.com>
> +Date: Tue, 12 May 2020 12:41:32 -0700
> +Subject: [PATCH] i2c: imx: add retries for NAK's on ventana boards
> +
> +Ventana boards have a Gateworks System Controller (gsc) module that can
> +nak i2c transactions when its busy in an ADC loop. In order to not have to
> +hack up the pca953x and ds1672 device drivers which the GSC emulates we will
> +just add i2c retries around NAK's.
> +
> +Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> +---
> + drivers/i2c/busses/i2c-imx.c | 6 ++++++
> + 1 file changed, 6 insertions(+)
> +
> +diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> +index a3b6133..b2378d8 100644
> +--- a/drivers/i2c/busses/i2c-imx.c
> ++++ b/drivers/i2c/busses/i2c-imx.c
> +@@ -467,6 +467,8 @@ static int i2c_imx_acked(struct imx_i2c_struct *i2c_imx)
> + {
> + 	if (imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR) & I2SR_RXAK) {
> + 		dev_dbg(&i2c_imx->adapter.dev, "<%s> No ACK\n", __func__);
> ++		if (i2c_imx->adapter.retries)
> ++			return -EAGAIN;
> + 		return -ENXIO;  /* No ACK */
> + 	}
> + 
> +@@ -1097,6 +1099,10 @@ static int i2c_imx_probe(struct platform_device *pdev)
> + 	i2c_imx->adapter.dev.of_node	= pdev->dev.of_node;
> + 	i2c_imx->base			= base;
> + 	ACPI_COMPANION_SET(&i2c_imx->adapter.dev, ACPI_COMPANION(&pdev->dev));
> ++	if (of_machine_is_compatible("gw,ventana") && phy_addr == 0x021a0000) {
> ++		dev_info(&pdev->dev, "Adding retries for Ventana GSC\n");
> ++		i2c_imx->adapter.retries = 3;
> ++	}
> + 
> + 	/* Get I2C clock */
> + 	i2c_imx->clk = devm_clk_get(&pdev->dev, NULL);
> +-- 
> +2.7.4
Tim Harvey June 3, 2020, 3:25 p.m. UTC | #2
On Wed, Jun 3, 2020 at 5:24 AM Petr Štetiar <ynezz@true.cz> wrote:
>
> Tim Harvey <tharvey@gateworks.com> [2020-05-28 06:56:44]:
>
> Hi,
>
> > The GSC sitting on i2c1 can NAK I2C transactions if it is busy
> > performing an ADC cycle. Allow enough retries to work around this.
>
> this looks like you either need to keep in your own vendor tree or try to fix
> properly upstream, thanks. So I'm not going to accept this as this would
> become just another maintenance burden during kernel version bumps etc.
>

What do you mean by keep it in my own vendor tree? Is there something
in OpenWrt you are talking about or are you simply saying it's not
upstream and therefore not accepted?

To be completely fair how does this differ from the 38 patches in
target/linux/generic/hack-5.4?

Thanks,

Tim
Petr Štetiar June 3, 2020, 3:44 p.m. UTC | #3
Tim Harvey <tharvey@gateworks.com> [2020-06-03 08:25:54]:

Hi,

> are you simply saying it's not upstream and therefore not accepted?

yes, that's my view/decision.

BTW I didn't rejected the patch, just changed the status from "Under review"
to "Needs review/ACK", but I'm not going to merge it as it means additional
workload and it can be fixed properly as you've hinted anyway :-)

-- ynezz
diff mbox series

Patch

diff --git a/target/linux/imx6/patches-5.4/200-i2c-imx-add-retries-for-NAK-s-on-ventana-boards.patch b/target/linux/imx6/patches-5.4/200-i2c-imx-add-retries-for-NAK-s-on-ventana-boards.patch
new file mode 100644
index 0000000..5f45342
--- /dev/null
+++ b/target/linux/imx6/patches-5.4/200-i2c-imx-add-retries-for-NAK-s-on-ventana-boards.patch
@@ -0,0 +1,42 @@ 
+From be183fc2af49e6020bb0b1ac8359351707383f63 Mon Sep 17 00:00:00 2001
+From: Tim Harvey <tharvey@gateworks.com>
+Date: Tue, 12 May 2020 12:41:32 -0700
+Subject: [PATCH] i2c: imx: add retries for NAK's on ventana boards
+
+Ventana boards have a Gateworks System Controller (gsc) module that can
+nak i2c transactions when its busy in an ADC loop. In order to not have to
+hack up the pca953x and ds1672 device drivers which the GSC emulates we will
+just add i2c retries around NAK's.
+
+Signed-off-by: Tim Harvey <tharvey@gateworks.com>
+---
+ drivers/i2c/busses/i2c-imx.c | 6 ++++++
+ 1 file changed, 6 insertions(+)
+
+diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
+index a3b6133..b2378d8 100644
+--- a/drivers/i2c/busses/i2c-imx.c
++++ b/drivers/i2c/busses/i2c-imx.c
+@@ -467,6 +467,8 @@ static int i2c_imx_acked(struct imx_i2c_struct *i2c_imx)
+ {
+ 	if (imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR) & I2SR_RXAK) {
+ 		dev_dbg(&i2c_imx->adapter.dev, "<%s> No ACK\n", __func__);
++		if (i2c_imx->adapter.retries)
++			return -EAGAIN;
+ 		return -ENXIO;  /* No ACK */
+ 	}
+ 
+@@ -1097,6 +1099,10 @@ static int i2c_imx_probe(struct platform_device *pdev)
+ 	i2c_imx->adapter.dev.of_node	= pdev->dev.of_node;
+ 	i2c_imx->base			= base;
+ 	ACPI_COMPANION_SET(&i2c_imx->adapter.dev, ACPI_COMPANION(&pdev->dev));
++	if (of_machine_is_compatible("gw,ventana") && phy_addr == 0x021a0000) {
++		dev_info(&pdev->dev, "Adding retries for Ventana GSC\n");
++		i2c_imx->adapter.retries = 3;
++	}
+ 
+ 	/* Get I2C clock */
+ 	i2c_imx->clk = devm_clk_get(&pdev->dev, NULL);
+-- 
+2.7.4
+