Re: [PATCH 2/4] media: i2c: replace of_graph_get_next_endpoint()

Kuninori Morimoto
2024-02-07T01:06:07Z

Hi Laurent, Hans

> >> From DT point of view, in general, drivers should be asking for a
> >> specific port number because their function is fixed in the binding.
> >>
> >> of_graph_get_next_endpoint() doesn't match to this concept.
> >>
> >> Simply replace
> >>
> >> - of_graph_get_next_endpoint(xxx, NULL);
> >> + of_graph_get_endpoint_by_regs(xxx, 0, -1);
(snip)
> >> /* Parse the endpoint. */
> >> - endpoint = of_graph_get_next_endpoint(np, NULL);
> >> + endpoint = of_graph_get_endpoint_by_regs(np, 0, -1);
> >
> > I think this should be port 1 for the adv7611 and port2 for the adv7612.
> > The adv7610 may need to use port 1 too, but the bindings likely need to
> > be updated.
> >
> > Hans, Krzysztof, any opinion ?
>
> It looks like it. But I suspect the code never worked. The endpoint parsing
> is only needed if a specific mbus type is used (i.e., not 'UNKNOWN'), and
> I don't think that is used in the device trees in the kernel. So everything
> silently falls back to UNKNOWN and some default bus config that 'just works' (tm).
>
> I'm pretty sure this code is wrong, but nobody ever noticed. Changing it
> to the new code just makes it bug-compatible :-)

Nice ;)
So, let's add /* FIXME */ here in v2

Thank you for your help !!

Best regards
---
Renesas Electronics
Ph.D. Kuninori Morimoto