Re: [PATCH v2 net-next/devicetree 4/5] arm64: dts: fsl: ls1028a: add node for Felix switch

[Michael Walle]

Hi Vladimir,

Am 2020-02-22 13:25, schrieb Vladimir Oltean:
> Hi Michael,
>
> On Sat, 22 Feb 2020 at 13:38, Michael Walle <michael@xxxxxxxx> wrote:
> > Hi,
>
> > > +
> > > +                     enetc_port2: ethernet@0,2 {
> > > +                             compatible = "fsl,enetc";
> > > +                             reg = <0x000200 0 0 0 0>;
> > > +                             phy-mode = "gmii";
> > Can we disable this port by default in this dtsi? As mentioned in the 
> > other
> > mail, I'd prefer to have all ports disabled because it doesn't make 
> > sense
> > to have this port while having all the external ports disabled.
>
> Ok. What would you want to happen with the "ethernet" property? Do you
> want the board dts to set that too?

That's something I've also thought about. And now that you've mention
this, I think it makes more sense to have that in the board too. Because
if you have the freedom to use either eno2/swp4 or eno3/swp5, then if I
choose the second one I'd have to delete the ethernet property from the
first, correct? I actually thought about adding the ethernet property
to both; but (1) I don't know if that is even possible (given that one
is always disabled) and (2) if one want to use the second port as an
additional link to the switch you'd have to remove the ethernet property
on that port. correct?


> > > +                                     /* Internal port with DSA tagging */
> > > +                                     mscc_felix_port4: port@4 {
> > > +                                             reg = <4>;
> > > +                                             phy-mode = "internal";
> > > +                                             ethernet = <&enetc_port2>;
> > Likewise, I'd prefer to have this disabled.
>
> Ok.
>
> > > +                     enetc_port3: ethernet@0,6 {
> > > +                             compatible = "fsl,enetc";
> > > +                             reg = <0x000600 0 0 0 0>;
> > > +                             status = "disabled";
> > > +                             phy-mode = "gmii";
> > shouldn't the status be after the phy-mode property?
>
> Why?

I thought that would be a rule. I just had a quick look on some other 
device
trees before and they all has the status property as the last property 
(before
any subnodes). I might be mistaken. If so, you could do it for 
consistency
reasons ;) all status property in the ls1028a.dtsi are the last ones.

-michael