Re: [PATCH -next] powerpc/85xx: Add support for X-ES MPC85xx boards

[Kumar Gala <galak@kernel.crashing.org>]

On Jun 10, 2009, at 4:46 PM, Nate Case wrote:

> Hi Kumar,
>
> Thanks for the feedback.  Response below:
>
>>> +
>>> +	root = of_find_node_by_path("/");
>>> +	if (root == NULL)
>>> +		return;
>>> +
>>> +	model = of_get_property(root, "model", NULL);
>>> +	if (strncasecmp(model, "xes,", strlen("xes,")) != 0)
>>> +		return;
>>
>> What is this check for?
>
> Historical accident.  A while back we somehow ended up with a case of
> probe() succeeding when it shouldn't have, and this was the  
> workaround.
> I'll remove it since it's useless now.
>
>>> +
>>> +	printk(KERN_INFO "X-ES MPC85xx-based single-board computer: %s\n",
>>> +	       model + strlen("xes,"));
>>
>> Why not print the name from machine_id.name
>
> Because machine_id->name will give us "X-ES MPC8572" from the machine
> description rather than the actual board name (e.g., xpedite5370).
>
> In our situation, we have several boards that share largely the same
> hardware design.  I decided to just put 1 machine definition per
> processor rather than having a machine definition for every board  
> (which
> would get pretty crazy for our case).  For example, we have at least 6
> MPC8572-based boards with more on the way, but all of their changes  
> can
> be described in the device tree (e.g., changes in number of ethernet
> ports or PCI express controllers), so there's no need for a machine
> definition for all of them.
>
>>> +machine_device_initcall(xes_mpc8572, xes_mpc85xx_publish_devices);
>>> +machine_device_initcall(xes_mpc8548, xes_mpc85xx_publish_devices);
>>
>> Do you not need this for xes_mpc8540?
>
> Yes, thanks.  I'll fix this and the other things you mentioned and
> re-submit.  Will this be able to make it into your next tree destined
> for 2.6.31?

If it get submitted pretty soon I'd be willing to take it for .31.   
Also can you split the patch into 3 pieces (defconfig, .dts, and  
actual platform code)

- k