[U-Boot] USB mass storage gadget patch

victor

author

2012-11-02 11:07 UTC #0 lore raw

Lukasz,

I am looking at the usb mass storage gadget patch from you at:
http://patchwork.ozlabs.org/patch/104516/

I am trying to use this patch in the u-boot. Some questions:
1) Is this patch in U-boot git release?
2) Now I can use the patch and process till SCSI MODE_SELECT_6 command, and then problem is encountered. Basically the SCSI MODE_SELECT_6 is sent to EP1 Out, then data is sent to EP1 out, and the fsg_main_thread only sees the data. I change the code to force the CSW.

The changes are:
1) fix the buf pointer in scsi inquiry
2) fix the buf pointer in scsi read format capacities
3) fix the buf pointer in CSW sending
4) fix attention condition checking in scsi command check
5) fix the context pointer before queue a request to read bulk only CBW
6) change bulk out endpoint from endpoint 2 to endpoint 1 to accommodate gen2 usb hardware limitation
7) in pad_with_zeros(), comment out the bh->inreq->length = nsend
8) in received_cbw(), force to send CSW if CBW is invalid. Otherwise, the IRQ routine keeps on reading the same data.

I attach the file for your reference. Please kindly advise me. Thanks.

Thanks,
victor

================================================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and may be protected by legal privilege. If you are not the intended addressee (or authorized to receive for the addressee). be aware that any disclosure, copying, distribution or use of this e-mail or any attachment is prohibited. If you have received this e-mail in error, please notify us immediately by returning it to the sender and delete this copy from your system. Thank you for your cooperation. KeyASIC Inc.
================================================================================================
-------------- next part --------------
A non-text attachment was scrubbed...
Name: file_storage.c
Type: application/octet-stream
Size: 106931 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121102/f6d8c100/attachment.obj>

Lukasz Majewski

2012-11-02 13:19 UTC #1 lore raw

Hi victor,

Quoted 3 lines

Lukasz,
I am looking at the usb mass storage gadget patch from you at:

USB mass storage gadget reference:
[*]
http://patchwork.ozlabs.org/patch/104516/

Quoted 3 lines

I am trying to use this patch in the u-boot. Some questions:
1) Is this patch in U-boot git release?

Unfortunately not. It has been posted, but so far wasn't accepted to mainline u-boot.

The reason is that in meantime I've added composite gadget support for the u-boot USB stack. DFU "function" uses this composite gadget framework.

UMS (ver. [*]) shall also be modified to work with the composite gadget framework. Posted patch [*] works as a standalone USB gadget.

Quoted 5 lines

2) Now I can use the patch and process till SCSI MODE_SELECT_6 command, and then problem is encountered. Basically the SCSI MODE_SELECT_6 is sent to EP1 Out, then data is sent to EP1 out, and the fsg_main_thread only sees the data. I change the code to force the CSW.

Can you explain further problems which you encounter?

I'm using this UMS version internally (the version which you pointed out with the link) for debugging our targets.

I'm using Debian Linux as a host with standard tools like dd and parted/fdisk. I don't have any problems with it (even with debug messages enabled).

[*] implementation of UMS has one (bit) flaw:
It doesn't provide "caching" for read/write operations. Therefore sending bigger files is very time consuming.

This and support for composite gadget infrastructure shall be developed for v3 of UMS patch set.

Quoted 13 lines

The changes are:
1) fix the buf pointer in scsi inquiry
2) fix the buf pointer in scsi read format capacities
3) fix the buf pointer in CSW sending
4) fix attention condition checking in scsi command check
5) fix the context pointer before queue a request to read bulk only CBW 6) change bulk out endpoint from endpoint 2 to endpoint 1 to accommodate gen2 usb hardware limitation
7) in pad_with_zeros(), comment out the bh->inreq->length = nsend
8) in received_cbw(), force to send CSW if CBW is invalid. Otherwise, the IRQ routine keeps on reading the same data.

Quoted 1 line

I attach the file for your reference. Please kindly advise me. Thanks.

Thanks for your fixes, but as I said previously, could you put some more shed on the problem?

Signature · 5 lines

Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group

victor

author

2012-11-05 02:30 UTC #2 lore raw

Lukasz,

Quoted 9 lines

Quoted 5 lines
2) Now I can use the patch and process till SCSI MODE_SELECT_6 command, and then problem is encountered. Basically the SCSI MODE_SELECT_6 is sent to EP1 Out, then data is sent to EP1 out, and the fsg_main_thread only sees the data. I change the code to force the CSW.
Can you explain further problems which you encounter?
I'm using this UMS version internally (the version w hich you

