Re: [PATCH] mtd: bcm47xxsflash: use ioremap_cachable() instead of KSEG0ADDR()

From: Maciej W. Rozycki
Date: Fri Feb 26 2016 - 09:34:57 EST


On Fri, 26 Feb 2016, RafaÅ MiÅecki wrote:

> >> KSEG0ADDR was translating 0x1c000000 into 0x9c000000. With
> >> ioremap_cachable we use MIPS's __ioremap (and remap_area_pages). This
> >> results in different address (e.g. 0xc0080000) but it still should be
> >> cached as expected and it was successfully tested with BCM47186B0.
> >
> > This is due to this piece:
> >
> > /*
> > * Map uncached objects in the low 512mb of address space using KSEG1,
> > * otherwise map using page tables.
> > */
> > if (IS_LOW512(phys_addr) && IS_LOW512(last_addr) &&
> > flags == _CACHE_UNCACHED)
> > return (void __iomem *) CKSEG1ADDR(phys_addr);
> >
> > special-casing uncached mapping only (replicated in 2 places). I think
> > there will really be no harm from returning a KSEG0 mapping for calls
> > requesting a caching mode equal to `_page_cachable_default', which --
> > depending on the cache architecture -- will have been either hardwired or
> > prearranged via Config.K0. I think there's really no need to put pressure
> > on the TLB, which may be small, in cases where a fixed mapping will do.
>
> No, it isn't hitting condition you pointed. We call ioremap_cachable
> which uses _page_cachable_default as a flag. This flag
> (_page_cachable_default) isn't equal to the _CACHE_UNCACHED.

That's exactly what I wrote: code I quoted is "special-casing uncached
mapping only" -- which as you have correctly observed does not apply after
your change anymore. Which is why previously you got an address in the
unmapped KSEG1 segment and now you get an address in the mapped KSEG2
rather than the unmapped KSEG0 segment.

> Moreover code you pointed uses CKSEG1ADDR which would result in
> setting bit KSEG1 (0xa0000000). As I pointed in the commit message
> address it ORed with KSEG2 (0xc0000000).

It's not merely ORed, it's actually mapped via the TLB.

I hope this makes things clear.

Maciej