diff mbox

[2/3] Documentation: add Device tree bindings for Hisilicon hix5hd2 ethernet

Message ID 1400504227-12047-3-git-send-email-zhangfei.gao@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Zhangfei Gao May 19, 2014, 12:57 p.m. UTC
Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
 .../bindings/net/hisilicon-hix5hd2-net.txt         |   37 ++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/hisilicon-hix5hd2-net.txt

Comments

Sergei Shtylyov May 19, 2014, 4:26 p.m. UTC | #1
Hello.

On 05/19/2014 04:57 PM, Zhangfei Gao wrote:

> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> ---
>   .../bindings/net/hisilicon-hix5hd2-net.txt         |   37 ++++++++++++++++++++
>   1 file changed, 37 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/net/hisilicon-hix5hd2-net.txt

> diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-net.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-net.txt
> new file mode 100644
> index 0000000..16b1f0e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-net.txt
> @@ -0,0 +1,37 @@
> +Hisilicon hix5hd2 Ethernet Controller
> +
> +Required properties:
> +- compatible: should be "hisilicon,hix5hd2-mac".
> +- reg : Specifies base physical address(s) and size of the device registers.

    Er, I must note that the colon style here is not consistent with the rest 
of the properties, i.e. there's space before the colon here, while in other 
cases there's none.

> +  The first region is the mac register base and size.

    s/mac/MAC/.

> +  The second region is external interface control register.
> +- interrupts: Should contain the MAC interrupts
> +- #address-cells: Must be <1>.
> +- #size-cells: Must be <0>.

    Descriptions should consistently start with a lower case, I think...

> +- phy-mode: see ethernet.txt [1].
> +- phy-handle: see ethernet.txt [1].
> +- mac-address: see ethernet.txt [1].
> +- clocks: clock phandle and specifier pair.
> +
> +- phy subnode: inherits from phy binding [2]

    s/phy/PHY/.

> +
> +[1] Documentation/devicetree/bindings/net/ethernet.txt
> +[2] Documentation/devicetree/bindings/net/phy.txt
> +
> +Example:
> +	gmac0: ethernet@f9840000 {
> +		compatible = "hisilicon,hix5hd2-mac";
> +		reg = <0xf9840000 0x1000>,<0xf984300c 0x4>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		phy-handle = <&phy0>;
> +		phy-mode = "mii";
> +		mac-address = [00 00 00 00 00 00];

    Don't think that's a good value, unless this is only a placeholder.

> +		interrupts = <0 71 4>;
> +		clocks = <&clock HIX5HD2_MAC0_CLK>;
> +
> +		phy0: ethernet-phy@2 {

    I'd rather label ut phy2...

> +			reg = <2>;
> +			compatible = "ethernet-phy-ieee802.3-c22";

    This is the default "compatible", i.e. there's no need to specify it, IIRC.

> +		};
> +	};

WBR, Sergei
Zhangfei Gao May 20, 2014, 2:12 a.m. UTC | #2
On 05/20/2014 12:26 AM, Sergei Shtylyov wrote:
> On 05/19/2014 04:57 PM, Zhangfei Gao wrote:
>> +Hisilicon hix5hd2 Ethernet Controller
>> +
>> +Required properties:
>> +- compatible: should be "hisilicon,hix5hd2-mac".
>> +- reg : Specifies base physical address(s) and size of the device
>> registers.
>
>     Er, I must note that the colon style here is not consistent with the
> rest of the properties, i.e. there's space before the colon here, while
> in other cases there's none.
Will remove the space.

>
>> +  The first region is the mac register base and size.
>
>     s/mac/MAC/.
>
>> +  The second region is external interface control register.
>> +- interrupts: Should contain the MAC interrupts
>> +- #address-cells: Must be <1>.
>> +- #size-cells: Must be <0>.
>
>     Descriptions should consistently start with a lower case, I think...
OK.
>
>> +- phy-mode: see ethernet.txt [1].
>> +- phy-handle: see ethernet.txt [1].
>> +- mac-address: see ethernet.txt [1].
>> +- clocks: clock phandle and specifier pair.
>> +
>> +- phy subnode: inherits from phy binding [2]
>
>     s/phy/PHY/.
>
>> +
>> +[1] Documentation/devicetree/bindings/net/ethernet.txt
>> +[2] Documentation/devicetree/bindings/net/phy.txt
>> +
>> +Example:
>> +    gmac0: ethernet@f9840000 {
>> +        compatible = "hisilicon,hix5hd2-mac";
>> +        reg = <0xf9840000 0x1000>,<0xf984300c 0x4>;
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        phy-handle = <&phy0>;
>> +        phy-mode = "mii";
>> +        mac-address = [00 00 00 00 00 00];
>
>     Don't think that's a good value, unless this is only a placeholder.
Yes, mac-address is placeholder here and would be filled by boot-loader.
Otherwise random mac addr will be used.

>
>> +        interrupts = <0 71 4>;
>> +        clocks = <&clock HIX5HD2_MAC0_CLK>;
>> +
>> +        phy0: ethernet-phy@2 {
>
>     I'd rather label ut phy2...
OK
>
>> +            reg = <2>;
>> +            compatible = "ethernet-phy-ieee802.3-c22";
>
>     This is the default "compatible", i.e. there's no need to specify
> it, IIRC.
Got it, find it is clarified in phy.txt.

Thanks
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-net.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-net.txt
new file mode 100644
index 0000000..16b1f0e
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-net.txt
@@ -0,0 +1,37 @@ 
+Hisilicon hix5hd2 Ethernet Controller
+
+Required properties:
+- compatible: should be "hisilicon,hix5hd2-mac".
+- reg : Specifies base physical address(s) and size of the device registers.
+  The first region is the mac register base and size.
+  The second region is external interface control register.
+- interrupts: Should contain the MAC interrupts
+- #address-cells: Must be <1>.
+- #size-cells: Must be <0>.
+- phy-mode: see ethernet.txt [1].
+- phy-handle: see ethernet.txt [1].
+- mac-address: see ethernet.txt [1].
+- clocks: clock phandle and specifier pair.
+
+- phy subnode: inherits from phy binding [2]
+
+[1] Documentation/devicetree/bindings/net/ethernet.txt
+[2] Documentation/devicetree/bindings/net/phy.txt
+
+Example:
+	gmac0: ethernet@f9840000 {
+		compatible = "hisilicon,hix5hd2-mac";
+		reg = <0xf9840000 0x1000>,<0xf984300c 0x4>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		phy-handle = <&phy0>;
+		phy-mode = "mii";
+		mac-address = [00 00 00 00 00 00];
+		interrupts = <0 71 4>;
+		clocks = <&clock HIX5HD2_MAC0_CLK>;
+
+		phy0: ethernet-phy@2 {
+			reg = <2>;
+			compatible = "ethernet-phy-ieee802.3-c22";
+		};
+	};