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

[Marek Vasut <marex@denx.de>]

On 6/29/22 15:19, Tomi Valkeinen wrote:
> On 29/06/2022 15:39, Marek Vasut wrote:
>> On 6/29/22 14:26, Tomi Valkeinen wrote:
>>
>> [...]
>>
>>>>> Perhaps the best way to solve this is just to remove the underscores
>>>>> from __v4l2_subdev_state_alloc, and change all the drivers which 
>>>>> create
>>>>> temporary v4l2_subdev_states to use that (and the free) functions. And
>>>>> also create the helper macro which can be used in those cases where 
>>>>> the
>>>>> call is simple (the state is not modified or accessed by the caller).
>>>>
>>>> As long as we prevent any new driver from using that API, that's fine
>>>> with me.
>>>
>>> An alternative would be to keep the v4l2_subdev_state as a local 
>>> variable (allocated in the stack), and add a new function, 
>>> v4l2_subdev_state_local_init() or such. The function would initialize 
>>> the given state, expecting the allocatable fields to be already 
>>> allocated (state->pads, which in the above cases points to another 
>>> local variable, i.e. stack).
>>>
>>> This would prevent the need of a free call, which, while not complex 
>>> as such, might cause a bigger amount of changes in some cases to 
>>> handle the error paths correctly.
>>>
>>> Of course, if the above-mentioned macro works, then that's the 
>>> easiest solution. But that won't work for all drivers.
>>
>> Don't you think a driver fix shouldn't involve "rework the subsystem" 
>> requirement to be applicable ?
> 
> No, but we should think what's the best way to do the fix, if the fix
> is controversial. Otherwise we might just break things even worse.
> Adding the macro seems like a much better way, and far from "rework the
> subsystem". Granted, this was just a quick edit without testing so it may
> fail miserably...
> 
> Can you try this out?

It seems to work as well. How shall we proceed ?