Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42134 )
Change subject: src/include: Add PnP/HWM unset_and_set functions ......................................................................
Patch Set 8:
(2 comments)
Please start a discussion about the naming convention on the mailing list. We already have `clrsetbits` and `pci_update_config` (with different semantics, I know). It doesn't feel right to add a third nomenclature during a review that is less visible to most of the community.
https://review.coreboot.org/c/coreboot/+/42134/8/src/include/device/pnp.h File src/include/device/pnp.h:
https://review.coreboot.org/c/coreboot/+/42134/8/src/include/device/pnp.h@98 PS8, Line 98: /* PNP indexed I/O operations */ Not your fault, this code is in the wrong place. I see nothing PnP specific below.
https://review.coreboot.org/c/coreboot/+/42134/8/src/include/device/pnp.h@14... PS8, Line 147: * Performing said negation inside this routine alleviates this problem. It would also work if the negation and cast would be done outside the function. The actual benefit, IMHO, is the implicit conversion to `u8` of the parameter. This way, we can still have overflow warnings (e.g. try to call this with 257) where it matters but can silence them where not (after the negation).