Messages in this thread | | | From | Rafał Miłecki <> | Date | Tue, 8 Nov 2016 11:05:44 +0100 | Subject | Re: [PATCH] mtd: bcm47xxsflash: use uncached MMIO access for BCM53573 |
| |
On 8 November 2016 at 03:23, Brian Norris <computersforpeace@gmail.com> wrote: > On Mon, Aug 15, 2016 at 02:21:28PM +0200, Rafał Miłecki wrote: >> From: Rafał Miłecki <rafal@milecki.pl> >> >> BCM53573 is a new series of Broadcom's SoCs. It's based on ARM and uses >> this old ChipCommon-based flash access. Early tests resulted in flash >> corruptions that were tracked down to using cached MMIO for flash read >> access. Switch to ioremap_nocache conditionally to support BCM53573 and >> don't break performance on old MIPS devices. >> >> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> >> --- >> drivers/mtd/devices/bcm47xxsflash.c | 24 +++++++++++++++++++----- >> 1 file changed, 19 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/mtd/devices/bcm47xxsflash.c b/drivers/mtd/devices/bcm47xxsflash.c >> index 1c65c15..514be04 100644 >> --- a/drivers/mtd/devices/bcm47xxsflash.c >> +++ b/drivers/mtd/devices/bcm47xxsflash.c >> @@ -296,16 +296,30 @@ static int bcm47xxsflash_bcma_probe(struct platform_device *pdev) >> dev_err(dev, "can't request region for resource %pR\n", res); >> return -EBUSY; >> } >> - b47s->window = ioremap_cache(res->start, resource_size(res)); >> - if (!b47s->window) { >> - dev_err(dev, "ioremap failed for resource %pR\n", res); >> - return -ENOMEM; >> - } >> >> b47s->bcma_cc = container_of(sflash, struct bcma_drv_cc, sflash); >> b47s->cc_read = bcm47xxsflash_bcma_cc_read; >> b47s->cc_write = bcm47xxsflash_bcma_cc_write; >> >> + /* >> + * On old MIPS devices cache was magically invalidated when needed, >> + * allowing us to use cached access and gain some performance. Trying >> + * the same on ARM based BCM53573 results in flash corruptions, we need >> + * to use uncached access for it. > > Is the word "magically" really used in the Broadcom reference code? I > wouldn't be too suprised actually :) > > I'd prefer getting a better explanation on this point, but without that, > I think it's fair to leave the caching for the old (working) MIPS SoCs > and avoid caching on the new ARM one.
I have to admit "magic" word was my choice ;) I guess Broadcom ppl know more about this, but they didn't release any datasheets/programming guide and they didn't document it well in the code.
Reference code can be found in Broadcom's SDK in the ccsflash.c.
1) ccsflash_init does: ccsflash.base = (uint32)REG_MAP(ccsflash.phybase, ccsflash.size); (and REG_MAP is a macro for ioremap_nocache)
2) ccsflash_read does: if (sih->ccrev == 12) { from = (uint8 *)OSL_UNCACHED((void *)SI_FLASH2 + offset); } else if (sih->ccrev == 54) { from = (uint8 *)((void *)sfl->base + offset); } else { from = (uint8 *)OSL_CACHED((void *)SI_FLASH2 + offset); }
I'm not aware of any market-released device with ChipCommon rev 12, so I check for rev 54 only.
>> + * It may be arch specific, but right now there is only 1 ARM SoC using >> + * this driver, so let's follow Broadcom's reference code and check >> + * ChipCommon revision. > > Yeah, if we get any more ARM chips that behave similarly, I think we'll > just want to do 'if !MIPS'. > > Applied to l2-mtd.git.
Thank you!
| |