Re: [RESEND RFC PATCH 02/12] sh: Update OF handling.

Hi Sato-san,

Thanks for your patch!

On Thu, Aug 31, 2023 at 9:08 AM Yoshinori Sato
<[email protected]> wrote:
> - avoid build warning.
> - reserve external dtb area.
> - use generic interfaces on internal peripheral driver.

... and much more.
Please split in separate patches.

> --- a/arch/sh/boards/of-generic.c
> +++ b/arch/sh/boards/of-generic.c

>  void __init __weak plat_irq_setup(void)
>  {
>  }
> +
> +static int __init sh_of_device_init(void)
> +{
> +       pr_info("SH generic board support: populating platform devices\n");
> +       if (of_have_populated_dt()) {
> +               of_platform_populate(NULL, of_default_bus_match_table,
> +                                    NULL, NULL);
> +       } else {
> +               pr_crit("Device tree not populated\n");
> +       }
> +       return 0;
> +}
> +arch_initcall_sync(sh_of_device_init);

Do you need this?
of_platform_populate() is already called through
drivers/of/platform.c:arch_initcall_sync(of_platform_default_populate_init);

> +
> +void intc_finalize(void)
> +{
> +}

Do you need this? Oh, for arch/sh/kernel/irq.c:void __init init_IRQ().
Perhaps define an inline dummy in a header file?

> --- a/arch/sh/kernel/head_32.S
> +++ b/arch/sh/kernel/head_32.S
> @@ -56,7 +56,7 @@ ENTRY(empty_zero_page)
>   */
>  ENTRY(_stext)
>         !                       Initialize Status Register
> -       mov.l   1f, r0          ! MD=1, RB=0, BL=0, IMASK=0xF
> +       mov.l   1f, r0          ! MD=1, RB=0, BL=1, IMASK=0xF

Why?

>         ldc     r0, sr
>         !                       Initialize global interrupt mask
>  #ifdef CONFIG_CPU_HAS_SR_RB

> --- a/arch/sh/kernel/setup.c
> +++ b/arch/sh/kernel/setup.c

> @@ -79,7 +80,9 @@ extern int root_mountflags;
>  #define RAMDISK_PROMPT_FLAG            0x8000
>  #define RAMDISK_LOAD_FLAG              0x4000
>
> +#ifndef CONFIG_OF
>  static char __initdata command_line[COMMAND_LINE_SIZE] = { 0, };
> +#endif

Having the ability to override the command line is still useful.
And below, the use of command_line[] depends on
!defined(CONFIG_OF) || defined(USE_BUILTIN_DTB).

> @@ -313,10 +332,15 @@ void __init setup_arch(char **cmdline_p)
>         strlcat(command_line, CONFIG_CMDLINE, sizeof(command_line));
>  #endif
>  #endif
> +#endif
>
> +#if !defined(CONFIG_OF) || defined(USE_BUILTIN_DTB)
>         /* Save unparsed command line copy for /proc/cmdline */
>         memcpy(boot_command_line, command_line, COMMAND_LINE_SIZE);
>         *cmdline_p = command_line;
> +#else
> +       *cmdline_p = boot_command_line;
> +#endif
>
>         parse_early_param();

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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