Re: [PATCH V2] mtd: bcm47xxsflash: use ioremap_cache() instead of KSEG0ADDR()
From: Brian Norris
Date: Mon Apr 04 2016 - 03:10:14 EST
On Fri, Mar 18, 2016 at 12:22:05PM +0100, Ard Biesheuvel wrote:
> On 5 March 2016 at 01:54, Brian Norris <computersforpeace@xxxxxxxxx> wrote:
> > + others
> >
> > On Fri, Feb 26, 2016 at 11:50:28AM +0100, RafaÅ MiÅecki wrote:
> >> From: Brian Norris <computersforpeace@xxxxxxxxx>
> >>
> >> Using KSEG0ADDR makes code highly MIPS dependent and not portable.
> >> Thanks to the fix a68f376 ("MIPS: io.h: Define `ioremap_cache'") we can
> >> use ioremap_cache which is generic and supported on MIPS as well now.
> >>
> >> KSEG0ADDR was translating 0x1c000000 into 0x9c000000. With ioremap_cache
> >> we use MIPS's __ioremap (and then 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.
> >>
> >> Other than that drivers/bcma/driver_chipcommon_sflash.c nicely setups a
> >> struct resource for access window, but we wren't using it. Use it now
> >> and drop duplicated info.
> >>
> >> Signed-off-by: Brian Norris <computersforpeace@xxxxxxxxx>
> >> Signed-off-by: RafaÅ MiÅecki <zajec5@xxxxxxxxx>
> >> ---
> >> V2:
> >> 1) Use ioremap_cache as we already have commit a68f376 in l2-mtd
> >> 2) Add iounmap to the error path
> >> 3) Update commit message
> >
> > Hmm, I'm a bit out of the loop on some things here, but it looks like
> > ioremap_cache() is going away. See kernel/memremap.c:
> >
> > /* temporary while we convert existing ioremap_cache users to memremap */
> > __weak void __iomem *ioremap_cache(resource_size_t offset, unsigned long size)
> >
> > Are we supposed to move to using memremap() here? (If so,
> > devm_memremap() can help us.)
> >
>
> I don't think that ioremap_cache() necessarily has to go away, but
> what we do need to address is its abuse to map things like firmware
> tables that reside in normal memory that may not be owned/tracked by
> the kernel. So for that case, we now have memremap() which should just
> give you a mapping of the requested region, either via the linear
> mapping if it happens to be mapped already, or via the vmalloc region
> if it is highmem or otherwise unavailable directly.
>
> However, memory mapped NOR flash remains a special case, since the
> __iomem annotation may or may not be appropriate depending on context
> (i.e., reads and writes may both have either memory or strongly
> ordered semantics)
OK, pushed to l2-mtd.git.