2009-12-29 08:00:24

by Luke Dashjr

[permalink] [raw]
Subject: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

Add platform data and initialization for USB on Nokia N800 and N810 devices
via the TUSB6010 chipset.

Tested on Nokia N810 in Linux-OMAP tree. Mainline is not bootable yet.

Signed-off-by: Luke Dashjr <[email protected]>
---
arch/arm/mach-omap2/Kconfig | 5 +
arch/arm/mach-omap2/Makefile | 1 +
arch/arm/mach-omap2/board-n8x0-usb.c | 173 ++++++++++++++++++++++++++++++++++
arch/arm/mach-omap2/board-n8x0.c | 8 ++
4 files changed, 187 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/board-n8x0-usb.c

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 10eafa7..e3c913c 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -117,6 +117,11 @@ config MACH_NOKIA_N8X0
select MACH_NOKIA_N810
select MACH_NOKIA_N810_WIMAX

+config MACH_NOKIA_N8X0_USB
+ bool
+ depends on MACH_NOKIA_N8X0 && MACH_OMAP2_TUSB6010
+ default y
+
config MACH_NOKIA_RX51
bool "Nokia RX-51 board"
depends on ARCH_OMAP3 && ARCH_OMAP34XX
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index b32678b..e890b2b 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -86,6 +86,7 @@ obj-$(CONFIG_MACH_OMAP3_PANDORA) += board-omap3pandora.o \
obj-$(CONFIG_MACH_OMAP_3430SDP) += board-3430sdp.o \
mmc-twl4030.o
obj-$(CONFIG_MACH_NOKIA_N8X0) += board-n8x0.o
+obj-$(CONFIG_MACH_NOKIA_N8X0_USB) += board-n8x0-usb.o
obj-$(CONFIG_MACH_NOKIA_RX51) += board-rx51.o \
board-rx51-sdram.o \
board-rx51-peripherals.o \
diff --git a/arch/arm/mach-omap2/board-n8x0-usb.c b/arch/arm/mach-omap2/board-n8x0-usb.c
new file mode 100644
index 0000000..2254ebd
--- /dev/null
+++ b/arch/arm/mach-omap2/board-n8x0-usb.c
@@ -0,0 +1,173 @@
+/*
+ * linux/arch/arm/mach-omap2/board-n8x0-usb.c
+ *
+ * Copyright (C) 2006 Nokia Corporation
+ * Author: Juha Yrjola
+ *
+ * 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.
+ */
+
+#include <linux/types.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/usb/musb.h>
+
+#include <plat/gpmc.h>
+
+#define TUSB_ASYNC_CS 1
+#define TUSB_SYNC_CS 4
+#define GPIO_TUSB_INT 58
+#define GPIO_TUSB_ENABLE 0
+
+static int tusb_set_power(int state);
+static int tusb_set_clock(struct clk *osc_ck, int state);
+
+#if defined(CONFIG_USB_MUSB_OTG)
+# define BOARD_MODE MUSB_OTG
+#elif defined(CONFIG_USB_MUSB_PERIPHERAL)
+# define BOARD_MODE MUSB_PERIPHERAL
+#else /* defined(CONFIG_USB_MUSB_HOST) */
+# define BOARD_MODE MUSB_HOST
+#endif
+
+static struct musb_hdrc_eps_bits musb_eps[] = {
+ { "ep1_tx", 5, },
+ { "ep1_rx", 5, },
+ { "ep2_tx", 5, },
+ { "ep2_rx", 5, },
+ { "ep3_tx", 3, },
+ { "ep3_rx", 3, },
+ { "ep4_tx", 3, },
+ { "ep4_rx", 3, },
+ { "ep5_tx", 2, },
+ { "ep5_rx", 2, },
+ { "ep6_tx", 2, },
+ { "ep6_rx", 2, },
+ { "ep7_tx", 2, },
+ { "ep7_rx", 2, },
+ { "ep8_tx", 2, },
+ { "ep8_rx", 2, },
+ { "ep9_tx", 2, },
+ { "ep9_rx", 2, },
+ { "ep10_tx", 2, },
+ { "ep10_rx", 2, },
+ { "ep11_tx", 2, },
+ { "ep11_rx", 2, },
+ { "ep12_tx", 2, },
+ { "ep12_rx", 2, },
+ { "ep13_tx", 2, },
+ { "ep13_rx", 2, },
+ { "ep14_tx", 2, },
+ { "ep14_rx", 2, },
+ { "ep15_tx", 2, },
+ { "ep15_rx", 2, },
+};
+
+static struct musb_hdrc_config musb_config = {
+ .multipoint = 1,
+ .dyn_fifo = 1,
+ .soft_con = 1,
+ .dma = 1,
+ .num_eps = 16,
+ .dma_channels = 7,
+ .ram_bits = 12,
+ .eps_bits = musb_eps,
+};
+
+static struct musb_hdrc_platform_data tusb_data = {
+ .mode = BOARD_MODE,
+ .set_power = tusb_set_power,
+ .set_clock = tusb_set_clock,
+ .min_power = 25, /* x2 = 50 mA drawn from VBUS as peripheral */
+ .power = 100, /* Max 100 mA VBUS for host mode */
+ .clock = "osc_ck",
+ .config = &musb_config,
+};
+
+/*
+ * Enable or disable power to TUSB6010. When enabling, turn on 3.3 V and
+ * 1.5 V voltage regulators of PM companion chip. Companion chip will then
+ * provide then PGOOD signal to TUSB6010 which will release it from reset.
+ */
+static int tusb_set_power(int state)
+{
+ int i, retval = 0;
+
+ if (state) {
+ gpio_set_value(GPIO_TUSB_ENABLE, 1);
+ msleep(1);
+
+ /* Wait until TUSB6010 pulls INT pin down */
+ i = 100;
+ while (i && gpio_get_value(GPIO_TUSB_INT)) {
+ msleep(1);
+ i--;
+ }
+
+ if (!i) {
+ printk(KERN_ERR "tusb: powerup failed\n");
+ retval = -ENODEV;
+ }
+ } else {
+ gpio_set_value(GPIO_TUSB_ENABLE, 0);
+ msleep(10);
+ }
+
+ return retval;
+}
+
+static int osc_ck_on;
+
+static int tusb_set_clock(struct clk *osc_ck, int state)
+{
+ if (state) {
+ if (osc_ck_on > 0)
+ return -ENODEV;
+
+ clk_enable(osc_ck);
+ osc_ck_on = 1;
+ } else {
+ if (osc_ck_on == 0)
+ return -ENODEV;
+
+ clk_disable(osc_ck);
+ osc_ck_on = 0;
+ }
+
+ return 0;
+}
+
+void __init n8x0_usb_init(void)
+{
+ int ret = 0;
+ static char announce[] __initdata = KERN_INFO "TUSB 6010\n";
+
+ /* PM companion chip power control pin */
+ ret = gpio_request(GPIO_TUSB_ENABLE, "TUSB6010 enable");
+ if (ret != 0) {
+ printk(KERN_ERR "Could not get TUSB power GPIO%i\n",
+ GPIO_TUSB_ENABLE);
+ return;
+ }
+ gpio_direction_output(GPIO_TUSB_ENABLE, 0);
+
+ tusb_set_power(0);
+
+ ret = tusb6010_setup_interface(&tusb_data, TUSB6010_REFCLK_19, 2,
+ TUSB_ASYNC_CS, TUSB_SYNC_CS,
+ GPIO_TUSB_INT, 0x3f);
+ if (ret != 0)
+ goto err;
+
+ printk(announce);
+
+ return;
+
+err:
+ gpio_free(GPIO_TUSB_ENABLE);
+}
diff --git a/arch/arm/mach-omap2/board-n8x0.c b/arch/arm/mach-omap2/board-n8x0.c
index 764ab1e..c9395e1 100644
--- a/arch/arm/mach-omap2/board-n8x0.c
+++ b/arch/arm/mach-omap2/board-n8x0.c
@@ -109,8 +109,16 @@ static void __init n8x0_init_irq(void)
omap_gpio_init();
}

