| Message ID | 5158260.vK2YuWzYIn@wuerfel |
|---|---|
| State | Superseded |
| Headers | show |
Hi Arnd, On Sat, Mar 05, 2016 at 01:19:21AM +0100, Arnd Bergmann wrote: > On Friday 04 March 2016 16:02:25 Brian Norris wrote: > > On Mon, Jan 25, 2016 at 04:41:50PM +0100, Arnd Bergmann wrote: > > > When XIP_KERNEL is enabled, some functions are defined in the .data > > > ELF section because we require them to be in RAM whenever we communicate > > > with the flash chip. However this causes problems when FTRACE is > > > enabled and gcc emits calls to __gnu_mcount_nc in the function > > > prolog: > > > > > > drivers/built-in.o: In function `cfi_chip_setup': > > > :(.data+0x272fc): relocation truncated to fit: R_ARM_CALL against symbol `__gnu_mcount_nc' defined in .text section in arch/arm/kernel/built-in.o > > > drivers/built-in.o: In function `cfi_probe_chip': > > > :(.data+0x27de8): relocation truncated to fit: R_ARM_CALL against symbol `__gnu_mcount_nc' defined in .text section in arch/arm/kernel/built-in.o > > > /tmp/ccY172rP.s: Assembler messages: > > > /tmp/ccY172rP.s:70: Warning: ignoring changed section attributes for .data > > > /tmp/ccY172rP.s: Error: 1 warning, treating warnings as errors > > > make[5]: *** [drivers/mtd/chips/cfi_probe.o] Error 1 > > > /tmp/ccK4rjeO.s: Assembler messages: > > > /tmp/ccK4rjeO.s:421: Warning: ignoring changed section attributes for .data > > > /tmp/ccK4rjeO.s: Error: 1 warning, treating warnings as errors > > > make[5]: *** [drivers/mtd/chips/cfi_util.o] Error 1 > > > /tmp/ccUvhCYR.s: Assembler messages: > > > /tmp/ccUvhCYR.s:1895: Warning: ignoring changed section attributes for .data > > > /tmp/ccUvhCYR.s: Error: 1 warning, treating warnings as errors > > > > Can you provide a sample .config that DOES build correctly with > > XIP_KERNEL enabled + this patch? My first attempt yields some other > > failures I don't care to fixup right now... > > > > Anyway, I don't doubt you have a good fix here, so I can probably take > > it. Any review from others would be welcome though. > > I found the config in the attachment in my logs. Thanks... > To get this config working, I also needed this hunk from my set of > old unsubmitted patches: ...but, does anyone care about XIP / MTD_XIP then, if the first two examples we have both have long-standing build issues? Regards, Brian > diff --git a/arch/arm/mach-sa1100/include/mach/mtd-xip.h b/arch/arm/mach-sa1100/include/mach/mtd-xip.h > index b3d684098fbf..cb76096a2e36 100644 > --- a/arch/arm/mach-sa1100/include/mach/mtd-xip.h > +++ b/arch/arm/mach-sa1100/include/mach/mtd-xip.h > @@ -20,7 +20,7 @@ > #define xip_irqpending() (ICIP & ICMR) > > /* we sample OSCR and convert desired delta to usec (1/4 ~= 1000000/3686400) */ > -#define xip_currtime() (OSCR) > -#define xip_elapsed_since(x) (signed)((OSCR - (x)) / 4) > +#define xip_currtime() readl_relaxed(OSCR) > +#define xip_elapsed_since(x) (signed)((readl_relaxed(OSCR) - (x)) / 4) > > #endif /* __ARCH_SA1100_MTD_XIP_H__ */ > > Arnd
On Fri, 2016-03-04 at 16:22 -0800, Brian Norris wrote: > > ...but, does anyone care about XIP / MTD_XIP then, if the first two > examples we have both have long-standing build issues? I think there are people trying to make it work on other platforms, yes.
On Friday 04 March 2016 16:22:03 Brian Norris wrote: > Hi Arnd, > > On Sat, Mar 05, 2016 at 01:19:21AM +0100, Arnd Bergmann wrote: > > On Friday 04 March 2016 16:02:25 Brian Norris wrote: > > > On Mon, Jan 25, 2016 at 04:41:50PM +0100, Arnd Bergmann wrote: > > > > When XIP_KERNEL is enabled, some functions are defined in the .data > > > > ELF section because we require them to be in RAM whenever we communicate > > > > with the flash chip. However this causes problems when FTRACE is > > > > enabled and gcc emits calls to __gnu_mcount_nc in the function > > > > prolog: > > > > > > > > drivers/built-in.o: In function `cfi_chip_setup': > > > > :(.data+0x272fc): relocation truncated to fit: R_ARM_CALL against symbol `__gnu_mcount_nc' defined in .text section in arch/arm/kernel/built-in.o > > > > drivers/built-in.o: In function `cfi_probe_chip': > > > > :(.data+0x27de8): relocation truncated to fit: R_ARM_CALL against symbol `__gnu_mcount_nc' defined in .text section in arch/arm/kernel/built-in.o > > > > /tmp/ccY172rP.s: Assembler messages: > > > > /tmp/ccY172rP.s:70: Warning: ignoring changed section attributes for .data > > > > /tmp/ccY172rP.s: Error: 1 warning, treating warnings as errors > > > > make[5]: *** [drivers/mtd/chips/cfi_probe.o] Error 1 > > > > /tmp/ccK4rjeO.s: Assembler messages: > > > > /tmp/ccK4rjeO.s:421: Warning: ignoring changed section attributes for .data > > > > /tmp/ccK4rjeO.s: Error: 1 warning, treating warnings as errors > > > > make[5]: *** [drivers/mtd/chips/cfi_util.o] Error 1 > > > > /tmp/ccUvhCYR.s: Assembler messages: > > > > /tmp/ccUvhCYR.s:1895: Warning: ignoring changed section attributes for .data > > > > /tmp/ccUvhCYR.s: Error: 1 warning, treating warnings as errors > > > > > > Can you provide a sample .config that DOES build correctly with > > > XIP_KERNEL enabled + this patch? My first attempt yields some other > > > failures I don't care to fixup right now... > > > > > > Anyway, I don't doubt you have a good fix here, so I can probably take > > > it. Any review from others would be welcome though. > > > > I found the config in the attachment in my logs. > > Thanks... > > > To get this config working, I also needed this hunk from my set of > > old unsubmitted patches: > > ...but, does anyone care about XIP / MTD_XIP then, if the first two > examples we have both have long-standing build issues? > Probably not. I just checked the third user (omap1), and it seems that one has been broken since 2009 with 941132606c76 ("OMAP: Remove OMAP_IO_ADDRESS, use OMAP1_IO_ADDRESS and OMAP2_IO_ADDRESS instead"), when Tony replaced the inline omap_readl() with an extern function, thus breaking the implementation of xip_irqpending() that must be inlined. The other two have apparently been broken since 2011, by other patches that also broke compilation. Arnd
On Sat, Mar 05, 2016 at 12:28:38AM +0000, David Woodhouse wrote: > On Fri, 2016-03-04 at 16:22 -0800, Brian Norris wrote: > > > > ...but, does anyone care about XIP / MTD_XIP then, if the first two > > examples we have both have long-standing build issues? > > I think there are people trying to make it work on other platforms, > yes. OK... can we kill the broken platforms though? I'm not confident that my and Arnd's workaround attempts will yield anything more than a pat on our own backs to say "yay, it compiles!" And given the number of practically unused things we're already maintaining in MTD, I'm always happy for an excuse to kill off some of it. Brian
* Arnd Bergmann <arnd@arndb.de> [160304 16:34]: > On Friday 04 March 2016 16:22:03 Brian Norris wrote: > > Hi Arnd, > > > > On Sat, Mar 05, 2016 at 01:19:21AM +0100, Arnd Bergmann wrote: > > > On Friday 04 March 2016 16:02:25 Brian Norris wrote: > > > > On Mon, Jan 25, 2016 at 04:41:50PM +0100, Arnd Bergmann wrote: > > > > > When XIP_KERNEL is enabled, some functions are defined in the .data > > > > > ELF section because we require them to be in RAM whenever we communicate > > > > > with the flash chip. However this causes problems when FTRACE is > > > > > enabled and gcc emits calls to __gnu_mcount_nc in the function > > > > > prolog: > > > > > > > > > > drivers/built-in.o: In function `cfi_chip_setup': > > > > > :(.data+0x272fc): relocation truncated to fit: R_ARM_CALL against symbol `__gnu_mcount_nc' defined in .text section in arch/arm/kernel/built-in.o > > > > > drivers/built-in.o: In function `cfi_probe_chip': > > > > > :(.data+0x27de8): relocation truncated to fit: R_ARM_CALL against symbol `__gnu_mcount_nc' defined in .text section in arch/arm/kernel/built-in.o > > > > > /tmp/ccY172rP.s: Assembler messages: > > > > > /tmp/ccY172rP.s:70: Warning: ignoring changed section attributes for .data > > > > > /tmp/ccY172rP.s: Error: 1 warning, treating warnings as errors > > > > > make[5]: *** [drivers/mtd/chips/cfi_probe.o] Error 1 > > > > > /tmp/ccK4rjeO.s: Assembler messages: > > > > > /tmp/ccK4rjeO.s:421: Warning: ignoring changed section attributes for .data > > > > > /tmp/ccK4rjeO.s: Error: 1 warning, treating warnings as errors > > > > > make[5]: *** [drivers/mtd/chips/cfi_util.o] Error 1 > > > > > /tmp/ccUvhCYR.s: Assembler messages: > > > > > /tmp/ccUvhCYR.s:1895: Warning: ignoring changed section attributes for .data > > > > > /tmp/ccUvhCYR.s: Error: 1 warning, treating warnings as errors > > > > > > > > Can you provide a sample .config that DOES build correctly with > > > > XIP_KERNEL enabled + this patch? My first attempt yields some other > > > > failures I don't care to fixup right now... > > > > > > > > Anyway, I don't doubt you have a good fix here, so I can probably take > > > > it. Any review from others would be welcome though. > > > > > > I found the config in the attachment in my logs. > > > > Thanks... > > > > > To get this config working, I also needed this hunk from my set of > > > old unsubmitted patches: > > > > ...but, does anyone care about XIP / MTD_XIP then, if the first two > > examples we have both have long-standing build issues? > > > > Probably not. I just checked the third user (omap1), and it seems that one > has been broken since 2009 with 941132606c76 ("OMAP: Remove OMAP_IO_ADDRESS, > use OMAP1_IO_ADDRESS and OMAP2_IO_ADDRESS instead"), when Tony replaced > the inline omap_readl() with an extern function, thus breaking the > implementation of xip_irqpending() that must be inlined. > > The other two have apparently been broken since 2011, by other patches > that also broke compilation. For omaps, I don't recall anybody working on xip for probably close to 10 years. Regards, Tony
On Friday 04 March 2016 16:43:36 Brian Norris wrote: > On Sat, Mar 05, 2016 at 12:28:38AM +0000, David Woodhouse wrote: > > On Fri, 2016-03-04 at 16:22 -0800, Brian Norris wrote: > > > > > > ...but, does anyone care about XIP / MTD_XIP then, if the first two > > > examples we have both have long-standing build issues? > > > > I think there are people trying to make it work on other platforms, > > yes. > > OK... can we kill the broken platforms though? I'm not confident that my > and Arnd's workaround attempts will yield anything more than a pat on > our own backs to say "yay, it compiles!" And given the number of > practically unused things we're already maintaining in MTD, I'm always > happy for an excuse to kill off some of it. Sorry for the late reply. As the three platforms that use this have all been broken for around ten years, it seems reasonable to assume nobody is going to miss the current implementation, but I'd leave it up to the individual platform maintainers. If we remove all three, that would also give us more flexibility regarding the interface definition for any future users of the feature. I think the most likely to use it is the Renesas RZ/A1H platform with its 10MB of on-chip SRAM, and this one doesn't work with the current method anyway. Arnd
diff --git a/arch/arm/mach-sa1100/include/mach/mtd-xip.h b/arch/arm/mach-sa1100/include/mach/mtd-xip.h index b3d684098fbf..cb76096a2e36 100644 --- a/arch/arm/mach-sa1100/include/mach/mtd-xip.h +++ b/arch/arm/mach-sa1100/include/mach/mtd-xip.h @@ -20,7 +20,7 @@ #define xip_irqpending() (ICIP & ICMR) /* we sample OSCR and convert desired delta to usec (1/4 ~= 1000000/3686400) */ -#define xip_currtime() (OSCR) -#define xip_elapsed_since(x) (signed)((OSCR - (x)) / 4) +#define xip_currtime() readl_relaxed(OSCR) +#define xip_elapsed_since(x) (signed)((readl_relaxed(OSCR) - (x)) / 4) #endif /* __ARCH_SA1100_MTD_XIP_H__ */