[PATCH v5 2/2] memory: ti-aemif: add bindings for AEMIF driver
Rob Herring
robherring2 at gmail.com
Thu Feb 20 08:44:48 EST 2014
On Thu, Feb 20, 2014 at 6:44 AM, Ivan Khoronzhuk <ivan.khoronzhuk at ti.com> wrote:
>
> On 02/19/2014 08:11 PM, Mark Rutland wrote:
>>
>> On Wed, Feb 19, 2014 at 01:40:10PM +0000, Ivan Khoronzhuk wrote:
>>>
>>> Add bindings for TI Async External Memory Interface (AEMIF) controller.
>>>
>>> The Async External Memory Interface (EMIF16/AEMIF) controller is intended
>>> to
>>> provide a glue-less interface to a variety of asynchronous memory devices
>>> like
>>> ASRA M, NOR and NAND memory. A total of 256M bytes of any of these
>>> memories
>>> can be accessed via 4 chip selects with 64M byte access per chip select.
>>>
>>> We are not encoding CS number in reg property, it's memory partition
>>> number.
>>> The CS number is encoded for Davinci NAND node using standalone property
>>> "ti,davinci-chipselect" and we need to provide two memory ranges to it,
>>> as result we can't encode CS number in "reg" for AEMIF child devices
>>> (NAND/NOR/etc), as it will break bindings compatibility.
>>>
>>> In this patch, NAND node is used just as an example of child node.
>>>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk at ti.com>
>>> ---
>>> .../bindings/memory-controllers/ti-aemif.txt | 210
>>> +++++++++++++++++++++
>>> 1 file changed, 210 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/memory-controllers/ti-aemif.txt
>>
>> [...]
>>
>>> +- ranges: Contains memory regions. There are two types of
>>> + ranges/partitions:
>>> + - CS-specific partition/range. If continuous,
>>> must be
>>> + set up to reflect the memory layout for 4
>>> chipselects,
>>> + if not then additional range/partition can be
>>> added and
>>> + child device can select the proper one.
>>> + - control partition which is common for all CS
>>> + interfaces.
>>
>> This really doesn't sound anything like the typical meaning of ranges on
>> a bus.
>>
>> Why isn't this information encoded in reg properties on the child nodes?
>>
>> [...]
>
>
> Note that we do not introduce NAND device bindings here.
> The Davinci NAND bindings was introduced and accepted more then one year
> ago.
> And the CS number is encoded for Davinci NAND node using standalone property
>
> "ti,davinci-chipselect" and we need to provide two memory ranges to it,
> as result we can't encode CS number in "reg" for AEMIF child devices
> (NAND/NOR/etc),
> as it will break bindings compatibility.
>
> In this document, NAND node is used just as an example of child node.
>
>
>>> +- ti,cs-ss: enable/disable select strobe mode
>>> + In select strobe mode chip select behaves
>>> as
>>> + the strobe and is active only during the
>>> strobe
>>> + period. If present then enable.
>>> +
>>> +- ti,cs-ew: enable/disable extended wait mode
>>> + if set, the controller monitors the
>>> EMIFWAIT pin
>>> + mapped to that chip select to determine
>>> if the
>>> + device wants to extend the strobe period.
>>> If
>>> + present then enable.
>>
>> When would you have these two in the DT and when wouldn't you? Why can't
>> the driver decide?
>
>
> These are properties of cs node, not the driver itself.
> The driver cannot know what child device you are going to attach to cs node
> , as result it cannot decide what settings you are using for particular cs
> node.
>
>
>> These names are also opaque. We can clearly do better.
>
>
> I propose the following names?:
>
> ti,cs-ss --> ti,cs-select-strobe-mode
> ti,cs-ew --> ti,cs-extended-waite-mode
>
> Are you OK with it?
>
>
>>> +
>>> +- ti,cs-ta: minimum turn around time, ns
>>> + Time between the end of one asynchronous
>>> memory
>>> + access and the start of another
>>> asynchronous
>>> + memory access. This delay is not incurred
>>> + between a read followed by read or a
>>> write
>>> + followed by a write to same chip select.
>>
>> The name is opaque. How about ti,min-turnaround-time-ns ?
>
>
> I like without -ns suffix and with cs- prefix:
> ti,cs-ta --> ti,cs-min-turnaround-time
>
> Is it OK?
>
>
>>> +
>>> +- ti,cs-rsetup: read setup width, ns
>>> + Time between the beginning of a memory
>>> cycle
>>> + and the activation of read strobe.
>>> + Minimum value is 1 (0 treated as 1).
>>> +
>>> +- ti,cs-rstobe: read strobe width, ns
>>> + Time between the activation and
>>> deactivation of
>>> + the read strobe.
>>> + Minimum value is 1 (0 treated as 1).
>>> +
>>> +- ti,cs-rhold: read hold width, ns
>>> + Time between the deactivation of the read
>>> + strobe and the end of the cycle (which
>>> may be
>>> + either an address change or the
>>> deactivation of
>>> + the chip select signal.
>>> + Minimum value is 1 (0 treated as 1).
>>> +
>>> +- ti,cs-wsetup: write setup width, ns
>>> + Time between the beginning of a memory
>>> cycle
>>> + and the activation of write strobe.
>>> + Minimum value is 1 (0 treated as 1).
>>> +
>>> +- ti,cs-wstrobe: write strobe width, ns
>>> + Time between the activation and
>>> deactivation of
>>> + the write strobe.
>>> + Minimum value is 1 (0 treated as 1).
>>> +
>>> +- ti,cs-whold: write hold width, ns
>>> + Time between the deactivation of the
>>> write
>>> + strobe and the end of the cycle (which
>>> may be
>>> + either an address change or the
>>> deactivation of
>>> + the chip select signal.
>>> + Minimum value is 1 (0 treated as 1).
>>
>> Likewise I think these can be given more descriptive names.
>
>
> Ok. How about:
>
> ti,cs-rsetup --> ti,cs-read-setup-time
> ti,cs-rstobe --> ti,cs-read-strobe-time
> ti,cs-rhold --> ti,cs-read-hold-time
> ti,cs-wsetup --> ti,cs-write-setup-time
> ti,cs-wstrobe --> ti,cs-write-strobe-time
> ti,cs-whold --> ti,cs-write-hold-time
Properties that have a unit of measure should have that unit in the
name. So replace "time" with "ns".
Rob
More information about the linux-arm-kernel
mailing list