[PATCH 0/2] cxl: DT property accessor cleanups

[PATCH 0/2] cxl: DT property accessor cleanups

[Rob Herring (Arm)]

This short series replaces the custom DT property accessors in the CXL 
driver with the common ones. The one difference is dumping the 
properties read out is removed. If that feature is really needed, then 
it should be implemented in the DT core for everyone. 

This is part of a larger effort to remove DT functions leaking DT data 
(e.g. of_get_property()).

Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
Rob Herring (Arm) (2):
      cxl: Drop printing of DT properties
      cxl: Use of_property_ accessor functions

 drivers/misc/cxl/of.c  | 207 ++++++-------------------------------------------
 drivers/misc/cxl/pci.c |  32 ++++----
 2 files changed, 36 insertions(+), 203 deletions(-)
---
base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
change-id: 20240729-dt-cxl-cleanup-eaf8185a99fc

Best regards,
-- 
Rob Herring (Arm) <robh@kernel.org>

[PATCH 1/2] cxl: Drop printing of DT properties

[Rob Herring (Arm)]

There's little reason to dump DT property values when they can be read
at any time from the DT in /proc/device-tree. If such a feature is
needed, then it really should be implemented in the DT core such that
any module/driver can use it.

Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
 drivers/misc/cxl/of.c | 101 ++------------------------------------------------
 1 file changed, 3 insertions(+), 98 deletions(-)

diff --git a/drivers/misc/cxl/of.c b/drivers/misc/cxl/of.c
index bcc005dff1c0..21ecce0f9403 100644
--- a/drivers/misc/cxl/of.c
+++ b/drivers/misc/cxl/of.c
@@ -19,8 +19,6 @@ static const __be32 *read_prop_string(const struct device_node *np,
 	const __be32 *prop;
 
 	prop = of_get_property(np, prop_name, NULL);
-	if (cxl_verbose && prop)
-		pr_info("%s: %s\n", prop_name, (char *) prop);
 	return prop;
 }
 
@@ -32,8 +30,6 @@ static const __be32 *read_prop_dword(const struct device_node *np,
 	prop = of_get_property(np, prop_name, NULL);
 	if (prop)
 		*val = be32_to_cpu(prop[0]);
-	if (cxl_verbose && prop)
-		pr_info("%s: %#x (%u)\n", prop_name, *val, *val);
 	return prop;
 }
 
@@ -45,8 +41,6 @@ static const __be64 *read_prop64_dword(const struct device_node *np,
 	prop = of_get_property(np, prop_name, NULL);
 	if (prop)
 		*val = be64_to_cpu(prop[0]);
-	if (cxl_verbose && prop)
-		pr_info("%s: %#llx (%llu)\n", prop_name, *val, *val);
 	return prop;
 }
 
@@ -100,9 +94,6 @@ static int read_phys_addr(struct device_node *np, char *prop_name,
 					type, prop_name);
 				return -EINVAL;
 			}
-			if (cxl_verbose)
-				pr_info("%s: %#x %#llx (size %#llx)\n",
-					prop_name, type, addr, size);
 		}
 	}
 	return 0;
