All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] nvmem: sunxi-sid: add support for H3 and A64's SID controller
@ 2017-01-26 11:33 ` Icenowy Zheng
  0 siblings, 0 replies; 14+ messages in thread
From: Icenowy Zheng @ 2017-01-26 11:33 UTC (permalink / raw)
  To: Srinivas Kandagatla, Maxime Ripard, Rob Herring, Chen-Yu Tsai
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng

H3 and A64 SoCs have a bigger SID controller, which has its direct read
address at 0x200 position in the SID block, not 0x0.

Also, H3 SID controller has some silicon bug that makes the direct read
value wrong at first, add code to workaround the bug. (This bug has
already been fixed on A64 and later SoCs)

Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
---
 .../bindings/nvmem/allwinner,sunxi-sid.txt         |  22 +++-
 drivers/nvmem/sunxi_sid.c                          | 112 +++++++++++++++++++--
 2 files changed, 123 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
index d543ed3f5363..d7400c415338 100644
--- a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
+++ b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
@@ -1,8 +1,16 @@
 Allwinner sunxi-sid
 
 Required properties:
-- compatible: "allwinner,sun4i-a10-sid" or "allwinner,sun7i-a20-sid"
-- reg: Should contain registers location and length
+- compatible: Should be one of the following (depending on your SoC):
+  "allwinner,sun4i-a10-sid"
+  "allwinner,sun7i-a20-sid"
+  "allwinner,sun8i-h3-sid"
+  "allwinner,sun50i-a64-sid"
+
+- reg: Should contain registers location and registers length, for A10 and A20
+  SoCs the length is directly the length of SID; for H3, A64 and newer SoCs
+  the length should be the sum of the length of SID and the offset of the value
+  (0x200).
 
 = Data cells =
 Are child nodes of qfprom, bindings of which as described in
@@ -19,3 +27,13 @@ Example for sun7i:
 		compatible = "allwinner,sun7i-a20-sid";
 		reg = <0x01c23800 0x200>
 	};
+
+Example for sun8i-h3:
+	sid@01c14000 {
+		compatible = "allwinner,sun8i-h3-sid";
+		/*
+		 * The length of SID on H3 is 0x100 bytes, add the value offset
+		 * 0x200, so the total length should be 0x300.
+		 */
+		reg = <0x01c14000 0x300>;
+	};
diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
index 1567ccca8de3..2e327a66a938 100644
--- a/drivers/nvmem/sunxi_sid.c
+++ b/drivers/nvmem/sunxi_sid.c
@@ -20,10 +20,28 @@
 #include <linux/module.h>
 #include <linux/nvmem-provider.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/random.h>
 
+/* Registers and special values for doing register-based SID readout on H3 */
+#define SUN8I_SID_PRCTL		0x40
+#define SUN8I_SID_RDKEY		0x60
+
+#define SUN8I_SID_OP_LOCK	0xAC
+#define SUN8I_SID_OFFSET_MASK	0x1FF
+#define SUN8I_SID_OFFSET_SHIFT	16
+#define SUN8I_SID_LOCK_SHIFT	8
+#define SUN8I_SID_READ		BIT(1)
+
+/*
+ * For newer SoCs with a larger eFUSE, the bytes beyond the first 16 bytes are
+ * sparse, which makes it not suitable for adding randomness; legacy SoCs' SID
+ * have only 16 bytes, so we choose to use at most 16 bytes to add randomness.
+ */
+#define SUNXI_SID_MAX_RANDOMNESS_SIZE	16
+
 static struct nvmem_config econfig = {
 	.name = "sunxi-sid",
 	.read_only = true,
@@ -32,8 +50,14 @@ static struct nvmem_config econfig = {
 	.owner = THIS_MODULE,
 };
 
+struct sunxi_sid_cfg {
+	u32	value_offset;
+	bool	need_register_readout;
+};
+
 struct sunxi_sid {
 	void __iomem		*base;
+	u32			value_offset;
 };
 
 /* We read the entire key, due to a 32 bit read alignment requirement. Since we
@@ -46,7 +70,8 @@ static u8 sunxi_sid_read_byte(const struct sunxi_sid *sid,
 {
 	u32 sid_key;
 
-	sid_key = ioread32be(sid->base + round_down(offset, 4));
+	sid_key = ioread32be(sid->base + sid->value_offset +
+			     round_down(offset, 4));
 	sid_key >>= (offset % 4) * 8;
 
 	return sid_key; /* Only return the last byte */
@@ -64,26 +89,77 @@ static int sunxi_sid_read(void *context, unsigned int offset,
 	return 0;
 }
 
+static int sun8i_sid_register_readout(const struct sunxi_sid *sid,
+				      const unsigned int word,
+				      u32 *out)
+{
+	u32 reg_val;
+	unsigned long expire = jiffies + msecs_to_jiffies(250);
+
+	/* Set word, lock access, and set read command */
+	reg_val = (word & SUN8I_SID_OFFSET_MASK)
+		  << SUN8I_SID_OFFSET_SHIFT;
+	reg_val |= SUN8I_SID_OP_LOCK << SUN8I_SID_LOCK_SHIFT;
+	reg_val |= SUN8I_SID_READ;
+	writel(reg_val, sid->base + SUN8I_SID_PRCTL);
+
+	do {
+		reg_val = readl(sid->base + SUN8I_SID_PRCTL);
+	} while (time_before(jiffies, expire) && (reg_val & SUN8I_SID_READ));
+
+	if (reg_val & SUN8I_SID_READ)
+		return -EIO;
+
+	if (out)
+		*out = readl(sid->base + SUN8I_SID_RDKEY);
+	writel(0, sid->base + SUN8I_SID_PRCTL);
+	return 0;
+}
+
 static int sunxi_sid_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	struct nvmem_device *nvmem;
 	struct sunxi_sid *sid;
-	int ret, i, size;
+	int ret, i, size, randomness_size;
 	char *randomness;
+	const struct sunxi_sid_cfg *cfg;
 
 	sid = devm_kzalloc(dev, sizeof(*sid), GFP_KERNEL);
 	if (!sid)
 		return -ENOMEM;
 
+	cfg = of_device_get_match_data(dev);
+	if (!cfg)
+		return -EINVAL;
+	sid->value_offset = cfg->value_offset;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	sid->base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(sid->base))
 		return PTR_ERR(sid->base);
 
-	size = resource_size(res) - 1;
-	econfig.size = resource_size(res);
+	size = resource_size(res) - cfg->value_offset;
+
+	if (cfg->need_register_readout) {
+		/*
+		 * H3's SID controller have a bug that the value at 0x200
+		 * offset is not the correct value when the hardware is reset.
+		 * However, after doing a register-based read operation, the
+		 * value become right.
+		 * Do a full read operation here, but ignore its value
+		 * (as it's more fast to read by direct MMIO value than
+		 * with registers)
+		 */
+		for (i = 0; i < (size >> 2); i++) {
+			ret = sun8i_sid_register_readout(sid, i, NULL);
+			if (ret)
+				return ret;
+		}
+	}
+
+	econfig.size = size;
 	econfig.dev = dev;
 	econfig.reg_read = sunxi_sid_read;
 	econfig.priv = sid;
@@ -91,16 +167,17 @@ static int sunxi_sid_probe(struct platform_device *pdev)
 	if (IS_ERR(nvmem))
 		return PTR_ERR(nvmem);
 
-	randomness = kzalloc(sizeof(u8) * (size), GFP_KERNEL);
+	randomness_size = max(size, SUNXI_SID_MAX_RANDOMNESS_SIZE);
+	randomness = kzalloc(sizeof(u8) * (randomness_size), GFP_KERNEL);
 	if (!randomness) {
 		ret = -EINVAL;
 		goto err_unreg_nvmem;
 	}
 
-	for (i = 0; i < size; i++)
+	for (i = 0; i < randomness_size; i++)
 		randomness[i] = sunxi_sid_read_byte(sid, i);
 
-	add_device_randomness(randomness, size);
+	add_device_randomness(randomness, randomness_size);
 	kfree(randomness);
 
 	platform_set_drvdata(pdev, nvmem);
@@ -119,9 +196,26 @@ static int sunxi_sid_remove(struct platform_device *pdev)
 	return nvmem_unregister(nvmem);
 }
 
+static const struct sunxi_sid_cfg sun4i_a10_cfg = {
+	.value_offset = 0,
+	.need_register_readout = false,
+};
+
+static const struct sunxi_sid_cfg sun8i_h3_cfg = {
+	.value_offset = 0x200,
+	.need_register_readout = true,
+};
+
+static const struct sunxi_sid_cfg sun50i_a64_cfg = {
+	.value_offset = 0x200,
+	.need_register_readout = false,
+};
+
 static const struct of_device_id sunxi_sid_of_match[] = {
-	{ .compatible = "allwinner,sun4i-a10-sid" },
-	{ .compatible = "allwinner,sun7i-a20-sid" },
+	{ .compatible = "allwinner,sun4i-a10-sid", .data = &sun4i_a10_cfg },
+	{ .compatible = "allwinner,sun7i-a20-sid", .data = &sun4i_a10_cfg },
+	{ .compatible = "allwinner,sun8i-h3-sid", .data = &sun8i_h3_cfg },
+	{ .compatible = "allwinner,sun50i-a64-sid", .data = &sun50i_a64_cfg },
 	{/* sentinel */},
 };
 MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 1/2] nvmem: sunxi-sid: add support for H3 and A64's SID controller
@ 2017-01-26 11:33 ` Icenowy Zheng
  0 siblings, 0 replies; 14+ messages in thread
From: Icenowy Zheng @ 2017-01-26 11:33 UTC (permalink / raw)
  To: linux-arm-kernel

H3 and A64 SoCs have a bigger SID controller, which has its direct read
address at 0x200 position in the SID block, not 0x0.

Also, H3 SID controller has some silicon bug that makes the direct read
value wrong at first, add code to workaround the bug. (This bug has
already been fixed on A64 and later SoCs)

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
 .../bindings/nvmem/allwinner,sunxi-sid.txt         |  22 +++-
 drivers/nvmem/sunxi_sid.c                          | 112 +++++++++++++++++++--
 2 files changed, 123 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
index d543ed3f5363..d7400c415338 100644
--- a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
+++ b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
@@ -1,8 +1,16 @@
 Allwinner sunxi-sid
 
 Required properties:
-- compatible: "allwinner,sun4i-a10-sid" or "allwinner,sun7i-a20-sid"
-- reg: Should contain registers location and length
+- compatible: Should be one of the following (depending on your SoC):
+  "allwinner,sun4i-a10-sid"
+  "allwinner,sun7i-a20-sid"
+  "allwinner,sun8i-h3-sid"
+  "allwinner,sun50i-a64-sid"
+
+- reg: Should contain registers location and registers length, for A10 and A20
+  SoCs the length is directly the length of SID; for H3, A64 and newer SoCs
+  the length should be the sum of the length of SID and the offset of the value
+  (0x200).
 
 = Data cells =
 Are child nodes of qfprom, bindings of which as described in
@@ -19,3 +27,13 @@ Example for sun7i:
 		compatible = "allwinner,sun7i-a20-sid";
 		reg = <0x01c23800 0x200>
 	};
+
+Example for sun8i-h3:
+	sid at 01c14000 {
+		compatible = "allwinner,sun8i-h3-sid";
+		/*
+		 * The length of SID on H3 is 0x100 bytes, add the value offset
+		 * 0x200, so the total length should be 0x300.
+		 */
+		reg = <0x01c14000 0x300>;
+	};
diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
index 1567ccca8de3..2e327a66a938 100644
--- a/drivers/nvmem/sunxi_sid.c
+++ b/drivers/nvmem/sunxi_sid.c
@@ -20,10 +20,28 @@
 #include <linux/module.h>
 #include <linux/nvmem-provider.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/random.h>
 
+/* Registers and special values for doing register-based SID readout on H3 */
+#define SUN8I_SID_PRCTL		0x40
+#define SUN8I_SID_RDKEY		0x60
+
+#define SUN8I_SID_OP_LOCK	0xAC
+#define SUN8I_SID_OFFSET_MASK	0x1FF
+#define SUN8I_SID_OFFSET_SHIFT	16
+#define SUN8I_SID_LOCK_SHIFT	8
+#define SUN8I_SID_READ		BIT(1)
+
+/*
+ * For newer SoCs with a larger eFUSE, the bytes beyond the first 16 bytes are
+ * sparse, which makes it not suitable for adding randomness; legacy SoCs' SID
+ * have only 16 bytes, so we choose to use at most 16 bytes to add randomness.
+ */
+#define SUNXI_SID_MAX_RANDOMNESS_SIZE	16
+
 static struct nvmem_config econfig = {
 	.name = "sunxi-sid",
 	.read_only = true,
@@ -32,8 +50,14 @@ static struct nvmem_config econfig = {
 	.owner = THIS_MODULE,
 };
 
+struct sunxi_sid_cfg {
+	u32	value_offset;
+	bool	need_register_readout;
+};
+
 struct sunxi_sid {
 	void __iomem		*base;
+	u32			value_offset;
 };
 
 /* We read the entire key, due to a 32 bit read alignment requirement. Since we
@@ -46,7 +70,8 @@ static u8 sunxi_sid_read_byte(const struct sunxi_sid *sid,
 {
 	u32 sid_key;
 
-	sid_key = ioread32be(sid->base + round_down(offset, 4));
+	sid_key = ioread32be(sid->base + sid->value_offset +
+			     round_down(offset, 4));
 	sid_key >>= (offset % 4) * 8;
 
 	return sid_key; /* Only return the last byte */
@@ -64,26 +89,77 @@ static int sunxi_sid_read(void *context, unsigned int offset,
 	return 0;
 }
 
+static int sun8i_sid_register_readout(const struct sunxi_sid *sid,
+				      const unsigned int word,
+				      u32 *out)
+{
+	u32 reg_val;
+	unsigned long expire = jiffies + msecs_to_jiffies(250);
+
+	/* Set word, lock access, and set read command */
+	reg_val = (word & SUN8I_SID_OFFSET_MASK)
+		  << SUN8I_SID_OFFSET_SHIFT;
+	reg_val |= SUN8I_SID_OP_LOCK << SUN8I_SID_LOCK_SHIFT;
+	reg_val |= SUN8I_SID_READ;
+	writel(reg_val, sid->base + SUN8I_SID_PRCTL);
+
+	do {
+		reg_val = readl(sid->base + SUN8I_SID_PRCTL);
+	} while (time_before(jiffies, expire) && (reg_val & SUN8I_SID_READ));
+
+	if (reg_val & SUN8I_SID_READ)
+		return -EIO;
+
+	if (out)
+		*out = readl(sid->base + SUN8I_SID_RDKEY);
+	writel(0, sid->base + SUN8I_SID_PRCTL);
+	return 0;
+}
+
 static int sunxi_sid_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	struct nvmem_device *nvmem;
 	struct sunxi_sid *sid;
-	int ret, i, size;
+	int ret, i, size, randomness_size;
 	char *randomness;
+	const struct sunxi_sid_cfg *cfg;
 
 	sid = devm_kzalloc(dev, sizeof(*sid), GFP_KERNEL);
 	if (!sid)
 		return -ENOMEM;
 
+	cfg = of_device_get_match_data(dev);
+	if (!cfg)
+		return -EINVAL;
+	sid->value_offset = cfg->value_offset;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	sid->base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(sid->base))
 		return PTR_ERR(sid->base);
 
-	size = resource_size(res) - 1;
-	econfig.size = resource_size(res);
+	size = resource_size(res) - cfg->value_offset;
+
+	if (cfg->need_register_readout) {
+		/*
+		 * H3's SID controller have a bug that the value at 0x200
+		 * offset is not the correct value when the hardware is reset.
+		 * However, after doing a register-based read operation, the
+		 * value become right.
+		 * Do a full read operation here, but ignore its value
+		 * (as it's more fast to read by direct MMIO value than
+		 * with registers)
+		 */
+		for (i = 0; i < (size >> 2); i++) {
+			ret = sun8i_sid_register_readout(sid, i, NULL);
+			if (ret)
+				return ret;
+		}
+	}
+
+	econfig.size = size;
 	econfig.dev = dev;
 	econfig.reg_read = sunxi_sid_read;
 	econfig.priv = sid;
@@ -91,16 +167,17 @@ static int sunxi_sid_probe(struct platform_device *pdev)
 	if (IS_ERR(nvmem))
 		return PTR_ERR(nvmem);
 
-	randomness = kzalloc(sizeof(u8) * (size), GFP_KERNEL);
+	randomness_size = max(size, SUNXI_SID_MAX_RANDOMNESS_SIZE);
+	randomness = kzalloc(sizeof(u8) * (randomness_size), GFP_KERNEL);
 	if (!randomness) {
 		ret = -EINVAL;
 		goto err_unreg_nvmem;
 	}
 
-	for (i = 0; i < size; i++)
+	for (i = 0; i < randomness_size; i++)
 		randomness[i] = sunxi_sid_read_byte(sid, i);
 
-	add_device_randomness(randomness, size);
+	add_device_randomness(randomness, randomness_size);
 	kfree(randomness);
 
 	platform_set_drvdata(pdev, nvmem);
@@ -119,9 +196,26 @@ static int sunxi_sid_remove(struct platform_device *pdev)
 	return nvmem_unregister(nvmem);
 }
 
