linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] riscv: resolve most warnings from sparse
@ 2019-09-19  8:26 Paul Walmsley
  2019-09-19 17:31 ` Christoph Hellwig
  0 siblings, 1 reply; 4+ messages in thread
From: Paul Walmsley @ 2019-09-19  8:26 UTC (permalink / raw)
  To: linux-riscv; +Cc: linux-kernel


Resolve most of the warnings emitted by sparse.  The objective here is
to keep arch/riscv as clean as possible with regards to sparse warnings,
and to maintain this bar for subsequent patches.

Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
---
 arch/riscv/include/asm/entry.h       | 29 ++++++++++++++++++++++++++++
 arch/riscv/include/asm/head.h        | 21 ++++++++++++++++++++
 arch/riscv/include/asm/irq.h         |  6 ++++++
 arch/riscv/include/asm/pgtable.h     |  3 +++
 arch/riscv/include/asm/processor.h   |  5 +++++
 arch/riscv/include/asm/ptrace.h      |  2 ++
 arch/riscv/include/asm/smp.h         |  2 ++
 arch/riscv/include/asm/switch_to.h   |  1 +
 arch/riscv/include/asm/syscall.h     |  3 +++
 arch/riscv/include/asm/thread_info.h |  2 ++
 arch/riscv/kernel/cpufeature.c       |  1 +
 arch/riscv/kernel/module-sections.c  |  1 +
 arch/riscv/kernel/process.c          |  2 ++
 arch/riscv/kernel/reset.c            |  1 +
 arch/riscv/kernel/setup.c            |  1 +
 arch/riscv/kernel/signal.c           |  1 +
 arch/riscv/kernel/smp.c              |  4 ++++
 arch/riscv/kernel/smpboot.c          |  2 ++
 arch/riscv/kernel/stacktrace.c       |  4 ++--
 arch/riscv/kernel/syscall_table.c    |  1 +
 arch/riscv/kernel/time.c             |  2 +-
 arch/riscv/kernel/traps.c            |  1 +
 arch/riscv/kernel/vdso.c             |  3 ++-
 arch/riscv/mm/context.c              |  1 +
 arch/riscv/mm/fault.c                |  1 +
 arch/riscv/mm/init.c                 | 16 +++++++--------
 arch/riscv/mm/sifive_l2_cache.c      |  2 +-
 27 files changed, 105 insertions(+), 13 deletions(-)
 create mode 100644 arch/riscv/include/asm/entry.h
 create mode 100644 arch/riscv/include/asm/head.h

diff --git a/arch/riscv/include/asm/entry.h b/arch/riscv/include/asm/entry.h
new file mode 100644
index 000000000000..73bfcda993d0
--- /dev/null
+++ b/arch/riscv/include/asm/entry.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 SiFive, Inc.
+ */
+#ifndef __ASM_ENTRY_H
+#define __ASM_ENTRY_H
+
+#include <linux/linkage.h>
+#include <linux/init.h>
+
+asmlinkage void do_trap_unknown(struct pt_regs *regs);
+asmlinkage void do_trap_insn_misaligned(struct pt_regs *regs);
+asmlinkage void do_trap_insn_fault(struct pt_regs *regs);
+asmlinkage void do_trap_insn_illegal(struct pt_regs *regs);
+asmlinkage void do_trap_load_misaligned(struct pt_regs *regs);
+asmlinkage void do_trap_load_fault(struct pt_regs *regs);
+asmlinkage void do_trap_store_misaligned(struct pt_regs *regs);
+asmlinkage void do_trap_store_fault(struct pt_regs *regs);
+asmlinkage void do_trap_ecall_u(struct pt_regs *regs);
+asmlinkage void do_trap_ecall_s(struct pt_regs *regs);
+asmlinkage void do_trap_ecall_m(struct pt_regs *regs);
+asmlinkage void do_trap_break(struct pt_regs *regs);
+
+asmlinkage void do_notify_resume(struct pt_regs *regs,
+				 unsigned long thread_info_flags);
+
+void __init trap_init(void);
+
+#endif /* __ASM__H */
diff --git a/arch/riscv/include/asm/head.h b/arch/riscv/include/asm/head.h
new file mode 100644
index 000000000000..105fb0496b24
--- /dev/null
+++ b/arch/riscv/include/asm/head.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 SiFive, Inc.
+ */
+#ifndef __ASM_HEAD_H
+#define __ASM_HEAD_H
+
+#include <linux/linkage.h>
+#include <linux/init.h>
+
+extern atomic_t hart_lottery;
+
+asmlinkage void do_page_fault(struct pt_regs *regs);
+asmlinkage void __init setup_vm(uintptr_t dtb_pa);
+
+extern void *__cpu_up_stack_pointer[];
+extern void *__cpu_up_task_pointer[];
+
+void __init parse_dtb(void);
+
+#endif /* __ASM_HEAD_H */
diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h
index 75576424c0f7..f0e9df6e6049 100644
--- a/arch/riscv/include/asm/irq.h
+++ b/arch/riscv/include/asm/irq.h
@@ -7,11 +7,17 @@
 #ifndef _ASM_RISCV_IRQ_H
 #define _ASM_RISCV_IRQ_H
 
+#include <linux/interrupt.h>
+#include <linux/linkage.h>
+
 #define NR_IRQS         0
 
 void riscv_timer_interrupt(void);
 void riscv_software_interrupt(void);
 
+asmlinkage void do_IRQ(struct pt_regs *regs);
+void __init init_IRQ(void);
+
 #include <asm-generic/irq.h>
 
 #endif /* _ASM_RISCV_IRQ_H */
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 4f4162d90586..1be4f70ab266 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -61,6 +61,9 @@
 
 #define PAGE_TABLE		__pgprot(_PAGE_TABLE)
 
+extern pgd_t swapper_pg_dir[];
+extern pgd_t trampoline_pg_dir[];
+extern pgd_t early_pg_dir[];
 extern pgd_t swapper_pg_dir[];
 
 /* MAP_PRIVATE permissions: xwr (copy-on-write) */
diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index f539149d04c2..f4fb93a2f282 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -9,6 +9,7 @@
 #include <linux/const.h>
 
 #include <asm/ptrace.h>
+#include <asm/processor.h>
 
 /*
  * This decides where the kernel will search for a free chunk of vm
@@ -78,6 +79,10 @@ int riscv_of_processor_hartid(struct device_node *node);
 
 extern void riscv_fill_hwcap(void);
 
+extern const struct seq_operations cpuinfo_op;
+
+void time_init(void);
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_RISCV_PROCESSOR_H */
diff --git a/arch/riscv/include/asm/ptrace.h b/arch/riscv/include/asm/ptrace.h
index d48d1e13973c..7efd655acd55 100644
--- a/arch/riscv/include/asm/ptrace.h
+++ b/arch/riscv/include/asm/ptrace.h
@@ -101,6 +101,8 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
 	return regs->a0;
 }
 
+void show_regs(struct pt_regs *);
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_RISCV_PTRACE_H */
diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index a83451d73a4e..e8e44360af85 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -15,6 +15,8 @@
 struct seq_file;
 extern unsigned long boot_cpu_hartid;
 
+extern asmlinkage void __init smp_callin(void);
+
 #ifdef CONFIG_SMP
 /*
  * Mapping between linux logical cpu index and hartid.
diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
index f0227bdce0f0..ee4f0ac62c9d 100644
--- a/arch/riscv/include/asm/switch_to.h
+++ b/arch/riscv/include/asm/switch_to.h
@@ -6,6 +6,7 @@
 #ifndef _ASM_RISCV_SWITCH_TO_H
 #define _ASM_RISCV_SWITCH_TO_H
 
+#include <linux/sched/task_stack.h>
 #include <asm/processor.h>
 #include <asm/ptrace.h>
 #include <asm/csr.h>
diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
index 42347d0981e7..640a88918632 100644
--- a/arch/riscv/include/asm/syscall.h
+++ b/arch/riscv/include/asm/syscall.h
@@ -89,4 +89,7 @@ static inline int syscall_get_arch(struct task_struct *task)
 #endif
 }
 
+void do_syscall_trace_enter(struct pt_regs *regs);
+void do_syscall_trace_exit(struct pt_regs *regs);
+
 #endif	/* _ASM_RISCV_SYSCALL_H */
diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
index 905372d7eeb8..d0d980d99019 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -58,6 +58,8 @@ struct thread_info {
 	.addr_limit	= KERNEL_DS,		\
 }
 
+extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
+
 #endif /* !__ASSEMBLY__ */
 
 /*
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index b1ade9a49347..a5ad00043104 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -10,6 +10,7 @@
 #include <asm/processor.h>
 #include <asm/hwcap.h>
 #include <asm/smp.h>
+#include <asm/switch_to.h>
 
 unsigned long elf_hwcap __read_mostly;
 #ifdef CONFIG_FPU
diff --git a/arch/riscv/kernel/module-sections.c b/arch/riscv/kernel/module-sections.c
index c9ae48333114..e264e59e596e 100644
--- a/arch/riscv/kernel/module-sections.c
+++ b/arch/riscv/kernel/module-sections.c
@@ -8,6 +8,7 @@
 #include <linux/elf.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/moduleloader.h>
 
 unsigned long module_emit_got_entry(struct module *mod, unsigned long val)
 {
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index fb3a082362eb..85e3c39bb60b 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -7,6 +7,7 @@
  * Copyright (C) 2017 SiFive
  */
 
+#include <linux/cpu.h>
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/sched/task_stack.h>
@@ -19,6 +20,7 @@
 #include <asm/csr.h>
 #include <asm/string.h>
 #include <asm/switch_to.h>
+#include <asm/thread_info.h>
 
 extern asmlinkage void ret_from_fork(void);
 extern asmlinkage void ret_from_kernel_thread(void);
diff --git a/arch/riscv/kernel/reset.c b/arch/riscv/kernel/reset.c
index d0fe623bfb8f..aa56bb135ec4 100644
--- a/arch/riscv/kernel/reset.c
+++ b/arch/riscv/kernel/reset.c
@@ -4,6 +4,7 @@
  */
 
 #include <linux/reboot.h>
+#include <linux/pm.h>
 #include <asm/sbi.h>
 
 static void default_power_off(void)
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index a990a6cb184f..aa60ac527be5 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -17,6 +17,7 @@
 #include <linux/sched/task.h>
 #include <linux/swiotlb.h>
 
+#include <asm/head.h>
 #include <asm/setup.h>
 #include <asm/sections.h>
 #include <asm/pgtable.h>
diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index b14d7647d800..0974b6efc888 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -12,6 +12,7 @@
 #include <linux/tracehook.h>
 #include <linux/linkage.h>
 
+#include <asm/entry.h>
 #include <asm/ucontext.h>
 #include <asm/vdso.h>
 #include <asm/switch_to.h>
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index 3836760d7aaf..d70e3c0ee983 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -8,7 +8,9 @@
  * Copyright (C) 2017 SiFive
  */
 
+#include <linux/cpu.h>
 #include <linux/interrupt.h>
+#include <linux/profile.h>
 #include <linux/smp.h>
 #include <linux/sched.h>
 #include <linux/seq_file.h>
@@ -66,11 +68,13 @@ bool arch_match_cpu_phys_id(int cpu, u64 phys_id)
 	return phys_id == cpuid_to_hartid_map(cpu);
 }
 
+#ifdef CONFIG_PROFILING
 /* Unsupported */
 int setup_profiling_timer(unsigned int multiplier)
 {
 	return -EINVAL;
 }
+#endif
 
 static void ipi_stop(void)
 {
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 18ae6da5115e..2b81e4404a5d 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -24,11 +24,13 @@
 #include <linux/of.h>
 #include <linux/sched/task_stack.h>
 #include <linux/sched/mm.h>
+#include <asm/head.h>
 #include <asm/irq.h>
 #include <asm/mmu_context.h>
 #include <asm/tlbflush.h>
 #include <asm/sections.h>
 #include <asm/sbi.h>
+#include <asm/smp.h>
 
 void *__cpu_up_stack_pointer[NR_CPUS];
 void *__cpu_up_task_pointer[NR_CPUS];
diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
index 0940681d2f68..42193cf161e4 100644
--- a/arch/riscv/kernel/stacktrace.c
+++ b/arch/riscv/kernel/stacktrace.c
@@ -19,8 +19,8 @@ struct stackframe {
 	unsigned long ra;
 };
 
-void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
-			     bool (*fn)(unsigned long, void *), void *arg)
+static void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
+				    bool (*fn)(unsigned long, void *), void *arg)
 {
 	unsigned long fp, sp, pc;
 
diff --git a/arch/riscv/kernel/syscall_table.c b/arch/riscv/kernel/syscall_table.c
index e5dd52d8f633..f1ead9df96ca 100644
--- a/arch/riscv/kernel/syscall_table.c
+++ b/arch/riscv/kernel/syscall_table.c
@@ -8,6 +8,7 @@
 #include <linux/syscalls.h>
 #include <asm-generic/syscalls.h>
 #include <asm/vdso.h>
+#include <asm/syscall.h>
 
 #undef __SYSCALL
 #define __SYSCALL(nr, call)	[nr] = (call),
diff --git a/arch/riscv/kernel/time.c b/arch/riscv/kernel/time.c
index 541a2b885814..517d2153a933 100644
--- a/arch/riscv/kernel/time.c
+++ b/arch/riscv/kernel/time.c
@@ -6,7 +6,7 @@
 
 #include <linux/clocksource.h>
 #include <linux/delay.h>
-#include <asm/sbi.h>
+#include <asm/processor.h>
 
 unsigned long riscv_timebase;
 
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 424eb72d56b1..091d2b2e8b51 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -18,6 +18,7 @@
 #include <asm/processor.h>
 #include <asm/ptrace.h>
 #include <asm/csr.h>
+#include <asm/entry.h>
 
 int show_unhandled_signals = 1;
 
diff --git a/arch/riscv/kernel/vdso.c b/arch/riscv/kernel/vdso.c
index c9c21e0d5641..e740e5dc8099 100644
--- a/arch/riscv/kernel/vdso.c
+++ b/arch/riscv/kernel/vdso.c
@@ -9,6 +9,7 @@
 #include <linux/mm.h>
 #include <linux/slab.h>
 #include <linux/binfmts.h>
+#include <linux/elf.h>
 #include <linux/err.h>
 
 #include <asm/vdso.h>
@@ -25,7 +26,7 @@ static union {
 	struct vdso_data	data;
 	u8			page[PAGE_SIZE];
 } vdso_data_store __page_aligned_data;
-struct vdso_data *vdso_data = &vdso_data_store.data;
+static struct vdso_data *vdso_data = &vdso_data_store.data;
 
 static int __init vdso_init(void)
 {
diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
index beeb5d7f92ea..ca66d44156b6 100644
--- a/arch/riscv/mm/context.c
+++ b/arch/riscv/mm/context.c
@@ -7,6 +7,7 @@
 #include <linux/mm.h>
 #include <asm/tlbflush.h>
 #include <asm/cacheflush.h>
+#include <asm/mmu_context.h>
 
 /*
  * When necessary, performs a deferred icache flush for the given MM context,
diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 96add1427a75..56cc6674bc9f 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -14,6 +14,7 @@
 #include <linux/signal.h>
 #include <linux/uaccess.h>
 
+#include <asm/head.h>
 #include <asm/pgalloc.h>
 #include <asm/ptrace.h>
 #include <asm/tlbflush.h>
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index f0ba71304b6e..74546fcc3e4c 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -13,6 +13,7 @@
 #include <linux/of_fdt.h>
 
 #include <asm/fixmap.h>
+#include <asm/head.h>
 #include <asm/tlbflush.h>
 #include <asm/sections.h>
 #include <asm/pgtable.h>
@@ -37,7 +38,7 @@ static void __init zone_sizes_init(void)
 	free_area_init_nodes(max_zone_pfns);
 }
 
-void setup_zero_page(void)
+static void setup_zero_page(void)
 {
 	memset((void *)empty_zero_page, 0, PAGE_SIZE);
 }
@@ -140,7 +141,7 @@ EXPORT_SYMBOL(pfn_base);
 void *dtb_early_va;
 pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
 pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
-pte_t fixmap_pte[PTRS_PER_PTE] __page_aligned_bss;
+static pte_t fixmap_pte[PTRS_PER_PTE] __page_aligned_bss;
 static bool mmu_enabled;
 
 #define MAX_EARLY_MAPPING_SIZE	SZ_128M
@@ -199,15 +200,15 @@ static void __init create_pte_mapping(pte_t *ptep,
 
 #ifndef __PAGETABLE_PMD_FOLDED
 
-pmd_t trampoline_pmd[PTRS_PER_PMD] __page_aligned_bss;
-pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss;
+static pmd_t trampoline_pmd[PTRS_PER_PMD] __page_aligned_bss;
+static pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss;
 
 #if MAX_EARLY_MAPPING_SIZE < PGDIR_SIZE
 #define NUM_EARLY_PMDS		1UL
 #else
 #define NUM_EARLY_PMDS		(1UL + MAX_EARLY_MAPPING_SIZE / PGDIR_SIZE)
 #endif
-pmd_t early_pmd[PTRS_PER_PMD * NUM_EARLY_PMDS] __initdata __aligned(PAGE_SIZE);
+static pmd_t early_pmd[PTRS_PER_PMD * NUM_EARLY_PMDS] __initdata __aligned(PAGE_SIZE);
 
 static pmd_t *__init get_pmd_virt(phys_addr_t pa)
 {
@@ -328,9 +329,8 @@ static uintptr_t __init best_map_size(phys_addr_t base, phys_addr_t size)
  * for init.o in mm/Makefile.
  */
 
-#ifndef __riscv_cmodel_medany
-#error "setup_vm() is called from head.S before relocate so it should "
-	"not use absolute addressing."
+#if !defined(__riscv_cmodel_medany) && !defined(__CHECKER__)
+#error "setup_vm() is called from head.S before relocate so it should not use absolute addressing."
 #endif
 
 asmlinkage void __init setup_vm(uintptr_t dtb_pa)
diff --git a/arch/riscv/mm/sifive_l2_cache.c b/arch/riscv/mm/sifive_l2_cache.c
index 2e637ad71c05..a9ffff3277c7 100644
--- a/arch/riscv/mm/sifive_l2_cache.c
+++ b/arch/riscv/mm/sifive_l2_cache.c
@@ -142,7 +142,7 @@ static irqreturn_t l2_int_handler(int irq, void *device)
 	return IRQ_HANDLED;
 }
 
-int __init sifive_l2_init(void)
+static int __init sifive_l2_init(void)
 {
 	struct device_node *np;
 	struct resource res;
-- 
2.23.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] riscv: resolve most warnings from sparse
  2019-09-19  8:26 [PATCH] riscv: resolve most warnings from sparse Paul Walmsley
@ 2019-09-19 17:31 ` Christoph Hellwig
  2019-09-21 10:07   ` Paul Walmsley
  2019-09-26 21:29   ` Luc Van Oostenryck
  0 siblings, 2 replies; 4+ messages in thread
