From: m.szyprowski@samsung•com (Marek Szyprowski)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v7 3/4] drivers: of: add initialization code for dma reserved memory
Date: Mon, 16 Sep 2013 10:17:25 +0200 [thread overview]
Message-ID: <5236BE95.3030305@samsung.com> (raw)
In-Reply-To: <5236AF64.80607@samsung.com>
On 9/16/2013 9:12 AM, Marek Szyprowski wrote:
> Hello,
>
> On 9/9/2013 6:01 PM, Grant Likely wrote:
>> On Fri, 30 Aug 2013 15:26:24 -0500, Kumar Gala <galak@codeaurora•org>
>> wrote:
>> > On Aug 30, 2013, at 7:39 AM, Marek Szyprowski wrote:
>> > > On 8/30/2013 12:46 AM, Grant Likely wrote:
>> > >> On Mon, 26 Aug 2013 16:39:18 +0200, Marek Szyprowski
>> <m.szyprowski@samsung•com> wrote:
>> > >> > + - "reserved-memory-region" - compatibility is defined, given
>> > >> > + region is assigned for exclusive usage for by the
>> respective
>> > >> > + devices.
>> > >>
>> > >> BTW, just so you're aware there is already a binding for marking
>> regions
>> > >> as reserved. It was recently added to arch/powerpc/kernel/prom.c.
>> > >> Unfortunately it doesn't look like it got documented. Search for
>> > >> "reserved-ranges". However, I don't think it blocks your work
>> here. That
>> > >> binding doesn't provide any way for matching devices to reserved
>> ranges.
>> > >> It is only for telling the kernel "hands of that memory".
>> > >
>> > > ok, good to know.
>> >
>> > So I think the most generic compatible should effectively be the same
>> > as 'reserved-ranges', ie this region shouldn't be touched by the
>> > kernel.
>> >
>> > Than we can build on top of that with more specific compatibles.
>> >
>> > I have examples from FSL networking SoCs where we would carve out
>> > memory for backing storage managed completely by the HW and Linux
>> > wouldn't need to touch it, however we might have a
>> > "fsl,qman-backing-store" compat encase we might want some debug code
>> > in linux to look at the region of memory.
>> >
>> > I've got examples at qualcomm where we have shared data structures in
>> > specific memory regions between different processors that aren't cache
>> > coherent, so want the memory not to be marked as cacheable by Linux
>> > when it accesses it. Again we'd have a "qcom,smem" prop on top of the
>> > "reserved-memory-region".
>>
>> I think that is a reasonable approach, and works really well for regions
>> associated with specific hardware because the hardware driver can be
>> responsible for wiring it up to CMA or manage the region directly.
>> Whatever the driver desires to do.
>>
>> It still remains what to do for generic regions that any device can use.
>> As I've previously said, I don't like calling out "CMA" specifically in
>> the compatible property because that happens to be a Linux
>> implementation specific details. Two years from now someone may propose
>> a new allocator that should take over what used to be handled by CMA
>> (kind of like how CMA may end up taking over from ION)....
>>
>> Alright, I've thought about it some more and it probably does make sense
>> to use an additional compatible string. Marek originally suggested
>> "linux,contiguous-memory-region". I later suggested
>> "shareable-memory-region",
>> but that's actually a worse name because it doesn't have any reference
>> to what the region is for (I was fixating on the kernel being able to
>> use unallocated pages; but that is also an implementation detail). I
>> exercise my right to change my mind and agree that contiguous is
>> appropriate here. But instead of calling out the contiguous allocator,
>> the binding should specifically lay out the usage model for these
>> regions. I would change the contiguous-memory binding to state the
>> following:
>> Contiguous-memory is a region of general purpose memory from
>> which large buffers of contiguous memory can be allocated.
>> Contiguous buffers are often used for DMA buffers. The operating
>> system may use the memory for any purpose, but must immediately
>> release the pages if a contiguous buffer is required.
>>
>> So the way I read things, the current proposal would be:
>>
>> The generic form for do-not-touch memory:
>> compatible = "reserved-memory";
>> - I've dropped '-region' because I think it is redundant
>> - The kernel will not make use of this memory except for when a
>> driver picks it up
>>
>> For contiguous memory:
>> compatible = "contiguous-memory", "reserved-memory"
>>
>> For contiguous memory that Linux will use by default if a specific
>> region isn't specified.
>> compatible = "contiguous-memory", "reserved-memory"
>> linux,default-contiguous-region;
>> - I stuck with a linux, prefix here because I haven't come up with a
>> non-linux way to describe this.
>>
>> Memory that specific hardware can pick up:
>> compatible = "qcom,smem", "reserved-memory"
>>
>> Nodes with a 'size' property and without a 'reg' property need to have a
>> region allocated and reserved. The allocated region needs to be cached
>> somewhere. We could create a data structure for tracking the reserved
>> regions, or could write it in directly. While I shudder at the
>> thought of
>> modifying the device tree at runtime by the kernel, it might just be the
>> sanest implementation at this time. I'd like to see what the
>> implementation looks like. (Since this is potentially controversial, I
>> would recommend implementing the exact regions in on patch, and add
>> dynamically allocated regions as a follow-up patch)
>>
>> As far as proper implementation is concerned, I think it should be
>> explicit in the binding that the kernel should automatically exclude
>> anything under the reserved-memory node, regardless of the compatible
>> value. Merely being a child of the reserved-memory node immediately
>> means that it is a reserved memory region.
>
> It looks that my proposal for the reserved memory nodes causes much more
> controversy than I expected when I've sent a pull request to Linus:
> https://lkml.org/lkml/2013/9/15/151
> http://www.spinics.net/lists/arm-kernel/msg273548.html
>
> Fixing those issues requires further discussion. Frankly, right now I
> really have no idea which way should I go. The /reserved-ranges node
> seems
> to be easy to match particular reserved memory region with a given
> device.
Huh, I've hurried to much. It should be:
The /reserved-ranges node approach seems to be easy to implement, but it
makes much harder to to match particular reserved memory region with a given
device.
> I'm also not really convinced if it makes sense to add a code for finding
> and matching a reserved memory region to every device driver, which might
> need it. I would really like to get some more feedback on the Ben's
> comment.
>
> In any case, the code will also change significantly, so I assume that
> the
> best, what can be done now is to revert the current version and start
> from
> the scratch with a new, complete proposal.
Best regards
--
Marek Szyprowski
Samsung R&D Institute Poland
next prev parent reply other threads:[~2013-09-16 8:17 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-26 14:39 [PATCH v7 0/4] Device Tree support for CMA (Contiguous Memory Allocator) Marek Szyprowski
2013-08-26 14:39 ` [PATCH v7 1/4] drivers: dma-contiguous: clean source code and prepare for device tree Marek Szyprowski
2013-08-26 14:39 ` [PATCH v7 2/4] drivers: of: add function to scan fdt nodes given by path Marek Szyprowski
2013-08-29 21:40 ` Grant Likely
2013-08-30 10:42 ` Marek Szyprowski
2013-08-30 10:46 ` Grant Likely
2013-08-26 14:39 ` [PATCH v7 3/4] drivers: of: add initialization code for dma reserved memory Marek Szyprowski
2013-08-29 22:46 ` Grant Likely
2013-08-30 12:39 ` Marek Szyprowski
2013-08-30 20:26 ` Kumar Gala
2013-09-09 16:01 ` Grant Likely
2013-09-10 19:53 ` Kumar Gala
2013-09-15 12:48 ` Grant Likely
2013-09-12 18:22 ` Kumar Gala
2013-09-15 12:50 ` Grant Likely
2013-09-16 7:12 ` Marek Szyprowski
2013-09-16 7:25 ` Benjamin Herrenschmidt
2013-09-16 13:43 ` Grant Likely
2013-09-18 3:48 ` Grant Likely
2013-09-18 11:07 ` Marek Szyprowski
2013-09-16 8:17 ` Marek Szyprowski [this message]
2013-09-09 13:05 ` Grant Likely
2013-08-29 22:48 ` Grant Likely
2013-09-27 15:47 ` Kumar Gala
2013-09-27 17:06 ` Matt Sealey
2013-08-26 14:39 ` [PATCH v7 4/4] ARM: init: add support for reserved memory defined by device tree Marek Szyprowski
2013-08-29 22:49 ` Grant Likely
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=5236BE95.3030305@samsung.com \
--to=m.szyprowski@samsung$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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