[Linux-kernel-mentees] [PATCH v3 3/3] lspci: Replace output for bridge with empty range from 'None' to '[empty]'

[skunberg.kelsey at gmail.com (Kelsey Skunberg)]

On Tue, Jun 11, 2019 at 03:25:00PM -0500, Bjorn Helgaas wrote:
> On Thu, May 16, 2019 at 4:23 PM Kelsey Skunberg
> <skunberg.kelsey at gmail.com> wrote:
> >
> > Change output displayed for memory behind bridge when the range is
> > empty to be consistent between each verbosity level. Replace 'None' with
> > '[empty]'. Old and new output examples listed below for each verbosity
> > level.
> >
> > Show_range() is not called unless verbose == true. No output given
> > unless a verbose argument is provided.
> >
> > OLD ouptut for -v and -vv which uses 'None' and -vvv uses '[empty]':
> 
> s/ouptut/output/
> 
> >   Memory behind bridge: None                          # lspci -v
> >   Memory behind bridge: None                          # lspci -vv
> >   Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv
> >
> > NEW output for -v and -vv to also use "[empty]":
> >
> >   Memory behind bridge: [empty]                       # lspci -v
> >   Memory behind bridge: [empty]                       # lspci -vv
> >   Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv
> 
> How about this alternative?  The spec doesn't actually use the term
> "window", but I think in terms of bridge windows to downstream
> devices, and the windows can be either enabled or disabled.
> 
>   Memory behind bridge: Disabled    # lspci -v or lspci -vv
>   Memory behind bridge: Disabled [0x0000e000-0x0000efff]   # lspci -vvv
> 

I like the alternative of using "Disabled". Could it be more suiting to use
"[disabled]" which is also used in show_bases(), show_rom(), and show_htype2()?
Then keep the range format the same. For example:

   Memory behind bridge: [disabled]    # lspci -v or lspci -vv
   Memory behind bridge: 0x0000e000-0x0000efff [disabled]   # lspci -vvv

I would be happy to submit an updated patch for either version thought to read
better.

> > Advantage is consistent output regardless of verbosity level chosen and
> > to simplify the code.
> >
> > Signed-off-by: Kelsey Skunberg <skunberg.kelsey at gmail.com>
> > ---
> >  lspci.c | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/lspci.c b/lspci.c
> > index 7418b07..0c00c91 100644
> > --- a/lspci.c
> > +++ b/lspci.c
> > @@ -376,16 +376,14 @@ show_size(u64 x)
> >  static void
> >  show_range(char *prefix, u64 base, u64 limit, int is_64bit)
> >  {
> > -  if (base > limit && verbose < 3)
> > +  printf("%s:", prefix);
> > +  if (base <= limit || verbose > 2)
> >      {
> > -      printf("%s: None\n", prefix);
> > -      return;
> > +      if (is_64bit)
> > +        printf(" %016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
> > +      else
> > +        printf(" %08x-%08x", (unsigned) base, (unsigned) limit);
> >      }
> > -  printf("%s: ", prefix);
> > -  if (is_64bit)
> > -    printf("%016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
> > -  else
> > -    printf("%08x-%08x", (unsigned) base, (unsigned) limit);
> >    if (base <= limit)
> >      show_size(limit - base + 1);
> >    else
> > --
> > 2.20.1
> >