+static const struct sunxi_sid_cfg sun4i_a10_cfg = {
+	.value_offset = 0,
+	.need_register_readout = false,
+};
+
+static const struct sunxi_sid_cfg sun8i_h3_cfg = {
+	.value_offset = 0x200,
+	.need_register_readout = true,
+};
+
+static const struct sunxi_sid_cfg sun50i_a64_cfg = {
+	.value_offset = 0x200,
+	.need_register_readout = false,
+};
+
 static const struct of_device_id sunxi_sid_of_match[] = {
-	{ .compatible = "allwinner,sun4i-a10-sid" },
-	{ .compatible = "allwinner,sun7i-a20-sid" },
+	{ .compatible = "allwinner,sun4i-a10-sid", .data = &sun4i_a10_cfg },
+	{ .compatible = "allwinner,sun7i-a20-sid", .data = &sun4i_a10_cfg },
+	{ .compatible = "allwinner,sun8i-h3-sid", .data = &sun8i_h3_cfg },
+	{ .compatible = "allwinner,sun50i-a64-sid", .data = &sun50i_a64_cfg },
 	{/* sentinel */},
 };
 MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] ARM: dts: sun8i: enable SID on Allwinner H3 SoC
  2017-01-26 11:33 ` Icenowy Zheng
@ 2017-01-26 11:33     ` Icenowy Zheng
  -1 siblings, 0 replies; 14+ messages in thread
From: Icenowy Zheng @ 2017-01-26 11:33 UTC (permalink / raw)
  To: Srinivas Kandagatla, Maxime Ripard, Rob Herring, Chen-Yu Tsai
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng

As we have already made sunxi_sid driver support the SID controller on
H3, enable it.

Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 08fd0860bb6b..cefebbe8ff19 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -206,6 +206,11 @@
 			#size-cells = <0>;
 		};
 
+		sid: eeprom@01c14000 {
+			compatible = "allwinner,sun8i-h3-sid";
+			reg = <0x01c14000 0x300>;
+		};
+
 		usbphy: phy@01c19400 {
 			compatible = "allwinner,sun8i-h3-usb-phy";
 			reg = <0x01c19400 0x2c>,
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] ARM: dts: sun8i: enable SID on Allwinner H3 SoC
@ 2017-01-26 11:33     ` Icenowy Zheng
  0 siblings, 0 replies; 14+ messages in thread
From: Icenowy Zheng @ 2017-01-26 11:33 UTC (permalink / raw)
  To: linux-arm-kernel

As we have already made sunxi_sid driver support the SID controller on
H3, enable it.

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 08fd0860bb6b..cefebbe8ff19 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -206,6 +206,11 @@
 			#size-cells = <0>;
 		};
 
+		sid: eeprom at 01c14000 {
+			compatible = "allwinner,sun8i-h3-sid";
+			reg = <0x01c14000 0x300>;
+		};
+
 		usbphy: phy at 01c19400 {
 			compatible = "allwinner,sun8i-h3-usb-phy";
 			reg = <0x01c19400 0x2c>,
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] nvmem: sunxi-sid: add support for H3 and A64's SID controller
       [not found] ` <20170126113345.33686-1-icenowy-ymACFijhrKM@public.gmane.org>
  2017-01-26 11:33     ` Icenowy Zheng
@ 2017-01-27  8:17   ` Maxime Ripard
  0 siblings, 0 replies; 14+ messages in thread
From: Maxime Ripard @ 2017-01-27  8:17 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Srinivas Kandagatla, Rob Herring, Chen-Yu Tsai, devicetree,
	linux-arm-kernel, linux-kernel, linux-sunxi

