Sergei,

Thanks for the comments...

On Tue, Jul 14, 2009 at 22:25:32, Sergei Shtylyov wrote:
> Hello.
> 
> Sudhakar Rajashekhara wrote:
> 
> > The DA850/OMAP-L138 is a new SoC from TI in the same family as
> > DA830/OMAP-L137.
> 
> > Major changes include better support for power management,
> > support for SATA devices and McBSP (same IP as DM644x).
> 
> > DA850/OMAP-L138 documents are available at
> > http://focus.ti.com/docs/prod/folders/print/omap-l138.html.
> 
> > Signed-off-by: Sudhakar Rajashekhara <[email protected]>
> 
> [...]
> 
> > diff --git a/arch/arm/mach-davinci/Makefile b/arch/arm/mach-davinci/Makefile
> > index d183b11..5f8a661 100644
> > --- a/arch/arm/mach-davinci/Makefile
> > +++ b/arch/arm/mach-davinci/Makefile
> > @@ -15,6 +15,7 @@ obj-$(CONFIG_ARCH_DAVINCI_DM355)        += dm355.o 
> > devices.o
> >  obj-$(CONFIG_ARCH_DAVINCI_DM646x)       += dm646x.o devices.o
> >  obj-$(CONFIG_ARCH_DAVINCI_DM365)   += dm365.o devices.o
> >  obj-$(CONFIG_ARCH_DAVINCI_DA830)        += da830.o devices-da8xx.o
> > +obj-$(CONFIG_ARCH_DAVINCI_DA850)        += da850.o devices-da8xx.o
> >  
> >  obj-$(CONFIG_AINTC)                        += irq.o
> >  obj-$(CONFIG_CP_INTC)                      += cp_intc.o
> > diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> > new file mode 100644
> > index 0000000..79d034a
> > --- /dev/null
> > +++ b/arch/arm/mach-davinci/da850.c
> > @@ -0,0 +1,623 @@
> > +/*
> > + * TI DA850/OMAP L138 chip specific setup
> > + *
> > + * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
> > + *
> > + * Derived from: arch/arm/mach-davinci/da830.c
> > + * Original Copyrights follow:
> > + *
> > + * 2009 (c) MontaVista Software, Inc. This file is licensed under
> > + * the terms of the GNU General Public License version 2. This program
> > + * is licensed "as is" without any warranty of any kind, whether express
> > + * or implied.
> > + */
> > +#include <linux/kernel.h>
> > +#include <linux/init.h>
> > +#include <linux/clk.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include <asm/mach/map.h>
> > +
> > +#include <mach/clock.h>
> > +#include <mach/psc.h>
> > +#include <mach/mux.h>
> > +#include <mach/irqs.h>
> > +#include <mach/cputype.h>
> > +#include <mach/common.h>
> > +#include <mach/time.h>
> > +#include <mach/da8xx.h>
> > +
> > +#include "clock.h"
> > +#include "mux.h"
> > +
> 
> > +#define PINMUX0                    0x00
> > +#define PINMUX1                    0x04
> > +#define PINMUX2                    0x08
> > +#define PINMUX3                    0x0c
> > +#define PINMUX4                    0x10
> > +#define PINMUX5                    0x14
> > +#define PINMUX6                    0x18
> > +#define PINMUX7                    0x1c
> > +#define PINMUX8                    0x20
> > +#define PINMUX9                    0x24
> > +#define PINMUX10           0x28
> > +#define PINMUX11           0x2c
> > +#define PINMUX12           0x30
> > +#define PINMUX13           0x34
> > +#define PINMUX14           0x38
> > +#define PINMUX15           0x3c
> > +#define PINMUX16           0x40
> > +#define PINMUX17           0x44
> > +#define PINMUX18           0x48
> > +#define PINMUX19           0x4c
> 
>     These registers are the same as on DA830. Could we place their #define's 
> into <mach/da8xx.h> as well?
> 

OK, I'll move them to da8xx.h.

> > +
> > +/*
> > + * Device specific mux setup
> > + *
> > + *         soc     description     mux     mode    mode    mux     dbg
> > + *                                 reg     offset  mask    mode
> > + */
> > +static const struct mux_config da850_pins[] = {
> 
>     Sigh, Mark should have renamed da830_pins[] to da830_pinmux[] since he 
> also has *_pins[] arrays of different, *short* type: pin (signal) lists.
> 
> > +#ifdef CONFIG_DAVINCI_MUX
> > +   /* UART0 function */
> > +   MUX_CFG(DA850, NUART0_CTS,      3,      24,     15,     2,      false)
> > +   MUX_CFG(DA850, NUART0_RTS,      3,      28,     15,     2,      false)
> > +   MUX_CFG(DA850, UART0_RXD,       3,      16,     15,     2,      false)
> > +   MUX_CFG(DA850, UART0_TXD,       3,      20,     15,     2,      false)
> > +   /* UART1 function */
> > +   MUX_CFG(DA850, UART1_RXD,       4,      24,     15,     2,      false)
> > +   MUX_CFG(DA850, UART1_TXD,       4,      28,     15,     2,      false)
> > +   /* UART2 function */
> > +   MUX_CFG(DA850, UART2_RXD,       4,      16,     15,     2,      false)
> > +   MUX_CFG(DA850, UART2_TXD,       4,      20,     15,     2,      false)
> > +   /* I2C1 function */
> > +   MUX_CFG(DA850, I2C1_SCL,        4,      16,     15,     4,      false)
> > +   MUX_CFG(DA850, I2C1_SDA,        4,      20,     15,     4,      false)
> > +   /* I2C0 function */
> > +   MUX_CFG(DA850, I2C0_SDA,        4,      12,     15,     2,      false)
> > +   MUX_CFG(DA850, I2C0_SCL,        4,      8,      15,     2,      false)
> > +#endif
> 
>     And that's all? :-O
> 

I have added only the pins which are required to boot
DA850/OMAP-L138 EVM using ramdisk. Others will be added
along with the respective module patches.

> > +int da850_pinmux_setup(const short pins[])
> > +{
> > +   int i, error = -EINVAL;
> > +
> > +   if (pins)
> > +           for (i = 0; pins[i] >= 0; i++) {
> > +                   error = davinci_cfg_reg(pins[i]);
> > +                   if (error)
> > +                           break;
> > +           }
> > +
> > +   return error;
> > +}
> 
>     This duplicates da830_pinmux_setup(). I'd consider moving this function 
> into mux.c...
> 

I also think moving this to mux.c would be proper.

> > +static struct davinci_timer_instance da850_timer_instance[2] = {
> > +   {
> > +           .base           = IO_ADDRESS(DA8XX_TIMER64P0_BASE),
> > +           .bottom_irq     = IRQ_DA8XX_TINT12_0,
> > +           .top_irq        = IRQ_DA8XX_TINT34_0,
> > +   },
> > +   {
> > +           .base           = IO_ADDRESS(DA8XX_TIMER64P1_BASE),
> > +           .bottom_irq     = IRQ_DA8XX_TINT12_1,
> > +           .top_irq        = IRQ_DA8XX_TINT34_1,
> > +   },
> > +};
> 
>     Aren't there 4 timers on DA830?
> 

There are 4 timers on DA850/OMAP-L138. I'll add them in
the structure above.

> > +/*
> > + * T0_BOT: Timer 0, bottom         : Used for clock_event & clocksource
> 
>     This cannot be: you haven't supplied the 'cmp_off' and 'cmp_irq' field 
> initializers for this to work right -- I guess the compare registers just 
> don't exist on timer 0?
> 

The above comments need to be corrected. I'll correct them.

> > + * T0_TOP: Timer 0, top                    : Used by DSP
> > + * T1_BOT, T1_TOP: Timer 1, bottom & top: Used for watchdog timer
> > + */
> > +static struct davinci_timer_info da850_timer_info = {
> > +   .timers         = da850_timer_instance,
> > +   .clockevent_id  = T0_BOT,
> > +   .clocksource_id = T0_TOP,
> > +};
> 
>     Certainly need to fix this...
> 

The structure is correct. Comments need to be modified though.

> [...]
> 
> > diff --git a/arch/arm/mach-davinci/include/mach/mux.h 
> > b/arch/arm/mach-davinci/include/mach/mux.h
> > index cce7509..3349fa5 100644
> > --- a/arch/arm/mach-davinci/include/mach/mux.h
> > +++ b/arch/arm/mach-davinci/include/mach/mux.h
> > @@ -704,6 +704,34 @@ enum da830_index {
> >     DA830_GPIO2_10,
> >  };
> >  
> > +enum davinci_da850_index {
> > +   /* UART0 function */
> > +   DA850_NUART0_CTS,
> > +   DA850_NUART0_RTS,
> > +   DA850_UART0_RXD,
> > +   DA850_UART0_TXD,
> > +
> > +   /* UART1 function */
> > +   DA850_NUART1_CTS,
> > +   DA850_NUART1_RTS,
> > +   DA850_UART1_RXD,
> > +   DA850_UART1_TXD,
> > +
> > +   /* UART2 function */
> > +   DA850_NUART2_CTS,
> > +   DA850_NUART2_RTS,
> > +   DA850_UART2_RXD,
> > +   DA850_UART2_TXD,
> > +
> > +   /* I2C1 function */
> > +   DA850_I2C1_SCL,
> > +   DA850_I2C1_SDA,
> > +
> > +   /* I2C0 function */
> > +   DA850_I2C0_SDA,
> > +   DA850_I2C0_SCL,
> > +};
> 
>     And that's all?..
> 

As, I mentioned earlier, others will be added by the respective
module patches.

> > +
> >  #ifdef CONFIG_DAVINCI_MUX
> >  /* setup pin muxing */
> >  extern int davinci_cfg_reg(unsigned long reg_cfg);
> > diff --git a/arch/arm/mach-davinci/include/mach/psc.h 
> > b/arch/arm/mach-davinci/include/mach/psc.h
> > index 6b9621d..9528980 100644
> > --- a/arch/arm/mach-davinci/include/mach/psc.h
> > +++ b/arch/arm/mach-davinci/include/mach/psc.h
> > @@ -177,6 +177,10 @@
> >  #define DA8XX_LPSC1_CR_P3_SS               26
> >  #define DA8XX_LPSC1_L3_CBA_RAM             31
> >  
> > +#define DA850_LPSC1_TPCC1          0
> > +#define DA850_LPSC1_TPTC2          21
> > +#define DA850_LPSC1_SATA           8
> 
>     Please arrange #define's in the numeric order.
> 

OK.

Regards, Sudhakar



_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to