| Message ID | 20260916-edac-v3-2-23e9ed2fbdf9@aspeedtech.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add Aspeed AST2700 SDRAM EDAC support | expand |
On Wed, Sep 16, 2026 at 01:30:39PM +0800, Ryan Chen wrote: > The driver never sets dimm->grain, leaving it zero. Since commit > 3724ace582d9 ("EDAC/mc: Fix grain_bits calculation") > edac_raw_mc_handle_error() runs WARN_ON_ONCE(!e->grain) and forces the > grain to 1, so the first ECC error reported on any Aspeed BMC SoC emits a > warning splat, e.g. on the AST2600: > > WARNING: CPU: 0 PID: 0 at drivers/edac/edac_mc.c:924 edac_raw_mc_handle_error+0x4b4/0x604 > ... > edac_raw_mc_handle_error from edac_mc_handle_error+0x364/0x4a8 > edac_mc_handle_error from count_rec+0xdc/0x124 > count_rec from mcr_isr+0x110/0x1e8 All of that is fine and dandy but you should talk about why the grain is chosen to be 16 here. The warning is just to make you choose a proper grain for your hw and doesn't need that much 'splainin'. > Fixes: 9b7e6242ee4e ("EDAC, aspeed: Add an Aspeed AST2500 EDAC driver") > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com> > Cc: stable@vger.kernel.org Is this really stable material and if so, why?
> Subject: Re: [PATCH v3 2/9] EDAC/aspeed: Set the DIMM grain > > On Wed, Sep 16, 2026 at 01:30:39PM +0800, Ryan Chen wrote: > > The driver never sets dimm->grain, leaving it zero. Since commit > > 3724ace582d9 ("EDAC/mc: Fix grain_bits calculation") > > edac_raw_mc_handle_error() runs WARN_ON_ONCE(!e->grain) and forces > the > > grain to 1, so the first ECC error reported on any Aspeed BMC SoC > > emits a warning splat, e.g. on the AST2600: > > > > WARNING: CPU: 0 PID: 0 at drivers/edac/edac_mc.c:924 > edac_raw_mc_handle_error+0x4b4/0x604 > > ... > > edac_raw_mc_handle_error from edac_mc_handle_error+0x364/0x4a8 > > edac_mc_handle_error from count_rec+0xdc/0x124 > > count_rec from mcr_isr+0x110/0x1e8 > > All of that is fine and dandy but you should talk about why the grain is chosen > to be 16 here. The warning is just to make you choose a proper grain for your > hw and doesn't need that much 'splainin'. Got it. v4 trims the warning down to a sentence and explains the value instead: the controller records the address of a failing access with 16-byte granularity - MCR58 and MCR5C hold it in bits [30:4], with the low four bits reserved. > > > Fixes: 9b7e6242ee4e ("EDAC, aspeed: Add an Aspeed AST2500 EDAC > > driver") > > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com> > > Cc: stable@vger.kernel.org > > Is this really stable material and if so, why? No. The error is still reported, only with the wrong granularity and a one-off warning, so it doesn't meet the bar in stable-kernel-rules.rst. I'll drop the tag and keep the Fixes: one. Ryan
diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c index 6e069b255595..83d60414f89a 100644 --- a/drivers/edac/aspeed_edac.c +++ b/drivers/edac/aspeed_edac.c @@ -266,6 +266,7 @@ static int init_csrows(struct mem_ctl_info *mci) dimm->mtype = dram_type; dimm->edac_mode = EDAC_SECDED; dimm->nr_pages = nr_pages / csrow->nr_channels; + dimm->grain = 16; dev_dbg(mci->pdev, "initialized dimm with first_page=0x%lx and nr_pages=0x%x\n", csrow->first_page, nr_pages);
The driver never sets dimm->grain, leaving it zero. Since commit 3724ace582d9 ("EDAC/mc: Fix grain_bits calculation") edac_raw_mc_handle_error() runs WARN_ON_ONCE(!e->grain) and forces the grain to 1, so the first ECC error reported on any Aspeed BMC SoC emits a warning splat, e.g. on the AST2600: WARNING: CPU: 0 PID: 0 at drivers/edac/edac_mc.c:924 edac_raw_mc_handle_error+0x4b4/0x604 ... edac_raw_mc_handle_error from edac_mc_handle_error+0x364/0x4a8 edac_mc_handle_error from count_rec+0xdc/0x124 count_rec from mcr_isr+0x110/0x1e8 Fixes: 9b7e6242ee4e ("EDAC, aspeed: Add an Aspeed AST2500 EDAC driver") Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com> Cc: stable@vger.kernel.org --- drivers/edac/aspeed_edac.c | 1 + 1 file changed, 1 insertion(+)