[PATCH 1/1] Staging: hv: Move the mouse driver out of staging

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh at suse.de]
> Sent: Tuesday, November 08, 2011 7:49 PM
> To: KY Srinivasan
> Cc: Dmitry Torokhov; linux-kernel at vger.kernel.org;
> devel at linuxdriverproject.org; ohering at suse.com; joe at perches.com;
> jkosina at suse.cz
> Subject: Re: [PATCH 1/1] Staging: hv: Move the mouse driver out of staging
> 
> On Wed, Nov 09, 2011 at 12:45:30AM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Dmitry Torokhov [mailto:dmitry.torokhov at gmail.com]
> > > Sent: Monday, November 07, 2011 12:51 AM
> > > To: KY Srinivasan
> > > Cc: gregkh at suse.de; linux-kernel at vger.kernel.org;
> > > devel at linuxdriverproject.org; virtualization at lists.osdl.org;
> ohering at suse.com;
> > > joe at perches.com; jkosina at suse.cz
> > > Subject: Re: [PATCH 1/1] Staging: hv: Move the mouse driver out of staging
> > >
> > > Hi K. Y,
> > >
> > > On Mon, Nov 07, 2011 at 01:04:53AM +0000, KY Srinivasan wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Dmitry Torokhov [mailto:dmitry.torokhov at gmail.com]
> > > > > Sent: Saturday, November 05, 2011 2:48 AM
> > > > > To: KY Srinivasan
> > > > > Cc: gregkh at suse.de; linux-kernel at vger.kernel.org;
> > > > > devel at linuxdriverproject.org; virtualization at lists.osdl.org;
> > > ohering at suse.com;
> > > > > joe at perches.com; jkosina at suse.cz
> > > > > Subject: Re: [PATCH 1/1] Staging: hv: Move the mouse driver out of
> staging
> > > > >
> > > > > Hi KY,
> > > >
> > > > Dimitry,
> > > >
> > > > Let me begin by thanking you for taking the time to review. I have
> incorporated
> > > > pretty much all your suggestions.
> > >
> > > Thank you very much for considering my suggestions.
> >
> > >
> > > > >
> > > > > Instead of potentially ever-increasing buffer that you also allocate
> > > > > (and it looks like leaking on every callback invocation) can you just
> > > > > repeat the read if you know that there are more data and use single
> > > > > pre-allocated buffer?
> > > >
> > > > The ring-buffer protocol is such that we need to consume the full message.
> > > > Also, why do you say we are leaking memory?
> > >
> > > Ah, OK, I see, we keep reading until read returns 0-sized reply and then
> > > we free the buffer... Never mind then.
> > >
> > > > > > +
> > > > > > +	hid_dev->ll_driver = &mousevsc_ll_driver;
> > > > > > +	hid_dev->driver = &mousevsc_hid_driver;
> > > > >
> > > > > You are not really hid driver; you are more of a "provider" so why do
> > > > > you need to set hid_dev->driver in addition to hid_dev->ll_driver?
> > > > >
> > > > True, but hid_parse_report() expects that the driver field be set; so I
> > > > need to fake this.
> > >
> > > If you supply .parse() method for your mousevsc_ll_driver structure and
> > > call hid_parse_report() from it then HID core will set the default
> > > driver and call parse at appropriate time.
> >
> > Would it be ok if I were to make this change after Jiri accepts the driver out of
> staging?
> 
> Why not fix this now?  No need to take a driver that is known to have
> issues, right?

The issue I am running into is that the mouse_vsc driver is both a low level driver as 
well as the higher level driver all rolled into one. This is based on my understanding of the
way the way the hid drivers are structured under driver/hid. Furthermore, there is going to
be only one driver using this hyperv low level driver and there is really nothing much to do in 
that driver either. Thus, in my mind it makes sense to structure the driver as I have currently structured.
In any event, I am going to send the patches out as I have them. If there is a consensus that further
changes are needed, I will obviously make them independent of where the driver is (either in staging 
or not). 

Regards,

K. Y