On 4/14/2025 08:11, Yazen Ghannam wrote:
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?
Yes, this should only be executed during module init. Get your point. Will remove the condition check and move it to the new helper function.
Thanks, Yazen