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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 18002C433F5 for ; Tue, 14 Dec 2021 12:48:44 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id CF72E811B9; Tue, 14 Dec 2021 13:48:41 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1639486122; bh=WQbuaRKKDIeK7ZaaU//6ciiPcqa1pdTEQ7IqFicyDw0=; h=Date:Subject:To:Cc:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=pFCI92PrZOx7yk+wKwIoZ+bbHdaB7DD8aHPQv5a3cCWoAWQO3LUdxrfqkEYIoqDM0 hdyhrF/QCqOwG07W6eKc8PQ7/cjTIQYF2zKKkJ51NldO5R6duA7fa+SaXXzgd1zCQ1 /Hg7vHiVPwoJr6vRMtDYHWCl7th+gVVilAee6f7TNE7iV2v9UpSZH6NZfguAgRHd8s ln3gDR0tBG5Xm+pBpC/j4yhpVAiESrRqg8ixyT4vbQBFiplWqR8jwEIv3nXuO5OtX7 btZZV9zG3TgHUtPsUPNkvuK0Xos2iXDTp/Nhv66DSUrCYAHhh7IAebOlL9WdC7jj7t sTZ33XUWVd2bw== Received: by phobos.denx.de (Postfix, from userid 109) id DAE558129B; Tue, 14 Dec 2021 13:48:39 +0100 (CET) Received: from mout-u-107.mailbox.org (mout-u-107.mailbox.org [91.198.250.252]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id D2977810ED for ; Tue, 14 Dec 2021 13:48:36 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=sr@denx.de Received: from smtp102.mailbox.org (unknown [91.198.250.119]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-u-107.mailbox.org (Postfix) with ESMTPS id 4JCyq81qYxzQl7N; Tue, 14 Dec 2021 13:48:36 +0100 (CET) Message-ID: <6cf901fd-01d7-6461-18e4-b362a37bac7c@denx.de> Date: Tue, 14 Dec 2021 13:48:31 +0100 MIME-Version: 1.0 Subject: Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible Content-Language: en-US To: =?UTF-8?Q?Marek_Beh=c3=ban?= , =?UTF-8?Q?Pali_Roh=c3=a1r?= Cc: u-boot@lists.denx.de, =?UTF-8?Q?Marek_Beh=c3=ban?= References: <20211126143738.23830-1-kabel@kernel.org> <20211126143738.23830-9-kabel@kernel.org> <20211214093600.e2sy5yrtpz7gvfna@pali> <20211214104515.2ae03d0d@thinkpad> <20211214111234.ie54zex6veoirfam@pali> <20211214134536.2baeb2a0@thinkpad> From: Stefan Roese In-Reply-To: <20211214134536.2baeb2a0@thinkpad> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.38 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 12/14/21 13:45, Marek Behún wrote: > On Tue, 14 Dec 2021 12:12:34 +0100 > Pali Rohár wrote: > >> On Tuesday 14 December 2021 10:45:15 Marek Behún wrote: >>> On Tue, 14 Dec 2021 10:36:00 +0100 >>> Pali Rohár wrote: >>> >>>> On Friday 26 November 2021 15:37:37 Marek Behún wrote: >>>>> @@ -340,17 +333,17 @@ void board_init_f(ulong dummy) >>>>> timer_init(); >>>>> >>>>> /* Armada 375 does not support SerDes and DDR3 init yet */ >>>>> -#if !defined(CONFIG_ARMADA_375) >>>>> - /* First init the serdes PHY's */ >>>>> - serdes_phy_config(); >>>>> - >>>>> - /* Setup DDR */ >>>>> - ret = ddr3_init(); >>>>> - if (ret) { >>>>> - debug("ddr3_init() failed: %d\n", ret); >>>>> - hang(); >>>>> + if (!IS_ENABLED(CONFIG_ARMADA_375)) { >>>>> + /* First init the serdes PHY's */ >>>>> + serdes_phy_config(); >>>>> + >>>>> + /* Setup DDR */ >>>>> + ret = ddr3_init(); >>>>> + if (ret) { >>>>> + debug("ddr3_init() failed: %d\n", ret); >>>>> + hang(); >>>>> + } >>>>> } >>>>> -#endif >>>> >>>> As written in comment above there is no SerDes and DDR3 support for >>>> Armada 375 and therefore there is no serdes_phy_config() or ddr3_init() >>>> function. So this code needs not to be compiled at all and usage of >>>> #ifdef is correct here. >>> >>> #ifdefs are almost never correct in C-files, for the parts of the code >>> they guard isn't put through syntactic analysis, and can therefore >>> contain bugs which we are not warned about. >>> >>> Using if (IS_ENABLED()) almost never producess a different binary, >>> because the code is optimized away. >>> >>> Marek >> >> There is no function serdes_phy_config() for Armada 375, so if you put >> it outside of #ifdef you will get compile error. > > The function is always declared in > arch/arm/mach-mvebu/include/mach/cpu.h > regardless of architecture. > > Thus an error will be raised only when linking, and the compliation was > done with -O0, which I don't think anyone does. > > Anyway, if we want to support -O0, this can and should be solved via > defining serdes_phy_config() as empty static inline function in the > cpu.h header, guarded by #ifdef. In header files #ifdefs are allowed, > in this manner: > #if X > declare function > #else > define that function as empty static inline > #endif > > So if you think we should support -O0, I can do this. > > But the #ifdefs should really go away from real C code, that is the way > both Linux and U-Boot are trying to go for the last couple of years, if > I understand it correctly. Yes, the #ifdef's really should be avoided if possible. So *if* your patch above does not generate any build issues, then I don't see any problems to include it. I personally don't think that we need to support -O0 builds. Thanks, Stefan