[U-Boot] [PATCH 3/5] zynq: Support CPU info display

[Michal Simek]

On 31.1.2018 22:25, Ezequiel Garcia wrote:
> On 26 January 2018 at 09:33, Michal Simek <michal.simek at xilinx.com> wrote:
>> Hi,
>>
>>
>> On 17.1.2018 14:56, Ezequiel Garcia wrote:
>>> This commit adds CPU and silicon version information
>>> consuming the SLCR IDCODE and DEVCFG MCTRL registers,
>>> respectively.
>>>
>>> Signed-off-by: Ariel D'Alessandro <ariel at vanguardiasur.com.ar>
>>> Signed-off-by: Ezequiel Garcia <ezequiel at vanguardiasur.com.ar>
>>> ---
>>>  arch/arm/mach-zynq/cpu.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 46 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-zynq/cpu.c b/arch/arm/mach-zynq/cpu.c
>>> index 53a07b0059c2..602f483c162b 100644
>>> --- a/arch/arm/mach-zynq/cpu.c
>>> +++ b/arch/arm/mach-zynq/cpu.c
>>> @@ -35,6 +35,25 @@ static const struct {
>>>  };
>>>  #endif
>>>
>>> +#ifdef CONFIG_DISPLAY_CPUINFO
>>> +static const struct {
>>> +     u8 idcode;
>>> +     const char *cpuinfo;
>>> +} zynq_cpu_info[] = {
>>> +     { .idcode = XILINX_ZYNQ_7007S,  .cpuinfo = XILINX_XC7Z007S_NAME },
>>> +     { .idcode = XILINX_ZYNQ_7010,   .cpuinfo = XILINX_XC7Z010_NAME },
>>> +     { .idcode = XILINX_ZYNQ_7012S,  .cpuinfo = XILINX_XC7Z012S_NAME },
>>> +     { .idcode = XILINX_ZYNQ_7014S,  .cpuinfo = XILINX_XC7Z014S_NAME },
>>> +     { .idcode = XILINX_ZYNQ_7015,   .cpuinfo = XILINX_XC7Z015_NAME },
>>> +     { .idcode = XILINX_ZYNQ_7020,   .cpuinfo = XILINX_XC7Z020_NAME },
>>> +     { .idcode = XILINX_ZYNQ_7030,   .cpuinfo = XILINX_XC7Z030_NAME },
>>> +     { .idcode = XILINX_ZYNQ_7035,   .cpuinfo = XILINX_XC7Z035_NAME },
>>> +     { .idcode = XILINX_ZYNQ_7045,   .cpuinfo = XILINX_XC7Z045_NAME },
>>> +     { .idcode = XILINX_ZYNQ_7100,   .cpuinfo = XILINX_XC7Z100_NAME },
>>> +     { /* Sentinel */ },
>>
>> This table pretty much reflect what it is in 2/5.
>>
>> static const struct {
>>         u8 idcode;
>>         const char *cpuinfo; /* or better name devicename */
>>         u32 fpga_size;
>> } zynq_cpu_info[] = {
>>
>> From xilinx_desc I think size is unused but we can keep in filled and
>> cookie is also not used and can be 0. The rest of data for xilinx_desc
>> is static anyway.
>>
>> It means doing this properly will be the best to fill that xilinx_desc
>> and also it doesn't make sense to call zynq_slcr_get_idcode() twice.
>> It should be enough to detect chip once and fill pointer to actual
>> configuration. And then when fpga should be add then use them. The same
>> for cpuinfo. Link is already setup and you can just use it.
>>
> 
> So you propose to have just one table instead of two
> zynq_cpu_info and zynq_fpga_descs ?

I was playing with it last Friday and I have sent that 2 patches as RFC.

> I guess that'll work and might result in simpler code.
> On the other side, the reason I kept them separate
> is because each of them are compiled-in via different
> compile-time options.
> 
> Having a single table will mean playing nasty ifdef
> games, which usually mean trouble.
> 
> Regarding calling zynq_slcr_get_idcode twice,
> is it really that bad?

It is not that bad. It is probably better than having global variable.

Take a look at that RFC. Buildman is also showing pretty nice size
reduction but there are still some pieces which can be done better.
It is based on your code that's why fell free to rework it.

Thanks,
Michal