Messages in this thread | | | Date | Fri, 5 Sep 2008 14:33:18 +0400 | From | Dmitry Baryshkov <> | Subject | Re: [PATCH] platform: add new device registration helper |
| |
On Thu, Sep 04, 2008 at 05:20:50PM -0700, Andrew Morton wrote: > On Fri, 5 Sep 2008 01:30:16 +0400 > Dmitry Baryshkov <dbaryshkov@gmail.com> wrote: > > > Add a helper that registers simple platform_device > > w/o resources but with parent and device data. > > > > This is usefull to cleanup platform code from code that > > registers such simple devices as leds-gpio, generic-bl, > > etc. > > > > nits: > > > + * > > + * This function creates a simple platform device that requires minimal > > + * resource and memory management. Canned release function freeing memory > > + * allocated for the device allows drivers using such devices to be > > + * unloaded without waiting for the last reference to the device to be > > + * dropped. > > Should document the return value (tested with IS_ERR)
Will the wording "Returns a valid platform device, or a valid IS_ERR condition containing errno" be appropriate?
> > +{ > > + struct platform_device *pdev; > > + int retval; > > + > > + pdev = platform_device_alloc(name, id); > > + if (!pdev) { > > + retval = -ENOMEM; > > + goto error; > > + } > > + > > + pdev->dev.parent = parent; > > + > > + if (size) { > > + retval = platform_device_add_data(pdev, data, size); > > + if (retval) > > + goto error; > > + } > > + > > + retval = platform_device_add(pdev); > > + if (retval) > > + goto error; > > + > > + return pdev; > > + > > +error: > > + platform_device_put(pdev); > > Are you sure this can't trigger a !kobj->state_initialized warning in > kobject_put()?
No. The kobject is initialised in platform_device_alloc() -> device_initialise()
> > > > ... > > > > +extern struct platform_device *platform_device_register_data(struct device *, > > + const char *, int, const void *, size_t); > > It's nice (IMO) to include the names of the args, for documentation > purposes. Obviously the surrounding code didn't agree.
I just copied the surrounding code style. If you wish, I can put the names for this function back.
BTW: would you prefer the followup "fix" patch or just the replacement one?
-- With best wishes Dmitry
| |