[PATCH 00/11] soc: samsung: exynos-chipid: add gs101 support

[PATCH 00/11] soc: samsung: exynos-chipid: add gs101 support

[Tudor Ambarus]

The patch set has the typical dependency of the DT patch depending on
the bindings patch. Plus, the dt patch references labels from the
efuse node, thus it depends on the bindings and dt patch from:
https://lore.kernel.org/linux-samsung-soc/20251031-gs101-otp-v1-0-2a54f6c4e7b6@linaro.org/

GS101 reads the Product ID and the Chip ID from the OTP controller
registers. Add suppot for Google GS101 SoC info.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
Tudor Ambarus (11):
      dt-bindings: hwinfo: samsung,exynos-chipid: add google,gs101 compatible
      soc: samsung: exynos-chipid: use a local dev variable
      soc: samsung: exynos-chipid: use heap allocated driver data
      soc: samsung: exynos-chipid: refer to match->data as data
      soc: samsung: exynos-chipid: introduce match_data->get_chipid_info()
      soc: samsung: exynos-chipid: make asv_init opt-in
      soc: samsung: exynos-chipid: add support for google,gs101-chipid
      soc: samsung: exynos-chipid: prepend exynos_ to a method's name
      soc: samsung: exynos-chipid: downgrade dev_info to dev_dbg for soc info
      arm64: dts: exynos: gs101: add the chipid node
      arm64: defconfig: enable Samsung Exynos chipid driver

 .../bindings/hwinfo/samsung,exynos-chipid.yaml     |  51 ++++++-
 arch/arm64/boot/dts/exynos/google/gs101.dtsi       |   6 +
 arch/arm64/configs/defconfig                       |   1 +
 drivers/soc/samsung/exynos-chipid.c                | 163 ++++++++++++++++-----
 4 files changed, 186 insertions(+), 35 deletions(-)
---
base-commit: 73f7017e663620a616171cc80d62504a624dc4de
change-id: 20251031-gs101-chipid-fd84da8afa2f

Best regards,
-- 
Tudor Ambarus <tudor.ambarus@linaro.org>

[PATCH 01/11] dt-bindings: hwinfo: samsung,exynos-chipid: add google,gs101 compatible

[Tudor Ambarus]

Google GS101 Chip ID references the nvmem cells from the OTP controller,
it doesn't need a reg space. Add the google,gs101-chipid compatible.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 .../bindings/hwinfo/samsung,exynos-chipid.yaml     | 51 +++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml b/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml
index b9cdfe52b62ff3a365d61368c39db21facff6565..6d6260be02d47712ebf1e4d31973802e8340cdfe 100644
--- a/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml
+++ b/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml
@@ -33,6 +33,16 @@ properties:
               - samsung,exynosautov9-chipid
               - samsung,exynosautov920-chipid
           - const: samsung,exynos850-chipid
+      - items:
+          - const: google,gs101-chipid
+
+  nvmem-cells:
+    maxItems: 2
+
+  nvmem-cell-names:
+    items:
+      - const: product-id
+      - const: chip-id
 
   reg:
     maxItems: 1
@@ -47,7 +57,46 @@ properties:
 
 required:
   - compatible
-  - reg
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            oneOf:
+              - enum:
+                  - samsung,exynos4210-chipid
+                  - samsung,exynos850-chipid
+              - items:
+                  - enum:
+                      - samsung,exynos5433-chipid
+                      - samsung,exynos7-chipid
+                      - samsung,exynos7870-chipid
+                      - samsung,exynos8890-chipid
+                  - const: samsung,exynos4210-chipid
+              - items:
+                  - enum:
+                      - samsung,exynos2200-chipid
+                      - samsung,exynos7885-chipid
+                      - samsung,exynos8895-chipid
+                      - samsung,exynos9610-chipid
+                      - samsung,exynos9810-chipid
+                      - samsung,exynos990-chipid
+                      - samsung,exynosautov9-chipid
+                      - samsung,exynosautov920-chipid
+                  - const: samsung,exynos850-chipid
+    then:
+      required:
+        - reg
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: google,gs101-chipid
+    then:
+      required:
+        - nvmem-cells
+        - nvmem-cell-names
 
 additionalProperties: false
 

-- 
2.51.1.930.gacf6e81ea2-goog

[PATCH 02/11] soc: samsung: exynos-chipid: use a local dev variable

[Tudor Ambarus]

Use a local variable for struct device to avoid dereferencing.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/soc/samsung/exynos-chipid.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
index 0f9f83a006795df08dae556ef37c2b78bd599ef5..d3b4b5508e0c808ee9f7b0039073ef57915d60fc 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -109,16 +109,17 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 	const struct exynos_chipid_variant *drv_data;
 	struct exynos_chipid_info soc_info;
 	struct soc_device_attribute *soc_dev_attr;
+	struct device *dev = &pdev->dev;
 	struct soc_device *soc_dev;
 	struct device_node *root;
 	struct regmap *regmap;
 	int ret;
 
-	drv_data = of_device_get_match_data(&pdev->dev);
+	drv_data = of_device_get_match_data(dev);
 	if (!drv_data)
 		return -EINVAL;
 
-	regmap = device_node_to_regmap(pdev->dev.of_node);
+	regmap = device_node_to_regmap(dev->of_node);
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
 
@@ -126,8 +127,7 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	soc_dev_attr = devm_kzalloc(&pdev->dev, sizeof(*soc_dev_attr),
-				    GFP_KERNEL);
+	soc_dev_attr = devm_kzalloc(dev, sizeof(*soc_dev_attr), GFP_KERNEL);
 	if (!soc_dev_attr)
 		return -ENOMEM;
 
@@ -137,8 +137,8 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 	of_property_read_string(root, "model", &soc_dev_attr->machine);
 	of_node_put(root);
 
-	soc_dev_attr->revision = devm_kasprintf(&pdev->dev, GFP_KERNEL,
-						"%x", soc_info.revision);
+	soc_dev_attr->revision = devm_kasprintf(dev, GFP_KERNEL, "%x",
+						soc_info.revision);
 	if (!soc_dev_attr->revision)
 		return -ENOMEM;
 	soc_dev_attr->soc_id = product_id_to_soc_id(soc_info.product_id);
@@ -152,13 +152,13 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 	if (IS_ERR(soc_dev))
 		return PTR_ERR(soc_dev);
 
