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

[Tomi Valkeinen]

On 30/06/2022 03:31, Marek Vasut wrote:
> 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 ?

I haven't had to look at this more closely, but I made proper patches of 
this and sent them for review. Note that they're not exactly the same as 
the diff here: the macro was missing lock and unlock for the subdev 
state, which I've added. Again, only compile tested.

  Tomi