Re: [patch 04/33] m68k: Atari keyboard and mouse support.



What's the reason for splitting this up? Things would be a quite a bit
simpler if all the code was directly in atakeyb.c.

+/*
+ * linux/atari/atakeyb.c

This is a good reason why filename comments are a really bad idea :)

+/* state: 0: off; >0: in progress; >1: 0xf1 received */
+static volatile int ikbd_self_test;
+/* timestamp when last received a char */
+static volatile unsigned long self_test_last_rcv;

Please don't use volatile variable in kernel code. While linux/m68k doesn't
support smp or preemptible kernels we should at least put in proper synchronization
at the API level.

+/* bitmap of keys reported as broken */
+static unsigned long broken_keys[128/(sizeof(unsigned long)*8)] = { 0, };

DECLARE_BITMAP()

+typedef enum kb_state_t {
+ KEYBOARD, AMOUSE, RMOUSE, JOYSTICK, CLOCK, RESYNC
+} KB_STATE_T;
+
+#define IS_SYNC_CODE(sc) ((sc) >= 0x04 && (sc) <= 0xfb)
+
+typedef struct keyboard_state {
+ unsigned char buf[6];
+ int len;
+ KB_STATE_T state;
+} KEYBOARD_STATE;

Please kill the typedefs and shouting names.

+#ifdef __MODULE__
+MODULE_PARM(mouse_threshold, "2i");
+#endif

__MODULE__ is never true and even if it was a MODULE_PARM wouldn't compile.
use an unconditional module_param instead.

--- linux-m68k-2.6.21.orig/include/linux/input.h
+++ linux-m68k-2.6.21/include/linux/input.h
@@ -676,6 +676,7 @@ struct input_absinfo {
#define BUS_I2C 0x18
#define BUS_HOST 0x19
#define BUS_GSC 0x1A
+#define BUS_ATARI 0x1B

Is this really a separate bus? Should't we have a BUS_ONBOARD or so instead?
-
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/



Relevant Pages

  • Re: SIMPLEQ_* macros from OpenBSD sys/queue.h
    ... this can be accomplished for kernel code, ... partly the reason why I noted that OpenBSD synchronized their queue.h ... > wouldn't hurt. ... As long as there *is* an equivalent macro that exactly matches the ...
    (freebsd-hackers)
  • Re: multi-user combo-box refresh not working
    ... i can't think of any reason that *not* splitting a multi-user database would ... be preferable to splitting it. ... monolithic database being accessed by more than one user at a time. ... this works as a single user, ...
    (microsoft.public.access.modulesdaovba)
  • Re: regex
    ... i thought compiled regex would be faster than splitting and .length. ... nethertheless, if you guy's say "OH NO!!!", i've no reason to demand ...
    (microsoft.public.dotnet.languages.csharp)
  • Re: News about DVRs
    ... from the splitting, "wood" be my guess. ... Well, with all the trees in my yard, they GIVE me kindling. ... You can't reason with someone whose first line of argument is ...
    (rec.arts.tv)
  • Re: Kernel tuning for large maxsockets
    ... >> Is there any reason I should not modify the kernel code to only let a small, ... I'd prefer separate tunables. ...
    (freebsd-net)