Skip to contentSkip to conversation

spi: kirkwood_spi: implement workaround for FE-9144572

PATCH Superseded Chris Packham 2 participants 4 messages Oct 16, 2017 – Jan 22, 2018

Subsystems (2)
Arm (32-bit) SPI

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Chris Packham

author
#0 lore raw

PATCH v2 1/1 spi: kirkwood_spi: implement workaround for FE-9144572

Erratum NO. FE-9144572: The device SPI interface supports frequencies of up to 50 MHz. However, due to this erratum, when the device core clock is 250 MHz and the SPI interfaces is configured for 50MHz SPI clock and CPOL=CPHA=1 there might occur data corruption on reads from the SPI device.

Implement the workaround by setting the TMISO_SAMPLE value to 0x2 in the timing1 register.

Signed-off-by Chris Packham <[email protected]>
Reviewed-by Stefan Roese <[email protected]>

---
I've based this as much as I can on the equivalent implementation in the Linux kernel[1], but there are differences in the u-boot spi infrastructure that means I can't be as specific when qualifying whether the workaround is needed.

[1] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/spi/spi-orion.c#n248

Changes in v2:
- collect reviewed-by from Stefan
- remove mvebu_spi_type
- describe errata above mvebu_spi_50mhz_ac_timing_erratum

 arch/arm/include/asm/arch-mvebu/spi.h |  6 ++++
 drivers/spi/kirkwood_spi.c            | 61 +++++++++++++++++++++++++++++++++--
 2 files changed, 64 insertions(+), 3 deletions(-)
