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:
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.
Done
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.
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
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
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
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
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:
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.
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