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

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

[Daniel Mack]

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 <zonque@gmail.com>
Cc: Mugunthan V N <mugunthanvnm@ti.com>
Cc: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: David S. Miller <davem@davemloft.net>
---
 Documentation/devicetree/bindings/net/cpsw.txt | 3 +++
 drivers/net/ethernet/ti/cpsw.c                 | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index dcaabe9..d87f7d2 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -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 ];
 		};
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index aa78168..3d7594e 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -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);
-- 
1.7.11.4

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

[Daniel Mack]

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 <zonque@gmail.com>
Cc: Mugunthan V N <mugunthanvnm@ti.com>
Cc: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: David S. Miller <davem@davemloft.net>
---
 drivers/net/ethernet/ti/cpsw.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 3d7594e..d88dbfa 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -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;
-- 
1.7.11.4

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

[N, Mugunthan V]

> -----Original Message-----
> From: Daniel Mack [mailto:zonque@gmail.com]
> Sent: Wednesday, September 26, 2012 10:54 PM
> To: netdev@vger.kernel.org
> Cc: devicetree-discuss@lists.ozlabs.org; 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 <zonque@gmail.com>
> Cc: Mugunthan V N <mugunthanvnm@ti.com>
> Cc: Vaibhav Hiremath <hvaibhav@ti.com>
> Cc: David S. Miller <davem@davemloft.net>
> ---
>  drivers/net/ethernet/ti/cpsw.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/ethernet/ti/cpsw.c
> b/drivers/net/ethernet/ti/cpsw.c
> index 3d7594e..d88dbfa 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -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

> +
>  		*link = true;
>  	} else {
>  		mac_control = 0;
> --
> 1.7.11.4

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

[Daniel Mack]

On 26.09.2012 20:50, N, Mugunthan V wrote:
>> 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 <zonque@gmail.com>
>> Cc: Mugunthan V N <mugunthanvnm@ti.com>
>> Cc: Vaibhav Hiremath <hvaibhav@ti.com>
>> Cc: David S. Miller <davem@davemloft.net>
>> ---
>>  drivers/net/ethernet/ti/cpsw.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/ti/cpsw.c
>> b/drivers/net/ethernet/ti/cpsw.c
>> index 3d7594e..d88dbfa 100644
>> --- a/drivers/net/ethernet/ti/cpsw.c
>> +++ b/drivers/net/ethernet/ti/cpsw.c
>> @@ -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.

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

> 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

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

[David Miller]

From: Daniel Mack <zonque-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Wed, 26 Sep 2012 19:24:27 +0200

> 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 <zonque-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Applied, thanks.

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

[Daniel Mack]

On 01.10.2012 23:11, David Miller wrote:
> From: Daniel Mack <zonque@gmail.com>
> Date: Wed, 26 Sep 2012 19:24:27 +0200
> 
>> 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 <zonque@gmail.com>
> 
> 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

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

[David Miller]

From: Daniel Mack <zonque@gmail.com>
Date: Mon, 01 Oct 2012 23:15:38 +0200

> On 01.10.2012 23:11, David Miller wrote:
>> From: Daniel Mack <zonque@gmail.com>
>> Date: Wed, 26 Sep 2012 19:24:27 +0200
>> 
>>> 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 <zonque@gmail.com>
>> 
>> 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.

Ok I'll revert.