Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak. Derek Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58002 )
Change subject: include/device: Generic interface for USB-C mux operations ......................................................................
Patch Set 4:
(14 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/58002/comment/6c75c190_06faa8be PS2, Line 9: Create a generic EC interface for USB-C mux operations. Register : Chrome EC USB-C mux operations to the generic interface.
These should be split into two CLs: […]
Done
File src/ec/google/chromeec/usbc_mux.c:
PS2:
SPDX license header is missing.
Done
https://review.coreboot.org/c/coreboot/+/58002/comment/44120b10_78335a58 PS2, Line 2:
#include <usbc_mux. […]
included in ec.h
File src/include/usbc_mux.h:
https://review.coreboot.org/c/coreboot/+/58002/comment/52941a9a_9b7842eb PS3, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */
I think a better place for this file would be src/include/device: https://review.coreboot. […]
Done
https://review.coreboot.org/c/coreboot/+/58002/comment/cee8af45_3f376581 PS3, Line 8: Filpped
Flipped
Done
https://review.coreboot.org/c/coreboot/+/58002/comment/5aadc3b6_d659011f PS3, Line 13: uint8_t
Why not bool?
This field indicates DP pin assignment:
0h:Reserved. 1h: Mode A. 2h: Mode B. 3h: Mode C. 4h: Mode D. 5h: Mode E. 6h: Mode F. 7h-Fh: Reserved
I changed the name and comment
File src/include/usbc_mux.h:
PS2:
SPDX license header is missing.
Done
https://review.coreboot.org/c/coreboot/+/58002/comment/6db47d90_044e41e9 PS2, Line 5: Activ
Active
Done
https://review.coreboot.org/c/coreboot/+/58002/comment/8cd73d04_7ef5113c PS2, Line 6: *
What are the options? What do 0 and 1 indicate?
Done
https://review.coreboot.org/c/coreboot/+/58002/comment/0e918201_211f0220 PS2, Line 9: bool ufp; : bool acc;
No comments for these members?
Done
https://review.coreboot.org/c/coreboot/+/58002/comment/cd6bc053_8bcacf45 PS2, Line 11: uint8_t
Why not bool?
It's DP pin mode, the value should be:
0h:Reserved. 1h: Mode A. 2h: Mode B. 3h: Mode C. 4h: Mode D. 5h: Mode E. 6h: Mode F. 7h-Fh: Reserved.
https://review.coreboot.org/c/coreboot/+/58002/comment/3127b6cb_be39d3e1 PS2, Line 33: int
long
Done
https://review.coreboot.org/c/coreboot/+/58002/comment/1f87c71e_88a6b44f PS2, Line 51: int
long
Done
https://review.coreboot.org/c/coreboot/+/58002/comment/16a79d17_10e17c2a PS2, Line 59: ec_
Do we need `ec_` in the name? Just wondering if we can ever have a mux interface implementation prov […]
Agree that we may have a mux interface implementation provided by a non-EC driver. Remove `ec_` in the name