On Wed, Oct 23, 2024 at 01:19:48PM -0500, Ira Weiny wrote: > Alison Schofield wrote: > > On Fri, Oct 18, 2024 at 02:46:26PM -0500, Ira Weiny wrote: > > > Now that there is a printf specifier for struct range use it to enhance > > > the debug output of CDAT data. > > > > > > Signed-off-by: Ira Weiny <ira.weiny@xxxxxxxxx> > > > --- > > > drivers/cxl/core/cdat.c | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c > > > index ef1621d40f05..438869df241a 100644 > > > --- a/drivers/cxl/core/cdat.c > > > +++ b/drivers/cxl/core/cdat.c > > > @@ -247,8 +247,8 @@ static void update_perf_entry(struct device *dev, struct dsmas_entry *dent, > > > dpa_perf->dpa_range = dent->dpa_range; > > > dpa_perf->qos_class = dent->qos_class; > > > dev_dbg(dev, > > > - "DSMAS: dpa: %#llx qos: %d read_bw: %d write_bw %d read_lat: %d write_lat: %d\n", > > > - dent->dpa_range.start, dpa_perf->qos_class, > > > + "DSMAS: dpa: %pra qos: %d read_bw: %d write_bw %d read_lat: %d write_lat: %d\n", > > > + &dent->dpa_range, dpa_perf->qos_class, > > > dent->coord[ACCESS_COORDINATE_CPU].read_bandwidth, > > > dent->coord[ACCESS_COORDINATE_CPU].write_bandwidth, > > > dent->coord[ACCESS_COORDINATE_CPU].read_latency, > > > @@ -279,8 +279,8 @@ static void cxl_memdev_set_qos_class(struct cxl_dev_state *cxlds, > > > range_contains(&pmem_range, &dent->dpa_range)) > > > update_perf_entry(dev, dent, &mds->pmem_perf); > > > else > > > - dev_dbg(dev, "no partition for dsmas dpa: %#llx\n", > > > - dent->dpa_range.start); > > > + dev_dbg(dev, "no partition for dsmas dpa: %pra\n", > > > + &dent->dpa_range); > > > } > > > } > > > > This is a bit different than what I expected to find as the initial use case > > because it wasn't printing a range. > > The reason this was chosen was I was adding to this code and found the > range to be advantageous while doing so. But the patch was stand alone > in the original DCD series so could be included here. > > > With this change we go from printing only > > the .start to printing the range. > > Yes that is why I mentioned that %pra is used ... "to enhance > the debug output of CDAT data." > > > > > Seems the wording of the dev_ message could > > change too since 'dpa' has been replaced with a 'dpa range'. > > Could be but it made sense to me to read: > > "... dpa [range 0x...-0x...]" > > Because %pra adds 'range'. > > > > > There are a few places that print the range now and can be cleaned up w this > > specifier. Those are the real 'uglies' like this: > > True this is ugly and I would like to clean this up. But the cdat code > was being modified and lead me to this particular call site. But it was > also stand alone enough to be used here. > > > > > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c > > index 223c273c0cd1..85a121b7b2b5 100644 > > --- a/drivers/cxl/core/hdm.c > > +++ b/drivers/cxl/core/hdm.c > > @@ -941,8 +941,8 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld, > > return rc; > > } > > > > - dev_dbg(&port->dev, "decoder%d.%d: range: %#llx-%#llx iw: %d ig: %d\n", > > - port->id, cxld->id, cxld->hpa_range.start, cxld->hpa_range.end, > > + dev_dbg(&port->dev, "decoder%d.%d: range: %pra iw: %d ig: %d\n", > > + port->id, cxld->id, &cxld->hpa_range, > > cxld->interleave_ways, cxld->interleave_granularity); > > > > > > I guess you could (ducks) pick them all up here, or we can leave it > > for a future cleanup, or we can just say no cleanups and we'll use > > %pra going forward only. > > I would say we get the specifier in then look at any clean up which works > for us going forward. Like in this case where I was editing the code > anyway. Thanks for the info. LGTM! Reviewed-by: Alison Schofield <alison.schofield@xxxxxxxxx> > > Ira
