Thanks for the review!
15 comments:
I'm afraid that this isn't the case here.
I reworked this patch according to your comments.
File src/ec/kontron/kempld/chip.h:
Patch Set #2, Line 6: #define KEMPLD_NUM_UARTS 2
I doubt using a tab here provides any visual benefit.
Ack
File src/ec/kontron/kempld/early_kempld.c:
This newline is intentional, since both types of headers are different.
Ack
Patch Set #2, Line 45: KEMPLD_UART_ENABLE | KEMPLD_UART_3F8 << KEMPLD_UART_IO_SHIFT);
Please keep the line aligned: […]
Done
Patch Set #2, Line 49: KEMPLD_UART_ENABLE | KEMPLD_UART_2F8 << KEMPLD_UART_IO_SHIFT);
Same here
Done
File src/ec/kontron/kempld/kempld.c:
Same as before
Ack
This blank line separates fixed constants from dev-dependent constants.
Ack
This one separates regular variable assignments from function calls.
Ack
Patch Set #2, Line 58: const unsigned int uart = dev->path.generic.subid;
I'd rather keep these where they were before. If we can't get the mutex, we don't need these.
Ack
File src/ec/kontron/kempld/kempld_i2c.c:
Why is this gone?
canceled
Same as before
canceled
Patch Set #2, Line 40: #define EBUSY 16
What does this fix? With the spaces, the numbers were aligned: ones with ones, tens with tens...
canceled
This was aligned with tabs so that the difference between types and member names was clearer.
canceled
Patch Set #2, Line 225: .ops_i2c_bus = &kempld_i2c_bus_ops,
Why break the alignment?
canceled
File src/ec/kontron/kempld/kempld_internal.h:
Patch Set #2, Line 11: #define KEMPLD_DAT 0xa81
These aren't indexed registers, so there's no need to align them with those.
Ack
To view, visit change 45372. To unsubscribe, or for help writing mail filters, visit settings.