RE: changes to OHCI code...

[James G. <jamesg@Filanet.com>]

Hi Sebastien,

Thanks for the response! When I get a free moment, will further
investigate some of these issues. Comments below.

Cheers!
--James

>-----Original Message-----
>From: Sebastien Rougeaux [mailto:Seb...@sy...]
>Sent: Saturday, April 22, 2000 1:21 AM
>To: jamesg@Filanet.com
>Cc: lin...@li...
>Subject: Re: changes to OHCI code...
>
>>>>>> "James" == James Goodwin <jamesg@Filanet.com> writes:
>
>James> 	// // We need to add delays after the soft reset,
setting LPS, and
>James> enabling our link. This fixes the self-id // reception problem
if we
>James> initialize with no devices attached (at least for my board).
Probably
>James> don't need the // delay after the soft reset, since are polling
for
>James> reset completion status, but the other two places need a //
delay. Yes,
>James> delays like these are evil, but at least they only happen at
driver
>James> initialization!  //
>
>I have added delays in the initialization loop. But I am not sure about
>the delay after enabling the link... Since enabling the link triggers a
>bus reset, I have moved it at the end of the initialization routine,
and
>I think the card should be ready immediatly after that to handle the
>bus reset... What do you think ?

I'm not sure that enabling the link should cause a bus reset. Hopefully
not... since higher levels call reset_host_bus right after init (would
result in two quick bus resets). Either way, since hardware interrupts
are not enabled until the very end of ohci_initialize it should be safe.
=)

>
>James> =================== In ohci_irq_handler:
>
>James> 	// // In many cases we will receive a selfidcomplete
interrupt while
>James> the bus is still bouncing around a bit, // especially due to
hot-plugs
>James> of devices. One way to deal with this is to spin until the node
id
>James> becomes // valid. Not the nicest thing to do in the world, and
we
>James> should time-out if we never get a valid nodeid.  // But, it
quickly
>James> deals with the case where we hot-plug a new device in and the
bus
>James> bounces around a // bit.  //
>
>If you are looping in the irq_handler until the node id is valid, isn't
>there any chance to miss self-id packets (or can the loop be a bit long
>for an irq handler ?).

I think it is a little ugly to loop in the irq_handler until the node id
is valid, since it can potentially be many ms in the isr. Would probably
be better to move all the self-id code, as well as the majority of the
rest of the irq_handler code to a bh. But, this change to loop on node
id valid has caused no problems in my testing (hot-plugging with up to
eight devices attached). 

Also, we shouldn't worry about losing self-ids packets. If we wait until
the node id is valid, we will always get the last set of self-ids (which
has been dma-ed into the self-id buffer)... the last set is all we
really care about anyway (who cares about sets of stale self-ids).
Again, I've done a decent bit of hot-plug testing with this change...   

>
>James> =================== In dma_rcv_bh:
>
>James> We need to handle write requests that are
>James> received to our middle address space (posted writes).  // In
this case,
>James> the hardware generates an ack_complete... but, if we pass the
packet up
>James> to ieee1394, // it will try and send a response (which it
shouldn't),
>James> because it assumes we returned ack_pending.
>
>James>                                 		// // Handle
case of posted
>James> writes. If we receive an ack_complete, // we should not send a
>James> response. Fake out upper layers by turning the // packet into a
>James> broadcast packet... we should really modify the core stack // to
accept
>James> an ack received argument and figure out whether to reply.  
>James> if (((d->spb[length/4-1]>>16)&0x1f) == 0x11) {
>                d-> spb[0] |= (ALL_NODES<<16);
>James  }
>
>Yes we should see that with Andreas. I also would like the subsystem to
>handle the single phy packet that is received once the bus reset is
>completed and that indicate the precise moment in the FIFO after which
>the new bus topology is in operation.

Yes, I think if we can report up to the higher level driver an "ack
sent" argument, it can determine whether it needs to send a response
packet.

>
>-- 
>Sebastien Rougeaux
>RSISE, The Australian National University