Re: [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc()

[Prasanna Kumar T S M <ptsm@linux.microsoft.com>]

Hi Boris,

On 24-03-2026 17:46, Borislav Petkov wrote:
> On Tue, Mar 24, 2026 at 12:23:12PM +0100, Borislav Petkov wrote:
>> So let's do it another way (totally untested ofc):
> 
> AI found a couple of issues, here's v2:
> 
> ---
> diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
> index b87fe57aa842..953f96c8fd6f 100644
> --- a/drivers/edac/versalnet_edac.c
> +++ b/drivers/edac/versalnet_edac.c
> @@ -772,12 +772,11 @@ static void remove_one_mc(struct mc_priv *priv, int i)
>   	edac_mc_free(mci);
>   }
>   
> -static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i)
> +static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, struct device *dev, int i)
>   {
>   	u32 num_chans, rank, dwidth, config;
>   	struct edac_mc_layer layers[2];
>   	struct mem_ctl_info *mci;
> -	struct device *dev;
>   	enum dev_type dt;
>   	char *name;
>   	int rc;
> @@ -802,7 +801,7 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
>   	}
>   
>   	if (dt == DEV_UNKNOWN)
> -		return 0;
> +		return -EINVAL;
>   
>   	/* Find the first enabled device and register that one. */
>   	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
> @@ -817,14 +816,10 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
>   	if (!name)
>   		return rc;
>   
> -	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> -	if (!dev)
> -		goto err_name_free;
> -
>   	mci = edac_mc_alloc(i, ARRAY_SIZE(layers), layers, sizeof(struct mc_priv));
>   	if (!mci) {
>   		edac_printk(KERN_ERR, EDAC_MC, "Failed memory allocation for MC%d\n", i);
> -		goto err_dev_free;
> +		goto err_name_free;
>   	}
>   
>   	sprintf(name, "versal-net-ddrmc5-edac-%d", i);
> @@ -856,8 +851,6 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
>   	device_unregister(mci->pdev);
>   err_mc_free:
>   	edac_mc_free(mci);
> -err_dev_free:
> -	kfree(dev);
>   err_name_free:
>   	kfree(name);
>   
> @@ -866,18 +859,26 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
>   
>   static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev)
>   {
> -	int rc, i;
> +	int rc = -ENOMEM, i;
>   
>   	for (i = 0; i < NUM_CONTROLLERS; i++) {
> -		rc = init_one_mc(priv, pdev, i);
> -		if (rc) {
> -			while (i--)
> -				remove_one_mc(priv, i);
> +		struct device *dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> +		if (!dev)
> +			goto free;
>   
> -			return rc;
> +		rc = init_one_mc(priv, pdev, dev, i);
> +		if (rc) {
> +			kfree(dev);
> +			goto free;
>   		}
>   	}
>   	return 0;
> +
> +free:
> +	while (i--)
> +		remove_one_mc(priv, i);
> +
> +	return rc;
>   }
>   
>   static void remove_versalnet(struct mc_priv *priv)
> 
> 

Thanks for sharing this.

Without rearranging the code I have simplified the error path and sent v2.

Thanks,
Prasanna Kumar