[-- Attachment #1: Type: text/plain, Size: 7234 bytes --]

Hi,

On Thu, Jan 26, 2017 at 07:33:44PM +0800, Icenowy Zheng wrote:
> H3 and A64 SoCs have a bigger SID controller, which has its direct read
> address at 0x200 position in the SID block, not 0x0.
> 
> Also, H3 SID controller has some silicon bug that makes the direct read
> value wrong at first, add code to workaround the bug. (This bug has
> already been fixed on A64 and later SoCs)
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> ---
>  .../bindings/nvmem/allwinner,sunxi-sid.txt         |  22 +++-
>  drivers/nvmem/sunxi_sid.c                          | 112 +++++++++++++++++++--
>  2 files changed, 123 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
> index d543ed3f5363..d7400c415338 100644
> --- a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
> +++ b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
> @@ -1,8 +1,16 @@
>  Allwinner sunxi-sid
>  
>  Required properties:
> -- compatible: "allwinner,sun4i-a10-sid" or "allwinner,sun7i-a20-sid"
> -- reg: Should contain registers location and length
> +- compatible: Should be one of the following (depending on your SoC):
> +  "allwinner,sun4i-a10-sid"
> +  "allwinner,sun7i-a20-sid"
> +  "allwinner,sun8i-h3-sid"
> +  "allwinner,sun50i-a64-sid"
> +
> +- reg: Should contain registers location and registers length, for A10 and A20
> +  SoCs the length is directly the length of SID; for H3, A64 and newer SoCs
> +  the length should be the sum of the length of SID and the offset of the value
> +  (0x200).
>  
>  = Data cells =
>  Are child nodes of qfprom, bindings of which as described in
> @@ -19,3 +27,13 @@ Example for sun7i:
>  		compatible = "allwinner,sun7i-a20-sid";
>  		reg = <0x01c23800 0x200>
>  	};
> +
> +Example for sun8i-h3:
> +	sid@01c14000 {
> +		compatible = "allwinner,sun8i-h3-sid";
> +		/*
> +		 * The length of SID on H3 is 0x100 bytes, add the value offset
> +		 * 0x200, so the total length should be 0x300.
> +		 */
> +		reg = <0x01c14000 0x300>;

No, it should be the size of the memory region used and documented for
that device, ie 1kB (0x400) in the H3 case, just like any other reg
property.

> +	};
> diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
> index 1567ccca8de3..2e327a66a938 100644
> --- a/drivers/nvmem/sunxi_sid.c
> +++ b/drivers/nvmem/sunxi_sid.c
> @@ -20,10 +20,28 @@
>  #include <linux/module.h>
>  #include <linux/nvmem-provider.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/random.h>
>  
> +/* Registers and special values for doing register-based SID readout on H3 */
> +#define SUN8I_SID_PRCTL		0x40
> +#define SUN8I_SID_RDKEY		0x60
> +
> +#define SUN8I_SID_OP_LOCK	0xAC
> +#define SUN8I_SID_OFFSET_MASK	0x1FF
> +#define SUN8I_SID_OFFSET_SHIFT	16
> +#define SUN8I_SID_LOCK_SHIFT	8
> +#define SUN8I_SID_READ		BIT(1)
> +
> +/*
> + * For newer SoCs with a larger eFUSE, the bytes beyond the first 16 bytes are
> + * sparse, which makes it not suitable for adding randomness; legacy SoCs' SID
> + * have only 16 bytes, so we choose to use at most 16 bytes to add randomness.
> + */
> +#define SUNXI_SID_MAX_RANDOMNESS_SIZE	16
> +
>  static struct nvmem_config econfig = {
>  	.name = "sunxi-sid",
>  	.read_only = true,
> @@ -32,8 +50,14 @@ static struct nvmem_config econfig = {
>  	.owner = THIS_MODULE,
>  };
>  
> +struct sunxi_sid_cfg {
> +	u32	value_offset;
> +	bool	need_register_readout;
> +};
> +
>  struct sunxi_sid {
>  	void __iomem		*base;
> +	u32			value_offset;
>  };
>  
>  /* We read the entire key, due to a 32 bit read alignment requirement. Since we
> @@ -46,7 +70,8 @@ static u8 sunxi_sid_read_byte(const struct sunxi_sid *sid,
>  {
>  	u32 sid_key;
>  
> -	sid_key = ioread32be(sid->base + round_down(offset, 4));
> +	sid_key = ioread32be(sid->base + sid->value_offset +
> +			     round_down(offset, 4));
>  	sid_key >>= (offset % 4) * 8;
>  
>  	return sid_key; /* Only return the last byte */
> @@ -64,26 +89,77 @@ static int sunxi_sid_read(void *context, unsigned int offset,
>  	return 0;
>  }
>  
> +static int sun8i_sid_register_readout(const struct sunxi_sid *sid,
> +				      const unsigned int word,
> +				      u32 *out)
> +{
> +	u32 reg_val;
> +	unsigned long expire = jiffies + msecs_to_jiffies(250);
> +
> +	/* Set word, lock access, and set read command */
> +	reg_val = (word & SUN8I_SID_OFFSET_MASK)
> +		  << SUN8I_SID_OFFSET_SHIFT;
> +	reg_val |= SUN8I_SID_OP_LOCK << SUN8I_SID_LOCK_SHIFT;
> +	reg_val |= SUN8I_SID_READ;
> +	writel(reg_val, sid->base + SUN8I_SID_PRCTL);
> +
> +	do {
> +		reg_val = readl(sid->base + SUN8I_SID_PRCTL);
> +	} while (time_before(jiffies, expire) && (reg_val & SUN8I_SID_READ));
> +
> +	if (reg_val & SUN8I_SID_READ)
> +		return -EIO;
> +
> +	if (out)
> +		*out = readl(sid->base + SUN8I_SID_RDKEY);
> +	writel(0, sid->base + SUN8I_SID_PRCTL);
> +	return 0;
> +}
> +
>  static int sunxi_sid_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct resource *res;
>  	struct nvmem_device *nvmem;
>  	struct sunxi_sid *sid;
> -	int ret, i, size;
> +	int ret, i, size, randomness_size;
>  	char *randomness;
> +	const struct sunxi_sid_cfg *cfg;
>  
>  	sid = devm_kzalloc(dev, sizeof(*sid), GFP_KERNEL);
>  	if (!sid)
>  		return -ENOMEM;
>  
> +	cfg = of_device_get_match_data(dev);
> +	if (!cfg)
> +		return -EINVAL;
> +	sid->value_offset = cfg->value_offset;
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	sid->base = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(sid->base))
>  		return PTR_ERR(sid->base);
>  
> -	size = resource_size(res) - 1;
> -	econfig.size = resource_size(res);
> +	size = resource_size(res) - cfg->value_offset;
> +
> +	if (cfg->need_register_readout) {
> +		/*
> +		 * H3's SID controller have a bug that the value at 0x200
> +		 * offset is not the correct value when the hardware is reset.
> +		 * However, after doing a register-based read operation, the
> +		 * value become right.
> +		 * Do a full read operation here, but ignore its value
> +		 * (as it's more fast to read by direct MMIO value than
> +		 * with registers)
> +		 */
> +		for (i = 0; i < (size >> 2); i++) {
> +			ret = sun8i_sid_register_readout(sid, i, NULL);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	econfig.size = size;
>  	econfig.dev = dev;
>  	econfig.reg_read = sunxi_sid_read;
>  	econfig.priv = sid;
> @@ -91,16 +167,17 @@ static int sunxi_sid_probe(struct platform_device *pdev)
>  	if (IS_ERR(nvmem))
>  		return PTR_ERR(nvmem);
>  
> -	randomness = kzalloc(sizeof(u8) * (size), GFP_KERNEL);
> +	randomness_size = max(size, SUNXI_SID_MAX_RANDOMNESS_SIZE);
> +	randomness = kzalloc(sizeof(u8) * (randomness_size), GFP_KERNEL);

Why is that change needed?

Thanks,
Maxime


-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] nvmem: sunxi-sid: add support for H3 and A64's SID controller
@ 2017-01-27  8:17   ` Maxime Ripard
  0 siblings, 0 replies; 14+ messages in thread
From: Maxime Ripard @ 2017-01-27  8:17 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Srinivas Kandagatla, Rob Herring, Chen-Yu Tsai,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

[-- Attachment #1: Type: text/plain, Size: 7038 bytes --]

Hi,

On Thu, Jan 26, 2017 at 07:33:44PM +0800, Icenowy Zheng wrote:
> H3 and A64 SoCs have a bigger SID controller, which has its direct read
> address at 0x200 position in the SID block, not 0x0.
> 
> Also, H3 SID controller has some silicon bug that makes the direct read
> value wrong at first, add code to workaround the bug. (This bug has
> already been fixed on A64 and later SoCs)
> 
> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
> ---
>  .../bindings/nvmem/allwinner,sunxi-sid.txt         |  22 +++-
>  drivers/nvmem/sunxi_sid.c                          | 112 +++++++++++++++++++--
>  2 files changed, 123 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
> index d543ed3f5363..d7400c415338 100644
> --- a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
> +++ b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
> @@ -1,8 +1,16 @@
>  Allwinner sunxi-sid
>  
>  Required properties:
> -- compatible: "allwinner,sun4i-a10-sid" or "allwinner,sun7i-a20-sid"
> -- reg: Should contain registers location and length
> +- compatible: Should be one of the following (depending on your SoC):
> +  "allwinner,sun4i-a10-sid"
> +  "allwinner,sun7i-a20-sid"
> +  "allwinner,sun8i-h3-sid"
> +  "allwinner,sun50i-a64-sid"
> +
> +- reg: Should contain registers location and registers length, for A10 and A20
> +  SoCs the length is directly the length of SID; for H3, A64 and newer SoCs
> +  the length should be the sum of the length of SID and the offset of the value
> +  (0x200).
>  
>  = Data cells =
>  Are child nodes of qfprom, bindings of which as described in
> @@ -19,3 +27,13 @@ Example for sun7i:
>  		compatible = "allwinner,sun7i-a20-sid";
>  		reg = <0x01c23800 0x200>
>  	};
> +
> +Example for sun8i-h3:
> +	sid@01c14000 {
> +		compatible = "allwinner,sun8i-h3-sid";
> +		/*
> +		 * The length of SID on H3 is 0x100 bytes, add the value offset
> +		 * 0x200, so the total length should be 0x300.
> +		 */
> +		reg = <0x01c14000 0x300>;

No, it should be the size of the memory region used and documented for
that device, ie 1kB (0x400) in the H3 case, just like any other reg
property.

> +	};
> diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
> index 1567ccca8de3..2e327a66a938 100644
> --- a/drivers/nvmem/sunxi_sid.c
> +++ b/drivers/nvmem/sunxi_sid.c
> @@ -20,10 +20,28 @@
>  #include <linux/module.h>
>  #include <linux/nvmem-provider.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/random.h>
>  
> +/* Registers and special values for doing register-based SID readout on H3 */
> +#define SUN8I_SID_PRCTL		0x40
> +#define SUN8I_SID_RDKEY		0x60
> +
> +#define SUN8I_SID_OP_LOCK	0xAC
> +#define SUN8I_SID_OFFSET_MASK	0x1FF
> +#define SUN8I_SID_OFFSET_SHIFT	16
> +#define SUN8I_SID_LOCK_SHIFT	8
> +#define SUN8I_SID_READ		BIT(1)
> +
> +/*
> + * For newer SoCs with a larger eFUSE, the bytes beyond the first 16 bytes are
> + * sparse, which makes it not suitable for adding randomness; legacy SoCs' SID
> + * have only 16 bytes, so we choose to use at most 16 bytes to add randomness.
> + */
> +#define SUNXI_SID_MAX_RANDOMNESS_SIZE	16
> +
>  static struct nvmem_config econfig = {
>  	.name = "sunxi-sid",
>  	.read_only = true,
> @@ -32,8 +50,14 @@ static struct nvmem_config econfig = {
>  	.owner = THIS_MODULE,
>  };
>  
> +struct sunxi_sid_cfg {
> +	u32	value_offset;
> +	bool	need_register_readout;
> +};
> +
>  struct sunxi_sid {
>  	void __iomem		*base;
> +	u32			value_offset;
>  };
>  
>  /* We read the entire key, due to a 32 bit read alignment requirement. Since we
> @@ -46,7 +70,8 @@ static u8 sunxi_sid_read_byte(const struct sunxi_sid *sid,
>  {
>  	u32 sid_key;
>  
> -	sid_key = ioread32be(sid->base + round_down(offset, 4));
> +	sid_key = ioread32be(sid->base + sid->value_offset +
> +			     round_down(offset, 4));
>  	sid_key >>= (offset % 4) * 8;
>  
>  	return sid_key; /* Only return the last byte */
> @@ -64,26 +89,77 @@ static int sunxi_sid_read(void *context, unsigned int offset,
>  	return 0;
>  }
>  
> +static int sun8i_sid_register_readout(const struct sunxi_sid *sid,
> +				      const unsigned int word,
> +				      u32 *out)
> +{
> +	u32 reg_val;
> +	unsigned long expire = jiffies + msecs_to_jiffies(250);
> +
> +	/* Set word, lock access, and set read command */
> +	reg_val = (word & SUN8I_SID_OFFSET_MASK)
> +		  << SUN8I_SID_OFFSET_SHIFT;
> +	reg_val |= SUN8I_SID_OP_LOCK << SUN8I_SID_LOCK_SHIFT;
> +	reg_val |= SUN8I_SID_READ;
> +	writel(reg_val, sid->base + SUN8I_SID_PRCTL);
> +
> +	do {
> +		reg_val = readl(sid->base + SUN8I_SID_PRCTL);
> +	} while (time_before(jiffies, expire) && (reg_val & SUN8I_SID_READ));
> +
> +	if (reg_val & SUN8I_SID_READ)
> +		return -EIO;
> +
> +	if (out)
> +		*out = readl(sid->base + SUN8I_SID_RDKEY);
> +	writel(0, sid->base + SUN8I_SID_PRCTL);
> +	return 0;
> +}
> +
>  static int sunxi_sid_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct resource *res;
>  	struct nvmem_device *nvmem;
>  	struct sunxi_sid *sid;
> -	int ret, i, size;
> +	int ret, i, size, randomness_size;
>  	char *randomness;
> +	const struct sunxi_sid_cfg *cfg;
>  
>  	sid = devm_kzalloc(dev, sizeof(*sid), GFP_KERNEL);
>  	if (!sid)
>  		return -ENOMEM;
>  
> +	cfg = of_device_get_match_data(dev);
> +	if (!cfg)
> +		return -EINVAL;
> +	sid->value_offset = cfg->value_offset;
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	sid->base = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(sid->base))
>  		return PTR_ERR(sid->base);
>  
> -	size = resource_size(res) - 1;
> -	econfig.size = resource_size(res);
> +	size = resource_size(res) - cfg->value_offset;
> +
> +	if (cfg->need_register_readout) {
> +		/*
> +		 * H3's SID controller have a bug that the value at 0x200
> +		 * offset is not the correct value when the hardware is reset.
> +		 * However, after doing a register-based read operation, the
> +		 * value become right.
> +		 * Do a full read operation here, but ignore its value
> +		 * (as it's more fast to read by direct MMIO value than
> +		 * with registers)
> +		 */
> +		for (i = 0; i < (size >> 2); i++) {
> +			ret = sun8i_sid_register_readout(sid, i, NULL);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	econfig.size = size;
>  	econfig.dev = dev;
>  	econfig.reg_read = sunxi_sid_read;
>  	econfig.priv = sid;
> @@ -91,16 +167,17 @@ static int sunxi_sid_probe(struct platform_device *pdev)
>  	if (IS_ERR(nvmem))
>  		return PTR_ERR(nvmem);
>  
> -	randomness = kzalloc(sizeof(u8) * (size), GFP_KERNEL);
> +	randomness_size = max(size, SUNXI_SID_MAX_RANDOMNESS_SIZE);
> +	randomness = kzalloc(sizeof(u8) * (randomness_size), GFP_KERNEL);

Why is that change needed?

Thanks,
Maxime


-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/2] nvmem: sunxi-sid: add support for H3 and A64's SID controller
@ 2017-01-27  8:17   ` Maxime Ripard
  0 siblings, 0 replies; 14+ messages in thread
From: Maxime Ripard @ 2017-01-27  8:17 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Thu, Jan 26, 2017 at 07:33:44PM +0800, Icenowy Zheng wrote:
> H3 and A64 SoCs have a bigger SID controller, which has its direct read
> address at 0x200 position in the SID block, not 0x0.
> 
> Also, H3 SID controller has some silicon bug that makes the direct read
> value wrong at first, add code to workaround the bug. (This bug has
> already been fixed on A64 and later SoCs)
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> ---
>  .../bindings/nvmem/allwinner,sunxi-sid.txt         |  22 +++-
>  drivers/nvmem/sunxi_sid.c                          | 112 +++++++++++++++++++--
>  2 files changed, 123 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
> index d543ed3f5363..d7400c415338 100644
> --- a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
> +++ b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
> @@ -1,8 +1,16 @@
>  Allwinner sunxi-sid
>  
>  Required properties:
> -- compatible: "allwinner,sun4i-a10-sid" or "allwinner,sun7i-a20-sid"
> -- reg: Should contain registers location and length
> +- compatible: Should be one of the following (depending on your SoC):
> +  "allwinner,sun4i-a10-sid"
> +  "allwinner,sun7i-a20-sid"
> +  "allwinner,sun8i-h3-sid"
> +  "allwinner,sun50i-a64-sid"
> +
> +- reg: Should contain registers location and registers length, for A10 and A20
> +  SoCs the length is directly the length of SID; for H3, A64 and newer SoCs
> +  the length should be the sum of the length of SID and the offset of the value
> +  (0x200).
>  
>  = Data cells =
>  Are child nodes of qfprom, bindings of which as described in
> @@ -19,3 +27,13 @@ Example for sun7i:
>  		compatible = "allwinner,sun7i-a20-sid";
>  		reg = <0x01c23800 0x200>
>  	};
> +
> +Example for sun8i-h3:
> +	sid at 01c14000 {
> +		compatible = "allwinner,sun8i-h3-sid";
> +		/*
> +		 * The length of SID on H3 is 0x100 bytes, add the value offset
> +		 * 0x200, so the total length should be 0x300.
> +		 */
> +		reg = <0x01c14000 0x300>;

No, it should be the size of the memory region used and documented for
that device, ie 1kB (0x400) in the H3 case, just like any other reg
property.

> +	};
> diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
> index 1567ccca8de3..2e327a66a938 100644
> --- a/drivers/nvmem/sunxi_sid.c
> +++ b/drivers/nvmem/sunxi_sid.c
> @@ -20,10 +20,28 @@
>  #include <linux/module.h>
>  #include <linux/nvmem-provider.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/random.h>
>  
> +/* Registers and special values for doing register-based SID readout on H3 */
> +#define SUN8I_SID_PRCTL		0x40
> +#define SUN8I_SID_RDKEY		0x60
> +
> +#define SUN8I_SID_OP_LOCK	0xAC
> +#define SUN8I_SID_OFFSET_MASK	0x1FF
> +#define SUN8I_SID_OFFSET_SHIFT	16
> +#define SUN8I_SID_LOCK_SHIFT	8
> +#define SUN8I_SID_READ		BIT(1)
> +
> +/*
> + * For newer SoCs with a larger eFUSE, the bytes beyond the first 16 bytes are
> + * sparse, which makes it not suitable for adding randomness; legacy SoCs' SID
> + * have only 16 bytes, so we choose to use at most 16 bytes to add randomness.
> + */
> +#define SUNXI_SID_MAX_RANDOMNESS_SIZE	16
> +
>  static struct nvmem_config econfig = {
>  	.name = "sunxi-sid",
>  	.read_only = true,
> @@ -32,8 +50,14 @@ static struct nvmem_config econfig = {
>  	.owner = THIS_MODULE,
>  };
>  
> +struct sunxi_sid_cfg {
> +	u32	value_offset;
> +	bool	need_register_readout;
> +};
> +
>  struct sunxi_sid {
>  	void __iomem		*base;
> +	u32			value_offset;
>  };
>  
>  /* We read the entire key, due to a 32 bit read alignment requirement. Since we
> @@ -46,7 +70,8 @@ static u8 sunxi_sid_read_byte(const struct sunxi_sid *sid,
>  {
>  	u32 sid_key;
>  
> -	sid_key = ioread32be(sid->base + round_down(offset, 4));
> +	sid_key = ioread32be(sid->base + sid->value_offset +
> +			     round_down(offset, 4));
>  	sid_key >>= (offset % 4) * 8;
>  
>  	return sid_key; /* Only return the last byte */
> @@ -64,26 +89,77 @@ static int sunxi_sid_read(void *context, unsigned int offset,
>  	return 0;
>  }
>  
> +static int sun8i_sid_register_readout(const struct sunxi_sid *sid,
> +				      const unsigned int word,
> +				      u32 *out)
> +{
> +	u32 reg_val;
> +	unsigned long expire = jiffies + msecs_to_jiffies(250);
> +
> +	/* Set word, lock access, and set read command */
> +	reg_val = (word & SUN8I_SID_OFFSET_MASK)
> +		  << SUN8I_SID_OFFSET_SHIFT;
> +	reg_val |= SUN8I_SID_OP_LOCK << SUN8I_SID_LOCK_SHIFT;
> +	reg_val |= SUN8I_SID_READ;
> +	writel(reg_val, sid->base + SUN8I_SID_PRCTL);
> +
> +	do {
> +		reg_val = readl(sid->base + SUN8I_SID_PRCTL);
> +	} while (time_before(jiffies, expire) && (reg_val & SUN8I_SID_READ));
> +
> +	if (reg_val & SUN8I_SID_READ)
> +		return -EIO;
> +
> +	if (out)
> +		*out = readl(sid->base + SUN8I_SID_RDKEY);
> +	writel(0, sid->base + SUN8I_SID_PRCTL);
> +	return 0;
> +}
> +
>  static int sunxi_sid_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct resource *res;
>  	struct nvmem_device *nvmem;
>  	struct sunxi_sid *sid;
> -	int ret, i, size;
> +	int ret, i, size, randomness_size;
>  	char *randomness;
> +	const struct sunxi_sid_cfg *cfg;
>  
>  	sid = devm_kzalloc(dev, sizeof(*sid), GFP_KERNEL);
>  	if (!sid)
>  		return -ENOMEM;
>  
> +	cfg = of_device_get_match_data(dev);
> +	if (!cfg)
> +		return -EINVAL;
> +	sid->value_offset = cfg->value_offset;
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	sid->base = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(sid->base))
>  		return PTR_ERR(sid->base);
>  
> -	size = resource_size(res) - 1;
> -	econfig.size = resource_size(res);
> +	size = resource_size(res) - cfg->value_offset;
> +
> +	if (cfg->need_register_readout) {
> +		/*
> +		 * H3's SID controller have a bug that the value at 0x200
> +		 * offset is not the correct value when the hardware is reset.
> +		 * However, after doing a register-based read operation, the
> +		 * value become right.
> +		 * Do a full read operation here, but ignore its value
> +		 * (as it's more fast to read by direct MMIO value than
> +		 * with registers)
> +		 */
> +		for (i = 0; i < (size >> 2); i++) {
> +			ret = sun8i_sid_register_readout(sid, i, NULL);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	econfig.size = size;
>  	econfig.dev = dev;
>  	econfig.reg_read = sunxi_sid_read;
>  	econfig.priv = sid;
> @@ -91,16 +167,17 @@ static int sunxi_sid_probe(struct platform_device *pdev)
>  	if (IS_ERR(nvmem))
>  		return PTR_ERR(nvmem);
>  
> -	randomness = kzalloc(sizeof(u8) * (size), GFP_KERNEL);
> +	randomness_size = max(size, SUNXI_SID_MAX_RANDOMNESS_SIZE);
> +	randomness = kzalloc(sizeof(u8) * (randomness_size), GFP_KERNEL);

Why is that change needed?

Thanks,
Maxime


-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170127/93c2de6c/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [linux-sunxi] Re: [PATCH 1/2] nvmem: sunxi-sid: add support for H3 and A64's SID controller
  2017-01-27  8:17   ` Maxime Ripard
  (?)
@ 2017-01-27  8:46     ` Julian Calaby
  -1 siblings, 0 replies; 14+ messages in thread
From: Julian Calaby @ 2017-01-27  8:46 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Icenowy Zheng, Srinivas Kandagatla, Rob Herring, Chen-Yu Tsai,
	devicetree, Mailing List, Arm, linux-kernel, linux-sunxi

Hi Maxime,

On Fri, Jan 27, 2017 at 7:17 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Thu, Jan 26, 2017 at 07:33:44PM +0800, Icenowy Zheng wrote:
>> H3 and A64 SoCs have a bigger SID controller, which has its direct read
>> address at 0x200 position in the SID block, not 0x0.
>>
>> Also, H3 SID controller has some silicon bug that makes the direct read
>> value wrong at first, add code to workaround the bug. (This bug has
>> already been fixed on A64 and later SoCs)
>>
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>> ---
>>  .../bindings/nvmem/allwinner,sunxi-sid.txt         |  22 +++-
>>  drivers/nvmem/sunxi_sid.c                          | 112 +++++++++++++++++++--
>>  2 files changed, 123 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
>> index 1567ccca8de3..2e327a66a938 100644
>> --- a/drivers/nvmem/sunxi_sid.c
>> +++ b/drivers/nvmem/sunxi_sid.c
>> @@ -20,10 +20,28 @@
>>  #include <linux/module.h>
>>  #include <linux/nvmem-provider.h>
>>  #include <linux/of.h>
>> +#include <linux/of_device.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/slab.h>
>>  #include <linux/random.h>
>>
>> +/* Registers and special values for doing register-based SID readout on H3 */
>> +#define SUN8I_SID_PRCTL              0x40
>> +#define SUN8I_SID_RDKEY              0x60
>> +
>> +#define SUN8I_SID_OP_LOCK    0xAC
>> +#define SUN8I_SID_OFFSET_MASK        0x1FF
>> +#define SUN8I_SID_OFFSET_SHIFT       16
>> +#define SUN8I_SID_LOCK_SHIFT 8
>> +#define SUN8I_SID_READ               BIT(1)
>> +
>> +/*
>> + * For newer SoCs with a larger eFUSE, the bytes beyond the first 16 bytes are
>> + * sparse, which makes it not suitable for adding randomness; legacy SoCs' SID
>> + * have only 16 bytes, so we choose to use at most 16 bytes to add randomness.
>> + */
>> +#define SUNXI_SID_MAX_RANDOMNESS_SIZE        16
>> +
>>  static struct nvmem_config econfig = {
>>       .name = "sunxi-sid",
>>       .read_only = true,
>> @@ -91,16 +167,17 @@ static int sunxi_sid_probe(struct platform_device *pdev)
>>       if (IS_ERR(nvmem))
>>               return PTR_ERR(nvmem);
>>
>> -     randomness = kzalloc(sizeof(u8) * (size), GFP_KERNEL);
>> +     randomness_size = max(size, SUNXI_SID_MAX_RANDOMNESS_SIZE);
>> +     randomness = kzalloc(sizeof(u8) * (randomness_size), GFP_KERNEL);
>
> Why is that change needed?

According to the definition of SUNXI_SID_MAX_RANDOMNESS_SIZE, only the
first 16 bytes of the SID data region are sufficiently non-zero to be
used for randomness.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Re: [PATCH 1/2] nvmem: sunxi-sid: add support for H3 and A64's SID controller
@ 2017-01-27  8:46     ` Julian Calaby
  0 siblings, 0 replies; 14+ messages in thread
From: Julian Calaby @ 2017-01-27  8:46 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Icenowy Zheng, Srinivas Kandagatla, Rob Herring, Chen-Yu Tsai,
	devicetree, Mailing List, Arm,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi

Hi Maxime,

On Fri, Jan 27, 2017 at 7:17 PM, Maxime Ripard
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hi,
>
> On Thu, Jan 26, 2017 at 07:33:44PM +0800, Icenowy Zheng wrote:
>> H3 and A64 SoCs have a bigger SID controller, which has its direct read
>> address at 0x200 position in the SID block, not 0x0.
>>
>> Also, H3 SID controller has some silicon bug that makes the direct read
>> value wrong at first, add code to workaround the bug. (This bug has
>> already been fixed on A64 and later SoCs)
>>
>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>> ---
>>  .../bindings/nvmem/allwinner,sunxi-sid.txt         |  22 +++-
>>  drivers/nvmem/sunxi_sid.c                          | 112 +++++++++++++++++++--
>>  2 files changed, 123 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
>> index 1567ccca8de3..2e327a66a938 100644
>> --- a/drivers/nvmem/sunxi_sid.c
>> +++ b/drivers/nvmem/sunxi_sid.c
>> @@ -20,10 +20,28 @@
>>  #include <linux/module.h>
>>  #include <linux/nvmem-provider.h>
>>  #include <linux/of.h>
>> +#include <linux/of_device.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/slab.h>
>>  #include <linux/random.h>
>>
>> +/* Registers and special values for doing register-based SID readout on H3 */
>> +#define SUN8I_SID_PRCTL              0x40
>> +#define SUN8I_SID_RDKEY              0x60
>> +
>> +#define SUN8I_SID_OP_LOCK    0xAC
>> +#define SUN8I_SID_OFFSET_MASK        0x1FF
>> +#define SUN8I_SID_OFFSET_SHIFT       16
>> +#define SUN8I_SID_LOCK_SHIFT 8
>> +#define SUN8I_SID_READ               BIT(1)
>> +
>> +/*
>> + * For newer SoCs with a larger eFUSE, the bytes beyond the first 16 bytes are
>> + * sparse, which makes it not suitable for adding randomness; legacy SoCs' SID
>> + * have only 16 bytes, so we choose to use at most 16 bytes to add randomness.
>> + */
>> +#define SUNXI_SID_MAX_RANDOMNESS_SIZE        16
>> +
>>  static struct nvmem_config econfig = {
>>       .name = "sunxi-sid",
>>       .read_only = true,
>> @@ -91,16 +167,17 @@ static int sunxi_sid_probe(struct platform_device *pdev)
>>       if (IS_ERR(nvmem))
>>               return PTR_ERR(nvmem);
>>
>> -     randomness = kzalloc(sizeof(u8) * (size), GFP_KERNEL);
>> +     randomness_size = max(size, SUNXI_SID_MAX_RANDOMNESS_SIZE);
>> +     randomness = kzalloc(sizeof(u8) * (randomness_size), GFP_KERNEL);
>
> Why is that change needed?

According to the definition of SUNXI_SID_MAX_RANDOMNESS_SIZE, only the
first 16 bytes of the SID data region are sufficiently non-zero to be
used for randomness.

Thanks,

-- 
Julian Calaby

Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [linux-sunxi] Re: [PATCH 1/2] nvmem: sunxi-sid: add support for H3 and A64's SID controller
@ 2017-01-27  8:46     ` Julian Calaby
  0 siblings, 0 replies; 14+ messages in thread
From: Julian Calaby @ 2017-01-27  8:46 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Maxime,

On Fri, Jan 27, 2017 at 7:17 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Thu, Jan 26, 2017 at 07:33:44PM +0800, Icenowy Zheng wrote:
>> H3 and A64 SoCs have a bigger SID controller, which has its direct read
>> address at 0x200 position in the SID block, not 0x0.
>>
>> Also, H3 SID controller has some silicon bug that makes the direct read
>> value wrong at first, add code to workaround the bug. (This bug has
>> already been fixed on A64 and later SoCs)
>>
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>> ---
>>  .../bindings/nvmem/allwinner,sunxi-sid.txt         |  22 +++-
>>  drivers/nvmem/sunxi_sid.c                          | 112 +++++++++++++++++++--
>>  2 files changed, 123 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
>> index 1567ccca8de3..2e327a66a938 100644
>> --- a/drivers/nvmem/sunxi_sid.c
>> +++ b/drivers/nvmem/sunxi_sid.c
>> @@ -20,10 +20,28 @@
>>  #include <linux/module.h>
>>  #include <linux/nvmem-provider.h>
>>  #include <linux/of.h>
>> +#include <linux/of_device.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/slab.h>
>>  #include <linux/random.h>
>>
>> +/* Registers and special values for doing register-based SID readout on H3 */
>> +#define SUN8I_SID_PRCTL              0x40
>> +#define SUN8I_SID_RDKEY              0x60
>> +
>> +#define SUN8I_SID_OP_LOCK    0xAC
>> +#define SUN8I_SID_OFFSET_MASK        0x1FF
>> +#define SUN8I_SID_OFFSET_SHIFT       16
>> +#define SUN8I_SID_LOCK_SHIFT 8
>> +#define SUN8I_SID_READ               BIT(1)
>> +
>> +/*
>> + * For newer SoCs with a larger eFUSE, the bytes beyond the first 16 bytes are
>> + * sparse, which makes it not suitable for adding randomness; legacy SoCs' SID
>> + * have only 16 bytes, so we choose to use at most 16 bytes to add randomness.
>> + */
>> +#define SUNXI_SID_MAX_RANDOMNESS_SIZE        16
>> +
>>  static struct nvmem_config econfig = {
>>       .name = "sunxi-sid",
>>       .read_only = true,
>> @@ -91,16 +167,17 @@ static int sunxi_sid_probe(struct platform_device *pdev)
>>       if (IS_ERR(nvmem))
>>               return PTR_ERR(nvmem);
>>
>> -     randomness = kzalloc(sizeof(u8) * (size), GFP_KERNEL);
>> +     randomness_size = max(size, SUNXI_SID_MAX_RANDOMNESS_SIZE);
>> +     randomness = kzalloc(sizeof(u8) * (randomness_size), GFP_KERNEL);
>
> Why is that change needed?

According to the definition of SUNXI_SID_MAX_RANDOMNESS_SIZE, only the
first 16 bytes of the SID data region are sufficiently non-zero to be
used for randomness.

Thanks,

-- 
Julian Calaby

Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] nvmem: sunxi-sid: add support for H3 and A64's SID controller
@ 2017-01-27  9:41   ` Maxime Ripard
  0 siblings, 0 replies; 14+ messages in thread
From: Maxime Ripard @ 2017-01-27  9:41 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Srinivas Kandagatla, Rob Herring, linux-kernel, linux-arm-kernel,
	linux-sunxi, devicetree, Chen-Yu Tsai

[-- Attachment #1: Type: text/plain, Size: 1194 bytes --]

On Fri, Jan 27, 2017 at 05:26:59PM +0800, Icenowy Zheng wrote:
> > > +Example for sun8i-h3: 
> > > + sid@01c14000 { 
> > > + compatible = "allwinner,sun8i-h3-sid"; 
> > > + /* 
> > > + * The length of SID on H3 is 0x100 bytes, add the value offset 
> > > + * 0x200, so the total length should be 0x300. 
> > > + */ 
> > > + reg = <0x01c14000 0x300>; 
> >
> > No, it should be the size of the memory region used and documented for 
> > that device, ie 1kB (0x400) in the H3 case, just like any other reg 
> > property. 
> 
> So bind the SID size with compatible?

If needed, yes.

> > > - randomness = kzalloc(sizeof(u8) * (size), GFP_KERNEL); 
> > > + randomness_size = max(size, SUNXI_SID_MAX_RANDOMNESS_SIZE); 
> > > + randomness = kzalloc(sizeof(u8) * (randomness_size), GFP_KERNEL); 
> >
> > Why is that change needed? 
> 
> On my H3/H2+ only 2 words after the first 4 words is not zero.
> 
> I don't feel like add so many 0s to randomness.

As far as I know, filling the entropy pool with zeros does no harm, it
just doesn't do anything either.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] nvmem: sunxi-sid: add support for H3 and A64's SID controller
@ 2017-01-27  9:41   ` Maxime Ripard
  0 siblings, 0 replies; 14+ messages in thread
From: Maxime Ripard @ 2017-01-27  9:41 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Srinivas Kandagatla, Rob Herring,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Chen-Yu Tsai

[-- Attachment #1: Type: text/plain, Size: 1156 bytes --]

On Fri, Jan 27, 2017 at 05:26:59PM +0800, Icenowy Zheng wrote:
> > > +Example for sun8i-h3: 
> > > + sid@01c14000 { 
> > > + compatible = "allwinner,sun8i-h3-sid"; 
> > > + /* 
> > > + * The length of SID on H3 is 0x100 bytes, add the value offset 
> > > + * 0x200, so the total length should be 0x300. 
> > > + */ 
> > > + reg = <0x01c14000 0x300>; 
> >
> > No, it should be the size of the memory region used and documented for 
> > that device, ie 1kB (0x400) in the H3 case, just like any other reg 
> > property. 
> 
> So bind the SID size with compatible?

If needed, yes.

> > > - randomness = kzalloc(sizeof(u8) * (size), GFP_KERNEL); 
> > > + randomness_size = max(size, SUNXI_SID_MAX_RANDOMNESS_SIZE); 
> > > + randomness = kzalloc(sizeof(u8) * (randomness_size), GFP_KERNEL); 
> >
> > Why is that change needed? 
> 
> On my H3/H2+ only 2 words after the first 4 words is not zero.
> 
> I don't feel like add so many 0s to randomness.

As far as I know, filling the entropy pool with zeros does no harm, it
just doesn't do anything either.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/2] nvmem: sunxi-sid: add support for H3 and A64's SID controller
@ 2017-01-27  9:41   ` Maxime Ripard
  0 siblings, 0 replies; 14+ messages in thread
From: Maxime Ripard @ 2017-01-27  9:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 27, 2017 at 05:26:59PM +0800, Icenowy Zheng wrote:
> > > +Example for sun8i-h3: 
> > > + sid at 01c14000 { 
> > > + compatible = "allwinner,sun8i-h3-sid"; 
> > > + /* 
> > > + * The length of SID on H3 is 0x100 bytes, add the value offset 
> > > + * 0x200, so the total length should be 0x300. 
> > > + */ 
> > > + reg = <0x01c14000 0x300>; 
> >
> > No, it should be the size of the memory region used and documented for 
> > that device, ie 1kB (0x400) in the H3 case, just like any other reg 
> > property. 
> 
> So bind the SID size with compatible?

If needed, yes.

> > > - randomness = kzalloc(sizeof(u8) * (size), GFP_KERNEL); 
> > > + randomness_size = max(size, SUNXI_SID_MAX_RANDOMNESS_SIZE); 
> > > + randomness = kzalloc(sizeof(u8) * (randomness_size), GFP_KERNEL); 
> >
> > Why is that change needed? 
> 
> On my H3/H2+ only 2 words after the first 4 words is not zero.
> 
> I don't feel like add so many 0s to randomness.

As far as I know, filling the entropy pool with zeros does no harm, it
just doesn't do anything either.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170127/b9bd8264/attachment.sig>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] nvmem: sunxi-sid: add support for H3 and A64's SID controller
@ 2017-01-27  9:26 Icenowy Zheng
  2017-01-27  9:41   ` Maxime Ripard
  0 siblings, 1 reply; 14+ messages in thread
From: Icenowy Zheng @ 2017-01-27  9:26 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Srinivas Kandagatla, Rob Herring,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Chen-Yu Tsai


2017年1月27日 16:17于 Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>写道:
>
> Hi, 
>
> On Thu, Jan 26, 2017 at 07:33:44PM +0800, Icenowy Zheng wrote: 
> > H3 and A64 SoCs have a bigger SID controller, which has its direct read 
> > address at 0x200 position in the SID block, not 0x0. 
> > 
> > Also, H3 SID controller has some silicon bug that makes the direct read 
> > value wrong at first, add code to workaround the bug. (This bug has 
> > already been fixed on A64 and later SoCs) 
> > 
> > Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> 
> > --- 
> >  .../bindings/nvmem/allwinner,sunxi-sid.txt         |  22 +++- 
> >  drivers/nvmem/sunxi_sid.c                          | 112 +++++++++++++++++++-- 
> >  2 files changed, 123 insertions(+), 11 deletions(-) 
> > 
> > diff --git a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt 
> > index d543ed3f5363..d7400c415338 100644 
> > --- a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt 
> > +++ b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt 
> > @@ -1,8 +1,16 @@ 
> >  Allwinner sunxi-sid 
> >  
> >  Required properties: 
> > -- compatible: "allwinner,sun4i-a10-sid" or "allwinner,sun7i-a20-sid" 
> > -- reg: Should contain registers location and length 
> > +- compatible: Should be one of the following (depending on your SoC): 
> > +  "allwinner,sun4i-a10-sid" 
> > +  "allwinner,sun7i-a20-sid" 
> > +  "allwinner,sun8i-h3-sid" 
> > +  "allwinner,sun50i-a64-sid" 
> > + 
> > +- reg: Should contain registers location and registers length, for A10 and A20 
> > +  SoCs the length is directly the length of SID; for H3, A64 and newer SoCs 
> > +  the length should be the sum of the length of SID and the offset of the value 
> > +  (0x200). 
> >  
> >  = Data cells = 
> >  Are child nodes of qfprom, bindings of which as described in 
> > @@ -19,3 +27,13 @@ Example for sun7i: 
> >  compatible = "allwinner,sun7i-a20-sid"; 
> >  reg = <0x01c23800 0x200> 
> >  }; 
> > + 
> > +Example for sun8i-h3: 
> > + sid@01c14000 { 
> > + compatible = "allwinner,sun8i-h3-sid"; 
> > + /* 
> > + * The length of SID on H3 is 0x100 bytes, add the value offset 
> > + * 0x200, so the total length should be 0x300. 
> > + */ 
> > + reg = <0x01c14000 0x300>; 
>
> No, it should be the size of the memory region used and documented for 
> that device, ie 1kB (0x400) in the H3 case, just like any other reg 
> property. 

So bind the SID size with compatible?

>
> > + }; 
> > diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c 
> > index 1567ccca8de3..2e327a66a938 100644 
> > --- a/drivers/nvmem/sunxi_sid.c 
> > +++ b/drivers/nvmem/sunxi_sid.c 
> > @@ -20,10 +20,28 @@ 
> >  #include <linux/module.h> 
> >  #include <linux/nvmem-provider.h> 
> >  #include <linux/of.h> 
> > +#include <linux/of_device.h> 
> >  #include <linux/platform_device.h> 
> >  #include <linux/slab.h> 
> >  #include <linux/random.h> 
> >  
> > +/* Registers and special values for doing register-based SID readout on H3 */ 
> > +#define SUN8I_SID_PRCTL 0x40 
> > +#define SUN8I_SID_RDKEY 0x60 
> > + 
> > +#define SUN8I_SID_OP_LOCK 0xAC 
> > +#define SUN8I_SID_OFFSET_MASK 0x1FF 
> > +#define SUN8I_SID_OFFSET_SHIFT 16 
> > +#define SUN8I_SID_LOCK_SHIFT 8 
> > +#define SUN8I_SID_READ BIT(1) 
> > + 
> > +/* 
> > + * For newer SoCs with a larger eFUSE, the bytes beyond the first 16 bytes are 
> > + * sparse, which makes it not suitable for adding randomness; legacy SoCs' SID 
> > + * have only 16 bytes, so we choose to use at most 16 bytes to add randomness. 
> > + */ 
> > +#define SUNXI_SID_MAX_RANDOMNESS_SIZE 16 
> > + 
> >  static struct nvmem_config econfig = { 
> >  .name = "sunxi-sid", 
> >  .read_only = true, 
> > @@ -32,8 +50,14 @@ static struct nvmem_config econfig = { 
> >  .owner = THIS_MODULE, 
> >  }; 
> >  
> > +struct sunxi_sid_cfg { 
> > + u32 value_offset; 
> > + bool need_register_readout; 
> > +}; 
> > + 
> >  struct sunxi_sid { 
> >  void __iomem *base; 
> > + u32 value_offset; 
> >  }; 
> >  
> >  /* We read the entire key, due to a 32 bit read alignment requirement. Since we 
> > @@ -46,7 +70,8 @@ static u8 sunxi_sid_read_byte(const struct sunxi_sid *sid, 
> >  { 
> >  u32 sid_key; 
> >  
> > - sid_key = ioread32be(sid->base + round_down(offset, 4)); 
> > + sid_key = ioread32be(sid->base + sid->value_offset + 
> > +      round_down(offset, 4)); 
> >  sid_key >>= (offset % 4) * 8; 
> >  
> >  return sid_key; /* Only return the last byte */ 
> > @@ -64,26 +89,77 @@ static int sunxi_sid_read(void *context, unsigned int offset, 
> >  return 0; 
> >  } 
> >  
> > +static int sun8i_sid_register_readout(const struct sunxi_sid *sid, 
> > +       const unsigned int word, 
> > +       u32 *out) 
> > +{ 
> > + u32 reg_val; 
> > + unsigned long expire = jiffies + msecs_to_jiffies(250); 
> > + 
> > + /* Set word, lock access, and set read command */ 
> > + reg_val = (word & SUN8I_SID_OFFSET_MASK) 
> > +   << SUN8I_SID_OFFSET_SHIFT; 
> > + reg_val |= SUN8I_SID_OP_LOCK << SUN8I_SID_LOCK_SHIFT; 
> > + reg_val |= SUN8I_SID_READ; 
> > + writel(reg_val, sid->base + SUN8I_SID_PRCTL); 
> > + 
> > + do { 
> > + reg_val = readl(sid->base + SUN8I_SID_PRCTL); 
> > + } while (time_before(jiffies, expire) && (reg_val & SUN8I_SID_READ)); 
> > + 
> > + if (reg_val & SUN8I_SID_READ) 
> > + return -EIO; 
> > + 
> > + if (out) 
> > + *out = readl(sid->base + SUN8I_SID_RDKEY); 
> > + writel(0, sid->base + SUN8I_SID_PRCTL); 
> > + return 0; 
> > +} 
> > + 
> >  static int sunxi_sid_probe(struct platform_device *pdev) 
> >  { 
> >  struct device *dev = &pdev->dev; 
> >  struct resource *res; 
> >  struct nvmem_device *nvmem; 
> >  struct sunxi_sid *sid; 
> > - int ret, i, size; 
> > + int ret, i, size, randomness_size; 
> >  char *randomness; 
> > + const struct sunxi_sid_cfg *cfg; 
> >  
> >  sid = devm_kzalloc(dev, sizeof(*sid), GFP_KERNEL); 
> >  if (!sid) 
> >  return -ENOMEM; 
> >  
> > + cfg = of_device_get_match_data(dev); 
> > + if (!cfg) 
> > + return -EINVAL; 
> > + sid->value_offset = cfg->value_offset; 
> > + 
> >  res = platform_get_resource(pdev, IORESOURCE_MEM, 0); 
> >  sid->base = devm_ioremap_resource(dev, res); 
> >  if (IS_ERR(sid->base)) 
> >  return PTR_ERR(sid->base); 
> >  
> > - size = resource_size(res) - 1; 
> > - econfig.size = resource_size(res); 
> > + size = resource_size(res) - cfg->value_offset; 
> > + 
> > + if (cfg->need_register_readout) { 
> > + /* 
> > + * H3's SID controller have a bug that the value at 0x200 
> > + * offset is not the correct value when the hardware is reset. 
> > + * However, after doing a register-based read operation, the 
> > + * value become right. 
> > + * Do a full read operation here, but ignore its value 
> > + * (as it's more fast to read by direct MMIO value than 
> > + * with registers) 
> > + */ 
> > + for (i = 0; i < (size >> 2); i++) { 
> > + ret = sun8i_sid_register_readout(sid, i, NULL); 
> > + if (ret) 
> > + return ret; 
> > + } 
> > + } 
> > + 
> > + econfig.size = size; 
> >  econfig.dev = dev; 
> >  econfig.reg_read = sunxi_sid_read; 
> >  econfig.priv = sid; 
> > @@ -91,16 +167,17 @@ static int sunxi_sid_probe(struct platform_device *pdev) 
> >  if (IS_ERR(nvmem)) 
> >  return PTR_ERR(nvmem); 
> >  
> > - randomness = kzalloc(sizeof(u8) * (size), GFP_KERNEL); 
> > + randomness_size = max(size, SUNXI_SID_MAX_RANDOMNESS_SIZE); 
> > + randomness = kzalloc(sizeof(u8) * (randomness_size), GFP_KERNEL); 
>
> Why is that change needed? 

On my H3/H2+ only 2 words after the first 4 words is not zero.

I don't feel like add so many 0s to randomness.

>
> Thanks, 
> Maxime 
>
>
> -- 
> Maxime Ripard, Free Electrons 
> Embedded Linux and Kernel engineering 
> http://free-electrons.com 

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-01-27  9:41 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-26 11:33 [PATCH 1/2] nvmem: sunxi-sid: add support for H3 and A64's SID controller Icenowy Zheng
2017-01-26 11:33 ` Icenowy Zheng
     [not found] ` <20170126113345.33686-1-icenowy-ymACFijhrKM@public.gmane.org>
2017-01-26 11:33   ` [PATCH 2/2] ARM: dts: sun8i: enable SID on Allwinner H3 SoC Icenowy Zheng
2017-01-26 11:33     ` Icenowy Zheng
2017-01-27  8:17 ` [PATCH 1/2] nvmem: sunxi-sid: add support for H3 and A64's SID controller Maxime Ripard
2017-01-27  8:17   ` Maxime Ripard
2017-01-27  8:17   ` Maxime Ripard
2017-01-27  8:46   ` [linux-sunxi] " Julian Calaby
2017-01-27  8:46     ` Julian Calaby
2017-01-27  8:46     ` Julian Calaby
2017-01-27  9:26 Icenowy Zheng
2017-01-27  9:41 ` Maxime Ripard
2017-01-27  9:41   ` Maxime Ripard
2017-01-27  9:41   ` Maxime Ripard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.