Messages in this thread | | | Date | Thu, 4 Mar 2004 15:49:21 -0700 | From | Tom Rini <> | Subject | Re: [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; > >#elif CONFIG_KGDB_ARCH_SERIAL > > extern ... kgdbarch_serial; > > kgdb_serial = &kgdbarch_serial; > >#elif CONFIG_KGDB_ETH > > 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 functions.
> 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 http://gate.crashing.org/~trini/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |