Kernel: Make USB HID interfaces configure all endpoints

This allows me to use my laptops own keyboard!

kernel/include/kernel/USB/HID/HIDDriver.h

@@ -67,6 +67,13 @@ namespace Kernel
 		BAN_NON_COPYABLE(USBHIDDriver);
 		BAN_NON_MOVABLE(USBHIDDriver);
 
+	public:
+		struct DeviceReport
+		{
+			BAN::Vector<USBHID::Report> inputs;
+			BAN::RefPtr<USBHIDDevice> device;
+		};
+
 	public:
 		static BAN::ErrorOr<BAN::UniqPtr<USBHIDDriver>> create(USBDevice&, const USBDevice::InterfaceDescriptor&, uint8_t interface_index);
 
@@ -78,7 +85,7 @@ namespace Kernel
 
 		BAN::ErrorOr<void> initialize();
 
-		void forward_collection_inputs(const USBHID::Collection&, BAN::Optional<uint8_t> report_id, BAN::ConstByteSpan& data, size_t bit_offset);
+		BAN::ErrorOr<BAN::Vector<DeviceReport>> initializes_device_reports(const BAN::Vector<USBHID::Collection>&);
 
 	private:
 		USBDevice& m_device;
@@ -86,10 +93,7 @@ namespace Kernel
 		const uint8_t m_interface_index;
 
 		bool m_uses_report_id { false };
-
+		BAN::Vector<DeviceReport> m_device_inputs;
-		uint8_t m_endpoint_id { 0 };
-		BAN::Vector<USBHID::Collection> m_collections;
-		BAN::RefPtr<USBHIDDevice> m_hid_device;
 
 		friend class BAN::UniqPtr<USBHIDDriver>;
 	};

kernel/kernel/USB/HID/HIDDriver.cpp

@@ -79,8 +79,9 @@ namespace Kernel
 
 	USBHIDDriver::~USBHIDDriver()
 	{
-		if (m_hid_device)
+		for (auto& device_input : m_device_inputs)
-			DevFileSystem::get().remove_device(m_hid_device);
+			if (device_input.device)
+				DevFileSystem::get().remove_device(device_input.device);
 	}
 
 	BAN::ErrorOr<void> USBHIDDriver::initialize()
@@ -89,24 +90,6 @@ namespace Kernel
 
 		ASSERT(static_cast<USB::InterfaceBaseClass>(m_interface.descriptor.bInterfaceClass) == USB::InterfaceBaseClass::HID);
 
-		size_t endpoint_index = static_cast<size_t>(-1);
-		for (size_t i = 0; i < m_interface.endpoints.size(); i++)
-		{
-			const auto& endpoint = m_interface.endpoints[i];
-			if (!(endpoint.descriptor.bEndpointAddress & 0x80))
-				continue;
-			if (endpoint.descriptor.bmAttributes != 0x03)
-				continue;
-			endpoint_index = i;
-			break;
-		}
-
-		if (endpoint_index >= m_interface.endpoints.size())
-		{
-			dwarnln("HID device does not contain IN interrupt endpoint");
-			return BAN::Error::from_errno(EFAULT);
-		}
-
 		bool hid_descriptor_invalid = false;
 		size_t hid_descriptor_index = static_cast<size_t>(-1);
 		for (size_t i = 0; i < m_interface.misc_descriptors.size(); i++)
@@ -203,44 +186,110 @@ namespace Kernel
 			return BAN::Error::from_errno(EFAULT);
 		}
 
-		// FIXME: Handle other collections?
+		m_device_inputs = TRY(initializes_device_reports(collections));
 
-		if (collections.front().usage_page != 0x01)
+		for (const auto& endpoint : m_interface.endpoints)
-		{
+			TRY(m_device.initialize_endpoint(endpoint.descriptor));
-			dwarnln("Top most collection is not generic desktop page");
-			return BAN::Error::from_errno(EFAULT);
-		}
-
-		switch (collections.front().usage_id)
-		{
-			case 0x02:
-				m_hid_device = TRY(BAN::RefPtr<USBMouse>::create());
-				dprintln("Initialized an USB Mouse");
-				break;
-			case 0x06:
-				m_hid_device = TRY(BAN::RefPtr<USBKeyboard>::create());
-				dprintln("Initialized an USB Keyboard");
-				break;
-			default:
-				dwarnln("Unsupported generic descript page usage 0x{2H}", collections.front().usage_id);
-				return BAN::Error::from_errno(ENOTSUP);
-		}
-		DevFileSystem::get().add_device(m_hid_device);
-
-		const auto& endpoint_descriptor = m_interface.endpoints[endpoint_index].descriptor;
-
-		m_endpoint_id = (endpoint_descriptor.bEndpointAddress & 0x0F) * 2 + !!(endpoint_descriptor.bEndpointAddress & 0x80);
-		m_collections = BAN::move(collections);
-
-		TRY(m_device.initialize_endpoint(endpoint_descriptor));
 
 		return {};
 	}
 
