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

Started by Richard Genoud on May 30, 2013 · 5 messages from 4 authors · Last reply May 30, 2013

open the first message on its own page

Message details and related content
2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [off-list ref]:
On 19:44 Wed 29 May     , Richard Genoud wrote:
quoted
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: <redacted>
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

48 more quoted lines

ok (I use a full DT board based on sam9g35)

so I'll add something like
sinfo->lcdcon_pol_negative = of_property_read_bool(display_np,
"atmel,lcdcon-pwm-pulse-low");
in /atmel_lcdfb.c

But I thought the goal of this patch was to separate driver data from
platform specific data, and IMHO, lcdcon_pol_negative is a specificity
of the platform.

Best regards,
Richard.
Message details and related content
On 08:39 Thu 30 May     , Richard Genoud wrote:
2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [off-list ref]:
quoted
On 19:44 Wed 29 May     , Richard Genoud wrote:
quoted
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: <redacted>
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

60 more quoted lines

You are right but as non one mainline use it as pdata I choose to drop it
and only keep it on the driver as we can still use it for DT

It's a way to force peopoe to switch to DT

Best Regards,
J.
Best regards,
Richard.
Message details and related content
Around Thu 30 May 2013 09:23:28 +0200 or thereabout, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 08:39 Thu 30 May     , Richard Genoud wrote:
quoted
2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [off-list ref]:
quoted
On 19:44 Wed 29 May     , Richard Genoud wrote:
quoted
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]:
<snipp diff>
quoted
quoted
quoted
quoted
quoted
quoted
+       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
ok (I use a full DT board based on sam9g35)

so I'll add something like
sinfo->lcdcon_pol_negative = of_property_read_bool(display_np,
"atmel,lcdcon-pwm-pulse-low");
in /atmel_lcdfb.c

But I thought the goal of this patch was to separate driver data from
platform specific data, and IMHO, lcdcon_pol_negative is a specificity
of the platform.
You are right but as non one mainline use it as pdata I choose to drop it
and only keep it on the driver as we can still use it for DT

It's a way to force peopoe to switch to DT
FYI AVR32 architecture doesn't support device tree, nor is it likely to do
so. However there are no users of the PWM output from the LCDC block for the
boards in the current kernel, and I doubt new boards will pop.

-- 
Hans-Christian Egtvedt
Message details and related content
On 30.05.13 09:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 08:39 Thu 30 May     , Richard Genoud wrote:
quoted
2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [off-list ref]:
quoted
On 19:44 Wed 29 May     , Richard Genoud wrote:
quoted
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: <redacted>
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

66 more quoted lines

well, we use it on an avr32 board (not mainlined; though no kernel
update planned currently) ... but no way to use DT currently.

Best regards

Andreas Bießmann
Message details and related content
On 16:27 Thu 30 May     , Andreas Bießmann wrote:
On 30.05.13 09:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
quoted
On 08:39 Thu 30 May     , Richard Genoud wrote:
quoted
2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [off-list ref]:
quoted
On 19:44 Wed 29 May     , Richard Genoud wrote:
quoted
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: <redacted>
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]

70 more quoted lines

If there is 1 ARV32 user it's an other story

Best Regards,
J.
Best regards

Andreas Bießmann
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help