Re: [PATCH 1/5] mx2_camera: change to register and probe

[Michael Grzeschik <mgr@pengutronix.de>]

On Wed, Aug 04, 2010 at 10:24:50AM +0200, Guennadi Liakhovetski wrote:
> On Wed, 4 Aug 2010, Sascha Hauer wrote:
> 
> > On Wed, Aug 04, 2010 at 01:01:34AM +0200, Guennadi Liakhovetski wrote:
> > > On Tue, 3 Aug 2010, Michael Grzeschik wrote:
> > > 
> > > > On Tue, Aug 03, 2010 at 08:22:13PM +0200, Guennadi Liakhovetski wrote:
> > > > > On Tue, 3 Aug 2010, Michael Grzeschik wrote:
> > > > > 
> > > > > > change this driver back to register and probe, since some platforms
> > > > > > first have to initialize an already registered power regulator to switch
> > > > > > on the camera.
> > > > > 
> > > > > Sorry, don't see a difference. Can you give an example of two call 
> > > > > sequences, where this change changes the behaviour?
> > > > >
> > > > 
> > > > Yes, when you look at the today posted patch [1] you find the function
> > > > pcm970_baseboard_init_late as an late_initcall. It uses an already
> > > > registred regulator device to turn on the power of the camera before the
> > > > cameras device registration.
> > > > 
> > > > [1] [PATCH 1/2] ARM: i.MX27 pcm970: Add camera support
> > > > http://lists.infradead.org/pipermail/linux-arm-kernel/2010-August/022317.html
> > > 
> > > Sorry again, still don't understand. What I mean is the following: take 
> > > two cases - before and after your patch. What is the difference? As far as 
> > > I know, the difference between platform_driver_probe() and 
> > > platform_driver_register() is just that the probe method gets discarded in 
> > > an __init section, which is suitable for non hotpluggable devices. I don't 
> > > know what the difference this should make for call order. So, that's what 
> > > I am asking about. Can you explain, how this patch changes the call order 
> > > in your case? Can you tell, that in the unpatches case the probe is called 
> > > at that moment, and in the patched case it is called at a different point 
> > > of time and that fixes the problem.
> > 
> > 
> > The following is above platform_driver_probe:
> > 
> >  * Use this instead of platform_driver_register() when you know the device
> >  * is not hotpluggable and has already been registered, and you want to
> >  * remove its run-once probe() infrastructure from memory after the
> >  * driver has bound to the device.
> > 
> > So platform_driver_probe will only call the probe function when the device
> > is already there when this function runs. This is not the case on our board.
> > We have to register the camera in late_initcall (to make sure the needed
> > regulators are already there). During late_initcall time the
> > platform_driver_probe has already run.
> 
> Ok, now I see. I missed the key-phrase: "before the cameras device 
> registration." Ok, in this case, it's certainly a valid reason for the 
> change. Just one more question: wouldn't calling 
> pcm970_baseboard_init_late() from device_initcall fix the problem without 
> requiring to change the driver?

No, sorry but this doesn't solve the problem. I tested it and get an
"unable to get regulator: -19" when i hit on that. The problem is the
device init order. The pcm970_baseboard_init_late comes first and
then the regulator. So i think we should keep that patch.

Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |