Re: [PATCH v2 1/2] ARM: mvebu: Add support to get the ID and the revision of a SoC

[Gregory CLEMENT <gregory.clement@free-electrons.com>]

On 03/01/2014 17:41, Andrew Lunn wrote:
>>>> Hi Gregory
>>>>
>>>> I'm away from my hardware at the moment. 
>>>>
>>>> Does this work when all the PCIe ports have status = "disabled";? We
>>>> have many kirkwood devices in NAS boxes which don't use PCIe, so all
>>>> the ports are disabled. But they still exist in the SoC, so we can
>>>> read the IDs from them. I just don't know if of_get_next_child() will
>>>> only return enabled children?
>>>
>>> There is a function named of_get_next_available_child, so I assumed that
>>> of_get_next_child() will return all the children. But I can test it to
>>> be sure of it.
>>>
>>
>> I have just removed all the PCIe part in the
>> armada-xp-openblocks-ax3-4.dts file (PCie is disable by default in the
>> dtsi file) and it worled as expected! :)
> 
> Great, thanks for testing.
>  
>> by the way waht do you think of adding this line in at the end of the
>> mvebu_soc_id_init() function:
>>
>> pr_info("MVEBU SoC ID=0x%X, Rev=0x%X\n", soc_dev_id, soc_rev);
> 
> Kirkwood prints actual strings, not numbers. More readable.

With this number we are sure to have always an information even if the
SoC ID or version was unknown by the kernel.

>  
>> Also keep in mind that currently you can't use it for kirkwood because
>> the build of the file depend on CONFIG_ARCH_MVEBU. But as kirkwood
>> will soon joined the mach-mvebu directory, it won't be a problem then.
> 
> I think it is possible to do ../mach-mvebu/soc_id.o sort of thing in
> the Makefile. Ugly, but might work until we move.

Yes it is doable, but then we also need to update the mvebu_pcie_of_match_table.
However I would prefer doing this as a separate patch because this one is
for fixing a bug. Later we can improve the kernel.

> 
>     Andrew
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com