Jonathan Cameron wrote:
> A few minor things inline from a fresh read.
>
> Other than maybe a missing header, the others are all trivial
> and you can make your own minds up.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxx>
>
[snip]
>
> > +static bool extents_contain(struct cxl_dax_region *cxlr_dax,
> > + struct cxl_endpoint_decoder *cxled,
> > + struct range *new_range)
> > +{
> > + struct match_data md = {
> > + .cxled = cxled,
> > + .new_range = new_range,
> > + };
> > +
> > + struct device *extent_device __free(put_device)
> > + = device_find_child(&cxlr_dax->dev, &md, match_contains);
> > + if (!extent_device)
> > + return false;
> > +
> > + return true;
> trivial but could do.
>
> return extent_device != NULL;
Sorry I tend to be more explicit...
[snip]
>
> > +}
>
> > +static int cxlr_rm_extent(struct device *dev, void *data)
> > +{
> > + struct region_extent *region_extent = to_region_extent(dev);
> > + struct range *region_hpa_range = data;
> > +
> > + if (!region_extent)
> > + return 0;
> > +
> > + /*
> > + * Any extent which 'touches' the released range is removed.
>
> Maybe single line comment syntax is fine here.
Ah... yea this is an artifact of refactoring. Later in the series the
comment becomes.
/*
* Any extent which 'touches' the released range is attempted to be
* removed.
*/
So...
>
> > + */
> > + if (range_overlaps(region_hpa_range, ®ion_extent->hpa_range)) {
> > + dev_dbg(dev, "Remove region extent HPA [range 0x%016llx-0x%016llx]\n",
> > + region_extent->hpa_range.start, region_extent->hpa_range.end);
> > + region_rm_extent(region_extent);
> > + }
> > + return 0;
> > +}
>
[snip]
> > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> > index 16e06b59d7f04762ca73a81740b0d6b2487301af..85b30a74a6fa5de1dd99c08c8318edd204e3e19d 100644
> > --- a/drivers/cxl/cxlmem.h
> > +++ b/drivers/cxl/cxlmem.h
>
> Is the xarray header included in here already?
> If not it should be.
Looking around we have been lax in this behavior. cxl.h does not explicitly
include xarray.h either. I agree they both should after this.
Let me send a follow on patch to add it.
Ira
>
> > @@ -506,6 +506,7 @@ static inline struct cxl_dev_state *mbox_to_cxlds(struct cxl_mailbox *cxl_mbox)
> > * @pmem_perf: performance data entry matched to PMEM partition
> > * @nr_dc_region: number of DC regions implemented in the memory device
> > * @dc_region: array containing info about the DC regions
