Re: [Kgdb-bugreport] [PATCH] Kill kgdb_serial

[Tom Rini <trini@kernel.crashing.org>]

On Thu, Mar 04, 2004 at 03:06:34PM -0800, George Anzinger wrote:
> Tom Rini wrote:
> >On Thu, Mar 04, 2004 at 10:11:39AM +0530, Amit S. Kale wrote:
> >
> >>On Wednesday 03 Mar 2004 8:46 pm, Tom Rini wrote:
> >>
> >>>On Wed, Mar 03, 2004 at 11:13:02AM +0530, Amit S. Kale wrote:
> >
> >[snip]
> >
> >>>>kgdb_serial isn't ugly. It's just a function switch, similar to several
> >>>>of them in the kernel. ppc is ugly, but that's anyway the case because 
> >>>>of
> >>>>so many varieties of ppc. If we are trying to make ppc code clean, it
> >>>>makes more sense to move this weak function thing into ppc specific 
> >>>>files
> >>>>IMHO.
> >>>
> >>>I think you missed the point.  The problem isn't with providing weak
> >>>functions, the problem is trying to set the function pointer.  PPC
> >>>becomes quite clean since the next step is to kill off
> >>>PPC_SIMPLE_SERIAL and just have kgdb_read/write_debug_char in the
> >>>relevant serial drivers.
> >>
> >>We can still have one single hardcoded function pointer for ppc and manage
> >>the rest in ppc specific files.
> >
> >
> >I think you're still missing the point.
> >
> >Regardless, the solution to this is what dwmw2 suggested on IRC I
> >believe, as this should remove all of the #ifdef mess.
> 
> I am afraid I don't quite understand what he was saying other than early 
> init stuff.  On of the problems with trying early init stuff, by the way, 
> is that a lot of things depend on having alloc up and that happens rather 
> late in the game.

I assume you aren't talking about kgdb stuff here (or what would be the
point of going so early) but I believe he was talking about allowing for
stuff that could be done early, to be done early.

> But back to the method.  Is he/are you suggesting that the init code plug 
> the array of functions into the kgdb table?  This could be done by 
> providing a register function in kgdb to register an i/o method.  Pass it a 
> pointer to a struc of entry points, keep the pointers in an array, etc.  
> All that is left is to define the default in some simple and clean way, a 
> way that might be overridden at command line parse time and so on.

What I'm suggesting is that we don't need this table stuff at all.
Roughly:
- Call kgdb_entry(), minus the kgdb_serial->hook(), as early as
  possible.
- We do early parsing of some cmd opts (this I believe is __early_setup)
  - This means that to use kgdboe you _must_ pass kgdboe=whatever, same
    for 8250, etc.  This would also do kgdb_serial = kgdboe_serial, or
    whatever.
  - 8250, kgdboe, etc, must be able to init themselves.
- If ('gdb' || 'kgdbwait') && kgdb_serial != NULL -> breakpoint();

-- 
Tom Rini
http://gate.crashing.org/~trini/