Message-ID: <f56ae1b6-657a-4809-b95c-e318462d4270@foss.st.com>
Date: Wed, 12 Feb 2025 17:18:30 +0100
From: Patrice CHOTARD <patrice.chotard@...s.st.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
CC: Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof
 Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Alexandre
 Torgue <alexandre.torgue@...s.st.com>,
        Philipp Zabel
	<p.zabel@...gutronix.de>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>, <linux-spi@...r.kernel.org>,
        <devicetree@...r.kernel.org>,
        <linux-stm32@...md-mailman.stormreply.com>,
        <linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
        <christophe.kerello@...s.st.com>
Subject: Re: [PATCH v3 4/8] memory: Add STM32 Octo Memory Manager driver



On 2/12/25 13:51, Geert Uytterhoeven wrote:
> Hi Patrice,
> 
> On Mon, 10 Feb 2025 at 14:21, <patrice.chotard@...s.st.com> wrote:
>> From: Patrice Chotard <patrice.chotard@...s.st.com>
>>
>> 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: Patrice Chotard <patrice.chotard@...s.st.com>
>> Signed-off-by: Christophe Kerello <christophe.kerello@...s.st.com>
> 
> Thanks for your patch!
> 
>> --- a/drivers/memory/Kconfig
>> +++ b/drivers/memory/Kconfig
>> @@ -225,6 +225,23 @@ 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 SPI_STM32_OSPI || TEST_COMPILE
> 
> COMPILE_TEST

 good catch 

> 
>> +       help
>> +         This driver manages the muxing between the 2 OSPI busses and
>> +         the 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
>> +         It also manages :
>> +           - the split of the memory area shared between the 2 OSPI instances.
>> +           - chip select selection override.
>> +           - the time between 2 transactions in multiplexed mode.
>> +
>>  source "drivers/memory/samsung/Kconfig"
>>  source "drivers/memory/tegra/Kconfig"
> 
>> --- /dev/null
>> +++ b/drivers/memory/stm32_omm.c
> 
>> +static int stm32_omm_set_amcr(struct device *dev, bool set)
>> +{
>> +       struct stm32_omm *omm = dev_get_drvdata(dev);
>> +       struct regmap *syscfg_regmap;
>> +       struct device_node *node;
>> +       struct resource res, res1;
>> +       resource_size_t mm_ospi2_size = 0;
>> +       static const char * const mm_name[] = { "ospi1", "ospi2" };
>> +       u32 amcr_base, amcr_mask;
>> +       int ret, i, idx;
> 
> unsigned int i

ok

> 
>> +       unsigned int amcr, read_amcr;
>> +
>> +       for (i = 0; i < omm->nb_child; i++) {
>> +               idx = of_property_match_string(dev->of_node,
>> +                                              "memory-region-names",
>> +                                              mm_name[i]);
>> +               if (idx < 0)
>> +                       continue;
>> +
>> +               /* res1 only used on second loop iteration */
>> +               res1.start = res.start;
>> +               res1.end = res.end;
>> +
>> +               node = of_parse_phandle(dev->of_node, "memory-region", idx);
>> +               if (!node)
>> +                       continue;
>> +
>> +               ret = of_address_to_resource(node, 0, &res);
>> +               if (ret) {
>> +                       dev_err(dev, "unable to resolve memory region\n");
>> +                       return ret;
>> +               }
>> +
>> +               /* check that memory region fits inside OMM memory map area */
>> +               if (!resource_contains(omm->mm_res, &res)) {
>> +                       dev_err(dev, "%s doesn't fit inside OMM memory map area\n",
>> +                               mm_name[i]);
>> +                       dev_err(dev, "[0x%llx-0x%llx] doesn't fit inside [0x%llx-0x%llx]\n",
>> +                               res.start, res.end,
>> +                               omm->mm_res->start, omm->mm_res->end);
> 
> As reported by the kernel test robot, this fails to build when
> resource_size_t differs from unsigned long long.  However, you can
> easily print the full resource instead:
> 
>     dev_err(dev, "%pR doesn't fit inside %pR\n", &res, omm->mm_res);
> 
> https://elixir.bootlin.com/linux/v6.13.2/source/Documentation/core-api/printk-formats.rst#L206

OK, thanks for this tips

Patrice

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

Powered by blists - more mailing lists