Hi Emilio,

On Thu, Jul 31, 2014 at 06:28:04PM -0300, Emilio L?pez wrote:

What's programmed in there in the case of the rev C?

+	}
+
+	iounmap(base);
+	of_node_put(np);
+
+	return ret;
+}
+
+/*
+ * On the sun5i SoCs (A10S, A13), we can read the revision information
+ * from the first bits in the Security ID. The detection logic is
+ * extracted from similar code on the Allwinner vendor tree, as this
+ * is undocumented on the user manual.
+ */
+
+static const struct of_device_id sun5i_sid_compatible[] __initconst = {
+	{ .compatible = "allwinner,sun4i-a10-sid", },
+	{},
+};
+
+static int __init sun5i_get_soc_revision(void)
+{
+	struct device_node *np;
+	void __iomem *sid;
+	u32 val;
+	int ret;
+
+	/* Find the SID node */
+	np = of_find_matching_node(NULL, sun5i_sid_compatible);
+	if (!np)
+		return -ENODEV;
+
+	/* Temporarily map it for reading */
+	sid = of_iomap(np, 0);
+	if (!sid) {
+		of_node_put(np);
+		return -ENOMEM;
+	}
+
+	/* Read and extract the chip revision from the SID */
+	val = readl(sid);
+	val = (val >> 8) & 0xffffff;
+
+	switch (val) {
+	case 0:        /* A10S/A13 rev A */
+	case 0x162541: /* A10S/A13 rev A */
+	case 0x162565: /* A13 rev A */

Some defines would be nice.

Also, isn't the SID supposed to identify any SoC, not just the sun5i?

+		ret = 'A';
+		break;
+	case 0x162542: /* A10S/A13 rev B */
+		ret = 'B';
+		break;
+	default:       /* Unknown chip revision */
+		ret = -ENODATA;
+	}
+
+	iounmap(sid);
+	of_node_put(np);
+
+	return ret;
+}
+
+int __init sunxi_soc_revision(void)
+{
+	static int revision = -ENODEV;
+
+	/* Try to query the hardware just once */
+	if (!IS_ERR_VALUE(revision))
+		return revision;
+
+	if (of_machine_is_compatible("allwinner,sun4i-a10")) {
+		revision = sun4i_get_soc_revision();
+	} else if (of_machine_is_compatible("allwinner,sun5i-a10s") ||
+		   of_machine_is_compatible("allwinner,sun5i-a13")) {
+		revision = sun5i_get_soc_revision();
+	}
+
+	return revision;
+}
+
+/* Matches for the sunxi SoCs we know of */
+static const struct of_device_id soc_matches[] __initconst = {
+	{ .compatible = "allwinner,sun4i-a10", .data = "A10 (sun4i)" },
+	{ .compatible = "allwinner,sun5i-a13", .data = "A13 (sun5i)" },
+	{ .compatible = "allwinner,sun5i-a10s", .data = "A10S (sun5i)" },
+	{ .compatible = "allwinner,sun6i-a31", .data = "A31 (sun6i)" },
+	{ .compatible = "allwinner,sun7i-a20", .data = "A20 (sun7i)" },
+	{ .compatible = "allwinner,sun8i-a23", .data = "A23 (sun8i)" },
+	{ /* sentinel */ },
+};

Hmmm, no, either auto-detect it, or don't, but that's useless. We can
already have the same information from the DT directly.

+static int __init sunxi_register_soc_device(void)
+{
+	struct soc_device_attribute *soc_dev_attr;
+	struct soc_device *soc_dev;
+	const struct of_device_id *match;
+	struct device_node *root;
+	int revision;
+
+	/* Only run on sunxi SoCs that we know of */
+	root = of_find_node_by_path("/");
+	match = of_match_node(soc_matches, root);
+	if (!match)
+		goto exit;
+
+	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
+	if (!soc_dev_attr)
+		goto exit;
+
+	/* Read the machine name if available */
+	of_property_read_string(root, "model", &soc_dev_attr->machine);
+
+	soc_dev_attr->family = kstrdup("Allwinner A Series", GFP_KERNEL);

I think the family should be sun*i

+	soc_dev_attr->soc_id = kstrdup(match->data, GFP_KERNEL);

And soc_id would be just the name of the SoC.

+
+	/* Revision may not always be available */
+	revision = sunxi_soc_revision();
+	if (IS_ERR_VALUE(revision))
+		soc_dev_attr->revision = kstrdup("Unknown", GFP_KERNEL);
+	else
+		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%c", revision);
+
+	soc_dev = soc_device_register(soc_dev_attr);
+	if (IS_ERR(soc_dev))
+		goto free_struct;
+
+	/*
+	 * Print an informational line mentioning the hardware details.
+	 * It may come in handy during bug reports, as some early SoC
+	 * revisions have hardware quirks and do not get much testing.
+	 */
+	pr_info("SoC bus registered, running %s %s, revision %s\n",
+		soc_dev_attr->family, soc_dev_attr->soc_id,
+		soc_dev_attr->revision);

Maybe a pr_fmt would be nice here.

+
+	return 0;
+
+free_struct:
+	kfree(soc_dev_attr->family);
+	kfree(soc_dev_attr->soc_id);
+	kfree(soc_dev_attr->revision);
+	kfree(soc_dev_attr);
+exit:
+	of_node_put(root);
+
+	return 0;
+}
+postcore_initcall(sunxi_register_soc_device)

I'm kind of reluctant to this. That would mean that it will run for
every SoC.

Can't you tie it to the system controller, and have it probed as
usual?

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