diff mbox

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

Message ID 1348680268-8194-2-git-send-email-zonque@gmail.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Daniel Mack Sept. 26, 2012, 5:24 p.m. UTC
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(+)

Comments

Mugunthan V N Sept. 26, 2012, 6:50 p.m. UTC | #1
> -----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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Mack Sept. 27, 2012, 11:42 a.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;