Re: [PATCH ndctl] cxl: Add list all option to the cxl command

[Matthew Ho <sunfishho12@gmail.com>]

On Thu, Jul 14, 2022 at 12:15:21PM -0700, Davidlohr Bueso wrote:
> On Thu, 14 Jul 2022, Verma, Vishal L wrote:
> 
> > On Thu, 2022-07-14 at 11:27 -0700, sunfishho12@gmail.com wrote:
> > > From: Matthew Ho <sunfishho12@gmail.com>
> > > 
> > > This adds a new subcommand cxl list --all, which is equivalent to cxl
> > > list -MBPEDTHIiu. This addition makes it easier to list all the CXL
> > > devices at once, as one does not need to append a subcommand for each
> > > device. --all is also easier to remember than -MBPEDTHIiu. When
> > > region support is added, this will be updated to include it.
> > > 
> > > Reviewed-by: Adam Manzanares <a.manzanares@samsung.com>
> > > Signed-off-by: Matthew Ho <sunfishho12@gmail.com>
> > > ---
> > >  cxl/filter.h |  1 +
> > >  cxl/list.c   | 15 +++++++++++++++
> > >  2 files changed, 16 insertions(+)
> 
> You also need to the manpage (which I also missed) via:
> 
>      Documentation/cxl/cxl-list.txt
> 
> and of course, same for Vishal's suggestions below.

Ah, will add.

> 
> > 
> > Hi Matthew, thanks for sending this - I agree it gets tedious trying to
> > list multiple things.
> > 
> > However, I think we should do this similar to ndctl-list's --verbose
> > options, where adding the number of 'v's adds in more and more detail.
> 
> Yeah I agree, and was not aware that ndctl has this. Furthermore perf
> tooling also uses this.
> 
> > 
> > For example:
> > 
> > cxl list (default) : Regions and memdevs (this isn't there today, but
> > in a pending series I'm about to send out).
> > 
> > cxl list -v: -RMBDPT
> > 
> > cxl list -vv: -RMBDPTi
> > 
> > cxl list -vvv: -RMBDPTiHI
> 
> Makes sense to me.

Sounds good, will implement it this way.

> > 
> > -u/--human can be excluded from the verbosity levels as that can be
> > passed in if needed, and there may be use cases where it isn't desired
> > (in scripts).
> > 
> > Thoughts on this - do you want to take a shot at implementing it this
> > way?
> 
> Sounds good to me.

Will do.

Thanks for all of the feedback!

Best,
Matthew