From mboxrd@z Thu Jan 1 00:00:00 1970 From: Albert ARIBAUD Date: Mon, 06 Sep 2010 08:45:46 +0200 Subject: [U-Boot] [PATCH] IDE: Don't assume there are always two devices per bus In-Reply-To: <20100906060557.793391506AA@gemini.denx.de> References: <20100815213046.A427B1606A5@gemini.denx.de> <1281937660-25632-2-git-send-email-user@aphrodite> <4C76691D.9070701@dawes.za.net> <4C8201CB.9080606@free.fr> <20100905221935.ACE1E1506AA@gemini.denx.de> <4C848200.7080401@free.fr> <20100906060557.793391506AA@gemini.denx.de> Message-ID: <4C848E1A.1070003@free.fr> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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.