diff mbox

[1/1] davinci: changed SRAM allocator to shared ram.

Message ID 1297434088-27995-1-git-send-email-subhasish@mistralsolutions.com (mailing list archive)
State Awaiting Upstream
Headers show

Commit Message

Subhasish Ghosh Feb. 11, 2011, 2:21 p.m. UTC
None

Comments

Ben Gardiner May 16, 2011, 7:08 p.m. UTC | #1
Hi Subhasish and Sekhar,

Subhashish, I was testing your patch here while investigating
davinci-pcm ping-pong buffers on da850.

On Fri, Feb 11, 2011 at 9:21 AM, Subhasish Ghosh
<subhasish@mistralsolutions.com> wrote:
> This patch modifies the sram allocator to allocate memory
> from the DA8XX shared RAM.
>
> Signed-off-by: Subhasish Ghosh <subhasish@mistralsolutions.com>
> ---
>  arch/arm/mach-davinci/da850.c              |    6 +++---
>  arch/arm/mach-davinci/include/mach/da8xx.h |    1 +

Since this changes only the da850 behaviour, a subject prefix of da850
might be more appropriate than 'davinci'.

>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> index 3443d97..8a4de97 100644
> --- a/arch/arm/mach-davinci/da850.c
> +++ b/arch/arm/mach-davinci/da850.c
> @@ -711,7 +711,7 @@ static struct map_desc da850_io_desc[] = {
>        },
>        {
>                .virtual        = SRAM_VIRT,
> -               .pfn            = __phys_to_pfn(DA8XX_ARM_RAM_BASE),
> +               .pfn            = __phys_to_pfn(DA8XX_SHARED_RAM_BASE),
>                .length         = SZ_8K,

Assigning only 8K to this iomap will result in a fault for the first
victim to access SRAM_VIRT+0x2000. This will happen for example with
mcasp ping-pong buffers totalling more than 8K on the da850.

Unfortunately SZ_128K cannot be used here since it will cause a panic
on boot. SZ_64K works though.

>                .type           = MT_DEVICE
>        },
> @@ -1083,8 +1083,8 @@ static struct davinci_soc_info davinci_soc_info_da850 = {
>        .gpio_irq               = IRQ_DA8XX_GPIO0,
>        .serial_dev             = &da8xx_serial_device,
>        .emac_pdata             = &da8xx_emac_pdata,
> -       .sram_dma               = DA8XX_ARM_RAM_BASE,
> -       .sram_len               = SZ_8K,
> +       .sram_dma               = DA8XX_SHARED_RAM_BASE,
> +       .sram_len               = SZ_128K,

This should probably be set to match whatever is reported in the map
entry above -- or an ioremap could be issued later but before the sram
init?

On Wed, Mar 2, 2011 at 12:12 PM, Nori, Sekhar <nsekhar@ti.com> wrote:
> [...]
>> root@arago:~# rtcwake -d /dev/rtc0 -s 20 -m mem
>> wakeup from "mem" at Tue Apr 21 14:31:13 2009
>> PM: Syncing filesystems ... done.
>> Freezing user space processes ... (elapsed 0.01 seconds) done.
>> Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done.
>> Suspending console(s) (use no_console_suspend to debug)
>>
>> From the Kconfig I had enabled RTC_DRV_OMAP, CONFIG_PM, CONFIG_SUSPEND.
>
> Nothing stands out from these logs. One thing to
> verify is if the RTC Alarm interrupt is really
> working by using the test program present in the
> Documentation/ folder.
>
> Anyway, the easiest way for you to move quickly is
> to use ramdisk so you can bypass all driver specific
> issues.

I tested suspend here with the patch applied on top of 2.6.39-rc7
using "rtcwake -d /dev/rtc0 -s 20 -m mem" and it works.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca
Sekhar Nori May 17, 2011, 6:25 p.m. UTC | #2
On Tue, May 17, 2011 at 00:38:21, Ben Gardiner wrote:
> Hi Subhasish and Sekhar,
> 
> Subhashish, I was testing your patch here while investigating
> davinci-pcm ping-pong buffers on da850.
> 
> On Fri, Feb 11, 2011 at 9:21 AM, Subhasish Ghosh
> <subhasish@mistralsolutions.com> wrote:
> > This patch modifies the sram allocator to allocate memory
> > from the DA8XX shared RAM.
> >
> > Signed-off-by: Subhasish Ghosh <subhasish@mistralsolutions.com>
> > ---
> >  arch/arm/mach-davinci/da850.c              |    6 +++---
> >  arch/arm/mach-davinci/include/mach/da8xx.h |    1 +
> 
> Since this changes only the da850 behaviour, a subject prefix of da850
> might be more appropriate than 'davinci'.

Please use "davinci: da850: ..."

> 
> >  2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> > index 3443d97..8a4de97 100644
> > --- a/arch/arm/mach-davinci/da850.c
> > +++ b/arch/arm/mach-davinci/da850.c
> > @@ -711,7 +711,7 @@ static struct map_desc da850_io_desc[] = {
> >        },
> >        {
> >                .virtual        = SRAM_VIRT,
> > -               .pfn            = __phys_to_pfn(DA8XX_ARM_RAM_BASE),
> > +               .pfn            = __phys_to_pfn(DA8XX_SHARED_RAM_BASE),
> >                .length         = SZ_8K,
> 
> Assigning only 8K to this iomap will result in a fault for the first
> victim to access SRAM_VIRT+0x2000. This will happen for example with
> mcasp ping-pong buffers totalling more than 8K on the da850.
> 
> Unfortunately SZ_128K cannot be used here since it will cause a panic

Okay, I am seeing this too. Kernel panics after freeing init memory.
No idea on this one, needs to be debugged.

Freeing init memory: 136K
Kernel panic - not syncing: Attempted to kill init!
[<c002f094>] (unwind_backtrace+0x0/0xec) from [<c024bf7c>] (panic+0x5c/0x184)
[<c024bf7c>] (panic+0x5c/0x184) from [<c00427dc>] (do_exit+0xb4/0x6c4)
[<c00427dc>] (do_exit+0xb4/0x6c4) from [<c0042ea4>] (do_group_exit+0xb8/0xe8)
[<c0042ea4>] (do_group_exit+0xb8/0xe8) from [<c004eaa4>] (get_signal_to_deliver+0x398/0x3f4)
[<c004eaa4>] (get_signal_to_deliver+0x398/0x3f4) from [<c002ca7c>] (do_notify_resume+0x60/0x610)
[<c002ca7c>] (do_notify_resume+0x60/0x610) from [<c002afd4>] (work_pending+0x24/0x28)

> on boot. SZ_64K works though.

Right.

> 
> >                .type           = MT_DEVICE
> >        },
> > @@ -1083,8 +1083,8 @@ static struct davinci_soc_info davinci_soc_info_da850 = {
> >        .gpio_irq               = IRQ_DA8XX_GPIO0,
> >        .serial_dev             = &da8xx_serial_device,
> >        .emac_pdata             = &da8xx_emac_pdata,
> > -       .sram_dma               = DA8XX_ARM_RAM_BASE,
> > -       .sram_len               = SZ_8K,
> > +       .sram_dma               = DA8XX_SHARED_RAM_BASE,
> > +       .sram_len               = SZ_128K,
> 
> This should probably be set to match whatever is reported in the map

You are right, the two sizes should match.

> entry above -- or an ioremap could be issued later but before the sram
> init?

Yes, ioremap would be better. I am not sure why a fixed mapping
for SRAM is required.

Please base these patches on Russell's SRAM consolidation patch.

Thanks,
Sekhar
diff mbox

Patch

diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 3443d97..8a4de97 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -711,7 +711,7 @@  static struct map_desc da850_io_desc[] = {
 	},
 	{
 		.virtual	= SRAM_VIRT,
-		.pfn		= __phys_to_pfn(DA8XX_ARM_RAM_BASE),
+		.pfn		= __phys_to_pfn(DA8XX_SHARED_RAM_BASE),
 		.length		= SZ_8K,
 		.type		= MT_DEVICE
 	},
@@ -1083,8 +1083,8 @@  static struct davinci_soc_info davinci_soc_info_da850 = {
 	.gpio_irq		= IRQ_DA8XX_GPIO0,
 	.serial_dev		= &da8xx_serial_device,
 	.emac_pdata		= &da8xx_emac_pdata,
-	.sram_dma		= DA8XX_ARM_RAM_BASE,
-	.sram_len		= SZ_8K,
+	.sram_dma		= DA8XX_SHARED_RAM_BASE,
+	.sram_len		= SZ_128K,
 	.reset_device		= &da8xx_wdt_device,
 };
 
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index cfcb223..c3c3339 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -70,6 +70,7 @@  extern unsigned int da850_max_speed;
 #define DA8XX_AEMIF_CTL_BASE	0x68000000
 #define DA8XX_DDR2_CTL_BASE	0xb0000000
 #define DA8XX_ARM_RAM_BASE	0xffff0000
+#define DA8XX_SHARED_RAM_BASE	0x80000000
 
 void __init da830_init(void);
 void __init da850_init(void);