Merge pull request managarm#679 from qookei/usb-fixes

drivers/kbd/src/main.cpp

@@ -270,13 +270,20 @@ async::result<void> Controller::KbdDevice::run() {
 	if(res1) {
 		// Make sure it is used.
 		auto res2 = co_await submitCommand(device_cmd::GetScancodeSet{});
-		assert(res2);
-		if (res2.value() != 1 && res2.value() != 2) {
-			std::cout << "\e[31m" "ps2-hid: Keyboard does supports neither scancode set 1 nor 2"
+		if (!res2) {
+			// Some keyboards don't seem to support this command, e.g. the
+			// built-in keyboard in my laptop (qookie).
+			// Sucks, but what can you do. Linux just assumes 2 in this case.
+			_codeSet = 2;
+		} else {
+			if (res2.value() != 1 && res2.value() != 2) {
+				std::cout << "\e[31m"
+					"ps2-hid: Keyboard does supports neither scancode set 1 nor 2"
 					"\e[39m" << std::endl;
-			co_return;
+				co_return;
+			}
+			_codeSet = res2.value();
 		}
-		_codeSet = res2.value();
 	} else {
 		_codeSet = 2;
 	}

drivers/usb/devices/hid/src/main.cpp

@@ -307,8 +307,12 @@ void HidDevice::parseReportDescriptor(proto::Device, uint8_t *p, uint8_t* limit)
 				if(local.usage.empty()) {
 					actual_id = local.usageMin.value() + i;
 				}else{
-					assert(local.usage.size() == global.reportCount.value());
-					actual_id = local.usage[i];
+					// Duplicate the last usage if we have excess value, as Linux does.
+					if (i >= local.usage.size()) {
+						actual_id = local.usage.back();
+					} else {
+						actual_id = local.usage[i];
+					}
 				}
 
 				if(!global.logicalMin || !global.logicalMax)

drivers/usb/hcds/xhci/src/main.cpp

@@ -587,26 +587,35 @@ async::detached Controller::Port::initPort() {
 		printf("xhci: port %u is not powered on\n", _id);
 	}
 
-	if (_proto->major == 2) {
-		// await CCS=1
-		co_await awaitFlag(portsc::connectStatus, true);
+	// Wait for something to connect to the port
+	co_await awaitFlag(portsc::connectStatus, true);
 
-		// The XHCI spec states that USB2 devices should enter the polling state at the
-		// same time they set CCS=1, but VirtualBox' XHCI does not implement this behavior.
-		auto linkStatus = getLinkStatus();
-		if(linkStatus != 7)
-			printf("\e[35m" "xhci: USB2 port did not enter polling state after CCS=1" "\e[39m\n");
+	// Notify the enumerator
+	_state.changes |= proto::HubStatus::connect;
+	_state.status |= proto::HubStatus::connect;
+	_pollEv.raise();
+}
 
-		reset();
+async::result<proto::PortState> Controller::Port::pollState() {
+	_pollSeq = co_await _pollEv.async_wait(_pollSeq);
+	co_return _state;
+}
 
-		// await PED=1
-		co_await awaitFlag(portsc::portEnable, true);
-	} else if (_proto->major == 3) {
-		// XXX: is this enough?
-		// await PED=1
-		co_await awaitFlag(portsc::portEnable, true);
+async::result<frg::expected<proto::UsbError, proto::DeviceSpeed>> Controller::Port::issueReset() {
+	// We know something is connected if we're here (CCS=1)
+
+	// Reset the port only for USB 2 devices.
+	// "A USB3 protocol port attempts to automatically advance to
+	//  the Enabled state as part of the attach process."
+	// "A USB2 protocol port requires software to reset the port
+	//  to advance the port to the Enabled state [...]"
+	if (_proto->major == 2) {
+		reset();
 	}
 
+	// Wait for the port to enable.
+	co_await awaitFlag(portsc::portEnable, true);
+
 	auto linkStatus = getLinkStatus();
 
 	printf("xhci: port link status is %u\n", linkStatus);
@@ -617,17 +626,13 @@ async::detached Controller::Port::initPort() {
 		assert(linkStatus == 0); // U0
 	}
 
-	_state.changes |= proto::HubStatus::connect | proto::HubStatus::enable;
-	_state.status |= proto::HubStatus::connect | proto::HubStatus::enable;
+	// Notify the enumerator.
+	_state.changes |= proto::HubStatus::enable;
+	_state.status |= proto::HubStatus::enable;
 	_pollEv.raise();
-}
 
-async::result<proto::PortState> Controller::Port::pollState() {
-	_pollSeq = co_await _pollEv.async_wait(_pollSeq);
-	co_return _state;
-}
+	// Figure out the device speed.
 
-async::result<frg::expected<proto::UsbError, proto::DeviceSpeed>> Controller::Port::getGenericSpeed() {
 	uint8_t speedId = getSpeed();
 
 	std::optional<proto::DeviceSpeed> speed;
@@ -686,9 +691,6 @@ async::result<frg::expected<proto::UsbError, proto::DeviceSpeed>> Controller::Po
 		}
 	}
 
-	// Raise the event here to make progress in the enumerator
-	_pollEv.raise();
-
 	if (speed) {
 		co_return speed.value();
 	} else {
@@ -720,7 +722,7 @@ async::result<proto::PortState> Controller::RootHub::pollState(int port) {
 }
 
 async::result<frg::expected<proto::UsbError, proto::DeviceSpeed>> Controller::RootHub::issueReset(int port) {
-	co_return FRG_CO_TRY(co_await _ports[port]->getGenericSpeed());
+	co_return FRG_CO_TRY(co_await _ports[port]->issueReset());
 }
 
 // ------------------------------------------------------------------------
@@ -799,7 +801,7 @@ Controller::Device::useConfiguration(int number) {
 	Transfer::buildControlChain([&] (RawTrb trb, bool last) {
 		if (last)
 			trb.val[3] |= 1 << 5; // IOC
-		pushRawTransfer(0, trb, last ? &comp : nullptr);
+		pushRawTransfer(0, trb, &comp);
 	}, { .request = proto::request_type::setConfig, .value = static_cast<uint16_t>(number), .length = 0 }, { }, false,
 			_maxPacketSizes[0]);
 
@@ -824,7 +826,7 @@ Controller::Device::transfer(proto::ControlTransfer info) {
 	Transfer::buildControlChain([&] (RawTrb trb, bool last) {
 		if (last)
 			trb.val[3] |= 1 << 5; // IOC
-		pushRawTransfer(0, trb, last ? &ev : nullptr);
+		pushRawTransfer(0, trb, &ev);
 	}, *info.setup.data(), info.buffer, info.flags == proto::kXferToHost, _maxPacketSizes[0]);
 
 	submit(1);
@@ -933,7 +935,7 @@ Controller::Device::readDescriptor(arch::dma_buffer_view dest, uint16_t desc) {
 	Transfer::buildControlChain([&] (RawTrb trb, bool last) {
 		if (last)
 			trb.val[3] |= 1 << 5; // IOC
-		pushRawTransfer(0, trb, last ? &ev : nullptr);
+		pushRawTransfer(0, trb, &ev);
 	}, { .type = 0x80, .request = proto::request_type::getDescriptor,
 		.value = desc, .length = static_cast<uint16_t>(dest.size()) }, dest, true,
 			_maxPacketSizes[0]);
@@ -1126,7 +1128,7 @@ Controller::EndpointState::transfer(proto::ControlTransfer info) {
 	Transfer::buildControlChain([&] (RawTrb trb, bool last) {
 		if (last)
 			trb.val[3] |= 1 << 5; // IOC
-		_device->pushRawTransfer(endpointId - 1, trb, last ? &ev : nullptr);
+		_device->pushRawTransfer(endpointId - 1, trb, &ev);
 	}, *info.setup.data(), info.buffer, info.flags == proto::kXferToHost,
 			_device->_maxPacketSizes[endpointId - 1]);
 
@@ -1147,7 +1149,7 @@ Controller::EndpointState::transfer(proto::InterruptTransfer info) {
 	Transfer::buildNormalChain([&] (RawTrb trb, bool last) {
 		if (last)
 			trb.val[3] |= 1 << 5; // IOC
-		_device->pushRawTransfer(endpointId - 1, trb, last ? &ev : nullptr);
+		_device->pushRawTransfer(endpointId - 1, trb, &ev);
 	}, info.buffer, _device->_maxPacketSizes[endpointId - 1]);
 
 	_device->submit(endpointId);
@@ -1167,7 +1169,7 @@ Controller::EndpointState::transfer(proto::BulkTransfer info) {
 	Transfer::buildNormalChain([&] (RawTrb trb, bool last) {
 		if (last)
 			trb.val[3] |= 1 << 5; // IOC
-		_device->pushRawTransfer(endpointId - 1, trb, last ? &ev : nullptr);
+		_device->pushRawTransfer(endpointId - 1, trb, &ev);
 	}, info.buffer, _device->_maxPacketSizes[endpointId - 1]);
 
 	_device->submit(endpointId);

drivers/usb/hcds/xhci/src/xhci.hpp

@@ -126,7 +126,7 @@ struct Controller final : proto::BaseController {
 		async::recurring_event _doorbell;
 
 		async::result<proto::PortState> pollState();
-		async::result<frg::expected<proto::UsbError, proto::DeviceSpeed>> getGenericSpeed();
+		async::result<frg::expected<proto::UsbError, proto::DeviceSpeed>> issueReset();
 
 	private:
 		uint8_t getLinkStatus();
@@ -317,5 +317,3 @@ struct Controller final : proto::BaseController {
 
 	mbus_ng::Entity _entity;
 };
-
-

protocols/usb/src/hub.cpp

@@ -4,6 +4,8 @@
 
 #include <async/recurring-event.hpp>
 
+#include <helix/timer.hpp>
+
 namespace protocols::usb {
 
 // ----------------------------------------------------------------
@@ -92,6 +94,7 @@ namespace PortFeatures {
 	//static constexpr uint16_t connect = 0;
 	//static constexpr uint16_t enable = 1;
 	static constexpr uint16_t reset = 4;
+	static constexpr uint16_t power = 8;
 	static constexpr uint16_t connectChange = 16;
 	static constexpr uint16_t enableChange = 17;
 	static constexpr uint16_t resetChange = 20;
@@ -157,6 +160,7 @@ async::result<frg::expected<UsbError>> StandardHub::initialize() {
 	struct [[gnu::packed]] HubDescriptor : public DescriptorBase {
 		uint8_t numPorts;
 		uint16_t hubCharacteristics;
+		uint8_t powerOnToPowerGood;
 	};
 
 	arch::dma_object<SetupPacket> getDescriptor{device_.setupPool()};
@@ -176,6 +180,25 @@ async::result<frg::expected<UsbError>> StandardHub::initialize() {
 	auto rawThinkTime = (hubDescriptor->hubCharacteristics >> 5) & 0b11;
 	characteristics_.ttThinkTime = 8 * (1 + rawThinkTime);
 
+	for (size_t port = 0; port < hubDescriptor->numPorts; port++) {
+		// Issue a SetPortFeature request to power on the port.
+		arch::dma_object<SetupPacket> powerReq{device_.setupPool()};
+		powerReq->type = setup_type::targetOther | setup_type::byClass
+			| setup_type::toDevice;
+		powerReq->request = ClassRequests::setFeature;
+		powerReq->value = PortFeatures::power;
+		powerReq->index = port + 1;
+		powerReq->length = 0;
+
+		FRG_CO_TRY(co_await device_.transfer(ControlTransfer{kXferToDevice,
+						powerReq, arch::dma_buffer_view{}}));
+	}
+
+	// Wait for the ports to power on (time is specified in 2 ms units).
+	// Linux waits for at least 100ms, even if the hub says less time is needed.
+	auto durationMs = std::max(hubDescriptor->powerOnToPowerGood * 2, 100);
+	co_await helix::sleepFor(durationMs * 1'000'000);
+
 	run_();
 	co_return {};
 }