All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
Cc: linux-edac@vger.kernel.org
Subject: [bug report] EDAC: Add a driver for the AMD Versal NET DDR controller
Date: Thu, 18 Sep 2025 09:55:52 +0300	[thread overview]
Message-ID: <aMus-KpPMcCk2iJr@stanley.mountain> (raw)

Hello Shubhrajyoti Datta,

Commit d5fe2fec6c40 ("EDAC: Add a driver for the AMD Versal NET DDR
controller") from Sep 8, 2025 (linux-next), leads to the following
Smatch static checker warning:

	drivers/edac/versalnet_edac.c:849 init_versalnet()
	warn: '_res' from device_register() not released on lines: 849.

drivers/edac/versalnet_edac.c
    761 static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev)
    762 {
    763         u32 num_chans, rank, dwidth, config;
    764         struct edac_mc_layer layers[2];
    765         struct mem_ctl_info *mci;
    766         struct device *dev;
    767         enum dev_type dt;
    768         char *name;
    769         int rc, i;
    770 
    771         for (i = 0; i < NUM_CONTROLLERS; i++) {
    772                 config = priv->adec[CONF + i * ADEC_NUM];
    773                 num_chans = FIELD_GET(MC5_NUM_CHANS_MASK, config);
    774                 rank = 1 << FIELD_GET(MC5_RANK_MASK, config);
    775                 dwidth = FIELD_GET(MC5_BUS_WIDTH_MASK, config);
    776 
    777                 switch (dwidth) {
    778                 case XDDR5_BUS_WIDTH_16:
    779                         dt = DEV_X16;
    780                         break;
    781                 case XDDR5_BUS_WIDTH_32:
    782                         dt = DEV_X32;
    783                         break;
    784                 case XDDR5_BUS_WIDTH_64:
    785                         dt = DEV_X64;
    786                         break;
    787                 default:
    788                         dt = DEV_UNKNOWN;
    789                 }
    790 
    791                 if (dt == DEV_UNKNOWN)
    792                         continue;
    793 
    794                 /* Find the first enabled device and register that one. */
    795                 layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
    796                 layers[0].size = rank;
    797                 layers[0].is_virt_csrow = true;
    798                 layers[1].type = EDAC_MC_LAYER_CHANNEL;
    799                 layers[1].size = num_chans;
    800                 layers[1].is_virt_csrow = false;
    801 
    802                 rc = -ENOMEM;
    803                 mci = edac_mc_alloc(i, ARRAY_SIZE(layers), layers,
    804                                     sizeof(struct mc_priv));
    805                 if (!mci) {
    806                         edac_printk(KERN_ERR, EDAC_MC, "Failed memory allocation for MC%d\n", i);
    807                         goto err_alloc;
    808                 }
    809 
    810                 priv->mci[i] = mci;
    811                 priv->dwidth = dt;
    812 
    813                 dev = kzalloc(sizeof(*dev), GFP_KERNEL);
    814                 dev->release = versal_edac_release;
    815                 name = kmalloc(32, GFP_KERNEL);
    816                 sprintf(name, "versal-net-ddrmc5-edac-%d", i);
    817                 dev->init_name = name;
    818                 rc = device_register(dev);
    819                 if (rc)
    820                         goto err_alloc;

This should do a edac_mc_free(mci) before the goto.

    821 
    822                 mci->pdev = dev;
    823 
    824                 platform_set_drvdata(pdev, priv);
    825 
    826                 mc_init(mci, dev);
    827                 rc = edac_mc_add_mc(mci);
    828                 if (rc) {
    829                         edac_printk(KERN_ERR, EDAC_MC, "Failed to register MC%d with EDAC core\n", i);
    830                         goto err_alloc;

I guess this should unregister and edac_mc_free mci before the goto.

    831                 }
    832         }
    833         return 0;
    834 
    835 err_alloc:
    836         while (i--) {
    837                 mci = priv->mci[i];
    838                 if (!mci)

No need for this check.  We know it's non-NULL.

    839                         continue;
    840 
    841                 if (mci->pdev) {

Unnecessary NULL check.

    842                         device_unregister(mci->pdev);
    843                         edac_mc_del_mc(mci->pdev);

I would have thought these would be in the other order but I don't know
if it matters.  I have a paragraph on unwinding from loops at the end of
my blog:

https://staticthinking.wordpress.com/2022/04/28/free-the-last-thing-style/

    844                 }
    845 
    846                 edac_mc_free(mci);
    847         }
    848 
--> 849         return rc;
    850 }

regards,
dan carpenter

             reply	other threads:[~2025-09-18  6:55 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-18  6:55 Dan Carpenter [this message]
2025-09-18 10:09 ` [bug report] EDAC: Add a driver for the AMD Versal NET DDR controller Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aMus-KpPMcCk2iJr@stanley.mountain \
    --to=dan.carpenter@linaro.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=shubhrajyoti.datta@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.