Re: [RFC PATCH] Consolidate SRAM support
* Tomi Valkeinen [110419 17:13]: > Hi Tony and Russell, > > On Mon, 2011-04-18 at 11:17 +0300, Tony Lindgren wrote: > > * Tomi Valkeinen [110418 09:57]: > > > > So, I can make a patch that removes the SRAM support from omapfb, and > > > queue it up for the next merge window. > > > > OK. That patch should probably go into Russell's tree along with > > other SRAM related patches to avoid conflicts. > > Here's a simple patch to remove SRAM support from omapfb. Or more > precisely, this just disables it. > > I started to remove the SRAM support from the drivers, but the patches > got quite big and will probably cause conflicts if they reside in > somebody else's tree than mine. So I believe it's easiest to divide SRAM > FB removal into this patch, handled by Russell (?), and then the rest, > handled by me. OK sounds like a plan. > Is there something else related to OMAP FB than the code removed in this > patch that is hindering the SRAM work? I think we can now also remove sram_ceil and omap_sram_push_address? Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
Hi Tony and Russell, On Mon, 2011-04-18 at 11:17 +0300, Tony Lindgren wrote: > * Tomi Valkeinen [110418 09:57]: > > So, I can make a patch that removes the SRAM support from omapfb, and > > queue it up for the next merge window. > > OK. That patch should probably go into Russell's tree along with > other SRAM related patches to avoid conflicts. Here's a simple patch to remove SRAM support from omapfb. Or more precisely, this just disables it. I started to remove the SRAM support from the drivers, but the patches got quite big and will probably cause conflicts if they reside in somebody else's tree than mine. So I believe it's easiest to divide SRAM FB removal into this patch, handled by Russell (?), and then the rest, handled by me. Is there something else related to OMAP FB than the code removed in this patch that is hindering the SRAM work? Tomi >From 8a32eb1762d9ecead1aee08cbe955748d67999fd Mon Sep 17 00:00:00 2001 From: Tomi Valkeinen Date: Tue, 19 Apr 2011 16:58:51 +0300 Subject: [PATCH] OMAP: Remove reserve_sram calls to omapfb This patch removes calls from arch/arm/plat-omap/sram.c to omapfb_reserve_sram() and omap_vram_reserve_sram(), effectively disabling SRAM support for the old and new display drivers. SRAM code in the display drivers will be removed in separate patches. Signed-off-by: Tomi Valkeinen --- arch/arm/plat-omap/sram.c | 13 - 1 files changed, 0 insertions(+), 13 deletions(-) diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c index a3f50b3..e10eeed 100644 --- a/arch/arm/plat-omap/sram.c +++ b/arch/arm/plat-omap/sram.c @@ -19,7 +19,6 @@ #include #include #include -#include #include #include @@ -32,7 +31,6 @@ #include #include "sram.h" -#include "fb.h" /* XXX These "sideways" includes are a sign that something is wrong */ #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3) @@ -170,17 +168,6 @@ static void __init omap_detect_sram(void) omap_sram_size = 0x4000; } } - reserved = omapfb_reserve_sram(omap_sram_start, omap_sram_base, - omap_sram_size, - omap_sram_start + SRAM_BOOTLOADER_SZ, - omap_sram_size - SRAM_BOOTLOADER_SZ); - omap_sram_size -= reserved; - - reserved = omap_vram_reserve_sram(omap_sram_start, omap_sram_base, - omap_sram_size, - omap_sram_start + SRAM_BOOTLOADER_SZ, - omap_sram_size - SRAM_BOOTLOADER_SZ); - omap_sram_size -= reserved; omap_sram_ceil = omap_sram_base + omap_sram_size; } -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
2011/4/15 Russell King - ARM Linux : > We have two SoCs using SRAM, both with their own allocation systems, > and both with their own ways of copying functions into the SRAM. > > Let's unify this before we have additional SoCs re-implementing this > obviously common functionality themselves. Great initiative! I'm thinking about how to unify this with the bits I have in kernel/tcm.c. For TCM memory (cacheless, inside CPU core, controlled by CP15 registers to recap) we can compile plain C into that memory too, which seems like a lot of these SRAM drivers actually want to do, but fail at and instead write some assembler and copy it there. The remainder of the TCM memory is put into an allocation pool of uneven size, maybe that should reuse this mechanism instead of rolling its own atleast. On the odd PB11MPCore the TCM memories are NUMA, so one on each CPU not reachable from other cores (like Magnus Damm mentioned for SH-ARM) and I have no idea on how to handle that, neither for an allocator nor for compiling code directly into it. Suggestions welcome. Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
* Tomi Valkeinen [110418 09:57]: > On Mon, 2011-04-18 at 09:48 +0300, Tony Lindgren wrote: > > * Russell King - ARM Linux [110416 16:06]: > > > On Sat, Apr 16, 2011 at 09:01:26PM +0800, Haojian Zhuang wrote: > > > > On Fri, Apr 15, 2011 at 9:06 PM, Russell King - ARM Linux > > > > > > > > > > This uses the physical address, and unlike Davinci's dma address > > > > > usage, > > > > > it always wants to have the physical address, and will always return > > > > > the corresponding physical address when passed that pointer. > > > > > > > > > > OMAP could probably do with some more work to make the omapfb and > > > > > other > > > > > allocations use the sram allocator, rather than hooking in before the > > > > > sram allocator is initialized - and then further cleanups so that we > > > > > have an initialization function which just does > > > > I think we can just remove the omapfb SRAM support for now as it should > > be optional. That will then leave out that dependency and can be added > > back once we have a common framework in place. > > > > Tomi do you see any problems with that? > > I agree. Only OMAP2 has enough SRAM to possibly have a framebuffer > there, and even on OMAP2 the SRAM size is so small that it works only > for rather small displays. I'm not aware of anyone using omapfb's SRAM > support. > > The SRAM support also makes our video ram allocator and omapfb more > complex, without much benefit, so I'm more than happy to remove it > totally. Additionally, I believe there is work going on with memory > allocators that omapfb could also use, and migrating to any new mem > allocator will no doubt be much easier if we just handle normal RAM. > > So, I can make a patch that removes the SRAM support from omapfb, and > queue it up for the next merge window. OK. That patch should probably go into Russell's tree along with other SRAM related patches to avoid conflicts. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Mon, 2011-04-18 at 09:48 +0300, Tony Lindgren wrote: > * Russell King - ARM Linux [110416 16:06]: > > On Sat, Apr 16, 2011 at 09:01:26PM +0800, Haojian Zhuang wrote: > > > On Fri, Apr 15, 2011 at 9:06 PM, Russell King - ARM Linux > > > > > > > > This uses the physical address, and unlike Davinci's dma address usage, > > > > it always wants to have the physical address, and will always return > > > > the corresponding physical address when passed that pointer. > > > > > > > > OMAP could probably do with some more work to make the omapfb and other > > > > allocations use the sram allocator, rather than hooking in before the > > > > sram allocator is initialized - and then further cleanups so that we > > > > have an initialization function which just does > > I think we can just remove the omapfb SRAM support for now as it should > be optional. That will then leave out that dependency and can be added > back once we have a common framework in place. > > Tomi do you see any problems with that? I agree. Only OMAP2 has enough SRAM to possibly have a framebuffer there, and even on OMAP2 the SRAM size is so small that it works only for rather small displays. I'm not aware of anyone using omapfb's SRAM support. The SRAM support also makes our video ram allocator and omapfb more complex, without much benefit, so I'm more than happy to remove it totally. Additionally, I believe there is work going on with memory allocators that omapfb could also use, and migrating to any new mem allocator will no doubt be much easier if we just handle normal RAM. So, I can make a patch that removes the SRAM support from omapfb, and queue it up for the next merge window. Tomi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
* Russell King - ARM Linux [110416 16:06]: > On Sat, Apr 16, 2011 at 09:01:26PM +0800, Haojian Zhuang wrote: > > On Fri, Apr 15, 2011 at 9:06 PM, Russell King - ARM Linux > > > > > > This uses the physical address, and unlike Davinci's dma address usage, > > > it always wants to have the physical address, and will always return > > > the corresponding physical address when passed that pointer. > > > > > > OMAP could probably do with some more work to make the omapfb and other > > > allocations use the sram allocator, rather than hooking in before the > > > sram allocator is initialized - and then further cleanups so that we > > > have an initialization function which just does I think we can just remove the omapfb SRAM support for now as it should be optional. That will then leave out that dependency and can be added back once we have a common framework in place. Tomi do you see any problems with that? > > TCM driver can allocate code into SRAM section in link stage. It needs to > > update link file and virtual memory layout. Is it worth to make SRAM driver > > support this behavior? The case of using SRAM as memory for instruction > > is switching frequency or entering/exiting low power mode in PXA silicons. > > This is more or less the same scenario which OMAP is using its SRAM > for, although that's explicitly SRAM and not TCM. > > I don't think we need position dependent code requiring fixup for > these applications as such things tend to be fairly easily coded in a > position independent way. There should not be any need code requiring fixup in most cases using parameters passed to the functions should be enough. Usually it's just register addresses that are different. > Also note that C functions can't be copied because C produces position > dependent code, and we have no way of extracting the relocation > dependencies from such code. C functions should not be needed. The SRAM size is small and typical usage is a loop waiting for some register to lock. For any larger SRAM areas NUMA can be used instead. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Friday 15 April 2011, Russell King - ARM Linux wrote: > On Fri, Apr 15, 2011 at 09:32:01AM -0600, Grant Likely wrote: > > Yes, once the infrastructure is in place, powerpc can do its own > > migration to the new code. I vote for putting it in lib at the > > outset. > > I don't agree with stuffing non-arch directories with code which people > haven't already agreed should be shared. As I've already said, in my > experience it's hard to get agreement in the first place and even when > you can the API generally needs to be changed from what you first think > would be reasonable. I believe we should be much more aggressive about putting code into generic locations, especially when there is nothing hardware specific to it. As we can see from all the mess regarding interrupt controllers, PCI busses or dma mapping, most of the smaller architectures get it wrong when they have to deal with these things, so when we have a common implementation, it's much easier to make sure we only have the bugs once and don't get incompatible interfaces. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Sat, Apr 16, 2011 at 09:01:26PM +0800, Haojian Zhuang wrote: > On Fri, Apr 15, 2011 at 9:06 PM, Russell King - ARM Linux > wrote: > > This is work in progress. > > > > We have two SoCs using SRAM, both with their own allocation systems, > > and both with their own ways of copying functions into the SRAM. > > > > Let's unify this before we have additional SoCs re-implementing this > > obviously common functionality themselves. > > > > Unfortunately, we end up with code growth through doing this, but that > > will become a win when we have another SoC using this (which I know > > there's at least one in the pipeline). > > > > One of the considerations here is that we can easily convert sram-pool.c > > to hook into device tree stuff, which can tell the sram allocator: > > - physical address of sram > > - size of sram > > - allocation granularity > > and then we just need to ensure that it is appropriately mapped. > > > > This uses the physical address, and unlike Davinci's dma address usage, > > it always wants to have the physical address, and will always return > > the corresponding physical address when passed that pointer. > > > > OMAP could probably do with some more work to make the omapfb and other > > allocations use the sram allocator, rather than hooking in before the > > sram allocator is initialized - and then further cleanups so that we > > have an initialization function which just does > > > > sram_create(phys, size) > > virt = map sram(phys, size) > > create sram pool(virt, phys, size, min_alloc_order) > > > > Another question is whether we should allow multiple SRAM pools or not - > > this code does allow multiple pools, but so far we only have one pool > > per SoC. Overdesign? Maybe, but it prevents SoCs wanting to duplicate > > it if they want to partition the SRAM, or have peripheral-local SRAMs. > > > > Lastly, uio_pruss should probably take the SRAM pool pointer via > > platform data so that it doesn't have to include Davinci specific > > includes. > > > > Signed-off-by: Russell King > > This common sram driver is good for us. It can benefit us on DMA usage. > I just have one question on SRAM for storing instruction. We still need to > copy code into SRAM and flush cache & TLB with this SRAM driver. There is the fncpy API for copying code to other regions of memory, including SRAM. The fncpy API is explicitly designed to cope with Thumb 2 and provide an appropriate function pointer for such code. Such code needs to be position independent to cope with multiple SRAM users, and also possible variability of SRAM mapping which may (eventually) exist when DT comes along. > TCM driver can allocate code into SRAM section in link stage. It needs to > update link file and virtual memory layout. Is it worth to make SRAM driver > support this behavior? The case of using SRAM as memory for instruction > is switching frequency or entering/exiting low power mode in PXA silicons. This is more or less the same scenario which OMAP is using its SRAM for, although that's explicitly SRAM and not TCM. I don't think we need position dependent code requiring fixup for these applications as such things tend to be fairly easily coded in a position independent way. Also note that C functions can't be copied because C produces position dependent code, and we have no way of extracting the relocation dependencies from such code. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 9:06 PM, Russell King - ARM Linux wrote: > This is work in progress. > > We have two SoCs using SRAM, both with their own allocation systems, > and both with their own ways of copying functions into the SRAM. > > Let's unify this before we have additional SoCs re-implementing this > obviously common functionality themselves. > > Unfortunately, we end up with code growth through doing this, but that > will become a win when we have another SoC using this (which I know > there's at least one in the pipeline). > > One of the considerations here is that we can easily convert sram-pool.c > to hook into device tree stuff, which can tell the sram allocator: > - physical address of sram > - size of sram > - allocation granularity > and then we just need to ensure that it is appropriately mapped. > > This uses the physical address, and unlike Davinci's dma address usage, > it always wants to have the physical address, and will always return > the corresponding physical address when passed that pointer. > > OMAP could probably do with some more work to make the omapfb and other > allocations use the sram allocator, rather than hooking in before the > sram allocator is initialized - and then further cleanups so that we > have an initialization function which just does > > sram_create(phys, size) > virt = map sram(phys, size) > create sram pool(virt, phys, size, min_alloc_order) > > Another question is whether we should allow multiple SRAM pools or not - > this code does allow multiple pools, but so far we only have one pool > per SoC. Overdesign? Maybe, but it prevents SoCs wanting to duplicate > it if they want to partition the SRAM, or have peripheral-local SRAMs. > > Lastly, uio_pruss should probably take the SRAM pool pointer via > platform data so that it doesn't have to include Davinci specific > includes. > > Signed-off-by: Russell King This common sram driver is good for us. It can benefit us on DMA usage. I just have one question on SRAM for storing instruction. We still need to copy code into SRAM and flush cache & TLB with this SRAM driver. TCM driver can allocate code into SRAM section in link stage. It needs to update link file and virtual memory layout. Is it worth to make SRAM driver support this behavior? The case of using SRAM as memory for instruction is switching frequency or entering/exiting low power mode in PXA silicons. Thanks Haojian -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
Hi Russell, [CC Paul Mundt] On Sat, Apr 16, 2011 at 12:41 AM, Russell King - ARM Linux wrote: > On Fri, Apr 15, 2011 at 09:32:01AM -0600, Grant Likely wrote: >> Yes, once the infrastructure is in place, powerpc can do its own >> migration to the new code. I vote for putting it in lib at the >> outset. > > I don't agree with stuffing non-arch directories with code which people > haven't already agreed should be shared. As I've already said, in my > experience it's hard to get agreement in the first place and even when > you can the API generally needs to be changed from what you first think > would be reasonable. > > So, lets wait to see whether the SH folk reply. The SH arch is using NUMA for on-chip SRAM on some CPUs, but for SH-Mobile ARM we have no software support at this point. The SH/ARM hardware has a bunch of different on-chip memories in place, and they all have individual power management support through both clock gating and power domains. I assume other vendors have similar setups. I'd like to have some refcounting in place for the SRAM code if possible, which would trickle down to runtime pm get/put which in turn would allow us to control the power dynamically. Not sure how easy that would be to accomplish though. Paul may have some ideas on how to share the code between ARM and SH. Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 09:19:25PM +0100, Russell King - ARM Linux wrote: > On Fri, Apr 15, 2011 at 10:11:07PM +0200, Uwe Kleine-König wrote: > > On Fri, Apr 15, 2011 at 02:06:07PM +0100, Russell King - ARM Linux wrote: > > > This is work in progress. > > > > > > We have two SoCs using SRAM, both with their own allocation systems, > > > and both with their own ways of copying functions into the SRAM. > > I havn't checked the details, but maybe the code in > > arch/arm/plat-mxc/iram_alloc.c could be migrated to your approach, too? > > Its already in there now that I have replies from Nguyen Dinh. It > looks like this presently: > > arch/arm/plat-mxc/Kconfig |2 +- > arch/arm/plat-mxc/include/mach/iram.h | 24 +++-- > arch/arm/plat-mxc/iram_alloc.c | 50 +--- > > and if we get rid of the iram_alloc/iram_free wrappers around the > sram_pool (now pv_pool) alloc/free in iram.h, in the same way I've > done for Davinci, then we get less new additions too. Great! Thanks, Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 10:11:07PM +0200, Uwe Kleine-König wrote: > On Fri, Apr 15, 2011 at 02:06:07PM +0100, Russell King - ARM Linux wrote: > > This is work in progress. > > > > We have two SoCs using SRAM, both with their own allocation systems, > > and both with their own ways of copying functions into the SRAM. > I havn't checked the details, but maybe the code in > arch/arm/plat-mxc/iram_alloc.c could be migrated to your approach, too? Its already in there now that I have replies from Nguyen Dinh. It looks like this presently: arch/arm/plat-mxc/Kconfig |2 +- arch/arm/plat-mxc/include/mach/iram.h | 24 +++-- arch/arm/plat-mxc/iram_alloc.c | 50 +--- and if we get rid of the iram_alloc/iram_free wrappers around the sram_pool (now pv_pool) alloc/free in iram.h, in the same way I've done for Davinci, then we get less new additions too. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 02:06:07PM +0100, Russell King - ARM Linux wrote: > This is work in progress. > > We have two SoCs using SRAM, both with their own allocation systems, > and both with their own ways of copying functions into the SRAM. I havn't checked the details, but maybe the code in arch/arm/plat-mxc/iram_alloc.c could be migrated to your approach, too? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH] Consolidate SRAM support
>-Original Message-
>From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk]
>Sent: Friday, April 15, 2011 2:40 PM
>To: Nguyen Dinh-R00091
>Cc: Kevin Hilman; davinci-linux-open-sou...@linux.davincidsp.com; Tony
>Lindgren; Sekhar Nori; linux-
>o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org
>Subject: Re: [RFC PATCH] Consolidate SRAM support
>
>On Fri, Apr 15, 2011 at 07:20:12PM +, Nguyen Dinh-R00091 wrote:
>>
>> >-Original Message-
>> >From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk]
>> >Sent: Friday, April 15, 2011 11:59 AM
>> >To: Nguyen Dinh-R00091
>> >Cc: Kevin Hilman; davinci-linux-open-sou...@linux.davincidsp.com; Tony
>> >Lindgren; Sekhar Nori;
>linux-
>> >o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org
>> >Subject: Re: [RFC PATCH] Consolidate SRAM support
>> >
>> >On Fri, Apr 15, 2011 at 05:20:45PM +0100, Russell King - ARM Linux wrote:
>> >> On Fri, Apr 15, 2011 at 04:18:23PM +, Nguyen Dinh-R00091 wrote:
>> >> > >Hmm, that's nice - except for one issue. According to my grep of
>> >> > >arch/arm/ and drivers/, nothing uses iram_alloc(). So, does anything
>> >> > >in
>> >> > >the MX stuff use iram_alloc.c, or is it dead code left over from a
>> >> > >previous experiment?
>> >> >
>> >> > This function will be used for suspend code in the mx5x series. I just
>> >> > got done submitting a series of patches to Sascha for a simple suspend
>> >> > that does not need running code out of IRAM yet. The next set of suspend
>> >> > patches will be using these iram functions.
>> >>
>> >> Okay, so does iram_alloc() need to return a dma_addr_t or a phys_addr_t ?
>> >> Are you dealing with physical addresses for it or DMA addresses?
>> >
>> >And another question: why does iram_alloc() return a void __iomem * for
>> >the virtual address, and the physical address via a pointer argument.
>> >However, iram_free() take an unsigned long for the address.
>>
>> Yes, should just be a void *iram_alloc().
>
>Thanks. As you didn't comment against the change below, I'm going to
>assume that you're happy with it. It means that the usage changes from:
Sorry...yes, your proposal looks fine.
Thanks,
Dinh
>
> unsigned long dma;
> void __iomem *addr = iram_alloc(SZ_4K, &dma);
> ...
> iram_free(dma, SZ_4K);
>
>to:
>
> phys_addr_t phys;
> void *addr = iram_alloc(SZ_4K, &phys);
> ...
> iram_free(addr, SZ_4K);
>
>> >It seems rather inconsistent that the parameter for free is returned via
>> >a pointer argument.
>> >
>> >If I convert this to sram-pool, can we change this to:
>> >
>> >static inline void *iram_alloc(size_t size, phys_addr_t *phys_addr)
>> >{
>> >return sram_pool_alloc(iram_pool, size, phys_addr);
>> >}
>> >
>> >static void iram_free(void *addr, size_t size)
>> >{
>> >sram_pool_free(iram_pool, addr, size);
>> >}
>> >
>> >and:
>> >
>> >int __init iram_init(unsigned long base, unsigned long size)
>> >
>> >becomes:
>> >
>> >int __init iram_init(phys_addr_t base, size_t size)
>> >
>> >?
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 07:20:12PM +, Nguyen Dinh-R00091 wrote:
>
> >-Original Message-
> >From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk]
> >Sent: Friday, April 15, 2011 11:59 AM
> >To: Nguyen Dinh-R00091
> >Cc: Kevin Hilman; davinci-linux-open-sou...@linux.davincidsp.com; Tony
> >Lindgren; Sekhar Nori; linux-
> >o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org
> >Subject: Re: [RFC PATCH] Consolidate SRAM support
> >
> >On Fri, Apr 15, 2011 at 05:20:45PM +0100, Russell King - ARM Linux wrote:
> >> On Fri, Apr 15, 2011 at 04:18:23PM +, Nguyen Dinh-R00091 wrote:
> >> > >Hmm, that's nice - except for one issue. According to my grep of
> >> > >arch/arm/ and drivers/, nothing uses iram_alloc(). So, does anything in
> >> > >the MX stuff use iram_alloc.c, or is it dead code left over from a
> >> > >previous experiment?
> >> >
> >> > This function will be used for suspend code in the mx5x series. I just
> >> > got done submitting a series of patches to Sascha for a simple suspend
> >> > that does not need running code out of IRAM yet. The next set of suspend
> >> > patches will be using these iram functions.
> >>
> >> Okay, so does iram_alloc() need to return a dma_addr_t or a phys_addr_t ?
> >> Are you dealing with physical addresses for it or DMA addresses?
> >
> >And another question: why does iram_alloc() return a void __iomem * for
> >the virtual address, and the physical address via a pointer argument.
> >However, iram_free() take an unsigned long for the address.
>
> Yes, should just be a void *iram_alloc().
Thanks. As you didn't comment against the change below, I'm going to
assume that you're happy with it. It means that the usage changes from:
unsigned long dma;
void __iomem *addr = iram_alloc(SZ_4K, &dma);
...
iram_free(dma, SZ_4K);
to:
phys_addr_t phys;
void *addr = iram_alloc(SZ_4K, &phys);
...
iram_free(addr, SZ_4K);
> >It seems rather inconsistent that the parameter for free is returned via
> >a pointer argument.
> >
> >If I convert this to sram-pool, can we change this to:
> >
> >static inline void *iram_alloc(size_t size, phys_addr_t *phys_addr)
> >{
> > return sram_pool_alloc(iram_pool, size, phys_addr);
> >}
> >
> >static void iram_free(void *addr, size_t size)
> >{
> > sram_pool_free(iram_pool, addr, size);
> >}
> >
> >and:
> >
> >int __init iram_init(unsigned long base, unsigned long size)
> >
> >becomes:
> >
> >int __init iram_init(phys_addr_t base, size_t size)
> >
> >?
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH] Consolidate SRAM support
>-Original Message-
>From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk]
>Sent: Friday, April 15, 2011 11:59 AM
>To: Nguyen Dinh-R00091
>Cc: Kevin Hilman; davinci-linux-open-sou...@linux.davincidsp.com; Tony
>Lindgren; Sekhar Nori; linux-
>o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org
>Subject: Re: [RFC PATCH] Consolidate SRAM support
>
>On Fri, Apr 15, 2011 at 05:20:45PM +0100, Russell King - ARM Linux wrote:
>> On Fri, Apr 15, 2011 at 04:18:23PM +, Nguyen Dinh-R00091 wrote:
>> > >Hmm, that's nice - except for one issue. According to my grep of
>> > >arch/arm/ and drivers/, nothing uses iram_alloc(). So, does anything in
>> > >the MX stuff use iram_alloc.c, or is it dead code left over from a
>> > >previous experiment?
>> >
>> > This function will be used for suspend code in the mx5x series. I just
>> > got done submitting a series of patches to Sascha for a simple suspend
>> > that does not need running code out of IRAM yet. The next set of suspend
>> > patches will be using these iram functions.
>>
>> Okay, so does iram_alloc() need to return a dma_addr_t or a phys_addr_t ?
>> Are you dealing with physical addresses for it or DMA addresses?
>
>And another question: why does iram_alloc() return a void __iomem * for
>the virtual address, and the physical address via a pointer argument.
>However, iram_free() take an unsigned long for the address.
Yes, should just be a void *iram_alloc().
>
>It seems rather inconsistent that the parameter for free is returned via
>a pointer argument.
>
>If I convert this to sram-pool, can we change this to:
>
>static inline void *iram_alloc(size_t size, phys_addr_t *phys_addr)
>{
> return sram_pool_alloc(iram_pool, size, phys_addr);
>}
>
>static void iram_free(void *addr, size_t size)
>{
> sram_pool_free(iram_pool, addr, size);
>}
>
>and:
>
>int __init iram_init(unsigned long base, unsigned long size)
>
>becomes:
>
>int __init iram_init(phys_addr_t base, size_t size)
>
>?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On 14:06 Fri 15 Apr , Russell King - ARM Linux wrote:
> This is work in progress.
>
> We have two SoCs using SRAM, both with their own allocation systems,
> and both with their own ways of copying functions into the SRAM.
>
> Let's unify this before we have additional SoCs re-implementing this
> obviously common functionality themselves.
>
> Unfortunately, we end up with code growth through doing this, but that
> will become a win when we have another SoC using this (which I know
> there's at least one in the pipeline).
>
> One of the considerations here is that we can easily convert sram-pool.c
> to hook into device tree stuff, which can tell the sram allocator:
> - physical address of sram
> - size of sram
> - allocation granularity
> and then we just need to ensure that it is appropriately mapped.
>
> This uses the physical address, and unlike Davinci's dma address usage,
> it always wants to have the physical address, and will always return
> the corresponding physical address when passed that pointer.
>
> OMAP could probably do with some more work to make the omapfb and other
> allocations use the sram allocator, rather than hooking in before the
> sram allocator is initialized - and then further cleanups so that we
> have an initialization function which just does
>
> sram_create(phys, size)
> virt = map sram(phys, size)
> create sram pool(virt, phys, size, min_alloc_order)
>
> Another question is whether we should allow multiple SRAM pools or not -
> this code does allow multiple pools, but so far we only have one pool
> per SoC. Overdesign? Maybe, but it prevents SoCs wanting to duplicate
> it if they want to partition the SRAM, or have peripheral-local SRAMs.
>
> Lastly, uio_pruss should probably take the SRAM pool pointer via
> platform data so that it doesn't have to include Davinci specific
> includes.
>
> Signed-off-by: Russell King
Hi,
I also work on it for at91
and already have some patch in the mm for this
[PATCH] genalloc: add support to specify the physical address
so now you can do this
as I do on at91 will send the patch after
static struct map_desc at91sam9g20_sram_desc[] __initdata = {
{
.virtual= AT91_IO_VIRT_BASE - AT91SAM9G20_SRAM_SIZE,
.pfn= __phys_to_pfn(AT91SAM9G20_SRAM_BASE),
.length = AT91SAM9G20_SRAM_SIZE,
.type = MT_DEVICE,
}
};
sram_pool = gen_pool_create(2, -1);
gen_pool_add_virt(sram_pool, io_desc->virtual
__pfn_to_phys(io_desc->pfn),
io_desc->length, -1)
and to get the physical address
phys = gen_pool_virt_to_phys(sram_pool, virt);
Best Resgards,
J.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 08:12:07PM +0200, Detlef Vollmann wrote: >> Second point is that you'll notice that the code converts to a phys >> address using this: phys = phys_base + (virt - virt_base). As soon as >> we start allowing multiple regions of SRAM, it becomes impossible to >> provide the phys address without a lot more complexity. > Yes, I understand that. This either requires some intrusive changes > to gen_pool code or quite some additional code in sram_pool_alloc. It would require sram_pool to track each individual buffer alongside the similar tracking that gen_pool does, and then look up the returned address to find out which buffer it corresponds with. As it looks though, this functionality isn't required on AT91. So lets not complicate the code unless we know we have to. While the alloc/free API is such that it'll be able to cope if we have to add it later - the initialization will have to become a little more complex. And then I start to wonder if gen_pool should just be extended for the phys address. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 05:20:45PM +0100, Russell King - ARM Linux wrote:
> On Fri, Apr 15, 2011 at 04:18:23PM +, Nguyen Dinh-R00091 wrote:
> > >Hmm, that's nice - except for one issue. According to my grep of
> > >arch/arm/ and drivers/, nothing uses iram_alloc(). So, does anything in
> > >the MX stuff use iram_alloc.c, or is it dead code left over from a
> > >previous experiment?
> >
> > This function will be used for suspend code in the mx5x series. I just
> > got done submitting a series of patches to Sascha for a simple suspend
> > that does not need running code out of IRAM yet. The next set of suspend
> > patches will be using these iram functions.
>
> Okay, so does iram_alloc() need to return a dma_addr_t or a phys_addr_t ?
> Are you dealing with physical addresses for it or DMA addresses?
And another question: why does iram_alloc() return a void __iomem * for
the virtual address, and the physical address via a pointer argument.
However, iram_free() take an unsigned long for the address.
It seems rather inconsistent that the parameter for free is returned via
a pointer argument.
If I convert this to sram-pool, can we change this to:
static inline void *iram_alloc(size_t size, phys_addr_t *phys_addr)
{
return sram_pool_alloc(iram_pool, size, phys_addr);
}
static void iram_free(void *addr, size_t size)
{
sram_pool_free(iram_pool, addr, size);
}
and:
int __init iram_init(unsigned long base, unsigned long size)
becomes:
int __init iram_init(phys_addr_t base, size_t size)
?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 04:18:23PM +, Nguyen Dinh-R00091 wrote: > >Hmm, that's nice - except for one issue. According to my grep of > >arch/arm/ and drivers/, nothing uses iram_alloc(). So, does anything in > >the MX stuff use iram_alloc.c, or is it dead code left over from a > >previous experiment? > > This function will be used for suspend code in the mx5x series. I just > got done submitting a series of patches to Sascha for a simple suspend > that does not need running code out of IRAM yet. The next set of suspend > patches will be using these iram functions. Okay, so does iram_alloc() need to return a dma_addr_t or a phys_addr_t ? Are you dealing with physical addresses for it or DMA addresses? -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH] Consolidate SRAM support
Hi Russell, >-Original Message- >From: linux-arm-kernel-boun...@lists.infradead.org [mailto:linux-arm-kernel- >boun...@lists.infradead.org] On Behalf Of Russell King - ARM Linux >Sent: Friday, April 15, 2011 11:04 AM >To: Rob Herring >Cc: Kevin Hilman; davinci-linux-open-sou...@linux.davincidsp.com; Tony >Lindgren; Sekhar Nori; linux- >o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org >Subject: Re: [RFC PATCH] Consolidate SRAM support > >On Fri, Apr 15, 2011 at 08:39:55AM -0500, Rob Herring wrote: >> Russell, >> >> On 04/15/2011 08:06 AM, Russell King - ARM Linux wrote: >>> This is work in progress. >>> >>> We have two SoCs using SRAM, both with their own allocation systems, >>> and both with their own ways of copying functions into the SRAM. >> >> It's more than that. Several i.MX chips use plat-mxc/iram_alloc.c. > >Hmm, that's nice - except for one issue. According to my grep of >arch/arm/ and drivers/, nothing uses iram_alloc(). So, does anything in >the MX stuff use iram_alloc.c, or is it dead code left over from a >previous experiment? This function will be used for suspend code in the mx5x series. I just got done submitting a series of patches to Sascha for a simple suspend that does not need running code out of IRAM yet. The next set of suspend patches will be using these iram functions. > >The commit says: > >ARM: imx: Add iram allocator functions > >Add IRAM(Internal RAM) allocation functions using GENERIC_ALLOCATOR. >The allocation size is 4KB multiples to guarantee alignment. The >idea for these functions is for i.MX platforms to use them >to dynamically allocate IRAM usage. > >Applies on 2.6.36-rc7 > >> lpc32xx and pnx4008 also use iram, but do not have an allocator (only 1 >> user). Both are doing a copy the suspend code to IRAM and run it which >> may also be a good thing to have generic code for. Several i.MX chips >> also need to run from IRAM for suspend. > >We have support for copying functions to other bits of memory and getting >the Thumb-ness right - see asm/fncpy.h. So that's a separate patch to >convert them over. > >___ >linux-arm-kernel mailing list >linux-arm-ker...@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
Le 15/04/2011 16:50, Detlef Vollmann : > On 04/15/11 15:06, Russell King - ARM Linux wrote: >> This is work in progress. > Thanks, very useful. [..] >> Another question is whether we should allow multiple SRAM pools or not - >> this code does allow multiple pools, but so far we only have one pool >> per SoC. Overdesign? Maybe, but it prevents SoCs wanting to duplicate >> it if they want to partition the SRAM, or have peripheral-local SRAMs. > Having the option to partition the SRAM is probably useful. > What I'm missing is sram_pool_add: on AT91SAM9G20 you have two banks > of SRAM, and you might want to combine them into a single pool. In fact on at91sam9g20 (and some other at91) you can use the mirroring of the SRAM until next bank... so you end up with a single pool. Base @ sram1 base - sram0 size size = sram 0 size + sram 1 size Best regards, -- Nicolas Ferre -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 08:39:55AM -0500, Rob Herring wrote: > Russell, > > On 04/15/2011 08:06 AM, Russell King - ARM Linux wrote: >> This is work in progress. >> >> We have two SoCs using SRAM, both with their own allocation systems, >> and both with their own ways of copying functions into the SRAM. > > It's more than that. Several i.MX chips use plat-mxc/iram_alloc.c. Hmm, that's nice - except for one issue. According to my grep of arch/arm/ and drivers/, nothing uses iram_alloc(). So, does anything in the MX stuff use iram_alloc.c, or is it dead code left over from a previous experiment? The commit says: ARM: imx: Add iram allocator functions Add IRAM(Internal RAM) allocation functions using GENERIC_ALLOCATOR. The allocation size is 4KB multiples to guarantee alignment. The idea for these functions is for i.MX platforms to use them to dynamically allocate IRAM usage. Applies on 2.6.36-rc7 > lpc32xx and pnx4008 also use iram, but do not have an allocator (only 1 > user). Both are doing a copy the suspend code to IRAM and run it which > may also be a good thing to have generic code for. Several i.MX chips > also need to run from IRAM for suspend. We have support for copying functions to other bits of memory and getting the Thumb-ness right - see asm/fncpy.h. So that's a separate patch to convert them over. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 09:32:01AM -0600, Grant Likely wrote: > Yes, once the infrastructure is in place, powerpc can do its own > migration to the new code. I vote for putting it in lib at the > outset. I don't agree with stuffing non-arch directories with code which people haven't already agreed should be shared. As I've already said, in my experience it's hard to get agreement in the first place and even when you can the API generally needs to be changed from what you first think would be reasonable. So, lets wait to see whether the SH folk reply. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 04:50:49PM +0200, Detlef Vollmann wrote: > I'd love to have the mapping inside the create pool, but that might > not be possible in general. No, think about it. What if you need to map the RAM area with some special attributes - eg, where ioremap() doesn't work. Eg, OMAP you need SRAM mapped as normal memory, except for OMAP34xx where it must be mapped normal memory non-cacheable. It's best to leave the mapping to the architecture. >> Another question is whether we should allow multiple SRAM pools or not - >> this code does allow multiple pools, but so far we only have one pool >> per SoC. Overdesign? Maybe, but it prevents SoCs wanting to duplicate >> it if they want to partition the SRAM, or have peripheral-local SRAMs. > Having the option to partition the SRAM is probably useful. > What I'm missing is sram_pool_add: on AT91SAM9G20 you have two banks > of SRAM, and you might want to combine them into a single pool. Do you already combine them into a single pool, or is this a wishlist? I'm really not interested in sorting out peoples wishlist items in the process of consolidation. Second point is that you'll notice that the code converts to a phys address using this: phys = phys_base + (virt - virt_base). As soon as we start allowing multiple regions of SRAM, it becomes impossible to provide the phys address without a lot more complexity. >> arch/arm/common/sram-pool.c | 69 >> +++ >> arch/arm/include/asm/sram-pool.h| 20 > Waht is ARM specific about this code? > Why not put it into lib/ and include/linux, respectively? Nothing, but this is the first stage of consolidation of this code. I'm not trying to consolidate the universe down to one implementation here - that's going to take _far_ too much effort in one go, and from my previous experiences interacting with other arch maintainers, that's the way to get precisely nothing done. In my experience, arch maintainers tend to object to each others patches, and the net result is no forward progress. See dma_mmap API as a brilliant example of that - the lack of which contines to cause problems for drivers many years after I initially tried (and gave up) trying to get agreement on that. So, let's do what we can to consolidate our stuff and if we get some agreement from other arch maintainers, look towards moving it out. Moving stuff out once its properly modularized is trivial. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 9:26 AM, Russell King - ARM Linux wrote: > On Fri, Apr 15, 2011 at 04:40:00PM +0200, Arnd Bergmann wrote: >> On Friday 15 April 2011 15:39:55 Rob Herring wrote: >> >> > On 04/15/2011 08:06 AM, Russell King - ARM Linux wrote: >> > > This is work in progress. >> > > >> > > We have two SoCs using SRAM, both with their own allocation systems, >> > > and both with their own ways of copying functions into the SRAM. >> > >> > It's more than that. Several i.MX chips use plat-mxc/iram_alloc.c. >> >> There are also non-ARM systems doing something like that, >> arch/blackfin/mm/sram-alloc.c comes to mind. On powerpc, the >> ppc7410 also has this feature in hardware, but I believe it was >> never supported in mainline Linux. > > Yes, but there's some horrible bits out there. PPC is one of them > which looks like it doesn't lend itself to consolidating with this > kind of implementation. > >> How about putting sram-pool.c into the top-level mm/ directory >> and sram-pool.h into include/linux? They seem to be completely >> generic to me, aside from the dependency on asm/fncpy.h which >> should probably remain arch specific. > > I've thought about lib - but first lets see whether other architectures > want to use it first. I've asked the sh folk off-list about this and > waiting for a reply. > > Blackfin and PPC look horrible to sort out though, so they can come at > a later date if they so wish. Yes, once the infrastructure is in place, powerpc can do its own migration to the new code. I vote for putting it in lib at the outset. g. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 04:40:00PM +0200, Arnd Bergmann wrote: > On Friday 15 April 2011 15:39:55 Rob Herring wrote: > > > On 04/15/2011 08:06 AM, Russell King - ARM Linux wrote: > > > This is work in progress. > > > > > > We have two SoCs using SRAM, both with their own allocation systems, > > > and both with their own ways of copying functions into the SRAM. > > > > It's more than that. Several i.MX chips use plat-mxc/iram_alloc.c. > > There are also non-ARM systems doing something like that, > arch/blackfin/mm/sram-alloc.c comes to mind. On powerpc, the > ppc7410 also has this feature in hardware, but I believe it was > never supported in mainline Linux. Yes, but there's some horrible bits out there. PPC is one of them which looks like it doesn't lend itself to consolidating with this kind of implementation. > How about putting sram-pool.c into the top-level mm/ directory > and sram-pool.h into include/linux? They seem to be completely > generic to me, aside from the dependency on asm/fncpy.h which > should probably remain arch specific. I've thought about lib - but first lets see whether other architectures want to use it first. I've asked the sh folk off-list about this and waiting for a reply. Blackfin and PPC look horrible to sort out though, so they can come at a later date if they so wish. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, Apr 15, 2011 at 04:52:38PM +0300, Eduardo Valentin wrote: > Hi Russel, > > Just small comment, > > On Fri, Apr 15, 2011 at 08:06:07AM -0500, Russell King wrote: > > > diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c > > index a3f50b3..3588749 100644 > > --- a/arch/arm/plat-omap/sram.c > > +++ b/arch/arm/plat-omap/sram.c > > @@ -75,7 +75,6 @@ > > static unsigned long omap_sram_start; > > static unsigned long omap_sram_base; > > static unsigned long omap_sram_size; > > -static unsigned long omap_sram_ceil; > > I think you missed one occurrence of omap_sram_ceil at > omap3_sram_restore_context. > Probably you have compile-tested w/o CONFIG_PM? I built OMAP2 only, which did have CONFIG_PM=y. Welcome to the problems of ifdef'ing code. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Friday 15 April 2011 15:39:55 Rob Herring wrote: > On 04/15/2011 08:06 AM, Russell King - ARM Linux wrote: > > This is work in progress. > > > > We have two SoCs using SRAM, both with their own allocation systems, > > and both with their own ways of copying functions into the SRAM. > > It's more than that. Several i.MX chips use plat-mxc/iram_alloc.c. There are also non-ARM systems doing something like that, arch/blackfin/mm/sram-alloc.c comes to mind. On powerpc, the ppc7410 also has this feature in hardware, but I believe it was never supported in mainline Linux. How about putting sram-pool.c into the top-level mm/ directory and sram-pool.h into include/linux? They seem to be completely generic to me, aside from the dependency on asm/fncpy.h which should probably remain arch specific. As long as CONFIG_ARM_SRAM_POOL (or perhaps just CONFIG_SRAM_POOL) is selected only by platforms that support it, the dependency should not hurt. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
On Fri, 2011-04-15 at 08:39 -0500, Rob Herring wrote: > lpc32xx and pnx4008 also use iram, but do not have an allocator (only > 1 user). Both are doing a copy the suspend code to IRAM and run it > which may also be a good thing to have generic code for. Several i.MX > chips also need to run from IRAM for suspend. There will be similar patches for my lpc2k architecture once it gets accepted, a common interface would be _very_ beneficial IMHO. Regards, Ithamar. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
Hi Russel, Just small comment, On Fri, Apr 15, 2011 at 08:06:07AM -0500, Russell King wrote: > diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c > index a3f50b3..3588749 100644 > --- a/arch/arm/plat-omap/sram.c > +++ b/arch/arm/plat-omap/sram.c > @@ -75,7 +75,6 @@ > static unsigned long omap_sram_start; > static unsigned long omap_sram_base; > static unsigned long omap_sram_size; > -static unsigned long omap_sram_ceil; I think you missed one occurrence of omap_sram_ceil at omap3_sram_restore_context. Probably you have compile-tested w/o CONFIG_PM? All best, -- Eduardo Valentin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Consolidate SRAM support
Russell, On 04/15/2011 08:06 AM, Russell King - ARM Linux wrote: This is work in progress. We have two SoCs using SRAM, both with their own allocation systems, and both with their own ways of copying functions into the SRAM. It's more than that. Several i.MX chips use plat-mxc/iram_alloc.c. lpc32xx and pnx4008 also use iram, but do not have an allocator (only 1 user). Both are doing a copy the suspend code to IRAM and run it which may also be a good thing to have generic code for. Several i.MX chips also need to run from IRAM for suspend. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH] Consolidate SRAM support
This is work in progress.
We have two SoCs using SRAM, both with their own allocation systems,
and both with their own ways of copying functions into the SRAM.
Let's unify this before we have additional SoCs re-implementing this
obviously common functionality themselves.
Unfortunately, we end up with code growth through doing this, but that
will become a win when we have another SoC using this (which I know
there's at least one in the pipeline).
One of the considerations here is that we can easily convert sram-pool.c
to hook into device tree stuff, which can tell the sram allocator:
- physical address of sram
- size of sram
- allocation granularity
and then we just need to ensure that it is appropriately mapped.
This uses the physical address, and unlike Davinci's dma address usage,
it always wants to have the physical address, and will always return
the corresponding physical address when passed that pointer.
OMAP could probably do with some more work to make the omapfb and other
allocations use the sram allocator, rather than hooking in before the
sram allocator is initialized - and then further cleanups so that we
have an initialization function which just does
sram_create(phys, size)
virt = map sram(phys, size)
create sram pool(virt, phys, size, min_alloc_order)
Another question is whether we should allow multiple SRAM pools or not -
this code does allow multiple pools, but so far we only have one pool
per SoC. Overdesign? Maybe, but it prevents SoCs wanting to duplicate
it if they want to partition the SRAM, or have peripheral-local SRAMs.
Lastly, uio_pruss should probably take the SRAM pool pointer via
platform data so that it doesn't have to include Davinci specific
includes.
Signed-off-by: Russell King
---
arch/arm/Kconfig|2 +
arch/arm/common/Kconfig |4 ++
arch/arm/common/Makefile|1 +
arch/arm/common/sram-pool.c | 69 +++
arch/arm/include/asm/sram-pool.h| 20
arch/arm/mach-davinci/da850.c |2 +-
arch/arm/mach-davinci/dm355.c |2 +-
arch/arm/mach-davinci/dm365.c |2 +-
arch/arm/mach-davinci/dm644x.c |2 +-
arch/arm/mach-davinci/dm646x.c |2 +-
arch/arm/mach-davinci/include/mach/common.h |2 +-
arch/arm/mach-davinci/include/mach/sram.h | 13 +
arch/arm/mach-davinci/pm.c | 12 +
arch/arm/mach-davinci/sram.c| 42 +++--
arch/arm/plat-omap/include/plat/sram.h | 17 ---
arch/arm/plat-omap/sram.c | 34 +-
drivers/uio/uio_pruss.c |8 ++-
17 files changed, 141 insertions(+), 93 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 5b9f78b..5c3401c 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -850,6 +850,7 @@ config ARCH_DAVINCI
bool "TI DaVinci"
select GENERIC_CLOCKEVENTS
select ARCH_REQUIRE_GPIOLIB
+ select ARM_SRAM_POOL
select ZONE_DMA
select HAVE_IDE
select CLKDEV_LOOKUP
@@ -863,6 +864,7 @@ config ARCH_OMAP
select HAVE_CLK
select ARCH_REQUIRE_GPIOLIB
select ARCH_HAS_CPUFREQ
+ select ARM_SRAM_POOL
select GENERIC_CLOCKEVENTS
select HAVE_SCHED_CLOCK
select ARCH_HAS_HOLES_MEMORYMODEL
diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
index ea5ee4d..ddbd20b 100644
--- a/arch/arm/common/Kconfig
+++ b/arch/arm/common/Kconfig
@@ -39,3 +39,7 @@ config SHARP_PARAM
config SHARP_SCOOP
bool
+
+config ARM_SRAM_POOL
+ bool
+ select GENERIC_ALLOCATOR
diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
index e7521bca..b79ad68 100644
--- a/arch/arm/common/Makefile
+++ b/arch/arm/common/Makefile
@@ -18,3 +18,4 @@ obj-$(CONFIG_ARCH_IXP23XX)+= uengine.o
obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o
obj-$(CONFIG_COMMON_CLKDEV)+= clkdev.o
obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o
+obj-$(CONFIG_ARM_SRAM_POOL)+= sram-pool.o
diff --git a/arch/arm/common/sram-pool.c b/arch/arm/common/sram-pool.c
new file mode 100644
index 000..9ff1466
--- /dev/null
+++ b/arch/arm/common/sram-pool.c
@@ -0,0 +1,69 @@
+/*
+ * Unified SRAM allocator, based on mach-davinci/sram.c, which was
+ * Copyright (C) 2009 David Brownell.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+struct sram_pool {
+ struct gen_pool *genpool;
+ void *cpu_base;
+ phys_addr_t phys_base;
+};
+
+void *sram_pool_alloc(struct sram_pool *pool, size_t len, phys_addr_t *
