diff mbox

scsi-bus: Add support for SCSI scanners

Message ID 20160629173612.GA17059@ks392938.kimsufi.com
State New
Headers show

Commit Message

Jarkko Lavinen June 29, 2016, 5:36 p.m. UTC
On Tue, Jun 28, 2016 at 07:14:55PM +0200, Paolo Bonzini wrote:
> This is wrong, because INQUIRY's byte 3 is defined to be part of the
> length in modern SCSI standards.

> This is wrong, because INQUIRY's byte 3 is defined to be part of the
> length in modern SCSI standards.
Ok. I was using outdated ANSI spec too pedantically. The first patch is
not needed at all.

> SCAN conflicts with START_STOP.  Add a comment please saying that
> START_STOP has cmd->xfer set to 0 in scsi_req_xfer for non-scanner
> devices.
Done.

I also removed this:

    case GET_DATA_BUFFER_STATUS:
        cmd->xfer = buf[8] | (buf[7] << 8);

Since it is again unneedes and handled by scsi_cdb_xfer.

I added longer sense buffer in Request Sense command for scanners.
Multi Pro has at least one sense bit in byte 18 and Minolta's own SW
uses 20 bytes as an allocation size in Request Sense command.

The extra sense bit seems to be a kind of internal busy/film holder
moving status. After having this sense bit available scanning programs 
have been very stable and every feature seems to be working.

Jarkko

Comments

Paolo Bonzini June 30, 2016, 3:53 p.m. UTC | #1
On 29/06/2016 19:36, Jarkko Lavinen wrote:
> On Tue, Jun 28, 2016 at 07:14:55PM +0200, Paolo Bonzini wrote:
>> This is wrong, because INQUIRY's byte 3 is defined to be part of the
>> length in modern SCSI standards.
> 
>> This is wrong, because INQUIRY's byte 3 is defined to be part of the
>> length in modern SCSI standards.
> Ok. I was using outdated ANSI spec too pedantically. The first patch is
> not needed at all.
> 
>> SCAN conflicts with START_STOP.  Add a comment please saying that
>> START_STOP has cmd->xfer set to 0 in scsi_req_xfer for non-scanner
>> devices.
> Done.
> 
> I also removed this:
> 
>     case GET_DATA_BUFFER_STATUS:
>         cmd->xfer = buf[8] | (buf[7] << 8);
> 
> Since it is again unneedes and handled by scsi_cdb_xfer.
> 
> I added longer sense buffer in Request Sense command for scanners.
> Multi Pro has at least one sense bit in byte 18 and Minolta's own SW
> uses 20 bytes as an allocation size in Request Sense command.
> 
> The extra sense bit seems to be a kind of internal busy/film holder
> moving status. After having this sense bit available scanning programs 
> have been very stable and every feature seems to be working.

Thanks!  Out of curiosity what HBA are you using on the host?

Paolo
Jarkko Lavinen June 30, 2016, 7:44 p.m. UTC | #2
On Thu, Jun 30, 2016 at 05:53:37PM +0200, Paolo Bonzini wrote:
> > The extra sense bit seems to be a kind of internal busy/film holder
> > moving status. After having this sense bit available scanning programs 
> > have been very stable and every feature seems to be working.
> 
> Thanks!  Out of curiosity what HBA are you using on the host?

I did the testing with Adaptec 29320LPE. I have also 29160N on a PCI slot, but it does not support MSI. Initially I was trying to use PCI pass-through with IOMMU which requires MSI. I have also seen one kernel crash when probing / initializing 29160N but that was with Xen and IOMMU enabled.

Multi Pros have also a Firewire port. These are known issue over time when scanner refuses to initialize if set to use Firewire port but still work with SCSI port.

Jarkko
diff mbox

Patch

From dd85d2aac375db97e860a0908e5bfd5192d0dc61 Mon Sep 17 00:00:00 2001
From: Jarkko Lavinen <jarkko.lavinen@iki.fi>
Date: Wed, 29 Jun 2016 03:11:46 +0300
Subject: [PATCH 2/2] scsi-bus: Use longer sense buffer with scanners

Scanners can provide additional sense bytes beyond 18 bytes.
VueScan uses 32 bytes alloc length with Request Sense command.

Signed-off-by: Jarkko Lavinen <jarkko.lavinen@iki.fi>
---
 hw/scsi/scsi-bus.c     | 10 +++++++++-
 include/hw/scsi/scsi.h |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index e0fbaee..367c5c2 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -461,6 +461,14 @@  static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
     return true;
 }
 
+static size_t scsi_sense_len(SCSIRequest *req)
+{
+    if (req->dev->type == TYPE_SCANNER)
+        return SCSI_SENSE_LEN_XTRA;
+    else
+        return SCSI_SENSE_LEN;
+}
+
 static int32_t scsi_target_send_command(SCSIRequest *req, uint8_t *buf)
 {
     SCSITargetReq *r = DO_UPCAST(SCSITargetReq, req, req);
@@ -477,7 +485,7 @@  static int32_t scsi_target_send_command(SCSIRequest *req, uint8_t *buf)
         }
         break;
     case REQUEST_SENSE:
-        scsi_target_alloc_buf(&r->req, SCSI_SENSE_LEN);
+        scsi_target_alloc_buf(&r->req, scsi_sense_len(req));
         r->len = scsi_device_get_sense(r->req.dev, r->buf,
                                        MIN(req->cmd.xfer, r->buf_len),
                                        (req->cmd.buf[1] & 1) == 0);
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 8acd3fa..76d121d 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -10,6 +10,7 @@ 
 
 #define SCSI_CMD_BUF_SIZE     16
 #define SCSI_SENSE_LEN      18
+#define SCSI_SENSE_LEN_XTRA 32
 #define SCSI_INQUIRY_LEN    36
 
 typedef struct SCSIBus SCSIBus;
-- 
2.1.4