[U-Boot-Users] IDE and x86 port
Hello all,
is anybody out there who has a working IDE device on x86 port and u-boot?
My u-boot prints:
<< Model: uSDnsi kDSFC-B 4 Firm: eR v.3010 Ser#: M 3T00580313 Type: Removable Hard Disk Capacitiy: 42188.8 MB = 41.2 GB (515899392 x 512)
But the device is a "SunDisk SDCFB-4" and has a capacity of only 7872 sectors.
7872 = 0x0000'1EC0 -> 515899392 = 0x1EC0'0000
See: SunDisk SDCFB-4 uSDnsi kDSFC-B 4
It's confusing me, because long time ago I have written a small routine to read the hard disk device information. I read 256 words into a buffer, too. To get the right capacity value I have to swap the upper and lower word of the capacity long word. u-boot does the same, but after the swap capcity has the wrong value. I have compiled the cmd_ide.c with "gcc -E" to see, to what the macros get expanded and it seems ok.
The swap seems the right way, but results into a wrong value (perhaps only on my port). Little endian <-> Big endian problem?
Best regards Juergen Beisert
Dear Jürgen,
in message 200308140926.54274.jbeisert@eurodsn.de you wrote:
My u-boot prints:
<< Model: uSDnsi kDSFC-B 4 Firm: eR v.3010 Ser#: M 3T00580313 Type: Removable Hard Disk Capacitiy: 42188.8 MB = 41.2 GB (515899392 x 512)
But the device is a "SunDisk SDCFB-4" and has a capacity of only 7872 sectors.
This is obviously an endianess problem:
"uSDnsi kDSFC-B 4" => "SunDisk SDCFB-4 "
1 2 3 4 => 2 1 4 3
The swap seems the right way, but results into a wrong value (perhaps only on my port). Little endian <-> Big endian problem?
Yes, definitely.
Best regards,
Wolfgang Denk
On Thu, Aug 14, 2003 at 11:04:36AM +0200, Wolfgang Denk wrote:
But the device is a "SunDisk SDCFB-4" and has a capacity of only 7872 sectors.
This is obviously an endianess problem:
"uSDnsi kDSFC-B 4" => "SunDisk SDCFB-4 "
1 2 3 4 => 2 1 4 3
The swap seems the right way, but results into a wrong value (perhaps only on my port). Little endian <-> Big endian problem?
Yes, definitely.
It isn't endianness as we might expect it. Hardware accesses are in 16 bit half-words. So, we might expect to need to do some swapping as we read data from the IDE interface on LSB CPUs. Yet, this isn't the case because the IO, which is nearly all 16 bit, is already done in an ordering-neutral manner.
There are two necessary changes to the IDE code. First, the ident_cpy routine does need to swap bytes. This is because the alignment of the string characters in memory is important whereas the alignment of bytes within an integer is not.
The other change appears below. For some reason, the existing IDE code reorders the half-words of the capacity value read from the IDE device. I don't have any MSB CPUs, so I cannot verify the need for this. AFAICT, it should not be necessary.
I'd make the conditional test hinge on endianness if I knew that was the reason for the swapping. So, either we add an __X86__ check, or we change the conditional.
@@ -1056,7 +1099,12 @@ #endif /* CONFIG_ATAPI */
/* swap shorts */ +#if !defined (__ARM__) dev_desc->lba = (iop->lba_capacity << 16) | (iop->lba_capacity >> 16); +#else + /* elf: it isn't clear why this code swaps the words. There's nothing i n the spec about it. */ + dev_desc->lba = iop->lba_capacity; +#endif /* assuming HD */ dev_desc->type=DEV_TYPE_HARDDISK; dev_desc->blksz=ATA_BLOCKSIZE;
Cheers.
In message 20030814154333.GD1468@buici.com you wrote:
The other change appears below. For some reason, the existing IDE code reorders the half-words of the capacity value read from the IDE device. I don't have any MSB CPUs, so I cannot verify the need for this. AFAICT, it should not be necessary.
Well, I have no LE CPU's where I can test a CF card easily.
But I can say that the current code works fine of PPC systems.
I'd make the conditional test hinge on endianness if I knew that was the reason for the swapping. So, either we add an __X86__ check, or we change the conditional.
@@ -1056,7 +1099,12 @@ #endif /* CONFIG_ATAPI */
/* swap shorts */+#if !defined (__ARM__)
I will reject such a patch. This is definitely not an ARM / not ARM issue.
+#else
/* elf: it isn't clear why this code swaps the words. There's nothing i
This comment puzzles me: what's "elf"? ELF file format??
Best regards,
Wolfgang Denk
On Thu, Aug 14, 2003 at 06:17:38PM +0200, Wolfgang Denk wrote:
I'd make the conditional test hinge on endianness if I knew that was the reason for the swapping. So, either we add an __X86__ check, or we change the conditional.
@@ -1056,7 +1099,12 @@ #endif /* CONFIG_ATAPI */
/* swap shorts */+#if !defined (__ARM__)
I will reject such a patch. This is definitely not an ARM / not ARM issue.
Are there any other MSB cpu's in u-boot?
Hello Marc,
Am Donnerstag, 14. August 2003 17:43 schrieb Marc Singer:
/* swap shorts */+#if !defined (__ARM__) dev_desc->lba = (iop->lba_capacity << 16) | (iop->lba_capacity >> 16); +#else
/* elf: it isn't clear why this code swaps the words. There'snothing i n the spec about it. */
dev_desc->lba = iop->lba_capacity;+#endif /* assuming HD */ dev_desc->type=DEV_TYPE_HARDDISK; dev_desc->blksz=ATA_BLOCKSIZE;
Perhaps
dev_desc->lba = __le32_to_cpu(iop->lba_capacity);
would be easier?
In the ide buffer I see at address &iop->lba_capacity: 0->0x00 1->0xe9 2->0x01 3->0x00. It looks like little endian, because my CompactFlash has 125184 (0x0001E900) sectors.
I have changed my ident_cpy() to:
static void ident_cpy (unsigned char *dest, unsigned char *src, unsigned int len) { unsigned short *pus,us; unsigned char *pc;
pus=(unsigned short*)src; pc=dest; len &= -4;
while (len) { us=__le16_to_cpu(*pus); /* swap bytes in word */ if (!(*pc++=us>>8)) break; if (!(*pc++=us)) break; pus++; len-=2; } *pc='\0';
pc=dest; while(*pc == ' ') /* skip trailing whitespace */ pc++; while(*pc) /* copy text itself */ *dest++=*pc++; *dest--=*pc; /* don't forget the \0! */ while(*dest==' ') /* skip leading whitespace */ *dest--='\0'; }
And I also think the members - unsigned char revision[8]; - unsigned char vendor[40]; - unsigned char product[20]; in structure block_dev_desc have to increase by one char, because the string readed from the ide buffer could have the same size (without a leading '\0'!). And in this case, where to place the terminating '\0'?
Hope it helps
Regards Juergen Beisert
On Thu, Aug 14, 2003 at 06:22:43PM +0200, Juergen Beisert wrote:
Hello Marc,
Am Donnerstag, 14. August 2003 17:43 schrieb Marc Singer:
/* swap shorts */+#if !defined (__ARM__) dev_desc->lba = (iop->lba_capacity << 16) | (iop->lba_capacity >> 16); +#else
/* elf: it isn't clear why this code swaps the words. There'snothing i n the spec about it. */
dev_desc->lba = iop->lba_capacity;+#endif /* assuming HD */ dev_desc->type=DEV_TYPE_HARDDISK; dev_desc->blksz=ATA_BLOCKSIZE;
Perhaps
dev_desc->lba = __le32_to_cpu(iop->lba_capacity);
would be easier?
It is the MSB CPU's that do the swap, not the LE ones.
I have made a patch to the IDE code that ought to fix some of this.
I don't think this is a big vs. little problem. It has to do with the odd way that data is stored in the CF card.
Wolfgang, I've sent a patch with this (and other things). Have you had a chance to look at it?
On Thu, Aug 14, 2003 at 09:26:54AM +0200, Juergen Beisert wrote:
Hello all,
is anybody out there who has a working IDE device on x86 port and u-boot?
My u-boot prints:
<< Model: uSDnsi kDSFC-B 4 Firm: eR v.3010 Ser#: M 3T00580313 Type: Removable Hard Disk Capacitiy: 42188.8 MB = 41.2 GB (515899392 x 512)
But the device is a "SunDisk SDCFB-4" and has a capacity of only 7872 sectors.
7872 = 0x0000'1EC0 -> 515899392 = 0x1EC0'0000
See: SunDisk SDCFB-4 uSDnsi kDSFC-B 4
participants (3)
-
Juergen Beisert -
Marc Singer -
Wolfgang Denk