[Cbe-oss-dev] [patch 04/16] powerpc and x86_64: Initial port of PCIe endpoint device driver for axon

[H Brett Bolen]

I looked at the notifier and determined I could not use it.  The 
notifier passes the same struct * to all clients,
and I need something to pass the instance struct back -- when you 
register for the callback, it takes
a callback function, and a generic void * pointer.  When you register 
for a notifier, it takes a callback function.
It seems like it also sent an int along, and I could have implemented a 
'lookup' in the host function to get
a void * ( or instance ptr), but I thought it was ugly.  I did not see a 
reason to fix something that already
works with a hack.

The advantage of a notifier, is that a list of callbacks is maintained 
-- multiple drivers can register
for the same event.  With my callback implementation, only a single 
callback can be registered
for an event type ( usually 0..7 for the int type).

Mark Nelson is correct in his post -- data[3] is invalid.


Brett


----- Forwarded by Howard B Bolen/Raleigh/IBM on 03/27/2008 06:51 AM -----

*Mark Nelson <markn at au1.ibm.com>*

03/27/2008 01:27 AM

	

To

	

Murali Iyer <mniyer at vnet.ibm.com>

cc

	

cbe-oss-dev at ozlabs.org, Howard B Bolen/Raleigh/IBM at IBMUS, Arnd Bergmann 
<arnd.bergmann at de.ibm.com>, Jay <fubar at linux.vnet.ibm.com>, Benjamin 
Herrenschmidt <bherren at au1.ibm.com>, Jesse P Arroyo/Rochester/IBM at IBMUS, 
Tim Schimke/Rochester/IBM at IBMUS

Subject

	

Re: [Cbe-oss-dev] [patch 04/16] powerpc and x86_64: Initial port of     
   PCIe endpoint device driver for Axon



	






Hi Murali,

I'm just starting to wrap my head around these patches and so far I just
have one comment (which may not be useful at all if you're reworking this
to use notifier lists anyway...)

Regards,
Mark.


Murali Iyer wrote:

+/**
+ * axon_d2d_callback - process d2d queue
+ * @type: msg type
+ * @data: msg data
+ */
+int                 axon_d2d_callback(int type, void *data)
+{
+                 struct d2d_message                 msg;
+                 struct axon *axon_priv = (struct axon  *) data;
+
+                 BUG_ON( axon_priv == NULL);
+
+                 /* drain the d2d message buffer */
+                 while (axon_receive_d2d_msg(axon_priv, &msg) == 0)     
            {
+                                  switch (msg.cmd) {
+                                  case D2D_MSG_DIAGS:
+                                                   pr_info("Diags D2D 
msg -- 0x%x 0x%x 0x%x 0x%x \n",
+                                                                   
 msg.data[0], msg.data[1], msg.data[2],
+                                                                   
 msg.data[3]);

 From patch #12:

struct d2d_message {                                  /* message for d2d 
communication (16 bytes) */
                u32                 cmd;
                u32                 data[3];
};

but in the line above you're trying to access msg.data[3] which is past
the end of the array.


+                                                   break;
+
+                                  case D2D_MSG_ENABLE:
+                                                   if ((msg.data[0] >= 
0) &&
+                                                       (msg.data[0] < 
AXON_MAX_HDW_MSGTYPE))
+                                                                   
 axon_priv->callbacks[msg.data[0]].
+                                                                       
              remote_enabled = 1;
+                                                   break;
+                                  case D2D_MSG_DISABLE:
+                                                   if ((msg.data[0] >= 
0) &&
+                                                       (msg.data[0] < 
AXON_MAX_HDW_MSGTYPE))
+                                                                   
 axon_priv->callbacks[msg.data[0]].
+                                                                       
              remote_enabled = 0;
+                                                   break;
+                                  default:
+                                                   printk(KERN_WARNING 
"received d2d msg with unrecognized"
+                                                        " cmd= 0x%x\n", 
msg.cmd);
+                                  }
+                 }
+
+                 return 0;
+}


-- 
Mark Nelson
Linux Kernel Developer, OzLabs,
Australia Development Laboratory & Linux Technology Centre
IBM Corporation
Office: +61 2 6201 8201  [T/L 70 18201]