PS/2 Mouse help (thread crapping epidemic, do not respond)

[Ycep]

I have tried to fix my mouse driver which stands unworking for months, and I failed.
Through mouse driver has some fixes, but still...

*Old code has been removed*
Through the only remaining problem is that WaitData() always times out. Code:

Code: Select all

void WaitData()//Function for waiting PS/2 semaphores
{
    uint tmo=40; //40ms time out.
    while(tmo--)
    {
      if((inb(0x64)&1)==1)
      {
         return;
      }
      sleep(1);
    }
    DebugLog(curTime(),"Waiting PS/2 timed out.");
    return;
}

[alexfru]

Can anybody find a problem here?

You are expected to at least minimally describe it. And "not working" doesn't cut it.

[iansjack]

I can see you declaring a global variable (cur) which you don't initialize. You then use it in a switch statement, and continually increment it, which doesn't cover most of its expected values. That alone makes the behaviour of your code unpredictable.

[alexfru]

I can see you declaring a global variable (cur) which you don't initialize.

Static variables are implicitly initialized with 0 (or NULL).

[iansjack]

And is that the correct value?

[osdever]

Hello, Lukand. What's the problem?

[Brendan]

Hi,

Can anybody find a problem here?

Trying to be thorough; the problems I can see include:

• The value in "cur" (at the "switch(cur)") would go 0, 1, 2, 1, 2, 1, 2, 1, 2, 1, 2, ... This is because for "case 2:" you set it to zero and then increment, so it ends up as 1.
• There's nothing to ensure that the mouse stays synchronised with the driver. If a byte goes missing or an extra byte is received for any reason, the driver will become "out of sync" and think that (e.g.) the first byte is the second, the second byte is the third, and the third byte is the first byte of the next packet for everything it receives after that. For mouse protocols there's always a "this bit is always set" flag somewhere that the driver should use to detect if it has become "out of sync" and auto-correct (so that it doesn't end up in a prolonged or permanent "out ouf sync" state and report garbage). It can also be a good idea to use timing information to assist in keeping driver in sync (e.g. if the time between receiving the first and second byte is greater than some threshold, or if the time between receiving the second and third byte is greater than some threshold, assume that something went wrong in the middle of the the previous packet and the byte you've just received is probably the first byte of a new packet).
• There's a variety of things you could receive that aren't handled ("echo/resend", "BAT" if user reconnects mouse, etc) that should be handled (and can/will put the driver into a "out of sync" state because they're not handled)
• Immediately after the IRQ handler receives the third byte (and checks that it's likely to be correct, etc); there's no notification sent to the rest of the driver or anything else. This means that the rest of the driver or something else has to continually poll. Continual polling is extremely retarded. You need some form of "don't give me CPU time at all, until/unless I receive some kind of notification" capability in your scheduler (which typically takes the form of some kind of IPC - messages, pipes, etc).
• There is nothing that allows "extremely retarded continual polling" to be done without race conditions - e.g. the "MousePckt", "MposX" and "MposY" variables could be changed while someone is reading them, causing the reader to end up with "half and old packet combined with half a new packet".
• The code assumes that a mouse exists and does no checking to make sure a mouse does exist. Assumptions like this guarantee that it will blow up in users' faces if the number of users ever increases.
• The code assumes that a PS/2 controller exists and does no checking to make sure the PS/2 controller exists. Assumptions like this guarantee that it will blow up in users' faces.
• The code assumes that the mouse is using a specific protocol (e.g. "3 button mouse") and will fail if the mouse is using a different protocol (e.g. "5 button with scroll wheel"), and won't try to detect and enable better protocols. The end result is that (e.g.) if the mouse happens to be "5 button with scroll wheel" the driver will either send trash (wrong protocol) or the extra buttons and scroll wheel won't work.
• The bytes received are not decoded properly for any mouse protocol. Don't forget that most mouse protocols use a "sign and magnitude" format for signed numbers and don't use "two's complement" (e.g. the raw byte 10000001b is -1 and not -127).
• The mouse driver assumes it is allowed to mess with the PS/2 controller directly (and you're not using a separate PS/2 controller driver that talks to the mouse driver). This means that you can expect conflicts between different drivers for different PS/2 devices (e.g. starting or restarting a keyboard driver will probably screw up the mouse driver, starting or restarting the mouse driver will probably screw up the keyboard driver, etc). It also means that you'd have to rewrite the drivers if you ever want to support a different kind of PS/2 controller.
• The mouse driver can't work if the mouse is plugged into a different PS/2 port. For example, if I have a USB keyboard, a PS/2 mouse in the first PS/2 port, and a bar-code scanner in the second PS/2 port; then your code will assume that the bar-code scanner is a mouse and screw that up (while probably also assuming that the mouse is a keyboard and screwing that up).
• The driver assumes that there are PIC chips (when sending EOI) and does not check if PIC chips exist and won't work if the OS is using IO APICs instead. You need to abstract this with some kind of "sendEOI()" kernel function, so that the driver/s can work regardless of whether PIC chips are being used or not.
• There's no "PS/2 controller sanity checking" (to determine if the PS/2 controller is usable or faulty), and no "PS/2 mouse sanity checking" (to determine if the mouse is usable or faulty). This means that as soon as the OS meets faulty hardware it will blow up in the user's face (rather than informing the user "Hey, this driver didn't start because your hardware is faulty", which allows the user to fix the problem and prevents the user from blaming the OS when it's not the OS's fault).
• Some software (most applications) want "absolute coordinates of mouse pointer" which needs to be scaled to the screen resolution and needs an acceleration curve, and some software (some games) need "relative movement". These are not the same; and you're providing neither (it looks like you're providing "absolute coordinates that aren't scaled, without acceleration curve").

Note: I do realise that it's likely that you're already planning to fix some of these problems later (after you get the code you have now to work); I just thought it'd be nice to have a relatively complete list to work towards.


Cheers,

Brendan

[Ycep]

To be more informative, the problem is that interrupts don't get called.
Brendan, I really appreciate your time and effort used on your long & very useful post.

The value in "cur" (at the "switch(cur)") would go 0, 1, 2, 1, 2, 1, 2, 1, 2, 1, 2, ... This is because for "case 2:" you set it to zero and then increment, so it ends up as 1.

I'm so blind... Fixed.

There's nothing to ensure that the mouse stays synchronised with the driver. If a byte goes missing or an extra byte is received for any reason, the driver will become "out of sync" and think that (e.g.) the first byte is the second, the second byte is the third, and the third byte is the first byte of the next packet for everything it receives after that. For mouse protocols there's always a "this bit is always set" flag somewhere that the driver should use to detect if it has become "out of sync" and auto-correct (so that it doesn't end up in a prolonged or permanent "out ouf sync" state and report garbage). It can also be a good idea to use timing information to assist in keeping driver in sync (e.g. if the time between receiving the first and second byte is greater than some threshold, or if the time between receiving the second and third byte is greater than some threshold, assume that something went wrong in the middle of the the previous packet and the byte you've just received is probably the first byte of a new packet).

Such PS/2 controller or mouse which does that would be easily clarified as faulty. Once this happened to me, when moving mouse to left made it go up, on Windows.

There's a variety of things you could receive that aren't handled ("echo/resend", "BAT" if user reconnects mouse, etc) that should be handled (and can/will put the driver into a "out of sync" state because they're not handled)

That is scheduled.

Immediately after the IRQ handler receives the third byte (and checks that it's likely to be correct, etc); there's no notification sent to the rest of the driver or anything else. This means that the rest of the driver or something else has to continually poll. Continual polling is extremely retarded. You need some form of "don't give me CPU time at all, until/unless I receive some kind of notification" capability in your scheduler (which typically takes the form of some kind of IPC - messages, pipes, etc).

The code assumes that a mouse exists and does no checking to make sure a mouse does exist. Assumptions like this guarantee that it will blow up in users' faces if the number of users ever increases.

(For these two above) First make it then improve
Through good advice

There is nothing that allows "extremely retarded continual polling" to be done without race conditions - e.g. the "MousePckt", "MposX" and "MposY" variables could be changed while someone is reading them, causing the reader to end up with "half and old packet combined with half a new packet".

I had troubled with such problems already (with keyboard, floppy driver, I don't remember what else) and that is one of very bad problems.

The code assumes that a mouse exists and does no checking to make sure a mouse does exist. Assumptions like this guarantee that it will blow up in users' faces if the number of users ever increases.
The code assumes that a PS/2 controller exists and does no checking to make sure the PS/2 controller exists. Assumptions like this guarantee that it will blow up in users' faces.

There is high possibility of that being a problem, since Bochs do not power-on with mouse connected.
I made it large so I would not miss it when I archive this thread. (phpBB could do that)

The code assumes that the mouse is using a specific protocol (e.g. "3 button mouse") and will fail if the mouse is using a different protocol (e.g. "5 button with scroll wheel"), and won't try to detect and enable better protocols. The end result is that (e.g.) if the mouse happens to be "5 button with scroll wheel" the driver will either send trash (wrong protocol) or the extra buttons and scroll wheel won't work.

As far I know InteliMouse extensions need to be enabled so they could start working.

The bytes received are not decoded properly for any mouse protocol. Don't forget that most mouse protocols use a "sign and magnitude" format for signed numbers and don't use "two's complement" (e.g. the raw byte 10000001b is -1 and not -127).

You probably did not readed my post.

Code: Select all

MposX+=mMsg; //For now let's not check for signed bit (I will not move mouse backwards ;)  )

The mouse driver assumes it is allowed to mess with the PS/2 controller directly (and you're not using a separate PS/2 controller driver that talks to the mouse driver). This means that you can expect conflicts between different drivers for different PS/2 devices (e.g. starting or restarting a keyboard driver will probably screw up the mouse driver, starting or restarting the mouse driver will probably screw up the keyboard driver, etc). It also means that you'd have to rewrite the drivers if you ever want to support a different kind of PS/2 controller.

I have been wondered for this because it actually happened - when I would uncomment my MouseInit(); function, keyboard would stop working; while when I would return commenting again - Keyboard works instead of mouse.

The mouse driver can't work if the mouse is plugged into a different PS/2 port. For example, if I have a USB keyboard, a PS/2 mouse in the first PS/2 port, and a bar-code scanner in the second PS/2 port; then your code will assume that the bar-code scanner is a mouse and screw that up (while probably also assuming that the mouse is a keyboard and screwing that up).

Today PS/2 controller is being used just for backwards compatibility for keyboards and mices, through USB mices emulate most of PS/2. I should try coding a check is mouse connector inside.

The driver assumes that there are PIC chips (when sending EOI) and does not check if PIC chips exist and won't work if the OS is using IO APICs instead. You need to abstract this with some kind of "sendEOI()" kernel function, so that the driver/s can work regardless of whether PIC chips are being used or not.

I actually have that function but removed it from most places of my operating system. Instead I could probably just make it inline and return SendEOI(). Very good idea.

There's no "PS/2 controller sanity checking" (to determine if the PS/2 controller is usable or faulty), and no "PS/2 mouse sanity checking" (to determine if the mouse is usable or faulty). This means that as soon as the OS meets faulty hardware it will blow up in the user's face (rather than informing the user "Hey, this driver didn't start because your hardware is faulty", which allows the user to fix the problem and prevents the user from blaming the OS when it's not the OS's fault).

That is scheduled too.

Some software (most applications) want "absolute coordinates of mouse pointer" which needs to be scaled to the screen resolution and needs an acceleration curve, and some software (some games) need "relative movement". These are not the same; and you're providing neither (it looks like you're providing "absolute coordinates that aren't scaled, without acceleration curve").

Acceleration curve should not be hard to be integrated - I think that by moving mouse each , for example second should make mouse move two times faster than last second.
I'm not sure what should I do to scale it to screen. Don't I need to have DPI of screen?

[Ycep]

Now it does not use streaming mode and plus I fixed a lot of errors in it, but doing so revealed another two problems:
WaitData() function seems to not be working. (Through it seems that you people did not even looked at WaitData function. It was same as WaitSignal).

WaitData() had two problems (now only one):

• WaitData() was same as WaitSignal(). Fixed now.
• Waiting always times out.

Current code:

Code: Select all

void WaitData()//Function for waiting PS/2 semaphores
{
    uint tmo=40; //40ms time out.
    while(tmo--)
    {
		if((inb(0x64)&1)==1)
		{
			return;
		}
		sleep(1);
    }
    DebugLog(curTime(),"Waiting PS/2 timed out.");
    return;
}

[Sik]

The bytes received are not decoded properly for any mouse protocol. Don't forget that most mouse protocols use a "sign and magnitude" format for signed numbers and don't use "two's complement" (e.g. the raw byte 10000001b is -1 and not -127).

The wiki disagrees, and so does this document and this one. I can't check right now but I'm going to assume it's indeed two's complement.

I recall some mouse protocols using sign+magnitude but it doesn't look like the case here.

[BenLunt]

The bytes received are not decoded properly for any mouse protocol. Don't forget that most mouse protocols use a "sign and magnitude" format for signed numbers and don't use "two's complement" (e.g. the raw byte 10000001b is -1 and not -127).

The wiki disagrees, and so does this document and this one. I can't check right now but I'm going to assume it's indeed two's complement.

I recall some mouse protocols using sign+magnitude but it doesn't look like the case here.

You are both right and you are both wrong

There is a bit in the first byte that specifies if the whole 8-bit byte is signed or not. If the sign bit is clear, the whole 8-bit byte is a positive value from 0 to 255. If the bit is set, then the whole 8-bit byte is a standard signed 8-bit char.

Therefore, you have a positive range of 0 to 255 (0x00 -> 0xFF) and a negative range of -128 to -1 (0x80 -> 0xFF)

Code: Select all

        if (byte0 & 0x10)
          packet.x = (int) (bit8s) byte1;
        else
          packet.x = (int) (bit16u) byte1;

Hope this helps.
Ben
http://www.fysnet.net/input_and_output_devices.htm

[Ycep]

Do I really need to say it multiple times? I will not care for the sign bit while I don't repair utmost problems.
Through in most recent code there is such checking.

[Ycep]

Now it does not use streaming mode and plus I fixed a lot of errors in it, but doing so revealed another two problems:
WaitData() function seems to not be working. (Through it seems that you people did not even looked at WaitData function. It was same as WaitSignal).

WaitData() had two problems (now only one):

• WaitData() was same as WaitSignal(). Fixed now.
• Waiting always times out.

Current code:

Code: Select all

void WaitData()//Function for waiting PS/2 semaphores
{
    uint tmo=40; //40ms time out.
    while(tmo--)
    {
		if((inb(0x64)&1)==1)
		{
			return;
		}
		sleep(1);
    }
    DebugLog(curTime(),"Waiting PS/2 timed out.");
    return;
}

[Sik]

You are both right and you are both wrong

There is a bit in the first byte that specifies if the whole 8-bit byte is signed or not. If the sign bit is clear, the whole 8-bit byte is a positive value from 0 to 255. If the bit is set, then the whole 8-bit byte is a standard signed 8-bit char.

Therefore, you have a positive range of 0 to 255 (0x00 -> 0xFF) and a negative range of -128 to -1 (0x80 -> 0xFF)

Code: Select all

        if (byte0 & 0x10)
          packet.x = (int) (bit8s) byte1;
        else
          packet.x = (int) (bit16u) byte1;

Hope this helps.
Ben
http://www.fysnet.net/input_and_output_devices.htm

Um, the bit you're talking about is the sign bit... that's essentially the 9th bit of the value (mouse motion is 9-bit, not 8-bit). The range is actually -255 to +255. (EDIT: oh and in case somebody wonders why not -256... mouses that return -0 are a thing sadly and you should be aware of them)

On that note, another document just to be safe =P (OK I'll stop, three should be enough I guess)

EDIT: wait I just saw you said "sign bit" explicitly >_> Though you make it sound like you meant "signed" instead of "sign".

Do I really need to say it multiple times? I will not care for the sign bit while I don't repair utmost problems.
Through in most recent code there is such checking.

I don't want somebody else to run into this thread and not realize there's a mistake >_>

[Schol-R-LEA]

Unfortunately, Lukand, you really can't ignore the sign bit (though you did say you are dealing it with it in your current code, at least). I may be wrong here, but: while the mouse initialization should set it to send either signed or unsigned values, IIUC most mice will only work in signed mode regardless of the initialization, and if the mouse reinitializes itself and you miss the report packet for that, you will have to check the sign bit anyway.

My understanding is that the sign bit is mainly there to allow a device to produce absolute values that exceed 128; most devices won't ever do that, do to the limits of the user's physical motion relative to the device's ability to track motion across a surface. Since most devices are meant to send signals relative to the starting point in two directions on the given axis, anyway, the standard assumption (again, as I understand it, please correct me if I am wrong) is that the device is in signed mode, and the sign bit in the frame byte is meant as an escape bit for that. This means that for most mice and mouse-like devices, the value byte will always be a two's complement signed value, regardless of the initialization.

Again, do not take what I just said as given; some or all of it may be wrong. It may be that my information sources were wrong, or I misunderstood them, or that this was once true but no longer holds. I would greatly appreciate any corrections.