Re: [PATCH 1/8] video: atmel_lcdfb: fix platform data struct

On 19:44 Wed 29 May     , Richard Genoud wrote:

2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [off-list ref]:

quoted

On 16:36 Wed 29 May     , Richard Genoud wrote:

quoted

2013/4/11 Jean-Christophe PLAGNIOL-VILLARD [off-list ref]:

quoted

Today we mix pdata and drivers data in the struct atmel_lcdfb_info
Fix it and introduce a new struct atmel_lcdfb_pdata for platform data only

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: Nicolas Ferre <redacted>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Hans-Christian Egtvedt <redacted>
---
 arch/arm/mach-at91/at91sam9261_devices.c    |    6 +-
 arch/arm/mach-at91/at91sam9263_devices.c    |    6 +-
 arch/arm/mach-at91/at91sam9g45_devices.c    |    6 +-
 arch/arm/mach-at91/at91sam9rl_devices.c     |    6 +-
 arch/arm/mach-at91/board-sam9261ek.c        |    6 +-
 arch/arm/mach-at91/board-sam9263ek.c        |    4 +-
 arch/arm/mach-at91/board-sam9m10g45ek.c     |    4 +-
 arch/arm/mach-at91/board-sam9rlek.c         |    4 +-
 arch/arm/mach-at91/board.h                  |    4 +-
 arch/avr32/boards/atngw100/evklcd10x.c      |    6 +-
 arch/avr32/boards/atngw100/mrmt.c           |    4 +-
 arch/avr32/boards/atstk1000/atstk1000.h     |    2 +-
 arch/avr32/boards/atstk1000/setup.c         |    2 +-
 arch/avr32/boards/favr-32/setup.c           |    2 +-
 arch/avr32/boards/hammerhead/setup.c        |    2 +-
 arch/avr32/boards/merisc/display.c          |    2 +-
 arch/avr32/boards/mimc200/setup.c           |    4 +-
 arch/avr32/mach-at32ap/at32ap700x.c         |    8 +--
 arch/avr32/mach-at32ap/include/mach/board.h |    4 +-
 drivers/video/atmel_lcdfb.c                 |  104 +++++++++++++++++----------
 include/video/atmel_lcdc.h                  |   24 +------
 21 files changed, 109 insertions(+), 101 deletions(-)

[snip]

quoted

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index c1a2914..98733cd4 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c

@@ -20,12 +20,45 @@
 #include <linux/gfp.h>
 #include <linux/module.h>
 #include <linux/platform_data/atmel.h>
+#include <video/of_display_timing.h>

 #include <mach/cpu.h>
 #include <asm/gpio.h>

 #include <video/atmel_lcdc.h>

+struct atmel_lcdfb_config {
+       bool have_alt_pixclock;
+       bool have_hozval;
+       bool have_intensity_bit;
+};
+
+ /* LCD Controller info data structure, stored in device platform_data */
+struct atmel_lcdfb_info {
+       spinlock_t              lock;
+       struct fb_info          *info;
+       void __iomem            *mmio;
+       int                     irq_base;
+       struct work_struct      task;
+
+       unsigned int            smem_len;
+       struct platform_device  *pdev;
+       struct clk              *bus_clk;
+       struct clk              *lcdc_clk;
+
+       struct backlight_device *backlight;
+       u8                      bl_power;
+       bool                    lcdcon_pol_negative;

I think lcdcon_pol_negative should be part of pdata, because it really
depends on how the PWM is wired on the board.

maybe but no one mainline use it on any pdata for non-dt boars
so I did not want to expose it

Well, at least, I'm using it :)
(and I guess that Andreas is using it also, otherwise he wouldn't have
introduce it !)

yes but pdata is for non-dt boards, for dt you can keep it in struct
atmel_lcdfb_info and add a property

if non-dt boards want it my answer is I do not care switch to DT

Best Regards,
J.

Best regards,
Richard.