On Fri, Dec 10, 2010 at 08:53:31AM +0100, Corentin Chary wrote:
> On Fri, Dec 10, 2010 at 8:31 AM, Dmitry Torokhov
> <dmitry.torok...@gmail.com> wrote:
> > On Fri, Dec 10, 2010 at 08:12:53AM +0100, Corentin Chary wrote:
> >> On Fri, Dec 10, 2010 at 1:08 AM, Dmitry Torokhov
> >> <dmitry.torok...@gmail.com> wrote:
> >> >
> >> > .remove is still needed but I guess Corentin refers to using
> >> > platform_driver_probe() instead of platform_driver_register() so the
> >> > megaware_probe can be marked __init and discarded after driver
> >> > initialization has been completed.
> >>
> >> Also, register won't return any error code if the probe function failed,
> >> and the driver will be left in a strange state.
> >>
> >> Calling directly platform_driver_probe() will return an error code.
> >>
> >> But since it's a singleton device, and since the real "probe" code
> >> is already in the module_init function (wmi_has_guid()) call, using
> >> the probe callback to initialize data structures doesn't seems to make
> >> sense.
> >>
> >
> > What's the other option (provided that we want to keep platform device)?
> > Manual binding device and driver? I think platform_driver_probe() is
> > better tested and is safer, emits all necessary uevents. etc, etc.
> >
> > I'd suggest platform_create_bundle() except I am not quite happy with
> > the API at the moment (needs to also accept drvdata I think).
> >
> 
> Well, I'd suggest something like
> http://git.kernel.org/?p=linux/kernel/git/mjg59/platform-drivers-x86.git;a=blob;f=drivers/platform/x86/eeepc-wmi.c;hb=linux-next#l887
> 
> It's like platform_create_bundle, but by hand, because I didn't like
> the API too.
> 
> Please note that I'd gladly accept any comment/patch on eeepc-wmi if you
> think it's not ok :).

Well, I see one, more theoretical than practical issue here. Your
platform driver does not suppress bind/unbind via sysfs and when I
unbind a driver from a device I'd really expect children disappear. But
your implementation of eeepc-wmi it does not work this way.

> 
> Maybe the wmi_has_guid() calls should be in the probe callback,
> and we should all call platform_driver_probe() ?
> 

Yes, I think this would work.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" 
in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to