lkml.org 
[lkml]   [2008]   [Sep]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [spi-devel-general] [PATCH RESEND] max3100 driver
On Thu, Sep 25, 2008 at 6:56 AM, David Brownell <david-b@pacbell.net> 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."


\
 
 \ /
  Last update: 2008-09-25 09:11    [W:0.075 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site