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 11B6BC19F2D for ; Tue, 9 Aug 2022 16:03:21 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 3F71684A68; Tue, 9 Aug 2022 18:03:19 +0200 (CEST) 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=1660060999; bh=58EYwRYnVRRtkw06hkp2dkX5cob+WOFqQ9BS6JwJrOQ=; h=Date:Subject:To:Cc:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=ElUAv+mFPDuDDZLfFyjYh2MascZNekZxU2xAksmGPQ8VG7biU2Q3hWlEgGC50nFpc 7326Zl0rNVADza/oqUOpqQX56AT8OdmNlpcl2cAon48IoXDHBujhz3l1PO/0rGSnxZ txTF5ZssDJvfpvdNQRZWfUKdCF+HYsGiLSyTafeagoE3Pgrh9mxfk93F8NCtgyMlfr 89dXvhZCFSvv2VADzatNWsD/8M+b9qvNqBE3ZZEwDmDlmUZryEliZ7xFQ2UCnz1dzW mGl0sJBwLOY1h0mRR5A4O+QCqXonbV7heMX3h5WIhmUovJIZRkg7q4LHy9hZBMkGcO RswAG/6sJ4WCQ== Received: from [127.0.0.1] (p578adb1c.dip0.t-ipconnect.de [87.138.219.28]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: marex@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id 586D384A14; Tue, 9 Aug 2022 18:03:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1660060997; bh=58EYwRYnVRRtkw06hkp2dkX5cob+WOFqQ9BS6JwJrOQ=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=QSXE7lZoVyyE0avWLR8alsV6DI+ePw/D15ptrZzqsQW6IZTume4s1LuPPO/73AqWU uMQvQoVxKc10QxqFWadxWAnfPkG1XkezVb5b/khVV6r0Jm8V7uAYK5lssqR+EXJDJQ QtydjhuX+TIzETux5DEVbIFUnA6awSFgDTxn+wXRHDLb8yJOuuZWAtZqyHtHuoX2IQ gSJpFj488A6V9PG44CrUWjnBad0Jyiu9WjHEVN9gj9t7HZBiD0QL5hwroiuxNjrghi W5pJUQ/QbazxhmZrDOcRpOPie0yxu/g4HSGVpt5QMdRPYcNhKBmVd0bDj5xn0CWAJM sKbEETaOiLA0A== Message-ID: <7a2a67b7-78ba-d687-55c9-23b4fd1203f6@denx.de> Date: Tue, 9 Aug 2022 18:03:15 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Subject: Re: [PATCH v3 1/3] Convert CONFIG_SYS_L2_PL310 to Kconfig Content-Language: en-US To: Tom Rini , =?UTF-8?Q?Pali_Roh=c3=a1r?= Cc: Philip Oberfichtner , u-boot@lists.denx.de, Christoph Niedermaier , Stefano Babic , Marcel Ziswiler , =?UTF-8?Q?Marek_Beh=c3=ban?= , Peng Fan , u-boot@dh-electronics.com References: <20220809100703.3101047-1-pro@denx.de> <20220809100703.3101047-2-pro@denx.de> <20220809105813.aa4vjcyiqm3mnqx4@pali> <41f76858-38d9-284b-1b54-b511f3b3cd67@denx.de> <20220809112151.jhrytuxraqcahzh5@pali> <4c124a1f-28cb-a805-ac6f-fd50ae2ee00e@denx.de> <20220809113224.4mvygsubvtb5wd7a@pali> <20220809134634.GV1146598@bill-the-cat> From: Marek Vasut In-Reply-To: <20220809134634.GV1146598@bill-the-cat> 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.39 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.6 at phobos.denx.de X-Virus-Status: Clean On 8/9/22 15:46, Tom Rini wrote: > On Tue, Aug 09, 2022 at 01:32:24PM +0200, Pali Rohár wrote: >> On Tuesday 09 August 2022 13:27:38 Marek Vasut wrote: >>> On 8/9/22 13:21, Pali Rohár wrote: >>> >>> (reducing the CC list) >>> >>>> On Tuesday 09 August 2022 13:16:41 Marek Vasut wrote: >>>>> On 8/9/22 12:58, Pali Rohár wrote: >>>>>> On Tuesday 09 August 2022 12:07:00 Philip Oberfichtner wrote: >>>>>>> This converts CONFIG_SYS_L2_PL310 to Kconfig. >>>>>> ... >>>>>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig >>>>>>> index 949ebb46ba..dde06bdd96 100644 >>>>>>> --- a/arch/arm/Kconfig >>>>>>> +++ b/arch/arm/Kconfig >>>>>>> @@ -488,6 +488,10 @@ config TPL_SYS_THUMB_BUILD >>>>>>> density. For ARM architectures that support Thumb2 this flag will >>>>>>> result in Thumb2 code generated by GCC. >>>>>>> +config SYS_L2_PL310 >>>>>>> + bool "ARM PL310 L2 cache controller" > > This needs a depends on !SYS_L2CACHE_OFF. > >>>>>>> + help >>>>>>> + Enable support for ARM PL310 L2 cache controller in U-Boot >>>>>>> config SYS_L2CACHE_OFF >>>>>>> bool "L2cache off" >>>>>>> diff --git a/arch/arm/mach-mvebu/include/mach/config.h b/arch/arm/mach-mvebu/include/mach/config.h >>>>>>> index 4add0d9e10..0bba0a4cf9 100644 >>>>>>> --- a/arch/arm/mach-mvebu/include/mach/config.h >>>>>>> +++ b/arch/arm/mach-mvebu/include/mach/config.h >>>>>>> @@ -25,8 +25,6 @@ >>>>>>> #define MV88F78X60 /* for the DDR training bin_hdr code */ >>>>>>> #endif >>>>>>> -#define CONFIG_SYS_L2_PL310 >>>>>>> - >>>>>>> #define MV_UART_CONSOLE_BASE MVEBU_UART0_BASE >>>>>>> /* Needed for SPI NOR booting in SPL */ >>>>>> >>>>>> This option is required for mvebu SoC and is not user (de)-selectable. >>>>>> So please do not define it in each individual mvebu board. It would make >>>>>> it harder to introduce a new mvebu board into U-Boot. Instead enable it >>>>>> for mvebu SoCs like it was before this change. It can be done e.g. by >>>>>> "select" Kconfig keyword in mvebu Kconfig file. >>>>> >>>>> Should it rather be 'default y if MVEBU' in that new PL310 Kconfig option ? >>>> >>>> No, because this is just default value of this option and still allows >>>> end-user to de-select this option. >>>> >>>> "select" is IIRC the only way how to force Kconfig to always enable some >>>> symbol without any option for end-user to disable it. >>>> >>>> At least I do not know a way how CONFIG_SYS_L2_PL310 symbol could decide >>>> that it is required for CONFIG_MVEBU. Just symbol CONFIG_MVEBU can >>>> decide that it requires CONFIG_SYS_L2_PL310 symbol (and not in opposite >>>> direction). >>> >>> So why should the user be unable to deselect L2CC support on MVEBU ? >>> I can very well disable L2CC support on MX6 and the platform works without >>> it just fine. Maybe the MVEBU needs to be fixed to support the same instead >>> ? >> >> Well, I'm not sure, currently it is non-deselectable option. Maybe it is >> a bug... but at least change which is doing kconfig conversion should >> not change behavior. > > It's tricky to say when to "select" or just "default y if .." or "imply" > a given option. It's not only a matter of "can you disable X and the > system is functional" but "would you normally want to". There are > certainly system bring-up cases and similar where you want to disable > L2CC because you're tracking down something. But it's really a feature > of the chip and expected to work and something we want enabled, and the > scope of when it would be disabled is very very narrow. Looking back at > the patch itself, the config.h files that enabled this are almost all a > SoC-common file (ti_am335x_common.h should have been setting this I bet, > something else for the TODO pile for me), so my thought is that it > should be a select'd option, but if there's strong opinion that it's > useful to make it easy to turn off when needed, imply'd instead by the > various ARCH's in question instead. > > That said, thinking about the missing dependency I listed above, that's > how to disable L2 when needed, so perhaps select SYS_L2_PL310 if > !SYS_L2CACHE_OFF under the various ARCH's is the right way. Uh, I didn't realize we also had this SYS_L2CACHE_OFF . In that case, the SYS_L2_PL310 selects the L2CC driver to be compiled in and that should likely be per-arch select indeed. And then SYS_L2CACHE_OFF should be 'default' or 'imply', so the user can configure it for the various hardware bring up cases ? [...]