| Message ID | 1246976397-7980-1-git-send-email-hemantp@ti.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
Hello. Hemant Pedanekar wrote: > This patch depends on "Add clock info and update mux setup for ATA" patch > submitted earlier. > An I2C driver is added for controlling the CPLD register 0, which drives > ATA_RSTn and ATA_PWD. > Signed-off-by: Hemant Pedanekar <hemantp@ti.com> [...] > diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c > index 575c6ca..99382d8 100644 > --- a/arch/arm/mach-davinci/board-dm646x-evm.c > +++ b/arch/arm/mach-davinci/board-dm646x-evm.c > @@ -44,6 +44,7 @@ > #include <mach/common.h> > #include <mach/psc.h> > #include <mach/serial.h> > +#include <mach/mux.h> > #include <mach/i2c.h> > #include <mach/mmc.h> > #include <mach/emac.h> > @@ -52,10 +53,88 @@ > #define DM646X_EVM_PHY_MASK (0x2) > #define DM646X_EVM_MDIO_FREQUENCY (2200000) /* PHY bus frequency */ > > +#define DAVINCI_CFC_ATA_BASE 0x01C66000 > + > static struct davinci_uart_config uart_config __initdata = { > .enabled_uarts = (1 << 0), > }; > > +static struct resource ide_resources[] = { > + { > + .start = DAVINCI_CFC_ATA_BASE, > + .end = DAVINCI_CFC_ATA_BASE + 0x7ff, > + .flags = IORESOURCE_MEM, > + }, > + { > + .start = IRQ_DM646X_IDE, > + .end = IRQ_DM646X_IDE, > + .flags = IORESOURCE_IRQ, > + }, > +}; > + > +static u64 ide_dma_mask = DMA_BIT_MASK(32); > + > +static struct platform_device ide_dev = { > + .name = "palm_bk3710", > + .id = -1, > + .resource = ide_resources, > + .num_resources = ARRAY_SIZE(ide_resources), > + .dev = { > + .dma_mask = &ide_dma_mask, > + .coherent_dma_mask = DMA_BIT_MASK(32), > + }, > +}; IDE is not board specific device -- why in the world are you adding it to the board file? Ah, I see: it's done that way in board-dm646x-evm.c... but I don't think it's correct. Kevin? > +/* CPLD Register 0: used for I/O Control */ > +static struct i2c_client *cpld_reg0_client; > + > +static int cpld_reg0_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + char data; > + struct i2c_msg msg[2] = { > + { > + .addr = client->addr, > + .flags = I2C_M_RD, > + .len = 1, > + .buf = &data, > + }, > + { > + .addr = client->addr, > + .flags = 0, > + .len = 1, > + .buf = (void __force *)&data, Why cast in this case and no cast in the previous case? > + }, > + }; > + > + cpld_reg0_client = client; > + > + /* Control on-board CPLD to enable ATA */ > + i2c_transfer(cpld_reg0_client->adapter, msg, 1); > + data &= ~3; > + i2c_transfer(cpld_reg0_client->adapter, msg + 1, 1); Ah, this controls ATA SEL and PWD signals... you could at least comment about this. > @@ -265,6 +348,20 @@ static void __init davinci_map_io(void) > dm646x_init(); > } > > +#if defined(CONFIG_IDE) || \ > + defined(CONFIG_IDE_MODULE) > +#define HAS_ATA 1 > +#else > +#define HAS_ATA 0 > +#endif > + > +#if defined(CONFIG_BLK_DEV_PALMCHIP_BK3710) || \ > + defined(CONFIG_BLK_DEV_PALMCHIP_BK3710_MODULE) > +#define HAS_ATA 1 > +#else > +#define HAS_ATA 0 This would redefine already defined HAS_ATA. MBR, Sergei
Sergei Shtylyov <sshtylyov@ru.mvista.com> writes: > Hello. > > Hemant Pedanekar wrote: > >> This patch depends on "Add clock info and update mux setup for ATA" patch >> submitted earlier. > >> An I2C driver is added for controlling the CPLD register 0, which drives >> ATA_RSTn and ATA_PWD. > >> Signed-off-by: Hemant Pedanekar <hemantp@ti.com> > > [...] > >> diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c >> index 575c6ca..99382d8 100644 >> --- a/arch/arm/mach-davinci/board-dm646x-evm.c >> +++ b/arch/arm/mach-davinci/board-dm646x-evm.c >> @@ -44,6 +44,7 @@ >> #include <mach/common.h> >> #include <mach/psc.h> >> #include <mach/serial.h> >> +#include <mach/mux.h> >> #include <mach/i2c.h> >> #include <mach/mmc.h> >> #include <mach/emac.h> >> @@ -52,10 +53,88 @@ >> #define DM646X_EVM_PHY_MASK (0x2) >> #define DM646X_EVM_MDIO_FREQUENCY (2200000) /* PHY bus frequency */ >> +#define DAVINCI_CFC_ATA_BASE 0x01C66000 >> + >> static struct davinci_uart_config uart_config __initdata = { >> .enabled_uarts = (1 << 0), >> }; >> +static struct resource ide_resources[] = { >> + { >> + .start = DAVINCI_CFC_ATA_BASE, >> + .end = DAVINCI_CFC_ATA_BASE + 0x7ff, >> + .flags = IORESOURCE_MEM, >> + }, >> + { >> + .start = IRQ_DM646X_IDE, >> + .end = IRQ_DM646X_IDE, >> + .flags = IORESOURCE_IRQ, >> + }, >> +}; >> + >> +static u64 ide_dma_mask = DMA_BIT_MASK(32); >> + >> +static struct platform_device ide_dev = { >> + .name = "palm_bk3710", >> + .id = -1, >> + .resource = ide_resources, >> + .num_resources = ARRAY_SIZE(ide_resources), >> + .dev = { >> + .dma_mask = &ide_dma_mask, >> + .coherent_dma_mask = DMA_BIT_MASK(32), >> + }, >> +}; > > IDE is not board specific device -- why in the world are you adding > it to the board file? Ah, I see: it's done that way in > board-dm646x-evm.c... but I don't think it's correct. Kevin? I agree. IDE is SoC specific and should be done there. What we need is the platform_device and an init/register function in <soc>.c (which includes mux init.) Then, the board code calls the register function if it wants IDE enabled for that board. Kevin
Sergei, Kevin, Thanks, I will incorporate your suggestions and send v2 for both patches. - Hemant > -----Original Message----- > From: Kevin Hilman [mailto:khilman@deeprootsystems.com] > Sent: Tuesday, July 07, 2009 10:11 PM > To: Sergei Shtylyov > Cc: Pedanekar, Hemant; davinci-linux-open-source@linux.davincidsp.com > Subject: Re: [PATCH] davinci: dm646x-evm: Add platform and resource data > and cpld setup for IDE > > Sergei Shtylyov <sshtylyov@ru.mvista.com> writes: > > > Hello. > > > > Hemant Pedanekar wrote: > > > >> This patch depends on "Add clock info and update mux setup for ATA" > patch > >> submitted earlier. > > > >> An I2C driver is added for controlling the CPLD register 0, which > drives > >> ATA_RSTn and ATA_PWD. > > > >> Signed-off-by: Hemant Pedanekar <hemantp@ti.com> > > > > [...] > > > >> diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach- > davinci/board-dm646x-evm.c > >> index 575c6ca..99382d8 100644 > >> --- a/arch/arm/mach-davinci/board-dm646x-evm.c > >> +++ b/arch/arm/mach-davinci/board-dm646x-evm.c > >> @@ -44,6 +44,7 @@ > >> #include <mach/common.h> > >> #include <mach/psc.h> > >> #include <mach/serial.h> > >> +#include <mach/mux.h> > >> #include <mach/i2c.h> > >> #include <mach/mmc.h> > >> #include <mach/emac.h> > >> @@ -52,10 +53,88 @@ > >> #define DM646X_EVM_PHY_MASK (0x2) > >> #define DM646X_EVM_MDIO_FREQUENCY (2200000) /* PHY bus frequency > */ > >> +#define DAVINCI_CFC_ATA_BASE 0x01C66000 > >> + > >> static struct davinci_uart_config uart_config __initdata = { > >> .enabled_uarts = (1 << 0), > >> }; > >> +static struct resource ide_resources[] = { > >> + { > >> + .start = DAVINCI_CFC_ATA_BASE, > >> + .end = DAVINCI_CFC_ATA_BASE + 0x7ff, > >> + .flags = IORESOURCE_MEM, > >> + }, > >> + { > >> + .start = IRQ_DM646X_IDE, > >> + .end = IRQ_DM646X_IDE, > >> + .flags = IORESOURCE_IRQ, > >> + }, > >> +}; > >> + > >> +static u64 ide_dma_mask = DMA_BIT_MASK(32); > >> + > >> +static struct platform_device ide_dev = { > >> + .name = "palm_bk3710", > >> + .id = -1, > >> + .resource = ide_resources, > >> + .num_resources = ARRAY_SIZE(ide_resources), > >> + .dev = { > >> + .dma_mask = &ide_dma_mask, > >> + .coherent_dma_mask = DMA_BIT_MASK(32), > >> + }, > >> +}; > > > > IDE is not board specific device -- why in the world are you adding > > it to the board file? Ah, I see: it's done that way in > > board-dm646x-evm.c... but I don't think it's correct. Kevin? > > I agree. > > IDE is SoC specific and should be done there. What we need is the > platform_device and an init/register function in <soc>.c (which > includes mux init.) Then, the board code calls the register > function if it wants IDE enabled for that board. > > Kevin
diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c index 575c6ca..99382d8 100644 --- a/arch/arm/mach-davinci/board-dm646x-evm.c +++ b/arch/arm/mach-davinci/board-dm646x-evm.c @@ -44,6 +44,7 @@ #include <mach/common.h> #include <mach/psc.h> #include <mach/serial.h> +#include <mach/mux.h> #include <mach/i2c.h> #include <mach/mmc.h> #include <mach/emac.h> @@ -52,10 +53,88 @@ #define DM646X_EVM_PHY_MASK (0x2) #define DM646X_EVM_MDIO_FREQUENCY (2200000) /* PHY bus frequency */ +#define DAVINCI_CFC_ATA_BASE 0x01C66000 + static struct davinci_uart_config uart_config __initdata = { .enabled_uarts = (1 << 0), }; +static struct resource ide_resources[] = { + { + .start = DAVINCI_CFC_ATA_BASE, + .end = DAVINCI_CFC_ATA_BASE + 0x7ff, + .flags = IORESOURCE_MEM, + }, + { + .start = IRQ_DM646X_IDE, + .end = IRQ_DM646X_IDE, + .flags = IORESOURCE_IRQ, + }, +}; + +static u64 ide_dma_mask = DMA_BIT_MASK(32); + +static struct platform_device ide_dev = { + .name = "palm_bk3710", + .id = -1, + .resource = ide_resources, + .num_resources = ARRAY_SIZE(ide_resources), + .dev = { + .dma_mask = &ide_dma_mask, + .coherent_dma_mask = DMA_BIT_MASK(32), + }, +}; + +/* CPLD Register 0: used for I/O Control */ +static struct i2c_client *cpld_reg0_client; + +static int cpld_reg0_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + char data; + struct i2c_msg msg[2] = { + { + .addr = client->addr, + .flags = I2C_M_RD, + .len = 1, + .buf = &data, + }, + { + .addr = client->addr, + .flags = 0, + .len = 1, + .buf = (void __force *)&data, + }, + }; + + cpld_reg0_client = client; + + /* Control on-board CPLD to enable ATA */ + i2c_transfer(cpld_reg0_client->adapter, msg, 1); + data &= ~3; + i2c_transfer(cpld_reg0_client->adapter, msg + 1, 1); + + return 0; +} + +static int cpld_reg0_remove(struct i2c_client *client) +{ + cpld_reg0_client = NULL; + return 0; +} + +static const struct i2c_device_id cpld_reg_ids[] = { + { "cpld_reg0", 0, }, + { /* end of list */ }, +}; + +static struct i2c_driver dm6467evm_cpld_driver = { + .driver.name = "cpld_reg0", + .id_table = cpld_reg_ids, + .probe = cpld_reg0_probe, + .remove = cpld_reg0_remove, +}; + /* LEDS */ static struct gpio_led evm_leds[] = { @@ -247,6 +326,9 @@ static struct i2c_board_info __initdata i2c_info[] = { I2C_BOARD_INFO("pcf8574a", 0x38), .platform_data = &pcf_data, }, + { + I2C_BOARD_INFO("cpld_reg0", 0x3a), + }, }; static struct davinci_i2c_platform_data i2c_pdata = { @@ -257,6 +339,7 @@ static struct davinci_i2c_platform_data i2c_pdata = { static void __init evm_init_i2c(void) { davinci_init_i2c(&i2c_pdata); + i2c_add_driver(&dm6467evm_cpld_driver); i2c_register_board_info(1, i2c_info, ARRAY_SIZE(i2c_info)); } @@ -265,6 +348,20 @@ static void __init davinci_map_io(void) dm646x_init(); } +#if defined(CONFIG_IDE) || \ + defined(CONFIG_IDE_MODULE) +#define HAS_ATA 1 +#else +#define HAS_ATA 0 +#endif + +#if defined(CONFIG_BLK_DEV_PALMCHIP_BK3710) || \ + defined(CONFIG_BLK_DEV_PALMCHIP_BK3710_MODULE) +#define HAS_ATA 1 +#else +#define HAS_ATA 0 +#endif + static __init void evm_init(void) { struct davinci_soc_info *soc_info = &davinci_soc_info; @@ -274,6 +371,11 @@ static __init void evm_init(void) dm646x_init_mcasp0(&dm646x_evm_snd_data[0]); dm646x_init_mcasp1(&dm646x_evm_snd_data[1]); + if (HAS_ATA) { + davinci_cfg_reg(DM646X_ATAEN); + platform_device_register(&ide_dev); + } + soc_info->emac_pdata->phy_mask = DM646X_EVM_PHY_MASK; soc_info->emac_pdata->mdio_max_freq = DM646X_EVM_MDIO_FREQUENCY; }
This patch depends on "Add clock info and update mux setup for ATA" patch submitted earlier. An I2C driver is added for controlling the CPLD register 0, which drives ATA_RSTn and ATA_PWD. Signed-off-by: Hemant Pedanekar <hemantp@ti.com> --- arch/arm/mach-davinci/board-dm646x-evm.c | 102 ++++++++++++++++++++++++++++++ 1 files changed, 102 insertions(+), 0 deletions(-)