net: ti cpsw ethernet: allow reading phy interface mode from DT

PATCH Applied Daniel Mack 4 participants 7 messages Sep 26, 2012 – Oct 1, 2012 1 message not in the archive

Subsystems (4)

Networking Network drivers Device tree bindings TI Ethernet switch driver (cpsw)

lore

Share & tools

lore

Daniel Mack

author

2012-09-26 17:24 UTC #0 lore raw

PATCH 1/2 net: ti cpsw ethernet: allow reading phy interface mode from DT

Allow users to specify the phy interface of the CPSW slaves. The new node parameter is called "phy_if_mode" and is optional. The original behaviour of the driver is preserved when not given.

Signed-off-by Daniel Mack <[email protected]>

Cc Mugunthan V N <[email protected]>

Cc Vaibhav Hiremath <[email protected]>

Cc David S. Miller <[email protected]>

---
 Documentation/devicetree/bindings/net/cpsw.txt | 3 +++
 drivers/net/ethernet/ti/cpsw.c                 | 3 +++
 2 files changed, 6 insertions(+)

Documentation/devicetree/bindings/net/cpsw.txt +3 -0

index dcaabe9..d87f7d2 100644
@@ -25,6 +25,8 @@ Required properties:
 - slave_reg_ofs		: Specifies slave register offset
 - sliver_reg_ofs	: Specifies slave sliver register offset
 - phy_id		: Specifies slave phy id
+- phy_if_mode		: Specified slave phy interface mode (optional)
+			  (one of the PHY_INTERFACE_MODE_* as numerical value)
 - mac-address		: Specifies slave MAC address
 
 Optional properties:

@@ -62,6 +64,7 @@ Examples:
 			slave_reg_ofs = <0x208>;
 			sliver_reg_ofs = <0xd80>;
 			phy_id = "davinci_mdio.16:00";
+			phy_if_mode = <6>; /* PHY_INTERFACE_MODE_RGMII */
 			/* Filled in by U-Boot */
 			mac-address = [ 00 00 00 00 00 00 ];
 		};

drivers/net/ethernet/ti/cpsw.c +3 -0

index aa78168..3d7594e 100644
@@ -850,6 +850,9 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 		}
 		slave_data->sliver_reg_ofs = prop;
 
+		if (!of_property_read_u32(slave_node, "phy_if_mode", &prop))
+			slave_data->phy_if = prop;
+
 		mac_addr = of_get_mac_address(slave_node);
 		if (mac_addr)
 			memcpy(slave_data->mac_addr, mac_addr, ETH_ALEN);

Signature · 1 line

1.7.11.4

Daniel Mack

author

2012-09-26 17:24 UTC #1 lore raw

PATCH 2/2 net: ti cpsw ethernet: set IFCTL_{A,B} bits for RMII mode

For RMII mode operation in 100Mbps, the CPSW needs to set the IFCTL_A / IFCTL_B bits in the MACCONTROL register.

Signed-off-by Daniel Mack <[email protected]>

Cc Mugunthan V N <[email protected]>

Cc Vaibhav Hiremath <[email protected]>

Cc David S. Miller <[email protected]>

---
 drivers/net/ethernet/ti/cpsw.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/net/ethernet/ti/cpsw.c +6 -0

