devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: "devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org"
	<devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Subject: Re: DT vs ARM static mappings
Date: Tue, 20 Sep 2011 09:37:58 -0500	[thread overview]
Message-ID: <4E78A546.9040604@gmail.com> (raw)
In-Reply-To: <1316527365.4611.354.camel-okZbbLrgpR/YkXV2EHHjLW3o5bpOHsLO@public.gmane.org>

Pawel,

On 09/20/2011 09:02 AM, Pawel Moll wrote:
>>> Of course the simplest solution would be to define two different
>>> compatible values, eg. "arm,vexpress-legacy" would execute the current
>>> map_io implementation, while "arm,vexpress-rs1" would use different one,
>>> setting up the other map_desc (the MMIO_P2V macro must die of course,
>>> replaced with a runtime-defined virtual base address for the
>>> peripherals).
>>>
>>> If you believe that's what I should do, say it and stop reading :-)
>>
>> Yes. Different tiles are fundamentally different boards, so they should
>> have different DTs. Using includes should help minimize duplication though.
> 
> You've misunderstood me or (most likely ;-) probably I wasn't clear
> enough.
> 
> There is no doubt the DTs will be different across the "portfolio".
> 
> We already have (patches soon) vexpress-v2p-ca9.dts that includes
> vexpress-v2m-legacy.dtsi.
> 
> A5 will be vexpress-v2p-ca5p.dts+vexpress-v2m-rs1.dtsi, A15
> vexpress-v2p-ca15.dts+vexpress-v2m-rs1.dtsi (notice that the A5/A15 are
> sharing the v2m bit, as the motherboard is common).
> 
> My point is that we should be able to handle _all_ of them using one
> DT_MACHINE_START with a single compat value "arm,vexpress". The only
> problem with this (so far) is the mapping.
> 

Yes, you should have 1 DT_MACHINE_START, but arm,vexpress is too
generic. You can and should have a list of compatible strings for each
board/machine.

>> Think about it this way. How would you solve this without DT? You
>> would have a bunch of duplicated data in the kernel for the different
>> configs. So you're not any worse off in this regard and still have the
>> other advantages of DT.
> 
> Exactly my point :-) I want to have as little duplication as possible.
> And the static mapping issue is in the way.
> 
>>> To my mind it looked like the whole mechanism was not flexible enough,
>>> so I wanted to explore other options...
>>>
>>> The obvious one was to describe the required static mapping in the DTS.
>>> I don't like this idea, though. It can hardly be called "hardware
>>> description". Besides, what node would carry such data? "chosen"?
>>> Hardly...
>>>
>>> Would it contain a "regs" property with the physical address and
>>> "virtual-reg" with the virtual one? Again, doesn't sound right to me
>>> (especially the virtual bit, however the virtual address could be common
>>> between different variants and be defined in the board support code, not
>>> the DTS).
>>>
>>> I have considered a reference (phandle or an alias?) to the node to be
>>> mapped ("peripherals" in my case), but where to define this reference?
>>> Any ideas?
>>
>> In "chosen" like the kernel command line would be the place, but I don't
>> think that is the right approach. Chosen is really for things that
>> change frequently and this doesn't really fall in that category.
> 
> Again, no argument from me here :-)
> 
> The question is - where should it be?
> 

Nowhere. It's an OS specific issue, not a h/w issue.

>>> There is an additional problem here... The "map_io" is executed before
>>> the tree is un-flattened, so:
>>>
>>> 1. One can't simply use "of_find_matching_node()" (as in the latest l2x0
>>> patches) to find the interesting nodes - the only way of going through
>>> the tree is raw of_scan_flat_dt() function. Therefore any conditions
>>> more complex then string comparison with the (full) node name are
>>> problematic.
>>>
>>> 2. The tree mappings (ranges) are not resolved yet, so one can't simply
>>> get the effective address of a node. Only "raw" properties are
>>> available, so all one can get scanning for "peripherals@7" node is "0 7
>>> 0 0x20000" array, instead of the "0x10000000 0x00020000" that is really
>>> important. 
>>
>> If you add a compatible field to "motherboard" node, then you can read
>> the ranges.
> 
> ... and then and then scan for the sysregs, and add the offset and base
> together... Sounds to me like duplication of the of_translate_*()?
> 
>>> Initially I wanted to find the mentioned devices and create individual
>>> mappings for them, so the MMIO_P2V would be still valid (if slightly
>>> "abused"), but I failed due to the problems mentioned above. And I can't
>>> delay this operation till the tree is un-flattened, as the core tile
>>> must be probed (via sysreg) in map_io (tile's specific code must be able
>>> to create its own mappings):
>>
>> Do you really need MMIO_P2V? If you have fixed virtual addresses in the
>> kernel and can pull the phys addresses from DT to populate the iotable,
>> is that sufficient?
> 
> For the third time, 100% agree :-) Well, 90%.
> 
> What I need is:
> 
> 1. Get the phys address from DT. But how? This is getting as back to my
> complaints about still-flat tree and ranges, the node to be used to
> describe the mapping.
> 
> 2. The offset inside the mapping will be different (for sysregs it will
> be 0 for old mapping, 0x10000 for the new one), so I have to work it out
> from the tree as well. And as we are in map_io, the tree is still flat
> and... read 1 :-)

So create a mapping per peripheral rather than per chip select. Then the
virtual address can always be the same.

> 
>> Generally, the trend is to get rid of static mappings as much as
>> possible. Doing that first might simplify things.
> 
> You can't do ioremap() before kmalloc() is up and running (correct me if
> I am wrong), at least you can't do this in map_io. So the static mapping
> is a must sometimes. And actually, with the latest Nico's changes:
> 
Correct. You can't do ioremap until init_irq. map_io and init_early are
too early. My point was if you can delay h/w access then you can remove
the static mappings. But yes, we generally can't remove them all. SCU
and LL debug uart are 2 examples.

For the short term, I would just have 2 static iotables and select the
right one based on the board's (or motherboard's) compatible string.

Long term, we should look into implementing a common early DT address
parsing function.

Rob

> http://thread.gmane.org/gmane.linux.ports.arm.kernel/132762
> 
> it may even be preferred for peripherals (one mapping shared across all
> users).
> 
> Cheers!
> 
> Paweł
> 
> 

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

  parent reply	other threads:[~2011-09-20 14:37 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-20 11:51 DT vs ARM static mappings Pawel Moll
     [not found] ` <1316519479.4611.150.camel-okZbbLrgpR/YkXV2EHHjLW3o5bpOHsLO@public.gmane.org>
2011-09-20 12:58   ` Rob Herring
2011-09-20 14:02     ` Pawel Moll
     [not found]       ` <1316527365.4611.354.camel-okZbbLrgpR/YkXV2EHHjLW3o5bpOHsLO@public.gmane.org>
2011-09-20 14:37         ` Rob Herring [this message]
2011-09-20 16:16           ` Pawel Moll
     [not found]             ` <1316535403.4611.534.camel-okZbbLrgpR/YkXV2EHHjLW3o5bpOHsLO@public.gmane.org>
2011-09-20 19:28               ` Arnd Bergmann
2011-09-21  9:41                 ` Pawel Moll
     [not found]                   ` <1316598109.4611.613.camel-okZbbLrgpR/YkXV2EHHjLW3o5bpOHsLO@public.gmane.org>
2011-09-21  9:59                     ` Dave Martin
2011-09-21 10:02                       ` Pawel Moll
2011-09-22 16:23                 ` Pawel Moll
     [not found]                   ` <1316708611.4611.873.camel-okZbbLrgpR/YkXV2EHHjLW3o5bpOHsLO@public.gmane.org>
2011-09-23 15:45                     ` Arnd Bergmann
2011-09-21 17:49   ` Nicolas Pitre
2011-09-22 13:04     ` Pawel Moll
     [not found]       ` <1316696696.4611.844.camel-okZbbLrgpR/YkXV2EHHjLW3o5bpOHsLO@public.gmane.org>
2011-09-22 13:13         ` Russell King - ARM Linux
2011-09-22 13:45           ` Pawel Moll
     [not found]             ` <1316699153.4611.858.camel-okZbbLrgpR/YkXV2EHHjLW3o5bpOHsLO@public.gmane.org>
2011-09-22 13:59               ` Russell King - ARM Linux

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E78A546.9040604@gmail.com \
    --to=robherring2-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).