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

[Maciej W. Rozycki]

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