Re: PPC KGDB changes and some help?

From: Tom Rini
Date: Fri Jan 23 2004 - 12:11:56 EST


On Thu, Jan 22, 2004 at 09:36:10PM +0530, Amit S. Kale wrote:

> On Thursday 22 Jan 2004 9:15 pm, Tom Rini wrote:
> > On Thu, Jan 22, 2004 at 09:25:19AM -0600, Hollis Blanchard wrote:
> > > On Jan 22, 2004, at 9:07 AM, Tom Rini wrote:
> > > >On Wed, Jan 21, 2004 at 03:12:25PM -0800, George Anzinger wrote:
> > > >>A question I have been meaning to ask: Why is the arch/common
> > > >>connection
> > > >>via a structure of addresses instead of just calls? I seems to me
> > > >>that
> > > >>just calling is a far cleaner way to do things here. All the struct
> > > >>seems
> > > >>to offer is a way to change the backend on the fly. I don't thing we
> > > >>ever
> > > >>want to do that. Am I missing something?
> > > >
> > > >I imagine it's a style thing. I don't have a preference either way.
> > >
> > > I think we in PPC land have gotten used to that "style" because we have
> > > one kernel that supports different "platforms", i.e. it selects the
> > > appropriate code at runtime as George says. In general that's a little
> > > bit slower and a little bit bigger.
> > >
> > > Unless you need to choose among PPC KGDB functions at runtime, which I
> > > don't think you do, you don't need it...
> >
> > That's certainly true, so if (and if I understand Georges question
> > right) Amit wants to change kgdb_arch into a set of required functions,
> > with stubs in, say, kernel/kgdbdummy.c, (and just keep the flags / etc
> > in the struct), that's fine with me.
>
> The penalty of keeping them consolidated in a structure isn't so high. I
> prefer to keep them that way. I'll work on reducing number of initialization
> functions, though.

Ok. After talking with George off-list, I think most of ppc_kgdb_init
can go. The only remaining part is the serial plugin, which could be
done ala the kgdb_8250 driver. But I've got a different idea:

arch/ppc/kernel/ppc-stub.c | 3 +--
drivers/serial/kgdb_8250.c | 4 +---
kernel/kgdbstub.c | 2 +-
3 files changed, 3 insertions(+), 6 deletions(-)
--- kernel/kgdbstub.c 2004-01-23 10:03:48.000000000 -0700
+++ kernel/kgdbstub.c 2004-01-23 10:06:19.000000000 -0700
@@ -61,7 +61,7 @@
gdb_breakpoint_t kgdb_break[MAX_BREAKPOINTS];
extern int pid_max;

-struct kgdb_serial *kgdb_serial;
+struct kgdb_serial *kgdb_serial = &kgdb_serial_driver;

int kgdb_initialized = 0;
int kgdb_enter = 0;
--- drivers/serial/kgdb_8250.c 2004-01-23 10:04:57.000000000 -0700
+++ drivers/serial/kgdb_8250.c 2004-01-23 09:57:26.000000000 -0700
@@ -370,7 +370,7 @@
rs_table[i].regshift = serial_req->regshift;
}

-struct kgdb_serial kgdb8250_serial = {
+struct kgdb_serial kgdb_serial_driver = {
.read_char = kgdb8250_read_char,
.write_char = kgdb8250_write_char,
.hook = kgdb8250_hook
@@ -396,8 +396,6 @@
kgdb8250_baud != 115200)
goto errout;

- kgdb_serial = &kgdb8250_serial;
-
return 1;

errout:
--- arch/ppc/kernel/ppc-stub.c 2004-01-23 10:04:46.000000000 -0700
+++ arch/ppc/kernel/ppc-stub.c 2004-01-23 09:57:47.000000000 -0700
@@ -264,7 +264,7 @@
return 0;
}

-struct kgdb_serial kgdbppc_serial = {
+struct kgdb_serial kgdb_serial_driver = {
.read_char = kgdbppc_read_char,
.write_char = kgdbppc_write_char,
.hook = kgdbppc_hook
@@ -272,5 +272,4 @@

void kgdbppc_init(void)
{
- kgdb_serial = &kgdbppc_serial;
}

This does mean that we can only have one serial driver per kernel, but I
don't see this as a problem. This also means that i386 would need
something like PPC's CONFIG_KGDB_TTYSx to pick something other than
ttyS0/115200 to use kgdb on, this early. But, IMHO, that's a small
price to pay.

--
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/