Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35456 )
Change subject: [RFC] sconfig: Allow to give static device nodes a name ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35456/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35456/1//COMMIT_MSG@16 PS1, Line 16: _static_serial_eeprom
What are your thoughts in promoting the name defined in devicetree as a global symbol w/o the hierarchy information? My concern is around having multiples of certain things and then we have something1 and something2 and those symbols baked into the code.
The problem with hierarchy information is how to communicate it. The cases where dev_find_slot_on_smbus() is used (with fragile bus numbers), show that there is a need to tell the driver of one device (or framework code, e.g. SMBIOS) about another device that can be anywhere.
I was first thinking about something like what is currently done in the nvidia/nic drivers (southbridge/nvidia/ck804/chip.h:26): Let the device- tree tell the driver about the path to the other device. However, any generic solution that I came up with wasn't very user friendly (for the dt author).
Hence my current approach: let the consuming code define the name and a devicetree using that code should follow (sorry for the brief commit message, I was too much in a hurry).
When should someone use the 1 variant vs the 2 variant?
I'm not sure where you are going...
Should we allow common code (soc/) changes to refer to names in mainboard devicetrees?
I would they yes, but only where necessary. Integrated devices are known to soc/ code anyway, so naming them in the dt explicitly would be a useless exercise.
Do you envision this approach proliferating?
Nope. It seems very rarely necessary to cross reference devices, dev_find_slot_on_smbus() has been there for a long time and has only 2 use cases.
https://review.coreboot.org/c/coreboot/+/35456/1/util/sconfig/main.c File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/35456/1/util/sconfig/main.c@1410 PS1, Line 1410: outputf[strlen(outputf) - 1] = 'h';
Looks like this patch is also making a new file. […]
Ack, will mention it (and most likely make it more obvious with another command-line parameter).