Re: [PATCH] media: stm32: dcmi: Switch to __v4l2_subdev_state_alloc()

From: Hugues FRUCHET <hugues.fruchet@foss.st.com>
To: Marek Vasut <marex@denx.de>,
	Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: <linux-media@vger.kernel.org>,
	Alain Volmat <alain.volmat@foss.st.com>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Amelie DELAUNAY <amelie.delaunay@foss.st.com>,
	Philippe CORNU <philippe.cornu@foss.st.com>,
	<linux-stm32@st-md-mailman.stormreply.com>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] media: stm32: dcmi: Switch to __v4l2_subdev_state_alloc()
Date: Mon, 27 Jun 2022 11:14:07 +0200	[thread overview]
Message-ID: <5ee6c0c0-8ab0-561c-e1f6-b26e4ec438af@foss.st.com> (raw)
In-Reply-To: <4d5b5c59-f3d5-ad5a-ae61-73277b4adefa@denx.de>

Hi Marek,

On 6/20/22 16:06, Marek Vasut wrote:
> On 6/20/22 11:44, Tomi Valkeinen wrote:
>> Hi,
> 
> Hello all,
> 
>>> On Sun, Jun 19, 2022 at 12:24:42AM +0200, Marek Vasut wrote:
>>>> Any local subdev state should be allocated and free'd using
>>>> __v4l2_subdev_state_alloc()/__v4l2_subdev_state_free(), which
>>>> takes care of calling .init_cfg() subdev op. Without this,
>>>> subdev internal state might be uninitialized by the time
>>>> any other subdev op is called.
>>
>> Does this fix a bug you have?
> 
> Yes

Which bug did you encounter exactly ?
This is strange that we have not yet encounter any problems around that 
through our tests campaigns... or we have to enforce with a new test, so 
better to know what your problem was exactly.

> 
>> Wasn't this broken even before the active state, as init_cfg was not 
>> called?
> 
> Yes, this was always broken. I suspect nobody tested this mode of 
> operation before. In my case, I have this DCMI driver connected directly 
> to MT9P006 sensor.

As far as I see, MT9P006 sensor is a 12 bits parallel interface sensor.
I don't see the difference with our OV5640 used in parallel mode which 
is a mainline config on our side, so one more time I wonder what the 
problem was.

> 
>> In any case, I think we have to do something like this, as the source 
>> subdev might depend on a valid subdev state.
> 
> Right.
> 
> [...]

BR,
Hugues.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel