Re: [U-Boot] IDE_BUS unconditionally expects 2 devices per bus

2010-08-14 Thread Albert ARIBAUD
Le 14/08/2010 12:46, Rogan Dawes a écrit :
> On 2010/08/14 12:41 PM, Rogan Dawes wrote:
>> -#defineIDE_BUS(dev)(dev>>   1)
>> +#defineIDE_BUS(dev)(dev>>   (CONFIG_SYS_IDE_MAXDEVICE /
>> CONFIG_SYS_IDE_MAXBUS - 1))
>>
>> #defineATA_CURR_BASE(dev)
>> (CONFIG_SYS_ATA_BASE_ADDR+ide_bus_offset[IDE_BUS(dev)])
>
> Ok, I'm an idiot! The reason was staring me in the face!
> ATA_CURR_BASE(dev) relies on IDE_BUS, which is why the same disk was
> being enumerated twice.
>
> It seems that the above patch is indeed correct.

Good findings, Rogan. :)

However, you should submit patches using git format-patch and git 
send-email, both properly configured -- make sure format-patch has the 
-s option and send-email has the proper e-mail address settings for 
sender and recipient(s).

And before submitting, think of checking the patch with linux's 
script/checkpatch.pl. --no-tree. At least it'll let you know about the 
long line. Mind you, ide.h itself has several long lines, that 
checkpatch won't tell you about since this is outside your patch. :)

I applied the change manually, With 2 busse and 2 devices on the ED Mini 
(which has only one disk), I don't get the duplicate drive. With 1 bus 
and 2 devices, I do see the disk twice. :(

Amicalement,
-- 
Albert.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] IDE_BUS unconditionally expects 2 devices per bus

2010-08-14 Thread Rogan Dawes
On 2010/08/14 12:41 PM, Rogan Dawes wrote:
> -#defineIDE_BUS(dev)(dev>>  1)
> +#defineIDE_BUS(dev)(dev>>  (CONFIG_SYS_IDE_MAXDEVICE /
> CONFIG_SYS_IDE_MAXBUS - 1))
>
>#defineATA_CURR_BASE(dev)
> (CONFIG_SYS_ATA_BASE_ADDR+ide_bus_offset[IDE_BUS(dev)])

Ok, I'm an idiot! The reason was staring me in the face! 
ATA_CURR_BASE(dev) relies on IDE_BUS, which is why the same disk was 
being enumerated twice.

It seems that the above patch is indeed correct.

Rogan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] IDE_BUS unconditionally expects 2 devices per bus

2010-08-14 Thread Rogan Dawes
Hi folks

Since include/ide.h defines IDE_BUS(dev) as (dev >> 1), it ignores the 
values of CONFIG_SYS_IDE_MAXBUS and CONFIG_SYS_MAXDEVICE, and 
unconditionally expects an IDE bus to have two devices.

This expectation falls down with the Orion5x SATA support, which uses 
that SATA controller in IDE compatability mode, but can obviously only 
have one device per bus.

I'm not 100% sure whether the problem is with the IDE code, or with the 
Orion5x code which identifies a drive at both "positions" on the IDE 
bus. i.e. if I define CONFIG_SYS_IDE_MAXDEVICE as 
(CONFIG_SYS_IDE_MAXBUS*2), I get the first drive shown twice, and the 
second drive shown twice.

The following RFC patch works for me, but may well break other boards 
that do not define CONFIG_SYS_IDE_MAXDEVICE and CONFIG_SYS_IDE_MAXBUS 
properly:

diff --git a/include/ide.h b/include/ide.h
index 6a1b7ae..3f81fb1 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -24,7 +24,7 @@
  #ifndef_IDE_H
  #define _IDE_H

-#defineIDE_BUS(dev)(dev >> 1)
+#defineIDE_BUS(dev)(dev >> (CONFIG_SYS_IDE_MAXDEVICE / 
CONFIG_SYS_IDE_MAXBUS - 1))

  #defineATA_CURR_BASE(dev) 
(CONFIG_SYS_ATA_BASE_ADDR+ide_bus_offset[IDE_BUS(dev)])

Ok, I'm sure it is line wrapped, tab-damaged, etc, and the line itself 
is too long, but conceptually, would that be acceptable?

Thanks

Rogan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot