Re: [PATCH v3 1/2] mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported

From: Patrick DELAUNAY <patrick.delaunay@foss.st.com>
To: Marek Vasut <marex@denx.de>, <u-boot@lists.denx.de>
Cc: "Priyanka Jain" <priyanka.jain@nxp.com>,
	"Heiko Schocher" <hs@denx.de>,
	"Jagan Teki" <jagan@amarulasolutions.com>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Christophe KERELLO" <christophe.kerello@foss.st.com>,
	"Patrice Chotard" <patrice.chotard@foss.st.com>,
	"Pali Rohár" <pali@kernel.org>,
	"U-Boot STM32" <uboot-stm32@st-md-mailman.stormreply.com>
Subject: Re: [PATCH v3 1/2] mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported
Date: Tue, 21 Sep 2021 14:38:30 +0200	[thread overview]
Message-ID: <74486cb8-3646-17be-161a-c6e834c0c588@foss.st.com> (raw)
In-Reply-To: <34d4bdb8-6d26-f7df-fa15-6aa986343f14@denx.de>

Hi,

On 9/17/21 3:36 PM, Marek Vasut wrote:
> On 9/17/21 12:55 PM, Patrick DELAUNAY wrote:
>> Hi Marek,
>>
>>  > Marek VasutSept. 16, 2021, 5:24 p.m. UTC | #1
>>
>>  > On 9/16/21 4:01 PM, Patrick Delaunay wrote:
>>
>>  >> When CONFIG_SYS_MAX_FLASH_BANKS_DETECT is activated,
>>  >> CONFIG_SYS_MAX_FLASH_BANKS is replaced by cfi_flash_num_flash_banks,
>>  >> but this variable is defined in drivers/mtd/cfi_flash.c, which is
>>  >> compiled only when CONFIG_FLASH_CFI_DRIVER is activated, in U-Boot
>>  >> or in SPL when CONFIG_SPL_MTD_SUPPORT is activated.
>>  >>
>>  >> This patch deactivates this feature 
>> CONFIG_SYS_MAX_FLASH_BANKS_DETECT
>>  >> when flash cfi driver is not activated to avoid compilation issue in
>>  >> the next patch, when CONFIG_SYS_MAX_FLASH_BANKS is used in 
>> spi_nor_scan().
>>
>>  > Maybe just migrate this config option to Kconfig and let Kconfig 
>> handle
>>  > the macro magic ?
>>
>>
>> Sorry for the format  of my answer (it is just copy paste from archive)
>>
>> because I don't received the U-Boot mails on my @foss.st.com mailbo
>>
>> since yesterday.
>>
>>
>> I think about migration but is difficult to don't break the existing 
>> behaviour in kconfig
>>
>> CONFIG_SYS_MAX_FLASH_BANKS and CONFIG_SYS_MAX_FLASH_BANKS_DETECT are 
>> define as 'int'
>>
>> but can be absent => 2 new config CONFIG_USE need to be added
>>
>> CONFIG_USE_SYS_MAX_FLASH_BANKS
>>
>> CONFIG_USE_SYS_MAX_FLASH_BANKS_DETECT
>>
>>
>> and I don't fully understood the mix between the 2 options and 
>> CFI_MAX_FLASH_BANKS
>>
>> in some part of code I think CONFIG_SYS_MAX_FLASH_BANKS should be 
>> replaced by CFI_MAX_FLASH_BANKS
>>
>> to avoid to define CONFIG_SYS_MAX_FLASH_BANKS = 
>> cfi_flash_num_flash_banks (as it is not possible in Kconfig)
>>
>>
>> => too huge task just to solve compilation issues.
>>
>>
>> and I also think to use CONFIG_IS_ENABLED(MTD_SUPPORT)
>>
>> but it not possible because today
>>
>> - CONFIG_SPL_MTD_SUPPORT exist
>>
>> - CONFIG_MTD_SUPPORT don't exit ( test on $(mtd-y) in Makefile)
>>
>>
>> => the creation of this config is a huge task just to solve 
>> compilation issue.
>
> All right, well, ew.
>
> Can you send a subsequent patchset _after_ this one to fix this flash 
> banks mess ?

I can try,

I will add it in my TODO list.

Patrick