lkml.org 
[lkml]   [2004]   [Mar]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    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;
    > >#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/

    \
     
     \ /
      Last update: 2005-03-22 14:01    [W:0.027 / U:0.012 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site