diff mbox series

soc: renesas: Consolidate product register handling

Message ID 057721f46c7499de4133135488f0f3da7fb39265.1636570669.git.geert+renesas@glider.be (mailing list archive)
State Accepted
Delegated to: Geert Uytterhoeven
Headers show
Series soc: renesas: Consolidate product register handling | expand

Commit Message

Geert Uytterhoeven Nov. 10, 2021, 7 p.m. UTC
Currently renesas_soc_init() scans the whole device tree up to three
times, to find a device node describing a product register.
Furthermore, the product register handling for the different variants is
very similar, with the major difference being the location of the
product bitfield inside the product register.

Reduce scanning to a single pass using of_find_matching_node_and_match()
instead.  Switch to a common handling of product registers, by storing
the intrinsics of each product register type in the data field of the
corresponding match entry.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Unfortunately the simplication of the source code is not reflected in
the actual object code size, due to the sheer size of struct
of_device_id (196 or 200 bytes on 32 vs. 64-bit).

"[PATCH/RFC] of: Shrink struct of_device_id"
https://lore.kernel.org/all/ef59d6fd3b2201b912d5eaa7f7a037d8f9adb744.1636561068.git.geert+renesas@glider.be
---
 drivers/soc/renesas/renesas-soc.c | 115 +++++++++++++++---------------
 1 file changed, 56 insertions(+), 59 deletions(-)

Comments

Lad Prabhakar Nov. 11, 2021, 10:53 a.m. UTC | #1
Hi Geert,

Thank you for the patch.

