Le 2 nov. 08 à 07:00, Finn Thain a écrit :
Nice work!
Thank you.
Critique follows.
On Sat, 1 Nov 2008, Laurent@xxxxxxxxxxxx wrote:
Index: linux-2.6/arch/m68k/mac/config.c
===================================================================
--- linux-2.6.orig/arch/m68k/mac/config.c 2008-11-01
06:13:53.000000000 +0100
+++ linux-2.6/arch/m68k/mac/config.c 2008-11-01 06:14:39.000000000
+0100
@@ -70,6 +70,7 @@
extern void oss_init(void);
extern void psc_init(void);
extern void baboon_init(void);
+extern void swim_init(void);
extern void mac_mksound(unsigned int, unsigned int);
@@ -815,6 +816,7 @@
oss_init();
psc_init();
baboon_init();
+ swim_init();
}
It would reduce coupling if config.c didn't need to know about the
floppy
drive... I suspect that the initialisation can be simplified with
device_initcall.
OK.
static void __init mac_report_hardware(void)
Index: linux-2.6/arch/m68k/mac/swim.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/m68k/mac/swim.c 2008-11-01 06:14:39.000000000
+0100
...
+#define writePhase *(SWIMBase + 0x0800)
+#define readPhase *(SWIMBase + 0x1800)
These are unused.
OK
...
+ default:
+ SWIMBase = NULL;
+ printk("SWIM: unknown Macintosh: report to maintainer !\n");
If macintosh_config->ident is not in mac_data_table, config.c prints
"Detected Macintosh model: Unknown", so there's no need to repeat it
here.
...
Index: linux-2.6/drivers/block/swim.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/drivers/block/swim.c 2008-11-01 10:36:05.000000000
+0100
@@ -0,0 +1,886 @@
+/*
+ * Driver for SWIM (Sander. Woz Integrated Machine) floppy
controller
Sander Woz Integrated Machine, also known as Super Woz Integrated
Machine.
OK
...
+
+/* bits in mode register */
+
+#define CLFIFO 0x01
+#define ENBL1 0x02
+#define ENBL2 0x04
+#define ACTION 0x08
+#define WRITE_MODE 0x10
+#define HEDSEL 0x20
+#define MOTON 0x80
Extra tabs.
OK
HEAD_SEL? MOTOR_ON?
Yes
...
+
+static inline void swim_head(head_t head)
+{
+ /* FIXME: IWM reads bits SEL, CA2, CA1 to wait drive ready... */
By SEL, do you mean VIA1A_vHeadSel or the chip register? I wonder if
CA1
and CA2 were used for the floppy on 68000 macs... CA1 and CA2 are
timers
on the Macs we support. Guide to Mac Family hardware might be able to
clarify this a bit.
I check that.
+
+ /* wait drive is ready */
+
+ if (head == UPPER_HEAD)
+ swim_select(READ_DATA_1);
+ else if (head == LOWER_HEAD)
+ swim_select(READ_DATA_0);
+}
+
+static inline int swim_step(void)
+{
+ int wait;
+
+ swim_action(STEP);
+
+ for (wait = 0; wait < 80; wait++) {
Should this loop counter be referred to HZ too?
I check that, too.
...
+
+static struct floppy_struct floppy_type[4] = {
+ { 0, 0,0, 0,0,0x00,0x00,0x00,0x00,NULL }, /* 0 no testing */
+ { 720, 9,1,80,0,0x2A,0x02,0xDF,0x50,NULL }, /* 3 360KB SS 3.5" */
+ { 1440, 9,2,80,0,0x2A,0x02,0xDF,0x50,NULL }, /* 4 720KB 3.5" */
+ { 2880,18,2,80,0,0x1B,0x00,0xCF,0x6C,NULL }, /* 7 1.44MB
3.5" */
+};
The last 3 entries are unused...
You're wrong...
+
+static int get_floppy_geometry(int drive, int type, struct
floppy_struct **g)
+{
+ struct floppy_state *fs;
+ fs = &unit[drive];
+
+ if (drive > floppy_count)
+ return -ENODEV;
+
+ if (type >= ARRAY_SIZE(floppy_type))
+ return -EINVAL;
+
+ if (type)
+ *g = &floppy_type[type];
+ else if (fs->type == HD_MEDIA) /* High-Density media */
+ *g = &floppy_type[3];
+ else if (fs->head_number == 2) /* double-sided */
+ *g = &floppy_type[2];
+ else
+ *g = &floppy_type[1];
...since type is always 0. I wonder how we should deal with 400K and
800K
floppies. It's academic I guess
type is always 0, but floppy_type[] is used with values 3, 2 and 1.
Well, as I've not a lot of time, I'm working only on the MFM support
(to read floppy written on a PC)
...
+
+static int __init swim_init(void)
+{
+ printk(KERN_INFO "Inserting SWIM floppy driver\n");
Is this for debugging purposes? I'd prefer to avoid the log spam.
OK
+
+ if (base) {
+ printk(KERN_INFO "SWIM: Setting SWIMBase to 0x%p\n", base);
+ SWIMBase = (struct swim *)base;
+ }
+
+ return swim_floppy_init();
+}
+module_init(swim_init)
Can we elimiate the additions to arch/m68k/mac and merge the init
routine
from arch/m68k/mac/swim.c with this one by making it a
device_initcall?
I try this.
Regards,
Laurent
----------------------- Laurent Vivier ----------------------
"The best way to predict the future is to invent it."
- Alan Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
