OSDev.org

Posted: **Tue Jul 20, 2010 4:27 am**

ide_buf is unsigned char

No it's not. Do you understand pointer arithmetic ? It's not enough to get rid of warnings. The code must be correct.

Posted: **Tue Jul 20, 2010 4:31 am**

gerryg400 wrote:
ide_buf is unsigned char
No it's not. Do you understand pointer arithmetic ? It's not enough to get rid of warnings. The code must be correct.

Code: Select all

unsigned char ide_buf[2048] = {0};

Thats the definition, and its more likely an unsigned int array...

Posted: **Tue Jul 20, 2010 4:57 am**

ide_buf is a pointer to unsigned char.

Code: Select all

ide_devices[count].Signature    = (unsigned int)ide_buf + ATA_IDENT_DEVICETYPE;

So you are casting a pointer to an integer and adding a constant, in this case 0. Is that what you want to do ? Why do that ?

Posted: **Tue Jul 20, 2010 5:14 am**

Code: Select all

ide_devices[count].Size   = ((unsigned int *)(ide_buf + ATA_IDENT_MAX_LBA_EXT));

I copied this line from the wiki. .Size is an unsigned int so this line will give a warning.

Listers, does this article need code review ?

Posted: **Tue Jul 20, 2010 9:18 am**

The things you need to know are on the talkpage. I nevertheless decided to be blunt and put a "do not use" warning on top for the noobs that can't read compiler messages. I think it should be stripped, with pointers to the various programming sub-articles. But those pages contain a lot of details I haven't bothered doing research into, so I'm not the best person to do the cleanup there.

Posted: **Tue Jul 20, 2010 9:45 am**

Found on wiki

Code: Select all

void ide_initialize(unsigned int BAR0, unsigned int BAR1, unsigned int BAR2, unsigned int BAR3,
unsigned int BAR4) {
 
   int j, k, count = 0;
 
   // 1- Detect I/O Ports which interface IDE Controller:
   channels[ATA_PRIMARY  ].base  = (BAR0 & 0xFFFFFFFC) + 0x1F0 * (!BAR0);
   channels[ATA_PRIMARY  ].ctrl  = (BAR1 & 0xFFFFFFFC) + 0x3F4 * (!BAR1);
   channels[ATA_SECONDARY].base  = (BAR2 & 0xFFFFFFFC) + 0x170 * (!BAR2);
   channels[ATA_SECONDARY].ctrl  = (BAR3 & 0xFFFFFFFC) + 0x374 * (!BAR3);
   channels[ATA_PRIMARY  ].bmide = (BAR4 & 0xFFFFFFFC) + 0; // Bus Master IDE
   channels[ATA_SECONDARY].bmide = (BAR4 & 0xFFFFFFFC) + 8; // Bus Master IDE

What on earth does this code do ? Well I can sort of see what it does, but why ?

Posted: **Tue Jul 20, 2010 9:47 am**

gerryg400 wrote:Found on wiki

Code: Select all

void ide_initialize(unsigned int BAR0, unsigned int BAR1, unsigned int BAR2, unsigned int BAR3,
unsigned int BAR4) {
 
   int j, k, count = 0;
 
   // 1- Detect I/O Ports which interface IDE Controller:
   channels[ATA_PRIMARY  ].base  = (BAR0 & 0xFFFFFFFC) + 0x1F0 * (!BAR0);
   channels[ATA_PRIMARY  ].ctrl  = (BAR1 & 0xFFFFFFFC) + 0x3F4 * (!BAR1);
   channels[ATA_SECONDARY].base  = (BAR2 & 0xFFFFFFFC) + 0x170 * (!BAR2);
   channels[ATA_SECONDARY].ctrl  = (BAR3 & 0xFFFFFFFC) + 0x374 * (!BAR3);
   channels[ATA_PRIMARY  ].bmide = (BAR4 & 0xFFFFFFFC) + 0; // Bus Master IDE
   channels[ATA_SECONDARY].bmide = (BAR4 & 0xFFFFFFFC) + 8; // Bus Master IDE

What on earth does this code do ? Well I can sort of see what it does, but why ?

It sets up the struct to be used by ide_write and ide_read...

Posted: **Tue Jul 20, 2010 9:50 am**

But why

Code: Select all

& 0xFFFFFFFC

?

And why

Code: Select all

* (!BAR0)

?

Posted: **Tue Jul 20, 2010 10:11 am**

gerryg400 wrote:But why
Code: Select all
& 0xFFFFFFFC
?

And why
Code: Select all
* (!BAR0)
?

Idk why, no explanation on the wiki...

Posted: **Tue Jul 20, 2010 10:14 am**

What makes it that you use code that you do not understand?

Posted: **Tue Jul 20, 2010 11:34 am**

gerryg400 wrote:But why
Code: Select all
& 0xFFFFFFFC
?

And why
Code: Select all
* (!BAR0)
?

To confuse people. It's what most people know as "optimized code"; code written for a processor instead of humans. We write code for humans and compilers though, so there's no need confusing it like that.

The "&0xFFFFFFFC" removes the bottom two bits, which indicate the type of BAR and don't form a part of the address.

The + 0x1F8 * (!BAR0) is a filthy trick. BAR0 is either a valid address or 0. If it is 0, !BAR0 is 1 (true), if it was a valid address, !BAR0 is 0. So, (0x1F8 * (!BAR0)) is either 0 if BAR0 is valid, or it is 0x1F8 when it isn't. Adding that to BAR0 results in:

0x1F8 when BAR0 is 0
BAR0 otherwise.

There are clearer ways of writing this which express how you want the reader to think:

Code: Select all

   channels[ATA_PRIMARY  ].base  = BAR0 ? (BAR0 & 0xFFFFFFFC) : 0x1F0;

OSDev.org

ide_initialize problem

Re: ide_initialize problem

Re: ide_initialize problem

Re: ide_initialize problem

Re: ide_initialize problem

Re: ide_initialize problem

Re: ide_initialize problem

Re: ide_initialize problem

Re: ide_initialize problem

Re: ide_initialize problem

Re: ide_initialize problem

Re: ide_initialize problem