linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] Fixing ethernet driver compilation error for i.MX31 ADS board
@ 2010-09-30 15:47 Ian Lartey
  2010-09-30 15:47 ` [PATCH 2/2] Adding cs89x0, standard ethernet driver for iMX31 platform Ian Lartey
  2010-09-30 18:14 ` [PATCH 1/2] Fixing ethernet driver compilation error for i.MX31 ADS board Uwe Kleine-König
  0 siblings, 2 replies; 11+ messages in thread
From: Ian Lartey @ 2010-09-30 15:47 UTC (permalink / raw)
  To: linux-arm-kernel

This is only a partial revert of "ARM: mx3/mx31ads: fold board header in
its only user" [commit ccfa7c269843001077df02d98918c6c9bde91395)]

As some of the the board defines are also used in the cs89x0 ethernet
driver by the i.MX31 ADS.

Signed-off-by: Ian Lartey <ian@opensource.wolfsonmicro.com>
---
 arch/arm/mach-mx3/mach-mx31ads.c               |   43 +++---------------
 arch/arm/plat-mxc/include/mach/board-mx31ads.h |   57 ++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 36 deletions(-)
 create mode 100644 arch/arm/plat-mxc/include/mach/board-mx31ads.h

diff --git a/arch/arm/mach-mx3/mach-mx31ads.c b/arch/arm/mach-mx3/mach-mx31ads.c
index 94b3e7c..7d13e99 100644
--- a/arch/arm/mach-mx3/mach-mx31ads.c
+++ b/arch/arm/mach-mx3/mach-mx31ads.c
@@ -29,6 +29,7 @@
 #include <asm/memory.h>
 #include <asm/mach/map.h>
 #include <mach/common.h>
+#include <mach/board-mx31ads.h>
 #include <mach/iomux-mx3.h>
 
 #ifdef CONFIG_MACH_MX31ADS_WM1133_EV1
@@ -40,42 +41,12 @@
 #include "devices-imx31.h"
 #include "devices.h"
 
-/* Base address of PBC controller */
-#define PBC_BASE_ADDRESS        MX31_CS4_BASE_ADDR_VIRT
-/* Offsets for the PBC Controller register */
-
-/* PBC Board interrupt status register */
-#define PBC_INTSTATUS           0x000016
-
-/* PBC Board interrupt current status register */
-#define PBC_INTCURR_STATUS      0x000018
-
-/* PBC Interrupt mask register set address */
-#define PBC_INTMASK_SET         0x00001A
-
-/* PBC Interrupt mask register clear address */
-#define PBC_INTMASK_CLEAR       0x00001C
-
-/* External UART A */
-#define PBC_SC16C652_UARTA      0x010000
-
-/* External UART B */
-#define PBC_SC16C652_UARTB      0x010010
-
-#define PBC_INTSTATUS_REG	(PBC_INTSTATUS + PBC_BASE_ADDRESS)
-#define PBC_INTMASK_SET_REG	(PBC_INTMASK_SET + PBC_BASE_ADDRESS)
-#define PBC_INTMASK_CLEAR_REG	(PBC_INTMASK_CLEAR + PBC_BASE_ADDRESS)
-#define EXPIO_PARENT_INT	IOMUX_TO_IRQ(MX31_PIN_GPIO1_4)
-
-#define MXC_EXP_IO_BASE		(MXC_BOARD_IRQ_START)
-#define MXC_IRQ_TO_EXPIO(irq)	((irq) - MXC_EXP_IO_BASE)
-
-#define EXPIO_INT_XUART_INTA	(MXC_EXP_IO_BASE + 10)
-#define EXPIO_INT_XUART_INTB	(MXC_EXP_IO_BASE + 11)
-
-#define MXC_MAX_EXP_IO_LINES	16
-/*
- * This file contains the board-specific initialization routines.
+/*!
+ * @file mx31ads.c
+ *
+ * @brief This file contains the board-specific initialization routines.
+ *
+ * @ingroup System
  */
 
 #if defined(CONFIG_SERIAL_8250) || defined(CONFIG_SERIAL_8250_MODULE)
