tools/ramdump : Add PM dumpparser #4224

hasw7569 · 2020-03-06T07:59:23Z

This commit adds PM dumpparser.
It displays wakeup reason and stay time of each pm state and activities of registered driver in each pm state.
also It displays accumulated stay time for each pm state.

ex)
Domain number is 0

Stay time for each PM state

State	Stay(sec)
PM_NORMAL	3
PM_IDLE	5
PM_STANDBY	7
PM_SLEEP	0

Activities of registered drivers and Wakeup reason of Sleep state

State	Stay(sec)	Wakeup Reason	Drv_name , activity	Drv_name , activity	Drv_name , activity
PM_NORMAL	3	None	UART , 0	TEST1 , 0	TEST2 , 0
PM_IDLE	5	None	UART , 0	TEST1 , 0	TEST2 , 0
PM_STANDBY	7	None	UART , 0	TEST1 , 0	TEST2 , 0

Current state is PM_SLEEP

seinfra · 2020-03-06T08:01:20Z

Target : [5386b0494d24a9cc15d643a7c5648452f877403b] - Code Rule Check (C++) OK.

seinfra · 2020-03-06T08:01:20Z

Target : [5386b0494d24a9cc15d643a7c5648452f877403b] - Code Rule Check OK.

seinfra · 2020-03-06T08:19:31Z

Target : [b0730e38f7dea20aadefddfb9b655642f650050e] - Code Rule Check OK.

seinfra · 2020-03-06T08:19:31Z

Target : [b0730e38f7dea20aadefddfb9b655642f650050e] - Code Rule Check (C++) OK.

seinfra · 2020-03-09T02:43:20Z

Target : [38e2e01bcc7331393089a46ea07fd649a15531fb] - Code Rule Check (C++) OK.

seinfra · 2020-03-09T02:43:24Z

Target : [38e2e01bcc7331393089a46ea07fd649a15531fb] - Code Rule Check OK.

seinfra · 2020-03-09T02:49:30Z

Target : [571604dee0216541c903b99f7bcc7a6cb3c0a8ee] - Code Rule Check (C++) OK.

seinfra · 2020-03-09T02:49:46Z

Target : [571604dee0216541c903b99f7bcc7a6cb3c0a8ee] - Code Rule Check OK.

sunghan-chang · 2020-03-09T05:32:39Z

@hasw7569 Could you find some way to check pm enabled without external file, .config?

junmin-kim

I added some trivial comment.
Would you check these comment?

junmin-kim · 2020-03-10T09:47:39Z

tools/ramdump/dumpParser.py

+
+			STATE_STAY_POINT = g_pmglobals + DOMAIN_POINT + SIZE_OF_DOMAIN + 20
+			print('')
+			print('Domain number is %d'%i)


Is there a reason for using old formatting?

Suggested change

print('Domain number is %d'%i)

print('Domain number is', i)

junmin-kim · 2020-03-10T09:49:00Z

tools/ramdump/dumpParser.py

+			print('************************')
+			print('   State  |  Stay(sec) |')
+			print('----------|------------|')
+			for i in range(0, 4):


The start praramter is optional.

Suggested change

for i in range(0, 4):

for i in range(4):

junmin-kim · 2020-03-10T09:57:47Z

tools/ramdump/dumpParser.py

+			print('----------|------------|')
+			for i in range(0, 4):
+				STAY_TIME = rParser.read_halfword(STATE_STAY_POINT + (i * 2))
+				print('{:<10}|'.format(PM_STATUS[i]), '{:^11}|'.format(STAY_TIME))


How about this?

Suggested change

print('{:<10}|'.format(PM_STATUS[i]), '{:^11}|'.format(STAY_TIME))

print(f'{PM_STATUS[i]:<10}| {STAY_TIME:^11}|')

Thank you for your comments : )
But it makes build error in my enviroment.
Is this right syntax ??

Which version of python do you use?
If below code work on your environment, how about this?

Suggested change

print('{:<10}|'.format(PM_STATUS[i]), '{:^11}|'.format(STAY_TIME))

print('{:<10}| {:^11}|'.format(PM_STATUS[i], STAY_TIME))

It works well : )
Thank you.

seinfra · 2020-03-11T08:04:20Z

Target : [898c54bb797a688674014e372553f838fa427949] - Code Rule Check (C++) OK.

seinfra · 2020-03-11T08:04:22Z

