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

Kumar Gala
2007-09-11T15:57:39Z

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