diff --git a/arch/arm/plat-mxc/include/mach/board-mx31ads.h b/arch/arm/plat-mxc/include/mach/board-mx31ads.h
new file mode 100644
index 0000000..6a019bd
--- /dev/null
+++ b/arch/arm/plat-mxc/include/mach/board-mx31ads.h
@@ -0,0 +1,57 @@
+/*
+ * Copyright 2005-2007 Freescale Semiconductor, Inc. All Rights Reserved.
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ASM_ARCH_MXC_BOARD_MX31ADS_H__
+#define __ASM_ARCH_MXC_BOARD_MX31ADS_H__
+
+#include <mach/hardware.h>
+
+/* Base address of PBC controller */
+#define PBC_BASE_ADDRESS        MX31_CS4_BASE_ADDR_VIRT
+/* Offsets for the PBC Controller register */
+
+/* PBC Board interrupt status register */
+#define PBC_INTSTATUS           0x000016
+
+/* PBC Board interrupt current status register */
+#define PBC_INTCURR_STATUS      0x000018
+
+/* PBC Interrupt mask register set address */
+#define PBC_INTMASK_SET         0x00001A
+
+/* PBC Interrupt mask register clear address */
+#define PBC_INTMASK_CLEAR       0x00001C
+
+/* External UART A */
+#define PBC_SC16C652_UARTA      0x010000
+
+/* External UART B */
+#define PBC_SC16C652_UARTB      0x010010
+
+/* Ethernet Controller IO base address */
+#define PBC_CS8900A_IOBASE      0x020000
+
+#define PBC_INTSTATUS_REG	(PBC_INTSTATUS + PBC_BASE_ADDRESS)
+#define PBC_INTMASK_SET_REG	(PBC_INTMASK_SET + PBC_BASE_ADDRESS)
+#define PBC_INTMASK_CLEAR_REG	(PBC_INTMASK_CLEAR + PBC_BASE_ADDRESS)
+#define EXPIO_PARENT_INT	IOMUX_TO_IRQ(MX31_PIN_GPIO1_4)
+
+#define MXC_EXP_IO_BASE		(MXC_BOARD_IRQ_START)
+#define MXC_IRQ_TO_EXPIO(irq)	((irq) - MXC_EXP_IO_BASE)
+
+
+#define EXPIO_INT_ENET_INT	(MXC_EXP_IO_BASE + 8)
+
+#define EXPIO_INT_XUART_INTA	(MXC_EXP_IO_BASE + 10)
+#define EXPIO_INT_XUART_INTB	(MXC_EXP_IO_BASE + 11)
+
+#define MXC_MAX_EXP_IO_LINES	16
+
+#endif /* __ASM_ARCH_MXC_BOARD_MX31ADS_H__ */
-- 
1.7.0.4

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

* [PATCH 2/2] Adding cs89x0, standard ethernet driver for iMX31 platform.
  2010-09-30 15:47 [PATCH 1/2] Fixing ethernet driver compilation error for i.MX31 ADS board Ian Lartey
@ 2010-09-30 15:47 ` Ian Lartey
  2010-09-30 18:23   ` Uwe Kleine-König
  2010-09-30 18:14 ` [PATCH 1/2] Fixing ethernet driver compilation error for i.MX31 ADS board Uwe Kleine-König
  1 sibling, 1 reply; 11+ messages in thread
From: Ian Lartey @ 2010-09-30 15:47 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Ian Lartey <ian@opensource.wolfsonmicro.com>
---
 arch/arm/configs/mx3_defconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/configs/mx3_defconfig b/arch/arm/configs/mx3_defconfig
index 161f907..b0b3409 100644
--- a/arch/arm/configs/mx3_defconfig
+++ b/arch/arm/configs/mx3_defconfig
@@ -68,6 +68,8 @@ CONFIG_NET_ETHERNET=y
 CONFIG_SMSC911X=y
 CONFIG_DNET=y
 CONFIG_FEC=y
+CONFIG_CS89x0=y
+CONFIG_CS89x0_NONISA_IRQ=y
 # CONFIG_NETDEV_1000 is not set
 # CONFIG_NETDEV_10000 is not set
 # CONFIG_INPUT is not set
-- 
1.7.0.4

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

* [PATCH 1/2] Fixing ethernet driver compilation error for i.MX31 ADS board
  2010-09-30 15:47 [PATCH 1/2] Fixing ethernet driver compilation error for i.MX31 ADS board Ian Lartey
  2010-09-30 15:47 ` [PATCH 2/2] Adding cs89x0, standard ethernet driver for iMX31 platform Ian Lartey
@ 2010-09-30 18:14 ` Uwe Kleine-König
  2010-10-01 10:33   ` Ian Lartey
  1 sibling, 1 reply; 11+ messages in thread
From: Uwe Kleine-König @ 2010-09-30 18:14 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On Thu, Sep 30, 2010 at 04:47:34PM +0100, Ian Lartey wrote:
> This is only a partial revert of "ARM: mx3/mx31ads: fold board header in
> its only user" [commit ccfa7c269843001077df02d98918c6c9bde91395)]
> 
> As some of the the board defines are also used in the cs89x0 ethernet
> driver by the i.MX31 ADS.
> 
> Signed-off-by: Ian Lartey <ian@opensource.wolfsonmicro.com>
It seems you got me wrong.  I wanted you to just define the constants in
arch/arm/plat-mxc/include/mach/board-mx31ads.h that are needed by the
cs89x0 driver.  That is PBC_BASE_ADDRESS, PBC_CS8900A_IOBASE and
EXPIO_INT_ENET_INT.  And I wanted a comment like:

	/* 
	 * These symbols are used by drivers/net/cs89x0.c.
	 * This is ugly as hell, but we have to provide them until
	 * someone fixed the driver.
	 */

And you definitly shouldn't reintroduce these doxygen comments.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

* [PATCH 2/2] Adding cs89x0, standard ethernet driver for iMX31 platform.
  2010-09-30 15:47 ` [PATCH 2/2] Adding cs89x0, standard ethernet driver for iMX31 platform Ian Lartey
@ 2010-09-30 18:23   ` Uwe Kleine-König
  2010-10-01 10:46     ` Ian Lartey
  0 siblings, 1 reply; 11+ messages in thread
From: Uwe Kleine-König @ 2010-09-30 18:23 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 30, 2010 at 04:47:35PM +0100, Ian Lartey wrote:
> Signed-off-by: Ian Lartey <ian@opensource.wolfsonmicro.com>
> ---
>  arch/arm/configs/mx3_defconfig |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/configs/mx3_defconfig b/arch/arm/configs/mx3_defconfig
> index 161f907..b0b3409 100644
> --- a/arch/arm/configs/mx3_defconfig
> +++ b/arch/arm/configs/mx3_defconfig
> @@ -68,6 +68,8 @@ CONFIG_NET_ETHERNET=y
>  CONFIG_SMSC911X=y
>  CONFIG_DNET=y
>  CONFIG_FEC=y
> +CONFIG_CS89x0=y
> +CONFIG_CS89x0_NONISA_IRQ=y
>  # CONFIG_NETDEV_1000 is not set
>  # CONFIG_NETDEV_10000 is not set
>  # CONFIG_INPUT is not set
This change is bad for two reasons:

 - I just tested a kernel with CONFIG_CS89x0=y on a Phytec Phycore
   pcm043 using your updated defconfig and it barfs:

	...
	cs89x0:cs89x0_probe(0x0)
	Unable to handle kernel paging request at virtual address f402030a
	pgd = c0004000
	[f402030a] *pgd=00000000
	Internal error: Oops: 5 [#1] PREEMPT
	last sysfs file: 
	Modules linked in:
	CPU: 0    Not tainted  (2.6.36-rc6-00104-g1374af3 #13)
	PC is at cs89x0_probe1+0xb4/0x6cc
	LR is at kmem_cache_alloc+0x94/0xa4
	pc : [<c00188dc>]    lr : [<c009da7c>]    psr: 60000013
	sp : c781df08  ip : c036d1cc  fp : 00000000
	r10: 00000000  r9 : 00000000  r8 : c0018670
	r7 : f4020300  r6 : f4020300  r5 : c79f2800  r4 : c79f2b20
	r3 : 00000000  r2 : 0000000a  r1 : f402030f  r0 : c79f9cc0
	Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
	Control: 00c5387d  Table: 80004008  DAC: 00000017
	Process swapper (pid: 1, stack limit = 0xc781c268)
	Stack: (0xc781df08 to 0xc781e000)
	df00:                   00000000 c781df7c c79f2803 00000002 c031f56c ffffffff
	df20: c031f56e c79f2800 3860d7ff c017daac ff0a0210 ffffffff 00000000 c781df40
	df40: ff0a0210 00000000 c79f2800 00000000 00000000 c0018670 00000000 00000000
	df60: c00224ec c79f2800 00000000 00000000 c0018670 00000000 00000000 c0018f80
	df80: 00000000 c00224b8 00000001 c0018634 c0025f2c 00000000 00000001 c0025f2c
	dfa0: 00000013 c00186f0 c001fa44 c001f9b0 c001fa44 c00243fc 00000039 00000000
	dfc0: 00000000 000000f4 c036edfc c001f9b0 c001fa44 c0025f2c 00000013 00000000
	dfe0: 00000000 c0008408 00000000 00000000 c0008370 c0025f2c fd59f476 ffdddbee
	[<c00188dc>] (cs89x0_probe1+0xb4/0x6cc) from [<c0018f80>] (cs89x0_probe+0x8c/0xe4)
	[<c0018f80>] (cs89x0_probe+0x8c/0xe4) from [<c0018634>] (probe_list2+0x34/0x70)
	[<c0018634>] (probe_list2+0x34/0x70) from [<c00186f0>] (net_olddevs_init+0x80/0xc8)
	[<c00186f0>] (net_olddevs_init+0x80/0xc8) from [<c00243fc>] (do_one_initcall+0xc4/0x190)
	[<c00243fc>] (do_one_initcall+0xc4/0x190) from [<c0008408>] (kernel_init+0x98/0x150)
	[<c0008408>] (kernel_init+0x98/0x150) from [<c0025f2c>] (kernel_thread_exit+0x0/0x8)
	Code: eb09cd60 ea000012 e3c77003 e3a0200a (e1d730ba) 
	---[ end trace feac84c64e9ed5aa ]---
	Kernel panic - not syncing: Attempted to kill init!
	[<c002a6b0>] (unwind_backtrace+0x0/0xec) from [<c028bd28>] (panic+0x60/0x18c)
	[<c028bd28>] (panic+0x60/0x18c) from [<c0040ad8>] (do_exit+0x64/0x634)
	[<c0040ad8>] (do_exit+0x64/0x634) from [<c0028bc0>] (die+0x2bc/0x2fc)
	[<c0028bc0>] (die+0x2bc/0x2fc) from [<c002b534>] (__do_kernel_fault+0x64/0x84)
	[<c002b534>] (__do_kernel_fault+0x64/0x84) from [<c002b7b0>] (do_bad_area+0x6c/0x78)
	[<c002b7b0>] (do_bad_area+0x6c/0x78) from [<c002b85c>] (do_translation_fault+0xa0/0xb0)
	[<c002b85c>] (do_translation_fault+0xa0/0xb0) from [<c00242d4>] (do_DataAbort+0x34/0x94)
	[<c00242d4>] (do_DataAbort+0x34/0x94) from [<c0024a4c>] (__dabt_svc+0x4c/0x80)
	Exception stack(0xc781dec0 to 0xc781df08)
	dec0: c79f9cc0 f402030f 0000000a 00000000 c79f2b20 c79f2800 f4020300 f4020300
	dee0: c0018670 00000000 00000000 00000000 c036d1cc c781df08 c009da7c c00188dc
	df00: 60000013 ffffffff
	[<c0024a4c>] (__dabt_svc+0x4c/0x80) from [<c00188dc>] (cs89x0_probe1+0xb4/0x6cc)
	[<c00188dc>] (cs89x0_probe1+0xb4/0x6cc) from [<c0018f80>] (cs89x0_probe+0x8c/0xe4)
	[<c0018f80>] (cs89x0_probe+0x8c/0xe4) from [<c0018634>] (probe_list2+0x34/0x70)
	[<c0018634>] (probe_list2+0x34/0x70) from [<c00186f0>] (net_olddevs_init+0x80/0xc8)
	[<c00186f0>] (net_olddevs_init+0x80/0xc8) from [<c00243fc>] (do_one_initcall+0xc4/0x190)
	[<c00243fc>] (do_one_initcall+0xc4/0x190) from [<c0008408>] (kernel_init+0x98/0x150)
	[<c0008408>] (kernel_init+0x98/0x150) from [<c0025f2c>] (kernel_thread_exit+0x0/0x8)

   (Did I already mention that this cs89x0 driver is ugly and needs a
   general overhaul?)

 - adding "CONFIG_CS89x0_NONISA_IRQ=y" is redundant as it is always on
   for MACH_MX31ADS.

So I suggest to just add CONFIG_CS89x0=m

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

* [PATCH 1/2] Fixing ethernet driver compilation error for i.MX31 ADS board
  2010-09-30 18:14 ` [PATCH 1/2] Fixing ethernet driver compilation error for i.MX31 ADS board Uwe Kleine-König
@ 2010-10-01 10:33   ` Ian Lartey
  2010-10-01 12:42     ` Uwe Kleine-König
  0 siblings, 1 reply; 11+ messages in thread
From: Ian Lartey @ 2010-10-01 10:33 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 2010-09-30 at 20:14 +0200, Uwe Kleine-K?nig wrote:
> Hello,
> 
> On Thu, Sep 30, 2010 at 04:47:34PM +0100, Ian Lartey wrote:
> > This is only a partial revert of "ARM: mx3/mx31ads: fold board header in
> > its only user" [commit ccfa7c269843001077df02d98918c6c9bde91395)]
> > 
> > As some of the the board defines are also used in the cs89x0 ethernet
> > driver by the i.MX31 ADS.
> > 
> > Signed-off-by: Ian Lartey <ian@opensource.wolfsonmicro.com>
> It seems you got me wrong.  I wanted you to just define the constants in
> arch/arm/plat-mxc/include/mach/board-mx31ads.h that are needed by the
> cs89x0 driver.  That is PBC_BASE_ADDRESS, PBC_CS8900A_IOBASE and
> EXPIO_INT_ENET_IAm I right in assuming NT.  And I wanted a comment like:
> 
> 	/* 
> 	 * These symbols are used by drivers/net/cs89x0.c.
> 	 * This is ugly as hell, but we have to provide them until
> 	 * someone fixed the driver.
> 	 */
> 

Ah, yes it is _very ugly which was why I was trying everything else but
I can understand your reasoning with the current state of the driver.


Just to be clear. You want just the above defines in board-mx31ads.h and
either:

1. Common defines to be removed from board-mx31ads.c to board-mx31ads.h
_or_
2. Duplicate defines in both board-mx31ads.c and board-mx31ads.h.


I'm assuming that you want 1. (2. is too ugly and dangerous). 

> And you definitly shouldn't reintroduce these doxygen comments.

Right. I'll watch this in future too.

> 
> Best regards
> Uwe
> 


Best Regards,
              Ian.

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

* [PATCH 2/2] Adding cs89x0, standard ethernet driver for iMX31 platform.
  2010-09-30 18:23   ` Uwe Kleine-König
@ 2010-10-01 10:46     ` Ian Lartey
  2010-10-01 12:49       ` Uwe Kleine-König
  0 siblings, 1 reply; 11+ messages in thread
From: Ian Lartey @ 2010-10-01 10:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 2010-09-30 at 20:23 +0200, Uwe Kleine-K?nig wrote:
> On Thu, Sep 30, 2010 at 04:47:35PM +0100, Ian Lartey wrote:
> > Signed-off-by: Ian Lartey <ian@opensource.wolfsonmicro.com>
> > ---
> >  arch/arm/configs/mx3_defconfig |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/configs/mx3_defconfig b/arch/arm/configs/mx3_defconfig
> > index 161f907..b0b3409 100644
> > --- a/arch/arm/configs/mx3_defconfig
> > +++ b/arch/arm/configs/mx3_defconfig
> > @@ -68,6 +68,8 @@ CONFIG_NET_ETHERNET=y
> >  CONFIG_SMSC911X=y
> >  CONFIG_DNET=y
> >  CONFIG_FEC=y
> > +CONFIG_CS89x0=y
> > +CONFIG_CS89x0_NONISA_IRQ=y
> >  # CONFIG_NETDEV_1000 is not set
> >  # CONFIG_NETDEV_10000 is not set
> >  # CONFIG_INPUT is not set
> This change is bad for two reasons:
> 
>  - I just tested a kernel with CONFIG_CS89x0=y on a Phytec Phycore
>    pcm043 using your updated defconfig and it barfs:
> 
> 	[.....]

Ouch, I'll see if I can correct that. (Needs to be done before I can add
the config file change). Uwe I don't have another i.MX3 (apart from the
i.MV31ADS) would you be able to test further changes for me please to
help catch this kind of thing ?
 
> 
>    (Did I already mention that this cs89x0 driver is ugly and needs a
>    general overhaul?)
> 

Yes, and I'm getting additional errors from the cs89x0 driver once it is
up and running so I was going to start looking for help once my changes
were in.... any volunteers ?

>  - adding "CONFIG_CS89x0_NONISA_IRQ=y" is redundant as it is always on
>    for MACH_MX31ADS.
> 
> So I suggest to just add CONFIG_CS89x0=m

Agreed.
 
> 
> Best regards
> Uwe
> 

Best Regards,
             Ian.

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

* [PATCH 1/2] Fixing ethernet driver compilation error for i.MX31 ADS board
  2010-10-01 10:33   ` Ian Lartey
@ 2010-10-01 12:42     ` Uwe Kleine-König
  0 siblings, 0 replies; 11+ messages in thread
From: Uwe Kleine-König @ 2010-10-01 12:42 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Ian,

On Fri, Oct 01, 2010 at 11:33:36AM +0100, Ian Lartey wrote:
> On Thu, 2010-09-30 at 20:14 +0200, Uwe Kleine-K?nig wrote:
> > Hello,
> > 
> > On Thu, Sep 30, 2010 at 04:47:34PM +0100, Ian Lartey wrote:
> > > This is only a partial revert of "ARM: mx3/mx31ads: fold board header in
> > > its only user" [commit ccfa7c269843001077df02d98918c6c9bde91395)]
> > > 
> > > As some of the the board defines are also used in the cs89x0 ethernet
> > > driver by the i.MX31 ADS.
> > > 
> > > Signed-off-by: Ian Lartey <ian@opensource.wolfsonmicro.com>
> > It seems you got me wrong.  I wanted you to just define the constants in
> > arch/arm/plat-mxc/include/mach/board-mx31ads.h that are needed by the
> > cs89x0 driver.  That is PBC_BASE_ADDRESS, PBC_CS8900A_IOBASE and
> > EXPIO_INT_ENET_IAm I right in assuming NT.  And I wanted a comment like:
> > 
> > 	/* 
> > 	 * These symbols are used by drivers/net/cs89x0.c.
> > 	 * This is ugly as hell, but we have to provide them until
> > 	 * someone fixed the driver.
> > 	 */
> > 
> 
> Ah, yes it is _very ugly which was why I was trying everything else but
> I can understand your reasoning with the current state of the driver.
> 
> 
> Just to be clear. You want just the above defines in board-mx31ads.h and
> either:
> 
> 1. Common defines to be removed from board-mx31ads.c to board-mx31ads.h
> _or_
> 2. Duplicate defines in both board-mx31ads.c and board-mx31ads.h.
> 
> I'm assuming that you want 1. (2. is too ugly and dangerous). 
Right, I want 1.
 
Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

* [PATCH 2/2] Adding cs89x0, standard ethernet driver for iMX31 platform.
  2010-10-01 10:46     ` Ian Lartey
