Re: [PATCH 4/4] arm/dts: am33xx: Add CPSW and MDIO module nodes for AM33XX
From: Benoit Cousson <hidden>
Date: 2012-10-31 15:17:27
Also in:
linux-arm-kernel, linux-omap
Hi, On 10/29/2012 09:21 AM, Vaibhav Hiremath wrote:
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?
quoted hunk ↗ jump to hunk
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 hunk ↗ jump to hunk
+}; + +&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.
+ 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?
+ slaves = <2>; + reg = <0x4a100000 0x800 + 0x4a101200 0x100 + 0x4a101000 0x100>;
Please align the address.
+ #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.
+ interrupts = <40 41 42 43>; + ranges;
You should add blank line here for readability.
+ cpsw_emac0: slave@0 {Mmm, you are using some address later and here some relative number, that does not looks very consistent.
+ 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
+ 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.
+ cpsw_emac1: slave@1 {
+ slave_reg_ofs = <0x308>;
+ sliver_reg_ofs = <0xdc0>;
+ /* Filled in by U-Boot */
+ mac-address = [ 00 00 00 00 00 00 ];
+ };
+
+ davinci_mdio: mdio@4a101000 {
+ compatible = "ti,davinci_mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ti,hwmods = "davinci_mdio";
+ bus_freq = <1000000>;
+ reg = <0x4a101000 0x100>;
+ };
+Stray change. Regards, Benoit