From: Christoph Hellwig @ 2019-09-19 17:31 UTC (permalink / raw)
  To: Paul Walmsley; +Cc: linux-riscv, linux-kernel

On Thu, Sep 19, 2019 at 01:26:38AM -0700, Paul Walmsley wrote:
> 
> Resolve most of the warnings emitted by sparse.  The objective here is
> to keep arch/riscv as clean as possible with regards to sparse warnings,
> and to maintain this bar for subsequent patches.

I think this patch does just way to many different things and needs
to be split up into one patch per issue / code module.

> --- /dev/null
> +++ b/arch/riscv/include/asm/entry.h

For example adding this file should be a patch on its own.  It can
also move to arch/riscv/kernel/ instead of polluting the <asm/*.h>
namespace.  That being said I'm not sure I like this and the
head.h patches.  Just adding a header for entry points used from
aseembly only seems rather pointless, I wonder if there is a way
to just shut up sparse on them.  Same for most of head.h.

> @@ -61,6 +61,9 @@
>  
>  #define PAGE_TABLE		__pgprot(_PAGE_TABLE)
>  
> +extern pgd_t swapper_pg_dir[];
> +extern pgd_t trampoline_pg_dir[];
> +extern pgd_t early_pg_dir[];
>  extern pgd_t swapper_pg_dir[];

This seems to add a duplicate definition of swapper_pg_dir.

> +extern asmlinkage void __init smp_callin(void);

No nee for the extern.

> index 905372d7eeb8..d0d980d99019 100644
> --- a/arch/riscv/include/asm/thread_info.h
> +++ b/arch/riscv/include/asm/thread_info.h
> @@ -58,6 +58,8 @@ struct thread_info {
>  	.addr_limit	= KERNEL_DS,		\
>  }
>  
> +extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);

This really needs to move to a header outside of arch/.  Also no need
for the extern and as-is this adds a line > 80 chars.

> +#ifdef CONFIG_PROFILING
>  /* Unsupported */
>  int setup_profiling_timer(unsigned int multiplier)
>  {
>  	return -EINVAL;
>  }
> +#endif

