Re: [PATCH RESEND 6/6] pci: mvebu: Add PCIe driver

[Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>]

On Fri, Jul 25, 2014 at 11:16 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Mittwoch, den 23.07.2014, 11:26 +0200 schrieb Sebastian Hesselbarth:
>> This adds a PCI driver for the controllers found on Marvell MVEBU SoCs.
>> Besides the functional driver itself, it also adds SoC specific PHY
>> setup required for PCIe. Currently, only Armada 370 is fully supported.
>>
>> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
>
> I can't really comment about the details, as I have no knowledge about
> this hardware in particular, but aside from the sysdata thing and one
> nit below this looks reasonable, so:
>
> Acked-by: Lucas Stach <l.stach@pengutronix.de>
>
>> ---
>> To: barebox@lists.infradead.org
>> To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Cc: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>> ---
> [...]
>> +
>> +static struct mvebu_pcie_ops armada_370_ops = {
>> +     .phy_setup = armada_370_phy_setup,
>> +};
>> +
>> +static struct of_device_id mvebu_pcie_dt_ids[] = {
>> +#if defined(CONFIG_ARCH_ARMADA_XP)
>> +     { .compatible = "marvell,armada-xp-pcie", },
>> +#endif
>> +#if defined(CONFIG_ARCH_ARMADA_370)
>> +     { .compatible = "marvell,armada-370-pcie", .data = (u32)&armada_370_ops, },
>> +#endif
>> +#if defined(CONFIG_ARCH_DOVE)
>> +     { .compatible = "marvell,dove-pcie", },
>> +#endif
>> +#if defined(CONFIG_ARCH_KIRKWOOD)
>> +     { .compatible = "marvell,kirkwood-pcie", },
>> +#endif
>> +     { },
>> +};
>> +
>
> Do those #if defined really buy us anything? I don't think they make a
> big difference on code size, so please get rid of those.

Hmm, without the #ifdef's we'd carry armada_370_ops _and_
armada_370_phy_setup on all SoCs, don't we?

Assuming there will also be armada_xp_foo, dove_foo, and kirkwood_foo
it will be quite a bunch of unnecessary code to carry around. But I admit,
I haven't checked code sizes, yet.

Sebastian

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox