From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven A. Falco Date: Fri, 15 Aug 2008 11:23:24 -0400 Subject: [U-Boot] [RFC] setting pio modes for IDE devices In-Reply-To: <20080814164225.GD23097@game.jcrosoft.org> References: <48A35533.1010901@harris.com> <20080813232020.1BCC6248CD@gemini.denx.de> <48A44235.2010606@harris.com> <20080814164225.GD23097@game.jcrosoft.org> Message-ID: <48A59F6C.3080907@harris.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Jean-Christophe PLAGNIOL-VILLARD wrote: >> --- >> common/cmd_ide.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ >> include/ata.h | 4 +++- >> 2 files changed, 55 insertions(+), 1 deletions(-) >> >> diff --git a/common/cmd_ide.c b/common/cmd_ide.c >> index b4d9719..0e435a7 100644 >> --- a/common/cmd_ide.c >> +++ b/common/cmd_ide.c >> @@ -167,6 +167,10 @@ static void input_data(int dev, ulong *sect_buf, int words); >> static void output_data(int dev, ulong *sect_buf, int words); >> static void ident_cpy (unsigned char *dest, unsigned char *src, unsigned int len); >> >> +#ifdef CONFIG_TUNE_PIO >> +int inline ide_set_piomode(int pio_mode); >> +#endif >> + >> #ifndef CFG_ATA_PORT_ADDR >> #define CFG_ATA_PORT_ADDR(port) (port) >> #endif >> @@ -838,6 +842,16 @@ __ide_inb(int dev, int port) >> unsigned char inline ide_inb(int dev, int port) >> __attribute__((weak, alias("__ide_inb"))); >> > why not do this? > > int inline ide_set_piomode(int pio_mode) __attribute__((weak)); >> #ifdef __PPC__ >> # ifdef CONFIG_AMIGAONEG3SE >> static void >> @@ -1054,6 +1068,10 @@ static void ide_ident (block_dev_desc_t *dev_desc) >> int do_retry = 0; >> #endif >> >> +#ifdef CONFIG_TUNE_PIO >> + int pio_mode; >> +#endif >> + >> #if 0 >> int mode, cycle_time; >> #endif >> @@ -1169,6 +1187,40 @@ static void ide_ident (block_dev_desc_t *dev_desc) >> else >> dev_desc->removable = 0; >> >> +#ifdef CONFIG_TUNE_PIO >> + /* Mode 0 - 2 only, are directly determined by word 51. */ >> + pio_mode = iop->tPIO; >> + if(pio_mode > 2) { >> + printf("WARNING: Invalid PIO (word 51 = %d).\n", pio_mode); >> + pio_mode = 0; /* Force it to dead slow, and hope for the best... */ >> + } >> + >> + /* Any CompactFlash Storage Card that supports PIO mode 3 or above >> + * shall set bit 1 of word 53 to one and support the fields contained >> + * in words 64 through 70. >> + */ >> + if(iop->field_valid & 0x02) { >> + /* Mode 3 and above are possible. Check in order from slow >> + * to fast, so we wind up with the highest mode allowed. >> + */ >> + if(iop->eide_pio_modes & 0x01) { >> + pio_mode = 3; >> + } >> + if(iop->eide_pio_modes & 0x02) { >> + pio_mode = 4; >> + } >> + if((iop->cf_advanced_caps & 0x07) == 0x01) { >> + pio_mode = 5; >> + } >> + if((iop->cf_advanced_caps & 0x07) == 0x02) { >> + pio_mode = 6; >> + } >> + } >> + >> + /* System-specific, depends on bus speeds, etc. */ > > if(ide_set_piomode) > ide_set_piomode(pio_mode); > > if no ide_set_piomode implementation is present gcc will drop it at > compile time IIRC. > > Best Regards, > J. > At least on the x86, this generates an explicit test, even with -O2. The generated code looks something like this: 80483cd: b8 00 00 00 00 mov $0x0,%eax 80483d2: 85 c0 test %eax,%eax 80483d4: 74 0c je 80483e2 80483d6: c7 04 24 0a 00 00 00 movl $0xa,(%esp) 80483dd: e8 1e 7c fb f7 call 0 <_init-0x8048250> Is there some explicit optimization flag that causes gcc to drop it at compile time? I basically was following the style of the existing code regarding weak linkage. See for example the ide_outb() implementation. Also, I don't think your approach eliminates any ifdefs, because we still would not want to perform the pio_mode calculation unless we intended to use the result. Steve