On Wed, Apr 09, 2025 at 12:48:57AM -0500, Naik, Avadhut wrote:
[...]
edac_dbg(1, "CS%d DIMM%d AddrMasks:\n", csrow_nr, dimm);
- edac_dbg(1, " Original AddrMask: 0x%x\n", addr_mask_orig);
- edac_dbg(1, " Deinterleaved AddrMask: 0x%x\n", addr_mask_deinterleaved);
- edac_dbg(1, " Primary AddrMask: 0x%x\n", addr_mask);
/* Register [31:1] = Address [39:9]. Size is in kBs here. */
- size = (addr_mask_deinterleaved >> 2) + 1;
- size = calculate_cs_size(addr_mask, cs_mode);
- if (addr_mask_sec) {
I think we can skip this check.
Commented below on this.
For debug messages, it doesn't hurt to be more explicit. So printing a 'mask: 0x0' message is more helpful/reassuring than 'no message'.
edac_dbg(1, " Secondary AddrMask: 0x%x\n", addr_mask);addr_mask -> addr_mask_sec
size += calculate_cs_size(addr_mask_sec, cs_mode);Maybe add a "!mask" check to return early if you want to save some cycles in this helper function.
In a way, this is the reason why I had added the above condition check. To avoid unnecessary function calls.
AFAIK, power-of-2 DIMMs are predominantly used, so the Secondary Address Mask register will seldom be used.
Would you agree?
I don't know. It seems non-power-of-2 (24G, 48G, 96G) is becoming more common for individual DIMMs and sets of DIMMs.
Thinking on it more, I don't think we need to be concerned about saving cycles here. These functions are only called during module init, correct?
Thanks, Yazen