diff mbox

[v2,2/4] Documentation: dts: Add bindings for APM X-Gene SoC ethernet driver

Message ID 1397271984-23405-3-git-send-email-isubramanian@apm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Iyappan Subramanian April 12, 2014, 3:06 a.m. UTC
This patch adds documentation for APM X-Gene SoC ethernet DTS binding.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Signed-off-by: Ravi Patel <rapatel@apm.com>
Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
---
 .../devicetree/bindings/net/apm-xgene-enet.txt     |   46 ++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/apm-xgene-enet.txt

Comments

Florian Fainelli April 12, 2014, 6:55 p.m. UTC | #1
2014-04-11 20:06 GMT-07:00 Iyappan Subramanian <isubramanian@apm.com>:
> This patch adds documentation for APM X-Gene SoC ethernet DTS binding.
>
> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
> Signed-off-by: Ravi Patel <rapatel@apm.com>
> Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
> ---
>  .../devicetree/bindings/net/apm-xgene-enet.txt     |   46 ++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/apm-xgene-enet.txt
>
> diff --git a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
> new file mode 100644
> index 0000000..0c983e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
> @@ -0,0 +1,46 @@
> +APM X-Gene SoC Ethernet nodes
> +
> +Ethernet nodes are defined to describe on-chip ethernet interfaces in
> +APM X-Gene SoC.
> +
> +Required properties:
> +- compatible           : Should be "apm,xgene-enet"
> +- reg                  : First resource is the ethernet base register set
> +                         Second resource is the ring base register set
> +                         Third resource is the ring command register set
> +- interrupts           : Ethernet main interrupt
> +- clocks               : Reference to the clock entry.
> +- local-mac-address    : Ethernet MAC address.
> +- phy-addr             : Ethernet MII phy address.
> +- phy-mode             : Ethernet MII mode.
> +
> +Optional properties:
> +- status               : Should be "ok" or "disabled" for enabled/disabled.
> +                         Default is "ok".
> +
> +Example:
> +       menetclk: menetclk {
> +               compatible = "apm,xgene-device-clock";
> +               clock-output-names = "menetclk";
> +               status = "ok";
> +       };
> +
> +       menet: ethernet@17020000 {
> +               compatible = "apm,xgene-enet";
> +               status = "disabled";
> +               reg = <0x0 0x17020000 0x0 0xd100>,
> +                     <0x0 0X17030000 0x0 0X400>,
> +                     <0x0 0X10000000 0x0 0X200>;
> +               interrupts = <0x0 0x3c 0x4>;
> +               clocks = <&menetclk 0>;
> +               local-mac-address = [00 01 73 00 00 01];
> +               phy-addr = <3>;

An Ethernet PHY node needs to follow the ePAPR recommended
representation, which is not this one. Please refer to
Documentation/devicetree/bindinsg/net/phy.txt for examples on how to
do this properly.

> +               phy-mode = "rgmii";
> +       };
> +
> +
> +/* Board-specific peripheral configurations */
> +
> +&menet {
> +        status = "ok";
> +};
> --
> 1.7.9.5
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Ben Dooks April 14, 2014, 1:06 p.m. UTC | #2
On 12/04/14 04:06, Iyappan Subramanian wrote:
> This patch adds documentation for APM X-Gene SoC ethernet DTS binding.
>
> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
> Signed-off-by: Ravi Patel <rapatel@apm.com>
> Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
> ---
>   .../devicetree/bindings/net/apm-xgene-enet.txt     |   46 ++++++++++++++++++++
>   1 file changed, 46 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/net/apm-xgene-enet.txt
>
> diff --git a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
> new file mode 100644
> index 0000000..0c983e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
> @@ -0,0 +1,46 @@
> +APM X-Gene SoC Ethernet nodes
> +
> +Ethernet nodes are defined to describe on-chip ethernet interfaces in
> +APM X-Gene SoC.
> +
> +Required properties:
> +- compatible		: Should be "apm,xgene-enet"
> +- reg			: First resource is the ethernet base register set
> +			  Second resource is the ring base register set
> +			  Third resource is the ring command register set
> +- interrupts		: Ethernet main interrupt
> +- clocks		: Reference to the clock entry.
> +- local-mac-address	: Ethernet MAC address.
> +- phy-addr		: Ethernet MII phy address.
> +- phy-mode		: Ethernet MII mode.
> +
> +Optional properties:
> +- status		: Should be "ok" or "disabled" for enabled/disabled.
> +			  Default is "ok".
> +
> +Example:
> +	menetclk: menetclk {
> +		compatible = "apm,xgene-device-clock";
> +		clock-output-names = "menetclk";
> +		status = "ok";
> +	};
> +
> +	menet: ethernet@17020000 {
> +		compatible = "apm,xgene-enet";
> +		status = "disabled";
> +		reg = <0x0 0x17020000 0x0 0xd100>,
> +		      <0x0 0X17030000 0x0 0X400>,
> +		      <0x0 0X10000000 0x0 0X200>;
> +		interrupts = <0x0 0x3c 0x4>;
> +		clocks = <&menetclk 0>;
> +		local-mac-address = [00 01 73 00 00 01];
> +		phy-addr = <3>;
> +		phy-mode = "rgmii";
> +	};

