Skip to contentSkip to conversation

soc/tegra: fuse: Add custom SoC attributes

PATCH Superseded Jon Hunter 2 participants 3 messages Mar 20, 2020 – Mar 25, 2020 Tegra architecture support

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Jon Hunter

author maintainer
#0 lore raw

PATCH 1/1 soc/tegra: fuse: Add custom SoC attributes

Add a custom SoC attribute for Tegra to expose the HIDREV register contents to userspace via the sysfs. This register provides additional details about the fabrication and versioning of the device. Exposing this information is useful for identifying the exact device revision and device type.

Please note that the fields in this register vary depending on the Tegra generation and so instead of exposing the individual fields, just expose the entire contents of the register. Details of the register fields can be found in the Technical Reference Manual for each Tegra device.

---
 drivers/soc/tegra/fuse/fuse-tegra.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
drivers/soc/tegra/fuse/fuse-tegra.c +19 -0
index 802717b9f6a3..217e326da232 100644
@@ -300,6 +300,24 @@ static void tegra_enable_fuse_clk(void __iomem *base)
 	writel(reg, base + 0x14);
 }
 
+static ssize_t tegra_soc_hidrev_show(struct device *dev,
+				     struct device_attribute *attr,
+				     char *buf)
+{
+	return sprintf(buf, "%d\n", tegra_read_chipid());
+}
+
+static DEVICE_ATTR(hidrev, S_IRUGO, tegra_soc_hidrev_show,  NULL);
+
+static struct attribute *tegra_soc_attr[] = {
+	&dev_attr_hidrev.attr,
+	NULL,
+};
+
+static const struct attribute_group tegra_soc_attr_group = {
+	.attrs = tegra_soc_attr,
+};
+
 struct device * __init tegra_soc_device_register(void)
 {
 	struct soc_device_attribute *attr;
@@ -312,6 +330,7 @@ struct device * __init tegra_soc_device_register(void)
 	attr->family = kasprintf(GFP_KERNEL, "Tegra");
 	attr->revision = kasprintf(GFP_KERNEL, "%d", tegra_sku_info.revision);
 	attr->soc_id = kasprintf(GFP_KERNEL, "%u", tegra_get_chip_id());
+	attr->custom_attr_group = &tegra_soc_attr_group;
 
 	dev = soc_device_register(attr);
 	if (IS_ERR(dev)) {
Signature · 1 line
2.17.1
#1 lore raw
Quoted 10 lines · Jon Hunter

Add a custom SoC attribute for Tegra to expose the HIDREV register contents to userspace via the sysfs. This register provides additional details about the fabrication and versioning of the device. Exposing this information is useful for identifying the exact device revision and device type.

Please note that the fields in this register vary depending on the Tegra generation and so instead of exposing the individual fields, just expose the entire contents of the register. Details of the register fields can be found in the Technical Reference Manual for each Tegra device.

That seems a little suboptimal to me. It's pretty trivial for the kernel to distinguish between different SoC generations in order to know what the fields are. It's a lot more difficult for userspace to do so. Is the register completely different between SoC generations or just slightly?

Having individual fields exposed as individual attributes seems like it would make it a lot easier for userspace to get at the needed bits.

Thierry

Quoted 11 lines
---
 drivers/soc/tegra/fuse/fuse-tegra.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
drivers/soc/tegra/fuse/fuse-tegra.c +0 -0
index 802717b9f6a3..217e326da232 100644

drivers/soc/tegra/fuse/fuse-tegra.c static void tegra_enable_fuse_clk(void __iomem *base) · line 307

Quoted 8 lines
drivers/soc/tegra/fuse/fuse-tegra.c +5 -0
@@ -300,6 +300,24 @@ static void tegra_enable_fuse_clk(void __iomem *base)
 	writel(reg, base + 0x14);
 }
 
+static ssize_t tegra_soc_hidrev_show(struct device *dev,
+				     struct device_attribute *attr,
+				     char *buf)
+{
+	return sprintf(buf, "%d\n", tegra_read_chipid());

Would it be better to print this as hexadecimal?

Quoted 17 lines
drivers/soc/tegra/fuse/fuse-tegra.c +13 -0
+}
+
+static DEVICE_ATTR(hidrev, S_IRUGO, tegra_soc_hidrev_show,  NULL);
+
+static struct attribute *tegra_soc_attr[] = {
+	&dev_attr_hidrev.attr,
+	NULL,
+};
+
+static const struct attribute_group tegra_soc_attr_group = {
+	.attrs = tegra_soc_attr,
+};
+
 struct device * __init tegra_soc_device_register(void)
 {
 	struct soc_device_attribute *attr;

drivers/soc/tegra/fuse/fuse-tegra.c struct device * __init tegra_soc_device_register(void) · line 332

Quoted 3 lines
drivers/soc/tegra/fuse/fuse-tegra.c +0 -0
@@ -312,6 +330,7 @@ struct device * __init tegra_soc_device_register(void)
 	attr->family = kasprintf(GFP_KERNEL, "Tegra");
 	attr->revision = kasprintf(GFP_KERNEL, "%d", tegra_sku_info.revision);
 	attr->soc_id = kasprintf(GFP_KERNEL, "%u", tegra_get_chip_id());

I guess we print all of these as decimal, so hidrev should probably be the same, so never mind the previous comment.

Thierry

author maintainer
#2 lore raw
Quoted 12 lines · Thierry Reding
Quoted 10 lines · Jon Hunter

Add a custom SoC attribute for Tegra to expose the HIDREV register contents to userspace via the sysfs. This register provides additional details about the fabrication and versioning of the device. Exposing this information is useful for identifying the exact device revision and device type.

Please note that the fields in this register vary depending on the Tegra generation and so instead of exposing the individual fields, just expose the entire contents of the register. Details of the register fields can be found in the Technical Reference Manual for each Tegra device.

Quoted 4 lines

That seems a little suboptimal to me. It's pretty trivial for the kernel to distinguish between different SoC generations in order to know what the fields are. It's a lot more difficult for userspace to do so. Is the register completely different between SoC generations or just slightly?

It looks like only Tegra194 is different which is a shame. Unfortunately, does not appear to be documented in the public TRM at the moment. Hopefully we can fix that.

Quoted 2 lines

Having individual fields exposed as individual attributes seems like it would make it a lot easier for userspace to get at the needed bits.

Yes that does make sense.

Jon

Signature · 1 line
nvpublic