Patchworkβ [oe] Proposed patch for usb-gadget

login
register
about
Submitter Tasslehoff Kjappfot
Date 2009-11-18 09:42:24
Message ID <ec7df6a60911180142j65980c0bnaf494f4c76fbe980@mail.gmail.com>
Download mbox | patch
Permalink /patch/1284/
State Applied, archived
Headers show

Comments

Tasslehoff Kjappfot - 2009-11-18 09:42:24
usb-gadget failed when MODULE_OPTIONS contained more than one option. Here's
the patch I used to make it work.

- Tasslehoff
Phil Blundell - 2009-11-18 09:48:57
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.
Tasslehoff Kjappfot - 2009-11-18 09:56:43
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
>
Phil Blundell - 2009-11-18 10:22:53
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.
Stefan Schmidt - 2010-02-16 08:06:26
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

Patch

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