2021-05-07 01:32:46

by Chris Packham

[permalink] [raw]
Subject: [PATCH v2 3/3] i2c: mpc: implement erratum A-004447 workaround

The P2040/P2041 has an erratum where the normal i2c recovery mechanism
does not work. Implement the alternative recovery mechanism documented
in the P2040 Chip Errata Rev Q.

Signed-off-by: Chris Packham <[email protected]>
---

Notes:
Changes in v2:
- Use readb_poll_timeout instead of open-coded loop

drivers/i2c/busses/i2c-mpc.c | 84 +++++++++++++++++++++++++++++++++++-
1 file changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 30d9e89a3db2..44e88a13a9e3 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -19,6 +19,7 @@

#include <linux/clk.h>
#include <linux/io.h>
+#include <linux/iopoll.h>
#include <linux/fsl_devices.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
@@ -45,6 +46,7 @@
#define CCR_MTX 0x10
#define CCR_TXAK 0x08
#define CCR_RSTA 0x04
+#define CCR_RSVD 0x02

#define CSR_MCF 0x80
#define CSR_MAAS 0x40
@@ -97,7 +99,7 @@ struct mpc_i2c {
u32 block;
int rc;
int expect_rxack;
-
+ bool has_errata_A004447;
};

struct mpc_i2c_divider {
@@ -136,6 +138,78 @@ static void mpc_i2c_fixup(struct mpc_i2c *i2c)
}
}

+static int i2c_mpc_wait_sr(struct mpc_i2c *i2c, int mask)
+{
+ int ret;
+ u8 val;
+
+ ret = readb_poll_timeout(i2c->base + MPC_I2C_SR, val,
+ val & mask, 0, 100);
+
+ return ret;
+}
+
+/*
+ * Workaround for Erratum A004447. From the P2040CE Rev Q
+ *
+ * 1. Set up the frequency divider and sampling rate.
+ * 2. I2CCR - a0h
+ * 3. Poll for I2CSR[MBB] to get set.
+ * 4. If I2CSR[MAL] is set (an indication that SDA is stuck low), then go to
+ * step 5. If MAL is not set, then go to step 13.
+ * 5. I2CCR - 00h
+ * 6. I2CCR - 22h
+ * 7. I2CCR - a2h
+ * 8. Poll for I2CSR[MBB] to get set.
+ * 9. Issue read to I2CDR.
+ * 10. Poll for I2CSR[MIF] to be set.
+ * 11. I2CCR - 82h
+ * 12. Workaround complete. Skip the next steps.
+ * 13. Issue read to I2CDR.
+ * 14. Poll for I2CSR[MIF] to be set.
+ * 15. I2CCR - 80h
+ */
+static void mpc_i2c_fixup_A004447(struct mpc_i2c *i2c)
+{
+ int ret;
+ u32 val;
+
+ writeccr(i2c, CCR_MEN | CCR_MSTA);
+ ret = i2c_mpc_wait_sr(i2c, CSR_MBB);
+ if (ret) {
+ dev_err(i2c->dev, "timeout waiting for CSR_MBB\n");
+ return;
+ }
+
+ val = readb(i2c->base + MPC_I2C_SR);
+
+ if (val & CSR_MAL) {
+ writeccr(i2c, 0x00);
+ writeccr(i2c, CCR_MSTA | CCR_RSVD);
+ writeccr(i2c, CCR_MEN | CCR_MSTA | CCR_RSVD);
+ ret = i2c_mpc_wait_sr(i2c, CSR_MBB);
+ if (ret) {
+ dev_err(i2c->dev, "timeout waiting for CSR_MBB\n");
+ return;
+ }
+ val = readb(i2c->base + MPC_I2C_DR);
+ ret = i2c_mpc_wait_sr(i2c, CSR_MIF);
+ if (ret) {
+ dev_err(i2c->dev, "timeout waiting for CSR_MIF\n");
+ return;
+ }
+ writeccr(i2c, CCR_MEN | CCR_RSVD);
+ } else {
+ val = readb(i2c->base + MPC_I2C_DR);
+ ret = i2c_mpc_wait_sr(i2c, CSR_MIF);
+ if (ret) {
+ dev_err(i2c->dev, "timeout waiting for CSR_MIF\n");
+ return;
+ }
+ writeccr(i2c, CCR_MEN);
+ }
+}
+
#if defined(CONFIG_PPC_MPC52xx) || defined(CONFIG_PPC_MPC512x)
static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
{20, 0x20}, {22, 0x21}, {24, 0x22}, {26, 0x23},
@@ -670,7 +744,10 @@ static int fsl_i2c_bus_recovery(struct i2c_adapter *adap)
{
struct mpc_i2c *i2c = i2c_get_adapdata(adap);

- mpc_i2c_fixup(i2c);
+ if (i2c->has_errata_A004447)
+ mpc_i2c_fixup_A004447(i2c);
+ else
+ mpc_i2c_fixup(i2c);

return 0;
}
@@ -767,6 +844,9 @@ static int fsl_i2c_probe(struct platform_device *op)
}
dev_info(i2c->dev, "timeout %u us\n", mpc_ops.timeout * 1000000 / HZ);

