Attention is currently required from: Subrata Banik, Robert Chen, Nick Vaccaro, Shon Wang. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60331 )
Change subject: mb/google/brya/var/vell: Add AMP driver setting ......................................................................
Patch Set 7:
(7 comments)
Patchset:
PS7: please split this into two changes:
1) for the driver 2) for the vell specific changes
File src/drivers/i2c/cs35l53/Kconfig:
https://review.coreboot.org/c/coreboot/+/60331/comment/8371dfab_5c28eb99 PS7, Line 4: This would be a good place to point to the kernel's firmware bindings documentation
File src/drivers/i2c/cs35l53/chip.h:
https://review.coreboot.org/c/coreboot/+/60331/comment/ccfab252_4aa4a3ee PS7, Line 39: nit: extra blank line
File src/drivers/i2c/cs35l53/cs35l53.c:
https://review.coreboot.org/c/coreboot/+/60331/comment/fb608a93_0bba82e5 PS7, Line 24: .speed = config->bus_speed ? : I2C_SPEED_FAST, shouldn't this be `.speed = config->bus_speed ? config->bus_speed : I2C_SPEED_FAST,`
https://review.coreboot.org/c/coreboot/+/60331/comment/e1d58ac4_76df4c5c PS7, Line 57: AAD what is AAD?
https://review.coreboot.org/c/coreboot/+/60331/comment/cf8acd0f_77f6a47e PS7, Line 100: static const char *cs35l53_acpi_name(const struct device *dev) : { : // return CS35L53_ACPI_NAME; : struct drivers_i2c_cs35l53_config *config = dev->chip_info; : static char name[5]; : : : : if (config->name){ : return config->name; : } : : : snprintf(name, sizeof(name), "D%03.3X", dev->path.i2c.device); : return name; : } please reformat this one
File src/mainboard/google/brya/variants/vell/overridetree.cb:
PS7: will there be a corresponding FW_CONFIG option for this in AUDIO? Not supporting the MAX98xxx codecs anymore?
-
Tim Wawrzynczak (Code Review)