On 08/14/2014 06:41 AM, Lukasz Majewski wrote:
This commit adds new test for UMS USB gadget to u-boot mainline tree.
It is similar in operation to the one already available in test/dfu
directory.

This looks good, although I have a few small comments...

diff --git a/test/ums/README b/test/ums/README

+Example usage:
+1. On the target:
+   create UMS exportable partitions (with e.g. gpt write)

I'd like to avoid that requirement. Perhaps the script can operate on raw devices without a partition table too. Can we enhance the script so that if the user specifies - as the partition ID, then $PART_NUM gets set to '' and hence the whole device is used?

If you do that, I'd like to add the following to the line I quoted above:

, or specify a partition ID of - to use the entire device.

You might want to insert an extra step in the documentation here:

Create a filesystem on the partition or device, or pass the -f option to the test script.

diff --git a/test/ums/ums_gadget_test.sh b/test/ums/ums_gadget_test.sh

+idVendor=`find /sys -type f -name "idVendor" -exec grep -w $VID {} \;`
+idProduct=`find /sys -type f -name "idProduct" -exec grep -w $PID {} \;`
+if [ -z "$idVendor" ] || [ -z "$idProduct" ]; then
+    echo "Device $VID:$PID not connected!"
+    exit 0
+fi

That ensures that the given VID/PID exist somewhere, but not necessarily even on the same USB device.

+USB_DEV=`find /sys -type f -name "idProduct" -exec grep -l $PID {} \;`
> +USB_DEV=`dirname $USB_DEV`

... and that simply finds a USB device with a matching PID, but ignores the VID.

This might work better:

#!/bin/bash

VID=0955
PID=701a

for f in `find /sys -type f -name idProduct`; do
    d=`dirname ${f}`
    if [ `cat ${d}/idVendor` != "${VID}" ]; then
        continue
    fi
    if [ `cat ${d}/idProduct` != "${PID}" ]; then
        continue
    fi
    USB_DEV=${d}
    break
done

if [ -z "${USB_DEV}" ]; then
    echo "Connect target"
    echo "e.g. ums 0 mmc 0"
    exit 1
fi

echo ${USB_DEV}

> +MEM_DEV=`find $USB_DEV -name "sd[a-z]" | awk -F/ '{print $(NF)}' -`

"-type d" might be a good idea there.

That's probably OK, although it worries me slightly to rely on the naming convention of the device. If the code above doesn't work for some people, perhaps we could find a file named "dev", and extract the device major/minor from it. However, we'd need some complex logic to distinguish between the many files named "dev"; in ${USB_DEV}/ itself, the various partitions on the device, and the scsi_generic and "bsg" (whatever that is) devices. So, it's probably not worth changing that now.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to