All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Long USB transfers problem
@ 2012-12-01 22:46 Aleš Nesrsta
  2012-12-10 10:57 ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-01-20 21:46 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 2 replies; 11+ messages in thread
From: Aleš Nesrsta @ 2012-12-01 22:46 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 1908 bytes --]

Hi,
I found some USB problem:

Some part of GRUB wants read "long" data block from USB mass storage
device when GRUB opens USB disk, e.g. when "ls" command is entered first
time after EHCI/UHCI/OHCI module is loaded.
"Long" means data block of 0x8000 bytes length - it is not too much
nowadays :-)

But:
Some USB devices have too low limit of bulk data packet size, e.g. 32
bytes or less - so, in such case the USB driver needs to use lot of
Transfer Descriptors (TDs) to transfer 32Kbytes of data.
Unfortunately, number of TDs is limited in GRUB driver(s) - and there is
not enough TDs in this situation.
The result is internal error, no data transfer is initiated by driver
and error code is returned to calling function from usbms.c.

It is vary bad situation: USB device receives CBW command to transfer
0x8000 bytes - but transfer of data is never started because driver run
out of TDs...
Some devices could be automatically recovered from such situation and
works normally later when GRUB tries read disk by smaller parts.
But some devices remains confused even if they are reset by USBMS
specific reset command.

So, I wrote simple experimental patch which splits "long" transfer into
smaller parts - it looks to solve this issue.
Patch solves only read transfer - AFAIK GRUB loader is not designed to
write some data into disk, so there probably never be transfer of "long"
data block in direction into USB mass storage device.
Maximal size 2Kbyte of USB data transfer data block (defined in
usbtrans.h) looks to be more or less optimal value.

It is probably not critical patch because this situation happens mainly
if USB device is full speed device - i.e. this problem is related mainly
to very old USB flash disks or some (older) special mass storage devices
like GPS devices, cameras, card readers etc. - which will be probably
never used as boot devices... (but - who knows...)  :-) 

BR,
Ales

[-- Attachment #2: usb_patch_121201_0 --]
[-- Type: text/x-patch, Size: 1421 bytes --]

--- ./grub/include/grub/usbtrans.h	2012-11-11 21:01:57.000000000 +0100
+++ ./grub_patched/include/grub/usbtrans.h	2012-11-30 21:35:11.000000000 +0100
@@ -19,6 +19,8 @@
 #ifndef	GRUB_USBTRANS_H
 #define	GRUB_USBTRANS_H	1
 
+#define MAX_USB_TRANSFER_LEN 0x0800
+
 typedef enum
   {
     GRUB_USB_TRANSFER_TYPE_IN,
--- ./grub/grub-core/bus/usb/usbtrans.c	2012-11-11 21:01:57.000000000 +0100
+++ ./grub_patched/grub-core/bus/usb/usbtrans.c	2012-11-30 22:42:04.000000000 +0100
@@ -351,11 +351,23 @@ grub_usb_err_t
 grub_usb_bulk_read (grub_usb_device_t dev,
 		    int endpoint, grub_size_t size, char *data)
 {
-  grub_size_t actual;
+  grub_size_t actual, transferred;
   grub_usb_err_t err;
-  err = grub_usb_bulk_readwrite (dev, endpoint, size, data,
-				 GRUB_USB_TRANSFER_TYPE_IN, 1000, &actual);
-  if (!err && actual != size)
+  grub_size_t current_size, position;
+
+  for (position = 0, transferred = 0;
+       position < size; position += MAX_USB_TRANSFER_LEN)
+    {
+      current_size = size - position;
+      if (current_size >= MAX_USB_TRANSFER_LEN)
+	current_size = MAX_USB_TRANSFER_LEN;
+      err = grub_usb_bulk_readwrite (dev, endpoint, current_size,
+              &data[position], GRUB_USB_TRANSFER_TYPE_IN, 1000, &actual);
+      transferred += actual;
+      if (err || (current_size != actual) ) break;
+    }
+
+  if (!err && transferred != size)
     err = GRUB_USB_ERR_DATA;
   return err;
 }


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Long USB transfers problem
  2012-12-01 22:46 [PATCH] Long USB transfers problem Aleš Nesrsta
@ 2012-12-10 10:57 ` Vladimir 'φ-coder/phcoder' Serbinenko
  2012-12-10 17:49   ` Aleš Nesrsta
  2013-01-20 21:46 ` Vladimir 'φ-coder/phcoder' Serbinenko
  1 sibling, 1 reply; 11+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-12-10 10:57 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 2540 bytes --]

On 01.12.2012 23:46, Aleš Nesrsta wrote:

> Hi,
> I found some USB problem:
> 
> Some part of GRUB wants read "long" data block from USB mass storage
> device when GRUB opens USB disk, e.g. when "ls" command is entered first
> time after EHCI/UHCI/OHCI module is loaded.
> "Long" means data block of 0x8000 bytes length - it is not too much
> nowadays :-)
> 
> But:
> Some USB devices have too low limit of bulk data packet size, e.g. 32
> bytes or less - so, in such case the USB driver needs to use lot of
> Transfer Descriptors (TDs) to transfer 32Kbytes of data.
> Unfortunately, number of TDs is limited in GRUB driver(s) - and there is
> not enough TDs in this situation.
> The result is internal error, no data transfer is initiated by driver
> and error code is returned to calling function from usbms.c.
> 

Can this be computed from the device info rather than hardwired to 2K?
Also do we have any other use for long USB transfers? If not, it's
easier to put this logic in scsi.c rather than adding it another time in
USB subsystem.

> It is vary bad situation: USB device receives CBW command to transfer
> 0x8000 bytes - but transfer of data is never started because driver run
> out of TDs...
> Some devices could be automatically recovered from such situation and
> works normally later when GRUB tries read disk by smaller parts.
> But some devices remains confused even if they are reset by USBMS
> specific reset command.
> 
> So, I wrote simple experimental patch which splits "long" transfer into
> smaller parts - it looks to solve this issue.
> Patch solves only read transfer - AFAIK GRUB loader is not designed to
> write some data into disk, so there probably never be transfer of "long"
> data block in direction into USB mass storage device.
> Maximal size 2Kbyte of USB data transfer data block (defined in
> usbtrans.h) looks to be more or less optimal value.
> 
> It is probably not critical patch because this situation happens mainly
> if USB device is full speed device - i.e. this problem is related mainly
> to very old USB flash disks or some (older) special mass storage devices
> like GPS devices, cameras, card readers etc. - which will be probably
> never used as boot devices... (but - who knows...)  :-) 
> 
> BR,
> Ales
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Long USB transfers problem
  2012-12-10 10:57 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2012-12-10 17:49   ` Aleš Nesrsta
  2012-12-10 20:33     ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 11+ messages in thread
From: Aleš Nesrsta @ 2012-12-10 17:49 UTC (permalink / raw)
  To: The development of GNU GRUB

Vladimir 'φ-coder/phcoder' Serbinenko píše v Po 10. 12. 2012 v 11:57
+0100:
> On 01.12.2012 23:46, Aleš Nesrsta wrote:
> 
> > Hi,
> > I found some USB problem:
> > 
> > Some part of GRUB wants read "long" data block from USB mass storage
> > device when GRUB opens USB disk, e.g. when "ls" command is entered first
> > time after EHCI/UHCI/OHCI module is loaded.
> > "Long" means data block of 0x8000 bytes length - it is not too much
> > nowadays :-)
> > 
> > But:
> > Some USB devices have too low limit of bulk data packet size, e.g. 32
> > bytes or less - so, in such case the USB driver needs to use lot of
> > Transfer Descriptors (TDs) to transfer 32Kbytes of data.
> > Unfortunately, number of TDs is limited in GRUB driver(s) - and there is
> > not enough TDs in this situation.
> > The result is internal error, no data transfer is initiated by driver
> > and error code is returned to calling function from usbms.c.
> > 
> 
> Can this be computed from the device info rather than hardwired to 2K?
That is very good question but I probably cannot say definite answer.
Theoretically we have two possible low limits related to each USBMS
device:
- SCSI device block size (sector size)
- USB device maximal packet size

I think - according to general USB specification - that the partial
intermediate bulk transfer of "large" data block can be any multiple of
endpoint wMaxPacketSize which is lower than or equal to unread length of
data.
But wMaxPacketSize itself is too low value in case of USB 1 devices, we
need some larger value to be more effective - i.e. at least we should
choose some multiplier...

So, probably better will be to use SCSI device block (sector) size.
Valid SCSI device block size should be included in data structure
returned by device as answer to Read Capacity X - this value is read by
grub_scsi_open function during USBMS device initialization.

Using of SCSI block size will be possible only in case of USB mass
storage device - but it is probably not problem, see my answer to Your
second question.


> Also do we have any other use for long USB transfers? If not, it's
> easier to put this logic in scsi.c rather than adding it another time in
> USB subsystem.
Currently GRUB supports only these three kind of USB devices:
- mass storage
- keyboard
- two USB-serial converters
Keyboard uses very short interrupt data blocks, serial converter
probably should not use longer bulk transfer than its internal buffer
size, i.e. around 256 bytes.
So, there is probably only one source of "long" transfers - mass storage
device, e.g. we can put splitting logic in scsi.c.

There is only one question, if scsi.c is really right place - scsi.c
could be potentially used for another kind of devices than USBMS and if
we split "long" transfers in scsi.c, we can lost performance on such
another devices.

As this problem is together USB implementation specific and USB mass
storage specific, I suggest to do splitting in usbms.c.
Functions grub_usbms_read and grub_usbms_write have access to related
grub_scsi structure, i.e. splitting in these functions can use SCSI
device block size of related USB mass storage device without any
additional modifications.

BR,
Ales



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Long USB transfers problem
  2012-12-10 17:49   ` Aleš Nesrsta
@ 2012-12-10 20:33     ` Vladimir 'φ-coder/phcoder' Serbinenko
  2012-12-11 20:40       ` Aleš Nesrsta
  0 siblings, 1 reply; 11+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-12-10 20:33 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 3259 bytes --]

On 10.12.2012 18:49, Aleš Nesrsta wrote:

> Vladimir 'φ-coder/phcoder' Serbinenko píše v Po 10. 12. 2012 v 11:57
> That is very good question but I probably cannot say definite answer.
> Theoretically we have two possible low limits related to each USBMS
> device:
> - SCSI device block size (sector size)
> - USB device maximal packet size
> 
> I think - according to general USB specification - that the partial
> intermediate bulk transfer of "large" data block can be any multiple of
> endpoint wMaxPacketSize which is lower than or equal to unread length of
> data.
> But wMaxPacketSize itself is too low value in case of USB 1 devices, we
> need some larger value to be more effective - i.e. at least we should
> choose some multiplier...
> 
> So, probably better will be to use SCSI device block (sector) size.
> Valid SCSI device block size should be included in data structure
> returned by device as answer to Read Capacity X - this value is read by
> grub_scsi_open function during USBMS device initialization.
> 
> Using of SCSI block size will be possible only in case of USB mass
> storage device - but it is probably not problem, see my answer to Your
> second question.
> 
> 
>> Also do we have any other use for long USB transfers? If not, it's
>> easier to put this logic in scsi.c rather than adding it another time in
>> USB subsystem.
> Currently GRUB supports only these three kind of USB devices:
> - mass storage
> - keyboard
> - two USB-serial converters
> Keyboard uses very short interrupt data blocks, serial converter
> probably should not use longer bulk transfer than its internal buffer
> size, i.e. around 256 bytes.
> So, there is probably only one source of "long" transfers - mass storage
> device, e.g. we can put splitting logic in scsi.c.
> 
> There is only one question, if scsi.c is really right place - scsi.c
> could be potentially used for another kind of devices than USBMS and if
> we split "long" transfers in scsi.c, we can lost performance on such
> another devices.
> 

We already have:
      /* PATA doesn't support more than 32K reads.
	 Not sure about AHCI and USB. If it's confirmed that either of
	 them can do bigger reads reliably this value can be moved to 'scsi'
	 structure.  */
      grub_size_t len = 32768 >> disk->log_sector_size;

So it would be that maximum transfer size would be part of structure.
Another 2 questions are:
1) Is this multi-schedule transfer reliable? What if we fail to add QD's
in time?
2) Do such transfers work with other device, in particular the ones we
may be interested in in the future?

> As this problem is together USB implementation specific and USB mass
> storage specific, I suggest to do splitting in usbms.c.
> Functions grub_usbms_read and grub_usbms_write have access to related
> grub_scsi structure, i.e. splitting in these functions can use SCSI
> device block size of related USB mass storage device without any
> additional modifications.
> 
> BR,
> Ales
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Long USB transfers problem
  2012-12-10 20:33     ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2012-12-11 20:40       ` Aleš Nesrsta
  0 siblings, 0 replies; 11+ messages in thread
From: Aleš Nesrsta @ 2012-12-11 20:40 UTC (permalink / raw)
  To: The development of GNU GRUB

Vladimir 'φ-coder/phcoder' Serbinenko píše v Po 10. 12. 2012 v 21:33
+0100:
> On 10.12.2012 18:49, Aleš Nesrsta wrote:
...
> So it would be that maximum transfer size would be part of structure.
> Another 2 questions are:
> 1) Is this multi-schedule transfer reliable?
I think yes - and it possibly depends on place where You will do
splitting:

a)
First possible place is in scsi.c inside grub_scsi_readXX before the
values are calculated for rd. In these functions You can calculate
smaller transfers and repeat whole SCSI command more than one time
(together with request sense).

From the USB point of view it will looks e.g. in this way:
Function grub_scsi_readXX receives request to read 4096 bytes and
maximal safe limit of QDs/TDs is 512 bytes for one bulk transfer for
related USB device.
So, function grub_scsi_readXX creates 8 smaller SCSI read transfers
which will on USB side result in
8-times repeating whole sequence of these transactions: 
- bulk OUT CBW (SCSI command: read 1 sector)
- bulk IN DATA (512 bytes)
- bulk IN CSW
- bulk OUT CBW (SCSI command: request sense)
- bulk IN DATA (sense data)
- bulk IN CSW

This kind of splitting should be absolutely correct according to any
specification - but there are very high overheads.


b)
Second possible place is in usbms.c inside grub_usbms_read (or also on
lower level - usbtrans.c, as I did in first experimental try). If
function grub_usbms_read receives read request of 4K data, its normal
flow without splitting will be:
- bulk OUT transaction for CBW (header & CDB contain length code of 4K)
- bulk IN transaction for 4K DATA transfer
- bulk IN transaction for CSW
With splitting it will be in this way:
- bulk OUT transaction for CBW (header & CDB contain length code of 4K)
- 8-times bulk IN transaction for 512 bytes DATA transfer
- bulk IN transaction for CSW

I.e., if we will do full SCSI command with final request sense with the
same value of size as in example in a), we will have only ONE execution
of following transaction sequence:
- bulk OUT CBW (SCSI command: read 4K)
- 8-times bulk IN DATA (512 bytes)
- bulk IN CSW
- bulk OUT CBW (SCSI command: request sense)
- bulk IN DATA (sense data)
- bulk IN CSW

I expect that also this kind of splitting is correct according to any
specification - in case of BULK transactions. It should be because bulk
transaction is nothing else than chain of IN/DATAx/ACK packets which I
can do on USB host controller sequentially in any chunks - there are not
any other "control" types of packets in bulk transactions which will
really tell "hello, device, there is end of bulk transfer planned in USB
host controller" :-) . This feature is related only to BULK transactions
(Maybe it will be work also on interrupt transfer - but I think probably
not. Fortunately, interrupt and control transfers should be never so
"long" to exhaust all TDs...).
There is important only one thing - all packets in chain should be of
wMaxPacketSize length (with exception of absolutely last packet) - even
if USBMS specification allows also smaller packets than wMaxPacketSize
(some devices (more precisely most of devices) have not proper
implementation of USBMS specification and have problems if intermediate
bulk transaction is not of wMaxPacketSize length).


If You compare a) and b), You will see big difference in number of
transactions which should be done - in a) case it should be 8x6=48, in
case b) there are only 13 transactions!
That is why I prefer to do splitting in usbms.c - or in lower level if
such "long" transfers can be generated by some another USB device than
USBMS.


>  What if we fail to add QD's
> in time?
I don't understand what do You mean by this question. Could You explain
it little bit more?


> 2) Do such transfers work with other device, in particular the ones we
> may be interested in in the future?
There I am also not sure if I good understood this question. Probably
You are afraid what happens if You will add some another kind of USB
device into GRUB (e.g. USB network class device etc.) - ?

If the new kind of USB device will generate also some "long" BULK
transfer which cannot be fulfilled with limited number of USB host
controller QDs/TDs, such bulk transfer should be also split.

As I tried to explain above, I am almost sure that splitting of BULK
transfers can be done generally and also on lowest possible level, i.e.
also in usbtrans.c like in my current experimantal patch - and it could
be done also for OUT bulk transactions, if necessary.

In usbtrans.c we know at least the wMaxPacketSize and possibly we can
more or less easily know here also the maximum of QDs/TDs of the USB
driver, so we can split bulk transfer here to be sure it allocates
maximally e.g. half of QDs/TDs etc.
If we will do bulk splitting in usbtrans.c, we will be (almost) sure we
are prepared for any kind of USB device... :-)
(OK, not for any kind - some USB device needs e.g. isochronous transfer
which we have not implemented in GRUB...)

-----

BTW, whole this discussion is related to one thing - all GRUB USB
drivers (UHCI, OHCI, EHCI) have limited number of QDs/TDs because of
static preallocation of these structures.

My question is - why is it done in this way? Why we have static
allocation of QDs/TDs?
(It was programmed in this way in OHCI/UHCI before I did any change. And
I did EHCI driver later in the same way only to keep the same philosophy
as OHCI/UHCI drivers, to be "compatible" as most as possible.)

Maybe is it according to necessary alignment of QDs/TDs which is/was not
possible in GRUB on dynamically allocated memory?
Or there are some high overheads if we will often allocate lot of small
memory chunks (specially of DMA "capable" memory) ?
Or?

