[U-Boot] [PATCH] IDE: Don't assume there are always two devices per bus

[Albert ARIBAUD]

Hi Wolfgang,

Le 06/09/2010 00:19, Wolfgang Denk a écrit :
> Dear Albert ARIBAUD,
>
> In message<4C8201CB.9080606 at free.fr>  you wrote:
>>
>> I think that, rather than modifying IDE_BUS(dev), you should introduce a
>> CONFIG_SYS_IDE_MAXDEVICE_PER_BUS config option that will limit how many
>> devices will be probed for on a given bus.
>> Without this config option, for each bus B there can be up to two
>> devices, numbered (B*2) and (B*2+1); with the config option, there can
>> be only one device numbered (B*2). In all cases, IDE_BUS(dev) can remain
>> defined as (dev>>  1) which will always amount to B.
>
> I'm not happy at all about this.
>
> First, CONFIG_SYS_IDE_* is for "IDE" (aka "Integrated Drive
> Electronics"), now usually references as "Parallel ATA", as defined
> by the underlying AT Attachment (ATA) and AT Attachment Packet
> Interface (ATAPI) standards.  It is my understanding that these
> standards allow for one or two devices on the bus. So if there was any
> CONFIG_SYS_IDE_MAXDEVICE_PER_BUS, it would have to be set to 2, which
> renders it useless.

You're right, and actually this is an overlook on my part, as the 
current set of port configs has _ATA_ in in addition to _IDE. Would the 
name CONFIG_SYS_ATA_IDE_MAXDEVICEPERBUS make you be happier?

> Second,  who says that one setting fits all busses in the system? What
> happens if you want (or need) to setb the limit to 1, and I insert a
> PCI PATA controller card with 2 devices attached to a bus?

Indeed; and additionally, who says you can only have two IDE busses? I 
am facing the case right now with the MV88SX6081, which is an 8-ports 
controller.

I have thus started a patch where the CONFIG_SYS_ATA_IDE{0,1}_OFFSET are 
replaced with a single one, CONFIG_SYS_ATA_IDE_OFFSETS, defined as an 
open array of values, which allows as many ports as required. For e.g. 
openrd_base, the config for ATA ports would change from:

/* ATA bus 0 is Kirkwood port 0 on openrd */
#define CONFIG_SYS_ATA_IDE0_OFFSET      KW_SATA_PORT0_OFFSET
/* ATA bus 1 is Kirkwood port 1 on openrd */
#define CONFIG_SYS_ATA_IDE1_OFFSET      KW_SATA_PORT1_OFFSET

to:

/* OpenRD has two ATA busses, provided by kirkwood */
#define CONFIG_SYS_ATA_IDE_OFFSETS { \
	KW_SATA_PORT0_OFFSET, \
	KW_SATA_PORT1_OFFSET \
}

I could easily extend my work to account for a maximum number of devices 
per bus by replacing CONFIG_SYS_ATA_IDE_OFFSETS with a more general 
CONFIG_SYS_ATA_IDE_CONFIG array of structs which would provide the port 
offset and maximum devices for each bus:

/* OpenRD's two kirkwood busses are SATA: 1 device per bux max) */
#define CONFIG_SYS_ATA_IDE_CONFIG { \
	{ KW_SATA_PORT0_OFFSET, 1}, \
	{ KW_SATA_PORT1_OFFSET, 1} \
}

> Sorry, this does not seem to fit IMO.

How about the two suggestions above?

Amicalement,
-- 
Albert.