diff mbox series

[v3,3/3] soc: ti: k3-socinfo: Revamp driver to accommodate different rev structs

Message ID 20231016101608.993921-4-n-francis@ti.com
State New
Headers
Series Revamp k3-socinfo driver |

Commit Message

Neha Malcom Francis Oct. 16, 2023, 10:16 a.m. UTC 
  k3-socinfo.c driver assumes silicon revisions for every platform are
incremental and one-to-one, corresponding to JTAG_ID's variant field:
1.0, 2.0 etc. This assumption is wrong for SoCs such as J721E, where the
variant field to revision mapping is 1.0, 1.1. Further, there are SoCs
such as AM65x where the sub-variant version requires custom decoding of
other registers.

Address this by using conditional handling per JTAG ID that requires an
exception with J721E as the first example. To facilitate this
conversion, use macros to identify the JTAG_ID part number and map them
to predefined string array.

Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
Co-developed-by: Thejasvi Konduru <t-konduru@ti.com>
Signed-off-by: Thejasvi Konduru <t-konduru@ti.com>
---
Changes since v2:
	- update commit message
	- move from double Signed-off-by to Co-developed-by
	- make j721e_rev_string_map[] a const char
	- drop k3_rev_string_map[] and continue using old
	  "variant++" logic for the typical cases
	- appropriate error handling with no overrides distinguishing
	  between ENODEV and ENOMEM
 
 drivers/soc/ti/k3-socinfo.c | 71 ++++++++++++++++++++++++++++---------
 1 file changed, 55 insertions(+), 16 deletions(-)

Comments

Nishanth Menon Oct. 18, 2023, 3:50 p.m. UTC | #1 
On 15:46-20231016, Neha Malcom Francis wrote:
> k3-socinfo.c driver assumes silicon revisions for every platform are
> incremental and one-to-one, corresponding to JTAG_ID's variant field:
> 1.0, 2.0 etc. This assumption is wrong for SoCs such as J721E, where the
> variant field to revision mapping is 1.0, 1.1. Further, there are SoCs
> such as AM65x where the sub-variant version requires custom decoding of
> other registers.
> 
> Address this by using conditional handling per JTAG ID that requires an
> exception with J721E as the first example. To facilitate this
> conversion, use macros to identify the JTAG_ID part number and map them
> to predefined string array.
> 
> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
> Co-developed-by: Thejasvi Konduru <t-konduru@ti.com>
> Signed-off-by: Thejasvi Konduru <t-konduru@ti.com>
> ---
> Changes since v2:
> 	- update commit message
> 	- move from double Signed-off-by to Co-developed-by
> 	- make j721e_rev_string_map[] a const char
> 	- drop k3_rev_string_map[] and continue using old
> 	  "variant++" logic for the typical cases
> 	- appropriate error handling with no overrides distinguishing
> 	  between ENODEV and ENOMEM
>  

Thanks to linux-next testing, looks like this created regression in networking.

https://lore.kernel.org/all/20231018140009.1725-1-r-gunasekaran@ti.com/

I will drop this patch for now from my queue. I suggest the following
for the eventual resolution:
a) Do all the prep work in a series of atomic bisectable patches prior to introducing
j721e SR change
b) Please sync with Ravi and netdev maintainers as to how to introduce
the changes
c) also introduce changes (if required) to other SoCs as required.
diff mbox series

Patch

diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
index 7fc3548e084c..7517a9c8c8fa 100644
--- a/drivers/soc/ti/k3-socinfo.c
+++ b/drivers/soc/ti/k3-socinfo.c
@@ -33,19 +33,33 @@ 
 
 #define CTRLMMR_WKUP_JTAGID_MFG_TI		0x17
 
