Re: [spi-devel-general] [PATCH RESEND] max3100 driver

[chri]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/