Attention is currently required from: Ana Carolina Cabral, Anand Vaikar, Paul Menzel.
Felix Held has posted comments on this change by Ana Carolina Cabral. ( https://review.coreboot.org/c/coreboot/+/84776?usp=email )
Change subject: drivers/amd/nova: Add Nova Card common driver ......................................................................
Patch Set 7:
(5 comments)
File Makefile.mk:
https://review.coreboot.org/c/coreboot/+/84776/comment/4d158f70_bace3a9c?usp... : PS7, Line 121: subdirs-y += src/mainboard/$(MAINBOARDDIR) $(wildcard src/mainboard/*/common) probably no longer needed after the code was moved to the driver directory
File src/drivers/amd/nova/Kconfig:
https://review.coreboot.org/c/coreboot/+/84776/comment/1933bca6_1b4f4e50?usp... : PS5, Line 4: def_bool n
It can be, I wasn`t sure if all mainboards have a nova card
the AMD mainboard with a slot for a nova card should select this kconfig directly and not via the defconfig
File src/drivers/amd/nova/chip.h:
https://review.coreboot.org/c/coreboot/+/84776/comment/e45255a7_ae50b552?usp... : PS7, Line 9: #define NOVA_CARD_EEPROM_CONNECTOR_TYPE_OFFSET 2 i'd move this define into the c file that uses this, since it's not needed outside of that c file
https://review.coreboot.org/c/coreboot/+/84776/comment/d02f6e69_42e08259?usp... : PS7, Line 10: #define NOVA_CARD_EEPROM_I2C_BUS 2 : #define NOVA_CARD_EEPROM_I2C_ADDRESS 0x55 those two are mainboard-dependend. sure most boards will use a similar setup, but it's not something i'd rely on
File src/drivers/amd/nova/nova_card.c:
https://review.coreboot.org/c/coreboot/+/84776/comment/aebe7ea7_02ac17ad?usp... : PS7, Line 9: #include <soc/amd/phoenix/chip_opensil.h> i'd like to avoid soc-specific code in a common driver