That doesn't look correct for a phy binding.
Iyappan Subramanian April 14, 2014, 10:11 p.m. UTC | #3
Hi Florian,

Thanks for the review comments.  Please find my responses inline.

On Sat, Apr 12, 2014 at 11:55 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> 2014-04-11 20:06 GMT-07:00 Iyappan Subramanian <isubramanian@apm.com>:
>> This patch adds documentation for APM X-Gene SoC ethernet DTS binding.
>>
>> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
>> Signed-off-by: Ravi Patel <rapatel@apm.com>
>> Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
>> ---
>>  .../devicetree/bindings/net/apm-xgene-enet.txt     |   46 ++++++++++++++++++++
>>  1 file changed, 46 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/apm-xgene-enet.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
>> new file mode 100644
>> index 0000000..0c983e8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
>> @@ -0,0 +1,46 @@
>> +APM X-Gene SoC Ethernet nodes
>> +
>> +Ethernet nodes are defined to describe on-chip ethernet interfaces in
>> +APM X-Gene SoC.
>> +
>> +Required properties:
>> +- compatible           : Should be "apm,xgene-enet"
>> +- reg                  : First resource is the ethernet base register set
>> +                         Second resource is the ring base register set
>> +                         Third resource is the ring command register set
>> +- interrupts           : Ethernet main interrupt
>> +- clocks               : Reference to the clock entry.
>> +- local-mac-address    : Ethernet MAC address.
>> +- phy-addr             : Ethernet MII phy address.
>> +- phy-mode             : Ethernet MII mode.
>> +
>> +Optional properties:
>> +- status               : Should be "ok" or "disabled" for enabled/disabled.
>> +                         Default is "ok".
>> +
>> +Example:
>> +       menetclk: menetclk {
>> +               compatible = "apm,xgene-device-clock";
>> +               clock-output-names = "menetclk";
>> +               status = "ok";
>> +       };
>> +
>> +       menet: ethernet@17020000 {
>> +               compatible = "apm,xgene-enet";
>> +               status = "disabled";
>> +               reg = <0x0 0x17020000 0x0 0xd100>,
>> +                     <0x0 0X17030000 0x0 0X400>,
>> +                     <0x0 0X10000000 0x0 0X200>;
>> +               interrupts = <0x0 0x3c 0x4>;
>> +               clocks = <&menetclk 0>;
>> +               local-mac-address = [00 01 73 00 00 01];
>> +               phy-addr = <3>;
>
> An Ethernet PHY node needs to follow the ePAPR recommended
> representation, which is not this one. Please refer to
> Documentation/devicetree/bindinsg/net/phy.txt for examples on how to
> do this properly.

Okay.  I will change the dts to use phy-handle property and remove
phy-addr.

>
>> +               phy-mode = "rgmii";
>> +       };
>> +
>> +
>> +/* Board-specific peripheral configurations */
>> +
>> +&menet {
>> +        status = "ok";
>> +};
>> --
>> 1.7.9.5
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
>
> --
> Florian
Iyappan Subramanian April 14, 2014, 10:15 p.m. UTC | #4
Hi Ben,

Thanks for the review comments. Please find my responses inline.

Thanks,
Iyappan

