Re: [PATCH v2 6/7] mfd: cros_ec: Instantiate sub-devices from device tree

Hello Lee,

Sorry for the delay but had been on holidays last week.

On 09/04/2014 10:25 AM, Lee Jones wrote:

quoted

 
-static const struct mfd_cell cros_devs[] = {
-	{
-		.name = "cros-ec-keyb",
-		.id = 1,
-		.of_compatible = "google,cros-ec-keyb",
-	},
-	{
-		.name = "cros-ec-i2c-tunnel",
-		.id = 2,
-		.of_compatible = "google,cros-ec-i2c-tunnel",
-	},
-};
-
 int cros_ec_register(struct cros_ec_device *ec_dev)
 {
 	struct device *dev = ec_dev->dev;
+#ifdef CONFIG_OF
+	struct device_node *node = dev->of_node;
+#endif

Drop the #ifdiffery.

I used the #ifdef guards because I remembered of_node being conditionally
build but I see that this is not longer true after commmit:

c9e358d driver-core: remove conditionals around devicetree pointers

So I'll remove it, thanks for the pointer.

quoted

 	int err = 0;
 
 	if (ec_dev->din_size) {

@@ -138,13 +129,19 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 
 	mutex_init(&ec_dev->lock);
 
-	err = mfd_add_devices(dev, 0, cros_devs,
-			      ARRAY_SIZE(cros_devs),
-			      NULL, ec_dev->irq, NULL);
-	if (err) {
-		dev_err(dev, "failed to add mfd devices\n");
-		return err;
+#ifdef CONFIG_OF

And here.

Nod.

quoted

+	/*
+	 * Add sub-devices declared in the device tree.  NOTE they should NOT be
+	 * declared in cros_devs
+	 */

Drop this comment.  cros_devs no longer exists and we know what
of_platform_populate() does.

Ok.

quoted

+	if (node) {
+		err = of_platform_populate(node, NULL, NULL, dev);
+		if (err) {
+			dev_err(dev, "fail to add %s\n", node->full_name);

That's not true.  Just put "Failed to register subordinate devices".

Ok.

quoted

+			return err;
+		}
 	}
+#endif
 
 	dev_info(dev, "Chrome EC device registered\n");
 

Thanks a lot for the feedback and best regards,
Javier