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@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxx
>
> 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
