Hi, On Tue, Jun 18, 2024 at 4:24 AM Daniel Thompson <daniel.thomp...@linaro.org> wrote: > > On Mon, Jun 17, 2024 at 05:34:36PM -0700, Douglas Anderson wrote: > > The documentation for the variouis "md" commands was inconsistent > > about documenting the command arguments. It was also hard to figure > > out what the differences between the "phys", "raw", and "symbolic" > > versions was. > > > > Update the help strings to make things more obvious. > > > > As part of this, add "bogus" commands to the table for "mdW" and > > "mdWcN" so we don't have to obscurely reference them in the normal > > "md" help. These bogus commands don't really hurt since kdb_md() > > validates argv[0] enough. > > > > Signed-off-by: Douglas Anderson <diand...@chromium.org> > > --- > > > > kernel/debug/kdb/kdb_main.c | 39 ++++++++++++++++++++++--------------- > > 1 file changed, 23 insertions(+), 16 deletions(-) > > > > diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c > > index cbeb203785b4..47e037c3c002 100644 > > --- a/kernel/debug/kdb/kdb_main.c > > +++ b/kernel/debug/kdb/kdb_main.c > > @@ -1516,14 +1516,9 @@ static int kdb_mdr(unsigned long addr, unsigned int > > count) > > } > > > > /* > > - * kdb_md - This function implements the 'md', 'md1', 'md2', 'md4', > > - * 'md8' 'mdr' and 'mds' commands. > > + * kdb_md - This function implements the guts of the various 'md' commands. > > * > > - * md|mds [<addr arg> [<line count> [<radix>]]] > > - * mdWcN [<addr arg> [<line count> [<radix>]]] > > - * where W = is the width (1, 2, 4 or 8) and N is the count. > > - * for eg., md1c20 reads 20 bytes, 1 at a time. > > - * mdr <addr arg>,<byte count> > > + * See the kdb help for syntax. > > */ > > static void kdb_md_line(const char *fmtstr, unsigned long addr, > > int symbolic, int nosect, int bytesperword, > > @@ -2677,26 +2672,38 @@ EXPORT_SYMBOL_GPL(kdb_unregister); > > static kdbtab_t maintab[] = { > > { .name = "md", > > .func = kdb_md, > > - .usage = "<vaddr>", > > - .help = "Display Memory Contents, also mdWcN, e.g. md8c1", > > + .usage = "<vaddr> [<lines> [<radix>]]", > > + .help = "Display RAM using BYTESPERWORD; show MDCOUNT lines", > > I'd prefer "memory" over "RAM" because it's what the mnemonic is > abbreviating. This applies to all of the below but I won't be adding a > "same here" for all of them. > > Where we have to crush something to fit into one line we'd than have to > break the pattern and choose from thing like: > > 1. Show memory > 2. Display RAM > 3. Display mem > > Personally I prefer #1 but could probably cope with #2.
I'm not dead set on RAM, but at least for me RAM was more clarifying. Specifically when it said "memory" I always assumed it would take in any memory address and when I first looked at this I tried to figure out why memory addresses in the IO space didn't work with these commands. I figured I was holding it wrong only to find out that the commands specifically limit you to just the RAM range of memory addresses. That being said, I don't feel strongly so if you really like "memory" I'll change it back. > > .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS, > > }, > > - { .name = "mdr", > > + { .name = "mdW", > > .func = kdb_md, > > - .usage = "<vaddr> <bytes>", > > - .help = "Display Raw Memory", > > + .usage = "<vaddr> [<lines> [<radix>]]", > > + .help = "Display RAM using word size (W) of 1, 2, 4, or 8", > > We need an "e.g. md8" in here somewhere. Otherwise it is not at all > obvious that W is a wildcard. > > I guess that alternatively you could also try naming the command with > hint that W is a wild card (what happens if you register a command > called md<W>?). > > > > + .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS, > > + }, > > + { .name = "mdWcN", > > + .func = kdb_md, > > + .usage = "<vaddr> [<lines> [<radix>]]", > > + .help = "Display RAM using word size (W); show N words", > > Same here. Sure, so: .name = "md<W>", .help = "Display RAM using word size 1, 2, 4, or 8; e.g. md8", .name = "md<W>c<N>", .help = "Display RAM using word size W; show N words; e.g. md4c6", ...or changing RAM to "memory" if you don't buy my argument above. We're definitely ending up over the 80 character mark here, but I assume that's OK. We were even before my change. I'll assume that I don't need the "e.g." for all the followup (mdp, mdi) variants introduced in later patches? Random question: for the mdWcN variant, should I make specifying <lines> illegal? It's pretty silly to let the user specify a word count and then immediately override it. In that case, do I bump "<radix>" to the 2nd argument or just don't allow "<radix>" for mdWcN? That would need to be done in a later patch, obviously... -Doug _______________________________________________ Kgdb-bugreport mailing list Kgdb-bugreport@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport