Re: [PATCH 11/25] powerpc: 4xx PLB to PCI Express support

[Stefan Roese <sr@denx.de>]

On Thursday 06 December 2007, Benjamin Herrenschmidt wrote:
> > > +/* Check that the core has been initied and if not, do it */
> > > +static int __init ppc4xx_pciex_check_core_init(struct device_node *np)
> > > +{
> > > +	static int core_init;
> > > +	int count = -ENODEV;
> > > +
> > > +	if (core_init++)
> > > +		return 0;
> > > +
> > > +#ifdef CONFIG_44x
> > > +	if (of_device_is_compatible(np, "ibm,plb-pciex-440speA"))
> > > +		ppc4xx_pciex_hwops = &ppc440speA_pcie_hwops;
> > > +	else if (of_device_is_compatible(np, "ibm,plb-pciex-440speB"))
> > > +		ppc4xx_pciex_hwops = &ppc440speB_pcie_hwops;
> >
> > We need some runtime detection of the 440SPe revision here. There are
> > boards out there (e.g. AMCC Yucca) which can be equipped with both PPC
> > revisions. :-(
>
> Ah... crap ! Do you think we should put that in the boot wrapper and
> fixup the device-tree or should we put it in the PCIe code proper ?

How about this: We change the compatible node to "plb-pciex-440spe" 
(without "A" and "B) and make a PVR runtime detection in the 4xx-pci driver.

What do you think? Should I prepare a patch for this?

> > > +#endif /* CONFIG_44x    */
> > > +#ifdef CONFIG_40x
> > > +	if (of_device_is_compatible(np, "ibm,plb-pciex-405ex"))
> > > +		ppc4xx_pciex_hwops = &ppc405ex_pcie_hwops;
> > > +#endif
> >
> > Why those #ifdef's? Just code-size reasons, since 40x and 44x will most
> > likely never be built into one image?
>
> Code size... I know how keen embedded people are to keep their kernel
> small and as of today, you can't build 40x and 44x support in the same
> image, so I didn't want to be blamed for adding bloat in that case :-)

Ok. Thanks.

Best regards,
Stefan