Re: [PATCH 2/6] media: subdev: add active state to struct v4l2_subdev

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

On 17/12/2021 14:07, Jacopo Mondi wrote:
> Hi Tomi,
> 
> On Fri, Dec 17, 2021 at 01:18:32PM +0200, Tomi Valkeinen wrote:
>> Add a new 'active_state' field to struct v4l2_subdev to which we can
>> store the active state of a subdev. This will place the subdev
>> configuration into a known place, allowing us to use the state directly
>> from the v4l2 framework, thus simplifying the drivers.
>>
>> Also add functions v4l2_subdev_init_finalize() and
>> v4l2_subdev_cleanup(), which will allocate and free the active state.
>> The functions are named in a generic way so that they can be also used
>> for other subdev initialization work.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>>   drivers/media/v4l2-core/v4l2-subdev.c | 21 ++++++++++
>>   include/media/v4l2-subdev.h           | 58 +++++++++++++++++++++++++++
>>   2 files changed, 79 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
>> index fe49c86a9b02..de160140d63b 100644
>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
>> @@ -943,3 +943,24 @@ void v4l2_subdev_notify_event(struct v4l2_subdev *sd,
>>   	v4l2_subdev_notify(sd, V4L2_DEVICE_NOTIFY_EVENT, (void *)ev);
>>   }
>>   EXPORT_SYMBOL_GPL(v4l2_subdev_notify_event);
>> +
>> +int v4l2_subdev_init_finalize(struct v4l2_subdev *sd)
>> +{
>> +	struct v4l2_subdev_state *state;
>> +
>> +	state = __v4l2_subdev_state_alloc(sd);
>> +	if (IS_ERR(state))
>> +		return PTR_ERR(state);
>> +
>> +	sd->active_state = state;
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_subdev_init_finalize);
>> +
>> +void v4l2_subdev_cleanup(struct v4l2_subdev *sd)
>> +{
>> +	__v4l2_subdev_state_free(sd->active_state);
>> +	sd->active_state = NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_subdev_cleanup);
>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>> index e52bf508c75b..eddf72768e10 100644
>> --- a/include/media/v4l2-subdev.h
>> +++ b/include/media/v4l2-subdev.h
>> @@ -645,6 +645,9 @@ struct v4l2_subdev_ir_ops {
>>    * This structure only needs to be passed to the pad op if the 'which' field
>>    * of the main argument is set to %V4L2_SUBDEV_FORMAT_TRY. For
>>    * %V4L2_SUBDEV_FORMAT_ACTIVE it is safe to pass %NULL.
>> + *
>> + * Note: This struct is also used in active state, and the try_ prefix is
>> + * historical and to be removed.
>>    */
>>   struct v4l2_subdev_pad_config {
>>   	struct v4l2_mbus_framefmt try_fmt;
>> @@ -898,6 +901,9 @@ struct v4l2_subdev_platform_data {
>>    * @subdev_notifier: A sub-device notifier implicitly registered for the sub-
>>    *		     device using v4l2_async_register_subdev_sensor().
>>    * @pdata: common part of subdevice platform data
>> + * @active_state: Active state for the subdev (NULL for subdevs tracking the
>> + *		  state internally). Initialized by calling
>> + *		  v4l2_subdev_init_finalize().
> 
> Is alignement a little off or is it my mail client ?

Looks correct to me. It's using tabs there. I don't like, I think it 
should be spaces, but I followed the convention used elsewhere.

  Tomi