Change in ...coreboot[master]: util/superiotool/nuvoton.c: add NCT5539D register dump

List overview All Threads
Download

newer

older

Change in ...coreboot[master]:...

Pavel Sayekat (Code Review)

12 Jun 2019 12 Jun '19

12:54 p.m.

Pavel Sayekat has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33421

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard. Signed-off-by: pavelsayekat pavelsayekat@gmail.com Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 --- M util/superiotool/nuvoton.c 1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/1

diff --git a/util/superiotool/nuvoton.c b/util/superiotool/nuvoton.c index 01d195c..f79b2e2 100644 --- a/util/superiotool/nuvoton.c +++ b/util/superiotool/nuvoton.c @@ -582,7 +582,7 @@ {0x30,0xe0,0xe1,0xe2,0xe3,EOT}, {0x20,0x20,0x04,0x05,0x01,EOT}}, {EOT}}}, - {0xd121, "NCT5539D (not all LDNs supported)", { + {0xd121, "NCT5539D", { {NOLDN, NULL, {0x07,0x10,0x11,0x13,0x14,0x1a,0x1b,0x1c,0x1d,0x20,0x21,0x22,0x24,0x25,0x26,0x27,0x28,0x2a,0x2b,0x2c,0x2d,0x2f,EOT}, {0x00,0xff,0xff,0x00,0x00,0x30,0x50,0x10,0x00,0xd1,0x21,0xff,0x04,0x00,MISC,0x03,0x00,0xc0,0x00,0x01,MISC,MISC,EOT}},

Attachments:

attachment.htm (text/html — 2.6 KB)

Show replies by date

HAOUAS Elyes (Code Review)

12 Jun 12 Jun

1:22 p.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/33421/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/#/c/33421/1//COMMIT_MSG@10 PS1, Line 10: Tested on ASUS-H110M-E/M.2 mainboard. : Signed-off-by: pavelsayekat pavelsayekat@gmail.com : Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Tested on ASUS-H110M-E/M.2 mainboard.

Signed-off-by: pavelsayekat pavelsayekat@gmail.com Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3

HAOUAS Elyes (Code Review)

1:26 p.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/33421/1/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/1/util/superiotool/nuvoton.c@585 PS1, Line 585: "NCT5539D" what does this fix?

Pavel Sayekat (Code Review)

1:50 p.m.

Hello Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#2).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/2

Pavel Sayekat (Code Review)

8:05 p.m.

Hello Felix Held, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#3).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard. Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: pavelsayekat pavelsayekat@gmail.com --- M util/superiotool/nuvoton.c 1 file changed, 50 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/3

Pavel Sayekat (Code Review)

8:32 p.m.

Hello Felix Held, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#4).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/4

HAOUAS Elyes (Code Review)

9:21 p.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/33421/4//COMMIT_MSG Commit Message:

https://review.coreboot.org/#/c/33421/4//COMMIT_MSG@10 PS4, Line 10: Tested on ASUS-H110M-E/M.2 mainboard. missing a blank line it should be as:

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard.

Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: pavelsayekat pavelsayekat@gmail.com

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 4 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: HAOUAS Elyes ehaouas@noos.fr Gerrit-Comment-Date: Wed, 12 Jun 2019 19:21:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

9:34 p.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/33421/4//COMMIT_MSG Commit Message:

https://review.coreboot.org/#/c/33421/4//COMMIT_MSG@12 PS4, Line 12: pavelsayekat Pavel Sayekat

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 4 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: HAOUAS Elyes ehaouas@noos.fr Gerrit-Comment-Date: Wed, 12 Jun 2019 19:34:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Arthur Heymans (Code Review)

9:35 p.m.

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/33421/4/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/4/util/superiotool/nuvoton.c@587 PS4, Line 587: {0x07,0x10,0x11,0x13,0x14,0x1a,0x1b,0x1c,0x1d,0x20,0x21,0x22,0x24,0x25,0x26,0x27,0x28,0x2a,0x2b,0x2c,0x2d,0x2f,EOT}, please add some newlines to make it easier to read.

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 4 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-CC: HAOUAS Elyes ehaouas@noos.fr Gerrit-Comment-Date: Wed, 12 Jun 2019 19:35:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Pavel Sayekat (Code Review)

10:22 p.m.

Hello Felix Held, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#5).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard. Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: Pavel Sayekat pavelsayekat@gmail.com --- M util/superiotool/nuvoton.c 1 file changed, 50 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/5

Pavel Sayekat (Code Review)

10:41 p.m.

Hello Felix Held, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#6).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard. Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: Pavel Sayekat pavelsayekat@gmail.com --- M util/superiotool/nuvoton.c 1 file changed, 50 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/6

Pavel Sayekat (Code Review)

10:50 p.m.

Hello Felix Held, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#7).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard. Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: Pavel Sayekat pavelsayekat@gmail.com --- M util/superiotool/nuvoton.c 1 file changed, 54 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/7

Pavel Sayekat (Code Review)

10:52 p.m.

Hello Felix Held, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#8).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard.

Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: Pavel Sayekat pavelsayekat@gmail.com --- M util/superiotool/nuvoton.c 1 file changed, 54 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/8

Pavel Sayekat (Code Review)

10:54 p.m.

Hello Felix Held, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#9).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard.

Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: Pavel Sayekat pavelsayekat@gmail.com --- M util/superiotool/nuvoton.c 1 file changed, 54 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/9

HAOUAS Elyes (Code Review)

11:06 p.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 9: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 9 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-Comment-Date: Wed, 12 Jun 2019 21:06:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Pavel Sayekat (Code Review)

11:13 p.m.

Pavel Sayekat has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 9:

(3 comments)

...

Patch Set 4:

(1 comment)

ok, the amendments have been made as suggested, thanks.

https://review.coreboot.org/#/c/33421/1//COMMIT_MSG Commit Message:

...

Tested on ASUS-H110M-E/M.2 mainboard. […]

I will try to put next commits in better order.

https://review.coreboot.org/#/c/33421/4//COMMIT_MSG Commit Message:

https://review.coreboot.org/#/c/33421/4//COMMIT_MSG@10 PS4, Line 10: Tested on ASUS-H110M-E/M.2 mainboard.

...

missing a blank line […]

Its done :)

https://review.coreboot.org/#/c/33421/1/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/1/util/superiotool/nuvoton.c@585 PS1, Line 585: "NCT5539D"

...

what does this fix?

Actually all LDNs are supported.

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 9 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-Comment-Date: Wed, 12 Jun 2019 21:13:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: HAOUAS Elyes ehaouas@noos.fr Gerrit-MessageType: comment

Felix Held (Code Review)

11:29 p.m.

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 9:

(31 comments)

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@587 PS5, Line 587: 0x20,0x21 ID registers don't have to be included here, since they're already used to select this chip's data structures in superiotool. doen't hurt to include them though.

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@587 PS5, Line 587: 0x07 LDN select register not really needed here

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@600 PS5, Line 600: 0x00 hm, the state of bits in this register where register 0xe0 is 0 is dependent on the external circuitry. since it's not only the input register, i don't have a too strong opinion on this one; however the default pin direction is input and in this case the register state is NANA

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@600 PS5, Line 600: 0x00 NANA

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@600 PS5, Line 600: 0x00 see my comment on 0xe1

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@601 PS5, Line 601: WDT3, WDT3 is mentioned twice; remove one

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@603 PS5, Line 603: 0x00 NANA

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@603 PS5, Line 603: 0x00 same comment as in LDN 7

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@587 PS9, Line 587: 0x07 LDN select register not needed here

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@587 PS9, Line 587: 0x20,0x21 chip id register not needed, since this is used to select this chip

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@602 PS9, Line 602: 0x00 NANA

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@602 PS9, Line 602: 0x00 when register 0xe0 is in its default state, this is rather NANA

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@602 PS9, Line 602: 0x00 see comment on LDN7 0xe1

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@603 PS9, Line 603: WDT3, WDT3 mentioned twice; remove one

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@605 PS9, Line 605: 0x00 see comment on LDN7 0xe1

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@605 PS9, Line 605: 0x00 NANA

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00 NANA

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00 see comment on LDN7 0xe1

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00 NANA

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00 see comment on LDN7 0xe1

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00 NANA

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00 see comment on LDN7 0xe1

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00 NANA

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@613 PS9, Line 613: {0x30,0x60,0x61,0x70,0xe0,0xe1,0xe2,0xe3,0xe4,0xe5,0xe6,0xe7,0xec,0xed,0xf0,0xf1,0xf2,0xf6,0xf7,0xf8,0xf9, : 0xfa,0xfb,0xfc,EOT}, : {0x00,0x00,0x00,0x00,0x7f,0x7f,0x7f,0x7f,0x7f,0xa8,0x08,0x7f, : 0x00,0x81,0x00,0x00,0x00,0x00,0x87,0x47,0x00,0x00,0x00,0x02,EOT}}, inconsistent formatting

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@626 PS9, Line 626: Register remove "Register"?

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@629 PS9, Line 629: Register remove "Register"?

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@632 PS9, Line 632: Register remove "Register"?

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@637 PS9, Line 637: 0xa0 this seems to be a bit odd to me; i'd have expected 0x00 here. did you try reading this from the chip on the mainboard and if so what value did you get there?

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 9 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-Comment-Date: Wed, 12 Jun 2019 21:29:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Felix Held (Code Review)

11:55 p.m.

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 9:

(1 comment)

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@637 PS9, Line 637: 0xa0

...

this seems to be a bit odd to me; i'd have expected 0x00 here. […]

from the superiotool output 0xa0 seems to be the correct value

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 9 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-Comment-Date: Wed, 12 Jun 2019 21:55:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held felix-coreboot@felixheld.de Gerrit-MessageType: comment

Pavel Sayekat (Code Review)

13 Jun 13 Jun

12:38 a.m.

Hello Felix Held, HAOUAS Elyes, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#10).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard.

Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: Pavel Sayekat pavelsayekat@gmail.com --- M util/superiotool/nuvoton.c 1 file changed, 60 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/10

Felix Held (Code Review)

2:29 a.m.

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 10:

(1 comment)

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c@611 PS10, Line 611: {0x00,0xff,0x00,0x00,0x00,0x7f,0x00,0x00,0x00,0x00,0x00,0x00, : 0x00,0x00,0xff,0x00,0x00,0xff,0x00,0x00,0x00,EOT}} see my comments on the last revision of this part

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 10 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-Comment-Date: Thu, 13 Jun 2019 00:29:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Angel Pons (Code Review)

4:48 a.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 10:

(9 comments)

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@637 PS9, Line 637: 0xa0

...

from the superiotool output 0xa0 seems to be the correct value

The datasheet says something about this register:

Attribute: R/W; Power well: VRTC; Reset by: Battery reset. Default: a0h

Bits 7-2 are reserved. Bit 1 is Deep S3 Enable. Bit 0 is Deep S5 Enable.

The default value of 'a0h' is rather weird. It's also the one using an hex digit in lowercase. Noteworthy.

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c@611 PS10, Line 611: 0x00 NANA

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c@612 PS10, Line 612: 0x00 NANA

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c@638 PS10, Line 638: Maximun Duty Cycle Value Please remove, it is specific to CR 0xf0

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 10 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-Comment-Date: Thu, 13 Jun 2019 02:48:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held felix-coreboot@felixheld.de Gerrit-MessageType: comment

Pavel Sayekat (Code Review)

7:59 a.m.

Hello Felix Held, HAOUAS Elyes, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#11).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard.

Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: Pavel Sayekat pavelsayekat@gmail.com --- M util/superiotool/nuvoton.c 1 file changed, 60 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/11

Angel Pons (Code Review)

1:31 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 12: Code-Review+1

(2 comments)

Just a few nits here and there, but at least the hex values look good.

https://review.coreboot.org/#/c/33421/12/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/12/util/superiotool/nuvoton.c@587 PS12, Line 587: {0x10,0x11,0x13,0x14,0x1a,0x1b,0x1c,0x1d,0x22,0x24,0x25,0x26,0x27,0x28,0x2a, : 0x2b,0x2c,0x2d,0x2f,EOT}, Please split this in two lines that are similarly long.

Same thing for the other long lines.

https://review.coreboot.org/#/c/33421/12/util/superiotool/nuvoton.c@638 PS12, Line 638: Led LED

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 12 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-Comment-Date: Thu, 13 Jun 2019 11:31:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Pavel Sayekat (Code Review)

1:37 p.m.

Hello Felix Held, HAOUAS Elyes, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#13).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard (Kabylake i3-7100 CPU).

Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: Pavel Sayekat pavelsayekat@gmail.com --- M util/superiotool/nuvoton.c 1 file changed, 60 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/13

Pavel Sayekat (Code Review)

1:54 p.m.

Hello Felix Held, HAOUAS Elyes, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#14).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard (Kabylake i3-7100 CPU).

Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: Pavel Sayekat pavelsayekat@gmail.com --- M util/superiotool/nuvoton.c 1 file changed, 60 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/14

Pavel Sayekat (Code Review)

2:34 p.m.

Hello Felix Held, HAOUAS Elyes, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#15).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard (Kabylake i3-7100 CPU).

Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: Pavel Sayekat pavelsayekat@gmail.com --- M util/superiotool/nuvoton.c 1 file changed, 64 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/15

Angel Pons (Code Review)

2:43 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 15:

(1 comment)

https://review.coreboot.org/#/c/33421/15/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/15/util/superiotool/nuvoton.c@390 PS15, Line 390: util/superiotool/nuvoton.c: add NCT5539D register dump : : Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). : Tested on ASUS-H110M-E/M.2 mainboard (Kabylake i3-7100 CPU). Oops.

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 15 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-Comment-Date: Thu, 13 Jun 2019 12:43:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Pavel Sayekat (Code Review)

2:49 p.m.

Hello Felix Held, HAOUAS Elyes, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#16).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard (Kabylake i3-7100 CPU).

Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: Pavel Sayekat pavelsayekat@gmail.com --- M util/superiotool/nuvoton.c 1 file changed, 60 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/16

Pavel Sayekat (Code Review)

3:11 p.m.

Pavel Sayekat has removed a vote on this change.

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Removed Verified+1 by build bot (Jenkins) no-reply@coreboot.org

Pavel Sayekat (Code Review)

3:22 p.m.

Hello Felix Held, HAOUAS Elyes, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#17).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard (Kabylake i3-7100 CPU).

Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: Pavel Sayekat pavelsayekat@gmail.com --- M util/superiotool/nuvoton.c 1 file changed, 60 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/17

Angel Pons (Code Review)

3:35 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 17: Code-Review+1

(2 comments)

https://review.coreboot.org/#/c/33421/17/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/17/util/superiotool/nuvoton.c@620 PS17, Line 620: 0xf9,0xfa,0xfb,0xfc,EOT}, Put this on another line please

https://review.coreboot.org/#/c/33421/17/util/superiotool/nuvoton.c@622 PS17, Line 622: 0x00,0x00,0x00,0x02,EOT}}, Same

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 17 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-Comment-Date: Thu, 13 Jun 2019 13:35:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Pavel Sayekat (Code Review)

3:44 p.m.

Hello Felix Held, HAOUAS Elyes, Angel Pons, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33421

to look at the new patch set (#18).

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard (Kabylake i3-7100 CPU).

Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: Pavel Sayekat pavelsayekat@gmail.com --- M util/superiotool/nuvoton.c 1 file changed, 62 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/33421/18

Angel Pons (Code Review)

3:49 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 18: Code-Review+2

Looks good.

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 18 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-Comment-Date: Thu, 13 Jun 2019 13:49:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Felix Held (Code Review)

4:11 p.m.

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 18: Code-Review+2

looks good to me; will merge this tomorrow :)

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 18 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-Comment-Date: Thu, 13 Jun 2019 14:11:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Felix Held (Code Review)

14 Jun 14 Jun

2:17 p.m.

Felix Held has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

util/superiotool/nuvoton.c: add NCT5539D register dump

Values taken from NCT5539D datasheet V1.1 (June 30th, 2015). Tested on ASUS-H110M-E/M.2 mainboard (Kabylake i3-7100 CPU).

Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Signed-off-by: Pavel Sayekat pavelsayekat@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/33421 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M util/superiotool/nuvoton.c 1 file changed, 62 insertions(+), 0 deletions(-)

Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved Angel Pons: Looks good to me, approved

diff --git a/util/superiotool/nuvoton.c b/util/superiotool/nuvoton.c index 82772e7..7552244 100644 --- a/util/superiotool/nuvoton.c +++ b/util/superiotool/nuvoton.c @@ -582,6 +582,68 @@ {0x30,0xe0,0xe1,0xe2,0xe3,EOT}, {0x20,0x20,0x04,0x05,0x01,EOT}}, {EOT}}}, + {0xd121, "NCT5539D", { + {NOLDN, NULL, + {0x10,0x11,0x13,0x14,0x1a,0x1b,0x1c,0x1d,0x22,0x24, + 0x25,0x26,0x27,0x28,0x2a,0x2b,0x2c,0x2d,0x2f,EOT}, + {0xff,0xff,0x00,0x00,0x30,0x50,0x10,0x00,0xff,0x04, + 0x00,MISC,0x03,0x00,0xc0,0x00,0x01,MISC,MISC,EOT}}, + {0x02, "UART A", + {0x30,0x60,0x61,0x70,0xf0,0xf2,EOT}, + {0x01,0x03,0xf8,0x04,0x00,0x00,EOT}}, + {0x05, "Keyboard Controller", + {0x30,0x60,0x61,0x62,0x63,0x70,0x72,0xf0,EOT}, + {0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x83,EOT}}, + {0x06, "CIR", + {0x30,0x60,0x61,0x70,EOT}, + {0x00,0x00,0x00,0x00,EOT}}, + {0x07, "GPIO 7, GPIO 8", + {0x30,0xe0,0xe1,0xe2,0xe3,0xe4,0xe5,0xe6,0xe7,0xec,0xed,EOT}, + {0x00,0xff,NANA,0x00,NANA,0xff,NANA,0x00,NANA,0x00,0x00,EOT}}, + {0x08, "WDT1, WDT3, GPIO 0, KBC P20", + {0x30,0x60,0x61,0xe0,0xe1,0xe2,0xe3,0xe4,0xf5,0xf6, + 0xf7,0xf8,0xf9,0xfa,0xfe,0xff,EOT}, + {0x00,0x00,0x00,0xff,NANA,0x00,NANA,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00, 0x00,EOT}}, + {0x09, "GPIO 2, GPIO 3, GPIO 4, GPIO 5", + {0x30,0xe0,0xe1,0xe2,0xe3,0xe4,0xe5,0xe6,0xe7,0xe8, + 0xe9,0xea,0xeb,0xee, 0xf0,0xf1,0xf2,0xf4,0xf5,0xf6,0xf7,EOT}, + {0x00,0xff,NANA,0x00,NANA,0x7f,NANA,0x00,NANA,NANA, + 0x00,0x00,0x00,0x00,0xff,NANA,0x00,0xff,NANA,0x00,NANA,EOT}}, + {0x0a, "ACPI", + {0xe0,0xe1,0xe2,0xe3,0xe4,0xe5,0xe6,0xe7,0xe9,0xec, + 0xed,0xee,0xf0,0xf1,0xf2,0xf3,0xf6,0xf7,0xfc,EOT}, + {0x01,0x00,0x00,0x00,0x00,0x02,0x1a,0x00,0x00,0x00, + 0x01,0x00,0x10,0x48,0x5c,0x00,0x00,0xc0,0x80,EOT}}, + {0x0b, "Hardware Monitor, Front Panel LED", + {0x30,0x60,0x61,0x70,0xe0,0xe1,0xe2,0xe3,0xe4,0xe5, + 0xe6,0xe7,0xec,0xed,0xf0,0xf1,0xf2,0xf6,0xf7,0xf8, + 0xf9,0xfa,0xfb,0xfc,EOT}, + {0x00,0x00,0x00,0x00,0x7f,0x7f,0x7f,0x7f,0x7f,0xa8, + 0x08,0x7f,0x00,0x81,0x00,0x00,0x00,0x00,0x87,0x47, + 0x00,0x00,0x00,0x02,EOT}}, + {0x0d, "BCLK, WDT2", + {0xe7,0xe8,0xeb,0xed,0xf0,0xf3,EOT}, + {0x10,0x32,0x14,0x00,0x00,0x00,EOT}}, + {0x0e, "CIR Wake-Up", + {0x30,0x60,0x61,0x70,EOT}, + {0x00,0x00,0x00,0x00,EOT}}, + {0x0f, "GPIO Push-Pull or Open-Drain selection", + {0xe1,0xe2,0xe3,0xe4,0xe6,0xe7,0xe9,EOT}, + {0xff,0x7f,0xff,0xff,0xff,0xff,0xff,EOT}}, + {0x11, "GPIO, RI PSOUT Wake-Up Status", + {0xe1,0xe2,0xe9,EOT}, + {0x00,0x00,0x00,EOT}}, + {0x12, "SW Error Control", + {0xe1,0xe7,0xea,0xeb,0xec,EOT}, + {0x00,0x00,0x10,0x00,0x00,EOT}}, + {0x15, "Fading LED", + {0xf0,0xf1,0xf2,0xf3,0xf4,0xf5,0xf6,0xf7,0xf8,EOT}, + {0xff,0x7f,0x00,0x56,0x56,0x17,0x00,0x0f,0x10,EOT}}, + {0x16, "Deep Sleep", + {0x30,0xe0,0xe1,0xe2,0xe3,EOT}, + {0xa0,0x20,0x04,0x05,0x01,EOT}}, + {EOT}}}, {EOT} };

Pavel Sayekat (Code Review)

30 Jun 30 Jun

7:43 a.m.

Pavel Sayekat has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33421 )

Change subject: util/superiotool/nuvoton.c: add NCT5539D register dump ......................................................................

Patch Set 19:

(49 comments)

https://review.coreboot.org/#/c/33421/4//COMMIT_MSG Commit Message:

https://review.coreboot.org/#/c/33421/4//COMMIT_MSG@12 PS4, Line 12: pavelsayekat

...

Pavel Sayekat

Done

https://review.coreboot.org/#/c/33421/4//COMMIT_MSG@12 PS4, Line 12: pavelsayekat

...

Pavel Sayekat

Its done :)

https://review.coreboot.org/#/c/33421/4/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

...

please add some newlines to make it easier to read.

Done

...

please add some newlines to make it easier to read.

Its done :).

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@587 PS5, Line 587: 0x20,0x21

...

ID registers don't have to be included here, since they're already used to select this chip's data s […]

Done

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@587 PS5, Line 587: 0x07

...

LDN select register not really needed here

Done

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@600 PS5, Line 600: 0x00

...

hm, the state of bits in this register where register 0xe0 is 0 is dependent on the external circuit […]

Done

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@600 PS5, Line 600: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@600 PS5, Line 600: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@600 PS5, Line 600: 0x00

...

see my comment on 0xe1

Done

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@601 PS5, Line 601: WDT3,

...

WDT3 is mentioned twice; remove one

Done

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@603 PS5, Line 603: 0x00

...

same comment as in LDN 7

Done

https://review.coreboot.org/#/c/33421/5/util/superiotool/nuvoton.c@603 PS5, Line 603: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@587 PS9, Line 587: 0x07

...

LDN select register not needed here

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@587 PS9, Line 587: 0x20,0x21

...

chip id register not needed, since this is used to select this chip

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@602 PS9, Line 602: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@602 PS9, Line 602: 0x00

...

see comment on LDN7 0xe1

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@602 PS9, Line 602: 0x00

...

when register 0xe0 is in its default state, this is rather NANA

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@602 PS9, Line 602: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@603 PS9, Line 603: WDT3,

...

WDT3 mentioned twice; remove one

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@605 PS9, Line 605: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@605 PS9, Line 605: 0x00

...

see comment on LDN7 0xe1

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00

...

see comment on LDN7 0xe1

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00

...

see comment on LDN7 0xe1

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00

...

see comment on LDN7 0xe1

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00

...

see comment on LDN7 0xe1

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@608 PS9, Line 608: 0x00

...

NANA

Done

...

inconsistent formatting

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@626 PS9, Line 626: Register

...

remove "Register"?

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@629 PS9, Line 629: Register

...

remove "Register"?

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@632 PS9, Line 632: Register

...

remove "Register"?

Done

https://review.coreboot.org/#/c/33421/9/util/superiotool/nuvoton.c@637 PS9, Line 637: 0xa0

...

The datasheet says something about this register: […]

Done

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c@611 PS10, Line 611: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c@611 PS10, Line 611: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c@611 PS10, Line 611: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c@611 PS10, Line 611: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c@612 PS10, Line 612: 0x00

...

NANA

Done

...

see my comments on the last revision of this part

Done

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c@612 PS10, Line 612: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c@612 PS10, Line 612: 0x00

...

NANA

Done

https://review.coreboot.org/#/c/33421/10/util/superiotool/nuvoton.c@638 PS10, Line 638: Maximun Duty Cycle Value

...

Please remove, it is specific to CR 0xf0

Done

https://review.coreboot.org/#/c/33421/12/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/12/util/superiotool/nuvoton.c@587 PS12, Line 587: {0x10,0x11,0x13,0x14,0x1a,0x1b,0x1c,0x1d,0x22,0x24,0x25,0x26,0x27,0x28,0x2a, : 0x2b,0x2c,0x2d,0x2f,EOT},

...

Please split this in two lines that are similarly long. […]