index 3d7594e..d88dbfa 100644
@@ -386,6 +386,12 @@ static void _cpsw_adjust_link(struct cpsw_slave *slave,
 			mac_control |= BIT(7);	/* GIGABITEN	*/
 		if (phy->duplex)
 			mac_control |= BIT(0);	/* FULLDUPLEXEN	*/
+
+		/* set speed_in input in case RMII mode is used in >10Mbps */
+		if (phy->speed > 10 && slave->slave_num < 2 &&
+		    phy->interface == PHY_INTERFACE_MODE_RMII)
+			mac_control |= BIT(15 + slave->slave_num);
+
 		*link = true;
 	} else {
 		mac_control = 0;

Signature · 1 line

1.7.11.4

N, Mugunthan V

2012-09-26 18:50 UTC #2 lore raw

RE: [PATCH 2/2] net: ti cpsw ethernet: set IFCTL_{A,B} bits for RMII mode

Quoted 26 lines

-----Original Message-----
From: Daniel Mack [mailto:[email protected]]
Sent: Wednesday, September 26, 2012 10:54 PM
To: [email protected]
Cc: [email protected]; Daniel Mack; N, Mugunthan V; Hiremath, Vaibhav; David S. Miller
Subject: [PATCH 2/2] net: ti cpsw ethernet: set IFCTL_{A,B} bits for RMII mode
For RMII mode operation in 100Mbps, the CPSW needs to set the IFCTL_A / IFCTL_B bits in the MACCONTROL register.
Signed-off-by Daniel Mack <[email protected]>
Cc Mugunthan V N <[email protected]>
Cc Vaibhav Hiremath <[email protected]>
Cc David S. Miller <[email protected]>
---
 drivers/net/ethernet/ti/cpsw.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/ethernet/ti/cpsw.c +0 -0
index 3d7594e..d88dbfa 100644

drivers/net/ethernet/ti/cpsw.c static void _cpsw_adjust_link(struct cpsw_slave · line 395

Quoted 10 lines

drivers/net/ethernet/ti/cpsw.c +5 -0

@@ -386,6 +386,12 @@ static void _cpsw_adjust_link(struct cpsw_slave
 *slave,
 			mac_control |= BIT(7);	/* GIGABITEN	*/
 		if (phy->duplex)
 			mac_control |= BIT(0);	/* FULLDUPLEXEN	*/
+
+		/* set speed_in input in case RMII mode is used in >10Mbps
 */
+		if (phy->speed > 10 && slave->slave_num < 2 &&
+		    phy->interface == PHY_INTERFACE_MODE_RMII)
+			mac_control |= BIT(15 + slave->slave_num);

Mac control register is separate for both the slaves and has same bit definitions, Bit 15 has to be set for 100Mbps link for RMII and RGMII Phy interface to control the RMII/RGMII gasket and in GMII this bit is Un-used by CPSW.
For slave 1, Bit 16 is set with the above code which is not used control the RMII/RGMII gasket control. So it is not required to pass the Phy mode from DT. This patch has to be reworked to set Bit 15 with any Phy mode connected.

The original driver present was tested with GMII (Beagle Bone A5) and RGMII (AM3358 EVM) phy , but CPSW works fine without setting this bit in RGMII phymode so this issue was not caught in testing.

Regards
Mugunthan V N

Quoted 6 lines

drivers/net/ethernet/ti/cpsw.c +1 -0
+
 		*link = true;
drivers/net/ethernet/ti/cpsw.c +0 -1
 	} else {
 		mac_control = 0;
--
1.7.11.4

Daniel Mack

author

2012-09-27 11:42 UTC #3 lore raw

Re: [PATCH 2/2] net: ti cpsw ethernet: set IFCTL_{A,B} bits for RMII mode

Quoted 26 lines · Mugunthan V

Quoted 26 lines
For RMII mode operation in 100Mbps, the CPSW needs to set the IFCTL_A / IFCTL_B bits in the MACCONTROL register.
Signed-off-by Daniel Mack <[email protected]>
Cc Mugunthan V N <[email protected]>
Cc Vaibhav Hiremath <[email protected]>
Cc David S. Miller <[email protected]>
---
 drivers/net/ethernet/ti/cpsw.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/ethernet/ti/cpsw.c +4 -0
index 3d7594e..d88dbfa 100644
@@ -386,6 +386,12 @@ static void _cpsw_adjust_link(struct cpsw_slave
 *slave,
 			mac_control |= BIT(7);	/* GIGABITEN	*/
 		if (phy->duplex)
 			mac_control |= BIT(0);	/* FULLDUPLEXEN	*/
+
+		/* set speed_in input in case RMII mode is used in >10Mbps
 */
+		if (phy->speed > 10 && slave->slave_num < 2 &&
+		    phy->interface == PHY_INTERFACE_MODE_RMII)

Quoted 8 lines

drivers/net/ethernet/ti/cpsw.c static void _cpsw_adjust_link(struct cpsw_slave *slave, · line 393
Quoted 1 line
drivers/net/ethernet/ti/cpsw.c +1 -0
+			mac_control |= BIT(15 + slave->slave_num);
Mac control register is separate for both the slaves and has same bit definitions, Bit 15 has to be set for 100Mbps link for RMII and RGMII Phy interface to control the RMII/RGMII gasket and in GMII this bit is Un-used by CPSW.
For slave 1, Bit 16 is set with the above code which is not used control the RMII/RGMII gasket control. So it is not required to pass the Phy mode from DT. This patch has to be reworked to set Bit 15 with any Phy mode connected.

Hmm, that's interesting. I read the datasheet differently, but I believe you're right.

Quoted 3 lines

The original driver present was tested with GMII (Beagle Bone A5) and RGMII (AM3358 EVM) phy , but CPSW works fine without setting this bit in RGMII phymode so this issue was not caught in testing.

Yes, it used to work fine for me too until the hardware was reworked from RGMII to RMII :)

Thanks a lot for the review - I just tested that setting bit 15 for all PHY interface modes works for me as well, so I'm fine with that solution. Will repost a new patch.

Daniel

A message that is not in the archive (sent off-list or not yet received): 1348680268-8194-1-git-send-email-zonque-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

David Miller

Applied

2012-10-01 21:11 UTC #5 lore raw

From: Daniel Mack <[email protected]>
Date: Wed, 26 Sep 2012 19:24:27 +0200

Quoted 5 lines

Allow users to specify the phy interface of the CPSW slaves. The new node parameter is called "phy_if_mode" and is optional. The original behaviour of the driver is preserved when not given.
Signed-off-by Daniel Mack <[email protected]>

Applied, thanks.

Daniel Mack

author

2012-10-01 21:15 UTC #6 lore raw

Quoted 11 lines · David Miller

From: Daniel Mack <[email protected]>
Date: Wed, 26 Sep 2012 19:24:27 +0200
Quoted 5 lines
Allow users to specify the phy interface of the CPSW slaves. The new node parameter is called "phy_if_mode" and is optional. The original behaviour of the driver is preserved when not given.
Signed-off-by Daniel Mack <[email protected]>
Applied, thanks.

Eh, no - sorry. My original understanding was that a bit in the cpsw registers has to be set only when a certain physical phy mode is in use. Hence we would have needed a way to pass that information in via DT. But as Mugunthan pointed out, that bit must always be set, and the cpsw slaves can stay agnostic to the actual phy mode.

So that patch isn't needed. Sorry for the confusion.

Daniel

David Miller

maintainer

2012-10-01 21:38 UTC #7 lore raw

From: Daniel Mack <[email protected]>
Date: Mon, 01 Oct 2012 23:15:38 +0200

Quoted 13 lines · David Miller

Quoted 11 lines
From: Daniel Mack <[email protected]>
Date: Wed, 26 Sep 2012 19:24:27 +0200
Quoted 5 lines
Allow users to specify the phy interface of the CPSW slaves. The new node parameter is called "phy_if_mode" and is optional. The original behaviour of the driver is preserved when not given.
Signed-off-by Daniel Mack <[email protected]>
Applied, thanks.

Quoted 7 lines

Eh, no - sorry. My original understanding was that a bit in the cpsw registers has to be set only when a certain physical phy mode is in use. Hence we would have needed a way to pass that information in via DT. But as Mugunthan pointed out, that bit must always be set, and the cpsw slaves can stay agnostic to the actual phy mode.
So that patch isn't needed. Sorry for the confusion.

Ok I'll revert.