lkml.org 
[lkml]   [2007]   [Jun]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] Documentation/firmware_class/firmware_sample_driver.c
    Satyam Sharma wrote:
    > Hi,
    >
    > [ It's good that you're trying to fix this, code in documentation should
    > be setting standards, clearly. ]
    >
    > On 6/27/07, Anders Blomdell <anders.blomdell@control.lth.se> wrote:
    >> [...]
    >> Minor modifications to make the example load and unload without Oops
    >> [...]
    >> static int sample_init(void)
    >> {
    >> - device_initialize(&ghost_device);
    >> + device_register(&ghost_device);
    >> /* since there is no real hardware insertion I just call the
    >> * sample probe functions here */
    >> - sample_probe_specific();
    >> + /* sample_probe_specific(); */
    >> sample_probe_default();
    >> - sample_probe_async();
    >> + /*sample_probe_async();*/
    >> return 0;
    >> }
    >
    > But IMO the above functions should be *fixed* to work properly instead
    > of simply commenting them out. If they are un-fixable, why even keep
    > the broken code in that file (only to mislead readers in future?). You
    > might as well remove those calls (and the function definitions) completely.
    > Best would be to fix them, of course.
    >
    > My Rs. 0.02,
    > Satyam

    Signed-off-by: Anders Blomdell <anders.blomdell@control.lth.se>

    Minor modifications to make the example load and unload without Oops, with
    suggestions from Satyam Sharma incorporated.

    This is what unpatched version generates with 2.6.21.3

    kernel: Oops: 0000 [#2]
    ...
    kernel: Call Trace:
    kernel: [<c04ae541>] sysfs_create_dir+0x49/0x63
    kernel: [<c04e76ae>] kobject_shadow_add+0xd5/0x17d
    kernel: [<c04e7908>] kobject_set_name+0x2b/0x92
    kernel: [<c0556444>] device_add+0x9a/0x58e
    kernel: [<c0620b65>] klist_init+0x23/0x2e
    kernel: [<c055bfee>] _request_firmware+0x116/0x29f
    kernel: [<c062149f>] wait_for_completion+0x8b/0x93
    kernel: [<c055c20b>] request_firmware+0xf/0x11
    kernel: [<e09420d9>] sample_init+0x59/0xc0 [firmware_sample_driver]
    kernel: [<c0446c92>] sys_init_module+0x16b7/0x17ee
    kernel: [<c04294de>] printk+0x0/0x1f
    kernel: [<c0488436>] mntput_no_expire+0x11/0x6e
    kernel: [<c0404fa8>] syscall_call+0x7/0xb




    diff -u Documentation/firmware_class/firmware_sample_driver.c
    /tmp/fw/firmware_sample_driver.c
    --- Documentation/firmware_class/firmware_sample_driver.c 2007-05-24
    23:22:47.000000000 +0200
    +++ /tmp/fw/firmware_sample_driver.c 2007-06-29 19:41:29.000000000 +0200
    @@ -15,11 +15,16 @@

    #include "linux/firmware.h"

    +static void ghost_release(struct device *dev)
    +{
    + printk(KERN_DEBUG "firmware_sample_driver: ghost_release\n");
    +}
    +
    static struct device ghost_device = {
    .bus_id = "ghost0",
    + .release = ghost_release
    };

    -
    static void sample_firmware_load(char *firmware, int size)
    {
    u8 buf[size+1];
    @@ -32,7 +37,7 @@
    {
    /* uses the default method to get the firmware */
    const struct firmware *fw_entry;
    - printk(KERN_INFO "firmware_sample_driver: a ghost device got inserted
    :)\n");
    + printk(KERN_INFO "firmware_sample_driver SYNC:\n");

    if(request_firmware(&fw_entry, "sample_driver_fw", &ghost_device)!=0)
    {
    @@ -47,27 +52,9 @@

    /* finish setting up the device */
    }
    -static void sample_probe_specific(void)
    -{
    - /* Uses some specific hotplug support to get the firmware from
    - * userspace directly into the hardware, or via some sysfs file */
    -
    - /* NOTE: This currently doesn't work */

    - printk(KERN_INFO "firmware_sample_driver: a ghost device got inserted
    :)\n");
    +static char *context = "async context";

    - if(request_firmware(NULL, "sample_driver_fw", &ghost_device)!=0)
    - {
    - printk(KERN_ERR
    - "firmware_sample_driver: Firmware load failed\n");
    - return;
    - }
    -
    - /* request_firmware blocks until userspace finished, so at
    - * this point the firmware should be already in the device */
    -
    - /* finish setting up the device */
    -}
    static void sample_probe_async_cont(const struct firmware *fw, void *context)
    {
    if(!fw){
    @@ -76,7 +63,7 @@
    return;
    }

    - printk(KERN_INFO "firmware_sample_driver: device pointer \"%s\"\n",
    + printk(KERN_INFO "firmware_sample_driver ASYNC: context \"%s\"\n",
    (char *)context);
    sample_firmware_load(fw->data, fw->size);
    }
    @@ -84,9 +71,9 @@
    {
    /* Let's say that I can't sleep */
    int error;
    - error = request_firmware_nowait (THIS_MODULE, FW_ACTION_NOHOTPLUG,
    + error = request_firmware_nowait (THIS_MODULE, FW_ACTION_HOTPLUG,
    "sample_driver_fw", &ghost_device,
    - "my device pointer",
    + context,
    sample_probe_async_cont);
    if(error){
    printk(KERN_ERR
    @@ -97,16 +84,18 @@

    static int sample_init(void)
    {
    - device_initialize(&ghost_device);
    + int ret = device_register(&ghost_device);
    + if (ret == 0) {
    /* since there is no real hardware insertion I just call the
    * sample probe functions here */
    - sample_probe_specific();
    sample_probe_default();
    sample_probe_async();
    - return 0;
    + }
    + return ret;
    }
    static void __exit sample_exit(void)
    {
    + device_unregister(&ghost_device);
    }

    module_init (sample_init);


    --
    Anders Blomdell Email: anders.blomdell@control.lth.se
    Department of Automatic Control
    Lund University Phone: +46 46 222 4625
    P.O. Box 118 Fax: +46 46 138118
    SE-221 00 Lund, Sweden
    -
    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/

    \
     
     \ /
      Last update: 2007-06-29 19:53    [W:5.330 / U:0.424 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site