Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32006 )
Change subject: fleex: Add VPD entry for deciding which touchscreen ACPI nodes to generate. ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/32006/3/src/mainboard/google/octopus/variant... File src/mainboard/google/octopus/variants/fleex/overridetree.cb:
https://review.coreboot.org/#/c/32006/3/src/mainboard/google/octopus/variant... PS3, Line 125: register "vpd_string_tag" = ""touchscreen""
What's a better location for this?
Well, I don't think we need to add these fields to the struct. We can do this in the code only without being prescriptive in the chip section.
There's a couple of things we need at a high level:
Indicate path of bus/device (pci 17.3) in this case for looking at all children and check for vpd entries that match hid and bus/device path. If a vpd entry is there look through all the children and look for a hid match. If one exists all other devices are disabled but that one. If no matches found keep everything enabled as it was in devicetree.
This gets complicated by a couple of things: 1. hierarchy of objects needs to be checked by way of matching chip_ops pointer value against known symbols (drivers_i2c_generic_ops, drivers_i2c_hid_ops). 2. DEVTREE_EARLY currently guards chip_ops field in struct device. This means this work can only be done in ramstage.
I was discussing some of this with Furquan yesterday. #2 above is a concern if one was going to sequence things from earlier stages -- there's no chip ops to disambiguate chip_info pointer.
We should brainstorm some ideas to cover these. Or just do the address probing at run time for the i2c device, but we'll still likely need a lot of the same constructs for power sequencing, etc. The only difference is the vpd lookup vs the probe.