Hi Sekhar,

I observed that sram_alloc is allocating memory from the Arm Internal Memory
(0xFFFF 0000) and
not the Shared Memory (0x8000 0000). In that case, we cannot use sram_alloc.


On Wed, Jan 5, 2011 at 2:48 PM, S.Ghosh <[email protected]> wrote:

>
>
> On Wed, Dec 15, 2010 at 5:41 PM, Nori, Sekhar <[email protected]> wrote:
>
>> On Fri, Dec 03, 2010 at 20:41:47, Subhasish Ghosh wrote:
>> > The patch adds support for emulated UART controllers
>> > on the programmable realtime unit (PRU) available on OMAPL138.
>> > This defines the system resource requirements such as pin mux,
>> > clock, iomem, interrupt etc and registers the platform device
>> > as per the Linux driver model.
>> >
>> > Signed-off-by: Subhasish Ghosh <[email protected]>
>> > ---
>> >  arch/arm/mach-davinci/da850.c               |   16 +++++
>> >  arch/arm/mach-davinci/devices-da8xx.c       |   95
>> ++++++++++++++++++++++++++-
>> >  arch/arm/mach-davinci/include/mach/da8xx.h  |    1 +
>> >  arch/arm/mach-davinci/include/mach/memory.h |    1 +
>> >  include/linux/serial_core.h                 |    3 +
>> >  5 files changed, 115 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/arch/arm/mach-davinci/da850.c
>> b/arch/arm/mach-davinci/da850.c
>> > index 63916b9..85508c2 100644
>> > --- a/arch/arm/mach-davinci/da850.c
>> > +++ b/arch/arm/mach-davinci/da850.c
>> > @@ -238,6 +238,12 @@ static struct clk tptc2_clk = {
>> >       .flags          = ALWAYS_ENABLED,
>> >  };
>> >
>> > +static struct clk pru_clk = {
>> > +     .name   = "pruss",
>> > +     .parent = &pll0_sysclk2,
>> > +     .lpsc   = DA8XX_LPSC0_DMAX,
>> > +};
>> > +
>> >  static struct clk uart0_clk = {
>> >       .name           = "uart0",
>> >       .parent         = &pll0_sysclk2,
>> > @@ -318,6 +324,14 @@ static struct clk mcasp_clk = {
>> >       .flags          = DA850_CLK_ASYNC3,
>> >  };
>> >
>> > +static struct clk mcasp_pru_clk = {
>> > +     .name           = "mcasp_pru",
>> > +     .parent         = &pll0_sysclk2,
>> > +     .lpsc           = DA8XX_LPSC1_McASP0,
>> > +     .gpsc           = 1,
>> > +     .flags          = DA850_CLK_ASYNC3,
>> > +};
>>
>> There is already a mcasp clock defined. Why not use it instead of
>> replicating it with a different name? Not doing so will cause a mess
>> with reference counting.
>>
>
> [SG] -- This McASP clock is bound with the McASP driver by the device ID.
> This device ID (davinci-mcasp.0) is not
> available to the SUART driver. Moreover, the McASP driver is written
> specifically for Audio applications. These API's
> are not suitable for our purposes. Hence, to enable the SUART we disable
> the sound sub-system completely and
> configured the McASP in the SUART API's.
>
>>
>> > +
>> >  static struct clk lcdc_clk = {
>> >       .name           = "lcdc",
>> >       .parent         = &pll0_sysclk2,
>> > @@ -373,6 +387,7 @@ static struct clk_lookup da850_clks[] = {
>> >       CLK(NULL,               "tpcc1",        &tpcc1_clk),
>> >       CLK(NULL,               "tptc2",        &tptc2_clk),
>> >       CLK(NULL,               "uart0",        &uart0_clk),
>> > +     CLK(NULL,               "pruss",        &pru_clk),
>> >       CLK(NULL,               "uart1",        &uart1_clk),
>> >       CLK(NULL,               "uart2",        &uart2_clk),
>> >       CLK(NULL,               "aintc",        &aintc_clk),
>> > @@ -381,6 +396,7 @@ static struct clk_lookup da850_clks[] = {
>> >       CLK(NULL,               "emif3",        &emif3_clk),
>> >       CLK(NULL,               "arm",          &arm_clk),
>> >       CLK(NULL,               "rmii",         &rmii_clk),
>> > +     CLK(NULL,               "mcasp_pru",    &mcasp_pru_clk),
>> >       CLK("davinci_emac.1",   NULL,           &emac_clk),
>> >       CLK("davinci-mcasp.0",  NULL,           &mcasp_clk),
>> >       CLK("da8xx_lcdc.0",     NULL,           &lcdc_clk),
>> > diff --git a/arch/arm/mach-davinci/devices-da8xx.c
>> b/arch/arm/mach-davinci/devices-da8xx.c
>> > index 9eec630..25de36d 100644
>> > --- a/arch/arm/mach-davinci/devices-da8xx.c
>> > +++ b/arch/arm/mach-davinci/devices-da8xx.c
>> > @@ -85,7 +85,100 @@ struct platform_device da8xx_serial_device = {
>> >       },
>> >  };
>> >
>> > -static const s8 da8xx_queue_tc_mapping[][2] = {
>> > +
>> > +#define OMAPL138_PRU_MEM_BASE           0x01C30000
>> > +
>> > +#define OMAPL138_INT_PRU_SUART_1 IRQ_DA8XX_EVTOUT0
>> > +#define OMAPL138_INT_PRU_SUART_2 IRQ_DA8XX_EVTOUT1
>> > +#define OMAPL138_INT_PRU_SUART_3 IRQ_DA8XX_EVTOUT2
>> > +#define OMAPL138_INT_PRU_SUART_4 IRQ_DA8XX_EVTOUT3
>> > +#define OMAPL138_INT_PRU_SUART_5 IRQ_DA8XX_EVTOUT4
>> > +#define OMAPL138_INT_PRU_SUART_6 IRQ_DA8XX_EVTOUT5
>> > +#define OMAPL138_INT_PRU_SUART_7 IRQ_DA8XX_EVTOUT6
>> > +#define OMAPL138_INT_PRU_SUART_8 IRQ_DA8XX_EVTOUT7
>>
>> Can you please use DA850/DA8XX prefix like is done
>> in other DA850 code? This will help reading and not
>> make this code stand out.
>>
>
> [SG] -- Will do.
>
>
>>
>> > +
>> > +static struct resource omapl138_pru_suart_resources[] = {
>> > +     {
>> > +             .name   = "omapl_pru_suart",
>> > +             .start  = OMAPL138_PRU_MEM_BASE,
>> > +             .end    = OMAPL138_PRU_MEM_BASE + 0xFFFF,
>> > +             .flags  = IORESOURCE_MEM,
>> > +     },
>> > +     {
>> > +             .start  = DAVINCI_DA8XX_MCASP0_REG_BASE,
>> > +             .end    = DAVINCI_DA8XX_MCASP0_REG_BASE + (SZ_1K * 12) -
>> 1,
>> > +             .flags  = IORESOURCE_MEM,
>> > +     },
>> > +     {
>> > +             .start  = DA8XX_PSC0_BASE,
>> > +             .end    = DA8XX_PSC0_BASE + (SZ_1K * 3) - 1,
>> > +             .flags  = IORESOURCE_MEM,
>> > +     },
>> > +     {
>> > +             .start  = DA8XX_PSC1_BASE,
>> > +             .end    = DA8XX_PSC1_BASE + (SZ_1K * 3) - 1,
>> > +             .flags  = IORESOURCE_MEM,
>> > +     },
>>
>> I thought you are going to remove these since PSCs
>> should only be worked using the clock API. Also, can
>> you please add a version number after the word "PATCH"
>> in the subject so it is easy for reviewers to locate
>> your latest patches?
>>
>
> [SG] -- Will do.
>
>>
>> > +     {
>> > +             .start  = DA8XX_SHARED_RAM_BASE,
>> > +             .end    = DA8XX_SHARED_RAM_BASE + (SZ_1K * 8) - 1,
>> > +             .flags  = IORESOURCE_MEM,
>> > +     },
>>
>> Shared RAM can be used by other drivers (like audio). So,
>> allocation of this RAM should happen using sram_alloc().
>>
>
> [SG] -- Will do.
>
>
>>
>> > +     {
>> > +             .start  = OMAPL138_INT_PRU_SUART_1,
>> > +             .end    = OMAPL138_INT_PRU_SUART_1,
>> > +             .flags  = IORESOURCE_IRQ,
>> > +     },
>> > +     {
>> > +             .start  = OMAPL138_INT_PRU_SUART_2,
>> > +             .end    = OMAPL138_INT_PRU_SUART_2,
>> > +             .flags  = IORESOURCE_IRQ,
>> > +     },
>> > +     {
>> > +             .start  = OMAPL138_INT_PRU_SUART_3,
>> > +             .end    = OMAPL138_INT_PRU_SUART_3,
>> > +             .flags  = IORESOURCE_IRQ,
>> > +     },
>> > +     {
>> > +             .start  = OMAPL138_INT_PRU_SUART_4,
>> > +             .end    = OMAPL138_INT_PRU_SUART_4,
>> > +             .flags  = IORESOURCE_IRQ,
>> > +     },
>> > +     {
>> > +             .start  = OMAPL138_INT_PRU_SUART_5,
>> > +             .end    = OMAPL138_INT_PRU_SUART_5,
>> > +             .flags  = IORESOURCE_IRQ,
>> > +     },
>> > +     {
>> > +             .start  = OMAPL138_INT_PRU_SUART_6,
>> > +             .end    = OMAPL138_INT_PRU_SUART_6,
>> > +             .flags  = IORESOURCE_IRQ,
>> > +     },
>> > +     {
>> > +             .start  = OMAPL138_INT_PRU_SUART_7,
>> > +             .end    = OMAPL138_INT_PRU_SUART_7,
>> > +             .flags  = IORESOURCE_IRQ,
>> > +     },
>> > +     {
>> > +             .start  = OMAPL138_INT_PRU_SUART_8,
>> > +             .end    = OMAPL138_INT_PRU_SUART_8,
>> > +             .flags  = IORESOURCE_IRQ,
>> > +     },
>> > +};
>> > +
>> > +struct platform_device omapl_pru_suart_device = {
>> > +     .name                   =       "davinci_pru_suart",
>> > +     .id                             =       -1,
>> > +     .num_resources  =       ARRAY_SIZE(omapl138_pru_suart_resources),
>> > +     .resource               =       omapl138_pru_suart_resources,
>> > +};
>> > +
>> > +int __init da8xx_register_pru_suart(void)
>> > +{
>> > +     return platform_device_register(&omapl_pru_suart_device);
>> > +}
>> > +
>> > +static  const s8 da8xx_queue_tc_mapping[][2] = {
>> >       /* {event queue no, TC no} */
>> >       {0, 0},
>> >       {1, 1},
>> > diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h
>> b/arch/arm/mach-davinci/include/mach/da8xx.h
>> > index 4247b3f..940dbd6 100644
>> > --- a/arch/arm/mach-davinci/include/mach/da8xx.h
>> > +++ b/arch/arm/mach-davinci/include/mach/da8xx.h
>> > @@ -74,6 +74,7 @@ int da8xx_register_watchdog(void);
>> >  int da8xx_register_usb20(unsigned mA, unsigned potpgt);
>> >  int da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata);
>> >  int da8xx_register_emac(void);
>> > +int da8xx_register_pru_suart(void);
>> >  int da8xx_register_lcdc(struct da8xx_lcdc_platform_data *pdata);
>> >  int da8xx_register_mmcsd0(struct davinci_mmc_config *config);
>> >  int da850_register_mmcsd1(struct davinci_mmc_config *config);
>> > diff --git a/arch/arm/mach-davinci/include/mach/memory.h
>> b/arch/arm/mach-davinci/include/mach/memory.h
>> > index 22eb97c..d3e48d9 100644
>> > --- a/arch/arm/mach-davinci/include/mach/memory.h
>> > +++ b/arch/arm/mach-davinci/include/mach/memory.h
>> > @@ -22,6 +22,7 @@
>> >
>> **************************************************************************/
>> >  #define DAVINCI_DDR_BASE     0x80000000
>> >  #define DA8XX_DDR_BASE               0xc0000000
>> > +#define DA8XX_SHARED_RAM_BASE                0x80000000
>>
>> Please add it in arch/arm/mach-davinci/include/mach/da8xx.h
>> along with rest of da8xx specific base address defines.
>>
>
> [SG] -- Will do
>
>>
>> Thanks,
>> Sekhar
>>
>>
>
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to