| Message ID | 20210916155040.v3.1.I81b4f1edfe925b001299e3b7ba0cf602d9268d59@changeid |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | mtd: spi: nor: force mtd name to "nor%d" | expand |
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 ?
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. Patrick
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 ?
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
diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h index 4963c89642..a1af6fc200 100644 --- a/include/mtd/cfi_flash.h +++ b/include/mtd/cfi_flash.h @@ -157,11 +157,17 @@ struct cfi_pri_hdr { * Use CONFIG_SYS_MAX_FLASH_BANKS_DETECT if defined */ #if defined(CONFIG_SYS_MAX_FLASH_BANKS_DETECT) -#define CONFIG_SYS_MAX_FLASH_BANKS (cfi_flash_num_flash_banks) #define CFI_MAX_FLASH_BANKS CONFIG_SYS_MAX_FLASH_BANKS_DETECT +/* map to cfi_flash_num_flash_banks only when supported */ +#if IS_ENABLED(CONFIG_FLASH_CFI_DRIVER) && \ + (!IS_ENABLED(CONFIG_SPL_BUILD) || IS_ENABLED(CONFIG_SPL_MTD_SUPPORT)) +#define CONFIG_SYS_MAX_FLASH_BANKS (cfi_flash_num_flash_banks) /* board code can update this variable before CFI detection */ extern int cfi_flash_num_flash_banks; #else +#define CONFIG_SYS_MAX_FLASH_BANKS CONFIG_SYS_MAX_FLASH_BANKS_DETECT +#endif +#else #define CFI_MAX_FLASH_BANKS CONFIG_SYS_MAX_FLASH_BANKS #endif
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(). Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com> --- see error in https://source.denx.de/u-boot/custodians/u-boot-stm/-/pipelines/9138 drivers/mtd/spi/spi-nor-core.o: in function `spi_nor_scan': drivers/mtd/spi/spi-nor-core.c:3672: undefined reference to `cfi_flash_num_flash_banks' compilation issue for the boards: - j721e_hs_evm_r5 - j721e_evm_r5j - j721e_hs_evm_a72 - j721e_evm_a72 - sagem_f@st1704_ram Changes in v3: - NEW: solve compilation issue when CONFIG_SYS_MAX_FLASH_BANKS is used include/mtd/cfi_flash.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)