Re: [PATCH 6/9] EDAC/versalnet: Initialize MCDI before RPMsg registration

On 7/24/2026 10:49 PM, Shubhrajyoti Datta wrote:
> setup_mcdi() currently allocates the MCDI instance, assigns the RPMsg
> endpoint, and retrieves DDR configuration data. The DDR configuration
> path invokes cdx_mcdi_rpc(), which requires a functional RPMsg endpoint.
> 
> Split setup_mcdi() so that only MCDI allocation and initialization are
> performed before RPMsg registration. Move endpoint assignment and DDR
> configuration retrieval until after register_rpmsg_driver() succeeds.
> Update the error paths to match the new initialization order, ensuring
> resources are released in reverse order of acquisition.
> 
> Signed-off-by: Shubhrajyoti Datta <[email protected]>
> ---
> 
>   drivers/edac/versalnet_edac.c | 29 ++++++++++++++---------------
>   1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
> index 1caaba653fc0..e9561242f292 100644
> --- a/drivers/edac/versalnet_edac.c
> +++ b/drivers/edac/versalnet_edac.c
> @@ -559,7 +559,7 @@ static void get_ddr_config(u32 index, u32 *buffer, struct cdx_mcdi *amd_mcdi)
>   static int setup_mcdi(struct mc_priv *mc_priv)
>   {
>   	struct cdx_mcdi *amd_mcdi;
> -	int ret, i;
> +	int ret;
>   
>   	amd_mcdi = kzalloc_obj(*amd_mcdi);
>   	if (!amd_mcdi)
> @@ -572,12 +572,7 @@ static int setup_mcdi(struct mc_priv *mc_priv)
>   		return ret;
>   	}
>   
> -	amd_mcdi->ept = mc_priv->ept;
>   	mc_priv->mcdi = amd_mcdi;
> -
> -	for (i = 0; i < NUM_CONTROLLERS; i++)
> -		get_ddr_config(i, &mc_priv->adec[ADEC_NUM * i], amd_mcdi);
> -
>   	return 0;
>   }
>   
> @@ -886,7 +881,7 @@ static int mc_probe(struct platform_device *pdev)
>   {
>   	struct mc_priv *priv;
>   	struct rproc *rp;
> -	int rc;
> +	int rc, i;
>   
>   	struct device_node *r5_core_node __free(device_node) =
>   		of_parse_phandle(pdev->dev.of_node, "amd,rproc", 0);
> @@ -914,18 +909,22 @@ static int mc_probe(struct platform_device *pdev)
>   	platform_set_drvdata(pdev, priv);
>   	amd_rpmsg_id_table[0].driver_data = (kernel_ulong_t)priv;
>   
> +	rc = setup_mcdi(priv);
> +	if (rc)
> +		goto err_alloc;
> +
>   	rc = register_rpmsg_driver(&amd_rpmsg_driver);
>   	if (rc) {
>   		edac_printk(KERN_ERR, EDAC_MC, "Failed to register RPMsg driver: %d\n", rc);
> -		goto err_alloc;
> -	}
> -
> -	rc = setup_mcdi(priv);
> -	if (rc)
>   		goto err_unreg;
> +	}
>   
> +	priv->mcdi->ept = priv->ept;
>   	priv->mcdi->r5_rproc = rp;
>   
> +	for (i = 0; i < NUM_CONTROLLERS; i++)
> +		get_ddr_config(i, &priv->adec[ADEC_NUM * i], priv->mcdi);
> +
>   	rc = init_versalnet(priv);
>   	if (rc)
>   		goto err_init;
> @@ -933,11 +932,11 @@ static int mc_probe(struct platform_device *pdev)
>   	return 0;
>   
>   err_init:
> -	cdx_mcdi_finish(priv->mcdi);
> -	kfree(priv->mcdi);
> +	unregister_rpmsg_driver(&amd_rpmsg_driver);
>   
>   err_unreg:
> -	unregister_rpmsg_driver(&amd_rpmsg_driver);
> +	cdx_mcdi_finish(priv->mcdi);
> +	kfree(priv->mcdi);

Nit - considering renaming the labels. Rest changes
looks fine to me.

>   
>   err_alloc:
>   	rproc_shutdown(rp);