Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33623 )
Change subject: superio/fintek: Add f81803a ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/#/c/33623/5/src/superio/fintek/f81803a/fan_contr... File src/superio/fintek/f81803a/fan_control.c:
https://review.coreboot.org/#/c/33623/5/src/superio/fintek/f81803a/fan_contr... PS5, Line 44: use_mask = mask << shift; : use_value = (value & mask) << shift;
Not sure if I understand what you're saying. […]
i meant replacing line 41, 44 and 45 with this:
u8 use_mask = mask << shift; u8 use_value = (value & mask) << shift;
shorter and imho a bit easier to read
https://review.coreboot.org/#/c/33623/5/src/superio/fintek/f81803a/fan_contr... PS5, Line 51: static inline void set_prog_sel(u16 address, u8 value)
Yes, registers 96 through 9e have 2 banks. Will add comment.
ok. a function name like "select_hwm_bank" would make the code easier to read imho
https://review.coreboot.org/#/c/33623/5/src/superio/fintek/f81803a/fan_contr... PS5, Line 232: 0
I try to exit pointing to bank 0 (default). But sometimes I have to access bank 1.
ok. i just wondered about this and when I'm not sure I ask if things are correct