pointed out with the link) for debugging our targets.

Quoted 2 lines

I'm using Debian Linux as a host with standard tools like dd and

parted/fdisk. I don't have any problems with it (even with debug messages enabled).

Quoted 2 lines

In cmd_usb_mass_storage.c, do_usb_mass_storage(), the while(1) loop, the usb_gadget_handle_interrupts() will handle the interrupts that is triggered, and received data from EP1 out, and bh->state is set to BUF_STATE_FULL in bulk_out_complete(). And then fsg_main_thread() in file_storage.c is called. And then get_next_command() is called. And then start_transfer() is called. In that function, the bh->state will be set to BUF_STATE_BUSY. Because of this change, for two consecutive data from EP1 out, one data is missed.

I can send the full source code to you if it is helpful. Thanks.

Regards,
Victor

================================================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and may be protected by legal privilege. If you are not the intended addressee (or authorized to receive for the addressee). be aware that any disclosure, copying, distribution or use of this e-mail or any attachment is prohibited. If you have received this e-mail in error, please notify us immediately by returning it to the sender and delete this copy from your system. Thank you for your cooperation. KeyASIC Inc.
================================================================================================

victor

author

2012-11-05 06:45 UTC #3 lore raw

In reply to Lukasz Majewski

Lukasz,

Quoted 9 lines

Quoted 9 lines
Quoted 5 lines
2) Now I can use the patch and process till SCSI MODE_SELECT_6 command, and then problem is encountered. Basically the SCSI MODE_SELECT_6 is sent to EP1 Out, then data is sent to EP1 out, and the fsg_main_thread only sees the data. I change the code to force the CSW.
Can you explain further problems which you encounter?
I'm using this UMS version internally (the version which you pointed out

with the link) for debugging our targets.

Quoted 2 lines

Quoted 2 lines
I'm using Debian Linux as a host with standard tools like dd and

parted/fdisk. I don't have any problems with it (even with debug messages enabled).

Quoted 4 lines

Quoted 2 lines
In cmd_usb_mass_storage.c, do_usb_mass_storage(), the while(1) loop, the

usb_gadget_handle_interrupts() will handle the interrupts that is triggered, and received data

Quoted 1 line

from EP1 out, and bh-state >is set to BUF_STATE_FULL in

bulk_out_complete(). And then fsg_main_thread() in file_storage.c is called. And then get_next_command() is called.

Quoted 1 line

And then start_transfer() is called. In that function, the bh->state will

be set to BUF_STATE_BUSY. Because of this change, for two consecutive data from EP1 out, one data is missed.

Quoted 1 line

I can send the full source code to you if it is helpful. Thanks.

On further research into this, when receiving the SCSI_MODE_SELECT_6 command, in bulk_out_complete(), the bh pointer is 0x1b872e4

====== debug statement from console ====== 0x43425355 0x507bc10 0x18 0x15060000 0x18000010 0x0 0x0 bulk_out_complete 0x1b872e4

Then, in get_next_command(), before calling start_transfer(), the bh pointer becomes 0x1b87304.

This changes in bh pointer can cause data not to be processed by the usb mass storage gadget code.

Regards,
Victor

================================================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and may be protected by legal privilege. If you are not the intended addressee (or authorized to receive for the addressee). be aware that any disclosure, copying, distribution or use of this e-mail or any attachment is prohibited. If you have received this e-mail in error, please notify us immediately by returning it to the sender and delete this copy from your system. Thank you for your cooperation. KeyASIC Inc.
================================================================================================

victor

author

2012-11-06 06:46 UTC #4 lore raw

In reply to Lukasz Majewski

Hi,

Quoted 1 line

On further research into this, when receiving the SCSI_MODE_SELECT_6

command, in bulk_out_complete(), the bh pointer is 0x1b872e4

Quoted 3 lines

====== debug statement from console ======
0x43425355 0x507bc10 0x18 0x15060000 0x18000010 0x0 0x0 bulk_out_complete

0x1b872e4

Quoted 2 lines

Then, in get_next_command(), before calling start_transfer(), the bh

pointer becomes 0x1b87304.

Quoted 2 lines

This changes in bh pointer can cause data not to be processed by the usb

mass storage gadget code.

