From: navych@126.com (Navy Cheng)
To: kernelnewbies@lists.kernelnewbies.org
Subject: [PATCH] staging: dgnc: Remove useless and deadly judgment
Date: Mon, 29 Feb 2016 20:27:41 +0800 [thread overview]
Message-ID: <20160229122740.GA11064@debian> (raw)
In-Reply-To: <15064.1456732696@turing-police.cc.vt.edu>
On Mon, Feb 29, 2016 at 02:58:16AM -0500, Valdis.Kletnieks at vt.edu wrote:
> On Mon, 29 Feb 2016 14:41:34 +0800, Navy Cheng said:
> > pci_unregister_driver() should be used once dgnc module exit. It has
> > nothing to do with dgnc_NumBoards. Remove the judgment of dgnc_NumBoards to
> > avoid pci_unregister_driver() is not used when dgnc_NumBoards is 0.
>
> > - if (dgnc_NumBoards)
> > - pci_unregister_driver(&dgnc_driver);
> > + pci_unregister_driver(&dgnc_driver);
>
> Does pci_unregister_driver do the right thing if there are 0 boards
> in the system?
I have no dgnc device in my laptop. After *insmod* and *rmmod* dgnc module,
I can re-insmod dgnc successfully. If dgnc is *rmmod*, the dgnc-file in /sys
will disappear.
> If this logic is wrong, shouldn't there also be a patch fixing the
> following in dgnc_init_module()?
Yes, I thought dgnc_init_module should be fixed. But I'm not familiar with
how to submitting patchset. I decide to fix dgnc_cleanup_module() first.
> /*
> * Find and configure all the cards
> */
> rc = pci_register_driver(&dgnc_driver);
>
> /*
> * If something went wrong in the scan, bail out of driver.
> */
> if (rc < 0) {
> /* Only unregister if it was actually registered. */
> if (dgnc_NumBoards)
> pci_unregister_driver(&dgnc_driver);
> else
> pr_warn("WARNING: dgnc driver load failed. No Digi Neo or Classic boards found.\n");
>
> dgnc_cleanup_module();
>
> While I'm at it, the entire NumBoards counting seems to be wonky - it looks
> like a *lot* of off-by-one errors. Looks like the programmer(s) weren't sure
> if they wanted to use that as a zero-based or one-based counter/index.
>
Maybe we can fix the *init* and *exit* like this:
static int __init dgnc_init_module(void)
{
int rc;
rc = dgnc_start();
if (rc < 0)
goto err1;
rc = pci_register_driver(&dgnc_driver)
if (rc <0) {
goto err1;
pr_warn("....");
}
rc = dgnc_create_driver_sysfiles(&dgnc_driver);
if (rc = 0)
goto err2;
return rc;
err2:
pci_unregister_driver(&dgnc_driver);
err1:
dgnc_end();
return rc;
}
static void dgnc_cleanup_module(void)
{
int i;
dgnc_remove_driver_sysfiles(&dgnc_driver);
for (i = 0; i < dgnc_NumBoards; ++i) {
dgnc_remove_ports_sysfiles(dgnc_Board[i]);
dgnc_tty_uninit(dgnc_Board[i]);
dgnc_cleanup_board(dgnc_Board[i]);
}
pci_unregister_driver(&dgnc_driver);
dgnc_end();
}
next prev parent reply other threads:[~2016-02-29 12:27 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-29 6:41 [PATCH] staging: dgnc: Remove useless and deadly judgment Navy Cheng
2016-02-29 7:58 ` Valdis.Kletnieks at vt.edu
2016-02-29 12:27 ` Navy Cheng [this message]
2016-03-11 21:29 ` Greg Kroah-Hartman
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=20160229122740.GA11064@debian \
--to=navych@126.com \
--cc=kernelnewbies@lists.kernelnewbies.org \
/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.