Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31238 )
Change subject: src: Remove unused include device/pnp_def.h ......................................................................
Patch Set 4: Code-Review+2
(6 comments)
looks good to me. while reviewing the change, i found some other issues that can be fixed in 2-3 follow-up patches
https://review.coreboot.org/#/c/31238/4/src/mainboard/asrock/g41c-gs/romstag... File src/mainboard/asrock/g41c-gs/romstage.c:
https://review.coreboot.org/#/c/31238/4/src/mainboard/asrock/g41c-gs/romstag... PS4, Line 47: pnp_ IIRC those functions are from <arch/io.h>, so that file should be included directly in this file
https://review.coreboot.org/#/c/31238/4/src/mainboard/asus/p5qpl-am/romstage... File src/mainboard/asus/p5qpl-am/romstage.c:
https://review.coreboot.org/#/c/31238/4/src/mainboard/asus/p5qpl-am/romstage... PS4, Line 74: pnp_ <arch/io.h> include needed
https://review.coreboot.org/#/c/31238/4/src/mainboard/intel/dg41wv/romstage.... File src/mainboard/intel/dg41wv/romstage.c:
https://review.coreboot.org/#/c/31238/4/src/mainboard/intel/dg41wv/romstage.... PS4, Line 42: pnp_ <arch/io.h> include needed
https://review.coreboot.org/#/c/31238/4/src/mainboard/kontron/ktqm77/romstag... File src/mainboard/kontron/ktqm77/romstage.c:
https://review.coreboot.org/#/c/31238/4/src/mainboard/kontron/ktqm77/romstag... PS4, Line 64: pnp_ <arch/io.h> include needed
https://review.coreboot.org/#/c/31238/4/src/mainboard/roda/rk886ex/romstage.... File src/mainboard/roda/rk886ex/romstage.c:
https://review.coreboot.org/#/c/31238/4/src/mainboard/roda/rk886ex/romstage.... PS4, Line 74: pnp_write_register this is a reimplementation of pnp_write_config from <arch/io.h>
https://review.coreboot.org/#/c/31238/4/src/superio/ite/it8772f/early_init.c File src/superio/ite/it8772f/early_init.c:
https://review.coreboot.org/#/c/31238/4/src/superio/ite/it8772f/early_init.c... PS4, Line 23: /* RAMstage equiv */ : /* u8 pnp_read_config(pnp_devfn_t dev, u8 reg) */ : u8 it8772f_sio_read(pnp_devfn_t dev, u8 reg) : { : u16 port = dev >> 8; : : outb(reg, port); : return inb(port + 1); : } : : /* RAMstage equiv */ : /* void pnp_write_config(pnp_devfn_t dev, u8 reg, u8 value) */ : void it8772f_sio_write(pnp_devfn_t dev, u8 reg, u8 value) : { : u16 port = dev >> 8; : : outb(reg, port); : outb(value, port + 1); : } those two functions are already implemented in <arch/io.h>; would be nice if those would be replaced with the generic functions