On Fri, Apr 07, 2023 at 04:53:52PM +1000, David Gwynne wrote:
> ethernet interfaces in device trees can have a "label" property which
> is generally used (when it is used) to identify which connector it is on
> the case or something like that. eg, eth2 in the turris omnia device
> tree has 'label = "wan"' on the mvneta interface.
>
> ive been using labels in the dts recently to help me figure out what
> goes where, so this has been useful to me. if/when we support switches
> (eg mvsw), their ports are often labelled so i'd like to apply this idea
> to them too.
>
> ok?
I think doing this is OK but I'm not sure if the usage of OF_getprop()
is correct. OF_getprop() uses 'memcpy(buf, data, min(len, buflen));'
to fill the if_description buffer. So if the provided label is larger than
sizeof(ifp->if_description) the description string is no longer NUL
terminated.
> Index: if_dwqe_fdt.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/fdt/if_dwqe_fdt.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 if_dwqe_fdt.c
> --- if_dwqe_fdt.c 7 Apr 2023 06:33:49 -0000 1.7
> +++ if_dwqe_fdt.c 7 Apr 2023 06:47:11 -0000
> @@ -213,6 +213,10 @@ dwqe_fdt_attach(struct device *parent, s
> printf("%s: can't establish interrupt\n", sc->sc_dev.dv_xname);
>
> ifp = &sc->sc_ac.ac_if;
> +
> + OF_getprop(faa->fa_node, "label",
> + ifp->if_description, sizeof(ifp->if_description));
> +
> sc->sc_ifd.if_node = faa->fa_node;
> sc->sc_ifd.if_ifp = ifp;
> if_register(&sc->sc_ifd);
> Index: if_mvneta.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/fdt/if_mvneta.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 if_mvneta.c
> --- if_mvneta.c 3 Apr 2023 01:46:18 -0000 1.29
> +++ if_mvneta.c 7 Apr 2023 06:47:12 -0000
> @@ -810,6 +810,9 @@ mvneta_attach_deferred(struct device *se
> if_attach(ifp);
> ether_ifattach(ifp);
>
> + OF_getprop(sc->sc_node, "label",
> + ifp->if_description, sizeof(ifp->if_description));
> +
> sc->sc_ifd.if_node = sc->sc_node;
> sc->sc_ifd.if_ifp = ifp;
> if_register(&sc->sc_ifd);
>
--
:wq Claudio