Is there any feedback on the buffer management problem in the U-boot usb mass storage gadget patch? Thanks.

Regards,
Victor

================================================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and may be protected by legal privilege. If you are not the intended addressee (or authorized to receive for the addressee). be aware that any disclosure, copying, distribution or use of this e-mail or any attachment is prohibited. If you have received this e-mail in error, please notify us immediately by returning it to the sender and delete this copy from your system. Thank you for your cooperation. KeyASIC Inc.
================================================================================================

Lukasz Majewski

2012-11-06 09:03 UTC #5 lore raw

Hi victor,

Regarding UMS support:

The last version posted at u-boot ML can be found at [**]:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/113004/match=ums

I've go through your fixes for file_storage.c.

Please find appended diff file [*] for changes which I was able to apply on the original file_storage.c file (taken from [**]).

I've attached output from Host dmesg and target's console (output.txt)

It seems that on my setup your patches are causing errors and UMS is not working (e.g. partitions aren't exported).

I don't have enough time now to delve to your solution, but I'm skeptical about hard setting EPs (like below).

+if (fsg_fs_bulk_out_desc.bEndpointAddress != 0x1)
+ fsg_fs_bulk_out_desc.bEndpointAddress = 0x1;
+

If possible, please align to patch set [**], so we can have the same setup.

However, I will try to find some time to help you with the problem (that's why I've asked for description of the problem, which you are trying to fix).

Signature · 16 lines

Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-UMS-CHanges-made-by-Victor-applied-to-UMS_v2-file-ap.patch
Type: text/x-patch
Size: 83556 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121106/6d847b5e/attachment.bin>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: output.txt
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121106/6d847b5e/attachment.txt>

victor

author

2012-11-06 09:58 UTC #6 lore raw

Lukasz,

Quoted 13 lines

Hi victor,
Regarding UMS support:
The last version posted at u-boot ML can be found at [**]:
http://thread.gmane.org/gmane.comp.boot-loaders.u-
boot/113004/match=ums
(quoted diff) +3 -0
+if (fsg_fs_bulk_out_desc.bEndpointAddress != 0x1)
+	fsg_fs_bulk_out_desc.bEndpointAddress = 0x1;
+

Quoted 1 line

If possible, please align to patch set [**], so we can have the same

setup.

Quoted 3 lines

However, I will try to find some time to help you with the problem (that's why I've asked for description of the problem, which you are trying to

fix).

Quoted 1 line

I hardcode bulk out endpoint to EP1 because the hardware only got one endpoint. So the code has to use EP1 for both bulk in and bulk out data transfer. Will this break the usb mass storage gadget code?

I will get the file_storage.c from [**].

The short description of the problem:
The U-boot usb mass storage gadget driver has problem with buffer management, sometimes the buffer returned by IRQ routine and the get_next_command function are different. It causes problem in processing SCSI_MODE_SELECT_6 command.

The detailed description of the problem:
When receiving the SCSI_MODE_SELECT_6 command, in bulk_out_complete(), the bh pointer is 0x1b872e4. Then, in get_next_command(), before calling start_transfer(), the bh pointer becomes 0x1b87304. This changes in bh pointer can cause data not to be processed by the usb mass storage gadget code.

Thanks,
Victor

================================================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and may be protected by legal privilege. If you are not the intended addressee (or authorized to receive for the addressee). be aware that any disclosure, copying, distribution or use of this e-mail or any attachment is prohibited. If you have received this e-mail in error, please notify us immediately by returning it to the sender and delete this copy from your system. Thank you for your cooperation. KeyASIC Inc.
================================================================================================

victor

author

2012-11-06 11:08 UTC #7 lore raw

In reply to Lukasz Majewski

Lukasz,

Quoted 11 lines

Lukasz,
Quoted 9 lines
Hi victor,
Regarding UMS support:
The last version posted at u-boot ML can be found at [**]:
http://thread.gmane.org/gmane.comp.boot-loaders.u-
boot/113004/match=ums

Quoted 5 lines

Quoted 5 lines
(quoted diff) +3 -0
+if (fsg_fs_bulk_out_desc.bEndpointAddress != 0x1)
+	fsg_fs_bulk_out_desc.bEndpointAddress = 0x1;
+
If possible, please align to patch set [**], so we can have the same

setup.

Quoted 3 lines

Quoted 3 lines
However, I will try to find some time to help you with the problem (that's why I've asked for description of the problem, which you are

trying

Quoted 22 lines

to fix).
Quoted 1 line
I hardcode bulk out endpoint to EP1 because the hardware only got one endpoint. So the code has to use EP1 for both bulk in and bulk out data transfer. Will this break the usb mass storage gadget code?
I will get the file_storage.c from [**].
The short description of the problem:
The U-boot usb mass storage gadget driver has problem with buffer management, sometimes the buffer returned by IRQ routine and the get_next_command function are different. It causes problem in processing SCSI_MODE_SELECT_6 command.
The detailed description of the problem:
When receiving the SCSI_MODE_SELECT_6 command, in bulk_out_complete(), the bh pointer is 0x1b872e4. Then, in get_next_command(), before calling start_transfer(), the bh pointer becomes 0x1b87304. This changes in bh pointer can cause data not to be processed by the usb mass storage gadget code.

Quoted 2 lines

Thanks,
Victor

I use the file_storage.c from [**]. Basically, I replace file_storage.c and hardcode bulk out endpoint to EP1. The IRQ routine can receive the SCSI_INQUIRY but the get_next_command() do not process it. The get_next_command is listening on a different bh than the one returned by IRQ routine.

Regards,
victor

================================================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and may be protected by legal privilege. If you are not the intended addressee (or authorized to receive for the addressee). be aware that any disclosure, copying, distribution or use of this e-mail or any attachment is prohibited. If you have received this e-mail in error, please notify us immediately by returning it to the sender and delete this copy from your system. Thank you for your cooperation. KeyASIC Inc.
================================================================================================

victor

author

2012-11-22 02:24 UTC #8 lore raw

In reply to Lukasz Majewski

Lukasz,

Quoted 20 lines

-----Original Message-----
From: victor [mailto:victor at keyasic.com]
Sent: Tuesday, 6 November, 2012 7:09 PM
To: 'Lukasz Majewski'
Cc: 'u-boot at lists.denx.de'; 'Marek Vasut'; 'Kyungmin Park'
Subject: RE: [U-Boot] USB mass storage gadget patch
Lukasz,
Quoted 11 lines
Lukasz,
Quoted 9 lines
Hi victor,
Regarding UMS support:
The last version posted at u-boot ML can be found at [**]:
http://thread.gmane.org/gmane.comp.boot-loaders.u-
boot/113004/match=ums

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
(quoted diff) +3 -0
+if (fsg_fs_bulk_out_desc.bEndpointAddress != 0x1)
+	fsg_fs_bulk_out_desc.bEndpointAddress = 0x1;
+
If possible, please align to patch set [**], so we can have the same

setup.

Quoted 28 lines

Quoted 20 lines
Quoted 4 lines
However, I will try to find some time to help you with the problem (that's why I've asked for description of the problem, which you are trying
to fix).
Quoted 1 line
I hardcode bulk out endpoint to EP1 because the hardware only got one endpoint. So the code has to use EP1 for both bulk in and bulk out data transfer. Will this break the usb mass storage gadget code?
I will get the file_storage.c from [**].
The short description of the problem:
The U-boot usb mass storage gadget driver has problem with buffer management, sometimes the buffer returned by IRQ routine and the get_next_command function are different. It causes problem in processing
SCSI_MODE_SELECT_6 command.
Quoted 2 lines
The detailed description of the problem:
When receiving the SCSI_MODE_SELECT_6 command, in
bulk_out_complete(),
Quoted 5 lines
the bh pointer is 0x1b872e4. Then, in get_next_command(), before calling start_transfer(), the bh pointer becomes 0x1b87304. This changes in bh pointer can cause data not to be processed by the usb mass storage gadget code.

Quoted 4 lines

Quoted 2 lines
Thanks,
Victor
I use the file_storage.c from [**]. Basically, I replace file_storage.c

and

Quoted 7 lines

hardcode bulk out endpoint to EP1. The IRQ routine can receive the SCSI_INQUIRY but the get_next_command() do not process it. The get_next_command is listening on a different bh than the one returned by IRQ routine.
Regards,
victor

Any comments or opinion on the above buffer mismatch issue in usb mass storage gadget?

Regards,
Victor

================================================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and may be protected by legal privilege. If you are not the intended addressee (or authorized to receive for the addressee). be aware that any disclosure, copying, distribution or use of this e-mail or any attachment is prohibited. If you have received this e-mail in error, please notify us immediately by returning it to the sender and delete this copy from your system. Thank you for your cooperation. KeyASIC Inc.
================================================================================================