OSDev.org

Bonfra wrote:

Code: Select all

second one:
  - type = USB_PACKET_TYPE_IN
  - maxlen = 0x800;
  - buffer = NULL;
  - toggle = 1;

Klakap wrote:

Bonfra wrote:

Code: Select all

second one:
  - type = USB_PACKET_TYPE_IN
  - maxlen = 0x800;
  - buffer = NULL;
  - toggle = 1;

I think problem may be in maxlen 0x800. In STATUS stage you have to transfer zero bytes. So, when you take in account that in UHCI packets you calculate maxlen by (transferred length-1), and maxlen have 11 bits, you should have value 0x7FF.

Klakap wrote: I think problem may be in maxlen 0x800. In STATUS stage you have to transfer zero bytes. So, when you take in account that in UHCI packets you calculate maxlen by (transferred length-1), and maxlen have 11 bits, you should have value 0x7FF.

for(size_t i = 0; i < num_packets; i++)
    {
        tds[i].link = i == num_packets - 1 ? TD_TERMINATE : ((uint32_t)(uint64_t)&tds[i + 1] | TD_DEPTH_FIRST);
        tds[i].flags = TD_STATUS_ACTIVE | TD_C_ERR;

        if(i == num_packets - 1)
            tds[i].flags |= TD_IOC;

        if(port_status(controller, addr) == USB_PORT_STATUS_CONNECT_LOW_SPEED)
            tds[i].flags |= TD_LOW_SPEED;
        
        tds[i].maxlen = ((packets[i].maxlen - 1)  << 21) | (endpoint << 15) | (addr << 8);
        
        switch (packets[i].type)
        {
        case USB_PACKET_TYPE_SETUP: tds[i].maxlen |= TD_PID_SETUP; break;
        case USB_PACKET_TYPE_IN: tds[i].maxlen |= TD_PID_IN; break;
        case USB_PACKET_TYPE_OUT: tds[i].maxlen |= TD_PID_OUT; break;
        }

        if(packets[i].toggle)
            tds[i].maxlen |= TD_DATA_TOGGLE;

        tds[i].bufptr = (uint32_t)(uint64_t)packets[i].buffer;
    }

BenLunt wrote: Sorry Bonfra, I had not made it back until now to have a look.

Klakap wrote:I think it may be helpful if you would post how all your packets look like in hex values before and after processing. If error is in packets, it should be faster way to recognize it than reading all your code.

BenLunt wrote:I would like to add to this request and ask for a (very) recent image file as well.

Klakap wrote:And I would also like to ask how are you filling Frame List.

Bonfra wrote:Would you like me to compile without VESA so you can test it on bochs or do you prefer the exact image that produced the outputs above?

Bonfra wrote:Basically, I'm starting and stopping the schedule for every transaction. before every operation, I clear it all with TD_TERMINATE and put a queue head as the first element pointing to the first TD so they get evaluated breath first.

BenLunt wrote: An image I can load and run with Bochs, please.

BenLunt wrote: Are your intentions Depth First instead of Breadth First?

BenLunt wrote: Does this make sense?

BenLunt wrote:

Bonfra wrote:Basically, I'm starting and stopping the schedule for every transaction. before every operation, I clear it all with TD_TERMINATE and put a queue head as the first element pointing to the first TD so they get evaluated breath first.

(Note that Breadth First is horizontal. Only one TD in that Queue will be executed, moving to the next Queue in this frame. Any concurrent TDs in any of these QHs will not be executed until the next frame or if any QH points back to any of these existing QHs. Are your intentions Depth First instead of Breadth First?)

In my opinion, it is not safe practice to start and stop the schedule. I am away from my desk, which means I am away from my notes, but if memory serves, the UHCI expects all memory to be static until the next frame. i.e.: during a frame, all memory that can be accessed by the controller is assumed to have already be written to physical memory. i.e.: you should not write to any Queue or TD that can be accessed by the UHCI during this frame (1ms). Same for reads, you should not assume any read is valid until the end-of-frame interrupt has been triggered.

Any read or write via an emulator will be instant. i.e.: as soon as the write is encountered, any further read from that memory will read the new value. However, on real hardware, the controller can and probably will read a full page (4k for example) of physical memory, using that cached page until a memory address is outside that 4k, triggering another 4k read. Therefore, any write to that 4k memory after the controller has read it, will not be valid until the next frame when the controller is required to read it again from physical memory.

At end-of-frame, the controller is to write back any memory that it may have modified (status flags, length, etc.), as well as when you stop the controller. What if you have written to a memory position that the controller thinks has been static, in turn writing back its values, over-writing your modification?

Again, this is all from memory, but if it serves me correctly, you should not write to any memory that the controller will access during that particular frame, and you must not read from any address that the controller can access during this frame until the end-of-frame is triggered.

A suggested way to do this is to have multiple periodical schedules within your frame's stack. i.e.: have every odd frame number point to one periodical stack, and every even frame number point to another stack. As for control/bulk TDs, do a similar technique. Every odd frame uses one stream of Control/Bulk Queues/TDs and every even numbered frame uses another stream. Then all you have to do is see which frame number the UHCI is on and use the correct periodical/control/bulk stream. With this technique, you can safely read and write to the "dormant" stream at will, as long as you take less than 1ms to make the modifications *and* none of the streams overlap each other. i.e.: no QH/TD in the odd stream is present in the even stream.

Klakap wrote:For debugging purposes you can do it even simpler. Just rewrite all frame list entries to be pointers to your first packet(or queue head), so your transfer will be processed immediately on next frame, no matter of frame number.