| Message ID | 1395132017-15928-2-git-send-email-zhangfei.gao@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Mar 18, 2014 at 08:40:15AM +0000, Zhangfei Gao wrote: > This patch adds the Device Tree bindings for the Hisilicon hip04 > Ethernet controller, including 100M / 1000M controller. > > Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> > --- > .../bindings/net/hisilicon-hip04-net.txt | 74 ++++++++++++++++++++ > 1 file changed, 74 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt > > diff --git a/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt b/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt > new file mode 100644 > index 0000000..c918f08 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt > @@ -0,0 +1,74 @@ > +Hisilicon hip04 Ethernet Controller > + > +* Ethernet controller node > + > +Required properties: > +- compatible: should be "hisilicon,hip04-mac". > +- reg: address and length of the register set for the device. > +- interrupts: interrupt for the device. > +- port: ppe port number connected to the controller: range from 0 to 31. ppe? Will there ever be more than one ppe? If so, describing the linkage to the ppe with a phandle + args approach is preferable. > +- speed: 100 (100M) or 1000 (1000M). Can you not query this from the hardware? > +- id: should be different and fe should be 0. This description is useless. What is this for, and why does this need to be in the dt? What is "fe"? > + > +Optional Properties: > +- phy-handle : the phandle to a PHY node > + > + > +* Ethernet ppe node: control rx & tx fifos of all ethernet controllers > + > +Required properties: > +- compatible: should be "hisilicon,hip04-ppebase". Why "ppebase" rather than "ppe"? > +- reg: address and length of the register set for the node. s/node/device/ Cheers, Mark. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 18-03-2014 12:40, Zhangfei Gao wrote: > This patch adds the Device Tree bindings for the Hisilicon hip04 > Ethernet controller, including 100M / 1000M controller. > Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> > --- > .../bindings/net/hisilicon-hip04-net.txt | 74 ++++++++++++++++++++ > 1 file changed, 74 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt > diff --git a/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt b/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt > new file mode 100644 > index 0000000..c918f08 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt > @@ -0,0 +1,74 @@ > +Hisilicon hip04 Ethernet Controller > + > +* Ethernet controller node > + > +Required properties: > +- compatible: should be "hisilicon,hip04-mac". > +- reg: address and length of the register set for the device. > +- interrupts: interrupt for the device. > +- port: ppe port number connected to the controller: range from 0 to 31. > +- speed: 100 (100M) or 1000 (1000M). There's standard "max-speed" property for that, see Documentation/devicetree/bindings/net/ethernet.txt in the 'net-next.git' repo. > +Optional Properties: > +- phy-handle : the phandle to a PHY node Please refer instead to the above mentioned file for this standard property -- it is already described there. See other binding files as the example. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2014-03-18 1:40 GMT-07:00 Zhangfei Gao <zhangfei.gao@linaro.org>: > This patch adds the Device Tree bindings for the Hisilicon hip04 > Ethernet controller, including 100M / 1000M controller. > > Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> > --- > .../bindings/net/hisilicon-hip04-net.txt | 74 ++++++++++++++++++++ > 1 file changed, 74 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt > > diff --git a/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt b/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt > new file mode 100644 > index 0000000..c918f08 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt > @@ -0,0 +1,74 @@ > +Hisilicon hip04 Ethernet Controller > + > +* Ethernet controller node > + > +Required properties: > +- compatible: should be "hisilicon,hip04-mac". > +- reg: address and length of the register set for the device. > +- interrupts: interrupt for the device. > +- port: ppe port number connected to the controller: range from 0 to 31. > +- speed: 100 (100M) or 1000 (1000M). > +- id: should be different and fe should be 0. > + > +Optional Properties: > +- phy-handle : the phandle to a PHY node > + > + > +* Ethernet ppe node: control rx & tx fifos of all ethernet controllers > + > +Required properties: > +- compatible: should be "hisilicon,hip04-ppebase". > +- reg: address and length of the register set for the node. > + > + > +* MDIO bus node: > + > +Required properties: > + > +- compatible: "hisilicon,hip04-mdio" > +- Inherets from MDIO bus node binding[1] > +[1] Documentation/devicetree/bindings/net/phy.txt > + > +Example: > + mdio { > + compatible = "hisilicon,hip04-mdio"; > + reg = <0x28f1000 0x1000>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + phy0: ethernet-phy@0 { > + reg = <0>; > + marvell,reg-init = <18 0x14 0 0x8001>; > + device_type = "ethernet-phy"; You are missing a compatible string such as "ethernet-phy-ieee802.3-c22", please take a look at Documentation/devicetree/bindings/net/phy.txt for an example. device_type is deprecated and should be removed. > + }; > + > + phy1: ethernet-phy@1 { > + reg = <1>; > + marvell,reg-init = <18 0x14 0 0x8001>; > + device_type = "ethernet-phy"; > + }; > + }; > + > + ppebase: ppebase@28c0000 { > + compatible = "hisilicon,hip04-ppebase"; > + reg = <0x28c0000 0x10000>; This should probably look like: #address-cells = <0>; #size-cells = <0>; eth0_port: port@0 { reg = <0>; }; eth1_port: port@1f { reg = <31>; }; This looks like something similar to mv643xx_eth, you should see Documentation/devicetree/bindings/marvell.txt for hints on how to model the representation in a similar fashion. > + }; > + > + fe: ethernet@28b0000 { > + compatible = "hisilicon,hip04-mac"; > + reg = <0x28b0000 0x10000>; > + interrupts = <0 413 4>; > + port = <31>; I do not think this is the right way to expose that, port should be specialized to e.g: hisilicon,port, or you should use a phandle to the "ppebase" node which exposes differents ports as subnodes: hisilicon,port-handle = <ð0_port>; > + speed = <100>; max-speed is the standard property for this > + id = <0>; id here is a software concept, either you create properly numbered aliases for these nodes, and use of_alias_get_id(), or you do not use these identifiers at all. > + }; > + > + ge0: ethernet@2800000 { > + compatible = "hisilicon,hip04-mac"; > + reg = <0x2800000 0x10000>; > + interrupts = <0 402 4>; > + port = <0>; > + speed = <1000>; > + id = <1>; > + phy-handle = <&phy0>; > + }; > -- > 1.7.9.5 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Dear Florian On Wed, Mar 19, 2014 at 1:39 AM, Florian Fainelli <f.fainelli@gmail.com> wrote: > 2014-03-18 1:40 GMT-07:00 Zhangfei Gao <zhangfei.gao@linaro.org>: >> +Example: >> + mdio { >> + compatible = "hisilicon,hip04-mdio"; >> + reg = <0x28f1000 0x1000>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + phy0: ethernet-phy@0 { >> + reg = <0>; >> + marvell,reg-init = <18 0x14 0 0x8001>; >> + device_type = "ethernet-phy"; > > You are missing a compatible string such as > "ethernet-phy-ieee802.3-c22", please take a look at > Documentation/devicetree/bindings/net/phy.txt for an example. > > device_type is deprecated and should be removed. Thanks for the info, will update. > >> + }; >> + >> + phy1: ethernet-phy@1 { >> + reg = <1>; >> + marvell,reg-init = <18 0x14 0 0x8001>; >> + device_type = "ethernet-phy"; >> + }; >> + }; >> + >> + ppebase: ppebase@28c0000 { >> + compatible = "hisilicon,hip04-ppebase"; >> + reg = <0x28c0000 0x10000>; > > This should probably look like: > > #address-cells = <0>; > #size-cells = <0>; > > eth0_port: port@0 { > reg = <0>; > }; > > eth1_port: port@1f { > reg = <31>; > }; > > This looks like something similar to mv643xx_eth, you should see > Documentation/devicetree/bindings/marvell.txt for hints on how to > model the representation in a similar fashion. Perfect, this looks more professional, just like the phy description. The ppe is common device with 2048 channels shared by all the controllers, only if channels are not overlapped. Two inputs required, One is port number, currently use reg=<>, The other is start channel, I used id before. Each controller use start channel as RX_DESC_NUM * priv->id. Do you think still use id from of_alias_get_id(), or add another property like start_chan etc. eth0_port: port@1f { reg = <31>; start-chan = <0>; }; eth1_port: port@0 { reg = <0>; start-chan = <1>; }; > >> + }; >> + >> + fe: ethernet@28b0000 { >> + compatible = "hisilicon,hip04-mac"; >> + reg = <0x28b0000 0x10000>; >> + interrupts = <0 413 4>; >> + port = <31>; > > I do not think this is the right way to expose that, port should be > specialized to e.g: hisilicon,port, or you should use a phandle to the > "ppebase" node which exposes differents ports as subnodes: > > hisilicon,port-handle = <ð0_port>; OK, perfect. > >> + speed = <100>; > > max-speed is the standard property for this The speed can be removed now, and the info can be get from phy-mode = "sgmii" > >> + id = <0>; > > id here is a software concept, either you create properly numbered > aliases for these nodes, and use of_alias_get_id(), or you do not use > these identifiers at all. Still not not sure whether use of_alias_get_id() or add property in the eth_port subnode. The id is used for specify the start channel. Thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Mark Thanks for the suggestion, will update accordingly. On Tue, Mar 18, 2014 at 8:34 PM, Mark Rutland <mark.rutland@arm.com> wrote: > On Tue, Mar 18, 2014 at 08:40:15AM +0000, Zhangfei Gao wrote: >> This patch adds the Device Tree bindings for the Hisilicon hip04 >> Ethernet controller, including 100M / 1000M controller. >> >> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> >> --- >> .../bindings/net/hisilicon-hip04-net.txt | 74 ++++++++++++++++++++ >> 1 file changed, 74 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt >> >> diff --git a/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt b/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt >> new file mode 100644 >> index 0000000..c918f08 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt >> @@ -0,0 +1,74 @@ >> +Hisilicon hip04 Ethernet Controller >> + >> +* Ethernet controller node >> + >> +Required properties: >> +- compatible: should be "hisilicon,hip04-mac". >> +- reg: address and length of the register set for the device. >> +- interrupts: interrupt for the device. >> +- port: ppe port number connected to the controller: range from 0 to 31. > > ppe? > > Will there ever be more than one ppe? If so, describing the linkage to > the ppe with a phandle + args approach is preferable. > >> +- speed: 100 (100M) or 1000 (1000M). > > Can you not query this from the hardware? Will remove speed. > >> +- id: should be different and fe should be 0. > > This description is useless. > > What is this for, and why does this need to be in the dt? What is "fe"? Use alias instead. > >> + >> +Optional Properties: >> +- phy-handle : the phandle to a PHY node >> + >> + >> +* Ethernet ppe node: control rx & tx fifos of all ethernet controllers >> + >> +Required properties: >> +- compatible: should be "hisilicon,hip04-ppebase". > > Why "ppebase" rather than "ppe"? > >> +- reg: address and length of the register set for the node. > > s/node/device/ OK. Thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Sergei Thanks for the kind suggestion, will update. On Tue, Mar 18, 2014 at 8:51 PM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: >> diff --git a/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt >> b/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt >> new file mode 100644 >> index 0000000..c918f08 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt >> @@ -0,0 +1,74 @@ >> +Hisilicon hip04 Ethernet Controller >> + >> +* Ethernet controller node >> + >> +Required properties: >> +- compatible: should be "hisilicon,hip04-mac". >> +- reg: address and length of the register set for the device. >> +- interrupts: interrupt for the device. >> +- port: ppe port number connected to the controller: range from 0 to 31. >> +- speed: 100 (100M) or 1000 (1000M). > > > There's standard "max-speed" property for that, see > Documentation/devicetree/bindings/net/ethernet.txt in the 'net-next.git' > repo. > > >> +Optional Properties: >> +- phy-handle : the phandle to a PHY node > > > Please refer instead to the above mentioned file for this standard > property -- it is already described there. See other binding files as the > example. Yes, Documentation/devicetree/bindings/net/ethernet.txt has defined many useful node. Will refer it directly. Thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt b/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt new file mode 100644 index 0000000..c918f08 --- /dev/null +++ b/Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt @@ -0,0 +1,74 @@ +Hisilicon hip04 Ethernet Controller + +* Ethernet controller node + +Required properties: +- compatible: should be "hisilicon,hip04-mac". +- reg: address and length of the register set for the device. +- interrupts: interrupt for the device. +- port: ppe port number connected to the controller: range from 0 to 31. +- speed: 100 (100M) or 1000 (1000M). +- id: should be different and fe should be 0. + +Optional Properties: +- phy-handle : the phandle to a PHY node + + +* Ethernet ppe node: control rx & tx fifos of all ethernet controllers + +Required properties: +- compatible: should be "hisilicon,hip04-ppebase". +- reg: address and length of the register set for the node. + + +* MDIO bus node: + +Required properties: + +- compatible: "hisilicon,hip04-mdio" +- Inherets from MDIO bus node binding[1] +[1] Documentation/devicetree/bindings/net/phy.txt + +Example: + mdio { + compatible = "hisilicon,hip04-mdio"; + reg = <0x28f1000 0x1000>; + #address-cells = <1>; + #size-cells = <0>; + + phy0: ethernet-phy@0 { + reg = <0>; + marvell,reg-init = <18 0x14 0 0x8001>; + device_type = "ethernet-phy"; + }; + + phy1: ethernet-phy@1 { + reg = <1>; + marvell,reg-init = <18 0x14 0 0x8001>; + device_type = "ethernet-phy"; + }; + }; + + ppebase: ppebase@28c0000 { + compatible = "hisilicon,hip04-ppebase"; + reg = <0x28c0000 0x10000>; + }; + + fe: ethernet@28b0000 { + compatible = "hisilicon,hip04-mac"; + reg = <0x28b0000 0x10000>; + interrupts = <0 413 4>; + port = <31>; + speed = <100>; + id = <0>; + }; + + ge0: ethernet@2800000 { + compatible = "hisilicon,hip04-mac"; + reg = <0x2800000 0x10000>; + interrupts = <0 402 4>; + port = <0>; + speed = <1000>; + id = <1>; + phy-handle = <&phy0>; + };
This patch adds the Device Tree bindings for the Hisilicon hip04 Ethernet controller, including 100M / 1000M controller. Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> --- .../bindings/net/hisilicon-hip04-net.txt | 74 ++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/hisilicon-hip04-net.txt