Message ID | 20171016213726.28555-1-judge.packham@gmail.com |
---|---|
State | Superseded |
Delegated to: | Prafulla Wadaskar |
Headers | show |
Series | [U-Boot,v2] spi: kirkwood_spi: implement workaround for FE-9144572 | expand |
On Tue, Oct 17, 2017 at 3:07 AM, Chris Packham <judge.packham@gmail.com> wrote: > 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 <judge.packham@gmail.com> > Reviewed-by: Stefan Roese <sr@denx.de> > --- > 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(-) > > diff --git a/arch/arm/include/asm/arch-mvebu/spi.h b/arch/arm/include/asm/arch-mvebu/spi.h > index 3545aed17347..1de510ea6da9 100644 > --- a/arch/arm/include/asm/arch-mvebu/spi.h > +++ b/arch/arm/include/asm/arch-mvebu/spi.h > @@ -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 */ > diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c > index 0c6bd295cde9..5b1ac0eed634 100644 > --- a/drivers/spi/kirkwood_spi.c > +++ b/drivers/spi/kirkwood_spi.c > @@ -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(®->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, ®->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(®->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, ®->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 = { > */ > }; > > +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!
On Tue, Oct 17, 2017 at 10:46 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: > On Tue, Oct 17, 2017 at 3:07 AM, Chris Packham <judge.packham@gmail.com> wrote: >> 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 <judge.packham@gmail.com> >> Reviewed-by: Stefan Roese <sr@denx.de> >> --- >> 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(-) >> >> diff --git a/arch/arm/include/asm/arch-mvebu/spi.h b/arch/arm/include/asm/arch-mvebu/spi.h >> index 3545aed17347..1de510ea6da9 100644 >> --- a/arch/arm/include/asm/arch-mvebu/spi.h >> +++ b/arch/arm/include/asm/arch-mvebu/spi.h >> @@ -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 */ >> diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c >> index 0c6bd295cde9..5b1ac0eed634 100644 >> --- a/drivers/spi/kirkwood_spi.c >> +++ b/drivers/spi/kirkwood_spi.c >> @@ -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(®->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, ®->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(®->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, ®->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 = { >> */ >> }; >> >> +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?
On Tue, Oct 17, 2017 at 4:15 AM, Chris Packham <judge.packham@gmail.com> wrote: > On Tue, Oct 17, 2017 at 10:46 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: >> On Tue, Oct 17, 2017 at 3:07 AM, Chris Packham <judge.packham@gmail.com> wrote: >>> 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 <judge.packham@gmail.com> >>> Reviewed-by: Stefan Roese <sr@denx.de> >>> --- >>> 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(-) >>> >>> diff --git a/arch/arm/include/asm/arch-mvebu/spi.h b/arch/arm/include/asm/arch-mvebu/spi.h >>> index 3545aed17347..1de510ea6da9 100644 >>> --- a/arch/arm/include/asm/arch-mvebu/spi.h >>> +++ b/arch/arm/include/asm/arch-mvebu/spi.h >>> @@ -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 */ >>> diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c >>> index 0c6bd295cde9..5b1ac0eed634 100644 >>> --- a/drivers/spi/kirkwood_spi.c >>> +++ b/drivers/spi/kirkwood_spi.c >>> @@ -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 >>> + */ Move the comments on function definition like Linux and make sure comments should be similar. >>> +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(®->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, ®->timing1); >>> +} >>> + Reviewed-by: Jagan Teki <jagan@openedev.com>
diff --git a/arch/arm/include/asm/arch-mvebu/spi.h b/arch/arm/include/asm/arch-mvebu/spi.h index 3545aed17347..1de510ea6da9 100644 --- a/arch/arm/include/asm/arch-mvebu/spi.h +++ b/arch/arm/include/asm/arch-mvebu/spi.h @@ -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 */ diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c index 0c6bd295cde9..5b1ac0eed634 100644 --- a/drivers/spi/kirkwood_spi.c +++ b/drivers/spi/kirkwood_spi.c @@ -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(®->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, ®->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(®->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, ®->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 = { */ }; +static const struct mvebu_spi_dev armada_xp_spi_dev_data = { +}; + +static const struct mvebu_spi_dev armada_375_spi_dev_data = { +}; + +static const struct mvebu_spi_dev armada_380_spi_dev_data = { + .is_errata_50mhz_ac = true, +}; + static const struct udevice_id mvebu_spi_ids[] = { - { .compatible = "marvell,armada-375-spi" }, - { .compatible = "marvell,armada-380-spi" }, - { .compatible = "marvell,armada-xp-spi" }, + { + .compatible = "marvell,armada-375-spi", + .data = (ulong)&armada_375_spi_dev_data + }, + { + .compatible = "marvell,armada-380-spi", + .data = (ulong)&armada_380_spi_dev_data + }, + { + .compatible = "marvell,armada-xp-spi", + .data = (ulong)&armada_xp_spi_dev_data + }, { } };