Re: [PATCH] Samsung LTV350QV LCD driver

[David Brownell <david-b@pacbell.net>]

On Tuesday 29 May 2007, Richard Purdie wrote:
> On Tue, 2007-05-29 at 21:52 +0200, Haavard Skinnemoen wrote:
> > This patch adds support for powering on and off the Samsung LTV350QV
> > LCD panel via SPI. The driver responds to framebuffer power
> > management, it powers off the panel on reboot/halt/poweroff, and it
> > can also be controlled through sysfs. The panel is powered up when the
> > module is loaded and off when the module is unloaded.
> > 
> > Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
> 
> The LCD class device bits look fine to me. The only change I'd request
> is not to change the order of things in drivers/video/backlight/Kconfig.
> If you do want to reword one of those entries (which is what I think you
> wanted to do), send it as a separate patch please.

One significant issue was screwed up sequencing causing
tools like 'xconfig' to display things wrong.  Appended
is a patch fixing just that goof.


> David, any objections to the SPI bits?

They seemed fine to me, other than the write_reg() macro
having an embedded 'goto' ... explicitly against the
Documentation/CodingStyle guidelines.  ("Chapter 12",
point 1 = "macros that affect code flow".)

I like seeing more of the SPI drivers go upstream.  ;)

- Dave



> 
> Richard
> 


======	CUT HERE
Switch the order of LCD_CLASS_DEVICE and BACKLIGHT_CLASS_DEVICE,
so that it's possible to insert LCD devices without borking the
dependency displays of xconfig and other config tools.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
 drivers/video/backlight/Kconfig |   26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

--- g26.orig/drivers/video/backlight/Kconfig	2007-05-29 14:31:41.000000000 -0700
+++ g26/drivers/video/backlight/Kconfig	2007-05-29 14:33:29.000000000 -0700
@@ -8,26 +8,32 @@ menuconfig BACKLIGHT_LCD_SUPPORT
 	  Enable this to be able to choose the drivers for controlling the
 	  backlight and the LCD panel on some platforms, for example on PDAs.
 
-config BACKLIGHT_CLASS_DEVICE
-        tristate "Lowlevel Backlight controls"
+#
+# LCD
+#
+config LCD_CLASS_DEVICE
+        tristate "Lowlevel LCD controls"
 	depends on BACKLIGHT_LCD_SUPPORT
 	default m
 	help
-	  This framework adds support for low-level control of the LCD
-          backlight. This includes support for brightness and power.
+	  This framework adds support for low-level control of LCD.
+	  Some framebuffer devices connect to platform-specific LCD modules
+	  in order to have a platform-specific way to control the flat panel
+	  (contrast and applying power to the LCD (not to the backlight!)).
 
 	  To have support for your specific LCD panel you will have to
 	  select the proper drivers which depend on this option.
 
-config LCD_CLASS_DEVICE
-        tristate "Lowlevel LCD controls"
+#
+# Backlight
+#
+config BACKLIGHT_CLASS_DEVICE
+        tristate "Lowlevel Backlight controls"
 	depends on BACKLIGHT_LCD_SUPPORT
 	default m
 	help
-	  This framework adds support for low-level control of LCD.
-	  Some framebuffer devices connect to platform-specific LCD modules
-	  in order to have a platform-specific way to control the flat panel
-	  (contrast and applying power to the LCD (not to the backlight!)).
+	  This framework adds support for low-level control of the LCD
+          backlight. This includes support for brightness and power.
 
 	  To have support for your specific LCD panel you will have to
 	  select the proper drivers which depend on this option.