Re: [Cbe-oss-dev] [PATCH 4/10] spidernet: zero out a pointer.

2007-05-22 Thread Linas Vepstas
On Thu, May 17, 2007 at 09:32:56AM +1000, Michael Ellerman wrote:
  +   hwdescr-buf_addr = 0x0;
 
 If you're going to be paranoid, shouldn't you do something here to make
 sure the value's hit the device?

I thought the whole point of paranoia is that its inexplicable.

Here's a delusional reply: I didn't see any point to it. 
1) a wmb would add overhead
2) the hardware is supposed to be looking at the status flag,
   anyway, and not misbehaving.
3) there is a wmb when the descr is actually refilled in such
   a way as to actually mean something to the hardware.

All that I really acomplished here is a minor trick to 
aid in debug printing when looking for something bad.

--linas

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Cbe-oss-dev] [PATCH 4/10] spidernet: zero out a pointer.

2007-05-22 Thread Segher Boessenkool
If you're going to be paranoid, shouldn't you do something here to 
make

sure the value's hit the device?


I thought the whole point of paranoia is that its inexplicable.

Here's a delusional reply: I didn't see any point to it.
1) a wmb would add overhead


A wmb() doesn't guarantee the write has reached the device.


2) the hardware is supposed to be looking at the status flag,
   anyway, and not misbehaving.


But you're paranoid, right?  Can't trust that device! :-)


Segher

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Cbe-oss-dev] [PATCH 4/10] spidernet: zero out a pointer.

2007-05-22 Thread Benjamin Herrenschmidt

 Here's a delusional reply: I didn't see any point to it. 
 1) a wmb would add overhead
 2) the hardware is supposed to be looking at the status flag,
anyway, and not misbehaving.
 3) there is a wmb when the descr is actually refilled in such
a way as to actually mean something to the hardware.
 
 All that I really acomplished here is a minor trick to 
 aid in debug printing when looking for something bad.

And the whole thing is moot because 0 is actually a perfectly valid DMA
address :-) I suspect spider will end up trying to hit some internal
register or it's PCIe or whatever it has mapped at 0 internally and will
blow up ... At least on spider, it's not RAM there

Ben.


-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Cbe-oss-dev] [PATCH 4/10] spidernet: zero out a pointer.

2007-05-16 Thread Michael Ellerman
On Wed, 2007-05-16 at 17:09 -0500, Linas Vepstas wrote:
 Invalidate a pointer as its pci_unmap'ed; this is a bit of 
 paranoia to make sure hardware doesn't continue trying to 
 DMA to it.
 
 Signed-off-by: Linas Vepstas [EMAIL PROTECTED]
 
 
  drivers/net/spider_net.c |7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)
 
 Index: linux-2.6.22-rc1/drivers/net/spider_net.c
 ===
 --- linux-2.6.22-rc1.orig/drivers/net/spider_net.c2007-05-15 
 13:31:11.0 -0500
 +++ linux-2.6.22-rc1/drivers/net/spider_net.c 2007-05-15 13:31:16.0 
 -0500
 @@ -1081,7 +1082,9 @@ spider_net_decode_one_descr(struct spide
   chain-tail = descr-next;
  
   /* unmap descriptor */
 - pci_unmap_single(card-pdev, hwdescr-buf_addr,
 + hw_buf_addr = hwdescr-buf_addr;
 + hwdescr-buf_addr = 0x0;

If you're going to be paranoid, shouldn't you do something here to make
sure the value's hit the device?

 + pci_unmap_single(card-pdev, hw_buf_addr,
   SPIDER_NET_MAX_FRAME, PCI_DMA_FROMDEVICE);
  

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person


signature.asc
Description: This is a digitally signed message part