If we will allocate/deallocate QDs/TDs dynamically, we can allocate any
possible number of them - and we will not need to limit size of bulk
transfers... But, of course, we should rewrite a lot in this case... :-(

BR,
Ales
> 
> > As this problem is together USB implementation specific and USB mass
> > storage specific, I suggest to do splitting in usbms.c.
> > Functions grub_usbms_read and grub_usbms_write have access to related
> > grub_scsi structure, i.e. splitting in these functions can use SCSI
> > device block size of related USB mass storage device without any
> > additional modifications.
> > 
> > BR,
> > Ales
> > 
> > 
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Long USB transfers problem
  2012-12-01 22:46 [PATCH] Long USB transfers problem Aleš Nesrsta
  2012-12-10 10:57 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-01-20 21:46 ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-02-11 23:13   ` Aleš Nesrsta
  1 sibling, 1 reply; 11+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-01-20 21:46 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 2329 bytes --]


Committed, thanks

On 01.12.2012 23:46, Aleš Nesrsta wrote:

> Hi,
> I found some USB problem:
> 
> Some part of GRUB wants read "long" data block from USB mass storage
> device when GRUB opens USB disk, e.g. when "ls" command is entered first
> time after EHCI/UHCI/OHCI module is loaded.
> "Long" means data block of 0x8000 bytes length - it is not too much
> nowadays :-)
> 
> But:
> Some USB devices have too low limit of bulk data packet size, e.g. 32
> bytes or less - so, in such case the USB driver needs to use lot of
> Transfer Descriptors (TDs) to transfer 32Kbytes of data.
> Unfortunately, number of TDs is limited in GRUB driver(s) - and there is
> not enough TDs in this situation.
> The result is internal error, no data transfer is initiated by driver
> and error code is returned to calling function from usbms.c.
> 
> It is vary bad situation: USB device receives CBW command to transfer
> 0x8000 bytes - but transfer of data is never started because driver run
> out of TDs...
> Some devices could be automatically recovered from such situation and
> works normally later when GRUB tries read disk by smaller parts.
> But some devices remains confused even if they are reset by USBMS
> specific reset command.
> 
> So, I wrote simple experimental patch which splits "long" transfer into
> smaller parts - it looks to solve this issue.
> Patch solves only read transfer - AFAIK GRUB loader is not designed to
> write some data into disk, so there probably never be transfer of "long"
> data block in direction into USB mass storage device.
> Maximal size 2Kbyte of USB data transfer data block (defined in
> usbtrans.h) looks to be more or less optimal value.
> 
> It is probably not critical patch because this situation happens mainly
> if USB device is full speed device - i.e. this problem is related mainly
> to very old USB flash disks or some (older) special mass storage devices
> like GPS devices, cameras, card readers etc. - which will be probably
> never used as boot devices... (but - who knows...)  :-) 
> 
> BR,
> Ales
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Long USB transfers problem
  2013-01-20 21:46 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-02-11 23:13   ` Aleš Nesrsta
  2013-02-24 18:37     ` Vladimir 'φ-coder/phcoder' Serbinenko
                       ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Aleš Nesrsta @ 2013-02-11 23:13 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 2418 bytes --]

Hi Vladimir,

in some e-mail in this thread You wrote:
> Can this be computed from the device info rather than hardwired to 2K?

So, there is "draft" of patch which makes maybe little bit more
sophisticated limiting of bulk transfer length.

How it works:
I added new variable "max_bulk_tds" into USB controller structure
"grub_usb_controller_dev". This value is controller specific, i.e. it is
set inside each USB controller driver during its registering.
Meaning of this variable is simple: How much TDs can be maximally used
per one bulk transfer.
This value is used inside function grub_usb_bulk_read together with
value of max. packet length to calculate max. possible bulk transfer
length in bytes - which is used instead of previous constant
MAX_USB_TRANSFER_LEN.
(The constant MAX_USB_TRANSFER_LEN is still used as some "safe value" in
case when max_bulk_tds is set to zero from some reason. This "safety"
code cen be deleted.)

Main problem is how to calculate proper value of "max_bulk_tds".
There shouldn't be maximal count of all TDs - because some TDs should be
reserved for potential simultaneous transfers. Even the GRUB is single
threaded, there can happen some simultaneous transfers (e.g.
"interrupts" for hubs or keyboards, possibly bulk transfer related to
USB/RS232 converter etc.) - all these transfers uses the same "heap" of
TDs (but, fortunately, they are using only few TDs, usually approx. 1-8
per each such transfer).

There is a problem to estimate how much devices could simultaneously
communicate in all real cases.
The only one thing which I hope I can expect (according to actual GRUB
single-threading) is: There should be no more than one "large" (i.e.
disc related) bulk transfer at the same time. (If this will be not
valid, then it will be necessary to do something else to prevent TDs
"exhausting".)

So, initially I tried to use very simple calculating/estimating of
"max_bulk_tds" = max. count of TDs * 3/4.
As the count of all TDs is usually about 600 in each kind of USB driver,
I hope there will be more than sufficient amount of "reserved" TDs and
also relative big count of TDs for large bulk transfer.

But, unfortunately, we still have some hardwired numbers here - only
with another meaning (maybe potentially more reasonable - ?) and
controller-specific (if necessary).

What do you prefer - this "improved" bulk transfer limiting or the
simple original one?

BR,
Ales

[-- Attachment #2: usb_patch_130211_0 --]
[-- Type: text/x-patch, Size: 4514 bytes --]

diff -purB ./grub/grub-core/bus/usb/ehci.c ./grub_patched/grub-core/bus/usb/ehci.c
--- ./grub/grub-core/bus/usb/ehci.c	2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/grub-core/bus/usb/ehci.c	2013-02-11 22:17:50.000000000 +0100
@@ -1902,7 +1897,9 @@ static struct grub_usb_controller_dev us
   .cancel_transfer = grub_ehci_cancel_transfer,
   .hubports = grub_ehci_hubports,
   .portstatus = grub_ehci_portstatus,
-  .detect_dev = grub_ehci_detect_dev
+  .detect_dev = grub_ehci_detect_dev,
+  /* estimated max. count of TDs for one bulk transfer */
+  .max_bulk_tds = GRUB_EHCI_N_TD * 3 / 4 
 };
 
 GRUB_MOD_INIT (ehci)
diff -purB ./grub/grub-core/bus/usb/ohci.c ./grub_patched/grub-core/bus/usb/ohci.c
--- ./grub/grub-core/bus/usb/ohci.c	2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/grub-core/bus/usb/ohci.c	2013-02-11 22:28:55.000000000 +0100
@@ -1431,7 +1431,9 @@ static struct grub_usb_controller_dev us
   .cancel_transfer = grub_ohci_cancel_transfer,
   .hubports = grub_ohci_hubports,
   .portstatus = grub_ohci_portstatus,
-  .detect_dev = grub_ohci_detect_dev
+  .detect_dev = grub_ohci_detect_dev,
+  /* estimated max. count of TDs for one bulk transfer */
+  .max_bulk_tds = GRUB_OHCI_TDS * 3 / 4
 };
 
 static struct grub_preboot *fini_hnd;
diff -purB ./grub/grub-core/bus/usb/uhci.c ./grub_patched/grub-core/bus/usb/uhci.c
--- ./grub/grub-core/bus/usb/uhci.c	2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/grub-core/bus/usb/uhci.c	2013-02-11 22:28:37.000000000 +0100
@@ -823,7 +823,9 @@ static struct grub_usb_controller_dev us
   .cancel_transfer = grub_uhci_cancel_transfer,
   .hubports = grub_uhci_hubports,
   .portstatus = grub_uhci_portstatus,
-  .detect_dev = grub_uhci_detect_dev
+  .detect_dev = grub_uhci_detect_dev,
+  /* estimated max. count of TDs for one bulk transfer */
+  .max_bulk_tds = N_TD * 3 / 4
 };
 
 GRUB_MOD_INIT(uhci)
diff -purB ./grub/grub-core/bus/usb/usbtrans.c ./grub_patched/grub-core/bus/usb/usbtrans.c
--- ./grub/grub-core/bus/usb/usbtrans.c	2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/grub-core/bus/usb/usbtrans.c	2013-02-11 22:28:04.000000000 +0100
@@ -354,13 +354,36 @@ grub_usb_bulk_read (grub_usb_device_t de
   grub_size_t actual, transferred;
   grub_usb_err_t err;
   grub_size_t current_size, position;
+  grub_size_t max_bulk_transfer_len = MAX_USB_TRANSFER_LEN;
+  grub_size_t max;
+
+  if (dev->controller.dev->max_bulk_tds)
+    {
+      /* Use the maximum packet size given in the endpoint descriptor.  */
+      /* This must be the same "calculation" of "max" as inside function */
+      /* grub_usb_bulk_setup_readwrite (should we create some function */
+      /* for it to prevent problems?) */
+      if (dev->initialized)
+        {
+          struct grub_usb_desc_endp *endpdesc;
+          endpdesc = grub_usb_get_endpdescriptor (dev, endpoint);
+          if (endpdesc)
+	    max = endpdesc->maxpacket;
+          else
+	    max = 64;
+        }
+      else
+        max = 64;
+      /* Calculate max. possible length of bulk transfer */
+      max_bulk_transfer_len = dev->controller.dev->max_bulk_tds * max;
+    }
 
   for (position = 0, transferred = 0;
-       position < size; position += MAX_USB_TRANSFER_LEN)
+       position < size; position += max_bulk_transfer_len)
     {
       current_size = size - position;
-      if (current_size >= MAX_USB_TRANSFER_LEN)
-	current_size = MAX_USB_TRANSFER_LEN;
+      if (current_size >= max_bulk_transfer_len)
+	current_size = max_bulk_transfer_len;
       err = grub_usb_bulk_readwrite (dev, endpoint, current_size,
               &data[position], GRUB_USB_TRANSFER_TYPE_IN, 1000, &actual);
       transferred += actual;
diff -purB ./grub/include/grub/usb.h ./grub_patched/include/grub/usb.h
--- ./grub/include/grub/usb.h	2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/include/grub/usb.h	2013-02-11 22:15:51.000000000 +0100
@@ -124,6 +124,13 @@ struct grub_usb_controller_dev
 
   /* Per controller flag - port reset pending, don't do another reset */
   grub_uint64_t pending_reset;
+
+  /* Max. number of transfer descriptors used per one bulk transfer */
+  /* The reason is to prevent "exhausting" of TD by large bulk */
+  /* transfer - number of TD is limited in USB host driver */
+  /* Value is calculated/estimated in driver - some TDs should be */
+  /* reserved for posible concurrent control or "interrupt" transfers */
+  grub_size_t max_bulk_tds;
   
   /* The next host controller.  */
   struct grub_usb_controller_dev *next;
 \f

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Long USB transfers problem
  2013-02-11 23:13   ` Aleš Nesrsta
@ 2013-02-24 18:37     ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-02-24 19:09     ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-02-24 19:25     ` Vladimir 'φ-coder/phcoder' Serbinenko
  2 siblings, 0 replies; 11+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-02-24 18:37 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 324 bytes --]


> What do you prefer - this "improved" bulk transfer limiting or the
> simple original one?

Did you really see the devices with max_packet_size more than 1024? It's
the maximum I see with my HDD adapters. And currently we don't look at
max_packet_size at all. This should be fixed first. I'm looking into it now.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Long USB transfers problem
  2013-02-11 23:13   ` Aleš Nesrsta
  2013-02-24 18:37     ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-02-24 19:09     ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-02-24 19:25     ` Vladimir 'φ-coder/phcoder' Serbinenko
  2 siblings, 0 replies; 11+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-02-24 19:09 UTC (permalink / raw)
  To: The development of GNU GRUB


[-- Attachment #1.1: Type: text/plain, Size: 27 bytes --]

Please try attached patch

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: usb_packet.diff --]
[-- Type: text/x-diff; name="usb_packet.diff", Size: 8127 bytes --]

=== modified file 'grub-core/bus/usb/serial/ftdi.c'
--- grub-core/bus/usb/serial/ftdi.c	2013-02-01 20:49:29 +0000
+++ grub-core/bus/usb/serial/ftdi.c	2013-02-24 18:58:04 +0000
@@ -128,7 +128,7 @@
 
   real_config (port);
 
-  grub_usb_bulk_write (port->usbdev, port->out_endp->endp_addr, 1, &cc);
+  grub_usb_bulk_write (port->usbdev, port->out_endp, 1, &cc);
 }
 
 static grub_err_t

=== modified file 'grub-core/bus/usb/serial/pl2303.c'
--- grub-core/bus/usb/serial/pl2303.c	2013-02-01 20:49:29 +0000
+++ grub-core/bus/usb/serial/pl2303.c	2013-02-24 18:57:48 +0000
@@ -146,7 +146,7 @@
 
   real_config (port);
 
-  grub_usb_bulk_write (port->usbdev, port->out_endp->endp_addr, 1, &cc);
+  grub_usb_bulk_write (port->usbdev, port->out_endp, 1, &cc);
 }
 
 static grub_err_t

=== modified file 'grub-core/bus/usb/serial/usbdebug_late.c'
--- grub-core/bus/usb/serial/usbdebug_late.c	2013-02-01 20:49:29 +0000
+++ grub-core/bus/usb/serial/usbdebug_late.c	2013-02-24 18:57:54 +0000
@@ -41,7 +41,7 @@
 {
   char cc = c;
 
-  grub_usb_bulk_write (port->usbdev, port->out_endp->endp_addr, 1, &cc);
+  grub_usb_bulk_write (port->usbdev, port->out_endp, 1, &cc);
 }
 
 static grub_err_t

=== modified file 'grub-core/bus/usb/usbtrans.c'
--- grub-core/bus/usb/usbtrans.c	2013-01-20 21:45:53 +0000
+++ grub-core/bus/usb/usbtrans.c	2013-02-24 18:57:19 +0000
@@ -333,38 +333,27 @@
   return err;
 }
 
-grub_usb_err_t
-grub_usb_bulk_write (grub_usb_device_t dev,
-		     int endpoint, grub_size_t size, char *data)
-{
-  grub_size_t actual;
-  grub_usb_err_t err;
-
-  err = grub_usb_bulk_readwrite (dev, endpoint, size, data,
-				 GRUB_USB_TRANSFER_TYPE_OUT, 1000, &actual);
-  if (!err && actual != size)
-    err = GRUB_USB_ERR_DATA;
-  return err;
-}
-
-grub_usb_err_t
-grub_usb_bulk_read (grub_usb_device_t dev,
-		    int endpoint, grub_size_t size, char *data)
+static grub_usb_err_t
+grub_usb_bulk_readwrite_packetize (grub_usb_device_t dev,
+				   struct grub_usb_desc_endp *endpoint,
+				   grub_transfer_type_t type,
+				   grub_size_t size, char *data)
 {
   grub_size_t actual, transferred;
   grub_usb_err_t err;
   grub_size_t current_size, position;
+  grub_size_t max_packet = endpoint->maxpacket;
 
   for (position = 0, transferred = 0;
-       position < size; position += MAX_USB_TRANSFER_LEN)
+       position < size; position += max_packet)
     {
       current_size = size - position;
-      if (current_size >= MAX_USB_TRANSFER_LEN)
-	current_size = MAX_USB_TRANSFER_LEN;
-      err = grub_usb_bulk_readwrite (dev, endpoint, current_size,
-              &data[position], GRUB_USB_TRANSFER_TYPE_IN, 1000, &actual);
+      if (current_size >= max_packet)
+	current_size = max_packet;
+      err = grub_usb_bulk_readwrite (dev, endpoint->endp_addr, current_size,
+              &data[position], type, 1000, &actual);
       transferred += actual;
-      if (err || (current_size != actual) ) break;
+      if (err || (current_size != actual)) break;
     }
 
   if (!err && transferred != size)
@@ -373,6 +362,26 @@
 }
 
 grub_usb_err_t
+grub_usb_bulk_read (grub_usb_device_t dev,
+		    struct grub_usb_desc_endp *endpoint,
+		    grub_size_t size, char *data)
+{
+  return grub_usb_bulk_readwrite_packetize (dev, endpoint,
+					    GRUB_USB_TRANSFER_TYPE_IN,
+					    size, data);
+}
+
+grub_usb_err_t
+grub_usb_bulk_write (grub_usb_device_t dev,
+		     struct grub_usb_desc_endp *endpoint,
+		     grub_size_t size, char *data)
+{
+  return grub_usb_bulk_readwrite_packetize (dev, endpoint,
+					    GRUB_USB_TRANSFER_TYPE_OUT,
+					    size, data);
+}
+
+grub_usb_err_t
 grub_usb_check_transfer (grub_usb_transfer_t transfer, grub_size_t *actual)
 {
   grub_usb_err_t err;

=== modified file 'grub-core/disk/usbms.c'
--- grub-core/disk/usbms.c	2013-01-20 15:52:15 +0000
+++ grub-core/disk/usbms.c	2013-02-24 18:56:43 +0000
@@ -326,7 +326,7 @@
 
   /* Write the request.
    * XXX: Error recovery is maybe still not fully correct. */
-  err = grub_usb_bulk_write (dev->dev, dev->out->endp_addr,
+  err = grub_usb_bulk_write (dev->dev, dev->out,
 			     sizeof (cbw), (char *) &cbw);
   if (err)
     {
@@ -341,7 +341,7 @@
   /* Read/write the data, (maybe) according to specification.  */
   if (size && (read_write == 0))
     {
-      err = grub_usb_bulk_read (dev->dev, dev->in->endp_addr, size, buf);
+      err = grub_usb_bulk_read (dev->dev, dev->in, size, buf);
       grub_dprintf ("usb", "read: %d %d\n", err, GRUB_USB_ERR_STALL); 
       if (err)
         {
@@ -362,7 +362,7 @@
     }
   else if (size)
     {
-      err = grub_usb_bulk_write (dev->dev, dev->out->endp_addr, size, buf);
+      err = grub_usb_bulk_write (dev->dev, dev->out, size, buf);
       grub_dprintf ("usb", "write: %d %d\n", err, GRUB_USB_ERR_STALL);
       grub_dprintf ("usb", "First 16 bytes of sent data:\n %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n",
   	buf[ 0], buf[ 1], buf[ 2], buf[ 3],
@@ -388,12 +388,12 @@
 
   /* Read the status - (maybe) according to specification.  */
 CheckCSW:
-  errCSW = grub_usb_bulk_read (dev->dev, dev->in->endp_addr,
+  errCSW = grub_usb_bulk_read (dev->dev, dev->in,
 		    sizeof (status), (char *) &status);
   if (errCSW)
     {
       grub_usb_clear_halt (dev->dev, dev->in->endp_addr);
-      errCSW = grub_usb_bulk_read (dev->dev, dev->in->endp_addr,
+      errCSW = grub_usb_bulk_read (dev->dev, dev->in,
 			        sizeof (status), (char *) &status);
       if (errCSW)
         { /* Bulk-only reset device. */
@@ -476,7 +476,7 @@
       else if (dev->protocol == GRUB_USBMS_PROTOCOL_CBI)
         {
           /* Try to get status from interrupt pipe */
-          err = grub_usb_bulk_read (dev->dev, dev->intrpt->endp_addr,
+          err = grub_usb_bulk_read (dev->dev, dev->intrpt,
                                     2, (char*)&status);
           grub_dprintf ("usb", "CBI cmdcb setup status: err=%d, status=0x%x\n", err, status);
         }
@@ -487,7 +487,7 @@
   /* Read/write the data, (maybe) according to specification.  */
   if (size && (read_write == 0))
     {
-      err = grub_usb_bulk_read (dev->dev, dev->in->endp_addr, size, buf);
+      err = grub_usb_bulk_read (dev->dev, dev->in, size, buf);
       grub_dprintf ("usb", "read: %d\n", err); 
       if (err)
         {
@@ -498,7 +498,7 @@
     }
   else if (size)
     {
-      err = grub_usb_bulk_write (dev->dev, dev->out->endp_addr, size, buf);
+      err = grub_usb_bulk_write (dev->dev, dev->out, size, buf);
       grub_dprintf ("usb", "write: %d\n", err);
       if (err)
         {
@@ -517,7 +517,7 @@
    * (we do not it yet) - ? */
   if (dev->protocol == GRUB_USBMS_PROTOCOL_CBI)
     { /* Check status in interrupt pipe */
-      err = grub_usb_bulk_read (dev->dev, dev->intrpt->endp_addr,
+      err = grub_usb_bulk_read (dev->dev, dev->intrpt,
                                 2, (char*)&status);
       grub_dprintf ("usb", "read status: %d\n", err);
       if (err)

=== modified file 'include/grub/usb.h'
--- include/grub/usb.h	2013-01-21 21:02:24 +0000
+++ include/grub/usb.h	2013-02-24 18:47:19 +0000
@@ -87,10 +87,12 @@
 
 grub_usb_err_t
 grub_usb_bulk_read (grub_usb_device_t dev,
-		    int endpoint, grub_size_t size, char *data);
+		    struct grub_usb_desc_endp *endpoint,
+		    grub_size_t size, char *data);
 grub_usb_err_t
 grub_usb_bulk_write (grub_usb_device_t dev,
-		     int endpoint, grub_size_t size, char *data);
+		     struct grub_usb_desc_endp *endpoint,
+		     grub_size_t size, char *data);
 
 grub_usb_err_t
 grub_usb_root_hub (grub_usb_controller_t controller);

=== modified file 'include/grub/usbtrans.h'
--- include/grub/usbtrans.h	2013-01-20 21:45:53 +0000
+++ include/grub/usbtrans.h	2013-02-24 18:48:55 +0000
@@ -19,8 +19,6 @@
 #ifndef	GRUB_USBTRANS_H
 #define	GRUB_USBTRANS_H	1
 
-#define MAX_USB_TRANSFER_LEN 0x0800
-
 typedef enum
   {
     GRUB_USB_TRANSFER_TYPE_IN,


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Long USB transfers problem
  2013-02-11 23:13   ` Aleš Nesrsta
  2013-02-24 18:37     ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-02-24 19:09     ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-02-24 19:25     ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-03-07 19:19       ` Aleš Nesrsta
  2 siblings, 1 reply; 11+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-02-24 19:25 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 418 bytes --]

> What do you prefer - this "improved" bulk transfer limiting or the

> simple original one?

I misread what patch does. It looks better now. Can you propagate the
change to bulk_write as well, like in the patch I sent?

> 
> BR,
> Ales
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Long USB transfers problem
  2013-02-24 19:25     ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-03-07 19:19       ` Aleš Nesrsta
  0 siblings, 0 replies; 11+ messages in thread
From: Aleš Nesrsta @ 2013-03-07 19:19 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]

Hi Vladimir,

do you mean something like that ?

Or do you want also to change "int endpoint" to "struct
grub_usb_desc_endp *endpoint" - like in Your patch ?


Additionally some (unrelated) note:
There is also waiting important USB patch in bug
https://savannah.gnu.org/bugs/?36740
(related to USB 2.0 Split Transfers) - could You look there?


(Sorry about long delay, I was on foreign business trip where I was too
busy... And, unfortunately, I will go there next week again for approx.
one month.)

BR,
Ales


Vladimir 'φ-coder/phcoder' Serbinenko píše v Ne 24. 02. 2013 v 20:25
+0100:
> > What do you prefer - this "improved" bulk transfer limiting or the
> 
> > simple original one?
> 
> I misread what patch does. It looks better now. Can you propagate the
> change to bulk_write as well, like in the patch I sent?
> 
> > 
> > BR,
> > Ales
> > 
> > 
> > 
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


[-- Attachment #2: usb_patch_130307_0 --]
[-- Type: text/x-patch, Size: 6996 bytes --]

diff -purB ./grub/grub-core/bus/usb/ehci.c ./grub_patched/grub-core/bus/usb/ehci.c
--- ./grub/grub-core/bus/usb/ehci.c	2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/grub-core/bus/usb/ehci.c	2013-03-07 19:14:33.000000000 +0100
@@ -1902,7 +1902,9 @@ static struct grub_usb_controller_dev us
   .cancel_transfer = grub_ehci_cancel_transfer,
   .hubports = grub_ehci_hubports,
   .portstatus = grub_ehci_portstatus,
-  .detect_dev = grub_ehci_detect_dev
+  .detect_dev = grub_ehci_detect_dev,
+  /* estimated max. count of TDs for one bulk transfer */
+  .max_bulk_tds = GRUB_EHCI_N_TD * 3 / 4 
 };
 
 GRUB_MOD_INIT (ehci)
