
Le 06/09/2010 10:18, Wolfgang Denk a écrit :
I don't understand what you want to tell me. As far as PATA is concerned (and this is all what "*_IDE_*" config options refer to), we can have zero, one or two devices per bus, and the code supports this.
For systems, where only one device can be physically attached, CONFIG_SYS_IDE_MAXDEVICE allows to save memory footprint because in common/cmd_ide.c we allocate only as many device descriptors as we need:
block_dev_desc_t ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
This is just an implementation detail to avoid wasting memory; it has nothing to do with restrictions in implementing the PATA specs.
Agreed -- more comments on CONFIG_SYS_IDE_MAXDEVICE below.
- 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.
Maybe. But if you are using it on such devices, please make sure not to apply terms that make sense on IDE / PATA devices only.
Besides, since cmd_ide's maximum bus feature is purely a design choice not dictated by standards, I assume your rebuttal addresses maximum
This "design choice" is merely the coinsequence of providing a static allocation of data structures with minimal memory footprint. This is OK for typical embedded systems where the configuration is inherently static and known in advance.
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?
Yes, I agree. But this is not a new feature, but already supported out of the box, isn't it?
Er... no, it isn't. cmd_ide can support no more than two busses, the offsets of which are defined by CONFIG_SYS_ATA_IDE0_OFFSET, and optional CONFIG_SYS_ATA_IDE1_OFFSET if CONFIG_SYS_IDE_MAXBUS > 0 (note that these have _IDE in their names although they are not IDE per se).
So you cannot have more than two busses.
Moreover, the way it is done, if you want a third bus you have to modify cmd_ide.c to introduce CONFIG_SYS_ATA_IDE2_OFFSET, and once again later if you need a fourth one -- this does not scale well.
OTOH, my proposal to group offsets in CONFIG_SYS_ATA_IDE_OFFSETS ("IDE" kept in name to match the original CONFIG_SYS_ATA_IDEx_OFFSET names, but I can live without this "IDE") makes it scalable: the config option provides both the number of busses and their offsets, and the cmd_ide code would need no change to accomodate any number of busses.
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
It may be "system-defined" (what exactly do you mean by that term?), but it is ATA-related, isn't it?
I meant that neither IDE or ATA standards restrict the maximum number of busses that can coexist in a given system; only the system designer can introduce such a limit.
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
What makes you think this is a "hack"?
I don't mean 'hack' in any negative way; what I mean is that restricting the number of devices to less than twice the number of busses is not mandated by any standard, and aims not at providing functionality but at reducing footprint.
would disappear from config files because it equals the sum of the "max device" fields in the struct.
This would just waste memory for data structures which will never be used.
I don't think there would be a waste of memory:
1) The new CONFIG_SYS_ATA_[IDE_]OFFSETS will not consome global memory, as it is only needed in ide_init() where it can initialize a local variable that ide_init() will run through to fill the existing ide_bus_offset[] and ide_dev_desc[].
2) as for ide_bus_offset[] and ide_dev_desc[], and any other existing array based on CONFIG_SYS_IDE_MAXBUS or CONFIG_SYS_IDE_MAXDEVICE, they are not going to grow any bigger with my proposal since neither config option will increase.
Amicalement,