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

[Javier Martinez Canillas <javier.martinez@collabora.co.uk>]

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