ide_initialize problem

Question about which tools to use, bugs, the best way to implement a function, etc should go here. Don't forget to see if your question is answered in the wiki first! When in doubt post here.
gerryg400
Member
Member
Posts: 1801
Joined: Thu Mar 25, 2010 11:26 pm
Location: Melbourne, Australia

Re: ide_initialize problem

Post by gerryg400 »

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.
If a trainstation is where trains stop, what is a workstation ?
User avatar
Almamu
Member
Member
Posts: 47
Joined: Wed Jul 07, 2010 9:41 am

Re: ide_initialize problem

Post by Almamu »

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...
Image
gerryg400
Member
Member
Posts: 1801
Joined: Thu Mar 25, 2010 11:26 pm
Location: Melbourne, Australia

Re: ide_initialize problem

Post by gerryg400 »

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 ?
If a trainstation is where trains stop, what is a workstation ?
gerryg400
Member
Member
Posts: 1801
Joined: Thu Mar 25, 2010 11:26 pm
Location: Melbourne, Australia

Re: ide_initialize problem

Post by gerryg400 »

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 ?
If a trainstation is where trains stop, what is a workstation ?
User avatar
Combuster
Member
Member
Posts: 9301
Joined: Wed Oct 18, 2006 3:45 am
Libera.chat IRC: [com]buster
Location: On the balcony, where I can actually keep 1½m distance
Contact:

Re: ide_initialize problem

Post by Combuster »

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.
"Certainly avoid yourself. He is a newbie and might not realize it. You'll hate his code deeply a few years down the road." - Sortie
[ My OS ] [ VDisk/SFS ]
gerryg400
Member
Member
Posts: 1801
Joined: Thu Mar 25, 2010 11:26 pm
Location: Melbourne, Australia

Re: ide_initialize problem

Post by gerryg400 »

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 ?
If a trainstation is where trains stop, what is a workstation ?
User avatar
Almamu
Member
Member
Posts: 47
Joined: Wed Jul 07, 2010 9:41 am

Re: ide_initialize problem

Post by Almamu »

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...
Image
gerryg400
Member
Member
Posts: 1801
Joined: Thu Mar 25, 2010 11:26 pm
Location: Melbourne, Australia

Re: ide_initialize problem

Post by gerryg400 »

But why

Code: Select all

& 0xFFFFFFFC
?

And why

Code: Select all

* (!BAR0)
?
If a trainstation is where trains stop, what is a workstation ?
User avatar
Almamu
Member
Member
Posts: 47
Joined: Wed Jul 07, 2010 9:41 am

Re: ide_initialize problem

Post by Almamu »

gerryg400 wrote:But why

Code: Select all

& 0xFFFFFFFC
?

And why

Code: Select all

* (!BAR0)
?
Idk why, no explanation on the wiki...
Image
User avatar
Combuster
Member
Member
Posts: 9301
Joined: Wed Oct 18, 2006 3:45 am
Libera.chat IRC: [com]buster
Location: On the balcony, where I can actually keep 1½m distance
Contact:

Re: ide_initialize problem

Post by Combuster »

What makes it that you use code that you do not understand?
"Certainly avoid yourself. He is a newbie and might not realize it. You'll hate his code deeply a few years down the road." - Sortie
[ My OS ] [ VDisk/SFS ]
User avatar
Candy
Member
Member
Posts: 3882
Joined: Tue Oct 17, 2006 11:33 pm
Location: Eindhoven

Re: ide_initialize problem

Post by Candy »

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;
Post Reply