[lkml]   [2008]   [Sep]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] max3100 driver
Sorry, sent HTML mail by mistake, so resending :-/

On Sat, Sep 20, 2008 at 10:24 AM, Andrew Morton
<> wrote:

> > +#define MAX3100_MAJOR 204
> Allocating a new major is a Big Deal. It involves getting the major
> registered by contacting
> It's better to dynamically allocate it - let udev handle it.

I looked at other serial driver as an example and checked devices.txt:
if I don't get it wrong major 204 should be already reserved for
serial port. Anyway I choose a minor number already allocated by
mistake (did not see the "...") and will correct that. Is this ok or I
*have to* move to dynamic major (it's a bit a nuisance since max3100
is used in embedded system where udev is not always used)?

> `struct max3100_port' is sufficient, and would be more typical.
> > + struct uart_port port;
> > + struct spi_device *spi;
> > +
> > + int cts:1; /* last CTS received for flow ctrl */
> > + int tx_empty:1; /* last TX empty bit */
> These two bits will share a word and hence locking is needed to prevent
> modifications to one from trashing modifications to the other on SMP.
> That's OK, but it would be best to document that locking right here, and
> to check that it is adhered to.

I did not realize this until you explained me. I'm not sure if actual
packing of bit-fields is implementation dependent but I think so. If
this is right I guess it's better to avoid bit-fields in structs that
can be accessed concurrently (or otherwise I have to lock the entire
struct). So, should I avoid bit-fields altogether?

I will correct the patch and resend.


Christian Pellegrin, see
"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-20 12:37    [from the cache]
©2003-2014 Jasper Spaans. hosted at Digital Ocean