Message ID | 1444813494-14985-1-git-send-email-javier@osg.samsung.com |
---|---|
State | Superseded |
Headers | show |
+ Rafał On Wed, Oct 14, 2015 at 11:04:54AM +0200, Javier Martinez Canillas 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), > ^ > > Make MTD_BCM47XXSFLASH depend on MIPS since has a buildtime dependency. > > Reported-by: Fengguang Wu <fengguang.wu@intel.com> > Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> > > --- > > drivers/mtd/devices/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/devices/Kconfig b/drivers/mtd/devices/Kconfig > index f73c41697a00..f5eab3c19356 100644 > --- a/drivers/mtd/devices/Kconfig > +++ b/drivers/mtd/devices/Kconfig > @@ -114,7 +114,7 @@ config MTD_SST25L > > config MTD_BCM47XXSFLASH > tristate "R/O support for serial flash on BCMA bus" > - depends on BCMA_SFLASH > + depends on BCMA_SFLASH && MIPS > help > BCMA bus can have various flash memories attached, they are > registered by bcma as platform devices. This enables driver for > -- > 2.4.3 >
On 14 October 2015 at 11:04, Javier Martinez Canillas <javier@osg.samsung.com> 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? Sorry for the late reply.
On Wed, Nov 04, 2015 at 10:04:26AM +0100, Rafał Miłecki wrote: > On 14 October 2015 at 11:04, Javier Martinez Canillas > <javier@osg.samsung.com> 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. Also (a bit of a tangent) couldn't "window" be better passed as a second resource by drivers/bcma/driver_chipcommon_sflash.c? Seems like that would fit the device/resource model better, and then you wouldn't have to do any __iomem casts in bcm47xxsflash.c. Brian
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
diff --git a/drivers/mtd/devices/Kconfig b/drivers/mtd/devices/Kconfig index f73c41697a00..f5eab3c19356 100644 --- a/drivers/mtd/devices/Kconfig +++ b/drivers/mtd/devices/Kconfig @@ -114,7 +114,7 @@ config MTD_SST25L config MTD_BCM47XXSFLASH tristate "R/O support for serial flash on BCMA bus" - depends on BCMA_SFLASH + depends on BCMA_SFLASH && MIPS help BCMA bus can have various flash memories attached, they are registered by bcma as platform devices. This enables driver for
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), ^ Make MTD_BCM47XXSFLASH depend on MIPS since has a buildtime dependency. Reported-by: Fengguang Wu <fengguang.wu@intel.com> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> --- drivers/mtd/devices/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)