Hi Maxime,
On Wed, Nov 18, 2020 at 06:48:05PM +0100, Maxime Ripard wrote:
> On Mon, Oct 12, 2020 at 02:00:30AM +0300, Laurent Pinchart wrote:
> > > -static int drm_of_lvds_get_remote_pixels_type(
> > > - const struct device_node *port_node)
> > > +static int drm_of_lvds_get_remote_pixels_type(const struct device_node *endpoint)
> > > {
> > > - struct device_node *endpoint = NULL;
> > > - int pixels_type = -EPIPE;
> > > + struct device_node *remote_port;
> > > + int pixels_type;
> > >
> > > - for_each_child_of_node(port_node, endpoint) {
> > > - struct device_node *remote_port;
> > > - int current_pt;
> > > -
> > > - if (!of_node_name_eq(endpoint, "endpoint"))
> > > - continue;
> > > -
> > > - remote_port = of_graph_get_remote_port(endpoint);
> > > - if (!remote_port) {
> > > - of_node_put(remote_port);
> > > - return -EPIPE;
> > > - }
> > > -
> > > - current_pt = drm_of_lvds_get_port_pixels_type(remote_port);
> > > + remote_port = of_graph_get_remote_port(endpoint);
> > > + if (!remote_port) {
> > > of_node_put(remote_port);
> >
> > You can drop this line.
> >
> > > - if (pixels_type < 0)
> > > - pixels_type = current_pt;
> > > -
> > > - /*
> > > - * Sanity check, ensure that all remote endpoints have the same
> > > - * pixel type. We may lift this restriction later if we need to
> > > - * support multiple sinks with different dual-link
> > > - * configurations by passing the endpoints explicitly to
> > > - * drm_of_lvds_get_dual_link_pixel_order().
> > > - */
> >
> > Shouldn't we keep this check when endpoint_id is -1 in
> > drm_of_lvds_get_dual_link_pixel_order() ?
>
> I tried to do that, and I'm not quite really sure how to go around it.
>
> This scans all the endpoints in a given port.
>
> However, now that we have the device, port id and endpoint id, we need
> to use of_graph_get_port_by_id to get the port matching the device and
> port id, and iterate over all its endpoint as done here.
>
> The trouble is that of_graph_get_port_by_id expects a !const
> device_node, yet drm_of_lvds_get_dual_link_pixel_order (and seems to be
> doing so rightfully), so that creates a warning because we drop the
> const there.
of_graph_get_port_by_id() doesn't seem to modify the device_node passed
to it, couldn't it be modified to take a const pointer ?
> Changing the prototype to passing only the port device_node doesn't
> really work either, since it would be const, and we would need to call
> of_graph_get_endpoint_by_regs (so having the parent device_node, through
> of_graph_get_port_parent) and of_graph_get_port_parent takes a !const
> port device_node.
>
> I guess we could drop const entirely from our function, but that doesn't
> look right either..
--
Regards,
Laurent Pinchart
