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.
>