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 <main+0x32>
 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to