Re: [PATCH] checks: relax graph checks for overlays

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



Hey dt and dtc people :-) ,

Am Dienstag, 4. Juni 2024, 16:48:46 CEST schrieb Michael Riesch:
> This is a gentle ping with David and the DT maintainers in Cc: (I am not
> sure what the protocol is here, so I'll simply give this a try).

it would be really great if someone could look at the patch and tell us
if that is the right direction to go, or something else needs to be done
instead.

Thanks a lot
Heiko

> On 5/22/24 16:32, Michael Riesch wrote:
> > In device tree overlays, the following patterns occur frequently:
> > 
> > board.dts:
> > /dts-v1/;
> > 
> > / {
> > 	display-controller {
> > 		ports {
> > 			#address-cells = <1>;
> > 			#size-cells = <0>;
> > 
> > 			vp0: port@0 {
> > 				reg = <0>;
> > 
> > 				vp0_out: endpoint {
> > 				};
> > 			};
> > 
> > 			vp1: port@1 {
> > 				reg = <1>;
> > 			};
> > 		};
> > 	};
> > };
> > 
> > overlay-endpoint.dtso:
> > /dts-v1/;
> > /plugin/;
> > 
> > &{/} {
> > 	hdmi-tx-connector {
> > 		port {
> > 			hdmi_tx_in: endpoint {
> > 				remote-endpoint = <&vp0_out>;
> > 			};
> > 		};
> > 	};
> > };
> > 
> > &vp0_out {
> > 	remote-endpoint = <&hdmi_tx_in>;
> > };
> > 
> > In this case, dtc expects that the node referenced by &vp0_out is
> > named "endpoint", but the name cannot be inferred. Also, dtc
> > complains about the connections between the endpoints not being
> > bidirectional.
> > 
> > Similarly, for a different overlay overlay-port.dtso:
> > /dts-v1/;
> > /plugin/;
> > 
> > &{/} {
> > 	panel {
> > 		port {
> > 			panel_in: endpoint {
> > 				remote-endpoint = <&vp1_out>;
> > 			};
> > 		};
> > 	};
> > };
> > 
> > &vp1 {
> > 	vp1_out: endpoint {
> > 		remote-endpoint = <&panel_in>;
> > 	};
> > };
> > 
> > dtc expects that the node referenced by &vp1 is named "port", but the
> > name cannot be inferred.
> > 
> > Relax the corresponding checks and skip the parts that are not reasonable
> > for device tree overlays.
> > 
> > Cc: Heiko Stuebner <heiko@xxxxxxxxx>
> > Signed-off-by: Michael Riesch <michael.riesch@xxxxxxxxxxxxxx>
> > ---
> > Habidere,
> > 
> > This patch fixes several warnings 
> >  - Warning (graph_port): /fragment@3: graph port node name should be
> >    'port'
> >  - Warning (graph_endpoint): /fragment@3/__overlay__: graph endpoint
> >    node name should be 'endpoint'
> >  - Warning (graph_endpoint): /fragment@3/__overlay__: graph connection
> >    to node '/fragment@2/__overlay__/.../port/endpoint' is not
> >    bidirectional
> > that appear when compiling device tree overlays.
> > 
> > This first warning popped up e.g., in the scope of [1], [2]. The latter
> > two warnings appear regularly when compiling our downstream overlays. As
> > we plan to submit them to mainline soon, we'll hit that blocker sooner or
> > later.
> > 
> > Looking forward to your comments!
> > 
> > Link: https://lore.kernel.org/lkml/20240412-feature-wolfvision-pf5-display-v1-1-f032f32dba1a@xxxxxxxxxxxxxx/ [1]
> > Link: https://lore.kernel.org/lkml/20240423082941.2626102-1-heiko@xxxxxxxxx/T/ [2]
> 
> Feedback on this issue would be most welcome!
> 
> Thanks a lot and best regards,
> Michael
> 
> > ---
> >  checks.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/checks.c b/checks.c
> > index 2fb7ee5..2ea5913 100644
> > --- a/checks.c
> > +++ b/checks.c
> > @@ -1822,10 +1822,14 @@ static void check_graph_port(struct check *c, struct dt_info *dti,
> >  	if (node->bus != &graph_port_bus)
> >  		return;
> >  
> > +	check_graph_reg(c, dti, node);
> > +
> > +	/* skip checks below for overlays */
> > +	if (dti->dtsflags & DTSF_PLUGIN)
> > +		return;
> > +
> >  	if (!strprefixeq(node->name, node->basenamelen, "port"))
> >  		FAIL(c, dti, node, "graph port node name should be 'port'");
> > -
> > -	check_graph_reg(c, dti, node);
> >  }
> >  WARNING(graph_port, check_graph_port, NULL, &graph_nodes);
> >  
> > @@ -1860,11 +1864,15 @@ static void check_graph_endpoint(struct check *c, struct dt_info *dti,
> >  	if (!node->parent || node->parent->bus != &graph_port_bus)
> >  		return;
> >  
> > +	check_graph_reg(c, dti, node);
> > +
> > +	/* skip checks below for overlays */
> > +	if (dti->dtsflags & DTSF_PLUGIN)
> > +		return;
> > +
> >  	if (!strprefixeq(node->name, node->basenamelen, "endpoint"))
> >  		FAIL(c, dti, node, "graph endpoint node name should be 'endpoint'");
> >  
> > -	check_graph_reg(c, dti, node);
> > -
> >  	remote_node = get_remote_endpoint(c, dti, node);
> >  	if (!remote_node)
> >  		return;
> > 
> > ---
> > base-commit: ae26223a056e040b2d812202283d47c6e034d063
> > change-id: 20240522-bugfix-relax-checks-for-overlays-e72b11df44a1
> > 
> > Best regards,
> 








[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux