[PATCH 1/1] Blackfin Watchdog Driver: split platform device/driver registering from actual watchdog device/driver registering so we can cleanly load/unload - Kernel

This is a discussion on [PATCH 1/1] Blackfin Watchdog Driver: split platform device/driver registering from actual watchdog device/driver registering so we can cleanly load/unload - Kernel ; From: Mike Frysinger Signed-off-by: Mike Frysinger Signed-off-by: Bryan Wu --- drivers/watchdog/bfin_wdt.c | 109 +++++++++++++++++++++++++++--------------- 1 files changed, 70 insertions(+), 39 deletions(-) diff --git a/drivers/watchdog/bfin_wdt.c b/drivers/watchdog/bfin_wdt.c index 1237113..c9944ba 100644 --- a/drivers/watchdog/bfin_wdt.c +++ b/drivers/watchdog/bfin_wdt.c @@ -29,6 +29,7 @@ #define stamp(fmt, args...) pr_debug("%s:%i: ...

+ Reply to Thread
Results 1 to 5 of 5

Thread: [PATCH 1/1] Blackfin Watchdog Driver: split platform device/driver registering from actual watchdog device/driver registering so we can cleanly load/unload

  1. [PATCH 1/1] Blackfin Watchdog Driver: split platform device/driver registering from actual watchdog device/driver registering so we can cleanly load/unload

    From: Mike Frysinger

    Signed-off-by: Mike Frysinger
    Signed-off-by: Bryan Wu
    ---
    drivers/watchdog/bfin_wdt.c | 109 +++++++++++++++++++++++++++---------------
    1 files changed, 70 insertions(+), 39 deletions(-)

    diff --git a/drivers/watchdog/bfin_wdt.c b/drivers/watchdog/bfin_wdt.c
    index 1237113..c9944ba 100644
    --- a/drivers/watchdog/bfin_wdt.c
    +++ b/drivers/watchdog/bfin_wdt.c
    @@ -29,6 +29,7 @@

    #define stamp(fmt, args...) pr_debug("%s:%i: " fmt "\n", __func__, __LINE__, ## args)
    #define stampit() stamp("here i am")
    +#define pr_devinit(fmt, args...) ({ static const __devinitdata char __fmt[] = fmt; printk(__fmt, ## args); })
    #define pr_init(fmt, args...) ({ static const __initdata char __fmt[] = fmt; printk(__fmt, ## args); })

    #define WATCHDOG_NAME "bfin-wdt"
    @@ -377,20 +378,6 @@ static int bfin_wdt_resume(struct platform_device *pdev)
    # define bfin_wdt_resume NULL
    #endif

    -static struct platform_device bfin_wdt_device = {
    - .name = WATCHDOG_NAME,
    - .id = -1,
    -};
    -
    -static struct platform_driver bfin_wdt_driver = {
    - .driver = {
    - .name = WATCHDOG_NAME,
    - .owner = THIS_MODULE,
    - },
    - .suspend = bfin_wdt_suspend,
    - .resume = bfin_wdt_resume,
    -};
    -
    static const struct file_operations bfin_wdt_fops = {
    .owner = THIS_MODULE,
    .llseek = no_llseek,
    @@ -418,11 +405,67 @@ static struct notifier_block bfin_wdt_notifier = {
    };

    /**
    - * bfin_wdt_init - Initialize module
    + * bfin_wdt_probe - Initialize module
    *
    - * Registers the device and notifier handler. Actual device
    + * Registers the misc device and notifier handler. Actual device
    * initialization is handled by bfin_wdt_open().
    */
    +static int __devinit bfin_wdt_probe(struct platform_device *pdev)
    +{
    + int ret;
    +
    + ret = register_reboot_notifier(&bfin_wdt_notifier);
    + if (ret) {
    + pr_devinit(KERN_ERR PFX "cannot register reboot notifier (err=%d)\n", ret);
    + return ret;
    + }
    +
    + ret = misc_register(&bfin_wdt_miscdev);
    + if (ret) {
    + pr_devinit(KERN_ERR PFX "cannot register miscdev on minor=%d (err=%d)\n",
    + WATCHDOG_MINOR, ret);
    + unregister_reboot_notifier(&bfin_wdt_notifier);
    + return ret;
    + }
    +
    + pr_devinit(KERN_INFO PFX "initialized: timeout=%d sec (nowayout=%d)\n",
    + timeout, nowayout);
    +
    + return 0;
    +}
    +
    +/**
    + * bfin_wdt_remove - Initialize module
    + *
    + * Unregisters the misc device and notifier handler. Actual device
    + * deinitialization is handled by bfin_wdt_close().
    + */
    +static int __devexit bfin_wdt_remove(struct platform_device *pdev)
    +{
    + misc_deregister(&bfin_wdt_miscdev);
    + unregister_reboot_notifier(&bfin_wdt_notifier);
    + return 0;
    +}
    +
    +static struct platform_device *bfin_wdt_device;
    +
    +static struct platform_driver bfin_wdt_driver = {
    + .probe = bfin_wdt_probe,
    + .remove = __devexit_p(bfin_wdt_remove),
    + .suspend = bfin_wdt_suspend,
    + .resume = bfin_wdt_resume,
    + .driver = {
    + .name = WATCHDOG_NAME,
    + .owner = THIS_MODULE,
    + },
    +};
    +
    +/**
    + * bfin_wdt_init - Initialize module
    + *
    + * Checks the module params and registers the platform device & driver.
    + * Real work is in the platform probe function.
    + */
    static int __init bfin_wdt_init(void)
    {
    int ret;
    @@ -436,44 +479,32 @@ static int __init bfin_wdt_init(void)
    /* Since this is an on-chip device and needs no board-specific
    * resources, we'll handle all the platform device stuff here.
    */
    - ret = platform_device_register(&bfin_wdt_device);
    - if (ret)
    - return ret;
    -
    - ret = platform_driver_probe(&bfin_wdt_driver, NULL);
    - if (ret)
    - return ret;
    -
    - ret = register_reboot_notifier(&bfin_wdt_notifier);
    + ret = platform_driver_register(&bfin_wdt_driver);
    if (ret) {
    - pr_init(KERN_ERR PFX "cannot register reboot notifier (err=%d)\n", ret);
    + pr_init(KERN_ERR PFX "unable to register driver\n");
    return ret;
    }

    - ret = misc_register(&bfin_wdt_miscdev);
    - if (ret) {
    - pr_init(KERN_ERR PFX "cannot register miscdev on minor=%d (err=%d)\n",
    - WATCHDOG_MINOR, ret);
    - unregister_reboot_notifier(&bfin_wdt_notifier);
    - return ret;
    + bfin_wdt_device = platform_device_register_simple(WATCHDOG_NAME, -1, NULL, 0);
    + if (IS_ERR(bfin_wdt_device)) {
    + pr_init(KERN_ERR PFX "unable to register device\n");
    + platform_driver_unregister(&bfin_wdt_driver);
    + return PTR_ERR(bfin_wdt_device);
    }

    - pr_init(KERN_INFO PFX "initialized: timeout=%d sec (nowayout=%d)\n",
    - timeout, nowayout);
    -
    return 0;
    }

    /**
    * bfin_wdt_exit - Deinitialize module
    *
    - * Unregisters the device and notifier handler. Actual device
    - * deinitialization is handled by bfin_wdt_close().
    + * Back out the platform device & driver steps. Real work is in the
    + * platform remove function.
    */
    static void __exit bfin_wdt_exit(void)
    {
    - misc_deregister(&bfin_wdt_miscdev);
    - unregister_reboot_notifier(&bfin_wdt_notifier);
    + platform_device_unregister(bfin_wdt_device);
    + platform_driver_unregister(&bfin_wdt_driver);
    }

    module_init(bfin_wdt_init);
    --
    1.5.4.3
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH 1/1] Blackfin Watchdog Driver: split platform device/driver registering from actual watchdog device/driver registering so we can cleanly load/unload

    On 03/27/2008 02:30 AM, Bryan Wu wrote:
    > From: Mike Frysinger
    >
    > Signed-off-by: Mike Frysinger
    > Signed-off-by: Bryan Wu
    > ---
    > drivers/watchdog/bfin_wdt.c | 109 +++++++++++++++++++++++++++---------------
    > 1 files changed, 70 insertions(+), 39 deletions(-)
    >
    > diff --git a/drivers/watchdog/bfin_wdt.c b/drivers/watchdog/bfin_wdt.c
    > index 1237113..c9944ba 100644
    > --- a/drivers/watchdog/bfin_wdt.c
    > +++ b/drivers/watchdog/bfin_wdt.c
    > @@ -29,6 +29,7 @@
    >
    > #define stamp(fmt, args...) pr_debug("%s:%i: " fmt "\n", __func__, __LINE__, ## args)
    > #define stampit() stamp("here i am")
    > +#define pr_devinit(fmt, args...) ({ static const __devinitdata char __fmt[] = fmt; printk(__fmt, ## args); })


    Ah and here. __devinitconst
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH 1/1] Blackfin Watchdog Driver: split platform device/driver registering from actual watchdog device/driver registering so we can cleanly load/unload

    2008/3/26 Jiri Slaby :
    > On 03/27/2008 02:30 AM, Bryan Wu wrote:
    > > From: Mike Frysinger
    > > +#define pr_devinit(fmt, args...) ({ static const __devinitdata char __fmt[] = fmt; printk(__fmt, ## args); })

    >
    > Ah and here. __devinitconst


    that define doesnt exist for me so it'd be hard to use it ... where
    are you finding this ?
    -mike
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH 1/1] Blackfin Watchdog Driver: split platform device/driver registering from actual watchdog device/driver registering so we can cleanly load/unload

    On Wed, Mar 26, 2008 at 06:30:01PM -0700, Bryan Wu wrote:
    > +#define pr_devinit(fmt, args...) ({ static const __devinitdata char __fmt[] = fmt; printk(__fmt, ## args); })
    > #define pr_init(fmt, args...) ({ static const __initdata char __fmt[] = fmt; printk(__fmt, ## args); })
    >

    WTF? You are trying to micro-optimize the string? And you have a printk
    that changes behaviour if you have hotplug enabled or not. That's so
    utterly bizarre that the rationale must be fascinating.

    pr_xxx() also is a protected namespace that belongs in
    include/linux/kernel.h, though I can see why you opted to hide these in
    your driver rather than post them for general inclusion.

    If you are hurting that badly for space, just turn off CONFIG_PRINTK and
    move on with life. This is just insane.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH 1/1] Blackfin Watchdog Driver: split platform device/driver registering from actual watchdog device/driver registering so we can cleanly load/unload

    On Wed, Mar 26, 2008 at 12:37 PM, Paul Mundt wrote:
    > On Wed, Mar 26, 2008 at 06:30:01PM -0700, Bryan Wu wrote:
    > > +#define pr_devinit(fmt, args...) ({ static const __devinitdata char __fmt[] = fmt; printk(__fmt, ## args); })
    > > #define pr_init(fmt, args...) ({ static const __initdata char __fmt[] = fmt; printk(__fmt, ## args); })

    >
    > WTF? You are trying to micro-optimize the string? And you have a printk
    > that changes behaviour if you have hotplug enabled or not. That's so
    > utterly bizarre that the rationale must be fascinating.


    the rationale behind it seems fairly obvious to me. the strings are
    placed into the respective init sections so they get released as
    needed. it isnt a matter of tying it to hotplug as tying it to the
    function: __init or __devinit.

    > pr_xxx() also is a protected namespace that belongs in
    > include/linux/kernel.h, though I can see why you opted to hide these in
    > your driver rather than post them for general inclusion.


    actually, it was already posted to lkml some time ago as an idea. the
    idea stalled so i just use it in my drivers in the meantime. it's
    certainly much cleaner than what was proposed by someone else
    originally:
    char __init foo[] = "moo";
    ....
    printk(foo, ...)
    ....

    > If you are hurting that badly for space, just turn off CONFIG_PRINTK and
    > move on with life. This is just insane.


    doing it this way seems pretty clean to me using existing section markings.
    -mike
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread