mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/3] fec: restart autoneg at open instead of init
@ 2012-09-06 19:39 Eric Bénard
  2012-09-06 19:39 ` [PATCH 2/3] i.MX51: unbreak FEC iomux Eric Bénard
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Eric Bénard @ 2012-09-06 19:39 UTC (permalink / raw)
  To: barebox

this saves some time during boot when ethernet is not needed in barebox

Signed-off-by: Eric Bénard <eric@eukrea.com>
---
 drivers/net/fec_imx.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/fec_imx.c b/drivers/net/fec_imx.c
index 599a9b4..2bc06a9 100644
--- a/drivers/net/fec_imx.c
+++ b/drivers/net/fec_imx.c
@@ -347,9 +347,6 @@ static int fec_init(struct eth_device *dev)
 	/* size of each buffer */
 	writel(FEC_MAX_PKT_SIZE, fec->regs + FEC_EMRBR);
 
-	if (fec->xcv_type != SEVENWIRE)
-		miidev_restart_aneg(&fec->miidev);
-
 	return 0;
 }
 
@@ -363,6 +360,9 @@ static int fec_open(struct eth_device *edev)
 	int ret;
 	u32 ecr;
 
+	if (fec->xcv_type != SEVENWIRE)
+		miidev_restart_aneg(&fec->miidev);
+
 	/*
 	 * Initialize RxBD/TxBD rings
 	 */
-- 
1.7.7.6


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 2/3] i.MX51: unbreak FEC iomux
  2012-09-06 19:39 [PATCH 1/3] fec: restart autoneg at open instead of init Eric Bénard
@ 2012-09-06 19:39 ` Eric Bénard
  2012-09-07  8:30   ` Sascha Hauer
  2012-09-06 19:39 ` [PATCH 3/3] miidev: fix 1G wrong detection Eric Bénard
  2012-09-07  6:57 ` [PATCH 1/3] fec: restart autoneg at open instead of init Sascha Hauer
  2 siblings, 1 reply; 18+ messages in thread
From: Eric Bénard @ 2012-09-06 19:39 UTC (permalink / raw)
  To: barebox

in commit 2bdc9f57a86dff41cfc1f87b644a2e53fdcce2b6 the iomux was synced
with the kernel but this leads to some changes in the PAD_CTRL of some
FEC pins leading to a non working FEC on our cpuimx51 board.

This patch set back the PAD_CTRL of the missing pins to the initial
value.

Signed-off-by: Eric Bénard <eric@eukrea.com>
---
 arch/arm/mach-imx/include/mach/iomux-mx51.h |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-imx/include/mach/iomux-mx51.h b/arch/arm/mach-imx/include/mach/iomux-mx51.h
index 36c8989..0252d41 100644
--- a/arch/arm/mach-imx/include/mach/iomux-mx51.h
+++ b/arch/arm/mach-imx/include/mach/iomux-mx51.h
@@ -649,20 +649,20 @@
 #define MX51_PAD_DI2_PIN2__DI2_PIN2		IOMUX_PAD(0x74c, 0x344, 0, __NA_, 0, NO_PAD_CTRL)
 #define MX51_PAD_DI2_PIN2__FEC_MDC		IOMUX_PAD(0x74c, 0x344, 2, __NA_, 0, MX51_PAD_CTRL_5)
 #define MX51_PAD_DI2_PIN3__DI2_PIN3		IOMUX_PAD(0x750, 0x348, 0, __NA_, 0, NO_PAD_CTRL)
-#define MX51_PAD_DI2_PIN3__FEC_MDIO		IOMUX_PAD(0x750, 0x348, 2, 0x954, 1, NO_PAD_CTRL)
+#define MX51_PAD_DI2_PIN3__FEC_MDIO		IOMUX_PAD(0x750, 0x348, 2, 0x954, 1, MX51_PAD_CTRL_5)
 #define MX51_PAD_DI2_DISP_CLK__DI2_DISP_CLK	IOMUX_PAD(0x754, 0x34c, 0, __NA_, 0, NO_PAD_CTRL)
 #define MX51_PAD_DI2_DISP_CLK__FEC_RDATA1	IOMUX_PAD(0x754, 0x34c, 2, 0x95c, 1, NO_PAD_CTRL)
 #define MX51_PAD_DI_GP4__DI2_PIN15		IOMUX_PAD(0x758, 0x350, 4, __NA_, 0, NO_PAD_CTRL)
 #define MX51_PAD_DI_GP4__DISP1_SER_DIN		IOMUX_PAD(0x758, 0x350, 0, 0x9c0, 1, NO_PAD_CTRL)
 #define MX51_PAD_DI_GP4__DISP2_PIN1		IOMUX_PAD(0x758, 0x350, 3, __NA_, 0, NO_PAD_CTRL)
-#define MX51_PAD_DI_GP4__FEC_RDATA2		IOMUX_PAD(0x758, 0x350, 2, 0x960, 1, NO_PAD_CTRL)
+#define MX51_PAD_DI_GP4__FEC_RDATA2		IOMUX_PAD(0x758, 0x350, 2, 0x960, 1, MX51_PAD_CTRL_5)
 #define MX51_PAD_DISP2_DAT0__DISP2_DAT0		IOMUX_PAD(0x75c, 0x354, 0, __NA_, 0, NO_PAD_CTRL)
-#define MX51_PAD_DISP2_DAT0__FEC_RDATA3		IOMUX_PAD(0x75c, 0x354, 2, 0x964, 1, NO_PAD_CTRL)
+#define MX51_PAD_DISP2_DAT0__FEC_RDATA3		IOMUX_PAD(0x75c, 0x354, 2, 0x964, 1, MX51_PAD_CTRL_5)
 #define MX51_PAD_DISP2_DAT0__KEY_COL6		IOMUX_PAD(0x75c, 0x354, 4, 0x9c8, 1, NO_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT0__UART3_RXD		IOMUX_PAD(0x75c, 0x354, 5, 0x9f4, 8, MX51_UART_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT0__USBH3_CLK		IOMUX_PAD(0x75c, 0x354, 3, 0x9f8, 1, MX51_UART_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT1__DISP2_DAT1		IOMUX_PAD(0x760, 0x358, 0, __NA_, 0, NO_PAD_CTRL)
-#define MX51_PAD_DISP2_DAT1__FEC_RX_ER		IOMUX_PAD(0x760, 0x358, 2, 0x970, 1, NO_PAD_CTRL)
+#define MX51_PAD_DISP2_DAT1__FEC_RX_ER		IOMUX_PAD(0x760, 0x358, 2, 0x970, 1, MX51_PAD_CTRL_5)
 #define MX51_PAD_DISP2_DAT1__KEY_COL7		IOMUX_PAD(0x760, 0x358, 4, 0x9cc, 1, NO_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT1__UART3_TXD		IOMUX_PAD(0x760, 0x358, 5, __NA_, 0, MX51_UART_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT1__USBH3_DIR		IOMUX_PAD(0x760, 0x358, 3, 0xa1c, 1, NO_PAD_CTRL)
@@ -692,17 +692,17 @@
 #define MX51_PAD_DISP2_DAT9__USBH3_DATA1	IOMUX_PAD(0x780, 0x378, 3, 0xa00, 1, NO_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT10__DISP2_DAT10	IOMUX_PAD(0x784, 0x37c, 0, __NA_, 0, NO_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT10__DISP2_SER_CS	IOMUX_PAD(0x784, 0x37c, 5, __NA_, 0, NO_PAD_CTRL)
-#define MX51_PAD_DISP2_DAT10__FEC_COL		IOMUX_PAD(0x784, 0x37c, 2, 0x94c, 1, NO_PAD_CTRL)
+#define MX51_PAD_DISP2_DAT10__FEC_COL		IOMUX_PAD(0x784, 0x37c, 2, 0x94c, 1, MX51_PAD_CTRL_5)
 #define MX51_PAD_DISP2_DAT10__KEY_ROW7		IOMUX_PAD(0x784, 0x37c, 4, 0x9dc, 1, NO_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT10__USBH3_DATA2	IOMUX_PAD(0x784, 0x37c, 3, 0xa04, 1, NO_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT11__AUD6_TXD		IOMUX_PAD(0x788, 0x380, 4, 0x8f0, 1, NO_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT11__DISP2_DAT11	IOMUX_PAD(0x788, 0x380, 0, __NA_, 0, NO_PAD_CTRL)
-#define MX51_PAD_DISP2_DAT11__FEC_RX_CLK	IOMUX_PAD(0x788, 0x380, 2, 0x968, 1, NO_PAD_CTRL)
+#define MX51_PAD_DISP2_DAT11__FEC_RX_CLK	IOMUX_PAD(0x788, 0x380, 2, 0x968, 1, MX51_PAD_CTRL_5)
 #define MX51_PAD_DISP2_DAT11__GPIO1_10		IOMUX_PAD(0x788, 0x380, 7, __NA_, 0, MX51_GPIO_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT11__USBH3_DATA3	IOMUX_PAD(0x788, 0x380, 3, 0xa08, 1, NO_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT12__AUD6_RXD		IOMUX_PAD(0x78c, 0x384, 4, 0x8ec, 1, NO_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT12__DISP2_DAT12	IOMUX_PAD(0x78c, 0x384, 0, __NA_, 0, NO_PAD_CTRL)
-#define MX51_PAD_DISP2_DAT12__FEC_RX_DV		IOMUX_PAD(0x78c, 0x384, 2, 0x96c, 1, NO_PAD_CTRL)
+#define MX51_PAD_DISP2_DAT12__FEC_RX_DV		IOMUX_PAD(0x78c, 0x384, 2, 0x96c, 1, MX51_PAD_CTRL_5)
 #define MX51_PAD_DISP2_DAT12__USBH3_DATA4	IOMUX_PAD(0x78c, 0x384, 3, 0xa0c, 1, NO_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT13__AUD6_TXC		IOMUX_PAD(0x790, 0x388, 4, 0x8fc, 1, NO_PAD_CTRL)
 #define MX51_PAD_DISP2_DAT13__DISP2_DAT13	IOMUX_PAD(0x790, 0x388, 0, __NA_, 0, NO_PAD_CTRL)
-- 
1.7.7.6


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 3/3] miidev: fix 1G wrong detection
  2012-09-06 19:39 [PATCH 1/3] fec: restart autoneg at open instead of init Eric Bénard
  2012-09-06 19:39 ` [PATCH 2/3] i.MX51: unbreak FEC iomux Eric Bénard
