Re: [PATCH v3 4/8] memory: Add STM32 Octo Memory Manager driver

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Patrice,

On Mon, 10 Feb 2025 at 14:21, <patrice.chotard@xxxxxxxxxxx> wrote:
> From: Patrice Chotard <patrice.chotard@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> Signed-off-by: Christophe Kerello <christophe.kerello@xxxxxxxxxxx>

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

> +       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

> +       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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds