[Letux-kernel] ov9655 madness in 4.20

[H. Nikolaus Schaller]

Hi Andreas,

> Am 05.01.2019 um 19:33 schrieb Andreas Kemnade <andreas at kemnade.info>:
> 
> Hi Nikolaus,
> 
> On Sat, 5 Jan 2019 19:01:16 +0100
> "H. Nikolaus Schaller" <hns at goldelico.com> wrote:
> 
>> Hi Andreas,
>> 
>>> Am 05.01.2019 um 18:58 schrieb Andreas Kemnade <andreas at kemnade.info>:
>>> 
>>> On Sat, 5 Jan 2019 15:49:28 +0100
>>> Andreas Kemnade <andreas at kemnade.info> wrote:
>>> 
>>> [...]  
>>>> [  290.168395] ov9655 1-0030: __ov9655_set_power on=1
>>>> [  290.223907] ov9655 1-0030: Failed to enable clock (-6)
>>>> <-- probably the interesting root cause for further problems.
>>>> 
>>> ok, found out what seems to happen here:
>>> If I rmmod omap3_isp, the ov9655 driver keeps a reference to the clock
>>> of that now unregistered module and continues to work with that
>>> reference.  
>> 
>> Ok, I see.
>> 
> /*
> * Empty clk_ops for unregistered clocks. These are used temporarily
> * after clk_unregister() was called on a clock and until last clock
> * consumer calls clk_put() and the struct clk object is freed.
> */
> static int clk_nodrv_prepare_enable(struct clk_hw *hw)
> {
>        return -ENXIO;
> }
> 
> we end up there. So nothing really bad happens as long as we handle the
> ENXIO.

Ok.

> 
>>> So the workaround is easily rmmod both omap3_isp and ov9655
>>> and this is nothing with high priority to fix.  
>> 
>> Well, should it be locked somehow? Other modules seems to be able
>> to describe dependencies and then you can't even rmmod them.
>> 
> ov9650 should either prevent removal of omap3_isp or the ov9650 should
> put() the clock when not in use. No idea what is better.

Probably a put(). The camera driver should know if it needs a clock or not...

> But that is low
> priority as long as we do not know how to give the OMAP ISP-hwmod
> the right kick in the a** to start again.

Indeed. This problem made me give up working on the driver many months ago,
because I wasn't able to solve it. So although the plan was to upstream
the driver (there was someone really intersted in it as well), I moved to
other topics (and there are always too many :)

Otherwise the driver should support SXGA and VGA. CIF and other formats
are implemented only partially.

> 
>>> Neither it is a symptome
>>> of more severe problems.
>>> 
>>> This does not recover the device from non-working states. So
>>> starting camera-demo, then rmmod ; inmod ; starting camera-demo again
>>> does not work.  
>> 
>> Ok, good research!
>> 
>> Unfortunately it does not reveal anything about the "starting camera-demo again does not work" problem.
>> 
> That is frustrating. If the ISP is in that state it does not make a
> difference whether the iommu patch is there or not.

"state" is a good keyword. I think it is possible to read out the ISP registers
through debugfs. So we could try to compare:
* before first start
* after first start
* after first end
* after second start

I also attended a FOSDEM talk last year about the image capture architecture:

https://archive.fosdem.org/2018/schedule/event/image_capture_on_embedded_linux/

What I do not remember is if I did oscilloscope the clock signals in
these states. I have removed the tiny wires from the GTA04 long time
ago after I could verity that the clocks and polarity settings were
correct. At least initially...

It was always a little risky setup because these open wires may damage
the OMAP3 if shorted with something else (e.g. 3.3 V supply to the camera).
So it was just a temporary lab setup stopping me from testing anything else.

BR,
Nikolaus