Re: [PATCH v15 07/19] media: subdev: add v4l2_subdev_set_routing helper()

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

On 12/10/2022 09:22, Yunke Cao wrote:
> Hi Tomi,
> 
> On Wed, Oct 12, 2022 at 2:03 PM Tomi Valkeinen
> <tomi.valkeinen@ideasonboard.com> wrote:
>>
>> Add a helper function to set the subdev routing. The helper can be used
>> from subdev driver's set_routing op to store the routing table.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>>   drivers/media/v4l2-core/v4l2-subdev.c | 31 +++++++++++++++++++++++++++
>>   include/media/v4l2-subdev.h           | 16 ++++++++++++++
>>   2 files changed, 47 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
>> index fff17b8536fc..3ae4f39a50e4 100644
>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
>> @@ -12,6 +12,7 @@
>>   #include <linux/ioctl.h>
>>   #include <linux/mm.h>
>>   #include <linux/module.h>
>> +#include <linux/overflow.h>
>>   #include <linux/slab.h>
>>   #include <linux/types.h>
>>   #include <linux/version.h>
>> @@ -1181,6 +1182,36 @@ int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
>>   }
>>   EXPORT_SYMBOL_GPL(v4l2_subdev_get_fmt);
>>
>> +int v4l2_subdev_set_routing(struct v4l2_subdev *sd,
>> +                           struct v4l2_subdev_state *state,
>> +                           const struct v4l2_subdev_krouting *routing)
>> +{
>> +       struct v4l2_subdev_krouting *dst = &state->routing;
>> +       const struct v4l2_subdev_krouting *src = routing;
>> +       struct v4l2_subdev_krouting new_routing = { 0 };
>> +       size_t bytes;
>> +
>> +       if (unlikely(check_mul_overflow(src->num_routes, sizeof(*src->routes),
> 
> Do we need to cast (size_t)src->num_routes here?
> My compiler is complaining:
> ./include/linux/overflow.h:85:22: error: comparison of distinct
> pointer types lacks a cast [-Werror]
>     85 |         (void) (&__a == &__b);                  \
> 

Yes, I think we should do that. Thanks! I need to remember to compile 
with other compilers than arm32 too =).

  Tomi