lkml.org 
[lkml]   [2008]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: Clarifying platform_device_unregister
    On Tue, Apr 01, 2008 at 12:47:54AM -0700, Jaya Kumar wrote:
    > On Mon, Mar 31, 2008 at 10:19 PM, Dmitry Torokhov
    > <dmitry.torokhov@gmail.com> wrote:
    > > On Mon, Mar 31, 2008 at 09:14:35PM -0400, Jaya Kumar wrote:
    > > > mytest_device->dev.platform_data = &mydata;
    > >
    > > Platform device code does kfree(pdev->dev.platform_data) unpon
    > > unregistration, so it is not a good idea to assign address of
    > > statically-allocated variable here. You should be using:
    > >
    > > platform_device_add_data(mytest_device, &mydata, sizeof(mydata));
    > >
    >
    > That's interesting. I noticed though that a lot of platform device
    > code assigns a statically allocated structure to platform_data. For
    > example:
    >
    > arch/arm/mach-pxa/corgi_pm.c
    > static struct sharpsl_charger_machinfo corgi_pm_machinfo = {
    > ...
    > }
    > corgipm_device->dev.platform_data = &corgi_pm_machinfo;
    >
    > same with spitz_pm.c.
    >
    > egrep "platform_data.*=.*\&" *.c shows quite a lot of users doing
    > that. I guess most of these below are probably okay since these
    > drivers can't be rmmoded.
    >

    Hmm, are you sure they can't be removed? Why do they all have
    module_exit methods?

    Even if they can't be unloaded the whole thing will blow to pieces
    if registration fails. Consider this:

    static int __devinit spitzpm_init(void)
    {
    int ret;

    spitzpm_device = platform_device_alloc("sharpsl-pm", -1);
    if (!spitzpm_device)
    return -ENOMEM;

    spitzpm_device->dev.platform_data = &spitz_pm_machinfo;
    ret = platform_device_add(spitzpm_device);

    if (ret)
    platform_device_put(spitzpm_device);
    ^^^^^^^^^^^
    This will try to kfree(spitzpm_device->dev.platform_data) and it gonna
    blow. We need to do spitzpm_device->dev.platform_data = NULL before doing
    put.

    Also spitzpm_init() shoudl be marked __init, not __devinit and
    spitzpm_exit() should be __exit() if it is event needed at all.

    Richard, I think you work with spitz and corgi, any comments?

    --
    Dmitry


    \
     
     \ /
      Last update: 2008-04-01 16:57    [W:2.396 / U:0.184 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site