lkml.org 
[lkml]   [2011]   [May]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] Cleanup pc87413 watchdog driver to use request_muxed_region for SuperIO area
    On Fri, May 20, 2011 at 02:21:29PM +0200, Wim Van Sebroeck wrote:
    > Hi Jonathan,
    >
    > > > @@ -528,7 +511,8 @@ static int __init pc87413_init(void)
    > > > printk(KERN_INFO PFX "Version " VERSION " at io 0x%X\n",
    > > > WDT_INDEX_IO_PORT);
    > > >
    > > > - /* request_region(io, 2, "pc87413"); */
    > > > + if (!request_muxed_region(io, 2, MODNAME))
    > > > + return -EBUSY;
    > > >
    > > > ret = register_reboot_notifier(&pc87413_notifier);
    > > > if (ret != 0) {
    > > > @@ -541,12 +525,32 @@ static int __init pc87413_init(void)
    > > > printk(KERN_ERR PFX
    > > > "cannot register miscdev on minor=%d (err=%d)\n",
    > > > WATCHDOG_MINOR, ret);
    > > > - unregister_reboot_notifier(&pc87413_notifier);
    > > > - return ret;
    > > > + goto reboot_unreg;
    > > > }
    > > > printk(KERN_INFO PFX "initialized. timeout=%d min \n", timeout);
    > > > +
    > > > + pc87413_select_wdt_out();
    > > > + pc87413_enable_swc();
    > > > + pc87413_get_swc_base_addr();
    > > > +
    > > > + if (!request_region(swc_base_addr, 0x20, MODNAME)) {
    > > > + printk(KERN_ERR PFX
    > > > + "cannot request SWC region at 0x%x\n", swc_base_addr);
    > > > + ret = -EBUSY;
    > > > + goto misc_unreg;
    > > > + }
    > > > +
    > > > pc87413_enable();
    > > > +
    > > > + release_region(io, 2);
    > > > return 0;
    > > > +
    > > > +misc_unreg:
    > > > + misc_deregister(&pc87413_miscdev);
    > > > +reboot_unreg:
    > > > + unregister_reboot_notifier(&pc87413_notifier);
    > > > + release_region(io, 2);
    > > > + return ret;
    > > > }
    > > >
    > > > /**
    > > > @@ -569,7 +573,7 @@ static void __exit pc87413_exit(void)
    > > >
    > > > misc_deregister(&pc87413_miscdev);
    > > > unregister_reboot_notifier(&pc87413_notifier);
    > > > - /* release_region(io, 2); */
    > > > + release_region(swc_base_addr, 0x20);
    > > >
    > > > printk(KERN_INFO MODNAME " watchdog component driver removed.\n");
    > > > }

    > I don't think this is the correct way to do this. You do the
    > request_muxed_region only at init and exit of the module.

    I do a single request_muxed_region in the init function, and release it
    at the end of the init function. It's only necessary to hold the LPC
    region while the WDT functionality is enabled and the base address for
    the SWC block is retrieved.

    After that point all access is via the SWC region, so there's no need to
    call request_muxed_region. The call in the exit function releases the
    SWC block. We're not holding the LPC region for the entire duration of
    the driver being loaded.

    J.

    --
    Hail Eris. All hail Discordia. | .''`. Debian GNU/Linux Developer
    Fnord? | : :' : Happy to accept PGP signed
    | `. `' or encrypted mail - RSA
    | `- key on the keyservers.


    \
     
     \ /
      Last update: 2011-05-20 18:23    [W:0.027 / U:1.392 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site