Kevin,

I'll submit a patch based on your comments.

I'll also resubmit NAND and EDMA DM365 patches.

Thanks,
Sandeep

> -----Original Message-----
> From: Kevin Hilman [mailto:[email protected]]
> Sent: Friday, June 19, 2009 5:54 PM
> To: Paulraj, Sandeep
> Cc: [email protected]
> Subject: Re: [RFC][PATCH] DM365 EMAC Support
> 
> [email protected] writes:
> 
> > From: Sandeep Paulraj <[email protected]>
> >
> > Patch adds Support for EMAC in DM365.
> > Patch also configures mux setings for the EMAC on DM365 though
> > this will already be done by U-Boot as U-Boot has EMAC support
> >
> > Signed-off-by: Sandeep Paulraj <[email protected]>
> 
> This looks mostly ok, a little nit pickiness below...
> 
> > ---
> >  arch/arm/mach-davinci/board-dm365-evm.c |   58
> ++++++++++++++++++++++++++-
> >  arch/arm/mach-davinci/dm365.c           |   68
> +++++++++++++++++++++++++++++++
> >  2 files changed, 125 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/arm/mach-davinci/board-dm365-evm.c b/arch/arm/mach-
> davinci/board-dm365-evm.c
> > index 08c2f12..9cb44c3 100644
> > --- a/arch/arm/mach-davinci/board-dm365-evm.c
> > +++ b/arch/arm/mach-davinci/board-dm365-evm.c
> > @@ -19,7 +19,7 @@
> >  #include <linux/i2c.h>
> >  #include <linux/io.h>
> >  #include <linux/clk.h>
> > -
> > +#include <linux/i2c/at24.h>
> >  #include <asm/setup.h>
> >  #include <asm/mach-types.h>
> >  #include <asm/mach/arch.h>
> > @@ -34,14 +34,63 @@
> >  #include <mach/serial.h>
> >  #include <mach/common.h>
> >
> > +#define DM365_EVM_PHY_MASK         (0x2)
> > +#define DM365_EVM_MDIO_FREQUENCY   (2200000) /* PHY bus frequency */
> > +
> > +static struct at24_platform_data eeprom_info = {
> > +   .byte_len       = (256*1024) / 8,
> > +   .page_size      = 64,
> > +   .flags          = AT24_FLAG_ADDR16,
> > +   .setup          = davinci_get_mac_addr,
> > +   .context        = (void *)0x7f00,
> > +};
> > +
> > +static struct i2c_board_info i2c_info[] = {
> > +   {
> > +           I2C_BOARD_INFO("24c256", 0x50),
> > +           .platform_data  = &eeprom_info,
> > +   },
> > +};
> > +
> >  static struct davinci_i2c_platform_data i2c_pdata = {
> >     .bus_freq       = 400   /* kHz */,
> >     .bus_delay      = 0     /* usec */,
> >  };
> >
> > +static void dm365_emac_enable(void)
> 
> How about dm365_emac_configure(void), since it doesn't really
> enable anything.
> 
> > +{
> > +   /*
> > +    * Bootloaders will correctly configure the mux settings
> > +    * but kernel will still setup PINMUX and INTMUX
> > +    */
> 
> This comment is not quite right for a couple reasons:
> 
> 1) we have no idea what "bootloaders" in the wild will do.  They
> may mux EMAC while in use, and then mux something else when done.
> 
> 2) From what I gathered on #davinci today, TI u-boot only configures
> pin mux since it uses EMAC in polling mode.  IOW, u-boot does nothing
> with INTMUX.
> 
> So basically, just remove this comment, and maybe summarize the
> potential mux conflicts:  pin mux conflicts with UART1, etc.
> INTMUX conflicts with GPIO8, etc.
> 
> > +   davinci_cfg_reg(DM365_EMAC_TX_EN);
> > +   davinci_cfg_reg(DM365_EMAC_TX_CLK);
> > +   davinci_cfg_reg(DM365_EMAC_COL);
> > +   davinci_cfg_reg(DM365_EMAC_TXD3);
> > +   davinci_cfg_reg(DM365_EMAC_TXD2);
> > +   davinci_cfg_reg(DM365_EMAC_TXD1);
> > +   davinci_cfg_reg(DM365_EMAC_TXD0);
> > +   davinci_cfg_reg(DM365_EMAC_RXD3);
> > +   davinci_cfg_reg(DM365_EMAC_RXD2);
> > +   davinci_cfg_reg(DM365_EMAC_RXD1);
> > +   davinci_cfg_reg(DM365_EMAC_RXD0);
> > +   davinci_cfg_reg(DM365_EMAC_RX_CLK);
> > +   davinci_cfg_reg(DM365_EMAC_RX_DV);
> > +   davinci_cfg_reg(DM365_EMAC_RX_ER);
> > +   davinci_cfg_reg(DM365_EMAC_CRS);
> > +   davinci_cfg_reg(DM365_EMAC_MDIO);
> > +   davinci_cfg_reg(DM365_EMAC_MDCLK);
> > +
> > +   davinci_cfg_reg(DM365_INT_EMAC_RXTHRESH);
> > +   davinci_cfg_reg(DM365_INT_EMAC_RXPULSE);
> > +   davinci_cfg_reg(DM365_INT_EMAC_TXPULSE);
> > +   davinci_cfg_reg(DM365_INT_EMAC_MISCPULSE);
> > +}
> > +
> >  static void __init evm_init_i2c(void)
> >  {
> >     davinci_init_i2c(&i2c_pdata);
> > +   i2c_register_board_info(1, i2c_info, ARRAY_SIZE(i2c_info));
> >  }
> >
> >  static struct davinci_uart_config uart_config __initdata = {
> > @@ -55,8 +104,15 @@ static void __init dm365_evm_map_io(void)
> >
> >  static __init void dm365_evm_init(void)
> >  {
> > +   struct davinci_soc_info *soc_info = &davinci_soc_info;
> > +
> >     evm_init_i2c();
> >     davinci_serial_init(&uart_config);
> > +
> > +   dm365_emac_enable();
> > +
> > +   soc_info->emac_pdata->phy_mask = DM365_EVM_PHY_MASK;
> > +   soc_info->emac_pdata->mdio_max_freq = DM365_EVM_MDIO_FREQUENCY;
> >  }
> >
> >  static __init void dm365_evm_irq_init(void)
> > diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-
> davinci/dm365.c
> > index 69d35d9..811a6f5 100644
> > --- a/arch/arm/mach-davinci/dm365.c
> > +++ b/arch/arm/mach-davinci/dm365.c
> > @@ -530,9 +530,65 @@ MUX_CFG(DM365,  EMAC_RX_ER,    3,   3,     1,    1,
> false)
> >  MUX_CFG(DM365,  EMAC_CRS,  3,   2,     1,    1,     false)
> >  MUX_CFG(DM365,  EMAC_MDIO, 3,   1,     1,    1,     false)
> >  MUX_CFG(DM365,  EMAC_MDCLK,        3,   0,     1,    1,     false)
> > +
> > +INT_CFG(DM365,  INT_EDMA_CC,             2,    1,    1,     false)
> > +INT_CFG(DM365,  INT_EDMA_TC0_ERR,     3,    1,    1,     false)
> > +INT_CFG(DM365,  INT_EDMA_TC1_ERR,     4,    1,    1,     false)
> > +INT_CFG(DM365,  INT_PRTCSS,              10,    1,    1,     false)
> > +INT_CFG(DM365,  INT_EMAC_RXTHRESH,    14,    1,    1,     false)
> > +INT_CFG(DM365,  INT_EMAC_RXPULSE,     15,    1,    1,     false)
> > +INT_CFG(DM365,  INT_EMAC_TXPULSE,     16,    1,    1,     false)
> > +INT_CFG(DM365,  INT_EMAC_MISCPULSE,   17,    1,    1,     false)
> > +
> 
> Minor nit on alignment here.  The last 5 lines are shifted from the
> lines above.
> 
> >  #endif
> >  };
> >
> > +static struct emac_platform_data dm365_emac_pdata = {
> > +   .ctrl_reg_offset        = DM365_EMAC_CNTRL_OFFSET,
> > +   .ctrl_mod_reg_offset    = DM365_EMAC_CNTRL_MOD_OFFSET,
> > +   .ctrl_ram_offset        = DM365_EMAC_CNTRL_RAM_OFFSET,
> > +   .mdio_reg_offset        = DM365_EMAC_MDIO_OFFSET,
> > +   .ctrl_ram_size          = DM365_EMAC_CNTRL_RAM_SIZE,
> > +   .version                = EMAC_VERSION_2,
> > +};
> > +
> > +static struct resource dm365_emac_resources[] = {
> > +   {
> > +           .start  = DM365_EMAC_BASE,
> > +           .end    = DM365_EMAC_BASE + 0x47ff,
> > +           .flags  = IORESOURCE_MEM,
> > +   },
> > +   {
> > +           .start  = IRQ_DM365_EMAC_RXTHRESH,
> > +           .end    = IRQ_DM365_EMAC_RXTHRESH,
> > +           .flags  = IORESOURCE_IRQ,
> > +   },
> > +   {
> > +           .start  = IRQ_DM365_EMAC_RXPULSE,
> > +           .end    = IRQ_DM365_EMAC_RXPULSE,
> > +           .flags  = IORESOURCE_IRQ,
> > +   },
> > +   {
> > +           .start  = IRQ_DM365_EMAC_TXPULSE,
> > +           .end    = IRQ_DM365_EMAC_TXPULSE,
> > +           .flags  = IORESOURCE_IRQ,
> > +   },
> > +   {
> > +           .start  = IRQ_DM365_EMAC_MISCPULSE,
> > +           .end    = IRQ_DM365_EMAC_MISCPULSE,
> > +           .flags  = IORESOURCE_IRQ,
> > +   },
> > +};
> > +
> > +static struct platform_device dm365_emac_device = {
> > +   .name           = "davinci_emac",
> > +   .id             = 1,
> > +   .dev = {
> > +           .platform_data  = &dm365_emac_pdata,
> > +   },
> > +   .num_resources  = ARRAY_SIZE(dm365_emac_resources),
> > +   .resource       = dm365_emac_resources,
> > +};
> >
> >  static u8 dm365_default_priorities[DAVINCI_N_AINTC_IRQ] = {
> >     [IRQ_VDINT0]                    = 2,
> > @@ -689,6 +745,7 @@ static struct davinci_soc_info
> davinci_soc_info_dm365 = {
> >     .gpio_num               = 104,
> >     .gpio_irq               = 44,
> >     .serial_dev             = &dm365_serial_device,
> > +   .emac_pdata             = &dm365_emac_pdata,
> >     .sram_dma               = 0x00010000,
> >     .sram_len               = SZ_32K,
> >  };
> > @@ -697,3 +754,14 @@ void __init dm365_init(void)
> >  {
> >     davinci_common_init(&davinci_soc_info_dm365);
> >  }
> > +
> > +static int __init dm365_init_devices(void)
> > +{
> > +   if (!cpu_is_davinci_dm365())
> > +           return 0;
> > +
> > +   platform_device_register(&dm365_emac_device);
> > +
> > +   return 0;
> > +}
> > +postcore_initcall(dm365_init_devices);
> > --
> > 1.6.0.4
> >
> > _______________________________________________
> > Davinci-linux-open-source mailing list
> > [email protected]
> > http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

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

Reply via email to