Re: Take 2: [RFC] Debugging with a HW probe.

[Michael Ellerman <michael@ellerman.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2684 bytes --]

On Tue, 2006-08-22 at 01:04 -0500, Milton Miller wrote:
> On Aug 14, 2006, at 5:16 AM, Jimi Xenidis jimix at watson.ibm.com  
> > @@ -693,6 +696,11 @@ #ifdef CONFIG_PPC64
> >         if (of_get_flat_dt_prop(node, "linux,iommu-force-on", NULL) != 
> > NULL)
> >                 iommu_force_on = 1;
> >  #endif
> > +#ifdef CONFIG_HW_PROBE_ENABLE
> > +       if (of_get_flat_dt_prop(node, "linux,hw-probe-enable", NULL) 
> > != NULL) {
> > +               hw_probe_enabled = 1;
> > +               DBG("HW Probe will be enabled\n");
> > +#endif
> 
> No.  [see next comment]
> 
> >
> >         /* mem=x on the command line is the preferred mechanism */
> >         lprop = of_get_flat_dt_prop(node, "linux,memory-limit", NULL);
> > diff --git a/arch/powerpc/kernel/prom_init.c 
> > b/arch/powerpc/kernel/prom_init.c
> > index 90972ef..26428de 100644
> > --- a/arch/powerpc/kernel/prom_init.c
> > +++ b/arch/powerpc/kernel/prom_init.c
> > @@ -587,6 +587,14 @@ #ifdef CONFIG_PPC64
> >                         RELOC(iommu_force_on) = 1;
> >         }
> >  #endif
> > +#ifdef CONFIG_HW_PROBE_ENABLE
> > +       opt = strstr(RELOC(prom_cmd_line), RELOC("hwprobe"));
> > +       if (opt) {
> > +               prom_printf("WARNING! HW Probe will be activated!\n");
> > +               prom_setprop(_prom->chosen, "/chosen",
> > +                            "linux,hw-probe-enable", NULL, 0);
> > +       }
> > +#endif
> >  }
> 
> Please, PLEASE do NOT do this.
> 
> prom_init.c is only used by one of the many flat device tree generators,
> namely the open-firmware client.  Adding a property like this requires 
> us
> to update all the other clients.
> 
> And there is no reason to parse it this early.
> 
> Instead, parse it from the command line like the other early parsing.
> 
> I thing a generic early_param would be fine.  However, xmon_init is an
> early_parm in setup-common, so if you really require it before the first
> call, then you could parse it next to mem= at the bottom of
> early_init_dt_scan_chosen.

What Milton said, we learnt that the hard way with kexec :)

Except that we don't do mem= in early_init_dt_scan_chosen anymore (get a
newer kernel Milton!).

The early_param parsing is done very early, and the xmon= parsing does
not jump into xmon, it waits until a little later before doing it. So an
early_param should be fine, ie. it will be parsed before xmon ever runs.

cheers

-- 
Michael Ellerman
IBM OzLabs

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]