[lkml]   [2004]   [Mar]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [Kgdb-bugreport] [PATCH] Kill kgdb_serial
On Thu, Mar 04, 2004 at 02:18:08PM -0800, George Anzinger wrote:
> Tom Rini wrote:
> >On Wed, Mar 03, 2004 at 04:27:57PM -0800, George Anzinger wrote:
> >
> >>Tom Rini wrote:
> >>
> >>>But that's not what you get with kgdb_serial. You get the possibility
> >>>of serial from point A to B and you will have eth from point B onward,
> >>>if compiled in. With an arch serial driver you get the possibility of
> >>>serial (or arch serial or whatever) from point A to B and eth from point
> >>>B onward, if compiled in.
> >>
> >>I don't think we want to switch. Rather we want to say something like:
> >>If no eth (or other input) options are on the command line then its is
> >>serial. If eth (or other input) is there, that is what we use.
> >>
> >>This does leave open what happens when "eth" is given and we hit a
> >>breakpoint prior to looking at the command line, but now this just fails
> >>so we would be hard put to do worse.
> >
> >
> >This doesn't fail right now, or rather it shouldn't. We would call
> >kgdb_arch_init() which would set it to 8250 (or arch serial) and go. If
> >8250||arch serial is compiled in.
> But, if I understand this right, now you can have either eth or serial. If
> you have eth and hit a breakpoint prior to its int, you are dead.

Ah yes, as a side effect of me being a bit too quick with my
kernel/Kconfig.kgdb changes, yes. Good catch :)

> >>>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.
> >>
> >>No, you just set the default at configure time. It is just done in such
> >>away as to allow it to be overridden.
> >
> >
> >Which means you have to either c&p this into kgdb_arch_init for every
> >arch that provides it's own, or (and I've been thinking that this isn't
> >necessarily a bad idea) standardize on names for the arch serial driver,
> >and in kernel/kgdb.c::kgdb_entry() do:
> >#ifdef CONFIG_KGDB_8250
> > extern ... kgdb8250_serial;
> > kgdb_serial = &kgdb8250_serial;
> > extern ... kgdbarch_serial;
> > kgdb_serial = &kgdbarch_serial;
> > extern ... kgdboe_serial;
> > kgdb_serial = &kgdboe_serial;
> >#endif
> I would rather standardize on the name of the INIT block. How about
> something like:

I would rather not have to duplicate the above code (or, any code) for
every arch. :)

> #include <linux/kdgb_io.h>
> :
> :
> struc kgdb_io_table kgdb_io_table[]=KGDB_IO_FUNCTIONS;
> then in linux/kgdb_io.h we have:
> #include <asm/kgdb_io.h>
> struc kgdb_io_table {
> char (*kgdb_read_char);
> void (*kgdb_write_char);
> :
> :
> }
> And in asm/kgdb_io.h we define:
> extern char my_read_char(void);
> :
> #define KGDB_IO_FUNCTIONS {{my_read_char, my_write_char,...},\
> {eth_read_char,...}}

But eth_read_char, etc aren't, don't need to be, and shouldn't be global

> So it is completely up to the arch asm/kgdb_io.h to define the names and
> even how many. We then assume that the first one is the default. An arch
> that wants to mess with different things can do it all in this file,
> including having several diffent serial drivers all with the same entry
> points, different default as set at configure time and so on.

I think this is backwards. Stop thinking in terms of 8250, eth and arch
serial, but instead io1, io2 and io3 (and io4, and io5, ...). I don't
see what's wrong with what dwmw2 suggested that I passed along.

Tom Rini
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

 \ /
  Last update: 2005-03-22 14:01    [W:0.075 / U:2.164 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site