Proposed patch for usb-gadget

Proposed patch for usb-gadget

[Tasslehoff Kjappfot]

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

usb-gadget failed when MODULE_OPTIONS contained more than one option. Here's
the patch I used to make it work.

- Tasslehoff

[-- Attachment #2: 0001-fix-usage-of-MODULE_OPTIONS-in-modprobe.patch --]
[-- Type: text/x-patch, Size: 1488 bytes --]

From ae6d9906bd4bfb5c064cdc12c4147e95a0e51669 Mon Sep 17 00:00:00 2001
Date: Wed, 18 Nov 2009 10:28:28 +0100
Subject: [PATCH] fix usage of MODULE_OPTIONS in modprobe

---
 usb-gadget |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/usb-gadget b/usb-gadget
index 3f25edc..8da88ad 100755
--- a/usb-gadget
+++ b/usb-gadget
@@ -23,16 +23,16 @@ go() {
 	test -e "$CONF_FILE" && . "$CONF_FILE" 
 	
 	case "$USB_MODE" in
-	networking)	setup_usb g_ether "$MODULE_OPTIONS" ;;
-	zero)		setup_usb g_zero "$MODULE_OPTIONS" ;;
-	midi)		setup_usb g_midi "$MODULE_OPTIONS" ;;
-	printer)	setup_usb g_printer "$MODULE_OPTIONS" ;;
-	gadgetfs) 	setup_usb gadgetfs "$MODULE_OPTIONS" ;;
-	composite)	setup_usb g_cdc "$MODULE_OPTIONS" ;;
-	serial)		setup_usb g_serial "$MODULE_OPTIONS" ;;
-	storage)	setup_usb g_file_storage "$MODULE_OPTIONS" ;;
+	networking)	setup_usb g_ether ;;
+	zero)		setup_usb g_zero ;;
+	midi)		setup_usb g_midi ;;
+	printer)	setup_usb g_printer ;;
+	gadgetfs) 	setup_usb gadgetfs ;;
+	composite)	setup_usb g_cdc ;;
+	serial)		setup_usb g_serial ;;
+	storage)	setup_usb g_file_storage ;;
 	hostmode)	unload_usb_gadgets
-			setup_usb ohci_hcd "$MODULE_OPTIONS" ;;
+			setup_usb ohci_hcd ;;
 	none)		unload_usb_gadgets ;;
 	esac		
 		
@@ -44,7 +44,7 @@ setup_usb() {
 	then
 		unload_usb_gadgets
 		echo "Loading [$1]"
-		modprobe "$1" "$MODULE_OPTIONS"
+		modprobe "$1" $MODULE_OPTIONS
 	else
 		echo "Already loaded: [$1]"
 	fi
-- 
1.6.3.3

Re: Proposed patch for usb-gadget

[Phil Blundell]

On Wed, 2009-11-18 at 10:42 +0100, Tasslehoff Kjappfot wrote:
> usb-gadget failed when MODULE_OPTIONS contained more than one option. Here's
> the patch I used to make it work.

Thanks for the patch.  However, it seems to do two different things:

-               modprobe "$1" "$MODULE_OPTIONS"
+               modprobe "$1" $MODULE_OPTIONS

I understand this bit, and it does seem reasonable since MODULE_OPTIONS
might, as you say, contain multiple strings.

-       networking)     setup_usb g_ether "$MODULE_OPTIONS" ;;
-       zero)           setup_usb g_zero "$MODULE_OPTIONS" ;;
-       midi)           setup_usb g_midi "$MODULE_OPTIONS" ;;
-       printer)        setup_usb g_printer "$MODULE_OPTIONS" ;;
-       gadgetfs)       setup_usb gadgetfs "$MODULE_OPTIONS" ;;
-       composite)      setup_usb g_cdc "$MODULE_OPTIONS" ;;
-       serial)         setup_usb g_serial "$MODULE_OPTIONS" ;;
-       storage)        setup_usb g_file_storage "$MODULE_OPTIONS" ;;
+       networking)     setup_usb g_ether ;;
+       zero)           setup_usb g_zero ;;
+       midi)           setup_usb g_midi ;;
+       printer)        setup_usb g_printer ;;
+       gadgetfs)       setup_usb gadgetfs ;;
+       composite)      setup_usb g_cdc ;;
+       serial)         setup_usb g_serial ;;

But this part seems a bit strange.  Can you expand on why this is
necessary and/or desirable?

p.

Re: Proposed patch for usb-gadget

[Tasslehoff]

The only reason I did this was because setup_usb() doesn't use the 
argument at all, but uses $MODULE_OPTIONS directly.

Another fix is:

-               modprobe "$1" "$MODULE_OPTIONS"
+               modprobe $1 $2

and then remove quotes around all occurences of $MODULE_OPTIONS, but I 
like the first one better :)

- Tasslehoff


On 11/18/2009 10:48 AM, Phil Blundell wrote:
> On Wed, 2009-11-18 at 10:42 +0100, Tasslehoff Kjappfot wrote:
>    
>> usb-gadget failed when MODULE_OPTIONS contained more than one option. Here's
>> the patch I used to make it work.
>>      
> Thanks for the patch.  However, it seems to do two different things:
>
> -               modprobe "$1" "$MODULE_OPTIONS"
> +               modprobe "$1" $MODULE_OPTIONS
>
> I understand this bit, and it does seem reasonable since MODULE_OPTIONS
> might, as you say, contain multiple strings.
>
> -       networking)     setup_usb g_ether "$MODULE_OPTIONS" ;;
> -       zero)           setup_usb g_zero "$MODULE_OPTIONS" ;;
> -       midi)           setup_usb g_midi "$MODULE_OPTIONS" ;;
> -       printer)        setup_usb g_printer "$MODULE_OPTIONS" ;;
> -       gadgetfs)       setup_usb gadgetfs "$MODULE_OPTIONS" ;;
> -       composite)      setup_usb g_cdc "$MODULE_OPTIONS" ;;
> -       serial)         setup_usb g_serial "$MODULE_OPTIONS" ;;
> -       storage)        setup_usb g_file_storage "$MODULE_OPTIONS" ;;
> +       networking)     setup_usb g_ether ;;
> +       zero)           setup_usb g_zero ;;
> +       midi)           setup_usb g_midi ;;
> +       printer)        setup_usb g_printer ;;
> +       gadgetfs)       setup_usb gadgetfs ;;
> +       composite)      setup_usb g_cdc ;;
> +       serial)         setup_usb g_serial ;;
>
> But this part seems a bit strange.  Can you expand on why this is
> necessary and/or desirable?
>
> p.
>
>
>
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel
>    

Re: Proposed patch for usb-gadget

[Phil Blundell]

On Wed, 2009-11-18 at 10:56 +0100, Tasslehoff wrote:
> The only reason I did this was because setup_usb() doesn't use the 
> argument at all, but uses $MODULE_OPTIONS directly.

Okay, that makes sense.  I checked this patch in.

thanks

p.

Re: Proposed patch for usb-gadget

[Stefan Schmidt]

Hello.

On Wed, 2009-11-18 at 10:22, Phil Blundell wrote:
> On Wed, 2009-11-18 at 10:56 +0100, Tasslehoff wrote:
> > The only reason I did this was because setup_usb() doesn't use the 
> > argument at all, but uses $MODULE_OPTIONS directly.
> 
> Okay, that makes sense.  I checked this patch in.

I was just going through patchwork and this one was still left open. Cheking
HEAD it seems not to be applied. Phil, did go got disturbed and forgot about
this one?

regards
Stefan Schmidt