>-----Original Message----- >From: Russell King - ARM Linux [mailto:linux@xxxxxxxxxxxxxxxx] >Sent: Friday, April 15, 2011 2:40 PM >To: Nguyen Dinh-R00091 >Cc: Kevin Hilman; davinci-linux-open-source@xxxxxxxxxxxxxxxxxxxx; Tony Lindgren; Sekhar Nori; linux- >omap@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx >Subject: Re: [RFC PATCH] Consolidate SRAM support > >On Fri, Apr 15, 2011 at 07:20:12PM +0000, Nguyen Dinh-R00091 wrote: >> >> >-----Original Message----- >> >From: Russell King - ARM Linux [mailto:linux@xxxxxxxxxxxxxxxx] >> >Sent: Friday, April 15, 2011 11:59 AM >> >To: Nguyen Dinh-R00091 >> >Cc: Kevin Hilman; davinci-linux-open-source@xxxxxxxxxxxxxxxxxxxx; Tony Lindgren; Sekhar Nori; >linux- >> >omap@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx >> >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 +0000, 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