@ 2010-10-01 12:49       ` Uwe Kleine-König
  2010-10-13 15:25         ` Ian Lartey
  0 siblings, 1 reply; 11+ messages in thread
From: Uwe Kleine-König @ 2010-10-01 12:49 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Oct 01, 2010 at 11:46:36AM +0100, Ian Lartey wrote:
> On Thu, 2010-09-30 at 20:23 +0200, Uwe Kleine-K?nig wrote:
> > On Thu, Sep 30, 2010 at 04:47:35PM +0100, Ian Lartey wrote:
> > > Signed-off-by: Ian Lartey <ian@opensource.wolfsonmicro.com>
> > > ---
> > >  arch/arm/configs/mx3_defconfig |    2 ++
> > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/arch/arm/configs/mx3_defconfig b/arch/arm/configs/mx3_defconfig
> > > index 161f907..b0b3409 100644
> > > --- a/arch/arm/configs/mx3_defconfig
> > > +++ b/arch/arm/configs/mx3_defconfig
> > > @@ -68,6 +68,8 @@ CONFIG_NET_ETHERNET=y
> > >  CONFIG_SMSC911X=y
> > >  CONFIG_DNET=y
> > >  CONFIG_FEC=y
> > > +CONFIG_CS89x0=y
> > > +CONFIG_CS89x0_NONISA_IRQ=y
> > >  # CONFIG_NETDEV_1000 is not set
> > >  # CONFIG_NETDEV_10000 is not set
> > >  # CONFIG_INPUT is not set
> > This change is bad for two reasons:
> > 
> >  - I just tested a kernel with CONFIG_CS89x0=y on a Phytec Phycore
> >    pcm043 using your updated defconfig and it barfs:
> > 
> > 	[.....]
> 
> Ouch, I'll see if I can correct that. (Needs to be done before I can add
> the config file change). Uwe I don't have another i.MX3 (apart from the
> i.MV31ADS) would you be able to test further changes for me please to
> help catch this kind of thing ?
I'm not sure if I have access to an mx31ads, but we have several i.MX
machines in our lab.  And I'm willing to give my comments to a general
cs89x0 cleanup.  (This drivers was one of the first things I saw when I
started kernel development.  Back then I gave up trying to understand
it. :-)  So maybe if you don't come around fixing it I might have some
energy to do it now with more knowledge about the kernel in general and
having seen *much* ugly code.

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

* [PATCH 2/2] Adding cs89x0, standard ethernet driver for iMX31 platform.
  2010-10-01 12:49       ` Uwe Kleine-König
@ 2010-10-13 15:25         ` Ian Lartey
  2010-10-13 16:11           ` Uwe Kleine-König
  0 siblings, 1 reply; 11+ messages in thread
From: Ian Lartey @ 2010-10-13 15:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 2010-10-01 at 14:49 +0200, Uwe Kleine-K?nig wrote:
> On Fri, Oct 01, 2010 at 11:46:36AM +0100, Ian Lartey wrote:
> > On Thu, 2010-09-30 at 20:23 +0200, Uwe Kleine-K?nig wrote:
> > > On Thu, Sep 30, 2010 at 04:47:35PM +0100, Ian Lartey wrote:
> > > > Signed-off-by: Ian Lartey <ian@opensource.wolfsonmicro.com>
> > > > ---
> > > >  arch/arm/configs/mx3_defconfig |    2 ++
> > > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/configs/mx3_defconfig b/arch/arm/configs/mx3_defconfig
> > > > index 161f907..b0b3409 100644
> > > > --- a/arch/arm/configs/mx3_defconfig
> > > > +++ b/arch/arm/configs/mx3_defconfig
> > > > @@ -68,6 +68,8 @@ CONFIG_NET_ETHERNET=y
> > > >  CONFIG_SMSC911X=y
> > > >  CONFIG_DNET=y
> > > >  CONFIG_FEC=y
> > > > +CONFIG_CS89x0=y
> > > > +CONFIG_CS89x0_NONISA_IRQ=y
> > > >  # CONFIG_NETDEV_1000 is not set
> > > >  # CONFIG_NETDEV_10000 is not set
> > > >  # CONFIG_INPUT is not set
> > > This change is bad for two reasons:
> > > 
> > >  - I just tested a kernel with CONFIG_CS89x0=y on a Phytec Phycore
> > >    pcm043 using your updated defconfig and it barfs:
> > > 
> > > 	[.....]
> > 
> > Ouch, I'll see if I can correct that. (Needs to be done before I can add
> > the config file change). Uwe I don't have another i.MX3 (apart from the
> > i.MV31ADS) would you be able to test further changes for me please to
> > help catch this kind of thing ?
> I'm not sure if I have access to an mx31ads, but we have several i.MX
> machines in our lab.  And I'm willing to give my comments to a general
> cs89x0 cleanup.  (This drivers was one of the first things I saw when I
> started kernel development.  Back then I gave up trying to understand
> it. :-)  So maybe if you don't come around fixing it I might have some
> energy to do it now with more knowledge about the kernel in general and
> having seen *much* ugly code.

I've had a look round but I cannot get a handle on fixing the issue.
I only have an i.mx31ads that failure doesn't occur, sorry I hope you
have more success than me.


For now I'll resubmit PATCH 1/2 with the changes to  board-mx31ads.h and
board-mx31ads.c but I'll not resubmit this patch to mx3_defconfig so
nobody will get it by default.

Does this seem OK to you ?

Cheers,
       Ian.

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

* [PATCH 2/2] Adding cs89x0, standard ethernet driver for iMX31 platform.
  2010-10-13 15:25         ` Ian Lartey
@ 2010-10-13 16:11           ` Uwe Kleine-König
  2010-10-13 16:32             ` Ian Lartey
  0 siblings, 1 reply; 11+ messages in thread
From: Uwe Kleine-König @ 2010-10-13 16:11 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Ian,

On Wed, Oct 13, 2010 at 04:25:13PM +0100, Ian Lartey wrote:
> On Fri, 2010-10-01 at 14:49 +0200, Uwe Kleine-K?nig wrote:
> > On Fri, Oct 01, 2010 at 11:46:36AM +0100, Ian Lartey wrote:
> > > On Thu, 2010-09-30 at 20:23 +0200, Uwe Kleine-K?nig wrote:
> > > > On Thu, Sep 30, 2010 at 04:47:35PM +0100, Ian Lartey wrote:
> > > > > Signed-off-by: Ian Lartey <ian@opensource.wolfsonmicro.com>
> > > > > ---
> > > > >  arch/arm/configs/mx3_defconfig |    2 ++
> > > > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > > > > 
> > > > > diff --git a/arch/arm/configs/mx3_defconfig b/arch/arm/configs/mx3_defconfig
> > > > > index 161f907..b0b3409 100644
> > > > > --- a/arch/arm/configs/mx3_defconfig
> > > > > +++ b/arch/arm/configs/mx3_defconfig
> > > > > @@ -68,6 +68,8 @@ CONFIG_NET_ETHERNET=y
> > > > >  CONFIG_SMSC911X=y
> > > > >  CONFIG_DNET=y
> > > > >  CONFIG_FEC=y
> > > > > +CONFIG_CS89x0=y
> > > > > +CONFIG_CS89x0_NONISA_IRQ=y
> > > > >  # CONFIG_NETDEV_1000 is not set
> > > > >  # CONFIG_NETDEV_10000 is not set
> > > > >  # CONFIG_INPUT is not set
> > > > This change is bad for two reasons:
> > > > 
> > > >  - I just tested a kernel with CONFIG_CS89x0=y on a Phytec Phycore
> > > >    pcm043 using your updated defconfig and it barfs:
> > > > 
> > > > 	[.....]
> > > 
> > > Ouch, I'll see if I can correct that. (Needs to be done before I can add
> > > the config file change). Uwe I don't have another i.MX3 (apart from the
> > > i.MV31ADS) would you be able to test further changes for me please to
> > > help catch this kind of thing ?
> > I'm not sure if I have access to an mx31ads, but we have several i.MX
> > machines in our lab.  And I'm willing to give my comments to a general
> > cs89x0 cleanup.  (This drivers was one of the first things I saw when I
> > started kernel development.  Back then I gave up trying to understand
> > it. :-)  So maybe if you don't come around fixing it I might have some
> > energy to do it now with more knowledge about the kernel in general and
> > having seen *much* ugly code.
> 
> I've had a look round but I cannot get a handle on fixing the issue.
no time or no clue?

> I only have an i.mx31ads that failure doesn't occur, sorry I hope you
> have more success than me.
I put in on my todo list, but I don't expect to get that done soon.

> For now I'll resubmit PATCH 1/2 with the changes to  board-mx31ads.h and
> board-mx31ads.c but I'll not resubmit this patch to mx3_defconfig so
> nobody will get it by default.
> 
> Does this seem OK to you ?
OK

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

