Re: [PATCH 3/6] media: rcar-vin: Generate a VIN group ID for Gen2

["Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>]

Hi Geert

Thanks for your comments.

On 2025-02-27 14:46:24 +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Mon, 24 Feb 2025 at 19:40, Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > Prepare to move Gen2 and earlier models to media controller by
> > generating a unique VIN group id for each VIN instance. On Gen3 and Gen4
> > it is important to have a specific id in the group as media graph routes
> > depend on this. On Gen2 and earlier models all that will matter is to
> > have a unique id in the range.
> >
> > Break out the id generation to a own function keeping the logic for Gen3
> > and Gen4 while generating a sequential id for Gen2 models.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> Thanks for your patch!
> 
> > --- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> > @@ -114,23 +114,41 @@ static void rvin_group_release(struct kref *kref)
> >         mutex_unlock(&rvin_group_lock);
> >  }
> >
> > +static int rvin_group_get_id(struct rvin_dev *vin)
> > +{
> > +       struct device_node *np;
> > +       unsigned int count;
> > +       u32 id;
> > +
> > +       switch (vin->info->model) {
> > +       case RCAR_GEN3:
> > +               if (!of_property_read_u32(vin->dev->of_node, "renesas,id", &id))
> > +                       return id;
> > +               break;
> 
> Please insert a blank line here.
> 
> > +       default:
> > +               count = 0;
> > +               for_each_matching_node(np, vin->dev->driver->of_match_table) {
> 
> This is a rather expensive operation.
> What about calling ida_alloc() instead?

That is a good idea, did not know about this. I opted to use 
ida_alloc_range() to make sure the ID is in supported range. Thanks for 
the tip.


> And probably moving the code to obtain the ID to rcar_vin_probe()?

Good idea!

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Kind Regards,
Niklas Söderlund