Target : [898c54bb797a688674014e372553f838fa427949] - Code Rule Check OK.

dev-hue · 2020-03-10T03:12:48Z

tools/ramdump/dumpParser.py

+		fd.close()
+
+		if not 'CONFIG_PM_METRICS=y' in data:
+			print('PM_METRICS is not enable. Enable PM_METRICS to see power management information')


How about raise syntax instead of print and return?

seinfra · 2020-03-12T03:43:23Z

Target : [8f28d75e94f76d6635348b5af26d9f7bc21de3e4] - Code Rule Check OK.

seinfra · 2020-03-12T03:43:28Z

Target : [8f28d75e94f76d6635348b5af26d9f7bc21de3e4] - Code Rule Check (C++) OK.

Taejun-Kwon · 2020-03-16T02:46:43Z

os/board/common/crashdump.c

@@ -40,6 +40,7 @@
 #if defined(CONFIG_BOARD_RAMDUMP_UART)
 #define HANDSHAKE_STRING                "RAMDUMP"
 #define HANDSHAKE_STR_LEN_MAX           (7)
+#define SETBIT(target, value, shift) target |= (value << shift)


Instead of macro, you'd better define values first.
It is good habit to classify with just one variable or flag.

#ifdef CONFIG_PM_METRICS //these config is not really necessary, actually because all values are diffrent. #define RAMDUMP_CONFIG_PM 0x00 // but 0x00 is not good choice. start it from 0x01 #endif #ifdef CONFIG_BINMGR_RECOVERY #define RAMDUMP_CONFIG_BM 0x02 #endif ....

And then,

#ifdef CONFIG_PM_METRICS pm_config_info |= RAMDUMP_CONFIG_PM; #endif .... ...

Taejun-Kwon · 2020-03-16T03:01:10Z

tools/ramdump/ramdump_tool.c

+			config_info_size = sizeof(uint32_t);
+			while (config_info_cnt) {
+				while (config_info_size) {
+					ret = read(dev_fd, &buf, 1);


Is there any specific reason why it ready and write byte by byte?
Why don't you read & write just once with bytes of config_info_size?
I/O always takes most long time than any other operation, as possible as we can, we should reduce I/O

Taejun-Kwon · 2020-03-16T03:02:19Z

tools/ramdump/ramdump_tool.c

+				fclose(bin_fp);
+				return -1;
+			}
+			ret = fwrite(&buf, 1, 1, bin_fp);


As per my experience, streaming is always slower than POSIX, you have any reason why use fwrite instead of write?
you are already using 'read' in line 334 with another file descriptor...

jeongchanKim · 2020-03-16T23:54:48Z

os/board/common/crashdump.c

@@ -71,6 +72,37 @@ static int ramdump_via_uart(void)
 	char host_reg[1] = "";
 	char *target_str = HANDSHAKE_STRING;
 	char host_buf[HANDSHAKE_STR_LEN_MAX] = "";
+	uint8_t config_info_cnt = 0;
+#ifdef CONFIG_PM_METRICS


Please add description about each bit.

This commit adds PM dumpparser. It displays wakeup reason and stay time of each pm state and activities of registered driver in each pm state. also It displays accumulated stay time for each pm state. ex) Domain number is 0 *************************** Stay time for each PM state *************************** ************************ State | Stay(sec) | ----------|------------| PM_NORMAL | 3 | PM_IDLE | 5 | PM_STANDBY| 7 | PM_SLEEP | 0 | ***************************************************************** Activities of registered drivers and Wakeup reason of Sleep state ***************************************************************** ******************************************************************************************************************* State | Stay(sec) | Wakeup Reason | Drv_name , activity| Drv_name , activity| Drv_name , activity| ----------|------------|---------------------|----------------------|----------------------|----------------------| PM_NORMAL | 3 | None | UART , 0 | TEST1 , 0 | TEST2 , 0 | PM_IDLE | 5 | None | UART , 0 | TEST1 , 0 | TEST2 , 0 | PM_STANDBY| 7 | None | UART , 0 | TEST1 , 0 | TEST2 , 0 | Current state is PM_SLEEP

This commit updates common crashdump code to insert configuration informations into dumpfile.

seinfra · 2020-03-17T11:19:23Z

Target : [7a5d633] - Code Rule Check (C++) OK.

seinfra · 2020-03-17T11:19:26Z

Target : [7a5d633] - Code Rule Check OK.