@ 2012-09-06 19:39 ` Eric Bénard
  2012-09-06 21:52   ` Jean-Christophe PLAGNIOL-VILLARD
                     ` (3 more replies)
  2012-09-07  6:57 ` [PATCH 1/3] fec: restart autoneg at open instead of init Sascha Hauer
  2 siblings, 4 replies; 18+ messages in thread
From: Eric Bénard @ 2012-09-06 19:39 UTC (permalink / raw)
  To: barebox

since 99e72c8bbdbdc690025a5868d831f1fe79ad56fc on an i.MX51 based board,
I get : "phy0: Link is up - 1000/Full". It seems miidev tries to probe
the PHY to early and gets 0x3ffff which leads to the wrong capabilities
setting.

Signed-off-by: Eric Bénard <eric@eukrea.com>
---
 drivers/net/miidev.c |   33 +++++++++------------------------
 1 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/drivers/net/miidev.c b/drivers/net/miidev.c
index 75b53e3..e0f9d67 100644
--- a/drivers/net/miidev.c
+++ b/drivers/net/miidev.c
@@ -131,6 +131,14 @@ int miidev_get_status(struct mii_device *mdev)
 
 	status = ret & BMSR_LSTATUS ? MIIDEV_STATUS_IS_UP : 0;
 
+	if (ret & BMSR_ESTATEN) {
+		ret = mii_read(mdev, mdev->address, MII_ESTATUS);
+		if (ret < 0)
+			goto err_out;
+		if (ret & (ESTATUS_1000_TFULL | ESTATUS_1000_THALF))
+			mdev->capabilities = MIIDEV_CAPABLE_1000M;
+	}
+
 	ret = mii_read(mdev, mdev->address, MII_BMCR);
 	if (ret < 0)
 		goto err_out;
@@ -239,27 +247,8 @@ static struct file_operations miidev_ops = {
 static int miidev_probe(struct device_d *dev)
 {
 	struct mii_device *mdev = dev->priv;
-	int val;
-	int caps = 0;
 
-	val = mii_read(mdev, mdev->address, MII_PHYSID1);
-	if (val < 0 || val == 0xffff)
-		goto err_out;
-	val = mii_read(mdev, mdev->address, MII_PHYSID2);
-	if (val < 0 || val == 0xffff)
-		goto err_out;
-	val = mii_read(mdev, mdev->address, MII_BMSR);
-	if (val < 0)
-		goto err_out;
-	if (val & BMSR_ESTATEN) {
-		val = mii_read(mdev, mdev->address, MII_ESTATUS);
-		if (val < 0)
-			goto err_out;
-		if (val & (ESTATUS_1000_TFULL | ESTATUS_1000_THALF))
-			caps = MIIDEV_CAPABLE_1000M;
-	}
-
-	mdev->capabilities = caps;
+	mdev->capabilities = 0;
 	mdev->cdev.name = asprintf("phy%d", dev->id);
 	mdev->cdev.size = 64;
 	mdev->cdev.ops = &miidev_ops;
@@ -268,10 +257,6 @@ static int miidev_probe(struct device_d *dev)
 	devfs_create(&mdev->cdev);
 	list_add_tail(&mdev->list, &miidev_list);
 	return 0;
-
-err_out:
-	dev_err(dev, "cannot read PHY registers (addr %d)\n", mdev->address);
-	return -ENODEV;
 }
 
 static void miidev_remove(struct device_d *dev)
-- 
1.7.7.6


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] miidev: fix 1G wrong detection
  2012-09-06 19:39 ` [PATCH 3/3] miidev: fix 1G wrong detection Eric Bénard
@ 2012-09-06 21:52   ` Jean-Christophe PLAGNIOL-VILLARD
  2012-09-07  6:44   ` Sascha Hauer
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 18+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-09-06 21:52 UTC (permalink / raw)
  To: Eric Bénard; +Cc: barebox

On 21:39 Thu 06 Sep     , Eric Bénard wrote:
> since 99e72c8bbdbdc690025a5868d831f1fe79ad56fc on an i.MX51 based board,
> I get : "phy0: Link is up - 1000/Full". It seems miidev tries to probe
> the PHY to early and gets 0x3ffff which leads to the wrong capabilities
> setting.
> 
> Signed-off-by: Eric Bénard <eric@eukrea.com>
Wait I going to drop this code instead of the phylib

Best Regards,
J.
> ---
>  drivers/net/miidev.c |   33 +++++++++------------------------
>  1 files changed, 9 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/miidev.c b/drivers/net/miidev.c
> index 75b53e3..e0f9d67 100644
> --- a/drivers/net/miidev.c
> +++ b/drivers/net/miidev.c
> @@ -131,6 +131,14 @@ int miidev_get_status(struct mii_device *mdev)
>  
>  	status = ret & BMSR_LSTATUS ? MIIDEV_STATUS_IS_UP : 0;
>  
> +	if (ret & BMSR_ESTATEN) {
> +		ret = mii_read(mdev, mdev->address, MII_ESTATUS);
> +		if (ret < 0)
> +			goto err_out;
> +		if (ret & (ESTATUS_1000_TFULL | ESTATUS_1000_THALF))
> +			mdev->capabilities = MIIDEV_CAPABLE_1000M;
> +	}
> +
>  	ret = mii_read(mdev, mdev->address, MII_BMCR);
>  	if (ret < 0)
>  		goto err_out;
> @@ -239,27 +247,8 @@ static struct file_operations miidev_ops = {
>  static int miidev_probe(struct device_d *dev)
>  {
>  	struct mii_device *mdev = dev->priv;
> -	int val;
> -	int caps = 0;
>  
> -	val = mii_read(mdev, mdev->address, MII_PHYSID1);
> -	if (val < 0 || val == 0xffff)
> -		goto err_out;
> -	val = mii_read(mdev, mdev->address, MII_PHYSID2);
> -	if (val < 0 || val == 0xffff)
> -		goto err_out;
> -	val = mii_read(mdev, mdev->address, MII_BMSR);
> -	if (val < 0)
> -		goto err_out;
> -	if (val & BMSR_ESTATEN) {
> -		val = mii_read(mdev, mdev->address, MII_ESTATUS);
> -		if (val < 0)
> -			goto err_out;
> -		if (val & (ESTATUS_1000_TFULL | ESTATUS_1000_THALF))
> -			caps = MIIDEV_CAPABLE_1000M;
> -	}
> -
> -	mdev->capabilities = caps;
> +	mdev->capabilities = 0;
>  	mdev->cdev.name = asprintf("phy%d", dev->id);
>  	mdev->cdev.size = 64;
>  	mdev->cdev.ops = &miidev_ops;
> @@ -268,10 +257,6 @@ static int miidev_probe(struct device_d *dev)
>  	devfs_create(&mdev->cdev);
>  	list_add_tail(&mdev->list, &miidev_list);
>  	return 0;
> -
> -err_out:
> -	dev_err(dev, "cannot read PHY registers (addr %d)\n", mdev->address);
> -	return -ENODEV;
>  }
>  
>  static void miidev_remove(struct device_d *dev)
> -- 
> 1.7.7.6
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] miidev: fix 1G wrong detection
  2012-09-06 19:39 ` [PATCH 3/3] miidev: fix 1G wrong detection Eric Bénard
  2012-09-06 21:52   ` Jean-Christophe PLAGNIOL-VILLARD
@ 2012-09-07  6:44   ` Sascha Hauer
  2012-09-07  7:28     ` Eric Bénard
  2012-09-07  7:43     ` Jean-Christophe PLAGNIOL-VILLARD
  2012-09-07  8:52   ` Johannes Stezenbach
  2012-09-14  7:03   ` Sascha Hauer
  3 siblings, 2 replies; 18+ messages in thread
