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

2018-02-01 Thread Ezequiel Garcia 
On 1 February 2018 at 05:26, Michal Simek  wrote:
> On 31.1.2018 22:25, Ezequiel Garcia wrote:
>> On 26 January 2018 at 09:33, Michal Simek  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 
 Signed-off-by: Ezequiel Garcia 
 ---
  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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2018-02-01 Thread Michal Simek 
On 31.1.2018 22:25, Ezequiel Garcia wrote:
> On 26 January 2018 at 09:33, Michal Simek  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 
>>> Signed-off-by: Ezequiel Garcia 
>>> ---
>>>  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



___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2018-01-31 Thread Ezequiel Garcia 
On 26 January 2018 at 09:33, Michal Simek  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 
>> Signed-off-by: Ezequiel Garcia 
>> ---
>>  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 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?
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2018-01-26 Thread Michal Simek 
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 
> Signed-off-by: Ezequiel Garcia 
> ---
>  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.

Thanks,
Michal
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2018-01-17 Thread Ezequiel Garcia 
This commit adds CPU and silicon version information
consuming the SLCR IDCODE and DEVCFG MCTRL registers,
respectively.

Signed-off-by: Ariel D'Alessandro 
Signed-off-by: Ezequiel Garcia 
---
 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 */ },
+};
+#endif
+
 int arch_cpu_init(void)
 {
zynq_slcr_unlock();
@@ -99,3 +118,30 @@ const xilinx_desc *zynq_fpga_desc(void)
return NULL;
 }
 #endif
+
+#ifdef CONFIG_DISPLAY_CPUINFO
+int print_cpuinfo(void)
+{
+   u32 idcode, version;
+   bool found;
+   u8 i;
+
+   idcode = zynq_slcr_get_idcode();
+   found = false;
+   for (i = 0; zynq_cpu_info[i].idcode; i++) {
+   if (zynq_cpu_info[i].idcode == idcode) {
+   found = true;
+   break;
+   }
+   }
+
+   version = zynq_get_silicon_version() << 1;
+   if (version > (PCW_SILICON_VERSION_3 << 1))
+   version += 1;
+   if (found) {
+   printf("CPU:   Zynq %s\n", zynq_cpu_info[i].cpuinfo);
+   printf("Silicon: v%d.%d\n", version >> 1, version & 1);
+   }
+   return 0;
+}
+#endif
-- 
2.15.1

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot