[3/6] arm: convert pcm037 platform to use smsc911x
diff mbox

Message ID 1232458114-10997-4-git-send-email-steve.glendinning@smsc.com
State Deferred, archived
Delegated to: David Miller
Headers show

Commit Message

Steve Glendinning Jan. 20, 2009, 1:28 p.m. UTC
Signed-off-by: Steve Glendinning <steve.glendinning@smsc.com>
---
 arch/arm/mach-mx3/pcm037.c |   22 ++++++++++++----------
 1 files changed, 12 insertions(+), 10 deletions(-)

Comments

Sascha Hauer Jan. 20, 2009, 3:29 p.m. UTC | #1
I just gave it a try on our pcm037 board and it turns out that it
doesn't work.

The reason for this is not the driver, it's just that the hardware
designers got the direction of the phy detect pin wrong. The pcm037 uses
an internal phy, but the detection pin is pulled high :(

The old driver worked around this by falling back to the internal phy
when no valid phy id is detected. The new driver lacks this fallback.

Instead of falling back we could also introduce a
SMSC911X_FORCE_INTERNAL_PHY flag.

What do you think?

Sascha
David Miller Jan. 20, 2009, 5:13 p.m. UTC | #2
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Tue, 20 Jan 2009 16:29:51 +0100

> I just gave it a try on our pcm037 board and it turns out that it
> doesn't work.
> 
> The reason for this is not the driver, it's just that the hardware
> designers got the direction of the phy detect pin wrong. The pcm037 uses
> an internal phy, but the detection pin is pulled high :(
> 
> The old driver worked around this by falling back to the internal phy
> when no valid phy id is detected. The new driver lacks this fallback.
> 
> Instead of falling back we could also introduce a
> SMSC911X_FORCE_INTERNAL_PHY flag.
> 
> What do you think?

You could pass this "invert detection pin" state in the platform
device private.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Jan. 21, 2009, 10:58 a.m. UTC | #3
On Tue, Jan 20, 2009 at 04:29:51PM +0100, Sascha Hauer wrote:

> The reason for this is not the driver, it's just that the hardware
> designers got the direction of the phy detect pin wrong. The pcm037 uses
> an internal phy, but the detection pin is pulled high :(

A brief survey of boards I have here suggests that this failure is not
unique.

> The old driver worked around this by falling back to the internal phy
> when no valid phy id is detected. The new driver lacks this fallback.

> Instead of falling back we could also introduce a
> SMSC911X_FORCE_INTERNAL_PHY flag.

If we have the explicit flag in the platform data my concern would be
that it would get reported back to the hardware designers and they might
fix the board in subsequent revisions.  That'd cause issues if the
revision of the board weren't identifiable from software (which wouldn't
surprise me at all).
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sascha Hauer Jan. 21, 2009, 2:38 p.m. UTC | #4
On Wed, Jan 21, 2009 at 10:58:15AM +0000, Mark Brown wrote:
> On Tue, Jan 20, 2009 at 04:29:51PM +0100, Sascha Hauer wrote:
> 
> > The reason for this is not the driver, it's just that the hardware
> > designers got the direction of the phy detect pin wrong. The pcm037 uses
> > an internal phy, but the detection pin is pulled high :(
> 
> A brief survey of boards I have here suggests that this failure is not
> unique.
> 
> > The old driver worked around this by falling back to the internal phy
> > when no valid phy id is detected. The new driver lacks this fallback.
> 
> > Instead of falling back we could also introduce a
> > SMSC911X_FORCE_INTERNAL_PHY flag.
> 
> If we have the explicit flag in the platform data my concern would be
> that it would get reported back to the hardware designers and they might
> fix the board in subsequent revisions.  That'd cause issues if the
> revision of the board weren't identifiable from software (which wouldn't
> surprise me at all).

That's why I haven't told the hardware designers yet ;)

Anyway, a FORCE_INTERNAL_PHY flag would still do the right thing. At
least as long the hardware designers do not decide to put an external
phy in the next hardware revision.
If we decide to add such a flag we should add an FORCE_EXTERNAL_PHY
aswell for the sake of completeness.

Sascha

Patch
diff mbox

diff --git a/arch/arm/mach-mx3/pcm037.c b/arch/arm/mach-mx3/pcm037.c
index 8cea825..64a34f6 100644
--- a/arch/arm/mach-mx3/pcm037.c
+++ b/arch/arm/mach-mx3/pcm037.c
@@ -24,7 +24,7 @@ 
 #include <linux/mtd/plat-ram.h>
 #include <linux/memory.h>
 #include <linux/gpio.h>
-#include <linux/smc911x.h>
+#include <linux/smsc911x.h>
 #include <linux/interrupt.h>
 
 #include <mach/hardware.h>
@@ -64,7 +64,7 @@  static struct imxuart_platform_data uart_pdata = {
 	.flags = IMXUART_HAVE_RTSCTS,
 };
 
-static struct resource smc911x_resources[] = {
+static struct resource smsc911x_resources[] = {
 	[0] = {
 		.start		= CS1_BASE_ADDR + 0x300,
 		.end		= CS1_BASE_ADDR + 0x300 + SZ_64K - 1,
@@ -73,22 +73,24 @@  static struct resource smc911x_resources[] = {
 	[1] = {
 		.start		= IOMUX_TO_IRQ(MX31_PIN_GPIO3_1),
 		.end		= IOMUX_TO_IRQ(MX31_PIN_GPIO3_1),
-		.flags		= IORESOURCE_IRQ,
+		.flags		= IORESOURCE_IRQ | IORESOURCE_IRQ_LOWLEVEL,
 	},
 };
 
-static struct smc911x_platdata smc911x_info = {
-	.flags		= SMC911X_USE_32BIT,
-	.irq_flags	= IRQF_SHARED | IRQF_TRIGGER_LOW,
+static struct smsc911x_platform_config smsc911x_info = {
+	.flags		= SMSC911X_USE_32BIT,
+	.irq_polarity	= SMSC911X_IRQ_POLARITY_ACTIVE_LOW,
+	.irq_type	= SMSC911X_IRQ_TYPE_OPEN_DRAIN,
+	.phy_interface	= PHY_INTERFACE_MODE_MII,
 };
 
 static struct platform_device pcm037_eth = {
-	.name		= "smc911x",
+	.name		= "smsc911x",
 	.id		= -1,
-	.num_resources	= ARRAY_SIZE(smc911x_resources),
-	.resource	= smc911x_resources,
+	.num_resources	= ARRAY_SIZE(smsc911x_resources),
+	.resource	= smsc911x_resources,
 	.dev		= {
-		.platform_data = &smc911x_info,
+		.platform_data = &smsc911x_info,
 	},
 };