[U-Boot] [PATCH 1/2] net: fec_mxc: configure MDIO hold time

[U-Boot] [PATCH 1/2] net: fec_mxc: configure MDIO hold time

[Mans Rullgard]

If the host clock frequency is higher than 100 MHz, the MDIO hold
time needs to be increased from its current setting of one cycle in
order to meet the specified minium of 10 ns.  Writing an appropriate
value to the HOLDTIME field of the MII_SPEED register achieves this.

Comment copied from Linux kernel.

Signed-off-by: Mans Rullgard <mans@mansr.com>
---
 drivers/net/fec_mxc.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 79f6737..1250d2a 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -131,13 +131,25 @@ static void fec_mii_setspeed(struct ethernet_regs *eth)
 	/*
 	 * Set MII_SPEED = (1/(mii_speed * 2)) * System Clock
 	 * and do not drop the Preamble.
+	 *
+	 * The i.MX28 and i.MX6 types have another filed in the MSCR (aka
+	 * MII_SPEED) register that defines the MDIO output hold time. Earlier
+	 * versions are RAZ there, so just ignore the difference and write the
+	 * register always.
+	 * The minimal hold time according to IEE802.3 (clause 22) is 10 ns.
+	 * HOLDTIME + 1 is the number of clk cycles the fec is holding the
+	 * output.
+	 * The HOLDTIME bitfield takes values between 0 and 7 (inclusive).
+	 * Given that ceil(clkrate / 5000000) <= 64, the calculation for
+	 * holdtime cannot result in a value greater than 3.
 	 */
-	register u32 speed = DIV_ROUND_UP(imx_get_fecclk(), 5000000);
+	u32 pclk = imx_get_fecclk();
+	u32 speed = DIV_ROUND_UP(pclk, 5000000);
+	u32 hold = DIV_ROUND_UP(pclk, 100000000) - 1;
 #ifdef FEC_QUIRK_ENET_MAC
 	speed--;
 #endif
-	speed <<= 1;
-	writel(speed, &eth->mii_speed);
+	writel(speed << 1 | hold << 8, &eth->mii_speed);
 	debug("%s: mii_speed %08x\n", __func__, readl(&eth->mii_speed));
 }
 
-- 
2.6.3

[U-Boot] [PATCH 2/2] net: fec_mxc: unregister mdio bus on probe error

[Mans Rullgard]

If fecmxc_initialize_multi() fails, it frees but does not unregister
the mdio bus, causing subsequent uses of the "mii" command to crash.
Fix this by adding mdio_unregister() calls where needed.

Signed-off-by: Mans Rullgard <mans@mansr.com>
---
 drivers/net/fec_mxc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 1250d2a..6c5e80b 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -1109,6 +1109,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
 #ifdef CONFIG_PHYLIB
 	phydev = phy_find_by_mask(bus, 1 << phy_id, PHY_INTERFACE_MODE_RGMII);
 	if (!phydev) {
+		mdio_unregister(bus);
 		free(bus);
 		return -ENOMEM;
 	}
@@ -1120,6 +1121,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
 #ifdef CONFIG_PHYLIB
 		free(phydev);
 #endif
+		mdio_unregister(bus);
 		free(bus);
 	}
 	return ret;
-- 
2.6.3

[U-Boot] [PATCH 2/2] net: fec_mxc: unregister mdio bus on probe error

[Eric Nelson]

Hi Mans,

On 12/08/2015 08:38 AM, Mans Rullgard wrote:
> If fecmxc_initialize_multi() fails, it frees but does not unregister
> the mdio bus, causing subsequent uses of the "mii" command to crash.
> Fix this by adding mdio_unregister() calls where needed.
> 
> Signed-off-by: Mans Rullgard <mans@mansr.com>
> ---
>  drivers/net/fec_mxc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> index 1250d2a..6c5e80b 100644
> --- a/drivers/net/fec_mxc.c
> +++ b/drivers/net/fec_mxc.c
> @@ -1109,6 +1109,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
>  #ifdef CONFIG_PHYLIB
>  	phydev = phy_find_by_mask(bus, 1 << phy_id, PHY_INTERFACE_MODE_RGMII);
>  	if (!phydev) {
> +		mdio_unregister(bus);

While you're in here, this should probably be mdio_free just
to prevent somebody else from searching as I did.

>  		free(bus);
>  		return -ENOMEM;
>  	}
> @@ -1120,6 +1121,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
>  #ifdef CONFIG_PHYLIB
>  		free(phydev);
>  #endif
> +		mdio_unregister(bus);

Ditto:
>  		free(bus);
>  	}
>  	return ret;
> 

Otherwise,

Reviewed-by: Eric Nelson <eric@nelint.com>

[U-Boot] [PATCH 2/2] net: fec_mxc: unregister mdio bus on probe error

[Måns Rullgård]

Eric Nelson <eric@nelint.com> writes:

> Hi Mans,
>
> On 12/08/2015 08:38 AM, Mans Rullgard wrote:
>> If fecmxc_initialize_multi() fails, it frees but does not unregister
>> the mdio bus, causing subsequent uses of the "mii" command to crash.
>> Fix this by adding mdio_unregister() calls where needed.
>> 
>> Signed-off-by: Mans Rullgard <mans@mansr.com>
>> ---
>>  drivers/net/fec_mxc.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
>> index 1250d2a..6c5e80b 100644
>> --- a/drivers/net/fec_mxc.c
>> +++ b/drivers/net/fec_mxc.c
>> @@ -1109,6 +1109,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
>>  #ifdef CONFIG_PHYLIB
>>  	phydev = phy_find_by_mask(bus, 1 << phy_id, PHY_INTERFACE_MODE_RGMII);
>>  	if (!phydev) {
>> +		mdio_unregister(bus);
>
> While you're in here, this should probably be mdio_free just
> to prevent somebody else from searching as I did.
>
>>  		free(bus);
>>  		return -ENOMEM;
>>  	}
>> @@ -1120,6 +1121,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
>>  #ifdef CONFIG_PHYLIB
>>  		free(phydev);
>>  #endif
>> +		mdio_unregister(bus);
>
> Ditto:
>>  		free(bus);
>>  	}
>>  	return ret;
>> 

Agreed, but that's a separate issue.

-- 
M?ns Rullg?rd
mans at mansr.com

[U-Boot] [PATCH 2/2] net: fec_mxc: unregister mdio bus on probe error

[Stefano Babic]

On 08/12/2015 16:38, Mans Rullgard wrote:
> If fecmxc_initialize_multi() fails, it frees but does not unregister
> the mdio bus, causing subsequent uses of the "mii" command to crash.
> Fix this by adding mdio_unregister() calls where needed.
> 
> Signed-off-by: Mans Rullgard <mans@mansr.com>
> ---

Applied to u-boot-imx, thanks!

Best regards,
Stefano Babic



-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 1/2] net: fec_mxc: configure MDIO hold time

[Eric Nelson]

Hi Mans,

On 12/08/2015 08:38 AM, Mans Rullgard wrote:
> If the host clock frequency is higher than 100 MHz, the MDIO hold
> time needs to be increased from its current setting of one cycle in
> order to meet the specified minium of 10 ns.  Writing an appropriate
> value to the HOLDTIME field of the MII_SPEED register achieves this.
> 
> Comment copied from Linux kernel.
> 
> Signed-off-by: Mans Rullgard <mans@mansr.com>
> ---
>  drivers/net/fec_mxc.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> index 79f6737..1250d2a 100644
> --- a/drivers/net/fec_mxc.c
> +++ b/drivers/net/fec_mxc.c
> @@ -131,13 +131,25 @@ static void fec_mii_setspeed(struct ethernet_regs *eth)
>  	/*
>  	 * Set MII_SPEED = (1/(mii_speed * 2)) * System Clock
>  	 * and do not drop the Preamble.
> +	 *

s/filed/field/

> +	 * The i.MX28 and i.MX6 types have another filed in the MSCR (aka
> +	 * MII_SPEED) register that defines the MDIO output hold time. Earlier
> +	 * versions are RAZ there, so just ignore the difference and write the
> +	 * register always.
> +	 * The minimal hold time according to IEE802.3 (clause 22) is 10 ns.
> +	 * HOLDTIME + 1 is the number of clk cycles the fec is holding the
> +	 * output.
> +	 * The HOLDTIME bitfield takes values between 0 and 7 (inclusive).
> +	 * Given that ceil(clkrate / 5000000) <= 64, the calculation for
> +	 * holdtime cannot result in a value greater than 3.
>  	 */
> -	register u32 speed = DIV_ROUND_UP(imx_get_fecclk(), 5000000);
> +	u32 pclk = imx_get_fecclk();
> +	u32 speed = DIV_ROUND_UP(pclk, 5000000);
> +	u32 hold = DIV_ROUND_UP(pclk, 100000000) - 1;
>  #ifdef FEC_QUIRK_ENET_MAC
>  	speed--;
>  #endif
> -	speed <<= 1;
> -	writel(speed, &eth->mii_speed);
> +	writel(speed << 1 | hold << 8, &eth->mii_speed);
>  	debug("%s: mii_speed %08x\n", __func__, readl(&eth->mii_speed));
>  }
>  
> 
Otherwise,

Reviewed-by: Eric Nelson <eric@nelint.com>

[U-Boot] [PATCH 1/2] net: fec_mxc: configure MDIO hold time

[Stefano Babic]

On 08/12/2015 17:08, Eric Nelson wrote:
> Hi Mans,
> 
> On 12/08/2015 08:38 AM, Mans Rullgard wrote:
>> If the host clock frequency is higher than 100 MHz, the MDIO hold
>> time needs to be increased from its current setting of one cycle in
>> order to meet the specified minium of 10 ns.  Writing an appropriate
>> value to the HOLDTIME field of the MII_SPEED register achieves this.
>>
>> Comment copied from Linux kernel.
>>
>> Signed-off-by: Mans Rullgard <mans@mansr.com>
>> ---
>>  drivers/net/fec_mxc.c | 18 +++++++++++++++---
>>  1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
>> index 79f6737..1250d2a 100644
>> --- a/drivers/net/fec_mxc.c
>> +++ b/drivers/net/fec_mxc.c
>> @@ -131,13 +131,25 @@ static void fec_mii_setspeed(struct ethernet_regs *eth)
>>  	/*
>>  	 * Set MII_SPEED = (1/(mii_speed * 2)) * System Clock
>>  	 * and do not drop the Preamble.
>> +	 *
> 
> s/filed/field/

Fixed by applying.

Applied to u-boot-imx, thanks!

Best regards,
Stefano Babic



-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================