+#define JTAG_ID_PARTNO_AM65X		0xBB5A
+#define JTAG_ID_PARTNO_J721E		0xBB64
+#define JTAG_ID_PARTNO_J7200		0xBB6D
+#define JTAG_ID_PARTNO_AM64X		0xBB38
+#define JTAG_ID_PARTNO_J721S2		0xBB75
+#define JTAG_ID_PARTNO_AM62X		0xBB7E
+#define JTAG_ID_PARTNO_J784S4		0xBB80
+#define JTAG_ID_PARTNO_AM62AX		0xBB8D
+#define JTAG_ID_PARTNO_AM62PX		0xBB9D
+
 static const struct k3_soc_id {
 	unsigned int id;
 	const char *family_name;
 } k3_soc_ids[] = {
-	{ 0xBB5A, "AM65X" },
-	{ 0xBB64, "J721E" },
-	{ 0xBB6D, "J7200" },
-	{ 0xBB38, "AM64X" },
-	{ 0xBB75, "J721S2"},
-	{ 0xBB7E, "AM62X" },
-	{ 0xBB80, "J784S4" },
-	{ 0xBB8D, "AM62AX" },
-	{ 0xBB9D, "AM62PX" },
+	{ JTAG_ID_PARTNO_AM65X, "AM65X" },
+	{ JTAG_ID_PARTNO_J721E, "J721E" },
+	{ JTAG_ID_PARTNO_J7200, "J7200" },
+	{ JTAG_ID_PARTNO_AM64X, "AM64X" },
+	{ JTAG_ID_PARTNO_J721S2, "J721S2"},
+	{ JTAG_ID_PARTNO_AM62X, "AM62X" },
+	{ JTAG_ID_PARTNO_J784S4, "J784S4" },
+	{ JTAG_ID_PARTNO_AM62AX, "AM62AX" },
+	{ JTAG_ID_PARTNO_AM62PX, "AM62PX" },
+};
+
+static const char * const j721e_rev_string_map[] = {
+	"1.0", "1.1",
 };
 
 static int
@@ -63,6 +77,32 @@  k3_chipinfo_partno_to_names(unsigned int partno,
 	return -ENODEV;
 }
 
+static int
+k3_chipinfo_variant_to_sr(unsigned int partno, unsigned int variant,
+			  struct soc_device_attribute *soc_dev_attr)
+{
+	switch (partno) {
+	case JTAG_ID_PARTNO_J721E:
+		if (variant >= ARRAY_SIZE(j721e_rev_string_map))
+			goto err_unknown_variant;
+		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%s",
+						   j721e_rev_string_map[variant]);
+		break;
+	default:
+		variant++;
+		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%x.0",
+						   variant);
+	}
+
+	if (!soc_dev_attr->revision)
+		return -ENOMEM;
+
+	return 0;
+
+err_unknown_variant:
+	return -ENODEV;
+}
+
 static int k3_chipinfo_probe(struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
@@ -94,7 +134,6 @@  static int k3_chipinfo_probe(struct platform_device *pdev)
 
 	variant = (jtag_id & CTRLMMR_WKUP_JTAGID_VARIANT_MASK) >>
 		  CTRLMMR_WKUP_JTAGID_VARIANT_SHIFT;
-	variant++;
 
 	partno_id = (jtag_id & CTRLMMR_WKUP_JTAGID_PARTNO_MASK) >>
 		 CTRLMMR_WKUP_JTAGID_PARTNO_SHIFT;
@@ -103,16 +142,16 @@  static int k3_chipinfo_probe(struct platform_device *pdev)
 	if (!soc_dev_attr)
 		return -ENOMEM;
 
-	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%x.0", variant);
-	if (!soc_dev_attr->revision) {
-		ret = -ENOMEM;
+	ret = k3_chipinfo_partno_to_names(partno_id, soc_dev_attr);
+	if (ret) {
+		dev_err(dev, "Unknown SoC JTAGID[0x%08X]: %d\n", jtag_id, ret);
 		goto err;
 	}
 
-	ret = k3_chipinfo_partno_to_names(partno_id, soc_dev_attr);
+	ret = k3_chipinfo_variant_to_sr(partno_id, variant, soc_dev_attr);
 	if (ret) {
-		dev_err(dev, "Unknown SoC JTAGID[0x%08X]: %d\n", jtag_id, ret);
-		goto err_free_rev;
+		dev_err(dev, "Unknown SoC SR[0x%08X]: %d\n", jtag_id, ret);
+		goto err;
 	}
 
 	node = of_find_node_by_path("/");