* [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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ messages in thread
* [PATCH 1/3] fec: restart autoneg at open instead of init @ 2012-09-06 19:38 Eric Bénard 2012-09-06 21:52 ` Jean-Christophe PLAGNIOL-VILLARD 2012-09-10 7:43 ` Sascha Hauer 0 siblings, 2 replies; 22+ messages in thread From: Eric Bénard @ 2012-09-06 19:38 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] 22+ messages in thread
* Re: [PATCH 1/3] fec: restart autoneg at open instead of init 2012-09-06 19:38 Eric Bénard @ 2012-09-06 21:52 ` Jean-Christophe PLAGNIOL-VILLARD 2012-09-10 7:43 ` Sascha Hauer 1 sibling, 0 replies; 22+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-09-06 21:52 UTC (permalink / raw) To: Eric Bénard; +Cc: barebox On 21:38 Thu 06 Sep , Eric Bénard wrote: > this saves some time during boot when ethernet is not needed in barebox > > Signed-off-by: Eric Bénard <eric@eukrea.com> > --- Wait I'm going to drop this to have the phy init only at the open of the net on all drivers Best Regards, J. > 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 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/3] fec: restart autoneg at open instead of init 2012-09-06 19:38 Eric Bénard 2012-09-06 21:52 ` Jean-Christophe PLAGNIOL-VILLARD @ 2012-09-10 7:43 ` Sascha Hauer 2012-09-10 7:51 ` Eric Bénard 1 sibling, 1 reply; 22+ messages in thread From: Sascha Hauer @ 2012-09-10 7:43 UTC (permalink / raw) To: Eric Bénard; +Cc: barebox Hi Eric, Regarding the phy issue I have the following plan: - for -next I think Jean-Christophes phylib will be ready to merge. Hopefully it will for for you then again. I think this patch is not necessary anymore then. - for -master I intend to take the "miidev: fix 1G wrong detection" patch which should solve your problem for the next release. 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] 22+ messages in thread
* Re: [PATCH 1/3] fec: restart autoneg at open instead of init 2012-09-10 7:43 ` Sascha Hauer @ 2012-09-10 7:51 ` Eric Bénard 0 siblings, 0 replies; 22+ messages in thread From: Eric Bénard @ 2012-09-10 7:51 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox Hi Sascha, Le Mon, 10 Sep 2012 09:43:36 +0200, Sascha Hauer <s.hauer@pengutronix.de> a écrit : > Regarding the phy issue I have the following plan: > > - for -next I think Jean-Christophes phylib will be ready to merge. > Hopefully it will for for you then again. I think this patch is > not necessary anymore then. > - for -master I intend to take the "miidev: fix 1G wrong detection" > patch which should solve your problem for the next release. > OK, I'll give a try to -next when I find some time. Eric _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2012-09-14 7:03 UTC | newest] Thread overview: 22+ 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 -- strict thread matches above, loose matches on Subject: below -- 2012-09-06 19:38 Eric Bénard 2012-09-06 21:52 ` Jean-Christophe PLAGNIOL-VILLARD 2012-09-10 7:43 ` Sascha Hauer 2012-09-10 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