+#ifdef CONFIG_MACH_NOKIA_N8X0_LCD
+extern void n8x0_usb_init(void);
+#else
+static inline void n8x0_usb_init (void) {}
+#endif
+
static void __init n8x0_init_machine(void)
{
+ n8x0_usb_init();
+
/* FIXME: add n810 spi devices */
spi_register_board_info(n800_spi_board_info,
ARRAY_SIZE(n800_spi_board_info));
--
1.6.4.4


2009-12-29 08:29:28

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

"Luke-Jr" <[email protected]> writes:

> Add platform data and initialization for USB on Nokia N800 and N810 devices
> via the TUSB6010 chipset.

Excellent, thank you for doing this.

> Tested on Nokia N810 in Linux-OMAP tree. Mainline is not bootable yet.

What's the problem in mainline? I haven't tested n8x0 for months :(

> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -117,6 +117,11 @@ config MACH_NOKIA_N8X0
> select MACH_NOKIA_N810
> select MACH_NOKIA_N810_WIMAX
>
> +config MACH_NOKIA_N8X0_USB
> + bool
> + depends on MACH_NOKIA_N8X0 && MACH_OMAP2_TUSB6010
> + default y
> +

Is is really needed to add new kconfig variable? Can't we just use
MACH_OMAP2_TUSB6010 in code?

> --- /dev/null
> +++ b/arch/arm/mach-omap2/board-n8x0-usb.c

I think a separate file is not needed, it's not that much code. I
recommend to just adding it directly to board-n8x0.c. It's simple and
easier for everyone.

> --- a/arch/arm/mach-omap2/board-n8x0.c
> +++ b/arch/arm/mach-omap2/board-n8x0.c
> @@ -109,8 +109,16 @@ static void __init n8x0_init_irq(void)
> omap_gpio_init();
> }
>
> +#ifdef CONFIG_MACH_NOKIA_N8X0_LCD
> +extern void n8x0_usb_init(void);
> +#else
> +static inline void n8x0_usb_init (void) {}
> +#endif

CONFIG_MACH_NOKIA_N8X0_LCD does not look right here.

--
Kalle Valo

2009-12-29 08:47:10

by Luke Dashjr

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

On Tuesday 29 December 2009 02:29:22 am Kalle Valo wrote:
> "Luke-Jr" <[email protected]> writes:
> > Tested on Nokia N810 in Linux-OMAP tree. Mainline is not bootable yet.
>
> What's the problem in mainline? I haven't tested n8x0 for months :(

Lack of platform support, as far as I know.
Until we have cbus, there's no way to get the LCD powered...

> > --- a/arch/arm/mach-omap2/Kconfig
> > +++ b/arch/arm/mach-omap2/Kconfig
> > @@ -117,6 +117,11 @@ config MACH_NOKIA_N8X0
> > select MACH_NOKIA_N810
> > select MACH_NOKIA_N810_WIMAX
> >
> > +config MACH_NOKIA_N8X0_USB
> > + bool
> > + depends on MACH_NOKIA_N8X0 && MACH_OMAP2_TUSB6010
> > + default y
> > +
>
> Is is really needed to add new kconfig variable? Can't we just use
> MACH_OMAP2_TUSB6010 in code?
>
> I think a separate file is not needed, it's not that much code. I
> recommend to just adding it directly to board-n8x0.c. It's simple and
> easier for everyone.

Seemed simpler to manage in a separate file, though not as necessary as the
LCD stuff (which I also have a patch against mainline for, but it needs
tahvo...)

> > --- a/arch/arm/mach-omap2/board-n8x0.c
> > +++ b/arch/arm/mach-omap2/board-n8x0.c
> > @@ -109,8 +109,16 @@ static void __init n8x0_init_irq(void)
> > omap_gpio_init();
> > }
> >
> > +#ifdef CONFIG_MACH_NOKIA_N8X0_LCD
> > +extern void n8x0_usb_init(void);
> > +#else
> > +static inline void n8x0_usb_init (void) {}
> > +#endif
>
> CONFIG_MACH_NOKIA_N8X0_LCD does not look right here.

Heh, looks like my isolate-and-merge-to-mainline got messed up there.
Of course it should be CONFIG_MACH_NOKIA_N8X0_USB :)