From: Sascha Hauer @ 2012-09-07  6:44 UTC (permalink / raw)
  To: Eric Bénard; +Cc: barebox

On Thu, Sep 06, 2012 at 09:39:31PM +0200, Eric Bénard wrote:
> since 99e72c8bbdbdc690025a5868d831f1fe79ad56fc on an i.MX51 based board,
> I get : "phy0: Link is up - 1000/Full". It seems miidev tries to probe
> the PHY to early and gets 0x3ffff which leads to the wrong capabilities
> setting.
> 
> Signed-off-by: Eric Bénard <eric@eukrea.com>
> ---
>  drivers/net/miidev.c |   33 +++++++++------------------------
>  1 files changed, 9 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/miidev.c b/drivers/net/miidev.c
> index 75b53e3..e0f9d67 100644
> --- a/drivers/net/miidev.c
> +++ b/drivers/net/miidev.c
> @@ -131,6 +131,14 @@ int miidev_get_status(struct mii_device *mdev)
>  
>  	status = ret & BMSR_LSTATUS ? MIIDEV_STATUS_IS_UP : 0;
>  
> +	if (ret & BMSR_ESTATEN) {
> +		ret = mii_read(mdev, mdev->address, MII_ESTATUS);
> +		if (ret < 0)
> +			goto err_out;
> +		if (ret & (ESTATUS_1000_TFULL | ESTATUS_1000_THALF))
> +			mdev->capabilities = MIIDEV_CAPABLE_1000M;
> +	}
> +
>  	ret = mii_read(mdev, mdev->address, MII_BMCR);
>  	if (ret < 0)
>  		goto err_out;
> @@ -239,27 +247,8 @@ static struct file_operations miidev_ops = {
>  static int miidev_probe(struct device_d *dev)
>  {
>  	struct mii_device *mdev = dev->priv;
> -	int val;
> -	int caps = 0;
>  
> -	val = mii_read(mdev, mdev->address, MII_PHYSID1);
> -	if (val < 0 || val == 0xffff)
> -		goto err_out;
> -	val = mii_read(mdev, mdev->address, MII_PHYSID2);
> -	if (val < 0 || val == 0xffff)
> -		goto err_out;

Is the above necessary? This was present before the patch which made the
regression.

Sascha

> -	val = mii_read(mdev, mdev->address, MII_BMSR);
> -	if (val < 0)
> -		goto err_out;
> -	if (val & BMSR_ESTATEN) {
> -		val = mii_read(mdev, mdev->address, MII_ESTATUS);
> -		if (val < 0)
> -			goto err_out;
> -		if (val & (ESTATUS_1000_TFULL | ESTATUS_1000_THALF))
> -			caps = MIIDEV_CAPABLE_1000M;
> -	}
> -
> -	mdev->capabilities = caps;
> +	mdev->capabilities = 0;
>  	mdev->cdev.name = asprintf("phy%d", dev->id);
>  	mdev->cdev.size = 64;
>  	mdev->cdev.ops = &miidev_ops;
> @@ -268,10 +257,6 @@ static int miidev_probe(struct device_d *dev)
>  	devfs_create(&mdev->cdev);
>  	list_add_tail(&mdev->list, &miidev_list);
>  	return 0;
> -
> -err_out:
> -	dev_err(dev, "cannot read PHY registers (addr %d)\n", mdev->address);
> -	return -ENODEV;
>  }
>  
>  static void miidev_remove(struct device_d *dev)
> -- 
> 1.7.7.6
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] fec: restart autoneg at open instead of init
  2012-09-06 19:39 [PATCH 1/3] fec: restart autoneg at open instead of init Eric Bénard
  2012-09-06 19:39 ` [PATCH 2/3] i.MX51: unbreak FEC iomux Eric Bénard
  2012-09-06 19:39 ` [PATCH 3/3] miidev: fix 1G wrong detection Eric Bénard
@ 2012-09-07  6:57 ` Sascha Hauer
  2012-09-07  7:30   ` Eric Bénard
  2 siblings, 1 reply; 18+ messages in thread
From: Sascha Hauer @ 2012-09-07  6:57 UTC (permalink / raw)
  To: Eric Bénard; +Cc: barebox

Hi Eric,

On Thu, Sep 06, 2012 at 09:39:29PM +0200, Eric Bénard wrote:
> this saves some time during boot when ethernet is not needed in barebox

Is this really significant on your hardware? I made a measurement here
and the miidev_restart_aneg() takes 130us on an i.MX27 board.

The original intention of doing it at init time was that autonegotation
is already completed when we actually use the network. And this time
really is significant:

without the patch:

barebox@Phytec phyCORE-i.MX27:/ time ping 192.168.23.4
warning: No MAC address set. Using random address 62:FE:FE:0B:09:A8
phy0: Link is up - 100/Full
host 192.168.23.4 is alive
time: 10ms

with the patch:

barebox@Phytec phyCORE-i.MX27:/ time ping 192.168.23.4
warning: No MAC address set. Using random address A6:1B:AB:80:FA:E7
phy0: Link is up - 100/Full
host 192.168.23.4 is alive
time: 2040ms


Sascha

> 
> Signed-off-by: Eric Bénard <eric@eukrea.com>
> ---
>  drivers/net/fec_imx.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/fec_imx.c b/drivers/net/fec_imx.c
> index 599a9b4..2bc06a9 100644
> --- a/drivers/net/fec_imx.c
> +++ b/drivers/net/fec_imx.c
> @@ -347,9 +347,6 @@ static int fec_init(struct eth_device *dev)
>  	/* size of each buffer */
>  	writel(FEC_MAX_PKT_SIZE, fec->regs + FEC_EMRBR);
>  
> -	if (fec->xcv_type != SEVENWIRE)
> -		miidev_restart_aneg(&fec->miidev);
> -
>  	return 0;
>  }
>  
> @@ -363,6 +360,9 @@ static int fec_open(struct eth_device *edev)
>  	int ret;
>  	u32 ecr;
>  
> +	if (fec->xcv_type != SEVENWIRE)
> +		miidev_restart_aneg(&fec->miidev);
> +
>  	/*
>  	 * Initialize RxBD/TxBD rings
>  	 */
> -- 
> 1.7.7.6
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] miidev: fix 1G wrong detection
  2012-09-07  6:44   ` Sascha Hauer
@ 2012-09-07  7:28     ` Eric Bénard
  2012-09-07  8:23       ` Sascha Hauer
  2012-09-07  7:43     ` Jean-Christophe PLAGNIOL-VILLARD
  1 sibling, 1 reply; 18+ messages in thread
From: Eric Bénard @ 2012-09-07  7:28 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Le Fri, 7 Sep 2012 08:44:19 +0200,
Sascha Hauer <s.hauer@pengutronix.de> a écrit :

> On Thu, Sep 06, 2012 at 09:39:31PM +0200, Eric Bénard wrote:
> > since 99e72c8bbdbdc690025a5868d831f1fe79ad56fc on an i.MX51 based board,
> > I get : "phy0: Link is up - 1000/Full". It seems miidev tries to probe
> > the PHY to early and gets 0x3ffff which leads to the wrong capabilities
> > setting.
> > 
> > Signed-off-by: Eric Bénard <eric@eukrea.com>
> > ---
> >  drivers/net/miidev.c |   33 +++++++++------------------------
> >  1 files changed, 9 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/net/miidev.c b/drivers/net/miidev.c
> > index 75b53e3..e0f9d67 100644
> > --- a/drivers/net/miidev.c
> > +++ b/drivers/net/miidev.c
> > @@ -131,6 +131,14 @@ int miidev_get_status(struct mii_device *mdev)
> >  
> >  	status = ret & BMSR_LSTATUS ? MIIDEV_STATUS_IS_UP : 0;
> >  
> > +	if (ret & BMSR_ESTATEN) {
> > +		ret = mii_read(mdev, mdev->address, MII_ESTATUS);
> > +		if (ret < 0)
> > +			goto err_out;
> > +		if (ret & (ESTATUS_1000_TFULL | ESTATUS_1000_THALF))
> > +			mdev->capabilities = MIIDEV_CAPABLE_1000M;
> > +	}
> > +
> >  	ret = mii_read(mdev, mdev->address, MII_BMCR);
> >  	if (ret < 0)
> >  		goto err_out;
> > @@ -239,27 +247,8 @@ static struct file_operations miidev_ops = {
> >  static int miidev_probe(struct device_d *dev)
> >  {
> >  	struct mii_device *mdev = dev->priv;
> > -	int val;
> > -	int caps = 0;
> >  
> > -	val = mii_read(mdev, mdev->address, MII_PHYSID1);
> > -	if (val < 0 || val == 0xffff)
> > -		goto err_out;
> > -	val = mii_read(mdev, mdev->address, MII_PHYSID2);
> > -	if (val < 0 || val == 0xffff)
> > -		goto err_out;
> 
> Is the above necessary? This was present before the patch which made the
> regression.
> 
well there is no interest as we don't check the value read (and in the
present case I also get 0x3ffff here).

Eric

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] fec: restart autoneg at open instead of init
  2012-09-07  6:57 ` [PATCH 1/3] fec: restart autoneg at open instead of init Sascha Hauer
@ 2012-09-07  7:30   ` Eric Bénard
  2012-09-07  7:47     ` Sascha Hauer
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Bénard @ 2012-09-07  7:30 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hi Sascha,

Le Fri, 7 Sep 2012 08:57:03 +0200,
Sascha Hauer <s.hauer@pengutronix.de> a écrit :

> Hi Eric,
> 
> On Thu, Sep 06, 2012 at 09:39:29PM +0200, Eric Bénard wrote:
> > this saves some time during boot when ethernet is not needed in barebox
> 
> Is this really significant on your hardware? I made a measurement here
> and the miidev_restart_aneg() takes 130us on an i.MX27 board.
> 
> The original intention of doing it at init time was that autonegotation
> is already completed when we actually use the network. And this time
> really is significant:
> 
> without the patch:
> 
> barebox@Phytec phyCORE-i.MX27:/ time ping 192.168.23.4
> warning: No MAC address set. Using random address 62:FE:FE:0B:09:A8
> phy0: Link is up - 100/Full
> host 192.168.23.4 is alive
> time: 10ms
> 
> with the patch:
> 
> barebox@Phytec phyCORE-i.MX27:/ time ping 192.168.23.4
> warning: No MAC address set. Using random address A6:1B:AB:80:FA:E7
> phy0: Link is up - 100/Full
> host 192.168.23.4 is alive
> time: 2040ms
> 
that strange. I don't see why miidev_restart_aneg() would take less
time at boot than at open.

Eric

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] miidev: fix 1G wrong detection
  2012-09-07  6:44   ` Sascha Hauer
  2012-09-07  7:28     ` Eric Bénard
@ 2012-09-07  7:43     ` Jean-Christophe PLAGNIOL-VILLARD
  1 sibling, 0 replies; 18+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-09-07  7:43 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On 08:44 Fri 07 Sep     , Sascha Hauer wrote:
> On Thu, Sep 06, 2012 at 09:39:31PM +0200, Eric Bénard wrote:
> > since 99e72c8bbdbdc690025a5868d831f1fe79ad56fc on an i.MX51 based board,
> > I get : "phy0: Link is up - 1000/Full". It seems miidev tries to probe
> > the PHY to early and gets 0x3ffff which leads to the wrong capabilities
> > setting.
> > 
> > Signed-off-by: Eric Bénard <eric@eukrea.com>
> > ---
> >  drivers/net/miidev.c |   33 +++++++++------------------------
> >  1 files changed, 9 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/net/miidev.c b/drivers/net/miidev.c
> > index 75b53e3..e0f9d67 100644
> > --- a/drivers/net/miidev.c
> > +++ b/drivers/net/miidev.c
> > @@ -131,6 +131,14 @@ int miidev_get_status(struct mii_device *mdev)
> >  
> >  	status = ret & BMSR_LSTATUS ? MIIDEV_STATUS_IS_UP : 0;
> >  
> > +	if (ret & BMSR_ESTATEN) {
> > +		ret = mii_read(mdev, mdev->address, MII_ESTATUS);
> > +		if (ret < 0)
> > +			goto err_out;
> > +		if (ret & (ESTATUS_1000_TFULL | ESTATUS_1000_THALF))
> > +			mdev->capabilities = MIIDEV_CAPABLE_1000M;
> > +	}
> > +
> >  	ret = mii_read(mdev, mdev->address, MII_BMCR);
> >  	if (ret < 0)
> >  		goto err_out;
> > @@ -239,27 +247,8 @@ static struct file_operations miidev_ops = {
> >  static int miidev_probe(struct device_d *dev)
> >  {
> >  	struct mii_device *mdev = dev->priv;
> > -	int val;
> > -	int caps = 0;
> >  
> > -	val = mii_read(mdev, mdev->address, MII_PHYSID1);
> > -	if (val < 0 || val == 0xffff)
> > -		goto err_out;
> > -	val = mii_read(mdev, mdev->address, MII_PHYSID2);
> > -	if (val < 0 || val == 0xffff)
> > -		goto err_out;
> 
> Is the above necessary? This was present before the patch which made the
> regression.
yes the previous patch made a regression

as it expect the mii to be ready at probe vs open before

Best Regards,
J.
> 
> Sascha
> 
> > -	val = mii_read(mdev, mdev->address, MII_BMSR);
> > -	if (val < 0)
> > -		goto err_out;
> > -	if (val & BMSR_ESTATEN) {
> > -		val = mii_read(mdev, mdev->address, MII_ESTATUS);
> > -		if (val < 0)
> > -			goto err_out;
> > -		if (val & (ESTATUS_1000_TFULL | ESTATUS_1000_THALF))
> > -			caps = MIIDEV_CAPABLE_1000M;
> > -	}
> > -
> > -	mdev->capabilities = caps;
> > +	mdev->capabilities = 0;
> >  	mdev->cdev.name = asprintf("phy%d", dev->id);
> >  	mdev->cdev.size = 64;
> >  	mdev->cdev.ops = &miidev_ops;
> > @@ -268,10 +257,6 @@ static int miidev_probe(struct device_d *dev)
> >  	devfs_create(&mdev->cdev);
> >  	list_add_tail(&mdev->list, &miidev_list);
> >  	return 0;
> > -
> > -err_out:
> > -	dev_err(dev, "cannot read PHY registers (addr %d)\n", mdev->address);
> > -	return -ENODEV;
> >  }
> >  
> >  static void miidev_remove(struct device_d *dev)
> > -- 
> > 1.7.7.6
> > 
> > 
> > _______________________________________________
> > barebox mailing list
> > barebox@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/barebox
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] fec: restart autoneg at open instead of init
  2012-09-07  7:30   ` Eric Bénard
@ 2012-09-07  7:47     ` Sascha Hauer
  2012-09-07  7:51       ` Eric Bénard
  0 siblings, 1 reply; 18+ messages in thread
From: Sascha Hauer @ 2012-09-07  7:47 UTC (permalink / raw)
  To: Eric Bénard; +Cc: barebox

On Fri, Sep 07, 2012 at 09:30:00AM +0200, Eric Bénard wrote:
> Hi Sascha,
> 
> Le Fri, 7 Sep 2012 08:57:03 +0200,
> Sascha Hauer <s.hauer@pengutronix.de> a écrit :
> 
> > Hi Eric,
> > 
> > On Thu, Sep 06, 2012 at 09:39:29PM +0200, Eric Bénard wrote:
> > > this saves some time during boot when ethernet is not needed in barebox
> > 
> > Is this really significant on your hardware? I made a measurement here
> > and the miidev_restart_aneg() takes 130us on an i.MX27 board.
> > 
> > The original intention of doing it at init time was that autonegotation
> > is already completed when we actually use the network. And this time
> > really is significant:
> > 
> > without the patch:
> > 
> > barebox@Phytec phyCORE-i.MX27:/ time ping 192.168.23.4
> > warning: No MAC address set. Using random address 62:FE:FE:0B:09:A8
> > phy0: Link is up - 100/Full
> > host 192.168.23.4 is alive
> > time: 10ms
> > 
> > with the patch:
> > 
> > barebox@Phytec phyCORE-i.MX27:/ time ping 192.168.23.4
> > warning: No MAC address set. Using random address A6:1B:AB:80:FA:E7
> > phy0: Link is up - 100/Full
> > host 192.168.23.4 is alive
> > time: 2040ms
> > 
> that strange. I don't see why miidev_restart_aneg() would take less
> time at boot than at open.

Well, it does not really take longer. The difference is that if you are
sitting before your board it usually takes a few seconds to interrupt
autoboot, think what you want to do, and finally enter some network
command. In the meantime autonegotiation will be complete. If we start
autonegotiation at open time, we will always have to wait for the full
autonegotiation time after starting the first network command.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] fec: restart autoneg at open instead of init
  2012-09-07  7:47     ` Sascha Hauer
@ 2012-09-07  7:51       ` Eric Bénard
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Bénard @ 2012-09-07  7:51 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hi Sascha,

Le Fri, 7 Sep 2012 09:47:34 +0200,
Sascha Hauer <s.hauer@pengutronix.de> a écrit :

> On Fri, Sep 07, 2012 at 09:30:00AM +0200, Eric Bénard wrote:
> > Hi Sascha,
> > 
> > Le Fri, 7 Sep 2012 08:57:03 +0200,
> > Sascha Hauer <s.hauer@pengutronix.de> a écrit :
> > 
> > > Hi Eric,
> > > 
> > > On Thu, Sep 06, 2012 at 09:39:29PM +0200, Eric Bénard wrote:
> > > > this saves some time during boot when ethernet is not needed in barebox
> > > 
> > > Is this really significant on your hardware? I made a measurement here
> > > and the miidev_restart_aneg() takes 130us on an i.MX27 board.
> > > 
> > > The original intention of doing it at init time was that autonegotation
> > > is already completed when we actually use the network. And this time
> > > really is significant:
> > > 
> > > without the patch:
> > > 
> > > barebox@Phytec phyCORE-i.MX27:/ time ping 192.168.23.4
> > > warning: No MAC address set. Using random address 62:FE:FE:0B:09:A8
> > > phy0: Link is up - 100/Full
> > > host 192.168.23.4 is alive
> > > time: 10ms
> > > 
> > > with the patch:
> > > 
> > > barebox@Phytec phyCORE-i.MX27:/ time ping 192.168.23.4
> > > warning: No MAC address set. Using random address A6:1B:AB:80:FA:E7
> > > phy0: Link is up - 100/Full
> > > host 192.168.23.4 is alive
> > > time: 2040ms
> > > 
> > that strange. I don't see why miidev_restart_aneg() would take less
> > time at boot than at open.
> 
> Well, it does not really take longer. The difference is that if you are
> sitting before your board it usually takes a few seconds to interrupt
> autoboot, think what you want to do, and finally enter some network
> command. In the meantime autonegotiation will be complete. If we start
> autonegotiation at open time, we will always have to wait for the full
> autonegotiation time after starting the first network command.
> 
I didn't check but on my board, the problem may be similar to the 1G
wrong detection : if miidev_restart_aneg is trying to acess the phy
when the MII link is not yet initialized, then it would wait 1s for
the reset to timeout in miidev_restart_aneg which would explain why I
save 1s with this patch.

Eric

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] miidev: fix 1G wrong detection
  2012-09-07  7:28     ` Eric Bénard
