On 10/30/2015 09:24, Hongbo Zhang wrote:
On 29 October 2015 at 21:25, Maxim Uvarov <maxim.uva...@linaro.org> wrote:
On 10/29/2015 13:51, Hongbo Zhang wrote:
This patch is for https://bugs.linaro.org/show_bug.cgi?id=1870, and
has been tested on Juno.

On 29 October 2015 at 18:45,  <hongbo.zh...@freescale.com> wrote:
From: Hongbo Zhang <hongbo.zh...@linaro.org>

In the default dummy function systemcpu(), only cpu_hz[0] and
model_str[0]
are set to dummy values, then in the validation code if iterate each CPU,
cores other than core 0 report failure, this patchs pad all the arrays to
default values to pass validation.

Signed-off-by: Hongbo Zhang <hongbo.zh...@linaro.org>
---
   platform/linux-generic/odp_system_info.c | 8 +++++---
   1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/platform/linux-generic/odp_system_info.c
b/platform/linux-generic/odp_system_info.c
index 8bd1584..5dbf68a 100644
--- a/platform/linux-generic/odp_system_info.c
+++ b/platform/linux-generic/odp_system_info.c
@@ -358,7 +358,7 @@ static int systemcpu(odp_system_info_t *sysinfo)

   static int systemcpu(odp_system_info_t *sysinfo)
   {
-       int ret;
+       int ret, i;

          ret = sysconf_cpu_count();
          if (ret == 0) {
@@ -371,10 +371,12 @@ static int systemcpu(odp_system_info_t *sysinfo)
          sysinfo->huge_page_size = huge_page_size();

          /* Dummy values */
-       sysinfo->cpu_hz[0]       = 1400000000;
          sysinfo->cache_line_size = 64;

-       strncpy(sysinfo->model_str[0], "UNKNOWN",
sizeof(sysinfo->model_str));
+       for (i = 0; i < MAX_CPU_NUMBER; i++) {
+               sysinfo->cpu_hz[i]       = 1400000000;
+               strcpy(sysinfo->model_str[i], "UNKNOWN");
why 1400000 and not 0.

In fact, this systemcpu() function is pseudo code, it is for ARM
platform, because there is no sufficient sysfs interface for such
info, this should be implemented to return real value sooner or later.
140000 is legacy data, and if 0 the validation code returns failure.

For the test, all the defects you pointed did exist there, I just
followed the previous code pattern. It is OK for me to take this
chance to fix them since I touched this part of code.

Thanks.

How about changing api and say that if returned hz is 0 that means function can not determine hz of current cpus. And make 0 as valid number. I don't like some temporary fixes with dummy numbers due to other people can relay
on that number in their apps.

Maxim.

Also I think test is wrong. At least there is missmatch between test and
implementation:

void system_test_odp_cpu_hz(void)
{
     uint64_t hz;

     hz = odp_cpu_hz();
     CU_ASSERT(0 < hz);  <-- unsigned less then 0???
}

uint64_t odp_cpu_hz(void)
{
     int id = sched_getcpu();

     return arch_cpu_hz_current(id);
}

And finally:
static uint64_t arch_cpu_hz_current(int id ODP_UNUSED)
{
     return -1;
}

So you return -1 for unsigned function.

In my understanding:
odp_cpu_hz(); on error has to return 0 and odp_err has to be set.

Btw, you have the same mismatches for all other functions:
odp_cpu_hz_id(cpu);
odp_cpu_hz_max_id(cpu);

All of that has to be fixed.

Thank you,
Maxim.



+       }

          return 0;
   }
--
2.1.4

_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to