On Thu, 20 Apr 2023 12:33:50 +0100
Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> wrote:
> On Wed, 19 Apr 2023 13:21:31 -0700
> Dave Jiang <dave.jiang@xxxxxxxxx> wrote:
>
> > Provide a callback function to the CDAT parser in order to parse the Device
> > Scoped Memory Affinity Structure (DSMAS). Each DSMAS structure contains the
> > DPA range and its associated attributes in each entry. See the CDAT
> > specification for details.
> >
> > Coherent Device Attribute Table 1.03 2.1 Device Scoped memory Affinity
> > Structure (DSMAS)
>
> I'm not sure what purpose of this is. If it's just detecting problems
> with the entry because we aren't interested in the content yet, then fine
> but good to make that clear in patch intro.
>
> Maybe I'm missing something!
>
Ah. Got to next patch. Perhaps a forwards reference to that will avoid
anyone else wondering what is going on here!
> Thanks,
>
> Jonathan
>
> >
> > Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
> >
> > ---
> > v3:
> > - Add spec section number. (Alison)
> > - Remove cast from void *. (Alison)
> > - Refactor cxl_port_probe() block. (Alison)
> > - Move CDAT parse to cxl_endpoint_port_probe()
> >
> > v2:
> > - Add DSMAS table size check. (Lukas)
> > - Use local DSMAS header for LE handling.
> > - Remove dsmas lock. (Jonathan)
> > - Fix handle size (Jonathan)
> > - Add LE to host conversion for DSMAS address and length.
> > - Make dsmas_list local
>
>
> > diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> > index 615e0ef6b440..3022bdd52439 100644
> > --- a/drivers/cxl/port.c
> > +++ b/drivers/cxl/port.c
> > @@ -57,6 +57,16 @@ static int discover_region(struct device *dev, void *root)
> > return 0;
> > }
>
> > static int cxl_switch_port_probe(struct cxl_port *port)
> > {
> > struct cxl_hdm *cxlhdm;
> > @@ -125,6 +135,18 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
> > device_for_each_child(&port->dev, root, discover_region);
> > put_device(&root->dev);
> >
> > + if (port->cdat.table) {
> > + LIST_HEAD(dsmas_list);
> > +
> > + rc = cdat_table_parse_dsmas(port->cdat.table,
> > + cxl_dsmas_parse_entry,
> > + (void *)&dsmas_list);
> > + if (rc < 0)
> > + dev_warn(&port->dev, "Failed to parse DSMAS: %d\n", rc);
> > +
> > + dsmas_list_destroy(&dsmas_list);
>
> I'm a little confused here. What's the point? Parse them then throw the info away?
> Maybe a comment if all we are trying to do is warn about CDAT problems.
>
>
> > + }
> > +
> > return 0;
> > }
> >
> >
> >
>