arch/arm/include/asm/arch-mvebu/spi.h +6 -0
index 3545aed17347..1de510ea6da9 100644
@@ -57,6 +57,12 @@ struct kwspi_registers {
 #define KWSPI_TXLSBF		(1 << 13)
 #define KWSPI_RXLSBF		(1 << 14)
 
+/* Timing Parameters 1 Register */
+#define KW_SPI_TMISO_SAMPLE_OFFSET	6
+#define KW_SPI_TMISO_SAMPLE_MASK	(0x3 << KW_SPI_TMISO_SAMPLE_OFFSET)
+#define KW_SPI_TMISO_SAMPLE_1		(1 << KW_SPI_TMISO_SAMPLE_OFFSET)
+#define KW_SPI_TMISO_SAMPLE_2		(2 << KW_SPI_TMISO_SAMPLE_OFFSET)
+
 #define KWSPI_IRQUNMASK		1 /* unmask SPI interrupt */
 #define KWSPI_IRQMASK		0 /* mask SPI interrupt */
 #define KWSPI_SMEMRDIRQ		1 /* SerMem data xfer ready irq */
drivers/spi/kirkwood_spi.c +58 -3

90 lines not on this page · raw message

Signature · 1 line
2.14.2
#1 lore raw

PATCH v2 1/1 spi: kirkwood_spi: implement workaround for FE-9144572

Quoted 111 lines · Chris Packham

Erratum NO. FE-9144572: The device SPI interface supports frequencies of up to 50 MHz. However, due to this erratum, when the device core clock is 250 MHz and the SPI interfaces is configured for 50MHz SPI clock and CPOL=CPHA=1 there might occur data corruption on reads from the SPI device.

Implement the workaround by setting the TMISO_SAMPLE value to 0x2 in the timing1 register.

Signed-off-by Chris Packham <[email protected]>
Reviewed-by Stefan Roese <[email protected]>

---
I've based this as much as I can on the equivalent implementation in the Linux kernel[1], but there are differences in the u-boot spi infrastructure that means I can't be as specific when qualifying whether the workaround is needed.

[1] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/spi/spi-orion.c#n248

Changes in v2:
- collect reviewed-by from Stefan
- remove mvebu_spi_type
- describe errata above mvebu_spi_50mhz_ac_timing_erratum

 arch/arm/include/asm/arch-mvebu/spi.h |  6 ++++
 drivers/spi/kirkwood_spi.c            | 61 +++++++++++++++++++++++++++++++++--
 2 files changed, 64 insertions(+), 3 deletions(-)
arch/arm/include/asm/arch-mvebu/spi.h +6 -0
index 3545aed17347..1de510ea6da9 100644
@@ -57,6 +57,12 @@ struct kwspi_registers {
 #define KWSPI_TXLSBF           (1 << 13)
 #define KWSPI_RXLSBF           (1 << 14)
 
+/* Timing Parameters 1 Register */
+#define KW_SPI_TMISO_SAMPLE_OFFSET     6
+#define KW_SPI_TMISO_SAMPLE_MASK       (0x3 << KW_SPI_TMISO_SAMPLE_OFFSET)
+#define KW_SPI_TMISO_SAMPLE_1          (1 << KW_SPI_TMISO_SAMPLE_OFFSET)
+#define KW_SPI_TMISO_SAMPLE_2          (2 << KW_SPI_TMISO_SAMPLE_OFFSET)
+
 #define KWSPI_IRQUNMASK                1 /* unmask SPI interrupt */
 #define KWSPI_IRQMASK          0 /* mask SPI interrupt */
 #define KWSPI_SMEMRDIRQ                1 /* SerMem data xfer ready irq */
drivers/spi/kirkwood_spi.c +36 -0
index 0c6bd295cde9..5b1ac0eed634 100644
@@ -243,6 +243,10 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
 
 /* Here now the DM part */
 
+struct mvebu_spi_dev {
+       bool                    is_errata_50mhz_ac;
+};
+
 struct mvebu_spi_platdata {
        struct kwspi_registers *spireg;
 };

@@ -269,10 +273,38 @@ static int mvebu_spi_set_speed(struct udevice *bus, uint hz)
        return 0;
 }
 
+/*
+ * FE-9144572: The device SPI interface supports frequencies of up to 50 MHz.
+ * However, due to this erratum, when the device core clock is 250 MHz and the
+ * SPI interfaces is configured for 50MHz SPI clock and CPOL=CPHA=1 there might
+ * occur data corruption on reads from the SPI device.
+ *
+ * Workaround: Set the TMISO_SAMPLE value to 0x2 in the SPI Timing Parameters 1
+ * register
+ */
+static void mvebu_spi_50mhz_ac_timing_erratum(struct udevice *bus, uint mode)
+{
+       struct mvebu_spi_platdata *plat = dev_get_platdata(bus);
+       struct kwspi_registers *reg = plat->spireg;
+       u32 data = readl(&reg->timing1);
+
+       data &= ~KW_SPI_TMISO_SAMPLE_MASK;
+
+       if (CONFIG_SYS_TCLK == 250000000 &&
+           mode & SPI_CPOL &&
+           mode & SPI_CPHA)
+               data |= KW_SPI_TMISO_SAMPLE_2;
+       else
+               data |= KW_SPI_TMISO_SAMPLE_1;
+
+       writel(data, &reg->timing1);
+}
+
 static int mvebu_spi_set_mode(struct udevice *bus, uint mode)
 {
        struct mvebu_spi_platdata *plat = dev_get_platdata(bus);
        struct kwspi_registers *reg = plat->spireg;
+       const struct mvebu_spi_dev *drvdata;
        u32 data = readl(&reg->cfg);
 
        data &= ~(KWSPI_CPHA | KWSPI_CPOL | KWSPI_RXLSBF | KWSPI_TXLSBF);

@@ -286,6 +318,10 @@ static int mvebu_spi_set_mode(struct udevice *bus, uint mode)
 
        writel(data, &reg->cfg);
 
+       drvdata = (struct mvebu_spi_dev *)dev_get_driver_data(bus);
+       if (drvdata->is_errata_50mhz_ac)
+               mvebu_spi_50mhz_ac_timing_erratum(bus, mode);
+
        return 0;
 }

drivers/spi/kirkwood_spi.c static const struct dm_spi_ops mvebu_spi_ops = { · line 386

Quoted 8 lines
drivers/spi/kirkwood_spi.c +5 -0
@@ -343,10 +379,29 @@ static const struct dm_spi_ops mvebu_spi_ops = {
         */
 };
 
+static const struct mvebu_spi_dev armada_xp_spi_dev_data = {
+};
+
+static const struct mvebu_spi_dev armada_375_spi_dev_data = {
+};

Unused, may be we can drop these and add NULL or equivalent to data

thanks!

Signature · 4 lines
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
author
#2 lore raw

PATCH v2 1/1 spi: kirkwood_spi: implement workaround for FE-9144572

Quoted 115 lines · Jagan Teki
Quoted 114 lines · Chris Packham

Erratum NO. FE-9144572: The device SPI interface supports frequencies of up to 50 MHz. However, due to this erratum, when the device core clock is 250 MHz and the SPI interfaces is configured for 50MHz SPI clock and CPOL=CPHA=1 there might occur data corruption on reads from the SPI device.

Implement the workaround by setting the TMISO_SAMPLE value to 0x2 in the timing1 register.

Signed-off-by Chris Packham <[email protected]>
Reviewed-by Stefan Roese <[email protected]>

---
I've based this as much as I can on the equivalent implementation in the Linux kernel[1], but there are differences in the u-boot spi infrastructure that means I can't be as specific when qualifying whether the workaround is needed.

[1] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/spi/spi-orion.c#n248

Changes in v2:
- collect reviewed-by from Stefan
- remove mvebu_spi_type
- describe errata above mvebu_spi_50mhz_ac_timing_erratum

 arch/arm/include/asm/arch-mvebu/spi.h |  6 ++++
 drivers/spi/kirkwood_spi.c            | 61 +++++++++++++++++++++++++++++++++--
 2 files changed, 64 insertions(+), 3 deletions(-)
arch/arm/include/asm/arch-mvebu/spi.h +6 -0
index 3545aed17347..1de510ea6da9 100644
@@ -57,6 +57,12 @@ struct kwspi_registers {
 #define KWSPI_TXLSBF           (1 << 13)
 #define KWSPI_RXLSBF           (1 << 14)
 
+/* Timing Parameters 1 Register */
+#define KW_SPI_TMISO_SAMPLE_OFFSET     6
+#define KW_SPI_TMISO_SAMPLE_MASK       (0x3 << KW_SPI_TMISO_SAMPLE_OFFSET)
+#define KW_SPI_TMISO_SAMPLE_1          (1 << KW_SPI_TMISO_SAMPLE_OFFSET)
+#define KW_SPI_TMISO_SAMPLE_2          (2 << KW_SPI_TMISO_SAMPLE_OFFSET)
+
 #define KWSPI_IRQUNMASK                1 /* unmask SPI interrupt */
 #define KWSPI_IRQMASK          0 /* mask SPI interrupt */
 #define KWSPI_SMEMRDIRQ                1 /* SerMem data xfer ready irq */
drivers/spi/kirkwood_spi.c +36 -0
index 0c6bd295cde9..5b1ac0eed634 100644
@@ -243,6 +243,10 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
 
 /* Here now the DM part */
 
+struct mvebu_spi_dev {
+       bool                    is_errata_50mhz_ac;
+};
+
 struct mvebu_spi_platdata {
        struct kwspi_registers *spireg;
 };

@@ -269,10 +273,38 @@ static int mvebu_spi_set_speed(struct udevice *bus, uint hz)
        return 0;
 }
 
+/*
+ * FE-9144572: The device SPI interface supports frequencies of up to 50 MHz.
+ * However, due to this erratum, when the device core clock is 250 MHz and the
+ * SPI interfaces is configured for 50MHz SPI clock and CPOL=CPHA=1 there might
+ * occur data corruption on reads from the SPI device.
+ *
+ * Workaround: Set the TMISO_SAMPLE value to 0x2 in the SPI Timing Parameters 1
+ * register
+ */
+static void mvebu_spi_50mhz_ac_timing_erratum(struct udevice *bus, uint mode)
+{
+       struct mvebu_spi_platdata *plat = dev_get_platdata(bus);
+       struct kwspi_registers *reg = plat->spireg;
+       u32 data = readl(&reg->timing1);
+
+       data &= ~KW_SPI_TMISO_SAMPLE_MASK;
+
+       if (CONFIG_SYS_TCLK == 250000000 &&
+           mode & SPI_CPOL &&
+           mode & SPI_CPHA)
+               data |= KW_SPI_TMISO_SAMPLE_2;
+       else
+               data |= KW_SPI_TMISO_SAMPLE_1;
+
+       writel(data, &reg->timing1);
+}
+
 static int mvebu_spi_set_mode(struct udevice *bus, uint mode)
 {
        struct mvebu_spi_platdata *plat = dev_get_platdata(bus);
        struct kwspi_registers *reg = plat->spireg;
+       const struct mvebu_spi_dev *drvdata;
        u32 data = readl(&reg->cfg);
 
        data &= ~(KWSPI_CPHA | KWSPI_CPOL | KWSPI_RXLSBF | KWSPI_TXLSBF);

@@ -286,6 +318,10 @@ static int mvebu_spi_set_mode(struct udevice *bus, uint mode)
 
        writel(data, &reg->cfg);
 
+       drvdata = (struct mvebu_spi_dev *)dev_get_driver_data(bus);
+       if (drvdata->is_errata_50mhz_ac)
+               mvebu_spi_50mhz_ac_timing_erratum(bus, mode);
+
        return 0;
 }
 

@@ -343,10 +379,29 @@ static const struct dm_spi_ops mvebu_spi_ops = {
         */
 };
Quoted 8 lines

drivers/spi/kirkwood_spi.c static const struct dm_spi_ops mvebu_spi_ops = { · line 386

Quoted 5 lines
drivers/spi/kirkwood_spi.c +5 -0
+static const struct mvebu_spi_dev armada_xp_spi_dev_data = {
+};
+
+static const struct mvebu_spi_dev armada_375_spi_dev_data = {
+};

Unused, may be we can drop these and add NULL or equivalent to data

Technically I'm relying on the c99 initialiser to set is_errata_50mhz_ac to 0. I could remove them but then I'd need to add an extra check to mvebu_spi_set_mode. I could also make the is_errata_50mhz_ac = false explicit. Any particular preference?

#3 lore raw

PATCH v2 1/1 spi: kirkwood_spi: implement workaround for FE-9144572

Quoted 66 lines · Chris Packham
Quoted 65 lines · Jagan Teki
Quoted 64 lines · Chris Packham

Erratum NO. FE-9144572: The device SPI interface supports frequencies of up to 50 MHz. However, due to this erratum, when the device core clock is 250 MHz and the SPI interfaces is configured for 50MHz SPI clock and CPOL=CPHA=1 there might occur data corruption on reads from the SPI device.

Implement the workaround by setting the TMISO_SAMPLE value to 0x2 in the timing1 register.

Signed-off-by Chris Packham <[email protected]>
Reviewed-by Stefan Roese <[email protected]>

---
I've based this as much as I can on the equivalent implementation in the Linux kernel[1], but there are differences in the u-boot spi infrastructure that means I can't be as specific when qualifying whether the workaround is needed.

[1] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/spi/spi-orion.c#n248

Changes in v2:
- collect reviewed-by from Stefan
- remove mvebu_spi_type
- describe errata above mvebu_spi_50mhz_ac_timing_erratum

 arch/arm/include/asm/arch-mvebu/spi.h |  6 ++++
 drivers/spi/kirkwood_spi.c            | 61 +++++++++++++++++++++++++++++++++--
 2 files changed, 64 insertions(+), 3 deletions(-)
arch/arm/include/asm/arch-mvebu/spi.h +6 -0
index 3545aed17347..1de510ea6da9 100644
@@ -57,6 +57,12 @@ struct kwspi_registers {
 #define KWSPI_TXLSBF           (1 << 13)
 #define KWSPI_RXLSBF           (1 << 14)
 
+/* Timing Parameters 1 Register */
+#define KW_SPI_TMISO_SAMPLE_OFFSET     6
+#define KW_SPI_TMISO_SAMPLE_MASK       (0x3 << KW_SPI_TMISO_SAMPLE_OFFSET)
+#define KW_SPI_TMISO_SAMPLE_1          (1 << KW_SPI_TMISO_SAMPLE_OFFSET)
+#define KW_SPI_TMISO_SAMPLE_2          (2 << KW_SPI_TMISO_SAMPLE_OFFSET)
+
 #define KWSPI_IRQUNMASK                1 /* unmask SPI interrupt */
 #define KWSPI_IRQMASK          0 /* mask SPI interrupt */
 #define KWSPI_SMEMRDIRQ                1 /* SerMem data xfer ready irq */
drivers/spi/kirkwood_spi.c +4 -0
index 0c6bd295cde9..5b1ac0eed634 100644
@@ -243,6 +243,10 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
 
 /* Here now the DM part */
 
+struct mvebu_spi_dev {
+       bool                    is_errata_50mhz_ac;
+};
+
 struct mvebu_spi_platdata {
        struct kwspi_registers *spireg;
 };

@@ -269,10 +273,38 @@ static int mvebu_spi_set_speed(struct udevice *bus, uint hz)
        return 0;
 }
Quoted 9 lines
Quoted 9 lines

drivers/spi/kirkwood_spi.c static int mvebu_spi_set_speed(struct udevice *bus, uint hz) · line 284

Quoted 9 lines
drivers/spi/kirkwood_spi.c +9 -0
+/*
+ * FE-9144572: The device SPI interface supports frequencies of up to 50 MHz.
+ * However, due to this erratum, when the device core clock is 250 MHz and the
+ * SPI interfaces is configured for 50MHz SPI clock and CPOL=CPHA=1 there might
+ * occur data corruption on reads from the SPI device.
+ *
+ * Workaround: Set the TMISO_SAMPLE value to 0x2 in the SPI Timing Parameters 1
+ * register
+ */

Move the comments on function definition like Linux and make sure comments should be similar.

Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
drivers/spi/kirkwood_spi.c +10 -0
+static void mvebu_spi_50mhz_ac_timing_erratum(struct udevice *bus, uint mode)
+{
+       struct mvebu_spi_platdata *plat = dev_get_platdata(bus);
+       struct kwspi_registers *reg = plat->spireg;
+       u32 data = readl(&reg->timing1);
+
+       data &= ~KW_SPI_TMISO_SAMPLE_MASK;
+
+       if (CONFIG_SYS_TCLK == 250000000 &&
+           mode & SPI_CPOL &&
Quoted 8 lines
Quoted 8 lines

drivers/spi/kirkwood_spi.c static int mvebu_spi_set_speed(struct udevice *bus, uint hz) · line 302

Quoted 8 lines
drivers/spi/kirkwood_spi.c +8 -0
+           mode & SPI_CPHA)
+               data |= KW_SPI_TMISO_SAMPLE_2;
+       else
+               data |= KW_SPI_TMISO_SAMPLE_1;
+
+       writel(data, &reg->timing1);
+}
+
Reviewed-by Jagan Teki <[email protected]>