LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] net: smc91x: improve neponset hack
@ 2015-02-18 19:47 Arnd Bergmann
  2015-02-19  0:35 ` Russell King - ARM Linux
  2015-02-20 21:23 ` David Miller
  0 siblings, 2 replies; 9+ messages in thread
From: Arnd Bergmann @ 2015-02-18 19:47 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-arm-kernel, rmk+kernel, Dmitry Eremin-Solenikov,
	linux-kernel, Linus Walleij

The smc91x driver tries to support multiple platforms at compile
time, but they are mutually exclusive at runtime, and not clearly
defined.

Trying to build for CONFIG_SA1100_ASSABET without CONFIG_ASSABET_NEPONSET
results in this link error:

drivers/built-in.o: In function `smc_drv_probe':
:(.text+0x33310c): undefined reference to `neponset_ncr_frob'

since the neponset_ncr_set function is not defined otherwise.

Similarly, building for both CONFIG_SA1100_ASSABET and CONFIG_SA1100_PLEB
results in a different build error:

smsc/smc91x.c: In function 'smc_drv_probe':
smsc/smc91x.c:2299:2: error: implicit declaration of function 'neponset_ncr_set' [-Werror=implicit-function-declaration]
  neponset_ncr_set(NCR_ENET_OSC_EN);
  ^
smsc/smc91x.c:2299:19: error: 'NCR_ENET_OSC_EN' undeclared (first use in this function)
  neponset_ncr_set(NCR_ENET_OSC_EN);
                   ^

This is an attempt to fix the call site responsible for both
errors, making sure we call the function exactly when the driver
is actually trying to run on the assabet/neponset machine. With
this patch, I no longer see randconfig build errors in this file.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/net/ethernet/smsc/smc91x.c b/drivers/net/ethernet/smsc/smc91x.c
index 88a55f95fe09..fa3f193b5f4d 100644
--- a/drivers/net/ethernet/smsc/smc91x.c
+++ b/drivers/net/ethernet/smsc/smc91x.c
@@ -2355,7 +2355,7 @@ static int smc_drv_probe(struct platform_device *pdev)
 	ret = smc_request_attrib(pdev, ndev);
 	if (ret)
 		goto out_release_io;
-#if defined(CONFIG_SA1100_ASSABET)
+#if defined(CONFIG_ASSABET_NEPONSET) && !defined(CONFIG_SA1100_PLEB)
 	neponset_ncr_set(NCR_ENET_OSC_EN);
 #endif
 	platform_set_drvdata(pdev, ndev);

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

* Re: [PATCH] net: smc91x: improve neponset hack
  2015-02-18 19:47 [PATCH] net: smc91x: improve neponset hack Arnd Bergmann
@ 2015-02-19  0:35 ` Russell King - ARM Linux
  2015-02-20 15:47   ` Arnd Bergmann
  2015-02-20 21:23 ` David Miller
  1 sibling, 1 reply; 9+ messages in thread
From: Russell King - ARM Linux @ 2015-02-19  0:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: netdev, davem, linux-arm-kernel, Dmitry Eremin-Solenikov,
	linux-kernel, Linus Walleij

On Wed, Feb 18, 2015 at 08:47:30PM +0100, Arnd Bergmann wrote:
> The smc91x driver tries to support multiple platforms at compile
> time, but they are mutually exclusive at runtime, and not clearly
> defined.

I'd prefer to rework this to fix that properly.  From what I remember,
the whole SA11x0 stuff in this driver was a mess.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

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

* Re: [PATCH] net: smc91x: improve neponset hack
  2015-02-19  0:35 ` Russell King - ARM Linux
@ 2015-02-20 15:47   ` Arnd Bergmann
  2015-02-20 20:00     ` Robert Jarzmik
                       ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Arnd Bergmann @ 2015-02-20 15:47 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Russell King - ARM Linux, Dmitry Eremin-Solenikov, netdev,
	Linus Walleij, linux-kernel, davem

On Thursday 19 February 2015 00:35:49 Russell King - ARM Linux wrote:
> On Wed, Feb 18, 2015 at 08:47:30PM +0100, Arnd Bergmann wrote:
> > The smc91x driver tries to support multiple platforms at compile
> > time, but they are mutually exclusive at runtime, and not clearly
> > defined.
> 
> I'd prefer to rework this to fix that properly.  From what I remember,
> the whole SA11x0 stuff in this driver was a mess.

I guess that's reasonable. I've looked through the driver and it seems we
did most of the multiplatform work but left a few things alone that should
have been converted a long time ago.

Can you check if the approach below makes sense to you?

I've verified that each machine that defines an smc91x device now sets
the correct platform data, irq flags and access width, which should be
enough to collapse all the CONFIG_ARM cases into one.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

 arch/arm/mach-msm/board-halibut.c    |   8 ++-
 arch/arm/mach-msm/board-qsd8x50.c    |   8 ++-
 arch/arm/mach-pxa/idp.c              |   5 ++
 arch/arm/mach-pxa/lpd270.c           |   8 ++-
 arch/arm/mach-realview/core.c        |   7 +++
 arch/arm/mach-realview/realview_eb.c |   2 +-
 arch/arm/mach-sa1100/neponset.c      |   6 ++
 arch/arm/mach-sa1100/pleb.c          |   7 +++
 drivers/net/ethernet/smsc/smc91x.c   |   9 ++-
 drivers/net/ethernet/smsc/smc91x.h   | 114 +----------------------------------
 10 files changed, 57 insertions(+), 117 deletions(-)

diff --git a/arch/arm/mach-msm/board-halibut.c b/arch/arm/mach-msm/board-halibut.c
index 61bfe584a9d7..fc832040c6e9 100644
--- a/arch/arm/mach-msm/board-halibut.c
+++ b/arch/arm/mach-msm/board-halibut.c
@@ -20,6 +20,7 @@
 #include <linux/input.h>
 #include <linux/io.h>
 #include <linux/delay.h>
+#include <linux/smc91x.h>
 
 #include <mach/hardware.h>
 #include <asm/mach-types.h>
@@ -46,15 +47,20 @@ static struct resource smc91x_resources[] = {
 	[1] = {
 		.start	= MSM_GPIO_TO_INT(49),
 		.end	= MSM_GPIO_TO_INT(49),
-		.flags	= IORESOURCE_IRQ,
+		.flags	= IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHLEVEL,
 	},
 };
 
+static struct smc91x_platdata smc91x_platdata = {
+	.flags = SMC91X_USE_16BIT | SMC91X_NOWAIT,
+};
+
 static struct platform_device smc91x_device = {
 	.name		= "smc91x",
 	.id		= 0,
 	.num_resources	= ARRAY_SIZE(smc91x_resources),
 	.resource	= smc91x_resources,
+	.dev.platform_data = &smc91x_platdata,
 };
 
 static struct platform_device *devices[] __initdata = {
diff --git a/arch/arm/mach-msm/board-qsd8x50.c b/arch/arm/mach-msm/board-qsd8x50.c
index 4c748616ef47..10016a3bc698 100644
--- a/arch/arm/mach-msm/board-qsd8x50.c
+++ b/arch/arm/mach-msm/board-qsd8x50.c
@@ -22,6 +22,7 @@
 #include <linux/usb/msm_hsusb.h>
 #include <linux/err.h>
 #include <linux/clkdev.h>
+#include <linux/smc91x.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -49,15 +50,20 @@ static struct resource smc91x_resources[] = {
 		.flags = IORESOURCE_MEM,
 	},
 	[1] = {
-		.flags = IORESOURCE_IRQ,
+		.flags	= IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHLEVEL,
 	},
 };
 
+static struct smc91x_platdata smc91x_platdata = {
+	.flags = SMC91X_USE_16BIT | SMC91X_NOWAIT,
+};
+
 static struct platform_device smc91x_device = {
 	.name           = "smc91x",
 	.id             = 0,
 	.num_resources  = ARRAY_SIZE(smc91x_resources),
 	.resource       = smc91x_resources,
+	.dev.platform_data = &smc91x_platdata,
 };
 
 static int __init msm_init_smc91x(void)
diff --git a/arch/arm/mach-pxa/idp.c b/arch/arm/mach-pxa/idp.c
index 343c4e3a7c5d..7d8eab857a93 100644
--- a/arch/arm/mach-pxa/idp.c
+++ b/arch/arm/mach-pxa/idp.c
@@ -81,11 +81,16 @@ static struct resource smc91x_resources[] = {
 	}
 };
 
+static struct smc91x_platdata smc91x_platdata = {
+	.flags = SMC91X_USE_32BIT | SMC91X_USE_DMA | SMC91X_NOWAIT,
+};
+
 static struct platform_device smc91x_device = {
 	.name		= "smc91x",
 	.id		= 0,
 	.num_resources	= ARRAY_SIZE(smc91x_resources),
 	.resource	= smc91x_resources,
+	.dev.platform_data = &smc91x_platdata,
 };
 
 static void idp_backlight_power(int on)
diff --git a/arch/arm/mach-pxa/lpd270.c b/arch/arm/mach-pxa/lpd270.c
index ad777b353bd5..28da319d389f 100644
--- a/arch/arm/mach-pxa/lpd270.c
+++ b/arch/arm/mach-pxa/lpd270.c
@@ -24,6 +24,7 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 #include <linux/pwm_backlight.h>
+#include <linux/smc91x.h>
 
 #include <asm/types.h>
 #include <asm/setup.h>
@@ -189,15 +190,20 @@ static struct resource smc91x_resources[] = {
 	[1] = {
 		.start	= LPD270_ETHERNET_IRQ,
 		.end	= LPD270_ETHERNET_IRQ,
-		.flags	= IORESOURCE_IRQ,
+		.flags	= IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHEDGE,
 	},
 };
 
+struct smc91x_platdata smc91x_platdata = {
+	.flags = SMC91X_USE_16BIT | SMC91X_NOWAIT;
+};
+
 static struct platform_device smc91x_device = {
 	.name		= "smc91x",
 	.id		= 0,
 	.num_resources	= ARRAY_SIZE(smc91x_resources),
 	.resource	= smc91x_resources,
+	.dev.platform_data = &smc91x_platdata,
 };
 
 static struct resource lpd270_flash_resources[] = {
diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index 850e506926df..838b327cf9ce 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -28,6 +28,7 @@
 #include <linux/platform_data/video-clcd-versatile.h>
 #include <linux/io.h>
 #include <linux/smsc911x.h>
+#include <linux/smsc91x.h>
 #include <linux/ata_platform.h>
 #include <linux/amba/mmci.h>
 #include <linux/gfp.h>
@@ -94,6 +95,10 @@ static struct smsc911x_platform_config smsc911x_config = {
 	.phy_interface	= PHY_INTERFACE_MODE_MII,
 };
 
+static struct smc91x_platdata smc91x_platdata =
+	.flags = SMC91X_USE_32BIT | SMC_NOWAIT,
+};
+
 static struct platform_device realview_eth_device = {
 	.name		= "smsc911x",
 	.id		= 0,
@@ -107,6 +112,8 @@ int realview_eth_register(const char *name, struct resource *res)
 	realview_eth_device.resource = res;
 	if (strcmp(realview_eth_device.name, "smsc911x") == 0)
 		realview_eth_device.dev.platform_data = &smsc911x_config;
+	else
+		realview_eth_device.dev.platform_data = &smc91x_platdata;
 
 	return platform_device_register(&realview_eth_device);
 }
diff --git a/arch/arm/mach-realview/realview_eb.c b/arch/arm/mach-realview/realview_eb.c
index 64c88d657f9e..b3869cbbcc68 100644
--- a/arch/arm/mach-realview/realview_eb.c
+++ b/arch/arm/mach-realview/realview_eb.c
@@ -234,7 +234,7 @@ static struct resource realview_eb_eth_resources[] = {
 	[1] = {
 		.start		= IRQ_EB_ETH,
 		.end		= IRQ_EB_ETH,
-		.flags		= IORESOURCE_IRQ,
+		.flags		= IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHEDGE,
 	},
 };
 
diff --git a/arch/arm/mach-sa1100/neponset.c b/arch/arm/mach-sa1100/neponset.c
index 169262e3040d..7b0cd3172354 100644
--- a/arch/arm/mach-sa1100/neponset.c
+++ b/arch/arm/mach-sa1100/neponset.c
@@ -12,6 +12,7 @@
 #include <linux/pm.h>
 #include <linux/serial_core.h>
 #include <linux/slab.h>
+#include <linux/smc91x.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/map.h>
@@ -258,12 +259,17 @@ static int neponset_probe(struct platform_device *dev)
 			0x02000000, "smc91x-attrib"),
 		{ .flags = IORESOURCE_IRQ },
 	};
+	struct smc91x_platdata smc91x_platdata = {
+		.flags = SMC91X_USE_8BIT | SMC91X_IO_SHIFT_2 | SMC91X_NOWAIT,
+	};
 	struct platform_device_info smc91x_devinfo = {
 		.parent = &dev->dev,
 		.name = "smc91x",
 		.id = 0,
 		.res = smc91x_resources,
 		.num_res = ARRAY_SIZE(smc91x_resources),
+		.data = &smc91c_platdata,
+		.size_data = sizeof(smc91c_platdata),
 	};
 	int ret, irq;
 
diff --git a/arch/arm/mach-sa1100/pleb.c b/arch/arm/mach-sa1100/pleb.c
index 091261878eff..696fd0fe4806 100644
--- a/arch/arm/mach-sa1100/pleb.c
+++ b/arch/arm/mach-sa1100/pleb.c
@@ -11,6 +11,7 @@
 #include <linux/irq.h>
 #include <linux/io.h>
 #include <linux/mtd/partitions.h>
+#include <linux/smc91x.h>
 
 #include <mach/hardware.h>
 #include <asm/setup.h>
@@ -43,12 +44,18 @@ static struct resource smc91x_resources[] = {
 #endif
 };
 
+static struct smc91x_platdata smc91x_platdata = {
+	.flags = SMC91X_USE_16BIT | SMC91X_NOWAIT,
+};
 
 static struct platform_device smc91x_device = {
 	.name		= "smc91x",
 	.id		= 0,
 	.num_resources	= ARRAY_SIZE(smc91x_resources),
 	.resource	= smc91x_resources,
+	.dev = {
+		.platform_data  = &smc91c_platdata,
+	},
 };
 
 static struct platform_device *devices[] __initdata = {
diff --git a/drivers/net/ethernet/smsc/smc91x.c b/drivers/net/ethernet/smsc/smc91x.c
index fa3f193b5f4d..f4700d4c9c77 100644
--- a/drivers/net/ethernet/smsc/smc91x.c
+++ b/drivers/net/ethernet/smsc/smc91x.c
@@ -91,6 +91,10 @@ static const char version[] =
 
 #include "smc91x.h"
 
+#if defined(CONFIG_ASSABET_NEPONSET)
+#include <mach/neponset.h>
+#endif
+
 #ifndef SMC_NOWAIT
 # define SMC_NOWAIT		0
 #endif
@@ -2355,8 +2359,9 @@ static int smc_drv_probe(struct platform_device *pdev)
 	ret = smc_request_attrib(pdev, ndev);
 	if (ret)
 		goto out_release_io;
-#if defined(CONFIG_ASSABET_NEPONSET) && !defined(CONFIG_SA1100_PLEB)
-	neponset_ncr_set(NCR_ENET_OSC_EN);
+#if defined(CONFIG_ASSABET_NEPONSET)
+	if (machine_is_assabet() && !machine_has_neponset())
+		neponset_ncr_set(NCR_ENET_OSC_EN);
 #endif
 	platform_set_drvdata(pdev, ndev);
 	ret = smc_enable_device(pdev);
diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h
index be67baf5f677..3a18501d1068 100644
--- a/drivers/net/ethernet/smsc/smc91x.h
+++ b/drivers/net/ethernet/smsc/smc91x.h
@@ -39,14 +39,7 @@
  * Define your architecture specific bus configuration parameters here.
  */
 
-#if defined(CONFIG_ARCH_LUBBOCK) ||\
-    defined(CONFIG_MACH_MAINSTONE) ||\
-    defined(CONFIG_MACH_ZYLONITE) ||\
-    defined(CONFIG_MACH_LITTLETON) ||\
-    defined(CONFIG_MACH_ZYLONITE2) ||\
-    defined(CONFIG_ARCH_VIPER) ||\
-    defined(CONFIG_MACH_STARGATE2) ||\
-    defined(CONFIG_ARCH_VERSATILE)
+#if defined(CONFIG_ARM)
 
 #include <asm/mach-types.h>
 
@@ -74,95 +67,8 @@
 /* We actually can't write halfwords properly if not word aligned */
 static inline void SMC_outw(u16 val, void __iomem *ioaddr, int reg)
 {
-	if ((machine_is_mainstone() || machine_is_stargate2()) && reg & 2) {
-		unsigned int v = val << 16;
-		v |= readl(ioaddr + (reg & ~2)) & 0xffff;
-		writel(v, ioaddr + (reg & ~2));
-	} else {
-		writew(val, ioaddr + reg);
-	}
-}
-
-#elif defined(CONFIG_SA1100_PLEB)
-/* We can only do 16-bit reads and writes in the static memory space. */
-#define SMC_CAN_USE_8BIT	1
-#define SMC_CAN_USE_16BIT	1
-#define SMC_CAN_USE_32BIT	0
-#define SMC_IO_SHIFT		0
-#define SMC_NOWAIT		1
-
-#define SMC_inb(a, r)		readb((a) + (r))
-#define SMC_insb(a, r, p, l)	readsb((a) + (r), p, (l))
-#define SMC_inw(a, r)		readw((a) + (r))
-#define SMC_insw(a, r, p, l)	readsw((a) + (r), p, l)
-#define SMC_outb(v, a, r)	writeb(v, (a) + (r))
-#define SMC_outsb(a, r, p, l)	writesb((a) + (r), p, (l))
-#define SMC_outw(v, a, r)	writew(v, (a) + (r))
-#define SMC_outsw(a, r, p, l)	writesw((a) + (r), p, l)
-
-#define SMC_IRQ_FLAGS		(-1)
-
-#elif defined(CONFIG_SA1100_ASSABET)
-
-#include <mach/neponset.h>
-
-/* We can only do 8-bit reads and writes in the static memory space. */
-#define SMC_CAN_USE_8BIT	1
-#define SMC_CAN_USE_16BIT	0
-#define SMC_CAN_USE_32BIT	0
-#define SMC_NOWAIT		1
-
-/* The first two address lines aren't connected... */
-#define SMC_IO_SHIFT		2
-
-#define SMC_inb(a, r)		readb((a) + (r))
-#define SMC_outb(v, a, r)	writeb(v, (a) + (r))
-#define SMC_insb(a, r, p, l)	readsb((a) + (r), p, (l))
-#define SMC_outsb(a, r, p, l)	writesb((a) + (r), p, (l))
-#define SMC_IRQ_FLAGS		(-1)	/* from resource */
-
-#elif	defined(CONFIG_MACH_LOGICPD_PXA270) ||	\
-	defined(CONFIG_MACH_NOMADIK_8815NHK)
-
-#define SMC_CAN_USE_8BIT	0
-#define SMC_CAN_USE_16BIT	1
-#define SMC_CAN_USE_32BIT	0
-#define SMC_IO_SHIFT		0
-#define SMC_NOWAIT		1
-
-#define SMC_inw(a, r)		readw((a) + (r))
-#define SMC_outw(v, a, r)	writew(v, (a) + (r))
-#define SMC_insw(a, r, p, l)	readsw((a) + (r), p, l)
-#define SMC_outsw(a, r, p, l)	writesw((a) + (r), p, l)
-
-#elif	defined(CONFIG_ARCH_INNOKOM) || \
-	defined(CONFIG_ARCH_PXA_IDP) || \
-	defined(CONFIG_ARCH_RAMSES) || \
-	defined(CONFIG_ARCH_PCM027)
-
-#define SMC_CAN_USE_8BIT	1
-#define SMC_CAN_USE_16BIT	1
-#define SMC_CAN_USE_32BIT	1
-#define SMC_IO_SHIFT		0
-#define SMC_NOWAIT		1
-#define SMC_USE_PXA_DMA		1
-
-#define SMC_inb(a, r)		readb((a) + (r))
-#define SMC_inw(a, r)		readw((a) + (r))
-#define SMC_inl(a, r)		readl((a) + (r))
-#define SMC_outb(v, a, r)	writeb(v, (a) + (r))
-#define SMC_outl(v, a, r)	writel(v, (a) + (r))
-#define SMC_insl(a, r, p, l)	readsl((a) + (r), p, l)
-#define SMC_outsl(a, r, p, l)	writesl((a) + (r), p, l)
-#define SMC_insw(a, r, p, l)	readsw((a) + (r), p, l)
-#define SMC_outsw(a, r, p, l)	writesw((a) + (r), p, l)
-#define SMC_IRQ_FLAGS		(-1)	/* from resource */
-
-/* We actually can't write halfwords properly if not word aligned */
-static inline void
-SMC_outw(u16 val, void __iomem *ioaddr, int reg)
-{
-	if (reg & 2) {
+	if ((machine_is_mainstone() || machine_is_stargate2() ||
+	     machine_is_pxa_idp()) && reg & 2) {
 		unsigned int v = val << 16;
 		v |= readl(ioaddr + (reg & ~2)) & 0xffff;
 		writel(v, ioaddr + (reg & ~2));
@@ -237,20 +143,6 @@ SMC_outw(u16 val, void __iomem *ioaddr, int reg)
 #define RPC_LSA_DEFAULT         RPC_LED_100_10
 #define RPC_LSB_DEFAULT         RPC_LED_TX_RX
 
-#elif defined(CONFIG_ARCH_MSM)
-
-#define SMC_CAN_USE_8BIT	0
-#define SMC_CAN_USE_16BIT	1
-#define SMC_CAN_USE_32BIT	0
-#define SMC_NOWAIT		1
-
-#define SMC_inw(a, r)		readw((a) + (r))
-#define SMC_outw(v, a, r)	writew(v, (a) + (r))
-#define SMC_insw(a, r, p, l)	readsw((a) + (r), p, l)
-#define SMC_outsw(a, r, p, l)	writesw((a) + (r), p, l)
-
-#define SMC_IRQ_FLAGS		IRQF_TRIGGER_HIGH
-
 #elif defined(CONFIG_COLDFIRE)
 
 #define SMC_CAN_USE_8BIT	0


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

* Re: [PATCH] net: smc91x: improve neponset hack
  2015-02-20 15:47   ` Arnd Bergmann
@ 2015-02-20 20:00     ` Robert Jarzmik
  2015-02-20 21:23     ` David Miller
  2015-03-09 12:20     ` Russell King - ARM Linux
  2 siblings, 0 replies; 9+ messages in thread
From: Robert Jarzmik @ 2015-02-20 20:00 UTC (permalink / raw)
  To: Arnd Bergmann, Russell King - ARM Linux
  Cc: linux-arm-kernel, Dmitry Eremin-Solenikov, netdev, Linus Walleij,
	linux-kernel, davem

Arnd Bergmann <arnd@arndb.de> writes:

> On Thursday 19 February 2015 00:35:49 Russell King - ARM Linux wrote:
>> On Wed, Feb 18, 2015 at 08:47:30PM +0100, Arnd Bergmann wrote:
>> > The smc91x driver tries to support multiple platforms at compile
>> > time, but they are mutually exclusive at runtime, and not clearly
>> > defined.
>> 
>> I'd prefer to rework this to fix that properly.  From what I remember,
>> the whole SA11x0 stuff in this driver was a mess.
>
> I guess that's reasonable. I've looked through the driver and it seems we
> did most of the multiplatform work but left a few things alone that should
> have been converted a long time ago.
>
> Can you check if the approach below makes sense to you?
>
> I've verified that each machine that defines an smc91x device now sets
> the correct platform data, irq flags and access width, which should be
> enough to collapse all the CONFIG_ARM cases into one.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
When you're both satisfied with the smc91x patch, I can offer to test it across
the pxa boards (lubbock, zylonite, and maybe mainstone). As I'm actively using
the first 2, I'm interested in them being as functional as now :)

Cheers.

--
Robert

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

* Re: [PATCH] net: smc91x: improve neponset hack
  2015-02-20 15:47   ` Arnd Bergmann
  2015-02-20 20:00     ` Robert Jarzmik
@ 2015-02-20 21:23     ` David Miller
  2015-03-09 12:20     ` Russell King - ARM Linux
  2 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2015-02-20 21:23 UTC (permalink / raw)
  To: arnd
  Cc: linux-arm-kernel, linux, dbaryshkov, netdev, linus.walleij, linux-kernel

From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 20 Feb 2015 16:47:06 +0100

> On Thursday 19 February 2015 00:35:49 Russell King - ARM Linux wrote:
>> On Wed, Feb 18, 2015 at 08:47:30PM +0100, Arnd Bergmann wrote:
>> > The smc91x driver tries to support multiple platforms at compile
>> > time, but they are mutually exclusive at runtime, and not clearly
>> > defined.
>> 
>> I'd prefer to rework this to fix that properly.  From what I remember,
>> the whole SA11x0 stuff in this driver was a mess.
> 
> I guess that's reasonable. I've looked through the driver and it seems we
> did most of the multiplatform work but left a few things alone that should
> have been converted a long time ago.
> 
> Can you check if the approach below makes sense to you?
> 
> I've verified that each machine that defines an smc91x device now sets
> the correct platform data, irq flags and access width, which should be
> enough to collapse all the CONFIG_ARM cases into one.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

This is a nice cleanup but for 'net' to fix the build error I prefer
the original one-line patch.

We can apply this thing here to net-next.

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

* Re: [PATCH] net: smc91x: improve neponset hack
  2015-02-18 19:47 [PATCH] net: smc91x: improve neponset hack Arnd Bergmann
  2015-02-19  0:35 ` Russell King - ARM Linux
@ 2015-02-20 21:23 ` David Miller
  1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2015-02-20 21:23 UTC (permalink / raw)
  To: arnd
  Cc: netdev, linux-arm-kernel, rmk+kernel, dbaryshkov, linux-kernel,
	linus.walleij

From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 18 Feb 2015 20:47:30 +0100

> The smc91x driver tries to support multiple platforms at compile
> time, but they are mutually exclusive at runtime, and not clearly
> defined.
> 
> Trying to build for CONFIG_SA1100_ASSABET without CONFIG_ASSABET_NEPONSET
> results in this link error:
> 
> drivers/built-in.o: In function `smc_drv_probe':
> :(.text+0x33310c): undefined reference to `neponset_ncr_frob'
> 
> since the neponset_ncr_set function is not defined otherwise.
> 
> Similarly, building for both CONFIG_SA1100_ASSABET and CONFIG_SA1100_PLEB
> results in a different build error:
> 
> smsc/smc91x.c: In function 'smc_drv_probe':
> smsc/smc91x.c:2299:2: error: implicit declaration of function 'neponset_ncr_set' [-Werror=implicit-function-declaration]
>   neponset_ncr_set(NCR_ENET_OSC_EN);
>   ^
> smsc/smc91x.c:2299:19: error: 'NCR_ENET_OSC_EN' undeclared (first use in this function)
>   neponset_ncr_set(NCR_ENET_OSC_EN);
>                    ^
> 
> This is an attempt to fix the call site responsible for both
> errors, making sure we call the function exactly when the driver
> is actually trying to run on the assabet/neponset machine. With
> this patch, I no longer see randconfig build errors in this file.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I know this isn't the desired long term solution to this problem, but
it's the most minimal risk-free fix for these build failures.

Thus, I've applied this, thanks.

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

* Re: [PATCH] net: smc91x: improve neponset hack
  2015-02-20 15:47   ` Arnd Bergmann
  2015-02-20 20:00     ` Robert Jarzmik
  2015-02-20 21:23     ` David Miller
@ 2015-03-09 12:20     ` Russell King - ARM Linux
  2015-03-09 12:34       ` Russell King - ARM Linux
  2 siblings, 1 reply; 9+ messages in thread
From: Russell King - ARM Linux @ 2015-03-09 12:20 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, Dmitry Eremin-Solenikov, netdev, Linus Walleij,
	linux-kernel, davem

On Fri, Feb 20, 2015 at 04:47:06PM +0100, Arnd Bergmann wrote:
> On Thursday 19 February 2015 00:35:49 Russell King - ARM Linux wrote:
> > On Wed, Feb 18, 2015 at 08:47:30PM +0100, Arnd Bergmann wrote:
> > > The smc91x driver tries to support multiple platforms at compile
> > > time, but they are mutually exclusive at runtime, and not clearly
> > > defined.
> > 
> > I'd prefer to rework this to fix that properly.  From what I remember,
> > the whole SA11x0 stuff in this driver was a mess.
> 
> I guess that's reasonable. I've looked through the driver and it seems we
> did most of the multiplatform work but left a few things alone that should
> have been converted a long time ago.
> 
> Can you check if the approach below makes sense to you?

I don't know who's carrying what patches, but this is still broken.

arch/arm/mach-sa1100/neponset.c: In function 'neponset_probe':
arch/arm/mach-sa1100/neponset.c:271:12: error: 'smc91c_platdata' undeclared (first use in this function)
drivers/net/ethernet/smsc/smc91x.c: In function 'smc_drv_probe':
drivers/net/ethernet/smsc/smc91x.c:2363:2: error: implicit declaration of function 'machine_has_neponset' [-Werror=implicit-function-declaration]

This is from building Linus' tree as of Saturday plus arm-soc.

Let's revert all this crap and start again, this time testing it better?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

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

* Re: [PATCH] net: smc91x: improve neponset hack
  2015-03-09 12:20     ` Russell King - ARM Linux
@ 2015-03-09 12:34       ` Russell King - ARM Linux
  2015-03-09 17:14         ` David Miller
  0 siblings, 1 reply; 9+ messages in thread
From: Russell King - ARM Linux @ 2015-03-09 12:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Dmitry Eremin-Solenikov, netdev, Linus Walleij, linux-kernel,
	davem, linux-arm-kernel

On Mon, Mar 09, 2015 at 12:20:12PM +0000, Russell King - ARM Linux wrote:
> On Fri, Feb 20, 2015 at 04:47:06PM +0100, Arnd Bergmann wrote:
> > On Thursday 19 February 2015 00:35:49 Russell King - ARM Linux wrote:
> > > On Wed, Feb 18, 2015 at 08:47:30PM +0100, Arnd Bergmann wrote:
> > > > The smc91x driver tries to support multiple platforms at compile
> > > > time, but they are mutually exclusive at runtime, and not clearly
> > > > defined.
> > > 
> > > I'd prefer to rework this to fix that properly.  From what I remember,
> > > the whole SA11x0 stuff in this driver was a mess.
> > 
> > I guess that's reasonable. I've looked through the driver and it seems we
> > did most of the multiplatform work but left a few things alone that should
> > have been converted a long time ago.
> > 
> > Can you check if the approach below makes sense to you?
> 
> I don't know who's carrying what patches, but this is still broken.
> 
> arch/arm/mach-sa1100/neponset.c: In function 'neponset_probe':
> arch/arm/mach-sa1100/neponset.c:271:12: error: 'smc91c_platdata' undeclared (first use in this function)
> drivers/net/ethernet/smsc/smc91x.c: In function 'smc_drv_probe':
> drivers/net/ethernet/smsc/smc91x.c:2363:2: error: implicit declaration of function 'machine_has_neponset' [-Werror=implicit-function-declaration]
> 
> This is from building Linus' tree as of Saturday plus arm-soc.
> 
> Let's revert all this crap and start again, this time testing it better?

I'm also getting:

arch/arm/mach-pxa/idp.c:84:15: error: variable 'smc91x_platdata' has initializer but incomplete type
arch/arm/mach-pxa/idp.c:85:2: error: unknown field 'flags' specified in initializer
arch/arm/mach-pxa/idp.c:85:11: error: 'SMC91X_USE_32BIT' undeclared here (not in a function)
arch/arm/mach-pxa/idp.c:85:30: error: 'SMC91X_USE_DMA' undeclared here (not in a function)
arch/arm/mach-pxa/idp.c:85:47: error: 'SMC91X_NOWAIT' undeclared here (not in a function)
arch/arm/mach-pxa/idp.c:85:2: warning: excess elements in struct initializer [enabled by default]
arch/arm/mach-pxa/idp.c:85:2: warning: (near initialization for 'smc91x_platdata') [enabled by default]
arch/arm/mach-pxa/lpd270.c:198:30: error: expected '}' before ';' token


-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

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

* Re: [PATCH] net: smc91x: improve neponset hack
  2015-03-09 12:34       ` Russell King - ARM Linux
@ 2015-03-09 17:14         ` David Miller
  0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2015-03-09 17:14 UTC (permalink / raw)
  To: linux
  Cc: arnd, dbaryshkov, netdev, linus.walleij, linux-kernel, linux-arm-kernel

From: Russell King - ARM Linux <linux@arm.linux.org.uk>
Date: Mon, 9 Mar 2015 12:34:47 +0000

> On Mon, Mar 09, 2015 at 12:20:12PM +0000, Russell King - ARM Linux wrote:
>> On Fri, Feb 20, 2015 at 04:47:06PM +0100, Arnd Bergmann wrote:
>> > On Thursday 19 February 2015 00:35:49 Russell King - ARM Linux wrote:
>> > > On Wed, Feb 18, 2015 at 08:47:30PM +0100, Arnd Bergmann wrote:
>> > > > The smc91x driver tries to support multiple platforms at compile
>> > > > time, but they are mutually exclusive at runtime, and not clearly
>> > > > defined.
>> > > 
>> > > I'd prefer to rework this to fix that properly.  From what I remember,
>> > > the whole SA11x0 stuff in this driver was a mess.
>> > 
>> > I guess that's reasonable. I've looked through the driver and it seems we
>> > did most of the multiplatform work but left a few things alone that should
>> > have been converted a long time ago.
>> > 
>> > Can you check if the approach below makes sense to you?
>> 
>> I don't know who's carrying what patches, but this is still broken.
>> 
>> arch/arm/mach-sa1100/neponset.c: In function 'neponset_probe':
>> arch/arm/mach-sa1100/neponset.c:271:12: error: 'smc91c_platdata' undeclared (first use in this function)
>> drivers/net/ethernet/smsc/smc91x.c: In function 'smc_drv_probe':
>> drivers/net/ethernet/smsc/smc91x.c:2363:2: error: implicit declaration of function 'machine_has_neponset' [-Werror=implicit-function-declaration]
>> 
>> This is from building Linus' tree as of Saturday plus arm-soc.
>> 
>> Let's revert all this crap and start again, this time testing it better?
> 
> I'm also getting:

I have the fix in my tree still and is pending to be sent to Linus.

Please be patient.

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

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

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-18 19:47 [PATCH] net: smc91x: improve neponset hack Arnd Bergmann
2015-02-19  0:35 ` Russell King - ARM Linux
2015-02-20 15:47   ` Arnd Bergmann
2015-02-20 20:00     ` Robert Jarzmik
2015-02-20 21:23     ` David Miller
2015-03-09 12:20     ` Russell King - ARM Linux
2015-03-09 12:34       ` Russell King - ARM Linux
2015-03-09 17:14         ` David Miller
2015-02-20 21:23 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).