From: Shubhrajyoti Datta Date: Tue, 4 Nov 2025 09:39:20 +0000 (+0530) Subject: EDAC/versalnet: Refactor memory controller initialization and cleanup X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=62a9fc50e8d947601ea3484e732b1a65a0a54b96;p=thirdparty%2Fkernel%2Flinux.git EDAC/versalnet: Refactor memory controller initialization and cleanup Simplify the initialization and cleanup flow for Versal Net DDRMC controllers in the EDAC driver by carving out the single controller init into a separate function which allows for a much better and more readable error handling and unwinding. [ bp: - do the kzalloc allocations first - "publish" the structures only after they've been initialized properly so that you don't need to unwind unnecessarily when it fails later - remove_versalnet() is now trivial ] Signed-off-by: Shubhrajyoti Datta Signed-off-by: Borislav Petkov (AMD) Link: https://patch.msgid.link/20251104093932.3838876-1-shubhrajyoti.datta@amd.com --- diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c index 2cbc13d9bd00b..0b47ed7fed638 100644 --- a/drivers/edac/versalnet_edac.c +++ b/drivers/edac/versalnet_edac.c @@ -70,6 +70,8 @@ #define XDDR5_BUS_WIDTH_32 1 #define XDDR5_BUS_WIDTH_16 2 +#define MC_NAME_LEN 32 + /** * struct ecc_error_info - ECC error log information. * @burstpos: Burst position. @@ -760,7 +762,17 @@ static void versal_edac_release(struct device *dev) kfree(dev); } -static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev) +static void remove_one_mc(struct mc_priv *priv, int i) +{ + struct mem_ctl_info *mci; + + mci = priv->mci[i]; + device_unregister(mci->pdev); + edac_mc_del_mc(mci->pdev); + edac_mc_free(mci); +} + +static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i) { u32 num_chans, rank, dwidth, config; struct edac_mc_layer layers[2]; @@ -768,102 +780,110 @@ static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev) struct device *dev; enum dev_type dt; char *name; - int rc, i; - - for (i = 0; i < NUM_CONTROLLERS; i++) { - config = priv->adec[CONF + i * ADEC_NUM]; - num_chans = FIELD_GET(MC5_NUM_CHANS_MASK, config); - rank = 1 << FIELD_GET(MC5_RANK_MASK, config); - dwidth = FIELD_GET(MC5_BUS_WIDTH_MASK, config); - - switch (dwidth) { - case XDDR5_BUS_WIDTH_16: - dt = DEV_X16; - break; - case XDDR5_BUS_WIDTH_32: - dt = DEV_X32; - break; - case XDDR5_BUS_WIDTH_64: - dt = DEV_X64; - break; - default: - dt = DEV_UNKNOWN; - } + int rc; - if (dt == DEV_UNKNOWN) - continue; + config = priv->adec[CONF + i * ADEC_NUM]; + num_chans = FIELD_GET(MC5_NUM_CHANS_MASK, config); + rank = 1 << FIELD_GET(MC5_RANK_MASK, config); + dwidth = FIELD_GET(MC5_BUS_WIDTH_MASK, config); + + switch (dwidth) { + case XDDR5_BUS_WIDTH_16: + dt = DEV_X16; + break; + case XDDR5_BUS_WIDTH_32: + dt = DEV_X32; + break; + case XDDR5_BUS_WIDTH_64: + dt = DEV_X64; + break; + default: + dt = DEV_UNKNOWN; + } - /* Find the first enabled device and register that one. */ - layers[0].type = EDAC_MC_LAYER_CHIP_SELECT; - layers[0].size = rank; - layers[0].is_virt_csrow = true; - layers[1].type = EDAC_MC_LAYER_CHANNEL; - layers[1].size = num_chans; - layers[1].is_virt_csrow = false; + if (dt == DEV_UNKNOWN) + return 0; - rc = -ENOMEM; - 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_alloc; - } + /* Find the first enabled device and register that one. */ + layers[0].type = EDAC_MC_LAYER_CHIP_SELECT; + layers[0].size = rank; + layers[0].is_virt_csrow = true; + layers[1].type = EDAC_MC_LAYER_CHANNEL; + layers[1].size = num_chans; + layers[1].is_virt_csrow = false; + + rc = -ENOMEM; + name = kzalloc(MC_NAME_LEN, GFP_KERNEL); + 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; + } - priv->mci[i] = mci; - priv->dwidth = dt; + sprintf(name, "versal-net-ddrmc5-edac-%d", i); - dev = kzalloc_obj(*dev); - dev->release = versal_edac_release; - name = kmalloc(32, GFP_KERNEL); - sprintf(name, "versal-net-ddrmc5-edac-%d", i); - dev->init_name = name; - rc = device_register(dev); - if (rc) - goto err_alloc; + dev->init_name = name; + dev->release = versal_edac_release; - mci->pdev = dev; + rc = device_register(dev); + if (rc) + goto err_mc_free; - platform_set_drvdata(pdev, priv); + mci->pdev = dev; + mc_init(mci, dev); - mc_init(mci, dev); - rc = edac_mc_add_mc(mci); - if (rc) { - edac_printk(KERN_ERR, EDAC_MC, "Failed to register MC%d with EDAC core\n", i); - goto err_alloc; - } + rc = edac_mc_add_mc(mci); + if (rc) { + edac_printk(KERN_ERR, EDAC_MC, "Failed to register MC%d with EDAC core\n", i); + goto err_unreg; } - return 0; -err_alloc: - while (i--) { - mci = priv->mci[i]; - if (!mci) - continue; - - if (mci->pdev) { - device_unregister(mci->pdev); - edac_mc_del_mc(mci->pdev); - } + priv->mci[i] = mci; + priv->dwidth = dt; - edac_mc_free(mci); - } + platform_set_drvdata(pdev, priv); + + return 0; + +err_unreg: + device_unregister(mci->pdev); +err_mc_free: + edac_mc_free(mci); +err_dev_free: + kfree(dev); +err_name_free: + kfree(name); return rc; } -static void remove_versalnet(struct mc_priv *priv) +static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev) { - struct mem_ctl_info *mci; - int i; + int rc, i; for (i = 0; i < NUM_CONTROLLERS; i++) { - device_unregister(priv->mci[i]->pdev); - mci = edac_mc_del_mc(priv->mci[i]->pdev); - if (!mci) - return; + rc = init_one_mc(priv, pdev, i); + if (rc) { + while (i--) + remove_one_mc(priv, i); - edac_mc_free(mci); + return rc; + } } + return 0; +} + +static void remove_versalnet(struct mc_priv *priv) +{ + for (int i = 0; i < NUM_CONTROLLERS; i++) + remove_one_mc(priv, i); } static int mc_probe(struct platform_device *pdev)