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..