[U-Boot-Users] IDE and x86 port

[U-Boot-Users] IDE and x86 port

[Juergen Beisert]

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
-- 
*********************************
EuroDesign
embedded technologies GmbH
Waldstr. 4a
85414 Kirchdorf a.d. Amper/Germany

Tel: +49/8166/99495-77
Fax: +49/8166/99495-81

EMAIL: jbeisert at eurodsn.de
Web: http://www.eurodsn.de
*********************************

[U-Boot-Users] IDE and x86 port

[Wolfgang Denk]

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

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
I was playing poker the other night... with Tarot cards. I got a full
house and 4 people died.                              - Steven Wright

[U-Boot-Users] IDE and x86 port

[Marc Singer]

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.

[U-Boot-Users] IDE and x86 port

[Wolfgang Denk]

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

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
"Intelligence without character is a dangerous thing."   - G. Steinem

[U-Boot-Users] IDE and x86 port

[Marc Singer]

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?

 

[U-Boot-Users] IDE and x86 port

[Juergen Beisert]

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'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;

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
-- 
*********************************
EuroDesign
embedded technologies GmbH
Waldstr. 4a
85414 Kirchdorf a.d. Amper/Germany

Tel: +49/8166/99495-77
Fax: +49/8166/99495-81

EMAIL: jbeisert at eurodsn.de
Web: http://www.eurodsn.de
*********************************

[U-Boot-Users] IDE and x86 port

[Marc Singer]

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'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;
> 
> 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.

[U-Boot-Users] IDE and x86 port

[Marc Singer]

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

[U-Boot-Users] IDE and x86 port

[Wolfgang Denk]

Dear Mark,

in message <20030814170613.GA15834@buici.com> you wrote:
> > 
> > That doesn't matter. It is not a ARM specific feature.
> 
> There currently aren't macros for endianness.  The compilers don't

Yes, there are. Just #include <include/asm/byteorder.h>

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
In an infinite universe all things are possible, including the possi-
bility that the universe does not exist.
                        - Terry Pratchett, _The Dark Side of the Sun_