Hello.

Mark A. Greer wrote:
On Fri, Sep 18, 2009 at 09:05:56PM +0400, Sergei Shtylyov wrote:

diff --git a/arch/arm/mach-davinci/Kconfig b/arch/arm/mach-davinci/Kconfig

+config DA830_UI_LCD
+       bool "LCD"
+       help
+         Say Y here to use the LCD as a framebuffer or simple character
+         display.
+
+endchoice
   Certainly my addition thu the help text was somewhat reworded...

Yes I reworded it.  What is your issue?

  No issues, just noticed the rewording...

diff --git a/arch/arm/mach-davinci/board-da830-evm.c 
b/arch/arm/mach-davinci/board-da830-evm.c

+static int da830_evm_ui_expander_setup(struct i2c_client *client, int gpio,
+               unsigned ngpio, void *context)
+{
+       gpio_request(gpio + 6, "MUX_MODE");
+#ifdef CONFIG_DA830_UI_LCD
+       gpio_direction_output(gpio + 6, 0);
+#else /* Must be NAND or NOR */
+       gpio_direction_output(gpio + 6, 1);
   One is the default value after reset, no need to reprogram it.

That's true but there's no harm in programming the proper value whether
or not its a detault.  In addition, you cannot always assume the hardware
was reset (e.g., kexec).

OK. Our internal code additionally had the expander set to all ones which this code doesn't do, so setting the value explicitly could indeed be useful...

@@ -175,6 +206,17 @@ static __init void da830_evm_init(void)
        if (ret)
                pr_warning("da830_evm_init: mmc/sd registration failed: %d\n",
                                ret);
+
+#ifdef CONFIG_DA830_UI_LCD
+       ret = da8xx_pinmux_setup(da830_lcdcntl_pins);
+       if (ret)
+               pr_warning("da830_evm_init: lcdcntl mux setup failed: %d\n",
+                               ret);
+
+       ret = da8xx_register_lcdc(&sharp_lcd035q3dg01_pdata);
It's again not clear why board specific LCD platfrom data ended up in devices-da8xx.c

Firstly, that's where the definition for sharp_lk043t1dg01_pdata
already was so I simply added to what already existed.  Secondly,
the lcd definitions are [more-or-less] board agnostic, not board specific.

That is what I'm finding most strange. How the LCD controller choice (with it chosen specifically for implementing the board) can be board agnostic? AFAIU, this is as board specific as e.g. a NAND chip, yet you put thr NAND platfrom data into the board file despite the possibility of the same NAND chip being used on several different boards...

So, devices-da8xx.c seems like a good place; although, if more come along,
they could be put in their own file.  Putting them in the board files
only eliminates the possibility of reusing the definitions.

The data definition seems so small that it doesn't seem worth reusing really...

Mark
--
WBR, Sergei



_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to