lkml.org 
[lkml]   [2021]   [Nov]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v2 1/2] devicetree: bindings: net: Add bindings doc for Sunplus SP7021.
Date
Hi,

> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + emac: emac@9c108000 {
> > + compatible = "sunplus,sp7021-emac";
> > + reg = <0x9c108000 0x400>, <0x9c000280 0x80>;
> > + reg-names = "emac", "moon5";
> > + interrupt-parent = <&intc>;
> > + interrupts = <66 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clkc 0xa7>;
> > + resets = <&rstc 0x97>;
> > + phy-handle1 = <&eth_phy0>;
> > + phy-handle2 = <&eth_phy1>;
> > + pinctrl-0 = <&emac_demo_board_v3_pins>;
> > + pinctrl-names = "default";
> > + nvmem-cells = <&mac_addr0>, <&mac_addr1>;
> > + nvmem-cell-names = "mac_addr0", "mac_addr1";
> > +
> > + mdio {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + eth_phy0: ethernet-phy@0 {
> > + reg = <0>;
> > + phy-mode = "rmii";
>
> This is in the wrong place. It is a MAC property. You usually put it next to phy-handle.

Yes, I'll move phy-mode to Ethernet-port next patch.


> > + };
> > + eth_phy1: ethernet-phy@1 {
> > + reg = <1>;
> > + phy-mode = "rmii";
> > + };
> > + };
>
> I would suggest you structure this differently to make it clear it is a two port switch:
>
> ethernet-ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> port@0 {
> reg = <0>;
> phy-handle = <&eth_phy0>;
> phy-mode = "rmii";
> }
>
> port@1 {
> reg = <1>;
> phy-handle = <&eth_phy1>;
> phy-mode = "rmii";
> }
> }
>
> Andrew

Yes, refer to new example:

examples:
- |
#include <dt-bindings/interrupt-controller/irq.h>
emac: emac@9c108000 {
compatible = "sunplus,sp7021-emac";
reg = <0x9c108000 0x400>, <0x9c000280 0x80>;
reg-names = "emac", "moon5";
interrupt-parent = <&intc>;
interrupts = <66 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clkc 0xa7>;
resets = <&rstc 0x97>;
pinctrl-0 = <&emac_demo_board_v3_pins>;
pinctrl-names = "default";
nvmem-cells = <&mac_addr0>, <&mac_addr1>;
nvmem-cell-names = "mac_addr0", "mac_addr1";

ethernet-ports {
#address-cells = <1>;
#size-cells = <0>;

port@0 {
reg = <0>;
phy-handle = <&eth_phy0>;
phy-mode = "rmii";
};

port@1 {
reg = <1>;
phy-handle = <&eth_phy1>;
phy-mode = "rmii";
};
};

mdio {
#address-cells = <1>;
#size-cells = <0>;

eth_phy0: ethernet-phy@0 {
reg = <0>;
};

eth_phy1: ethernet-phy@1 {
reg = <1>;
};
};
};

Is it correct?

Thank you very much for your review.

\
 
 \ /
  Last update: 2021-11-12 03:51    [W:1.398 / U:0.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site