[haiku-commits] Re: r42058 - haiku/trunk/src/add-ons/kernel/drivers/disk/usb/usb_disk
- From: kallisti5 <kallisti5@xxxxxxxxxxx>
- To: <haiku-commits@xxxxxxxxxxxxx>
- Date: Wed, 08 Jun 2011 17:21:18 -0500
On Wed, 8 Jun 2011 23:47:01 +0200 (CEST), mmlr@xxxxxxxx wrote:
On June 8, 2011 at 10:04 PM Siarzhuk Zharski <zharik@xxxxxx> wrote:
> ...
> If I understand right, you are trying to add support of CD/DVD
drives to
usb_disk? As far as I remember from my usb_scsi work the SCSI
command
sets for HDD and for CD-ROMs are different. From other side the
system
works with CDs differenttly from HDDs. Doesn't messing all into
single
driver overcomplicating it?
CD/DVD _readers_ will usually work just fine with usb_disk (unless
they time
out, what kallisti5 is trying to fix) because they support the normal
SCSI read
commands.
+1. The CD device shows up ok, mounts, and is stable.
BUT it isn't our long term goal to just stuff things into usb_disk to
make it
work with more device types (I think that's what Siarzhuk was getting
at). CD
recorders come to mind for example that require the MMC SCSI command
set. The
idea is to get usb_scsi working again to have these devices as actual
SCSI
devices. This will allow all of the supported SCSI commands to go
through to the
device without having to re-implement SCSI inside usb_disk (and it'll
bring
support for the IOScheduler as well).
+1. My goals were to improve what was out there to give end users a
better
perspective until a final "full" solution exists.
See http://dev.haiku-os.org/ticket/6181 as an example, the system
prompts the
end user to mount the CD-ROM Read only or read write... the icon is
also a usb
stick. While this is stable and does work, it is a little confusing in
the mean
time.
> - snooze(10000);
> + uint32_t snoozeTime = 1000000 * tries;
> + TRACE("snoozing %u microseconds for usb
lun\n",
> snoozeTime);
> + snooze(snoozeTime);
Why you not using the developer branch for such playings?
I think these changes don't add side effects for devices that were
working
previously already, so putting this particular change in trunk
shouldn't be
problematic.
I was pretty careful with these changes and did *lots* of testing with
various
devices to ensure there weren't any negative/nasty side effects. Now
if I began
refactoring large large sections of usb_disk (which I wouldn't without
notifying
the list first), then i could easily see a separate branch.
But as Axel already mentioned, just go with sensible types
(bigtime_t in this case) and don't change code just for the sake of
it (the
uint32 -> uint8 change). Thanks for using proper commit message style
+1, sorry for the spam. Corrected in r42062... with caps in the commit
message :)
Thanks!
-- Alex
Other related posts:
