On Fri, Jul 25, 2014 at 11:16 AM, Lucas Stach <l.stach@xxxxxxxxxxxxxx> 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@xxxxxxxxx>
>
> 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@xxxxxxxxxxxxxx>
>
>> ---
>> To: barebox@xxxxxxxxxxxxxxxxxxx
>> To: Sebastian Hesselbarth <sebastian.hesselbarth@xxxxxxxxx>
>> Cc: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx>
>> Cc: Ezequiel Garcia <ezequiel.garcia@xxxxxxxxxxxxxxxxxx>
>> ---
> [...]
>> +
>> +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@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
