diff mbox series

[v2,1/8] cxl: Add helper function to retrieve a feature entry

Message ID 20250320180450.539-2-shiju.jose@huawei.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series cxl: support CXL memory RAS features | expand

Commit Message

Shiju Jose March 20, 2025, 6:04 p.m. UTC
From: Shiju Jose <shiju.jose@huawei.com>

Add helper function to retrieve a feature entry from the supported
features list, if supported.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Fan Ni <fan.ni@samsung.com>
Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 drivers/cxl/core/core.h     |  2 ++
 drivers/cxl/core/features.c | 23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+)

Comments

Dan Williams March 26, 2025, 9:32 p.m. UTC | #1
shiju.jose@ wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
> 
> Add helper function to retrieve a feature entry from the supported
> features list, if supported.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Fan Ni <fan.ni@samsung.com>
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> ---
>  drivers/cxl/core/core.h     |  2 ++
>  drivers/cxl/core/features.c | 23 +++++++++++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 1803aedb25ca..16bc717376fc 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -123,6 +123,8 @@ int cxl_ras_init(void);
>  void cxl_ras_exit(void);
>  
>  #ifdef CONFIG_CXL_FEATURES
> +struct cxl_feat_entry *cxl_get_feature_entry(struct cxl_dev_state *cxlds,
> +					     const uuid_t *feat_uuid);

It is unfortunate that this naming choice is too similar to
cxl_get_feature(). However, as I go to suggest a new name I find that
this is a duplicate of get_support_feature_info() in Dave's fwctl
series. Just drop this patch in favor of that.
Shiju Jose March 27, 2025, 4:59 p.m. UTC | #2
>-----Original Message-----
>From: Dan Williams <dan.j.williams@intel.com>
>Sent: 26 March 2025 21:33
>To: Shiju Jose <shiju.jose@huawei.com>; linux-cxl@vger.kernel.org;
>dan.j.williams@intel.com; dave@stgolabs.net; Jonathan Cameron
><jonathan.cameron@huawei.com>; dave.jiang@intel.com;
>alison.schofield@intel.com; vishal.l.verma@intel.com; ira.weiny@intel.com;
>david@redhat.com; Vilas.Sridharan@amd.com
>Cc: linux-edac@vger.kernel.org; linux-acpi@vger.kernel.org; linux-
>mm@kvack.org; linux-kernel@vger.kernel.org; bp@alien8.de;
>tony.luck@intel.com; rafael@kernel.org; lenb@kernel.org;
>mchehab@kernel.org; leo.duran@amd.com; Yazen.Ghannam@amd.com;
>rientjes@google.com; jiaqiyan@google.com; Jon.Grimm@amd.com;
>dave.hansen@linux.intel.com; naoya.horiguchi@nec.com;
>james.morse@arm.com; jthoughton@google.com; somasundaram.a@hpe.com;
>erdemaktas@google.com; pgonda@google.com; duenwen@google.com;
>gthelen@google.com; wschwartz@amperecomputing.com;
>dferguson@amperecomputing.com; wbs@os.amperecomputing.com;
>nifan.cxl@gmail.com; tanxiaofei <tanxiaofei@huawei.com>; Zengtao (B)
><prime.zeng@hisilicon.com>; Roberto Sassu <roberto.sassu@huawei.com>;
>kangkang.shen@futurewei.com; wanghuiqiang <wanghuiqiang@huawei.com>;
>Linuxarm <linuxarm@huawei.com>; Shiju Jose <shiju.jose@huawei.com>
>Subject: Re: [PATCH v2 1/8] cxl: Add helper function to retrieve a feature entry
>
>shiju.jose@ wrote:
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> Add helper function to retrieve a feature entry from the supported
>> features list, if supported.
>>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Reviewed-by: Fan Ni <fan.ni@samsung.com>
>> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>> ---
>>  drivers/cxl/core/core.h     |  2 ++
>>  drivers/cxl/core/features.c | 23 +++++++++++++++++++++++
>>  2 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h index
>> 1803aedb25ca..16bc717376fc 100644
>> --- a/drivers/cxl/core/core.h
>> +++ b/drivers/cxl/core/core.h
>> @@ -123,6 +123,8 @@ int cxl_ras_init(void);  void cxl_ras_exit(void);
>>
>>  #ifdef CONFIG_CXL_FEATURES
>> +struct cxl_feat_entry *cxl_get_feature_entry(struct cxl_dev_state *cxlds,
>> +					     const uuid_t *feat_uuid);
>
>It is unfortunate that this naming choice is too similar to cxl_get_feature().
>However, as I go to suggest a new name I find that this is a duplicate of
>get_support_feature_info() in Dave's fwctl series. Just drop this patch in favor of
>that.

Hi Dan,

I am fine to use get_support_feature_info() for the EDAC features. 
However this function is defined as static in the fwctl series and 
takes struct fwctl_rpc_cxl * as input for RPC instead of  uuid_t *
as in cxl_get_feature_entry().

static struct cxl_feat_entry *
get_support_feature_info(struct cxl_features_state *cxlfs,
			 const struct fwctl_rpc_cxl *rpc_in)

Can you suggest how to use get_support_feature_info() from within the CXL driver 
to retrieve a supported feature entry (for e.g an EDAC feature)?

>

Thanks,
Shiju
diff mbox series

Patch

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 1803aedb25ca..16bc717376fc 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -123,6 +123,8 @@  int cxl_ras_init(void);
 void cxl_ras_exit(void);
 
 #ifdef CONFIG_CXL_FEATURES
+struct cxl_feat_entry *cxl_get_feature_entry(struct cxl_dev_state *cxlds,
+					     const uuid_t *feat_uuid);
 size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
 		       enum cxl_get_feat_selection selection,
 		       void *feat_out, size_t feat_out_size, u16 offset,
diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 048ba4fc3538..202c8c21930c 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -203,6 +203,29 @@  int devm_cxl_setup_features(struct cxl_dev_state *cxlds)
 }
 EXPORT_SYMBOL_NS_GPL(devm_cxl_setup_features, "CXL");
 
+struct cxl_feat_entry *cxl_get_feature_entry(struct cxl_dev_state *cxlds,
+					     const uuid_t *feat_uuid)
+{
+	struct cxl_features_state *cxlfs = to_cxlfs(cxlds);
+	struct cxl_feat_entry *feat_entry;
+	int count;
+
+	if (!cxlfs || !cxlfs->entries || !cxlfs->entries->num_features)
+		return NULL;
+
+	/*
+	 * Retrieve the feature entry from the supported features list,
+	 * if the feature is supported.
+	 */
+	feat_entry = cxlfs->entries->ent;
+	for (count = 0; count < cxlfs->entries->num_features; count++, feat_entry++) {
+		if (uuid_equal(&feat_entry->uuid, feat_uuid))
+			return feat_entry;
+	}
+
+	return NULL;
+}
+
 size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
 		       enum cxl_get_feat_selection selection,
 		       void *feat_out, size_t feat_out_size, u16 offset,