On 5/24/22 2:19 AM, Krzysztof Kozlowski wrote:
On 23/05/2022 23:38, Tanmay Shah wrote:
Thanks for reviews Krzysztof. Please find my comments below.
On 5/21/22 8:12 AM, Krzysztof Kozlowski wrote:
On 18/05/2022 21:44, Tanmay Shah wrote:
+description: |
+ The Xilinx platforms include a pair of Cortex-R5F processors (RPU) for
+ real-time processing based on the Cortex-R5F processor core from ARM.
+ The Cortex-R5F processor implements the Arm v7-R architecture and includes a
+ floating-point unit that implements the Arm VFPv3 instruction set.
+
+properties:
+ compatible:
+ const: xlnx,zynqmp-r5fss
+
+ xlnx,cluster-mode:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1, 2]
+ description: |
+ The RPU MPCore can operate in split mode(Dual-processor performance), Safety
+ lock-step mode(Both RPU cores execute the same code in lock-step,
+ clock-for-clock) or Single CPU mode (RPU core 0 can be held in reset while
+ core 1 runs normally). The processor does not support dynamic configuration.
+ Switching between modes is only permitted immediately after a processor reset.
+ If set to 1 then lockstep mode and if 0 then split mode.
+ If set to 2 then single CPU mode. When not defined, default will be lockstep mode.
+
+patternProperties:
+ "^r5f-[a-f0-9]+$":
+ type: object
+ description: |
+ The RPU is located in the Low Power Domain of the Processor Subsystem.
+ Each processor includes separate L1 instruction and data caches and
+ tightly coupled memories (TCM). System memory is cacheable, but the TCM
+ memory space is non-cacheable.
+
+ Each RPU contains one 64KB memory and two 32KB memories that
+ are accessed via the TCM A and B port interfaces, for a total of 128KB
+ per processor. In lock-step mode, the processor has access to 256KB of
+ TCM memory.
+
+ properties:
+ compatible:
+ const: xlnx,zynqmp-r5f
+
+ power-domains:
+ description: RPU core PM domain specifier
+ maxItems: 1
+
+ mboxes:
+ items:
+ - description: mailbox channel to send data to RPU
+ - description: mailbox channel to receive data from RPU
+ minItems: 1
+
+ mbox-names:
+ items:
+ - const: tx
+ - const: rx
+ minItems: 1
+
+ sram:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ minItems: 1
maxItems instead
Here, I am not sure how many maxItems are really needed as TCM bindings
are not
defined yet. For now, I will just keep maxItems as 8. i.e. 4 OCM banks
and 4 TCM
banks. However, that can change once bindings are defined.
Is that fine?
Yes, although shrinking might not be allowed once binding is being used.
Ok. we don't expect shrinking if we set 'maxItems: 8'.
Thanks.
+ description: |
+ phandles to one or more reserved on-chip SRAM regions. Other than TCM,
+ the RPU can execute instructions and access data from, the OCM memory,
+ the main DDR memory, and other system memories.
+
+ The regions should be defined as child nodes of the respective SRAM
+ node, and should be defined as per the generic bindings in,
+ Documentation/devicetree/bindings/sram/sram.yaml
+
+ memory-region:
+ description: |
+ List of phandles to the reserved memory regions associated with the
+ remoteproc device. This is variable and describes the memories shared with
+ the remote processor (e.g. remoteproc firmware and carveouts, rpmsg
+ vrings, ...). This reserved memory region will be allocated on DDR memory.
+ minItems: 1
+ items:
+ - description: region used for RPU firmware image section
+ - description: vdev buffer
+ - description: vring0
+ - description: vring1
+ additionalItems: true
How did this one appear here? It does not look correct, so why do you
need it?
Memory regions listed in items: field here are used for default current
OpenAMP demos. However,
other demos can be developed by user that can use more number of memory
regions.
As description says, memory-region can have variable number phandles
based on
user requirement. So, by additionalItems I just want to notify that user can
define more number of regions. We can limit memory-regions with
'maxItems: 8'.
In that case, I will add 'maxItems:' field in next revision and even,
that can change in future.
That sounds fine.
But, User should have flexibility to define more memory regions than
what is in list
of 'items:' field. I think this is similar to what is defined in
ti,k3-r5 bindings.
Please let me know your thoughts.
I see. If schema accepts such combination (listing items + maxItems +
additionalItems), then it's fine.
Ok great. I will set 'maxItems: 8' in that case in next revision.
With this, I will add 'maxItems: 8' in sram and memory-region properties.
If everything else looks good on schema in this revision, could you
please also review next (dts) patch in this series?
If that looks good, can I get your 'rb' on that?
so we can reduce scope of reviews for next revisions?
Best regards,
Krzysztof
