RE: [PATCH 4/4] arm/dts: am33xx: Add CPSW and MDIO module nodes for AM33XX

-----Original Message-----
From: Cousson, Benoit
Sent: Wednesday, October 31, 2012 8:47 PM
To: Hiremath, Vaibhav
Cc: netdev@vger.kernel.org; paul@pwsan.com; linux-arm-
kernel@lists.infradead.org; linux-omap@vger.kernel.org; N, Mugunthan V;
Richard Cochran
Subject: Re: [PATCH 4/4] arm/dts: am33xx: Add CPSW and MDIO module
nodes for AM33XX

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?

Though CPSW is a single IP in AM335X, CPSW has sub modules like CPDMA, ALE,
SLIVER, CPTS and SLAVES where is IP integrator can locate it at different
offsets. For example comparing the CPSW ip in TI814X and AM335X all the
above offsets are changed. So I have kept all these offsets in DT as driver
should not hold any SoC related informations

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

I have followed the naming convention as per the TRM which is mentioned as
Slave 0 and Slave 1 and its offset is from 0x4a100000. I have taken the
reference from  arch/arm/boot/dts/picoxcell-pc3x2.dtsi+167 and followed
the same naming convention from TRM. There is one more method to implement
in reference from arch/arm/boot/dts/imx6q.dtsi+408 as below

+			cpsw_emac0: slave@208 {
+				slave_reg_ofs = <0x208>;
+				sliver_reg_ofs = <0xd80>;
+				/* Filled in by U-Boot */
+				mac-address = [ 00 00 00 00 00 00 ];
+			};

Regards
Mugunthan V N