-	void USBHIDDriver::forward_collection_inputs(const Collection& collection, BAN::Optional<uint8_t> report_id, BAN::ConstByteSpan& data, size_t bit_offset)
+	static BAN::ErrorOr<void> gather_collection_inputs(const USBHID::Collection& collection, BAN::Vector<USBHID::Report>& output)
 	{
+		for (const auto& entry : collection.entries)
+		{
+			if (entry.has<USBHID::Collection>())
+			{
+				TRY(gather_collection_inputs(entry.get<USBHID::Collection>(), output));
+				continue;
+			}
+
+			const auto& report = entry.get<USBHID::Report>();
+			if (report.type != USBHID::Report::Type::Input)
+				continue;
+
+			TRY(output.push_back(report));
+		}
+
+		return {};
+	}
+
+	BAN::ErrorOr<BAN::Vector<USBHIDDriver::DeviceReport>> USBHIDDriver::initializes_device_reports(const BAN::Vector<USBHID::Collection>& collection_list)
+	{
+		BAN::Vector<USBHIDDriver::DeviceReport> result;
+		TRY(result.reserve(collection_list.size()));
+
+		for (size_t i = 0; i < collection_list.size(); i++)
+		{
+			const auto& collection = collection_list[i];
+
+			USBHIDDriver::DeviceReport report;
+			TRY(gather_collection_inputs(collection, report.inputs));
+
+			if (collection.usage_page == 0x01)
+			{
+				switch (collection.usage_id)
+				{
+					case 0x02:
+						report.device = TRY(BAN::RefPtr<USBMouse>::create());
+						dprintln("Initialized an USB Mouse");
+						break;
+					case 0x06:
+						report.device = TRY(BAN::RefPtr<USBKeyboard>::create());
+						dprintln("Initialized an USB Keyboard");
+						break;
+					default:
+						dwarnln("Unsupported generic descript page usage 0x{2H}", collection.usage_id);
+						break;
+				}
+			}
+
+			TRY(result.push_back(BAN::move(report)));
+		}
+
+		for (auto& report : result)
+			if (report.device)
+				DevFileSystem::get().add_device(report.device);
+		return BAN::move(result);
+	}
+
+	void USBHIDDriver::handle_input_data(BAN::ConstByteSpan data, uint8_t endpoint_id)
+	{
+		{
+			bool found = false;
+			for (const auto& endpoint : m_interface.endpoints)
+			{
+				const auto& desc = endpoint.descriptor;
+				if (endpoint_id == (desc.bEndpointAddress & 0x0F) * 2 + !!(desc.bEndpointAddress & 0x80))
+				{
+					found = true;
+					break;
+				}
+			}
+			// If this packet is not for us, skip it
+			if (!found)
+				return;
+		}
+
+		if constexpr(DEBUG_HID)
+		{
+			const auto nibble_to_hex = [](uint8_t x) -> char { return x + (x < 10 ? '0' : 'A' - 10); };
+
+			char buffer[512];
+			char* ptr = buffer;
+			for (size_t i = 0; i < BAN::Math::min<size_t>((sizeof(buffer) - 1) / 3, data.size()); i++)
+			{
+				*ptr++ = nibble_to_hex(data[i] >> 4);
+				*ptr++ = nibble_to_hex(data[i] & 0xF);
+				*ptr++ = ' ';
+			}
+			*ptr = '\0';
+
+			dprintln_if(DEBUG_HID, "Received {} bytes from endpoint {}: {}", data.size(), endpoint_id, buffer);
+		}
+
 		const auto extract_bits =
-			[data](size_t bit_offset, size_t bit_count, bool as_unsigned) -> int64_t
+			[&data](size_t bit_offset, size_t bit_count, bool as_unsigned) -> int64_t
 			{
 				if (bit_offset >= data.size() * 8)
 					return 0;
@@ -272,78 +321,6 @@ namespace Kernel
 				return result;
 			};
 
-		for (const auto& entry : collection.entries)
-		{
-			if (entry.has<Collection>())
-			{
-				forward_collection_inputs(entry.get<Collection>(), report_id, data, bit_offset);
-				continue;
-			}
-
-			ASSERT(entry.has<Report>());
-			const auto& input = entry.get<Report>();
-			if (input.type != Report::Type::Input)
-				continue;
-			if (report_id.value_or(input.report_id) != input.report_id)
-				continue;
-
-			ASSERT(input.report_size <= 32);
-
-			if (input.usage_id == 0 && input.usage_minimum == 0 && input.usage_maximum == 0)
-			{
-				bit_offset += input.report_size * input.report_count;
-				continue;
-			}
-
-			for (uint32_t i = 0; i < input.report_count; i++)
-			{
-				const int64_t logical = extract_bits(bit_offset, input.report_size, input.logical_minimum >= 0);
-				if (logical < input.logical_minimum || logical > input.logical_maximum)
-				{
-					bit_offset += input.report_size;
-					continue;
-				}
-
-				const int64_t physical =
-					(input.physical_maximum - input.physical_minimum) *
-					(logical - input.logical_minimum) /
-					(input.logical_maximum - input.logical_minimum) +
-					input.physical_minimum;
-
-				const uint32_t usage_base = input.usage_id ? input.usage_id : input.usage_minimum;
-				if (input.flags & 0x02)
-					m_hid_device->handle_variable(input.usage_page, usage_base + i, physical);
-				else
-					m_hid_device->handle_array(input.usage_page, usage_base + physical);
-
-				bit_offset += input.report_size;
-			}
-		}
-	}
-
-	void USBHIDDriver::handle_input_data(BAN::ConstByteSpan data, uint8_t endpoint_id)
-	{
-		// If this packet is not for us, skip it
-		if (m_endpoint_id != endpoint_id)
-			return;
-
-		if constexpr(DEBUG_HID)
-		{
-			const auto nibble_to_hex = [](uint8_t x) -> char { return x + (x < 10 ? '0' : 'A' - 10); };
-
-			char buffer[512];
-			char* ptr = buffer;
-			for (size_t i = 0; i < BAN::Math::min<size_t>((sizeof(buffer) - 1) / 3, data.size()); i++)
-			{
-				*ptr++ = nibble_to_hex(data[i] >> 4);
-				*ptr++ = nibble_to_hex(data[i] & 0xF);
-				*ptr++ = ' ';
-			}
-			*ptr = '\0';
-
-			dprintln_if(DEBUG_HID, "Received {} bytes from endpoint {}: {}", data.size(), endpoint_id, buffer);
-		}
-
 		BAN::Optional<uint8_t> report_id;
 		if (m_uses_report_id)
 		{
@@ -351,10 +328,53 @@ namespace Kernel
 			data = data.slice(1);
 		}
 
-		m_hid_device->start_report();
+		size_t bit_offset = 0;
-		// FIXME: Handle other collections?
+		for (auto& device_input : m_device_inputs)
-		forward_collection_inputs(m_collections.front(), report_id, data, 0);
+		{
-		m_hid_device->stop_report();
+			if (device_input.device)
+				device_input.device->start_report();
+
+			for (const auto& input : device_input.inputs)
+			{
+				if (report_id.value_or(input.report_id) != input.report_id)
+					continue;
+
+				ASSERT(input.report_size <= 32);
+
+				if (!device_input.device || (input.usage_id == 0 && input.usage_minimum == 0 && input.usage_maximum == 0))
+				{
+					bit_offset += input.report_size * input.report_count;
+					continue;
+				}
+
+				for (uint32_t i = 0; i < input.report_count; i++)
+				{
+					const int64_t logical = extract_bits(bit_offset, input.report_size, input.logical_minimum >= 0);
+					if (logical < input.logical_minimum || logical > input.logical_maximum)
+					{
+						bit_offset += input.report_size;
+						continue;
+					}
+
+					const int64_t physical =
+						(input.physical_maximum - input.physical_minimum) *
+						(logical - input.logical_minimum) /
+						(input.logical_maximum - input.logical_minimum) +
+						input.physical_minimum;
+
+					const uint32_t usage_base = input.usage_id ? input.usage_id : input.usage_minimum;
+					if (input.flags & 0x02)
+						device_input.device->handle_variable(input.usage_page, usage_base + i, physical);
+					else
+						device_input.device->handle_array(input.usage_page, usage_base + physical);
+
+					bit_offset += input.report_size;
+				}
+			}
+
+			if (device_input.device)
+				device_input.device->stop_report();
+		}
 	}
 
 	BAN::ErrorOr<BAN::Vector<Collection>> parse_report_descriptor(BAN::ConstByteSpan report_data, bool& out_use_report_id)

kernel/kernel/USB/HID/Keyboard.cpp

@@ -80,6 +80,8 @@ namespace Kernel
 			dprintln_if(DEBUG_KEYBOARD, "Unsupported keyboard usage page {2H}", usage_page);
 			return;
 		}
+		if (!state)
+			return;
 		if (usage >= 4 && usage < m_keyboard_state_temp.size())
 			m_keyboard_state_temp[usage] = state;
 	}

kernel/kernel/USB/XHCI/Device.cpp

@@ -15,7 +15,6 @@ namespace Kernel
 		return TRY(BAN::UniqPtr<XHCIDevice>::create(controller, port_id, slot_id));
 	}
 
-
 	uint64_t XHCIDevice::calculate_port_bits_per_second(XHCIController& controller, uint32_t port_id)
 	{
 		const uint32_t portsc = controller.operational_regs().ports[port_id - 1].portsc;
@@ -203,7 +202,29 @@ namespace Kernel
 
 	BAN::ErrorOr<void> XHCIDevice::initialize_endpoint(const USBEndpointDescriptor& endpoint_descriptor)
 	{
-		const uint32_t endpoint_id = (endpoint_descriptor.bEndpointAddress & 0x0F) * 2 + !!(endpoint_descriptor.bEndpointAddress & 0x80);
+		ASSERT(m_controller.port(m_port_id).revision_major == 2);
+
+		const uint32_t endpoint_id        = (endpoint_descriptor.bEndpointAddress & 0x0F) * 2 + !!(endpoint_descriptor.bEndpointAddress & 0x80);
+		const uint32_t max_packet_size    = endpoint_descriptor.wMaxPacketSize & 0x07FF;
+		const uint32_t max_burst_size     = (endpoint_descriptor.wMaxPacketSize >> 11) & 0x0003;
+		const uint32_t max_esit_payload   = max_packet_size * (max_burst_size + 1);
+		const uint32_t interval           = determine_interval(endpoint_descriptor, m_speed_class);
+		const uint32_t average_trb_length = ((endpoint_descriptor.bmAttributes & 3) == 0b00) ? 8 : max_esit_payload;
+		const uint32_t error_count        = ((endpoint_descriptor.bmAttributes & 3) == 0b01) ? 0 : 3;
+
+		XHCI::EndpointType endpoint_type;
+		switch ((endpoint_descriptor.bEndpointAddress & 0x80) | (endpoint_descriptor.bmAttributes & 0x03))
+		{
+			case 0x00:
+			case 0x80: endpoint_type = XHCI::EndpointType::Control;      break;
+			case 0x01: endpoint_type = XHCI::EndpointType::IsochOut;     break;
+			case 0x81: endpoint_type = XHCI::EndpointType::IsochIn;      break;
+			case 0x02: endpoint_type = XHCI::EndpointType::BulkOut;      break;
+			case 0x82: endpoint_type = XHCI::EndpointType::BulkIn;       break;
+			case 0x03: endpoint_type = XHCI::EndpointType::InterruptOut; break;
+			case 0x83: endpoint_type = XHCI::EndpointType::InterruptIn;  break;
+			default: ASSERT_NOT_REACHED();
+		}
 
 		auto& endpoint = m_endpoints[endpoint_id - 1];
 		ASSERT(!endpoint.transfer_ring);
@@ -214,7 +235,7 @@ namespace Kernel
 				last_valid_endpoint_id = i + 1;
 
 		endpoint.transfer_ring   = TRY(DMARegion::create(m_transfer_ring_trb_count * sizeof(XHCI::TRB)));
-		endpoint.max_packet_size = endpoint_descriptor.wMaxPacketSize & 0x07FF;
+		endpoint.max_packet_size = max_packet_size;
 		endpoint.dequeue_index   = 0;
 		endpoint.enqueue_index   = 0;
 		endpoint.cycle_bit       = 1;
@@ -237,23 +258,16 @@ namespace Kernel
 			slot_context.context_entries = last_valid_endpoint_id;
 			// FIXME: 4.5.2 hub
 
-			ASSERT(endpoint_descriptor.bEndpointAddress & 0x80);
-			ASSERT((endpoint_descriptor.bmAttributes & 0x03) == 3);
-			ASSERT(m_controller.port(m_port_id).revision_major == 2);
-
-			const uint32_t max_esit_payload = endpoint_context.max_packet_size * (endpoint_context.max_burst_size + 1);
-			const uint32_t interval = determine_interval(endpoint_descriptor, m_speed_class);
-
 			memset(&endpoint_context, 0, context_size);
-			endpoint_context.endpoint_type       = XHCI::EndpointType::InterruptIn;
+			endpoint_context.endpoint_type       = endpoint_type;
-			endpoint_context.max_packet_size     = endpoint.max_packet_size;
+			endpoint_context.max_packet_size     = max_packet_size;
-			endpoint_context.max_burst_size      = (endpoint_descriptor.wMaxPacketSize >> 11) & 0x0003;
+			endpoint_context.max_burst_size      = max_burst_size;
 			endpoint_context.mult                = 0;
-			endpoint_context.error_count         = 3;
+			endpoint_context.error_count         = error_count;
 			endpoint_context.tr_dequeue_pointer  = endpoint.transfer_ring->paddr() | 1;
 			endpoint_context.max_esit_payload_lo = max_esit_payload & 0xFFFF;
 			endpoint_context.max_esit_payload_hi = max_esit_payload >> 16;
-			endpoint_context.average_trb_length  = max_esit_payload;
+			endpoint_context.average_trb_length  = average_trb_length;
 			endpoint_context.interval            = interval;
 		}
 
@@ -264,19 +278,28 @@ namespace Kernel
 		configure_endpoint.configure_endpoint_command.slot_id               = m_slot_id;
 		TRY(m_controller.send_command(configure_endpoint));
 
-		auto& trb = *reinterpret_cast<volatile XHCI::TRB*>(endpoint.transfer_ring->vaddr());
+		if (endpoint_type == XHCI::EndpointType::InterruptIn)
-		memset(const_cast<XHCI::TRB*>(&trb), 0, sizeof(XHCI::TRB));
+		{
-		trb.normal.trb_type                  = XHCI::TRBType::Normal;
+			auto& trb = *reinterpret_cast<volatile XHCI::TRB*>(endpoint.transfer_ring->vaddr());
-		trb.normal.data_buffer_pointer       = endpoint.data_region->paddr();
+			memset(const_cast<XHCI::TRB*>(&trb), 0, sizeof(XHCI::TRB));
-		trb.normal.trb_transfer_length       = endpoint.data_region->size();
+			trb.normal.trb_type                  = XHCI::TRBType::Normal;
-		trb.normal.td_size                   = 0;
+			trb.normal.data_buffer_pointer       = endpoint.data_region->paddr();
-		trb.normal.interrupt_target          = 0;
+			trb.normal.trb_transfer_length       = endpoint.data_region->size();
-		trb.normal.cycle_bit                 = 1;
+			trb.normal.td_size                   = 0;
-		trb.normal.interrupt_on_completion   = 1;
+			trb.normal.interrupt_target          = 0;
-		trb.normal.interrupt_on_short_packet = 1;
+			trb.normal.cycle_bit                 = 1;
-		advance_endpoint_enqueue(endpoint, false);
+			trb.normal.interrupt_on_completion   = 1;
+			trb.normal.interrupt_on_short_packet = 1;
+			advance_endpoint_enqueue(endpoint, false);
 
-		m_controller.doorbell_reg(m_slot_id) = endpoint_id;
+			m_controller.doorbell_reg(m_slot_id) = endpoint_id;
+		}
+		else
+		{
+			dwarnln("Configured unsupported endpoint {2H}",
+				(endpoint_descriptor.bEndpointAddress & 0x80) | (endpoint_descriptor.bmAttributes & 0x03)
+			);
+		}
 
 		return {};
 	}