Re: [PATCH 1/6] v4l: ti-vpe: Create a vpdma helper library

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Archit,

On Tuesday 20 August 2013 18:46:38 Archit Taneja wrote:
> On Tuesday 20 August 2013 05:09 PM, Laurent Pinchart wrote:
> 
> <snip>
> 
> >>>> +static int vpdma_load_firmware(struct vpdma_data *vpdma)
> >>>> +{
> >>>> +	int r;
> >>>> +	struct device *dev = &vpdma->pdev->dev;
> >>>> +
> >>>> +	r = request_firmware_nowait(THIS_MODULE, 1,
> >>>> +		(const char *) VPDMA_FIRMWARE, dev, GFP_KERNEL, vpdma,
> >>>> +		vpdma_firmware_cb);
> >>> 
> >>> Is there a reason not to use the synchronous interface ? That would
> >>> simplify both this code and the callers, as they won't have to check
> >>> whether the firmware has been correctly loaded.
> >> 
> >> I'm not clear what you mean by that, the firmware would be stored in the
> >> filesystem. If the driver is built-in, then the synchronous interface
> >> wouldn't work unless the firmware is appended to the kernel image. Am I
> >> missing something here? I'm not very aware of the firmware api.
> > 
> > request_firmware() would just sleep (with a 30 seconds timeout if I'm not
> > mistaken) until userspace provides the firmware. As devices are probed
> > asynchronously (in kernel threads) the system will just boot normally, and
> > the request_firmware() call will return when the firmware is available.
>
> Sorry, I sent the previous mail bit too early.
> 
> With request_firmware() and the driver built-in, I see that the kernel
> stalls for 10 seconds at the driver's probe, and the firware loading fails
> since we didn't enter userspace where the file is.
> 
> The probing of devices asynchronously with kernel threads makes sense, so
> it's possible that I'm doing something wrong here. I'll give it a try again

I might have spoken too fast. It looks like module initcalls are not run in 
threads. I've most probably mistaken that with asynchronous probing of hot-
pluggable devices.

If your driver is built-in then it looks like the correct solution is to build 
the firmware in the kernel image as well, or use the asynchronous API as you 
did.

-- 
Regards,

Laurent Pinchart