[U-Boot] [PATCH 0/3]: arm:Kirkwood network driver fixes
Hi!
Three patches to fix various network driver issues on kirkwood. Patch 3 is a repost of the patch sent earlier today which is rebased on top of the other two.
// Simon
Signed-off-by: Simon Kagstrom simon.kagstrom@netinsight.net --- drivers/net/kirkwood_egiga.c | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/kirkwood_egiga.c b/drivers/net/kirkwood_egiga.c index f31fefc..065e335 100644 --- a/drivers/net/kirkwood_egiga.c +++ b/drivers/net/kirkwood_egiga.c @@ -38,6 +38,8 @@ #include <asm/arch/kirkwood.h> #include "kirkwood_egiga.h"
+#define KIRKWOOD_PHY_ADR_REQUEST 0xee + /* * smi_reg_read - miiphy_read callback function. * @@ -52,7 +54,8 @@ static int smi_reg_read(char *devname, u8 phy_adr, u8 reg_ofs, u16 * data) u32 timeout;
/* Phyadr read request */ - if (phy_adr == 0xEE && reg_ofs == 0xEE) { + if (phy_adr == KIRKWOOD_PHY_ADR_REQUEST && + reg_ofs == KIRKWOOD_PHY_ADR_REQUEST) { /* */ *data = (u16) (KWGBEREG_RD(regs->phyadr) & PHYADR_MASK); return 0; @@ -127,7 +130,8 @@ static int smi_reg_write(char *devname, u8 phy_adr, u8 reg_ofs, u16 data) u32 timeout;
/* Phyadr write request*/ - if (phy_adr == 0xEE && reg_ofs == 0xEE) { + if (phy_adr == KIRKWOOD_PHY_ADR_REQUEST && + reg_ofs == KIRKWOOD_PHY_ADR_REQUEST) { KWGBEREG_WR(regs->phyadr, data); return 0; } @@ -444,7 +448,8 @@ static int kwgbe_init(struct eth_device *dev) #if (defined (CONFIG_MII) || defined (CONFIG_CMD_MII)) \ && defined (CONFIG_SYS_FAULT_ECHO_LINK_DOWN) u16 phyadr; - miiphy_read(dev->name, 0xEE, 0xEE, &phyadr); + miiphy_read(dev->name, KIRKWOOD_PHY_ADR_REQUEST, + KIRKWOOD_PHY_ADR_REQUEST, &phyadr); if (!miiphy_link(dev->name, phyadr)) { printf("%s: No link on %s\n", __FUNCTION__, dev->name); return -1; @@ -670,7 +675,8 @@ int kirkwood_egiga_initialize(bd_t * bis) #if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) miiphy_register(dev->name, smi_reg_read, smi_reg_write); /* Set phy address of the port */ - miiphy_write(dev->name, 0xEE, 0xEE, PHY_BASE_ADR + devnum); + miiphy_write(dev->name, KIRKWOOD_PHY_ADR_REQUEST, + KIRKWOOD_PHY_ADR_REQUEST, PHY_BASE_ADR + devnum); #endif } return 0;
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Simon Kagstrom Sent: Thursday, August 20, 2009 1:42 PM Cc: U-Boot ML Subject: [U-Boot] [PATCH 1/3]: arm:kirkwood Define kirkwood phy address magic number
Signed-off-by: Simon Kagstrom simon.kagstrom@netinsight.net
drivers/net/kirkwood_egiga.c | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/kirkwood_egiga.c b/drivers/net/kirkwood_egiga.c index f31fefc..065e335 100644 --- a/drivers/net/kirkwood_egiga.c +++ b/drivers/net/kirkwood_egiga.c @@ -38,6 +38,8 @@ #include <asm/arch/kirkwood.h> #include "kirkwood_egiga.h"
+#define KIRKWOOD_PHY_ADR_REQUEST 0xee
define this in header file
Basically this is needed in drivers/net/phy/mv88e61xx.c for multi chip support in this case we need to define this in include/miiphy.h. which conflicts with other phy address definition, that's why not done earlier
It makes more sense to add APIs miiphy_read/write_phyadr to miiutils
Regards.. Prafulla . .
On Thu, 20 Aug 2009 02:40:48 -0700 Prafulla Wadaskar prafulla@marvell.com wrote:
+#define KIRKWOOD_PHY_ADR_REQUEST 0xee
define this in header file
Basically this is needed in drivers/net/phy/mv88e61xx.c for multi chip support in this case we need to define this in include/miiphy.h. which conflicts with other phy address definition, that's why not done earlier
It makes more sense to add APIs miiphy_read/write_phyadr to miiutils
But is this really general functionality? miiphy.h is something I suppose should be generic between phys and not contain device-specific things like this.
Perhaps we should revive the patches that move out the phy initialization from sheevaplug.c and place the define and some implementation there?
// Simon
-----Original Message----- From: Simon Kagstrom [mailto:simon.kagstrom@netinsight.net] Sent: Thursday, August 20, 2009 5:17 PM To: Prafulla Wadaskar Cc: U-Boot ML; Ashish Karkare; Prabhanjan Sarnaik Subject: Re: [U-Boot] [PATCH 1/3]: arm:kirkwood Define kirkwood phy address magic number
On Thu, 20 Aug 2009 02:40:48 -0700 Prafulla Wadaskar prafulla@marvell.com wrote:
+#define KIRKWOOD_PHY_ADR_REQUEST 0xee
define this in header file
Basically this is needed in drivers/net/phy/mv88e61xx.c for
multi chip support
in this case we need to define this in include/miiphy.h. which conflicts with other phy address definition, that's
why not done earlier
It makes more sense to add APIs miiphy_read/write_phyadr to miiutils
But is this really general functionality? miiphy.h is something I suppose should be generic between phys and not contain device-specific things like this.
Agreed, Current PHY support need to be re-architected to support switches in generic ways This is old pending issue, may be we should re-trigger it again after scheduled release
Perhaps we should revive the patches that move out the phy initialization from sheevaplug.c and place the define and some implementation there?
Sure the code can be abstracted from sheevaplug as well as rd6281a for MV88E1116 initialization. In the beginning I had posted my first patch as separate driver for MV88E1116, but again this is gated due to above rework to be done
Regards.. Prafulla . .
// Simon
On Thu, 20 Aug 2009 20:20:08 -0700 Prafulla Wadaskar prafulla@marvell.com wrote:
+#define KIRKWOOD_PHY_ADR_REQUEST 0xee
define this in header file
Basically this is needed in drivers/net/phy/mv88e61xx.c for
multi chip support
in this case we need to define this in include/miiphy.h. which conflicts with other phy address definition, that's
why not done earlier
It makes more sense to add APIs miiphy_read/write_phyadr to miiutils
But is this really general functionality? miiphy.h is something I suppose should be generic between phys and not contain device-specific things like this.
Agreed, Current PHY support need to be re-architected to support switches in generic ways This is old pending issue, may be we should re-trigger it again after scheduled release
Right. But for now, perhaps it's OK to merge the patch as-is and then correct the location of the define when the restructuring is done?
In the same way, I guess we should postspone moving out PHY initialization from sheevaplug and keep it duplicated until it's clear what to do there?
// Simon
Simon Kagstrom wrote:
Signed-off-by: Simon Kagstrom simon.kagstrom@netinsight.net
drivers/net/kirkwood_egiga.c | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-)
Patches 1-3 applied to net repo.
thanks, Ben
This patch makes the device wait for up to 5 seconds for the link to come up, similar to what many of the other network drivers do. This avoids confusing situations where, e.g., a tftp fails when initiated early after U-boot has started (before the link has come up).
v2: Remove function name from printout
Signed-off-by: Simon Kagstrom simon.kagstrom@netinsight.net --- drivers/net/kirkwood_egiga.c | 20 ++++++++++++++------ 1 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/net/kirkwood_egiga.c b/drivers/net/kirkwood_egiga.c index 065e335..9f36633 100644 --- a/drivers/net/kirkwood_egiga.c +++ b/drivers/net/kirkwood_egiga.c @@ -400,6 +400,7 @@ static int kwgbe_init(struct eth_device *dev) { struct kwgbe_device *dkwgbe = to_dkwgbe(dev); struct kwgbe_registers *regs = dkwgbe->regs; + int i;
/* setup RX rings */ kwgbe_init_rx_desc_ring(dkwgbe); @@ -447,13 +448,20 @@ static int kwgbe_init(struct eth_device *dev)
#if (defined (CONFIG_MII) || defined (CONFIG_CMD_MII)) \ && defined (CONFIG_SYS_FAULT_ECHO_LINK_DOWN) - u16 phyadr; - miiphy_read(dev->name, KIRKWOOD_PHY_ADR_REQUEST, - KIRKWOOD_PHY_ADR_REQUEST, &phyadr); - if (!miiphy_link(dev->name, phyadr)) { - printf("%s: No link on %s\n", __FUNCTION__, dev->name); - return -1; + /* Wait up to 5s for the link status */ + for (i = 0; i < 5; i++) { + u16 phyadr; + + miiphy_read(dev->name, KIRKWOOD_PHY_ADR_REQUEST, + KIRKWOOD_PHY_ADR_REQUEST, &phyadr); + /* Return if we get link up */ + if (miiphy_link(dev->name, phyadr)) + return 0; + udelay(1000000); } + + printf("No link on %s\n", dev->name); + return -1; #endif return 0; }
U-boot might use non-8-byte-aligned addresses for sending data, which the kwgbe_send doesn't accept (bootp does this for me). This patch copies the data to be sent to a malloced temporary buffer if it is non-aligned.
v2: Malloc send buffer v3: No need to use jumbo frames, use 1518 bytes buffer instead v4: Correct alignment passed to memalign (should be 8!), allocate buffer at initialization(), use PKTSIZE_ALIGN
Signed-off-by: Simon Kagstrom simon.kagstrom@netinsight.net --- drivers/net/kirkwood_egiga.c | 21 +++++++++++++++++---- drivers/net/kirkwood_egiga.h | 1 + 2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/net/kirkwood_egiga.c b/drivers/net/kirkwood_egiga.c index 9f36633..479035d 100644 --- a/drivers/net/kirkwood_egiga.c +++ b/drivers/net/kirkwood_egiga.c @@ -500,18 +500,26 @@ static int kwgbe_send(struct eth_device *dev, volatile void *dataptr, struct kwgbe_device *dkwgbe = to_dkwgbe(dev); struct kwgbe_registers *regs = dkwgbe->regs; struct kwgbe_txdesc *p_txdesc = dkwgbe->p_txdesc; + void *p = (void *)dataptr; u32 cmd_sts;
+ /* Copy buffer if it's misaligned */ if ((u32) dataptr & 0x07) { - printf("Err..(%s) xmit dataptr not 64bit aligned\n", - __FUNCTION__); - return -1; + if (datasize > PKTSIZE_ALIGN) { + printf("Non-aligned data too large (%d)\n", + datasize); + return -1; + } + + memcpy(dkwgbe->p_aligned_txbuf, p, datasize); + p = dkwgbe->p_aligned_txbuf; } + p_txdesc->cmd_sts = KWGBE_ZERO_PADDING | KWGBE_GEN_CRC; p_txdesc->cmd_sts |= KWGBE_TX_FIRST_DESC | KWGBE_TX_LAST_DESC; p_txdesc->cmd_sts |= KWGBE_BUFFER_OWNED_BY_DMA; p_txdesc->cmd_sts |= KWGBE_TX_EN_INTERRUPT; - p_txdesc->buf_ptr = (u8 *) dataptr; + p_txdesc->buf_ptr = (u8 *) p; p_txdesc->byte_cnt = datasize;
/* Apply send command using zeroth RXUQ */ @@ -628,8 +636,13 @@ int kirkwood_egiga_initialize(bd_t * bis) * PKTSIZE_ALIGN + 1))) goto error3;
+ if (!(dkwgbe->p_aligned_txbuf = memalign(8, PKTSIZE_ALIGN))) + goto error4; + if (!(dkwgbe->p_txdesc = (struct kwgbe_txdesc *) memalign(PKTALIGN, sizeof(struct kwgbe_txdesc) + 1))) { + free(dkwgbe->p_aligned_txbuf); + error4: free(dkwgbe->p_rxbuf); error3: free(dkwgbe->p_rxdesc); diff --git a/drivers/net/kirkwood_egiga.h b/drivers/net/kirkwood_egiga.h index 9c893d1..16d5214 100644 --- a/drivers/net/kirkwood_egiga.h +++ b/drivers/net/kirkwood_egiga.h @@ -499,6 +499,7 @@ struct kwgbe_device { struct kwgbe_rxdesc *p_rxdesc; struct kwgbe_rxdesc *p_rxdesc_curr; u8 *p_rxbuf; + u8 *p_aligned_txbuf; };
#endif /* __EGIGA_H__ */
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Simon Kagstrom Sent: Thursday, August 20, 2009 1:44 PM To: U-Boot ML Subject: [U-Boot] [PATCH 3/3] [repost]: arm: kirkwood: See to it that sent data is 8-byte aligned
U-boot might use non-8-byte-aligned addresses for sending data, which the kwgbe_send doesn't accept (bootp does this for me). This patch copies the data to be sent to a malloced temporary buffer if it is non-aligned.
<snip...>
diff --git a/drivers/net/kirkwood_egiga.h b/drivers/net/kirkwood_egiga.h index 9c893d1..16d5214 100644 --- a/drivers/net/kirkwood_egiga.h +++ b/drivers/net/kirkwood_egiga.h @@ -499,6 +499,7 @@ struct kwgbe_device { struct kwgbe_rxdesc *p_rxdesc; struct kwgbe_rxdesc *p_rxdesc_curr; u8 *p_rxbuf;
- u8 *p_aligned_txbuf;
};
#endif /* __EGIGA_H__ */
1.6.0.4
Ack, Technically this patch is okay, Unless we all agree this to be done in low level drivers :-)
Regards.. Prafulla . .
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
participants (3)
-
Ben Warren -
Prafulla Wadaskar -
Simon Kagstrom