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. > >> >> 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. If omm-manager has no child enabled, omm-manager must be disabled as well in DT. > 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) Thanks Patrice > > > Best regards, > Krzysztof