Also, scripts/checkpatch.pl gave me a warning about the extern in a .c file,
but I couldn't figure out where else it could reasonably go. Any ideas? :/

Luke

2009-12-29 08:55:51

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

"Luke-Jr" <[email protected]> writes:

> On Tuesday 29 December 2009 02:29:22 am Kalle Valo wrote:
>> "Luke-Jr" <[email protected]> writes:
>> > Tested on Nokia N810 in Linux-OMAP tree. Mainline is not bootable yet.
>>
>> What's the problem in mainline? I haven't tested n8x0 for months :(
>
> Lack of platform support, as far as I know. Until we have cbus,
> there's no way to get the LCD powered...

Oh that. I have only used serial console with mainline. Luckily Felipe
did some cbus patches, I haven't checked them yet though.

>> I think a separate file is not needed, it's not that much code. I
>> recommend to just adding it directly to board-n8x0.c. It's simple and
>> easier for everyone.
>
> Seemed simpler to manage in a separate file

Having lots of small files is difficult IMHO.

> though not as necessary as the LCD stuff (which I also have a patch
> against mainline for, but it needs tahvo...)

Nice.

>> CONFIG_MACH_NOKIA_N8X0_LCD does not look right here.
>
> Heh, looks like my isolate-and-merge-to-mainline got messed up there.
> Of course it should be CONFIG_MACH_NOKIA_N8X0_USB :)

Yeah, maybe eating too much ham caused it ;)

> Also, scripts/checkpatch.pl gave me a warning about the extern in a .c file,
> but I couldn't figure out where else it could reasonably go. Any ideas? :/

Yes, just put everything into one file :) No need for externs then.

--
Kalle Valo

2009-12-29 12:57:59

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

Hi,

On Tue, Dec 29, 2009 at 09:55:46AM +0100, ext Kalle Valo wrote:
>Oh that. I have only used serial console with mainline. Luckily Felipe
>did some cbus patches, I haven't checked them yet though.

nothing for mainline yet though, cbus needs lots of work.

anyone with free time, please help :-p

--
balbi

2009-12-29 19:24:14

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

Hi,

* Luke-Jr <[email protected]> [091228 23:51]:
> Add platform data and initialization for USB on Nokia N800 and N810 devices
> via the TUSB6010 chipset.
>
> Tested on Nokia N810 in Linux-OMAP tree. Mainline is not bootable yet.
>
> Signed-off-by: Luke Dashjr <[email protected]>
> ---
> arch/arm/mach-omap2/Kconfig | 5 +
> arch/arm/mach-omap2/Makefile | 1 +
> arch/arm/mach-omap2/board-n8x0-usb.c | 173 ++++++++++++++++++++++++++++++++++
> arch/arm/mach-omap2/board-n8x0.c | 8 ++
> 4 files changed, 187 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/mach-omap2/board-n8x0-usb.c
>

<snip>

> diff --git a/arch/arm/mach-omap2/board-n8x0-usb.c b/arch/arm/mach-omap2/board-n8x0-usb.c
> new file mode 100644
> index 0000000..2254ebd
> --- /dev/null
> +++ b/arch/arm/mach-omap2/board-n8x0-usb.c

<snip>

> +static int osc_ck_on;
> +
> +static int tusb_set_clock(struct clk *osc_ck, int state)
> +{
> + if (state) {
> + if (osc_ck_on > 0)
> + return -ENODEV;
> +
> + clk_enable(osc_ck);
> + osc_ck_on = 1;
> + } else {
> + if (osc_ck_on == 0)
> + return -ENODEV;
> +
> + clk_disable(osc_ck);
> + osc_ck_on = 0;
> + }
> +
> + return 0;
> +}

Great. The comment from Russell earlier was that we should be now using
clkdev to do things like this and get rid of the _set_clock functions.
In musb_core.c are already using the clk directly if set_clock is NULL.

Can you try the patch below? If it works, just merge it into your
patch.

Regards,

Tony


Attachments:
(No filename) (1.48 kB)
tusb-clkdev.patch (2.49 kB)
Download all attachments

2009-12-29 20:02:20

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

Hi,

On Tue, 2009-12-29 at 11:24 -0800, Tony Lindgren wrote:
> Great. The comment from Russell earlier was that we should be now using
> clkdev to do things like this and get rid of the _set_clock functions.
> In musb_core.c are already using the clk directly if set_clock is NULL.
>
> Can you try the patch below? If it works, just merge it into your
> patch.

Yes, let's do like Tony suggested. I'm removing the old set_clock stuff
completely, patches are available at my git tree, here are the links:

http://gitorious.org/usb/usb/commit/ca415cf06a4843e93041fe73f7f1867e066b48a3
http://gitorious.org/usb/usb/commit/013f8457a1ba0dd3f36811f647b0fe904128292e
http://gitorious.org/usb/usb/commit/e2ecae4c5b01415dbbf714c7217178c31a9a5dd1
http://gitorious.org/usb/usb/commit/874afb373ea75ff0a3a360802a53357affa27df0
http://gitorious.org/usb/usb/commit/6d47fc9858c17faea8a454e147839c878737a0f1
http://gitorious.org/usb/usb/commit/62b8d94df7739cb60bae0369b8265e588f518841
http://gitorious.org/usb/usb/commit/e06629acd44936fe87d34d188c8dfb54abd7915c

Tony, if you want I can send you a pull request will all the patches I
have prepared on top of your tree. There's only one dependence which I
can ship in the same pull request.

Sames goes to Kevin regarding davinci tree. I just have to subscribe in
order to be able to send the patches there.

--
balbi