tizen-build · 2023-03-21T02:16:49Z

Target : [7a5d633] - Code Rule Check OK.

hasw7569 force-pushed the dumpparser branch from 5386b04 to b0730e3 Compare March 6, 2020 08:16

hasw7569 force-pushed the dumpparser branch from b0730e3 to 38e2e01 Compare March 9, 2020 02:41

hasw7569 force-pushed the dumpparser branch from 38e2e01 to 571604d Compare March 9, 2020 02:48

sunghan-chang requested a review from davidfather March 9, 2020 05:44

junmin-kim reviewed Mar 10, 2020

View reviewed changes

hasw7569 force-pushed the dumpparser branch from 571604d to 898c54b Compare March 11, 2020 08:02

hasw7569 changed the title ~~TizenRT/tools/ramdump/dumpParser.py : Add PM dumpparser~~ tools/ramdump : Add PM dumpparser Mar 11, 2020

dev-hue reviewed Mar 12, 2020

View reviewed changes

hasw7569 force-pushed the dumpparser branch from 898c54b to 8f28d75 Compare March 12, 2020 03:42

Taejun-Kwon reviewed Mar 16, 2020

View reviewed changes

jeongchanKim reviewed Mar 16, 2020

View reviewed changes

hasw7569 added 2 commits March 17, 2020 20:13

board/common : Update common crashdump code

7a5d633

This commit updates common crashdump code to insert configuration informations into dumpfile.

hasw7569 force-pushed the dumpparser branch from 8f28d75 to 7a5d633 Compare March 17, 2020 11:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tools/ramdump : Add PM dumpparser #4224

tools/ramdump : Add PM dumpparser #4224

hasw7569 commented Mar 6, 2020 •

edited

Loading

seinfra commented Mar 6, 2020

seinfra commented Mar 6, 2020

seinfra commented Mar 6, 2020

seinfra commented Mar 6, 2020

seinfra commented Mar 9, 2020

seinfra commented Mar 9, 2020

seinfra commented Mar 9, 2020

seinfra commented Mar 9, 2020

sunghan-chang commented Mar 9, 2020

junmin-kim left a comment

junmin-kim Mar 10, 2020

junmin-kim Mar 10, 2020

junmin-kim Mar 10, 2020

hasw7569 Mar 10, 2020

junmin-kim Mar 11, 2020 •

edited

Loading

hasw7569 Mar 11, 2020

seinfra commented Mar 11, 2020

seinfra commented Mar 11, 2020

dev-hue Mar 10, 2020

seinfra commented Mar 12, 2020

seinfra commented Mar 12, 2020

Taejun-Kwon Mar 16, 2020

Taejun-Kwon Mar 16, 2020

Taejun-Kwon Mar 16, 2020

jeongchanKim Mar 16, 2020

seinfra commented Mar 17, 2020

seinfra commented Mar 17, 2020

tizen-build commented Mar 21, 2023

	print('{:<10}\|'.format(PM_STATUS[i]), '{:^11}\|'.format(STAY_TIME))
	print(f'{PM_STATUS[i]:<10}\| {STAY_TIME:^11}\|')

	print('{:<10}\|'.format(PM_STATUS[i]), '{:^11}\|'.format(STAY_TIME))
	print('{:<10}\| {:^11}\|'.format(PM_STATUS[i], STAY_TIME))

tools/ramdump : Add PM dumpparser #4224

Are you sure you want to change the base?

tools/ramdump : Add PM dumpparser #4224

Conversation

hasw7569 commented Mar 6, 2020 • edited Loading

seinfra commented Mar 6, 2020

seinfra commented Mar 6, 2020

seinfra commented Mar 6, 2020

seinfra commented Mar 6, 2020

seinfra commented Mar 9, 2020

seinfra commented Mar 9, 2020

seinfra commented Mar 9, 2020

seinfra commented Mar 9, 2020

sunghan-chang commented Mar 9, 2020

junmin-kim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junmin-kim Mar 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seinfra commented Mar 11, 2020

seinfra commented Mar 11, 2020

Choose a reason for hiding this comment

seinfra commented Mar 12, 2020

seinfra commented Mar 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seinfra commented Mar 17, 2020

seinfra commented Mar 17, 2020

tizen-build commented Mar 21, 2023

hasw7569 commented Mar 6, 2020 •

edited

Loading

junmin-kim Mar 11, 2020 •

edited

Loading