@@ -139,27 +130,13 @@ int cxl_of_read_afu_handle(struct cxl_afu *afu, struct device_node *afu_np)
 
 int cxl_of_read_afu_properties(struct cxl_afu *afu, struct device_node *np)
 {
-	int i, len, rc;
-	char *p;
+	int i, rc;
 	const __be32 *prop;
 	u16 device_id, vendor_id;
 	u32 val = 0, class_code;
 
 	/* Properties are read in the same order as listed in PAPR */
 
-	if (cxl_verbose) {
-		pr_info("Dump of the 'ibm,coherent-platform-function' node properties:\n");
-
-		prop = of_get_property(np, "compatible", &len);
-		i = 0;
-		while (i < len) {
-			p = (char *) prop + i;
-			pr_info("compatible: %s\n", p);
-			i += strlen(p) + 1;
-		}
-		read_prop_string(np, "name");
-	}
-
 	rc = read_phys_addr(np, "reg", afu);
 	if (rc)
 		return rc;
@@ -173,19 +150,10 @@ int cxl_of_read_afu_properties(struct cxl_afu *afu, struct device_node *np)
 	else
 		afu->psa = true;
 
-	if (cxl_verbose) {
-		read_prop_string(np, "ibm,loc-code");
-		read_prop_string(np, "device_type");
-	}
-
 	read_prop_dword(np, "ibm,#processes", &afu->max_procs_virtualised);
 
-	if (cxl_verbose) {
-		read_prop_dword(np, "ibm,scratchpad-size", &val);
-		read_prop_dword(np, "ibm,programmable", &val);
-		read_prop_string(np, "ibm,phandle");
+	if (cxl_verbose)
 		read_vpd(NULL, afu);
-	}
 
 	read_prop_dword(np, "ibm,max-ints-per-process", &afu->guest->max_ints);
 	afu->irqs_max = afu->guest->max_ints;
@@ -199,17 +167,9 @@ int cxl_of_read_afu_properties(struct cxl_afu *afu, struct device_node *np)
 		afu->pp_irqs--;
 	}
 
-	if (cxl_verbose) {
-		read_prop_dword(np, "ibm,max-ints", &val);
-		read_prop_dword(np, "ibm,vpd-size", &val);
-	}
-
 	read_prop64_dword(np, "ibm,error-buffer-size", &afu->eb_len);
 	afu->eb_offset = 0;
 
-	if (cxl_verbose)
-		read_prop_dword(np, "ibm,config-record-type", &val);
-
 	read_prop64_dword(np, "ibm,config-record-size", &afu->crs_len);
 	afu->crs_offset = 0;
 
@@ -235,15 +195,6 @@ int cxl_of_read_afu_properties(struct cxl_afu *afu, struct device_node *np)
 					i, class_code);
 			}
 		}
-
-		read_prop_dword(np, "ibm,function-number", &val);
-		read_prop_dword(np, "ibm,privileged-function", &val);
-		read_prop_dword(np, "vendor-id", &val);
-		read_prop_dword(np, "device-id", &val);
-		read_prop_dword(np, "revision-id", &val);
-		read_prop_dword(np, "class-code", &val);
-		read_prop_dword(np, "subsystem-vendor-id", &val);
-		read_prop_dword(np, "subsystem-id", &val);
 	}
 	/*
 	 * if "ibm,process-mmio" doesn't exist then per-process mmio is
@@ -256,12 +207,6 @@ int cxl_of_read_afu_properties(struct cxl_afu *afu, struct device_node *np)
 	else
 		afu->pp_psa = false;
 
-	if (cxl_verbose) {
-		read_prop_dword(np, "ibm,supports-aur", &val);
-		read_prop_dword(np, "ibm,supports-csrp", &val);
-		read_prop_dword(np, "ibm,supports-prr", &val);
-	}
-
 	prop = read_prop_dword(np, "ibm,function-error-interrupt", &val);
 	if (prop)
 		afu->serr_hwirq = val;
@@ -343,49 +288,15 @@ int cxl_of_read_adapter_handle(struct cxl *adapter, struct device_node *np)
 
 int cxl_of_read_adapter_properties(struct cxl *adapter, struct device_node *np)
 {
-	int rc, len, naddr, i;
-	char *p;
+	int rc;
 	const __be32 *prop;
 	u32 val = 0;
 
 	/* Properties are read in the same order as listed in PAPR */
 
-	naddr = of_n_addr_cells(np);
-
-	if (cxl_verbose) {
-		pr_info("Dump of the 'ibm,coherent-platform-facility' node properties:\n");
-
-		read_prop_dword(np, "#address-cells", &val);
-		read_prop_dword(np, "#size-cells", &val);
-
-		prop = of_get_property(np, "compatible", &len);
-		i = 0;
-		while (i < len) {
-			p = (char *) prop + i;
-			pr_info("compatible: %s\n", p);
-			i += strlen(p) + 1;
-		}
-		read_prop_string(np, "name");
-		read_prop_string(np, "model");
-
-		prop = of_get_property(np, "reg", NULL);
-		if (prop) {
-			pr_info("reg: addr:%#llx size:%#x\n",
-				of_read_number(prop, naddr),
-				be32_to_cpu(prop[naddr]));
-		}
-
-		read_prop_string(np, "ibm,loc-code");
-	}
-
 	if ((rc = read_adapter_irq_config(adapter, np)))
 		return rc;
 
-	if (cxl_verbose) {
-		read_prop_string(np, "device_type");
-		read_prop_string(np, "ibm,phandle");
-	}
-
 	prop = read_prop_dword(np, "ibm,caia-version", &val);
 	if (prop) {
 		adapter->caia_major = (val & 0xFF00) >> 8;
@@ -411,12 +322,6 @@ int cxl_of_read_adapter_properties(struct cxl *adapter, struct device_node *np)
 	if (prop)
 		adapter->guest->device = val;
 
-	if (cxl_verbose) {
-		read_prop_dword(np, "ibm,privileged-facility", &val);
-		read_prop_dword(np, "revision-id", &val);
-		read_prop_dword(np, "class-code", &val);
-	}
-
 	prop = read_prop_dword(np, "subsystem-vendor-id", &val);
 	if (prop)
 		adapter->guest->subsystem_vendor = val;

-- 
2.43.0

[PATCH 2/2] cxl: Use of_property_ accessor functions

[Rob Herring (Arm)]

The CXL driver has its own custom implementations of typed DT property
accessors. Replace the custom property accessor functions with the
common DT property functions.

This clean-up is part of a larger effort to remove of_get_property() and
other DT functions which leak pointers to DT node and property data.

Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
 drivers/misc/cxl/of.c  | 106 ++++++++++---------------------------------------
 drivers/misc/cxl/pci.c |  32 ++++++---------
 2 files changed, 33 insertions(+), 105 deletions(-)

diff --git a/drivers/misc/cxl/of.c b/drivers/misc/cxl/of.c
index 21ecce0f9403..03633cccd043 100644
--- a/drivers/misc/cxl/of.c
+++ b/drivers/misc/cxl/of.c
@@ -7,59 +7,12 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_platform.h>
 
 #include "cxl.h"
 
-
-static const __be32 *read_prop_string(const struct device_node *np,
-				const char *prop_name)
-{
-	const __be32 *prop;
-
-	prop = of_get_property(np, prop_name, NULL);
-	return prop;
-}
-
-static const __be32 *read_prop_dword(const struct device_node *np,
-				const char *prop_name, u32 *val)
-{
-	const __be32 *prop;
-
-	prop = of_get_property(np, prop_name, NULL);
-	if (prop)
-		*val = be32_to_cpu(prop[0]);
-	return prop;
-}
-
-static const __be64 *read_prop64_dword(const struct device_node *np,
-				const char *prop_name, u64 *val)
-{
-	const __be64 *prop;
-
-	prop = of_get_property(np, prop_name, NULL);
-	if (prop)
-		*val = be64_to_cpu(prop[0]);
-	return prop;
-}
-
-
-static int read_handle(struct device_node *np, u64 *handle)
-{
-	const __be32 *prop;
-	u64 size;
-
-	/* Get address and size of the node */
-	prop = of_get_address(np, 0, &size, NULL);
-	if (size)
-		return -EINVAL;
-
-	/* Helper to read a big number; size is in cells (not bytes) */
-	*handle = of_read_number(prop, of_n_addr_cells(np));
-	return 0;
-}
-
 static int read_phys_addr(struct device_node *np, char *prop_name,
 			struct cxl_afu *afu)
 {
@@ -121,17 +74,12 @@ static int read_vpd(struct cxl *adapter, struct cxl_afu *afu)
 
 int cxl_of_read_afu_handle(struct cxl_afu *afu, struct device_node *afu_np)
 {
-	if (read_handle(afu_np, &afu->guest->handle))
-		return -EINVAL;
-	pr_devel("AFU handle: 0x%.16llx\n", afu->guest->handle);
-
-	return 0;
+	return of_property_read_reg(afu_np, 0, &afu->guest->handle, NULL);
 }
 
 int cxl_of_read_afu_properties(struct cxl_afu *afu, struct device_node *np)
 {
 	int i, rc;
-	const __be32 *prop;
 	u16 device_id, vendor_id;
 	u32 val = 0, class_code;
 
@@ -150,16 +98,15 @@ int cxl_of_read_afu_properties(struct cxl_afu *afu, struct device_node *np)
 	else
 		afu->psa = true;
 
-	read_prop_dword(np, "ibm,#processes", &afu->max_procs_virtualised);
+	of_property_read_u32(np, "ibm,#processes", &afu->max_procs_virtualised);
 
 	if (cxl_verbose)
 		read_vpd(NULL, afu);
 
-	read_prop_dword(np, "ibm,max-ints-per-process", &afu->guest->max_ints);
+	of_property_read_u32(np, "ibm,max-ints-per-process", &afu->guest->max_ints);
 	afu->irqs_max = afu->guest->max_ints;
 
-	prop = read_prop_dword(np, "ibm,min-ints-per-process", &afu->pp_irqs);
-	if (prop) {
+	if (!of_property_read_u32(np, "ibm,min-ints-per-process", &afu->pp_irqs)) {
 		/* One extra interrupt for the PSL interrupt is already
 		 * included. Remove it now to keep only AFU interrupts and
 		 * match the native case.
@@ -167,13 +114,13 @@ int cxl_of_read_afu_properties(struct cxl_afu *afu, struct device_node *np)
 		afu->pp_irqs--;
 	}
 
-	read_prop64_dword(np, "ibm,error-buffer-size", &afu->eb_len);
+	of_property_read_u64(np, "ibm,error-buffer-size", &afu->eb_len);
 	afu->eb_offset = 0;
 
-	read_prop64_dword(np, "ibm,config-record-size", &afu->crs_len);
+	of_property_read_u64(np, "ibm,config-record-size", &afu->crs_len);
 	afu->crs_offset = 0;
 
-	read_prop_dword(np, "ibm,#config-records", &afu->crs_num);
+	of_property_read_u32(np, "ibm,#config-records", &afu->crs_num);
 
 	if (cxl_verbose) {
 		for (i = 0; i < afu->crs_num; i++) {
@@ -201,14 +148,12 @@ int cxl_of_read_afu_properties(struct cxl_afu *afu, struct device_node *np)
 	 * not supported
 	 */
 	val = 0;
-	prop = read_prop_dword(np, "ibm,process-mmio", &val);
-	if (prop && val == 1)
+	if (!of_property_read_u32(np, "ibm,process-mmio", &val) && val == 1)
 		afu->pp_psa = true;
 	else
 		afu->pp_psa = false;
 
-	prop = read_prop_dword(np, "ibm,function-error-interrupt", &val);
-	if (prop)
+	if (!of_property_read_u32(np, "ibm,function-error-interrupt", &val))
 		afu->serr_hwirq = val;
 
 	pr_devel("AFU handle: %#llx\n", afu->guest->handle);
@@ -279,17 +224,13 @@ static int read_adapter_irq_config(struct cxl *adapter, struct device_node *np)
 
 int cxl_of_read_adapter_handle(struct cxl *adapter, struct device_node *np)
 {
-	if (read_handle(np, &adapter->guest->handle))
-		return -EINVAL;
-	pr_devel("Adapter handle: 0x%.16llx\n", adapter->guest->handle);
-
-	return 0;
+	return of_property_read_reg(np, 0, &adapter->guest->handle, NULL);
 }
 
 int cxl_of_read_adapter_properties(struct cxl *adapter, struct device_node *np)
 {
 	int rc;
-	const __be32 *prop;
+	const char *p;
 	u32 val = 0;
 
 	/* Properties are read in the same order as listed in PAPR */
@@ -297,37 +238,30 @@ int cxl_of_read_adapter_properties(struct cxl *adapter, struct device_node *np)
 	if ((rc = read_adapter_irq_config(adapter, np)))
 		return rc;
 
-	prop = read_prop_dword(np, "ibm,caia-version", &val);
-	if (prop) {
+	if (!of_property_read_u32(np, "ibm,caia-version", &val)) {
 		adapter->caia_major = (val & 0xFF00) >> 8;
 		adapter->caia_minor = val & 0xFF;
 	}
 
-	prop = read_prop_dword(np, "ibm,psl-revision", &val);
-	if (prop)
+	if (!of_property_read_u32(np, "ibm,psl-revision", &val))
 		adapter->psl_rev = val;
 
-	prop = read_prop_string(np, "status");
-	if (prop) {
-		adapter->guest->status = kasprintf(GFP_KERNEL, "%s", (char *) prop);
+	if (!of_property_read_string(np, "status", &p)) {
+		adapter->guest->status = kasprintf(GFP_KERNEL, "%s", p);
 		if (adapter->guest->status == NULL)
 			return -ENOMEM;
 	}
 
-	prop = read_prop_dword(np, "vendor-id", &val);
-	if (prop)
+	if (!of_property_read_u32(np, "vendor-id", &val))
 		adapter->guest->vendor = val;
 
-	prop = read_prop_dword(np, "device-id", &val);
-	if (prop)
+	if (!of_property_read_u32(np, "device-id", &val))
 		adapter->guest->device = val;
 
-	prop = read_prop_dword(np, "subsystem-vendor-id", &val);
-	if (prop)
+	if (!of_property_read_u32(np, "subsystem-vendor-id", &val))
 		adapter->guest->subsystem_vendor = val;
 
-	prop = read_prop_dword(np, "subsystem-id", &val);
-	if (prop)
+	if (!of_property_read_u32(np, "subsystem-id", &val))
 		adapter->guest->subsystem = val;
 
 	if (cxl_verbose)
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 4cf9e7c42a24..3d52f9b92d0d 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -363,17 +363,17 @@ int cxl_calc_capp_routing(struct pci_dev *dev, u64 *chipid,
 {
 	int rc;
 	struct device_node *np;
-	const __be32 *prop;
+	u32 id;
 
 	if (!(np = pnv_pci_get_phb_node(dev)))
 		return -ENODEV;
 
-	while (np && !(prop = of_get_property(np, "ibm,chip-id", NULL)))
+	while (np && of_property_read_u32(np, "ibm,chip-id", &id))
 		np = of_get_next_parent(np);
 	if (!np)
 		return -ENODEV;
 
-	*chipid = be32_to_cpup(prop);
+	*chipid = id;
 
 	rc = get_phb_index(np, phb_index);
 	if (rc) {
@@ -398,32 +398,26 @@ static DEFINE_MUTEX(indications_mutex);
 static int get_phb_indications(struct pci_dev *dev, u64 *capiind, u64 *asnind,
 			       u64 *nbwind)
 {
-	static u64 nbw, asn, capi = 0;
+	static u32 val[3];
 	struct device_node *np;
-	const __be32 *prop;
 
 	mutex_lock(&indications_mutex);
-	if (!capi) {
+	if (!val[0]) {
 		if (!(np = pnv_pci_get_phb_node(dev))) {
 			mutex_unlock(&indications_mutex);
 			return -ENODEV;
 		}
 
-		prop = of_get_property(np, "ibm,phb-indications", NULL);
-		if (!prop) {
-			nbw = 0x0300UL; /* legacy values */
-			asn = 0x0400UL;
-			capi = 0x0200UL;
-		} else {
-			nbw = (u64)be32_to_cpu(prop[2]);
-			asn = (u64)be32_to_cpu(prop[1]);
-			capi = (u64)be32_to_cpu(prop[0]);
+		if (of_property_read_u32_array(np, "ibm,phb-indications", val, 3)) {
+			val[2] = 0x0300UL; /* legacy values */
+			val[1] = 0x0400UL;
+			val[0] = 0x0200UL;
 		}
 		of_node_put(np);
 	}
-	*capiind = capi;
-	*asnind = asn;
-	*nbwind = nbw;
+	*capiind = val[0];
+	*asnind = val[1];
+	*nbwind = val[2];
 	mutex_unlock(&indications_mutex);
 	return 0;
 }
@@ -605,7 +599,7 @@ static void cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
 
 	/* Do not fail when CAPP timebase sync is not supported by OPAL */
 	of_node_get(np);
-	if (! of_get_property(np, "ibm,capp-timebase-sync", NULL)) {
+	if (!of_property_present(np, "ibm,capp-timebase-sync")) {
 		of_node_put(np);
 		dev_info(&dev->dev, "PSL timebase inactive: OPAL support missing\n");
 		return;

-- 
2.43.0

Re: [PATCH 1/2] cxl: Drop printing of DT properties

[Andrew Donnellan]

On Mon, 2024-07-29 at 16:36 -0600, Rob Herring (Arm) wrote:
> There's little reason to dump DT property values when they can be
> read
> at any time from the DT in /proc/device-tree. If such a feature is
> needed, then it really should be implemented in the DT core such that
> any module/driver can use it.
> 
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>

I'm okay with this, I think this level of dmesg verbosity was more
useful in the early days of implementing guest support than it is now,
and I'm hoping to rip the cxl driver out completely at some point
anyway.

Acked-by: Andrew Donnellan <ajd@linux.ibm.com>

> ---
>  drivers/misc/cxl/of.c | 101 ++--------------------------------------
> ----------
>  1 file changed, 3 insertions(+), 98 deletions(-)
> 
> diff --git a/drivers/misc/cxl/of.c b/drivers/misc/cxl/of.c
> index bcc005dff1c0..21ecce0f9403 100644
> --- a/drivers/misc/cxl/of.c
> +++ b/drivers/misc/cxl/of.c
> @@ -19,8 +19,6 @@ static const __be32 *read_prop_string(const struct
> device_node *np,
>  	const __be32 *prop;
>  
>  	prop = of_get_property(np, prop_name, NULL);
> -	if (cxl_verbose && prop)
> -		pr_info("%s: %s\n", prop_name, (char *) prop);
>  	return prop;
>  }
>  
> @@ -32,8 +30,6 @@ static const __be32 *read_prop_dword(const struct
> device_node *np,
>  	prop = of_get_property(np, prop_name, NULL);
>  	if (prop)
>  		*val = be32_to_cpu(prop[0]);
> -	if (cxl_verbose && prop)
> -		pr_info("%s: %#x (%u)\n", prop_name, *val, *val);
>  	return prop;
>  }
>  
> @@ -45,8 +41,6 @@ static const __be64 *read_prop64_dword(const struct
> device_node *np,
>  	prop = of_get_property(np, prop_name, NULL);
>  	if (prop)
>  		*val = be64_to_cpu(prop[0]);
> -	if (cxl_verbose && prop)
> -		pr_info("%s: %#llx (%llu)\n", prop_name, *val,
> *val);
>  	return prop;
>  }
>  
> @@ -100,9 +94,6 @@ static int read_phys_addr(struct device_node *np,
> char *prop_name,
>  					type, prop_name);
>  				return -EINVAL;
>  			}
> -			if (cxl_verbose)
> -				pr_info("%s: %#x %#llx (size
> %#llx)\n",
> -					prop_name, type, addr,
> size);
>  		}
>  	}
>  	return 0;
> @@ -139,27 +130,13 @@ int cxl_of_read_afu_handle(struct cxl_afu *afu,
> struct device_node *afu_np)
>  
>  int cxl_of_read_afu_properties(struct cxl_afu *afu, struct
> device_node *np)
>  {
> -	int i, len, rc;
> -	char *p;
> +	int i, rc;
>  	const __be32 *prop;
>  	u16 device_id, vendor_id;
>  	u32 val = 0, class_code;
>  
>  	/* Properties are read in the same order as listed in PAPR
> */
>  
> -	if (cxl_verbose) {
> -		pr_info("Dump of the 'ibm,coherent-platform-
> function' node properties:\n");
> -
> -		prop = of_get_property(np, "compatible", &len);
> -		i = 0;
> -		while (i < len) {
> -			p = (char *) prop + i;
> -			pr_info("compatible: %s\n", p);
> -			i += strlen(p) + 1;
> -		}
> -		read_prop_string(np, "name");
> -	}
> -
>  	rc = read_phys_addr(np, "reg", afu);
>  	if (rc)
>  		return rc;
> @@ -173,19 +150,10 @@ int cxl_of_read_afu_properties(struct cxl_afu
> *afu, struct device_node *np)
>  	else
>  		afu->psa = true;
>  
> -	if (cxl_verbose) {
> -		read_prop_string(np, "ibm,loc-code");
> -		read_prop_string(np, "device_type");
> -	}
> -
>  	read_prop_dword(np, "ibm,#processes", &afu-
> >max_procs_virtualised);
>  
> -	if (cxl_verbose) {
> -		read_prop_dword(np, "ibm,scratchpad-size", &val);
> -		read_prop_dword(np, "ibm,programmable", &val);
> -		read_prop_string(np, "ibm,phandle");
> +	if (cxl_verbose)
>  		read_vpd(NULL, afu);
> -	}
>  
>  	read_prop_dword(np, "ibm,max-ints-per-process", &afu->guest-
> >max_ints);
>  	afu->irqs_max = afu->guest->max_ints;
> @@ -199,17 +167,9 @@ int cxl_of_read_afu_properties(struct cxl_afu
> *afu, struct device_node *np)
>  		afu->pp_irqs--;
>  	}
>  
> -	if (cxl_verbose) {
> -		read_prop_dword(np, "ibm,max-ints", &val);
> -		read_prop_dword(np, "ibm,vpd-size", &val);
> -	}
> -
>  	read_prop64_dword(np, "ibm,error-buffer-size", &afu-
> >eb_len);
>  	afu->eb_offset = 0;
>  
> -	if (cxl_verbose)
> -		read_prop_dword(np, "ibm,config-record-type", &val);
> -
>  	read_prop64_dword(np, "ibm,config-record-size", &afu-
> >crs_len);
>  	afu->crs_offset = 0;
>  
> @@ -235,15 +195,6 @@ int cxl_of_read_afu_properties(struct cxl_afu
> *afu, struct device_node *np)
>  					i, class_code);
>  			}
>  		}
> -
> -		read_prop_dword(np, "ibm,function-number", &val);
> -		read_prop_dword(np, "ibm,privileged-function",
> &val);
> -		read_prop_dword(np, "vendor-id", &val);
> -		read_prop_dword(np, "device-id", &val);
> -		read_prop_dword(np, "revision-id", &val);
> -		read_prop_dword(np, "class-code", &val);
> -		read_prop_dword(np, "subsystem-vendor-id", &val);
> -		read_prop_dword(np, "subsystem-id", &val);
>  	}
>  	/*
>  	 * if "ibm,process-mmio" doesn't exist then per-process mmio
> is
> @@ -256,12 +207,6 @@ int cxl_of_read_afu_properties(struct cxl_afu
> *afu, struct device_node *np)
>  	else
>  		afu->pp_psa = false;
>  
> -	if (cxl_verbose) {
> -		read_prop_dword(np, "ibm,supports-aur", &val);
> -		read_prop_dword(np, "ibm,supports-csrp", &val);
> -		read_prop_dword(np, "ibm,supports-prr", &val);
> -	}
> -
>  	prop = read_prop_dword(np, "ibm,function-error-interrupt",
> &val);
>  	if (prop)
>  		afu->serr_hwirq = val;
> @@ -343,49 +288,15 @@ int cxl_of_read_adapter_handle(struct cxl
> *adapter, struct device_node *np)
>  
>  int cxl_of_read_adapter_properties(struct cxl *adapter, struct
> device_node *np)
>  {
> -	int rc, len, naddr, i;
> -	char *p;
> +	int rc;
>  	const __be32 *prop;
>  	u32 val = 0;
>  
>  	/* Properties are read in the same order as listed in PAPR
> */
>  
> -	naddr = of_n_addr_cells(np);
> -
> -	if (cxl_verbose) {
> -		pr_info("Dump of the 'ibm,coherent-platform-
> facility' node properties:\n");
> -
> -		read_prop_dword(np, "#address-cells", &val);
> -		read_prop_dword(np, "#size-cells", &val);
> -
> -		prop = of_get_property(np, "compatible", &len);
> -		i = 0;
> -		while (i < len) {
> -			p = (char *) prop + i;
> -			pr_info("compatible: %s\n", p);
> -			i += strlen(p) + 1;
> -		}
> -		read_prop_string(np, "name");
> -		read_prop_string(np, "model");
> -
> -		prop = of_get_property(np, "reg", NULL);
> -		if (prop) {
> -			pr_info("reg: addr:%#llx size:%#x\n",
> -				of_read_number(prop, naddr),
> -				be32_to_cpu(prop[naddr]));
> -		}
> -
> -		read_prop_string(np, "ibm,loc-code");
> -	}
> -
>  	if ((rc = read_adapter_irq_config(adapter, np)))
>  		return rc;
>  
> -	if (cxl_verbose) {
> -		read_prop_string(np, "device_type");
> -		read_prop_string(np, "ibm,phandle");
> -	}
> -
>  	prop = read_prop_dword(np, "ibm,caia-version", &val);
>  	if (prop) {
>  		adapter->caia_major = (val & 0xFF00) >> 8;
> @@ -411,12 +322,6 @@ int cxl_of_read_adapter_properties(struct cxl
> *adapter, struct device_node *np)
>  	if (prop)
>  		adapter->guest->device = val;
>  
> -	if (cxl_verbose) {
> -		read_prop_dword(np, "ibm,privileged-facility",
> &val);
> -		read_prop_dword(np, "revision-id", &val);
> -		read_prop_dword(np, "class-code", &val);
> -	}
> -
>  	prop = read_prop_dword(np, "subsystem-vendor-id", &val);
>  	if (prop)
>  		adapter->guest->subsystem_vendor = val;
> 

-- 
Andrew Donnellan    OzLabs, ADL Canberra
ajd@linux.ibm.com   IBM Australia Limited

Re: [PATCH 2/2] cxl: Use of_property_ accessor functions

[Andrew Donnellan]

On Mon, 2024-07-29 at 16:36 -0600, Rob Herring (Arm) wrote:
> The CXL driver has its own custom implementations of typed DT
> property
> accessors. Replace the custom property accessor functions with the
> common DT property functions.
> 
> This clean-up is part of a larger effort to remove of_get_property()
> and
> other DT functions which leak pointers to DT node and property data.
> 
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>

This patch leaves two uses of of_get_property() (in read_phys_addr()
and read_adapter_irq_config()) - are you planning to get rid of those
in a future patch?

Acked-by: Andrew Donnellan <ajd@linux.ibm.com>

-- 
Andrew Donnellan    OzLabs, ADL Canberra
ajd@linux.ibm.com   IBM Australia Limited