Received: by 10.213.65.68 with SMTP id h4csp34930imn; Fri, 16 Mar 2018 16:39:18 -0700 (PDT) X-Google-Smtp-Source: AG47ELuyAFNdqC1xVRl9Pch/kt/fIC7Wv2ZxQJmDo2e4xRV6p71elA3y4KkKmxBKA0PMOPDBlRBM X-Received: by 2002:a17:902:604e:: with SMTP id a14-v6mr3859809plt.356.1521243558167; Fri, 16 Mar 2018 16:39:18 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1521243558; cv=none; d=google.com; s=arc-20160816; b=H6IUNjrJVLVa2wTdQmmxapqaFoAPks+6DZJfYlpvtEF9zFYhZ2gpHjIKMBCgSRawg1 ZqapuTRP6klWa/reenqV4R4xGtYsPYC0pqSc4BuP9GA4fh2fvm3GE75Zq/xX8LW+mpS2 Pk5LflMeBvFmeaWKzvqdQizHYfFzAOUDHtHKkdkuXlU34E03cTj9Xtqg49xH6ZWGZFJH hxzhsCfagvcXkEiYCdkNR3lp7HibyZ9bBdXTLwF15YBJCWAzujTtWxPQOsTZVLXE5/Oy FuvFQ/UkMtrApnVnNZwctmpm8z9B7MuDYLSkQo7Hd1dnVO9jyjFEmZVNZ/ThhcnQnP8X Gn/A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature:arc-authentication-results; bh=Y/RRBZChYW7teket3J6JfSRUzEJTBD0Q0/R6PAWAYys=; b=edhaX+iBaqmNmwfd3XbB2dmP6A3nA/C9bk2oYA4Gcv18pCDehyHENeeSMrajcq1HL+ mcpJGfsGBoO4YwDrje1R3YcP9bZXu3QfOo/STllBxWomWA04s8HazYHde5j+avqr3eEE aguelX7QJG85RK4ZqQa3BkGMgVYHVwaxbJqzabVlQGKEZs5H9/pMDMv8ecEnZZ9meoRu ontX35PzvVEwYnjnlfurDGo3pJwJBA7wTUdvQhPhF2kbAUnFg8KQhuuxFvc8gzsgHgYJ 6LqEek57vLp/90I5sYGMckcnYgspwxkAu9z7MyNB3mRBlk8eMJvEmxMDM6vOjfzThfVA 0tHg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=NvWjMk0P; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id z123si5739735pgb.466.2018.03.16.16.39.02; Fri, 16 Mar 2018 16:39:18 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=NvWjMk0P; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752609AbeCPXiF (ORCPT + 99 others); Fri, 16 Mar 2018 19:38:05 -0400 Received: from mail-pl0-f67.google.com ([209.85.160.67]:33884 "EHLO mail-pl0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751437AbeCPXiD (ORCPT ); Fri, 16 Mar 2018 19:38:03 -0400 Received: by mail-pl0-f67.google.com with SMTP id u11-v6so3120633plq.1; Fri, 16 Mar 2018 16:38:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Y/RRBZChYW7teket3J6JfSRUzEJTBD0Q0/R6PAWAYys=; b=NvWjMk0P+2refRofkBQhGjuyWpDK+dlgRyhJyLT8n6gyjtyFdtZKiBvGfOqo3cVJg0 hYbXIRoVeqgcgJHG5KvZmSNkhKTP8ujFN54BnfS0rf+sif/Svx1fganwElytHbsfBK35 NYWNdy0kkUA62y75B4iTmpHYKpW4B54vNVYESqyuQ0xa2wnhAMX+QA2grbN813lVCv6K +6cIA7zELFO6mUmdA+VvcZeWjEsuFZKPrE82JO6RW30mcHp/7PI2Bc1TVjnQhoCjQijI ZTCDrA+APMjtZ80dNILBG1l/qxPicaYHiDJslJZI3sgn/FfzZf9tTaEPGOhi4JKfmDv0 dgQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Y/RRBZChYW7teket3J6JfSRUzEJTBD0Q0/R6PAWAYys=; b=FdTb0tXw/hCRqhS9AIMabrWkMD4tyNVjscx1kmpMjySpo3y05twj9nxdOM5BOR0zUU Y+T07I+dLkq5+9x/2cILKVX82zse2IELHrB08+XJnhlYh4NVJNR6F4Gm8b91CvR0tCQG Q3SxzV3LOc+FYSR5uZG/85iAnUvU8MUNWJQJGdvBvzIUP+QE4pfLTL2Gmni1YNFJRC3i BEhi8wZUb00zspJyWE0zxbV84AUP2Dl6kb6vJwtD33b+5hKrFIx5ESPaLbjidfOjRVI1 6UC5amtduFI4uEPwjIyTx1GnS1YyogtjIZHx18cosqkoDNqqE5lZIJudcJkzzc1a+DrP SPDw== X-Gm-Message-State: AElRT7F85kizExwK3ZkXdFFfRrbtE5sxPjqDyZks8bl20sZB96iDTGwU FG3SGHD2fo50lJ79VlT/+B8= X-Received: by 2002:a17:902:5852:: with SMTP id f18-v6mr3758732plj.289.1521243482751; Fri, 16 Mar 2018 16:38:02 -0700 (PDT) Received: from dtor-ws ([2620:0:1000:1511:8de6:27a8:ed13:2ef5]) by smtp.gmail.com with ESMTPSA id z79sm17262240pfa.139.2018.03.16.16.38.01 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 16 Mar 2018 16:38:02 -0700 (PDT) Date: Fri, 16 Mar 2018 16:38:00 -0700 From: Dmitry Torokhov To: jacopo mondi Cc: Laurent Pinchart , Lee Jones , Daniel Thompson , Jingoo Han , linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-sh@vger.kernel.org, Yoshinori Sato , Rich Felker , Linus Walleij Subject: Re: [RFC 2/4] sh: ecovec24: conditionally register backlight device Message-ID: <20180316233800.GA32310@dtor-ws> References: <20180315224202.96668-1-dmitry.torokhov@gmail.com> <20180315224202.96668-3-dmitry.torokhov@gmail.com> <20180316100748.GH16424@w540> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180316100748.GH16424@w540> User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Jacopo, On Fri, Mar 16, 2018 at 11:07:48AM +0100, jacopo mondi wrote: > Hello Dmitry > > FYI I am brushing the ecovec board these days as well > https://www.spinics.net/lists/linux-sh/msg52536.html > What is the ecovec board BTW? Is it some devkit or what? It seems quite old to me. > 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 > > --- > > 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. We could do that, but why would that be better? > > > > + 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 I would rather not, as the backlight device would be in unknown state here, and using dev_warn with device that has not been fully registered does not give any benefits. There is also no ambiguity as there is only one backlight. > are missing a '__func__' as second function argument. I'll fix this. > > Also, you may want to return error. How would caller handle this error? Should we kill all successfully registered devices on error adding backlight? Thanks. -- Dmitry