[PATCH v7 00/22] RISC-V Kendryte K210 support improvements
Damien Le Moal
Damien.LeMoal at wdc.com
Thu Dec 10 07:36:30 EST 2020
On Thu, 2020-12-10 at 11:04 +0100, Geert Uytterhoeven wrote:
> Hi Damien,
>
> On Thu, Dec 10, 2020 at 4:41 AM Damien Le Moal <damien.lemoal at wdc.com> wrote:
> > Changes from v6:
> > * Annotate struct platform_driver variables with __refdata to avoid
> > section mismatch compilation errors
>
> Blindly following the advice from kernel test robot <lkp at intel.com> is
> not always a good idea:
>
> The variable k210_rst_driver references
> the function __init set_reset_devices()
> If the reference is valid then annotate the
> variable with or __refdata (see linux/init.h) or name the variable:
>
> If your driver's probe function is annotated with __init, you cannot
> have a pointer to it in the driver structure, as any binding done after
> the freeing of initmem will cause a crash. Adding the __refdata merely
> suppresses the warning, and won't avoid the crash.
Hmm... I must be misunderstanding something here. free_initmem() is called from
kernel_init() right before starting the user init process. That is late enough
that all drivers are already probed and initialized. At least that is what I
thought, especially considering that none of the k210 drivers can be modules
and are all builtin. What am I missing here ?
> There are two solutions for this:
> 1. Remove the .probe pointer, and use platform_driver_probe() instead
> of platform_driver_register().
> This guarantees the probe method will be called only once, before
> initmem is freed, but does mean that probe deferral cannot work.
> 2. Drop the __init annotation.
> This means the probe method can be called anytime, and supports
> both probe deferral and driver unbind/rebind cycles.
>
> Given the limited amount of RAM on k210, I think option 1 is preferred,
> but may require careful tuning of the initialization order using
> *_initcall*(), to make sure probe deferral won't ever be needed.
I really would prefer to avoid the *_initcall() dance. That is not pretty.
Simply removing the __init annotation, I see 766 pages free with "cat
/proc/vmstat" right after boot, and the same number with __init too... So it
does not look like the impact is significant. The biggest probe function is for
the clock driver and this one seems fine with __init since it uses
CLK_OF_DECLARE(). I do have a doubt about this now though. Looking at other
drivers, all seem to have the __init annotation for their clock driver init
function.
So I think I will go with option 2. It is simpler and safer. We can always
revisit and optimize later. I would prefer this series to land first :)
>
> Gr{oetje,eeting}s,
>
> Geert
>
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
--
Damien Le Moal
Western Digital
More information about the linux-riscv
mailing list