Yikes.  All architectures either just return 0 or -EINVAL here,
and the caller has a spurious extern for it.  Please just remove
this arch hook and add a Kconfig variable that the few architectures
currently returning 0 select insted.

> +static void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,

This adds an > 80 char line.

> -pmd_t early_pmd[PTRS_PER_PMD * NUM_EARLY_PMDS] __initdata __aligned(PAGE_SIZE);
> +static pmd_t early_pmd[PTRS_PER_PMD * NUM_EARLY_PMDS] __initdata __aligned(PAGE_SIZE);

Another one.

> --- a/arch/riscv/mm/sifive_l2_cache.c
> +++ b/arch/riscv/mm/sifive_l2_cache.c
> @@ -142,7 +142,7 @@ static irqreturn_t l2_int_handler(int irq, void *device)
>  	return IRQ_HANDLED;
>  }
>  
> -int __init sifive_l2_init(void)
> +static int __init sifive_l2_init(void)
>  {
>  	struct device_node *np;
>  	struct resource res;

And this needs to be applied after this file moves to the right place
and isn't completely bogusly built into every RISC-V kernel.  Not all
the world is a SiFive..

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] riscv: resolve most warnings from sparse
  2019-09-19 17:31 ` Christoph Hellwig
@ 2019-09-21 10:07   ` Paul Walmsley
  2019-09-26 21:29   ` Luc Van Oostenryck
  1 sibling, 0 replies; 4+ messages in thread
From: Paul Walmsley @ 2019-09-21 10:07 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-riscv, linux-kernel

Hi Christoph,

On Thu, 19 Sep 2019, Christoph Hellwig wrote:

> On Thu, Sep 19, 2019 at 01:26:38AM -0700, Paul Walmsley wrote:
> > 
> > Resolve most of the warnings emitted by sparse.  The objective here is
> > to keep arch/riscv as clean as possible with regards to sparse warnings,
> > and to maintain this bar for subsequent patches.
> 
> I think this patch does just way to many different things and needs
> to be split up into one patch per issue / code module.

I agree that it's hard to review as it is, and will split it up into a few 
smaller patches.

> 
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/entry.h
> 
> For example adding this file should be a patch on its own.  It can
> also move to arch/riscv/kernel/ instead of polluting the <asm/*.h>
> namespace.  That being said I'm not sure I like this and the
> head.h patches.  Just adding a header for entry points used from
> aseembly only seems rather pointless, I wonder if there is a way
> to just shut up sparse on them.  Same for most of head.h.

If you have another suggestion, please let me know.  Adding this 
prototypes is perhaps not ideal, but I personally don't see any downside, 
aside from aesthetics.  Several other architectures have either asm/ or 
kernel/entry.h, and a few others have head.h, so it's at least in line 
with existing practice.

> 
> > @@ -61,6 +61,9 @@
> >  
> >  #define PAGE_TABLE		__pgprot(_PAGE_TABLE)
> >  
> > +extern pgd_t swapper_pg_dir[];
> > +extern pgd_t trampoline_pg_dir[];
> > +extern pgd_t early_pg_dir[];
> >  extern pgd_t swapper_pg_dir[];
> 
> This seems to add a duplicate definition of swapper_pg_dir.
> 
> > +extern asmlinkage void __init smp_callin(void);
> 
> No nee for the extern.
> 
> > index 905372d7eeb8..d0d980d99019 100644
> > --- a/arch/riscv/include/asm/thread_info.h
> > +++ b/arch/riscv/include/asm/thread_info.h
> > @@ -58,6 +58,8 @@ struct thread_info {
> >  	.addr_limit	= KERNEL_DS,		\
> >  }
> >  
> > +extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
> 
> This really needs to move to a header outside of arch/.  Also no need
> for the extern and as-is this adds a line > 80 chars.
> 
> > +#ifdef CONFIG_PROFILING
> >  /* Unsupported */
> >  int setup_profiling_timer(unsigned int multiplier)
> >  {
> >  	return -EINVAL;
> >  }
> > +#endif
> 
> Yikes.  All architectures either just return 0 or -EINVAL here,
> and the caller has a spurious extern for it.  Please just remove
> this arch hook and add a Kconfig variable that the few architectures
> currently returning 0 select insted.
> 
> > +static void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
> 
> This adds an > 80 char line.
> 
> > -pmd_t early_pmd[PTRS_PER_PMD * NUM_EARLY_PMDS] __initdata __aligned(PAGE_SIZE);
> > +static pmd_t early_pmd[PTRS_PER_PMD * NUM_EARLY_PMDS] __initdata __aligned(PAGE_SIZE);
> 
> Another one.

Thanks, will review the above issues.

> > --- a/arch/riscv/mm/sifive_l2_cache.c
> > +++ b/arch/riscv/mm/sifive_l2_cache.c
> > @@ -142,7 +142,7 @@ static irqreturn_t l2_int_handler(int irq, void *device)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > -int __init sifive_l2_init(void)
> > +static int __init sifive_l2_init(void)
> >  {
> >  	struct device_node *np;
> >  	struct resource res;
> 
> And this needs to be applied after this file moves to the right place
> and isn't completely bogusly built into every RISC-V kernel.  Not all
> the world is a SiFive..

Once you can get the ack from the EDAC maintainers, I'll most likely apply 
your previous patch.  In the meantime I'm planning to get the sparse fixes 
in early, since they are long overdue.


Thanks for the review,

- Paul

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] riscv: resolve most warnings from sparse
  2019-09-19 17:31 ` Christoph Hellwig
  2019-09-21 10:07   ` Paul Walmsley
@ 2019-09-26 21:29   ` Luc Van Oostenryck
  1 sibling, 0 replies; 4+ messages in thread
