Re: [PATCH] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

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

On Sep 11, 2007, at 10:55 AM, Olof Johansson wrote:

> On Tue, Sep 11, 2007 at 10:50:18AM -0500, Kumar Gala wrote:
>>>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>> b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>> index 3a5c3c4..1e2eba8 100644
>>>> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>> +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>> @@ -181,6 +181,23 @@ static int __init mpc8544_ds_probe(void)
>>>>  	}
>>>>  }
>>>>
>>>> +/*
>>>> + * Called very early, device-tree isn't unflattened
>>>> + */
>>>> +static int __init mpc8572_ds_probe(void)
>>>> +{
>>>> +	unsigned long root = of_get_flat_dt_root();
>>>> +
>>>> +	if (of_flat_dt_is_compatible(root, "MPC8572DS")) {
>>>> +#ifdef CONFIG_PCI
>>>> +		primary_phb_addr = 0x8000;
>>>> +#endif
>>>> +		return 1;
>>>> +	} else {
>>>> +		return 0;
>>>> +	}
>>>> +}
>>>> +
>>>>  define_machine(mpc8544_ds) {
>>>>  	.name			= "MPC8544 DS",
>>>>  	.probe			= mpc8544_ds_probe,
>>>> @@ -194,3 +211,17 @@ define_machine(mpc8544_ds) {
>>>>  	.calibrate_decr		= generic_calibrate_decr,
>>>>  	.progress		= udbg_progress,
>>>>  };
>>>> +
>>>> +define_machine(mpc8572_ds) {
>>>> +	.name			= "MPC8572 DS",
>>>> +	.probe			= mpc8572_ds_probe,
>>>> +	.setup_arch		= mpc85xx_ds_setup_arch,
>>>> +	.init_IRQ		= mpc85xx_ds_pic_init,
>>>> +#ifdef CONFIG_PCI
>>>> +	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>>>> +#endif
>>>> +	.get_irq		= mpic_get_irq,
>>>> +	.restart		= mpc85xx_restart,
>>>> +	.calibrate_decr		= generic_calibrate_decr,
>>>> +	.progress		= udbg_progress,
>>>> +};
>>>
>>> How different are these boards really? Could you just detect  
>>> MPC85xxDS
>>> and have a generic platform for them, or are they different  
>>> enough that
>>> you need individual ones for it?
>>
>> I wanted a different probe.  I figured having a different struct  
>> was a
>> simple solution.
>
> Seems like the only reason to need that is the setting of
> primary_phb_addr.  Can't that information be derived out of the device
> tree instead? That'd avoid alot of code duplication (code that  
> includes
> ifdefs, FWIW :-)

well the ifdefs are orthogonal.  We don't have a way of knowing  
primary from the device tree today.

> It just seems like a slippery slope. I'm not objecting directly to  
> this
> patch, but I think it should be fixed for the longer term.

Once we have a clean way of knowing primary PHB than I'm happy to fixup.

- k