Am 20.08.2013 15:38, schrieb Laurent Pinchart:
> Hi Mauro,
>
> On Sunday 18 August 2013 12:20:08 Mauro Carvalho Chehab wrote:
>> Em Sun, 18 Aug 2013 13:40:25 +0200 Frank Schäfer escreveu:
>>> Am 17.08.2013 12:51, schrieb Guennadi Liakhovetski:
>>>> Hi Frank,
>>>> As I mentioned on the list, I'm currently on a holiday, so, replying
>>>> briefly.
>>> Sorry, I missed that (can't read all mails on the list).
>>>
>>>> Since em28xx is a USB device, I conclude, that it's supplying clock to
>>>> its components including the ov2640 sensor. So, yes, I think the driver
>>>> should export a V4L2 clock.
>>> Ok, so it's mandatory on purpose ?
>>> I'll take a deeper into the v4l2-clk code and the
>>> em28xx/ov2640/soc-camera interaction this week.
>>> Have a nice holiday !
>> commit 9aea470b399d797e88be08985c489855759c6c60
>> Author: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
>> Date: Fri Dec 21 13:01:55 2012 -0300
>>
>> [media] soc-camera: switch I2C subdevice drivers to use v4l2-clk
>>
>> Instead of centrally enabling and disabling subdevice master clocks in
>> soc-camera core, let subdevice drivers do that themselves, using the
>> V4L2 clock API and soc-camera convenience wrappers.
>>
>> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
>> Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>> Acked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>
>>
>> (c/c the ones that acked with this broken changeset)
>>
>> We need to fix it ASAP or to revert the ov2640 changes, as some em28xx
>> cameras are currently broken on 3.10.
>>
>> I'll also reject other ports to the async API if the drivers are
>> used outside an embedded driver, as no PC driver currently defines
>> any clock source. The same applies to regulators.
>>
>> Guennadi,
>>
>> Next time, please check if the i2c drivers are used outside soc_camera
>> and apply the fixes where needed, as no regressions are allowed.
> We definitely need to check all users of our sensor drivers when making such a
> change. Mistakes happen, so let's fix them.
>
> Guennadi is on holidays until the end of this week. Would that be too late to
> fix the issue (given that 3.10 is already broken) ? The fix shouldn't be too
> complex, registering a dummy V4L2 clock in the em28xx driver should be enough.
I would prefer either a) making the clock optional in the senor
driver(s) or b) implementing a real V4L2 clock.
Reading the soc-camera code, it looks like NULL-pointers for struct
v4l2_clk are handled correctly. so a) should be pretty simple:
priv->clk = v4l2_clk_get(&client->dev, "mclk");
- if (IS_ERR(priv->clk)) {
- ret = PTR_ERR(priv->clk);
- goto eclkget;
- }
+ if (IS_ERR(priv->clk))
+ priv->clk = NULL;
Some additional NULL-pointer checks might be necessary, e.g. before
calling v4l2_clk_put().
Concerning b): I'm not yet sure if it is really needed/makes sense...
Who is supposed to configure/enable/disable the clock in a constellation
like em28xx+ov2640 ?
For UXGA for example, the clock needs to be switched to 12MHz, while
24MHz is used for smaller reolutions.
But I'm not sure if it is a good idea to let the sensor driver do the
switch (to define fixed bindings between resoultions and clock frequencies).
Btw, what if a frequency is requested which isn't supported ? Set the
clock to the next nearest supported frequency ?
Regards,
Frank
>
> v4l2-clk.c should provide a helper function to do so as that will be a pretty
> common operation.
>
--
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