On Mon, Apr 14, 2014 at 6:06 AM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> On 12/04/14 04:06, Iyappan Subramanian wrote:
>>
>> This patch adds documentation for APM X-Gene SoC ethernet DTS binding.
>>
>> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
>> Signed-off-by: Ravi Patel <rapatel@apm.com>
>> Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
>> ---
>>   .../devicetree/bindings/net/apm-xgene-enet.txt     |   46
>> ++++++++++++++++++++
>>   1 file changed, 46 insertions(+)
>>   create mode 100644
>> Documentation/devicetree/bindings/net/apm-xgene-enet.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
>> b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
>> new file mode 100644
>> index 0000000..0c983e8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
>> @@ -0,0 +1,46 @@
>> +APM X-Gene SoC Ethernet nodes
>> +
>> +Ethernet nodes are defined to describe on-chip ethernet interfaces in
>> +APM X-Gene SoC.
>> +
>> +Required properties:
>> +- compatible           : Should be "apm,xgene-enet"
>> +- reg                  : First resource is the ethernet base register set
>> +                         Second resource is the ring base register set
>> +                         Third resource is the ring command register set
>> +- interrupts           : Ethernet main interrupt
>> +- clocks               : Reference to the clock entry.
>> +- local-mac-address    : Ethernet MAC address.
>> +- phy-addr             : Ethernet MII phy address.
>> +- phy-mode             : Ethernet MII mode.
>> +
>> +Optional properties:
>> +- status               : Should be "ok" or "disabled" for
>> enabled/disabled.
>> +                         Default is "ok".
>> +
>> +Example:
>> +       menetclk: menetclk {
>> +               compatible = "apm,xgene-device-clock";
>> +               clock-output-names = "menetclk";
>> +               status = "ok";
>> +       };
>> +
>> +       menet: ethernet@17020000 {
>> +               compatible = "apm,xgene-enet";
>> +               status = "disabled";
>> +               reg = <0x0 0x17020000 0x0 0xd100>,
>> +                     <0x0 0X17030000 0x0 0X400>,
>> +                     <0x0 0X10000000 0x0 0X200>;
>> +               interrupts = <0x0 0x3c 0x4>;
>> +               clocks = <&menetclk 0>;
>> +               local-mac-address = [00 01 73 00 00 01];
>> +               phy-addr = <3>;
>> +               phy-mode = "rgmii";
>> +       };
>
>
> That doesn't look correct for a phy binding.
>

Okay.  I will change the dts to use phy-handle property and remove
phy-addr.  I will also rename phy-mode to phy-connection-type as per
Documentation/devicetree/bindings/net/ethernet.txt

>
> --
> Ben Dooks                               http://www.codethink.co.uk/
> Senior Engineer                         Codethink - Providing Genius
Sergei Shtylyov April 18, 2014, 7:51 p.m. UTC | #5
Hello.

On 04/12/2014 07:06 AM, Iyappan Subramanian wrote:

> This patch adds documentation for APM X-Gene SoC ethernet DTS binding.

> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
> Signed-off-by: Ravi Patel <rapatel@apm.com>
> Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
> ---
>   .../devicetree/bindings/net/apm-xgene-enet.txt     |   46 ++++++++++++++++++++
>   1 file changed, 46 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/net/apm-xgene-enet.txt

> diff --git a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
> new file mode 100644
> index 0000000..0c983e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
> @@ -0,0 +1,46 @@
> +APM X-Gene SoC Ethernet nodes
> +
> +Ethernet nodes are defined to describe on-chip ethernet interfaces in
> +APM X-Gene SoC.
> +
> +Required properties:
> +- compatible		: Should be "apm,xgene-enet"
> +- reg			: First resource is the ethernet base register set
> +			  Second resource is the ring base register set
> +			  Third resource is the ring command register set
> +- interrupts		: Ethernet main interrupt
> +- clocks		: Reference to the clock entry.
> +- local-mac-address	: Ethernet MAC address.

    This property is described in 
Documentation/devicetree/bindings/net/ethernet.txt, please refer to it.

> +- phy-addr		: Ethernet MII phy address.

    No, thus won't do. Please use "phy-handle" property to refer to a PHY subnode.

> +- phy-mode		: Ethernet MII mode.

    This one is described in 
Documentation/devicetree/bindings/net/ethernet.txt as well.

WBR, Sergei
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
new file mode 100644
index 0000000..0c983e8
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
@@ -0,0 +1,46 @@ 
+APM X-Gene SoC Ethernet nodes
+
+Ethernet nodes are defined to describe on-chip ethernet interfaces in
+APM X-Gene SoC.
+
+Required properties:
+- compatible		: Should be "apm,xgene-enet"
+- reg			: First resource is the ethernet base register set
+			  Second resource is the ring base register set
+			  Third resource is the ring command register set
+- interrupts		: Ethernet main interrupt
+- clocks		: Reference to the clock entry.
+- local-mac-address	: Ethernet MAC address.
+- phy-addr		: Ethernet MII phy address.
+- phy-mode		: Ethernet MII mode.
+
+Optional properties:
+- status		: Should be "ok" or "disabled" for enabled/disabled.
+			  Default is "ok".
+
+Example:
+	menetclk: menetclk {
+		compatible = "apm,xgene-device-clock";
+		clock-output-names = "menetclk";
+		status = "ok";
+	};
+
+	menet: ethernet@17020000 {
+		compatible = "apm,xgene-enet";
+		status = "disabled";
+		reg = <0x0 0x17020000 0x0 0xd100>,
+		      <0x0 0X17030000 0x0 0X400>,
+		      <0x0 0X10000000 0x0 0X200>;
+		interrupts = <0x0 0x3c 0x4>;
+		clocks = <&menetclk 0>;
+		local-mac-address = [00 01 73 00 00 01];
+		phy-addr = <3>;
+		phy-mode = "rgmii";
+	};
+
+
+/* Board-specific peripheral configurations */
+
+&menet {
+        status = "ok";
+};