On Thu, Sep 01, 2011 at 01:27:33PM -0700, Greg KH wrote:
> On Wed, Aug 31, 2011 at 03:16:51PM -0700, K. Y. Srinivasan wrote:
> > 2) With your help, we have fixed all of the issues related to these drivers
> > conforming to the Linux Device Driver model. One of the TODO work items is
> > "audit the vmbus to verify it is working properly with the driver model".
>
> I have a few comments about this, I'll respond in another email.
Ok, it looks a _lot_ better, but I have a few minor nits, and one larger
one:
- rename the vmbus_child_* functions to vmbus_* as there's no need to
think of "children" here.
- vmbus_onoffer comment is incorrect. You handle offers from lots of
other types. Or if not, am I reading the code incorrectly?
- the static hv_cb_utils array needs to go away. In the hv_utils.c
util_probe() call, properly register the channel callback, and the
same goes for the util_remove() call, unregister things there.
Note, you can use the driver_data field to determine exactly which
callback needs to be registered to make things easy. Something like
the following (pseudo code only):
static const struct hv_vmbus_device_id id_table[] = {
/* Shutdown guid */
{ VMBUS_DEVICE(0x31, 0x60, 0x0B, 0X0E, 0x13, 0x52, 0x34, 0x49,
0x81, 0x8B, 0x38, 0XD9, 0x0C, 0xED, 0x39, 0xDB),
.driver_data = &shutdown_onchannelcallback },
....
};
util_probe(struct hv_device *dev, const struct hv_vmbus_device_id *id)
[ Yes, you will have to change the probe callback signature, but that's fine. ]
{
void *fn(void *context);
u8 *buffer;
fn = id->driver_data;
buffer = kmalloc(PAGE_SIZE, GFP_KERNEL);
/* Hook up callback and buffer with a call to the proper vmbus
* function
*/
...
}
util_remove()
{
/* undo what you did in util_probe(), unhooking the callback and
* freeing the data */
}
Does that make any sense?
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
