[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Patch] [bug #26237] multiple problems with usb devices
|
From: |
Vladimir 'φ-coder/phcoder' Serbinenko |
|
Subject: |
Re: [Patch] [bug #26237] multiple problems with usb devices |
|
Date: |
Wed, 07 Apr 2010 22:39:04 +0200 |
|
User-agent: |
Mozilla-Thunderbird 2.0.0.22 (X11/20091109) |
Aleš Nesrsta wrote:
>
>
>> + * So we should:
>> + * - allow WritebackDoneHead interrupt (WDH) by proper setting of last TD
>> + * token - it is done above in transaction settings
>> + * - detect setting of WDH bit in HcInterruptStatus register
>> + * - compare HccaDoneHead value with address of last-1 TD. If it is not
>> + * equal, check ED for halt and if not so, reset WDH bit and wait again
>> + * - but it should not happen - debug it!
>> Are the comments from you or any part copied from spec. We need no copy from
>> spec as spec is available anyway and sentences copied from it may cause
>> copyright problems
>>
>
> It is not copy from specification, it is only inspired by OHCI
> specification. I wrote it mainly for myself (and possibly others) to
> explain what is the main change against previous algorithm, which is not
> safe (from my point of view). It can be removed if it is not
> interesting.
>
Ok then.
> "Copied" are "official" names of OHCI registers, bits etc., i.e.
> HccaDoneHead, WDH bit, HcInterruptStatus etc. - but from my point of
> view it could be no problem - it is "keyword" which is necessary to
> "speak with the same language".
Ok, then.
> And it should be aim of such
> specifications, specially of such "free" specifications as USB
> specifications are. Or not?
>
>
Normally yes, but the details of license may be a problem. Many
specifications has a clause which forbids modification. GPL on the other
hand forbids forbiding modifications. So they are incompatible.
>
> Some questions and others:
>
> I am not so far oriented in such kind of development, so I am asking:
> Will You correct my patch yourself (as maintainer of code) or should I
> prepare new corrected code and related patch? And against which version
> - I noted that in meantime new official version was released...
>
Normally we ask contributors to clean their patches themselves. But
given the great importance of your changes I'll cleaned them up myself.
You can find the partially cleaned version in attachment.
I recommend using bzr trunk but USB code hasn't changed since few months
due to lack of dev time
> Did You tests on OHCI/UHCI and what was the result?
> What HW are You using for testing - some real PC or qemu ?
>
>
I have a computer with integrated UHCI and OHCI+EHCI PCI card.
Testing on it showed that your patch fixed UHCI but had no effect on
OHCI. with OHCI grub_ohci_detect_dev always sees status = 0. I see the
similar behaviour when testing usb on grub-yeeloong-firmware (but not
when chinloaded by pmon). I guess some register initialisation is missing.
> It was little bit surprise for me that OHCI and UHCI drivers are not
> using IRQ (and maybe also some other drivers). Is it intention of GRUB?
> Nothing against such philosophy, some things are easier in this way.
>
>
GRUB has chosen to be single thread for reducing complexity. But we have
plans to add IRQs nevertheless because it will easier porting drivers
and allow parallel uncompress and read.
> I shortly read the UHCI specification and I was thought about my
> comments of TD buffer size optimalization included in patch - such
> optimization cannot be done simply because TD preparation code is common
> for OHCI and UHCI and:
> - UHCI needs (as far as I understand) TD buffer size length up to packet
> size only (<=64 bytes).
> - In contradiction, OHCI allows up to 8Kbytes TD buffer size (if buffer
> is aligned to page).
> So there is the question if it make sense to do any optimization for
> OHCI - probably not. In fact slow data transfer and wasting of memory
> caused by not optimized TD buffer is probably not so big problem.
>
>
Keep it simple in this case.
> I read also part of EHCI specification. There is question if GRUB 2
> needs support of EHCI because:
> 1. According to EHCI specification, each EHCI interface should normaly
> have also OHCI (or UHCI ?) interface. So, any USB 2.0 interface should
> work (slowly) via GRUB2 UHCI/OHCI support.
> 2. EHCI is relative new HW - so it was usually used on PCs with BIOS
> which supports booting from USB - so, probably, there is not necessary
> to prepare special support in GRUB for such HW. But maybe I am wrong.
>
>
Unfortunately BIOS is far from being bugless especially in USB. We have
seen e.g. 128GiB limits. On Yeeloong grub is the firmware. And this
reasoning doesn't apply to PCI expansion cards.
And on my laptop a bug in legacy support prevents UHCI controller from
working properly w/o EHCI.
> Best regards
> Ales
>
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
=== modified file 'bus/usb/ohci.c'
--- bus/usb/ohci.c 2010-01-20 19:40:30 +0000
+++ bus/usb/ohci.c 2010-03-13 08:08:37 +0000
@@ -254,20 +274,33 @@
break;
}
- /* Generate no interrupts. */
+ /* Generate no interrupts. */
token |= 7 << 21;
/* Set the token. */
token |= toggle << 24;
token |= 1 << 25;
- buffer = (grub_uint32_t) data;
- buffer_end = buffer + size - 1;
+ /* Set "Not accessed" error code */
+ token |= 15 << 28;
+
+ /* Set correct buffer values in TD if zero transfer occurs */
+ if (size)
+ {
+ buffer = (grub_uint32_t) data;
+ buffer_end = buffer + size - 1;
+ td->buffer = grub_cpu_to_le32 (buffer);
+ td->buffer_end = grub_cpu_to_le32 (buffer_end);
+ }
+ else
+ {
+ td->buffer = 0;
+ td->buffer_end = 0;
+ }
+ /* Set the rest of TD */
td->token = grub_cpu_to_le32 (token);
- td->buffer = grub_cpu_to_le32 (buffer);
td->next_td = 0;
- td->buffer_end = grub_cpu_to_le32 (buffer_end);
}
static grub_usb_err_t
@@ -283,6 +316,9 @@
grub_uint32_t status;
grub_uint32_t control;
grub_usb_err_t err;
+
+ grub_uint8_t errcode = 0;
+ grub_ohci_td_t tderr = NULL;
int i;
/* Allocate an Endpoint Descriptor. */
@@ -310,6 +346,18 @@
td_list[i].next_td = grub_cpu_to_le32 (&td_list[i + 1]);
}
+ /* The last-1 TD token we should change to enable interrupt when TD finishes.
+ * As OHCI interrupts are disabled, it does only setting of WDH bit in
+ * HcInterruptStatus register - and that is what we want to safely detect
+ * end of all transactions. */
+ td_list[transfer->transcnt - 1].token &= ~(7 << 21);
+
+ td_list[transfer->transcnt].token = 0;
+ td_list[transfer->transcnt].buffer = 0;
+ td_list[transfer->transcnt].buffer_end = 0;
+ td_list[transfer->transcnt].next_td =
+ (grub_uint32_t) &td_list[transfer->transcnt];
+
/* Setup the Endpoint Descriptor. */
/* Set the device address. */
@@ -336,30 +384,48 @@
grub_dprintf ("ohci", "program OHCI\n");
/* Program the OHCI to actually transfer. */
+
+ /* Disable the Control and Bulk lists. */
+ control = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CONTROL);
+ control &= ~(3 << 4);
+ grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROL, control);
+
+ /* Clear BulkListFilled and ControlListFilled. */
+ status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CMDSTATUS);
+ status &= ~(3 << 1);
+ grub_ohci_writereg32 (o, GRUB_OHCI_REG_CMDSTATUS, status);
+
+ /* Now we should wait for start of next frame. Because we are not using
+ * interrupt, we reset SF bit and wait when it goes to 1. */
+ /* SF bit reset. (SF bit indicates Start Of Frame (SOF) packet) */
+ grub_ohci_writereg32 (o, GRUB_OHCI_REG_INTSTATUS, (1<<2));
+ /* Wait for new SOF */
+ while ((grub_ohci_readreg32 (o, GRUB_OHCI_REG_INTSTATUS) & 0x4) == 0);
+ /* Now it should be safe to change CONTROL and BULK lists. */
+
+ /* This we do for safety's sake - it should be done in previous call
+ * of grub_ohci_transfer and nobody should change it in meantime...
+ * It should be done before start of control or bulk OHCI list. */
+ o->hcca->donehead = 0;
+ grub_ohci_writereg32 (o, GRUB_OHCI_REG_INTSTATUS, (1 << 1)); /* Clears WDH */
+
switch (transfer->type)
{
case GRUB_USB_TRANSACTION_TYPE_BULK:
{
grub_dprintf ("ohci", "add to bulk list\n");
- status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CMDSTATUS);
- control = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CONTROL);
-
- /* Disable the Control and Bulk lists. */
- control &= ~(3 << 4);
- grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROL, control);
-
- /* Clear BulkListFilled. */
- status &= ~(1 << 2);
- grub_ohci_writereg32 (o, GRUB_OHCI_REG_CMDSTATUS, status);
-
+ /* Set BulkList Head and Current */
grub_ohci_writereg32 (o, GRUB_OHCI_REG_BULKHEAD, (grub_uint32_t) ed);
+ grub_ohci_writereg32 (o, GRUB_OHCI_REG_BULKCURR, 0);
/* Enable the Bulk list. */
+ control = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CONTROL);
control |= 1 << 5;
grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROL, control);
/* Set BulkListFilled. */
+ status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CMDSTATUS);
status |= 1 << 2;
grub_ohci_writereg32 (o, GRUB_OHCI_REG_CMDSTATUS, status);
@@ -368,22 +434,11 @@
case GRUB_USB_TRANSACTION_TYPE_CONTROL:
{
- grub_dprintf ("ohci", "add to control list\n");
- status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CMDSTATUS);
- control = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CONTROL);
-
- /* Disable the Control and Bulk lists. */
- control &= ~(3 << 4);
- grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROL, control);
-
- /* Clear ControlListFilled. */
- status &= ~(1 << 1);
- grub_ohci_writereg32 (o, GRUB_OHCI_REG_CMDSTATUS, status);
-
+ /* Set ControlList Head and Current */
grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROLHEAD,
(grub_uint32_t) ed);
- grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROLHEAD+1,
- (grub_uint32_t) ed);
+ grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROLCURR,
+ 0);
/* Enable the Control list. */
control |= 1 << 4;
@@ -402,33 +457,72 @@
grub_ohci_readreg32 (o, GRUB_OHCI_REG_CMDSTATUS));
/* Wait until the transfer is completed or STALLs. */
- while ((ed->td_head & ~0xf) != (ed->td_tail & ~0xf))
+ do
{
grub_cpu_idle ();
- grub_dprintf ("ohci", "head=0x%02x tail=0x%02x\n", ed->td_head,
ed->td_tail);
-
- /* Detected a STALL. */
- if (ed->td_head & 1)
- break;
+ /* Detected a HALT. */
+ if (grub_le_to_cpu32 (&ed->td_head) & 1)
+ break;
+
+ /* The original style of detection of finished transaction looks hazardeous.
+ while ((grub_le_to_cpu32 (grub_ohci_read_mem32 (&ed->td_head)) & ~0xf)
+ != (grub_le_to_cpu32 (grub_ohci_read_mem32 (&ed->td_tail)) & ~0xf));
+ */
+ /* This should be according to OHCI specification:
+ * TD is finished and ED is updated when TD is retired and HcDoneHead
+ * register updated and, if allowed by WDH bit, written into HccaDoneHead.
+ * So we should:
+ * - allow WritebackDoneHead interrupt (WDH) by proper setting of last TD
+ * token - it is done above in transaction settings
+ * - detect setting of WDH bit in HcInterruptStatus register
+ * - compare HccaDoneHead value with address of last-1 TD. If it is not
+ * equal, check ED for halt and if not so, reset WDH bit and wait again
+ * - but it should not happen - debug it!
+ */
+ if ((grub_ohci_readreg32 (o, GRUB_OHCI_REG_INTSTATUS) & 0x2) != 0)
+ {
+ if ((grub_le_to_cpu32 (o->hcca->donehead) & ~0xf)
+ == (grub_uint32_t) &td_list[transfer->transcnt - 1])
+ break;
+
+ /* Done Head can be updated on some another place if ED is halted.
*/
+ if (grub_le_to_cpu32 (&ed->td_head) & 1)
+ break;
+
+ /* If there is not HALT in ED, it is not correct, so debug it, reset
+ * donehead and WDH and continue waiting. */
+ grub_dprintf ("ohci", "Incorrect HccaDoneHead=0x%08x",
+ o->hcca->donehead);
+ o->hcca->donehead = 0;
+ grub_ohci_writereg32 (o, GRUB_OHCI_REG_INTSTATUS, (1 << 1));
+ continue;
+ }
}
-
- grub_dprintf ("ohci", "complete\n");
-
-/* if (ed->td_head & 1) */
-/* err = GRUB_USB_ERR_STALL; */
-/* else if (ed->td */
-
-
- if (ed->td_head & 1)
+ while (1);
+
+ /*
+ grub_dprintf ("ohci", "completed\n");
+ */
+
+ /* Additional debug - to be removed. */
+ /*
+ grub_dprintf ("ohci", "DoneHead_reg=0x%08x DoneHead_HCCA=0x%08x\n",
+ grub_ohci_readreg32 (o, GRUB_OHCI_REG_DONEHEAD),
+ grub_le_to_cpu32 (o->hcca->donehead));
+ */
+
+ if (grub_le_to_cpu32 (ed->td_head) & 1)
{
- grub_uint8_t errcode;
- grub_ohci_td_t tderr;
-
- tderr = (grub_ohci_td_t) grub_ohci_readreg32 (o,
- GRUB_OHCI_REG_DONEHEAD);
- errcode = tderr->token >> 28;
-
+ tderr = (grub_ohci_td_t)
+ (grub_ohci_readreg32 (o, GRUB_OHCI_REG_DONEHEAD) & ~0xf);
+ if (tderr == 0)
+ /* If DONEHEAD==0 it means that correct address is in HCCA.
+ * It should be always now! */
+ tderr = (grub_ohci_td_t) (grub_le_to_cpu32 (o->hcca->donehead) & ~0xf);
+
+ errcode = grub_le_to_cpu32 (tderr->token) >> 28;
+
switch (errcode)
{
case 0:
@@ -515,9 +639,38 @@
/* Clear BulkListFilled and ControlListFilled. */
status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CMDSTATUS);
- status &= ~((1 << 2) | (1 << 3));
+ status &= ~(3 << 1);
grub_ohci_writereg32 (o, GRUB_OHCI_REG_CMDSTATUS, status);
-
+
+ /* Set ED to be skipped - for safety */
+ ed->target |= grub_cpu_to_le32 (1 << 14);
+
+ /* Now we should wait for start of next frame. Because we are not using
+ * interrupt, we reset SF bit and wait when it goes to 1. */
+ /* SF bit reset. (SF bit indicates Start Of Frame (SOF) packet) */
+ grub_ohci_writereg32 (o, GRUB_OHCI_REG_INTSTATUS, (1<<2));
+ /* Wait for new SOF */
+ while ((grub_ohci_readreg32 (o, GRUB_OHCI_REG_INTSTATUS) & 0x4) == 0);
+ /* Now it should be safe to change CONTROL and BULK lists. */
+
+ /* Important cleaning. */
+ o->hcca->donehead = 0;
+ /* Additional debug print. to be removed.
+ grub_dprintf ("ohci", "Before reset of WDH: INTSTATUS=0x%08x\n",
+ grub_ohci_readreg32 (o, GRUB_OHCI_REG_INTSTATUS));
+ */
+ grub_ohci_writereg32 (o, GRUB_OHCI_REG_INTSTATUS, (1 << 1)); /* Clears WDH */
+ /* Additional debug print. to be removed.
+ grub_dprintf ("ohci", "After reset of WDH: INTSTATUS=0x%08x\n",
+ grub_ohci_readreg32 (o, GRUB_OHCI_REG_INTSTATUS));
+ */
+ grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROLHEAD, 0);
+ grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROLCURR, 0);
+ grub_ohci_writereg32 (o, GRUB_OHCI_REG_BULKHEAD, 0);
+ grub_ohci_writereg32 (o, GRUB_OHCI_REG_BULKCURR, 0);
+
+ grub_dprintf ("ohci", "OHCI finished, freeing, err=0x%02x\n", err);
+
/* XXX */
grub_free (td_list);
grub_free (ed);
=== modified file 'disk/scsi.c'
--- disk/scsi.c 2010-03-05 14:29:28 +0000
+++ disk/scsi.c 2010-03-13 07:59:11 +0000
@@ -50,6 +50,57 @@
}
+/* Check result of previous operation. */
+static grub_err_t
+grub_scsi_request_sense (grub_scsi_t scsi)
+{
+ struct grub_scsi_request_sense rs;
+ struct grub_scsi_request_sense_data rsd;
+ grub_err_t err;
+
+ rs.opcode = grub_scsi_cmd_request_sense;
+ rs.lun = scsi->lun << GRUB_SCSI_LUN_SHIFT;
+ rs.reserved1 = 0;
+ rs.reserved2 = 0;
+ rs.alloc_length = 0x12; /* XXX: Hardcoded for now */
+ rs.control = 0;
+
+ err = scsi->dev->read (scsi, sizeof (rs), (char *) &rs,
+ sizeof (rsd), (char *) &rsd);
+ if (err)
+ return err;
+
+ return GRUB_ERR_NONE;
+}
+/* Self commenting... */
+static grub_err_t
+grub_scsi_test_unit_ready (grub_scsi_t scsi)
+{
+ struct grub_scsi_test_unit_ready tur;
+ grub_err_t err;
+ grub_err_t err_sense;
+
+ tur.opcode = grub_scsi_cmd_test_unit_ready;
+ tur.lun = scsi->lun << GRUB_SCSI_LUN_SHIFT;
+ tur.reserved1 = 0;
+ tur.reserved2 = 0;
+ tur.reserved3 = 0;
+ tur.control = 0;
+
+ err = scsi->dev->read (scsi, sizeof (tur), (char *) &tur,
+ 0, NULL);
+
+ /* Each SCSI command should be followed by Request Sense.
+ If not so, many devices STALLs or definitely freezes. */
+ err_sense = grub_scsi_request_sense (scsi);
+ /* err_sense is ignored for now and Request Sense Data also... */
+
+ if (err)
+ return err;
+
+ return GRUB_ERR_NONE;
+}
+
/* Determine the the device is removable and the type of the device
SCSI. */
static grub_err_t
@@ -58,15 +109,23 @@
struct grub_scsi_inquiry iq;
struct grub_scsi_inquiry_data iqd;
grub_err_t err;
+ grub_err_t err_sense;
iq.opcode = grub_scsi_cmd_inquiry;
iq.lun = scsi->lun << GRUB_SCSI_LUN_SHIFT;
+ iq.page = 0;
iq.reserved = 0;
iq.alloc_length = 0x24; /* XXX: Hardcoded for now */
- iq.reserved2 = 0;
+ iq.control = 0;
err = scsi->dev->read (scsi, sizeof (iq), (char *) &iq,
sizeof (iqd), (char *) &iqd);
+
+ /* Each SCSI command should be followed by Request Sense.
+ If not so, many devices STALLs or definitely freezes. */
+ err_sense = grub_scsi_request_sense (scsi);
+ /* err_sense is ignored for now and Request Sense Data also... */
+
if (err)
return err;
@@ -83,13 +142,24 @@
struct grub_scsi_read_capacity rc;
struct grub_scsi_read_capacity_data rcd;
grub_err_t err;
+ grub_err_t err_sense;
rc.opcode = grub_scsi_cmd_read_capacity;
rc.lun = scsi->lun << GRUB_SCSI_LUN_SHIFT;
- grub_memset (rc.reserved, 0, sizeof (rc.reserved));
+ rc.logical_block_addr = 0;
+ rc.reserved1 = 0;
+ rc.reserved2 = 0;
+ rc.PMI = 0;
+ rc.control = 0;
err = scsi->dev->read (scsi, sizeof (rc), (char *) &rc,
sizeof (rcd), (char *) &rcd);
+
+ /* Each SCSI command should be followed by Request Sense.
+ If not so, many devices STALLs or definitely freezes. */
+ err_sense = grub_scsi_request_sense (scsi);
+ /* err_sense is ignored for now and Request Sense Data also... */
+
if (err)
return err;
@@ -107,6 +177,8 @@
{
grub_scsi_t scsi;
struct grub_scsi_read10 rd;
+ grub_err_t err;
+ grub_err_t err_sense;
scsi = disk->data;
@@ -118,7 +190,14 @@
rd.reserved2 = 0;
rd.pad = 0;
- return scsi->dev->read (scsi, sizeof (rd), (char *) &rd, size *
scsi->blocksize, buf);
+ err = scsi->dev->read (scsi, sizeof (rd), (char *) &rd, size *
scsi->blocksize, buf);
+
+ /* Each SCSI command should be followed by Request Sense.
+ If not so, many devices STALLs or definitely freezes. */
+ err_sense = grub_scsi_request_sense (scsi);
+ /* err_sense is ignored for now and Request Sense Data also... */
+
+ return err;
}
/* Send a SCSI request for DISK: read SIZE sectors starting with
@@ -129,6 +208,8 @@
{
grub_scsi_t scsi;
struct grub_scsi_read12 rd;
+ grub_err_t err;
+ grub_err_t err_sense;
scsi = disk->data;
@@ -139,7 +220,14 @@
rd.reserved = 0;
rd.control = 0;
- return scsi->dev->read (scsi, sizeof (rd), (char *) &rd, size *
scsi->blocksize, buf);
+ err = scsi->dev->read (scsi, sizeof (rd), (char *) &rd, size *
scsi->blocksize, buf);
+
+ /* Each SCSI command should be followed by Request Sense.
+ If not so, many devices STALLs or definitely freezes. */
+ err_sense = grub_scsi_request_sense (scsi);
+ /* err_sense is ignored for now and Request Sense Data also... */
+
+ return err;
}
#if 0
@@ -151,6 +239,8 @@
{
grub_scsi_t scsi;
struct grub_scsi_write10 wr;
+ grub_err_t err;
+ grub_err_t err_sense;
scsi = disk->data;
@@ -162,7 +252,14 @@
wr.reserved2 = 0;
wr.pad = 0;
- return scsi->dev->write (scsi, sizeof (wr), (char *) &wr, size *
scsi->blocksize, buf);
+ err = scsi->dev->write (scsi, sizeof (wr), (char *) &wr, size *
scsi->blocksize, buf);
+
+ /* Each SCSI command should be followed by Request Sense.
+ If not so, many devices STALLs or definitely freezes. */
+ err_sense = grub_scsi_request_sense (scsi);
+ /* err_sense is ignored for now and Request Sense Data also... */
+
+ return err;
}
/* Send a SCSI request for DISK: write the data stored in BUF to SIZE
@@ -173,6 +270,8 @@
{
grub_scsi_t scsi;
struct grub_scsi_write10 wr;
+ grub_err_t err;
+ grub_err_t err_sense;
scsi = disk->data;
@@ -183,7 +282,14 @@
wr.reserved = 0;
wr.pad = 0;
- return scsi->dev->write (scsi, sizeof (wr), (char *) &wr, size *
scsi->blocksize, buf);
+ err = scsi->dev->write (scsi, sizeof (wr), (char *) &wr, size *
scsi->blocksize, buf);
+
+ /* Each SCSI command should be followed by Request Sense.
+ If not so, many devices STALLs or definitely freezes. */
+ err_sense = grub_scsi_request_sense (scsi);
+ /* err_sense is ignored for now and Request Sense Data also... */
+
+ return err;
}
#endif
@@ -292,6 +398,16 @@
else
disk->has_partitions = 1;
+
+ /* According to USB MS tests, issue Test Unit Ready until OK */
+ /* XXX: there should be some timeout... */
+ do
+ {
+ err = grub_scsi_test_unit_ready (scsi);
+ }
+ while (err == GRUB_ERR_READ_ERROR);
+
+ /* Read capacity of media */
err = grub_scsi_read_capacity (scsi);
if (err)
{
@@ -302,12 +418,14 @@
/* SCSI blocks can be something else than 512, although GRUB
wants 512 byte blocks. */
- disk->total_sectors = ((scsi->size * scsi->blocksize)
- << GRUB_DISK_SECTOR_BITS);
+ disk->total_sectors = ((grub_uint64_t)scsi->size
+ * (grub_uint64_t)scsi->blocksize)
+ >> GRUB_DISK_SECTOR_BITS;
- grub_dprintf ("scsi", "capacity=%llu, blksize=%d\n",
- (unsigned long long) disk->total_sectors,
- scsi->blocksize);
+ grub_dprintf ("scsi", "blocks=%u, blocksize=%u\n",
+ scsi->size, scsi->blocksize);
+ grub_dprintf ("scsi", "Disk total 512 sectors = %llu\n",
+ disk->total_sectors);
return GRUB_ERR_NONE;
}
=== modified file 'disk/usbms.c'
--- disk/usbms.c 2010-01-20 08:12:47 +0000
+++ disk/usbms.c 2010-03-13 07:59:11 +0000
@@ -125,14 +125,16 @@
{
/* Bulk IN endpoint. */
usbms->in = endp;
- grub_usb_clear_halt (usbdev, endp->endp_addr & 128);
+ /* Clear Halt is not possible yet! */
+ /* grub_usb_clear_halt (usbdev, endp->endp_addr); */
usbms->in_maxsz = endp->maxpacket;
}
else if (!(endp->endp_addr & 128) && (endp->attrib & 3) == 2)
{
/* Bulk OUT endpoint. */
usbms->out = endp;
- grub_usb_clear_halt (usbdev, endp->endp_addr & 128);
+ /* Clear Halt is not possible yet! */
+ /* grub_usb_clear_halt (usbdev, endp->endp_addr); */
usbms->out_maxsz = endp->maxpacket;
}
}
@@ -143,6 +145,9 @@
return 0;
}
+ /* XXX: Activate the first configuration. */
+ grub_usb_set_configuration (usbdev, 1);
+
/* Query the amount of LUNs. */
err = grub_usb_control_msg (usbdev, 0xA1, 254,
0, i, 1, (char *) &luns);
@@ -151,8 +156,8 @@
/* In case of a stall, clear the stall. */
if (err == GRUB_USB_ERR_STALL)
{
- grub_usb_clear_halt (usbdev, usbms->in->endp_addr & 3);
- grub_usb_clear_halt (usbdev, usbms->out->endp_addr & 3);
+ grub_usb_clear_halt (usbdev, usbms->in->endp_addr);
+ grub_usb_clear_halt (usbdev, usbms->out->endp_addr);
}
/* Just set the amount of LUNs to one. */
@@ -164,8 +169,9 @@
/* XXX: Check the magic values, does this really make
sense? */
- grub_usb_control_msg (usbdev, (1 << 6) | 1, 255,
- 0, i, 0, 0);
+ /* What is it? Removed. */
+ /* grub_usb_control_msg (usbdev, (1 << 6) | 1, 255,
+ 0, i, 0, 0); */
/* XXX: To make Qemu work? */
if (usbms->luns == 0)
@@ -174,12 +180,12 @@
usbms->next = grub_usbms_dev_list;
grub_usbms_dev_list = usbms;
- /* XXX: Activate the first configuration. */
- grub_usb_set_configuration (usbdev, 1);
-
+ /* Reset recovery procedure */
/* Bulk-Only Mass Storage Reset, after the reset commands
will be accepted. */
grub_usbms_reset (usbdev, i);
+ grub_usb_clear_halt (usbdev, usbms->in->endp_addr);
+ grub_usb_clear_halt (usbdev, usbms->out->endp_addr);
return 0;
}
@@ -225,6 +231,7 @@
static grub_uint32_t tag = 0;
grub_usb_err_t err = GRUB_USB_ERR_NONE;
int retrycnt = 3 + 1;
+ grub_size_t i;
retry:
retrycnt--;
@@ -240,70 +247,86 @@
cbw.lun = scsi->lun << GRUB_SCSI_LUN_SHIFT;
cbw.length = cmdsize;
grub_memcpy (cbw.cbwcb, cmd, cmdsize);
+
+ /* Debug print of CBW content. */
+ grub_dprintf ("usb", "CBW: sign=0x%08x tag=0x%08x len=0x%08x\n",
+ cbw.signature, cbw.tag, cbw.transfer_length);
+ grub_dprintf ("usb", "CBW: flags=0x%02x lun=0x%02x CB_len=0x%02x\n",
+ cbw.flags, cbw.lun, cbw.length);
+ grub_dprintf ("usb", "CBW: cmd:\n %02x %02x %02x %02x %02x %02x %02x %02x
%02x %02x %02x %02x %02x %02x %02x %02x\n",
+ cbw.cbwcb[ 0], cbw.cbwcb[ 1], cbw.cbwcb[ 2], cbw.cbwcb[ 3],
+ cbw.cbwcb[ 4], cbw.cbwcb[ 5], cbw.cbwcb[ 6], cbw.cbwcb[ 7],
+ cbw.cbwcb[ 8], cbw.cbwcb[ 9], cbw.cbwcb[10], cbw.cbwcb[11],
+ cbw.cbwcb[12], cbw.cbwcb[13], cbw.cbwcb[14], cbw.cbwcb[15]);
- /* Write the request. */
+ /* Write the request. XXX: Error recovery should be corrected! */
err = grub_usb_bulk_write (dev->dev, dev->out->endp_addr & 15,
sizeof (cbw), (char *) &cbw);
if (err)
{
if (err == GRUB_USB_ERR_STALL)
{
+ grub_usb_clear_halt (dev->dev, dev->in->endp_addr);
grub_usb_clear_halt (dev->dev, dev->out->endp_addr);
goto retry;
}
return grub_error (GRUB_ERR_IO, "USB Mass Storage request failed");
}
- /* Read/write the data. */
- if (read_write == 0)
+ /* Read/write the data, (maybe) according to specification. */
+ if (size && (read_write == 0))
{
err = grub_usb_bulk_read (dev->dev, dev->in->endp_addr & 15, size, buf);
- grub_dprintf ("usb", "read: %d %d\n", err, GRUB_USB_ERR_STALL);
- if (err)
- {
- if (err == GRUB_USB_ERR_STALL)
- {
- grub_usb_clear_halt (dev->dev, dev->in->endp_addr);
- goto retry;
- }
- return grub_error (GRUB_ERR_READ_ERROR,
- "can't read from USB Mass Storage device");
- }
+ grub_dprintf ("usb", "read: %d %d\n", err, GRUB_USB_ERR_STALL);
+ if (err) goto CheckCSW;
+ /* Debug print of received data. */
+ grub_dprintf ("usb", "buf:\n");
+ if (size <= 256)
+ for (i=0; i<size; i++)
+ grub_dprintf ("usb", "0x%02x: 0x%02x\n", i, buf[i]);
+ else
+ grub_dprintf ("usb", "Too much data for debug print...\n");
}
- else
+ else if (size)
{
- err = grub_usb_bulk_write (dev->dev, dev->in->endp_addr & 15, size, buf);
+ err = grub_usb_bulk_write (dev->dev, dev->out->endp_addr & 15, size,
buf);
grub_dprintf ("usb", "write: %d %d\n", err, GRUB_USB_ERR_STALL);
- if (err)
- {
- if (err == GRUB_USB_ERR_STALL)
- {
- grub_usb_clear_halt (dev->dev, dev->out->endp_addr);
- goto retry;
- }
- return grub_error (GRUB_ERR_WRITE_ERROR,
- "can't write to USB Mass Storage device");
- }
+ grub_dprintf ("usb", "buf:\n");
+ /* Debug print of sent data. */
+ if (size <= 256)
+ for (i=0; i<size; i++)
+ grub_dprintf ("usb", "0x%02x: 0x%02x\n", i, buf[i]);
+ else
+ grub_dprintf ("usb", "Too much data for debug print...\n");
+ if (err) goto CheckCSW;
}
- /* Read the status. */
+ /* Read the status - (maybe) according to specification. */
+CheckCSW:
err = grub_usb_bulk_read (dev->dev, dev->in->endp_addr & 15,
sizeof (status), (char *) &status);
if (err)
{
- if (err == GRUB_USB_ERR_STALL)
- {
- grub_usb_clear_halt (dev->dev, dev->in->endp_addr);
+ grub_usb_clear_halt (dev->dev, dev->in->endp_addr);
+ err = grub_usb_bulk_read (dev->dev, dev->in->endp_addr & 15,
+ sizeof (status), (char *) &status);
+ if (err)
+ { /* Bulk-only reset device. */
+ grub_usbms_reset (dev->dev, dev->interface);
+ grub_usb_clear_halt (dev->dev, dev->in->endp_addr);
+ grub_usb_clear_halt (dev->dev, dev->out->endp_addr);
goto retry;
- }
- return grub_error (GRUB_ERR_READ_ERROR,
- "can't read status from USB Mass Storage device");
+ }
}
- /* XXX: Magic and check this code. */
+ /* Debug print of CSW content. */
+ grub_dprintf ("usb", "CSW: sign=0x%08x tag=0x%08x resid=0x%08x\n",
+ status.signature, status.tag, status.residue);
+ grub_dprintf ("usb", "CSW: status=0x%02x\n", status.status);
+
+ /* If phase error, do bulk-only reset device. */
if (status.status == 2)
{
- /* XXX: Phase error, reset device. */
grub_usbms_reset (dev->dev, dev->interface);
grub_usb_clear_halt (dev->dev, dev->in->endp_addr);
grub_usb_clear_halt (dev->dev, dev->out->endp_addr);
=== modified file 'include/grub/scsicmd.h'
--- include/grub/scsicmd.h 2008-08-27 15:05:00 +0000
+++ include/grub/scsicmd.h 2010-03-13 07:59:12 +0000
@@ -25,14 +25,24 @@
#define GRUB_SCSI_REMOVABLE_BIT 7
#define GRUB_SCSI_LUN_SHIFT 5
+struct grub_scsi_test_unit_ready
+{
+ grub_uint8_t opcode;
+ grub_uint8_t lun; /* 7-5 LUN, 4-0 reserved */
+ grub_uint8_t reserved1;
+ grub_uint8_t reserved2;
+ grub_uint8_t reserved3;
+ grub_uint8_t control;
+} __attribute__((packed));
+
struct grub_scsi_inquiry
{
grub_uint8_t opcode;
- grub_uint8_t lun;
- grub_uint16_t reserved;
- grub_uint16_t alloc_length;
- grub_uint8_t reserved2;
- grub_uint8_t pad[5];
+ grub_uint8_t lun; /* 7-5 LUN, 4-1 reserved, 0 EVPD */
+ grub_uint8_t page; /* page code if EVPD=1 */
+ grub_uint8_t reserved;
+ grub_uint8_t alloc_length;
+ grub_uint8_t control;
} __attribute__((packed));
struct grub_scsi_inquiry_data
@@ -47,12 +57,40 @@
char prodrev[4];
} __attribute__((packed));
+struct grub_scsi_request_sense
+{
+ grub_uint8_t opcode;
+ grub_uint8_t lun; /* 7-5 LUN, 4-0 reserved */
+ grub_uint8_t reserved1;
+ grub_uint8_t reserved2;
+ grub_uint8_t alloc_length;
+ grub_uint8_t control;
+} __attribute__((packed));
+
+struct grub_scsi_request_sense_data
+{
+ grub_uint8_t error_code; /* 7 Valid, 6-0 Err. code */
+ grub_uint8_t segment_number;
+ grub_uint8_t sense_key; /*7 FileMark, 6 EndOfMedia, 5 ILI, 4-0 sense key */
+ grub_uint32_t information;
+ grub_uint8_t additional_sense_length;
+ grub_uint32_t cmd_specific_info;
+ grub_uint8_t additional_sense_code;
+ grub_uint8_t additional_sense_code_qualifier;
+ grub_uint8_t field_replaceable_unit_code;
+ grub_uint8_t sense_key_specific[3];
+ /* there can be additional sense field */
+} __attribute__((packed));
+
struct grub_scsi_read_capacity
{
grub_uint8_t opcode;
- grub_uint8_t lun;
- grub_uint8_t reserved[8];
- grub_uint8_t pad[2];
+ grub_uint8_t lun; /* 7-5 LUN, 4-1 reserved, 0 reserved */
+ grub_uint32_t logical_block_addr; /* only if PMI=1 */
+ grub_uint8_t reserved1;
+ grub_uint8_t reserved2;
+ grub_uint8_t PMI;
+ grub_uint8_t control;
} __attribute__((packed));
struct grub_scsi_read_capacity_data
@@ -106,11 +144,13 @@
typedef enum
{
grub_scsi_cmd_inquiry = 0x12,
+ grub_scsi_cmd_test_unit_ready = 0x00,
grub_scsi_cmd_read_capacity = 0x25,
grub_scsi_cmd_read10 = 0x28,
grub_scsi_cmd_write10 = 0x2a,
grub_scsi_cmd_read12 = 0xa8,
- grub_scsi_cmd_write12 = 0xaa
+ grub_scsi_cmd_write12 = 0xaa,
+ grub_scsi_cmd_request_sense = 0x03
} grub_scsi_cmd_t;
typedef enum
=== modified file 'include/grub/usbtrans.h'
--- include/grub/usbtrans.h 2009-02-08 17:58:32 +0000
+++ include/grub/usbtrans.h 2010-03-13 07:59:12 +0000
@@ -86,9 +86,9 @@
#define GRUB_USB_REQ_HUB_GET_PORT_STATUS 0x00
-#define GRUB_USB_FEATURE_ENDP_HALT 0x01
-#define GRUB_USB_FEATURE_DEV_REMOTE_WU 0x02
-#define GRUB_USB_FEATURE_TEST_MODE 0x04
+#define GRUB_USB_FEATURE_ENDP_HALT 0x00
+#define GRUB_USB_FEATURE_DEV_REMOTE_WU 0x01
+#define GRUB_USB_FEATURE_TEST_MODE 0x02
#define GRUB_USB_HUB_STATUS_CONNECTED (1 << 0)
#define GRUB_USB_HUB_STATUS_LOWSPEED (1 << 9)
signature.asc
Description: OpenPGP digital signature
- Re: [Patch] [bug #26237] multiple problems with usb devices, Vladimir 'φ-coder/phcoder' Serbinenko, 2010/04/02
- Re: [Patch] [bug #26237] multiple problems with usb devices, Aleš Nesrsta, 2010/04/07
- Re: [Patch] [bug #26237] multiple problems with usb devices,
Vladimir 'φ-coder/phcoder' Serbinenko <=
- Re: [Patch] [bug #26237] multiple problems with usb devices, Aleš Nesrsta, 2010/04/08
- Re: [Patch] [bug #26237] multiple problems with usb devices, Vladimir 'φ-coder/phcoder' Serbinenko, 2010/04/16
- Re: [Patch] [bug #26237] multiple problems with usb devices, Aleš Nesrsta, 2010/04/20
- Re: [Patch] [bug #26237] multiple problems with usb devices, Vladimir 'φ-coder/phcoder' Serbinenko, 2010/04/24
- Re: [Patch] [bug #26237] multiple problems with usb devices, Aleš Nesrsta, 2010/04/28