From: robherring2@gmail.com (Rob Herring)
To: linux-arm-kernel@lists.infradead.org
Subject: 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@hornet.cambridge.arm.com>
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 at 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?
>
>
WARNING: multiple messages have this Message-ID (diff)
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
next prev parent reply other threads:[~2011-09-20 14:37 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-20 11:51 DT vs ARM static mappings Pawel Moll
2011-09-20 11:51 ` Pawel Moll
2011-09-20 12:58 ` Rob Herring
2011-09-20 12:58 ` Rob Herring
2011-09-20 14:02 ` Pawel Moll
2011-09-20 14:02 ` Pawel Moll
2011-09-20 14:37 ` Rob Herring [this message]
2011-09-20 14:37 ` Rob Herring
2011-09-20 16:16 ` Pawel Moll
2011-09-20 16:16 ` Pawel Moll
2011-09-20 19:28 ` Arnd Bergmann
2011-09-20 19:28 ` Arnd Bergmann
2011-09-21 9:41 ` Pawel Moll
2011-09-21 9:41 ` Pawel Moll
2011-09-21 9:59 ` Dave Martin
2011-09-21 9:59 ` Dave Martin
2011-09-21 10:02 ` Pawel Moll
2011-09-21 10:02 ` Pawel Moll
2011-09-22 16:23 ` Pawel Moll
2011-09-22 16:23 ` Pawel Moll
2011-09-23 15:45 ` Arnd Bergmann
2011-09-23 15:45 ` Arnd Bergmann
2011-09-21 17:49 ` Nicolas Pitre
2011-09-21 17:49 ` Nicolas Pitre
2011-09-22 13:04 ` Pawel Moll
2011-09-22 13:04 ` Pawel Moll
2011-09-22 13:13 ` Russell King - ARM Linux
2011-09-22 13:13 ` Russell King - ARM Linux
2011-09-22 13:45 ` Pawel Moll
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@gmail.com \
--cc=linux-arm-kernel@lists.infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.