On 03/02/2025 11:46, Patrice CHOTARD wrote: > > > On 1/30/25 16:09, Krzysztof Kozlowski wrote: >> On 30/01/2025 14:32, Patrice CHOTARD wrote: >>> >>> >>> On 1/30/25 13:12, Krzysztof Kozlowski wrote: >>>> On 30/01/2025 09:57, Patrice CHOTARD wrote: >>>>> >>>>> >>>>> On 1/29/25 08:52, Krzysztof Kozlowski wrote: >>>>>> On Tue, Jan 28, 2025 at 09:17:25AM +0100, patrice.chotard@xxxxxxxxxxx wrote: >>>>>>> From: Patrice Chotard <patrice.chotard@xxxxxxxxxxx> >>>>>>> >>>>>>> Add bindings for STM32 Octo Memory Manager (OMM) controller. >>>>>>> >>>>>>> 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. >>>>>>> >>>>>>> Signed-off-by: Patrice Chotard <patrice.chotard@xxxxxxxxxxx> >>>>>>> --- >>>>>>> .../memory-controllers/st,stm32-omm.yaml | 190 ++++++++++++++++++ >>>>>>> 1 file changed, 190 insertions(+) >>>>>>> create mode 100644 Documentation/devicetree/bindings/memory-controllers/st,stm32-omm.yaml >>>>>>> >>>>>>> diff --git a/Documentation/devicetree/bindings/memory-controllers/st,stm32-omm.yaml b/Documentation/devicetree/bindings/memory-controllers/st,stm32-omm.yaml >>>>>>> new file mode 100644 >>>>>>> index 000000000000..7e0b150e0005 >>>>>>> --- /dev/null >>>>>>> +++ b/Documentation/devicetree/bindings/memory-controllers/st,stm32-omm.yaml >>>>>> >>>>>> >>>>>> Filename as compatible, so st,stm32mp25-omm.yaml >>>>>> >>>>>> You already received this comment. >>>>> >>>>> Sorry, i missed this update >>>>> >>>>>> >>>>>>> @@ -0,0 +1,190 @@ >>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>>>>> +%YAML 1.2 >>>>>>> +--- >>>>>>> +$id: http://devicetree.org/schemas/memory-controllers/st,stm32-omm.yaml# >>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>>>>> + >>>>>>> +title: STM32 Octo Memory Manager (OMM) >>>>>>> + >>>>>>> +maintainers: >>>>>>> + - Patrice Chotard <patrice.chotard@xxxxxxxxxxx> >>>>>>> + >>>>>>> +description: | >>>>>>> + The STM32 Octo Memory Manager is a low-level interface that enables an >>>>>>> + efficient OCTOSPI pin assignment with a full I/O matrix (before alternate >>>>>>> + function map) and multiplex of single/dual/quad/octal SPI interfaces over >>>>>>> + the same bus. It Supports up to: >>>>>>> + - Two single/dual/quad/octal SPI interfaces >>>>>>> + - Two ports for pin assignment >>>>>>> + >>>>>>> +properties: >>>>>>> + compatible: >>>>>>> + const: st,stm32mp25-omm >>>>>>> + >>>>>>> + "#address-cells": >>>>>>> + const: 2 >>>>>>> + >>>>>>> + "#size-cells": >>>>>>> + const: 1 >>>>>>> + >>>>>>> + ranges: >>>>>>> + description: | >>>>>>> + Reflects the memory layout with four integer values per OSPI instance. >>>>>>> + Format: >>>>>>> + <chip-select> 0 <registers base address> <size> >>>>>> >>>>>> Do you always have two children? If so, this should have maxItems. >>>>> >>>>> No, we can have one child. >>>> >>>> For the same SoC? How? You put the spi@ in the soc, so I don't >>>> understand how one child is possible. >>> >>> Yes for the same SoC, in DTSI file, the both OCTOSPI child are declared >>> but are disabled by default. >> >> But the child node is there anyway so are the ranges. > > if both child are disabled, omm-manager should be disabled as well, > omm-manager alone makes no sense. Yes, it is obvious, but how is this related? > >> >>> >>> In the DTS board file, 0,1 or 2 OCTOSPI instance can be enabled depending of the board design. >>> >>> In our case, on stm32mp257f-ev1 board, one SPI-NOR is soldered on PCB, so only one OCTOSPI >>> instance is needed and enabled. >>> >>> Internally we got validation boards with several memory devices connected to OCTOSPI1 and >>> OCTOSPI2, in this case, both OCTOSPI instance are needed and enabled. >> >> I could imagine that you would not want to have unused reserved ranges, >> so that one indeed is flexible, I agree. >> >>> >>>> >>>>> >>>>>> >>>>>>> + >>>>>>> + reg: >>>>>>> + items: >>>>>>> + - description: OMM registers >>>>>>> + - description: OMM memory map area >>>>>>> + >>>>>>> + reg-names: >>>>>>> + items: >>>>>>> + - const: regs >>>>>>> + - const: memory_map >>>>>>> + >>>>>>> + memory-region: >>>>>>> + description: Phandle to node describing memory-map region to used. >>>>>>> + minItems: 1 >>>>>>> + maxItems: 2 >>>>>> >>>>>> List the items with description instead with optional minItems. Why is >>>>>> this flexible in number of items? >>>>> >>>>> If only one child (OCTOSPI instance), only one memory-region is needed. >>>> >>>> Which is not possible... look at your DTSI. >>> >>> It's possible. if one OCTOSPI is used (the second one is kept disabled), only >>> one memory-region is needed. >> >> Ack. >> >>> >>>> >>>>> >>>>> Another update, i will reintroduce "memory-region-names:" which was >>>>> wrongly removed in V2, i have forgotten one particular case. >>>>> >>>>> We need memory-region-names in case only one OCTOSPI instance is >>>>> used. If it's OCTOCPI2 and the whole memory-map region >>>>> is dedicated to OCTOSPI2 (OCTOSPI1 unmapped, OCTOSPI2 (256 Mbytes) >>>>> >>>>> We need to know to which OCTOSPI instance the memory region is associated >>>>> with, in order to check "st,syscfg-amcr" 's value which must be coherent >>>>> with memory region declared. >>>>> >>>>> so i will add : >>>>> >>>>> memory-region-names: >>>>> description: | >>>>> OCTOSPI instance's name to which memory region is associated >>>>> items: >>>>> - const: ospi1 >>>>> - const: ospi2 >>>>> >>>> >>>> I don't think this matches what you are saying to us. Let's talk about >>>> the hardware which is directly represented by DTS/DTSI. You always have >>>> two instances. >>>> >>>> >>> >>> We have 2 instances, but both not always enabled. >>> In case only one is enabled, only one memory-region-names is needed. >>> >>> We must know to which OCTCOSPI the memory-region makes reference to, in order >>> to configure and/or check the memory region split configuration. That' swhy >>> the memory-regions-names must specify if it's the OCTOSPI1 or OCTOSPI2 instance. >> >> Well, in that case two comments. >> 1. Above syntax does not allow you to skip one item. You would need: >> items: >> enum: [ospi1, ospi2] >> minItems: 1 >> maxItems: 2 >> > > ok > >> 2. But this points to other problem. From the omm-manager node point of >> view, you should define all the resources regardless whether the child >> is enabled or not. You do not skip some part of 'reg' if child is >> missing. Do not skip interrupts, access controllers, clocks etc. >> If some resource is to be skipped, it means that it belongs to the >> child, not to the parent, IMO. > > I didn't get your point. > > The resource declared in omm-manager's node pnly belongs to omm-manager > (reg/clocks/resets/access-controllers/st,syscfg-amcr/power-domains), regardless > there are 1 or 2 children. None of them can be skipped. That's not true, you skip ranges and memory region. > > If omm-manager has no child enabled, omm-manager must be disabled as well in DT. That's not what we talk about. We do not talk about enabled or disabled. We talk about being there in the first place. > >> Therefore memory-region looks like child's property. >> >> Imagine different case: runtime loaded overlay. In your setup, you probe >> omm-manager with one memory-region and one child. Then someone loads >> overlay enabling the second child, second SPI. >> >> That's of course imaginary case, but shows the concept how the parent >> would work. >> >> It's the same with other buses in the kernel. You can load overlay with >> any new child and the parent should not get new properties. >> > > In case of runtime loaded overlay, if a second child is added with an associated > memory-region, omm-manager must be unbind/bind to : > _ check the added child's access rights. > _ take into account the added child's memory-region configuration (to set > the syscfg-amcr register accordingly) That's driver part, we talk about bindings and DTS. Best regards, Krzysztof