| Message ID | 1406013622-27869-2-git-send-email-varkabhadram@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: varkabhadram@gmail.com Date: Tue, 22 Jul 2014 12:50:21 +0530 > @@ -1887,11 +1887,7 @@ static int cp_init_one (struct pci_dev *pdev, const struct pci_device_id *ent) > resource_size_t pciaddr; > unsigned int addr_len, i, pci_using_dac; > > -#ifndef MODULE > - static int version_printed; > - if (version_printed++ == 0) > - pr_info("%s", version); > -#endif > + pr_info("%s", version); Now you're changing behavior undesirably, it will now print the version string into the logs for every instance of the device which is discovered. Seriously, the driver is worse off after these "cleanups". -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/22/2014 12:58 PM, David Miller wrote: > From: varkabhadram@gmail.com > Date: Tue, 22 Jul 2014 12:50:21 +0530 > >> @@ -1887,11 +1887,7 @@ static int cp_init_one (struct pci_dev *pdev, const struct pci_device_id *ent) >> resource_size_t pciaddr; >> unsigned int addr_len, i, pci_using_dac; >> >> -#ifndef MODULE >> - static int version_printed; >> - if (version_printed++ == 0) >> - pr_info("%s", version); >> -#endif >> + pr_info("%s", version); > Now you're changing behavior undesirably, it will now print the > version string into the logs for every instance of the device which is > discovered. > > Seriously, the driver is worse off after these "cleanups". Here we are having two possibilities here: 1. Removing the version info completely. Ofcourse this is not desirable. 2. Accepting the version info to be print into log messages. I will put like this by removing #ifdefs: static int version_printed; if (version_printed++ == 0) pr_info("%s", version); Will it be OK ..?
On 07/22/2014 11:20 AM, varkabhadram@gmail.com wrote: > From: Varka Bhadram <varkab@cdac.in> > This patch converts to use the macro module_pci_driver, which makes > the code smaller and simpler. > Signed-off-by: Varka Bhadram <varkab@cdac.in> > --- > drivers/net/ethernet/realtek/8139cp.c | 22 ++-------------------- > 1 file changed, 2 insertions(+), 20 deletions(-) > diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c > index 2bc728e..9fb68b9 100644 > --- a/drivers/net/ethernet/realtek/8139cp.c > +++ b/drivers/net/ethernet/realtek/8139cp.c > @@ -1887,11 +1887,7 @@ static int cp_init_one (struct pci_dev *pdev, const struct pci_device_id *ent) > resource_size_t pciaddr; > unsigned int addr_len, i, pci_using_dac; > > -#ifndef MODULE > - static int version_printed; > - if (version_printed++ == 0) > - pr_info("%s", version); > -#endif > + pr_info("%s", version); Use pr_info_once(). WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 22 July 2014 08:08 PM, Sergei Shtylyov wrote: > On 07/22/2014 11:20 AM, varkabhadram@gmail.com wrote: > >> From: Varka Bhadram <varkab@cdac.in> > >> This patch converts to use the macro module_pci_driver, which makes >> the code smaller and simpler. > >> Signed-off-by: Varka Bhadram <varkab@cdac.in> >> --- >> drivers/net/ethernet/realtek/8139cp.c | 22 ++-------------------- >> 1 file changed, 2 insertions(+), 20 deletions(-) > >> diff --git a/drivers/net/ethernet/realtek/8139cp.c >> b/drivers/net/ethernet/realtek/8139cp.c >> index 2bc728e..9fb68b9 100644 >> --- a/drivers/net/ethernet/realtek/8139cp.c >> +++ b/drivers/net/ethernet/realtek/8139cp.c >> @@ -1887,11 +1887,7 @@ static int cp_init_one (struct pci_dev *pdev, >> const struct pci_device_id *ent) >> resource_size_t pciaddr; >> unsigned int addr_len, i, pci_using_dac; >> >> -#ifndef MODULE >> - static int version_printed; >> - if (version_printed++ == 0) >> - pr_info("%s", version); >> -#endif >> + pr_info("%s", version); > > Use pr_info_once(). > > WBR, Sergei > I am going to utilize the removed code by removing the #ifndef MODULE... It will be like: static int version_printed; if (version_printed++ == 0) pr_info("%s", version); Will it be Ok...?
On 07/22/2014 06:43 PM, Varka Bhadram wrote: >>> From: Varka Bhadram <varkab@cdac.in> >>> This patch converts to use the macro module_pci_driver, which makes >>> the code smaller and simpler. >>> Signed-off-by: Varka Bhadram <varkab@cdac.in> >>> --- >>> drivers/net/ethernet/realtek/8139cp.c | 22 ++-------------------- >>> 1 file changed, 2 insertions(+), 20 deletions(-) >>> diff --git a/drivers/net/ethernet/realtek/8139cp.c >>> b/drivers/net/ethernet/realtek/8139cp.c >>> index 2bc728e..9fb68b9 100644 >>> --- a/drivers/net/ethernet/realtek/8139cp.c >>> +++ b/drivers/net/ethernet/realtek/8139cp.c >>> @@ -1887,11 +1887,7 @@ static int cp_init_one (struct pci_dev *pdev, const >>> struct pci_device_id *ent) >>> resource_size_t pciaddr; >>> unsigned int addr_len, i, pci_using_dac; >>> >>> -#ifndef MODULE >>> - static int version_printed; >>> - if (version_printed++ == 0) >>> - pr_info("%s", version); >>> -#endif >>> + pr_info("%s", version); >> Use pr_info_once(). >> WBR, Sergei > I am going to utilize the removed code by removing the #ifndef MODULE... > It will be like: > > static int version_printed; > > if (version_printed++ == 0) > pr_info("%s", version); pr_info_once() is much shorter and the effect is the same. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 22 July 2014 08:31 PM, Sergei Shtylyov wrote: > On 07/22/2014 06:43 PM, Varka Bhadram wrote: > >>>> From: Varka Bhadram <varkab@cdac.in> > >>>> This patch converts to use the macro module_pci_driver, which makes >>>> the code smaller and simpler. > >>>> Signed-off-by: Varka Bhadram <varkab@cdac.in> >>>> --- >>>> drivers/net/ethernet/realtek/8139cp.c | 22 ++-------------------- >>>> 1 file changed, 2 insertions(+), 20 deletions(-) > >>>> diff --git a/drivers/net/ethernet/realtek/8139cp.c >>>> b/drivers/net/ethernet/realtek/8139cp.c >>>> index 2bc728e..9fb68b9 100644 >>>> --- a/drivers/net/ethernet/realtek/8139cp.c >>>> +++ b/drivers/net/ethernet/realtek/8139cp.c >>>> @@ -1887,11 +1887,7 @@ static int cp_init_one (struct pci_dev >>>> *pdev, const >>>> struct pci_device_id *ent) >>>> resource_size_t pciaddr; >>>> unsigned int addr_len, i, pci_using_dac; >>>> >>>> -#ifndef MODULE >>>> - static int version_printed; >>>> - if (version_printed++ == 0) >>>> - pr_info("%s", version); >>>> -#endif >>>> + pr_info("%s", version); > >>> Use pr_info_once(). > >>> WBR, Sergei > >> I am going to utilize the removed code by removing the #ifndef MODULE... > >> It will be like: >> >> static int version_printed; >> >> if (version_printed++ == 0) >> pr_info("%s", version); > > pr_info_once() is much shorter and the effect is the same. > > WBR, Sergei > Thanks for your suggestion. I will do this in next version.
diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c index 2bc728e..9fb68b9 100644 --- a/drivers/net/ethernet/realtek/8139cp.c +++ b/drivers/net/ethernet/realtek/8139cp.c @@ -1887,11 +1887,7 @@ static int cp_init_one (struct pci_dev *pdev, const struct pci_device_id *ent) resource_size_t pciaddr; unsigned int addr_len, i, pci_using_dac; -#ifndef MODULE - static int version_printed; - if (version_printed++ == 0) - pr_info("%s", version); -#endif + pr_info("%s", version); if (pdev->vendor == PCI_VENDOR_ID_REALTEK && pdev->device == PCI_DEVICE_ID_REALTEK_8139 && pdev->revision < 0x20) { @@ -2121,18 +2117,4 @@ static struct pci_driver cp_driver = { #endif }; -static int __init cp_init (void) -{ -#ifdef MODULE - pr_info("%s", version); -#endif - return pci_register_driver(&cp_driver); -} - -static void __exit cp_exit (void) -{ - pci_unregister_driver (&cp_driver); -} - -module_init(cp_init); -module_exit(cp_exit); +module_pci_driver(cp_driver);