@ 2012-09-07  8:23       ` Sascha Hauer
  0 siblings, 0 replies; 18+ messages in thread
From: Sascha Hauer @ 2012-09-07  8:23 UTC (permalink / raw)
  To: Eric Bénard; +Cc: barebox

On Fri, Sep 07, 2012 at 09:28:30AM +0200, Eric Bénard wrote:
> Le Fri, 7 Sep 2012 08:44:19 +0200,
> Sascha Hauer <s.hauer@pengutronix.de> a écrit :
> 
> > On Thu, Sep 06, 2012 at 09:39:31PM +0200, Eric Bénard wrote:
> > > since 99e72c8bbdbdc690025a5868d831f1fe79ad56fc on an i.MX51 based board,
> > > I get : "phy0: Link is up - 1000/Full". It seems miidev tries to probe
> > > the PHY to early and gets 0x3ffff which leads to the wrong capabilities
> > > setting.
> > > 
> > > Signed-off-by: Eric Bénard <eric@eukrea.com>
> > > ---
> > >  drivers/net/miidev.c |   33 +++++++++------------------------
> > >  1 files changed, 9 insertions(+), 24 deletions(-)
> > > 
> > > diff --git a/drivers/net/miidev.c b/drivers/net/miidev.c
> > > index 75b53e3..e0f9d67 100644
> > > --- a/drivers/net/miidev.c
> > > +++ b/drivers/net/miidev.c
> > > @@ -131,6 +131,14 @@ int miidev_get_status(struct mii_device *mdev)
> > >  
> > >  	status = ret & BMSR_LSTATUS ? MIIDEV_STATUS_IS_UP : 0;
> > >  
> > > +	if (ret & BMSR_ESTATEN) {
> > > +		ret = mii_read(mdev, mdev->address, MII_ESTATUS);
> > > +		if (ret < 0)
> > > +			goto err_out;
> > > +		if (ret & (ESTATUS_1000_TFULL | ESTATUS_1000_THALF))
> > > +			mdev->capabilities = MIIDEV_CAPABLE_1000M;
> > > +	}
> > > +
> > >  	ret = mii_read(mdev, mdev->address, MII_BMCR);
> > >  	if (ret < 0)
> > >  		goto err_out;
> > > @@ -239,27 +247,8 @@ static struct file_operations miidev_ops = {
> > >  static int miidev_probe(struct device_d *dev)
> > >  {
> > >  	struct mii_device *mdev = dev->priv;
> > > -	int val;
> > > -	int caps = 0;
> > >  
> > > -	val = mii_read(mdev, mdev->address, MII_PHYSID1);
> > > -	if (val < 0 || val == 0xffff)
> > > -		goto err_out;
> > > -	val = mii_read(mdev, mdev->address, MII_PHYSID2);
> > > -	if (val < 0 || val == 0xffff)
> > > -		goto err_out;
> > 
> > Is the above necessary? This was present before the patch which made the
> > regression.
> > 
> well there is no interest as we don't check the value read (and in the
> present case I also get 0x3ffff here).

Ah, I see. I thought the above lines were there for longer, but they
were introduced right before the commit adding 1000mbit support.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] i.MX51: unbreak FEC iomux
  2012-09-06 19:39 ` [PATCH 2/3] i.MX51: unbreak FEC iomux Eric Bénard
@ 2012-09-07  8:30   ` Sascha Hauer
  0 siblings, 0 replies; 18+ messages in thread
From: Sascha Hauer @ 2012-09-07  8:30 UTC (permalink / raw)
  To: Eric Bénard; +Cc: barebox

Hi Eric,

On Thu, Sep 06, 2012 at 09:39:30PM +0200, Eric Bénard wrote:
> in commit 2bdc9f57a86dff41cfc1f87b644a2e53fdcce2b6 the iomux was synced
> with the kernel but this leads to some changes in the PAD_CTRL of some
> FEC pins leading to a non working FEC on our cpuimx51 board.
> 
> This patch set back the PAD_CTRL of the missing pins to the initial
> value.

I normally say that I want to keep the iomux files in sync with the
kernel which means send it for inclusion to the kernel first.

Anyway, the days of these files in the kernel seem to be counted, they
will go away pretty soon I guess.

So I applied this one.

Sascha

> 
> Signed-off-by: Eric Bénard <eric@eukrea.com>
> ---
>  arch/arm/mach-imx/include/mach/iomux-mx51.h |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/include/mach/iomux-mx51.h b/arch/arm/mach-imx/include/mach/iomux-mx51.h
> index 36c8989..0252d41 100644
> --- a/arch/arm/mach-imx/include/mach/iomux-mx51.h
> +++ b/arch/arm/mach-imx/include/mach/iomux-mx51.h
> @@ -649,20 +649,20 @@
>  #define MX51_PAD_DI2_PIN2__DI2_PIN2		IOMUX_PAD(0x74c, 0x344, 0, __NA_, 0, NO_PAD_CTRL)
>  #define MX51_PAD_DI2_PIN2__FEC_MDC		IOMUX_PAD(0x74c, 0x344, 2, __NA_, 0, MX51_PAD_CTRL_5)
>  #define MX51_PAD_DI2_PIN3__DI2_PIN3		IOMUX_PAD(0x750, 0x348, 0, __NA_, 0, NO_PAD_CTRL)
> -#define MX51_PAD_DI2_PIN3__FEC_MDIO		IOMUX_PAD(0x750, 0x348, 2, 0x954, 1, NO_PAD_CTRL)
> +#define MX51_PAD_DI2_PIN3__FEC_MDIO		IOMUX_PAD(0x750, 0x348, 2, 0x954, 1, MX51_PAD_CTRL_5)
>  #define MX51_PAD_DI2_DISP_CLK__DI2_DISP_CLK	IOMUX_PAD(0x754, 0x34c, 0, __NA_, 0, NO_PAD_CTRL)
>  #define MX51_PAD_DI2_DISP_CLK__FEC_RDATA1	IOMUX_PAD(0x754, 0x34c, 2, 0x95c, 1, NO_PAD_CTRL)
>  #define MX51_PAD_DI_GP4__DI2_PIN15		IOMUX_PAD(0x758, 0x350, 4, __NA_, 0, NO_PAD_CTRL)
>  #define MX51_PAD_DI_GP4__DISP1_SER_DIN		IOMUX_PAD(0x758, 0x350, 0, 0x9c0, 1, NO_PAD_CTRL)
>  #define MX51_PAD_DI_GP4__DISP2_PIN1		IOMUX_PAD(0x758, 0x350, 3, __NA_, 0, NO_PAD_CTRL)
> -#define MX51_PAD_DI_GP4__FEC_RDATA2		IOMUX_PAD(0x758, 0x350, 2, 0x960, 1, NO_PAD_CTRL)
> +#define MX51_PAD_DI_GP4__FEC_RDATA2		IOMUX_PAD(0x758, 0x350, 2, 0x960, 1, MX51_PAD_CTRL_5)
>  #define MX51_PAD_DISP2_DAT0__DISP2_DAT0		IOMUX_PAD(0x75c, 0x354, 0, __NA_, 0, NO_PAD_CTRL)
> -#define MX51_PAD_DISP2_DAT0__FEC_RDATA3		IOMUX_PAD(0x75c, 0x354, 2, 0x964, 1, NO_PAD_CTRL)
> +#define MX51_PAD_DISP2_DAT0__FEC_RDATA3		IOMUX_PAD(0x75c, 0x354, 2, 0x964, 1, MX51_PAD_CTRL_5)
>  #define MX51_PAD_DISP2_DAT0__KEY_COL6		IOMUX_PAD(0x75c, 0x354, 4, 0x9c8, 1, NO_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT0__UART3_RXD		IOMUX_PAD(0x75c, 0x354, 5, 0x9f4, 8, MX51_UART_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT0__USBH3_CLK		IOMUX_PAD(0x75c, 0x354, 3, 0x9f8, 1, MX51_UART_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT1__DISP2_DAT1		IOMUX_PAD(0x760, 0x358, 0, __NA_, 0, NO_PAD_CTRL)
> -#define MX51_PAD_DISP2_DAT1__FEC_RX_ER		IOMUX_PAD(0x760, 0x358, 2, 0x970, 1, NO_PAD_CTRL)
> +#define MX51_PAD_DISP2_DAT1__FEC_RX_ER		IOMUX_PAD(0x760, 0x358, 2, 0x970, 1, MX51_PAD_CTRL_5)
>  #define MX51_PAD_DISP2_DAT1__KEY_COL7		IOMUX_PAD(0x760, 0x358, 4, 0x9cc, 1, NO_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT1__UART3_TXD		IOMUX_PAD(0x760, 0x358, 5, __NA_, 0, MX51_UART_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT1__USBH3_DIR		IOMUX_PAD(0x760, 0x358, 3, 0xa1c, 1, NO_PAD_CTRL)
> @@ -692,17 +692,17 @@
>  #define MX51_PAD_DISP2_DAT9__USBH3_DATA1	IOMUX_PAD(0x780, 0x378, 3, 0xa00, 1, NO_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT10__DISP2_DAT10	IOMUX_PAD(0x784, 0x37c, 0, __NA_, 0, NO_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT10__DISP2_SER_CS	IOMUX_PAD(0x784, 0x37c, 5, __NA_, 0, NO_PAD_CTRL)
> -#define MX51_PAD_DISP2_DAT10__FEC_COL		IOMUX_PAD(0x784, 0x37c, 2, 0x94c, 1, NO_PAD_CTRL)
> +#define MX51_PAD_DISP2_DAT10__FEC_COL		IOMUX_PAD(0x784, 0x37c, 2, 0x94c, 1, MX51_PAD_CTRL_5)
>  #define MX51_PAD_DISP2_DAT10__KEY_ROW7		IOMUX_PAD(0x784, 0x37c, 4, 0x9dc, 1, NO_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT10__USBH3_DATA2	IOMUX_PAD(0x784, 0x37c, 3, 0xa04, 1, NO_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT11__AUD6_TXD		IOMUX_PAD(0x788, 0x380, 4, 0x8f0, 1, NO_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT11__DISP2_DAT11	IOMUX_PAD(0x788, 0x380, 0, __NA_, 0, NO_PAD_CTRL)
> -#define MX51_PAD_DISP2_DAT11__FEC_RX_CLK	IOMUX_PAD(0x788, 0x380, 2, 0x968, 1, NO_PAD_CTRL)
> +#define MX51_PAD_DISP2_DAT11__FEC_RX_CLK	IOMUX_PAD(0x788, 0x380, 2, 0x968, 1, MX51_PAD_CTRL_5)
>  #define MX51_PAD_DISP2_DAT11__GPIO1_10		IOMUX_PAD(0x788, 0x380, 7, __NA_, 0, MX51_GPIO_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT11__USBH3_DATA3	IOMUX_PAD(0x788, 0x380, 3, 0xa08, 1, NO_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT12__AUD6_RXD		IOMUX_PAD(0x78c, 0x384, 4, 0x8ec, 1, NO_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT12__DISP2_DAT12	IOMUX_PAD(0x78c, 0x384, 0, __NA_, 0, NO_PAD_CTRL)
> -#define MX51_PAD_DISP2_DAT12__FEC_RX_DV		IOMUX_PAD(0x78c, 0x384, 2, 0x96c, 1, NO_PAD_CTRL)
> +#define MX51_PAD_DISP2_DAT12__FEC_RX_DV		IOMUX_PAD(0x78c, 0x384, 2, 0x96c, 1, MX51_PAD_CTRL_5)
>  #define MX51_PAD_DISP2_DAT12__USBH3_DATA4	IOMUX_PAD(0x78c, 0x384, 3, 0xa0c, 1, NO_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT13__AUD6_TXC		IOMUX_PAD(0x790, 0x388, 4, 0x8fc, 1, NO_PAD_CTRL)
>  #define MX51_PAD_DISP2_DAT13__DISP2_DAT13	IOMUX_PAD(0x790, 0x388, 0, __NA_, 0, NO_PAD_CTRL)
> -- 
> 1.7.7.6
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] miidev: fix 1G wrong detection
  2012-09-06 19:39 ` [PATCH 3/3] miidev: fix 1G wrong detection Eric Bénard
  2012-09-06 21:52   ` Jean-Christophe PLAGNIOL-VILLARD
  2012-09-07  6:44   ` Sascha Hauer
