Re: [PATCH v12 2/3] memory: Add STM32 Octo Memory Manager driver

[Krzysztof Kozlowski <krzk@kernel.org>]

On 06/05/2025 13:16, Patrice CHOTARD wrote:
> 
> 
> On 5/6/25 10:02, Krzysztof Kozlowski wrote:
>> On 06/05/2025 09:52, Patrice Chotard wrote:
>>> Octo Memory Manager driver (OMM) manages:
>>>   - the muxing between 2 OSPI busses and 2 output ports.
>>>     There are 4 possible muxing configurations:
>>>       - direct mode (no multiplexing): OSPI1 output is on port 1 and OSPI2
>>>         output is on port 2
>>>       - OSPI1 and OSPI2 are multiplexed over the same output port 1
>>>       - swapped mode (no multiplexing), OSPI1 output is on port 2,
>>>         OSPI2 output is on port 1
>>>       - OSPI1 and OSPI2 are multiplexed over the same output port 2
>>>   - the split of the memory area shared between the 2 OSPI instances.
>>>   - chip select selection override.
>>>   - the time between 2 transactions in multiplexed mode.
>>>   - check firewall access.
>>>
>>> Signed-off-by: Christophe Kerello <christophe.kerello@foss.st.com>
>>> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
>>> ---
>>>  drivers/memory/Kconfig     |  18 ++
>>>  drivers/memory/Makefile    |   1 +
>>>  drivers/memory/stm32_omm.c | 476 +++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 495 insertions(+)
>>>
>>> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
>>> index c82d8d8a16eaf154c247c0dbb9aff428b7c81402..bc7ab46bd8b98a89f0d9173e884a99b778cdc9c4 100644
>>> --- a/drivers/memory/Kconfig
>>> +++ b/drivers/memory/Kconfig
>>> @@ -225,6 +225,24 @@ config STM32_FMC2_EBI
>>>  	  devices (like SRAM, ethernet adapters, FPGAs, LCD displays, ...) on
>>>  	  SOCs containing the FMC2 External Bus Interface.
>>>  
>>> +config STM32_OMM
>>> +	tristate "STM32 Octo Memory Manager"
>>> +	depends on ARCH_STM32 || COMPILE_TEST
>>> +	depends on SPI_STM32_OSPI
>>
>> I don't think you tested for the reported issue. I reported that
>> firewall symbols are missing and you add dependency on ospi. How is that
>> related? How does this solve any problem?
> 
> Hi Krzysztof
> 
> The dependency with SPI_STM32_OSPI was already present since the beginning.
> I just added dependency on ARCH_STM32 on this current version to avoid issue on x86_64 arch.

Ah, I missed that in the diff.

Anyway this does not solve the case - you still won't get your
bus/firewall code.

> 
> On my side i tested compilation on arm, arm64 and x86_64 without any issue.

Oh man... do you understand that this is compile test? Enable STM32_OMM
on x86_64 and immediately you will see the error.


> 
> I tried to reproduce your build process:
> 
> 
> 
> make -j16 defconfig
>   HOSTCC  scripts/basic/fixdep
>   HOSTCC  scripts/kconfig/conf.o
>   HOSTCC  scripts/kconfig/confdata.o
>   HOSTCC  scripts/kconfig/expr.o
>   LEX     scripts/kconfig/lexer.lex.c
>   YACC    scripts/kconfig/parser.tab.[ch]
>   HOSTCC  scripts/kconfig/menu.o
>   HOSTCC  scripts/kconfig/preprocess.o
>   HOSTCC  scripts/kconfig/symbol.o
>   HOSTCC  scripts/kconfig/util.o
>   HOSTCC  scripts/kconfig/lexer.lex.o
>   HOSTCC  scripts/kconfig/parser.tab.o
>   HOSTLD  scripts/kconfig/conf
> *** Default configuration is based on 'x86_64_defconfig'
> #
> # configuration written to .config
> #
> 
> scripts/config --file .config -e COMPILE_TEST -e OF -e SRAM -e MEMORY -e PM_DEVFREQ -e FPGA -e FPGA_DFL
> 
> scripts/config --file .config -e SAMSUNG_MC
> scripts/config --file .config -e EXYNOS5422_DMC
> scripts/config --file .config -e EXYNOS_SROM
> scripts/config --file .config -e TEGRA_MC
> scripts/config --file .config -e TEGRA20_EMC
> scripts/config --file .config -e TEGRA30_EMC
> scripts/config --file .config -e TEGRA124_EMC
> scripts/config --file .config -e TEGRA210_EMC_TABLE
> scripts/config --file .config -e TEGRA210_EMC
> scripts/config --file .config -e MEMORY
> scripts/config --file .config -e DDR
> scripts/config --file .config -e ARM_PL172_MPMC
> scripts/config --file .config -e ATMEL_EBI
> scripts/config --file .config -e BRCMSTB_DPFE
> scripts/config --file .config -e BRCMSTB_MEMC
> scripts/config --file .config -e BT1_L2_CTL
> scripts/config --file .config -e TI_AEMIF
> scripts/config --file .config -e TI_EMIF
> scripts/config --file .config -e OMAP_GPMC
> scripts/config --file .config -e OMAP_GPMC_DEBUG
> scripts/config --file .config -e TI_EMIF_SRAM
> scripts/config --file .config -e FPGA_DFL_EMIF
> scripts/config --file .config -e MVEBU_DEVBUS
> scripts/config --file .config -e FSL_CORENET_CF
> scripts/config --file .config -e FSL_IFC
> scripts/config --file .config -e JZ4780_NEMC
> scripts/config --file .config -e MTK_SMI
> scripts/config --file .config -e DA8XX_DDRCTL
> scripts/config --file .config -e PL353_SMC
> scripts/config --file .config -e RENESAS_RPCIF
> scripts/config --file .config -e STM32_FMC2_EBI
> scripts/config --file .config -e STM32_OMM

That's the code from previous version, which would lead you to the bug.
Once you understand the bug, you should understand that SPI_STM32_OSPI
is not selected here, thus STM32_OMM is not there.

You did not fix the bug, you just masked it for one given configuration,
but still having the bug for other.

Answer to yourself: where are firewall functions? Then, answer: is this
thing with firewall selected (or expressed as dependency) when you
select this driver?

If not, do you have stubs for the firewall?
If yes, do you have stubs for module case (one is a module, other is not)?

This all will lead you to missing dependency for the firewall kconfig.
Now the dependency must be also tested for module & non-module cases
(see longer explanation in docs kconfig syntax).

Best regards,
Krzysztof