[PATCH 07/16] ARM: mvebu: Make the CPU idle initialization more generic

[Gregory CLEMENT]

Hi Thomas,

	struct device_node *np;
>> +	const struct of_device_id *match;
>> +
>> +	np = of_find_matching_node_and_match(NULL, of_cpuidle_table,
>> +					&match);
> 
> I'm not sure I like using of_find_matching_node_and_match() on the
> entire tree to find the root node compatible string. Wouldn't it be
> simpler and shorter to just do:
> 
> 	if (of_machine_is_compatible("marvell,armadaxp"))
> 		ret = armadaxp_cpuidle_init();
> 	else if (of_machine_is_compatible("marvell,armada370"))
> 		ret = armada370_cpuidle_init();
> 	else if (of_machine_is_compaitble("marvell,armada380"))
> 		ret = armada38x_cpuidle_init();
> 
> 	if (ret)
> 		return ret;

Indeed, as I don't use any resource from the device tree here, using
of_find_matching_node_and_match is overkill. And your alternative
will be enough.

> 
> Also, using a boolean to return a success/error status is not the
> kernel way of doing things, you should use an int instead and use
> proper error codes or return 0 on success.

OK


Thanks,

Gregory

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com