Done

https://review.coreboot.org/#/c/33421/12/util/superiotool/nuvoton.c@638 PS12, Line 638: Led

...

LED

Done

https://review.coreboot.org/#/c/33421/15/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

...

Oops.

Done

https://review.coreboot.org/#/c/33421/17/util/superiotool/nuvoton.c File util/superiotool/nuvoton.c:

https://review.coreboot.org/#/c/33421/17/util/superiotool/nuvoton.c@620 PS17, Line 620: 0xf9,0xfa,0xfb,0xfc,EOT},

...

Put this on another line please

Done

https://review.coreboot.org/#/c/33421/17/util/superiotool/nuvoton.c@622 PS17, Line 622: 0x00,0x00,0x00,0x02,EOT}},

...

Same

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/33421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib55068035ca574b22d0fea81e1a291f7cf0329d3 Gerrit-Change-Number: 33421 Gerrit-PatchSet: 19 Gerrit-Owner: Pavel Sayekat Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Pavel Sayekat Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-Comment-Date: Sun, 30 Jun 2019 05:43:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: HAOUAS Elyes ehaouas@noos.fr Comment-In-Reply-To: Felix Held felix-coreboot@felixheld.de Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Comment-In-Reply-To: Arthur Heymans arthur@aheymans.xyz Gerrit-MessageType: comment

1911

days inactive

1929

days old

coreboot-gerrit@coreboot.org

35 comments

5 participants

tags (0)

participants (5)

Angel Pons (Code Review)
Arthur Heymans (Code Review)
Felix Held (Code Review)
HAOUAS Elyes (Code Review)
Pavel Sayekat (Code Review)