On 2/3/25 12:40, Krzysztof Kozlowski wrote: > 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? As described in the commit message, OMM manages the muxing of the 2 OSPI buses (its 2 child), that's the relation. Do you want i add this directly in yaml file ? > >> >>> >>>> >>>> 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 i have correctly understood, you want a constraint on range and memory-regions properties ? Is it what you expect ? ranges: description: | Reflects the memory layout with four integer values per OSPI instance. Format: <chip-select> 0 <registers base address> <size> minItems: 1 maxItems: 2 memory-region-names: description: | OCTOSPI instance's name to which memory region is associated items: enum: [ospi1, ospi2] minItems: 1 maxItems: 2 Thanks Patrice > >> >> 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