* [PATCH 2/2] Adding cs89x0, standard ethernet driver for iMX31 platform.
  2010-10-13 16:11           ` Uwe Kleine-König
@ 2010-10-13 16:32             ` Ian Lartey
  0 siblings, 0 replies; 11+ messages in thread
From: Ian Lartey @ 2010-10-13 16:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 2010-10-13 at 18:11 +0200, Uwe Kleine-K?nig wrote:
> Hi Ian,
> 
> On Wed, Oct 13, 2010 at 04:25:13PM +0100, Ian Lartey wrote:
> > On Fri, 2010-10-01 at 14:49 +0200, Uwe Kleine-K?nig wrote:
> > > On Fri, Oct 01, 2010 at 11:46:36AM +0100, Ian Lartey wrote:
> > > > On Thu, 2010-09-30 at 20:23 +0200, Uwe Kleine-K?nig wrote:
> > > > > On Thu, Sep 30, 2010 at 04:47:35PM +0100, Ian Lartey wrote:
> > > > > > Signed-off-by: Ian Lartey <ian@opensource.wolfsonmicro.com>
> > > > > > ---
> > > > > >  arch/arm/configs/mx3_defconfig |    2 ++
> > > > > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > > > > > 
> > > > > > diff --git a/arch/arm/configs/mx3_defconfig b/arch/arm/configs/mx3_defconfig
> > > > > > index 161f907..b0b3409 100644
> > > > > > --- a/arch/arm/configs/mx3_defconfig
> > > > > > +++ b/arch/arm/configs/mx3_defconfig
> > > > > > @@ -68,6 +68,8 @@ CONFIG_NET_ETHERNET=y
> > > > > >  CONFIG_SMSC911X=y
> > > > > >  CONFIG_DNET=y
> > > > > >  CONFIG_FEC=y
> > > > > > +CONFIG_CS89x0=y
> > > > > > +CONFIG_CS89x0_NONISA_IRQ=y
> > > > > >  # CONFIG_NETDEV_1000 is not set
> > > > > >  # CONFIG_NETDEV_10000 is not set
> > > > > >  # CONFIG_INPUT is not set
> > > > > This change is bad for two reasons:
> > > > > 
> > > > >  - I just tested a kernel with CONFIG_CS89x0=y on a Phytec Phycore
> > > > >    pcm043 using your updated defconfig and it barfs:
> > > > > 
> > > > > 	[.....]
> > > > 
> > > > Ouch, I'll see if I can correct that. (Needs to be done before I can add
> > > > the config file change). Uwe I don't have another i.MX3 (apart from the
> > > > i.MV31ADS) would you be able to test further changes for me please to
> > > > help catch this kind of thing ?
> > > I'm not sure if I have access to an mx31ads, but we have several i.MX
> > > machines in our lab.  And I'm willing to give my comments to a general
> > > cs89x0 cleanup.  (This drivers was one of the first things I saw when I
> > > started kernel development.  Back then I gave up trying to understand
> > > it. :-)  So maybe if you don't come around fixing it I might have some
> > > energy to do it now with more knowledge about the kernel in general and
> > > having seen *much* ugly code.
> > 
> > I've had a look round but I cannot get a handle on fixing the issue.
> no time or no clue?

A bit of both, but I think no-clue takes priority in this case!
> 
> > I only have an i.mx31ads that failure doesn't occur, sorry I hope you
> > have more success than me.
> I put in on my todo list, but I don't expect to get that done soon.

OK.

> 
> > For now I'll resubmit PATCH 1/2 with the changes to  board-mx31ads.h and
> > board-mx31ads.c but I'll not resubmit this patch to mx3_defconfig so
> > nobody will get it by default.
> > 
> > Does this seem OK to you ?
> OK

Ian.

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

end of thread, other threads:[~2010-10-13 16:32 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-30 15:47 [PATCH 1/2] Fixing ethernet driver compilation error for i.MX31 ADS board Ian Lartey
2010-09-30 15:47 ` [PATCH 2/2] Adding cs89x0, standard ethernet driver for iMX31 platform Ian Lartey
2010-09-30 18:23   ` Uwe Kleine-König
2010-10-01 10:46     ` Ian Lartey
2010-10-01 12:49       ` Uwe Kleine-König
2010-10-13 15:25         ` Ian Lartey
2010-10-13 16:11           ` Uwe Kleine-König
2010-10-13 16:32             ` Ian Lartey
2010-09-30 18:14 ` [PATCH 1/2] Fixing ethernet driver compilation error for i.MX31 ADS board Uwe Kleine-König
2010-10-01 10:33   ` Ian Lartey
2010-10-01 12:42     ` Uwe Kleine-König

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).