-	ret = exynos_asv_init(&pdev->dev, regmap);
+	ret = exynos_asv_init(dev, regmap);
 	if (ret)
 		goto err;
 
 	platform_set_drvdata(pdev, soc_dev);
 
-	dev_info(&pdev->dev, "Exynos: CPU[%s] PRO_ID[0x%x] REV[0x%x] Detected\n",
+	dev_info(dev, "Exynos: CPU[%s] PRO_ID[0x%x] REV[0x%x] Detected\n",
 		 soc_dev_attr->soc_id, soc_info.product_id, soc_info.revision);
 
 	return 0;

-- 
2.51.1.930.gacf6e81ea2-goog

[PATCH 03/11] soc: samsung: exynos-chipid: use heap allocated driver data

[Tudor Ambarus]

Prepare for GS101 chipid addition, it will requre more driver data. Use
heap allocated driver data, it's easily extensible. Together with
the change s/soc_info/exynos_chipid, it better indicates that it is the
driver data structure.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/soc/samsung/exynos-chipid.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
index d3b4b5508e0c808ee9f7b0039073ef57915d60fc..f5107b8b339e8884d6a67a80650a95441510c66c 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -82,7 +82,7 @@ static const char *product_id_to_soc_id(unsigned int product_id)
 
 static int exynos_chipid_get_chipid_info(struct regmap *regmap,
 		const struct exynos_chipid_variant *data,
-		struct exynos_chipid_info *soc_info)
+		struct exynos_chipid_info *exynos_chipid)
 {
 	int ret;
 	unsigned int val, main_rev, sub_rev;
@@ -90,7 +90,7 @@ static int exynos_chipid_get_chipid_info(struct regmap *regmap,
 	ret = regmap_read(regmap, EXYNOS_CHIPID_REG_PRO_ID, &val);
 	if (ret < 0)
 		return ret;
-	soc_info->product_id = val & EXYNOS_MASK;
+	exynos_chipid->product_id = val & EXYNOS_MASK;
 
 	if (data->rev_reg != EXYNOS_CHIPID_REG_PRO_ID) {
 		ret = regmap_read(regmap, data->rev_reg, &val);
@@ -99,7 +99,7 @@ static int exynos_chipid_get_chipid_info(struct regmap *regmap,
 	}
 	main_rev = (val >> data->main_rev_shift) & EXYNOS_REV_PART_MASK;
 	sub_rev = (val >> data->sub_rev_shift) & EXYNOS_REV_PART_MASK;
-	soc_info->revision = (main_rev << EXYNOS_REV_PART_SHIFT) | sub_rev;
+	exynos_chipid->revision = (main_rev << EXYNOS_REV_PART_SHIFT) | sub_rev;
 
 	return 0;
 }
@@ -107,7 +107,7 @@ static int exynos_chipid_get_chipid_info(struct regmap *regmap,
 static int exynos_chipid_probe(struct platform_device *pdev)
 {
 	const struct exynos_chipid_variant *drv_data;
-	struct exynos_chipid_info soc_info;
+	struct exynos_chipid_info *exynos_chipid;
 	struct soc_device_attribute *soc_dev_attr;
 	struct device *dev = &pdev->dev;
 	struct soc_device *soc_dev;
@@ -119,11 +119,15 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 	if (!drv_data)
 		return -EINVAL;
 
+	exynos_chipid = devm_kzalloc(dev, sizeof(*exynos_chipid), GFP_KERNEL);
+	if (!exynos_chipid)
+		return -ENOMEM;
+
 	regmap = device_node_to_regmap(dev->of_node);
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
 
-	ret = exynos_chipid_get_chipid_info(regmap, drv_data, &soc_info);
+	ret = exynos_chipid_get_chipid_info(regmap, drv_data, exynos_chipid);
 	if (ret < 0)
 		return ret;
 
@@ -138,10 +142,10 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 	of_node_put(root);
 
 	soc_dev_attr->revision = devm_kasprintf(dev, GFP_KERNEL, "%x",
-						soc_info.revision);
+						exynos_chipid->revision);
 	if (!soc_dev_attr->revision)
 		return -ENOMEM;
-	soc_dev_attr->soc_id = product_id_to_soc_id(soc_info.product_id);
+	soc_dev_attr->soc_id = product_id_to_soc_id(exynos_chipid->product_id);
 	if (!soc_dev_attr->soc_id) {
 		pr_err("Unknown SoC\n");
 		return -ENODEV;
@@ -159,7 +163,8 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, soc_dev);
 
 	dev_info(dev, "Exynos: CPU[%s] PRO_ID[0x%x] REV[0x%x] Detected\n",
-		 soc_dev_attr->soc_id, soc_info.product_id, soc_info.revision);
+		 soc_dev_attr->soc_id, exynos_chipid->product_id,
+		 exynos_chipid->revision);
 
 	return 0;
 

-- 
2.51.1.930.gacf6e81ea2-goog

[PATCH 04/11] soc: samsung: exynos-chipid: refer to match->data as data

[Tudor Ambarus]

Be consistent across the driver and refer to match->data as data.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/soc/samsung/exynos-chipid.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
index f5107b8b339e8884d6a67a80650a95441510c66c..ab6bdf24a754a0faf974190c1fa1f99735cbef8e 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -106,7 +106,7 @@ static int exynos_chipid_get_chipid_info(struct regmap *regmap,
 
 static int exynos_chipid_probe(struct platform_device *pdev)
 {
-	const struct exynos_chipid_variant *drv_data;
+	const struct exynos_chipid_variant *data;
 	struct exynos_chipid_info *exynos_chipid;
 	struct soc_device_attribute *soc_dev_attr;
 	struct device *dev = &pdev->dev;
@@ -115,8 +115,8 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 	struct regmap *regmap;
 	int ret;
 
-	drv_data = of_device_get_match_data(dev);
-	if (!drv_data)
+	data = of_device_get_match_data(dev);
+	if (!data)
 		return -EINVAL;
 
 	exynos_chipid = devm_kzalloc(dev, sizeof(*exynos_chipid), GFP_KERNEL);
@@ -127,7 +127,7 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
 
-	ret = exynos_chipid_get_chipid_info(regmap, drv_data, exynos_chipid);
+	ret = exynos_chipid_get_chipid_info(regmap, data, exynos_chipid);
 	if (ret < 0)
 		return ret;
 
@@ -181,13 +181,13 @@ static void exynos_chipid_remove(struct platform_device *pdev)
 	soc_device_unregister(soc_dev);
 }
 
-static const struct exynos_chipid_variant exynos4210_chipid_drv_data = {
+static const struct exynos_chipid_variant exynos4210_chipid_data = {
 	.rev_reg	= 0x0,
 	.main_rev_shift	= 4,
 	.sub_rev_shift	= 0,
 };
 
-static const struct exynos_chipid_variant exynos850_chipid_drv_data = {
+static const struct exynos_chipid_variant exynos850_chipid_data = {
 	.rev_reg	= 0x10,
 	.main_rev_shift	= 20,
 	.sub_rev_shift	= 16,
@@ -196,10 +196,10 @@ static const struct exynos_chipid_variant exynos850_chipid_drv_data = {
 static const struct of_device_id exynos_chipid_of_device_ids[] = {
 	{
 		.compatible	= "samsung,exynos4210-chipid",
-		.data		= &exynos4210_chipid_drv_data,
+		.data		= &exynos4210_chipid_data,
 	}, {
 		.compatible	= "samsung,exynos850-chipid",
-		.data		= &exynos850_chipid_drv_data,
+		.data		= &exynos850_chipid_data,
 	},
 	{ }
 };

-- 
2.51.1.930.gacf6e81ea2-goog

[PATCH 05/11] soc: samsung: exynos-chipid: introduce match_data->get_chipid_info()

[Tudor Ambarus]

Newer SoCs, like GS101, don't have a dedicated Chip ID controller.
The GS101 Chip ID info is available as part of the OTP controller
registers, among other things. For GS101 we will read the Chip ID from
the OTP controller using the nvmem API.

Extend the match_data with a get_chipid_info() method, to allow nvmem
integration.

`struct exynos_chipid_info` is moved to the top of the file to avoid a
forward declaration. The structure is extended with pointers to device
and regmap to allow current implementation to obtain the regmap in the
newly introduced exynos_chipid_get_regmap_chipid_info() method. The
nvmem consumer support that will follow won't use the regmap, and
instead will use the nvmem API. It will need the pointer to the device
to report errors.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/soc/samsung/exynos-chipid.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
index ab6bdf24a754a0faf974190c1fa1f99735cbef8e..7b1951f28e8d4958ab941af91dab4b0183ceda5f 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -26,17 +26,21 @@
 
 #include "exynos-asv.h"
 
+struct exynos_chipid_info {
+	struct regmap *regmap;
+	struct device *dev;
+	u32 product_id;
+	u32 revision;
+};
+
 struct exynos_chipid_variant {
+	int (*get_chipid_info)(const struct exynos_chipid_variant *data,
+			       struct exynos_chipid_info *exynos_chipid);
 	unsigned int rev_reg;		/* revision register offset */
 	unsigned int main_rev_shift;	/* main revision offset in rev_reg */
 	unsigned int sub_rev_shift;	/* sub revision offset in rev_reg */
 };
 
-struct exynos_chipid_info {
-	u32 product_id;
-	u32 revision;
-};
-
 static const struct exynos_soc_id {
 	const char *name;
 	unsigned int id;
@@ -80,13 +84,19 @@ static const char *product_id_to_soc_id(unsigned int product_id)
 	return NULL;
 }
 
-static int exynos_chipid_get_chipid_info(struct regmap *regmap,
-		const struct exynos_chipid_variant *data,
+static int exynos_chipid_get_regmap_chipid_info(const struct exynos_chipid_variant *data,
 		struct exynos_chipid_info *exynos_chipid)
 {
 	int ret;
+	struct regmap *regmap;
 	unsigned int val, main_rev, sub_rev;
 
+	regmap = device_node_to_regmap(exynos_chipid->dev->of_node);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	exynos_chipid->regmap = regmap;
+
 	ret = regmap_read(regmap, EXYNOS_CHIPID_REG_PRO_ID, &val);
 	if (ret < 0)
 		return ret;
@@ -112,7 +122,6 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct soc_device *soc_dev;
 	struct device_node *root;
-	struct regmap *regmap;
 	int ret;
 
 	data = of_device_get_match_data(dev);
@@ -123,11 +132,9 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 	if (!exynos_chipid)
 		return -ENOMEM;
 
-	regmap = device_node_to_regmap(dev->of_node);
-	if (IS_ERR(regmap))
-		return PTR_ERR(regmap);
+	exynos_chipid->dev = dev;
 
-	ret = exynos_chipid_get_chipid_info(regmap, data, exynos_chipid);
+	ret = data->get_chipid_info(data, exynos_chipid);
 	if (ret < 0)
 		return ret;
 
@@ -156,7 +163,7 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 	if (IS_ERR(soc_dev))
 		return PTR_ERR(soc_dev);
 
-	ret = exynos_asv_init(dev, regmap);
+	ret = exynos_asv_init(dev, exynos_chipid->regmap);
 	if (ret)
 		goto err;
 
@@ -182,12 +189,14 @@ static void exynos_chipid_remove(struct platform_device *pdev)
 }
 
 static const struct exynos_chipid_variant exynos4210_chipid_data = {
+	.get_chipid_info = exynos_chipid_get_regmap_chipid_info,
 	.rev_reg	= 0x0,
 	.main_rev_shift	= 4,
 	.sub_rev_shift	= 0,
 };
 
 static const struct exynos_chipid_variant exynos850_chipid_data = {
+	.get_chipid_info = exynos_chipid_get_regmap_chipid_info,
 	.rev_reg	= 0x10,
 	.main_rev_shift	= 20,
 	.sub_rev_shift	= 16,

-- 
2.51.1.930.gacf6e81ea2-goog

[PATCH 06/11] soc: samsung: exynos-chipid: make asv_init opt-in

[Tudor Ambarus]

Current ASV handling is tightly coupled with a specific Product ID
(0xe5422000) - only one from the list of Product IDs defined in
the soc_ids[] array.

Since the implementation is highly specific, make te asv_init opt-in.

Note that the exynos850_chipid_data was intentionally filled with
.asv_init = true to not add any change in functionality, even if ASV
bails out early for these chips. We can follow up with a patch to
remove the ASV init for these chips, or better, to update them to use
the nvmem API that GS101 will use, as I suspect they reference OTP
controller registers too.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/soc/samsung/exynos-chipid.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
index 7b1951f28e8d4958ab941af91dab4b0183ceda5f..5678bc85c4d93547e446caade64a0b650d06a332 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -39,6 +39,7 @@ struct exynos_chipid_variant {
 	unsigned int rev_reg;		/* revision register offset */
 	unsigned int main_rev_shift;	/* main revision offset in rev_reg */
 	unsigned int sub_rev_shift;	/* sub revision offset in rev_reg */
+	bool asv_init;
 };
 
 static const struct exynos_soc_id {
@@ -163,9 +164,11 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 	if (IS_ERR(soc_dev))
 		return PTR_ERR(soc_dev);
 
-	ret = exynos_asv_init(dev, exynos_chipid->regmap);
-	if (ret)
-		goto err;
+	if (data->asv_init) {
+		ret = exynos_asv_init(dev, exynos_chipid->regmap);
+		if (ret)
+			goto err;
+	}
 
 	platform_set_drvdata(pdev, soc_dev);
 
@@ -193,6 +196,7 @@ static const struct exynos_chipid_variant exynos4210_chipid_data = {
 	.rev_reg	= 0x0,
 	.main_rev_shift	= 4,
 	.sub_rev_shift	= 0,
+	.asv_init	= true,
 };
 
 static const struct exynos_chipid_variant exynos850_chipid_data = {
@@ -200,6 +204,7 @@ static const struct exynos_chipid_variant exynos850_chipid_data = {
 	.rev_reg	= 0x10,
 	.main_rev_shift	= 20,
 	.sub_rev_shift	= 16,
+	.asv_init	= true,
 };
 
 static const struct of_device_id exynos_chipid_of_device_ids[] = {

-- 
2.51.1.930.gacf6e81ea2-goog

[PATCH 07/11] soc: samsung: exynos-chipid: add support for google,gs101-chipid

[Tudor Ambarus]

GS101 reads the Product ID and the Chip ID from the OTP controller
registers. Add suppot for google,gs101-chipid.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/soc/samsung/exynos-chipid.c | 76 +++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
index 5678bc85c4d93547e446caade64a0b650d06a332..9c43c615167e4bbf15d00d42c2e52cc64d7dae1a 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -13,10 +13,12 @@
  */
 
 #include <linux/array_size.h>
+#include <linux/bitfield.h>
 #include <linux/device.h>
 #include <linux/errno.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -26,6 +28,11 @@
 
 #include "exynos-asv.h"
 
+#define EXYNOS_GS101_MAIN_REV		GENMASK(3, 0)
+#define EXYNOS_GS101_SUB_REV		GENMASK(19, 16)
+#define EXYNOS_GS101_CHIP_ID_SIZE	16
+#define EXYNOS_GS101_PRODUCT_ID_SIZE	4
+
 struct exynos_chipid_info {
 	struct regmap *regmap;
 	struct device *dev;
@@ -73,6 +80,8 @@ static const struct exynos_soc_id {
 	{ "EXYNOS990", 0xE9830000 },
 	{ "EXYNOSAUTOV9", 0xAAA80000 },
 	{ "EXYNOSAUTOV920", 0x0A920000 },
+	/* Compatible with: google,gs101-chipid */
+	{ "GS101", 0x09845000 },
 };
 
 static const char *product_id_to_soc_id(unsigned int product_id)
@@ -85,6 +94,66 @@ static const char *product_id_to_soc_id(unsigned int product_id)
 	return NULL;
 }
 
+static int exynos_chipid_get_efuse_data(struct device *dev,
+					const char *nvmem_cell_name,
+					u32 **efuse, size_t *size,
+					size_t expected_size)
+{
+	struct nvmem_cell *cell;
+
+	cell = nvmem_cell_get(dev, nvmem_cell_name);
+	if (IS_ERR(cell)) {
+		dev_err(dev, "no \"%s\"? %ld\n", nvmem_cell_name,
+			PTR_ERR(cell));
+		return PTR_ERR(cell);
+	}
+
+	*efuse = nvmem_cell_read(cell, size);
+	nvmem_cell_put(cell);
+	if (IS_ERR(*efuse))
+		return PTR_ERR(*efuse);
+
+	if (*size != expected_size) {
+		dev_err(dev, "Invalid efuse data size %zu\n", *size);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int exynos_chipid_get_efuse_chipid_info(const struct exynos_chipid_variant *data,
+		struct exynos_chipid_info *exynos_chipid)
+{
+	struct device *dev = exynos_chipid->dev;
+	size_t product_id_size, chip_id_size;
+	u32 *product_id, *chip_id;
+	u32 main_rev, sub_rev;
+	int ret;
+
+	ret = exynos_chipid_get_efuse_data(dev, "product-id", &product_id,
+					   &product_id_size,
+					   EXYNOS_GS101_PRODUCT_ID_SIZE);
+	if (ret)
+		return ret;
+
+	ret = exynos_chipid_get_efuse_data(dev, "chip-id", &chip_id,
+					   &chip_id_size,
+					   EXYNOS_GS101_CHIP_ID_SIZE);
+	if (ret)
+		return ret;
+
+	exynos_chipid->product_id = product_id[0] & EXYNOS_MASK;
+
+	main_rev = FIELD_GET(EXYNOS_GS101_MAIN_REV, product_id[0]);
+	sub_rev = FIELD_GET(EXYNOS_GS101_SUB_REV, chip_id[3]);
+	exynos_chipid->revision = (main_rev << EXYNOS_REV_PART_SHIFT) | sub_rev;
+
+	kfree(product_id);
+	kfree(chip_id);
+
+	return 0;
+}
+
 static int exynos_chipid_get_regmap_chipid_info(const struct exynos_chipid_variant *data,
 		struct exynos_chipid_info *exynos_chipid)
 {
@@ -191,6 +260,10 @@ static void exynos_chipid_remove(struct platform_device *pdev)
 	soc_device_unregister(soc_dev);
 }
 
+static const struct exynos_chipid_variant gs101_chipid_data = {
+	.get_chipid_info = exynos_chipid_get_efuse_chipid_info,
+};
+
 static const struct exynos_chipid_variant exynos4210_chipid_data = {
 	.get_chipid_info = exynos_chipid_get_regmap_chipid_info,
 	.rev_reg	= 0x0,
@@ -209,6 +282,9 @@ static const struct exynos_chipid_variant exynos850_chipid_data = {
 
 static const struct of_device_id exynos_chipid_of_device_ids[] = {
 	{
+		.compatible	= "google,gs101-chipid",
+		.data		= &gs101_chipid_data,
+	}, {
 		.compatible	= "samsung,exynos4210-chipid",
 		.data		= &exynos4210_chipid_data,
 	}, {

-- 
2.51.1.930.gacf6e81ea2-goog

[PATCH 08/11] soc: samsung: exynos-chipid: prepend exynos_ to a method's name

[Tudor Ambarus]

Avoid name space pollution and prepend exynos_ to
product_id_to_soc_id().

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/soc/samsung/exynos-chipid.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
index 9c43c615167e4bbf15d00d42c2e52cc64d7dae1a..92cd5dfc0a9b641428d0d047cfefd4256a893ac0 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -84,7 +84,7 @@ static const struct exynos_soc_id {
 	{ "GS101", 0x09845000 },
 };
 
-static const char *product_id_to_soc_id(unsigned int product_id)
+static const char *exynos_product_id_to_soc_id(unsigned int product_id)
 {
 	int i;
 
@@ -222,7 +222,7 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 						exynos_chipid->revision);
 	if (!soc_dev_attr->revision)
 		return -ENOMEM;
-	soc_dev_attr->soc_id = product_id_to_soc_id(exynos_chipid->product_id);
+	soc_dev_attr->soc_id = exynos_product_id_to_soc_id(exynos_chipid->product_id);
 	if (!soc_dev_attr->soc_id) {
 		pr_err("Unknown SoC\n");
 		return -ENODEV;

-- 
2.51.1.930.gacf6e81ea2-goog

[PATCH 09/11] soc: samsung: exynos-chipid: downgrade dev_info to dev_dbg for soc info

[Tudor Ambarus]

The SoC information is exposed to userspace using the standard soc
interface. Downgrade to dev_dbg to stop polluting the console log.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/soc/samsung/exynos-chipid.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
index 92cd5dfc0a9b641428d0d047cfefd4256a893ac0..70e09ac656e4b95d5d252272bbe02bcf0f6fb2ab 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -241,9 +241,9 @@ static int exynos_chipid_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, soc_dev);
 
-	dev_info(dev, "Exynos: CPU[%s] PRO_ID[0x%x] REV[0x%x] Detected\n",
-		 soc_dev_attr->soc_id, exynos_chipid->product_id,
-		 exynos_chipid->revision);
+	dev_dbg(dev, "Exynos: CPU[%s] PRO_ID[0x%x] REV[0x%x] Detected\n",
+		soc_dev_attr->soc_id, exynos_chipid->product_id,
+		exynos_chipid->revision);
 
 	return 0;
 

-- 
2.51.1.930.gacf6e81ea2-goog

[PATCH 10/11] arm64: dts: exynos: gs101: add the chipid node

[Tudor Ambarus]

Add the chipid node.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 arch/arm64/boot/dts/exynos/google/gs101.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
index d06d1d05f36408137a8acd98e43d48ea7d4f4292..11622da2d46ff257b447a3dfdc98abdf29a45b9a 100644
--- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
+++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
@@ -467,6 +467,12 @@ opp-2802000000 {
 		};
 	};
 
+	chipid {
+		compatible = "google,gs101-chipid";
+		nvmem-cells = <&product_id>, <&chip_id>;
+		nvmem-cell-names = "product-id", "chip-id";
+	};
+
 	/* ect node is required to be present by bootloader */
 	ect {
 	};

-- 
2.51.1.930.gacf6e81ea2-goog

[PATCH 11/11] arm64: defconfig: enable Samsung Exynos chipid driver

[Tudor Ambarus]

The Samsung Exynos chipid driver provides SoC information to userspace
using the standard soc interface. Enable this to allow certain user
space tools to acquire this information.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 646097e94efe7f1a18fb59d5b6dfc6268be91383..cf1a548034d28db201bab0d48b838b4136dbb59c 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1530,6 +1530,7 @@ CONFIG_QCOM_APR=m
 CONFIG_QCOM_ICC_BWMON=m
 CONFIG_QCOM_PBS=m
 CONFIG_ROCKCHIP_IODOMAIN=y
+CONFIG_EXYNOS_CHIPID=m
 CONFIG_ARCH_TEGRA_132_SOC=y
 CONFIG_ARCH_TEGRA_210_SOC=y
 CONFIG_ARCH_TEGRA_186_SOC=y

-- 
2.51.1.930.gacf6e81ea2-goog

Re: [PATCH 01/11] dt-bindings: hwinfo: samsung,exynos-chipid: add google,gs101 compatible

[Krzysztof Kozlowski]

On Fri, Oct 31, 2025 at 12:56:00PM +0000, Tudor Ambarus wrote:
> Google GS101 Chip ID references the nvmem cells from the OTP controller,
> it doesn't need a reg space. Add the google,gs101-chipid compatible.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> ---
>  .../bindings/hwinfo/samsung,exynos-chipid.yaml     | 51 +++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml b/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml
> index b9cdfe52b62ff3a365d61368c39db21facff6565..6d6260be02d47712ebf1e4d31973802e8340cdfe 100644
> --- a/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml
> +++ b/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml
> @@ -33,6 +33,16 @@ properties:
>                - samsung,exynosautov9-chipid
>                - samsung,exynosautov920-chipid
>            - const: samsung,exynos850-chipid
> +      - items:
> +          - const: google,gs101-chipid

That's a part of first enum at the beginning.

> +
> +  nvmem-cells:
> +    maxItems: 2
> +
> +  nvmem-cell-names:
> +    items:
> +      - const: product-id
> +      - const: chip-id
>  
>    reg:
>      maxItems: 1
> @@ -47,7 +57,46 @@ properties:
>  
>  required:
>    - compatible
> -  - reg
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            oneOf:

No, don't copy. It's just redundant. Just like the fallbacks under one
enum.

> +              - enum:
> +                  - samsung,exynos4210-chipid
> +                  - samsung,exynos850-chipid
> +              - items:
> +                  - enum:
> +                      - samsung,exynos5433-chipid
> +                      - samsung,exynos7-chipid
> +                      - samsung,exynos7870-chipid
> +                      - samsung,exynos8890-chipid
> +                  - const: samsung,exynos4210-chipid
> +              - items:
> +                  - enum:
> +                      - samsung,exynos2200-chipid
> +                      - samsung,exynos7885-chipid
> +                      - samsung,exynos8895-chipid
> +                      - samsung,exynos9610-chipid
> +                      - samsung,exynos9810-chipid
> +                      - samsung,exynos990-chipid
> +                      - samsung,exynosautov9-chipid
> +                      - samsung,exynosautov920-chipid
> +                  - const: samsung,exynos850-chipid
> +    then:

properties:
  nvmem-cells: false
  same for names

> +      required:
> +        - reg
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: google,gs101-chipid
> +    then:

reg: false, similarly.

> +      required:
> +        - nvmem-cells
> +        - nvmem-cell-names
>  
>  additionalProperties: false
>  
> 
> -- 
> 2.51.1.930.gacf6e81ea2-goog
> 

Re: (subset) [PATCH 02/11] soc: samsung: exynos-chipid: use a local dev variable

[Krzysztof Kozlowski]

On Fri, 31 Oct 2025 12:56:01 +0000, Tudor Ambarus wrote:
> Use a local variable for struct device to avoid dereferencing.
> 
> 

Applied, thanks!

[02/11] soc: samsung: exynos-chipid: use a local dev variable
        https://git.kernel.org/krzk/linux/c/bea18c67a0ec75cc602aabc523d726a4626e1341

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Re: [PATCH 03/11] soc: samsung: exynos-chipid: use heap allocated driver data

[Krzysztof Kozlowski]

On Fri, Oct 31, 2025 at 12:56:02PM +0000, Tudor Ambarus wrote:
> Prepare for GS101 chipid addition, it will requre more driver data. Use
> heap allocated driver data, it's easily extensible. Together with

Moving from stack to heap does not give you "extensibility". It is
extensible in current form as well. It allows to use it past probe,
which probably is what you wanted for some reason.

> the change s/soc_info/exynos_chipid, it better indicates that it is the
> driver data structure.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 04/11] soc: samsung: exynos-chipid: refer to match->data as data

[Krzysztof Kozlowski]

On Fri, Oct 31, 2025 at 12:56:03PM +0000, Tudor Ambarus wrote:
> Be consistent across the driver and refer to match->data as data.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> ---
>  drivers/soc/samsung/exynos-chipid.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> index f5107b8b339e8884d6a67a80650a95441510c66c..ab6bdf24a754a0faf974190c1fa1f99735cbef8e 100644
> --- a/drivers/soc/samsung/exynos-chipid.c
> +++ b/drivers/soc/samsung/exynos-chipid.c
> @@ -106,7 +106,7 @@ static int exynos_chipid_get_chipid_info(struct regmap *regmap,
>  
>  static int exynos_chipid_probe(struct platform_device *pdev)
>  {
> -	const struct exynos_chipid_variant *drv_data;
> +	const struct exynos_chipid_variant *data;

I would prefer name drv_data, to mark clearly this is driver data.
Sometimes driver state container is called "data".

Best regards,
Krzysztof

Re: [PATCH 11/11] arm64: defconfig: enable Samsung Exynos chipid driver

[Krzysztof Kozlowski]

On 31/10/2025 13:56, Tudor Ambarus wrote:
> The Samsung Exynos chipid driver provides SoC information to userspace
> using the standard soc interface. Enable this to allow certain user

It is already enabled.

> space tools to acquire this information.
> 

Best regards,
Krzysztof

Re: [PATCH 05/11] soc: samsung: exynos-chipid: introduce match_data->get_chipid_info()

[Krzysztof Kozlowski]

On Fri, Oct 31, 2025 at 12:56:04PM +0000, Tudor Ambarus wrote:
> Newer SoCs, like GS101, don't have a dedicated Chip ID controller.

This would suggest that these are completely different devices and
should not be part of the same bindings. Actually bindings also
suggested this - changing programming model.

> The GS101 Chip ID info is available as part of the OTP controller
> registers, among other things. For GS101 we will read the Chip ID from
> the OTP controller using the nvmem API.
> 
> Extend the match_data with a get_chipid_info() method, to allow nvmem
> integration.
> 
> `struct exynos_chipid_info` is moved to the top of the file to avoid a
> forward declaration. The structure is extended with pointers to device
> and regmap to allow current implementation to obtain the regmap in the
> newly introduced exynos_chipid_get_regmap_chipid_info() method. The
> nvmem consumer support that will follow won't use the regmap, and
> instead will use the nvmem API. It will need the pointer to the device
> to report errors.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> ---
>  drivers/soc/samsung/exynos-chipid.c | 35 ++++++++++++++++++++++-------------
>  1 file changed, 22 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> index ab6bdf24a754a0faf974190c1fa1f99735cbef8e..7b1951f28e8d4958ab941af91dab4b0183ceda5f 100644
> --- a/drivers/soc/samsung/exynos-chipid.c
> +++ b/drivers/soc/samsung/exynos-chipid.c
> @@ -26,17 +26,21 @@
>  
>  #include "exynos-asv.h"
>  
> +struct exynos_chipid_info {
> +	struct regmap *regmap;
> +	struct device *dev;
> +	u32 product_id;
> +	u32 revision;
> +};
> +
>  struct exynos_chipid_variant {
> +	int (*get_chipid_info)(const struct exynos_chipid_variant *data,
> +			       struct exynos_chipid_info *exynos_chipid);
>  	unsigned int rev_reg;		/* revision register offset */
>  	unsigned int main_rev_shift;	/* main revision offset in rev_reg */
>  	unsigned int sub_rev_shift;	/* sub revision offset in rev_reg */
>  };
>  
> -struct exynos_chipid_info {
> -	u32 product_id;
> -	u32 revision;
> -};
> -
>  static const struct exynos_soc_id {
>  	const char *name;
>  	unsigned int id;
> @@ -80,13 +84,19 @@ static const char *product_id_to_soc_id(unsigned int product_id)
>  	return NULL;
>  }
>  
> -static int exynos_chipid_get_chipid_info(struct regmap *regmap,
> -		const struct exynos_chipid_variant *data,
> +static int exynos_chipid_get_regmap_chipid_info(const struct exynos_chipid_variant *data,
>  		struct exynos_chipid_info *exynos_chipid)
>  {

This function now gets both regmap and chip info, that's too much.
Probably all ASV and regmap getting should be somehow split/customized
per variant.

I don't know yet, need to read rest of patches.

Best regards,
Krzysztof

Re: [PATCH 10/11] arm64: dts: exynos: gs101: add the chipid node

[Krzysztof Kozlowski]

On Fri, Oct 31, 2025 at 12:56:09PM +0000, Tudor Ambarus wrote:
> Add the chipid node.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> ---
>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> index d06d1d05f36408137a8acd98e43d48ea7d4f4292..11622da2d46ff257b447a3dfdc98abdf29a45b9a 100644
> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> @@ -467,6 +467,12 @@ opp-2802000000 {
>  		};
>  	};
>  
> +	chipid {
> +		compatible = "google,gs101-chipid";

That's not a real device, sorry.

I had some doubts when reading the bindings, then more when reading
driver - like chipid probe() was basically empty, no single device
access, except calling other kernel subsystem - and now here no single
actual hardware resource, except reference to other node.

Are you REALLY REALLY sure you have in your datasheet such device as
chipid?

It is damn basic question, which you should start with.


Best regards,
Krzysztof

Re: [PATCH 10/11] arm64: dts: exynos: gs101: add the chipid node

[Tudor Ambarus]

On 11/3/25 12:18 PM, Krzysztof Kozlowski wrote:
> On Fri, Oct 31, 2025 at 12:56:09PM +0000, Tudor Ambarus wrote:
>> Add the chipid node.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>> ---
>>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>> index d06d1d05f36408137a8acd98e43d48ea7d4f4292..11622da2d46ff257b447a3dfdc98abdf29a45b9a 100644
>> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>> @@ -467,6 +467,12 @@ opp-2802000000 {
>>  		};
>>  	};
>>  
>> +	chipid {
>> +		compatible = "google,gs101-chipid";
> 
> That's not a real device, sorry.
> 
> I had some doubts when reading the bindings, then more when reading
> driver - like chipid probe() was basically empty, no single device
> access, except calling other kernel subsystem - and now here no single
> actual hardware resource, except reference to other node.
> 
> Are you REALLY REALLY sure you have in your datasheet such device as
> chipid?
> 
> It is damn basic question, which you should start with.

Documentation says that  GS101 "includes a CHIPID block for the software
that sends and receives APB interface signals to and from the bus system.
The first address of the SFR region (0x1000_0000) contains the product ID."

0x1000_0000 is the base address of the OTP controller (OTP_CON_TOP).

"CHIPID block" tells it's a device, no? But now I think it was just an
unfortunate datasheet description. Do you have an advice on how I shall
treat this next please? Maybe register to the soc interface directly from
the OTP controller driver?

Thanks!
ta

Re: [PATCH 10/11] arm64: dts: exynos: gs101: add the chipid node

[Krzysztof Kozlowski]

On 03/11/2025 11:50, Tudor Ambarus wrote:
> 
> 
> On 11/3/25 12:18 PM, Krzysztof Kozlowski wrote:
>> On Fri, Oct 31, 2025 at 12:56:09PM +0000, Tudor Ambarus wrote:
>>> Add the chipid node.
>>>
>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>>> ---
>>>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>> index d06d1d05f36408137a8acd98e43d48ea7d4f4292..11622da2d46ff257b447a3dfdc98abdf29a45b9a 100644
>>> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>> @@ -467,6 +467,12 @@ opp-2802000000 {
>>>  		};
>>>  	};
>>>  
>>> +	chipid {
>>> +		compatible = "google,gs101-chipid";
>>
>> That's not a real device, sorry.
>>
>> I had some doubts when reading the bindings, then more when reading
>> driver - like chipid probe() was basically empty, no single device
>> access, except calling other kernel subsystem - and now here no single
>> actual hardware resource, except reference to other node.
>>
>> Are you REALLY REALLY sure you have in your datasheet such device as
>> chipid?
>>
>> It is damn basic question, which you should start with.
> 
> Documentation says that  GS101 "includes a CHIPID block for the software
> that sends and receives APB interface signals to and from the bus system.
> The first address of the SFR region (0x1000_0000) contains the product ID."
> 
> 0x1000_0000 is the base address of the OTP controller (OTP_CON_TOP).
> 
> "CHIPID block" tells it's a device, no? But now I think it was just an
> unfortunate datasheet description. Do you have an advice on how I shall
> treat this next please? Maybe register to the soc interface directly from
> the OTP controller driver?
> 


Huh, then I am confused, because:
1. That's the same message as in other Exynos and it has SFR region
2. Your binding said there is no SFR region.
3. Anyway, please post complete DTS, so if this has SFR region it must
have proper reg entry. You cannot skip it.

Of course next question would be what is the OTP controller...


Best regards,
Krzysztof

Re: [PATCH 10/11] arm64: dts: exynos: gs101: add the chipid node

[Tudor Ambarus]

On 11/3/25 1:01 PM, Krzysztof Kozlowski wrote:
> On 03/11/2025 11:50, Tudor Ambarus wrote:
>>
>>
>> On 11/3/25 12:18 PM, Krzysztof Kozlowski wrote:
>>> On Fri, Oct 31, 2025 at 12:56:09PM +0000, Tudor Ambarus wrote:
>>>> Add the chipid node.
>>>>
>>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>> ---
>>>>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>>> index d06d1d05f36408137a8acd98e43d48ea7d4f4292..11622da2d46ff257b447a3dfdc98abdf29a45b9a 100644
>>>> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>>> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>>> @@ -467,6 +467,12 @@ opp-2802000000 {
>>>>  		};
>>>>  	};
>>>>  
>>>> +	chipid {
>>>> +		compatible = "google,gs101-chipid";
>>>
>>> That's not a real device, sorry.
>>>
>>> I had some doubts when reading the bindings, then more when reading
>>> driver - like chipid probe() was basically empty, no single device
>>> access, except calling other kernel subsystem - and now here no single
>>> actual hardware resource, except reference to other node.
>>>
>>> Are you REALLY REALLY sure you have in your datasheet such device as
>>> chipid?
>>>
>>> It is damn basic question, which you should start with.
>>
>> Documentation says that  GS101 "includes a CHIPID block for the software
>> that sends and receives APB interface signals to and from the bus system.
>> The first address of the SFR region (0x1000_0000) contains the product ID."
>>
>> 0x1000_0000 is the base address of the OTP controller (OTP_CON_TOP).
>>
>> "CHIPID block" tells it's a device, no? But now I think it was just an
>> unfortunate datasheet description. Do you have an advice on how I shall
>> treat this next please? Maybe register to the soc interface directly from
>> the OTP controller driver?
>>
> 
> 
> Huh, then I am confused, because:
> 1. That's the same message as in other Exynos and it has SFR region
> 2. Your binding said there is no SFR region.
> 3. Anyway, please post complete DTS, so if this has SFR region it must
> have proper reg entry. You cannot skip it.
> 
> Of course next question would be what is the OTP controller...

The CHIPID block, which has a dedicated chapter and all :), consists of two
registers:

Product ID
Address = Base Address (0x1000_0000) + 0x0000, Reset Value = 0xE383_0000

Chipid 3
Address = Base Address (0x1000_0000) + 0x0010, Reset Value = 0x0000_0000

While the Product ID is fixed (fused I assume), the CHIPID registers:
"depend on the OTP value. When the power-on sequence progresses, the OTP
values are loaded to the registers. These registers can read the loaded
current OTP values."

OTP values are from the OTP memory (32Kbit) from address 5'b00000, 160 bits
in total. Even if not explicitly stated, I think the OTP controller copies
the CHIP ID from the OTP memory to its registers, so that the "CHIPID block"
can access them. You notice that the reset value of the CHIPID OTP registers
is zero. They're then filled at power-on.

This is all. I lean towards thinking the CHIPID block is not really a device.
It's just a way software gets the product and chip IDs, which is by querying
the OTP registers. I think I lean towards registering to the soc interface
directly via the OTP device. Or maybe you think differently?

Thanks,
ta

Re: [PATCH 10/11] arm64: dts: exynos: gs101: add the chipid node

[Krzysztof Kozlowski]

On 03/11/2025 11:50, Tudor Ambarus wrote:
> 
> 
> On 11/3/25 12:18 PM, Krzysztof Kozlowski wrote:
>> On Fri, Oct 31, 2025 at 12:56:09PM +0000, Tudor Ambarus wrote:
>>> Add the chipid node.
>>>
>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>>> ---
>>>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>> index d06d1d05f36408137a8acd98e43d48ea7d4f4292..11622da2d46ff257b447a3dfdc98abdf29a45b9a 100644
>>> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>> @@ -467,6 +467,12 @@ opp-2802000000 {
>>>  		};
>>>  	};
>>>  
>>> +	chipid {
>>> +		compatible = "google,gs101-chipid";
>>
>> That's not a real device, sorry.
>>
>> I had some doubts when reading the bindings, then more when reading
>> driver - like chipid probe() was basically empty, no single device
>> access, except calling other kernel subsystem - and now here no single
>> actual hardware resource, except reference to other node.
>>
>> Are you REALLY REALLY sure you have in your datasheet such device as
>> chipid?
>>
>> It is damn basic question, which you should start with.
> 
> Documentation says that  GS101 "includes a CHIPID block for the software
> that sends and receives APB interface signals to and from the bus system.
> The first address of the SFR region (0x1000_0000) contains the product ID."

So chipid@1000_0000

> 
> 0x1000_0000 is the base address of the OTP controller (OTP_CON_TOP).


and efuse@1000_0000 from your other patchset and your sentence above.

Please add them to DTS and check for warnings.

> 
> "CHIPID block" tells it's a device, no? But now I think it was just an
> unfortunate datasheet description. Do you have an advice on how I shall
> treat this next please? Maybe register to the soc interface directly from
> the OTP controller driver?
I think in the SoC it is impossible or at least never happening that you
create two devices for the same address, therefore either chipid is a
device or efuse is a device.

Best regards,
Krzysztof

Re: [PATCH 10/11] arm64: dts: exynos: gs101: add the chipid node

[Tudor Ambarus]

On 11/4/25 9:14 AM, Krzysztof Kozlowski wrote:
> On 03/11/2025 11:50, Tudor Ambarus wrote:
>>
>>
>> On 11/3/25 12:18 PM, Krzysztof Kozlowski wrote:
>>> On Fri, Oct 31, 2025 at 12:56:09PM +0000, Tudor Ambarus wrote:
>>>> Add the chipid node.
>>>>
>>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>> ---
>>>>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>>> index d06d1d05f36408137a8acd98e43d48ea7d4f4292..11622da2d46ff257b447a3dfdc98abdf29a45b9a 100644
>>>> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>>> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>>> @@ -467,6 +467,12 @@ opp-2802000000 {
>>>>  		};
>>>>  	};
>>>>  
>>>> +	chipid {
>>>> +		compatible = "google,gs101-chipid";
>>>
>>> That's not a real device, sorry.
>>>
>>> I had some doubts when reading the bindings, then more when reading
>>> driver - like chipid probe() was basically empty, no single device
>>> access, except calling other kernel subsystem - and now here no single
>>> actual hardware resource, except reference to other node.
>>>
>>> Are you REALLY REALLY sure you have in your datasheet such device as
>>> chipid?
>>>
>>> It is damn basic question, which you should start with.
>>
>> Documentation says that  GS101 "includes a CHIPID block for the software
>> that sends and receives APB interface signals to and from the bus system.
>> The first address of the SFR region (0x1000_0000) contains the product ID."
> 
> So chipid@1000_0000
> 
>>
>> 0x1000_0000 is the base address of the OTP controller (OTP_CON_TOP).
> 
> 
> and efuse@1000_0000 from your other patchset and your sentence above.
> 
> Please add them to DTS and check for warnings.

One would get an unique_unit_address warning, yes.

> 
>>
>> "CHIPID block" tells it's a device, no? But now I think it was just an
>> unfortunate datasheet description. Do you have an advice on how I shall
>> treat this next please? Maybe register to the soc interface directly from
>> the OTP controller driver?
> I think in the SoC it is impossible or at least never happening that you
> create two devices for the same address, therefore either chipid is a
> device or efuse is a device.
> 
Right. The ChipID "block" does not have its own registers. It references
the OTP registers. I'll describe just the efuse device in the DT and
register to the soc interface directly from the efuse driver. Please let
me know if you think there's a better way to handle this.

Thanks,
ta