+ if (of_property_read_bool(op->dev.of_node, "fsl,i2c-erratum-a004447"))
+ i2c->has_errata_A004447 = true;
+
i2c->adap = mpc_ops;
scnprintf(i2c->adap.name, sizeof(i2c->adap.name),
"MPC adapter (%s)", of_node_full_name(op->dev.of_node));
--
2.31.1


2021-05-07 15:10:06

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] i2c: mpc: implement erratum A-004447 workaround

On Fri, May 7, 2021 at 3:40 AM Chris Packham
<[email protected]> wrote:
>
> The P2040/P2041 has an erratum where the normal i2c recovery mechanism
> does not work. Implement the alternative recovery mechanism documented
> in the P2040 Chip Errata Rev Q.

Thanks.

> +static int i2c_mpc_wait_sr(struct mpc_i2c *i2c, int mask)
> +{
> + int ret;
> + u8 val;
> +
> + ret = readb_poll_timeout(i2c->base + MPC_I2C_SR, val,
> + val & mask, 0, 100);
> +
> + return ret;
> +}

So, now you may shrink it even further, i.e.

void __iomem *sr = i2c->base + MPC_I2C_SR;
u8 val;

return readb_poll_timeout(sr, val, val & mask, 0, 100);

--
With Best Regards,
Andy Shevchenko

2021-05-07 17:24:12

by Joakim Tjernlund

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] i2c: mpc: implement erratum A-004447 workaround

On Fri, 2021-05-07 at 14:46 +0300, Andy Shevchenko wrote:
> On Fri, May 7, 2021 at 3:40 AM Chris Packham
> <[email protected]> wrote:
> >
> > The P2040/P2041 has an erratum where the normal i2c recovery mechanism
> > does not work. Implement the alternative recovery mechanism documented
> > in the P2040 Chip Errata Rev Q.
>
> Thanks.
>
> > +static int i2c_mpc_wait_sr(struct mpc_i2c *i2c, int mask)
> > +{
> > + int ret;
> > + u8 val;
> > +
> > + ret = readb_poll_timeout(i2c->base + MPC_I2C_SR, val,
> > + val & mask, 0, 100);
> > +
> > + return ret;
> > +}
>
> So, now you may shrink it even further, i.e.
>
>        void __iomem *sr = i2c->base + MPC_I2C_SR;
>        u8 val;
>
>        return readb_poll_timeout(sr, val, val & mask, 0, 100);
>

val looks uninitialised before use?

2021-05-07 17:32:32

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] i2c: mpc: implement erratum A-004447 workaround

On Fri, May 7, 2021 at 5:52 PM Joakim Tjernlund
<[email protected]> wrote:
> On Fri, 2021-05-07 at 14:46 +0300, Andy Shevchenko wrote:
> > On Fri, May 7, 2021 at 3:40 AM Chris Packham
> > <[email protected]> wrote:

...

> > So, now you may shrink it even further, i.e.
> >
> > void __iomem *sr = i2c->base + MPC_I2C_SR;
> > u8 val;
> >
> > return readb_poll_timeout(sr, val, val & mask, 0, 100);
> >
>
> val looks uninitialised before use?

Nope.

Thinking about naming, perhaps

void __iomem *addr = i2c->base + MPC_I2C_SR;
u8 sr; // or leave as val?

return readb_poll_timeout(addr, sr, sr & mask, 0, 100);

would be more clear.

--
With Best Regards,
Andy Shevchenko