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

fix some HUB related issues #414

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 34 additions & 20 deletions Usb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -581,21 +581,33 @@ uint8_t USB::DefaultAddressing(uint8_t parent, uint8_t port, bool lowspeed) {
return 0;
};

void USB::ResetHubPort(uint8_t parent, uint8_t port) {
if (parent == 0) {
// Send a bus reset on the root interface.
regWr(rHCTL, bmBUSRST); //issue bus reset
delay(102); // delay 102ms, compensate for clock inaccuracy.
} else {
for (uint8_t i = 0; i < USB_NUMDEVICES; i++) {
if (devConfig[i]) {
UsbDeviceAddress addr;
addr.devAddress = devConfig[i]->GetAddress();
if (addr.bmHub && addr.bmAddress == parent) {
devConfig[i]->ResetHubPort(port);
break;
}
}
}
}
}

uint8_t USB::AttemptConfig(uint8_t driver, uint8_t parent, uint8_t port, bool lowspeed) {
//printf("AttemptConfig: parent = %i, port = %i\r\n", parent, port);
uint8_t retries = 0;

again:
uint8_t rcode = devConfig[driver]->ConfigureDevice(parent, port, lowspeed);
if(rcode == USB_ERROR_CONFIG_REQUIRES_ADDITIONAL_RESET) {
if(parent == 0) {
// Send a bus reset on the root interface.
regWr(rHCTL, bmBUSRST); //issue bus reset
delay(102); // delay 102ms, compensate for clock inaccuracy.
} else {
// reset parent port
devConfig[parent]->ResetHubPort(port);
}
ResetHubPort(parent, port);
} else if(rcode == hrJERR && retries < 3) { // Some devices returns this when plugged in - trying to initialize the device again usually works
delay(100);
retries++;
Expand All @@ -611,14 +623,7 @@ uint8_t USB::AttemptConfig(uint8_t driver, uint8_t parent, uint8_t port, bool lo
}
if(rcode) {
// Issue a bus reset, because the device may be in a limbo state
if(parent == 0) {
// Send a bus reset on the root interface.
regWr(rHCTL, bmBUSRST); //issue bus reset
delay(102); // delay 102ms, compensate for clock inaccuracy.
} else {
// reset parent port
devConfig[parent]->ResetHubPort(port);
}
ResetHubPort(parent, port);
}
return rcode;
}
Expand Down Expand Up @@ -762,13 +767,22 @@ uint8_t USB::Configuring(uint8_t parent, uint8_t port, bool lowspeed) {
}

uint8_t USB::ReleaseDevice(uint8_t addr) {
UsbDeviceAddress a;
if(!addr)
return 0;

for(uint8_t i = 0; i < USB_NUMDEVICES; i++) {
if(!devConfig[i]) continue;
if(devConfig[i]->GetAddress() == addr)
a.devAddress = addr;
if(a.bmHub) {
for(uint8_t i = 0; i < USB_NUMDEVICES; i++) {
if(!devConfig[i]) continue;
if(devConfig[i]->GetAddress() == addr)
return devConfig[i]->Release();
}
} else {
for(uint8_t i = 0; i < USB_NUMDEVICES; i++) {
if(!devConfig[i]) continue;
if(devConfig[i]->GetPortAddress() == addr)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this would be better to replace this with:

if((a.bmHub && devConfig[i]->GetAddress() == addr) || (!a.bmHub && devConfig[i]->GetPortAddress() == addr))

And then get rid of the if(a.bmHub) if-statement above.

return devConfig[i]->Release();
}
}
return 0;
}
Expand Down
5 changes: 5 additions & 0 deletions UsbCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,10 @@ class USBDeviceConfig {
return 0;
}

virtual uint8_t GetPortAddress() {
return GetAddress();
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please fix the indentation here.

virtual void ResetHubPort(uint8_t port __attribute__((unused))) {
return;
} // Note used for hubs only!
Expand Down Expand Up @@ -274,6 +278,7 @@ class USB : public MAX3421E {
uint8_t SetAddress(uint8_t addr, uint8_t ep, EpInfo **ppep, uint16_t *nak_limit);
uint8_t OutTransfer(EpInfo *pep, uint16_t nak_limit, uint16_t nbytes, uint8_t *data);
uint8_t InTransfer(EpInfo *pep, uint16_t nak_limit, uint16_t *nbytesptr, uint8_t *data, uint8_t bInterval = 0);
void ResetHubPort(uint8_t parent, uint8_t port);
uint8_t AttemptConfig(uint8_t driver, uint8_t parent, uint8_t port, bool lowspeed);
};

Expand Down
20 changes: 20 additions & 0 deletions usbhub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ bool USBHub::bResetInitiated = false;
USBHub::USBHub(USB *p) :
pUsb(p),
bAddress(0),
bPortAddress(0),
bNbrPorts(0),
//bInitState(0),
qNextPollTime(0),
Expand Down Expand Up @@ -104,6 +105,14 @@ uint8_t USBHub::Init(uint8_t parent, uint8_t port, bool lowspeed) {
if(!bAddress)
return USB_ERROR_OUT_OF_ADDRESS_SPACE_IN_POOL;

{
UsbDeviceAddress a;
a.devAddress = bAddress;
a.bmHub = 0;
a.bmAddress = port ? : 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure what this does when port is non-zero. Is it the equivalent of
if (port) {
a.bmAddress = ;
} else {
a.bmAddress = 1;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Port numbers start at 1.
Right now commits are stopped, but eventually should be resumed.

bPortAddress = a.devAddress;
}

// Extract Max Packet Size from the device descriptor
epInfo[0].maxPktSize = udd->bMaxPacketSize0;

Expand All @@ -115,6 +124,7 @@ uint8_t USBHub::Init(uint8_t parent, uint8_t port, bool lowspeed) {
p->epinfo = oldep_ptr;
addrPool.FreeAddress(bAddress);
bAddress = 0;
bPortAddress = 0;
return rcode;
}

Expand Down Expand Up @@ -214,12 +224,22 @@ uint8_t USBHub::Init(uint8_t parent, uint8_t port, bool lowspeed) {
}

uint8_t USBHub::Release() {
UsbDeviceAddress a;
a.devAddress = 0;
a.bmHub = 0;
a.bmParent = bAddress;
for (uint8_t j = 1; j <= bNbrPorts; j++) {
a.bmAddress = j;
pUsb->ReleaseDevice(a.devAddress);
}

pUsb->GetAddressPool().FreeAddress(bAddress);

if(bAddress == 0x41)
pUsb->SetHubPreMask();

bAddress = 0;
bPortAddress = 0;
bNbrPorts = 0;
qNextPollTime = 0;
bPollEnable = false;
Expand Down
7 changes: 6 additions & 1 deletion usbhub.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ class USBHub : USBDeviceConfig {
EpInfo epInfo[2]; // interrupt endpoint info structure

uint8_t bAddress; // address
uint8_t bPortAddress; // port address
uint8_t bNbrPorts; // number of ports
// uint8_t bInitState; // initialization state variable
uint32_t qNextPollTime; // next poll time
Expand Down Expand Up @@ -200,7 +201,11 @@ class USBHub : USBDeviceConfig {
return bAddress;
};

virtual bool DEVCLASSOK(uint8_t klass) {
virtual uint8_t GetPortAddress() {
return bPortAddress;
};

virtual bool DEVCLASSOK(uint8_t klass) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please also fix indentation here.

return (klass == 0x09);
}

Expand Down