From: Luc Van Oostenryck @ 2019-09-26 21:29 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-riscv, linux-kernel, Paul Walmsley

On Thu, Sep 19, 2019 at 10:31:42AM -0700, Christoph Hellwig wrote:
> On Thu, Sep 19, 2019 at 01:26:38AM -0700, Paul Walmsley wrote:
> > 
> > Resolve most of the warnings emitted by sparse.  The objective here is
> > to keep arch/riscv as clean as possible with regards to sparse warnings,
> > and to maintain this bar for subsequent patches.
> 
> I think this patch does just way to many different things and needs
> to be split up into one patch per issue / code module.
> 
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/entry.h
> 
> For example adding this file should be a patch on its own.  It can
> also move to arch/riscv/kernel/ instead of polluting the <asm/*.h>
> namespace.  That being said I'm not sure I like this and the
> head.h patches.  Just adding a header for entry points used from
> aseembly only seems rather pointless, I wonder if there is a way
> to just shut up sparse on them.  Same for most of head.h.

The pseudo-specifier '__visible' (for __attribute__((__externally_visible__)))
is defined for this.

Best regards,
-- Luc Van Oostenryck

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-09-26 21:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-19  8:26 [PATCH] riscv: resolve most warnings from sparse Paul Walmsley
2019-09-19 17:31 ` Christoph Hellwig
2019-09-21 10:07   ` Paul Walmsley
2019-09-26 21:29   ` Luc Van Oostenryck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).