k3-socinfo driver doesn't account for difference series of silicon
revisions instead of the typical 1.0, 2.0 etc case. This exception is
currently already seen in J721E. This series aims to modify the driver
to account for those exceptions as well as clean things up a bit.
Changes since v2:
- Nishanth:
- 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
- add patch for error handling initial cleanup
- reorder patches
Changes since v1:
- Nishanth:
- undo churning of family attribute
- remove unnecessary code relocation
- add Thejasvi to Signed-off-by as we are now similar to
the initial attempt [1]
- separate out typo fixes to another patch (2/2)
Boot log: https://gist.github.com/nehamalcom/ff9375dcde681dd78712ee8473b24a50
(See relevant k3-socinfo dev_info print on line 276)
v2: https://lore.kernel.org/lkml/[email protected]/T/
v1: https://lore.kernel.org/linux-arm-kernel/[email protected]/T/
[1] https://lore.kernel.org/all/[email protected]/
Neha Malcom Francis (3):
soc: ti k3-socinfo: Fix typo
soc: ti: k3-socinfo: Avoid overriding ret
soc: ti: k3-socinfo: Revamp driver to accommodate different rev
structs
drivers/soc/ti/k3-socinfo.c | 76 +++++++++++++++++++++++++++----------
1 file changed, 57 insertions(+), 19 deletions(-)
--
2.34.1
Avoid overriding the return value and make sure the right error code is
reflected. Here, if the partno is none of the identified partnos present
in k3_soc_ids[], return -ENODEV.
Signed-off-by: Neha Malcom Francis <[email protected]>
---
Changes since v2:
- new patch
drivers/soc/ti/k3-socinfo.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
index d45f5cb955a6..7fc3548e084c 100644
--- a/drivers/soc/ti/k3-socinfo.c
+++ b/drivers/soc/ti/k3-socinfo.c
@@ -60,7 +60,7 @@ k3_chipinfo_partno_to_names(unsigned int partno,
return 0;
}
- return -EINVAL;
+ return -ENODEV;
}
static int k3_chipinfo_probe(struct platform_device *pdev)
@@ -111,8 +111,7 @@ static int k3_chipinfo_probe(struct platform_device *pdev)
ret = k3_chipinfo_partno_to_names(partno_id, soc_dev_attr);
if (ret) {
- dev_err(dev, "Unknown SoC JTAGID[0x%08X]\n", jtag_id);
- ret = -ENODEV;
+ dev_err(dev, "Unknown SoC JTAGID[0x%08X]: %d\n", jtag_id, ret);
goto err_free_rev;
}
--
2.34.1
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 <[email protected]>
Co-developed-by: Thejasvi Konduru <[email protected]>
Signed-off-by: Thejasvi Konduru <[email protected]>
---
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(-)
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("/");
--
2.34.1
Fix typo in driver that comments out wrong bit.
Signed-off-by: Neha Malcom Francis <[email protected]>
---
No change since v2
drivers/soc/ti/k3-socinfo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
index 6ea9b8c7d335..d45f5cb955a6 100644
--- a/drivers/soc/ti/k3-socinfo.c
+++ b/drivers/soc/ti/k3-socinfo.c
@@ -20,7 +20,7 @@
* 31-28 VARIANT Device variant
* 27-12 PARTNO Part number
* 11-1 MFG Indicates TI as manufacturer (0x17)
- * 1 Always 1
+ * 0 Always 1
*/
#define CTRLMMR_WKUP_JTAGID_VARIANT_SHIFT (28)
#define CTRLMMR_WKUP_JTAGID_VARIANT_MASK GENMASK(31, 28)
--
2.34.1
Hi Neha Malcom Francis,
On Mon, 16 Oct 2023 15:46:05 +0530, Neha Malcom Francis wrote:
> k3-socinfo driver doesn't account for difference series of silicon
> revisions instead of the typical 1.0, 2.0 etc case. This exception is
> currently already seen in J721E. This series aims to modify the driver
> to account for those exceptions as well as clean things up a bit.
>
> Changes since v2:
> - Nishanth:
> - 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
> - add patch for error handling initial cleanup
> - reorder patches
>
> [...]
I have applied the following to branch ti-drivers-soc-next on [1] with minor
fixups.
Thank you!
[1/3] soc: ti k3-socinfo: Fix typo
commit: 8dec342ead710dace27dc82096144bf7a1011827
[2/3] soc: ti: k3-socinfo: Avoid overriding ret
commit: 3aeb0d3694e16b5066db82aa1152884f2e6aace0
[3/3] soc: ti: k3-socinfo: Revamp driver to accommodate different rev structs
commit: 07e651db2d78eac4c41d7144eb5ea734af2328fc
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent up the chain during
the next merge window (or sooner if it is a relevant bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
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 <[email protected]>
> Co-developed-by: Thejasvi Konduru <[email protected]>
> Signed-off-by: Thejasvi Konduru <[email protected]>
> ---
> 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/[email protected]/
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.
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
Hi Neha Malcom Francis,
On Mon, 16 Oct 2023 15:46:05 +0530, Neha Malcom Francis wrote:
> k3-socinfo driver doesn't account for difference series of silicon
> revisions instead of the typical 1.0, 2.0 etc case. This exception is
> currently already seen in J721E. This series aims to modify the driver
> to account for those exceptions as well as clean things up a bit.
>
> Changes since v2:
> - Nishanth:
> - 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
> - add patch for error handling initial cleanup
> - reorder patches
>
> [...]
I have applied the following to branch ti-drivers-soc-next on [1].
Thank you!
[3/3] soc: ti: k3-socinfo: Revamp driver to accommodate different rev structs
commit: 3ba080bf46e4a9039d0d41356f4a2515e00bf747
The networking dependency previously identified has been sorted out
and available in linux-next[2] since next-20231206. So, this is a good
time for us to merge the patch that I have been keeping held up. At least
on my local scan, I don't see any other in-tree driver impacted by this.
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent up the chain during
the next merge window (or sooner if it is a relevant bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git
[2] https://lore.kernel.org/all/[email protected]/
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D