Re: [PATCH] mtd: Make MTD_BCM47XXSFLASH to depend on MIPS

From: Maciej W. Rozycki
Date: Thu Jan 07 2016 - 16:06:56 EST


On Wed, 4 Nov 2015, Brian Norris wrote:

> > > The bcm47xxsflash driver uses the KSEG0ADDR() function to map an address
> > > to a certain kernel segment. But that is only defined if the MIPS config
> > > symbol is enabled. The driver does not have an explicit dependency on it
> > > and relies on a transitive dependency relation:
> > >
> > > MTD_BCM47XXSFLASH -> BCMA_SFLASH -> BCMA_DRIVER_MIPS -> BCMA && MIPS
> > >
> > > But BCMA_SFLASH and BCMA_DRIVER_MIPS have only runtime and not buildtime
> > > dependency with MIPS so can be changed to be built test using the config
> > > COMPILE_TEST symbol. But that would make MTD_BCM47XXSFLASH be built with
> > > MIPS not enabled and cause the following build error:
> > >
> > > drivers/mtd/devices//bcm47xxsflash.c: In function 'bcm47xxsflash_read':
> > > drivers/mtd/devices//bcm47xxsflash.c:112:2: error: implicit declaration of function 'KSEG0ADDR' [-Werror=implicit-function-declaration]
> > > memcpy_fromio(buf, (void __iomem *)KSEG0ADDR(b47s->window + from),
> >
> > I think we're not really supposed to use KSEG0ADDR anyway. What about
> > replacing it with ioremap_nocache?
>
> I'm not really a MIPS expert, but isn't KSEG0 actually *cached*? (And is
> that correct, then?)
>
> AIUI, ioremap_nocache() will actually get you a KSEG1 address here, I
> think.

Depending on configuration `ioremap_nocache' may give you a KSEG1, an
XKPHYS or an uncached virtual (KSEG2) address. Drivers are not supposed
to use KSEG0ADDR, etc. macros, these are only for low-level core platform
code. Besides, KSEG0ADDR is not portable to 64-bit systems -- which may
not be an issue here, but still this means it shouldn't appear in a driver
as this causes a portability and maintenance issue.

Use plain `ioremap' instead for a cached MMIO mapping; of course you need
to place it elsewhere as at the very least you need to `iounmap' the area
mappad later on. In most cases it makes sense to handle the mapping and
unmapping in device initialisation and shutdown respectively, and then
carry the pointer obtained through and use it throughout the use of the
device.

HTH,

Maciej