@ 2012-09-07  8:52   ` Johannes Stezenbach
  2012-09-07  9:02     ` Eric Bénard
  2012-09-14  7:03   ` Sascha Hauer
  3 siblings, 1 reply; 18+ messages in thread
From: Johannes Stezenbach @ 2012-09-07  8:52 UTC (permalink / raw)
  To: Eric Bénard; +Cc: barebox

Hi,

On Thu, Sep 06, 2012 at 09:39:31PM +0200, Eric Bénard wrote:
> since 99e72c8bbdbdc690025a5868d831f1fe79ad56fc on an i.MX51 based board,
> I get : "phy0: Link is up - 1000/Full". It seems miidev tries to probe
> the PHY to early and gets 0x3ffff which leads to the wrong capabilities
> setting.

Hm, MII registers are only 16bit, why does your mii_read()
implementation return 0x3ffff?

Also, what exactly do you mean by "too early"?  Your code
shouldn't call mii_register() before the MDIO clock is stable.

However, I have no objection to your change, it should
work for me, too.


Johannes

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] miidev: fix 1G wrong detection
  2012-09-07  8:52   ` Johannes Stezenbach
@ 2012-09-07  9:02     ` Eric Bénard
  2012-09-07  9:28       ` Johannes Stezenbach
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Bénard @ 2012-09-07  9:02 UTC (permalink / raw)
  To: Johannes Stezenbach; +Cc: barebox

Hi,

Le Fri, 7 Sep 2012 10:52:16 +0200,
Johannes Stezenbach <js@sig21.net> a écrit :
> On Thu, Sep 06, 2012 at 09:39:31PM +0200, Eric Bénard wrote:
> > since 99e72c8bbdbdc690025a5868d831f1fe79ad56fc on an i.MX51 based board,
> > I get : "phy0: Link is up - 1000/Full". It seems miidev tries to probe
> > the PHY to early and gets 0x3ffff which leads to the wrong capabilities
> > setting.
> 
> Hm, MII registers are only 16bit, why does your mii_read()
> implementation return 0x3ffff?
> 
in fec_imx it returns the 32 bit register. I though we could mask it to
only return the data but that wouldn't solve the problem as the tests
in miidev would fail because the data is 0xFFFF.

> Also, what exactly do you mean by "too early"?  Your code
> shouldn't call mii_register() before the MDIO clock is stable.
> 
fec_imx.c does that.

Eric

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] miidev: fix 1G wrong detection
  2012-09-07  9:02     ` Eric Bénard
@ 2012-09-07  9:28       ` Johannes Stezenbach
  2012-09-07  9:32         ` Eric Bénard
  0 siblings, 1 reply; 18+ messages in thread
From: Johannes Stezenbach @ 2012-09-07  9:28 UTC (permalink / raw)
  To: Eric Bénard; +Cc: barebox

Hi,

On Fri, Sep 07, 2012 at 11:02:18AM +0200, Eric Bénard wrote:
> Le Fri, 7 Sep 2012 10:52:16 +0200,
> Johannes Stezenbach <js@sig21.net> a écrit :
> > On Thu, Sep 06, 2012 at 09:39:31PM +0200, Eric Bénard wrote:
> > > since 99e72c8bbdbdc690025a5868d831f1fe79ad56fc on an i.MX51 based board,
> > > I get : "phy0: Link is up - 1000/Full". It seems miidev tries to probe
> > > the PHY to early and gets 0x3ffff which leads to the wrong capabilities
> > > setting.
> > 
> > Hm, MII registers are only 16bit, why does your mii_read()
> > implementation return 0x3ffff?
> > 
> in fec_imx it returns the 32 bit register. I though we could mask it to
> only return the data but that wouldn't solve the problem as the tests
> in miidev would fail because the data is 0xFFFF.

Well, the check for the PHY ID registers was added for a purpose:
It allows barebox to print a useful error message if it can't talk
to the PHY, which is *much* better than letting you guess why
you ethernet doesn't work.  You could change the check to ">= 0xffff"
but it looks strange.  IMHO it would be better to mask the invalid
bits in your mii_read().

> > Also, what exactly do you mean by "too early"?  Your code
> > shouldn't call mii_register() before the MDIO clock is stable.
> > 
> fec_imx.c does that.

But why is it too early?  What do you need to wait for?
Maybe something in eth_device.open() enables MDIO?

Maybe it is actually better to defer PHY probing until
eth_device.open() is called, to save a few milliseconds during
boot from flash when ethernet isn't used.  But I have a board which can
have different PHY (e.g. 100Mbit or 1Gbit), and where one
of them doesn't answer to address 0, so I need to probe the address.
Then I would need to defer the mii_register() also
until eth_device.open()?


Thanks,
Johannes

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] miidev: fix 1G wrong detection
  2012-09-07  9:28       ` Johannes Stezenbach
