From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 69BF6C433EF for ; Tue, 7 Sep 2021 22:42:44 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9A5DF60E77 for ; Tue, 7 Sep 2021 22:42:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 9A5DF60E77 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id BB51381A26; Wed, 8 Sep 2021 00:42:40 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id ADCB481E14; Wed, 8 Sep 2021 00:42:38 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id B9E8F80F4B for ; Wed, 8 Sep 2021 00:42:34 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=andre.przywara@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BCE866D; Tue, 7 Sep 2021 15:42:33 -0700 (PDT) Received: from slackpad.fritz.box (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C334A3F73D; Tue, 7 Sep 2021 15:42:32 -0700 (PDT) Date: Tue, 7 Sep 2021 23:42:16 +0100 From: Andre Przywara To: Arnaud Ferraris Cc: u-boot@lists.denx.de, Jagan Teki , Samuel Holland , Maxime Ripard Subject: Re: [PATCH 1/2] board: sunxi: enable status LED early Message-ID: <20210907234216.2446b437@slackpad.fritz.box> In-Reply-To: References: <20210906205753.176175-1-arnaud.ferraris@collabora.com> <20210906205753.176175-2-arnaud.ferraris@collabora.com> <20210907004644.290710e8@slackpad.fritz.box> Organization: Arm Ltd. X-Mailer: Claws Mail 3.17.1 (GTK+ 2.24.31; x86_64-slackware-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On Tue, 7 Sep 2021 13:41:11 +0200 Arnaud Ferraris wrote: Hi Arnaud, > Thanks for your feedback! >=20 > Le 07/09/2021 =C3=A0 01:46, Andre Przywara a =C3=A9crit=C2=A0: > > On Mon, 6 Sep 2021 22:57:52 +0200 > > Arnaud Ferraris wrote: > >=20 > > Hi Arnaud, > > =20 > >> diff --git a/board/sunxi/board.c b/board/sunxi/board.c > >> index 1a46100e40..6e0bf5fbf9 100644 > >> --- a/board/sunxi/board.c > >> +++ b/board/sunxi/board.c > >> @@ -46,6 +46,9 @@ > >> #include > >> #include > >> #include > >> +#if defined(CONFIG_LED_STATUS) && defined(CONFIG_SPL_DRIVERS_MISC) > >> +#include =20 > >=20 > > status_led.h already guards for CONFIG_LED_STATUS, so you can just > > unconditionally include this here, no #ifdefs needed. =20 >=20 > Understood, will do. >=20 > > =20 > >> +#endif > >> =20 > >> #if defined CONFIG_VIDEO_LCD_PANEL_I2C && !(defined CONFIG_SPL_BUILD) > >> /* So that we can use pin names in Kconfig and sunxi_name_to_gpio() */ > >> @@ -672,6 +675,10 @@ void sunxi_board_init(void) > >> { > >> int power_failed =3D 0; > >> =20 > >> +#if defined(CONFIG_LED_STATUS) && defined(CONFIG_SPL_DRIVERS_MISC) =20 > >=20 > > Unfortunately status_led.h doesn't define a dummy prototype for this, > > so we need the #ifdef CONFIG_LED_STATUS. But I don't think you need > > CONFIG_SPL_DRIVERS_MISC here. If that should only trigger in the SPL, u= se: > >=20 > > if (IS_ENABLED(CONFIG_SPL_BUILD)) > > status_led_init(); > > =20 >=20 > Actually the status_led driver isn't compiled into SPL unless > CONFIG_SPL_DRIVERS_MISC is set, resulting in a link error if that's not > the case. We should therefore check for both CONFIG_LED_STATUS and > CONFIG_SPL_DRIVERS_MISC to prevent build errors both at compile time and > link time. >=20 > The alternative would be: >=20 > #ifdef CONFIG_LED_STATUS > if (IS_ENABLED(CONFIG_SPL_DRIVERS_MISC)) > status_led_init(); > #endif Right, the nasty bit here is that you could just have LED_STATUS for U-Boot proper, but there is no separate CONFIG_SPL_LED_STATUS, so we need both symbols, although it looks weird. > Which is more or less the same, except that it relies on the compiler > optimizing out this function call if CONFIG_SPL_DRIVERS_MISC isn't > defined. That assumption feels less safe to me, but overall I'm fine > with both implementations. Yes, but I still prefer to have as little #ifdef as possible, because ... > Please also note the whole sunxi_board_init() function itself is already > inside a #ifdef CONFIG_SPL_BUILD block. You are right, I missed that. That whole file is an #ifdef nightmare, and especially nested #ifdefs are the worst. So I would like to not *add* more of them, hence asking for IS_ENABLED() wherever possible. Thanks, Andre > Cheers, > Arnaud >=20 > > Cheers, > > Andre > > =20 > >> + status_led_init(); > >> +#endif > >> + > >> #ifdef CONFIG_SY8106A_POWER > >> power_failed =3D sy8106a_set_vout1(CONFIG_SY8106A_VOUT1_VOLT); > >> #endif =20