# Re: \[PATCH 1/8\] video: atmel\_lcdfb: fix platform data struct

- List: linux-fbdev
- 5 messages, 4 authors
- Page: https://ratatoskr.run/linux-fbdev/2013/05/8072406/t

## Re: \[PATCH 1/8\] video: atmel\_lcdfb: fix platform data struct

- From: Richard Genoud \<hidden\>
- Date: 2013-05-30 06:39:58 
- Message: https://ratatoskr.run/linux-fbdev/2013/05/8072406.md

```text
2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [ref]:
> On 19:44 Wed 29 May     , Richard Genoud wrote:
>> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [ref]:
>> > On 16:36 Wed 29 May     , Richard Genoud wrote:
>> >> 2013/4/11 Jean-Christophe PLAGNIOL-VILLARD [ref]:
>> >> > 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]
>> >> > 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

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.
```

## Re: \[PATCH 1/8\] video: atmel\_lcdfb: fix platform data struct

- From: Jean-Christophe PLAGNIOL-VILLARD \<hidden\>
- Date: 2013-05-30 07:23:28 
- Message: https://ratatoskr.run/linux-fbdev/2013/05/8072408.md

```text
On 08:39 Thu 30 May     , Richard Genoud wrote:
> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [ref]:
> > On 19:44 Wed 29 May     , Richard Genoud wrote:
> >> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [ref]:
> >> > On 16:36 Wed 29 May     , Richard Genoud wrote:
> >> >> 2013/4/11 Jean-Christophe PLAGNIOL-VILLARD [ref]:
> >> >> > 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]
> >> >> > 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
> 
> 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

Best Regards,
J.
> 
> Best regards,
> Richard.
```

## Re: \[PATCH 1/8\] video: atmel\_lcdfb: fix platform data struct

- From: Hans-Christian Egtvedt \<hidden\>
- Date: 2013-05-30 14:05:55 
- Message: https://ratatoskr.run/linux-fbdev/2013/05/8072433.md

```text
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:
>> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [ref]:
>> > On 19:44 Wed 29 May     , Richard Genoud wrote:
>> >> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [ref]:
>> >> > On 16:36 Wed 29 May     , Richard Genoud wrote:
>> >> >> 2013/4/11 Jean-Christophe PLAGNIOL-VILLARD [ref]:

<snipp diff>

>> >> >> > +       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
```

## Re: \[PATCH 1/8\] video: atmel\_lcdfb: fix platform data struct

- From: Andreas Bießmann \<hidden\>
- Date: 2013-05-30 14:27:38 
- Message: https://ratatoskr.run/linux-fbdev/2013/05/8072435.md

```text
On 30.05.13 09:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 08:39 Thu 30 May     , Richard Genoud wrote:
>> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [ref]:
>>> On 19:44 Wed 29 May     , Richard Genoud wrote:
>>>> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [ref]:
>>>>> On 16:36 Wed 29 May     , Richard Genoud wrote:
>>>>>> 2013/4/11 Jean-Christophe PLAGNIOL-VILLARD [ref]:
>>>>>>> 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]
>>>>>>> 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
>>
>> 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

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
```

## Re: \[PATCH 1/8\] video: atmel\_lcdfb: fix platform data struct

- From: Jean-Christophe PLAGNIOL-VILLARD \<hidden\>
- Date: 2013-05-30 14:59:38 
- Message: https://ratatoskr.run/linux-fbdev/2013/05/8072449.md

```text
On 16:27 Thu 30 May     , Andreas Bießmann wrote:
> On 30.05.13 09:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 08:39 Thu 30 May     , Richard Genoud wrote:
> >> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [ref]:
> >>> On 19:44 Wed 29 May     , Richard Genoud wrote:
> >>>> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD [ref]:
> >>>>> On 16:36 Wed 29 May     , Richard Genoud wrote:
> >>>>>> 2013/4/11 Jean-Christophe PLAGNIOL-VILLARD [ref]:
> >>>>>>> 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]
> >>>>>>> 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
> >>
> >> 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
> 
> well, we use it on an avr32 board (not mainlined; though no kernel
> update planned currently) ... but no way to use DT currently.

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

Best Regards,
J.
> 
> Best regards
> 
> Andreas Bießmann
```