> -----Original Message-----
> From: Geert Uytterhoeven <geert+renesas@glider.be>
> Sent: 10 November 2021 19:01
> To: Magnus Damm <magnus.damm@gmail.com>; Chris Brandt <Chris.Brandt@renesas.com>; Prabhakar Mahadev
> Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Cc: linux-renesas-soc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Geert Uytterhoeven
> <geert+renesas@glider.be>
> Subject: [PATCH] soc: renesas: Consolidate product register handling
> 
> Currently renesas_soc_init() scans the whole device tree up to three times, to find a device node
> describing a product register.
> Furthermore, the product register handling for the different variants is very similar, with the major
> difference being the location of the product bitfield inside the product register.
> 
> Reduce scanning to a single pass using of_find_matching_node_and_match() instead.  Switch to a common
> handling of product registers, by storing the intrinsics of each product register type in the data
> field of the corresponding match entry.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Unfortunately the simplication of the source code is not reflected in the actual object code size, due
> to the sheer size of struct of_device_id (196 or 200 bytes on 32 vs. 64-bit).
> 
> "[PATCH/RFC] of: Shrink struct of_device_id"
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2Fef59d6fd3b22
> 01b912d5eaa7f7a037d8f9adb744.1636561068.git.geert%2Brenesas%40glider.be&amp;data=04%7C01%7Cprabhakar.m
> ahadev-
> lad.rj%40bp.renesas.com%7C97f0415c949a4bd5f67e08d9a47c6e5c%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%
> 7C637721676621408059%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXV
> CI6Mn0%3D%7C1000&amp;sdata=2Yp%2Fz9uQmdg4rrZ3uHIB7QAWKlJcNAo5xyszYqDRWcM%3D&amp;reserved=0
> ---
>  drivers/soc/renesas/renesas-soc.c | 115 +++++++++++++++---------------
>  1 file changed, 56 insertions(+), 59 deletions(-)
> 
Tested this patch with RFC patch applied on SMARC EVK.

Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c
> index 7961b0be1850922d..7da0ea3587c4eab8 100644
> --- a/drivers/soc/renesas/renesas-soc.c
> +++ b/drivers/soc/renesas/renesas-soc.c
> @@ -328,16 +328,49 @@ static const struct of_device_id renesas_socs[] __initconst = {
>  	{ /* sentinel */ }
>  };
> 
> +struct renesas_id {
> +	unsigned int offset;
> +	u32 mask;
> +};
> +
> +static const struct renesas_id id_bsid __initconst = {
> +	.offset = 0,
> +	.mask = 0xff0000,
> +	/*
> +	 * TODO: Upper 4 bits of BSID are for chip version, but the format is
> +	 * not known at this time so we don't know how to specify eshi and eslo
> +	 */
> +};
> +
> +static const struct renesas_id id_rzg2l __initconst = {
> +	.offset = 0xa04,
> +	.mask = 0xfffffff,
> +};
> +
> +static const struct renesas_id id_prr __initconst = {
> +	.offset = 0,
> +	.mask = 0xff00,
> +};
> +
> +static const struct of_device_id renesas_ids[] __initconst = {
> +	{ .compatible = "renesas,bsid",			.data = &id_bsid },
> +	{ .compatible = "renesas,r9a07g044-sysc",	.data = &id_rzg2l },
> +	{ .compatible = "renesas,prr",			.data = &id_prr },
> +	{ /* sentinel */ }
> +};
> +
>  static int __init renesas_soc_init(void)  {
>  	struct soc_device_attribute *soc_dev_attr;
> +	unsigned int product, eshi = 0, eslo;
>  	const struct renesas_family *family;
>  	const struct of_device_id *match;
>  	const struct renesas_soc *soc;
> +	const struct renesas_id *id;
>  	void __iomem *chipid = NULL;
>  	struct soc_device *soc_dev;
>  	struct device_node *np;
> -	unsigned int product, eshi = 0, eslo;
> +	const char *soc_id;
> 
>  	match = of_match_node(renesas_socs, of_root);
>  	if (!match)
> @@ -345,77 +378,42 @@ static int __init renesas_soc_init(void)
> 
>  	soc = match->data;
>  	family = soc->family;
> +	soc_id = strchr(match->compatible, ',') + 1;
> 
> -	np = of_find_compatible_node(NULL, NULL, "renesas,bsid");
> +	np = of_find_matching_node_and_match(NULL, renesas_ids, &match);
>  	if (np) {
> +		id = match->data;
>  		chipid = of_iomap(np, 0);
>  		of_node_put(np);
> -
> -		if (chipid) {
> -			product = readl(chipid);
> -			iounmap(chipid);
> -
> -			if (soc->id && ((product >> 16) & 0xff) != soc->id) {
> -				pr_warn("SoC mismatch (product = 0x%x)\n",
> -					product);
> -				return -ENODEV;
> -			}
> -		}
> -
> -		/*
> -		 * TODO: Upper 4 bits of BSID are for chip version, but the
> -		 * format is not known at this time so we don't know how to
> -		 * specify eshi and eslo
> -		 */
> -
> -		goto done;
> +	} else if (soc->id && family->reg) {
> +		/* Try hardcoded CCCR/PRR fallback */
> +		id = &id_prr;
> +		chipid = ioremap(family->reg, 4);
>  	}
> 
> -	np = of_find_compatible_node(NULL, NULL, "renesas,r9a07g044-sysc");
> -	if (np) {
> -		chipid = of_iomap(np, 0);
> -		of_node_put(np);
> +	if (chipid) {
> +		product = readl(chipid + id->offset);
> +		iounmap(chipid);
> 
> -		if (chipid) {
> -			product = readl(chipid + 0x0a04);
> -			iounmap(chipid);
> +		if (id == &id_prr) {
> +			/* R-Car M3-W ES1.1 incorrectly identifies as ES2.0 */
> +			if ((product & 0x7fff) == 0x5210)
> +				product ^= 0x11;
> +			/* R-Car M3-W ES1.3 incorrectly identifies as ES2.1 */
> +			if ((product & 0x7fff) == 0x5211)
> +				product ^= 0x12;
> 
> -			if (soc->id && (product & 0xfffffff) != soc->id) {
> -				pr_warn("SoC mismatch (product = 0x%x)\n",
> -					product);
> -				return -ENODEV;
> -			}
> +			eshi = ((product >> 4) & 0x0f) + 1;
> +			eslo = product & 0xf;
>  		}
> 
> -		goto done;
> -	}
> -
> -	/* Try PRR first, then hardcoded fallback */
> -	np = of_find_compatible_node(NULL, NULL, "renesas,prr");
> -	if (np) {
> -		chipid = of_iomap(np, 0);
> -		of_node_put(np);
> -	} else if (soc->id && family->reg) {
> -		chipid = ioremap(family->reg, 4);
> -	}
> -	if (chipid) {
> -		product = readl(chipid);
> -		iounmap(chipid);
> -		/* R-Car M3-W ES1.1 incorrectly identifies as ES2.0 */
> -		if ((product & 0x7fff) == 0x5210)
> -			product ^= 0x11;
> -		/* R-Car M3-W ES1.3 incorrectly identifies as ES2.1 */
> -		if ((product & 0x7fff) == 0x5211)
> -			product ^= 0x12;
> -		if (soc->id && ((product >> 8) & 0xff) != soc->id) {
> +		if (soc->id &&
> +		    ((product & id->mask) >> __ffs(id->mask)) != soc->id) {
>  			pr_warn("SoC mismatch (product = 0x%x)\n", product);
>  			return -ENODEV;
>  		}
> -		eshi = ((product >> 4) & 0x0f) + 1;
> -		eslo = product & 0xf;
>  	}
> 
> -done:
>  	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
>  	if (!soc_dev_attr)
>  		return -ENOMEM;
> @@ -425,8 +423,7 @@ static int __init renesas_soc_init(void)
>  	of_node_put(np);
> 
>  	soc_dev_attr->family = kstrdup_const(family->name, GFP_KERNEL);
> -	soc_dev_attr->soc_id = kstrdup_const(strchr(match->compatible, ',') + 1,
> -					     GFP_KERNEL);
> +	soc_dev_attr->soc_id = kstrdup_const(soc_id, GFP_KERNEL);
>  	if (eshi)
>  		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "ES%u.%u", eshi,
>  						   eslo);
> --
> 2.25.1
diff mbox series

Patch

diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c
index 7961b0be1850922d..7da0ea3587c4eab8 100644
--- a/drivers/soc/renesas/renesas-soc.c
+++ b/drivers/soc/renesas/renesas-soc.c
@@ -328,16 +328,49 @@  static const struct of_device_id renesas_socs[] __initconst = {
 	{ /* sentinel */ }
 };
 
+struct renesas_id {
+	unsigned int offset;
+	u32 mask;
+};
+
+static const struct renesas_id id_bsid __initconst = {
+	.offset = 0,
+	.mask = 0xff0000,
+	/*
+	 * TODO: Upper 4 bits of BSID are for chip version, but the format is
+	 * not known at this time so we don't know how to specify eshi and eslo
+	 */
+};
+
+static const struct renesas_id id_rzg2l __initconst = {
+	.offset = 0xa04,
+	.mask = 0xfffffff,
+};
+
+static const struct renesas_id id_prr __initconst = {
+	.offset = 0,
+	.mask = 0xff00,
+};
+
+static const struct of_device_id renesas_ids[] __initconst = {
+	{ .compatible = "renesas,bsid",			.data = &id_bsid },
+	{ .compatible = "renesas,r9a07g044-sysc",	.data = &id_rzg2l },
+	{ .compatible = "renesas,prr",			.data = &id_prr },
+	{ /* sentinel */ }
+};
+
 static int __init renesas_soc_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
+	unsigned int product, eshi = 0, eslo;
 	const struct renesas_family *family;
 	const struct of_device_id *match;
 	const struct renesas_soc *soc;
+	const struct renesas_id *id;
 	void __iomem *chipid = NULL;
 	struct soc_device *soc_dev;
 	struct device_node *np;
-	unsigned int product, eshi = 0, eslo;
+	const char *soc_id;
 
 	match = of_match_node(renesas_socs, of_root);
 	if (!match)
@@ -345,77 +378,42 @@  static int __init renesas_soc_init(void)
 
 	soc = match->data;
 	family = soc->family;
+	soc_id = strchr(match->compatible, ',') + 1;
 
-	np = of_find_compatible_node(NULL, NULL, "renesas,bsid");
+	np = of_find_matching_node_and_match(NULL, renesas_ids, &match);
 	if (np) {
+		id = match->data;
 		chipid = of_iomap(np, 0);
 		of_node_put(np);
-
-		if (chipid) {
-			product = readl(chipid);
-			iounmap(chipid);
-
-			if (soc->id && ((product >> 16) & 0xff) != soc->id) {
-				pr_warn("SoC mismatch (product = 0x%x)\n",
-					product);
-				return -ENODEV;
-			}
-		}
-
-		/*
-		 * TODO: Upper 4 bits of BSID are for chip version, but the
-		 * format is not known at this time so we don't know how to
-		 * specify eshi and eslo
-		 */
-
-		goto done;
+	} else if (soc->id && family->reg) {
+		/* Try hardcoded CCCR/PRR fallback */
+		id = &id_prr;
+		chipid = ioremap(family->reg, 4);
 	}
 
-	np = of_find_compatible_node(NULL, NULL, "renesas,r9a07g044-sysc");
-	if (np) {
-		chipid = of_iomap(np, 0);
-		of_node_put(np);
+	if (chipid) {
+		product = readl(chipid + id->offset);
+		iounmap(chipid);
 
-		if (chipid) {
-			product = readl(chipid + 0x0a04);
-			iounmap(chipid);
+		if (id == &id_prr) {
+			/* R-Car M3-W ES1.1 incorrectly identifies as ES2.0 */
+			if ((product & 0x7fff) == 0x5210)
+				product ^= 0x11;
+			/* R-Car M3-W ES1.3 incorrectly identifies as ES2.1 */
+			if ((product & 0x7fff) == 0x5211)
+				product ^= 0x12;
 
-			if (soc->id && (product & 0xfffffff) != soc->id) {
-				pr_warn("SoC mismatch (product = 0x%x)\n",
-					product);
-				return -ENODEV;
-			}
+			eshi = ((product >> 4) & 0x0f) + 1;
+			eslo = product & 0xf;
 		}
 
-		goto done;
-	}
-
-	/* Try PRR first, then hardcoded fallback */
-	np = of_find_compatible_node(NULL, NULL, "renesas,prr");
-	if (np) {
-		chipid = of_iomap(np, 0);
-		of_node_put(np);
-	} else if (soc->id && family->reg) {
-		chipid = ioremap(family->reg, 4);
-	}
-	if (chipid) {
-		product = readl(chipid);
-		iounmap(chipid);
-		/* R-Car M3-W ES1.1 incorrectly identifies as ES2.0 */
-		if ((product & 0x7fff) == 0x5210)
-			product ^= 0x11;
-		/* R-Car M3-W ES1.3 incorrectly identifies as ES2.1 */
-		if ((product & 0x7fff) == 0x5211)
-			product ^= 0x12;
-		if (soc->id && ((product >> 8) & 0xff) != soc->id) {
+		if (soc->id &&
+		    ((product & id->mask) >> __ffs(id->mask)) != soc->id) {
 			pr_warn("SoC mismatch (product = 0x%x)\n", product);
 			return -ENODEV;
 		}
-		eshi = ((product >> 4) & 0x0f) + 1;
-		eslo = product & 0xf;
 	}
 
-done:
 	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
 	if (!soc_dev_attr)
 		return -ENOMEM;
@@ -425,8 +423,7 @@  static int __init renesas_soc_init(void)
 	of_node_put(np);
 
 	soc_dev_attr->family = kstrdup_const(family->name, GFP_KERNEL);
-	soc_dev_attr->soc_id = kstrdup_const(strchr(match->compatible, ',') + 1,
-					     GFP_KERNEL);
+	soc_dev_attr->soc_id = kstrdup_const(soc_id, GFP_KERNEL);
 	if (eshi)
 		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "ES%u.%u", eshi,
 						   eslo);