Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RTL8168 Implementation #1652

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

RTL8168 Implementation #1652

wants to merge 9 commits into from

Conversation

valentinbreiz
Copy link
Member

No description provided.

@valentinbreiz valentinbreiz linked an issue Jan 22, 2021 that may be closed by this pull request
3 tasks
@valentinbreiz valentinbreiz added Area: Networking All about network stack, protocols and network drivers Work in progress labels Jan 25, 2021
@Arawn-Davies Arawn-Davies requested review from quajak and Arawn-Davies and removed request for Arawn-Davies June 12, 2021 00:27
/// </summary>
/// <param name="port"></param>
/// <returns></returns>
public static byte InB(ushort port)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer we go with the same approach we have for other drivers as well, which is that we define all the the IOPorts we need and then access the one we actually need. For example see

public class ATA : IOGroup

That approach should be quicker since we arnt constantly creating new objects and less error prone than calculating the port address every time.


if (((GetMacVersion() & 0x7cf00000) == 0x54100000) || ((GetMacVersion() & 0x7cf00000) == 0x54000000))
{
Console.WriteLine("8168H Detected!");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this Console.WriteLine

public bool Reset()
{
Ports.OutB((ushort)(BaseAddress + 0x37), 0x10); /* Send the Reset bit to the Command register */
while ((Ports.InB((ushort)(BaseAddress + 0x37)) & 0x10) != 0) { } /* Wait for the chip to finish resetting */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have timeout in this method, in which case we return false?

}

protected void HandleNetworkInterrupt(ref IRQContext aContext)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all the Console.WriteLines in this method required? They seem like debug output and should be removable


Console.WriteLine("NIC IRQ: " + device.InterruptLine);

var RTL8168Device = new RTL8168(device);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is the device accessible from outside the class? Again I dont like the Console.WriteLines, maybe make the Init(bool aShowOutput) have a toggle for console output?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Networking All about network stack, protocols and network drivers Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RTL8168/9 support
3 participants