Re: [PATCH v1 1/3] cxl: support Type2 when initializing cxl_dev_state

[Alison Schofield <alison.schofield@intel.com>]

On Mon, Feb 23, 2026 at 02:26:31PM +0000, alejandro.lucero-palau@amd.com wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> In preparation for type2 drivers add function and macro for
> differentiating CXL memory expanders (type 3) from CXL device
> accelerators (type 2) helping accel drivers to embed cxl_dev_state
> inside a private struct.
> 
> Update type3 driver for using this same initialization.

I've got 2 simple nits that could be fixed-up upon applying,
and one question below.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

snip

> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index f547d8ac34c7..ad131f053bf5 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -656,6 +656,30 @@ static void detach_memdev(struct work_struct *work)
>  
>  static struct lock_class_key cxl_memdev_key;
>  
> +struct cxl_dev_state *_devm_cxl_dev_state_create(struct device *dev,
> +						 enum cxl_devtype type,
> +						 u64 serial, u16 dvsec,
> +						 size_t size, bool has_mbox)
> +{
> +	struct cxl_dev_state *cxlds = devm_kzalloc(dev, size, GFP_KERNEL);
> +
> +	if (!cxlds)
> +		return NULL;
> +
> +	cxlds->dev = dev;
> +	cxlds->type = type;
> +	cxlds->serial = serial;
> +	cxlds->cxl_dvsec =dvsec;

Nit one: space around =

> +	cxlds->reg_map.host = dev;
> +	cxlds->reg_map.resource = CXL_RESOURCE_NONE;
> +
> +	if (has_mbox)
> +		cxlds->cxl_mbox.host = dev;
> +
> +	return cxlds;
> +}
> +EXPORT_SYMBOL_NS_GPL(_devm_cxl_dev_state_create, "CXL");

Why do we name this with leading underscore when we intend to EXPORT it?
Thinking leading underscores are meant for internal helpers?


> +
>  static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
>  					   const struct file_operations *fops,
>  					   const struct cxl_memdev_attach *attach)
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index e21d744d639b..9daf4bc42692 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -523,6 +523,37 @@ to_cxl_memdev_state(struct cxl_dev_state *cxlds)
>  	return container_of(cxlds, struct cxl_memdev_state, cxlds);
>  }
>  
> +struct cxl_dev_state *_devm_cxl_dev_state_create(struct device *dev,
> +						 enum cxl_devtype type,
> +						 u64 serial, u16 dvsec,
> +						 size_t size, bool has_mbox);
> +
> +/**
> + * cxl_dev_state_create - safely create and cast a cxl dev state embedded in a
> + * driver specific struct.

Nit two: function name is actually devm_cxl_dev_state_create

> + *
> + * @parent: device behind the request
> + * @type: CXL device type
> + * @serial: device identification
> + * @dvsec: dvsec capability offset
> + * @drv_struct: driver struct embedding a cxl_dev_state struct
> + * @member: drv_struct member as cxl_dev_state
> + * @mbox: true if mailbox supported
> + *
> + * Returns a pointer to the drv_struct allocated and embedding a cxl_dev_state
> + * struct initialized.
> + *
> + * Introduced for Type2 driver support.
> + */
> +#define devm_cxl_dev_state_create(parent, type, serial, dvsec, drv_struct, member, mbox)	\
> +	({										\
> +		static_assert(__same_type(struct cxl_dev_state,				\
> +			      ((drv_struct *)NULL)->member));				\
> +		static_assert(offsetof(drv_struct, member) == 0);			\
> +		(drv_struct *)_devm_cxl_dev_state_create(parent, type, serial, dvsec,	\
> +						      sizeof(drv_struct), mbox);	\
> +	})
> +

snip