On Tuesday 02 December 2014 17:39:12 Sean Paul wrote:
.../devicetree/bindings/video/backlight/lp855x.txt | 2 ++
drivers/video/backlight/lp855x_bl.c | 18 ++++++++++++++++++
include/linux/platform_data/lp855x.h | 2 ++
3 files changed, 22 insertions(+)
While your two patches are both correct (and applied already), I took
a look at the platform_data header and noticed that all users of this
file have been converted to DT a while ago, so it would be nice to clean
it up by integrating the linux/platform_data/lp855x.h header file into the
drivers/video/backlight/lp855x_bl.c itself.
You can then merge struct lp855x_platform_data into lp855x to avoid the
extra dynamic allocation and remove all the 'pdata' references in the
driver.
Arnd
On Wed, Dec 3, 2014 at 6:11 AM, Arnd Bergmann [off-list ref] wrote:
On Tuesday 02 December 2014 17:39:12 Sean Paul wrote:
quoted
.../devicetree/bindings/video/backlight/lp855x.txt | 2 ++
drivers/video/backlight/lp855x_bl.c | 18 ++++++++++++++++++
include/linux/platform_data/lp855x.h | 2 ++
3 files changed, 22 insertions(+)
While your two patches are both correct (and applied already), I took
a look at the platform_data header and noticed that all users of this
file have been converted to DT a while ago, so it would be nice to clean
it up by integrating the linux/platform_data/lp855x.h header file into the
drivers/video/backlight/lp855x_bl.c itself.
You can then merge struct lp855x_platform_data into lp855x to avoid the
extra dynamic allocation and remove all the 'pdata' references in the
driver.
Hi Arnd,
Thanks for the suggestion. Indeed, that will be much cleaner, I'll
send a follow-up patch.
Sean
Arnd
Hi Arnd,
Good to talk to you again. It's been a long time since I met you in
Linaro Connect HK few years ago :)
On 12/3/2014 11:11 PM, Arnd Bergmann wrote:
While your two patches are both correct (and applied already), I took
a look at the platform_data header and noticed that all users of this
file have been converted to DT a while ago, so it would be nice to clean
it up by integrating the linux/platform_data/lp855x.h header file into the
drivers/video/backlight/lp855x_bl.c itself.
I like this cleanup but need to consider few things.
- Platform which does not support the DT
In this case, there is no way to configure options like backlight device
control mode. Without the DT, only default options are set.
To change the settings, the driver modification is required.
I don't want to write project/platform dependency code inside the driver.
- Backward compatibility
Some customers have been using this platform data in their board-*.c
(but this *.c has not been mainlined yet).
My concern is the backward compatibility issue may happen in their projects.
Best regards,
Milo