lkml.org 
[lkml]   [2018]   [Mar]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC 2/4] sh: ecovec24: conditionally register backlight device
    Hello Dmitry

    FYI I am brushing the ecovec board these days as well
    https://www.spinics.net/lists/linux-sh/msg52536.html

    And I have a board to test with but without any display panel, I'm
    afraid.

    On Thu, Mar 15, 2018 at 03:42:00PM -0700, Dmitry Torokhov wrote:
    > Commit fe79f919f47e ("sh: ecovec24: Use gpio-backlight") removed custom
    > backlight support and switched over to generic gpio-backlight driver. The
    > comment when we run with DVI states "no backlight", but setting
    > gpio_backlight_data.fbdev to NULL actually makes gpio-backlight to react to
    > events from any framebuffer device, not ignore them.
    >
    > We want to get rid of platform data in favor of generic device properties
    > in gpio_backlight driver, so we can not have kernel pointers passed around
    > to tie the framebuffer device to backlight. Assuming that the intent of the
    > above referenced commit was to indeed not export backlight when using DVI,
    > let's switch to conditionally registering backlight device so it is not
    > present at all in DVI case.
    >
    > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
    > ---
    > arch/sh/boards/mach-ecovec24/setup.c | 24 +++++++++++++++++-------
    > 1 file changed, 17 insertions(+), 7 deletions(-)
    >
    > diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
    > index 6f929abe0b50f..67633d2d42390 100644
    > --- a/arch/sh/boards/mach-ecovec24/setup.c
    > +++ b/arch/sh/boards/mach-ecovec24/setup.c
    > @@ -368,7 +368,6 @@ static struct platform_device lcdc_device = {
    > };
    >
    > static struct gpio_backlight_platform_data gpio_backlight_data = {
    > - .fbdev = &lcdc_device.dev,
    > .gpio = GPIO_PTR1,
    > .def_value = 1,
    > .name = "backlight",
    > @@ -987,7 +986,6 @@ static struct platform_device *ecovec_devices[] __initdata = {
    > &usb1_common_device,
    > &usbhs_device,
    > &lcdc_device,
    > - &gpio_backlight_device,
    > &ceu0_device,
    > &ceu1_device,
    > &keysc_device,
    > @@ -1077,6 +1075,8 @@ static int __init arch_setup(void)
    > {
    > struct clk *clk;
    > bool cn12_enabled = false;
    > + bool use_backlight = false;
    > + int error;
    >
    > /* register board specific self-refresh code */
    > sh_mobile_register_self_refresh(SUSP_SH_STANDBY | SUSP_SH_SF |
    > @@ -1193,9 +1193,6 @@ static int __init arch_setup(void)
    > lcdc_info.ch[0].lcd_modes = ecovec_dvi_modes;
    > lcdc_info.ch[0].num_modes = ARRAY_SIZE(ecovec_dvi_modes);
    >
    > - /* No backlight */
    > - gpio_backlight_data.fbdev = NULL;
    > -
    > gpio_set_value(GPIO_PTA2, 1);
    > gpio_set_value(GPIO_PTU1, 1);
    > } else {
    > @@ -1217,6 +1214,8 @@ static int __init arch_setup(void)
    > /* enable TouchScreen */
    > i2c_register_board_info(0, &ts_i2c_clients, 1);
    > irq_set_irq_type(IRQ0, IRQ_TYPE_LEVEL_LOW);
    > +
    > + use_backlight = true;
    > }
    >
    > /* enable CEU0 */
    > @@ -1431,8 +1430,19 @@ static int __init arch_setup(void)
    > gpio_set_value(GPIO_PTG4, 1);
    > #endif
    >
    > - return platform_add_devices(ecovec_devices,
    > - ARRAY_SIZE(ecovec_devices));
    > + error = platform_add_devices(ecovec_devices,
    > + ARRAY_SIZE(ecovec_devices));

    I would invert this.
    Register the backlight first, then all other devices.


    > + if (error)
    > + return error;
    > +
    > + if (use_backlight) {
    > + error = platform_device_add(&gpio_backlight_device);
    > + if (error)
    > + pr_warn("%s: failed to register backlight: %d\n",
    > + error);

    Could you use dev_warn here? Also the format is wrong, I assume you
    are missing a '__func__' as second function argument.

    Also, you may want to return error.

    Thanks
    j

    > + }
    > +
    > + return 0;
    > }
    > arch_initcall(arch_setup);
    >
    > --
    > 2.16.2.804.g6dcf76e118-goog
    >
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2018-03-16 11:10    [W:5.759 / U:0.088 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site