Arnd Bergmann wrote:
+       case MGSL_IOCGPARAMS32:
+               rc = get_params32(info, (struct MGSL_PARAMS32 __user 
*)compat_ptr(arg));
+               break;

No need for the cast here.

OK, for some reason I remember getting a warning
error without it. I must have been mistaken.

+
+       spin_lock_irqsave(&info->lock, flags);

no need for _irqsave, just use spin_{,un}lock_irq() when you know that
interrupts are enabled.

That makes me a little uneasy. The locking
mechanisms (and just about everything else) above the driver
seem to change frequently. This involves not just the VFS but
the tty core as well.

If you are confident this will not change, I will
switch to spin_lock(). I used spin_lock_irqsave() to be
more robust against changes to behavior outside my driver.

+/* provide 32 bit ioctl compatibility on 64 bit systems */
+struct MGSL_PARAMS32
+{
+       unsigned int    mode;
+       unsigned char   loopback;
+       unsigned short  flags;
+       unsigned char   encoding;
+       unsigned int    clock_speed;
+       unsigned char   addr_filter;
+       unsigned short  crc_type;
+       unsigned char   preamble_length;
+       unsigned char   preamble;
+       unsigned int    data_rate;
+       unsigned char   data_bits;
+       unsigned char   stop_bits;
+       unsigned char   parity;
+};

The definition is correct, but by convention it would be better to use
compat_ulong_t instead of unsigned int for those fields that are an
unsigned long in native mode.

OK

--
Paul
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to