> Please throw any comments, questions, suggestions, hard objects this way...
First obvious comments
+ while (uCount-- != 0) {
+ unsigned short val_lo, val_hi;
+ cli();
+ val_lo = InWordDsp(DSP_MsaDataISLow);
+ val_hi = InWordDsp(DSP_MsaDataDSISHigh);
+ put_user(val_lo, pusBuffer++);
+ put_user(val_hi, pusBuffer++);
+ sti();
+
1. Please use spinlocks not cli/sti as they will go away probably in 2.5
2. You can't touch user space holding interrupts off as it can't handle
page faults
+void PaceMsaAccess(unsigned short usDspBaseIO)
+{
+ schedule();
+ udelay(100);
+ schedule();
+}
If you are trying to be friendly then add
if(current->need_resched)
schedule()
just to be more efficient
+BOOLEAN dsp3780I_GetIPCSource(unsigned short usDspBaseIO,
+ unsigned short *pusIPCSourc
s/BOOLEAN/int
s/TRUE/0
s/FALSE/-Eappropriateval
would be more in keeping. Not a bug by any means
The ioctl locking seems wrong. It doesnt look like the DSP accesses are
locked against one another and you can issue multiple ioctls in parallel in
different threads
If mwave_read/write do nothing then they should really be returning an error
code. 0 is EOF, count on write is success.
+BOOLEAN smapi_init()
you want (void) or you get compiler warnings on some compiler revisions
+ PRINTK_1(TRACE_SMAPI, "smapi::smapi_init entry\n");
+
+ usSmapiID = CMOS_READ(0x7C);
+ usSmapiID |= (CMOS_READ(0x7D) << 8);
CMOS reads/writes must be done holding the lock against other cmos users
+int Initialize(THINKPAD_BD_DATA * pBDData)
Please dont have globals with names so generic
Hope those are useful
Alan
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Wed May 23 2001 - 21:00:21 EST