Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1694462

Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver

From Oleksij Rempel <linux@rempel-privat.de>
Newsgroups linux.kernel
Subject Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
Date 2017-07-24 08:00 +0200
Message-ID <u6Ezn-Jj-1@gated-at.bofh.it> (permalink)
References <u1IaC-6eJ-17@gated-at.bofh.it> <u1IaC-6eJ-23@gated-at.bofh.it> <u1Pc5-2m4-5@gated-at.bofh.it> <u4wmB-51-11@gated-at.bofh.it> <u4DHs-4J6-17@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


[Multipart message — attachments visible in raw view] - view raw

Am 18.07.2017 um 18:38 schrieb Bjorn Andersson:
> 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 <o.rempel@pengutronix.de>
>>>> ---
>>>>  .../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?

Yes, from Cortex-m4 the same memory regions are aliased to different
address ranges to provide different cache optimizations.
From Cortex-A7 side - not. But on this side we don't care about meaning
of it, it is just some memory region.


>>> 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.

Ok. Should only system memory region be assigned over reserved-memory or
all of named regions?

-- 
Regards,
Oleksij

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D  Remote Processor Controller driver Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-07-18 18:40 +0200
  Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D  Remote Processor Controller driver Oleksij Rempel <linux@rempel-privat.de> - 2017-07-24 08:00 +0200
    Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D  Remote Processor Controller driver Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-07-26 19:20 +0200

csiph-web