[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <710569e305924a0a84e9792bc779d37a24011477.camel@pengutronix.de>
Date: Tue, 08 Apr 2025 10:59:05 +0200
From: Philipp Zabel <p.zabel@...gutronix.de>
To: Patrice Chotard <patrice.chotard@...s.st.com>, Krzysztof Kozlowski
<krzk@...nel.org>, Rob Herring <robh@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>, Krzysztof Kozlowski
<krzk+dt@...nel.org>, Catalin Marinas <catalin.marinas@....com>, Will
Deacon <will@...nel.org>
Cc: christophe.kerello@...s.st.com, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v8 3/7] memory: Add STM32 Octo Memory Manager driver
On Mo, 2025-04-07 at 15:27 +0200, 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@...s.st.com>
> Signed-off-by: Patrice Chotard <patrice.chotard@...s.st.com>
> ---
> drivers/memory/Kconfig | 17 ++
> drivers/memory/Makefile | 1 +
> drivers/memory/stm32_omm.c | 474 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 492 insertions(+)
>
[...]
> diff --git a/drivers/memory/stm32_omm.c b/drivers/memory/stm32_omm.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..db50aeffb0aa32a9d51a205d8ba30ab2299e1c34
> --- /dev/null
> +++ b/drivers/memory/stm32_omm.c
> @@ -0,0 +1,474 @@
[...]
> +static int stm32_omm_disable_child(struct device *dev)
> +{
> + static const char * const resets_name[] = {"ospi1", "ospi2"};
> + struct stm32_omm *omm = dev_get_drvdata(dev);
> + struct reset_control *reset;
> + int ret;
> + u8 i;
> +
> + ret = stm32_omm_toggle_child_clock(dev, true);
> + if (!ret)
> + return ret;
> +
> + for (i = 0; i < omm->nb_child; i++) {
> + reset = reset_control_get_exclusive(dev, resets_name[i]);
> + if (IS_ERR(reset)) {
> + dev_err(dev, "Can't get %s reset\n", resets_name[i]);
> + return PTR_ERR(reset);
> + };
> +
> + /* reset OSPI to ensure CR_EN bit is set to 0 */
> + reset_control_assert(reset);
> + udelay(2);
> + reset_control_deassert(reset);
> +
> + reset_control_put(reset);
With this reset_control_put(), you are effectively stating that you
don't care about the state of the reset line after this point. To
guarantee the reset line stays deasserted, the driver should keep the
reset control around.
Are you requesting and dropping the reset controls here so the child
drivers can request them at some point? If so, there is an
acquire/relase mechanism for this:
https://docs.kernel.org/driver-api/reset.html#c.reset_control_acquire
Either way, reset_control_get/put() belong in probe/remove.
regards
Philipp
Powered by blists - more mailing lists