lkml.org 
[lkml]   [2009]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/6] omap iommu: omap3 iommu device registration
    On Fri, Jan 16, 2009 at 10:37:20AM +0200, Hiroshi DOYU wrote:
    > +#include <linux/io.h>

    Is linux/io.h needed, or will a more specific include be better?

    > +#include <linux/platform_device.h>
    > +
    > +#include <mach/iommu.h>
    > +
    > +#define DEVNAME "omap-iommu"

    I'm not sure this DEVNAME definition really helps anything.

    > +static void omap3_iommu_release(struct device *dev)
    > +{
    > +}

    Err, no. Never ever ever provide a NULL release function. Providing
    such a function is a screaming message that what you're doing is buggy.

    And if you get a warning through not providing such a function, it's
    telling you that what your overall approach with the driver API is
    buggy (and you haven't understood the implications of refcounted
    object management.)

    > +
    > +static struct platform_device omap3_iommu_pdev[] = {
    > + {
    > + .name = DEVNAME,
    > + .id = 1,
    > + .num_resources = ARRAY_SIZE(iommu1_res),
    > + .resource = iommu1_res,
    > + .dev = {
    > + .release = omap3_iommu_release,
    > + .platform_data = &omap3_iommu_pdata[0],
    > + },
    > + },
    > + {
    > + .name = DEVNAME,
    > + .id = 2,
    > + .num_resources = ARRAY_SIZE(iommu2_res),
    > + .resource = iommu2_res,
    > + .dev = {
    > + .release = omap3_iommu_release,
    > + .platform_data = &omap3_iommu_pdata[1],
    > + },
    > + },
    > +};
    > +
    > +static int __init omap3_iommu_init(void)
    > +{
    > + int i;
    > + for (i = 0; i < ARRAY_SIZE(omap3_iommu_pdev); i++)
    > + platform_device_register(&omap3_iommu_pdev[i]);
    > + return 0;
    > +}
    > +module_init(omap3_iommu_init);
    > +
    > +static void __exit omap3_iommu_exit(void)
    > +{
    > + int i;
    > + for (i = 0; i < ARRAY_SIZE(omap3_iommu_pdev); i++)
    > + platform_device_unregister(&omap3_iommu_pdev[i]);

    So... this can never be bug free - you can _never_ unregister statically
    allocated devices. Not even if you provide an empty release function.

    If you want to register and unregister device structures, it must be
    done using the correct APIs, and in the case of platform devices, that's
    the platform_device_alloc(), platform_device_add() and
    platform_device_unregister() APIs.


    \
     
     \ /
      Last update: 2009-01-17 17:25    [W:0.021 / U:13.432 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site