Thread: [jamesg@Filanet.com: changes to OHCI code...]
Brought to you by:
aeb,
bencollins
From: Sebastien R. <Seb...@sy...> - 2000-04-22 08:29:27
|
Here is a mail I have received from James Goodwin that can be of interest for the list, especially for people who still have problem with the reception of self-id packets. Most of the suggestion will be implemented in CVS soon. Sebastien Rougeaux ------- Start of forwarded message ------- From: James Goodwin <jamesg@Filanet.com> To: Seb...@sy... Subject: changes to OHCI code... Date: Fri, 21 Apr 2000 14:25:40 -0700 Content-Type: text/plain; charset="iso-8859-1" Hi Sebastien, I've been playing around with the 1394 stack a bit and your OHCI driver, and had a couple changes to the OHCI driver that you might be interested in checking out (have been running some sbp2 support on top of your driver). I can provide you a patch if you'd like, but figured that it would be better to explain the changes I made, and you could incorporate them as you see fit... or argue with me about the changes (my comments are noted by //)... ;-) One general issue I have for you (which I have no good fix for yet) is the bh handling of received async packets. Sometimes packets are missed on receive (the bh is not run). I think we need to loop in our bh to handle all async packets received, instead of only trying to handle one received packet in each bh run. My temp hack workaround for my sbp2 stuff right now is to run the dma_rcv_bh directly from the interrupt handler (but this is bad!). Cheers, - --James p.s. I'm a bit behind on synching up with the latest stack, so please excuse any duplicate fixes... =) =================== In ohci_initialize: // // We need to add delays after the soft reset, setting LPS, and enabling our link. This fixes the self-id // reception problem if we initialize with no devices attached (at least for my board). Probably don't need the // delay after the soft reset, since are polling for reset completion status, but the other two places need a // delay. Yes, delays like these are evil, but at least they only happen at driver initialization! // /* Soft reset */ if ((retval=ohci_soft_reset(ohci))<0) return retval; // // Delay aftger soft reset to make sure everything has settled down (sanity) // mdelay(100); /* Set Link Power Status (LPS) */ reg_write(ohci, OHCI1394_HCControlSet, 0x00080000); // // Delay after setting LPS in order to make sure link/phy communication is established // mdelay(100); /* Set the bus number */ reg_write(ohci, OHCI1394_NodeID, 0x0000ffc0); ... /* Enable link */ reg_write(ohci, OHCI1394_HCControlSet, 0x00020000); // // Delay after enabling the link to give the link some time // mdelay(100); =================== In ohci_irq_handler: // // In many cases we will receive a selfidcomplete interrupt while the bus is still bouncing around a bit, // especially due to hot-plugs of devices. One way to deal with this is to spin until the node id becomes // valid. Not the nicest thing to do in the world, and we should time-out if we never get a valid nodeid. // But, it quickly deals with the case where we hot-plug a new device in and the bus bounces around a // bit. // if (event & OHCI1394_selfIDComplete) { if (host->in_bus_reset) { // // Wait for our nodeid to become valid (could still be invalid if the bus is bouncing around // because of a hot-plug). TODO. Add a timeout in this loop. All of this code should really // be run from a bh, instead of from our ISR. // while (1) { node_id = reg_read(ohci, OHCI1394_NodeID); if (node_id & 0x80000000) { break; } } node_id = reg_read(ohci, OHCI1394_NodeID); =================== In handle_selfid: // // In many cases with current OHCI hardware, we get the selfIDError bit set on hot-plugs of devices. Quick fix // for this is to generate another bus reset in response to seeing this (which clears things up). This is what // Microsoft's OHCI implementation does as well. // /* Check status of self-id reception */ if ((self_id_count&0x80000000) || ((self_id_count&0x00FF0000) != (q[0]&0x00FF0000))) { PRINT(KERN_ERR, ohci->id, "Error in reception of self-id packets" "Self-id count: %08x q[0]: %08x", self_id_count, q[0]); // // We had an error, generate another bus reset in response. TODO. Actually read // the current value in the phy before generating a bus reset (read modify write). This way // we don't stomp any current gap count settings, etc. Also should keep a count of // number of times we do this error recovery bus reset, so that there is no chance of us // getting into an endless loop of bus resets. // reg_write(ohci, OHCI1394_PhyControl, 0x000041ff); return -1; } =================== In alloc_dma_rcv_ctx and alloc_dma_trm_ctx: spin_lock_init(&d->lock); /* initialize bottom handler */ d->task.sync = 0; // Need to make sure and initialize sync and next, in case the memory was not d->task.next = NULL; // zeroed when you allocated it... d->task.routine = dma_rcv_bh; d->task.data = (void*)d; =================== In ohci_irq_handler: /* Accept Physical requests from all nodes. */ reg_write(ohci,OHCI1394_AsReqFilterHiSet, 0xffffffff); reg_write(ohci,OHCI1394_AsReqFilterLoSet, 0xffffffff); // // Turn on phys dma reception. We should probably manage the filtering somehow, instead // of blindly turning it on. // reg_write(ohci,OHCI1394_PhyReqFilterHiSet, 0xffffffff); reg_write(ohci,OHCI1394_PhyReqFilterLoSet, 0xffffffff); reg_write(ohci,OHCI1394_PhyUpperBound, 0xffff0000); =================== In dma_rcv_bh: // // We need to handle write requests that are received to our middle address space (posted writes). // In this case, the hardware generates an ack_complete... but, if we pass the packet up to ieee1394, // it will try and send a response (which it shouldn't), because it assumes we returned ack_pending. // if (tcode != 0xE) { DBGMSG(ohci->id, "Split packet received from" " node %d ack=0x%02X spd=%d tcode=0x%X" " length=%d data=0x%08x ctx=%d", (d->spb[1]>>16)&0x3f, (d->spb[length/4-1]>>16)&0x1f, (d->spb[length/4-1]>>21)&0x3, tcode, length, d->spb[3], d->ctx); // // Handle case of posted writes. If we receive an ack_complete, // we should not send a response. Fake out upper layers by turning the // packet into a broadcast packet... we should really modify the core stack // to accept an ack received argument and figure out whether to reply. // if (((d->spb[length/4-1]>>16)&0x1f) == 0x11) { d->spb[0] |= (ALL_NODES<<16); } hpsb_packet_received(ohci->host, d->spb, length); } ... if (tcode != 0xE) { DBGMSG(ohci->id, "Packet received from node" " %d ack=0x%02X spd=%d tcode=0x%X" " length=%d data=0x%08x ctx=%d", (buf_ptr[1]>>16)&0x3f, (buf_ptr[length/4-1]>>16)&0x1f, (buf_ptr[length/4-1]>>21)&0x3, tcode, length, buf_ptr[3], d->ctx); // // Handle case of posted writes. If we receive an ack_complete, // we should not send a response. Fake out upper layers by turning the // packet into a broadcast packet... we should really modify the core stack // to accept an ack received argument and figure out whether to reply. // if (((buf_ptr[length/4-1]>>16)&0x1f) == 0x11) { buf_ptr[0] |= (ALL_NODES<<16); } hpsb_packet_received(ohci->host, buf_ptr, length); } ------- End of forwarded message ------- |