Messages in this thread | | | Date | Tue, 27 Apr 2010 03:36:43 +0530 (IST) | From | Pavan Savoy <> | Subject | Re: [PATCH] drivers:staging: sources for ST core |
| |
Alan,
--- On Tue, 27/4/10, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> From: Alan Cox <alan@lxorguk.ukuu.org.uk> > Subject: Re: [PATCH] drivers:staging: sources for ST core > To: pavan_savoy@ti.com > Cc: gregkh@suse.de, linux-kernel@vger.kernel.org, npelly@google.com, pavan_savoy@yahoo.co.in > Date: Tuesday, 27 April, 2010, 3:33 AM > > > +/* function pointer pointing to either, > > + * st_kim_recv during registration to receive fw > download responses > > + * st_int_recv after registration to receive proto > stack responses > > + */ > > +void (*st_recv) (void*, const unsigned char*, long); > > You still need a context of some kind as I said before to > avoid thse > globals and single device limits.
The st_recv is a global pointer to a function, and changes only once in lifetime of the driver (after a function called download_firmware). This can be easily avoided by having a flag in my st_tty_recieve function, as in
if (test_bit(FIRMWARE_DOWNLOADED, st_gdata->st_state) /* * if fw download is in progress then route incoming data * to KIM for validation */ st_int_recv(data, count); else kim_recv(data, count); ST_DRV_VER("done %s", __func__);
However, the other global reference which was st_gdata, now is inside the tty->disc_data reference.
Should I put in the above ?
> And all the other issues - no explanation of the locking > etc don't seem > to have been touched either.
I have removed most un-wanted locking stuff, I could find. What else is required ? please suggest.
> I don't really see how I am supposed to review this stuff > when the > locking and flow control assumptions are completely opaque
Didn't get what is that is required about flow-control ? > Alan >
Send free SMS to your Friends on Mobile from your Yahoo! Messenger. Download Now! http://messenger.yahoo.com/download.php
| |