[KJ] [PATCH]
Subsystems (2)
PATCH 1/1 [KJ] [PATCH]
This is my first try to submit a patch here, so go gentle on me.. ;) At least [email protected] thought it seemed ok..
This little patch moves from pci_find_device() in alim1535_wdt.c and alim7101_wdt.c to pci_get_device()/pci_dev_put() I also found that alim1535.c uses 0x1535/0x7101 instead of the PCI_DEVICE_ID_AL_* defines.
I compared my usage of pci_dev_put() with other drivers and I hope that I've understood it right.
drivers/char/watchdog/alim1535_wdt.c +15 -6
@@ -312,7 +312,7 @@*/ static struct pci_device_id ali_pci_tbl[] = { - {PCI_VENDOR_ID_AL ,0x1535 ,PCI_ANY_ID ,PCI_ANY_ID ,}, + {PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M1535 ,PCI_ANY_ID ,PCI_ANY_ID ,}, {0 , }, };MODULE_DEVICE_TABLE (pci, ali_pci_tbl);
@@ -330,17 +330,25 @@u32 wdog;/* Check for a 1535 series bridge */ - pdev =pci_find_device (PCI_VENDOR_ID_AL ,0x1535 ,NULL ); -if (pdev =NULL ) + pdev =pci_get_device (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M1535 ,NULL ); +if (!pdev) { +pci_dev_put (pdev);return -ENODEV ; + }else { +pci_dev_put (pdev); + }/* Check for the a 7101 PMU */ - pdev =pci_find_device (PCI_VENDOR_ID_AL ,0x7101 ,NULL ); -if (pdev =NULL ) + pdev =pci_get_device (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M7101 ,NULL ); +if (!pdev) { +pci_dev_put (pdev);return -ENODEV ; + } -if (pci_enable_device (pdev)) +if (pci_enable_device (pdev)) { +pci_dev_put (pdev);return -EIO ; + } ali_pci = pdev;
@@ -447,6 +455,7 @@/* Deregister */ unregister_reboot_notifier (&ali_notifier);misc_deregister (&ali_miscdev); +pci_dev_put (ali_pci); }module_init (watchdog_init);
drivers/char/watchdog/alim7101_wdt.c +13 -2
@@ -332,6 +332,7 @@wdt_turnoff ();/* Deregister */ misc_deregister (&wdt_miscdev); +pci_dev_put (alim7101_pmu);unregister_reboot_notifier (&wdt_notifier); }
@@ -342,17 +343,20 @@char tmp; printk(KERN_INFO PFX "Steve Hill <[email protected]>.\n"); - alim7101_pmu =pci_find_device (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M7101 ,NULL ); + alim7101_pmu =pci_get_device (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M7101 ,NULL );if (!alim7101_pmu) { printk(KERN_INFO PFX "ALi M7101 PMU not present - WDT not set\n"); +pci_dev_put (alim7101_pmu);return -EBUSY ; }/* Set the WDT in the PMU to 1 second */ pci_write_config_byte (alim7101_pmu,ALI_7101_WDT ,0x02 ); - ali1543_south =pci_find_device (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M1533 ,NULL ); + ali1543_south =pci_get_device (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M1533 ,NULL );if (!ali1543_south) { +pci_dev_put (ali1543_south); +pci_dev_put (alim7101_pmu); printk(KERN_INFO PFX "ALi 1543 South-Bridge not present - WDT not set\n");return -EBUSY ; }
@@ -360,11 +364,15 @@if ((tmp &0x1e ) =0x00 ) {if (!use_gpio) { printk(KERN_INFO PFX "Detected old alim7101 revision 'a1d'. If this is a cobalt board, set the 'use_gpio' module parameter.\n"); +pci_dev_put (ali1543_south); +pci_dev_put (alim7101_pmu);return -EBUSY ; } nowayout =1 ; }else if ((tmp &0x1e ) !=0x12 && (tmp &0x1e ) !=0x00 ) { printk(KERN_INFO PFX "ALi 1543 South-Bridge does not have the correct revision number (???1001?) - WDT not set\n"); +pci_dev_put (ali1543_south); +pci_dev_put (alim7101_pmu);return -EBUSY ; }
@@ -397,6 +405,7 @@__module_get (THIS_MODULE ); } +pci_dev_put (ali1543_south); printk(KERN_INFO PFX "WDT driver for ALi M7101 initialised. timeout=%d sec (nowayout=%d)\n", timeout, nowayout);return 0 ;
@@ -404,6 +413,8 @@ err_out_miscdev:misc_deregister (&wdt_miscdev); err_out: +pci_dev_put (alim7101_pmu); +pci_dev_put (ali1543_south);return rc; }
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 21 lines · Patrik Kullman
This is my first try to submit a patch here, so go gentle on me.. ;) At least [email protected] thought it seemed ok..
This little patch moves from
pci_find_device()inalim1535_wdt.candalim7101_wdt.cto pci_get_device()/pci_dev_put() I also found thatalim1535.cuses 0x1535/0x7101 instead of the PCI_DEVICE_ID_AL_* defines.I compared my usage of
pci_dev_put()with other drivers and I hope that I've understood it right.Signed-off-by Patrik Kullman <[email protected]>drivers/char/watchdog/alim1535_wdt.c +1
-1@@ -312,7 +312,7 @@*/ static struct pci_device_id ali_pci_tbl[] = { - {PCI_VENDOR_ID_AL ,0x1535 ,PCI_ANY_ID ,PCI_ANY_ID ,}, + {PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M1535 ,PCI_ANY_ID ,PCI_ANY_ID ,}, {0 , }, };MODULE_DEVICE_TABLE (pci, ali_pci_tbl);
drivers/char/watchdog/alim1535_wdt.c line 335
Quoted 8 lines
drivers/char/watchdog/alim1535_wdt.c +3
-2@@ -330,17 +330,25 @@u32 wdog;/* Check for a 1535 series bridge */ - pdev =pci_find_device (PCI_VENDOR_ID_AL ,0x1535 ,NULL ); -if (pdev =NULL ) + pdev =pci_get_device (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M1535 ,NULL ); +if (!pdev) { +pci_dev_put (pdev);
pci_dev_put(NULL)?
drivers/char/watchdog/alim1535_wdt.c line 342
Quoted 9 lines
drivers/char/watchdog/alim1535_wdt.c +4
-2return -ENODEV ; + }else { +pci_dev_put (pdev); + }/* Check for the a 7101 PMU */ - pdev =pci_find_device (PCI_VENDOR_ID_AL ,0x7101 ,NULL ); -if (pdev =NULL ) + pdev =pci_get_device (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M7101 ,NULL );
What if the previous _and_ this one succeed?
Quoted 40 lines
drivers/char/watchdog/alim1535_wdt.c +7
-1+if (!pdev) { +pci_dev_put (pdev);return -ENODEV ; + } -if (pci_enable_device (pdev)) +if (pci_enable_device (pdev)) { +pci_dev_put (pdev);return -EIO ; + } ali_pci = pdev;@@ -447,6 +455,7 @@/* Deregister */ unregister_reboot_notifier (&ali_notifier);misc_deregister (&ali_miscdev); +pci_dev_put (ali_pci); }module_init (watchdog_init);drivers/char/watchdog/alim7101_wdt.c +3
-1@@ -332,6 +332,7 @@wdt_turnoff ();/* Deregister */ misc_deregister (&wdt_miscdev); +pci_dev_put (alim7101_pmu);unregister_reboot_notifier (&wdt_notifier); }@@ -342,17 +343,20 @@char tmp; printk(KERN_INFO PFX "Steve Hill <[email protected]>.\n"); - alim7101_pmu =pci_find_device (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M7101 ,NULL ); + alim7101_pmu =pci_get_device (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M7101 ,NULL );if (!alim7101_pmu) { printk(KERN_INFO PFX "ALi M7101 PMU not present - WDT not set\n"); +pci_dev_put (alim7101_pmu);
drivers/char/watchdog/alim7101_wdt.c line 358
Quoted 10 lines
drivers/char/watchdog/alim7101_wdt.c +2
-1return -EBUSY ; }/* Set the WDT in the PMU to 1 second */ pci_write_config_byte (alim7101_pmu,ALI_7101_WDT ,0x02 ); - ali1543_south =pci_find_device (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M1533 ,NULL ); + ali1543_south =pci_get_device (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M1533 ,NULL );if (!ali1543_south) { +pci_dev_put (ali1543_south);
pci_dev_put(NULL)
drivers/char/watchdog/alim7101_wdt.c line 359
Quoted 1 line
drivers/char/watchdog/alim7101_wdt.c +1
-0+pci_dev_put (alim7101_pmu);
Goto err_out (or something appropriate).
drivers/char/watchdog/alim7101_wdt.c line 368
Quoted 9 lines
drivers/char/watchdog/alim7101_wdt.c +2
-0printk(KERN_INFO PFX "ALi 1543 South-Bridge not present - WDT not set\n");return -EBUSY ; }@@ -360,11 +364,15 @@if ((tmp &0x1e ) =0x00 ) {if (!use_gpio) { printk(KERN_INFO PFX "Detected old alim7101 revision 'a1d'. If this is a cobalt board, set the 'use_gpio' module parameter.\n"); +pci_dev_put (ali1543_south); +pci_dev_put (alim7101_pmu);
ditto
drivers/char/watchdog/alim7101_wdt.c line 375
Quoted 7 lines
drivers/char/watchdog/alim7101_wdt.c +2
-0return -EBUSY ; } nowayout =1 ; }else if ((tmp &0x1e ) !=0x12 && (tmp &0x1e ) !=0x00 ) { printk(KERN_INFO PFX "ALi 1543 South-Bridge does not have the correct revision number (???1001?) - WDT not set\n"); +pci_dev_put (ali1543_south); +pci_dev_put (alim7101_pmu);
ditto
Quoted 22 lines
drivers/char/watchdog/alim7101_wdt.c +3
-0return -EBUSY ; }@@ -397,6 +405,7 @@__module_get (THIS_MODULE ); } +pci_dev_put (ali1543_south); printk(KERN_INFO PFX "WDT driver for ALi M7101 initialised. timeout=%d sec (nowayout=%d)\n", timeout, nowayout);return 0 ;@@ -404,6 +413,8 @@ err_out_miscdev:misc_deregister (&wdt_miscdev); err_out: +pci_dev_put (alim7101_pmu); +pci_dev_put (ali1543_south);return rc; }
Quoted 4 lines
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 12 lines · Patrik Kullman
Quoted 11 lines
drivers/char/watchdog/alim1535_wdt.c line 342
Quoted 9 lines
drivers/char/watchdog/alim1535_wdt.c +4
-2return -ENODEV ; + }else { +pci_dev_put (pdev); + }/* Check for the a 7101 PMU */ - pdev =pci_find_device (PCI_VENDOR_ID_AL ,0x7101 ,NULL ); -if (pdev =NULL ) + pdev =pci_get_device (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M7101 ,NULL );What if the previous _and_ this one succeed?
Quoted 4 lines
Since the original developer chose to just search for the 1535 and then overwrite the variable with the 7101 (that will be used), I thought I'd clear the memory of the first device before assigning the new device to it.
Guess I misunderstood the meaning.
Still... you only pci_dev_put one device on exit.
btw. nice to know patch-tester still works :-)
Domen
Quoted 6 lines
I guess I shouldn't have.
Will fix.
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors