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

[Albert ARIBAUD <albert.aribaud@free.fr>]

Hi again Wolfgang,

Le 06/09/2010 08:05, Wolfgang Denk a ?crit :
> Dear Albert ARIBAUD,
>
> In message<4C848200.7080401@free.fr>  you wrote:
>>
>>> 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?
>
> No. See the AT Attachment (ATA) and AT Attachment Packe Interface
> (ATAPI) standards.
>
>>> 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.
>
> But this is NOT an PATA controller, right?

Correct. It is a SATA controller with ATA, not PATA, compatibility.

>> 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:
>
> We should not use "IDE" code on non-IDE devices. If there are common
> code parts, these should be split off and generalized as needed
> (without reference to "ATA" or "IDE").

For IDE as such, I agree. However:

1) Some systems simply do not allow more that one IDE drive for 
mechanical reasons. I suspect this is the reason behind the 
CONFIG_SYS_IDE_MAXDEVICE option, which, as such, contradicts IDE since 
it prevents cmd_ide to handle all 2*MAXBUS devices it could.

2) More generally, the "cmd_ide.c" actually uses ATA to communicate with 
the devices it manages; thus I feel cmd_ide.c is rightly used when on 
non-IDE-but-ATA-compatible devices, and that the issue is of naming only.

Besides, since cmd_ide's maximum bus feature is purely a design choice 
not dictated by standards, I assume your rebuttal addresses maximum 
devices per bus but not maximum busses, right? Would you agree at least 
to the minimal idea of having more than two ATA-compatible busses 
managed by cmd_ide?

>> /* 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} \
>
> No. It is inherently wrong to use "*IDE*" in combination with a SATA
> device.

How about this: we could remove "IDE" altogether from config options 
which are not actually IDE related. This includes CONFIG_SYS_IDE_MAXBUS 
which is neither IDE- or ATA-related but system-defined

We would then have:

#define CONFIG_SYS_ATA_CONFIG { \
 >> 	{ KW_SATA_PORT0_OFFSET, 1}, \
 >> 	{ KW_SATA_PORT1_OFFSET, 1} \

We would also remove ATA from non-ATA related symbols used in cmd_ide, 
but so far the only one I see is CONFIG_SYS_IDE_MAXDEVICE, which is 
purely a hack to accommodate system limitations; and in my proposal, it 
would disappear from config files because it equals the sum of the "max 
device" fields in the struct.

Is this proposal better?

Amicalement,
-- 
Albert.