Message-ID: <464045F0.3040906@gmail.com>
Date:	Tue, 08 May 2007 11:42:08 +0200
From:	Rene Herman <rene.herman@...il.com>
To:	Andrew Morton <akpm@...l.org>
CC:	Pekka Enberg <penberg@...helsinki.fi>,
	Jens Axboe <jens.axboe@...cle.com>,
	Linux Kernel <linux-kernel@...r.kernel.org>
Subject: New Mitsumi legacy CD-ROM driver

Good day.

This is (the start of) a new driver for the legacy Mitsumi CD-ROM (LU-00xS 
and FX-001x) drives that used to be supported by drivers/cdrom/mcd(x).c, a 
driver that hasn't worked for a long time.

It's being tested on my LU-005S and is being submitted to -mm in the hope of 
maybe finding a few more testers and also in the hope of provoking a review. 
I'm co-authoring this with Pekka Enberg but certainly for me it's the first 
ever contact with anything having anything to do with block.

A few specific points/questions:

* This driver's core transfer function is doing:

   rq_for_each_bio(bio, req)
       bio_for_each_segment(bvec, bio, segno) {
           buffer = bvec_kmap_irq(bvec, &flags);
           ioread8_rep(ioaddr, buffer, bvec->bv_len);
           bvec_kunmap_irq(buffer, &flags);
       }
			
I've been slightly worried about struct bio having a bi_sector field. If 
we're not guaranteed that bio[n].bi_sector == bio[n-1].bi_sector + 1 during 
the rq_for_each_bio() loop we can't read from I/O into bio's directly but 
will need to use a temporary buffer and transfer into the bio's later. Is 
this guaranteed?

* This driver purports to support highmem. Given the number of machines out 
there that have both a Mitsumi legacy CD-ROM drive and enough physical 
memory installed to want highmem support this may come across as being 
overly... optimistic but is there much point to not supporting it? I believe 
we do always need the full rq_for_each_bio/bio_for_each_segment loop meaning 
that only the bvec_k{,un}map_irq could go, right?

* Some controllers do not support an IRQ; ading polled operation will not be 
hard. Some other controllers support DMA and I'll try and get one going and 
add that later as well.

* The hardware communication (mitsumi_{read,write}_cmd) can be improved 
upon. It was mostly taken from the old mcdx driver but every user is calling 
mitsumi_read_cmd with a 1 byte command, noone cares for the returned status 
and using MITSUMI_CMD #defines instead of spraying literals around as this 
is doing would be a bit nicer. This is subject to a later patch when all 
user's have been introduced though.

* All timeouts are up for grabs as well. Currently everything is using 
whole-second resolution timeouts but I'll need to instrument this a bit 
(preferabbly on more hardware samples than just my own) to see what makes 
sense. The current timeouts are mostly from the old driver.

* The next thing will be adding audio support to the driver. In fact, some 
parts were present already but have been deleted for now again to not muddy 
up the driver for review purposes. Some choices in there such as keeping the 
toc around as an array of struct cdrom_tocentry have been made with adding 
back mitsumi_audio_ioctl() in mind though and audio should prove relatively 
painless.

The plan...

Ofcourse not many have these old drives around anymore and even people that 
do aren't guaranteed to run 2.6 on the machines that host them. All legacy 
CD-ROM drivers in drivers/cdrom that I've looked at are and have been for 
quite some time broken and most do not serve a purpose anymore.

Mitsumi (mcdx, this driver), panasonic (sbpcd) and sony (cdu31a) are 
somewhat of an exception in so far that controllers for those three are 
still around on a lot of old ISA soundcards though. Moreover, I have the 
drives to test that trio so after this mitsumi driver is complete and in 
good shape I'll do the other two as well.

A few Media Vision based ISA soundcards have Philips CM206 controllers 
sitting on them and I may want to try and locate a matching drive for that 
but the rest is probably not useful to anyone for any purpose anymore and 
I'll submit a patch removing them when the ones that I want to keep are done.

Comments welcome...

Rene.


View attachment "mitsumi.diff" of type "text/plain" (20410 bytes)

Powered by blists - more mailing lists