diff mbox

[RESEND,6/7] mfd: cros_ec: Instantiate sub-devices from device tree

Message ID 1408536812-7836-7-git-send-email-javier.martinez@collabora.co.uk
State Superseded
Headers show

Commit Message

Javier Martinez Canillas Aug. 20, 2014, 12:13 p.m. UTC
From: Todd Broch <tbroch@chromium.org>

If the EC device tree node has sub-nodes, try to instantiate them as
MFD sub-devices.  We can configure the EC features provided by the board.

Signed-off-by: Todd Broch <tbroch@chromium.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/mfd/cros_ec.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

Comments

Lee Jones Aug. 21, 2014, 2:25 p.m. UTC | #1
On Wed, 20 Aug 2014, Javier Martinez Canillas wrote:

> From: Todd Broch <tbroch@chromium.org>
> 
> If the EC device tree node has sub-nodes, try to instantiate them as
> MFD sub-devices.  We can configure the EC features provided by the board.
> 
> Signed-off-by: Todd Broch <tbroch@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/mfd/cros_ec.c | 40 ++++++++++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 634c434..96c926c 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -21,6 +21,7 @@
>  #include <linux/slab.h>
>  #include <linux/module.h>
>  #include <linux/mfd/core.h>
> +#include <linux/of_platform.h>
>  #include <linux/mfd/cros_ec.h>
>  #include <linux/mfd/cros_ec_commands.h>
>  #include <linux/delay.h>
> @@ -109,22 +110,16 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
>  EXPORT_SYMBOL(cros_ec_cmd_xfer);
>  
>  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",
> -	},
>  };

Why are you keeping this round if it's empty?

>  int cros_ec_register(struct cros_ec_device *ec_dev)
>  {
>  	struct device *dev = ec_dev->dev;
>  	int err = 0;
> +#ifdef CONFIG_OF
> +	struct device_node *node;
> +	int id = ARRAY_SIZE(cros_devs);
> +#endif
>  
>  	if (ec_dev->din_size) {
>  		ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
> @@ -146,6 +141,31 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  		dev_err(dev, "failed to add mfd devices\n");
>  		return err;
>  	}
> +#ifdef CONFIG_OF
> +	/*
> +	 * Add sub-devices declared in the device tree.  NOTE they should NOT be
> +	 * declared in cros_devs
> +	 */
> +	for_each_child_of_node(dev->of_node, node) {
> +		char name[128];
> +		struct mfd_cell cell = {
> +			.id = 0,
> +			.name = name,
> +		};
> +
> +		if (of_modalias_node(node, name, sizeof(name)) < 0) {
> +			dev_err(dev, "modalias failure on %s\n",
> +				node->full_name);
> +			continue;
> +		}
> +		dev_dbg(dev, "adding MFD sub-device %s\n", node->name);
> +		cell.of_compatible = of_get_property(node, "compatible", NULL);
> +		err = mfd_add_devices(dev, ++id, &cell, 1, NULL, ec_dev->irq,
> +				      NULL);
> +		if (err)
> +			dev_err(dev, "fail to add %s\n", node->full_name);
> +	}
> +#endif

This is grim!

Why don't you use of_platform_populate()?

>  	dev_info(dev, "Chrome EC device registered\n");
>
Javier Martinez Canillas Aug. 22, 2014, 11:27 a.m. UTC | #2
Hello Lee,

On 08/21/2014 04:25 PM, Lee Jones wrote:
>>  
>>  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",
>> -	},
>>  };
> 
> Why are you keeping this round if it's empty?
>

Right, I'll just remove it.

>>  	}
>> +#ifdef CONFIG_OF
>> +	/*
>> +	 * Add sub-devices declared in the device tree.  NOTE they should NOT be
>> +	 * declared in cros_devs
>> +	 */
>> +	for_each_child_of_node(dev->of_node, node) {
>> +		char name[128];
>> +		struct mfd_cell cell = {
>> +			.id = 0,
>> +			.name = name,
>> +		};
>> +
>> +		if (of_modalias_node(node, name, sizeof(name)) < 0) {
>> +			dev_err(dev, "modalias failure on %s\n",
>> +				node->full_name);
>> +			continue;
>> +		}
>> +		dev_dbg(dev, "adding MFD sub-device %s\n", node->name);
>> +		cell.of_compatible = of_get_property(node, "compatible", NULL);
>> +		err = mfd_add_devices(dev, ++id, &cell, 1, NULL, ec_dev->irq,
>> +				      NULL);
>> +		if (err)
>> +			dev_err(dev, "fail to add %s\n", node->full_name);
>> +	}
>> +#endif
> 
> This is grim!
> 
> Why don't you use of_platform_populate()?
>

Indeed, I think it may just work and all these is not needed.

Thanks for your feedback and best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 634c434..96c926c 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -21,6 +21,7 @@ 
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/mfd/core.h>
+#include <linux/of_platform.h>
 #include <linux/mfd/cros_ec.h>
 #include <linux/mfd/cros_ec_commands.h>
 #include <linux/delay.h>
@@ -109,22 +110,16 @@  int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
 EXPORT_SYMBOL(cros_ec_cmd_xfer);
 
 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;
 	int err = 0;
+#ifdef CONFIG_OF
+	struct device_node *node;
+	int id = ARRAY_SIZE(cros_devs);
+#endif
 
 	if (ec_dev->din_size) {
 		ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
@@ -146,6 +141,31 @@  int cros_ec_register(struct cros_ec_device *ec_dev)
 		dev_err(dev, "failed to add mfd devices\n");
 		return err;
 	}
+#ifdef CONFIG_OF
+	/*
+	 * Add sub-devices declared in the device tree.  NOTE they should NOT be
+	 * declared in cros_devs
+	 */
+	for_each_child_of_node(dev->of_node, node) {
+		char name[128];
+		struct mfd_cell cell = {
+			.id = 0,
+			.name = name,
+		};
+
+		if (of_modalias_node(node, name, sizeof(name)) < 0) {
+			dev_err(dev, "modalias failure on %s\n",
+				node->full_name);
+			continue;
+		}
+		dev_dbg(dev, "adding MFD sub-device %s\n", node->name);
+		cell.of_compatible = of_get_property(node, "compatible", NULL);
+		err = mfd_add_devices(dev, ++id, &cell, 1, NULL, ec_dev->irq,
+				      NULL);
+		if (err)
+			dev_err(dev, "fail to add %s\n", node->full_name);
+	}
+#endif
 
 	dev_info(dev, "Chrome EC device registered\n");