diff -purB ./grub/grub-core/bus/usb/ohci.c ./grub_patched/grub-core/bus/usb/ohci.c
--- ./grub/grub-core/bus/usb/ohci.c	2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/grub-core/bus/usb/ohci.c	2013-03-07 19:14:33.000000000 +0100
@@ -1431,7 +1431,9 @@ static struct grub_usb_controller_dev us
   .cancel_transfer = grub_ohci_cancel_transfer,
   .hubports = grub_ohci_hubports,
   .portstatus = grub_ohci_portstatus,
-  .detect_dev = grub_ohci_detect_dev
+  .detect_dev = grub_ohci_detect_dev,
+  /* estimated max. count of TDs for one bulk transfer */
+  .max_bulk_tds = GRUB_OHCI_TDS * 3 / 4
 };
 
 static struct grub_preboot *fini_hnd;
diff -purB ./grub/grub-core/bus/usb/uhci.c ./grub_patched/grub-core/bus/usb/uhci.c
--- ./grub/grub-core/bus/usb/uhci.c	2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/grub-core/bus/usb/uhci.c	2013-03-07 19:14:33.000000000 +0100
@@ -823,7 +823,9 @@ static struct grub_usb_controller_dev us
   .cancel_transfer = grub_uhci_cancel_transfer,
   .hubports = grub_uhci_hubports,
   .portstatus = grub_uhci_portstatus,
-  .detect_dev = grub_uhci_detect_dev
+  .detect_dev = grub_uhci_detect_dev,
+  /* estimated max. count of TDs for one bulk transfer */
+  .max_bulk_tds = N_TD * 3 / 4
 };
 
 GRUB_MOD_INIT(uhci)
diff -purB ./grub/grub-core/bus/usb/usbtrans.c ./grub_patched/grub-core/bus/usb/usbtrans.c
--- ./grub/grub-core/bus/usb/usbtrans.c	2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/grub-core/bus/usb/usbtrans.c	2013-03-07 20:09:38.000000000 +0100
@@ -25,6 +25,26 @@
 #include <grub/usbtrans.h>
 #include <grub/time.h>
 
+
+static inline unsigned int
+grub_usb_bulk_maxpacket (grub_usb_device_t dev, int endpoint)
+{
+  unsigned int max = 64;
+
+  /* Use the maximum packet size given in the endpoint descriptor.  */
+  if (dev->initialized)
+    {
+      struct grub_usb_desc_endp *endpdesc;
+      endpdesc = grub_usb_get_endpdescriptor (dev, endpoint);
+
+      if (endpdesc)
+	max = endpdesc->maxpacket;
+    }
+
+  return max;
+}
+
+
 static grub_usb_err_t
 grub_usb_execute_and_wait_transfer (grub_usb_device_t dev, 
 				    grub_usb_transfer_t transfer,
@@ -224,20 +244,6 @@ grub_usb_bulk_setup_readwrite (grub_usb_
   if (type == GRUB_USB_TRANSFER_TYPE_OUT)
     grub_memcpy ((char *) data, data_in, size);
 
-  /* Use the maximum packet size given in the endpoint descriptor.  */
-  if (dev->initialized)
-    {
-      struct grub_usb_desc_endp *endpdesc;
-      endpdesc = grub_usb_get_endpdescriptor (dev, endpoint);
-
-      if (endpdesc)
-	max = endpdesc->maxpacket;
-      else
-	max = 64;
-    }
-  else
-    max = 64;
-
   /* Create a transfer.  */
   transfer = grub_malloc (sizeof (struct grub_usb_transfer));
   if (! transfer)
@@ -246,6 +252,8 @@ grub_usb_bulk_setup_readwrite (grub_usb_
       return NULL;
     }
 
+  max = grub_usb_bulk_maxpacket (dev, endpoint);
+
   datablocks = ((size + max - 1) / max);
   transfer->transcnt = datablocks;
   transfer->size = size - 1;
@@ -333,38 +341,36 @@ grub_usb_bulk_readwrite (grub_usb_device
   return err;
 }
 
-grub_usb_err_t
-grub_usb_bulk_write (grub_usb_device_t dev,
-		     int endpoint, grub_size_t size, char *data)
-{
-  grub_size_t actual;
-  grub_usb_err_t err;
-
-  err = grub_usb_bulk_readwrite (dev, endpoint, size, data,
-				 GRUB_USB_TRANSFER_TYPE_OUT, 1000, &actual);
-  if (!err && actual != size)
-    err = GRUB_USB_ERR_DATA;
-  return err;
-}
-
-grub_usb_err_t
-grub_usb_bulk_read (grub_usb_device_t dev,
-		    int endpoint, grub_size_t size, char *data)
+static grub_usb_err_t
+grub_usb_bulk_readwrite_packetize (grub_usb_device_t dev,
+				   int endpoint,
+				   grub_transfer_type_t type,
+				   grub_size_t size, char *data)
 {
   grub_size_t actual, transferred;
   grub_usb_err_t err;
   grub_size_t current_size, position;
+  grub_size_t max_bulk_transfer_len = MAX_USB_TRANSFER_LEN;
+  grub_size_t max;
+
+  if (dev->controller.dev->max_bulk_tds)
+    {
+      max = grub_usb_bulk_maxpacket (dev, endpoint);
+
+      /* Calculate max. possible length of bulk transfer */
+      max_bulk_transfer_len = dev->controller.dev->max_bulk_tds * max;
+    }
 
   for (position = 0, transferred = 0;
-       position < size; position += MAX_USB_TRANSFER_LEN)
+       position < size; position += max_bulk_transfer_len)
     {
       current_size = size - position;
-      if (current_size >= MAX_USB_TRANSFER_LEN)
-	current_size = MAX_USB_TRANSFER_LEN;
+      if (current_size >= max_bulk_transfer_len)
+	current_size = max_bulk_transfer_len;
       err = grub_usb_bulk_readwrite (dev, endpoint, current_size,
-              &data[position], GRUB_USB_TRANSFER_TYPE_IN, 1000, &actual);
+              &data[position], type, 1000, &actual);
       transferred += actual;
-      if (err || (current_size != actual) ) break;
+      if (err || (current_size != actual)) break;
     }
 
   if (!err && transferred != size)
@@ -373,6 +379,24 @@ grub_usb_bulk_read (grub_usb_device_t de
 }
 
 grub_usb_err_t
+grub_usb_bulk_write (grub_usb_device_t dev,
+		     int endpoint, grub_size_t size, char *data)
+{
+  return grub_usb_bulk_readwrite_packetize (dev, endpoint,
+					    GRUB_USB_TRANSFER_TYPE_OUT,
+					    size, data);
+}
+
+grub_usb_err_t
+grub_usb_bulk_read (grub_usb_device_t dev,
+		    int endpoint, grub_size_t size, char *data)
+{
+  return grub_usb_bulk_readwrite_packetize (dev, endpoint,
+					    GRUB_USB_TRANSFER_TYPE_IN,
+					    size, data);
+}
+
+grub_usb_err_t
 grub_usb_check_transfer (grub_usb_transfer_t transfer, grub_size_t *actual)
 {
   grub_usb_err_t err;
diff -purB ./grub/include/grub/usb.h ./grub_patched/include/grub/usb.h
--- ./grub/include/grub/usb.h	2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/include/grub/usb.h	2013-03-07 19:14:33.000000000 +0100
@@ -124,6 +124,13 @@ struct grub_usb_controller_dev
 
   /* Per controller flag - port reset pending, don't do another reset */
   grub_uint64_t pending_reset;
+
+  /* Max. number of transfer descriptors used per one bulk transfer */
+  /* The reason is to prevent "exhausting" of TD by large bulk */
+  /* transfer - number of TD is limited in USB host driver */
+  /* Value is calculated/estimated in driver - some TDs should be */
+  /* reserved for posible concurrent control or "interrupt" transfers */
+  grub_size_t max_bulk_tds;
   
   /* The next host controller.  */
   struct grub_usb_controller_dev *next;

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-03-07 19:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-01 22:46 [PATCH] Long USB transfers problem Aleš Nesrsta
2012-12-10 10:57 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-12-10 17:49   ` Aleš Nesrsta
2012-12-10 20:33     ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-12-11 20:40       ` Aleš Nesrsta
2013-01-20 21:46 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-02-11 23:13   ` Aleš Nesrsta
2013-02-24 18:37     ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-02-24 19:09     ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-02-24 19:25     ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-03-07 19:19       ` Aleš Nesrsta

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.