Attention is currently required from: Julius Werner. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56543 )
Change subject: helpers: Add GENMASK macro ......................................................................
Patch Set 6:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56543/comment/7014c074_2eb95b8c PS5, Line 12: helpers.h, for unsigned long and unsigned long long types, respectively.
Do we really need two helpers? What happens if you just make it ULL to begin with, does it throw war […]
I was worried that this would accidentally change the existing behavior that we might not be aware of. In most cases the macro is used for read32/write32, so I guess we'll be fine.
File src/commonlib/bsd/include/commonlib/bsd/helpers.h:
https://review.coreboot.org/c/coreboot/+/56543/comment/04c91151_08cee17c PS5, Line 65: #define GENMASK_ULL(h, l) (((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
This file is BSD-licensed, so please don't copy Linux code wholesale (including the comment) in here […]
Done. Changed back to my own comment.
File src/soc/mediatek/common/pll.c:
https://review.coreboot.org/c/coreboot/+/56543/comment/7a59900c_a2598d9a PS5, Line 3: #include <commonlib/bsd/helpers.h>
Prefer to just #include <types.h> instead of including this directly.
Done.
Could you explain more on that? Is it because types.h is the place where we group all common headers together?
/* types.h is supposed to provide the standard headers defined in here: */