Hi Sakari,
On 20-03-11 10:15, Sakari Ailus wrote:
> Hi Marco,
>
> On Mon, Mar 09, 2020 at 11:14:13AM +0100, Marco Felsch wrote:
> > A link is between two endpoints not between two ports to be more
> > precise. Add the local_id/remote_id field which stores the endpoint
> > reg/port property to the link. Now the link holds all necessary
> > information about a link.
> >
> > Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx>
> > ---
> >
> > v12:
> > - New in this serie
> >
> > drivers/media/v4l2-core/v4l2-fwnode.c | 2 ++
> > include/media/v4l2-fwnode.h | 4 ++++
> > 2 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index 6ece4320e1d2..78c32aebbe03 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -565,6 +565,7 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *__fwnode,
> >
> > memset(link, 0, sizeof(*link));
> >
> > + fwnode_property_read_u32(__fwnode, "reg", &link->local_id);
> > fwnode = fwnode_get_parent(__fwnode);
> > fwnode_property_read_u32(fwnode, port_prop, &link->local_port);
> > fwnode = fwnode_get_next_parent(fwnode);
> > @@ -578,6 +579,7 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *__fwnode,
> > return -ENOLINK;
> > }
> >
> > + fwnode_property_read_u32(fwnode, "reg", &link->remote_id);
>
> This code really should use fwnode_graph_parse_endpoint(), and not
> implement the parsing locally.
>
> I do think the end result would be better if the code using this function
> would be calling the fwnode graph APIs directly.
I think doing it manually was used due to the
"port" (ACPI) <-> "reg" (DT) property difference. I'm completely with
you to change it to fwnode_graph_parse_endpoint() since ACPI also uses
"reg" as port numbering (as you explained it to me).
Regards,
Marco
> > fwnode = fwnode_get_parent(fwnode);
> > fwnode_property_read_u32(fwnode, port_prop, &link->remote_port);
> > fwnode = fwnode_get_next_parent(fwnode);
> > diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> > index f6a7bcd13197..7bba6dfa3fd6 100644
> > --- a/include/media/v4l2-fwnode.h
> > +++ b/include/media/v4l2-fwnode.h
> > @@ -113,14 +113,18 @@ struct v4l2_fwnode_endpoint {
> > * struct v4l2_fwnode_link - a link between two endpoints
> > * @local_node: pointer to device_node of this endpoint
> > * @local_port: identifier of the port this endpoint belongs to
> > + * @local_id: identifier of the id this endpoint belongs to
> > * @remote_node: pointer to device_node of the remote endpoint
> > * @remote_port: identifier of the port the remote endpoint belongs to
> > + * @remote_id: identifier of the id the remote endpoint belongs to
> > */
> > struct v4l2_fwnode_link {
> > struct fwnode_handle *local_node;
> > unsigned int local_port;
> > + unsigned int local_id;
> > struct fwnode_handle *remote_node;
> > unsigned int remote_port;
> > + unsigned int remote_id;
> > };
> >
> > /**
>
> --
> Sakari Ailus
>
