Re: [PATCH 6/6] cxl/mem: Introduce a memdev creation ->probe() operation

[<dan.j.williams@intel.com>]

Cheatham, Benjamin wrote:
> [snip]
> 
> > +static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
> > +{
> > +	struct cxl_memdev *ret = cxlmd;
> > +	int rc;
> > +
> > +	/*
> > +	 * If ops is provided fail if the driver is not attached upon
> > +	 * return. The ->endpoint ERR_PTR may have a more precise error
> > +	 * code to convey. Note that failure here could be the result of
> > +	 * a race to teardown the CXL port topology. I.e.
> > +	 * cxl_mem_probe() could have succeeded and then cxl_mem unbound
> > +	 * before the lock is acquired.
> > +	 */
> > +	guard(device)(&cxlmd->dev);
> > +	if (cxlmd->ops && !cxlmd->dev.driver) {
> > +		ret = ERR_PTR(-ENXIO);
> > +		if (IS_ERR(cxlmd->endpoint))
> > +			ret = ERR_CAST(cxlmd->endpoint);
> > +		cxl_memdev_unregister(cxlmd);
> > +		return ret;
> > +	}
> 
> Two minor gripes here:
> 
> 1. The cxlmd->endpoint portion of this is unused. I think the idea is since this is prep work for
> Alejandro's set to just put in here, but I would argue it should be introduced in his set since that's
> where it's actually used.

Fair enough, a follow-on changes can add that back.

> 2. I don't particularly like drivers having to provide a cxlmd->ops to
> automatically unregister the device on cxl_mem probe failure. It's
> probably not likely, but it possible that a driver wouldn't have any
> extra set up to do for CXL.mem but still needs to fallback to PCIe
> mode if it can't be properly set up.
> 
> I don't want to nit-pick without alternatives, so my first thought was
> a flag passed into devm_cxl_add_memdev() (and eventually this
> function) that dictates whether failure to attach to cxl_mem is a
> failure condition. Then that flag replaces the cxlmd->ops check. It
> may not be worth adding that degree of versatility before anyone wants
> it though, so I'm fine with the above approach if you feel it's the
> way to go.

So the problem is that cxl_pci definitely does not care about immediate
cxl_memdev attachment the way that a Type-2 driver probably should. The
way I would handle the case of drivers that only want probe failures and
nothing else is to just provide a common default @ops implementation
that those drivers can use to get that behavior.

That is functionally equivalent to a new devm_cxl_add_memdev() flag, and
is something that can come later when such a driver arrives.