2009-12-29 19:42:30

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

* Felipe Balbi <[email protected]> [091229 11:34]:
> Hi,
>
> On Tue, 2009-12-29 at 11:24 -0800, Tony Lindgren wrote:
> > Great. The comment from Russell earlier was that we should be now using
> > clkdev to do things like this and get rid of the _set_clock functions.
> > In musb_core.c are already using the clk directly if set_clock is NULL.
> >
> > Can you try the patch below? If it works, just merge it into your
> > patch.
>
> Yes, let's do like Tony suggested. I'm removing the old set_clock stuff
> completely, patches are available at my git tree, here are the links:
>
> http://gitorious.org/usb/usb/commit/ca415cf06a4843e93041fe73f7f1867e066b48a3
> http://gitorious.org/usb/usb/commit/013f8457a1ba0dd3f36811f647b0fe904128292e
> http://gitorious.org/usb/usb/commit/e2ecae4c5b01415dbbf714c7217178c31a9a5dd1
> http://gitorious.org/usb/usb/commit/874afb373ea75ff0a3a360802a53357affa27df0
> http://gitorious.org/usb/usb/commit/6d47fc9858c17faea8a454e147839c878737a0f1
> http://gitorious.org/usb/usb/commit/62b8d94df7739cb60bae0369b8265e588f518841
> http://gitorious.org/usb/usb/commit/e06629acd44936fe87d34d188c8dfb54abd7915c
>
> Tony, if you want I can send you a pull request will all the patches I
> have prepared on top of your tree. There's only one dependence which I
> can ship in the same pull request.

That would be great!

> Sames goes to Kevin regarding davinci tree. I just have to subscribe in
> order to be able to send the patches there.

FYI, I think Kevin is on vacation right now.

Regards,

Tony

2009-12-29 19:49:47

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

Hi,

On Tue, 2009-12-29 at 11:42 -0800, Tony Lindgren wrote:
> That would be great!

Ok good. So I'll mark all my pending patches as Superseeded or something
better on patchwork then by tomorrow I'll resend all of them, plus the
pull request.

> > Sames goes to Kevin regarding davinci tree. I just have to subscribe in
> > order to be able to send the patches there.
>
> FYI, I think Kevin is on vacation right now.

good to know. The patch going to Greg right now only marks the fields as
__deprecated so it shouldn't be a big problem to wait a bit.


--
balbi

2009-12-29 19:54:05

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

* Felipe Balbi <[email protected]> [091229 11:48]:
> Hi,
>
> On Tue, 2009-12-29 at 11:42 -0800, Tony Lindgren wrote:
> > That would be great!
>
> Ok good. So I'll mark all my pending patches as Superseeded or something
> better on patchwork then by tomorrow I'll resend all of them, plus the
> pull request.

Sounds good. Please keep two separate queues: Fixes for the -rc cycle,
and new code for next merge window.

> > > Sames goes to Kevin regarding davinci tree. I just have to subscribe in
> > > order to be able to send the patches there.
> >
> > FYI, I think Kevin is on vacation right now.
>
> good to know. The patch going to Greg right now only marks the fields as
> __deprecated so it shouldn't be a big problem to wait a bit.

OK

Tony

2009-12-29 19:57:49

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

Hi,

On Tue, 2009-12-29 at 11:53 -0800, Tony Lindgren wrote:
> Sounds good. Please keep two separate queues: Fixes for the -rc cycle,
> and new code for next merge window.

will do.

--
balbi

2009-12-29 20:29:00

by Luke Dashjr

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

