| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 10 May 2016 17:25:36 +0900
From: Yoshinori Sato <ysato@...rs.sourceforge.jp>
To: Rich Felker <dalias@...c.org>
Cc: linux-sh@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND 05/12] sh: DeviceTree support update
On Wed, 04 May 2016 12:10:05 +0900,
Rich Felker wrote:
>
> On Sun, May 01, 2016 at 02:08:29PM +0900, Yoshinori Sato wrote:
> > Changes bellow
> > - FDT setup timing fix.
> > - chosen/bootargs support.
> > - zImage support.
> > - DT binding helper macro.
> >
> > Signed-off-by: Yoshinori Sato <ysato@...rs.sourceforge.jp>
> > ---
> > arch/sh/boards/of-generic.c | 23 +++++++++++-----------
> > arch/sh/boot/compressed/head_32.S | 5 +++--
> > arch/sh/boot/dts/include/dt-bindings | 1 +
> > arch/sh/kernel/setup.c | 19 ++++++++++++++++++
> > include/dt-bindings/interrupt-controller/sh_intc.h | 2 ++
> > 5 files changed, 36 insertions(+), 14 deletions(-)
> > create mode 120000 arch/sh/boot/dts/include/dt-bindings
> > create mode 100644 include/dt-bindings/interrupt-controller/sh_intc.h
> >
> > diff --git a/arch/sh/boards/of-generic.c b/arch/sh/boards/of-generic.c
> > index bf3a166..9570873 100644
> > --- a/arch/sh/boards/of-generic.c
> > +++ b/arch/sh/boards/of-generic.c
> > @@ -112,29 +112,25 @@ static int noopi(void)
> > return 0;
> > }
> >
> > -static void __init sh_of_mem_reserve(void)
> > +static void __init sh_of_mem_init(void)
> > {
> > early_init_fdt_reserve_self();
> > early_init_fdt_scan_reserved_mem();
> > }
> >
> > -static void __init sh_of_time_init(void)
> > -{
> > - pr_info("SH generic board support: scanning for clocksource devices\n");
> > - clocksource_probe();
> > -}
>
> Why did you remove this? Without it you won't get clock
> event/clocksource devices from the device tree so the only way to have
> a working timer interrupt is if the driver is hard-coded somewhere.
It not needed on Common Clock Framework.
tmu define in dts.
> > static void __init sh_of_setup(char **cmdline_p)
> > {
> > - unflatten_device_tree();
> > -
> > - board_time_init = sh_of_time_init;
> > + struct device_node *cpu;
> > + int freq;
> >
> > sh_mv.mv_name = of_flat_dt_get_machine_name();
> > if (!sh_mv.mv_name)
> > sh_mv.mv_name = "Unknown SH model";
> >
> > sh_of_smp_probe();
> > + cpu = of_find_node_by_name(NULL, "cpu");
> > + if (!of_property_read_u32(cpu, "clock-frequency", &freq))
> > + preset_lpj = freq / 500;
> > }
>
> I setup the DT-based pseudo-board to use the generic calibrate-delay
> rather than hard-coding lpj. Ideally we could just get rid of bogomips
> completely but there are probably still some things using it. Is there
> a reason you prefer making up a value for lpj based on the cpu clock
> rate?
clockevent initalize after calibrate delay.
So don't work interrupt based calibrate.
> > static int sh_of_irq_demux(int irq)
> > @@ -167,8 +163,7 @@ static struct sh_machine_vector __initmv sh_of_generic_mv = {
> > .mv_init_irq = sh_of_init_irq,
> > .mv_clk_init = sh_of_clk_init,
> > .mv_mode_pins = noopi,
> > - .mv_mem_init = noop,
> > - .mv_mem_reserve = sh_of_mem_reserve,
> > + .mv_mem_init = sh_of_mem_init,
>
> Is there a reason for this renaming? The function seems to be dealing
> purely with reserving memory ranges.
mv_mem_reserve too late call in MMU system.
>
> > struct sh_clk_ops;
> > @@ -194,3 +189,7 @@ static int __init sh_of_device_init(void)
> > return 0;
> > }
> > arch_initcall_sync(sh_of_device_init);
> > +
> > +void intc_finalize(void)
> > +{
> > +}
> > diff --git a/arch/sh/boot/compressed/head_32.S b/arch/sh/boot/compressed/head_32.S
> > index 3e15032..cd34377 100644
> > --- a/arch/sh/boot/compressed/head_32.S
> > +++ b/arch/sh/boot/compressed/head_32.S
> > @@ -14,7 +14,8 @@ startup:
> > /* Load initial status register */
> > mov.l init_sr, r1
> > ldc r1, sr
> > -
> > + /* Save FDT address */
> > + mov r4, r13
> > /* Move myself to proper location if necessary */
> > mova 1f, r0
> > mov.l 1f, r2
> > @@ -83,7 +84,7 @@ l1:
> > /* Jump to the start of the decompressed kernel */
> > mov.l kernel_start_addr, r0
> > jmp @r0
> > - nop
> > + mov r13,r4
> >
> > .align 2
> > bss_start_addr:
>
> This should probably be its own patch, adding DT support for
> compressed kernel images. It's independent from everything else in
> this patch.
OK. separated.
> > diff --git a/arch/sh/boot/dts/include/dt-bindings b/arch/sh/boot/dts/include/dt-bindings
> > new file mode 120000
> > index 0000000..08c00e4
> > --- /dev/null
> > +++ b/arch/sh/boot/dts/include/dt-bindings
> > @@ -0,0 +1 @@
> > +../../../../../include/dt-bindings
> > \ No newline at end of file
> > diff --git a/arch/sh/kernel/setup.c b/arch/sh/kernel/setup.c
> > index 5d34605..f6bb105 100644
> > --- a/arch/sh/kernel/setup.c
> > +++ b/arch/sh/kernel/setup.c
> > @@ -177,7 +177,12 @@ disable:
> > #ifndef CONFIG_GENERIC_CALIBRATE_DELAY
> > void calibrate_delay(void)
> > {
> > +#ifndef CONFIG_OF
> > struct clk *clk = clk_get(NULL, "cpu_clk");
> > +#else
> > + struct device_node *cpu = of_find_node_by_name(NULL, "cpu");
> > + struct clk *clk = of_clk_get_by_name(cpu, NULL);
> > +#endif
> >
> > if (IS_ERR(clk))
> > panic("Need a sane CPU clock definition!");
> > @@ -251,7 +256,11 @@ void __ref sh_fdt_init(phys_addr_t dt_phys)
> > /* Avoid calling an __init function on secondary cpus. */
> > if (done) return;
> >
> > +#ifdef CONFIG_USE_BUILTIN_DTB
> > + dt_virt = __dtb_start;
> > +#else
> > dt_virt = phys_to_virt(dt_phys);
> > +#endif
>
> This is also part of the bultin-dtb patch, which seems to have been
> spread out across several of your patches.
Sorry. It my mistake.
> > if (!dt_virt || !early_init_dt_scan(dt_virt)) {
> > pr_crit("Error: invalid device tree blob"
> > @@ -267,8 +276,13 @@ void __ref sh_fdt_init(phys_addr_t dt_phys)
> >
> > void __init setup_arch(char **cmdline_p)
> > {
> > +#ifdef CONFIG_OF
> > + unflatten_device_tree();
> > +#endif
> > enable_mmu();
>
> Was this moved to setup_arch to have it before enable_mmu? I think
> that makes sense.
early_init_dt_alloc_memory_arch used physical address.
It override on sh-specific, can after enable_mmu.
But I don't feel an advantage.
> > +#ifndef CONFIG_OF
> > ROOT_DEV = old_decode_dev(ORIG_ROOT_DEV);
> >
> > printk(KERN_NOTICE "Boot params:\n"
> > @@ -290,6 +304,7 @@ void __init setup_arch(char **cmdline_p)
> >
> > if (!MOUNT_ROOT_RDONLY)
> > root_mountflags &= ~MS_RDONLY;
> > +#endif
>
> Do these boot params only make sense for non-DT setups?
>
> > +#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
>
> I think this is wrong -- it causes the builtin command line and
> bootloader-provided command line to be ignored on DT kernels. Do you
> just want to deprecate builtin and bootloader-provided command lines?
> Or is it just a side effect of adding support for chosen/bootarg?
Yes. I think zero-page passing only non-DT.
DT using chosen/bootargs.
> > parse_early_param();
> >
> > diff --git a/include/dt-bindings/interrupt-controller/sh_intc.h b/include/dt-bindings/interrupt-controller/sh_intc.h
> > new file mode 100644
> > index 0000000..8c9dcdc
> > --- /dev/null
> > +++ b/include/dt-bindings/interrupt-controller/sh_intc.h
> > @@ -0,0 +1,2 @@
> > +#define evt2irq(evt) (((evt) >> 5) - 16)
> > +#define irq2evt(irq) (((irq) + 16) << 5)
>
> This seems unrelated to other things in this patch.
It using DT define.
> Rich
--
Yoshinori Sato
<ysato@...rs.sourceforge.jp>
Powered by blists - more mailing lists