On Wed, Oct 31, 2012 at 20:47:27, Cousson, Benoit wrote:

Hi,

On 10/29/2012 09:21 AM, Vaibhav Hiremath wrote:

quoted

From: Mugunthan V N <redacted>

Add CPSW and MDIO related device tree data for AM33XX.
Also enable them into board/evm dts files by providing
respective phy-id.

Is there any bindings documentation for that device?

Yes, the base DT binding documentation is present in file
Documentation/devicetree/bindings/net/cpsw.txt

quoted

Signed-off-by: Mugunthan V N <redacted>
Signed-off-by: Vaibhav Hiremath <redacted>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Benoit Cousson <redacted>
---
 arch/arm/boot/dts/am335x-bone.dts |    8 ++++++
 arch/arm/boot/dts/am335x-evm.dts  |    8 ++++++
 arch/arm/boot/dts/am33xx.dtsi     |   50 +++++++++++++++++++++++++++++++++++++
 3 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
index c634f87..e233cfa 100644
--- a/arch/arm/boot/dts/am335x-bone.dts
+++ b/arch/arm/boot/dts/am335x-bone.dts

@@ -78,3 +78,11 @@
 		};
 	};
 };
+
+&cpsw_emac0 {
+	phy_id = "4a101000.mdio:00";

Why are you using that kind of interface? You seem to want a reference
to a device.

Cannot you have something less hard coded like:
phy_id = <&davinci_mdio>, <0>;

quoted

+};
+
+&cpsw_emac1 {
+	phy_id = "4a101000.mdio:01";
+};

diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index 185d632..415c3b3 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts

@@ -118,3 +118,11 @@
 		};
 	};
 };
+
+&cpsw_emac0 {
+	phy_id = "4a101000.mdio:00";
+};
+
+&cpsw_emac1 {
+	phy_id = "4a101000.mdio:01";
+};

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index bb31bff..f6bea04 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi

@@ -210,5 +210,55 @@
 			interrupt-parent = <&intc>;
 			interrupts = <91>;
 		};
+
+		mac: ethernet@4A100000 {

hexa data should be in lower case.

Oops. Will correct it.

quoted

+			compatible = "ti,cpsw";
+			ti,hwmods = "cpgmac0";
+			cpdma_channels = <8>;
+			host_port_no = <0>;
+			cpdma_reg_ofs = <0x800>;
+			cpdma_sram_ofs = <0xa00>;
+			ale_reg_ofs = <0xd00>;
+			ale_entries = <1024>;
+			host_port_reg_ofs = <0x108>;
+			hw_stats_reg_ofs = <0x900>;
+			bd_ram_ofs = <0x2000>;
+			bd_ram_size = <0x2000>;
+			no_bd_ram = <0>;
+			rx_descs = <64>;
+			mac_control = <0x20>;

Do you have to store all these data in the DTS? Cannot it be in the driver?

Do you expect to have several instance of the same IP with different
parameters here?

I will let Mugunthan respond to this.

quoted

+			slaves = <2>;
+			reg = <0x4a100000 0x800
+				0x4a101200 0x100
+				0x4a101000 0x100>;

Please align the address.

Ok.

quoted

+			#address-cells = <1>;
+			#size-cells = <1>;
+			interrupt-parent = <&intc>;
+			/* c0_rx_thresh_pend c0_rx_pend c0_tx_pend c0_misc_pend*/

Please use a standard multi-line comment instead of trying to put
everything in one line.

Oops. Will correct it.

quoted

+			interrupts = <40 41 42 43>;
+			ranges;

You should add blank line here for readability.

OK.

quoted

+			cpsw_emac0: slave@0 {

Mmm, you are using some address later and here some relative number,
that does not looks very consistent.

quoted

+				slave_reg_ofs = <0x208>;

Is it an offset from 4a100000? Cannot you use the address for the slave
name?

Something like that: cpsw_emac0: slave@4a100208

quoted

+				sliver_reg_ofs = <0xd80>;
+				/* Filled in by U-Boot */
+				mac-address = [ 00 00 00 00 00 00 ];
+			};

You should add blank line here for readability.

Ok

Thanks,
Vaibhav