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

[Ezequiel Garcia]

On 1 February 2018 at 05:26, Michal Simek <michal.simek at xilinx.com> wrote:
> 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 see.

>> 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.
>

Indeed, it doesn't look bad at all. Let me try those patches,
see how they work.
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar