x86: i8042: Give the keyboard controller a little TLC.

Information
Submitter:	Gabe Black
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 10612:740dbb64ce34
---------------------------
x86: i8042: Give the keyboard controller a little TLC.

This CL fixes a few minor bugs, and fills out the general implementation a bit
so that it can more readily support actually returning keyboard input.

Testing Done

Issue Summary

Description	From	Last Updated	Status
We don't need this nack().	Nilay Vaish	Nov. 25, 2014, 2:23 p.m.	Open
Is some code missing here? You did not write anything to the output port.	Nilay Vaish	Nov. 25, 2014, 2:23 p.m.	Open
Do we need this change? This does not exceed the line length limit.	Nilay Vaish	Nov. 25, 2014, 2:23 p.m.	Open
Do we need this change? This does not exceed the line length limit.	Nilay Vaish	Nov. 25, 2014, 2:23 p.m.	Open
Do we need this change? This does not exceed the line length limit.	Nilay Vaish	Nov. 25, 2014, 2:23 p.m.	Open

src/dev/x86/i8042.cc (Diff revision 1)

We don't need this nack().

Show all issues

Gabe Black Dec. 16, 2014, 11:34 p.m. (Dec. 16, 2014, 11:34 p.m.)
```
I changed the panic to a warn and left the nack in.
```

src/dev/x86/i8042.cc (Diff revision 1)

Is some code missing here?  You did not write anything to the output port.

Show all issues

Gabe Black Dec. 16, 2014, 11:34 p.m. (Dec. 16, 2014, 11:34 p.m.)

The output port is really a bunch of GPIOs on the keyboard controller attached to a bunch of random things. It's not really worth implementing right now.

src/dev/x86/i8042.cc (Diff revision 1)

Do we need this change?  This does not exceed the line length limit.

Show all issues

Gabe Black Dec. 16, 2014, 11:34 p.m. (Dec. 16, 2014, 11:34 p.m.)
```
Yes it does. If I collapse it back down, the semicolon wraps.
```

src/dev/x86/i8042.cc (Diff revision 1)

Do we need this change?  This does not exceed the line length limit.

Show all issues

Gabe Black Dec. 16, 2014, 11:34 p.m. (Dec. 16, 2014, 11:34 p.m.)

Yes it does. If I collapse it back down, the closing parenthesis and semicolon wrap.

src/dev/x86/i8042.cc (Diff revision 1)

Do we need this change?  This does not exceed the line length limit.

Show all issues

Gabe Black Dec. 16, 2014, 11:34 p.m. (Dec. 16, 2014, 11:34 p.m.)
```
Nope.
```

src/dev/x86/i8042.cc (Diff revision 1)

A single line seems possible.

Gabe Black Dec. 16, 2014, 11:34 p.m. (Dec. 16, 2014, 11:34 p.m.)
```
Nope.
```

src/dev/x86/i8042.cc (Diff revision 1)

A single line seems possible.

Gabe Black Dec. 16, 2014, 11:34 p.m. (Dec. 16, 2014, 11:34 p.m.)
```
Nope.
```

What's TLC and CL?

Gabe Black Dec. 16, 2014, 11:19 p.m. (Dec. 16, 2014, 11:19 p.m.)
```
"tender loving care" and "change list"
```

Description:

~		Changeset 10555:595f40e72481
	~	Changeset 10612:740dbb64ce34

		x86: i8042: Give the keyboard controller a little TLC.

		This CL fixes a few minor bugs, and fills out the general implementation a bit
		so that it can more readily support actually returning keyboard input.

Diff:

Revision 2 (+177 -99)

Show changes

	src/dev/x86/i8042.hh
	src/dev/x86/i8042.cc

seems fine to me, although I can't say I know the code well

You have a pending review.

Review Board 2.0.15

Screenshots

Files

Issue Summary

Description:

Changeset 10555:595f40e72481

Changeset 10612:740dbb64ce34

Diff: