On Tue 18 Jul 01:45 PDT 2017, Oleksij Rempel wrote: > Hallo Bjorn, > > On 11.07.2017 00:14, Bjorn Andersson wrote: > > On Mon 10 Jul 07:42 PDT 2017, Oleksij Rempel wrote: > > > > > Signed-off-by: Oleksij Rempel <[email protected]> > > > --- > > > .../devicetree/bindings/remoteproc/imx-rproc.txt | 44 > > > ++++++++++++++++++++++ > > > 1 file changed, 44 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/remoteproc/imx-rproc.txt > > > > > > diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt > > > b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt > > > new file mode 100644 > > > index 000000000000..e7c61993e1b8 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt > > > @@ -0,0 +1,44 @@ > > > +NXP iMX6SX/iMX7D Co-Processor Bindings > > > +---------------------------------------- > > > + > > > +This binding provides support for ARM Cortex M4 Co-processor found on > > > some > > > +NXP iMX SoCs. > > > + > > > +Required properties: > > > +- compatible Should be one of: > > > + "fsl,imx7d-rproc" > > > + "fsl,imx6sx-rproc" > > > +- clocks Clock for co-processor (See: > > > ../clock/clock-bindings.txt) > > > +- syscfg Phandle to syscon block which provide access to > > > > This is called "syscon" in the code and the example. > > done. > > > > + System Reset Controller > > > + > > > +Optional properties: > > > +- reg: Should contain the address ranges for some of > > > internal > > > + memory regions. > > > > Something less hand-wavy, like: "Should list the memory regions for the > > remoteproc" > > > > > +- reg-names: Contains the corresponding names for the memory > > > + regions. These should be named "ddr", "ocram", > > > "ocram-s", > > > + "ocram-epdc" or "ocram-pxp". > > > > Make this comment define which memory regions are required for each of > > the systems. > > done. > > > > +Fallowing memory ranges are expected: > > > +- For "fsl,imx7d-rproc" > > > + <0x00900000 0x00020000> - "ocram" > > > + <0x00920000 0x00020000> - "ocram-epdc" > > > + <0x00940000 0x00008000> - "ocram-pxp" > > > + <0x80000000 0x0FFF0000> - "ddr" (code area) > > > + <0x80000000 0x60000000> - "ddr" (data area) > > > > There's no reason to list the actual regions in the binding > > document. Just list the requires regions for each system. > > > > > +- For "fsl,imx6sx-rproc" > > > + <0x008F8000 0x00004000> - "ocram-s" > > > + <0x80000000 0x0FFF8000> - "ddr" (code area) > > > + <0x80000000 0x60000000> - "ddr" (data area) > > > + > > > +Note: the "ddr" code and data ranges are overlapping. Code area is > > > smaller > > > +than data area. So this range should be carefully chosen according to > > > your > > > +system and application requirements. > > > + > > > > This is a source of future issues as this indicates that I should have > > reg-names list "ddr" twice. > > Then I will name it: > "ddri" (incstruction/code area), > "ddrd" (data area) > > unless there are other suggestions. >
But are you saying that it's correct that these two memory regions should overlap? > > Also, as you warn about the user needing to pick the values for "ddr", > > does that mean that it's a carveout of System RAM? > > Yes, it is a part of System RAM. > Then there will be an associated reserved-memory region for the region(s), you should add a label to this and use "memory-region" to get hold of the addresses instead. Regards, Bjorn
