Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41758 )
Change subject: mb/intel/jasperlake_rvp: camera SSDT changes for JSLRVP ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41758/5/src/mainboard/intel/jasperl... File src/mainboard/intel/jasperlake_rvp/variants/jslrvp/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/41758/5/src/mainboard/intel/jasperl... PS5, Line 320: register "on_seq.ops[0].type" = "IMGCLK" : register "on_seq.ops[0].index" = "0" #clk enable : register "on_seq.ops[0].action" = "ENABLE" suggestion: making a macro to build these structs could make this easier to read.
maybe:
#define SEQ_OPS_CLK(index, act) \ {.type=IMGCLK, .index = (index), .action = (act) } and #define SEQ_OPS_GPIO(index, act, delay) \ {.type=GPIO, .index = (index), .action = (act), .delay_ms = (delay) }
etc.
it would also reduce each one to one line which greatly shortens the devicetree 😊
https://review.coreboot.org/c/coreboot/+/41758/5/src/mainboard/intel/jasperl... PS5, Line 446: register "cio2_lanes_used[0]" = "2" : register "cio2_lanes_used[1]" = "2" could be register "cio2_lanes_used" = "{2, 2}"