@ 2012-09-07  9:32         ` Eric Bénard
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Bénard @ 2012-09-07  9:32 UTC (permalink / raw)
  To: Johannes Stezenbach; +Cc: barebox

Hi,

Le Fri, 7 Sep 2012 11:28:33 +0200,
Johannes Stezenbach <js@sig21.net> a écrit :

> Hi,
> 
> On Fri, Sep 07, 2012 at 11:02:18AM +0200, Eric Bénard wrote:
> > Le Fri, 7 Sep 2012 10:52:16 +0200,
> > Johannes Stezenbach <js@sig21.net> a écrit :
> > > On Thu, Sep 06, 2012 at 09:39:31PM +0200, Eric Bénard wrote:
> > > > since 99e72c8bbdbdc690025a5868d831f1fe79ad56fc on an i.MX51 based board,
> > > > I get : "phy0: Link is up - 1000/Full". It seems miidev tries to probe
> > > > the PHY to early and gets 0x3ffff which leads to the wrong capabilities
> > > > setting.
> > > 
> > > Hm, MII registers are only 16bit, why does your mii_read()
> > > implementation return 0x3ffff?
> > > 
> > in fec_imx it returns the 32 bit register. I though we could mask it to
> > only return the data but that wouldn't solve the problem as the tests
> > in miidev would fail because the data is 0xFFFF.
> 
> Well, the check for the PHY ID registers was added for a purpose:
> It allows barebox to print a useful error message if it can't talk
> to the PHY, which is *much* better than letting you guess why
> you ethernet doesn't work.  You could change the check to ">= 0xffff"
> but it looks strange.  IMHO it would be better to mask the invalid
> bits in your mii_read().
> 
that's the cleanest solution, but in the present case if we do that
miidev will fail to probe on this board.

> > > Also, what exactly do you mean by "too early"?  Your code
> > > shouldn't call mii_register() before the MDIO clock is stable.
> > > 
> > fec_imx.c does that.
> 
> But why is it too early?  What do you need to wait for?
> Maybe something in eth_device.open() enables MDIO?
> 
> Maybe it is actually better to defer PHY probing until
> eth_device.open() is called, to save a few milliseconds during
> boot from flash when ethernet isn't used.  But I have a board which can
> have different PHY (e.g. 100Mbit or 1Gbit), and where one
> of them doesn't answer to address 0, so I need to probe the address.
> Then I would need to defer the mii_register() also
> until eth_device.open()?
> 
sure fec_imx may need some rework to fit with mii_dev in trhe actual
state.

Eric

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] miidev: fix 1G wrong detection
  2012-09-06 19:39 ` [PATCH 3/3] miidev: fix 1G wrong detection Eric Bénard
                     ` (2 preceding siblings ...)
  2012-09-07  8:52   ` Johannes Stezenbach
@ 2012-09-14  7:03   ` Sascha Hauer
  3 siblings, 0 replies; 18+ messages in thread
From: Sascha Hauer @ 2012-09-14  7:03 UTC (permalink / raw)
  To: Eric Bénard; +Cc: barebox

On Thu, Sep 06, 2012 at 09:39:31PM +0200, Eric Bénard wrote:
> since 99e72c8bbdbdc690025a5868d831f1fe79ad56fc on an i.MX51 based board,
> I get : "phy0: Link is up - 1000/Full". It seems miidev tries to probe
> the PHY to early and gets 0x3ffff which leads to the wrong capabilities
> setting.
> 
> Signed-off-by: Eric Bénard <eric@eukrea.com>

Applied to master.

Thanks
 Sascha

> ---
>  drivers/net/miidev.c |   33 +++++++++------------------------
>  1 files changed, 9 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/miidev.c b/drivers/net/miidev.c
> index 75b53e3..e0f9d67 100644
> --- a/drivers/net/miidev.c
> +++ b/drivers/net/miidev.c
> @@ -131,6 +131,14 @@ int miidev_get_status(struct mii_device *mdev)
>  
>  	status = ret & BMSR_LSTATUS ? MIIDEV_STATUS_IS_UP : 0;
>  
> +	if (ret & BMSR_ESTATEN) {
> +		ret = mii_read(mdev, mdev->address, MII_ESTATUS);
> +		if (ret < 0)
> +			goto err_out;
> +		if (ret & (ESTATUS_1000_TFULL | ESTATUS_1000_THALF))
> +			mdev->capabilities = MIIDEV_CAPABLE_1000M;
> +	}
> +
>  	ret = mii_read(mdev, mdev->address, MII_BMCR);
>  	if (ret < 0)
>  		goto err_out;
> @@ -239,27 +247,8 @@ static struct file_operations miidev_ops = {
>  static int miidev_probe(struct device_d *dev)
>  {
>  	struct mii_device *mdev = dev->priv;
> -	int val;
> -	int caps = 0;
>  
> -	val = mii_read(mdev, mdev->address, MII_PHYSID1);
> -	if (val < 0 || val == 0xffff)
> -		goto err_out;
> -	val = mii_read(mdev, mdev->address, MII_PHYSID2);
> -	if (val < 0 || val == 0xffff)
> -		goto err_out;
> -	val = mii_read(mdev, mdev->address, MII_BMSR);
> -	if (val < 0)
> -		goto err_out;
> -	if (val & BMSR_ESTATEN) {
> -		val = mii_read(mdev, mdev->address, MII_ESTATUS);
> -		if (val < 0)
> -			goto err_out;
> -		if (val & (ESTATUS_1000_TFULL | ESTATUS_1000_THALF))
> -			caps = MIIDEV_CAPABLE_1000M;
> -	}
> -
> -	mdev->capabilities = caps;
> +	mdev->capabilities = 0;
>  	mdev->cdev.name = asprintf("phy%d", dev->id);
>  	mdev->cdev.size = 64;
>  	mdev->cdev.ops = &miidev_ops;
> @@ -268,10 +257,6 @@ static int miidev_probe(struct device_d *dev)
>  	devfs_create(&mdev->cdev);
>  	list_add_tail(&mdev->list, &miidev_list);
>  	return 0;
> -
> -err_out:
> -	dev_err(dev, "cannot read PHY registers (addr %d)\n", mdev->address);
> -	return -ENODEV;
>  }
>  
>  static void miidev_remove(struct device_d *dev)
> -- 
> 1.7.7.6
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2012-09-14  7:03 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-06 19:39 [PATCH 1/3] fec: restart autoneg at open instead of init Eric Bénard
2012-09-06 19:39 ` [PATCH 2/3] i.MX51: unbreak FEC iomux Eric Bénard
2012-09-07  8:30   ` Sascha Hauer
2012-09-06 19:39 ` [PATCH 3/3] miidev: fix 1G wrong detection Eric Bénard
2012-09-06 21:52   ` Jean-Christophe PLAGNIOL-VILLARD
2012-09-07  6:44   ` Sascha Hauer
2012-09-07  7:28     ` Eric Bénard
2012-09-07  8:23       ` Sascha Hauer
2012-09-07  7:43     ` Jean-Christophe PLAGNIOL-VILLARD
2012-09-07  8:52   ` Johannes Stezenbach
2012-09-07  9:02     ` Eric Bénard
2012-09-07  9:28       ` Johannes Stezenbach
2012-09-07  9:32         ` Eric Bénard
2012-09-14  7:03   ` Sascha Hauer
2012-09-07  6:57 ` [PATCH 1/3] fec: restart autoneg at open instead of init Sascha Hauer
2012-09-07  7:30   ` Eric Bénard
2012-09-07  7:47     ` Sascha Hauer
2012-09-07  7:51       ` Eric Bénard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox