On 30/06/2022 03:31, Marek Vasut wrote:

On 6/29/22 15:19, Tomi Valkeinen wrote:

quoted

On 29/06/2022 15:39, Marek Vasut wrote:

quoted

On 6/29/22 14:26, Tomi Valkeinen wrote:

[...]

quoted

quoted

quoted

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

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