On 3/31/2021 3:55 PM, Maciej W. Rozycki wrote:
> On Tue, 30 Mar 2021, Thomas Bogendoerfer wrote:
>
>>> With ath79_defconfig enabling CONFIG_MIPS_ELF_APPENDED_DTB gives a
>>> compilation error. This patch fixes it.
>>>
>>> Build log:
>>> ...
>>> CC kernel/locking/percpu-rwsem.o
>>> ../arch/mips/kernel/setup.c:46:39: error: conflicting types for
>>> '__appended_dtb'
>>> const char __section(".appended_dtb") __appended_dtb[0x100000];
>>> ^~~~~~~~~~~~~~
>>> In file included from ../arch/mips/kernel/setup.c:34:
>>> ../arch/mips/include/asm/bootinfo.h:118:13: note: previous declaration
>>> of '__appended_dtb' was here
>>> extern char __appended_dtb[];
>>> ^~~~~~~~~~~~~~
>>> CC fs/attr.o
>>> make[4]: *** [../scripts/Makefile.build:271: arch/mips/kernel/setup.o]
>>> Error 1
>>> ...
>>>
>>> Root cause seems to be:
>>> Fixes: b83ba0b9df56 ("MIPS: of: Introduce helper function to get DTB")
>>>
>>> Signed-off-by: Mauri Sandberg <sandberg@xxxxxxxxxxxxx>
>>> Reviewed-by: Thomas Bogendoerfer <tsbogend@xxxxxxxxxxxxxxxx>
>>> Tested-by: Thomas Bogendoerfer <tsbogend@xxxxxxxxxxxxxxxx>
>>> Cc: trivial@xxxxxxxxxx
>>> ---
>>> arch/mips/kernel/setup.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> applied to mips-fixes. I dropped the Tested-by as this would imply
>> for me booting that kernel, which I didn't.
>
> Why removing the `const' qualifier rather than adding it to the
> declaration? Is the DTB supposed to be r/w for the kernel?
In premise the appended dtb could be modified by the kernel early on if
needed even before being handed over to __dt_setup_arch(), so keeping it
read/write for the kernel sort of makes sense. No upstream platform
seems to do that though. If we were to re-instate the const qualifier
(only build tested) the path of least resistance appears to be this:
diff --git a/arch/mips/include/asm/bootinfo.h
b/arch/mips/include/asm/bootinfo.h
index 5be10ece3ef0..31919e1e2dc7 100644
--- a/arch/mips/include/asm/bootinfo.h
+++ b/arch/mips/include/asm/bootinfo.h
@@ -115,14 +115,14 @@ extern unsigned long fw_arg0, fw_arg1, fw_arg2,
fw_arg3;
#include <linux/libfdt.h>
#include <linux/of_fdt.h>
-extern char __appended_dtb[];
+extern const char __appended_dtb[];
static inline void *get_fdt(void)
{
if (IS_ENABLED(CONFIG_MIPS_RAW_APPENDED_DTB) ||
IS_ENABLED(CONFIG_MIPS_ELF_APPENDED_DTB))
if (fdt_magic(&__appended_dtb) == FDT_MAGIC)
- return &__appended_dtb;
+ return (void *)&__appended_dtb;
if (fw_arg0 == -2) /* UHI interface */
return (void *)fw_arg1;
diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 23a140327a0b..279be0153f8b 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -43,7 +43,7 @@
#include <asm/prom.h>
#ifdef CONFIG_MIPS_ELF_APPENDED_DTB
-char __section(".appended_dtb") __appended_dtb[0x100000];
+const char __section(".appended_dtb") __appended_dtb[0x100000];
#endif /* CONFIG_MIPS_ELF_APPENDED_DTB */
struct cpuinfo_mips cpu_data[NR_CPUS] __read_mostly;
--
Florian
