Re: [PATCH 1/9] EDAC/versalnet: Add NULL check for mci in handle_error()
"Pandey, Radhey Shyam" <[email protected]> Tue, 28 Jul 2026 23:57:28 +0530
| Newsgroups | org.kernel.vger.linux-edac,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 7/28/2026 7:07 AM, Borislav Petkov wrote: > On Mon, Jul 27, 2026 at 12:18:48PM +0530, Pandey, Radhey Shyam wrote: >> The gap in the driver is that init_one_mc() returns success without >> setting priv->mci[i] when the bus width decodes to DEV_UNKNOWN, while >> buggy firmware / future firmware rpmsg error path can still calls >> handle_error(). > > Aha, right, so I missed that. So the right fix is to always return an error if > the function fails. So that the proper unwinding in init_versalnet() works. Thanks Boris. One clarification on DEV_UNKNOWN: on Versal NET, the driver exposes 8 MC5 controller slots, but a given platform may not have all of them configured. The existing path treats unrecognized bus width as a skip (continue), and the refactor kept that as return 0 without setting priv->mci[i]. For that case, returning -EINVAL would fail probe on designs with fewer than 8 active controllers. Actual init failures (alloc, device_add, edac_mc_add_mc) already return an error and init_versalnet() unwinds as intended. The NULL guards in handle_error() cover the skip path where priv->mci[i] is intentionally unset. It can also be a check in rpmsg_cb() before calling handle_error(). Thanks, Radhey > >> Happy to drop the check if you prefer otherwise we can keep it as a >> cheap guard for a firmware/driver mismatch. > > You already have that at the beginning of handle_error(). > > Thx. >