Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
> "Peter" == Peter Korsgaard <[EMAIL PROTECTED]> writes:
Hi,
Peter> I'll give your driver a try and report back.
Ok, the driver seems to be working (after fixing up the accessor
routines for my hw setup) and performance is comparable to Dustin's
driver.
It would be nice if the driver would enable the byte swapping support
in the hw if it detects the wrong endian - Like this:
Index: linux/drivers/net/smsc911x.c
===
--- linux.orig/drivers/net/smsc911x.c
+++ linux/drivers/net/smsc911x.c
@@ -1787,6 +1787,13 @@
return -ENODEV;
}
+ /* check endian */
+ if (smsc911x_reg_read(pdata, BYTE_TEST) == 0x43218765) {
+ SMSC_TRACE("Byte test looks swapped, inverting");
+ smsc911x_reg_write(~smsc911x_reg_read(pdata, ENDIAN),
+ pdata, ENDIAN);
+ }
+
/* Default generation to zero (all workarounds apply) */
pdata->generation = 0;
--
Bye, Peter Korsgaard
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
> "Steve" == Steve Glendinning <[EMAIL PROTECTED]> writes: Hi, >> What's the problem with Dustin's driver? It seems to work fine here >> with a lan9117. Why not just add lan921x support to the existing >> driver? Steve> I have heard Dustin's driver works very well on PXA, but on Steve> others it doesn't even compile (hence why it depends on Steve> ARCH_PXA). I'm using it on PPC. Steve> Dustin's driver was based on the smc91x code, but these two Steve> ethernet devices share nothing other than a similar name. Steve> smsc911x is a new, completely platform-independent driver with Steve> workarounds for several hardware issues, and it doesn't suffer Steve> from quite as much macro abuse :-) The macros are not that bad as the hw people sligtly misconnected the chip, so I need special access routines (enable the big endian mode, not byteswap on normal registar access and byteswap on fifo access). I'll give your driver a try and report back. -- Bye, Peter Korsgaard - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Hi Peter, > What's the problem with Dustin's driver? It seems to work fine here > with a lan9117. Why not just add lan921x support to the existing > driver? I have heard Dustin's driver works very well on PXA, but on others it doesn't even compile (hence why it depends on ARCH_PXA). Dustin's driver was based on the smc91x code, but these two ethernet devices share nothing other than a similar name. smsc911x is a new, completely platform-independent driver with workarounds for several hardware issues, and it doesn't suffer from quite as much macro abuse :-) Regards, -- Steve Glendinning SMSC GmbH m: +44 777 933 9124 e: [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
> "Steve" == Steve Glendinning <[EMAIL PROTECTED]> writes: Hi, Steve> Attached is a driver for SMSC's LAN911x and LAN921x families Steve> of embedded ethernet controllers. Steve> There is an existing smc911x driver in the tree; this is intended to Steve> replace it. Dustin McIntire (the author of the smc911x driver) has Steve> expressed his support for switching to this driver. What's the problem with Dustin's driver? It seems to work fine here with a lan9117. Why not just add lan921x support to the existing driver? -- Bye, Peter Korsgaard - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Hi Jeff, > Where is the hard_start_xmit/TX-completion locking? The entire PIO Tx operation is performed within hard_start_xmit, so should be covered by netif_tx_lock. Regards, -- Steve Glendinning SMSC GmbH m: +44 777 933 9124 e: [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Where is the hard_start_xmit/TX-completion locking? - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Steve Glendinning wrote: Attached is a driver for SMSC's LAN911x and LAN921x families of embedded ethernet controllers. There is an existing smc911x driver in the tree; this is intended to replace it. This driver contains workarounds for all known hardware issues, and has been tested on all flavours of the chip on multiple architectures. Dustin McIntire (the author of the smc911x driver) has expressed his support for switching to this driver. Signed-off-by: Steve Glendinning <[EMAIL PROTECTED]> This patch has just been tested on my Freescale mx31 board, with a smsc9117, and Linux version 2.6.16.19. I agree to include it in mainstream. old smc911x is restricted to one architecture (PXA) and one chip (9118). Acked-by: Pierre Tardy <[EMAIL PROTECTED]> -- Pierre Tardy - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
On Thu, 3 Aug 2006, [EMAIL PROTECTED] wrote: Hi Scott, I think the main concern that my co-workers here at SOMA Networks and I have is that having two drivers dilutes the community effort. There's also the fact that smc911x seems to have DMA support, which smsc911x does not. Unless there are plans to enhance smsc911x with DMA support, it would seem to me to make more sense to apply the LAN921x support and hardware workaround changes to smc911x. I have kept this driver simple for now, focussing on stable support for multiple platforms. While arm is certainly popular, we also have a large number of customers on sh, x86 and others, and currently there is no in-kernel support (DMA or not) for these people. Many of our customers use our proprietary drivers (which support DMA on many platforms), and we would like to migrate them over to a standard in-kernel driver - it would make support easier and everyone can benefit from ongoing driver improvements. The DMA code within these proprietary drivers is the next target for submission, although I would like to defer additional complexity until we are all happy with the current PIO-only implementation. Okay, I guess I'll shut up. We're going to stick with smc911x for now and will probably submit some patches to it in the near future. If smsc911x gets accepted into mainline by Jeff and gains DMA support, we may reconsider. Scott -- == Scott Murray, [EMAIL PROTECTED] "Good, bad ... I'm the guy with the gun." - Ash, "Army of Darkness" - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Hi Scott, > I think the main concern that my co-workers here at SOMA Networks > and I have is that having two drivers dilutes the community effort. > There's also the fact that smc911x seems to have DMA support, which > smsc911x does not. Unless there are plans to enhance smsc911x with > DMA support, it would seem to me to make more sense to apply the > LAN921x support and hardware workaround changes to smc911x. I have kept this driver simple for now, focussing on stable support for multiple platforms. While arm is certainly popular, we also have a large number of customers on sh, x86 and others, and currently there is no in-kernel support (DMA or not) for these people. Many of our customers use our proprietary drivers (which support DMA on many platforms), and we would like to migrate them over to a standard in-kernel driver - it would make support easier and everyone can benefit from ongoing driver improvements. The DMA code within these proprietary drivers is the next target for submission, although I would like to defer additional complexity until we are all happy with the current PIO-only implementation. Regards, -- Steve Glendinning SMSC GmbH m: +44 777 933 9124 e: [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
[EMAIL PROTECTED] <[EMAIL PROTECTED]> :
> Mezigues :
[...]
> > Does the platform guarantees that the register write has actually
> reached
> > the real register when the udelay is issued ?
>
> I think so, but maybe you can help me check. The LAN911x device is always
> directly connected to a simple SRAM-like host bus, and smsc911x_reg_write
> is implemented using readl. Does this implicitly guarantee it to be
> volatile?
(s/readl/writel/)
It's probably safe if it's non-cached SRAM like but I strongly suggest to
read Documentation/DocBook/deviceiobook.tmpl. It explains better than me.
[...]
> > > spin_lock_irqsave(&pdata->phy_lock, flags);
> >
> > flags useless: ->open() is issued in irq-enabled context.
>
> How do you mean? I thought an irq-enabled context meant i DO have to
> disable irqs?
Yes but you can disable unconditionally and later enable unconditionnally
because you know that the irq are _always_ enabled before the lock (in
->open()).
'flags' saves the state. If the state is constant, you can either:
- s/spin_{lock_irqsave/unlock_irqrestore}/spin_{lock/unlock}_irq/
(irq always on before the lock)
or:
- s/spin_{lock_irqsave/unlock_irqrestore}/spin_{lock/unlock}/
(irq always off before the lock)
--
Ueimor
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
On Wed, Aug 02, 2006 at 08:23:40PM +0100, [EMAIL PROTECTED] wrote: > > Does this need the magic "for (addr=1; addr <=32; addr++)" trick that > > has become idiomatic for PHY discovery in our drivers? > > I don't understand the question - surely 32 is not a valid PHY address? That's why it is magic! :-) The idea is to probe PHY addr 0 last in the series. Apparently some PHYs don't like seeing addr 0 or somesuch, so you try it last to avoid screwing them up. It may well be folklore and legend at this point. Still, you will find several examples in the various drivers. The sundance driver is one example. John -- John W. Linville [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Hi Francois,
Thanks again for all your feedback. I have implemented most of your
suggestions,
> > /* Enable phy clocks to the MAC */
> > hwcfg &= (~HW_CFG_PHY_CLK_SEL_);
> > hwcfg |= HW_CFG_PHY_CLK_SEL_EXT_PHY_;
> > smsc911x_reg_write(hwcfg, pdata, HW_CFG);
> > udelay(10); /* Enough time
for clocks to restart */
>
> (back to my original question that I should have reworded in a different
> thread)
>
> Does the platform guarantees that the register write has actually
reached
> the real register when the udelay is issued ?
I think so, but maybe you can help me check. The LAN911x device is always
directly connected to a simple SRAM-like host bus, and smsc911x_reg_write
is implemented using readl. Does this implicitly guarantee it to be
volatile?
> >
> > if (!pdata->software_irq_signal) {
> > printk(KERN_WARNING "%s: ISR failed
signaling test (IRQ %d)\n",
> > dev->name, dev->irq);
> > return -ENODEV;
> > }
> > SMSC_TRACE("IRQ handler passed test using IRQ %d",
dev->irq);
> >
> > printk(KERN_INFO "%s: SMSC911x/921x identified at %#08lx,
IRQ: %d\n",
> > dev->name, (unsigned long)pdata->ioaddr,
dev->irq);
> >
> > spin_lock_irqsave(&pdata->phy_lock, flags);
>
> flags useless: ->open() is issued in irq-enabled context.
How do you mean? I thought an irq-enabled context meant i DO have to
disable irqs?
> > unsigned long flags;
> >
> > SMSC_TRACE("ioctl cmd 0x%x", cmd);
> > switch (cmd) {
> > case SIOCGMIIPHY:
> > case SIOCDEVPRIVATE:
>
> The SIOCDEVPRIVATE can/should be removed.
I have removed these, they were only in as a quick fix because mii-tool
here sends SIOCDEVPRIVATE instead of SIOCGMIIPHY. I fixed my copy of
mii-tool instead :o)
Best Regards,
--
Steve Glendinning
SMSC GmbH
m: +44 777 933 9124
e: [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Hi John,
Thanks for all your feedback.
> > +/* waits for MAC not busy, with timeout. Assumes MacPhyAccessLock
has
> > + * already been acquired */
> > +static int smsc911x_mac_notbusy(struct smsc911x_data *pdata)
> > +{
> > +int i;
> > +
> > +for (i = 0; i < 40; i++) {
> > +if ((smsc911x_reg_read(pdata,
MAC_CSR_CMD)
> > + & MAC_CSR_CMD_CSR_BUSY_) == 0) {
> > +return 1;
> > +}
> > +}
> > +SMSC_WARNING("Timed out waiting for MAC not BUSY. "
> > + "MAC_CSR_CMD: 0x%08X",
smsc911x_reg_read(pdata,
> > + MAC_CSR_CMD));
> > +return 0;
> > +}
>
> How is the length of this timeout controlled? IOW, what prevents
> it from being too short when the Omegatron 128 running at 10GHz hits
> the market? Are you relying on the MII clock rate?
The LAN911x and LAN921x devices uses an SRAM-like bus interface with a
minimum cycle time of 45ns, so smsc_reg_read() and smsc_reg_write() are
guaranteed to take at least 45ns. The MAC operates a little slower, but
the operation shouldn't take longer than 225ns (5 read cycles).
The PHY is accessed via slave registers in the MAC (which then relays the
command over mii), so its timeout works in the same way.
The timeouts are only there to prevent total lockup if the hardware fails,
if the part is working it should take nowhere near 40 iterations.
> > +/* Auto-detect PHY */
> > +for (address = 0; address <= 31;
address++) {
> > +pdata->phy_address =
address;
> > +phyid1 =
smsc911x_phy_read(pdata, MII_PHYSID1);
> > +phyid2 =
smsc911x_phy_read(pdata, MII_PHYSID2);
> > +if ((phyid1 != 0xU)
|| (phyid2 != 0xU)) {
> > + SMSC_TRACE("Detected PHY at address = "
> > + "0x%02X = %d", address, address);
> > +break;
> > +}
> > +}
>
> Does this need the magic "for (addr=1; addr <=32; addr++)" trick that
> has become idiomatic for PHY discovery in our drivers?
I don't understand the question - surely 32 is not a valid PHY address?
> > +/* SMSC911x registers and bitfields */
> > +#define RX_DATA_FIFO0x00
> > +
> > +#define TX_DATA_FIFO0x20
> > +#define TX_CMD_A_ON_COMP_ 0x8000
> > +#define TX_CMD_A_BUF_END_ALGN_ 0x0300
> > +#define TX_CMD_A_4_BYTE_ALGN_ 0x
> > +#define TX_CMD_A_16_BYTE_ALGN_ 0x0100
> > +#define TX_CMD_A_32_BYTE_ALGN_ 0x0200
> > +#define TX_CMD_A_DATA_OFFSET_ 0x001F
> > +#define TX_CMD_A_FIRST_SEG_ 0x2000
> > +#define TX_CMD_A_LAST_SEG_ 0x1000
> > +#define TX_CMD_A_BUF_SIZE_ 0x07FF
> > +#define TX_CMD_B_PKT_TAG_ 0x
> > +#define TX_CMD_B_ADD_CRC_DISABLE_ 0x2000
> > +#define TX_CMD_B_DISABLE_PADDING_ 0x1000
> > +#define TX_CMD_B_PKT_BYTE_LENGTH_ 0x07FF
>
> Looks like something went haywire w/ your tabbing in this file...?
Its just the "+ " in the patch, once applied it looks quite pretty!
Best Regards,
--
Steve Glendinning
SMSC GmbH
m: +44 777 933 9124
e: [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
On Tue, 1 Aug 2006, [EMAIL PROTECTED] wrote: Hi Scott Sorry to be coming in late, but I'm curious about why this work is being submitted as a separate driver, rather than as patches against the driver from Dustin McIntire that was added a few months ago. Is the intention to go forward with two different drivers for these chips? I was waiting for someone to ask this! This driver has been developed by SMSC & ARM, and has several advantages over the already merged smc911x: - The current driver is arm specific, our smsc911x driver is tested and supported on arm, sh, i386 - smsc911x contains support for the new LAN921x family, as well as LAN911x - smsc911x contains important workarounds for currently known hardware issues - It's shorted, and I believe the coding style to be cleaner and easier to follow. so I'm presenting this as an alternative. Thoughts? I think the main concern that my co-workers here at SOMA Networks and I have is that having two drivers dilutes the community effort. There's also the fact that smc911x seems to have DMA support, which smsc911x does not. Unless there are plans to enhance smsc911x with DMA support, it would seem to me to make more sense to apply the LAN921x support and hardware workaround changes to smc911x. Scott -- == Scott Murray, [EMAIL PROTECTED] "Good, bad ... I'm the guy with the gun." - Ash, "Army of Darkness" - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
[...]
> /* Writes a packet to the TX_DATA_FIFO */
> static inline void
> smsc911x_tx_writefifo(struct smsc911x_data *pdata, unsigned int *buf,
> unsigned int wordcount)
> {
> while (wordcount--) {
> smsc911x_reg_write(*buf++, pdata, TX_DATA_FIFO);
> }
Curly braces are not needed around single line blocks.
[...]
> /* waits for MAC not busy, with timeout. Assumes MacPhyAccessLock has
> * already been acquired */
> static int smsc911x_mac_notbusy(struct smsc911x_data *pdata)
> {
> int i;
>
> for (i = 0; i < 40; i++) {
> if ((smsc911x_reg_read(pdata, MAC_CSR_CMD)
>& MAC_CSR_CMD_CSR_BUSY_) == 0) {
> return 1;
> }
> }
> SMSC_WARNING("Timed out waiting for MAC not BUSY. "
>"MAC_CSR_CMD: 0x%08X", smsc911x_reg_read(pdata,
> MAC_CSR_CMD));
> return 0;
u32 val;
for (i = 0; i < 40; i++) {
val = smsc911x_reg_read(pdata, MAC_CSR_CMD);
if (!(val & MAC_CSR_CMD_CSR_BUSY_))
return 1;
}
SMSC_WARNING("Timed out waiting for MAC not BUSY. "
"MAC_CSR_CMD: 0x%08X", val);
-> no painful line-breaking (s/val/{reg/cmd}/ at will).
> }
>
> /* Fetches a MAC register value. Assumes phy_lock is acquired */
> static unsigned int
> smsc911x_mac_read(struct smsc911x_data *pdata, unsigned int offset)
> {
> unsigned int result = 0x;
> unsigned int temp;
>
> /* Wait until not busy */
> if (unlikely
> (smsc911x_reg_read(pdata, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY_)) {
> SMSC_WARNING("smsc911x_mac_read failed, "
>"MAC already busy at entry");
> return result;
> }
>
> /* Send the MAC cmd */
> smsc911x_reg_write(((offset & 0x00FF) | MAC_CSR_CMD_CSR_BUSY_
> | MAC_CSR_CMD_R_NOT_W_), pdata, MAC_CSR_CMD);
>
> /* Workaround for hardware read-after-write restriction */
> temp = smsc911x_reg_read(pdata, BYTE_TEST);
>
> /* Wait for the read to happen */
> if (unlikely(!smsc911x_mac_notbusy(pdata)))
Double negation (at least :o) ).
[...]
> /* Gets a phy register, phy_lock must be acquired before calling */
> static unsigned int
> smsc911x_phy_read(struct smsc911x_data *pdata, unsigned int index)
> {
> unsigned int addr;
> unsigned int result = 0x;
> int i;
>
> /* Confirm MII not busy */
> if (unlikely
> ((smsc911x_mac_read(pdata, MII_ACC) & MII_ACC_MII_BUSY_) != 0)) {
> SMSC_WARNING("MII is busy in smsc911x_phy_read???");
> return 0;
> }
>
> /* Set the address, index & direction (read from PHY) */
> addr = (((pdata->phy_address) & 0x1F) << 11)
> | ((index & 0x1F) << 6);
> smsc911x_mac_write(pdata, MII_ACC, addr);
>
> /* Wait for read to complete w/ timeout */
> for (i = 0; i < 100; i++) {
> /* See if MII is finished yet */
> if ((smsc911x_mac_read(pdata, MII_ACC)
>& MII_ACC_MII_BUSY_) == 0) {
> result = smsc911x_mac_read(pdata, MII_DATA);
> return result;
> }
if (!(smsc911x_mac_read(pdata, MII_ACC) & MII_ACC_MII_BUSY_))
return smsc911x_mac_read(pdata, MII_DATA);
> }
> SMSC_WARNING("Timed out waiting for MII write to finish");
>
> return result;
I'd go with return 0x; here and remove 'result' altogether.
> }
>
> /* Sets a phy register, phy_lock must be acquired before calling */
> static void smsc911x_phy_write(struct smsc911x_data *pdata,
> unsigned int index, unsigned int val)
> {
> unsigned int addr;
> int i;
>
> /* Confirm MII not busy */
> if (unlikely
> ((smsc911x_mac_read(pdata, MII_ACC) & MII_ACC_MII_BUSY_) != 0)) {
Factor through a smsc911x_mac_busy(pdata) helper ?
[...]
> static int smsc911x_phy_initialise_external(struct smsc911x_data *pdata)
> {
> unsigned int address;
> unsigned int hwcfg;
> unsigned int phyid1;
> unsigned int phyid2;
>
> hwcfg = smsc911x_reg_read(pdata, HW_CFG);
>
> /* External phy is requested, supported, and detected */
> if (hwcfg & HW_CFG_EXT_PHY_DET_) {
>
> /* Attempt to switch to external phy for auto-detecting
>* its address. Assuming tx and rx are stopped because
>* smsc911x_phy_initialise is called before
>* smsc911x_rx_initialise and tx_initialise.
>*/
>
> /* Disable phy clocks to the MAC */
> hwcfg &= (~HW_CFG_PHY_CLK_SEL_);
> hwcfg |= HW_CFG_PHY_CLK_SEL_CLK_DIS_;
> smsc911x_
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Hi Scott > Sorry to be coming in late, but I'm curious about why this work is being > submitted as a separate driver, rather than as patches against the driver > from Dustin McIntire that was added a few months ago. Is the intention to > go forward with two different drivers for these chips? I was waiting for someone to ask this! This driver has been developed by SMSC & ARM, and has several advantages over the already merged smc911x: - The current driver is arm specific, our smsc911x driver is tested and supported on arm, sh, i386 - smsc911x contains support for the new LAN921x family, as well as LAN911x - smsc911x contains important workarounds for currently known hardware issues - It's shorted, and I believe the coding style to be cleaner and easier to follow. so I'm presenting this as an alternative. Thoughts? Regards, -- Steve Glendinning SMSC GmbH m: +44 777 933 9124 e: [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
On Tue, 1 Aug 2006, Steve Glendinning wrote: Attached is a driver patch for SMSC911x family of ethernet chips, generated against 2.6.18-rc1 sources. There's a similar driver in the tree; this one has been tested by SMSC on all flavors of the chip and claimed to be efficient. Updated after feedback from Stephen Hemminger. Driver updated to also support LAN921x family. Workarounds added for known hardware issues. Many improvements following feedback from Stephen Hemminger and Francois Romieu: - Tasklet removed, NAPI poll used instead - Multiple device support - style fixes & minor improvements Sorry to be coming in late, but I'm curious about why this work is being submitted as a separate driver, rather than as patches against the driver from Dustin McIntire that was added a few months ago. Is the intention to go forward with two different drivers for these chips? Scott -- == Scott Murray, [EMAIL PROTECTED] "Good, bad ... I'm the guy with the gun." - Ash, "Army of Darkness" - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
On Tue, Aug 01, 2006 at 04:12:15PM +0100, Steve Glendinning wrote:
> > > Attached is a driver patch for SMSC911x family of ethernet chips,
> > > generated against 2.6.18-rc1 sources. There's a similar driver in the
> > > tree; this one has been tested by SMSC on all flavors of the chip and
> > > claimed to be efficient.
> >
> > Updated after feedback from Stephen Hemminger.
> >
> > Driver updated to also support LAN921x family. Workarounds added for
> > known hardware issues.
>
> Many improvements following feedback from Stephen Hemminger and
> Francois Romieu:
> - Tasklet removed, NAPI poll used instead
> - Multiple device support
> - style fixes & minor improvements
> +/* waits for MAC not busy, with timeout. Assumes MacPhyAccessLock has
> + * already been acquired */
> +static int smsc911x_mac_notbusy(struct smsc911x_data *pdata)
> +{
> + int i;
> +
> + for (i = 0; i < 40; i++) {
> + if ((smsc911x_reg_read(pdata, MAC_CSR_CMD)
> + & MAC_CSR_CMD_CSR_BUSY_) == 0) {
> + return 1;
> + }
> + }
> + SMSC_WARNING("Timed out waiting for MAC not BUSY. "
> + "MAC_CSR_CMD: 0x%08X", smsc911x_reg_read(pdata,
> + MAC_CSR_CMD));
> + return 0;
> +}
How is the length of this timeout controlled? IOW, what prevents
it from being too short when the Omegatron 128 running at 10GHz hits
the market? Are you relying on the MII clock rate?
> +/* Gets a phy register, phy_lock must be acquired before calling */
> +static unsigned int
> +smsc911x_phy_read(struct smsc911x_data *pdata, unsigned int index)
> +{
> + unsigned int addr;
> + unsigned int result = 0x;
> + int i;
> +
> + /* Confirm MII not busy */
> + if (unlikely
> + ((smsc911x_mac_read(pdata, MII_ACC) & MII_ACC_MII_BUSY_) != 0)) {
> + SMSC_WARNING("MII is busy in smsc911x_phy_read???");
> + return 0;
> + }
> +
> + /* Set the address, index & direction (read from PHY) */
> + addr = (((pdata->phy_address) & 0x1F) << 11)
> + | ((index & 0x1F) << 6);
> + smsc911x_mac_write(pdata, MII_ACC, addr);
> +
> + /* Wait for read to complete w/ timeout */
> + for (i = 0; i < 100; i++) {
> + /* See if MII is finished yet */
> + if ((smsc911x_mac_read(pdata, MII_ACC)
> + & MII_ACC_MII_BUSY_) == 0) {
> + result = smsc911x_mac_read(pdata, MII_DATA);
> + return result;
> + }
> + }
> + SMSC_WARNING("Timed out waiting for MII write to finish");
Ditto?
> +/* Sets a phy register, phy_lock must be acquired before calling */
> +static void smsc911x_phy_write(struct smsc911x_data *pdata,
> +unsigned int index, unsigned int val)
> +{
> + unsigned int addr;
> + int i;
> +
> + /* Confirm MII not busy */
> + if (unlikely
> + ((smsc911x_mac_read(pdata, MII_ACC) & MII_ACC_MII_BUSY_) != 0)) {
> + SMSC_WARNING("MII is busy in smsc911x_write_phy???");
> + return;
> + }
> +
> + /* Put the data to write in the MAC */
> + smsc911x_mac_write(pdata, MII_DATA, val);
> +
> + /* Set the address, index & direction (write to PHY) */
> + addr = (((pdata->phy_address) & 0x1F) << 11) |
> + ((index & 0x1F) << 6) | MII_ACC_MII_WRITE_;
> + smsc911x_mac_write(pdata, MII_ACC, addr);
> +
> + /* Wait for write to complete w/ timeout */
> + for (i = 0; i < 100; i++) {
> + /* See if MII is finished yet */
> + if ((smsc911x_mac_read(pdata, MII_ACC)
> + & MII_ACC_MII_BUSY_) == 0)
> + return;
> + }
> + SMSC_WARNING("Timed out waiting for MII write to finish");
> +}
Ditto?
> +/* Autodetects and initialises external phy for SMSC9115 and SMSC9117
> flavors.
> + * If something goes wrong, returns -ENODEV to revert back to internal phy.
> + * Performed at initialisation only, phy_lock already acquired. */
> +static int smsc911x_phy_initialise_external(struct smsc911x_data *pdata)
> +{
> + unsigned int address;
> + unsigned int hwcfg;
> + unsigned int phyid1;
> + unsigned int phyid2;
> +
> + hwcfg = smsc911x_reg_read(pdata, HW_CFG);
> +
> + /* External phy is requested, supported, and detected */
> + if (hwcfg & HW_CFG_EXT_PHY_DET_) {
> +
> + /* Attempt to switch to external phy for auto-detecting
> + * its address. Assuming tx and rx are stopped because
> + * smsc911x_phy_initialise is called before
> + * smsc911x_rx_initialise and tx_initialise.
> + */
> +
> + /* Disable phy clocks to the MAC */
> + hwcfg &= (~HW_CFG_PHY_CLK_SEL_);
> + hwcfg |= HW_CFG_PHY_CLK_SEL_CLK_DIS_;
> + smsc911x_reg_write(hwcfg, pdata, HW_CFG);
> + udelay(10); /* Enoug
