Hi S.Ghosh,

On Fri, Jan 07, 2011 at 16:09:32, S.Ghosh wrote:
> 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. 

I don't think sram_alloc() can allocate from multiple banks.
It should be OK to change sram_alloc() to allocate from shared memory.
That's the largest chunk of internal memory on these chips 
and it will be a pity if drivers cannot utilize it.

Thanks,
Sekhar

> 
> 
> 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