[L] Change in coreboot[master]: ec/google/chromeec: Add handles for ori, usb role switch
Attention is currently required from: Robert Zieba, Tarun Tuli, Subrata Banik, Caveh Jalali, Kapil Porwal, Nick Vaccaro, Boris Mittelberg.
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72370 )
Change subject: ec/google/chromeec: Add handles for ori, usb role switch ......................................................................
Patch Set 4:
(1 comment)
File src/ec/google/chromeec/ec_acpi.c:
https://review.coreboot.org/c/coreboot/+/72370/comment/0eaa210b_7343c867 PS3, Line 195: .orientation_switch = config->ori_conn[i], : .usb_role_switch = config->usb_role_conn[i],
could you add the skyrim case as the next patch so we have some insight
into what's coming up? i didn't see anything interesting USB related in the existing skyrim device tree.
Sure. I can add a CL to the stack. It's still under development, so we don't want to enable it yet.
sounds like part of the problem is that you can't disable
orientation_switch or usb_role_switch by just explicitly setting them to NULL in the device tree since that's indistinguishable from not being present. would it make sense to introduce explicit no_orientation_switch , no_usb_role_switch parameters to the device tree? fill_ssdt_typec_device() will have to check for those.
I guess? I dunno, that seems like too many tags to be worth the trouble: - For Intel case: just define mux_conn. - For AMD case: define mux_conn but define no_usb_role_switch and no_orientation_switch - For potential use case where there is a mode-switch, and role/orientation-switch, but the two are distinct: define mux_conn, *and* define usb_role/orientation_switch
That seems like overcomplicating things (as opposed to just always being explicit about the aliases). Then again, IANA-Coreboot expert, so if the other folks reviewing are OK with the "no_*" option, I don't mind adopting that approach.
-
Prashant Malani (Code Review)