Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk.
Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72807 )
Change subject: ch341a_spi: Refactor singleton states into reentrant pattern ......................................................................
Patch Set 6:
(18 comments)
Patchset:
PS5:
I added many cleanups you can do as a follow up patch to make type usage consistent, const correct a […]
Thank you! I've marked them as resolved, because they are not the part of this patch.
File ch341a_spi.c:
https://review.coreboot.org/c/flashrom/+/72807/comment/a6b5aaf3_5d5b2b72 PS3, Line 80: struct libusb_transfer *transfer_out; : struct libusb_transfer *transfer_ins[USB_IN_TRANSFERS]; : : /* Accumulate delays to be plucked between CS deassertion and CS assertions. */ : unsigned int stored_delay_us; : : struct libusb_device_handle *handle;
I recommend putting the handle at the start of the struct so that overflows in the above transfer bu […]
Done
https://review.coreboot.org/c/flashrom/+/72807/comment/07b8e673_ff953ee0 PS3, Line 100: i = 0
`size_t i = 0;`
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/f28af886_3524c81f PS3, Line 234: unsigned int i;
delete
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/95d8801e_e2e4370c PS3, Line 235: i
probably should be `size_t i = 0;`
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/f012149e_b35157d2 PS3, Line 268: uint8_t
`const`?
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/2348ed18_a852ce3a PS3, Line 311: stored_delay_us
`delay_us`
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/a8428fe6_ea553959 PS3, Line 315: int
`unsigned int` ?
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/214eadc1_8ab5fa95 PS3, Line 323: i = 0
`unsigned int i = 0;`
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/49b9779b_1b4bb053 PS3, Line 365: nsigned int p;
delete
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/036b14e2_bd057526 PS3, Line 366: p
actually should be `size_t p = 0;`
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/b1ffb19c_81525f9f PS3, Line 387: i
`unsigned int i`
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/c40887bd_8d95b5db PS3, Line 419: 4 * 1024
`4*KiB` in another patch?
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/c26c0007_5b7fae82 PS3, Line 460: ret != 0
`ret`, C will auto-cast to make the predicate boolean.
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/5a641500_337db69b PS3, Line 465: (ret != 0)
`(ret)` is canonical.
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/112c2e12_ca8c4316 PS3, Line 495: i
`int i` in another patch?
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/71ae74ff_96a80eca PS3, Line 504: i = 0
ditto
Ack
https://review.coreboot.org/c/flashrom/+/72807/comment/6519b07c_5138c0df PS3, Line 513: i = 0
ditto
Ack