Quick followup... I haven't had the chance to make those clock changes or move
it into the main board file yet, but just discovered my recent boot problems
were "fixed" by disabling USB support, so apparently there's something else
broken with my patch. :(

Unfortunately, without a trackback, I'm totally lost:

Unable to handle kernel NULL pointer dereference at virtual address 00000031
pgd = c0004000
[00000031] *pgd=000000000
Internal error: Oops: 17 [#1] PREEMPT

2009-12-29 20:35:56

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

On Tue, 2009-12-29 at 14:28 -0600, Luke-Jr wrote:
> Quick followup... I haven't had the chance to make those clock changes or move
> it into the main board file yet, but just discovered my recent boot problems
> were "fixed" by disabling USB support, so apparently there's something else
> broken with my patch. :(
>
> Unfortunately, without a trackback, I'm totally lost:
>
> Unable to handle kernel NULL pointer dereference at virtual address 00000031
> pgd = c0004000
> [00000031] *pgd=000000000
> Internal error: Oops: 17 [#1] PREEMPT

can you post the entire trace ?

I need the function call trace that BUG() prints

--
balbi

2009-12-29 21:06:09

by Luke Dashjr

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

On Tuesday 29 December 2009 02:55:46 am Kalle Valo wrote:
> Having lots of small files is difficult IMHO.

The last-working N8x0 support (Linux-OMAP 2.6.30-rc8) required 1706 lines of
platform data and initialization, split up into 6 files:
590 N800 + i2c + FM + keyboard + light sensor + LED + LCD + temp + SPI + WiFi
+ touchscreen + OneNAND
47 N810
175 USB
362 MMC
156 DSP
376 camera

The largest OMAP2 single-board file is merely 689 lines (3430sdp).
N900/RX-51, totalling 863 lines itself, is split over 2 files.

I think 4 or 5 files with ~350 lines is more manageable than a single 1700-
line board file, especially when considering that parts depend on multiple
configuration options (LCD, which turns out to be a 128-line file, depends on
3 config options: N8x0, Blizzard, and MIPID).

Thoughts?

Luke

2009-12-30 03:11:05

by Luke Dashjr

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

On Tuesday 29 December 2009 03:05:46 pm Luke-Jr wrote:
> The last-working N8x0 support (Linux-OMAP 2.6.30-rc8) required 1706 lines
> of platform data and initialization, split up into 6 files:

Upon trying to debug my non-working keyboard, I learned that the status of
mainlining is actually worse off than I thought, and my "last 2.6.30-rc8
working" revision was actually already missing platform data.

It seems logical from looking at the code needed to divide the board into at
least 5 files (line number estimates based on old code):
460 board-n8x0-common.c
175 board-n800.c (FM radio, touchscreen)
260 board-n810.c (keyboard, light sensor, LEDs, touchscreen)
600 board-n8x0-multimedia.c (camera, audio, DSP)
725 board-n8x0-storage.c (flash, MMC)
120 board-n8x0-lcd.c

On that topic, is there a reason the newer code is under a unified "N8x0"
config option instead of the split N800/N810 that was used for the old code,
or is that simply a symptom of the currently-supported features all being
common?

Luke

2009-12-30 07:17:16

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

"Luke-Jr" <[email protected]> writes:

> On that topic, is there a reason the newer code is under a unified "N8x0"
> config option instead of the split N800/N810 that was used for the old code,
> or is that simply a symptom of the currently-supported features all being
> common?

IIRC, I just used one common n8x0 file because the n810 board file was
very small and I didn't see any benefit from having it separately.

--
Kalle Valo

2009-12-30 09:03:06

by Luke Dashjr

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

On Wednesday 30 December 2009 01:17:11 am Kalle Valo wrote:
> "Luke-Jr" <[email protected]> writes:
> > On that topic, is there a reason the newer code is under a unified "N8x0"
> > config option instead of the split N800/N810 that was used for the old
> > code, or is that simply a symptom of the currently-supported features all
> > being common?
>
> IIRC, I just used one common n8x0 file because the n810 board file was
> very small and I didn't see any benefit from having it separately.

Yeah, that's because someone put all the N810 code in the N800 file ;)

Will probably at least split out the differences, if there's no other reason,
then. Since I'm doing this stuff only for my own personal interest, I'll
probably not bother to do much with N800 support unless someone feels like
sending me one when I finish N810 (definitely not touching N800 before then).

Luke

2009-12-30 12:04:44

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] OMAP2: add USB platform data and initialization for Nokia N800 and N810

Hi,

On Wed, 2009-12-30 at 03:02 -0600, Luke-Jr wrote:
> On Wednesday 30 December 2009 01:17:11 am Kalle Valo wrote:
> > "Luke-Jr" <[email protected]> writes:
> > > On that topic, is there a reason the newer code is under a unified "N8x0"
> > > config option instead of the split N800/N810 that was used for the old
> > > code, or is that simply a symptom of the currently-supported features all
> > > being common?
> >
> > IIRC, I just used one common n8x0 file because the n810 board file was
> > very small and I didn't see any benefit from having it separately.
>
> Yeah, that's because someone put all the N810 code in the N800 file ;)
>
> Will probably at least split out the differences, if there's no other reason,
> then. Since I'm doing this stuff only for my own personal interest, I'll
> probably not bother to do much with N800 support unless someone feels like
> sending me one when I finish N810 (definitely not touching N800 before then).

I think it's not worth the effort. Those boards share a lot of code.
You'll end up with several small board files

--
balbi