Re: [PM-QA] question about cpufreq subtest

[Daniel Lezcano]

On 18/08/2017 08:29, Viresh Kumar wrote:
> On 18 August 2017 at 11:52, Dai Xiang  wrote:

[ ... ]

> I can see all of them fail and the reason is quite simple to see. Even
> intel-pstate
> doesn't provide a frequency table and so whatsoever depends on a frequency
> table will fail.
> 
> These tests aren't really meant for x86 stuff, sorry about that :(

Actually they were meant for x86 but they were implemented before the
intel_pstate driver and did not evolve to take it into account.


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PM-QA] question about cpufreq subtest

[Daniel Lezcano]

On 18/08/2017 08:20, Viresh Kumar wrote:
> On 18 August 2017 at 11:48, Daniel Lezcano <daniel.lezc...@linaro.org> wrote:
>> I don't know the passive / active mode but the path changed.
>>
>> /sys/devices/system/cpu/cpufreq/policy[0-9]/scaling_driver
> 
> Naah, we wouldn't have broken userspace like that :)
> 
> $ ls -l /sys/devices/system/cpu/cpu0/cpufreq
> lrwxrwxrwx 1 root root 0 Aug 16 08:41
> /sys/devices/system/cpu/cpu0/cpufreq -> ../cpufreq/policy0/

Ah, yes. I'm not fully awake this morning :)


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PM-QA] question about cpufreq subtest

[Daniel Lezcano]

On 18/08/2017 07:43, Viresh Kumar wrote:
> On 18 August 2017 at 11:11, Dai Xiang  wrote:
> 
>> In fact, i has used intel_pstate driver:
>> $ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_driver
>> intel_pstate
> 
> It may be running in *active* mode and you need to put it in *passive* mode.
> 
> Try passing following with cmdline:
> 
> intel_pstate=passive

I don't know the passive / active mode but the path changed.

/sys/devices/system/cpu/cpufreq/policy[0-9]/scaling_driver

So there is probably something to do in PM-QA.


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH PM-QA] Fixed array initializations to calculate freq deviation.

[Daniel Lezcano]

On 05/05/2016 12:06 PM, Saul Romero wrote:

Changes in cpufreq_06.sh to calculate summatory and
average of frequency measurements.

Signed-off-by: Saul Romero 
---


Hi Saul,

can you elaborate in the changelog what happens with the current and why 
it must be fixed. Also, a proper fix should not comment the code.


Thanks.

  -- Daniel

--
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 1/6] Change options to flag approach.

[Daniel Lezcano]

On 02/22/2016 04:19 PM, David Brown wrote:

On Fri, Feb 19, 2016 at 02:32:40PM +, Daniel Lezcano wrote:


From: Daniel Lezcano <daniel.lezc...@free.fr>


Did you mean to have these patches authored by your @free.fr address,
but sent from Linaro?


Yes, it is just one of them.


Subject: Re: [PATCH 1/6] Change options to flag approach.


You might want to put something in to state what repo the patches are
intended for.  This helps with a list like linaro-dev which gets email
regarding multiple topics.

I've typically seen things like:

  [project-name][PATCH 1/6] Description here
  [PATCH 1/6][project] Description here


Ah, yes. I should have added the project name, my bad.


It's also probably a good idea with a patch series to write a
coverletter explaining the purpose of the series.  Although that
probably depends a little on what project the patch is going to.


Thanks for the heads up.

  -- Daniel

--
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 6/6] Pass options to init functions.

[Daniel Lezcano]

Signed-off-by: Daniel Lezcano <daniel.lezc...@linaro.org>
---
 clocks.c |  5 -
 gpio.c   |  5 -
 powerdebug.c | 19 ---
 powerdebug.h | 20 
 regulator.c  |  5 -
 sensor.c |  5 -
 6 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/clocks.c b/clocks.c
index c751ec1..b57de3d 100644
--- a/clocks.c
+++ b/clocks.c
@@ -447,10 +447,13 @@ static struct display_ops clock_ops = {
 /*
  * Initialize the clock framework
  */
-int clock_init(void)
+int clock_init(struct powerdebug_options *options)
 {
char clk_dir_path[MAX+1][PATH_MAX];
 
+   if (!(options->flags & CLOCK_OPTION))
+   return 0;
+
if (locate_debugfs(clk_dir_path[CCF]) || 
locate_debugfs(clk_dir_path[OCF]))
return -1;
 
diff --git a/gpio.c b/gpio.c
index 8fd3e5c..36dedd1 100644
--- a/gpio.c
+++ b/gpio.c
@@ -375,10 +375,13 @@ out:
 /*
  * Initialize the gpio framework
  */
-int gpio_init(void)
+int gpio_init(struct powerdebug_options *options)
 {
int ret = 0;
 
+   if (!(options->flags & GPIO_OPTION))
+   return 0;
+
ret = display_register(GPIO, _ops);
if (ret)
printf("error: gpio display register failed");
diff --git a/powerdebug.c b/powerdebug.c
index 0f5c98a..b626b05 100644
--- a/powerdebug.c
+++ b/powerdebug.c
@@ -33,17 +33,6 @@
 
 extern void sigwinch_handler(int);
 
-#define REGULATOR_OPTION   0x01
-#define SENSOR_OPTION  0x02
-#define CLOCK_OPTION   0x04
-#define GPIO_OPTION0x08
-
-#define VERBOSE_OPTION 0x10
-#define DUMP_OPTION0x20
-
-#define DEFAULT_OPTION (REGULATOR_OPTION | SENSOR_OPTION | \
-   CLOCK_OPTION | GPIO_OPTION)
-
 void usage(void)
 {
printf("Usage: powerdebug [OPTIONS]\n");
@@ -243,22 +232,22 @@ int main(int argc, char **argv)
return 1;
}
 
-   if ((options->flags & REGULATOR_OPTION) && regulator_init()) {
+   if (regulator_init(options)) {
printf("failed to initialize regulator\n");
options->flags &= ~REGULATOR_OPTION;
}
 
-   if ((options->flags & CLOCK_OPTION) && clock_init()) {
+   if (clock_init(options)) {
printf("failed to initialize clock details (check debugfs)\n");
options->flags &= ~CLOCK_OPTION;
}
 
-   if ((options->flags & SENSOR_OPTION) && sensor_init()) {
+   if (sensor_init(options)) {
printf("failed to initialize sensors\n");
options->flags &= SENSOR_OPTION;
}
 
-   if ((options->flags & GPIO_OPTION) && gpio_init()) {
+   if (gpio_init(options)) {
printf("failed to initialize gpios\n");
options->flags &= GPIO_OPTION;
}
diff --git a/powerdebug.h b/powerdebug.h
index 33463a0..2df0a27 100644
--- a/powerdebug.h
+++ b/powerdebug.h
@@ -21,6 +21,17 @@
 
 #define VERSION "0.7.3"
 
+#define REGULATOR_OPTION   0x01
+#define SENSOR_OPTION  0x02
+#define CLOCK_OPTION   0x04
+#define GPIO_OPTION0x08
+
+#define VERBOSE_OPTION 0x10
+#define DUMP_OPTION0x20
+
+#define DEFAULT_OPTION (REGULATOR_OPTION | SENSOR_OPTION | \
+   CLOCK_OPTION | GPIO_OPTION)
+
 struct powerdebug_options {
int flags;
unsigned int ticktime;
@@ -28,14 +39,15 @@ struct powerdebug_options {
char *clkname;
 };
 
-extern int clock_init(void);
+extern int clock_init(struct powerdebug_options *options);
 extern int clock_dump(char *clk);
 
-extern int regulator_init(void);
+extern int regulator_init(struct powerdebug_options *options);
 extern int regulator_dump(void);
 
-extern int gpio_init(void);
+extern int gpio_init(struct powerdebug_options *options);
 extern int gpio_dump(void);
 
+extern int sensor_init(struct powerdebug_options *options);
 extern int sensor_dump(void);
-extern int sensor_init(void);
+
diff --git a/regulator.c b/regulator.c
index b38ddab..549a744 100644
--- a/regulator.c
+++ b/regulator.c
@@ -265,10 +265,13 @@ static struct display_ops regulator_ops = {
.display = regulator_display,
 };
 
-int regulator_init(void)
+int regulator_init(struct powerdebug_options *options)
 {
int ret = 0;
 
+   if (!(options->flags & REGULATOR_OPTION))
+   return 0;
+
ret = display_register(REGULATOR, _ops);
if (ret)
printf("error: regulator display register failed");
diff --git a/sensor.c b/sensor.c
index 008b662..3b37bc6 100644
--- a/sensor.c
+++ b/sensor.c
@@ -295,10 +295,13 @@ static struct display_ops sensor_ops = {
.display = sensor_display,
 };
 
-int sensor_init(void)
+int sensor_init(struct powerdebug_options *options)
 {
   

[PATCH 5/6] Consolidate the headers into a single one.

[Daniel Lezcano]

Signed-off-by: Daniel Lezcano <daniel.lezc...@linaro.org>
---
 clocks.c |  1 -
 clocks.h | 23 ---
 display.c|  1 -
 gpio.h   | 23 ---
 powerdebug.c | 11 ---
 powerdebug.h | 19 +++
 regulator.c  |  3 ---
 regulator.h  | 23 ---
 sensor.c |  1 -
 sensor.h | 23 ---
 10 files changed, 19 insertions(+), 109 deletions(-)
 delete mode 100644 clocks.h
 delete mode 100644 gpio.h
 delete mode 100644 regulator.h
 delete mode 100644 sensor.h

diff --git a/clocks.c b/clocks.c
index 4e34e0c..c751ec1 100644
--- a/clocks.c
+++ b/clocks.c
@@ -34,7 +34,6 @@
 
 #include "powerdebug.h"
 #include "display.h"
-#include "clocks.h"
 #include "tree.h"
 #include "utils.h"
 
diff --git a/clocks.h b/clocks.h
deleted file mode 100644
index b205a28..000
--- a/clocks.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/*
- * Power debug tool (powerdebug)
- *
- * Copyright (C) 2016, Linaro Limited.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version 2
- * of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, 
USA.
- *
- */
-
-extern int clock_init(void);
-extern int clock_dump(char *clk);
diff --git a/display.c b/display.c
index 8aafc32..ed010b3 100644
--- a/display.c
+++ b/display.c
@@ -28,7 +28,6 @@
 #include 
 #include "powerdebug.h"
 #include "mainloop.h"
-#include "regulator.h"
 #include "display.h"
 
 enum { PT_COLOR_DEFAULT = 1,
diff --git a/gpio.h b/gpio.h
deleted file mode 100644
index 9f51386..000
--- a/gpio.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/*
- * Power debug tool (powerdebug)
- *
- * Copyright (C) 2016, Linaro Limited.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version 2
- * of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, 
USA.
- *
- */
-
-extern int gpio_init(void);
-extern int gpio_dump(void);
diff --git a/powerdebug.c b/powerdebug.c
index d4ce1f6..0f5c98a 100644
--- a/powerdebug.c
+++ b/powerdebug.c
@@ -27,11 +27,7 @@
 #include 
 #include 
 #include 
-#include "regulator.h"
 #include "display.h"
-#include "clocks.h"
-#include "sensor.h"
-#include "gpio.h"
 #include "mainloop.h"
 #include "powerdebug.h"
 
@@ -102,13 +98,6 @@ static struct option long_options[] = {
{ 0, 0, 0, 0 }
 };
 
-struct powerdebug_options {
-   int flags;
-   unsigned int ticktime;
-   int selectedwindow;
-   char *clkname;
-};
-
 int getoptions(int argc, char *argv[], struct powerdebug_options *options)
 {
int c;
diff --git a/powerdebug.h b/powerdebug.h
index 3603018..33463a0 100644
--- a/powerdebug.h
+++ b/powerdebug.h
@@ -20,3 +20,22 @@
  */
 
 #define VERSION "0.7.3"
+
+struct powerdebug_options {
+   int flags;
+   unsigned int ticktime;
+   int selectedwindow;
+   char *clkname;
+};
+
+extern int clock_init(void);
+extern int clock_dump(char *clk);
+
+extern int regulator_init(void);
+extern int regulator_dump(void);
+
+extern int gpio_init(void);
+extern int gpio_dump(void);
+
+extern int sensor_dump(void);
+extern int sensor_init(void);
diff --git a/regulator.c b/regulator.c
index dc29769..b38ddab 100644
--- a/regulator.c
+++ b/regulator.c
@@ -20,9 +20,6 @@
  * License along with this library; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
-
-#include "regulator.h"
-
 #define SYSFS_REGULATOR "/sys/class/regulator"
 #define VALUE_MAX 16
 
diff --git a/regulator.h b/regulator.h
deleted file mode 100644
index b4b4f78..000
--- a/regulator.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/*
- * Power debug

[PATCH 4/6] Fix license. Linaro switched years ago to GPL.

[Daniel Lezcano]

Signed-off-by: Daniel Lezcano <daniel.lezc...@linaro.org>
---
 LICENSE  | 567 ---
 clocks.c |  28 +--
 clocks.h |  28 +--
 display.c|  28 +--
 display.h|  28 +--
 gpio.c   |  28 +--
 gpio.h   |  28 +--
 mainloop.c   |  26 +--
 mainloop.h   |  26 +--
 powerdebug.c |  28 +--
 powerdebug.h |  28 +--
 regulator.c  |  32 ++--
 regulator.h  |  28 +--
 sensor.c |  28 +--
 sensor.h |  28 +--
 tree.c   |  26 +--
 tree.h   |  26 +--
 utils.c  |  28 +--
 utils.h  |  28 +--
 19 files changed, 649 insertions(+), 418 deletions(-)

diff --git a/LICENSE b/LICENSE
index be6f09f..6f50a71 100644
--- a/LICENSE
+++ b/LICENSE
@@ -1,221 +1,348 @@
-Copyright (C) 2010 Linaro Ltd.
-
-License: EPL-1.0
- Eclipse Public License - v 1.0
- 
- THE ACCOMPANYING PROGRAM IS PROVIDED UNDER THE TERMS OF THIS ECLIPSE
- PUBLIC LICENSE ("AGREEMENT"). ANY USE, REPRODUCTION OR DISTRIBUTION OF THE
- PROGRAM CONSTITUTES RECIPIENT'S ACCEPTANCE OF THIS AGREEMENT.
- 
- 1. DEFINITIONS
- 
- "Contribution" means:
- 
- a) in the case of the initial Contributor, the initial code and documentation
- distributed under this Agreement, and
- b) in the case of each subsequent Contributor:
- 
- i) changes to the Program, and
- 
- ii) additions to the Program;
- 
- where such changes and/or additions to the Program originate from and are
- distributed by that particular Contributor. A Contribution 'originates'
- from a Contributor if it was added to the Program by such Contributor itself
- or anyone acting on such Contributor's behalf. Contributions do not include
- additions to the Program which: (i) are separate modules of software
- distributed in conjunction with the Program under their own license
- agreement, and (ii) are not derivative works of the Program.
- 
- "Contributor" means any person or entity that distributes the Program.
- 
- "Licensed Patents " mean patent claims licensable by a Contributor which are
- necessarily infringed by the use or sale of its Contribution alone or when
- combined with the Program.
- 
- "Program" means the Contributions distributed in accordance with this
- Agreement.
- 
- "Recipient" means anyone who receives the Program under this Agreement,
- including all Contributors.
- 
- 2. GRANT OF RIGHTS
- 
- a) Subject to the terms of this Agreement, each Contributor hereby grants
- Recipient a non-exclusive, worldwide, royalty-free copyright license to
- reproduce, prepare derivative works of, publicly display, publicly perform,
- distribute and sublicense the Contribution of such Contributor, if any,
- and such derivative works, in source code and object code form.
- 
- b) Subject to the terms of this Agreement, each Contributor hereby grants
- Recipient a non-exclusive, worldwide, royalty-free patent license under
- Licensed Patents to make, use, sell, offer to sell, import and otherwise
- transfer the Contribution of such Contributor, if any, in source code and
- object code form. This patent license shall apply to the combination of
- the Contribution and the Program if, at the time the Contribution is added
- by the Contributor, such addition of the Contribution causes such
- combination to be covered by the Licensed Patents. The patent license shall
- not apply to any other combinations which include the Contribution. No
- hardware per se is licensed hereunder.
- 
- c) Recipient understands that although each Contributor grants the licenses
- to its Contributions set forth herein, no assurances are provided by any
- Contributor that the Program does not infringe the patent or other
- intellectual property rights of any other entity. Each Contributor disclaims
- any liability to Recipient for claims brought by any other entity based on
- infringement of intellectual property rights or otherwise. As a condition to
- exercising the rights and licenses granted hereunder, each Recipient hereby
- assumes sole responsibility to secure any other intellectual property rights
- needed, if any. For example, if a third party patent license is required to
- allow Recipient to distribute the Program, it is Recipient's responsibility
- to acquire that license before distributing the Program.
- 
- d) Each Contributor represents that to its knowledge it has sufficient
- copyright rights in its Contribution, if any, to grant the copyright license
- set forth in this Agreement.
- 
- 3. REQUIREMENTS
- 
- A Contributor may choose to distribute the Program in object code form under
- its own license agreement, provided that:
- 
- a) it complies with the terms and conditions of this Agreement; and
- 
- b) its license agreement:
- 
- i) effectively disclaims on behalf of all Contributors all warranties and
- conditions, express and implied, including warranties or conditions of title
- and non-infringement, and implied warranties or conditions of merchantability
- a

[PATCH 2/6] Fix compilation warnings.

[Daniel Lezcano]

Signed-off-by: Daniel Lezcano <daniel.lezc...@linaro.org>
---
 display.c |  7 ++-
 gpio.c| 11 ++-
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/display.c b/display.c
index ee9..1114366 100644
--- a/display.c
+++ b/display.c
@@ -198,13 +198,13 @@ static int display_prev_panel(void)
 
 static int display_next_line(void)
 {
-   int maxx, maxy;
+   int maxy;
int cursor = windata[current_win].cursor;
int nrdata = windata[current_win].nrdata;
int scrolling = windata[current_win].scrolling;
struct rowdata *rowdata = windata[current_win].rowdata;
 
-   getmaxyx(stdscr, maxy, maxx);
+   maxy = getmaxy(stdscr);
 
if (cursor >= nrdata)
return cursor;
@@ -320,9 +320,6 @@ struct find_data *display_find_init(void)
const size_t len = 64;
regex_t *reg;
char *search4;
-   int maxx, maxy;
-
-   getmaxyx(stdscr, maxy, maxx);
 
reg = malloc(sizeof(*reg));
if (!reg)
diff --git a/gpio.c b/gpio.c
index 3c02b8e..7b5bbb2 100644
--- a/gpio.c
+++ b/gpio.c
@@ -325,12 +325,13 @@ void export_free_gpios(void)
FILE *fgpio, *fgpio_export;
int i, gpio_max = 0;
char *line = NULL;
-   ssize_t read, len = 0;
+   ssize_t read;
+   size_t len = 0;
 
fgpio = fopen("/sys/kernel/debug/gpio", "r");
if (!fgpio) {
printf("failed to read debugfs gpio file\n");
-   goto out;
+   return;
}
 
fgpio_export = fopen("/sys/class/gpio/export", "w");
@@ -342,7 +343,7 @@ void export_free_gpios(void)
/* export the gpios */
while ((read = getline(, , fgpio)) != -1) {
if (strstr(line, "GPIOs"))
-   sscanf(line, "%*[^-]-%d%*", _max);
+   sscanf(line, "%*[^-]-%d", _max);
}
 
printf("log: total gpios = %d\n", gpio_max);
@@ -355,10 +356,10 @@ void export_free_gpios(void)
}
 
free(line);
-out:
+
if (fgpio)
fclose(fgpio);
-
+out:
if (fgpio_export)
fclose(fgpio_export);
 
-- 
1.9.1

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 3/6] Prevent to init a subsystem if it is not selected.

[Daniel Lezcano]

Signed-off-by: Daniel Lezcano <daniel.lezc...@linaro.org>
---
 powerdebug.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/powerdebug.c b/powerdebug.c
index 555beea..bc8fc92 100644
--- a/powerdebug.c
+++ b/powerdebug.c
@@ -208,13 +208,12 @@ static struct powerdebug_options *powerdebug_init(void)
 {
struct powerdebug_options *options;
 
+   signal(SIGWINCH, sigwinch_handler);
+
options = malloc(sizeof(*options));
if (!options)
return NULL;
 
-   memset(options, 0, sizeof(*options));
-   signal(SIGWINCH, sigwinch_handler);
-
return options;
 }
 
@@ -249,22 +248,22 @@ int main(int argc, char **argv)
return 1;
}
 
-   if (regulator_init()) {
+   if ((options->flags & REGULATOR_OPTION) && regulator_init()) {
printf("failed to initialize regulator\n");
options->flags &= ~REGULATOR_OPTION;
}
 
-   if (clock_init()) {
+   if ((options->flags & CLOCK_OPTION) && clock_init()) {
printf("failed to initialize clock details (check debugfs)\n");
options->flags &= ~CLOCK_OPTION;
}
 
-   if (sensor_init()) {
+   if ((options->flags & SENSOR_OPTION) && sensor_init()) {
printf("failed to initialize sensors\n");
options->flags &= SENSOR_OPTION;
}
 
-   if (gpio_init()) {
+   if ((options->flags & GPIO_OPTION) && gpio_init()) {
printf("failed to initialize gpios\n");
options->flags &= GPIO_OPTION;
}
-- 
1.9.1

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 1/6] Change options to flag approach.

[Daniel Lezcano]

From: Daniel Lezcano <daniel.lezc...@free.fr>

That has the benefit to simplify the code.

Signed-off-by: Daniel Lezcano <daniel.lezc...@linaro.org>
---
 powerdebug.c | 65 +++-
 1 file changed, 34 insertions(+), 31 deletions(-)

diff --git a/powerdebug.c b/powerdebug.c
index 6cf3a1b..555beea 100644
--- a/powerdebug.c
+++ b/powerdebug.c
@@ -31,6 +31,17 @@
 
 extern void sigwinch_handler(int);
 
+#define REGULATOR_OPTION   0x01
+#define SENSOR_OPTION  0x02
+#define CLOCK_OPTION   0x04
+#define GPIO_OPTION0x08
+
+#define VERBOSE_OPTION 0x10
+#define DUMP_OPTION0x20
+
+#define DEFAULT_OPTION (REGULATOR_OPTION | SENSOR_OPTION | \
+   CLOCK_OPTION | GPIO_OPTION)
+
 void usage(void)
 {
printf("Usage: powerdebug [OPTIONS]\n");
@@ -86,12 +97,7 @@ static struct option long_options[] = {
 };
 
 struct powerdebug_options {
-   bool verbose;
-   bool regulators;
-   bool sensors;
-   bool clocks;
-   bool gpios;
-   bool dump;
+   int flags;
unsigned int ticktime;
int selectedwindow;
char *clkname;
@@ -115,39 +121,38 @@ int getoptions(int argc, char *argv[], struct 
powerdebug_options *options)
 
switch (c) {
case 'r':
-   options->regulators = true;
+   options->flags |= REGULATOR_OPTION;
options->selectedwindow = REGULATOR;
break;
case 's':
-   options->sensors = true;
+   options->flags |= SENSOR_OPTION;
options->selectedwindow = SENSOR;
break;
case 'c':
-   options->clocks = true;
+   options->flags |= CLOCK_OPTION;
options->selectedwindow = CLOCK;
break;
case 'g':
-   options->gpios = true;
+   options->flags |= GPIO_OPTION;
options->selectedwindow = GPIO;
break;
+   case 'd':
+   options->flags |= DUMP_OPTION;
+   break;
+   case 'v':
+   options->flags |= VERBOSE_OPTION;
+   break;
case 'p':
options->clkname = strdup(optarg);
if (!options->clkname) {
fprintf(stderr, "failed to allocate memory");
return -1;
}
-   options->dump = true;   /* Assume -dc in case of -p */
-   options->clocks = true;
+   options->flags |= (DUMP_OPTION | CLOCK_OPTION);
break;
case 't':
options->ticktime = atoi(optarg);
break;
-   case 'd':
-   options->dump = true;
-   break;
-   case 'v':
-   options->verbose = true;
-   break;
case 'V':
version();
break;
@@ -160,10 +165,8 @@ int getoptions(int argc, char *argv[], struct 
powerdebug_options *options)
}
 
/* No system specified to be dump, let's default to all */
-   if (!options->regulators && !options->clocks &&
-   !options->sensors && !options->gpios)
-   options->regulators = options->clocks =
-   options->sensors = options->gpios = true;
+   if (!(options->flags & DEFAULT_OPTION))
+   options->flags |= DEFAULT_OPTION;
 
if (options->selectedwindow == -1)
options->selectedwindow = CLOCK;
@@ -173,16 +176,16 @@ int getoptions(int argc, char *argv[], struct 
powerdebug_options *options)
 
 static int powerdebug_dump(struct powerdebug_options *options)
 {
-   if (options->regulators)
+   if (options->flags & REGULATOR_OPTION)
regulator_dump();
 
-   if (options->clocks)
+   if (options->flags & CLOCK_OPTION)
clock_dump(options->clkname);
 
-   if (options->sensors)
+   if (options->flags & SENSOR_OPTION)
sensor_dump();
 
-   if (options->gpios)
+   if (options->flags & GPIO_OPTION)
gpio_dump();
 
return 0;
@@ -248,25 +251,25 @@ int main(int argc, char **argv)
 
if (regulator_init()) {
printf("failed to initialize regulator\n");
-   options->regulators = f

Re: [PATCH v3] idlestat: Allocate struct cpufreq_pstate dynamically

[Daniel Lezcano]

On 10/04/2014 06:33 AM, pi-cheng.chen wrote:

Initialize struct cpufreq_pstates with initial P-state of CPUs and allocate
struct cpufreq_pstate dynamically when parsing trace file to solve the issue
caused by missing scaling_avaialable_freqs attr when using intel_pstate
driver.

Changes v2 to v3:
Initialize struct cpufreq_pstates with initial P-state of all CPUs and
beginning timestamp before trace acquisition and close all P-states with
ending timestamp

hanges v1 to v2:
Sort the cpufreq_pstate list when parsing events

Signed-off-by: Pi-Cheng Chen pi-cheng.c...@linaro.org


At the first glance it looks ok for me.

Thanks
  -- Daniel


---
  idlestat.c | 259 -
  idlestat.h |   7 ++
  trace.h|   1 +
  3 files changed, 195 insertions(+), 72 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index da615cb..8230067 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -536,6 +536,121 @@ static struct cpuidle_cstates *build_cstate_info(int 
nrcpus)
return cstates;
  }

+#define TRACE_STAT_FORMAT %*[^:]:%lf
+
+static double get_trace_ts(void)
+{
+   FILE *f;
+   double ts;
+
+   f = fopen(TRACE_STAT_FILE, r);
+   if (!f)
+   return -1;
+
+   while (fgets(buffer, BUFSIZE, f)) {
+   if (!strstr(buffer, now ts))
+   continue;
+   if (!sscanf(buffer, TRACE_STAT_FORMAT, ts))
+   ts = -1;
+   break;
+   }
+   fclose(f);
+
+   return ts;
+}
+
+static void release_init_pstates(struct init_pstates *init)
+{
+   free(init-freqs);
+   free(init);
+}
+
+static struct init_pstates *build_init_pstates(void)
+{
+   struct init_pstates *init;
+   int nr_cpus, cpu;
+   unsigned int *freqs;
+
+   nr_cpus = sysconf(_SC_NPROCESSORS_CONF);
+   if (nr_cpus  0)
+   return NULL;
+
+   init = malloc(sizeof(*init));
+   if (!init)
+   return NULL;
+
+   freqs = calloc(nr_cpus, sizeof(*freqs));
+   if (!freqs) {
+   free(init);
+   return NULL;
+   }
+   memset(freqs, 0, sizeof(*freqs) * nr_cpus);
+
+   for (cpu = 0; cpu  nr_cpus; cpu++) {
+   char *fpath;
+   unsigned int *freq = (freqs[cpu]);
+
+   if (asprintf(fpath, CPUFREQ_CURFREQ_PATH_FORMAT, cpu)  0) {
+   release_init_pstates(init);
+   return NULL;
+   }
+   if (read_int(fpath, (int *)freq))
+   *freq = 0;
+   free(fpath);
+   }
+   init-nr_cpus = nr_cpus;
+   init-freqs = freqs;
+
+   return init;
+}
+
+/**
+ * alloc_pstate - allocate, sort, and initialize pstate struct
+ * to maintain statistics of P-state transitions
+ * @pstates: per-CPU P-state statistics struct
+ * @freq: frequency for which the newly pstate is allocated
+ *
+ * Return: the index of the newly allocated pstate struct
+ */
+static int alloc_pstate(struct cpufreq_pstates *pstates, unsigned int freq)
+{
+   struct cpufreq_pstate *pstate, *tmp;
+   int nrfreq, i, next = 0;
+
+   pstate = pstates-pstate;
+   nrfreq = pstates-max;
+
+   tmp = realloc(pstate, sizeof(*pstate) * (nrfreq + 1));
+   if (!tmp) {
+   perror(realloc pstate);
+   return -1;
+   }
+   pstate = tmp;
+   pstates-pstate = tmp;
+   pstates-max = nrfreq + 1;
+
+   for (i = 0; i  nrfreq  freq = pstate[i].freq; i++)
+   ;
+
+   next = i;
+   for (i = nrfreq; i  next  i  0; i--) {
+   pstate[i] = pstate[i - 1];
+   pstate[i].id = i;
+   pstates-current = (pstates-current == i - 1)?
+   i : pstates-current;
+   }
+
+   pstate[next].id = next;
+   pstate[next].freq = freq;
+   pstate[next].count = 0;
+   pstate[next].min_time = DBL_MAX;
+   pstate[next].max_time = 0;
+   pstate[next].avg_time = 0;
+   pstate[next].duration = 0;
+
+   return next;
+}
+
  /**
   * release_pstate_info - free all P-state related structs
   * @pstates: per-cpu array of P-state statistics structs
@@ -560,14 +675,16 @@ static void release_pstate_info(struct cpufreq_pstates 
*pstates, int nrcpus)
return;
  }

-/**
- * build_pstate_info - parse cpufreq sysfs entries and build per-CPU
- * structs to maintain statistics of P-state transitions
+/* build_pstate_info - allocate and initialize per-CPU structs to
+ * maintain statistics of P-state transitions
   * @nrcpus: number of CPUs
+ * @initp: initial P-state of CPUs before trace acquistion
+ * @start_ts: timestamp when trace acquisition started
   *
   * Return: per-CPU array of structs (success) or NULL (error)
   */
-static struct cpufreq_pstates *build_pstate_info(int nrcpus)
+static struct cpufreq_pstates *build_pstate_info(int nrcpus,
+   struct init_pstates *initp, double 

Re: [PATCH v3] idlestat: Allocate struct cpufreq_pstate dynamically

[Daniel Lezcano]

On 10/21/2014 03:47 PM, Tuukka Tikkanen wrote:

On Tue, Oct 21, 2014 at 3:42 PM, Daniel Lezcano
daniel.lezc...@linaro.org wrote:

On 10/04/2014 06:33 AM, pi-cheng.chen wrote:


Initialize struct cpufreq_pstates with initial P-state of CPUs and
allocate
struct cpufreq_pstate dynamically when parsing trace file to solve the
issue
caused by missing scaling_avaialable_freqs attr when using intel_pstate
driver.

Changes v2 to v3:
Initialize struct cpufreq_pstates with initial P-state of all CPUs and
beginning timestamp before trace acquisition and close all P-states with
ending timestamp

hanges v1 to v2:
Sort the cpufreq_pstate list when parsing events

Signed-off-by: Pi-Cheng Chen pi-cheng.c...@linaro.org



At the first glance it looks ok for me.

Thanks
   -- Daniel


Good enough for a reviewed-by with your name?


Good enough for an Acked-by: Daniel Lezcano daniel.lezc...@linaro.org



Tuukka





---
   idlestat.c | 259
-
   idlestat.h |   7 ++
   trace.h|   1 +
   3 files changed, 195 insertions(+), 72 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index da615cb..8230067 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -536,6 +536,121 @@ static struct cpuidle_cstates *build_cstate_info(int
nrcpus)
 return cstates;
   }

+#define TRACE_STAT_FORMAT %*[^:]:%lf
+
+static double get_trace_ts(void)
+{
+   FILE *f;
+   double ts;
+
+   f = fopen(TRACE_STAT_FILE, r);
+   if (!f)
+   return -1;
+
+   while (fgets(buffer, BUFSIZE, f)) {
+   if (!strstr(buffer, now ts))
+   continue;
+   if (!sscanf(buffer, TRACE_STAT_FORMAT, ts))
+   ts = -1;
+   break;
+   }
+   fclose(f);
+
+   return ts;
+}
+
+static void release_init_pstates(struct init_pstates *init)
+{
+   free(init-freqs);
+   free(init);
+}
+
+static struct init_pstates *build_init_pstates(void)
+{
+   struct init_pstates *init;
+   int nr_cpus, cpu;
+   unsigned int *freqs;
+
+   nr_cpus = sysconf(_SC_NPROCESSORS_CONF);
+   if (nr_cpus  0)
+   return NULL;
+
+   init = malloc(sizeof(*init));
+   if (!init)
+   return NULL;
+
+   freqs = calloc(nr_cpus, sizeof(*freqs));
+   if (!freqs) {
+   free(init);
+   return NULL;
+   }
+   memset(freqs, 0, sizeof(*freqs) * nr_cpus);
+
+   for (cpu = 0; cpu  nr_cpus; cpu++) {
+   char *fpath;
+   unsigned int *freq = (freqs[cpu]);
+
+   if (asprintf(fpath, CPUFREQ_CURFREQ_PATH_FORMAT, cpu) 
0) {
+   release_init_pstates(init);
+   return NULL;
+   }
+   if (read_int(fpath, (int *)freq))
+   *freq = 0;
+   free(fpath);
+   }
+   init-nr_cpus = nr_cpus;
+   init-freqs = freqs;
+
+   return init;
+}
+
+/**
+ * alloc_pstate - allocate, sort, and initialize pstate struct
+ * to maintain statistics of P-state transitions
+ * @pstates: per-CPU P-state statistics struct
+ * @freq: frequency for which the newly pstate is allocated
+ *
+ * Return: the index of the newly allocated pstate struct
+ */
+static int alloc_pstate(struct cpufreq_pstates *pstates, unsigned int
freq)
+{
+   struct cpufreq_pstate *pstate, *tmp;
+   int nrfreq, i, next = 0;
+
+   pstate = pstates-pstate;
+   nrfreq = pstates-max;
+
+   tmp = realloc(pstate, sizeof(*pstate) * (nrfreq + 1));
+   if (!tmp) {
+   perror(realloc pstate);
+   return -1;
+   }
+   pstate = tmp;
+   pstates-pstate = tmp;
+   pstates-max = nrfreq + 1;
+
+   for (i = 0; i  nrfreq  freq = pstate[i].freq; i++)
+   ;
+
+   next = i;
+   for (i = nrfreq; i  next  i  0; i--) {
+   pstate[i] = pstate[i - 1];
+   pstate[i].id = i;
+   pstates-current = (pstates-current == i - 1)?
+   i : pstates-current;
+   }
+
+   pstate[next].id = next;
+   pstate[next].freq = freq;
+   pstate[next].count = 0;
+   pstate[next].min_time = DBL_MAX;
+   pstate[next].max_time = 0;
+   pstate[next].avg_time = 0;
+   pstate[next].duration = 0;
+
+   return next;
+}
+
   /**
* release_pstate_info - free all P-state related structs
* @pstates: per-cpu array of P-state statistics structs
@@ -560,14 +675,16 @@ static void release_pstate_info(struct
cpufreq_pstates *pstates, int nrcpus)
 return;
   }

-/**
- * build_pstate_info - parse cpufreq sysfs entries and build per-CPU
- * structs to maintain statistics of P-state transitions
+/* build_pstate_info - allocate and initialize per-CPU structs to
+ * maintain statistics of P-state transitions
* @nrcpus: number of CPUs
+ * @initp: initial P-state of CPUs before trace acquistion
+ * @start_ts: timestamp when trace

Re: [PATCH v2] idlestat: Allocate struct cpufreq_pstate dynamically

[Daniel Lezcano]

On 09/10/2014 09:33 AM, pi-cheng.chen wrote:

When using intel_pstate driver, scaling_available_freqs attr is not
exported to sysfs. It causes assertion of idlestat due to memory of
struct cpufreq_pstate was not allocated.

Allocate struct cpufreq_pstate dynamically when getting frequency
information from trace file instead of parsing available frequencies
from sysfs

Changes v1 to v2:
Sort the cpufreq_pstate list when parsing events

Signed-off-by: Pi-Cheng Chen pi-cheng.c...@linaro.org


Tested on intel_pstate and legacy cpufreq driver.

In complement of this feature, I think the cpus cpufreq must be 
initialized with the 'cpuinfo_cur_freq' with the start time initialized 
to the beginning of the acquisition. If a cpu is in a specific freq 
without any changes (understand: ftrace transitions which trigger the 
p-state creation), we will have a long freq residency (the initial one).


Today we don't have this, so the cpus without freq changes are not 
displayed.


At least the minimal number of p-state should be '1' not '0' (except if 
cpufreq is disabled).


 -- Daniel


---
  idlestat.c | 118 +
  1 file changed, 56 insertions(+), 62 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index da615cb..d03f12e 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -561,8 +561,8 @@ static void release_pstate_info(struct cpufreq_pstates 
*pstates, int nrcpus)
  }

  /**
- * build_pstate_info - parse cpufreq sysfs entries and build per-CPU
- * structs to maintain statistics of P-state transitions
+ * build_pstate_info - allocate and initialize per-CPU structs to
+ * maintain statistics of P-state transitions
   * @nrcpus: number of CPUs
   *
   * Return: per-CPU array of structs (success) or NULL (error)
@@ -578,55 +578,8 @@ static struct cpufreq_pstates *build_pstate_info(int 
nrcpus)
memset(pstates, 0, sizeof(*pstates) * nrcpus);

for (cpu = 0; cpu  nrcpus; cpu++) {
-   struct cpufreq_pstate *pstate;
-   int nrfreq;
-   char *fpath, *freq, line[256];
-   FILE *sc_av_freq;
-
-   if (asprintf(fpath, CPUFREQ_AVFREQ_PATH_FORMAT, cpu)  0)
-   goto clean_exit;
-
-   /* read scaling_available_frequencies for the CPU */
-   sc_av_freq = fopen(fpath, r);
-   free(fpath);
-   if (!sc_av_freq) {
-   fprintf(stderr, warning: P-states not supported for 
-   CPU%d\n, cpu);
-   continue;
-   }
-   freq = fgets(line, sizeof(line)/sizeof(line[0]), sc_av_freq);
-   fclose(sc_av_freq);
-   if (!freq) {
-   /* unlikely to be here, but just in case... */
-   fprintf(stderr, warning: P-state info not found for 
-   CPU%d\n, cpu);
-   continue;
-   }
-
-   /* tokenize line and populate each frequency */
-   nrfreq = 0;
-   pstate = NULL;
-   while ((freq = strtok(freq, \n )) != NULL) {
-   struct cpufreq_pstate *tmp = realloc(pstate, 
sizeof(*pstate) * (nrfreq+1));
-   if (!tmp)
-   goto clean_exit;
-   pstate = tmp;
-
-   /* initialize pstate record */
-   pstate[nrfreq].id = nrfreq;
-   pstate[nrfreq].freq = atol(freq);
-   pstate[nrfreq].count = 0;
-   pstate[nrfreq].min_time = DBL_MAX;
-   pstate[nrfreq].max_time = 0.;
-   pstate[nrfreq].avg_time = 0.;
-   pstate[nrfreq].duration = 0.;
-   nrfreq++;
-   freq = NULL;
-   }
-
-   /* now populate cpufreq_pstates for this CPU */
-   pstates[cpu].pstate = pstate;
-   pstates[cpu].max = nrfreq;
+   pstates[cpu].pstate = NULL;
+   pstates[cpu].max = 0;
pstates[cpu].current = -1;  /* unknown */
pstates[cpu].idle = -1; /* unknown */
pstates[cpu].time_enter = 0.;
@@ -634,10 +587,52 @@ static struct cpufreq_pstates *build_pstate_info(int 
nrcpus)
}

return pstates;
+}

-clean_exit:
-   release_pstate_info(pstates, nrcpus);
-   return NULL;
+/**
+ * alloc_pstate - allocate, sort, and initialize pstate struct
+ * to maintain statistics of P-state transitions
+ * @pstates: per-CPU P-state statistics struct
+ * @freq: frequency for which the newly pstate is allocated
+ *
+ * Return: the index of the newly allocated pstate struct
+ */
+static int alloc_pstate(struct cpufreq_pstates *pstates, unsigned int freq)
+{
+   struct cpufreq_pstate *pstate, *tmp;
+   int nrfreq, i, next = 0;
+
+   pstate = 

Re: [PATCH v2] idlestat: Allocate struct cpufreq_pstate dynamically

[Daniel Lezcano]

On 09/10/2014 03:08 PM, Pi-Cheng Chen wrote:

On 10 September 2014 17:15, Daniel Lezcano daniel.lezc...@linaro.org wrote:

On 09/10/2014 09:33 AM, pi-cheng.chen wrote:


When using intel_pstate driver, scaling_available_freqs attr is not
exported to sysfs. It causes assertion of idlestat due to memory of
struct cpufreq_pstate was not allocated.

Allocate struct cpufreq_pstate dynamically when getting frequency
information from trace file instead of parsing available frequencies
from sysfs

Changes v1 to v2:
Sort the cpufreq_pstate list when parsing events

Signed-off-by: Pi-Cheng Chen pi-cheng.c...@linaro.org



Tested on intel_pstate and legacy cpufreq driver.

In complement of this feature, I think the cpus cpufreq must be initialized
with the 'cpuinfo_cur_freq' with the start time initialized to the beginning
of the acquisition. If a cpu is in a specific freq without any changes
(understand: ftrace transitions which trigger the p-state creation), we will
have a long freq residency (the initial one).

Today we don't have this, so the cpus without freq changes are not
displayed.

At least the minimal number of p-state should be '1' not '0' (except if
cpufreq is disabled).

  -- Daniel


How about to create some fake records manually in the trace file?
Get p-states of all CPUs first before starting tracing and write them
as fake records of p-state transitions before storing the traces from
sysfs to trace file. So that the report will be consistent with that
generated by import mode.


Well I think initializing directly the p-state structure with one 
element at init time will be simpler and cleaner. For the C-state it is 
not possible because we don't know idle state the cpu is, so we have to 
force a wakeup on all cpus but for the p-state this info is available in 
the sysfs.



--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH idlestat] Add -o option to save output report to a file

[Daniel Lezcano]

On 08/19/2014 09:22 AM, pi-cheng.chen wrote:

Currently the serial terminal connected to the boards running idlestat are
restricted to be at least 80 characters wide to output the report. Otherwise
idlestat quits with message The terminal must be at least 80 columns wide.

Fix it by adding a -o option to save report output to a file.


Yes, but please do it in the unix way.

1. open file
2. close stdout
3. dup[2] opened file to stdout (fd 1)
4. close opened file

So no need to change all the code around.

Thanks
  -- Daniel


Signed-off-by: Pi-Cheng Chen pi-cheng.c...@linaro.org
---
  idlestat.c | 136 -
  idlestat.h |   1 +
  2 files changed, 90 insertions(+), 47 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index bba8951..fb0ae8f 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -51,6 +51,7 @@
  #define USEC_PER_SEC 100

  static char buffer[BUFSIZE];
+static FILE *output;

  static inline int error(const char *str)
  {
@@ -68,19 +69,40 @@ static void charrep(char c, int count)
  {
int i;
for (i = 0; i  count; i++)
-   printf(%c, c);
+   fprintf(output, %c, c);
+}
+
+static int open_report_file(const char *path)
+{
+   if (path) {
+   output = fopen(path, w+);
+
+   if (!output) {
+   fprintf(stderr, %s: failed to open '%s'\n, __func__, 
path);
+   return -1;
+   }
+   } else
+   output = stdout;
+
+   return 0;
+}
+
+static void close_report_file(void)
+{
+   if (output != stdout)
+   fclose(output);
  }

  static void display_cpu_header(char *cpu, int length)
  {
charrep('-', length);
-   printf(\n);
+   fprintf(output, \n);

if (strstr(cpu, cluster))
-   printf(| %-*s |\n, length - 4, cpu);
+   fprintf(output, | %-*s |\n, length - 4, cpu);
else if (strstr(cpu, core))
-   printf(|  %-*s |\n, length - 9, cpu);
-   else printf(| %-*s |\n, length - 16, cpu);
+   fprintf(output, |  %-*s |\n, length - 9, cpu);
+   else fprintf(output, | %-*s |\n, length - 16, cpu);
  }

  static void display_factored_time(double time, int align)
@@ -89,15 +111,15 @@ static void display_factored_time(double time, int align)

if (time  1000) {
sprintf(buffer, %.0lfus, time);
-   printf(%*s, align, buffer);
+   fprintf(output, %*s, align, buffer);
}
else if (time  100) {
sprintf(buffer, %.2lfms, time / 1000.0);
-   printf(%*s, align, buffer);
+   fprintf(output, %*s, align, buffer);
}
else {
sprintf(buffer, %.2lfs, time / 100.0);
-   printf(%*s, align, buffer);
+   fprintf(output, %*s, align, buffer);
}
  }

@@ -107,28 +129,29 @@ static void display_factored_freq(int freq, int align)

if (freq  1000) {
sprintf(buffer, %dHz, freq);
-   printf(%*s, align, buffer);
+   fprintf(output, %*s, align, buffer);
} else if (freq  100) {
sprintf(buffer, %.2fMHz, (float)freq / 1000.0);
-   printf(%*s, align, buffer);
+   fprintf(output, %*s, align, buffer);
} else {
sprintf(buffer, %.2fGHz, (float)freq / 100.0);
-   printf(%*s, align, buffer);
+   fprintf(output, %*s, align, buffer);
}
  }

  static void display_cstates_header(void)
  {
charrep('-', 80);
-   printf(\n);
+   fprintf(output, \n);

-   printf(| C-state  |   min|   max|   avg|   total  | hits  |  
over | under |\n);
+   fprintf(output, | C-state  |   min|   max|   avg|   total
+ | hits  |  over | under |\n);
  }

  static void display_cstates_footer(void)
  {
charrep('-', 80);
-   printf(\n\n);
+   fprintf(output, \n\n);
  }

  static int display_cstates(void *arg, char *cpu)
@@ -148,23 +171,23 @@ static int display_cstates(void *arg, char *cpu)
display_cpu_header(cpu, 80);
cpu_header = true;
charrep('-', 80);
-   printf(\n);
+   fprintf(output, \n);
}

-   printf(| %8s | , c-name);
+   fprintf(output, | %8s | , c-name);
display_factored_time(c-min_time == DBL_MAX ? 0. :
  c-min_time, 8);
-   printf( | );
+   fprintf(output,  | );
display_factored_time(c-max_time, 8);
-   printf( | );
+   fprintf(output,  | );
display_factored_time(c-avg_time, 8);
-   printf( | );
+   fprintf(output,  | );

Re: [GIT PULL] Create a workload generator tool

[Daniel Lezcano]

On 08/08/2014 07:27 AM, Amit Kucheria wrote:

[ ... ]


We also said that the name of the tool could be changed, any opinions
on this? Something like workload-gen or wload-gen ?


IMHO, the name change is just a nice to have and not critical to the
success of this project.


Workload Generator is the obvious choice of name but not very
Google-friendly. Can someone think of a suitable qualifier before it?
It'd stick with rt-app until then.


1. Poetic

Sisyphean workload generator ?

As a punishment for his trickery, King Sisyphus was made to endlessly 
roll a huge boulder up a steep hill. The maddening nature of the 
punishment was reserved for King Sisyphus due to his hubristic belief 
that his cleverness surpassed that of Zeus himself. Zeus accordingly 
displayed his own cleverness by enchanting the boulder into rolling away 
from King Sisyphus before he reached the top which ended up consigning 
Sisyphus to an eternity of useless efforts and unending frustration. 
Thus it came to pass that pointless or interminable activities are 
sometimes described as Sisyphean.



2. Optimistic

Flexible workload generator ?


3. Straightforward

Programmable workload generator ?






--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH] Check the terminal window size is greater or equal to 80 columns

[Daniel Lezcano]

All the output assumes the terminal width is 80 cols min, check the terminal
size in order to detect a smaller terminal and prevent to have a nested
output.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   24 
 1 file changed, 24 insertions(+)

diff --git a/idlestat.c b/idlestat.c
index a0ef2dd..245be9e 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -34,6 +34,7 @@
 #include sched.h
 #include string.h
 #include float.h
+#include sys/ioctl.h
 #include sys/time.h
 #include sys/types.h
 #include sys/resource.h
@@ -1380,6 +1381,23 @@ static int execute(int argc, char *argv[], char *const 
envp[],
return -1;
 }
 
+static int check_window_size(void)
+{
+   struct winsize winsize;
+   
+   /* Output is redirected */
+   if (!isatty(STDOUT_FILENO))
+   return 0;
+
+   /* Get terminal window size */
+   ioctl(STDOUT_FILENO, TIOCGWINSZ, winsize);
+
+   if (winsize.ws_col = 80)
+   return 0;
+
+   return -1;
+}
+
 int main(int argc, char *argv[], char *const envp[])
 {
struct cpuidle_datas *datas;
@@ -1397,6 +1415,12 @@ int main(int argc, char *argv[], char *const envp[])
return -1;
}
 
+   if (check_window_size()) {
+   fprintf(stderr, The terminal must be at least 
+   80 columns wide\n);
+   return -1;
+   }
+
/* init cpu topoinfo */
init_cpu_topo_info();
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH V2 idlestat 06/12] Remove dump and iterations options

[Daniel Lezcano]

These options are pointless and not interesting. Removing them.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   72 ++--
 idlestat.h |2 --
 topology.c |   12 +-
 topology.h |5 ++---
 4 files changed, 19 insertions(+), 72 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index 8138e35..3e5b4bc 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -68,34 +68,8 @@ static inline void *ptrerror(const char *str)
return NULL;
 }
 
-static int dump_states(struct cpuidle_cstates *cstates,
-  struct cpufreq_pstates *pstates,
-  int count, char *str)
-{
-   int j, k, kmax;
-   struct cpuidle_cstate *cstate;
-
-   for (j = 0; j  cstates-cstate_max + 1; j++) {
-   cstate = cstates-cstate[j];
-
-   kmax = count  0 ? MIN(count, cstate-nrdata) : cstate-nrdata;
-   for (k = 0; k  kmax; k++) {
-   printf(%lf: enter %s\n, cstate-data[k].begin,
-  cstate-name);
-   printf(%lf: exit %s\n, cstate-data[k].end,
-  cstate-name);
-   }
-
-   /* add a break */
-   printf(\n);
-   }
-
-   return 0;
-}
-
 static int display_states(struct cpuidle_cstates *cstates,
- struct cpufreq_pstates *pstates,
- int count, char *str)
+ struct cpufreq_pstates *pstates, char *str)
 {
int j;
 
@@ -171,9 +145,9 @@ static int display_states(struct cpuidle_cstates *cstates,
return 0;
 }
 
-int dump_all_data(struct cpuidle_datas *datas, int count,
-   int (*dump)(struct cpuidle_cstates *,
-   struct cpufreq_pstates *, int, char *))
+int dump_all_data(struct cpuidle_datas *datas,
+ int (*dump)(struct cpuidle_cstates *,
+ struct cpufreq_pstates *, char *))
 {
int i = 0, nrcpus = datas-nrcpus;
struct cpuidle_cstates *cstates;
@@ -188,7 +162,7 @@ int dump_all_data(struct cpuidle_datas *datas, int count,
else
sprintf(buffer, cpu%d, i);
 
-   dump(cstates, pstates, count, buffer);
+   dump(cstates, pstates, buffer);
 
i++;
 
@@ -1041,12 +1015,10 @@ static void help(const char *cmd)
 {
fprintf(stderr,
\nUsage:\nTrace mode: %s --trace -f|--trace-file filename
--t|--duration seconds 
-   [-z|--dump] [-i|--iterations number]\n,
+-t|--duration seconds ,
basename(cmd));
fprintf(stderr,
-   \nReporting mode: %s --import -f|--trace-file filename
-   [-z|--dump] [-i|--iterations number]\n,
+   \nReporting mode: %s --import -f|--trace-file filename,
basename(cmd));
fprintf(stderr,
\nExample:\n%s --trace -f /tmp/myoutput -t 30\n, 
basename(cmd));
@@ -1066,10 +1038,8 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
{ import,  no_argument,   options-mode, IMPORT },
{ trace-file,  required_argument, NULL, 'f' },
{ help,no_argument,   NULL, 'h' },
-   { iterations,  required_argument, NULL, 'i' },
{ duration,required_argument, NULL, 't' },
{ version, no_argument,   NULL, 'V' },
-   { dump,no_argument,   NULL, 'z' },
{ verbose, no_argument,   NULL, 'v' },
{ 0, 0, 0, 0 }
};
@@ -1083,7 +1053,7 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
 
int optindex = 0;
 
-   c = getopt_long(argc, argv, :df:hi:t:Vvz,
+   c = getopt_long(argc, argv, :df:ht:Vv,
long_options, optindex);
if (c == -1)
break;
@@ -1096,9 +1066,6 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
help(argv[0]);
exit(0);
break;
-   case 'i':
-   options-iterations = atoi(optarg);
-   break;
case 't':
options-duration = atoi(optarg);
break;
@@ -1106,9 +1073,6 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
version(argv[0]);
exit(0);
break;
-   case 'z':
-   options-dump = true;
-   break;
case 'v':
options-verbose++;
break;
@@ -1127,9 +1091,6 @@ int getoptions(int argc, char *argv[], struct

[PATCH V2 idlestat 05/12] Add incremental verbose option

[Daniel Lezcano]

This patch adds the '-v' option in the command line.

The level of verbosity is choosen by the number of time the -v occurs.

no -v option : verbosity 0
-v   : verbosity 1
-vv  : verbosity 2
-vvv : verbosity 3
etc ...

Note this option is not yet used in the code.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |6 +-
 idlestat.h |1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/idlestat.c b/idlestat.c
index abbb787..8138e35 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -1070,6 +1070,7 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
{ duration,required_argument, NULL, 't' },
{ version, no_argument,   NULL, 'V' },
{ dump,no_argument,   NULL, 'z' },
+   { verbose, no_argument,   NULL, 'v' },
{ 0, 0, 0, 0 }
};
int c;
@@ -1082,7 +1083,7 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
 
int optindex = 0;
 
-   c = getopt_long(argc, argv, :df:hi:t:Vz,
+   c = getopt_long(argc, argv, :df:hi:t:Vvz,
long_options, optindex);
if (c == -1)
break;
@@ -1108,6 +1109,9 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
case 'z':
options-dump = true;
break;
+   case 'v':
+   options-verbose++;
+   break;
case 0: /* getopt_long() set a variable, just keep going */
break;
case ':':   /* missing option argument */
diff --git a/idlestat.h b/idlestat.h
index 3317a8f..f874bb8 100644
--- a/idlestat.h
+++ b/idlestat.h
@@ -133,6 +133,7 @@ struct program_options {
int format;
unsigned int duration;
char *filename;
+   int verbose;
 };
 
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH V2 idlestat 12/12] Report even if the application returned an error code

[Daniel Lezcano]

In case the application is killed by the timeout and it handles the SIGTERM, it
may return an error, thus idlestat won't display the result.

Ignore the return code of the application.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/idlestat.c b/idlestat.c
index 77f6d0a..a0ef2dd 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -1364,7 +1364,7 @@ static int execute(int argc, char *argv[], char *const 
envp[],
goto again;
}
 
-   if (WIFEXITED(status)  !WEXITSTATUS(status)) {
+   if (WIFEXITED(status)) {
/*
 * Cancel the timer in case the program
 * finished before the timeout
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH V2 idlestat 11/12] Split the display to show the wakeup separetely

[Daniel Lezcano]

The display functions are tied together in the code. Split this, so we can
choose each subsystem to display separately.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |  102 ++--
 topology.c |   11 ---
 2 files changed, 71 insertions(+), 42 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index 94e570a..77f6d0a 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -75,34 +75,6 @@ static void charrep(char c, int count)
printf(%c, c);
 }
 
-static void display_cstates_header(void)
-{
-   charrep('-', 80);
-   printf(\n);
-
-   printf(| C-state  |   min|   max|   avg|   total  | hits  
|  over | under |\n);
-}
-
-static void display_cstates_footer(void)
-{
-   charrep('-', 80);
-   printf(\n\n);
-}
-
-static void display_pstates_header(void)
-{
-   charrep('-', 64);
-   printf(\n);
-
-   printf(| P-state  |   min|   max|   avg|   total  | hits  
|\n);
-}
-
-static void display_pstates_footer(void)
-{
-   charrep('-', 64);
-   printf(\n\n);
-}
-
 static void display_cpu_header(char *cpu, int length)
 {
charrep('-', length);
@@ -145,6 +117,20 @@ static void display_factored_freq(int freq, int align)
}
 }
 
+static void display_cstates_header(void)
+{
+   charrep('-', 80);
+   printf(\n);
+
+   printf(| C-state  |   min|   max|   avg|   total  | hits  
|  over | under |\n);
+}
+
+static void display_cstates_footer(void)
+{
+   charrep('-', 80);
+   printf(\n\n);
+}
+
 static int display_cstates(void *arg, char *cpu)
 {
int i;
@@ -184,6 +170,20 @@ static int display_cstates(void *arg, char *cpu)
return 0;
 }
 
+static void display_pstates_header(void)
+{
+   charrep('-', 64);
+   printf(\n);
+
+   printf(| P-state  |   min|   max|   avg|   total  | hits  
|\n);
+}
+
+static void display_pstates_footer(void)
+{
+   charrep('-', 64);
+   printf(\n\n);
+}
+
 static int display_pstates(void *arg, char *cpu)
 {
int i;
@@ -225,19 +225,41 @@ static int display_pstates(void *arg, char *cpu)
return 0;
 }
 
-static int display_wakeup_sources(struct wakeup_info *wakeinfo, char *str)
+static void display_wakeup_header(void)
+{
+   charrep('-', 44);
+   printf(\n);
+
+   printf(| Wakeup |  #  |   Name  |  Count  |\n);
+}
+
+static void display_wakeup_footer(void)
+{
+   charrep('-', 44);
+   printf(\n\n);
+}
+
+static int display_wakeup(void *arg, char *cpu)
 {
int i;
+   bool cpu_header = false;
+   struct cpuidle_cstates *cstates = arg;
+   struct wakeup_info *wakeinfo = cstates-wakeinfo;
struct wakeup_irq *irqinfo = wakeinfo-irqinfo;
 
-   printf(%s wakeups \tname \t\tcount\tunexpected\n, str);
for (i = 0; i  wakeinfo-nrdata; i++, irqinfo++) {
-   printf(%*c %s%03d\t%-15.15s\t%d\t%d\n, (int)strlen(str),
-  ' ',
+
+   if (!cpu_header) {
+   display_cpu_header(cpu, 44);
+   cpu_header = true;
+   charrep('-', 44);
+   printf(\n);
+   }
+
+   printf(| %-6s | %-3d | %15.15s | %7d |\n,
   (irqinfo-irq_type  IRQ_TYPE_MAX) ?
-  irq_type_name[irqinfo-irq_type] : NULL,
-  irqinfo-id, irqinfo-name, irqinfo-count,
-  irqinfo-not_predicted);
+  irq_type_name[irqinfo-irq_type] : ???,
+  irqinfo-id, irqinfo-name, irqinfo-count);
}
 
return 0;
@@ -1446,15 +1468,21 @@ int main(int argc, char *argv[], char *const envp[])
 
if (options.display  IDLE_DISPLAY) {
display_cstates_header();
-   dump_cpu_topo_info(display_cstates, 0);
+   dump_cpu_topo_info(display_cstates, 1);
display_cstates_footer();
}
 
if (options.display  FREQUENCY_DISPLAY) {
display_pstates_header();
-   dump_cpu_topo_info(display_pstates, 1);
+   dump_cpu_topo_info(display_pstates, 0);
display_pstates_footer();
}
+
+   if (options.display  WAKEUP_DISPLAY) {
+   display_wakeup_header();
+   dump_cpu_topo_info(display_wakeup, 1);
+   display_wakeup_footer();
+   }
}
 
release_cpu_topo_cstates();
diff --git a/topology.c b/topology.c
index 5bf88a1..00c1fe1 100644
--- a/topology.c
+++ b/topology.c
@@ -449,7 +449,7 @@ int establish_idledata_to_topo(struct cpuidle_datas *datas)
return 0;
 }
 
-int dump_cpu_topo_info(int (*dump)(void *, char *), int pstate)
+int dump_cpu_topo_info(int (*dump)(void *, char *), int cstate

[PATCH V2 idlestat 01/12] Fix CPU affinity mask

[Daniel Lezcano]

From: Patrick Bellasi patrick.bell...@arm.com

To properly identify the initial PState of each CPU, the idlestat_wake_all
function is used to force an idle state exit on all CPUs by pinning idlestat
on each CPUs. However, this routine forgets to recover the original mask
at the end of the pinning process.

This patch solve this issue, by properly recovering the original CPU
affinity mask. Moreover, it avoids to wake up CPUs that will not be used
to run the workload.

Signed-off-by: Patrick Bellasi patrick.bell...@arm.com
Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   11 +++
 1 file changed, 11 insertions(+)

diff --git a/idlestat.c b/idlestat.c
index 0e0e1df..2a0c549 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -1210,6 +1210,7 @@ static int idlestat_wake_all(void)
 {
int rcpu, i, ret;
cpu_set_t cpumask;
+   cpu_set_t original_cpumask;
 
ret = sysconf(_SC_NPROCESSORS_CONF);
if (ret  0)
@@ -1219,18 +1220,28 @@ static int idlestat_wake_all(void)
if (rcpu  0)
return -1;
 
+   /* Keep track of the CPUs we will run on */
+   sched_getaffinity(0, sizeof(original_cpumask), original_cpumask);
+
for (i = 0; i  ret; i++) {
 
/* Pointless to wake up ourself */
if (i == rcpu)
continue;
 
+   /* Pointless to wake CPUs we will not run on */
+   if (!CPU_ISSET(i, original_cpumask))
+   continue;
+
CPU_ZERO(cpumask);
CPU_SET(i, cpumask);
 
sched_setaffinity(0, sizeof(cpumask), cpumask);
}
 
+   /* Enable all the CPUs of the original mask */
+   sched_setaffinity(0, sizeof(original_cpumask), original_cpumask);
+
return 0;
 }
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH V2 idlestat 08/12] Don't double check with the estimate function

[Daniel Lezcano]

We want the output to be always the same even if the prediction accuracy is
not possible because we are working on an old kernel.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index 83655bd..fc7a31a 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -72,15 +72,7 @@ static int display_cstates(struct cpuidle_cstates *cstates, 
char *str)
 {
int i;
 
-   /* If the first C-state does not have target_residency
-* chances are pretty high that we dont have it for any node.
-*/
-   if (cstates-cstate[0].target_residency = 0)
-   printf(%s@state 
hits\tover\tunder\t\ttotal(us)\tavg(us)\tmin(us)\tmax(us)\n,
-   str);
-   else
-   printf(%s@state 
hits\t\ttotal(us)\tavg(us)\tmin(us)\tmax(us)\n,
-   str);
+   printf(%s@state 
hits\tover\tunder\t\ttotal(us)\tavg(us)\tmin(us)\tmax(us)\n, str);
 
for (i = 0; i  cstates-cstate_max + 1; i++) {
struct cpuidle_cstate *c = cstates-cstate[i];
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH V2 idlestat 10/12] Add option to choose the data to be displayed

[Daniel Lezcano]

Add the options to select c-states, p-states and wakeup output.

--idle, -c  : show c-states
--frequency, -p : show p-states
--wakeup, -w: show wakeup sources

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 README |7 ++-
 idlestat.c |   35 +++
 idlestat.h |4 
 3 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/README b/README
index b9edf4f..dccce36 100644
--- a/README
+++ b/README
@@ -38,9 +38,14 @@ Example Usage
 Trace mode:
 sudo ./idlestat --trace -f /tmp/mytrace -t 10
 
-Reporing mode (/tmp/mytrace already contains traces):
+Reporting mode (/tmp/mytrace already contains traces):
 sudo ./idlestat --import -f /tmp/mytrace
 
 Trace mode with workload (e.g. sleep, cyclictest):
 sudo ./idlestat --trace -f /tmp/mytrace -t 10 -- /bin/sleep 10
 sudo ./idlestat --trace -f /tmp/myoutput -t 10 -- cyclictest -t 4 -i 2000 -q 
-D 5
+
+Selective trace output
+sudo ./idlestate --import -f /tmp/mytrace -w
+sudo ./idlestate --import -f /tmp/mytrace -c -p
+sudo ./idlestate --import -f /tmp/mytrace -p -w
\ No newline at end of file
diff --git a/idlestat.c b/idlestat.c
index 3fe2af6..94e570a 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -1087,7 +1087,7 @@ static void help(const char *cmd)
 {
fprintf(stderr,
\nUsage:\nTrace mode: %s --trace -f|--trace-file filename
--t|--duration seconds ,
+-t|--duration seconds -c|--idle -p|--frequency -w|--wakeup,
basename(cmd));
fprintf(stderr,
\nReporting mode: %s --import -f|--trace-file filename,
@@ -1113,6 +1113,9 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
{ duration,required_argument, NULL, 't' },
{ version, no_argument,   NULL, 'V' },
{ verbose, no_argument,   NULL, 'v' },
+   { idle,no_argument,   NULL, 'c' },
+   { frequency,   no_argument,   NULL, 'p' },
+   { wakeup,  no_argument,   NULL, 'w' },
{ 0, 0, 0, 0 }
};
int c;
@@ -1125,7 +1128,7 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
 
int optindex = 0;
 
-   c = getopt_long(argc, argv, :df:ht:Vv,
+   c = getopt_long(argc, argv, :df:ht:cpwVv,
long_options, optindex);
if (c == -1)
break;
@@ -1141,6 +1144,15 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
case 't':
options-duration = atoi(optarg);
break;
+   case 'c':
+   options-display |= IDLE_DISPLAY;
+   break;
+   case 'p':
+   options-display |= FREQUENCY_DISPLAY;
+   break;
+   case 'w':
+   options-display |= WAKEUP_DISPLAY;
+   break;
case 'V':
version(argv[0]);
exit(0);
@@ -1180,6 +1192,9 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
}
}
 
+   if (options-display == 0)
+   options-display = IDLE_DISPLAY;
+
return optind;
 }
 
@@ -1429,13 +1444,17 @@ int main(int argc, char *argv[], char *const envp[])
 */
if (0 == establish_idledata_to_topo(datas)) {
 
-   display_cstates_header();
-   dump_cpu_topo_info(display_cstates, 0);
-   display_cstates_footer();
+   if (options.display  IDLE_DISPLAY) {
+   display_cstates_header();
+   dump_cpu_topo_info(display_cstates, 0);
+   display_cstates_footer();
+   }
 
-   display_pstates_header();
-   dump_cpu_topo_info(display_pstates, 1);
-   display_pstates_footer();
+   if (options.display  FREQUENCY_DISPLAY) {
+   display_pstates_header();
+   dump_cpu_topo_info(display_pstates, 1);
+   display_pstates_footer();
+   }
}
 
release_cpu_topo_cstates();
diff --git a/idlestat.h b/idlestat.h
index 31df587..1d5f961 100644
--- a/idlestat.h
+++ b/idlestat.h
@@ -129,10 +129,14 @@ enum formats {
 struct program_options {
int mode;
int format;
+   int display;
unsigned int duration;
char *filename;
int verbose;
 };
 
+#define IDLE_DISPLAY  0x1
+#define FREQUENCY_DISPLAY 0x2
+#define WAKEUP_DISPLAY0x4
 
 #endif
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH V2 idlestat 04/12] Add 'patches' directory to the ignored list of files in .gitignore

[Daniel Lezcano]

The quilt tool is still widely used in conjunction with git, it needs a
specific directory, by convention 'patches' to store the patches for the
serie.

Add this directory in the .gitignore file, so we are no longer annoyed by this
directory when we check the status of git reporting it as an untracked file.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 .gitignore |1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 968abde..4ee98b8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -30,6 +30,7 @@
 *.orig
 *~
 \#*#
+patches
 
 #
 # Top-level files
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH V2 idlestat 03/12] Remove the debug option

[Daniel Lezcano]

This function is pointless. It was added initially for debugging purpose,
remove it.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   16 ++--
 idlestat.h |1 -
 2 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index 30860db..abbb787 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -1042,11 +1042,11 @@ static void help(const char *cmd)
fprintf(stderr,
\nUsage:\nTrace mode: %s --trace -f|--trace-file filename
 -t|--duration seconds 
-   [-z|--dump] [-i|--iterations number] [-d|--debug]\n,
+   [-z|--dump] [-i|--iterations number]\n,
basename(cmd));
fprintf(stderr,
\nReporting mode: %s --import -f|--trace-file filename
-   [-z|--dump] [-i|--iterations number] [-d|--debug]\n,
+   [-z|--dump] [-i|--iterations number]\n,
basename(cmd));
fprintf(stderr,
\nExample:\n%s --trace -f /tmp/myoutput -t 30\n, 
basename(cmd));
@@ -1064,7 +1064,6 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
struct option long_options[] = {
{ trace,   no_argument,   options-mode, TRACE },
{ import,  no_argument,   options-mode, IMPORT },
-   { debug,   no_argument,   NULL, 'd' },
{ trace-file,  required_argument, NULL, 'f' },
{ help,no_argument,   NULL, 'h' },
{ iterations,  required_argument, NULL, 'i' },
@@ -1089,9 +1088,6 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
break;
 
switch (c) {
-   case 'd':
-   options-debug = true;
-   break;
case 'f':
options-filename = optarg;
break;
@@ -1315,7 +1311,6 @@ int main(int argc, char *argv[], char *const envp[])
struct cpuidle_datas *datas;
struct cpuidle_datas *cluster;
struct program_options options;
-   struct rusage rusage;
int args;
 
args = getoptions(argc, argv, options);
@@ -1420,13 +1415,6 @@ int main(int argc, char *argv[], char *const envp[])
free(cluster);
}
 
-   /* Computation could be heavy, let's give some information
-* about the memory consumption */
-   if (options.debug) {
-   getrusage(RUSAGE_SELF, rusage);
-   printf(max rss : %ld kB\n, rusage.ru_maxrss);
-   }
-
release_cpu_topo_cstates();
release_cpu_topo_info();
release_pstate_info(datas-pstates, datas-nrcpus);
diff --git a/idlestat.h b/idlestat.h
index 4397984..3317a8f 100644
--- a/idlestat.h
+++ b/idlestat.h
@@ -127,7 +127,6 @@ enum formats {
 };
 
 struct program_options {
-   bool debug;
bool dump;
int iterations;
int mode;
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH V2 idlestat 02/12] Fix some lines to comply with 80 chars length

[Daniel Lezcano]

Fix lines exceeding 80 chars, so fulfill the conventional coding style
of the opensource projects.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index 2a0c549..30860db 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -1162,7 +1162,8 @@ static int idlestat_file_for_each_line(const char *path, 
void *data,
f = fopen(path, r);
 
if (!f) {
-   fprintf(stderr, %s: failed to open '%s': %m\n, __func__, 
path);
+   fprintf(stderr, %s: failed to open '%s': %m\n,
+   __func__, path);
return -1;
}
 
@@ -1189,7 +1190,8 @@ static int idlestat_store(const char *path)
f = fopen(path, w+);
 
if (!f) {
-   fprintf(stderr, %s: failed to open '%s': %m\n, __func__, 
path);
+   fprintf(stderr, %s: failed to open '%s': %m\n,
+   __func__, path);
return -1;
}
 
@@ -1320,7 +1322,8 @@ int main(int argc, char *argv[], char *const envp[])
if (args = 0)
return 1;
 
-   /* Tracing requires manipulation of some files only accessible to root 
*/
+   /* Tracing requires manipulation of some files only accessible
+* to root */
if ((options.mode == TRACE)  getuid()) {
fprintf(stderr, must be root to run traces\n);
return -1;
@@ -1337,7 +1340,8 @@ int main(int argc, char *argv[], char *const envp[])
 
/* Stop tracing (just in case) */
if (idlestat_trace_enable(false)) {
-   fprintf(stderr, idlestat requires kernel Ftrace and 
debugfs mounted on /sys/kernel/debug\n);
+   fprintf(stderr, idlestat requires kernel Ftrace and 
+   debugfs mounted on /sys/kernel/debug\n);
return -1;
}
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH idlestat 04/12] Add 'patches' directory to the ignored list of files in .gitignore

[Daniel Lezcano]

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 .gitignore |1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 968abde..4ee98b8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -30,6 +30,7 @@
 *.orig
 *~
 \#*#
+patches
 
 #
 # Top-level files
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH idlestat 06/12] Remove dump and iterations options

[Daniel Lezcano]

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   72 ++--
 idlestat.h |2 --
 topology.c |   12 +-
 topology.h |5 ++---
 4 files changed, 19 insertions(+), 72 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index 8138e35..e4dbb89 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -68,34 +68,8 @@ static inline void *ptrerror(const char *str)
return NULL;
 }
 
-static int dump_states(struct cpuidle_cstates *cstates,
-  struct cpufreq_pstates *pstates,
-  int count, char *str)
-{
-   int j, k, kmax;
-   struct cpuidle_cstate *cstate;
-
-   for (j = 0; j  cstates-cstate_max + 1; j++) {
-   cstate = cstates-cstate[j];
-
-   kmax = count  0 ? MIN(count, cstate-nrdata) : cstate-nrdata;
-   for (k = 0; k  kmax; k++) {
-   printf(%lf: enter %s\n, cstate-data[k].begin,
-  cstate-name);
-   printf(%lf: exit %s\n, cstate-data[k].end,
-  cstate-name);
-   }
-
-   /* add a break */
-   printf(\n);
-   }
-
-   return 0;
-}
-
 static int display_states(struct cpuidle_cstates *cstates,
- struct cpufreq_pstates *pstates,
- int count, char *str)
+ struct cpufreq_pstates *pstates, char *str)
 {
int j;
 
@@ -171,9 +145,9 @@ static int display_states(struct cpuidle_cstates *cstates,
return 0;
 }
 
-int dump_all_data(struct cpuidle_datas *datas, int count,
-   int (*dump)(struct cpuidle_cstates *,
-   struct cpufreq_pstates *, int, char *))
+int dump_all_data(struct cpuidle_datas *datas,
+ int (*dump)(struct cpuidle_cstates *,
+ struct cpufreq_pstates *, char *))
 {
int i = 0, nrcpus = datas-nrcpus;
struct cpuidle_cstates *cstates;
@@ -188,7 +162,7 @@ int dump_all_data(struct cpuidle_datas *datas, int count,
else
sprintf(buffer, cpu%d, i);
 
-   dump(cstates, pstates, count, buffer);
+   dump(cstates, pstates, buffer);
 
i++;
 
@@ -1041,12 +1015,10 @@ static void help(const char *cmd)
 {
fprintf(stderr,
\nUsage:\nTrace mode: %s --trace -f|--trace-file filename
--t|--duration seconds 
-   [-z|--dump] [-i|--iterations number]\n,
+-t|--duration seconds ,
basename(cmd));
fprintf(stderr,
-   \nReporting mode: %s --import -f|--trace-file filename
-   [-z|--dump] [-i|--iterations number]\n,
+   \nReporting mode: %s --import -f|--trace-file filename,
basename(cmd));
fprintf(stderr,
\nExample:\n%s --trace -f /tmp/myoutput -t 30\n, 
basename(cmd));
@@ -1066,10 +1038,8 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
{ import,  no_argument,   options-mode, IMPORT },
{ trace-file,  required_argument, NULL, 'f' },
{ help,no_argument,   NULL, 'h' },
-   { iterations,  required_argument, NULL, 'i' },
{ duration,required_argument, NULL, 't' },
{ version, no_argument,   NULL, 'V' },
-   { dump,no_argument,   NULL, 'z' },
{ verbose, no_argument,   NULL, 'v' },
{ 0, 0, 0, 0 }
};
@@ -1083,7 +1053,7 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
 
int optindex = 0;
 
-   c = getopt_long(argc, argv, :df:hi:t:Vvz,
+   c = getopt_long(argc, argv, :df:h:t:Vv,
long_options, optindex);
if (c == -1)
break;
@@ -1096,9 +1066,6 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
help(argv[0]);
exit(0);
break;
-   case 'i':
-   options-iterations = atoi(optarg);
-   break;
case 't':
options-duration = atoi(optarg);
break;
@@ -1106,9 +1073,6 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
version(argv[0]);
exit(0);
break;
-   case 'z':
-   options-dump = true;
-   break;
case 'v':
options-verbose++;
break;
@@ -1127,9 +1091,6 @@ int getoptions(int argc, char *argv[], struct 
program_options *options

[PATCH idlestat 11/12] Split the display to show the wakeup separetely

[Daniel Lezcano]

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |  102 ++--
 topology.c |   10 +++---
 2 files changed, 70 insertions(+), 42 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index da1e14b..24248b2 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -75,34 +75,6 @@ static void charrep(char c, int count)
printf(%c, c);
 }
 
-static void display_cstates_header(void)
-{
-   charrep('-', 80);
-   printf(\n);
-
-   printf(| C-state  |   min|   max|   avg|   total  | hits  
|  over | under |\n);
-}
-
-static void display_cstates_footer(void)
-{
-   charrep('-', 80);
-   printf(\n\n);
-}
-
-static void display_pstates_header(void)
-{
-   charrep('-', 64);
-   printf(\n);
-
-   printf(| P-state  |   min|   max|   avg|   total  | hits  
|\n);
-}
-
-static void display_pstates_footer(void)
-{
-   charrep('-', 64);
-   printf(\n\n);
-}
-
 static void display_cpu_header(char *cpu, int length)
 {
charrep('-', length);
@@ -145,6 +117,20 @@ static void display_factored_freq(int freq, int align)
}
 }
 
+static void display_cstates_header(void)
+{
+   charrep('-', 80);
+   printf(\n);
+
+   printf(| C-state  |   min|   max|   avg|   total  | hits  
|  over | under |\n);
+}
+
+static void display_cstates_footer(void)
+{
+   charrep('-', 80);
+   printf(\n\n);
+}
+
 static int display_cstates(void *arg, char *cpu)
 {
int i;
@@ -184,6 +170,20 @@ static int display_cstates(void *arg, char *cpu)
return 0;
 }
 
+static void display_pstates_header(void)
+{
+   charrep('-', 64);
+   printf(\n);
+
+   printf(| P-state  |   min|   max|   avg|   total  | hits  
|\n);
+}
+
+static void display_pstates_footer(void)
+{
+   charrep('-', 64);
+   printf(\n\n);
+}
+
 static int display_pstates(void *arg, char *cpu)
 {
int i;
@@ -225,19 +225,41 @@ static int display_pstates(void *arg, char *cpu)
return 0;
 }
 
-static int display_wakeup_sources(struct wakeup_info *wakeinfo, char *str)
+static void display_wakeup_header(void)
+{
+   charrep('-', 44);
+   printf(\n);
+
+   printf(| Wakeup |  #  |   Name  |  Count  |\n);
+}
+
+static void display_wakeup_footer(void)
+{
+   charrep('-', 44);
+   printf(\n\n);
+}
+
+static int display_wakeup(void *arg, char *cpu)
 {
int i;
+   bool cpu_header = false;
+   struct cpuidle_cstates *cstates = arg;
+   struct wakeup_info *wakeinfo = cstates-wakeinfo;
struct wakeup_irq *irqinfo = wakeinfo-irqinfo;
 
-   printf(%s wakeups \tname \t\tcount\tunexpected\n, str);
for (i = 0; i  wakeinfo-nrdata; i++, irqinfo++) {
-   printf(%*c %s%03d\t%-15.15s\t%d\t%d\n, (int)strlen(str),
-  ' ',
+
+   if (!cpu_header) {
+   display_cpu_header(cpu, 44);
+   cpu_header = true;
+   charrep('-', 44);
+   printf(\n);
+   }
+
+   printf(| %-6s | %-3d | %15.15s | %7d |\n,
   (irqinfo-irq_type  IRQ_TYPE_MAX) ?
-  irq_type_name[irqinfo-irq_type] : NULL,
-  irqinfo-id, irqinfo-name, irqinfo-count,
-  irqinfo-not_predicted);
+  irq_type_name[irqinfo-irq_type] : ???,
+  irqinfo-id, irqinfo-name, irqinfo-count);
}
 
return 0;
@@ -1446,15 +1468,21 @@ int main(int argc, char *argv[], char *const envp[])
 
if (options.display  IDLE_DISPLAY) {
display_cstates_header();
-   dump_cpu_topo_info(display_cstates, 0);
+   dump_cpu_topo_info(display_cstates, 1);
display_cstates_footer();
}
 
if (options.display  FREQUENCY_DISPLAY) {
display_pstates_header();
-   dump_cpu_topo_info(display_pstates, 1);
+   dump_cpu_topo_info(display_pstates, 0);
display_pstates_footer();
}
+
+   if (options.display  WAKEUP_DISPLAY) {
+   display_wakeup_header();
+   dump_cpu_topo_info(display_wakeup, 1);
+   display_wakeup_footer();
+   }
}
 
release_cpu_topo_cstates();
diff --git a/topology.c b/topology.c
index 5bf88a1..b47c348 100644
--- a/topology.c
+++ b/topology.c
@@ -449,7 +449,7 @@ int establish_idledata_to_topo(struct cpuidle_datas *datas)
return 0;
 }
 
-int dump_cpu_topo_info(int (*dump)(void *, char *), int pstate)
+int dump_cpu_topo_info(int (*dump)(void *, char *), int cstate)
 {
struct cpu_physical *s_phy;
struct cpu_core *s_core;
@@ -461,11 +461,11 @@ int dump_cpu_topo_info(int

[PATCH idlestat 08/12] Don't double check with the estimate function

[Daniel Lezcano]

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index b084b5d..e5dde8c 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -72,15 +72,7 @@ static int display_cstates(struct cpuidle_cstates *cstates, 
char *str)
 {
int i;
 
-   /* If the first C-state does not have target_residency
-* chances are pretty high that we dont have it for any node.
-*/
-   if (cstates-cstate[0].target_residency = 0)
-   printf(%s@state 
hits\tover\tunder\t\ttotal(us)\tavg(us)\tmin(us)\tmax(us)\n,
-   str);
-   else
-   printf(%s@state 
hits\t\ttotal(us)\tavg(us)\tmin(us)\tmax(us)\n,
-   str);
+   printf(%s@state 
hits\tover\tunder\t\ttotal(us)\tavg(us)\tmin(us)\tmax(us)\n, str);
 
for (i = 0; i  cstates-cstate_max + 1; i++) {
struct cpuidle_cstate *c = cstates-cstate[i];
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH idlestat 02/12] Fix some lines to comply with 80 chars length

[Daniel Lezcano]

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index 2a0c549..30860db 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -1162,7 +1162,8 @@ static int idlestat_file_for_each_line(const char *path, 
void *data,
f = fopen(path, r);
 
if (!f) {
-   fprintf(stderr, %s: failed to open '%s': %m\n, __func__, 
path);
+   fprintf(stderr, %s: failed to open '%s': %m\n,
+   __func__, path);
return -1;
}
 
@@ -1189,7 +1190,8 @@ static int idlestat_store(const char *path)
f = fopen(path, w+);
 
if (!f) {
-   fprintf(stderr, %s: failed to open '%s': %m\n, __func__, 
path);
+   fprintf(stderr, %s: failed to open '%s': %m\n,
+   __func__, path);
return -1;
}
 
@@ -1320,7 +1322,8 @@ int main(int argc, char *argv[], char *const envp[])
if (args = 0)
return 1;
 
-   /* Tracing requires manipulation of some files only accessible to root 
*/
+   /* Tracing requires manipulation of some files only accessible
+* to root */
if ((options.mode == TRACE)  getuid()) {
fprintf(stderr, must be root to run traces\n);
return -1;
@@ -1337,7 +1340,8 @@ int main(int argc, char *argv[], char *const envp[])
 
/* Stop tracing (just in case) */
if (idlestat_trace_enable(false)) {
-   fprintf(stderr, idlestat requires kernel Ftrace and 
debugfs mounted on /sys/kernel/debug\n);
+   fprintf(stderr, idlestat requires kernel Ftrace and 
+   debugfs mounted on /sys/kernel/debug\n);
return -1;
}
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH idlestat 12/12] Report even if the application returned an error code

[Daniel Lezcano]

In case the application is killed by the timeout and it handles the SIGTERM, it
may return an error, thus idlestat won't display the result.

Ignore the return code of the application.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/idlestat.c b/idlestat.c
index 24248b2..88cb11f 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -1364,7 +1364,7 @@ static int execute(int argc, char *argv[], char *const 
envp[],
goto again;
}
 
-   if (WIFEXITED(status)  !WEXITSTATUS(status)) {
+   if (WIFEXITED(status)) {
/*
 * Cancel the timer in case the program
 * finished before the timeout
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH idlestat 09/12] Improve the output

[Daniel Lezcano]

 |
|   C6-IVB | 24us |   2.34ms |   1.01ms |  32.32ms |32 | 0 | 0 |
|   C7-IVB |  1us |  31.43ms |   2.70ms |   25.33s |  9369 | 0 | 0 |

|   cpu3   |

| POLL | 11us | 11us | 11us | 11us | 1 | 0 | 0 |
|   C1-IVB |  1us |   4.80ms | 94us |  42.66ms |   455 | 0 | 0 |
|   C3-IVB |  1us |   2.00ms |420us |  28.59ms |68 | 0 | 0 |
|   C6-IVB | 31us |   1.42ms |800us |   8.00ms |10 | 0 | 0 |
|   C7-IVB |  0us |  98.04ms |   6.92ms |   26.99s |  3899 | 0 | 0 |


Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |  209 +---
 topology.c |   27 
 topology.h |4 +-
 3 files changed, 143 insertions(+), 97 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index e5dde8c..d369867 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -68,11 +68,88 @@ static inline void *ptrerror(const char *str)
return NULL;
 }
 
-static int display_cstates(struct cpuidle_cstates *cstates, char *str)
+static void charrep(char c, int count)
 {
int i;
+   for (i = 0; i  count; i++)
+   printf(%c, c);
+}
+
+static void display_cstates_header(void)
+{
+   charrep('-', 80);
+   printf(\n);
+
+   printf(| C-state  |   min|   max|   avg|   total  | hits  
|  over | under |\n);
+}
+
+static void display_cstates_footer(void)
+{
+   charrep('-', 80);
+   printf(\n\n);
+}
+
+static void display_pstates_header(void)
+{
+   charrep('-', 64);
+   printf(\n);
+
+   printf(| P-state  |   min|   max|   avg|   total  | hits  
|\n);
+}
+
+static void display_pstates_footer(void)
+{
+   charrep('-', 64);
+   printf(\n\n);
+}
+
+static void display_cpu_header(char *cpu, int length)
+{
+   charrep('-', length);
+   printf(\n);
+
+   printf(|%*s%*s|\n, (length / 2) - 1, cpu, ((length / 2) - 1), );
+}
+
+static void display_factored_time(double time, int align)
+{
+   char buffer[128];
+
+   if (time  1000) {
+   sprintf(buffer, %.0lfus, time);
+   printf(%*s, align, buffer);
+   }
+   else if (time  100) {
+   sprintf(buffer, %.2lfms, time / 1000.0);
+   printf(%*s, align, buffer);
+   }
+   else {
+   sprintf(buffer, %.2lfs, time / 100.0);
+   printf(%*s, align, buffer);
+   }
+}
+
+static void display_factored_freq(int freq, int align)
+{
+   char buffer[128];
+
+   if (freq  1000) {
+   sprintf(buffer, %dHz, freq);
+   printf(%*s, align, buffer);
+   } else if (freq  100) {
+   sprintf(buffer, %.2fMHz, (float)freq / 1000.0);
+   printf(%*s, align, buffer);
+   } else {
+   sprintf(buffer, %.2fGHz, (float)freq / 100.0);
+   printf(%*s, align, buffer);
+   }
+}
 
-   printf(%s@state 
hits\tover\tunder\t\ttotal(us)\tavg(us)\tmin(us)\tmax(us)\n, str);
+static int display_cstates(void *arg, char *cpu)
+{
+   int i;
+   bool cpu_header = false;
+   struct cpuidle_cstates *cstates = arg;
 
for (i = 0; i  cstates-cstate_max + 1; i++) {
struct cpuidle_cstate *c = cstates-cstate[i];
@@ -81,33 +158,37 @@ static int display_cstates(struct cpuidle_cstates 
*cstates, char *str)
/* nothing to report for this state */
continue;
 
-   if (c-target_residency = 0) {
-   printf(%*c 
%-10s%d\t%d\t%d\t%15.2lf\t%15.2lf\t%.2lf\t%.2lf\n,
-   (int)strlen(str), ' ',
-   c-name, c-nrdata,
-   c-premature_wakeup,
-   c-could_sleep_more,
-   c-duration,
-   c-avg_time,
-   (c-min_time == DBL_MAX ? 0. : c-min_time),
-   c-max_time);
-   } else {
-   printf(%*c %-10s%d\t%15.2lf\t%15.2lf\t%.2lf\t%.2lf\n,
-   (int)strlen(str), ' ',
-   c-name, c-nrdata,
-   c-duration,
-   c-avg_time,
-   (c-min_time == DBL_MAX ? 0. : c-min_time),
-   c-max_time);
+   if (!cpu_header) {
+   display_cpu_header(cpu, 80);
+   cpu_header = true;
+   charrep('-', 80

[PATCH idlestat 01/12] Fix CPU affinity mask

[Daniel Lezcano]

From: Patrick Bellasi patrick.bell...@arm.com

To properly identify the initial PState of each CPU, the idlestat_wake_all
function is used to force an idle state exit on all CPUs by pinning idlestat
on each CPUs. However, this routine forgets to recover the original mask
at the end of the pinning process.

This patch solve this issue, by properly recovering the original CPU
affinity mask. Moreover, it avoids to wake up CPUs that will not be used
to run the workload.

Signed-off-by: Patrick Bellasi patrick.bell...@arm.com
Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   11 +++
 1 file changed, 11 insertions(+)

diff --git a/idlestat.c b/idlestat.c
index 0e0e1df..2a0c549 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -1210,6 +1210,7 @@ static int idlestat_wake_all(void)
 {
int rcpu, i, ret;
cpu_set_t cpumask;
+   cpu_set_t original_cpumask;
 
ret = sysconf(_SC_NPROCESSORS_CONF);
if (ret  0)
@@ -1219,18 +1220,28 @@ static int idlestat_wake_all(void)
if (rcpu  0)
return -1;
 
+   /* Keep track of the CPUs we will run on */
+   sched_getaffinity(0, sizeof(original_cpumask), original_cpumask);
+
for (i = 0; i  ret; i++) {
 
/* Pointless to wake up ourself */
if (i == rcpu)
continue;
 
+   /* Pointless to wake CPUs we will not run on */
+   if (!CPU_ISSET(i, original_cpumask))
+   continue;
+
CPU_ZERO(cpumask);
CPU_SET(i, cpumask);
 
sched_setaffinity(0, sizeof(cpumask), cpumask);
}
 
+   /* Enable all the CPUs of the original mask */
+   sched_setaffinity(0, sizeof(original_cpumask), original_cpumask);
+
return 0;
 }
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH idlestat 10/12] Add option to choose the data to be displayed

[Daniel Lezcano]

Add the options to select c-states, p-states and wakeup output.

--idle, -c  : show c-states
--frequency, -p : show p-states
--wakeup, -w: show wakeup sources

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   33 ++---
 idlestat.h |4 
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index d369867..da1e14b 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -1113,6 +1113,9 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
{ duration,required_argument, NULL, 't' },
{ version, no_argument,   NULL, 'V' },
{ verbose, no_argument,   NULL, 'v' },
+   { idle,no_argument,   NULL, 'c' },
+   { frequency,   no_argument,   NULL, 'p' },
+   { wakeup,  no_argument,   NULL, 'w' },
{ 0, 0, 0, 0 }
};
int c;
@@ -1125,7 +1128,7 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
 
int optindex = 0;
 
-   c = getopt_long(argc, argv, :df:h:t:Vv,
+   c = getopt_long(argc, argv, :df:h:t:cpwVv,
long_options, optindex);
if (c == -1)
break;
@@ -1141,6 +1144,15 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
case 't':
options-duration = atoi(optarg);
break;
+   case 'c':
+   options-display |= IDLE_DISPLAY;
+   break;
+   case 'p':
+   options-display |= FREQUENCY_DISPLAY;
+   break;
+   case 'w':
+   options-display |= WAKEUP_DISPLAY;
+   break;
case 'V':
version(argv[0]);
exit(0);
@@ -1180,6 +1192,9 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
}
}
 
+   if (options-display == 0)
+   options-display = IDLE_DISPLAY;
+
return optind;
 }
 
@@ -1429,13 +1444,17 @@ int main(int argc, char *argv[], char *const envp[])
 */
if (0 == establish_idledata_to_topo(datas)) {
 
-   display_cstates_header();
-   dump_cpu_topo_info(display_cstates, 0);
-   display_cstates_footer();
+   if (options.display  IDLE_DISPLAY) {
+   display_cstates_header();
+   dump_cpu_topo_info(display_cstates, 0);
+   display_cstates_footer();
+   }
 
-   display_pstates_header();
-   dump_cpu_topo_info(display_pstates, 1);
-   display_pstates_footer();
+   if (options.display  FREQUENCY_DISPLAY) {
+   display_pstates_header();
+   dump_cpu_topo_info(display_pstates, 1);
+   display_pstates_footer();
+   }
}
 
release_cpu_topo_cstates();
diff --git a/idlestat.h b/idlestat.h
index 31df587..1d5f961 100644
--- a/idlestat.h
+++ b/idlestat.h
@@ -129,10 +129,14 @@ enum formats {
 struct program_options {
int mode;
int format;
+   int display;
unsigned int duration;
char *filename;
int verbose;
 };
 
+#define IDLE_DISPLAY  0x1
+#define FREQUENCY_DISPLAY 0x2
+#define WAKEUP_DISPLAY0x4
 
 #endif
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH idlestat 07/12] Split display function

[Daniel Lezcano]

Make some code separations to prepare the code for cleanup.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   84 ++--
 1 file changed, 54 insertions(+), 30 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index e4dbb89..b084b5d 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -68,10 +68,9 @@ static inline void *ptrerror(const char *str)
return NULL;
 }
 
-static int display_states(struct cpuidle_cstates *cstates,
- struct cpufreq_pstates *pstates, char *str)
+static int display_cstates(struct cpuidle_cstates *cstates, char *str)
 {
-   int j;
+   int i;
 
/* If the first C-state does not have target_residency
 * chances are pretty high that we dont have it for any node.
@@ -83,8 +82,8 @@ static int display_states(struct cpuidle_cstates *cstates,
printf(%s@state 
hits\t\ttotal(us)\tavg(us)\tmin(us)\tmax(us)\n,
str);
 
-   for (j = 0; j  cstates-cstate_max + 1; j++) {
-   struct cpuidle_cstate *c = cstates-cstate[j];
+   for (i = 0; i  cstates-cstate_max + 1; i++) {
+   struct cpuidle_cstate *c = cstates-cstate[i];
 
if (c-nrdata == 0)
/* nothing to report for this state */
@@ -111,40 +110,65 @@ static int display_states(struct cpuidle_cstates *cstates,
}
}
 
-   if (pstates) {
-   for (j = 0; j  pstates-max; j++) {
-   struct cpufreq_pstate *p = (pstates-pstate[j]);
+   return 0;
+}
 
-   if (p-count == 0)
-   /* nothing to report for this state */
-   continue;
+static int display_pstates(struct cpufreq_pstates *pstates, char *str)
+{
+   int i;
 
-   printf(%*c 
%-10d\t%d\t%15.2lf\t%15.2lf\t%.2lf\t%.2lf\n,
-   (int)strlen(str), ' ',
-   p-freq/1000, p-count, p-duration,
-   p-avg_time,
-   (p-min_time == DBL_MAX ? 0. : p-min_time),
-   p-max_time);
-   }
+   for (i = 0; i  pstates-max; i++) {
+
+   struct cpufreq_pstate *p = (pstates-pstate[i]);
+
+   if (p-count == 0)
+   /* nothing to report for this state */
+   continue;
+
+   printf(%*c %-10d\t%d\t%15.2lf\t%15.2lf\t%.2lf\t%.2lf\n,
+  (int)strlen(str), ' ',
+  p-freq/1000, p-count, p-duration,
+  p-avg_time,
+  (p-min_time == DBL_MAX ? 0. : p-min_time),
+  p-max_time);
}
 
-   if (strstr(str, IRQ_WAKEUP_UNIT_NAME)) {
-   struct wakeup_info *wakeinfo = cstates-wakeinfo;
-   struct wakeup_irq *irqinfo = wakeinfo-irqinfo;
-   printf(%s wakeups \tname \t\tcount\tunexpected\n, str);
-   for (j = 0; j  wakeinfo-nrdata; j++, irqinfo++) {
-   printf(%*c %s%03d\t%-15.15s\t%d\t%d\n, 
(int)strlen(str),
-   ' ',
-   (irqinfo-irq_type  IRQ_TYPE_MAX) ?
-   irq_type_name[irqinfo-irq_type] : NULL,
-   irqinfo-id, irqinfo-name, irqinfo-count,
-   irqinfo-not_predicted);
-   }
+   return 0;
+}
+
+static int display_wakeup_sources(struct wakeup_info *wakeinfo, char *str)
+{
+   int i;
+   struct wakeup_irq *irqinfo = wakeinfo-irqinfo;
+
+   printf(%s wakeups \tname \t\tcount\tunexpected\n, str);
+   for (i = 0; i  wakeinfo-nrdata; i++, irqinfo++) {
+   printf(%*c %s%03d\t%-15.15s\t%d\t%d\n, (int)strlen(str),
+  ' ',
+  (irqinfo-irq_type  IRQ_TYPE_MAX) ?
+  irq_type_name[irqinfo-irq_type] : NULL,
+  irqinfo-id, irqinfo-name, irqinfo-count,
+  irqinfo-not_predicted);
}
 
return 0;
 }
 
+static int display_states(struct cpuidle_cstates *cstates,
+ struct cpufreq_pstates *pstates, char *str)
+{
+   if (cstates)
+   display_cstates(cstates, str);
+
+   if (pstates)
+   display_pstates(pstates, str);
+
+   if (strstr(str, IRQ_WAKEUP_UNIT_NAME))
+   display_wakeup_sources(cstates-wakeinfo, str);
+
+   return 0;
+}
+
 int dump_all_data(struct cpuidle_datas *datas,
  int (*dump)(struct cpuidle_cstates *,
  struct cpufreq_pstates *, char *))
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH idlestat 05/12] Add incremental verbose option

[Daniel Lezcano]

This patch adds the '-v' option in the command line.

The level of verbosity is choosen by the number of time the -v occurs.

no -v option : verbosity 0
-v   : verbosity 1
-vv  : verbosity 2
-vvv : verbosity 3
etc ...

Note this option is not yet used in the code.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |6 +-
 idlestat.h |1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/idlestat.c b/idlestat.c
index abbb787..8138e35 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -1070,6 +1070,7 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
{ duration,required_argument, NULL, 't' },
{ version, no_argument,   NULL, 'V' },
{ dump,no_argument,   NULL, 'z' },
+   { verbose, no_argument,   NULL, 'v' },
{ 0, 0, 0, 0 }
};
int c;
@@ -1082,7 +1083,7 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
 
int optindex = 0;
 
-   c = getopt_long(argc, argv, :df:hi:t:Vz,
+   c = getopt_long(argc, argv, :df:hi:t:Vvz,
long_options, optindex);
if (c == -1)
break;
@@ -1108,6 +1109,9 @@ int getoptions(int argc, char *argv[], struct 
program_options *options)
case 'z':
options-dump = true;
break;
+   case 'v':
+   options-verbose++;
+   break;
case 0: /* getopt_long() set a variable, just keep going */
break;
case ':':   /* missing option argument */
diff --git a/idlestat.h b/idlestat.h
index 3317a8f..f874bb8 100644
--- a/idlestat.h
+++ b/idlestat.h
@@ -133,6 +133,7 @@ struct program_options {
int format;
unsigned int duration;
char *filename;
+   int verbose;
 };
 
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH IDLESTAT] fix execute() function

[Daniel Lezcano]

On 06/04/2014 08:00 AM, Mohammad Merajul Islam Molla wrote:

Patch to fix the below two issues -

1. Fix the part where parent waits to fetch child status. In case
SIGALRM is received, fetch new status after killing child so that
subsequent check for termination status passes.
2. In case of normal termination, just check if program exited
normally (checking exit status is not necessary)


Sounds good.

Thanks for the patch.

  -- Daniel


diff --git a/idlestat.c b/idlestat.c
index 787d7b0..179e7a7 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -1196,12 +1196,12 @@ static int execute(int argc, char *argv[],
char *const envp[],
 alarm(options-duration);
 again:
 if (waitpid(pid, status, 0)  0) {
-   if (errno != EINTR || !sigalrm)
-   goto again;
-   kill(pid, SIGTERM);
+   if (errno == EINTR  sigalrm)
+   kill(pid, SIGTERM);
+   goto again;
 }

-   if (WIFEXITED(status)  !WEXITSTATUS(status)) {
+   if (WIFEXITED(status)) {
 /*
  * Cancel the timer in case the program
  * finished before the timeout

--
Thanks,
-Meraj




--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: idlestat: execute() function problems?

[Daniel Lezcano]

On 06/03/2014 11:53 AM, Mohammad Merajul Islam Molla wrote:

Hello,

I noticed some odd behaviors with idlestate.c/execute() function.

1. I wrote a sample program hello.c with infinite loop -

 int main() {
   while(1);
 }

 #sudo ../idlestat/idlestat -o /tmp/myoutput ../temp/hello
idlestat never terminates, because there is no alarm set, hello
program never terminates.


Right, this is the expected behavior.


2.  If I change and invoke idlestat with below command -
 #sudo ../idlestat/idlestat -o /tmp/myoutput -t 5 ../temp/hello
  idlestat outputs only below line and exits -

  Total trace buffer: 153896 kB


The running program is not sleeping at all, so the cpu is no going to 
sleep, so no idle traces. Do you have more traces if you do:


int main() {
sleep(3600);
}

?


  What is happening is the below check fails -

   if (WIFSIGNALED(status)  WTERMSIG(status) == SIGTERM) {
 return 0;
   }

   here condition is always false. Because the status variable was
fetched before killing the process. So status does not contain valid
signal number.

   if (waitpid(pid, status, 0)  0) {
  if (errno != EINTR || !sigalrm)
 goto again;
 kill(pid, SIGTERM);
  }
  Possible Fix: call waitpid again after kill and fetch new status.


Right, it should be:

again:
if (waitpid(pid, status, 0)  0) {
if (errno == EINTR  sigalrm)
kill(pid, SIGTERM);
goto again;
}



3. The below check makes idlestat dependent on the hello program run -

 if (WIFEXITED(status)  !WEXITSTATUS(status)) {
 }

 If my program is written to return a non-zero value, the second
condition will be always false.

 So with the below program -
 int main() {
return 1;
}
 idlestat outputs only since WEXITSTATUS(status) returns nonzero
value -
 Total trace buffer: 153896 kB

​   Possible Fix: Just check WIFEXITED(status) and ommit the fetching of
exit status.


Right may be print the exit status of the running program should suffice 
instead of propagating the error code.



In any case, it should not be allowed to run idlestat with -t 0, since
that will results in problem 1 again.


--
Thanks,
- Meraj



--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 2/9] Sort options in alphabetic order of the single-char options

[Daniel Lezcano]

On 05/29/2014 12:17 AM, Amit Kucheria wrote:

Signed-off-by: Amit Kucheria amit.kuche...@linaro.org


Acked-by: Daniel Lezcano daniel.lezc...@linaro.org

--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 3/9] Adding missing 'duration' option in long_options

[Daniel Lezcano]

On 05/29/2014 12:17 AM, Amit Kucheria wrote:

commit 1e80e5eaf71b606405cbdf6751aa7446533d8b3a (idlestat: get ride of
 trace-cmd) seems to have missed adding this

Signed-off-by: Amit Kucheria amit.kuche...@linaro.org


Good catch.

Acked-by: Daniel Lezcano daniel.lezc...@linaro.org


--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 4/9] Specify which options need arguments and which don't to help getopt_long()

[Daniel Lezcano]

On 05/29/2014 12:17 AM, Amit Kucheria wrote:

Signed-off-by: Amit Kucheria amit.kuche...@linaro.org


Acked-by: Daniel Lezcano daniel.lezc...@linaro.org




--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 5/9] Stick to -d for debug according to widely used convention

[Daniel Lezcano]

On 05/29/2014 12:17 AM, Amit Kucheria wrote:

Switch the dump option (which might be used infrequently) to use -m

Signed-off-by: Amit Kucheria amit.kuche...@linaro.org


Acked-by: Daniel Lezcano daniel.lezc...@linaro.org


---
  idlestat.c | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index 48e4166..a3b9363 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -946,7 +946,7 @@ struct cpuidle_cstates *physical_cluster_data(struct 
cpu_physical *s_phy)
  static void help(const char *cmd)
  {
fprintf(stderr,
-   %s [-d|--dump] [-t seconds] -o|--output-file file\n,
+   %s [-m|--dump] [-t seconds] -o|--output-file file\n,
basename(cmd));
  }

@@ -956,10 +956,10 @@ static void version(const char *cmd)
  }

  static struct option long_options[] = {
-   { dump,no_argument,   0, 'd' },
-   { debug,   no_argument,   0, 'g' },
+   { debug,   no_argument,   0, 'd' },
{ help,no_argument,   0, 'h' },
{ iterations,  required_argument, 0, 'i' },
+   { dump,no_argument,   0, 'm' },
{ output-file, required_argument, 0, 'o' },
{ duration,required_argument, 0, 't' },
{ version, no_argument,   0, 'V' },
@@ -985,16 +985,13 @@ int getoptions(int argc, char *argv[], struct 
idledebug_options *options)

int optindex = 0;

-   c = getopt_long(argc, argv, dghi:o:t:V,
+   c = getopt_long(argc, argv, dhi:mo:t:V,
long_options, optindex);
if (c == -1)
break;

switch (c) {
case 'd':
-   options-dump = true;
-   break;
-   case 'g':
options-debug = true;
break;
case 'h':
@@ -1004,6 +1001,9 @@ int getoptions(int argc, char *argv[], struct 
idledebug_options *options)
case 'i':
options-iterations = atoi(optarg);
break;
+   case 'm':
+   options-dump = true;
+   break;
case 'o':
options-filename = optarg;
break;




--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 6/9] Make getopt_long handling more robust

[Daniel Lezcano]

On 05/29/2014 12:17 AM, Amit Kucheria wrote:

Adding ':' as first character of optstring to getopt_long allows separating
error message for invalid option and invalid option arguments

Signed-off-by: Amit Kucheria amit.kuche...@linaro.org


Acked-by: Daniel Lezcano daniel.lezc...@linaro.org


---
  idlestat.c | 15 ++-
  1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index a3b9363..96fc1f2 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -985,7 +985,7 @@ int getoptions(int argc, char *argv[], struct 
idledebug_options *options)

int optindex = 0;

-   c = getopt_long(argc, argv, dhi:mo:t:V,
+   c = getopt_long(argc, argv, :dhi:mo:t:V,
long_options, optindex);
if (c == -1)
break;
@@ -1014,11 +1014,16 @@ int getoptions(int argc, char *argv[], struct 
idledebug_options *options)
version(argv[0]);
exit(0);
break;
-   case '?':
-   fprintf(stderr, %s: Unknown option %c'.\n,
+   case 0: /* getopt_long() set a variable, just keep going */
+   break;
+   case ':':   /* missing option argument */
+   fprintf(stderr, %s: option `-%c' requires an 
argument\n,
argv[0], optopt);
-   /* fall through */
+   return -1;
+   case '?':   /* invalid option */
default:
+   fprintf(stderr, %s: Unknown option `-%c'.\n,
+   argv[0], optopt);
return -1;
}
}
@@ -1027,7 +1032,7 @@ int getoptions(int argc, char *argv[], struct 
idledebug_options *options)
fprintf(stderr, dump values must be a positive value\n);

if (NULL == options-filename) {
-   fprintf(stderr, expected filename\n);
+   fprintf(stderr, expected -o filename\n);
return -1;
}





--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 7/9] struct option long_options doesn't need to be global

[Daniel Lezcano]

On 05/29/2014 12:17 AM, Amit Kucheria wrote:

Change the flag field to NULL while we're at it

Signed-off-by: Amit Kucheria amit.kuche...@linaro.org
---
  idlestat.c | 21 ++---
  1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index 96fc1f2..c4a405a 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -955,17 +955,6 @@ static void version(const char *cmd)
printf(%s version %s\n, basename(cmd), IDLESTAT_VERSION);
  }

-static struct option long_options[] = {
-   { debug,   no_argument,   0, 'd' },
-   { help,no_argument,   0, 'h' },
-   { iterations,  required_argument, 0, 'i' },
-   { dump,no_argument,   0, 'm' },
-   { output-file, required_argument, 0, 'o' },
-   { duration,required_argument, 0, 't' },
-   { version, no_argument,   0, 'V' },
-   { 0, 0, 0, 0 }
-};
-
  struct idledebug_options {
bool debug;
bool dump;
@@ -983,6 +972,16 @@ int getoptions(int argc, char *argv[], struct 
idledebug_options *options)

while (1) {

+   static struct option long_options[] = {
+   { debug,   no_argument,   NULL, 'd' },
+   { help,no_argument,   NULL, 'h' },
+   { iterations,  required_argument, NULL, 'i' },
+   { dump,no_argument,   NULL, 'm' },
+   { output-file, required_argument, NULL, 'o' },
+   { duration,required_argument, NULL, 't' },
+   { version, no_argument,   NULL, 'V' },
+   { 0, 0, 0, 0 }
+   };



Why not move this declaration at the beginning of the function and 
remove the 'static' ?



int optindex = 0;

c = getopt_long(argc, argv, :dhi:mo:t:V,




--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 9/9] idledebug? When did we ever call it that?

[Daniel Lezcano]

On 05/29/2014 12:17 AM, Amit Kucheria wrote:

Signed-off-by: Amit Kucheria amit.kuche...@linaro.org


Yep, powerdebug = idledebug = idlestat :)

Legacy copy/paste from powerdebug.

Acked-by: Daniel Lezcano daniel.lezc...@linaro.org


---
  idlestat.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index 849b6dd..00b2032 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -959,7 +959,7 @@ static void version(const char *cmd)
printf(%s version %s\n, basename(cmd), IDLESTAT_VERSION);
  }

-struct idledebug_options {
+struct program_options {
bool debug;
bool dump;
int iterations;
@@ -967,7 +967,7 @@ struct idledebug_options {
char *filename;
  };

-int getoptions(int argc, char *argv[], struct idledebug_options *options)
+int getoptions(int argc, char *argv[], struct program_options *options)
  {
int c;

@@ -1135,7 +1135,7 @@ static void sighandler(int sig)
  }

  static int execute(int argc, char *argv[], char *const envp[],
-  struct idledebug_options *options)
+  struct program_options *options)
  {
pid_t pid;
int status;
@@ -1192,7 +1192,7 @@ int main(int argc, char *argv[], char *const envp[])
  {
struct cpuidle_datas *datas;
struct cpuidle_datas *cluster;
-   struct idledebug_options options;
+   struct program_options options;
struct rusage rusage;
int args;





--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 8/9] improve help - show all available options

[Daniel Lezcano]

On 05/29/2014 12:17 AM, Amit Kucheria wrote:

Signed-off-by: Amit Kucheria amit.kuche...@linaro.org


Acked-by: Daniel Lezcano daniel.lezc...@linaro.org


---
  README |  7 +--
  idlestat.c | 11 ---
  2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/README b/README
index 3a8eb49..24485c5 100644
--- a/README
+++ b/README
@@ -28,14 +28,17 @@ Requirements
  1. It must be run as root in order to access /sys/kernel/debug.
  2. It requires tracing to be enabled in the kernel

+Help
+
+./idlestat -h will show all the options
+
  Example Usage
  -

-./idlestat -d -o myoutput -t 10
+./idlestat -o /tmp/myoutput -t 10

  where,

--d : dump the states at the end
  -o : output file to store the traces
  -t : the duration in seconds

diff --git a/idlestat.c b/idlestat.c
index c4a405a..849b6dd 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -946,8 +946,12 @@ struct cpuidle_cstates *physical_cluster_data(struct 
cpu_physical *s_phy)
  static void help(const char *cmd)
  {
fprintf(stderr,
-   %s [-m|--dump] [-t seconds] -o|--output-file file\n,
+   \nUsage:\n%s -o|--output-file file [-m|--dump]
+[-t|--duration seconds] [-i|--iterations number]
+[-d|--debug]\n,
basename(cmd));
+   fprintf(stderr,
+   \nExample:\n%s -o /tmp/myoutput -t 30\n, basename(cmd));
  }

  static void version(const char *cmd)
@@ -1017,12 +1021,13 @@ int getoptions(int argc, char *argv[], struct 
idledebug_options *options)
break;
case ':':   /* missing option argument */
fprintf(stderr, %s: option `-%c' requires an 
argument\n,
-   argv[0], optopt);
+   basename(argv[0]), optopt);
return -1;
case '?':   /* invalid option */
default:
fprintf(stderr, %s: Unknown option `-%c'.\n,
-   argv[0], optopt);
+   basename(argv[0]), optopt);
+   help(argv[0]);
return -1;
}
}




--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH PMQA] cpuidle_killer: allocate per cpu process array dynamically

[Daniel Lezcano]

On 04/30/2014 10:34 AM, Mohammad Merajul Islam Molla wrote:

Hello Sanjay,
As far I know, if option argument is 0, the parent will wait for
specified child pid to terminate, its not for immediate return as in
case of WNOHANG. This is probably the intended use of the code (author
will be able to confirm). Changing 0 to WNOHANG macro will change the
meaning of the code.


Right.


Also, from header file -
#define WNOHANG 0x0001
WNOHANG is defined as 1, not zero.
Which makes me think of two cases below -
1. If the real intended purpose is to not to wait infinitely, replacing
0 with WNOHANG macro will do.
2. But if the real intended purpose is to wait infinitely, then the
solution I proposed earlier should be real fix, i.e.,
 if (pids[i] != 0)
  waitpid(pids[i], status, 0); [ and keeping options
argument as 0]
--


There is an alternative if you want to simplify the code.

Instead of using nrcpus and waitpid, use *wait*.

for (;;) {
pid_t pid = wait(status);

if (pid  0  errno == ECHILD)
break;

if (status != 0)
fprintf(stderr, blabla \n);
}



Thanks,
-Meraj

On Wed, Apr 30, 2014 at 1:25 PM, Sanjay Singh Rawat
sanjay.ra...@linaro.org mailto:sanjay.ra...@linaro.org wrote:

On Wednesday 30 April 2014 12:14 PM, Mohammad Merajul Islam Molla wrote:

Hello,

I would like to share two observations -

1. Is it necessary to initialize nrcpus = 2 anymore?


thanks, ack


2. Another problem may happen in the code below where waitpid is
called -

   for (i = 0; i  nrcpus; i++) {
  int status;

  waitpid(pids[i], status, 0);
  if (status != 0) {
  fprintf(stderr, test for cpu %d has
failed\n, i);
  ret = 1;
  }
  }

 Since for offline cpus, no child process is created, now
these cpus
pid[i]'s will be zero (due to calloc). This will change the
meaning of
waitpid function as man page says -

  pid 0  -meaning wait for any child process whose
process group
ID is equal to that of the calling process.

I think a check should be added before waitpid call -

  if (pids[i] != 0)
  waitpid(pids[i], status, 0);


Here Parent will not wait for child infinitely if status is not
visible, the option argument is 0(NOHANG). I will add the macro for
readability. thanks



​--​
​Thanks,
-Meraj​


On Wed, Apr 30, 2014 at 11:08 AM, Sanjay Singh Rawat
sanjay.ra...@linaro.org mailto:sanjay.ra...@linaro.org
mailto:sanjay.rawat@linaro.__org
mailto:sanjay.ra...@linaro.org wrote:

 currently percpu process array is set to 2, which results
in segfault

 Signed-off-by: Sanjay Singh Rawat sanjay.ra...@linaro.org
mailto:sanjay.ra...@linaro.org
 mailto:sanjay.rawat@linaro.__org
mailto:sanjay.ra...@linaro.org
 ---
   cpuidle/cpuidle_killer.c |7 ++-
   1 file changed, 6 insertions(+), 1 deletion(-)

 diff --git a/cpuidle/cpuidle_killer.c
b/cpuidle/cpuidle_killer.c
 index 5e7320f..09009ef 100644
 --- a/cpuidle/cpuidle_killer.c
 +++ b/cpuidle/cpuidle_killer.c
 @@ -104,7 +104,7 @@ int main(int argc, char *argv[])
   {
  int ret, i, nrcpus = 2;
  int nrsleeps, delay;
 -   pid_t pids[nrcpus];
 +   pid_t *pids;
  struct timex timex = { 0 };

  if (adjtimex(timex)  0) {
 @@ -121,6 +121,11 @@ int main(int argc, char *argv[])
  }

  fprintf(stderr, found %d cpu(s)\n, nrcpus);
 +   pids = (pid_t *) calloc(nrcpus, sizeof(pid_t));
 +   if (pids == NULL) {
 +   fprintf(stderr, error: calloc failed\n);
 +   return 1;
 +   }

  for (i = 0; i  nrcpus; i++) {

 --
 1.7.10.4


 _
 linaro-dev mailing list
linaro-dev@lists.linaro.org mailto:linaro-dev@lists.linaro.org
mailto:linaro-dev@lists.__linaro.org
mailto:linaro-dev@lists.linaro.org
http://lists.linaro.org/__mailman/listinfo/linaro-dev
http://lists.linaro.org/mailman/listinfo/linaro-dev




--
sanjay




--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ 

Re: Arndale 5250 crash when unplugging CPU1

[Daniel Lezcano]

On 04/14/2014 08:29 AM, Chander Kashyap wrote:

Hi Daniel,

I am using ML u-boot and ML kernel with ramdisk as rootfs for my testing.


Ok thx for the info. I am trying with the ML u-boot.

  -- Daniel


On 13 April 2014 03:49, Victor Kamensky victor.kamen...@linaro.org wrote:

Hi Daniel,

Note these patches:

https://groups.google.com/a/linaro.org/forum/#!searchin/linaro-networking/Arndale$20hotplug/linaro-networking/MdvG8m_Bjms/YSvvIYKo4NQJ

Look at PATCH 1/2. It is about wfi instruction hex code use in
CONFIG_THUMB2_KERNEL case.
Even if patch tile says BE image the issue is applicable to LE thumb2
issue. Check
your kernel config if THUMB2_KERNEL set the issue is applicable to you.

Thanks,
Victor

On 12 April 2014 14:51, Daniel Lezcano daniel.lezc...@linaro.org wrote:


Hi all,

I used the image:

 arndale-saucy_server_20140222-611.img

I boots well but when I unplug CPU1, the kernel crashes.

It seems related to the AFTR state (an idle state of cpuidle) and the u-boot
version which is 2013-rc1. It seems this issue does not happen with a more
recent u-boot (I was not yet able to test it).

I don't know where to fill the bug for this release.

Chander, any pointer to the right u-boot version you are using successfully
?

Thanks

   -- Daniel

--
  http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev







--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: Arndale 5250 crash when unplugging CPU1

[Daniel Lezcano]

On 04/14/2014 08:29 AM, Chander Kashyap wrote:

Hi Daniel,

I am using ML u-boot and ML kernel with ramdisk as rootfs for my testing.


Hi Chander,

I tried to use u-boot ML but it fails with the error:

** Unrecognized filesystem type **
** Unrecognized filesystem type **
** Unrecognized filesystem type **
Wrong Image Format for bootm command
ERROR: can't get kernel image!
ARNDALE # fatls mmc 0:2
** Unrecognized filesystem type **
ARNDALE # fatls mmc 0:1

0 file(s), 0 dir(s)

ARNDALE # fatls mmc 0:3
** Unrecognized filesystem type **
ARNDALE # fatls mmc 0:

In order to set it up I did:

make arndale_config all CROSS_COMPILE=arm-linux-gnueabi-

sudo dd if=arndale-bl1.bin of=/dev/mmcblk0 bs=512 seek=1
sudo dd if=spl/arndale-spl.bin of=/dev/mmcblk0 bs=512 seek=17
sudo dd if=u-boot-dtb.bin of=/dev/mmcblk0 bs=512 seek=49

fdisk -l /dev/mmcblk0

Disk /dev/mmcblk0: 3951 MB, 3951034368 bytes
128 heads, 32 sectors/track, 1884 cylinders, total 7716864 sectors
Units = sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disk identifier: 0x

Device Boot  Start End  Blocks   Id  System
/dev/mmcblk0p1   181914095+  da  Non-FS data
/dev/mmcblk0p2   *8192  114687   53248c  W95 FAT32 (LBA)
/dev/mmcblk0p3  114688 2097151  991232   83  Linux

Did I missed something ?

Thanks
  -- Daniel


On 13 April 2014 03:49, Victor Kamensky victor.kamen...@linaro.org wrote:

Hi Daniel,

Note these patches:

https://groups.google.com/a/linaro.org/forum/#!searchin/linaro-networking/Arndale$20hotplug/linaro-networking/MdvG8m_Bjms/YSvvIYKo4NQJ

Look at PATCH 1/2. It is about wfi instruction hex code use in
CONFIG_THUMB2_KERNEL case.
Even if patch tile says BE image the issue is applicable to LE thumb2
issue. Check
your kernel config if THUMB2_KERNEL set the issue is applicable to you.

Thanks,
Victor

On 12 April 2014 14:51, Daniel Lezcano daniel.lezc...@linaro.org wrote:


Hi all,

I used the image:

 arndale-saucy_server_20140222-611.img

I boots well but when I unplug CPU1, the kernel crashes.

It seems related to the AFTR state (an idle state of cpuidle) and the u-boot
version which is 2013-rc1. It seems this issue does not happen with a more
recent u-boot (I was not yet able to test it).

I don't know where to fill the bug for this release.

Chander, any pointer to the right u-boot version you are using successfully
?

Thanks

   -- Daniel

--
  http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev







--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: Arndale 5250 crash when unplugging CPU1

[Daniel Lezcano]

On 04/13/2014 12:19 AM, Victor Kamensky wrote:

Hi Daniel,

Note these patches:

https://groups.google.com/a/linaro.org/forum/#!searchin/linaro-networking/Arndale$20hotplug/linaro-networking/MdvG8m_Bjms/YSvvIYKo4NQJ

Look at PATCH 1/2. It is about wfi instruction hex code use in
CONFIG_THUMB2_KERNEL case.
Even if patch tile says BE image the issue is applicable to LE thumb2
issue. Check
your kernel config if THUMB2_KERNEL set the issue is applicable to you.


Hi Victor, thanks for your help.

so for the record, the issue happens when a mainline kernel is used with 
a Linaro u-boot which enables some hypervisor things (while the mainline 
u-boot doesn't). So an additional patch must be used for the kernel to 
get/set some more resources [1]


Thanks

  -- Daniel

[1] 
https://git.linaro.org/landing-teams/working/samsung/kernel.git/commit/d0799d335287e87ffdcfbbbe6ef09a2c07a55ee0




On 12 April 2014 14:51, Daniel Lezcano daniel.lezc...@linaro.org wrote:


Hi all,

I used the image:

 arndale-saucy_server_20140222-611.img

I boots well but when I unplug CPU1, the kernel crashes.

It seems related to the AFTR state (an idle state of cpuidle) and the u-boot
version which is 2013-rc1. It seems this issue does not happen with a more
recent u-boot (I was not yet able to test it).

I don't know where to fill the bug for this release.

Chander, any pointer to the right u-boot version you are using successfully
?

Thanks

   -- Daniel

--
  http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev



--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Arndale 5250 crash when unplugging CPU1

[Daniel Lezcano]

Hi all,

I used the image:

arndale-saucy_server_20140222-611.img

I boots well but when I unplug CPU1, the kernel crashes.

It seems related to the AFTR state (an idle state of cpuidle) and the 
u-boot version which is 2013-rc1. It seems this issue does not happen 
with a more recent u-boot (I was not yet able to test it).


I don't know where to fill the bug for this release.

Chander, any pointer to the right u-boot version you are using 
successfully ?


Thanks

  -- Daniel

--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH] Produce a situation where L1 cache flush missing vs cpu power down hangs the system

[Daniel Lezcano]

Hi Sandeep,


On 04/09/2014 07:15 AM, Sandeep Tripathy wrote:

  Hi Daniel,
  - L1 D$ clean ( SCTLR C bit clear, DCCISW, clear SMP) is part
of the recommended sequence for Individual core power down .


Yes, absolutely. It is what Lorenzo and I discussed.

The macro v7_exit_coherency_flush should be used for that.


  - If a core is powered down having dirty lines in L1 then the
system should encounter an issue (abort) very easily. May be the first
idle attempt itself is sufficient to break things.




   Does any platform (exynos4 ?) work without doing L1 D cache
clean in cpu idle individual core power down sequence ?


Yes, the power down sequence is done but without L1 D cache clean and it 
is very very hard to make the board hang. It is so rare, I can't say 
100% it is related to the driver itself or something else.


If there is a way to spot the issue, I will be happy to test it.

Thanks
  -- Daniel



On 8 April 2014 17:07, Daniel Lezcano daniel.lezc...@linaro.org
mailto:daniel.lezc...@linaro.org wrote:

On 04/08/2014 01:07 PM, Amit Kucheria wrote:

Hi Daniel,

Have you noticed this on any platform yet with this test?


I have noticed a very very rare hang on the exynos4 board with this
test and the dual cpu support but it is not reproducible enough to
check if the cache flush fixes it or not (or it is related to the
cpuidle driver). I had a long discussion yesterday with Lorenzo who
explained me what could happen without flushing the cache and
exiting the SMP mode.

AFAICT,

* The tc2 flushes its cache.

* exynos5 still need to disable cpu1 to enter the AFTR state (which
is broken today), so cache is flushed in the hotplug code path. I
hope I can spot the issue with a quad core.

* omap4 flushes its cache.

* omap3 is not concerned by this because it is an UP system.

* vinci, s3c64, imx5, imx6, ux500, kirkwood does not support cpu
power down.

* calxeda hides that through the firmware I believe

* I don't know for tegra but I assume they are flushing and
disabling the cache

I hope with this test we can spot the issue, if any, with multiple
runs on the boards, especially when new drivers will be implemented.

--
  http://www.linaro.org/ Linaro.org │ Open source software for ARM
SoCs

Follow Linaro:  http://www.facebook.com/__pages/Linaro
http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/__linaroorg
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-__blog/
http://www.linaro.org/linaro-blog/ Blog


_
linaro-dev mailing list
linaro-dev@lists.linaro.org mailto:linaro-dev@lists.linaro.org
http://lists.linaro.org/__mailman/listinfo/linaro-dev
http://lists.linaro.org/mailman/listinfo/linaro-dev





--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH] Produce a situation where L1 cache flush missing vs cpu power down hangs the system

[Daniel Lezcano]

On 04/09/2014 02:19 PM, Sandeep Tripathy wrote:


Hi Daniel,
   Just for test you may try removing the flush_cache_louis();
from __cpu_suspend_save .
   Because if the driver is using cpu_suspend() in idle path then
almost all the important data on core L1 is clean.
   Still it can fail iff the code after that ( cpu_suspend())
modifies some cacheable important data because SCTLR.C bit
   is not cleared yet.
   It should just fail if no cache clean is done before powering down.
   And it should work  iff  v7_exit_coherency_flush() or similar is
done.
   Note: This is based on my observation on A7 quad. Please correct
me if the understanding is wrong.


Thanks Sandeep for the info.

I think Lorenzo spotted the SCTLR.C bit must be cleared before powering 
down the cpu and what does cpu_suspend is not enough because some data 
could be fetch from the other cpu. The only way to properly handle this 
is to call always v7_exit_coherency_flush before powering down the cpu.




On 9 April 2014 13:24, Daniel Lezcano daniel.lezc...@linaro.org
mailto:daniel.lezc...@linaro.org wrote:


Hi Sandeep,



On 04/09/2014 07:15 AM, Sandeep Tripathy wrote:

   Hi Daniel,
   - L1 D$ clean ( SCTLR C bit clear, DCCISW, clear SMP)
is part
of the recommended sequence for Individual core power down .


Yes, absolutely. It is what Lorenzo and I discussed.

The macro v7_exit_coherency_flush should be used for that.


   - If a core is powered down having dirty lines in L1
then the
system should encounter an issue (abort) very easily. May be the
first
 idle attempt itself is sufficient to break things.



Does any platform (exynos4 ?) work without doing L1
D cache
clean in cpu idle individual core power down sequence ?


Yes, the power down sequence is done but without L1 D cache clean
and it is very very hard to make the board hang. It is so rare, I
can't say 100% it is related to the driver itself or something else.

If there is a way to spot the issue, I will be happy to test it.

Thanks
   -- Daniel


On 8 April 2014 17:07, Daniel Lezcano daniel.lezc...@linaro.org
mailto:daniel.lezc...@linaro.org
mailto:daniel.lezcano@linaro.__org
mailto:daniel.lezc...@linaro.org wrote:

 On 04/08/2014 01:07 PM, Amit Kucheria wrote:

 Hi Daniel,

 Have you noticed this on any platform yet with this test?


 I have noticed a very very rare hang on the exynos4 board
with this
 test and the dual cpu support but it is not reproducible
enough to
 check if the cache flush fixes it or not (or it is related
to the
 cpuidle driver). I had a long discussion yesterday with
Lorenzo who
 explained me what could happen without flushing the cache and
 exiting the SMP mode.

 AFAICT,

 * The tc2 flushes its cache.

 * exynos5 still need to disable cpu1 to enter the AFTR
state (which
 is broken today), so cache is flushed in the hotplug code
path. I
 hope I can spot the issue with a quad core.

 * omap4 flushes its cache.

 * omap3 is not concerned by this because it is an UP system.

 * vinci, s3c64, imx5, imx6, ux500, kirkwood does not
support cpu
 power down.

 * calxeda hides that through the firmware I believe

 * I don't know for tegra but I assume they are flushing and
 disabling the cache

 I hope with this test we can spot the issue, if any, with
multiple
 runs on the boards, especially when new drivers will be
implemented.

 --
   http://www.linaro.org/ Linaro.org │ Open source
software for ARM
 SoCs

 Follow Linaro:  http://www.facebook.com/pages/Linaro
http://www.facebook.com/__pages/Linaro
 http://www.facebook.com/__pages/Linaro
http://www.facebook.com/pages/Linaro Facebook |
 http://twitter.com/#!/linaroorg
http://twitter.com/#!/__linaroorg
 http://twitter.com/#!/__linaroorg
http://twitter.com/#!/linaroorg Twitter |
 http://www.linaro.org/linaro-blog/
http://www.linaro.org/linaro-__blog/
 http://www.linaro.org/linaro-__blog/
http://www.linaro.org/linaro-blog/ Blog


 ___
 linaro-dev mailing list
linaro-dev@lists.linaro.org mailto:linaro-dev@lists.linaro.org
mailto:linaro-dev@lists.__linaro.org
mailto:linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH] Produce a situation where L1 cache flush missing vs cpu power down hangs the system

[Daniel Lezcano]

As pointed by Lorenzo, when a cpu powers down, the L1 cache must be flushed
before, otherwise:

 * data cachelines are not empty and the other cpu may fetch data
 * cpu will lost some data leading to a memory corruption

Note this bug is very difficult to reproduce and this test will not spot the
issue everytime.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 cpuidle/cpuidle-l1.c   |   72 
 cpuidle/cpuidle_05.sh  |   42 
 cpuidle/cpuidle_05.txt |1 +
 3 files changed, 115 insertions(+)
 create mode 100644 cpuidle/cpuidle-l1.c
 create mode 100755 cpuidle/cpuidle_05.sh
 create mode 100644 cpuidle/cpuidle_05.txt

diff --git a/cpuidle/cpuidle-l1.c b/cpuidle/cpuidle-l1.c
new file mode 100644
index 000..bbcde28
--- /dev/null
+++ b/cpuidle/cpuidle-l1.c
@@ -0,0 +1,72 @@
+#include stdio.h
+#include stdlib.h
+#include sys/types.h
+#include sys/wait.h
+#include unistd.h
+#include pthread.h
+#define BUFSIZE (4*1024)
+#define DEADBEEF 0xDEADBEEF
+static int buffer[BUFSIZE];
+
+static pthread_t threads[64];
+
+void *thread_routine(void *arg)
+{
+   int i, display = *(int *)arg;
+   int dummy;
+
+   for (i = 0; i  100; i++) {
+
+   int j;
+   
+   for (j = 0; j  BUFSIZE * 1000; j++) {
+   dummy = buffer[j % BUFSIZE];
+   dummy++;
+   }
+   
+   usleep(20);
+   
+   if (buffer[i] != DEADBEEF) {
+   fprintf(stderr, memory corruption\n);
+   return (void *)-1;
+   }
+
+   if (display == 0)
+   printf(%d%%%s, i, i  10 ? \b\b : \b\b\b);
+   }
+
+   if (display == 0)
+   printf(\b\b\b\b);
+
+   return NULL;
+}
+
+int main(int argc, char *argv[])
+{
+   int i, ret = 0;
+   int nrcpus = sysconf(_SC_NPROCESSORS_ONLN);
+
+   for (i = 0; i  BUFSIZE; i++)
+   buffer[i] = DEADBEEF;
+
+   setbuf(stdout, NULL);
+
+   for(i = 0; i  nrcpus; i++) {
+
+   if (pthread_create(threads[i], NULL, thread_routine, i)) {
+   perror(pthread_create);
+   return 1;
+   }
+
+   }
+
+   for (i = 0; i  nrcpus; i++) {
+   void *result;
+   pthread_join(threads[i], result);
+
+   if (result == (void *)-1)
+   ret = 1;
+   }
+
+   return ret; 
+}
diff --git a/cpuidle/cpuidle_05.sh b/cpuidle/cpuidle_05.sh
new file mode 100755
index 000..679439d
--- /dev/null
+++ b/cpuidle/cpuidle_05.sh
@@ -0,0 +1,42 @@
+#!/bin/bash
+#
+# PM-QA validation test suite for the power management on Linux
+#
+# Copyright (C) 2011, Linaro Limited.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, 
USA.
+#
+# Contributors:
+# Daniel Lezcano daniel.lezc...@linaro.org (IBM Corporation)
+#   - initial API and implementation
+#
+
+# URL : 
https://wiki.linaro.org/WorkingGroups/PowerManagement/Resources/TestSuite/PmQaSpecification#cpuidle_05
+
+source ../include/functions.sh
+
+CPUIDLE_L1=./cpuidle-l1
+
+if [ $(id -u) -ne 0 ]; then
+log_skip run as non-root
+exit 0
+fi
+
+check_cpuidle_l1() {
+check Fill L1 cache and sleep ./$CPUIDLE_L1
+}
+
+check_cpuidle_l1
+test_status_show
diff --git a/cpuidle/cpuidle_05.txt b/cpuidle/cpuidle_05.txt
new file mode 100644
index 000..1f80e36
--- /dev/null
+++ b/cpuidle/cpuidle_05.txt
@@ -0,0 +1 @@
+Run cpuidle L1 test program to catch L1 flush missing vs cpu power down
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH] Produce a situation where L1 cache flush missing vs cpu power down hangs the system

[Daniel Lezcano]

On 04/08/2014 01:07 PM, Amit Kucheria wrote:

Hi Daniel,

Have you noticed this on any platform yet with this test?


I have noticed a very very rare hang on the exynos4 board with this test 
and the dual cpu support but it is not reproducible enough to check if 
the cache flush fixes it or not (or it is related to the cpuidle 
driver). I had a long discussion yesterday with Lorenzo who explained me 
what could happen without flushing the cache and exiting the SMP mode.


AFAICT,

* The tc2 flushes its cache.

* exynos5 still need to disable cpu1 to enter the AFTR state (which is 
broken today), so cache is flushed in the hotplug code path. I hope I 
can spot the issue with a quad core.


* omap4 flushes its cache.

* omap3 is not concerned by this because it is an UP system.

* vinci, s3c64, imx5, imx6, ux500, kirkwood does not support cpu power down.

* calxeda hides that through the firmware I believe

* I don't know for tegra but I assume they are flushing and disabling 
the cache


I hope with this test we can spot the issue, if any, with multiple runs 
on the boards, especially when new drivers will be implemented.


--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH] idlestat: add timeout for the exec'ed command

[daniel . lezcano]

From: Daniel Lezcano daniel.lezc...@linaro.org

Add a timeout for the specified command.

The timeout option is shared with the -t option. When no command line has to be
executed, the timeout becomes the duration of the acquisition, otherwise the
command is executed during an amount of time and terminated if the timeout
occur.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   36 ++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index 7ed9f85..d9ce30a 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -25,6 +25,7 @@
  *
  */
 #define _GNU_SOURCE
+#include errno.h
 #include getopt.h
 #include stdio.h
 #include stdlib.h
@@ -1126,6 +1127,14 @@ static int idlestat_wake_all(void)
return 0;
 }
 
+static volatile sig_atomic_t sigalrm = 0;
+
+static void sighandler(int sig)
+{
+   if (sig == SIGALRM)
+   sigalrm = 1;
+}
+
 static int execute(int argc, char *argv[], char *const envp[],
   struct idledebug_options *options)
 {
@@ -1148,9 +1157,32 @@ static int execute(int argc, char *argv[], char *const 
envp[],
}
 
if (pid) {
-   waitpid(pid, status, 0);
 
-   if (WIFEXITED(status)  !WEXITSTATUS(status))
+   struct sigaction s = {
+   .sa_handler = sighandler,
+   .sa_flags = SA_RESETHAND,
+   };
+
+   sigaddset(s.sa_mask, SIGALRM);
+   sigaction(SIGALRM, s, NULL);
+   alarm(options-duration);
+   again:
+   if (waitpid(pid, status, 0)  0) {
+   if (errno != EINTR || !sigalrm)
+   goto again;
+   kill(pid, SIGTERM);
+   }
+
+   if (WIFEXITED(status)  !WEXITSTATUS(status)) {
+   /*
+* Cancel the timer in case the program
+* finished before the timeout
+*/
+   alarm(0);
+   return 0;
+   }
+
+   if (WIFSIGNALED(status)  WTERMSIG(status) == SIGTERM)
return 0;
}
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH] idlestat: run a command with idlestat

[daniel . lezcano]

From: Daniel Lezcano daniel.lezc...@linaro.org

Add the support to run a command line with idlestat:

./idlestat -o out.dat -- /bin/sleep 3

Without the '-t' option, idlestat will blindly use the current buffer
size which may not fit with the 'command' duration and lead to a buffer
overflow.

With the example, it would make sense to set the buffer size:

./idlestat -o out.dat -t 3 -- /bin/sleep 3

This is a workaround, until we find a way to set the buffer size automatically.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   49 +++--
 1 file changed, 43 insertions(+), 6 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index 53ca1c7..f0c6467 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -36,6 +36,7 @@
 #include sys/time.h
 #include sys/types.h
 #include sys/resource.h
+#include sys/wait.h
 #include assert.h
 
 #include idlestat.h
@@ -112,6 +113,7 @@ static int display_states(struct cpuidle_cstates *cstates,
(c-min_time == DBL_MAX ? 0. : c-min_time),
c-max_time);
}
+
if (pstates) {
for (j = 0; j  pstates-max; j++) {
struct cpufreq_pstate *p = (pstates-pstate[j]);
@@ -1022,7 +1024,7 @@ int getoptions(int argc, char *argv[], struct 
idledebug_options *options)
return -1;
}
 
-   return 0;
+   return optind;
 }
 
 static int idlestat_file_for_each_line(const char *path, void *data,
@@ -1108,14 +1110,47 @@ static int idlestat_wake_all(void)
return 0;
 }
 
-int main(int argc, char *argv[])
+static int execute(int argc, char *argv[], char *const envp[],
+  struct idledebug_options *options)
+{
+   pid_t pid;
+   int status;
+
+   /* Nothing to execute, just wait an amount of time */
+   if (!argc)
+   return sleep(options-duration);
+
+   pid = fork();
+   if (pid  0) {
+   perror(fork);
+   return -1;
+   }
+
+   if (pid == 0  execve(argv[0], argv, envp)) {
+   perror(execv);
+   exit(1);
+   }
+
+   if (pid) {
+   waitpid(pid, status, 0);
+
+   if (WIFEXITED(status)  !WEXITSTATUS(status))
+   return 0;
+   }
+
+   return -1;
+}
+
+int main(int argc, char *argv[], char *const envp[])
 {
struct cpuidle_datas *datas;
struct cpuidle_datas *cluster;
struct idledebug_options options;
struct rusage rusage;
+   int args;
 
-   if (getoptions(argc, argv, options))
+   args = getoptions(argc, argv, options);
+   if (args = 0)
return 1;
 
/* We have to manipulate some files only accessible to root */
@@ -1128,7 +1163,7 @@ int main(int argc, char *argv[])
init_cpu_topo_info();
 
/* Acquisition time specified means we will get the traces */
-   if (options.duration) {
+   if (options.duration || args  argc) {
 
/* Read cpu topology info from sysfs */
read_sysfs_cpu_topo();
@@ -1151,14 +1186,16 @@ int main(int argc, char *argv[])
/* Start the recording */
if (idlestat_trace_enable(true))
return -1;
+
/* We want to prevent to begin the acquisition with a cpu in
 * idle state because we won't be able later to close the
 * state and to determine which state it was. */
if (idlestat_wake_all())
return -1;
 
-   /* Do nothing */
-   sleep(options.duration);
+   /* Execute the command or wait a specified delay */
+   if (execute(argc - args, argv[args], envp, options))
+   return -1;
 
/* Wake up all cpus again to account for last idle state */
if (idlestat_wake_all())
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[GIT PULL] idlestat: some fixes on error path

[Daniel Lezcano]

Hi Zoran,

Colin King sent me some patches to fixup memory leaks and some nits in 
the error code path.


Please, pull these changes in your tree.

The following changes since commit da6a8c94a8f8124711db0ae84a3ef4e0e186b388:

  Fixing improperly initialized cstate_max per-CPU. (2014-01-29 
15:47:47 -0800)


are available in the git repository at:

  git://git.linaro.org/people/daniel.lezcano/idlestat.git master

for you to fetch changes up to 41505eca08f1bd0f2d35733cd631be5967e38724:

  Use the correct format specifier for size_t type (2014-02-18 08:51:30 
+0100)



Colin Ian King (6):
  Write open failure message to stderr
  Free memory on realloc failure.
  Fix memory leak, result is being leaked on a realloc failure
  check s_cpu rather than s_core on memory allocation
  Fix file and memory resource leak on error exit in idlestat_load
  Use the correct format specifier for size_t type

 idlestat.c |   36 ++--
 topology.c |2 +-
 2 files changed, 27 insertions(+), 11 deletions(-)


--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 1/6] Write open failure message to stderr

[Daniel Lezcano]

From: Colin Ian King colin.k...@canonical.com

Error message should be written to stderr rather than the failed
to open file.

Signed-off-by: Colin Ian King colin.k...@canonical.com
Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/idlestat.c b/idlestat.c
index 3abfaaf..fce6112 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -659,7 +659,7 @@ static int idlestat_file_for_each_line(const char *path, 
void *data,
f = fopen(path, r);
 
if (!f) {
-   fprintf(f, failed to open '%s': %m\n, path);
+   fprintf(stderr, failed to open '%s': %m\n, path);
return -1;
}
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 3/6] Fix memory leak, result is being leaked on a realloc failure

[Daniel Lezcano]

From: Colin Ian King colin.k...@canonical.com

A realloc failure leaks result on the error return path.

Signed-off-by: Colin Ian King colin.k...@canonical.com
Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/idlestat.c b/idlestat.c
index 41ed3ab..739f6dc 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -221,6 +221,7 @@ static struct cpuidle_cstate *inter(struct cpuidle_cstate 
*c1,
   (result-nrdata + 1));
if (!tmp) {
free(data);
+   free(result);
return NULL;
}
data = tmp;
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 2/6] Free memory on realloc failure.

[Daniel Lezcano]

From: Colin Ian King colin.k...@canonical.com

Fix a common coding error where realloc fails and the original
allocated data is not free'd.

Signed-off-by: Colin Ian King colin.k...@canonical.com
Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index fce6112..41ed3ab 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -188,6 +188,7 @@ static struct cpuidle_cstate *inter(struct cpuidle_cstate 
*c1,
for (i = 0, index = 0; i  c1-nrdata; i++) {
 
for (j = index; j  c2-nrdata; j++) {
+   struct cpuidle_data *tmp;
 
/* intervals are ordered, no need to go further */
if (c1-data[i].end  c2-data[j].begin)
@@ -216,10 +217,13 @@ static struct cpuidle_cstate *inter(struct cpuidle_cstate 
*c1,
 
result-nrdata++;
 
-   data = realloc(data, sizeof(*data) *
+   tmp = realloc(data, sizeof(*data) *
   (result-nrdata + 1));
-   if (!data)
+   if (!tmp) {
+   free(data);
return NULL;
+   }
+   data = tmp;
 
result-data = data;
result-data[result-nrdata - 1] = *interval;
@@ -236,7 +240,7 @@ static int store_data(double time, int state, int cpu,
 {
struct cpuidle_cstates *cstates = datas-cstates[cpu];
struct cpuidle_cstate *cstate;
-   struct cpuidle_data *data;
+   struct cpuidle_data *data, *tmp;
int nrdata, last_cstate = cstates-last_cstate;
 
/* ignore when we got a closing state first */
@@ -280,9 +284,12 @@ static int store_data(double time, int state, int cpu,
return 0;
}
 
-   data = realloc(data, sizeof(*data) * (nrdata + 1));
-   if (!data)
+   tmp = realloc(data, sizeof(*data) * (nrdata + 1));
+   if (!tmp) {
+   free(data);
return error(realloc data);;
+   }
+   data = tmp;
 
data[nrdata].begin = time;
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 5/6] Fix file and memory resource leak on error exit in idlestat_load

[Daniel Lezcano]

From: Colin Ian King colin.k...@canonical.com

The error returns in idlestat_load fail to close an open file and
free allocated memory on the heap. This patch addresses this.

Signed-off-by: Colin Ian King colin.k...@canonical.com
Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index 739f6dc..0961463 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -396,16 +396,24 @@ static struct cpuidle_datas *idlestat_load(const char 
*path)
sscanf(buffer, cpus=%u, nrcpus);
}
 
-   if (!nrcpus)
+   if (!nrcpus) {
+   fclose(f);
return ptrerror(read error for 'cpus=' in trace file);
+   }
 
datas = malloc(sizeof(*datas));
-   if (!datas)
+   if (!datas) {
+   fclose(f);
return ptrerror(malloc datas);
+   }
 
datas-cstates = calloc(sizeof(*datas-cstates), nrcpus);
-   if (!datas-cstates)
+   if (!datas-cstates) {
+   free(datas);
+   fclose(f);
return ptrerror(calloc cstate);
+   }
+
/* initialize cstate_max for each cpu */
for (cpu = 0; cpu  nrcpus; cpu++)
datas-cstates[cpu].cstate_max = -1;
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 6/6] Use the correct format specifier for size_t type

[Daniel Lezcano]

From: Colin Ian King colin.k...@canonical.com

Signed-off-by: Colin Ian King colin.k...@canonical.com
Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/idlestat.c b/idlestat.c
index 0961463..3f6076e 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -447,7 +447,7 @@ static struct cpuidle_datas *idlestat_load(const char *path)
 
fclose(f);
 
-   fprintf(stderr, Log is %lf secs long with %d events\n,
+   fprintf(stderr, Log is %lf secs long with %zd events\n,
end - begin, (int)count);
 
return datas;
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 4/6] check s_cpu rather than s_core on memory allocation

[Daniel Lezcano]

From: Colin Ian King colin.k...@canonical.com

s_cpu is not being checked for a memory allocation failure, instead
s_core is being checked.  Fix this cut-n-paste error.

Signed-off-by: Colin Ian King colin.k...@canonical.com
Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 topology.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/topology.c b/topology.c
index 052ddb9..9de1354 100644
--- a/topology.c
+++ b/topology.c
@@ -128,7 +128,7 @@ int add_topo_info(struct cpu_topology *topo_list, struct 
topology_info *info)
ptr = check_exist_from_head(s_core-cpu_head, info-cpu_id);
if (!ptr) {
s_cpu = calloc(sizeof(struct cpu_cpu), 1);
-   if (!s_core)
+   if (!s_cpu)
return -1;
 
s_cpu-cpu_id = info-cpu_id;
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 2/6] Free memory on realloc failure.

[Daniel Lezcano]

From: Colin Ian King colin.k...@canonical.com

Fix a common coding error where realloc fails and the original
allocated data is not free'd.

Signed-off-by: Colin Ian King colin.k...@canonical.com
Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index fce6112..41ed3ab 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -188,6 +188,7 @@ static struct cpuidle_cstate *inter(struct cpuidle_cstate 
*c1,
for (i = 0, index = 0; i  c1-nrdata; i++) {
 
for (j = index; j  c2-nrdata; j++) {
+   struct cpuidle_data *tmp;
 
/* intervals are ordered, no need to go further */
if (c1-data[i].end  c2-data[j].begin)
@@ -216,10 +217,13 @@ static struct cpuidle_cstate *inter(struct cpuidle_cstate 
*c1,
 
result-nrdata++;
 
-   data = realloc(data, sizeof(*data) *
+   tmp = realloc(data, sizeof(*data) *
   (result-nrdata + 1));
-   if (!data)
+   if (!tmp) {
+   free(data);
return NULL;
+   }
+   data = tmp;
 
result-data = data;
result-data[result-nrdata - 1] = *interval;
@@ -236,7 +240,7 @@ static int store_data(double time, int state, int cpu,
 {
struct cpuidle_cstates *cstates = datas-cstates[cpu];
struct cpuidle_cstate *cstate;
-   struct cpuidle_data *data;
+   struct cpuidle_data *data, *tmp;
int nrdata, last_cstate = cstates-last_cstate;
 
/* ignore when we got a closing state first */
@@ -280,9 +284,12 @@ static int store_data(double time, int state, int cpu,
return 0;
}
 
-   data = realloc(data, sizeof(*data) * (nrdata + 1));
-   if (!data)
+   tmp = realloc(data, sizeof(*data) * (nrdata + 1));
+   if (!tmp) {
+   free(data);
return error(realloc data);;
+   }
+   data = tmp;
 
data[nrdata].begin = time;
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 4/6] check s_cpu rather than s_core on memory allocation

[Daniel Lezcano]

From: Colin Ian King colin.k...@canonical.com

s_cpu is not being checked for a memory allocation failure, instead
s_core is being checked.  Fix this cut-n-paste error.

Signed-off-by: Colin Ian King colin.k...@canonical.com
Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 topology.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/topology.c b/topology.c
index 052ddb9..9de1354 100644
--- a/topology.c
+++ b/topology.c
@@ -128,7 +128,7 @@ int add_topo_info(struct cpu_topology *topo_list, struct 
topology_info *info)
ptr = check_exist_from_head(s_core-cpu_head, info-cpu_id);
if (!ptr) {
s_cpu = calloc(sizeof(struct cpu_cpu), 1);
-   if (!s_core)
+   if (!s_cpu)
return -1;
 
s_cpu-cpu_id = info-cpu_id;
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 6/6] Use the correct format specifier for size_t type

[Daniel Lezcano]

From: Colin Ian King colin.k...@canonical.com

Signed-off-by: Colin Ian King colin.k...@canonical.com
Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/idlestat.c b/idlestat.c
index 0961463..3f6076e 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -447,7 +447,7 @@ static struct cpuidle_datas *idlestat_load(const char *path)
 
fclose(f);
 
-   fprintf(stderr, Log is %lf secs long with %d events\n,
+   fprintf(stderr, Log is %lf secs long with %zd events\n,
end - begin, (int)count);
 
return datas;
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 1/6] Write open failure message to stderr

[Daniel Lezcano]

From: Colin Ian King colin.k...@canonical.com

Error message should be written to stderr rather than the failed
to open file.

Signed-off-by: Colin Ian King colin.k...@canonical.com
Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/idlestat.c b/idlestat.c
index 3abfaaf..fce6112 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -659,7 +659,7 @@ static int idlestat_file_for_each_line(const char *path, 
void *data,
f = fopen(path, r);
 
if (!f) {
-   fprintf(f, failed to open '%s': %m\n, path);
+   fprintf(stderr, failed to open '%s': %m\n, path);
return -1;
}
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 5/6] Fix file and memory resource leak on error exit in idlestat_load

[Daniel Lezcano]

From: Colin Ian King colin.k...@canonical.com

The error returns in idlestat_load fail to close an open file and
free allocated memory on the heap. This patch addresses this.

Signed-off-by: Colin Ian King colin.k...@canonical.com
Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |   14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/idlestat.c b/idlestat.c
index 739f6dc..0961463 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -396,16 +396,24 @@ static struct cpuidle_datas *idlestat_load(const char 
*path)
sscanf(buffer, cpus=%u, nrcpus);
}
 
-   if (!nrcpus)
+   if (!nrcpus) {
+   fclose(f);
return ptrerror(read error for 'cpus=' in trace file);
+   }
 
datas = malloc(sizeof(*datas));
-   if (!datas)
+   if (!datas) {
+   fclose(f);
return ptrerror(malloc datas);
+   }
 
datas-cstates = calloc(sizeof(*datas-cstates), nrcpus);
-   if (!datas-cstates)
+   if (!datas-cstates) {
+   free(datas);
+   fclose(f);
return ptrerror(calloc cstate);
+   }
+
/* initialize cstate_max for each cpu */
for (cpu = 0; cpu  nrcpus; cpu++)
datas-cstates[cpu].cstate_max = -1;
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 3/6] Fix memory leak, result is being leaked on a realloc failure

[Daniel Lezcano]

From: Colin Ian King colin.k...@canonical.com

A realloc failure leaks result on the error return path.

Signed-off-by: Colin Ian King colin.k...@canonical.com
Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 idlestat.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/idlestat.c b/idlestat.c
index 41ed3ab..739f6dc 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -221,6 +221,7 @@ static struct cpuidle_cstate *inter(struct cpuidle_cstate 
*c1,
   (result-nrdata + 1));
if (!tmp) {
free(data);
+   free(result);
return NULL;
}
data = tmp;
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

arndale ethernet not working on u-boot

[Daniel Lezcano]

Hi all,

I have an arndale and I am trying to have the ethernet working in u-boot 
but without success.


(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
scanning usb for storage devices... 0 Storage Device(s) found
scanning usb for ethernet devices... 0 Ethernet Device(s) found

I found different bugs in launchpad but without this solved.

I tried different u-boot but without success.

Did someone faced this issue and solved it ?

Thanks

-- Daniel

--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: arndale ethernet not working on u-boot

[Daniel Lezcano]

On 11/08/2013 10:13 AM, ADITYAPRATAP SHARMA wrote:

it works with Linaro Uboot (Samsung LT Uboot branch)


This repo ?

git://git.linaro.org/landing-teams/working/samsung/u-boot.git




--- Original Message ---
Sender : Tushar Beheratushar.beh...@linaro.org
Date   : Nov 08, 2013 14:39 (GMT+05:30)
Title  : Re: arndale ethernet not working on u-boot

On 8 November 2013 13:44, Daniel Lezcano daniel.lezc...@linaro.org wrote:


Hi all,

I have an arndale and I am trying to have the ethernet working in u-boot but
without success.

(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
scanning usb for storage devices... 0 Storage Device(s) found
scanning usb for ethernet devices... 0 Ethernet Device(s) found

I found different bugs in launchpad but without this solved.

I tried different u-boot but without success.

Did someone faced this issue and solved it ?

Thanks

-- Daniel



We have observed the issue with upstream u-boot, but haven't got any
solution to this problem yet.




--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [tool recommendation] tmate terminal sharing

[Daniel Lezcano]

On 10/17/2013 03:09 AM, Christoffer Dall wrote:

Hi all,

[apologies in advance for the spam nature of this message]

I have been using a tool for remote collaboration and problem solving
that I find perfect in the context of remote collaboration.  It allows
you to very easily share a terminal session even behind most firewalls
etc.

If you're interested, take a look at http://tmate.io/

It's based on tmux keymappings and configuration so if you already use
that, you will be extra happy.


Nice tool.

Thanks for sharing.

  -- Daniel


--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: Beaglebone Black Core temperature

[Daniel Lezcano]

On 10/01/2013 03:50 PM, Sanjay Singh Rawat wrote:

On Tuesday 01 October 2013 06:45 PM, Gaurang Shastri wrote:

Hi,

Do you mean to say something like lm-sensor kind of utility?? May be
from this package you will get to know what you want :-)

//Gaurang Shastri


On Tue, Oct 1, 2013 at 5:32 PM, Dave Pigott dave.pig...@linaro.org
mailto:dave.pig...@linaro.org wrote:

Hi all,

Does anyone know how we can get the current temperature of the CPU
on a beaglebone black?


default path for such hardware is at /sys/class/hwmon/hwmon* (mostly it
will be associated with CPU only). You can trying cat temp*_input file
at this path


Won't powerdebug show this information ?

--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

u-boot and dnw

[Daniel Lezcano]

Hi,

I am using the latest mainstream u-boot but the dnw command is not
there. Is there an equivalent to download the kernel from USB (except
sending the file through the console) ?

Thanks
  -- Daniel

-- 
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: ux500 not booting

[Daniel Lezcano]

On 06/07/2013 09:34 AM, Linus Walleij wrote:
 On Thu, Jun 6, 2013 at 6:15 PM, Daniel Lezcano
 daniel.lezc...@linaro.org wrote:
 
 I tried to boot my ux500
 
 Is this an HREF (black plastic box) or the Snowball board?
 
  with the 3.10-rc4 kernel but it hangs at boot
 time with the following messages:
 
 Hm, looks like some very real corruption, Ulf what do you say?

Here is so far what I got when trying to git bisect on a vanilla kernel.

So for your information, here is the result for different kernel booted
at different tags.

I always used the u8500_defconfig before compiling.

Hoping that helps.


Tag v3.4: kernel panic
---
Unable to handle kernel NULL pointer dereference at virtual address 0004
pgd = c0004000
[0004] *pgd=
Internal error: Oops: 5 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 0Not tainted  (3.4.0 #42)
PC is at ab8500_charger_probe+0x64/0x7b0
LR is at ab8500_gpadc_get+0x48/0x68
pc : [c028bdd4]lr : [c019e2c4]psr: 6013
sp : ebc49eb0  ip :   fp : ebd39e08
r10: ebc48000  r9 : c03974bc  r8 : 
r7 :   r6 : ebd39e00  r5 : c041563c  r4 : ebcb5000
r3 :   r2 :   r1 : ebd26fcf  r0 : ebe6b700
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 10c5787d  Table: 404a  DAC: 0015
Process swapper/0 (pid: 1, stack limit = 0xebc482f0)
Stack: (0xebc49eb0 to 0xebc4a000)
9ea0: ebd39e08 c01973fc ebe6bb80
ebc2c980
9ec0: ebd27940  c041563c ebd39e08 c041563c ebd39e3c c03e6b34

9ee0: c03974bc ebc48000  c019150c c01914f4 c01902cc 
ebd39e08
9f00: c03e6b34 ebd39e3c ebe6bb80 c01904e0  c03e6b34 c0190454
c018ea7c
9f20: ebc2a260 ebd25a38 c03e6b34 c03e6b34 c03e2c20 c018fafc c0334eb8
c014de94
9f40: ebc2a200 c03e6b34 0004 c039dfc4 c03ed4c0 0041 ebc48000
c0190998
9f60:  c03a1f6c 0004 c039dfc4 c03ed4c0 0041 ebc48000
c0008698
9f80: c09a82a0 c02a62cc 0041 c0035110 c03642d0 c0317b60 0004
0004
9fa0: c03d9924 c03a1f6c 0004 c039dfc4 c03ed4c0 0041 c039dfd8
c037f27c
9fc0:  c037f8f4 0004 0004 c037f27c  
c037f804
9fe0: c000f36c 0013    c000f36c fffbf5ff
3bfb
[c028bdd4] (ab8500_charger_probe+0x64/0x7b0) from [c019150c]
(platform_drv_probe+0x18/0x1c)
[c019150c] (platform_drv_probe+0x18/0x1c) from [c01902cc]
(driver_probe_device+0x8c/0x214)
[c01902cc] (driver_probe_device+0x8c/0x214) from [c01904e0]
(__driver_attach+0x8c/0x90)
[c01904e0] (__driver_attach+0x8c/0x90) from [c018ea7c]
(bus_for_each_dev+0x4c/0x80)
[c018ea7c] (bus_for_each_dev+0x4c/0x80) from [c018fafc]
(bus_add_driver+0x178/0x248)
[c018fafc] (bus_add_driver+0x178/0x248) from [c0190998]
(driver_register+0x78/0x138)
[c0190998] (driver_register+0x78/0x138) from [c0008698]
(do_one_initcall+0x10c/0x170)
[c0008698] (do_one_initcall+0x10c/0x170) from [c037f8f4]
(kernel_init+0xf0/0x1ac)
[c037f8f4] (kernel_init+0xf0/0x1ac) from [c000f36c]
(kernel_thread_exit+0x0/0x8)
Code: e5847040 e5847044 e5840020 e596305c (e5930004)
---[ end trace 1b75b31a2719ed1c ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b

CPU1: stopping
[c001460c] (unwind_backtrace+0x0/0xf8) from [c0012eb0]
(handle_IPI+0x124/0x150)
[c0012eb0] (handle_IPI+0x124/0x150) from [c000850c]
(gic_handle_irq+0x50/0x58)
[c000850c] (gic_handle_irq+0x50/0x58) from [c000de00]
(__irq_svc+0x40/0x70)
Exception stack(0xebc65f88 to 0xebc65fd0)
5f80:   0020 c03cfae0 0001  ebc64000
ebc64000
5fa0: c03ed588 c03d2eb0 ebc64000 412fc091 c029b0e4  c03cfae0
ebc65fd0
5fc0: c000f3d0 c000f3d4 6013 
[c000de00] (__irq_svc+0x40/0x70) from [c000f3d4]
(default_idle+0x28/0x30)
[c000f3d4] (default_idle+0x28/0x30) from [c000f5ec] (cpu_idle+0x98/0xe4)
[c000f5ec] (cpu_idle+0x98/0xe4) from [0028fa54] (0x28fa54)



Tag v3.5: kernel panic
---
EXT3-fs (mmcblk1p2): error: couldn't mount because of unsupported
optional features (240)
EXT2-fs (mmcblk1p2): error: couldn't mount because of unsupported
optional features (240)
List of all partitions:
b300 3702784 mmcblk0  driver: mmcblk
  b3013968 mmcblk0p1 ----
  b302  131072 mmcblk0p2 ----
  b303  524288 mmcblk0p3 ----
  b304   1 mmcblk0p4 ----
  b305  262128 mmcblk0p5 ----
  b306  524272 mmcblk0p6 ----
  b307   90096 mmcblk0p7 ----
b3102048 mmcblk0boot1  (driver?)
b3082048 mmcblk0boot0  (driver?)
b318 1931264 mmcblk1  driver: mmcblk
  b319   53216 mmcblk1p1 --

Re: ux500 not booting

[Daniel Lezcano]

On 06/07/2013 09:34 AM, Linus Walleij wrote:
 On Thu, Jun 6, 2013 at 6:15 PM, Daniel Lezcano
 daniel.lezc...@linaro.org wrote:
 
 I tried to boot my ux500
 
 Is this an HREF (black plastic box) or the Snowball board?

It is the snowball board.

  with the 3.10-rc4 kernel but it hangs at boot
 time with the following messages:
 
 Hm, looks like some very real corruption, Ulf what do you say?
 
 Linus
 


-- 
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

ux500 not booting

[Daniel Lezcano]

Hi Linus,

I tried to boot my ux500 with the 3.10-rc4 kernel but it hangs at boot
time with the following messages:

mmcblk0: error -110 transferring data, sector 2891778, nr 6, cmd
response 0x900, card status 0x0
mmci-pl18x sdi4: error during DMA transfer!
mmcblk0: error -110 transferring data, sector 2891779, nr 5, cmd
response 0x900, card status 0x0
mmci-pl18x sdi4: error during DMA transfer!
mmcblk0: error -110 transferring data, sector 2891780, nr 4, cmd
response 0x900, card status 0x0
mmci-pl18x sdi4: error during DMA transfer!
mmcblk0: error -110 transferring data, sector 2891781, nr 3, cmd
response 0x900, card status 0x0
mmci-pl18x sdi4: error during DMA transfer!
mmcblk0: error -110 transferring data, sector 2891782, nr 2, cmd
response 0x900, card status 0x0
mmci-pl18x sdi4: error during DMA transfer!
mmcblk0: error -110 transferring data, sector 2891783, nr 1, cmd
response 0x900, card status 0x0
Buffer I/O error on device mmcblk0, logical block 361472
 mmcblk0: p1 p2 p3 p4  p5 p6 
mmc0: Card stuck in programming state! __mmc_switch
blk_update_request: 6 callbacks suppressed
end_request: I/O error, dev mmcblk0boot1, sector 0
Buffer I/O error on device mmcblk0boot1, logical block 0
end_request: I/O error, dev mmcblk0boot1, sector 0
Buffer I/O error on device mmcblk0boot1, logical block 0
end_request: I/O error, dev mmcblk0boot1, sector 0
Buffer I/O error on device mmcblk0boot1, logical block 0
 mmcblk0boot1: unable to read partition table
end_request: I/O error, dev mmcblk0boot0, sector 0
Buffer I/O error on device mmcblk0boot0, logical block 0
end_request: I/O error, dev mmcblk0boot0, sector 0
Buffer I/O error on device mmcblk0boot0, logical block 0
end_request: I/O error, dev mmcblk0boot0, sector 0
Buffer I/O error on device mmcblk0boot0, logical block 0
 mmcblk0boot0: unable to read partition table
Waiting for root device /dev/mmcblk1p2...


I thought my sd-card was dead and tried another one but I got the same
problem.

Is it a known problem ?

Thanks in advance

  -- Daniel


-- 
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH] cpuidle: arm_big_little: route target residency to mcpm

[Daniel Lezcano]

On 05/15/2013 05:24 PM, Liviu Dudau wrote:
 Hi Sebastian,
 
 On Mon, May 13, 2013 at 07:53:42PM +0100, Sebastian Capella wrote:
 Pass residency information to the mcpm_cpu_suspend.  The information
 is taken from the target_residency of the intended C-state.

 When a platform uses multiple powerdown cstates, the residency information
 indicates which powerdown state is targeted.  Multiple powerdown cstate
 information can be maintained in the device tree and the vendor specific
 handling will then have enough information to determine what power state to
 enter without needing additional changes to the big_little framework.

 Signed-off-by: Sebastian Capella sebastian.cape...@linaro.org
 ---
  drivers/cpuidle/arm_big_little.c |6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

 diff --git a/drivers/cpuidle/arm_big_little.c 
 b/drivers/cpuidle/arm_big_little.c
 index a430800..8332b05 100644
 --- a/drivers/cpuidle/arm_big_little.c
 +++ b/drivers/cpuidle/arm_big_little.c
 
 I could not find a branch that contains this file. Which git tree and branch
 are you using?

I believe it should apply to:

https://git.linaro.org/gitweb?p=landing-teams/working/arm/kernel.git;a=blob;f=drivers/cpuidle/arm_big_little.c;h=a430800d4a74c8c5b759cd34fb5e272d8cf8f8a3;hb=67b5adf6a402921655d337d52845d6d48c6ef2d2#l66

 
 @@ -89,7 +89,7 @@ static int notrace bl_powerdown_finisher(unsigned long arg)
   unsigned int cpu = mpidr  0xf;

   mcpm_set_entry_vector(cpu, cluster, cpu_resume);
 - mcpm_cpu_suspend(0);  /* 0 should be replaced with better value here */
 + mcpm_cpu_suspend(arg);
   return 1;
  }

 @@ -107,6 +107,7 @@ static int bl_enter_powerdown(struct cpuidle_device *dev,
  {
   struct timespec ts_preidle, ts_postidle, ts_idle;
   int ret;
 + struct cpuidle_state *state = drv-states[idx];

   /* Used to keep track of the total time in idle */
   getnstimeofday(ts_preidle);
 @@ -117,7 +118,8 @@ static int bl_enter_powerdown(struct cpuidle_device *dev,

   clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, dev-cpu);

 - ret = cpu_suspend((unsigned long) dev, bl_powerdown_finisher);
 + ret = cpu_suspend((unsigned long) state-target_residency,
 + bl_powerdown_finisher);
 
 I don't think you should pass the target residency here but the intended
 C-state. Think about what will happen when you run this in a guest kernel: is
 the target_residency the same if the guest has been migrated from a big core
 that might have a faster execution of the down/up path to a little core that
 is slower? The intended C-state should stay the same, regardless of the actual
 time it takes to get there and out, which affects the actual time spent inside
 the state.
 
 Best regards,
 Liviu
 


-- 
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[resend] Timer broadcast question

[Daniel Lezcano]

Hi,

I am working on identifying the different wakeup sources from the
interrupts and I have a question regarding the timer broadcast.

The broadcast timer is setup to the next event and that will wake up any
idle cpu belonging to the broadcast cpumask, right ?

The cpu which has been woken up will look for each cpu the next-event
and send an IPI to wake it up.

Although, it is possible the sender of this IPI may not be concerned by
the timer expiration and has been woken up just for sending the IPI, right ?

If this is correct, is it possible to setup the timer irq affinity to a
cpu which will be concerned by the timer expiration ? so we prevent an
unnecessary wake up for a cpu.

For example, let's say we have a 2 cpus system.

cpu0, cpu1 are idle

The next event is for cpu1 but cpu0 is wake up by the broadcast timer,
after checking it has nothing to do except send a IPI_TIMER to cpu1 and
then goes to idle again.

Wouldn't be worth to set the broadcast timer affinity to cpu1, so cpu0
is not wake up ?

Did I missed something or does it sound correct ?

Thanks
  -- Daniel



-- 
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [resend] Timer broadcast question

[Daniel Lezcano]

On 02/19/2013 07:10 PM, Thomas Gleixner wrote:
 On Tue, 19 Feb 2013, Daniel Lezcano wrote:
 I am working on identifying the different wakeup sources from the
 interrupts and I have a question regarding the timer broadcast.

 The broadcast timer is setup to the next event and that will wake up any
 idle cpu belonging to the broadcast cpumask, right ?

 The cpu which has been woken up will look for each cpu the next-event
 and send an IPI to wake it up.
  
 Although, it is possible the sender of this IPI may not be concerned by
 the timer expiration and has been woken up just for sending the IPI, right ?
 
 Correct.
  
 If this is correct, is it possible to setup the timer irq affinity to a
 cpu which will be concerned by the timer expiration ? so we prevent an
 unnecessary wake up for a cpu.
 
 It is possible, but we never implemented it.
 
 If we go there, we want to make that conditional on a property flag,
 because some interrupt controllers especially on x86 only allow to
 move the affinity from interrupt context, which is pointless.

Thanks Thomas for your quick answer. I will write a RFC patchset.

  -- Daniel

-- 
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 0/4][V2] davinci: cpuidle - some cleanups

[Daniel Lezcano]

This patchset does some cleanup. It could have been folded in a single
patch but the review would have been less clean than splitting it into
small and trivial patches.

The main purpose of this patch is to remove the usage of the driver_data
field from the state_usage structure. Len Brown is doing this cleanup in
the intel_idle.c file. With this patchset, the processor_idle.c file will
be the last user of this field.

Also, the patchset simplify the code and makes it a bit more clear to read.

Thanks to Sekhar for testing the patchset.

Changelog:
V2:
 * removed global static variable initialization to 'false'
 * added Acked-by: Sekhar Nori nsek...@ti.com
V1: initial post

Daniel Lezcano (4):
  davinci: cpuidle - use global variable for ddr2 flag
  davinci: cpuidle - move code to prevent forward declaration
  davinci: cpuidle - remove the ops
  davinci: cpuidle - remove useless initialization

 arch/arm/mach-davinci/cpuidle.c |   84 ---
 1 file changed, 25 insertions(+), 59 deletions(-)

-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 3/4][V2] davinci: cpuidle - remove the ops

[Daniel Lezcano]

With one function handling the idle state and a single variable,
the usage of the davinci_ops is overkill.

This patch removes these ops and simplify the code.

Furthermore, the 'driver_data' field is no longer used, we have
1 of the 3 remaining user of this field removed.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
Acked-by: Sekhar Nori nsek...@ti.com
---
 arch/arm/mach-davinci/cpuidle.c |   33 ++---
 1 file changed, 2 insertions(+), 31 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index 697febe..5e430bf 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -25,12 +25,6 @@
 
 #define DAVINCI_CPUIDLE_MAX_STATES 2
 
-struct davinci_ops {
-   void (*enter) (void);
-   void (*exit) (void);
-   u32 flags;
-};
-
 static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
 static void __iomem *ddr2_reg_base;
 static bool ddr2_pdown;
@@ -54,39 +48,17 @@ static void davinci_save_ddr_power(int enter, bool pdown)
__raw_writel(val, ddr2_reg_base + DDR2_SDRCR_OFFSET);
 }
 
-static void davinci_c2state_enter(void)
-{
-   davinci_save_ddr_power(1, ddr2_pdown);
-}
-
-static void davinci_c2state_exit(void)
-{
-   davinci_save_ddr_power(0, ddr2_pdown);
-}
-
-static struct davinci_ops davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
-   [1] = {
-   .enter  = davinci_c2state_enter,
-   .exit   = davinci_c2state_exit,
-   },
-};
-
 /* Actual code that puts the SoC in different idle states */
 static int davinci_enter_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
 {
-   struct cpuidle_state_usage *state_usage = dev-states_usage[index];
-   struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
-
-   if (ops  ops-enter)
-   ops-enter();
+   davinci_save_ddr_power(1, ddr2_pdown);
 
index = cpuidle_wrap_enter(dev, drv, index,
arm_cpuidle_simple_enter);
 
-   if (ops  ops-exit)
-   ops-exit();
+   davinci_save_ddr_power(0, ddr2_pdown);
 
return index;
 }
@@ -123,7 +95,6 @@ static int __init davinci_cpuidle_probe(struct 
platform_device *pdev)
ddr2_reg_base = pdata-ddr2_ctlr_base;
 
ddr2_pdown = pdata-ddr2_pdown;
-   cpuidle_set_statedata(device-states_usage[1], davinci_states[1]);
 
device-state_count = DAVINCI_CPUIDLE_MAX_STATES;
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 2/4][V2] davinci: cpuidle - move code to prevent forward declaration

[Daniel Lezcano]

The patch is mindless, it just moves the idle function below in the file
in order to prevent forward declaration in the next patch.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
Acked-by: Sekhar Nori nsek...@ti.com
---
 arch/arm/mach-davinci/cpuidle.c |   72 +++
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index 5fbd470..697febe 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -31,42 +31,6 @@ struct davinci_ops {
u32 flags;
 };
 
-/* Actual code that puts the SoC in different idle states */
-static int davinci_enter_idle(struct cpuidle_device *dev,
-   struct cpuidle_driver *drv,
-   int index)
-{
-   struct cpuidle_state_usage *state_usage = dev-states_usage[index];
-   struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
-
-   if (ops  ops-enter)
-   ops-enter();
-
-   index = cpuidle_wrap_enter(dev, drv, index,
-   arm_cpuidle_simple_enter);
-
-   if (ops  ops-exit)
-   ops-exit();
-
-   return index;
-}
-
-static struct cpuidle_driver davinci_idle_driver = {
-   .name   = cpuidle-davinci,
-   .owner  = THIS_MODULE,
-   .en_core_tk_irqen   = 1,
-   .states[0]  = ARM_CPUIDLE_WFI_STATE,
-   .states[1]  = {
-   .enter  = davinci_enter_idle,
-   .exit_latency   = 10,
-   .target_residency   = 10,
-   .flags  = CPUIDLE_FLAG_TIME_VALID,
-   .name   = DDR SR,
-   .desc   = WFI and DDR Self Refresh,
-   },
-   .state_count = DAVINCI_CPUIDLE_MAX_STATES,
-};
-
 static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
 static void __iomem *ddr2_reg_base;
 static bool ddr2_pdown;
@@ -107,6 +71,42 @@ static struct davinci_ops 
davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
},
 };
 
+/* Actual code that puts the SoC in different idle states */
+static int davinci_enter_idle(struct cpuidle_device *dev,
+   struct cpuidle_driver *drv,
+   int index)
+{
+   struct cpuidle_state_usage *state_usage = dev-states_usage[index];
+   struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
+
+   if (ops  ops-enter)
+   ops-enter();
+
+   index = cpuidle_wrap_enter(dev, drv, index,
+   arm_cpuidle_simple_enter);
+
+   if (ops  ops-exit)
+   ops-exit();
+
+   return index;
+}
+
+static struct cpuidle_driver davinci_idle_driver = {
+   .name   = cpuidle-davinci,
+   .owner  = THIS_MODULE,
+   .en_core_tk_irqen   = 1,
+   .states[0]  = ARM_CPUIDLE_WFI_STATE,
+   .states[1]  = {
+   .enter  = davinci_enter_idle,
+   .exit_latency   = 10,
+   .target_residency   = 10,
+   .flags  = CPUIDLE_FLAG_TIME_VALID,
+   .name   = DDR SR,
+   .desc   = WFI and DDR Self Refresh,
+   },
+   .state_count = DAVINCI_CPUIDLE_MAX_STATES,
+};
+
 static int __init davinci_cpuidle_probe(struct platform_device *pdev)
 {
int ret;
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 1/4][V2] davinci: cpuidle - use global variable for ddr2 flag

[Daniel Lezcano]

Replace the flag by a simple global boolean in the cpuidle.c.
That will allow to cleanup the rest of the code right after,
because the ops won't make sense.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
Acked-by: Sekhar Nori nsek...@ti.com
---
 arch/arm/mach-davinci/cpuidle.c |   23 ++-
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index 9107691..5fbd470 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -26,8 +26,8 @@
 #define DAVINCI_CPUIDLE_MAX_STATES 2
 
 struct davinci_ops {
-   void (*enter) (u32 flags);
-   void (*exit) (u32 flags);
+   void (*enter) (void);
+   void (*exit) (void);
u32 flags;
 };
 
@@ -40,20 +40,17 @@ static int davinci_enter_idle(struct cpuidle_device *dev,
struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
 
if (ops  ops-enter)
-   ops-enter(ops-flags);
+   ops-enter();
 
index = cpuidle_wrap_enter(dev, drv, index,
arm_cpuidle_simple_enter);
 
if (ops  ops-exit)
-   ops-exit(ops-flags);
+   ops-exit();
 
return index;
 }
 
-/* fields in davinci_ops.flags */
-#define DAVINCI_CPUIDLE_FLAGS_DDR2_PWDNBIT(0)
-
 static struct cpuidle_driver davinci_idle_driver = {
.name   = cpuidle-davinci,
.owner  = THIS_MODULE,
@@ -72,6 +69,7 @@ static struct cpuidle_driver davinci_idle_driver = {
 
 static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
 static void __iomem *ddr2_reg_base;
+static bool ddr2_pdown;
 
 static void davinci_save_ddr_power(int enter, bool pdown)
 {
@@ -92,14 +90,14 @@ static void davinci_save_ddr_power(int enter, bool pdown)
__raw_writel(val, ddr2_reg_base + DDR2_SDRCR_OFFSET);
 }
 
-static void davinci_c2state_enter(u32 flags)
+static void davinci_c2state_enter(void)
 {
-   davinci_save_ddr_power(1, !!(flags  DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN));
+   davinci_save_ddr_power(1, ddr2_pdown);
 }
 
-static void davinci_c2state_exit(u32 flags)
+static void davinci_c2state_exit(void)
 {
-   davinci_save_ddr_power(0, !!(flags  DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN));
+   davinci_save_ddr_power(0, ddr2_pdown);
 }
 
 static struct davinci_ops davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
@@ -124,8 +122,7 @@ static int __init davinci_cpuidle_probe(struct 
platform_device *pdev)
 
ddr2_reg_base = pdata-ddr2_ctlr_base;
 
-   if (pdata-ddr2_pdown)
-   davinci_states[1].flags |= DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN;
+   ddr2_pdown = pdata-ddr2_pdown;
cpuidle_set_statedata(device-states_usage[1], davinci_states[1]);
 
device-state_count = DAVINCI_CPUIDLE_MAX_STATES;
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 4/4][V2] davinci: cpuidle - remove useless initialization

[Daniel Lezcano]

The device-state_count is initialized in the cpuidle_register_device
function.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
Acked-by: Sekhar Nori nsek...@ti.com
---
 arch/arm/mach-davinci/cpuidle.c |2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index 5e430bf..5ac9e93 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -96,8 +96,6 @@ static int __init davinci_cpuidle_probe(struct 
platform_device *pdev)
 
ddr2_pdown = pdata-ddr2_pdown;
 
-   device-state_count = DAVINCI_CPUIDLE_MAX_STATES;
-
ret = cpuidle_register_driver(davinci_idle_driver);
if (ret) {
dev_err(pdev-dev, failed to register driver\n);
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 0/4] davinci: cpuidle - some cleanups

[Daniel Lezcano]

On 02/01/2013 07:49 PM, Len Brown wrote:
 On 02/01/2013 08:48 AM, Daniel Lezcano wrote:
 This patchset does some cleanup. It could have been folded in a single
 patch but the review would have been less clean than splitting it into
 small and trivial patches.

 The main purpose of this patch is to remove the usage of the driver_data
 field from the state_usage structure. Len Brown is doing this cleanup in
 the intel_idle.c file. With this patchset, the processor_idle.c file will
 be the last user of this field.
 
 Daniel,
 Thanks for this cleanup.
 Hopefully we can hear from somebody with davinci HW who can test it?
 
 Looks like it is now up to me to address processor_idle.c and
 finish the job of expunging driver_data.

Are you ok if I take care of that ?

  -- Daniel


-- 
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 2/4] ACPI / idle : remove pointless headers

[Daniel Lezcano]

These different headers are not needed.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 drivers/acpi/processor_idle.c |   13 +
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 5cb5f24..82626e9 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -28,19 +28,12 @@
  * ~~
  */
 
-#include linux/kernel.h
 #include linux/module.h
-#include linux/init.h
-#include linux/cpufreq.h
-#include linux/slab.h
 #include linux/acpi.h
 #include linux/dmi.h
-#include linux/moduleparam.h
-#include linux/sched.h   /* need_resched() */
-#include linux/pm_qos.h
+#include linux/sched.h   /* need_resched() */
 #include linux/clockchips.h
 #include linux/cpuidle.h
-#include linux/irqflags.h
 
 /*
  * Include the apic definitions for x86 to have the APIC timer related defines
@@ -52,12 +45,8 @@
 #include asm/apic.h
 #endif
 
-#include asm/io.h
-#include asm/uaccess.h
-
 #include acpi/acpi_bus.h
 #include acpi/processor.h
-#include asm/processor.h
 
 #define PREFIX ACPI: 
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 3/4] ACPI / idle: pass the cpuidle_device parameter

[Daniel Lezcano]

The cpuidle_device is retrieved in the function by using directly
the global variable. But the caller of this function already have
this device and it can be passed as a parameter. That is one small
step to encapsulate the code more.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 drivers/acpi/processor_idle.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 82626e9..6837065 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -928,13 +928,14 @@ struct cpuidle_driver acpi_idle_driver = {
  * device i.e. per-cpu data
  *
  * @pr: the ACPI processor
+ * @dev : the cpuidle device
  */
-static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr)
+static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
+  struct cpuidle_device *dev)
 {
int i, count = CPUIDLE_DRIVER_STATE_START;
struct acpi_processor_cx *cx;
struct cpuidle_state_usage *state_usage;
-   struct cpuidle_device *dev = per_cpu(acpi_cpuidle_device, pr-id);
 
if (!pr-flags.power_setup_done)
return -EINVAL;
@@ -1089,7 +1090,7 @@ int acpi_processor_hotplug(struct acpi_processor *pr)
cpuidle_disable_device(dev);
acpi_processor_get_power_info(pr);
if (pr-flags.power) {
-   acpi_processor_setup_cpuidle_cx(pr);
+   acpi_processor_setup_cpuidle_cx(pr, dev);
ret = cpuidle_enable_device(dev);
}
cpuidle_resume_and_unlock();
@@ -1147,8 +1148,8 @@ int acpi_processor_cst_has_changed(struct acpi_processor 
*pr)
continue;
acpi_processor_get_power_info(_pr);
if (_pr-flags.power) {
-   acpi_processor_setup_cpuidle_cx(_pr);
dev = per_cpu(acpi_cpuidle_device, cpu);
+   acpi_processor_setup_cpuidle_cx(_pr, dev);
cpuidle_enable_device(dev);
}
}
@@ -1217,7 +1218,7 @@ int __cpuinit acpi_processor_power_init(struct 
acpi_processor *pr)
return -ENOMEM;
per_cpu(acpi_cpuidle_device, pr-id) = dev;
 
-   acpi_processor_setup_cpuidle_cx(pr);
+   acpi_processor_setup_cpuidle_cx(pr, dev);
 
/* Register per-cpu cpuidle_device. Cpuidle driver
 * must already be registered before registering device
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 1/4] ACPI / idle: remove unused definition

[Daniel Lezcano]

The different definitions are not used anywhere in the code.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 drivers/acpi/processor_idle.c |5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index e606e36..5cb5f24 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -64,11 +64,6 @@
 #define ACPI_PROCESSOR_CLASSprocessor
 #define _COMPONENT  ACPI_PROCESSOR_COMPONENT
 ACPI_MODULE_NAME(processor_idle);
-#define PM_TIMER_TICK_NS   (10ULL/ACPI_PM_TIMER_FREQUENCY)
-#define C2_OVERHEAD1   /* 1us */
-#define C3_OVERHEAD1   /* 1us */
-#define PM_TIMER_TICKS_TO_US(p)\
-   (((p) * 1000)/(ACPI_PM_TIMER_FREQUENCY/1000))
 
 static unsigned int max_cstate __read_mostly = ACPI_PROCESSOR_MAX_POWER;
 module_param(max_cstate, uint, );
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 4/4] ACPI / idle: remove usage of the statedata

[Daniel Lezcano]

Len Brown sent a patch to remove this field in the intel_idle driver.
The other user of this field is the davinci cpuidle driver and a
patch has been sent to remove the usage of it.

This patch removes the last user of this field.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 drivers/acpi/processor_idle.c |   19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 6837065..8b433cb 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -66,6 +66,8 @@ module_param(latency_factor, uint, 0644);
 
 static DEFINE_PER_CPU(struct cpuidle_device *, acpi_cpuidle_device);
 
+static struct acpi_processor_cx *acpi_cstate[CPUIDLE_STATE_MAX];
+
 static int disabled_by_idle_boot_param(void)
 {
return boot_option_idle_override == IDLE_POLL ||
@@ -721,8 +723,7 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int index)
 {
struct acpi_processor *pr;
-   struct cpuidle_state_usage *state_usage = dev-states_usage[index];
-   struct acpi_processor_cx *cx = cpuidle_get_statedata(state_usage);
+   struct acpi_processor_cx *cx = acpi_cstate[index];
 
pr = __this_cpu_read(processors);
 
@@ -745,8 +746,7 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
  */
 static int acpi_idle_play_dead(struct cpuidle_device *dev, int index)
 {
-   struct cpuidle_state_usage *state_usage = dev-states_usage[index];
-   struct acpi_processor_cx *cx = cpuidle_get_statedata(state_usage);
+   struct acpi_processor_cx *cx = acpi_cstate[index];
 
ACPI_FLUSH_CPU_CACHE();
 
@@ -776,8 +776,7 @@ static int acpi_idle_enter_simple(struct cpuidle_device 
*dev,
struct cpuidle_driver *drv, int index)
 {
struct acpi_processor *pr;
-   struct cpuidle_state_usage *state_usage = dev-states_usage[index];
-   struct acpi_processor_cx *cx = cpuidle_get_statedata(state_usage);
+   struct acpi_processor_cx *cx = acpi_cstate[index];
 
pr = __this_cpu_read(processors);
 
@@ -835,8 +834,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int index)
 {
struct acpi_processor *pr;
-   struct cpuidle_state_usage *state_usage = dev-states_usage[index];
-   struct acpi_processor_cx *cx = cpuidle_get_statedata(state_usage);
+   struct acpi_processor_cx *cx = acpi_cstate[index];
 
pr = __this_cpu_read(processors);
 
@@ -935,7 +933,6 @@ static int acpi_processor_setup_cpuidle_cx(struct 
acpi_processor *pr,
 {
int i, count = CPUIDLE_DRIVER_STATE_START;
struct acpi_processor_cx *cx;
-   struct cpuidle_state_usage *state_usage;
 
if (!pr-flags.power_setup_done)
return -EINVAL;
@@ -954,7 +951,6 @@ static int acpi_processor_setup_cpuidle_cx(struct 
acpi_processor *pr,
 
for (i = 1; i  ACPI_PROCESSOR_MAX_POWER  i = max_cstate; i++) {
cx = pr-power.states[i];
-   state_usage = dev-states_usage[count];
 
if (!cx-valid)
continue;
@@ -965,8 +961,7 @@ static int acpi_processor_setup_cpuidle_cx(struct 
acpi_processor *pr,
!(acpi_gbl_FADT.flags  ACPI_FADT_C2_MP_SUPPORTED))
continue;
 #endif
-
-   cpuidle_set_statedata(state_usage, cx);
+   acpi_cstate[count] = cx;
 
count++;
if (count == CPUIDLE_STATE_MAX)
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 0/4] davinci: cpuidle - some cleanups

[Daniel Lezcano]

On 02/03/2013 02:00 PM, Sekhar Nori wrote:
 On 2/3/2013 5:52 PM, Daniel Lezcano wrote:
 On 02/03/2013 12:54 PM, Sekhar Nori wrote:
 On 2/2/2013 12:19 AM, Len Brown wrote:
 On 02/01/2013 08:48 AM, Daniel Lezcano wrote:
 This patchset does some cleanup. It could have been folded in a single
 patch but the review would have been less clean than splitting it into
 small and trivial patches.

 The main purpose of this patch is to remove the usage of the driver_data
 field from the state_usage structure. Len Brown is doing this cleanup in
 the intel_idle.c file. With this patchset, the processor_idle.c file will
 be the last user of this field.

 Daniel,
 Thanks for this cleanup.
 Hopefully we can hear from somebody with davinci HW who can test it?

 Can someone forward the patches to
 davinci-linux-open-sou...@linux.davincidsp.com? I can help verify.

 I cc'ed this mailing list but I have not subscribed to it. The mailing
 list description says moderated for non-subscribers in the MAINTAINERS
 file.

 I assumed the moderator will accept the patchset.

 Shall I subscribe and resend ?
 
 No need. I see the patches on the list. I will test and get back.

Cool ! Thanks !

  -- Daniel

-- 
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 1/4] intel_idle: stop using driver_data for static flags

[Daniel Lezcano]

On 02/02/2013 03:16 AM, Len Brown wrote:
 
 intel_idle already uses a driver-specific flag:

 #define CPUIDLE_FLAG_TLB_FLUSHED0x1

 This patch just uses 4 more bits along with that one.

 Ok. In this case it would make sense to move this flag out of the
 generic core code to the intel_idle.c file ?
 
 This flag is already local to intel_idle.c.
 If another architecture finds it useful,
 then yes, it would make sense to move it to the shared header.

Oh, right. Sorry I puzzled myself with the name. I was convinced it was
in the shared header.

 and move the [dec/en]coding
 macro flags_2_MWAIT_EAX and MWAIT_EAX_2_flags (with a more appropriate
 name for a generic code) to the cpuidle.h file ?
 
 I think that a driver's private flag definitions
 should remain local to the driver.  It makes no sense
 to pollute the name space of other drivers for stuff
 that doesn't mean anything to them.  MWAIT is pretty
 specific to x86 -- and re-naming it to something 'generic'
 isn't going to make the code easier to read.

Ok, let me rephrase it because I think how it was presented was not clear.

As we want to use the half of the state flags for private purpose, I
suggested to add the encoding/decoding function in the shared header file.

The mwait eax flags are not encoded and the CPUIDLE_FLAG_TLB_FLUSHED is
encoded.

I suggested to unify both and to use an encoding function from the
shared header file.

#define CPUIDLE_PRIVATE_FLAGS(_flags_) \
((_flags_)  16)  CPUIDLE_DRIVER_FLAGS_MASK

For example:

#define FLAG_TLB_FLUSHED CPUIDLE_PRIVATE_FLAGS(0x1)
#define FLAG_MWAIT_C1CPUIDLE_PRIVATE_FLAGS(0x0)
#define FLAG_MWAIT_C2CPUIDLE_PRIVATE_FLAGS(0x10)
#define FLAG_MWAIT_C3CPUIDLE_PRIVATE_FLAGS(0x20)
#define FLAG_MWAIT_C4CPUIDLE_PRIVATE_FLAGS(0x30)
#define FLAG_MWAIT_C5CPUIDLE_PRIVATE_FLAGS(0x40)
#define FLAG_MWAIT_C6CPUIDLE_PRIVATE_FLAGS(0x52)

And then:

...

.flags = FLAG_TLB_FLUSHED | FLAG_MWAIT_C2 | CPUIDLE_FLAG_TIME_VALID

...

But in the idle function, you need to retrieve the 'value' of the EAX
not a flag, so there is the need for an extra macro conversion and mask
the TLB flag.

Well, this is a detail, so feel free to ignore this suggestion :)

Thanks
  -- Daniel

-- 
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 4/4] davinci: cpuidle - remove useless initialization

[Daniel Lezcano]

The device-state_count is initialized in the cpuidle_register_device
function.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 arch/arm/mach-davinci/cpuidle.c |2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index 28cc8e8..144839b 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -96,8 +96,6 @@ static int __init davinci_cpuidle_probe(struct 
platform_device *pdev)
 
ddr2_pdown = pdata-ddr2_pdown;
 
-   device-state_count = DAVINCI_CPUIDLE_MAX_STATES;
-
ret = cpuidle_register_driver(davinci_idle_driver);
if (ret) {
dev_err(pdev-dev, failed to register driver\n);
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 0/4] davinci: cpuidle - some cleanups

[Daniel Lezcano]

This patchset does some cleanup. It could have been folded in a single
patch but the review would have been less clean than splitting it into
small and trivial patches.

The main purpose of this patch is to remove the usage of the driver_data
field from the state_usage structure. Len Brown is doing this cleanup in
the intel_idle.c file. With this patchset, the processor_idle.c file will
be the last user of this field.

Also, the patchset simplify the code and makes it a bit more clear to read.

I don't have this hardware, the code is not tested.

Daniel Lezcano (4):
  davinci: cpuidle - use global variable for ddr2 flag
  davinci: cpuidle - move code to prevent forward declaration
  davinci: cpuidle - remove the ops
  davinci: cpuidle - remove useless initialization

 arch/arm/mach-davinci/cpuidle.c |   84 ---
 1 file changed, 25 insertions(+), 59 deletions(-)

-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 2/4] davinci: cpuidle - move code to prevent forward declaration

[Daniel Lezcano]

The patch is mindless, it just moves the idle function below in the file
in order to prevent forward declaration in the next patch.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 arch/arm/mach-davinci/cpuidle.c |   72 +++
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index ebb8808..9438672 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -31,42 +31,6 @@ struct davinci_ops {
u32 flags;
 };
 
-/* Actual code that puts the SoC in different idle states */
-static int davinci_enter_idle(struct cpuidle_device *dev,
-   struct cpuidle_driver *drv,
-   int index)
-{
-   struct cpuidle_state_usage *state_usage = dev-states_usage[index];
-   struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
-
-   if (ops  ops-enter)
-   ops-enter();
-
-   index = cpuidle_wrap_enter(dev, drv, index,
-   arm_cpuidle_simple_enter);
-
-   if (ops  ops-exit)
-   ops-exit();
-
-   return index;
-}
-
-static struct cpuidle_driver davinci_idle_driver = {
-   .name   = cpuidle-davinci,
-   .owner  = THIS_MODULE,
-   .en_core_tk_irqen   = 1,
-   .states[0]  = ARM_CPUIDLE_WFI_STATE,
-   .states[1]  = {
-   .enter  = davinci_enter_idle,
-   .exit_latency   = 10,
-   .target_residency   = 10,
-   .flags  = CPUIDLE_FLAG_TIME_VALID,
-   .name   = DDR SR,
-   .desc   = WFI and DDR Self Refresh,
-   },
-   .state_count = DAVINCI_CPUIDLE_MAX_STATES,
-};
-
 static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
 static void __iomem *ddr2_reg_base;
 static bool ddr2_pdown = false;
@@ -107,6 +71,42 @@ static struct davinci_ops 
davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
},
 };
 
+/* Actual code that puts the SoC in different idle states */
+static int davinci_enter_idle(struct cpuidle_device *dev,
+   struct cpuidle_driver *drv,
+   int index)
+{
+   struct cpuidle_state_usage *state_usage = dev-states_usage[index];
+   struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
+
+   if (ops  ops-enter)
+   ops-enter();
+
+   index = cpuidle_wrap_enter(dev, drv, index,
+   arm_cpuidle_simple_enter);
+
+   if (ops  ops-exit)
+   ops-exit();
+
+   return index;
+}
+
+static struct cpuidle_driver davinci_idle_driver = {
+   .name   = cpuidle-davinci,
+   .owner  = THIS_MODULE,
+   .en_core_tk_irqen   = 1,
+   .states[0]  = ARM_CPUIDLE_WFI_STATE,
+   .states[1]  = {
+   .enter  = davinci_enter_idle,
+   .exit_latency   = 10,
+   .target_residency   = 10,
+   .flags  = CPUIDLE_FLAG_TIME_VALID,
+   .name   = DDR SR,
+   .desc   = WFI and DDR Self Refresh,
+   },
+   .state_count = DAVINCI_CPUIDLE_MAX_STATES,
+};
+
 static int __init davinci_cpuidle_probe(struct platform_device *pdev)
 {
int ret;
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 3/4] davinci: cpuidle - remove the ops

[Daniel Lezcano]

With one function handling the idle state and a single variable,
the usage of the davinci_ops is overkill.

This patch removes these ops and simplify the code.

Furthermore, the 'driver_data' field is no longer used, we have
1 of the 3 remaining user of this field removed.

Signed-off-by: Daniel Lezcano daniel.lezc...@linaro.org
---
 arch/arm/mach-davinci/cpuidle.c |   33 ++---
 1 file changed, 2 insertions(+), 31 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index 9438672..28cc8e8 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -25,12 +25,6 @@
 
 #define DAVINCI_CPUIDLE_MAX_STATES 2
 
-struct davinci_ops {
-   void (*enter) (void);
-   void (*exit) (void);
-   u32 flags;
-};
-
 static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
 static void __iomem *ddr2_reg_base;
 static bool ddr2_pdown = false;
@@ -54,39 +48,17 @@ static void davinci_save_ddr_power(int enter, bool pdown)
__raw_writel(val, ddr2_reg_base + DDR2_SDRCR_OFFSET);
 }
 
-static void davinci_c2state_enter(void)
-{
-   davinci_save_ddr_power(1, ddr2_pdown);
-}
-
-static void davinci_c2state_exit(void)
-{
-   davinci_save_ddr_power(0, ddr2_pdown);
-}
-
-static struct davinci_ops davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
-   [1] = {
-   .enter  = davinci_c2state_enter,
-   .exit   = davinci_c2state_exit,
-   },
-};
-
 /* Actual code that puts the SoC in different idle states */
 static int davinci_enter_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
 {
-   struct cpuidle_state_usage *state_usage = dev-states_usage[index];
-   struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
-
-   if (ops  ops-enter)
-   ops-enter();
+   davinci_save_ddr_power(1, ddr2_pdown);
 
index = cpuidle_wrap_enter(dev, drv, index,
arm_cpuidle_simple_enter);
 
-   if (ops  ops-exit)
-   ops-exit();
+   davinci_save_ddr_power(0, ddr2_pdown);
 
return index;
 }
@@ -123,7 +95,6 @@ static int __init davinci_cpuidle_probe(struct 
platform_device *pdev)
ddr2_reg_base = pdata-ddr2_ctlr_base;
 
ddr2_pdown = pdata-ddr2_pdown;
-   cpuidle_set_statedata(device-states_usage[1], davinci_states[1]);
 
device-state_count = DAVINCI_CPUIDLE_MAX_STATES;
 
-- 
1.7.9.5


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev