On Thu, Sep 25, 2008 at 6:56 AM, David Brownell <david-b@xxxxxxxxxxx> wrote: > > Small suggestion: next time you resend this, make sure > that $SUBJECT mentions it's a UART driver. Maybe even that > it's a SPI UART driver. That should help get more comments. > OK! > > This is a bit picky, but it's the first thing I noticed when > scanning the patch ... wierd comment layout! Either indent > those all, or (better) convert to kerneldoc style. I will have a look at kerneldoc and do the change. I'm waiting for a minor number in "Low-density serial ports" before resending the modifications asked by Andrew and Alan, so I will do this too. I just trusted M-q in emacs for the comment layout. > > Potentially less picky: probe() doesn't lock max3100s[], > neither does remove(), and in fact there seems to be no > lock for that table. Which suggests trouble in cases like I (wrongly!) was assuming that probing of devices is serialized. I will add the lock. > > And is that workqueue single threaded? > it's freezeabe and (looking at include/linux/workqueue.) it implies that it's single-threaded. This is important because I don't do locking since I presume all the I/O to the MAX3100 is done in just one workqueue. When I do I/O in other places (suspend for example) I assume that the worqueue is friezed so not running. Best regards and thanks for the review, -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
