Hi Kamil,
On Thursday 13 March 2014 05:18 PM, Kamil Debski wrote:
Hi Archit,
From: Archit Taneja [mailto:archit@xxxxxx]
Sent: Tuesday, March 11, 2014 9:34 AM
vpe fops(vpe_open in particular) should be called only when VPDMA
firmware is loaded. File operations on the video device are possible
the moment it is registered.
Currently, we register the video device for VPE at driver probe, after
calling a vpdma helper to initialize VPDMA and load firmware. This
function is non-blocking(it calls request_firmware_nowait()), and
doesn't ensure that the firmware is actually loaded when it returns.
We remove the device registration from vpe probe, and move it to a
callback provided by the vpe driver to the vpdma library, through
vpdma_create().
The ready field in vpdma_data is no longer needed since we always have
firmware loaded before the device is registered.
A minor problem with this approach is that if the video_register_device
fails(which doesn't really happen), the vpe platform device would be
registered.
however, there won't be any v4l2 device corresponding to it.
Could you explain to me one thing. request_firmware cannot be used in
probe, thus you are using request_firmware_nowait. Why cannot the firmware
be
loaded on open with a regular request_firmware that is waiting?
I totally agree with you here. Placing the firmware in open() would
probably make more sense.
The reason I didn't place it in open() is because I didn't want to
release firmware in a corresponding close(), and be in a situation where
the firmware is loaded multiple times in the driver's lifetime.
There are some reasons for doing this. First, it will require more
testing with respect to whether the firmware is loaded correctly
successive times :), the second is that loading firmware might probably
take a bit of time, and we don't want to make applications too slow(I
haven't measured the time taken, so I don't have a strong case for this
either)
This patch seems to swap one problem for another. The possibility that open
fails (because firmware is not yet loaded) is swapped for a vague
possibility
that video_register_device.
The driver will work fine in most cases even without this patch(apart
from the possibility mentioned as above).
We could discard this patch from this series, and I can work on a patch
which moves firmware loading to the vpe_open() call, and hence solving
the issue in the right manner. Does that sound fine?
Thanks,
Archit
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
