Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42064 )
Change subject: soc/amd/picasso: Allow modification of i2c base addresses in PSP ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42064/14/src/soc/amd/picasso/i2c.c File src/soc/amd/picasso/i2c.c:
https://review.coreboot.org/c/coreboot/+/42064/14/src/soc/amd/picasso/i2c.c@... PS14, Line 28: APU_I2C2_BASE
For ARM, this will have x86 addresses by default. […]
While that's obviously doable, I don't think it's an actual worry, and makes the code more complex. If we need to remove the x86 addresses, I'd prefer to to that in a follow-on patch.
https://review.coreboot.org/c/coreboot/+/42064/14/src/soc/amd/picasso/i2c.c@... PS14, Line 71: APU_I2C2_BASE
Think that also includes these.
This function isn't used in the arm code, so will be remove by the linker. Because of that, I don't think this is a concern at all. Obviously this can be rewritten to something like this, but I don't see the need.
if (dev->path.mmio.addr == dw_i2c_base_address[2]) return "I2C2"; else if (dev->path.mmio.addr == dw_i2c_base_address[3]) return "I2C3"; else if (dev->path.mmio.addr == dw_i2c_base_address[4]) return "I2C4"; else return NULL;