Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35726 )
Change subject: drivers/aspeed/common: Add support for high resolution framebuffer ......................................................................
Patch Set 7:
(6 comments)
https://review.coreboot.org/c/coreboot/+/35726/5/src/drivers/aspeed/common/a... File src/drivers/aspeed/common/ast_i2c.c:
https://review.coreboot.org/c/coreboot/+/35726/5/src/drivers/aspeed/common/a... PS5, Line 123: backup = software_i2c[ASPEED_BUS]; : : software_i2c[ASPEED_BUS] = &ast_ops; :
Well, I see what this code is doing. But it's just weird and a bad […]
I don't see a better solution
https://review.coreboot.org/c/coreboot/+/35726/5/src/drivers/aspeed/common/a... PS5, Line 133: ast = ast_priv;
I see that too.
Done
https://review.coreboot.org/c/coreboot/+/35726/6/src/drivers/aspeed/common/a... File src/drivers/aspeed/common/ast_mode.c:
https://review.coreboot.org/c/coreboot/+/35726/6/src/drivers/aspeed/common/a... PS6, Line 29: /* : * Authors: Dave Airlie airlied@redhat.com : * Authors: 9Elements Agency GmbH patrick.rudolph@9elements.com
move up below copyright?
Done
https://review.coreboot.org/c/coreboot/+/35726/6/src/drivers/aspeed/common/a... PS6, Line 34: */ : #include
newline after comment
Done
https://review.coreboot.org/c/coreboot/+/35726/6/src/drivers/aspeed/common/a... PS6, Line 37:
why two newlines?
in case one doesn't work well enough
https://review.coreboot.org/c/coreboot/+/35726/6/src/drivers/aspeed/common/a... File src/drivers/aspeed/common/ast_tables.h:
https://review.coreboot.org/c/coreboot/+/35726/6/src/drivers/aspeed/common/a... PS6, Line 147: 0x67, : {0x00, 0x03, 0x00, 0x02}, : {0x5f, 0x4f, 0x50, 0x82, 0x55, 0x81, 0xbf, 0x1f, : 0x00, 0x4f, 0x0d, 0x0e, 0x00, 0x00, 0x00, 0x00, : 0x9c, 0x8e, 0x8f, 0x28, 0x1f, 0x96, 0xb9, 0xa3, : 0xff}, : {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x14, 0x07, : 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f, : 0x0c, 0x00, 0x0f, 0x08}, : {0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x0e, 0x00, : 0xff}
This is all cosmetic?
no the linter errors out if there's no whitespace