[PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

This patch adds removable mass storage support to g_nokia gadget (for N900).
It means that at runtime block device can be exported or unexported.
So it does not export anything by default and thus allows to use MyDocs
partition as before...

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/usb/gadget/legacy/Kconfig |    1 +
 drivers/usb/gadget/legacy/nokia.c |  102 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 102 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig
index fd48ef3..36f6ba4 100644
--- a/drivers/usb/gadget/legacy/Kconfig
+++ b/drivers/usb/gadget/legacy/Kconfig
@@ -345,6 +345,7 @@ config USB_G_NOKIA
 	select USB_F_OBEX
 	select USB_F_PHONET
 	select USB_F_ECM
+	select USB_F_MASS_STORAGE
 	help
 	  The Nokia composite gadget provides support for acm, obex
 	  and phonet in only one composite gadget driver.
diff --git a/drivers/usb/gadget/legacy/nokia.c b/drivers/usb/gadget/legacy/nokia.c
index 9b8fd70..a09bb50 100644
--- a/drivers/usb/gadget/legacy/nokia.c
+++ b/drivers/usb/gadget/legacy/nokia.c
@@ -24,6 +24,7 @@
 #include "u_phonet.h"
 #include "u_ecm.h"
 #include "gadget_chips.h"
+#include "f_mass_storage.h"
 
 /* Defines */
 
@@ -34,6 +35,29 @@ USB_GADGET_COMPOSITE_OPTIONS();
 
 USB_ETHERNET_MODULE_PARAMETERS();
 
+static struct fsg_module_parameters fsg_mod_data = {
+	.stall = 0,
+	.luns = 2,
+	.removable_count = 2,
+	.removable = { 1, 1, },
+};
+
+FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
+
+#ifdef CONFIG_USB_GADGET_DEBUG_FILES
+
+static unsigned int fsg_num_buffers = CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS;
+
+#else
+
+/*
+ * Number of buffers we will use.
+ * 2 is usually enough for good buffering pipeline
+ */
+#define fsg_num_buffers	CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS
+
+#endif /* CONFIG_USB_DEBUG */
+
 #define NOKIA_VENDOR_ID			0x0421	/* Nokia */
 #define NOKIA_PRODUCT_ID		0x01c8	/* Nokia Gadget */
 
@@ -94,6 +118,8 @@ static struct usb_function *f_obex1_cfg2;
 static struct usb_function *f_obex2_cfg2;
 static struct usb_function *f_phonet_cfg1;
 static struct usb_function *f_phonet_cfg2;
+static struct usb_function *f_msg_cfg1;
+static struct usb_function *f_msg_cfg2;
 
 
 static struct usb_configuration nokia_config_500ma_driver = {
@@ -117,6 +143,7 @@ static struct usb_function_instance *fi_ecm;
 static struct usb_function_instance *fi_obex1;
 static struct usb_function_instance *fi_obex2;
 static struct usb_function_instance *fi_phonet;
+static struct usb_function_instance *fi_msg;
 
 static int __init nokia_bind_config(struct usb_configuration *c)
 {
@@ -125,6 +152,8 @@ static int __init nokia_bind_config(struct usb_configuration *c)
 	struct usb_function *f_obex1 = NULL;
 	struct usb_function *f_ecm;
 	struct usb_function *f_obex2 = NULL;
+	struct usb_function *f_msg;
+	struct fsg_opts *fsg_opts;
 	int status = 0;
 	int obex1_stat = -1;
 	int obex2_stat = -1;
@@ -160,6 +189,12 @@ static int __init nokia_bind_config(struct usb_configuration *c)
 		goto err_get_ecm;
 	}
 
+	f_msg = usb_get_function(fi_msg);
+	if (IS_ERR(f_msg)) {
+		status = PTR_ERR(f_msg);
+		goto err_get_msg;
+	}
+
 	if (!IS_ERR_OR_NULL(f_phonet)) {
 		phonet_stat = usb_add_function(c, f_phonet);
 		if (phonet_stat)
@@ -187,21 +222,36 @@ static int __init nokia_bind_config(struct usb_configuration *c)
 		pr_debug("could not bind ecm config %d\n", status);
 		goto err_ecm;
 	}
+
+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
+
+	status = fsg_common_run_thread(fsg_opts->common);
+	if (status)
+		goto err_msg;
+
+	status = usb_add_function(c, f_msg);
+	if (status)
+		goto err_msg;
+
 	if (c == &nokia_config_500ma_driver) {
 		f_acm_cfg1 = f_acm;
 		f_ecm_cfg1 = f_ecm;
 		f_phonet_cfg1 = f_phonet;
 		f_obex1_cfg1 = f_obex1;
 		f_obex2_cfg1 = f_obex2;
+		f_msg_cfg1 = f_msg;
 	} else {
 		f_acm_cfg2 = f_acm;
 		f_ecm_cfg2 = f_ecm;
 		f_phonet_cfg2 = f_phonet;
 		f_obex1_cfg2 = f_obex1;
 		f_obex2_cfg2 = f_obex2;
+		f_msg_cfg2 = f_msg;
 	}
 
 	return status;
+err_msg:
+	usb_remove_function(c, f_ecm);
 err_ecm:
 	usb_remove_function(c, f_acm);
 err_conf:
@@ -211,6 +261,8 @@ err_conf:
 		usb_remove_function(c, f_obex1);
 	if (!phonet_stat)
 		usb_remove_function(c, f_phonet);
+	usb_put_function(f_msg);
+err_get_msg:
 	usb_put_function(f_ecm);
 err_get_ecm:
 	usb_put_function(f_acm);
@@ -227,6 +279,8 @@ err_get_acm:
 static int __init nokia_bind(struct usb_composite_dev *cdev)
 {
 	struct usb_gadget	*gadget = cdev->gadget;
+	struct fsg_opts		*fsg_opts;
+	struct fsg_config	fsg_config;
 	int			status;
 
 	status = usb_string_ids_tab(cdev, strings_dev);
@@ -267,11 +321,46 @@ static int __init nokia_bind(struct usb_composite_dev *cdev)
 		goto err_acm_inst;
 	}
 
+	fi_msg = usb_get_function_instance("mass_storage");
+	if (IS_ERR(fi_msg)) {
+		status = PTR_ERR(fi_msg);
+		goto err_ecm_inst;
+	}
+
+	/* set up mass storage function */
+	fsg_config_from_params(&fsg_config, &fsg_mod_data, fsg_num_buffers);
+	fsg_config.vendor_name = "Nokia";
+	fsg_config.product_name = "N900";
+
+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
+	fsg_opts->no_configfs = true;
+
+	status = fsg_common_set_num_buffers(fsg_opts->common, fsg_num_buffers);
+	if (status)
+		goto err_msg_inst;
+
+	status = fsg_common_set_nluns(fsg_opts->common, fsg_config.nluns);
+	if (status)
+		goto err_msg_buf;
+
+	status = fsg_common_set_cdev(fsg_opts->common, cdev, fsg_config.can_stall);
+	if (status)
+		goto err_msg_set_nluns;
+
+	fsg_common_set_sysfs(fsg_opts->common, true);
+
+	status = fsg_common_create_luns(fsg_opts->common, &fsg_config);
+	if (status)
+		goto err_msg_set_nluns;
+
+	fsg_common_set_inquiry_string(fsg_opts->common, fsg_config.vendor_name,
+				      fsg_config.product_name);
+
 	/* finally register the configuration */
 	status = usb_add_config(cdev, &nokia_config_500ma_driver,
 			nokia_bind_config);
 	if (status < 0)
-		goto err_ecm_inst;
+		goto err_msg_set_cdev;
 
 	status = usb_add_config(cdev, &nokia_config_100ma_driver,
 			nokia_bind_config);
@@ -292,6 +381,14 @@ err_put_cfg1:
 	if (!IS_ERR_OR_NULL(f_phonet_cfg1))
 		usb_put_function(f_phonet_cfg1);
 	usb_put_function(f_ecm_cfg1);
+err_msg_set_cdev:
+	fsg_common_remove_luns(fsg_opts->common);
+err_msg_set_nluns:
+	fsg_common_free_luns(fsg_opts->common);
+err_msg_buf:
+	fsg_common_free_buffers(fsg_opts->common);
+err_msg_inst:
+	usb_put_function_instance(fi_msg);
 err_ecm_inst:
 	usb_put_function_instance(fi_ecm);
 err_acm_inst:
@@ -325,7 +422,10 @@ static int __exit nokia_unbind(struct usb_composite_dev *cdev)
 	usb_put_function(f_acm_cfg2);
 	usb_put_function(f_ecm_cfg1);
 	usb_put_function(f_ecm_cfg2);
+	usb_put_function(f_msg_cfg1);
+	usb_put_function(f_msg_cfg2);
 
+	usb_put_function_instance(fi_msg);
 	usb_put_function_instance(fi_ecm);
 	if (!IS_ERR(fi_obex2))
 		usb_put_function_instance(fi_obex2);
-- 
1.7.9.5

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Andrzej Pietrasiewicz]

Hello Pali,

W dniu 31.01.2015 o 10:53, Pali Rohár pisze:
> This patch adds removable mass storage support to g_nokia gadget (for N900).
> It means that at runtime block device can be exported or unexported.

For a hint please see this thread:
http://www.spinics.net/lists/linux-usb/msg119669.html

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Felipe Balbi]

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

Hi,

On Sat, Jan 31, 2015 at 10:53:30AM +0100, Pali Rohár wrote:
> This patch adds removable mass storage support to g_nokia gadget (for N900).
> It means that at runtime block device can be exported or unexported.
> So it does not export anything by default and thus allows to use MyDocs
> partition as before...
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

thanks, but no thanks. Build your own using configfs.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 648 bytes --]

On Monday 02 February 2015 19:54:58 Felipe Balbi wrote:
> Hi,
> 
> On Sat, Jan 31, 2015 at 10:53:30AM +0100, Pali Rohár wrote:
> > This patch adds removable mass storage support to g_nokia
> > gadget (for N900). It means that at runtime block device
> > can be exported or unexported. So it does not export
> > anything by default and thus allows to use MyDocs partition
> > as before...
> > 
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> 
> thanks, but no thanks. Build your own using configfs.

But it needs some userspace interaction right?
Then its not possible for nfsboot.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Felipe Balbi]

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

Hi,

On Mon, Feb 02, 2015 at 07:58:59PM +0100, Pali Rohár wrote:
> On Monday 02 February 2015 19:54:58 Felipe Balbi wrote:
> > Hi,
> > 
> > On Sat, Jan 31, 2015 at 10:53:30AM +0100, Pali Rohár wrote:
> > > This patch adds removable mass storage support to g_nokia
> > > gadget (for N900). It means that at runtime block device
> > > can be exported or unexported. So it does not export
> > > anything by default and thus allows to use MyDocs partition
> > > as before...
> > > 
> > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > 
> > thanks, but no thanks. Build your own using configfs.
> 
> But it needs some userspace interaction right?
> Then its not possible for nfsboot.

oh, right... you're using nfsboot through g_nokia. Hmm, sounds like you
need initramfs.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1086 bytes --]

On Monday 02 February 2015 20:01:11 Felipe Balbi wrote:
> Hi,
> 
> On Mon, Feb 02, 2015 at 07:58:59PM +0100, Pali Rohár wrote:
> > On Monday 02 February 2015 19:54:58 Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Sat, Jan 31, 2015 at 10:53:30AM +0100, Pali Rohár wrote:
> > > > This patch adds removable mass storage support to
> > > > g_nokia gadget (for N900). It means that at runtime
> > > > block device can be exported or unexported. So it does
> > > > not export anything by default and thus allows to use
> > > > MyDocs partition as before...
> > > > 
> > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > 
> > > thanks, but no thanks. Build your own using configfs.
> > 
> > But it needs some userspace interaction right?
> > Then its not possible for nfsboot.
> 
> oh, right... you're using nfsboot through g_nokia. Hmm, sounds
> like you need initramfs.

Also compiling usb gadgets as external .ko modules is broken.
So I cannot use configfs, when I compile g_nokia even if I use 
initramfs...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Felipe Balbi]

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

On Mon, Feb 02, 2015 at 08:07:51PM +0100, Pali Rohár wrote:
> On Monday 02 February 2015 20:01:11 Felipe Balbi wrote:
> > Hi,
> > 
> > On Mon, Feb 02, 2015 at 07:58:59PM +0100, Pali Rohár wrote:
> > > On Monday 02 February 2015 19:54:58 Felipe Balbi wrote:
> > > > Hi,
> > > > 
> > > > On Sat, Jan 31, 2015 at 10:53:30AM +0100, Pali Rohár wrote:
> > > > > This patch adds removable mass storage support to
> > > > > g_nokia gadget (for N900). It means that at runtime
> > > > > block device can be exported or unexported. So it does
> > > > > not export anything by default and thus allows to use
> > > > > MyDocs partition as before...
> > > > > 
> > > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > > 
> > > > thanks, but no thanks. Build your own using configfs.
> > > 
> > > But it needs some userspace interaction right?
> > > Then its not possible for nfsboot.
> > 
> > oh, right... you're using nfsboot through g_nokia. Hmm, sounds
> > like you need initramfs.
> 
> Also compiling usb gadgets as external .ko modules is broken.
> So I cannot use configfs, when I compile g_nokia even if I use 
> initramfs...

yeah, there are people working on that and some patches already flying
around for it. Meanwhile, you can make it built-in and use initramfs to
add mass_storage through configfs to g_nokia, no issues.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 2504 bytes --]

On Monday 02 February 2015 20:14:35 Felipe Balbi wrote:
> On Mon, Feb 02, 2015 at 08:07:51PM +0100, Pali Rohár wrote:
> > On Monday 02 February 2015 20:01:11 Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Mon, Feb 02, 2015 at 07:58:59PM +0100, Pali Rohár wrote:
> > > > On Monday 02 February 2015 19:54:58 Felipe Balbi wrote:
> > > > > Hi,
> > > > > 
> > > > > On Sat, Jan 31, 2015 at 10:53:30AM +0100, Pali Rohár 
wrote:
> > > > > > This patch adds removable mass storage support to
> > > > > > g_nokia gadget (for N900). It means that at runtime
> > > > > > block device can be exported or unexported. So it
> > > > > > does not export anything by default and thus allows
> > > > > > to use MyDocs partition as before...
> > > > > > 
> > > > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > > > 
> > > > > thanks, but no thanks. Build your own using configfs.
> > > > 
> > > > But it needs some userspace interaction right?
> > > > Then its not possible for nfsboot.
> > > 
> > > oh, right... you're using nfsboot through g_nokia. Hmm,
> > > sounds like you need initramfs.
> > 
> > Also compiling usb gadgets as external .ko modules is
> > broken. So I cannot use configfs, when I compile g_nokia
> > even if I use initramfs...
> 
> yeah, there are people working on that and some patches
> already flying around for it. Meanwhile, you can make it
> built-in and use initramfs to add mass_storage through
> configfs to g_nokia, no issues.

Sorry but nfsboot is used without initramfs. I do not need to use 
any initramfs and I do not see reason for it because of kernel 
usb bugs...

My patch for mass storage mode in g_nokia.ko driver (which is 
used for Nokia N900 only) fix usage of both g_nokia functions and 
mass storage functions.

Because in current state on N900 this is easy and *working* 
solution (g_nokia static linked into zImage). So I do not see 
reason why not to include my patch into upstream. Driver g_nokia 
is N900 specific and all developers can benefit this patch, 
because they would be able to use *both* usb networking and mass 
storage mode *without* need to recompile kernel and restart n900 
device!

And please do not tell me that either usb network or mass storage 
support is deprecated or so and nobody should not use it! This is 
only way how to develop & debug n900 device without any other 
Nokia (TM) equipment which is not available for non Nokias...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Ivaylo Dimitrov]

On  2.02.2015 21:14, Felipe Balbi wrote:
> On Mon, Feb 02, 2015 at 08:07:51PM +0100, Pali Rohár wrote:
>> On Monday 02 February 2015 20:01:11 Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Mon, Feb 02, 2015 at 07:58:59PM +0100, Pali Rohár wrote:
>>>> On Monday 02 February 2015 19:54:58 Felipe Balbi wrote:
>>>>> Hi,
>>>>>
>>>>> On Sat, Jan 31, 2015 at 10:53:30AM +0100, Pali Rohár wrote:
>>>>>> This patch adds removable mass storage support to
>>>>>> g_nokia gadget (for N900). It means that at runtime
>>>>>> block device can be exported or unexported. So it does
>>>>>> not export anything by default and thus allows to use
>>>>>> MyDocs partition as before...
>>>>>>
>>>>>> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
>>>>>
>>>>> thanks, but no thanks. Build your own using configfs.
>>>>
>>>> But it needs some userspace interaction right?
>>>> Then its not possible for nfsboot.
>>>
>>> oh, right... you're using nfsboot through g_nokia. Hmm, sounds
>>> like you need initramfs.
>>
>> Also compiling usb gadgets as external .ko modules is broken.
>> So I cannot use configfs, when I compile g_nokia even if I use
>> initramfs...
>
> yeah, there are people working on that and some patches already flying
> around for it. Meanwhile, you can make it built-in and use initramfs to
> add mass_storage through configfs to g_nokia, no issues.
>

Bellow is the bactrace of the crash when g_nokia is modprobed. Any hint 
on where to look for is appreciated.

<0>[   33.751983] Kernel panic - not syncing: Fatal exception
ÿÿT!19\00]\00]<6>[   59.570159] bq2415x-charger 2-006b: automode enabled
<1>[   59.597534] [bf286014] *pgd=8eda8811, *pte=00000000, *ppte=00000000
<6>[   59.609405] bq2415x-charger 2-006b: driver registered
<0>[   59.640472] Internal error: Oops: 80000007 [#1] PREEMPT ARM
<4>[   59.650421] Modules linked in: ipv6(+) bq2415x_charger(+) 
g_mass_storage usb_f_mass_storage libcomposite configfs uinput adp1653 
ad5820 et8ek8 smiaregs hsi_char radio_platform_si4713 joydev 
omap_ssi_port wl1251_spi wl1251 rx51_battery mac80211 isp1704_charger 
smc91x mii cfg80211 si4713 v4l2_common crc7 videodev tsc2005 media 
tsl2563 twl4030_vibra ff_memless lis3lv02d_i2c lis3lv02d omap_ssi 
input_polldev hsi twl4030_wdt omap_wdt rtc_twl leds_lp5523 
leds_lp55xx_common
<4>[   59.715332] CPU: 0 PID: 13 Comm: kworker/0:1 Not tainted 
3.19.0-rc5+ #2
<4>[   59.726501] Hardware name: Nokia RX-51 board
<4>[   59.735168] Workqueue: events usb_gadget_work
<4>[   59.743896] task: cf8ab900 ti: cf992000 task.ti: cf992000
<4>[   59.753692] PC is at 0xbf286014
<4>[   59.761108] LR is at try_get_usb_function_instance+0x90/0x9c 
[libcomposite]
<4>[   59.772613] pc : [<bf286014>]    lr : [<bf2640c8>]    psr: 80000113
<4>[   59.772613] sp : cf993e50  ip : 0000001c  fp : 00000000
<4>[   59.793060] r10: 00000000  r9 : 00000000  r8 : cfcad600
<4>[   59.802581] r7 : cedaab80  r6 : 00000000  r5 : bf28444c  r4 : cedaab80
<4>[   59.813476] r3 : cedd8f38  r2 : 00000001  r1 : 00000000  r0 : cedd8f04
<4>[   59.824310] Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM 
Segment kernel
<4>[   59.836090] Control: 10c5387d  Table: 8e970019  DAC: 00000015
<0>[   59.846252] Process kworker/0:1 (pid: 13, stack limit = 0xcf992238)
<0>[   59.857086] Stack: (0xcf993e50 to 0xcf994000)
<0>[   59.865814] 3e40:                                     c03fb2d8 
bf266e68 cf1fdf00 c015d0dc
<0>[   59.878723] 3e60: 92000000 00001000 00000000 c015d8d8 00001000 
00000000 c03fb2d8 bf266e68
<0>[   59.891693] 3e80: 00000000 00000001 00000000 c00f58f8 cf801c80 
cedaab80 cf03b0b8 cedaac00
<0>[   59.904663] 3ea0: bf284424 c015d980 00000000 bf266e68 00000000 
cedaab80 bf28444c 00000000
<0>[   59.917572] 3ec0: cf03b0b8 cfcad600 00000000 bf263dd8 cf03b0b8 
bf28444c cf03b0b8 cf1fea00
<0>[   59.930450] 3ee0: c0620cd8 bf28444c c0609868 cfcad600 00000000 
00000000 00000000 c02c5478
<0>[   59.943420] 3f00: bf284470 c0620cd8 c0606ff8 c02c5f54 cf98d080 
bf284470 c0606ff8 c0047288
<0>[   59.956390] 3f20: cf98d080 bf284470 00000001 cf98d080 c0606ff8 
c0607008 c0609868 cf98d098
<0>[   59.969360] 3f40: 00000008 00000000 00000000 c00477ec cf98a580 
00000000 cf98d080 c00474c4
<0>[   59.982330] 3f60: 00000000 00000000 00000000 c004b148 22000000 
00000000 11880000 cf98d080
<0>[   59.995208] 3f80: 00000000 cf993f84 cf993f84 00000000 cf993f90 
cf993f90 cf993fac cf98a580
<0>[   60.008026] 3fa0: c004b07c 00000000 00000000 c000e058 00000000 
00000000 00000000 00000000
<0>[   60.020782] 3fc0: 00000000 00000000 00000000 00000000 00000000 
00000000 00000000 00000000
<0>[   60.033355] 3fe0: 00000000 00000000 00000000 00000000 00000013 
00000000 40000500 04000401
<4>[   60.045867] [<bf2640c8>] (try_get_usb_function_instance 
[libcomposite]) from [<c015d0dc>] (__kernfs_create_file+0x7c/0xa0)
<4>[   60.065155] [<c015d0dc>] (__kernfs_create_file) from [<c015d8d8>] 
(sysfs_add_file_mode_ns+0x134/0x1ac)
<4>[   60.082977] [<c015d8d8>] (sysfs_add_file_mode_ns) from 
[<c015d980>] (sysfs_create_file_ns+0x30/0x44)
<4>[   60.100952] [<c015d980>] (sysfs_create_file_ns) from [<cedaab80>] 
(0xcedaab80)
<0>[   60.112884] Code: bad PC value
<6>[   60.210144] NET: Registered protocol family 10
<6>[   60.671600] bq27x00-battery 2-0055: support ver. 1.2.0 enabled
<6>[   60.832885] bq27x00-battery 2-0055: battery is not calibrated! 
ignoring capacity values
<6>[   63.788269] Adding 786428k swap on /dev/mmcblk0p3.  Priority:-1 
extents:1 across:786428k SS
<5>[   64.057617] random: nonblocking pool is initialized
<6>[   64.554016] NET: Registered protocol family 35
<4>[   65.667510] ---[ end trace 85b8d26ec312a8c1 ]---


Regards,
Ivo

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Ivaylo Dimitrov]

Hi,

On  7.02.2015 20:01, Ivaylo Dimitrov wrote:
>
>
> On  2.02.2015 21:14, Felipe Balbi wrote:
>> On Mon, Feb 02, 2015 at 08:07:51PM +0100, Pali Rohár wrote:
>>> On Monday 02 February 2015 20:01:11 Felipe Balbi wrote:
>>>> Hi,
>>>>
>>>> On Mon, Feb 02, 2015 at 07:58:59PM +0100, Pali Rohár wrote:
>>>>> On Monday 02 February 2015 19:54:58 Felipe Balbi wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Sat, Jan 31, 2015 at 10:53:30AM +0100, Pali Rohár wrote:
>>>>>>> This patch adds removable mass storage support to
>>>>>>> g_nokia gadget (for N900). It means that at runtime
>>>>>>> block device can be exported or unexported. So it does
>>>>>>> not export anything by default and thus allows to use
>>>>>>> MyDocs partition as before...
>>>>>>>
>>>>>>> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
>>>>>>
>>>>>> thanks, but no thanks. Build your own using configfs.
>>>>>
>>>>> But it needs some userspace interaction right?
>>>>> Then its not possible for nfsboot.
>>>>
>>>> oh, right... you're using nfsboot through g_nokia. Hmm, sounds
>>>> like you need initramfs.
>>>
>>> Also compiling usb gadgets as external .ko modules is broken.
>>> So I cannot use configfs, when I compile g_nokia even if I use
>>> initramfs...
>>
>> yeah, there are people working on that and some patches already flying
>> around for it. Meanwhile, you can make it built-in and use initramfs to
>> add mass_storage through configfs to g_nokia, no issues.
>>
>
> Bellow is the bactrace of the crash when g_nokia is modprobed. Any hint
> on where to look for is appreciated.
>
> <0>[   33.751983] Kernel panic - not syncing: Fatal exception
> ÿÿT!19\00]\00]<6>[   59.570159] bq2415x-charger 2-006b: automode enabled
> <1>[   59.597534] [bf286014] *pgd=8eda8811, *pte=00000000, *ppte=00000000
> <6>[   59.609405] bq2415x-charger 2-006b: driver registered
> <0>[   59.640472] Internal error: Oops: 80000007 [#1] PREEMPT ARM
> <4>[   59.650421] Modules linked in: ipv6(+) bq2415x_charger(+)
> g_mass_storage usb_f_mass_storage libcomposite configfs uinput adp1653
> ad5820 et8ek8 smiaregs hsi_char radio_platform_si4713 joydev
> omap_ssi_port wl1251_spi wl1251 rx51_battery mac80211 isp1704_charger
> smc91x mii cfg80211 si4713 v4l2_common crc7 videodev tsc2005 media
> tsl2563 twl4030_vibra ff_memless lis3lv02d_i2c lis3lv02d omap_ssi
> input_polldev hsi twl4030_wdt omap_wdt rtc_twl leds_lp5523
> leds_lp55xx_common
> <4>[   59.715332] CPU: 0 PID: 13 Comm: kworker/0:1 Not tainted
> 3.19.0-rc5+ #2
> <4>[   59.726501] Hardware name: Nokia RX-51 board
> <4>[   59.735168] Workqueue: events usb_gadget_work
> <4>[   59.743896] task: cf8ab900 ti: cf992000 task.ti: cf992000
> <4>[   59.753692] PC is at 0xbf286014
> <4>[   59.761108] LR is at try_get_usb_function_instance+0x90/0x9c
> [libcomposite]
> <4>[   59.772613] pc : [<bf286014>]    lr : [<bf2640c8>]    psr: 80000113
> <4>[   59.772613] sp : cf993e50  ip : 0000001c  fp : 00000000
> <4>[   59.793060] r10: 00000000  r9 : 00000000  r8 : cfcad600
> <4>[   59.802581] r7 : cedaab80  r6 : 00000000  r5 : bf28444c  r4 :
> cedaab80
> <4>[   59.813476] r3 : cedd8f38  r2 : 00000001  r1 : 00000000  r0 :
> cedd8f04
> <4>[   59.824310] Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM
> Segment kernel
> <4>[   59.836090] Control: 10c5387d  Table: 8e970019  DAC: 00000015
> <0>[   59.846252] Process kworker/0:1 (pid: 13, stack limit = 0xcf992238)
> <0>[   59.857086] Stack: (0xcf993e50 to 0xcf994000)
> <0>[   59.865814] 3e40:                                     c03fb2d8
> bf266e68 cf1fdf00 c015d0dc
> <0>[   59.878723] 3e60: 92000000 00001000 00000000 c015d8d8 00001000
> 00000000 c03fb2d8 bf266e68
> <0>[   59.891693] 3e80: 00000000 00000001 00000000 c00f58f8 cf801c80
> cedaab80 cf03b0b8 cedaac00
> <0>[   59.904663] 3ea0: bf284424 c015d980 00000000 bf266e68 00000000
> cedaab80 bf28444c 00000000
> <0>[   59.917572] 3ec0: cf03b0b8 cfcad600 00000000 bf263dd8 cf03b0b8
> bf28444c cf03b0b8 cf1fea00
> <0>[   59.930450] 3ee0: c0620cd8 bf28444c c0609868 cfcad600 00000000
> 00000000 00000000 c02c5478
> <0>[   59.943420] 3f00: bf284470 c0620cd8 c0606ff8 c02c5f54 cf98d080
> bf284470 c0606ff8 c0047288
> <0>[   59.956390] 3f20: cf98d080 bf284470 00000001 cf98d080 c0606ff8
> c0607008 c0609868 cf98d098
> <0>[   59.969360] 3f40: 00000008 00000000 00000000 c00477ec cf98a580
> 00000000 cf98d080 c00474c4
> <0>[   59.982330] 3f60: 00000000 00000000 00000000 c004b148 22000000
> 00000000 11880000 cf98d080
> <0>[   59.995208] 3f80: 00000000 cf993f84 cf993f84 00000000 cf993f90
> cf993f90 cf993fac cf98a580
> <0>[   60.008026] 3fa0: c004b07c 00000000 00000000 c000e058 00000000
> 00000000 00000000 00000000
> <0>[   60.020782] 3fc0: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> <0>[   60.033355] 3fe0: 00000000 00000000 00000000 00000000 00000013
> 00000000 40000500 04000401
> <4>[   60.045867] [<bf2640c8>] (try_get_usb_function_instance
> [libcomposite]) from [<c015d0dc>] (__kernfs_create_file+0x7c/0xa0)
> <4>[   60.065155] [<c015d0dc>] (__kernfs_create_file) from [<c015d8d8>]
> (sysfs_add_file_mode_ns+0x134/0x1ac)
> <4>[   60.082977] [<c015d8d8>] (sysfs_add_file_mode_ns) from
> [<c015d980>] (sysfs_create_file_ns+0x30/0x44)
> <4>[   60.100952] [<c015d980>] (sysfs_create_file_ns) from [<cedaab80>]
> (0xcedaab80)
> <0>[   60.112884] Code: bad PC value
> <6>[   60.210144] NET: Registered protocol family 10
> <6>[   60.671600] bq27x00-battery 2-0055: support ver. 1.2.0 enabled
> <6>[   60.832885] bq27x00-battery 2-0055: battery is not calibrated!
> ignoring capacity values
> <6>[   63.788269] Adding 786428k swap on /dev/mmcblk0p3.  Priority:-1
> extents:1 across:786428k SS
> <5>[   64.057617] random: nonblocking pool is initialized
> <6>[   64.554016] NET: Registered protocol family 35
> <4>[   65.667510] ---[ end trace 85b8d26ec312a8c1 ]---
>
>
> Regards,
> Ivo

Removing __init from nokia_bind declaration makes g_nokia working again. 
I guess someone who knows better than me that sections magic should take 
it from here.

Ivo

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 6665 bytes --]

On Saturday 07 February 2015 19:33:53 Ivaylo Dimitrov wrote:
> Hi,
> 
> On  7.02.2015 20:01, Ivaylo Dimitrov wrote:
> > On  2.02.2015 21:14, Felipe Balbi wrote:
> >> On Mon, Feb 02, 2015 at 08:07:51PM +0100, Pali Rohár wrote:
> >>> On Monday 02 February 2015 20:01:11 Felipe Balbi wrote:
> >>>> Hi,
> >>>> 
> >>>> On Mon, Feb 02, 2015 at 07:58:59PM +0100, Pali Rohár 
wrote:
> >>>>> On Monday 02 February 2015 19:54:58 Felipe Balbi wrote:
> >>>>>> Hi,
> >>>>>> 
> >>>>>> On Sat, Jan 31, 2015 at 10:53:30AM +0100, Pali Rohár 
wrote:
> >>>>>>> This patch adds removable mass storage support to
> >>>>>>> g_nokia gadget (for N900). It means that at runtime
> >>>>>>> block device can be exported or unexported. So it does
> >>>>>>> not export anything by default and thus allows to use
> >>>>>>> MyDocs partition as before...
> >>>>>>> 
> >>>>>>> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> >>>>>> 
> >>>>>> thanks, but no thanks. Build your own using configfs.
> >>>>> 
> >>>>> But it needs some userspace interaction right?
> >>>>> Then its not possible for nfsboot.
> >>>> 
> >>>> oh, right... you're using nfsboot through g_nokia. Hmm,
> >>>> sounds like you need initramfs.
> >>> 
> >>> Also compiling usb gadgets as external .ko modules is
> >>> broken. So I cannot use configfs, when I compile g_nokia
> >>> even if I use initramfs...
> >> 
> >> yeah, there are people working on that and some patches
> >> already flying around for it. Meanwhile, you can make it
> >> built-in and use initramfs to add mass_storage through
> >> configfs to g_nokia, no issues.
> > 
> > Bellow is the bactrace of the crash when g_nokia is
> > modprobed. Any hint on where to look for is appreciated.
> > 
> > <0>[   33.751983] Kernel panic - not syncing: Fatal
> > exception ÿÿT!19\00]\00]<6>[   59.570159] bq2415x-charger
> > 2-006b: automode enabled <1>[   59.597534] [bf286014]
> > *pgd=8eda8811, *pte=00000000, *ppte=00000000 <6>[  
> > 59.609405] bq2415x-charger 2-006b: driver registered <0>[  
> > 59.640472] Internal error: Oops: 80000007 [#1] PREEMPT ARM
> > <4>[   59.650421] Modules linked in: ipv6(+)
> > bq2415x_charger(+) g_mass_storage usb_f_mass_storage
> > libcomposite configfs uinput adp1653 ad5820 et8ek8 smiaregs
> > hsi_char radio_platform_si4713 joydev omap_ssi_port
> > wl1251_spi wl1251 rx51_battery mac80211 isp1704_charger
> > smc91x mii cfg80211 si4713 v4l2_common crc7 videodev
> > tsc2005 media tsl2563 twl4030_vibra ff_memless
> > lis3lv02d_i2c lis3lv02d omap_ssi input_polldev hsi
> > twl4030_wdt omap_wdt rtc_twl leds_lp5523 leds_lp55xx_common
> > <4>[   59.715332] CPU: 0 PID: 13 Comm: kworker/0:1 Not
> > tainted 3.19.0-rc5+ #2
> > <4>[   59.726501] Hardware name: Nokia RX-51 board
> > <4>[   59.735168] Workqueue: events usb_gadget_work
> > <4>[   59.743896] task: cf8ab900 ti: cf992000 task.ti:
> > cf992000 <4>[   59.753692] PC is at 0xbf286014
> > <4>[   59.761108] LR is at
> > try_get_usb_function_instance+0x90/0x9c [libcomposite]
> > <4>[   59.772613] pc : [<bf286014>]    lr : [<bf2640c8>]   
> > psr: 80000113 <4>[   59.772613] sp : cf993e50  ip :
> > 0000001c  fp : 00000000 <4>[   59.793060] r10: 00000000  r9
> > : 00000000  r8 : cfcad600 <4>[   59.802581] r7 : cedaab80 
> > r6 : 00000000  r5 : bf28444c  r4 : cedaab80
> > <4>[   59.813476] r3 : cedd8f38  r2 : 00000001  r1 :
> > 00000000  r0 : cedd8f04
> > <4>[   59.824310] Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32
> >  ISA ARM Segment kernel
> > <4>[   59.836090] Control: 10c5387d  Table: 8e970019  DAC:
> > 00000015 <0>[   59.846252] Process kworker/0:1 (pid: 13,
> > stack limit = 0xcf992238) <0>[   59.857086] Stack:
> > (0xcf993e50 to 0xcf994000) <0>[   59.865814] 3e40:         
> >                            c03fb2d8 bf266e68 cf1fdf00
> > c015d0dc
> > <0>[   59.878723] 3e60: 92000000 00001000 00000000 c015d8d8
> > 00001000 00000000 c03fb2d8 bf266e68
> > <0>[   59.891693] 3e80: 00000000 00000001 00000000 c00f58f8
> > cf801c80 cedaab80 cf03b0b8 cedaac00
> > <0>[   59.904663] 3ea0: bf284424 c015d980 00000000 bf266e68
> > 00000000 cedaab80 bf28444c 00000000
> > <0>[   59.917572] 3ec0: cf03b0b8 cfcad600 00000000 bf263dd8
> > cf03b0b8 bf28444c cf03b0b8 cf1fea00
> > <0>[   59.930450] 3ee0: c0620cd8 bf28444c c0609868 cfcad600
> > 00000000 00000000 00000000 c02c5478
> > <0>[   59.943420] 3f00: bf284470 c0620cd8 c0606ff8 c02c5f54
> > cf98d080 bf284470 c0606ff8 c0047288
> > <0>[   59.956390] 3f20: cf98d080 bf284470 00000001 cf98d080
> > c0606ff8 c0607008 c0609868 cf98d098
> > <0>[   59.969360] 3f40: 00000008 00000000 00000000 c00477ec
> > cf98a580 00000000 cf98d080 c00474c4
> > <0>[   59.982330] 3f60: 00000000 00000000 00000000 c004b148
> > 22000000 00000000 11880000 cf98d080
> > <0>[   59.995208] 3f80: 00000000 cf993f84 cf993f84 00000000
> > cf993f90 cf993f90 cf993fac cf98a580
> > <0>[   60.008026] 3fa0: c004b07c 00000000 00000000 c000e058
> > 00000000 00000000 00000000 00000000
> > <0>[   60.020782] 3fc0: 00000000 00000000 00000000 00000000
> > 00000000 00000000 00000000 00000000
> > <0>[   60.033355] 3fe0: 00000000 00000000 00000000 00000000
> > 00000013 00000000 40000500 04000401
> > <4>[   60.045867] [<bf2640c8>]
> > (try_get_usb_function_instance [libcomposite]) from
> > [<c015d0dc>] (__kernfs_create_file+0x7c/0xa0) <4>[  
> > 60.065155] [<c015d0dc>] (__kernfs_create_file) from
> > [<c015d8d8>] (sysfs_add_file_mode_ns+0x134/0x1ac)
> > <4>[   60.082977] [<c015d8d8>] (sysfs_add_file_mode_ns) from
> > [<c015d980>] (sysfs_create_file_ns+0x30/0x44)
> > <4>[   60.100952] [<c015d980>] (sysfs_create_file_ns) from
> > [<cedaab80>] (0xcedaab80)
> > <0>[   60.112884] Code: bad PC value
> > <6>[   60.210144] NET: Registered protocol family 10
> > <6>[   60.671600] bq27x00-battery 2-0055: support ver. 1.2.0
> > enabled <6>[   60.832885] bq27x00-battery 2-0055: battery
> > is not calibrated! ignoring capacity values
> > <6>[   63.788269] Adding 786428k swap on /dev/mmcblk0p3. 
> > Priority:-1 extents:1 across:786428k SS
> > <5>[   64.057617] random: nonblocking pool is initialized
> > <6>[   64.554016] NET: Registered protocol family 35
> > <4>[   65.667510] ---[ end trace 85b8d26ec312a8c1 ]---
> > 
> > 
> > Regards,
> > Ivo
> 
> Removing __init from nokia_bind declaration makes g_nokia
> working again. I guess someone who knows better than me that
> sections magic should take it from here.
> 
> Ivo

No, it is not working on my devices. kernel does not crash, but 
usb communication does not work. all usb networking packets are 
lost.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 794 bytes --]

On Saturday 31 January 2015 10:53:30 Pali Rohár wrote:
> This patch adds removable mass storage support to g_nokia
> gadget (for N900). It means that at runtime block device can
> be exported or unexported. So it does not export anything by
> default and thus allows to use MyDocs partition as before...
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  drivers/usb/gadget/legacy/Kconfig |    1 +
>  drivers/usb/gadget/legacy/nokia.c |  102
> ++++++++++++++++++++++++++++++++++++- 2 files changed, 102
> insertions(+), 1 deletion(-)
> 

Compiling gadgets as external .ko modules is still broken and 
also for nfs boot, external modules are useless. So I would 
propose my solution for usb mass storage in g_nokia again.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

On Saturday 31 January 2015 10:53:30 Pali Rohár wrote:
> This patch adds removable mass storage support to g_nokia gadget (for N900).
> It means that at runtime block device can be exported or unexported.
> So it does not export anything by default and thus allows to use MyDocs
> partition as before...
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  drivers/usb/gadget/legacy/Kconfig |    1 +
>  drivers/usb/gadget/legacy/nokia.c |  102 ++++++++++++++++++++++++++++++++++++-
>  2 files changed, 102 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig
> index fd48ef3..36f6ba4 100644
> --- a/drivers/usb/gadget/legacy/Kconfig
> +++ b/drivers/usb/gadget/legacy/Kconfig
> @@ -345,6 +345,7 @@ config USB_G_NOKIA
>  	select USB_F_OBEX
>  	select USB_F_PHONET
>  	select USB_F_ECM
> +	select USB_F_MASS_STORAGE
>  	help
>  	  The Nokia composite gadget provides support for acm, obex
>  	  and phonet in only one composite gadget driver.
> diff --git a/drivers/usb/gadget/legacy/nokia.c b/drivers/usb/gadget/legacy/nokia.c
> index 9b8fd70..a09bb50 100644
> --- a/drivers/usb/gadget/legacy/nokia.c
> +++ b/drivers/usb/gadget/legacy/nokia.c
> @@ -24,6 +24,7 @@
>  #include "u_phonet.h"
>  #include "u_ecm.h"
>  #include "gadget_chips.h"
> +#include "f_mass_storage.h"
>  
>  /* Defines */
>  
> @@ -34,6 +35,29 @@ USB_GADGET_COMPOSITE_OPTIONS();
>  
>  USB_ETHERNET_MODULE_PARAMETERS();
>  
> +static struct fsg_module_parameters fsg_mod_data = {
> +	.stall = 0,
> +	.luns = 2,
> +	.removable_count = 2,
> +	.removable = { 1, 1, },
> +};
> +
> +FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
> +
> +#ifdef CONFIG_USB_GADGET_DEBUG_FILES
> +
> +static unsigned int fsg_num_buffers = CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS;
> +
> +#else
> +
> +/*
> + * Number of buffers we will use.
> + * 2 is usually enough for good buffering pipeline
> + */
> +#define fsg_num_buffers	CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS
> +
> +#endif /* CONFIG_USB_DEBUG */
> +
>  #define NOKIA_VENDOR_ID			0x0421	/* Nokia */
>  #define NOKIA_PRODUCT_ID		0x01c8	/* Nokia Gadget */
>  
> @@ -94,6 +118,8 @@ static struct usb_function *f_obex1_cfg2;
>  static struct usb_function *f_obex2_cfg2;
>  static struct usb_function *f_phonet_cfg1;
>  static struct usb_function *f_phonet_cfg2;
> +static struct usb_function *f_msg_cfg1;
> +static struct usb_function *f_msg_cfg2;
>  
>  
>  static struct usb_configuration nokia_config_500ma_driver = {
> @@ -117,6 +143,7 @@ static struct usb_function_instance *fi_ecm;
>  static struct usb_function_instance *fi_obex1;
>  static struct usb_function_instance *fi_obex2;
>  static struct usb_function_instance *fi_phonet;
> +static struct usb_function_instance *fi_msg;
>  
>  static int __init nokia_bind_config(struct usb_configuration *c)
>  {
> @@ -125,6 +152,8 @@ static int __init nokia_bind_config(struct usb_configuration *c)
>  	struct usb_function *f_obex1 = NULL;
>  	struct usb_function *f_ecm;
>  	struct usb_function *f_obex2 = NULL;
> +	struct usb_function *f_msg;
> +	struct fsg_opts *fsg_opts;
>  	int status = 0;
>  	int obex1_stat = -1;
>  	int obex2_stat = -1;
> @@ -160,6 +189,12 @@ static int __init nokia_bind_config(struct usb_configuration *c)
>  		goto err_get_ecm;
>  	}
>  
> +	f_msg = usb_get_function(fi_msg);
> +	if (IS_ERR(f_msg)) {
> +		status = PTR_ERR(f_msg);
> +		goto err_get_msg;
> +	}
> +
>  	if (!IS_ERR_OR_NULL(f_phonet)) {
>  		phonet_stat = usb_add_function(c, f_phonet);
>  		if (phonet_stat)
> @@ -187,21 +222,36 @@ static int __init nokia_bind_config(struct usb_configuration *c)
>  		pr_debug("could not bind ecm config %d\n", status);
>  		goto err_ecm;
>  	}
> +
> +	fsg_opts = fsg_opts_from_func_inst(fi_msg);
> +
> +	status = fsg_common_run_thread(fsg_opts->common);
> +	if (status)
> +		goto err_msg;
> +
> +	status = usb_add_function(c, f_msg);
> +	if (status)
> +		goto err_msg;
> +
>  	if (c == &nokia_config_500ma_driver) {
>  		f_acm_cfg1 = f_acm;
>  		f_ecm_cfg1 = f_ecm;
>  		f_phonet_cfg1 = f_phonet;
>  		f_obex1_cfg1 = f_obex1;
>  		f_obex2_cfg1 = f_obex2;
> +		f_msg_cfg1 = f_msg;
>  	} else {
>  		f_acm_cfg2 = f_acm;
>  		f_ecm_cfg2 = f_ecm;
>  		f_phonet_cfg2 = f_phonet;
>  		f_obex1_cfg2 = f_obex1;
>  		f_obex2_cfg2 = f_obex2;
> +		f_msg_cfg2 = f_msg;
>  	}
>  
>  	return status;
> +err_msg:
> +	usb_remove_function(c, f_ecm);
>  err_ecm:
>  	usb_remove_function(c, f_acm);
>  err_conf:
> @@ -211,6 +261,8 @@ err_conf:
>  		usb_remove_function(c, f_obex1);
>  	if (!phonet_stat)
>  		usb_remove_function(c, f_phonet);
> +	usb_put_function(f_msg);
> +err_get_msg:
>  	usb_put_function(f_ecm);
>  err_get_ecm:
>  	usb_put_function(f_acm);
> @@ -227,6 +279,8 @@ err_get_acm:
>  static int __init nokia_bind(struct usb_composite_dev *cdev)
>  {
>  	struct usb_gadget	*gadget = cdev->gadget;
> +	struct fsg_opts		*fsg_opts;
> +	struct fsg_config	fsg_config;
>  	int			status;
>  
>  	status = usb_string_ids_tab(cdev, strings_dev);
> @@ -267,11 +321,46 @@ static int __init nokia_bind(struct usb_composite_dev *cdev)
>  		goto err_acm_inst;
>  	}
>  
> +	fi_msg = usb_get_function_instance("mass_storage");
> +	if (IS_ERR(fi_msg)) {
> +		status = PTR_ERR(fi_msg);
> +		goto err_ecm_inst;
> +	}
> +
> +	/* set up mass storage function */
> +	fsg_config_from_params(&fsg_config, &fsg_mod_data, fsg_num_buffers);
> +	fsg_config.vendor_name = "Nokia";
> +	fsg_config.product_name = "N900";
> +
> +	fsg_opts = fsg_opts_from_func_inst(fi_msg);
> +	fsg_opts->no_configfs = true;
> +
> +	status = fsg_common_set_num_buffers(fsg_opts->common, fsg_num_buffers);
> +	if (status)
> +		goto err_msg_inst;
> +
> +	status = fsg_common_set_nluns(fsg_opts->common, fsg_config.nluns);
> +	if (status)
> +		goto err_msg_buf;
> +
> +	status = fsg_common_set_cdev(fsg_opts->common, cdev, fsg_config.can_stall);
> +	if (status)
> +		goto err_msg_set_nluns;
> +
> +	fsg_common_set_sysfs(fsg_opts->common, true);
> +
> +	status = fsg_common_create_luns(fsg_opts->common, &fsg_config);
> +	if (status)
> +		goto err_msg_set_nluns;
> +
> +	fsg_common_set_inquiry_string(fsg_opts->common, fsg_config.vendor_name,
> +				      fsg_config.product_name);
> +
>  	/* finally register the configuration */
>  	status = usb_add_config(cdev, &nokia_config_500ma_driver,
>  			nokia_bind_config);
>  	if (status < 0)
> -		goto err_ecm_inst;
> +		goto err_msg_set_cdev;
>  
>  	status = usb_add_config(cdev, &nokia_config_100ma_driver,
>  			nokia_bind_config);
> @@ -292,6 +381,14 @@ err_put_cfg1:
>  	if (!IS_ERR_OR_NULL(f_phonet_cfg1))
>  		usb_put_function(f_phonet_cfg1);
>  	usb_put_function(f_ecm_cfg1);
> +err_msg_set_cdev:
> +	fsg_common_remove_luns(fsg_opts->common);
> +err_msg_set_nluns:
> +	fsg_common_free_luns(fsg_opts->common);
> +err_msg_buf:
> +	fsg_common_free_buffers(fsg_opts->common);
> +err_msg_inst:
> +	usb_put_function_instance(fi_msg);
>  err_ecm_inst:
>  	usb_put_function_instance(fi_ecm);
>  err_acm_inst:
> @@ -325,7 +422,10 @@ static int __exit nokia_unbind(struct usb_composite_dev *cdev)
>  	usb_put_function(f_acm_cfg2);
>  	usb_put_function(f_ecm_cfg1);
>  	usb_put_function(f_ecm_cfg2);
> +	usb_put_function(f_msg_cfg1);
> +	usb_put_function(f_msg_cfg2);
>  
> +	usb_put_function_instance(fi_msg);
>  	usb_put_function_instance(fi_ecm);
>  	if (!IS_ERR(fi_obex2))
>  		usb_put_function_instance(fi_obex2);

Opening discussion about this patch again as this is still only one
solution how to use use mass storage without breaking other stuff...

Please understand finally that usb networking is very very important for
development on this device and usb mass storage is needed for
transferring bigger data.

Also to clarify potential regressions: This patch adds *optional* usb
mass storage support and block device can be attached/detached to driver
at runtime. It does *not* enforce to export some (mmc) device
automatically. It is optional and userspace can decide...

So MyDocs N900 partition is not automatically enforced to be exported
via usb (as some people thought) and so it does not break usb networking
or mass storage mode or MyDocs parition on N900 with Maemo system.

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Krzysztof Opasiak]

On 05/28/2015 09:47 AM, Pali Rohár wrote:
> On Saturday 31 January 2015 10:53:30 Pali Rohár wrote:
>> This patch adds removable mass storage support to g_nokia gadget (for N900).
>> It means that at runtime block device can be exported or unexported.
>> So it does not export anything by default and thus allows to use MyDocs
>> partition as before...
>>
>> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
>> ---
>>   drivers/usb/gadget/legacy/Kconfig |    1 +
>>   drivers/usb/gadget/legacy/nokia.c |  102 ++++++++++++++++++++++++++++++++++++-
>>   2 files changed, 102 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig
>> index fd48ef3..36f6ba4 100644
>> --- a/drivers/usb/gadget/legacy/Kconfig
>> +++ b/drivers/usb/gadget/legacy/Kconfig
>> @@ -345,6 +345,7 @@ config USB_G_NOKIA
>>   	select USB_F_OBEX
>>   	select USB_F_PHONET
>>   	select USB_F_ECM
>> +	select USB_F_MASS_STORAGE
>>   	help
>>   	  The Nokia composite gadget provides support for acm, obex
>>   	  and phonet in only one composite gadget driver.
>> diff --git a/drivers/usb/gadget/legacy/nokia.c b/drivers/usb/gadget/legacy/nokia.c
>> index 9b8fd70..a09bb50 100644
>> --- a/drivers/usb/gadget/legacy/nokia.c
>> +++ b/drivers/usb/gadget/legacy/nokia.c
>> @@ -24,6 +24,7 @@
>>   #include "u_phonet.h"
>>   #include "u_ecm.h"
>>   #include "gadget_chips.h"
>> +#include "f_mass_storage.h"
>>
>>   /* Defines */
>>
>> @@ -34,6 +35,29 @@ USB_GADGET_COMPOSITE_OPTIONS();
>>
>>   USB_ETHERNET_MODULE_PARAMETERS();
>>
>> +static struct fsg_module_parameters fsg_mod_data = {
>> +	.stall = 0,
>> +	.luns = 2,
>> +	.removable_count = 2,
>> +	.removable = { 1, 1, },
>> +};
>> +
>> +FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
>> +
>> +#ifdef CONFIG_USB_GADGET_DEBUG_FILES
>> +
>> +static unsigned int fsg_num_buffers = CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS;
>> +
>> +#else
>> +
>> +/*
>> + * Number of buffers we will use.
>> + * 2 is usually enough for good buffering pipeline
>> + */
>> +#define fsg_num_buffers	CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS
>> +
>> +#endif /* CONFIG_USB_DEBUG */
>> +
>>   #define NOKIA_VENDOR_ID			0x0421	/* Nokia */
>>   #define NOKIA_PRODUCT_ID		0x01c8	/* Nokia Gadget */
>>
>> @@ -94,6 +118,8 @@ static struct usb_function *f_obex1_cfg2;
>>   static struct usb_function *f_obex2_cfg2;
>>   static struct usb_function *f_phonet_cfg1;
>>   static struct usb_function *f_phonet_cfg2;
>> +static struct usb_function *f_msg_cfg1;
>> +static struct usb_function *f_msg_cfg2;
>>
>>
>>   static struct usb_configuration nokia_config_500ma_driver = {
>> @@ -117,6 +143,7 @@ static struct usb_function_instance *fi_ecm;
>>   static struct usb_function_instance *fi_obex1;
>>   static struct usb_function_instance *fi_obex2;
>>   static struct usb_function_instance *fi_phonet;
>> +static struct usb_function_instance *fi_msg;
>>
>>   static int __init nokia_bind_config(struct usb_configuration *c)
>>   {
>> @@ -125,6 +152,8 @@ static int __init nokia_bind_config(struct usb_configuration *c)
>>   	struct usb_function *f_obex1 = NULL;
>>   	struct usb_function *f_ecm;
>>   	struct usb_function *f_obex2 = NULL;
>> +	struct usb_function *f_msg;
>> +	struct fsg_opts *fsg_opts;
>>   	int status = 0;
>>   	int obex1_stat = -1;
>>   	int obex2_stat = -1;
>> @@ -160,6 +189,12 @@ static int __init nokia_bind_config(struct usb_configuration *c)
>>   		goto err_get_ecm;
>>   	}
>>
>> +	f_msg = usb_get_function(fi_msg);
>> +	if (IS_ERR(f_msg)) {
>> +		status = PTR_ERR(f_msg);
>> +		goto err_get_msg;
>> +	}
>> +
>>   	if (!IS_ERR_OR_NULL(f_phonet)) {
>>   		phonet_stat = usb_add_function(c, f_phonet);
>>   		if (phonet_stat)
>> @@ -187,21 +222,36 @@ static int __init nokia_bind_config(struct usb_configuration *c)
>>   		pr_debug("could not bind ecm config %d\n", status);
>>   		goto err_ecm;
>>   	}
>> +
>> +	fsg_opts = fsg_opts_from_func_inst(fi_msg);
>> +
>> +	status = fsg_common_run_thread(fsg_opts->common);
>> +	if (status)
>> +		goto err_msg;
>> +
>> +	status = usb_add_function(c, f_msg);
>> +	if (status)
>> +		goto err_msg;
>> +
>>   	if (c == &nokia_config_500ma_driver) {
>>   		f_acm_cfg1 = f_acm;
>>   		f_ecm_cfg1 = f_ecm;
>>   		f_phonet_cfg1 = f_phonet;
>>   		f_obex1_cfg1 = f_obex1;
>>   		f_obex2_cfg1 = f_obex2;
>> +		f_msg_cfg1 = f_msg;
>>   	} else {
>>   		f_acm_cfg2 = f_acm;
>>   		f_ecm_cfg2 = f_ecm;
>>   		f_phonet_cfg2 = f_phonet;
>>   		f_obex1_cfg2 = f_obex1;
>>   		f_obex2_cfg2 = f_obex2;
>> +		f_msg_cfg2 = f_msg;
>>   	}
>>
>>   	return status;
>> +err_msg:
>> +	usb_remove_function(c, f_ecm);
>>   err_ecm:
>>   	usb_remove_function(c, f_acm);
>>   err_conf:
>> @@ -211,6 +261,8 @@ err_conf:
>>   		usb_remove_function(c, f_obex1);
>>   	if (!phonet_stat)
>>   		usb_remove_function(c, f_phonet);
>> +	usb_put_function(f_msg);
>> +err_get_msg:
>>   	usb_put_function(f_ecm);
>>   err_get_ecm:
>>   	usb_put_function(f_acm);
>> @@ -227,6 +279,8 @@ err_get_acm:
>>   static int __init nokia_bind(struct usb_composite_dev *cdev)
>>   {
>>   	struct usb_gadget	*gadget = cdev->gadget;
>> +	struct fsg_opts		*fsg_opts;
>> +	struct fsg_config	fsg_config;
>>   	int			status;
>>
>>   	status = usb_string_ids_tab(cdev, strings_dev);
>> @@ -267,11 +321,46 @@ static int __init nokia_bind(struct usb_composite_dev *cdev)
>>   		goto err_acm_inst;
>>   	}
>>
>> +	fi_msg = usb_get_function_instance("mass_storage");
>> +	if (IS_ERR(fi_msg)) {
>> +		status = PTR_ERR(fi_msg);
>> +		goto err_ecm_inst;
>> +	}
>> +
>> +	/* set up mass storage function */
>> +	fsg_config_from_params(&fsg_config, &fsg_mod_data, fsg_num_buffers);
>> +	fsg_config.vendor_name = "Nokia";
>> +	fsg_config.product_name = "N900";
>> +
>> +	fsg_opts = fsg_opts_from_func_inst(fi_msg);
>> +	fsg_opts->no_configfs = true;
>> +
>> +	status = fsg_common_set_num_buffers(fsg_opts->common, fsg_num_buffers);
>> +	if (status)
>> +		goto err_msg_inst;
>> +
>> +	status = fsg_common_set_nluns(fsg_opts->common, fsg_config.nluns);
>> +	if (status)
>> +		goto err_msg_buf;
>> +
>> +	status = fsg_common_set_cdev(fsg_opts->common, cdev, fsg_config.can_stall);
>> +	if (status)
>> +		goto err_msg_set_nluns;
>> +
>> +	fsg_common_set_sysfs(fsg_opts->common, true);
>> +
>> +	status = fsg_common_create_luns(fsg_opts->common, &fsg_config);
>> +	if (status)
>> +		goto err_msg_set_nluns;
>> +
>> +	fsg_common_set_inquiry_string(fsg_opts->common, fsg_config.vendor_name,
>> +				      fsg_config.product_name);
>> +
>>   	/* finally register the configuration */
>>   	status = usb_add_config(cdev, &nokia_config_500ma_driver,
>>   			nokia_bind_config);
>>   	if (status < 0)
>> -		goto err_ecm_inst;
>> +		goto err_msg_set_cdev;
>>
>>   	status = usb_add_config(cdev, &nokia_config_100ma_driver,
>>   			nokia_bind_config);
>> @@ -292,6 +381,14 @@ err_put_cfg1:
>>   	if (!IS_ERR_OR_NULL(f_phonet_cfg1))
>>   		usb_put_function(f_phonet_cfg1);
>>   	usb_put_function(f_ecm_cfg1);
>> +err_msg_set_cdev:
>> +	fsg_common_remove_luns(fsg_opts->common);
>> +err_msg_set_nluns:
>> +	fsg_common_free_luns(fsg_opts->common);
>> +err_msg_buf:
>> +	fsg_common_free_buffers(fsg_opts->common);
>> +err_msg_inst:
>> +	usb_put_function_instance(fi_msg);
>>   err_ecm_inst:
>>   	usb_put_function_instance(fi_ecm);
>>   err_acm_inst:
>> @@ -325,7 +422,10 @@ static int __exit nokia_unbind(struct usb_composite_dev *cdev)
>>   	usb_put_function(f_acm_cfg2);
>>   	usb_put_function(f_ecm_cfg1);
>>   	usb_put_function(f_ecm_cfg2);
>> +	usb_put_function(f_msg_cfg1);
>> +	usb_put_function(f_msg_cfg2);
>>
>> +	usb_put_function_instance(fi_msg);
>>   	usb_put_function_instance(fi_ecm);
>>   	if (!IS_ERR(fi_obex2))
>>   		usb_put_function_instance(fi_obex2);
>
> Opening discussion about this patch again as this is still only one
> solution how to use use mass storage without breaking other stuff...
>
> Please understand finally that usb networking is very very important for
> development on this device and usb mass storage is needed for
> transferring bigger data.
>
> Also to clarify potential regressions: This patch adds *optional* usb
> mass storage support and block device can be attached/detached to driver
> at runtime. It does *not* enforce to export some (mmc) device
> automatically. It is optional and userspace can decide...
>
> So MyDocs N900 partition is not automatically enforced to be exported
> via usb (as some people thought) and so it does not break usb networking
> or mass storage mode or MyDocs parition on N900 with Maemo system.
>

Instead of adding mass storage to legacy gadget you may switch to 
configfs interface and compose equivalent of g_nokia + mass storage 
without any changes in kernel.

Best regards,

-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

On Thursday 28 May 2015 16:27:44 Krzysztof Opasiak wrote:
> 
> 
> On 05/28/2015 09:47 AM, Pali Rohár wrote:
> >On Saturday 31 January 2015 10:53:30 Pali Rohár wrote:
> >>This patch adds removable mass storage support to g_nokia gadget (for N900).
> >>It means that at runtime block device can be exported or unexported.
> >>So it does not export anything by default and thus allows to use MyDocs
> >>partition as before...
> >>
> >>Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> >>---
> >>  drivers/usb/gadget/legacy/Kconfig |    1 +
> >>  drivers/usb/gadget/legacy/nokia.c |  102 ++++++++++++++++++++++++++++++++++++-
> >>  2 files changed, 102 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig
> >>index fd48ef3..36f6ba4 100644
> >>--- a/drivers/usb/gadget/legacy/Kconfig
> >>+++ b/drivers/usb/gadget/legacy/Kconfig
> >>@@ -345,6 +345,7 @@ config USB_G_NOKIA
> >>  	select USB_F_OBEX
> >>  	select USB_F_PHONET
> >>  	select USB_F_ECM
> >>+	select USB_F_MASS_STORAGE
> >>  	help
> >>  	  The Nokia composite gadget provides support for acm, obex
> >>  	  and phonet in only one composite gadget driver.
> >>diff --git a/drivers/usb/gadget/legacy/nokia.c b/drivers/usb/gadget/legacy/nokia.c
> >>index 9b8fd70..a09bb50 100644
> >>--- a/drivers/usb/gadget/legacy/nokia.c
> >>+++ b/drivers/usb/gadget/legacy/nokia.c
> >>@@ -24,6 +24,7 @@
> >>  #include "u_phonet.h"
> >>  #include "u_ecm.h"
> >>  #include "gadget_chips.h"
> >>+#include "f_mass_storage.h"
> >>
> >>  /* Defines */
> >>
> >>@@ -34,6 +35,29 @@ USB_GADGET_COMPOSITE_OPTIONS();
> >>
> >>  USB_ETHERNET_MODULE_PARAMETERS();
> >>
> >>+static struct fsg_module_parameters fsg_mod_data = {
> >>+	.stall = 0,
> >>+	.luns = 2,
> >>+	.removable_count = 2,
> >>+	.removable = { 1, 1, },
> >>+};
> >>+
> >>+FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
> >>+
> >>+#ifdef CONFIG_USB_GADGET_DEBUG_FILES
> >>+
> >>+static unsigned int fsg_num_buffers = CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS;
> >>+
> >>+#else
> >>+
> >>+/*
> >>+ * Number of buffers we will use.
> >>+ * 2 is usually enough for good buffering pipeline
> >>+ */
> >>+#define fsg_num_buffers	CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS
> >>+
> >>+#endif /* CONFIG_USB_DEBUG */
> >>+
> >>  #define NOKIA_VENDOR_ID			0x0421	/* Nokia */
> >>  #define NOKIA_PRODUCT_ID		0x01c8	/* Nokia Gadget */
> >>
> >>@@ -94,6 +118,8 @@ static struct usb_function *f_obex1_cfg2;
> >>  static struct usb_function *f_obex2_cfg2;
> >>  static struct usb_function *f_phonet_cfg1;
> >>  static struct usb_function *f_phonet_cfg2;
> >>+static struct usb_function *f_msg_cfg1;
> >>+static struct usb_function *f_msg_cfg2;
> >>
> >>
> >>  static struct usb_configuration nokia_config_500ma_driver = {
> >>@@ -117,6 +143,7 @@ static struct usb_function_instance *fi_ecm;
> >>  static struct usb_function_instance *fi_obex1;
> >>  static struct usb_function_instance *fi_obex2;
> >>  static struct usb_function_instance *fi_phonet;
> >>+static struct usb_function_instance *fi_msg;
> >>
> >>  static int __init nokia_bind_config(struct usb_configuration *c)
> >>  {
> >>@@ -125,6 +152,8 @@ static int __init nokia_bind_config(struct usb_configuration *c)
> >>  	struct usb_function *f_obex1 = NULL;
> >>  	struct usb_function *f_ecm;
> >>  	struct usb_function *f_obex2 = NULL;
> >>+	struct usb_function *f_msg;
> >>+	struct fsg_opts *fsg_opts;
> >>  	int status = 0;
> >>  	int obex1_stat = -1;
> >>  	int obex2_stat = -1;
> >>@@ -160,6 +189,12 @@ static int __init nokia_bind_config(struct usb_configuration *c)
> >>  		goto err_get_ecm;
> >>  	}
> >>
> >>+	f_msg = usb_get_function(fi_msg);
> >>+	if (IS_ERR(f_msg)) {
> >>+		status = PTR_ERR(f_msg);
> >>+		goto err_get_msg;
> >>+	}
> >>+
> >>  	if (!IS_ERR_OR_NULL(f_phonet)) {
> >>  		phonet_stat = usb_add_function(c, f_phonet);
> >>  		if (phonet_stat)
> >>@@ -187,21 +222,36 @@ static int __init nokia_bind_config(struct usb_configuration *c)
> >>  		pr_debug("could not bind ecm config %d\n", status);
> >>  		goto err_ecm;
> >>  	}
> >>+
> >>+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
> >>+
> >>+	status = fsg_common_run_thread(fsg_opts->common);
> >>+	if (status)
> >>+		goto err_msg;
> >>+
> >>+	status = usb_add_function(c, f_msg);
> >>+	if (status)
> >>+		goto err_msg;
> >>+
> >>  	if (c == &nokia_config_500ma_driver) {
> >>  		f_acm_cfg1 = f_acm;
> >>  		f_ecm_cfg1 = f_ecm;
> >>  		f_phonet_cfg1 = f_phonet;
> >>  		f_obex1_cfg1 = f_obex1;
> >>  		f_obex2_cfg1 = f_obex2;
> >>+		f_msg_cfg1 = f_msg;
> >>  	} else {
> >>  		f_acm_cfg2 = f_acm;
> >>  		f_ecm_cfg2 = f_ecm;
> >>  		f_phonet_cfg2 = f_phonet;
> >>  		f_obex1_cfg2 = f_obex1;
> >>  		f_obex2_cfg2 = f_obex2;
> >>+		f_msg_cfg2 = f_msg;
> >>  	}
> >>
> >>  	return status;
> >>+err_msg:
> >>+	usb_remove_function(c, f_ecm);
> >>  err_ecm:
> >>  	usb_remove_function(c, f_acm);
> >>  err_conf:
> >>@@ -211,6 +261,8 @@ err_conf:
> >>  		usb_remove_function(c, f_obex1);
> >>  	if (!phonet_stat)
> >>  		usb_remove_function(c, f_phonet);
> >>+	usb_put_function(f_msg);
> >>+err_get_msg:
> >>  	usb_put_function(f_ecm);
> >>  err_get_ecm:
> >>  	usb_put_function(f_acm);
> >>@@ -227,6 +279,8 @@ err_get_acm:
> >>  static int __init nokia_bind(struct usb_composite_dev *cdev)
> >>  {
> >>  	struct usb_gadget	*gadget = cdev->gadget;
> >>+	struct fsg_opts		*fsg_opts;
> >>+	struct fsg_config	fsg_config;
> >>  	int			status;
> >>
> >>  	status = usb_string_ids_tab(cdev, strings_dev);
> >>@@ -267,11 +321,46 @@ static int __init nokia_bind(struct usb_composite_dev *cdev)
> >>  		goto err_acm_inst;
> >>  	}
> >>
> >>+	fi_msg = usb_get_function_instance("mass_storage");
> >>+	if (IS_ERR(fi_msg)) {
> >>+		status = PTR_ERR(fi_msg);
> >>+		goto err_ecm_inst;
> >>+	}
> >>+
> >>+	/* set up mass storage function */
> >>+	fsg_config_from_params(&fsg_config, &fsg_mod_data, fsg_num_buffers);
> >>+	fsg_config.vendor_name = "Nokia";
> >>+	fsg_config.product_name = "N900";
> >>+
> >>+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
> >>+	fsg_opts->no_configfs = true;
> >>+
> >>+	status = fsg_common_set_num_buffers(fsg_opts->common, fsg_num_buffers);
> >>+	if (status)
> >>+		goto err_msg_inst;
> >>+
> >>+	status = fsg_common_set_nluns(fsg_opts->common, fsg_config.nluns);
> >>+	if (status)
> >>+		goto err_msg_buf;
> >>+
> >>+	status = fsg_common_set_cdev(fsg_opts->common, cdev, fsg_config.can_stall);
> >>+	if (status)
> >>+		goto err_msg_set_nluns;
> >>+
> >>+	fsg_common_set_sysfs(fsg_opts->common, true);
> >>+
> >>+	status = fsg_common_create_luns(fsg_opts->common, &fsg_config);
> >>+	if (status)
> >>+		goto err_msg_set_nluns;
> >>+
> >>+	fsg_common_set_inquiry_string(fsg_opts->common, fsg_config.vendor_name,
> >>+				      fsg_config.product_name);
> >>+
> >>  	/* finally register the configuration */
> >>  	status = usb_add_config(cdev, &nokia_config_500ma_driver,
> >>  			nokia_bind_config);
> >>  	if (status < 0)
> >>-		goto err_ecm_inst;
> >>+		goto err_msg_set_cdev;
> >>
> >>  	status = usb_add_config(cdev, &nokia_config_100ma_driver,
> >>  			nokia_bind_config);
> >>@@ -292,6 +381,14 @@ err_put_cfg1:
> >>  	if (!IS_ERR_OR_NULL(f_phonet_cfg1))
> >>  		usb_put_function(f_phonet_cfg1);
> >>  	usb_put_function(f_ecm_cfg1);
> >>+err_msg_set_cdev:
> >>+	fsg_common_remove_luns(fsg_opts->common);
> >>+err_msg_set_nluns:
> >>+	fsg_common_free_luns(fsg_opts->common);
> >>+err_msg_buf:
> >>+	fsg_common_free_buffers(fsg_opts->common);
> >>+err_msg_inst:
> >>+	usb_put_function_instance(fi_msg);
> >>  err_ecm_inst:
> >>  	usb_put_function_instance(fi_ecm);
> >>  err_acm_inst:
> >>@@ -325,7 +422,10 @@ static int __exit nokia_unbind(struct usb_composite_dev *cdev)
> >>  	usb_put_function(f_acm_cfg2);
> >>  	usb_put_function(f_ecm_cfg1);
> >>  	usb_put_function(f_ecm_cfg2);
> >>+	usb_put_function(f_msg_cfg1);
> >>+	usb_put_function(f_msg_cfg2);
> >>
> >>+	usb_put_function_instance(fi_msg);
> >>  	usb_put_function_instance(fi_ecm);
> >>  	if (!IS_ERR(fi_obex2))
> >>  		usb_put_function_instance(fi_obex2);
> >
> >Opening discussion about this patch again as this is still only one
> >solution how to use use mass storage without breaking other stuff...
> >
> >Please understand finally that usb networking is very very important for
> >development on this device and usb mass storage is needed for
> >transferring bigger data.
> >
> >Also to clarify potential regressions: This patch adds *optional* usb
> >mass storage support and block device can be attached/detached to driver
> >at runtime. It does *not* enforce to export some (mmc) device
> >automatically. It is optional and userspace can decide...
> >
> >So MyDocs N900 partition is not automatically enforced to be exported
> >via usb (as some people thought) and so it does not break usb networking
> >or mass storage mode or MyDocs parition on N900 with Maemo system.
> >
> 
> Instead of adding mass storage to legacy gadget you may switch to
> configfs interface and compose equivalent of g_nokia + mass storage
> without any changes in kernel.
> 
> Best regards,
> 

That requires userspace. And there are developers who use nfsroot via
usb networking and so userspace will be there after properly
initializing usb gadget... So it is not easy and reason why I'm opening
this discussion again.

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Krzysztof Opasiak]

On 05/28/2015 04:31 PM, Pali Rohár wrote:
> On Thursday 28 May 2015 16:27:44 Krzysztof Opasiak wrote:
>>
>>
>> On 05/28/2015 09:47 AM, Pali Rohár wrote:
>>> On Saturday 31 January 2015 10:53:30 Pali Rohár wrote:
>>>> This patch adds removable mass storage support to g_nokia gadget (for N900).
>>>> It means that at runtime block device can be exported or unexported.
>>>> So it does not export anything by default and thus allows to use MyDocs
>>>> partition as before...
>>>>
>>>> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
>>>> ---
>>>>   drivers/usb/gadget/legacy/Kconfig |    1 +
>>>>   drivers/usb/gadget/legacy/nokia.c |  102 ++++++++++++++++++++++++++++++++++++-
>>>>   2 files changed, 102 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig
>>>> index fd48ef3..36f6ba4 100644
>>>> --- a/drivers/usb/gadget/legacy/Kconfig
>>>> +++ b/drivers/usb/gadget/legacy/Kconfig
>>>> @@ -345,6 +345,7 @@ config USB_G_NOKIA
>>>>   	select USB_F_OBEX
>>>>   	select USB_F_PHONET
>>>>   	select USB_F_ECM
>>>> +	select USB_F_MASS_STORAGE
>>>>   	help
>>>>   	  The Nokia composite gadget provides support for acm, obex
>>>>   	  and phonet in only one composite gadget driver.
>>>> diff --git a/drivers/usb/gadget/legacy/nokia.c b/drivers/usb/gadget/legacy/nokia.c
>>>> index 9b8fd70..a09bb50 100644
>>>> --- a/drivers/usb/gadget/legacy/nokia.c
>>>> +++ b/drivers/usb/gadget/legacy/nokia.c
>>>> @@ -24,6 +24,7 @@
>>>>   #include "u_phonet.h"
>>>>   #include "u_ecm.h"
>>>>   #include "gadget_chips.h"
>>>> +#include "f_mass_storage.h"
>>>>
>>>>   /* Defines */
>>>>
>>>> @@ -34,6 +35,29 @@ USB_GADGET_COMPOSITE_OPTIONS();
>>>>
>>>>   USB_ETHERNET_MODULE_PARAMETERS();
>>>>
>>>> +static struct fsg_module_parameters fsg_mod_data = {
>>>> +	.stall = 0,
>>>> +	.luns = 2,
>>>> +	.removable_count = 2,
>>>> +	.removable = { 1, 1, },
>>>> +};
>>>> +
>>>> +FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
>>>> +
>>>> +#ifdef CONFIG_USB_GADGET_DEBUG_FILES
>>>> +
>>>> +static unsigned int fsg_num_buffers = CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS;
>>>> +
>>>> +#else
>>>> +
>>>> +/*
>>>> + * Number of buffers we will use.
>>>> + * 2 is usually enough for good buffering pipeline
>>>> + */
>>>> +#define fsg_num_buffers	CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS
>>>> +
>>>> +#endif /* CONFIG_USB_DEBUG */
>>>> +
>>>>   #define NOKIA_VENDOR_ID			0x0421	/* Nokia */
>>>>   #define NOKIA_PRODUCT_ID		0x01c8	/* Nokia Gadget */
>>>>
>>>> @@ -94,6 +118,8 @@ static struct usb_function *f_obex1_cfg2;
>>>>   static struct usb_function *f_obex2_cfg2;
>>>>   static struct usb_function *f_phonet_cfg1;
>>>>   static struct usb_function *f_phonet_cfg2;
>>>> +static struct usb_function *f_msg_cfg1;
>>>> +static struct usb_function *f_msg_cfg2;
>>>>
>>>>
>>>>   static struct usb_configuration nokia_config_500ma_driver = {
>>>> @@ -117,6 +143,7 @@ static struct usb_function_instance *fi_ecm;
>>>>   static struct usb_function_instance *fi_obex1;
>>>>   static struct usb_function_instance *fi_obex2;
>>>>   static struct usb_function_instance *fi_phonet;
>>>> +static struct usb_function_instance *fi_msg;
>>>>
>>>>   static int __init nokia_bind_config(struct usb_configuration *c)
>>>>   {
>>>> @@ -125,6 +152,8 @@ static int __init nokia_bind_config(struct usb_configuration *c)
>>>>   	struct usb_function *f_obex1 = NULL;
>>>>   	struct usb_function *f_ecm;
>>>>   	struct usb_function *f_obex2 = NULL;
>>>> +	struct usb_function *f_msg;
>>>> +	struct fsg_opts *fsg_opts;
>>>>   	int status = 0;
>>>>   	int obex1_stat = -1;
>>>>   	int obex2_stat = -1;
>>>> @@ -160,6 +189,12 @@ static int __init nokia_bind_config(struct usb_configuration *c)
>>>>   		goto err_get_ecm;
>>>>   	}
>>>>
>>>> +	f_msg = usb_get_function(fi_msg);
>>>> +	if (IS_ERR(f_msg)) {
>>>> +		status = PTR_ERR(f_msg);
>>>> +		goto err_get_msg;
>>>> +	}
>>>> +
>>>>   	if (!IS_ERR_OR_NULL(f_phonet)) {
>>>>   		phonet_stat = usb_add_function(c, f_phonet);
>>>>   		if (phonet_stat)
>>>> @@ -187,21 +222,36 @@ static int __init nokia_bind_config(struct usb_configuration *c)
>>>>   		pr_debug("could not bind ecm config %d\n", status);
>>>>   		goto err_ecm;
>>>>   	}
>>>> +
>>>> +	fsg_opts = fsg_opts_from_func_inst(fi_msg);
>>>> +
>>>> +	status = fsg_common_run_thread(fsg_opts->common);
>>>> +	if (status)
>>>> +		goto err_msg;
>>>> +
>>>> +	status = usb_add_function(c, f_msg);
>>>> +	if (status)
>>>> +		goto err_msg;
>>>> +
>>>>   	if (c == &nokia_config_500ma_driver) {
>>>>   		f_acm_cfg1 = f_acm;
>>>>   		f_ecm_cfg1 = f_ecm;
>>>>   		f_phonet_cfg1 = f_phonet;
>>>>   		f_obex1_cfg1 = f_obex1;
>>>>   		f_obex2_cfg1 = f_obex2;
>>>> +		f_msg_cfg1 = f_msg;
>>>>   	} else {
>>>>   		f_acm_cfg2 = f_acm;
>>>>   		f_ecm_cfg2 = f_ecm;
>>>>   		f_phonet_cfg2 = f_phonet;
>>>>   		f_obex1_cfg2 = f_obex1;
>>>>   		f_obex2_cfg2 = f_obex2;
>>>> +		f_msg_cfg2 = f_msg;
>>>>   	}
>>>>
>>>>   	return status;
>>>> +err_msg:
>>>> +	usb_remove_function(c, f_ecm);
>>>>   err_ecm:
>>>>   	usb_remove_function(c, f_acm);
>>>>   err_conf:
>>>> @@ -211,6 +261,8 @@ err_conf:
>>>>   		usb_remove_function(c, f_obex1);
>>>>   	if (!phonet_stat)
>>>>   		usb_remove_function(c, f_phonet);
>>>> +	usb_put_function(f_msg);
>>>> +err_get_msg:
>>>>   	usb_put_function(f_ecm);
>>>>   err_get_ecm:
>>>>   	usb_put_function(f_acm);
>>>> @@ -227,6 +279,8 @@ err_get_acm:
>>>>   static int __init nokia_bind(struct usb_composite_dev *cdev)
>>>>   {
>>>>   	struct usb_gadget	*gadget = cdev->gadget;
>>>> +	struct fsg_opts		*fsg_opts;
>>>> +	struct fsg_config	fsg_config;
>>>>   	int			status;
>>>>
>>>>   	status = usb_string_ids_tab(cdev, strings_dev);
>>>> @@ -267,11 +321,46 @@ static int __init nokia_bind(struct usb_composite_dev *cdev)
>>>>   		goto err_acm_inst;
>>>>   	}
>>>>
>>>> +	fi_msg = usb_get_function_instance("mass_storage");
>>>> +	if (IS_ERR(fi_msg)) {
>>>> +		status = PTR_ERR(fi_msg);
>>>> +		goto err_ecm_inst;
>>>> +	}
>>>> +
>>>> +	/* set up mass storage function */
>>>> +	fsg_config_from_params(&fsg_config, &fsg_mod_data, fsg_num_buffers);
>>>> +	fsg_config.vendor_name = "Nokia";
>>>> +	fsg_config.product_name = "N900";
>>>> +
>>>> +	fsg_opts = fsg_opts_from_func_inst(fi_msg);
>>>> +	fsg_opts->no_configfs = true;
>>>> +
>>>> +	status = fsg_common_set_num_buffers(fsg_opts->common, fsg_num_buffers);
>>>> +	if (status)
>>>> +		goto err_msg_inst;
>>>> +
>>>> +	status = fsg_common_set_nluns(fsg_opts->common, fsg_config.nluns);
>>>> +	if (status)
>>>> +		goto err_msg_buf;
>>>> +
>>>> +	status = fsg_common_set_cdev(fsg_opts->common, cdev, fsg_config.can_stall);
>>>> +	if (status)
>>>> +		goto err_msg_set_nluns;
>>>> +
>>>> +	fsg_common_set_sysfs(fsg_opts->common, true);
>>>> +
>>>> +	status = fsg_common_create_luns(fsg_opts->common, &fsg_config);
>>>> +	if (status)
>>>> +		goto err_msg_set_nluns;
>>>> +
>>>> +	fsg_common_set_inquiry_string(fsg_opts->common, fsg_config.vendor_name,
>>>> +				      fsg_config.product_name);
>>>> +
>>>>   	/* finally register the configuration */
>>>>   	status = usb_add_config(cdev, &nokia_config_500ma_driver,
>>>>   			nokia_bind_config);
>>>>   	if (status < 0)
>>>> -		goto err_ecm_inst;
>>>> +		goto err_msg_set_cdev;
>>>>
>>>>   	status = usb_add_config(cdev, &nokia_config_100ma_driver,
>>>>   			nokia_bind_config);
>>>> @@ -292,6 +381,14 @@ err_put_cfg1:
>>>>   	if (!IS_ERR_OR_NULL(f_phonet_cfg1))
>>>>   		usb_put_function(f_phonet_cfg1);
>>>>   	usb_put_function(f_ecm_cfg1);
>>>> +err_msg_set_cdev:
>>>> +	fsg_common_remove_luns(fsg_opts->common);
>>>> +err_msg_set_nluns:
>>>> +	fsg_common_free_luns(fsg_opts->common);
>>>> +err_msg_buf:
>>>> +	fsg_common_free_buffers(fsg_opts->common);
>>>> +err_msg_inst:
>>>> +	usb_put_function_instance(fi_msg);
>>>>   err_ecm_inst:
>>>>   	usb_put_function_instance(fi_ecm);
>>>>   err_acm_inst:
>>>> @@ -325,7 +422,10 @@ static int __exit nokia_unbind(struct usb_composite_dev *cdev)
>>>>   	usb_put_function(f_acm_cfg2);
>>>>   	usb_put_function(f_ecm_cfg1);
>>>>   	usb_put_function(f_ecm_cfg2);
>>>> +	usb_put_function(f_msg_cfg1);
>>>> +	usb_put_function(f_msg_cfg2);
>>>>
>>>> +	usb_put_function_instance(fi_msg);
>>>>   	usb_put_function_instance(fi_ecm);
>>>>   	if (!IS_ERR(fi_obex2))
>>>>   		usb_put_function_instance(fi_obex2);
>>>
>>> Opening discussion about this patch again as this is still only one
>>> solution how to use use mass storage without breaking other stuff...
>>>
>>> Please understand finally that usb networking is very very important for
>>> development on this device and usb mass storage is needed for
>>> transferring bigger data.
>>>
>>> Also to clarify potential regressions: This patch adds *optional* usb
>>> mass storage support and block device can be attached/detached to driver
>>> at runtime. It does *not* enforce to export some (mmc) device
>>> automatically. It is optional and userspace can decide...
>>>
>>> So MyDocs N900 partition is not automatically enforced to be exported
>>> via usb (as some people thought) and so it does not break usb networking
>>> or mass storage mode or MyDocs parition on N900 with Maemo system.
>>>
>>
>> Instead of adding mass storage to legacy gadget you may switch to
>> configfs interface and compose equivalent of g_nokia + mass storage
>> without any changes in kernel.
>>
>> Best regards,
>>
>
> That requires userspace. And there are developers who use nfsroot via
> usb networking and so userspace will be there after properly
> initializing usb gadget... So it is not easy and reason why I'm opening
> this discussion again.
>

Couldn't you simply use initramfs to compose your gadget?

-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

On Thursday 28 May 2015 16:51:07 Krzysztof Opasiak wrote:
> 
> 
> On 05/28/2015 04:31 PM, Pali Rohár wrote:
> >On Thursday 28 May 2015 16:27:44 Krzysztof Opasiak wrote:
> >>
> >>
> >>On 05/28/2015 09:47 AM, Pali Rohár wrote:
> >>>On Saturday 31 January 2015 10:53:30 Pali Rohár wrote:
> >>>>This patch adds removable mass storage support to g_nokia gadget (for N900).
> >>>>It means that at runtime block device can be exported or unexported.
> >>>>So it does not export anything by default and thus allows to use MyDocs
> >>>>partition as before...
> >>>>
> >>>>Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> >>>>---
> >>>>  drivers/usb/gadget/legacy/Kconfig |    1 +
> >>>>  drivers/usb/gadget/legacy/nokia.c |  102 ++++++++++++++++++++++++++++++++++++-
> >>>>  2 files changed, 102 insertions(+), 1 deletion(-)
> >>>>
> >>>>diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig
> >>>>index fd48ef3..36f6ba4 100644
> >>>>--- a/drivers/usb/gadget/legacy/Kconfig
> >>>>+++ b/drivers/usb/gadget/legacy/Kconfig
> >>>>@@ -345,6 +345,7 @@ config USB_G_NOKIA
> >>>>  	select USB_F_OBEX
> >>>>  	select USB_F_PHONET
> >>>>  	select USB_F_ECM
> >>>>+	select USB_F_MASS_STORAGE
> >>>>  	help
> >>>>  	  The Nokia composite gadget provides support for acm, obex
> >>>>  	  and phonet in only one composite gadget driver.
> >>>>diff --git a/drivers/usb/gadget/legacy/nokia.c b/drivers/usb/gadget/legacy/nokia.c
> >>>>index 9b8fd70..a09bb50 100644
> >>>>--- a/drivers/usb/gadget/legacy/nokia.c
> >>>>+++ b/drivers/usb/gadget/legacy/nokia.c
> >>>>@@ -24,6 +24,7 @@
> >>>>  #include "u_phonet.h"
> >>>>  #include "u_ecm.h"
> >>>>  #include "gadget_chips.h"
> >>>>+#include "f_mass_storage.h"
> >>>>
> >>>>  /* Defines */
> >>>>
> >>>>@@ -34,6 +35,29 @@ USB_GADGET_COMPOSITE_OPTIONS();
> >>>>
> >>>>  USB_ETHERNET_MODULE_PARAMETERS();
> >>>>
> >>>>+static struct fsg_module_parameters fsg_mod_data = {
> >>>>+	.stall = 0,
> >>>>+	.luns = 2,
> >>>>+	.removable_count = 2,
> >>>>+	.removable = { 1, 1, },
> >>>>+};
> >>>>+
> >>>>+FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
> >>>>+
> >>>>+#ifdef CONFIG_USB_GADGET_DEBUG_FILES
> >>>>+
> >>>>+static unsigned int fsg_num_buffers = CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS;
> >>>>+
> >>>>+#else
> >>>>+
> >>>>+/*
> >>>>+ * Number of buffers we will use.
> >>>>+ * 2 is usually enough for good buffering pipeline
> >>>>+ */
> >>>>+#define fsg_num_buffers	CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS
> >>>>+
> >>>>+#endif /* CONFIG_USB_DEBUG */
> >>>>+
> >>>>  #define NOKIA_VENDOR_ID			0x0421	/* Nokia */
> >>>>  #define NOKIA_PRODUCT_ID		0x01c8	/* Nokia Gadget */
> >>>>
> >>>>@@ -94,6 +118,8 @@ static struct usb_function *f_obex1_cfg2;
> >>>>  static struct usb_function *f_obex2_cfg2;
> >>>>  static struct usb_function *f_phonet_cfg1;
> >>>>  static struct usb_function *f_phonet_cfg2;
> >>>>+static struct usb_function *f_msg_cfg1;
> >>>>+static struct usb_function *f_msg_cfg2;
> >>>>
> >>>>
> >>>>  static struct usb_configuration nokia_config_500ma_driver = {
> >>>>@@ -117,6 +143,7 @@ static struct usb_function_instance *fi_ecm;
> >>>>  static struct usb_function_instance *fi_obex1;
> >>>>  static struct usb_function_instance *fi_obex2;
> >>>>  static struct usb_function_instance *fi_phonet;
> >>>>+static struct usb_function_instance *fi_msg;
> >>>>
> >>>>  static int __init nokia_bind_config(struct usb_configuration *c)
> >>>>  {
> >>>>@@ -125,6 +152,8 @@ static int __init nokia_bind_config(struct usb_configuration *c)
> >>>>  	struct usb_function *f_obex1 = NULL;
> >>>>  	struct usb_function *f_ecm;
> >>>>  	struct usb_function *f_obex2 = NULL;
> >>>>+	struct usb_function *f_msg;
> >>>>+	struct fsg_opts *fsg_opts;
> >>>>  	int status = 0;
> >>>>  	int obex1_stat = -1;
> >>>>  	int obex2_stat = -1;
> >>>>@@ -160,6 +189,12 @@ static int __init nokia_bind_config(struct usb_configuration *c)
> >>>>  		goto err_get_ecm;
> >>>>  	}
> >>>>
> >>>>+	f_msg = usb_get_function(fi_msg);
> >>>>+	if (IS_ERR(f_msg)) {
> >>>>+		status = PTR_ERR(f_msg);
> >>>>+		goto err_get_msg;
> >>>>+	}
> >>>>+
> >>>>  	if (!IS_ERR_OR_NULL(f_phonet)) {
> >>>>  		phonet_stat = usb_add_function(c, f_phonet);
> >>>>  		if (phonet_stat)
> >>>>@@ -187,21 +222,36 @@ static int __init nokia_bind_config(struct usb_configuration *c)
> >>>>  		pr_debug("could not bind ecm config %d\n", status);
> >>>>  		goto err_ecm;
> >>>>  	}
> >>>>+
> >>>>+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
> >>>>+
> >>>>+	status = fsg_common_run_thread(fsg_opts->common);
> >>>>+	if (status)
> >>>>+		goto err_msg;
> >>>>+
> >>>>+	status = usb_add_function(c, f_msg);
> >>>>+	if (status)
> >>>>+		goto err_msg;
> >>>>+
> >>>>  	if (c == &nokia_config_500ma_driver) {
> >>>>  		f_acm_cfg1 = f_acm;
> >>>>  		f_ecm_cfg1 = f_ecm;
> >>>>  		f_phonet_cfg1 = f_phonet;
> >>>>  		f_obex1_cfg1 = f_obex1;
> >>>>  		f_obex2_cfg1 = f_obex2;
> >>>>+		f_msg_cfg1 = f_msg;
> >>>>  	} else {
> >>>>  		f_acm_cfg2 = f_acm;
> >>>>  		f_ecm_cfg2 = f_ecm;
> >>>>  		f_phonet_cfg2 = f_phonet;
> >>>>  		f_obex1_cfg2 = f_obex1;
> >>>>  		f_obex2_cfg2 = f_obex2;
> >>>>+		f_msg_cfg2 = f_msg;
> >>>>  	}
> >>>>
> >>>>  	return status;
> >>>>+err_msg:
> >>>>+	usb_remove_function(c, f_ecm);
> >>>>  err_ecm:
> >>>>  	usb_remove_function(c, f_acm);
> >>>>  err_conf:
> >>>>@@ -211,6 +261,8 @@ err_conf:
> >>>>  		usb_remove_function(c, f_obex1);
> >>>>  	if (!phonet_stat)
> >>>>  		usb_remove_function(c, f_phonet);
> >>>>+	usb_put_function(f_msg);
> >>>>+err_get_msg:
> >>>>  	usb_put_function(f_ecm);
> >>>>  err_get_ecm:
> >>>>  	usb_put_function(f_acm);
> >>>>@@ -227,6 +279,8 @@ err_get_acm:
> >>>>  static int __init nokia_bind(struct usb_composite_dev *cdev)
> >>>>  {
> >>>>  	struct usb_gadget	*gadget = cdev->gadget;
> >>>>+	struct fsg_opts		*fsg_opts;
> >>>>+	struct fsg_config	fsg_config;
> >>>>  	int			status;
> >>>>
> >>>>  	status = usb_string_ids_tab(cdev, strings_dev);
> >>>>@@ -267,11 +321,46 @@ static int __init nokia_bind(struct usb_composite_dev *cdev)
> >>>>  		goto err_acm_inst;
> >>>>  	}
> >>>>
> >>>>+	fi_msg = usb_get_function_instance("mass_storage");
> >>>>+	if (IS_ERR(fi_msg)) {
> >>>>+		status = PTR_ERR(fi_msg);
> >>>>+		goto err_ecm_inst;
> >>>>+	}
> >>>>+
> >>>>+	/* set up mass storage function */
> >>>>+	fsg_config_from_params(&fsg_config, &fsg_mod_data, fsg_num_buffers);
> >>>>+	fsg_config.vendor_name = "Nokia";
> >>>>+	fsg_config.product_name = "N900";
> >>>>+
> >>>>+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
> >>>>+	fsg_opts->no_configfs = true;
> >>>>+
> >>>>+	status = fsg_common_set_num_buffers(fsg_opts->common, fsg_num_buffers);
> >>>>+	if (status)
> >>>>+		goto err_msg_inst;
> >>>>+
> >>>>+	status = fsg_common_set_nluns(fsg_opts->common, fsg_config.nluns);
> >>>>+	if (status)
> >>>>+		goto err_msg_buf;
> >>>>+
> >>>>+	status = fsg_common_set_cdev(fsg_opts->common, cdev, fsg_config.can_stall);
> >>>>+	if (status)
> >>>>+		goto err_msg_set_nluns;
> >>>>+
> >>>>+	fsg_common_set_sysfs(fsg_opts->common, true);
> >>>>+
> >>>>+	status = fsg_common_create_luns(fsg_opts->common, &fsg_config);
> >>>>+	if (status)
> >>>>+		goto err_msg_set_nluns;
> >>>>+
> >>>>+	fsg_common_set_inquiry_string(fsg_opts->common, fsg_config.vendor_name,
> >>>>+				      fsg_config.product_name);
> >>>>+
> >>>>  	/* finally register the configuration */
> >>>>  	status = usb_add_config(cdev, &nokia_config_500ma_driver,
> >>>>  			nokia_bind_config);
> >>>>  	if (status < 0)
> >>>>-		goto err_ecm_inst;
> >>>>+		goto err_msg_set_cdev;
> >>>>
> >>>>  	status = usb_add_config(cdev, &nokia_config_100ma_driver,
> >>>>  			nokia_bind_config);
> >>>>@@ -292,6 +381,14 @@ err_put_cfg1:
> >>>>  	if (!IS_ERR_OR_NULL(f_phonet_cfg1))
> >>>>  		usb_put_function(f_phonet_cfg1);
> >>>>  	usb_put_function(f_ecm_cfg1);
> >>>>+err_msg_set_cdev:
> >>>>+	fsg_common_remove_luns(fsg_opts->common);
> >>>>+err_msg_set_nluns:
> >>>>+	fsg_common_free_luns(fsg_opts->common);
> >>>>+err_msg_buf:
> >>>>+	fsg_common_free_buffers(fsg_opts->common);
> >>>>+err_msg_inst:
> >>>>+	usb_put_function_instance(fi_msg);
> >>>>  err_ecm_inst:
> >>>>  	usb_put_function_instance(fi_ecm);
> >>>>  err_acm_inst:
> >>>>@@ -325,7 +422,10 @@ static int __exit nokia_unbind(struct usb_composite_dev *cdev)
> >>>>  	usb_put_function(f_acm_cfg2);
> >>>>  	usb_put_function(f_ecm_cfg1);
> >>>>  	usb_put_function(f_ecm_cfg2);
> >>>>+	usb_put_function(f_msg_cfg1);
> >>>>+	usb_put_function(f_msg_cfg2);
> >>>>
> >>>>+	usb_put_function_instance(fi_msg);
> >>>>  	usb_put_function_instance(fi_ecm);
> >>>>  	if (!IS_ERR(fi_obex2))
> >>>>  		usb_put_function_instance(fi_obex2);
> >>>
> >>>Opening discussion about this patch again as this is still only one
> >>>solution how to use use mass storage without breaking other stuff...
> >>>
> >>>Please understand finally that usb networking is very very important for
> >>>development on this device and usb mass storage is needed for
> >>>transferring bigger data.
> >>>
> >>>Also to clarify potential regressions: This patch adds *optional* usb
> >>>mass storage support and block device can be attached/detached to driver
> >>>at runtime. It does *not* enforce to export some (mmc) device
> >>>automatically. It is optional and userspace can decide...
> >>>
> >>>So MyDocs N900 partition is not automatically enforced to be exported
> >>>via usb (as some people thought) and so it does not break usb networking
> >>>or mass storage mode or MyDocs parition on N900 with Maemo system.
> >>>
> >>
> >>Instead of adding mass storage to legacy gadget you may switch to
> >>configfs interface and compose equivalent of g_nokia + mass storage
> >>without any changes in kernel.
> >>
> >>Best regards,
> >>
> >
> >That requires userspace. And there are developers who use nfsroot via
> >usb networking and so userspace will be there after properly
> >initializing usb gadget... So it is not easy and reason why I'm opening
> >this discussion again.
> >
> 
> Couldn't you simply use initramfs to compose your gadget?
> 

This is another next complication... I could spend time on hacking
serious problems on Nokia N900 (camera not working, bluetooth not
working, improving cellular audio/voice calls) or start playing with
initramfs and other kernel infrastructure just because usb is broken and
small fix which enable all needed usb functionality (usb networking, usb
TTY, usb phonet, usb mass storage) cannot be accepted and merged because
it really helps people to develop... :-( I see that it is under legacy
drivers, but without it is hard (and maybe not possible) to develop for
Nokia N900 device. Nobody until now proposed any other patch which fix
our problems and provide working usb functionality for mass storage,
network, TTY and phonet.

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Felipe Balbi]

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

On Thu, May 28, 2015 at 04:59:18PM +0200, Pali Rohár wrote:
> On Thursday 28 May 2015 16:51:07 Krzysztof Opasiak wrote:
> > 
> > 
> > On 05/28/2015 04:31 PM, Pali Rohár wrote:
> > >On Thursday 28 May 2015 16:27:44 Krzysztof Opasiak wrote:
> > >>
> > >>
> > >>On 05/28/2015 09:47 AM, Pali Rohár wrote:
> > >>>On Saturday 31 January 2015 10:53:30 Pali Rohár wrote:
> > >>>>This patch adds removable mass storage support to g_nokia gadget (for N900).
> > >>>>It means that at runtime block device can be exported or unexported.
> > >>>>So it does not export anything by default and thus allows to use MyDocs
> > >>>>partition as before...
> > >>>>
> > >>>>Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > >>>>---
> > >>>>  drivers/usb/gadget/legacy/Kconfig |    1 +
> > >>>>  drivers/usb/gadget/legacy/nokia.c |  102 ++++++++++++++++++++++++++++++++++++-
> > >>>>  2 files changed, 102 insertions(+), 1 deletion(-)
> > >>>>
> > >>>>diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig
> > >>>>index fd48ef3..36f6ba4 100644
> > >>>>--- a/drivers/usb/gadget/legacy/Kconfig
> > >>>>+++ b/drivers/usb/gadget/legacy/Kconfig
> > >>>>@@ -345,6 +345,7 @@ config USB_G_NOKIA
> > >>>>  	select USB_F_OBEX
> > >>>>  	select USB_F_PHONET
> > >>>>  	select USB_F_ECM
> > >>>>+	select USB_F_MASS_STORAGE
> > >>>>  	help
> > >>>>  	  The Nokia composite gadget provides support for acm, obex
> > >>>>  	  and phonet in only one composite gadget driver.
> > >>>>diff --git a/drivers/usb/gadget/legacy/nokia.c b/drivers/usb/gadget/legacy/nokia.c
> > >>>>index 9b8fd70..a09bb50 100644
> > >>>>--- a/drivers/usb/gadget/legacy/nokia.c
> > >>>>+++ b/drivers/usb/gadget/legacy/nokia.c
> > >>>>@@ -24,6 +24,7 @@
> > >>>>  #include "u_phonet.h"
> > >>>>  #include "u_ecm.h"
> > >>>>  #include "gadget_chips.h"
> > >>>>+#include "f_mass_storage.h"
> > >>>>
> > >>>>  /* Defines */
> > >>>>
> > >>>>@@ -34,6 +35,29 @@ USB_GADGET_COMPOSITE_OPTIONS();
> > >>>>
> > >>>>  USB_ETHERNET_MODULE_PARAMETERS();
> > >>>>
> > >>>>+static struct fsg_module_parameters fsg_mod_data = {
> > >>>>+	.stall = 0,
> > >>>>+	.luns = 2,
> > >>>>+	.removable_count = 2,
> > >>>>+	.removable = { 1, 1, },
> > >>>>+};
> > >>>>+
> > >>>>+FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
> > >>>>+
> > >>>>+#ifdef CONFIG_USB_GADGET_DEBUG_FILES
> > >>>>+
> > >>>>+static unsigned int fsg_num_buffers = CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS;
> > >>>>+
> > >>>>+#else
> > >>>>+
> > >>>>+/*
> > >>>>+ * Number of buffers we will use.
> > >>>>+ * 2 is usually enough for good buffering pipeline
> > >>>>+ */
> > >>>>+#define fsg_num_buffers	CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS
> > >>>>+
> > >>>>+#endif /* CONFIG_USB_DEBUG */
> > >>>>+
> > >>>>  #define NOKIA_VENDOR_ID			0x0421	/* Nokia */
> > >>>>  #define NOKIA_PRODUCT_ID		0x01c8	/* Nokia Gadget */
> > >>>>
> > >>>>@@ -94,6 +118,8 @@ static struct usb_function *f_obex1_cfg2;
> > >>>>  static struct usb_function *f_obex2_cfg2;
> > >>>>  static struct usb_function *f_phonet_cfg1;
> > >>>>  static struct usb_function *f_phonet_cfg2;
> > >>>>+static struct usb_function *f_msg_cfg1;
> > >>>>+static struct usb_function *f_msg_cfg2;
> > >>>>
> > >>>>
> > >>>>  static struct usb_configuration nokia_config_500ma_driver = {
> > >>>>@@ -117,6 +143,7 @@ static struct usb_function_instance *fi_ecm;
> > >>>>  static struct usb_function_instance *fi_obex1;
> > >>>>  static struct usb_function_instance *fi_obex2;
> > >>>>  static struct usb_function_instance *fi_phonet;
> > >>>>+static struct usb_function_instance *fi_msg;
> > >>>>
> > >>>>  static int __init nokia_bind_config(struct usb_configuration *c)
> > >>>>  {
> > >>>>@@ -125,6 +152,8 @@ static int __init nokia_bind_config(struct usb_configuration *c)
> > >>>>  	struct usb_function *f_obex1 = NULL;
> > >>>>  	struct usb_function *f_ecm;
> > >>>>  	struct usb_function *f_obex2 = NULL;
> > >>>>+	struct usb_function *f_msg;
> > >>>>+	struct fsg_opts *fsg_opts;
> > >>>>  	int status = 0;
> > >>>>  	int obex1_stat = -1;
> > >>>>  	int obex2_stat = -1;
> > >>>>@@ -160,6 +189,12 @@ static int __init nokia_bind_config(struct usb_configuration *c)
> > >>>>  		goto err_get_ecm;
> > >>>>  	}
> > >>>>
> > >>>>+	f_msg = usb_get_function(fi_msg);
> > >>>>+	if (IS_ERR(f_msg)) {
> > >>>>+		status = PTR_ERR(f_msg);
> > >>>>+		goto err_get_msg;
> > >>>>+	}
> > >>>>+
> > >>>>  	if (!IS_ERR_OR_NULL(f_phonet)) {
> > >>>>  		phonet_stat = usb_add_function(c, f_phonet);
> > >>>>  		if (phonet_stat)
> > >>>>@@ -187,21 +222,36 @@ static int __init nokia_bind_config(struct usb_configuration *c)
> > >>>>  		pr_debug("could not bind ecm config %d\n", status);
> > >>>>  		goto err_ecm;
> > >>>>  	}
> > >>>>+
> > >>>>+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
> > >>>>+
> > >>>>+	status = fsg_common_run_thread(fsg_opts->common);
> > >>>>+	if (status)
> > >>>>+		goto err_msg;
> > >>>>+
> > >>>>+	status = usb_add_function(c, f_msg);
> > >>>>+	if (status)
> > >>>>+		goto err_msg;
> > >>>>+
> > >>>>  	if (c == &nokia_config_500ma_driver) {
> > >>>>  		f_acm_cfg1 = f_acm;
> > >>>>  		f_ecm_cfg1 = f_ecm;
> > >>>>  		f_phonet_cfg1 = f_phonet;
> > >>>>  		f_obex1_cfg1 = f_obex1;
> > >>>>  		f_obex2_cfg1 = f_obex2;
> > >>>>+		f_msg_cfg1 = f_msg;
> > >>>>  	} else {
> > >>>>  		f_acm_cfg2 = f_acm;
> > >>>>  		f_ecm_cfg2 = f_ecm;
> > >>>>  		f_phonet_cfg2 = f_phonet;
> > >>>>  		f_obex1_cfg2 = f_obex1;
> > >>>>  		f_obex2_cfg2 = f_obex2;
> > >>>>+		f_msg_cfg2 = f_msg;
> > >>>>  	}
> > >>>>
> > >>>>  	return status;
> > >>>>+err_msg:
> > >>>>+	usb_remove_function(c, f_ecm);
> > >>>>  err_ecm:
> > >>>>  	usb_remove_function(c, f_acm);
> > >>>>  err_conf:
> > >>>>@@ -211,6 +261,8 @@ err_conf:
> > >>>>  		usb_remove_function(c, f_obex1);
> > >>>>  	if (!phonet_stat)
> > >>>>  		usb_remove_function(c, f_phonet);
> > >>>>+	usb_put_function(f_msg);
> > >>>>+err_get_msg:
> > >>>>  	usb_put_function(f_ecm);
> > >>>>  err_get_ecm:
> > >>>>  	usb_put_function(f_acm);
> > >>>>@@ -227,6 +279,8 @@ err_get_acm:
> > >>>>  static int __init nokia_bind(struct usb_composite_dev *cdev)
> > >>>>  {
> > >>>>  	struct usb_gadget	*gadget = cdev->gadget;
> > >>>>+	struct fsg_opts		*fsg_opts;
> > >>>>+	struct fsg_config	fsg_config;
> > >>>>  	int			status;
> > >>>>
> > >>>>  	status = usb_string_ids_tab(cdev, strings_dev);
> > >>>>@@ -267,11 +321,46 @@ static int __init nokia_bind(struct usb_composite_dev *cdev)
> > >>>>  		goto err_acm_inst;
> > >>>>  	}
> > >>>>
> > >>>>+	fi_msg = usb_get_function_instance("mass_storage");
> > >>>>+	if (IS_ERR(fi_msg)) {
> > >>>>+		status = PTR_ERR(fi_msg);
> > >>>>+		goto err_ecm_inst;
> > >>>>+	}
> > >>>>+
> > >>>>+	/* set up mass storage function */
> > >>>>+	fsg_config_from_params(&fsg_config, &fsg_mod_data, fsg_num_buffers);
> > >>>>+	fsg_config.vendor_name = "Nokia";
> > >>>>+	fsg_config.product_name = "N900";
> > >>>>+
> > >>>>+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
> > >>>>+	fsg_opts->no_configfs = true;
> > >>>>+
> > >>>>+	status = fsg_common_set_num_buffers(fsg_opts->common, fsg_num_buffers);
> > >>>>+	if (status)
> > >>>>+		goto err_msg_inst;
> > >>>>+
> > >>>>+	status = fsg_common_set_nluns(fsg_opts->common, fsg_config.nluns);
> > >>>>+	if (status)
> > >>>>+		goto err_msg_buf;
> > >>>>+
> > >>>>+	status = fsg_common_set_cdev(fsg_opts->common, cdev, fsg_config.can_stall);
> > >>>>+	if (status)
> > >>>>+		goto err_msg_set_nluns;
> > >>>>+
> > >>>>+	fsg_common_set_sysfs(fsg_opts->common, true);
> > >>>>+
> > >>>>+	status = fsg_common_create_luns(fsg_opts->common, &fsg_config);
> > >>>>+	if (status)
> > >>>>+		goto err_msg_set_nluns;
> > >>>>+
> > >>>>+	fsg_common_set_inquiry_string(fsg_opts->common, fsg_config.vendor_name,
> > >>>>+				      fsg_config.product_name);
> > >>>>+
> > >>>>  	/* finally register the configuration */
> > >>>>  	status = usb_add_config(cdev, &nokia_config_500ma_driver,
> > >>>>  			nokia_bind_config);
> > >>>>  	if (status < 0)
> > >>>>-		goto err_ecm_inst;
> > >>>>+		goto err_msg_set_cdev;
> > >>>>
> > >>>>  	status = usb_add_config(cdev, &nokia_config_100ma_driver,
> > >>>>  			nokia_bind_config);
> > >>>>@@ -292,6 +381,14 @@ err_put_cfg1:
> > >>>>  	if (!IS_ERR_OR_NULL(f_phonet_cfg1))
> > >>>>  		usb_put_function(f_phonet_cfg1);
> > >>>>  	usb_put_function(f_ecm_cfg1);
> > >>>>+err_msg_set_cdev:
> > >>>>+	fsg_common_remove_luns(fsg_opts->common);
> > >>>>+err_msg_set_nluns:
> > >>>>+	fsg_common_free_luns(fsg_opts->common);
> > >>>>+err_msg_buf:
> > >>>>+	fsg_common_free_buffers(fsg_opts->common);
> > >>>>+err_msg_inst:
> > >>>>+	usb_put_function_instance(fi_msg);
> > >>>>  err_ecm_inst:
> > >>>>  	usb_put_function_instance(fi_ecm);
> > >>>>  err_acm_inst:
> > >>>>@@ -325,7 +422,10 @@ static int __exit nokia_unbind(struct usb_composite_dev *cdev)
> > >>>>  	usb_put_function(f_acm_cfg2);
> > >>>>  	usb_put_function(f_ecm_cfg1);
> > >>>>  	usb_put_function(f_ecm_cfg2);
> > >>>>+	usb_put_function(f_msg_cfg1);
> > >>>>+	usb_put_function(f_msg_cfg2);
> > >>>>
> > >>>>+	usb_put_function_instance(fi_msg);
> > >>>>  	usb_put_function_instance(fi_ecm);
> > >>>>  	if (!IS_ERR(fi_obex2))
> > >>>>  		usb_put_function_instance(fi_obex2);
> > >>>
> > >>>Opening discussion about this patch again as this is still only one
> > >>>solution how to use use mass storage without breaking other stuff...
> > >>>
> > >>>Please understand finally that usb networking is very very important for
> > >>>development on this device and usb mass storage is needed for
> > >>>transferring bigger data.
> > >>>
> > >>>Also to clarify potential regressions: This patch adds *optional* usb
> > >>>mass storage support and block device can be attached/detached to driver
> > >>>at runtime. It does *not* enforce to export some (mmc) device
> > >>>automatically. It is optional and userspace can decide...
> > >>>
> > >>>So MyDocs N900 partition is not automatically enforced to be exported
> > >>>via usb (as some people thought) and so it does not break usb networking
> > >>>or mass storage mode or MyDocs parition on N900 with Maemo system.
> > >>>
> > >>
> > >>Instead of adding mass storage to legacy gadget you may switch to
> > >>configfs interface and compose equivalent of g_nokia + mass storage
> > >>without any changes in kernel.
> > >>
> > >>Best regards,
> > >>
> > >
> > >That requires userspace. And there are developers who use nfsroot via
> > >usb networking and so userspace will be there after properly
> > >initializing usb gadget... So it is not easy and reason why I'm opening
> > >this discussion again.
> > >
> > 
> > Couldn't you simply use initramfs to compose your gadget?
> > 
> 
> This is another next complication... I could spend time on hacking
> serious problems on Nokia N900 (camera not working, bluetooth not
> working, improving cellular audio/voice calls) or start playing with
> initramfs and other kernel infrastructure just because usb is broken and
> small fix which enable all needed usb functionality (usb networking, usb
> TTY, usb phonet, usb mass storage) cannot be accepted and merged because
> it really helps people to develop... :-( I see that it is under legacy
> drivers, but without it is hard (and maybe not possible) to develop for
> Nokia N900 device. Nobody until now proposed any other patch which fix
> our problems and provide working usb functionality for mass storage,
> network, TTY and phonet.

Sure this causes no problems to original Maemo userland ? Also, IIRC, we
were running out of both FIFO space and physical endpoints with g_nokia
already. Do all functions still work if used all at once ?

If you can confirm these two questions I'll take this patch. As long as
there are no regressions with original userland from Nokia, it should be
fine.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 14168 bytes --]

On Thursday 28 May 2015 18:34:31 Felipe Balbi wrote:
> On Thu, May 28, 2015 at 04:59:18PM +0200, Pali Rohár wrote:
> > On Thursday 28 May 2015 16:51:07 Krzysztof Opasiak wrote:
> > > On 05/28/2015 04:31 PM, Pali Rohár wrote:
> > > >On Thursday 28 May 2015 16:27:44 Krzysztof Opasiak wrote:
> > > >>On 05/28/2015 09:47 AM, Pali Rohár wrote:
> > > >>>On Saturday 31 January 2015 10:53:30 Pali Rohár wrote:
> > > >>>>This patch adds removable mass storage support to g_nokia
> > > >>>>gadget (for N900). It means that at runtime block device can
> > > >>>>be exported or unexported. So it does not export anything by
> > > >>>>default and thus allows to use MyDocs partition as before...
> > > >>>>
> > > >>>>Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > >>>>---
> > > >>>>
> > > >>>>  drivers/usb/gadget/legacy/Kconfig |    1 +
> > > >>>>  drivers/usb/gadget/legacy/nokia.c |  102
> > > >>>>  ++++++++++++++++++++++++++++++++++++- 2 files changed, 102
> > > >>>>  insertions(+), 1 deletion(-)
> > > >>>>
> > > >>>>diff --git a/drivers/usb/gadget/legacy/Kconfig
> > > >>>>b/drivers/usb/gadget/legacy/Kconfig index fd48ef3..36f6ba4
> > > >>>>100644
> > > >>>>--- a/drivers/usb/gadget/legacy/Kconfig
> > > >>>>+++ b/drivers/usb/gadget/legacy/Kconfig
> > > >>>>@@ -345,6 +345,7 @@ config USB_G_NOKIA
> > > >>>>
> > > >>>>  	select USB_F_OBEX
> > > >>>>  	select USB_F_PHONET
> > > >>>>  	select USB_F_ECM
> > > >>>>
> > > >>>>+	select USB_F_MASS_STORAGE
> > > >>>>
> > > >>>>  	help
> > > >>>>  	
> > > >>>>  	  The Nokia composite gadget provides support for acm,
> > > >>>>  	  obex and phonet in only one composite gadget driver.
> > > >>>>
> > > >>>>diff --git a/drivers/usb/gadget/legacy/nokia.c
> > > >>>>b/drivers/usb/gadget/legacy/nokia.c index 9b8fd70..a09bb50
> > > >>>>100644
> > > >>>>--- a/drivers/usb/gadget/legacy/nokia.c
> > > >>>>+++ b/drivers/usb/gadget/legacy/nokia.c
> > > >>>>@@ -24,6 +24,7 @@
> > > >>>>
> > > >>>>  #include "u_phonet.h"
> > > >>>>  #include "u_ecm.h"
> > > >>>>  #include "gadget_chips.h"
> > > >>>>
> > > >>>>+#include "f_mass_storage.h"
> > > >>>>
> > > >>>>  /* Defines */
> > > >>>>
> > > >>>>@@ -34,6 +35,29 @@ USB_GADGET_COMPOSITE_OPTIONS();
> > > >>>>
> > > >>>>  USB_ETHERNET_MODULE_PARAMETERS();
> > > >>>>
> > > >>>>+static struct fsg_module_parameters fsg_mod_data = {
> > > >>>>+	.stall = 0,
> > > >>>>+	.luns = 2,
> > > >>>>+	.removable_count = 2,
> > > >>>>+	.removable = { 1, 1, },
> > > >>>>+};
> > > >>>>+
> > > >>>>+FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
> > > >>>>+
> > > >>>>+#ifdef CONFIG_USB_GADGET_DEBUG_FILES
> > > >>>>+
> > > >>>>+static unsigned int fsg_num_buffers =
> > > >>>>CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS; +
> > > >>>>+#else
> > > >>>>+
> > > >>>>+/*
> > > >>>>+ * Number of buffers we will use.
> > > >>>>+ * 2 is usually enough for good buffering pipeline
> > > >>>>+ */
> > > >>>>+#define
> > > >>>>fsg_num_buffers	CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS +
> > > >>>>+#endif /* CONFIG_USB_DEBUG */
> > > >>>>+
> > > >>>>
> > > >>>>  #define NOKIA_VENDOR_ID			0x0421	/* Nokia */
> > > >>>>  #define NOKIA_PRODUCT_ID		0x01c8	/* Nokia Gadget */
> > > >>>>
> > > >>>>@@ -94,6 +118,8 @@ static struct usb_function *f_obex1_cfg2;
> > > >>>>
> > > >>>>  static struct usb_function *f_obex2_cfg2;
> > > >>>>  static struct usb_function *f_phonet_cfg1;
> > > >>>>  static struct usb_function *f_phonet_cfg2;
> > > >>>>
> > > >>>>+static struct usb_function *f_msg_cfg1;
> > > >>>>+static struct usb_function *f_msg_cfg2;
> > > >>>>
> > > >>>>  static struct usb_configuration nokia_config_500ma_driver =
> > > >>>>  {
> > > >>>>
> > > >>>>@@ -117,6 +143,7 @@ static struct usb_function_instance
> > > >>>>*fi_ecm;
> > > >>>>
> > > >>>>  static struct usb_function_instance *fi_obex1;
> > > >>>>  static struct usb_function_instance *fi_obex2;
> > > >>>>  static struct usb_function_instance *fi_phonet;
> > > >>>>
> > > >>>>+static struct usb_function_instance *fi_msg;
> > > >>>>
> > > >>>>  static int __init nokia_bind_config(struct
> > > >>>>  usb_configuration *c) {
> > > >>>>
> > > >>>>@@ -125,6 +152,8 @@ static int __init
> > > >>>>nokia_bind_config(struct usb_configuration *c)
> > > >>>>
> > > >>>>  	struct usb_function *f_obex1 = NULL;
> > > >>>>  	struct usb_function *f_ecm;
> > > >>>>  	struct usb_function *f_obex2 = NULL;
> > > >>>>
> > > >>>>+	struct usb_function *f_msg;
> > > >>>>+	struct fsg_opts *fsg_opts;
> > > >>>>
> > > >>>>  	int status = 0;
> > > >>>>  	int obex1_stat = -1;
> > > >>>>  	int obex2_stat = -1;
> > > >>>>
> > > >>>>@@ -160,6 +189,12 @@ static int __init
> > > >>>>nokia_bind_config(struct usb_configuration *c)
> > > >>>>
> > > >>>>  		goto err_get_ecm;
> > > >>>>  	
> > > >>>>  	}
> > > >>>>
> > > >>>>+	f_msg = usb_get_function(fi_msg);
> > > >>>>+	if (IS_ERR(f_msg)) {
> > > >>>>+		status = PTR_ERR(f_msg);
> > > >>>>+		goto err_get_msg;
> > > >>>>+	}
> > > >>>>+
> > > >>>>
> > > >>>>  	if (!IS_ERR_OR_NULL(f_phonet)) {
> > > >>>>  	
> > > >>>>  		phonet_stat = usb_add_function(c, f_phonet);
> > > >>>>  		if (phonet_stat)
> > > >>>>
> > > >>>>@@ -187,21 +222,36 @@ static int __init
> > > >>>>nokia_bind_config(struct usb_configuration *c)
> > > >>>>
> > > >>>>  		pr_debug("could not bind ecm config %d\n", status);
> > > >>>>  		goto err_ecm;
> > > >>>>  	
> > > >>>>  	}
> > > >>>>
> > > >>>>+
> > > >>>>+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
> > > >>>>+
> > > >>>>+	status = fsg_common_run_thread(fsg_opts->common);
> > > >>>>+	if (status)
> > > >>>>+		goto err_msg;
> > > >>>>+
> > > >>>>+	status = usb_add_function(c, f_msg);
> > > >>>>+	if (status)
> > > >>>>+		goto err_msg;
> > > >>>>+
> > > >>>>
> > > >>>>  	if (c == &nokia_config_500ma_driver) {
> > > >>>>  	
> > > >>>>  		f_acm_cfg1 = f_acm;
> > > >>>>  		f_ecm_cfg1 = f_ecm;
> > > >>>>  		f_phonet_cfg1 = f_phonet;
> > > >>>>  		f_obex1_cfg1 = f_obex1;
> > > >>>>  		f_obex2_cfg1 = f_obex2;
> > > >>>>
> > > >>>>+		f_msg_cfg1 = f_msg;
> > > >>>>
> > > >>>>  	} else {
> > > >>>>  	
> > > >>>>  		f_acm_cfg2 = f_acm;
> > > >>>>  		f_ecm_cfg2 = f_ecm;
> > > >>>>  		f_phonet_cfg2 = f_phonet;
> > > >>>>  		f_obex1_cfg2 = f_obex1;
> > > >>>>  		f_obex2_cfg2 = f_obex2;
> > > >>>>
> > > >>>>+		f_msg_cfg2 = f_msg;
> > > >>>>
> > > >>>>  	}
> > > >>>>  	
> > > >>>>  	return status;
> > > >>>>
> > > >>>>+err_msg:
> > > >>>>+	usb_remove_function(c, f_ecm);
> > > >>>>
> > > >>>>  err_ecm:
> > > >>>>  	usb_remove_function(c, f_acm);
> > > >>>>  
> > > >>>>  err_conf:
> > > >>>>@@ -211,6 +261,8 @@ err_conf:
> > > >>>>  		usb_remove_function(c, f_obex1);
> > > >>>>  	
> > > >>>>  	if (!phonet_stat)
> > > >>>>  	
> > > >>>>  		usb_remove_function(c, f_phonet);
> > > >>>>
> > > >>>>+	usb_put_function(f_msg);
> > > >>>>
> > > >>>>+err_get_msg:
> > > >>>>  	usb_put_function(f_ecm);
> > > >>>>  
> > > >>>>  err_get_ecm:
> > > >>>>  	usb_put_function(f_acm);
> > > >>>>
> > > >>>>@@ -227,6 +279,8 @@ err_get_acm:
> > > >>>>  static int __init nokia_bind(struct usb_composite_dev
> > > >>>>  *cdev) {
> > > >>>>  
> > > >>>>  	struct usb_gadget	*gadget = cdev->gadget;
> > > >>>>
> > > >>>>+	struct fsg_opts		*fsg_opts;
> > > >>>>+	struct fsg_config	fsg_config;
> > > >>>>
> > > >>>>  	int			status;
> > > >>>>  	
> > > >>>>  	status = usb_string_ids_tab(cdev, strings_dev);
> > > >>>>
> > > >>>>@@ -267,11 +321,46 @@ static int __init nokia_bind(struct
> > > >>>>usb_composite_dev *cdev)
> > > >>>>
> > > >>>>  		goto err_acm_inst;
> > > >>>>  	
> > > >>>>  	}
> > > >>>>
> > > >>>>+	fi_msg = usb_get_function_instance("mass_storage");
> > > >>>>+	if (IS_ERR(fi_msg)) {
> > > >>>>+		status = PTR_ERR(fi_msg);
> > > >>>>+		goto err_ecm_inst;
> > > >>>>+	}
> > > >>>>+
> > > >>>>+	/* set up mass storage function */
> > > >>>>+	fsg_config_from_params(&fsg_config, &fsg_mod_data,
> > > >>>>fsg_num_buffers); +	fsg_config.vendor_name = "Nokia";
> > > >>>>+	fsg_config.product_name = "N900";
> > > >>>>+
> > > >>>>+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
> > > >>>>+	fsg_opts->no_configfs = true;
> > > >>>>+
> > > >>>>+	status = fsg_common_set_num_buffers(fsg_opts->common,
> > > >>>>fsg_num_buffers); +	if (status)
> > > >>>>+		goto err_msg_inst;
> > > >>>>+
> > > >>>>+	status = fsg_common_set_nluns(fsg_opts->common,
> > > >>>>fsg_config.nluns); +	if (status)
> > > >>>>+		goto err_msg_buf;
> > > >>>>+
> > > >>>>+	status = fsg_common_set_cdev(fsg_opts->common, cdev,
> > > >>>>fsg_config.can_stall); +	if (status)
> > > >>>>+		goto err_msg_set_nluns;
> > > >>>>+
> > > >>>>+	fsg_common_set_sysfs(fsg_opts->common, true);
> > > >>>>+
> > > >>>>+	status = fsg_common_create_luns(fsg_opts->common,
> > > >>>>&fsg_config); +	if (status)
> > > >>>>+		goto err_msg_set_nluns;
> > > >>>>+
> > > >>>>+	fsg_common_set_inquiry_string(fsg_opts->common,
> > > >>>>fsg_config.vendor_name, +				      
fsg_config.product_name);
> > > >>>>+
> > > >>>>
> > > >>>>  	/* finally register the configuration */
> > > >>>>  	status = usb_add_config(cdev, &nokia_config_500ma_driver,
> > > >>>>  	
> > > >>>>  			nokia_bind_config);
> > > >>>>  	
> > > >>>>  	if (status < 0)
> > > >>>>
> > > >>>>-		goto err_ecm_inst;
> > > >>>>+		goto err_msg_set_cdev;
> > > >>>>
> > > >>>>  	status = usb_add_config(cdev, &nokia_config_100ma_driver,
> > > >>>>  	
> > > >>>>  			nokia_bind_config);
> > > >>>>
> > > >>>>@@ -292,6 +381,14 @@ err_put_cfg1:
> > > >>>>  	if (!IS_ERR_OR_NULL(f_phonet_cfg1))
> > > >>>>  	
> > > >>>>  		usb_put_function(f_phonet_cfg1);
> > > >>>>  	
> > > >>>>  	usb_put_function(f_ecm_cfg1);
> > > >>>>
> > > >>>>+err_msg_set_cdev:
> > > >>>>+	fsg_common_remove_luns(fsg_opts->common);
> > > >>>>+err_msg_set_nluns:
> > > >>>>+	fsg_common_free_luns(fsg_opts->common);
> > > >>>>+err_msg_buf:
> > > >>>>+	fsg_common_free_buffers(fsg_opts->common);
> > > >>>>+err_msg_inst:
> > > >>>>+	usb_put_function_instance(fi_msg);
> > > >>>>
> > > >>>>  err_ecm_inst:
> > > >>>>  	usb_put_function_instance(fi_ecm);
> > > >>>>  
> > > >>>>  err_acm_inst:
> > > >>>>@@ -325,7 +422,10 @@ static int __exit nokia_unbind(struct
> > > >>>>usb_composite_dev *cdev)
> > > >>>>
> > > >>>>  	usb_put_function(f_acm_cfg2);
> > > >>>>  	usb_put_function(f_ecm_cfg1);
> > > >>>>  	usb_put_function(f_ecm_cfg2);
> > > >>>>
> > > >>>>+	usb_put_function(f_msg_cfg1);
> > > >>>>+	usb_put_function(f_msg_cfg2);
> > > >>>>
> > > >>>>+	usb_put_function_instance(fi_msg);
> > > >>>>
> > > >>>>  	usb_put_function_instance(fi_ecm);
> > > >>>>  	if (!IS_ERR(fi_obex2))
> > > >>>>  	
> > > >>>>  		usb_put_function_instance(fi_obex2);
> > > >>>
> > > >>>Opening discussion about this patch again as this is still
> > > >>>only one solution how to use use mass storage without
> > > >>>breaking other stuff...
> > > >>>
> > > >>>Please understand finally that usb networking is very very
> > > >>>important for development on this device and usb mass storage
> > > >>>is needed for transferring bigger data.
> > > >>>
> > > >>>Also to clarify potential regressions: This patch adds
> > > >>>*optional* usb mass storage support and block device can be
> > > >>>attached/detached to driver at runtime. It does *not* enforce
> > > >>>to export some (mmc) device automatically. It is optional and
> > > >>>userspace can decide...
> > > >>>
> > > >>>So MyDocs N900 partition is not automatically enforced to be
> > > >>>exported via usb (as some people thought) and so it does not
> > > >>>break usb networking or mass storage mode or MyDocs parition
> > > >>>on N900 with Maemo system.
> > > >>
> > > >>Instead of adding mass storage to legacy gadget you may switch
> > > >>to configfs interface and compose equivalent of g_nokia + mass
> > > >>storage without any changes in kernel.
> > > >>
> > > >>Best regards,
> > > >
> > > >That requires userspace. And there are developers who use
> > > >nfsroot via usb networking and so userspace will be there after
> > > >properly initializing usb gadget... So it is not easy and
> > > >reason why I'm opening this discussion again.
> > > 
> > > Couldn't you simply use initramfs to compose your gadget?
> > 
> > This is another next complication... I could spend time on hacking
> > serious problems on Nokia N900 (camera not working, bluetooth not
> > working, improving cellular audio/voice calls) or start playing
> > with initramfs and other kernel infrastructure just because usb is
> > broken and small fix which enable all needed usb functionality
> > (usb networking, usb TTY, usb phonet, usb mass storage) cannot be
> > accepted and merged because it really helps people to develop...
> > :-( I see that it is under legacy drivers, but without it is hard
> > (and maybe not possible) to develop for Nokia N900 device. Nobody
> > until now proposed any other patch which fix our problems and
> > provide working usb functionality for mass storage, network, TTY
> > and phonet.
> 
> Sure this causes no problems to original Maemo userland ? Also, IIRC,
> we were running out of both FIFO space and physical endpoints with
> g_nokia already. Do all functions still work if used all at once ?
> 
> If you can confirm these two questions I'll take this patch. As long
> as there are no regressions with original userland from Nokia, it
> should be fine.
> 
> cheers

In this my patch usb gadget support is removable (see fsg_mod_data). It 
means that at runtime (when module is already loaded and initialized!) 
userspace can decide to export some block device via usb. And unexport 
it at every time. Also this patch does not export any device by default. 
So it does not lock any block device (like MyDocs), so there is really 
no regressions like you though about MyDocs. usb networking, tty and 
also phone will still work via g_nokia. And if you do not export block 
device MyDocs it will stay normally mounted in N900 VFS.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Felipe Balbi]

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

On Thu, May 28, 2015 at 11:40:07PM +0200, Pali Rohár wrote:
> On Thursday 28 May 2015 18:34:31 Felipe Balbi wrote:
> > On Thu, May 28, 2015 at 04:59:18PM +0200, Pali Rohár wrote:
> > > On Thursday 28 May 2015 16:51:07 Krzysztof Opasiak wrote:
> > > > On 05/28/2015 04:31 PM, Pali Rohár wrote:
> > > > >On Thursday 28 May 2015 16:27:44 Krzysztof Opasiak wrote:
> > > > >>On 05/28/2015 09:47 AM, Pali Rohár wrote:
> > > > >>>On Saturday 31 January 2015 10:53:30 Pali Rohár wrote:
> > > > >>>>This patch adds removable mass storage support to g_nokia
> > > > >>>>gadget (for N900). It means that at runtime block device can
> > > > >>>>be exported or unexported. So it does not export anything by
> > > > >>>>default and thus allows to use MyDocs partition as before...
> > > > >>>>
> > > > >>>>Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > > >>>>---
> > > > >>>>
> > > > >>>>  drivers/usb/gadget/legacy/Kconfig |    1 +
> > > > >>>>  drivers/usb/gadget/legacy/nokia.c |  102
> > > > >>>>  ++++++++++++++++++++++++++++++++++++- 2 files changed, 102
> > > > >>>>  insertions(+), 1 deletion(-)
> > > > >>>>
> > > > >>>>diff --git a/drivers/usb/gadget/legacy/Kconfig
> > > > >>>>b/drivers/usb/gadget/legacy/Kconfig index fd48ef3..36f6ba4
> > > > >>>>100644
> > > > >>>>--- a/drivers/usb/gadget/legacy/Kconfig
> > > > >>>>+++ b/drivers/usb/gadget/legacy/Kconfig
> > > > >>>>@@ -345,6 +345,7 @@ config USB_G_NOKIA
> > > > >>>>
> > > > >>>>  	select USB_F_OBEX
> > > > >>>>  	select USB_F_PHONET
> > > > >>>>  	select USB_F_ECM
> > > > >>>>
> > > > >>>>+	select USB_F_MASS_STORAGE
> > > > >>>>
> > > > >>>>  	help
> > > > >>>>  	
> > > > >>>>  	  The Nokia composite gadget provides support for acm,
> > > > >>>>  	  obex and phonet in only one composite gadget driver.
> > > > >>>>
> > > > >>>>diff --git a/drivers/usb/gadget/legacy/nokia.c
> > > > >>>>b/drivers/usb/gadget/legacy/nokia.c index 9b8fd70..a09bb50
> > > > >>>>100644
> > > > >>>>--- a/drivers/usb/gadget/legacy/nokia.c
> > > > >>>>+++ b/drivers/usb/gadget/legacy/nokia.c
> > > > >>>>@@ -24,6 +24,7 @@
> > > > >>>>
> > > > >>>>  #include "u_phonet.h"
> > > > >>>>  #include "u_ecm.h"
> > > > >>>>  #include "gadget_chips.h"
> > > > >>>>
> > > > >>>>+#include "f_mass_storage.h"
> > > > >>>>
> > > > >>>>  /* Defines */
> > > > >>>>
> > > > >>>>@@ -34,6 +35,29 @@ USB_GADGET_COMPOSITE_OPTIONS();
> > > > >>>>
> > > > >>>>  USB_ETHERNET_MODULE_PARAMETERS();
> > > > >>>>
> > > > >>>>+static struct fsg_module_parameters fsg_mod_data = {
> > > > >>>>+	.stall = 0,
> > > > >>>>+	.luns = 2,
> > > > >>>>+	.removable_count = 2,
> > > > >>>>+	.removable = { 1, 1, },
> > > > >>>>+};
> > > > >>>>+
> > > > >>>>+FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
> > > > >>>>+
> > > > >>>>+#ifdef CONFIG_USB_GADGET_DEBUG_FILES
> > > > >>>>+
> > > > >>>>+static unsigned int fsg_num_buffers =
> > > > >>>>CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS; +
> > > > >>>>+#else
> > > > >>>>+
> > > > >>>>+/*
> > > > >>>>+ * Number of buffers we will use.
> > > > >>>>+ * 2 is usually enough for good buffering pipeline
> > > > >>>>+ */
> > > > >>>>+#define
> > > > >>>>fsg_num_buffers	CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS +
> > > > >>>>+#endif /* CONFIG_USB_DEBUG */
> > > > >>>>+
> > > > >>>>
> > > > >>>>  #define NOKIA_VENDOR_ID			0x0421	/* Nokia */
> > > > >>>>  #define NOKIA_PRODUCT_ID		0x01c8	/* Nokia Gadget */
> > > > >>>>
> > > > >>>>@@ -94,6 +118,8 @@ static struct usb_function *f_obex1_cfg2;
> > > > >>>>
> > > > >>>>  static struct usb_function *f_obex2_cfg2;
> > > > >>>>  static struct usb_function *f_phonet_cfg1;
> > > > >>>>  static struct usb_function *f_phonet_cfg2;
> > > > >>>>
> > > > >>>>+static struct usb_function *f_msg_cfg1;
> > > > >>>>+static struct usb_function *f_msg_cfg2;
> > > > >>>>
> > > > >>>>  static struct usb_configuration nokia_config_500ma_driver =
> > > > >>>>  {
> > > > >>>>
> > > > >>>>@@ -117,6 +143,7 @@ static struct usb_function_instance
> > > > >>>>*fi_ecm;
> > > > >>>>
> > > > >>>>  static struct usb_function_instance *fi_obex1;
> > > > >>>>  static struct usb_function_instance *fi_obex2;
> > > > >>>>  static struct usb_function_instance *fi_phonet;
> > > > >>>>
> > > > >>>>+static struct usb_function_instance *fi_msg;
> > > > >>>>
> > > > >>>>  static int __init nokia_bind_config(struct
> > > > >>>>  usb_configuration *c) {
> > > > >>>>
> > > > >>>>@@ -125,6 +152,8 @@ static int __init
> > > > >>>>nokia_bind_config(struct usb_configuration *c)
> > > > >>>>
> > > > >>>>  	struct usb_function *f_obex1 = NULL;
> > > > >>>>  	struct usb_function *f_ecm;
> > > > >>>>  	struct usb_function *f_obex2 = NULL;
> > > > >>>>
> > > > >>>>+	struct usb_function *f_msg;
> > > > >>>>+	struct fsg_opts *fsg_opts;
> > > > >>>>
> > > > >>>>  	int status = 0;
> > > > >>>>  	int obex1_stat = -1;
> > > > >>>>  	int obex2_stat = -1;
> > > > >>>>
> > > > >>>>@@ -160,6 +189,12 @@ static int __init
> > > > >>>>nokia_bind_config(struct usb_configuration *c)
> > > > >>>>
> > > > >>>>  		goto err_get_ecm;
> > > > >>>>  	
> > > > >>>>  	}
> > > > >>>>
> > > > >>>>+	f_msg = usb_get_function(fi_msg);
> > > > >>>>+	if (IS_ERR(f_msg)) {
> > > > >>>>+		status = PTR_ERR(f_msg);
> > > > >>>>+		goto err_get_msg;
> > > > >>>>+	}
> > > > >>>>+
> > > > >>>>
> > > > >>>>  	if (!IS_ERR_OR_NULL(f_phonet)) {
> > > > >>>>  	
> > > > >>>>  		phonet_stat = usb_add_function(c, f_phonet);
> > > > >>>>  		if (phonet_stat)
> > > > >>>>
> > > > >>>>@@ -187,21 +222,36 @@ static int __init
> > > > >>>>nokia_bind_config(struct usb_configuration *c)
> > > > >>>>
> > > > >>>>  		pr_debug("could not bind ecm config %d\n", status);
> > > > >>>>  		goto err_ecm;
> > > > >>>>  	
> > > > >>>>  	}
> > > > >>>>
> > > > >>>>+
> > > > >>>>+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
> > > > >>>>+
> > > > >>>>+	status = fsg_common_run_thread(fsg_opts->common);
> > > > >>>>+	if (status)
> > > > >>>>+		goto err_msg;
> > > > >>>>+
> > > > >>>>+	status = usb_add_function(c, f_msg);
> > > > >>>>+	if (status)
> > > > >>>>+		goto err_msg;
> > > > >>>>+
> > > > >>>>
> > > > >>>>  	if (c == &nokia_config_500ma_driver) {
> > > > >>>>  	
> > > > >>>>  		f_acm_cfg1 = f_acm;
> > > > >>>>  		f_ecm_cfg1 = f_ecm;
> > > > >>>>  		f_phonet_cfg1 = f_phonet;
> > > > >>>>  		f_obex1_cfg1 = f_obex1;
> > > > >>>>  		f_obex2_cfg1 = f_obex2;
> > > > >>>>
> > > > >>>>+		f_msg_cfg1 = f_msg;
> > > > >>>>
> > > > >>>>  	} else {
> > > > >>>>  	
> > > > >>>>  		f_acm_cfg2 = f_acm;
> > > > >>>>  		f_ecm_cfg2 = f_ecm;
> > > > >>>>  		f_phonet_cfg2 = f_phonet;
> > > > >>>>  		f_obex1_cfg2 = f_obex1;
> > > > >>>>  		f_obex2_cfg2 = f_obex2;
> > > > >>>>
> > > > >>>>+		f_msg_cfg2 = f_msg;
> > > > >>>>
> > > > >>>>  	}
> > > > >>>>  	
> > > > >>>>  	return status;
> > > > >>>>
> > > > >>>>+err_msg:
> > > > >>>>+	usb_remove_function(c, f_ecm);
> > > > >>>>
> > > > >>>>  err_ecm:
> > > > >>>>  	usb_remove_function(c, f_acm);
> > > > >>>>  
> > > > >>>>  err_conf:
> > > > >>>>@@ -211,6 +261,8 @@ err_conf:
> > > > >>>>  		usb_remove_function(c, f_obex1);
> > > > >>>>  	
> > > > >>>>  	if (!phonet_stat)
> > > > >>>>  	
> > > > >>>>  		usb_remove_function(c, f_phonet);
> > > > >>>>
> > > > >>>>+	usb_put_function(f_msg);
> > > > >>>>
> > > > >>>>+err_get_msg:
> > > > >>>>  	usb_put_function(f_ecm);
> > > > >>>>  
> > > > >>>>  err_get_ecm:
> > > > >>>>  	usb_put_function(f_acm);
> > > > >>>>
> > > > >>>>@@ -227,6 +279,8 @@ err_get_acm:
> > > > >>>>  static int __init nokia_bind(struct usb_composite_dev
> > > > >>>>  *cdev) {
> > > > >>>>  
> > > > >>>>  	struct usb_gadget	*gadget = cdev->gadget;
> > > > >>>>
> > > > >>>>+	struct fsg_opts		*fsg_opts;
> > > > >>>>+	struct fsg_config	fsg_config;
> > > > >>>>
> > > > >>>>  	int			status;
> > > > >>>>  	
> > > > >>>>  	status = usb_string_ids_tab(cdev, strings_dev);
> > > > >>>>
> > > > >>>>@@ -267,11 +321,46 @@ static int __init nokia_bind(struct
> > > > >>>>usb_composite_dev *cdev)
> > > > >>>>
> > > > >>>>  		goto err_acm_inst;
> > > > >>>>  	
> > > > >>>>  	}
> > > > >>>>
> > > > >>>>+	fi_msg = usb_get_function_instance("mass_storage");
> > > > >>>>+	if (IS_ERR(fi_msg)) {
> > > > >>>>+		status = PTR_ERR(fi_msg);
> > > > >>>>+		goto err_ecm_inst;
> > > > >>>>+	}
> > > > >>>>+
> > > > >>>>+	/* set up mass storage function */
> > > > >>>>+	fsg_config_from_params(&fsg_config, &fsg_mod_data,
> > > > >>>>fsg_num_buffers); +	fsg_config.vendor_name = "Nokia";
> > > > >>>>+	fsg_config.product_name = "N900";
> > > > >>>>+
> > > > >>>>+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
> > > > >>>>+	fsg_opts->no_configfs = true;
> > > > >>>>+
> > > > >>>>+	status = fsg_common_set_num_buffers(fsg_opts->common,
> > > > >>>>fsg_num_buffers); +	if (status)
> > > > >>>>+		goto err_msg_inst;
> > > > >>>>+
> > > > >>>>+	status = fsg_common_set_nluns(fsg_opts->common,
> > > > >>>>fsg_config.nluns); +	if (status)
> > > > >>>>+		goto err_msg_buf;
> > > > >>>>+
> > > > >>>>+	status = fsg_common_set_cdev(fsg_opts->common, cdev,
> > > > >>>>fsg_config.can_stall); +	if (status)
> > > > >>>>+		goto err_msg_set_nluns;
> > > > >>>>+
> > > > >>>>+	fsg_common_set_sysfs(fsg_opts->common, true);
> > > > >>>>+
> > > > >>>>+	status = fsg_common_create_luns(fsg_opts->common,
> > > > >>>>&fsg_config); +	if (status)
> > > > >>>>+		goto err_msg_set_nluns;
> > > > >>>>+
> > > > >>>>+	fsg_common_set_inquiry_string(fsg_opts->common,
> > > > >>>>fsg_config.vendor_name, +				      
> fsg_config.product_name);
> > > > >>>>+
> > > > >>>>
> > > > >>>>  	/* finally register the configuration */
> > > > >>>>  	status = usb_add_config(cdev, &nokia_config_500ma_driver,
> > > > >>>>  	
> > > > >>>>  			nokia_bind_config);
> > > > >>>>  	
> > > > >>>>  	if (status < 0)
> > > > >>>>
> > > > >>>>-		goto err_ecm_inst;
> > > > >>>>+		goto err_msg_set_cdev;
> > > > >>>>
> > > > >>>>  	status = usb_add_config(cdev, &nokia_config_100ma_driver,
> > > > >>>>  	
> > > > >>>>  			nokia_bind_config);
> > > > >>>>
> > > > >>>>@@ -292,6 +381,14 @@ err_put_cfg1:
> > > > >>>>  	if (!IS_ERR_OR_NULL(f_phonet_cfg1))
> > > > >>>>  	
> > > > >>>>  		usb_put_function(f_phonet_cfg1);
> > > > >>>>  	
> > > > >>>>  	usb_put_function(f_ecm_cfg1);
> > > > >>>>
> > > > >>>>+err_msg_set_cdev:
> > > > >>>>+	fsg_common_remove_luns(fsg_opts->common);
> > > > >>>>+err_msg_set_nluns:
> > > > >>>>+	fsg_common_free_luns(fsg_opts->common);
> > > > >>>>+err_msg_buf:
> > > > >>>>+	fsg_common_free_buffers(fsg_opts->common);
> > > > >>>>+err_msg_inst:
> > > > >>>>+	usb_put_function_instance(fi_msg);
> > > > >>>>
> > > > >>>>  err_ecm_inst:
> > > > >>>>  	usb_put_function_instance(fi_ecm);
> > > > >>>>  
> > > > >>>>  err_acm_inst:
> > > > >>>>@@ -325,7 +422,10 @@ static int __exit nokia_unbind(struct
> > > > >>>>usb_composite_dev *cdev)
> > > > >>>>
> > > > >>>>  	usb_put_function(f_acm_cfg2);
> > > > >>>>  	usb_put_function(f_ecm_cfg1);
> > > > >>>>  	usb_put_function(f_ecm_cfg2);
> > > > >>>>
> > > > >>>>+	usb_put_function(f_msg_cfg1);
> > > > >>>>+	usb_put_function(f_msg_cfg2);
> > > > >>>>
> > > > >>>>+	usb_put_function_instance(fi_msg);
> > > > >>>>
> > > > >>>>  	usb_put_function_instance(fi_ecm);
> > > > >>>>  	if (!IS_ERR(fi_obex2))
> > > > >>>>  	
> > > > >>>>  		usb_put_function_instance(fi_obex2);
> > > > >>>
> > > > >>>Opening discussion about this patch again as this is still
> > > > >>>only one solution how to use use mass storage without
> > > > >>>breaking other stuff...
> > > > >>>
> > > > >>>Please understand finally that usb networking is very very
> > > > >>>important for development on this device and usb mass storage
> > > > >>>is needed for transferring bigger data.
> > > > >>>
> > > > >>>Also to clarify potential regressions: This patch adds
> > > > >>>*optional* usb mass storage support and block device can be
> > > > >>>attached/detached to driver at runtime. It does *not* enforce
> > > > >>>to export some (mmc) device automatically. It is optional and
> > > > >>>userspace can decide...
> > > > >>>
> > > > >>>So MyDocs N900 partition is not automatically enforced to be
> > > > >>>exported via usb (as some people thought) and so it does not
> > > > >>>break usb networking or mass storage mode or MyDocs parition
> > > > >>>on N900 with Maemo system.
> > > > >>
> > > > >>Instead of adding mass storage to legacy gadget you may switch
> > > > >>to configfs interface and compose equivalent of g_nokia + mass
> > > > >>storage without any changes in kernel.
> > > > >>
> > > > >>Best regards,
> > > > >
> > > > >That requires userspace. And there are developers who use
> > > > >nfsroot via usb networking and so userspace will be there after
> > > > >properly initializing usb gadget... So it is not easy and
> > > > >reason why I'm opening this discussion again.
> > > > 
> > > > Couldn't you simply use initramfs to compose your gadget?
> > > 
> > > This is another next complication... I could spend time on hacking
> > > serious problems on Nokia N900 (camera not working, bluetooth not
> > > working, improving cellular audio/voice calls) or start playing
> > > with initramfs and other kernel infrastructure just because usb is
> > > broken and small fix which enable all needed usb functionality
> > > (usb networking, usb TTY, usb phonet, usb mass storage) cannot be
> > > accepted and merged because it really helps people to develop...
> > > :-( I see that it is under legacy drivers, but without it is hard
> > > (and maybe not possible) to develop for Nokia N900 device. Nobody
> > > until now proposed any other patch which fix our problems and
> > > provide working usb functionality for mass storage, network, TTY
> > > and phonet.
> > 
> > Sure this causes no problems to original Maemo userland ? Also, IIRC,
> > we were running out of both FIFO space and physical endpoints with
> > g_nokia already. Do all functions still work if used all at once ?
> > 
> > If you can confirm these two questions I'll take this patch. As long
> > as there are no regressions with original userland from Nokia, it
> > should be fine.
> > 
> > cheers
> 
> In this my patch usb gadget support is removable (see fsg_mod_data). It 
> means that at runtime (when module is already loaded and initialized!) 
> userspace can decide to export some block device via usb. And unexport 
> it at every time. Also this patch does not export any device by default. 
> So it does not lock any block device (like MyDocs), so there is really 
> no regressions like you though about MyDocs. usb networking, tty and 
> also phone will still work via g_nokia. And if you do not export block 
> device MyDocs it will stay normally mounted in N900 VFS.

All right, I tried merging it and it added build breaks to the tree.
I'll wait for another version for v4.3 merge window.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 6857 bytes --]

On Friday 29 May 2015 18:57:06 Felipe Balbi wrote:
> All right, I tried merging it and it added build breaks to the tree.
> I'll wait for another version for v4.3 merge window.
> 
> cheers

Hello, version below compiles fine on top of linus tree. Please apply:

diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig
index d5a7102..ddef41f 100644
--- a/drivers/usb/gadget/legacy/Kconfig
+++ b/drivers/usb/gadget/legacy/Kconfig
@@ -346,6 +346,7 @@ config USB_G_NOKIA
 	select USB_F_OBEX
 	select USB_F_PHONET
 	select USB_F_ECM
+	select USB_F_MASS_STORAGE
 	help
 	  The Nokia composite gadget provides support for acm, obex
 	  and phonet in only one composite gadget driver.
diff --git a/drivers/usb/gadget/legacy/nokia.c b/drivers/usb/gadget/legacy/nokia.c
index 4bb498a..021c8d9 100644
--- a/drivers/usb/gadget/legacy/nokia.c
+++ b/drivers/usb/gadget/legacy/nokia.c
@@ -24,6 +24,7 @@
 #include "u_phonet.h"
 #include "u_ecm.h"
 #include "gadget_chips.h"
+#include "f_mass_storage.h"
 
 /* Defines */
 
@@ -34,6 +35,29 @@ USB_GADGET_COMPOSITE_OPTIONS();
 
 USB_ETHERNET_MODULE_PARAMETERS();
 
+static struct fsg_module_parameters fsg_mod_data = {
+	.stall = 0,
+	.luns = 2,
+	.removable_count = 2,
+	.removable = { 1, 1, },
+};
+
+FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
+
+#ifdef CONFIG_USB_GADGET_DEBUG_FILES
+
+static unsigned int fsg_num_buffers = CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS;
+
+#else
+
+/*
+ * Number of buffers we will use.
+ * 2 is usually enough for good buffering pipeline
+ */
+#define fsg_num_buffers	CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS
+
+#endif /* CONFIG_USB_DEBUG */
+
 #define NOKIA_VENDOR_ID			0x0421	/* Nokia */
 #define NOKIA_PRODUCT_ID		0x01c8	/* Nokia Gadget */
 
@@ -94,6 +118,8 @@ static struct usb_function *f_obex1_cfg2;
 static struct usb_function *f_obex2_cfg2;
 static struct usb_function *f_phonet_cfg1;
 static struct usb_function *f_phonet_cfg2;
+static struct usb_function *f_msg_cfg1;
+static struct usb_function *f_msg_cfg2;
 
 
 static struct usb_configuration nokia_config_500ma_driver = {
@@ -117,6 +143,7 @@ static struct usb_function_instance *fi_ecm;
 static struct usb_function_instance *fi_obex1;
 static struct usb_function_instance *fi_obex2;
 static struct usb_function_instance *fi_phonet;
+static struct usb_function_instance *fi_msg;
 
 static int nokia_bind_config(struct usb_configuration *c)
 {
@@ -125,6 +152,8 @@ static int nokia_bind_config(struct usb_configuration *c)
 	struct usb_function *f_obex1 = NULL;
 	struct usb_function *f_ecm;
 	struct usb_function *f_obex2 = NULL;
+	struct usb_function *f_msg;
+	struct fsg_opts *fsg_opts;
 	int status = 0;
 	int obex1_stat = -1;
 	int obex2_stat = -1;
@@ -160,6 +189,12 @@ static int nokia_bind_config(struct usb_configuration *c)
 		goto err_get_ecm;
 	}
 
+	f_msg = usb_get_function(fi_msg);
+	if (IS_ERR(f_msg)) {
+		status = PTR_ERR(f_msg);
+		goto err_get_msg;
+	}
+
 	if (!IS_ERR_OR_NULL(f_phonet)) {
 		phonet_stat = usb_add_function(c, f_phonet);
 		if (phonet_stat)
@@ -187,21 +222,36 @@ static int nokia_bind_config(struct usb_configuration *c)
 		pr_debug("could not bind ecm config %d\n", status);
 		goto err_ecm;
 	}
+
+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
+
+	status = fsg_common_run_thread(fsg_opts->common);
+	if (status)
+		goto err_msg;
+
+	status = usb_add_function(c, f_msg);
+	if (status)
+		goto err_msg;
+
 	if (c == &nokia_config_500ma_driver) {
 		f_acm_cfg1 = f_acm;
 		f_ecm_cfg1 = f_ecm;
 		f_phonet_cfg1 = f_phonet;
 		f_obex1_cfg1 = f_obex1;
 		f_obex2_cfg1 = f_obex2;
+		f_msg_cfg1 = f_msg;
 	} else {
 		f_acm_cfg2 = f_acm;
 		f_ecm_cfg2 = f_ecm;
 		f_phonet_cfg2 = f_phonet;
 		f_obex1_cfg2 = f_obex1;
 		f_obex2_cfg2 = f_obex2;
+		f_msg_cfg2 = f_msg;
 	}
 
 	return status;
+err_msg:
+	usb_remove_function(c, f_ecm);
 err_ecm:
 	usb_remove_function(c, f_acm);
 err_conf:
@@ -211,6 +261,8 @@ err_conf:
 		usb_remove_function(c, f_obex1);
 	if (!phonet_stat)
 		usb_remove_function(c, f_phonet);
+	usb_put_function(f_msg);
+err_get_msg:
 	usb_put_function(f_ecm);
 err_get_ecm:
 	usb_put_function(f_acm);
@@ -227,6 +279,8 @@ err_get_acm:
 static int nokia_bind(struct usb_composite_dev *cdev)
 {
 	struct usb_gadget	*gadget = cdev->gadget;
+	struct fsg_opts		*fsg_opts;
+	struct fsg_config	fsg_config;
 	int			status;
 
 	status = usb_string_ids_tab(cdev, strings_dev);
@@ -267,11 +321,46 @@ static int nokia_bind(struct usb_composite_dev *cdev)
 		goto err_acm_inst;
 	}
 
+	fi_msg = usb_get_function_instance("mass_storage");
+	if (IS_ERR(fi_msg)) {
+		status = PTR_ERR(fi_msg);
+		goto err_ecm_inst;
+	}
+
+	/* set up mass storage function */
+	fsg_config_from_params(&fsg_config, &fsg_mod_data, fsg_num_buffers);
+	fsg_config.vendor_name = "Nokia";
+	fsg_config.product_name = "N900";
+
+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
+	fsg_opts->no_configfs = true;
+
+	status = fsg_common_set_num_buffers(fsg_opts->common, fsg_num_buffers);
+	if (status)
+		goto err_msg_inst;
+
+	status = fsg_common_set_nluns(fsg_opts->common, fsg_config.nluns);
+	if (status)
+		goto err_msg_buf;
+
+	status = fsg_common_set_cdev(fsg_opts->common, cdev, fsg_config.can_stall);
+	if (status)
+		goto err_msg_set_nluns;
+
+	fsg_common_set_sysfs(fsg_opts->common, true);
+
+	status = fsg_common_create_luns(fsg_opts->common, &fsg_config);
+	if (status)
+		goto err_msg_set_nluns;
+
+	fsg_common_set_inquiry_string(fsg_opts->common, fsg_config.vendor_name,
+				      fsg_config.product_name);
+
 	/* finally register the configuration */
 	status = usb_add_config(cdev, &nokia_config_500ma_driver,
 			nokia_bind_config);
 	if (status < 0)
-		goto err_ecm_inst;
+		goto err_msg_set_cdev;
 
 	status = usb_add_config(cdev, &nokia_config_100ma_driver,
 			nokia_bind_config);
@@ -292,6 +381,14 @@ err_put_cfg1:
 	if (!IS_ERR_OR_NULL(f_phonet_cfg1))
 		usb_put_function(f_phonet_cfg1);
 	usb_put_function(f_ecm_cfg1);
+err_msg_set_cdev:
+	fsg_common_remove_luns(fsg_opts->common);
+err_msg_set_nluns:
+	fsg_common_free_luns(fsg_opts->common);
+err_msg_buf:
+	fsg_common_free_buffers(fsg_opts->common);
+err_msg_inst:
+	usb_put_function_instance(fi_msg);
 err_ecm_inst:
 	usb_put_function_instance(fi_ecm);
 err_acm_inst:
@@ -325,7 +422,10 @@ static int nokia_unbind(struct usb_composite_dev *cdev)
 	usb_put_function(f_acm_cfg2);
 	usb_put_function(f_ecm_cfg1);
 	usb_put_function(f_ecm_cfg2);
+	usb_put_function(f_msg_cfg1);
+	usb_put_function(f_msg_cfg2);
 
+	usb_put_function_instance(fi_msg);
 	usb_put_function_instance(fi_ecm);
 	if (!IS_ERR(fi_obex2))
 		usb_put_function_instance(fi_obex2);


-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Felipe Balbi]

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

Hi,

On Fri, Jun 05, 2015 at 10:09:03PM +0200, Pali Rohár wrote:
> On Friday 29 May 2015 18:57:06 Felipe Balbi wrote:
> > All right, I tried merging it and it added build breaks to the tree.
> > I'll wait for another version for v4.3 merge window.
> > 
> > cheers
> 
> Hello, version below compiles fine on top of linus tree. Please apply:

no signed-off-by, not commit log. Sorry, can't apply.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 845 bytes --]

On Friday 05 June 2015 22:17:09 Felipe Balbi wrote:
> Hi,
> 
> On Fri, Jun 05, 2015 at 10:09:03PM +0200, Pali Rohár wrote:
> > On Friday 29 May 2015 18:57:06 Felipe Balbi wrote:
> > > All right, I tried merging it and it added build breaks to the
> > > tree. I'll wait for another version for v4.3 merge window.
> > > 
> > > cheers
> > 
> > Hello, version below compiles fine on top of linus tree. Please
> > apply:
> 
> no signed-off-by,

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

> not commit log.

This patch adds removable mass storage support to g_nokia gadget (for 
N900). It means that at runtime block device can be exported or 
unexported. So it does not export anything by default and thus allows to 
use MyDocs partition as before...

> Sorry, can't apply.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Felipe Balbi]

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

On Sat, Jun 06, 2015 at 10:04:55AM +0200, Pali Rohár wrote:
> On Friday 05 June 2015 22:17:09 Felipe Balbi wrote:
> > Hi,
> > 
> > On Fri, Jun 05, 2015 at 10:09:03PM +0200, Pali Rohár wrote:
> > > On Friday 29 May 2015 18:57:06 Felipe Balbi wrote:
> > > > All right, I tried merging it and it added build breaks to the
> > > > tree. I'll wait for another version for v4.3 merge window.
> > > > 
> > > > cheers
> > > 
> > > Hello, version below compiles fine on top of linus tree. Please
> > > apply:
> > 
> > no signed-off-by,
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> 
> > not commit log.
> 
> This patch adds removable mass storage support to g_nokia gadget (for 
> N900). It means that at runtime block device can be exported or 
> unexported. So it does not export anything by default and thus allows to 
> use MyDocs partition as before...

sorry, but you gotta resend it, you're just a git commit && git
send-email -1 away.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

This patch adds removable mass storage support to g_nokia gadget (for N900).
It means that at runtime block device can be exported or unexported.
So it does not export anything by default and thus allows to use MyDocs
partition as before...

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/usb/gadget/legacy/Kconfig |    1 +
 drivers/usb/gadget/legacy/nokia.c |  102 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 102 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig
index d5a7102..ddef41f 100644
--- a/drivers/usb/gadget/legacy/Kconfig
+++ b/drivers/usb/gadget/legacy/Kconfig
@@ -346,6 +346,7 @@ config USB_G_NOKIA
 	select USB_F_OBEX
 	select USB_F_PHONET
 	select USB_F_ECM
+	select USB_F_MASS_STORAGE
 	help
 	  The Nokia composite gadget provides support for acm, obex
 	  and phonet in only one composite gadget driver.
diff --git a/drivers/usb/gadget/legacy/nokia.c b/drivers/usb/gadget/legacy/nokia.c
index 4bb498a..021c8d9 100644
--- a/drivers/usb/gadget/legacy/nokia.c
+++ b/drivers/usb/gadget/legacy/nokia.c
@@ -24,6 +24,7 @@
 #include "u_phonet.h"
 #include "u_ecm.h"
 #include "gadget_chips.h"
+#include "f_mass_storage.h"
 
 /* Defines */
 
@@ -34,6 +35,29 @@ USB_GADGET_COMPOSITE_OPTIONS();
 
 USB_ETHERNET_MODULE_PARAMETERS();
 
+static struct fsg_module_parameters fsg_mod_data = {
+	.stall = 0,
+	.luns = 2,
+	.removable_count = 2,
+	.removable = { 1, 1, },
+};
+
+FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
+
+#ifdef CONFIG_USB_GADGET_DEBUG_FILES
+
+static unsigned int fsg_num_buffers = CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS;
+
+#else
+
+/*
+ * Number of buffers we will use.
+ * 2 is usually enough for good buffering pipeline
+ */
+#define fsg_num_buffers	CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS
+
+#endif /* CONFIG_USB_DEBUG */
+
 #define NOKIA_VENDOR_ID			0x0421	/* Nokia */
 #define NOKIA_PRODUCT_ID		0x01c8	/* Nokia Gadget */
 
@@ -94,6 +118,8 @@ static struct usb_function *f_obex1_cfg2;
 static struct usb_function *f_obex2_cfg2;
 static struct usb_function *f_phonet_cfg1;
 static struct usb_function *f_phonet_cfg2;
+static struct usb_function *f_msg_cfg1;
+static struct usb_function *f_msg_cfg2;
 
 
 static struct usb_configuration nokia_config_500ma_driver = {
@@ -117,6 +143,7 @@ static struct usb_function_instance *fi_ecm;
 static struct usb_function_instance *fi_obex1;
 static struct usb_function_instance *fi_obex2;
 static struct usb_function_instance *fi_phonet;
+static struct usb_function_instance *fi_msg;
 
 static int nokia_bind_config(struct usb_configuration *c)
 {
@@ -125,6 +152,8 @@ static int nokia_bind_config(struct usb_configuration *c)
 	struct usb_function *f_obex1 = NULL;
 	struct usb_function *f_ecm;
 	struct usb_function *f_obex2 = NULL;
+	struct usb_function *f_msg;
+	struct fsg_opts *fsg_opts;
 	int status = 0;
 	int obex1_stat = -1;
 	int obex2_stat = -1;
@@ -160,6 +189,12 @@ static int nokia_bind_config(struct usb_configuration *c)
 		goto err_get_ecm;
 	}
 
+	f_msg = usb_get_function(fi_msg);
+	if (IS_ERR(f_msg)) {
+		status = PTR_ERR(f_msg);
+		goto err_get_msg;
+	}
+
 	if (!IS_ERR_OR_NULL(f_phonet)) {
 		phonet_stat = usb_add_function(c, f_phonet);
 		if (phonet_stat)
@@ -187,21 +222,36 @@ static int nokia_bind_config(struct usb_configuration *c)
 		pr_debug("could not bind ecm config %d\n", status);
 		goto err_ecm;
 	}
+
+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
+
+	status = fsg_common_run_thread(fsg_opts->common);
+	if (status)
+		goto err_msg;
+
+	status = usb_add_function(c, f_msg);
+	if (status)
+		goto err_msg;
+
 	if (c == &nokia_config_500ma_driver) {
 		f_acm_cfg1 = f_acm;
 		f_ecm_cfg1 = f_ecm;
 		f_phonet_cfg1 = f_phonet;
 		f_obex1_cfg1 = f_obex1;
 		f_obex2_cfg1 = f_obex2;
+		f_msg_cfg1 = f_msg;
 	} else {
 		f_acm_cfg2 = f_acm;
 		f_ecm_cfg2 = f_ecm;
 		f_phonet_cfg2 = f_phonet;
 		f_obex1_cfg2 = f_obex1;
 		f_obex2_cfg2 = f_obex2;
+		f_msg_cfg2 = f_msg;
 	}
 
 	return status;
+err_msg:
+	usb_remove_function(c, f_ecm);
 err_ecm:
 	usb_remove_function(c, f_acm);
 err_conf:
@@ -211,6 +261,8 @@ err_conf:
 		usb_remove_function(c, f_obex1);
 	if (!phonet_stat)
 		usb_remove_function(c, f_phonet);
+	usb_put_function(f_msg);
+err_get_msg:
 	usb_put_function(f_ecm);
 err_get_ecm:
 	usb_put_function(f_acm);
@@ -227,6 +279,8 @@ err_get_acm:
 static int nokia_bind(struct usb_composite_dev *cdev)
 {
 	struct usb_gadget	*gadget = cdev->gadget;
+	struct fsg_opts		*fsg_opts;
+	struct fsg_config	fsg_config;
 	int			status;
 
 	status = usb_string_ids_tab(cdev, strings_dev);
@@ -267,11 +321,46 @@ static int nokia_bind(struct usb_composite_dev *cdev)
 		goto err_acm_inst;
 	}
 
+	fi_msg = usb_get_function_instance("mass_storage");
+	if (IS_ERR(fi_msg)) {
+		status = PTR_ERR(fi_msg);
+		goto err_ecm_inst;
+	}
+
+	/* set up mass storage function */
+	fsg_config_from_params(&fsg_config, &fsg_mod_data, fsg_num_buffers);
+	fsg_config.vendor_name = "Nokia";
+	fsg_config.product_name = "N900";
+
+	fsg_opts = fsg_opts_from_func_inst(fi_msg);
+	fsg_opts->no_configfs = true;
+
+	status = fsg_common_set_num_buffers(fsg_opts->common, fsg_num_buffers);
+	if (status)
+		goto err_msg_inst;
+
+	status = fsg_common_set_nluns(fsg_opts->common, fsg_config.nluns);
+	if (status)
+		goto err_msg_buf;
+
+	status = fsg_common_set_cdev(fsg_opts->common, cdev, fsg_config.can_stall);
+	if (status)
+		goto err_msg_set_nluns;
+
+	fsg_common_set_sysfs(fsg_opts->common, true);
+
+	status = fsg_common_create_luns(fsg_opts->common, &fsg_config);
+	if (status)
+		goto err_msg_set_nluns;
+
+	fsg_common_set_inquiry_string(fsg_opts->common, fsg_config.vendor_name,
+				      fsg_config.product_name);
+
 	/* finally register the configuration */
 	status = usb_add_config(cdev, &nokia_config_500ma_driver,
 			nokia_bind_config);
 	if (status < 0)
-		goto err_ecm_inst;
+		goto err_msg_set_cdev;
 
 	status = usb_add_config(cdev, &nokia_config_100ma_driver,
 			nokia_bind_config);
@@ -292,6 +381,14 @@ err_put_cfg1:
 	if (!IS_ERR_OR_NULL(f_phonet_cfg1))
 		usb_put_function(f_phonet_cfg1);
 	usb_put_function(f_ecm_cfg1);
+err_msg_set_cdev:
+	fsg_common_remove_luns(fsg_opts->common);
+err_msg_set_nluns:
+	fsg_common_free_luns(fsg_opts->common);
+err_msg_buf:
+	fsg_common_free_buffers(fsg_opts->common);
+err_msg_inst:
+	usb_put_function_instance(fi_msg);
 err_ecm_inst:
 	usb_put_function_instance(fi_ecm);
 err_acm_inst:
@@ -325,7 +422,10 @@ static int nokia_unbind(struct usb_composite_dev *cdev)
 	usb_put_function(f_acm_cfg2);
 	usb_put_function(f_ecm_cfg1);
 	usb_put_function(f_ecm_cfg2);
+	usb_put_function(f_msg_cfg1);
+	usb_put_function(f_msg_cfg2);
 
+	usb_put_function_instance(fi_msg);
 	usb_put_function_instance(fi_ecm);
 	if (!IS_ERR(fi_obex2))
 		usb_put_function_instance(fi_obex2);
-- 
1.7.9.5

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Felipe Balbi]

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

On Mon, Jun 08, 2015 at 08:20:05AM +0200, Pali Rohár wrote:
> This patch adds removable mass storage support to g_nokia gadget (for N900).
> It means that at runtime block device can be exported or unexported.
> So it does not export anything by default and thus allows to use MyDocs
> partition as before...
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

now in my testing/next, thanks

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 628 bytes --]

On Monday 08 June 2015 18:08:19 Felipe Balbi wrote:
> On Mon, Jun 08, 2015 at 08:20:05AM +0200, Pali Rohár wrote:
> > This patch adds removable mass storage support to g_nokia gadget
> > (for N900). It means that at runtime block device can be exported
> > or unexported. So it does not export anything by default and thus
> > allows to use MyDocs partition as before...
> > 
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> 
> now in my testing/next, thanks

Hello, what happened with this patch? Version 4.2-rc1 was released, but 
I do not see it included yet...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Felipe Balbi]

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

On Mon, Jul 06, 2015 at 01:59:18PM +0200, Pali Rohár wrote:
> On Monday 08 June 2015 18:08:19 Felipe Balbi wrote:
> > On Mon, Jun 08, 2015 at 08:20:05AM +0200, Pali Rohár wrote:
> > > This patch adds removable mass storage support to g_nokia gadget
> > > (for N900). It means that at runtime block device can be exported
> > > or unexported. So it does not export anything by default and thus
> > > allows to use MyDocs partition as before...
> > > 
> > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > 
> > now in my testing/next, thanks
> 
> Hello, what happened with this patch? Version 4.2-rc1 was released, but 
> I do not see it included yet...

By the time I merged this to testing/next, my tree was already closed to
v4.2, it's queued for v4.3. And I also fixed the NULL pointer
dereference your patch caused, btw.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Felipe Balbi]

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

On Mon, Jul 06, 2015 at 12:40:49PM -0500, Felipe Balbi wrote:
> On Mon, Jul 06, 2015 at 01:59:18PM +0200, Pali Rohár wrote:
> > On Monday 08 June 2015 18:08:19 Felipe Balbi wrote:
> > > On Mon, Jun 08, 2015 at 08:20:05AM +0200, Pali Rohár wrote:
> > > > This patch adds removable mass storage support to g_nokia gadget
> > > > (for N900). It means that at runtime block device can be exported
> > > > or unexported. So it does not export anything by default and thus
> > > > allows to use MyDocs partition as before...
> > > > 
> > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > 
> > > now in my testing/next, thanks
> > 
> > Hello, what happened with this patch? Version 4.2-rc1 was released, but 
> > I do not see it included yet...
> 
> By the time I merged this to testing/next, my tree was already closed to
> v4.2, it's queued for v4.3. And I also fixed the NULL pointer
> dereference your patch caused, btw.

sorry, it was a build error.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1177 bytes --]

On Monday 06 July 2015 19:44:21 Felipe Balbi wrote:
> On Mon, Jul 06, 2015 at 12:40:49PM -0500, Felipe Balbi wrote:
> > On Mon, Jul 06, 2015 at 01:59:18PM +0200, Pali Rohár wrote:
> > > On Monday 08 June 2015 18:08:19 Felipe Balbi wrote:
> > > > On Mon, Jun 08, 2015 at 08:20:05AM +0200, Pali Rohár wrote:
> > > > > This patch adds removable mass storage support to g_nokia
> > > > > gadget (for N900). It means that at runtime block device can
> > > > > be exported or unexported. So it does not export anything by
> > > > > default and thus allows to use MyDocs partition as before...
> > > > > 
> > > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > > 
> > > > now in my testing/next, thanks
> > > 
> > > Hello, what happened with this patch? Version 4.2-rc1 was
> > > released, but I do not see it included yet...
> > 
> > By the time I merged this to testing/next, my tree was already
> > closed to v4.2, it's queued for v4.3. And I also fixed the NULL
> > pointer dereference your patch caused, btw.
> 
> sorry, it was a build error.

When I sent this patch, it compiled without any error...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Felipe Balbi]

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

Hi,

On Mon, Jul 06, 2015 at 08:07:25PM +0200, Pali Rohár wrote:
> On Monday 06 July 2015 19:44:21 Felipe Balbi wrote:
> > On Mon, Jul 06, 2015 at 12:40:49PM -0500, Felipe Balbi wrote:
> > > On Mon, Jul 06, 2015 at 01:59:18PM +0200, Pali Rohár wrote:
> > > > On Monday 08 June 2015 18:08:19 Felipe Balbi wrote:
> > > > > On Mon, Jun 08, 2015 at 08:20:05AM +0200, Pali Rohár wrote:
> > > > > > This patch adds removable mass storage support to g_nokia
> > > > > > gadget (for N900). It means that at runtime block device can
> > > > > > be exported or unexported. So it does not export anything by
> > > > > > default and thus allows to use MyDocs partition as before...
> > > > > > 
> > > > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > > > 
> > > > > now in my testing/next, thanks
> > > > 
> > > > Hello, what happened with this patch? Version 4.2-rc1 was
> > > > released, but I do not see it included yet...
> > > 
> > > By the time I merged this to testing/next, my tree was already
> > > closed to v4.2, it's queued for v4.3. And I also fixed the NULL
> > > pointer dereference your patch caused, btw.
> > 
> > sorry, it was a build error.
> 
> When I sent this patch, it compiled without any error...

there was a build error due to undefined nlus (IIRC). In any case, it's
fixed. No harm done.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pavel Machek]

On Mon 2013-01-21 10:05:06, Felipe Balbi wrote:
> Hi,
> 
> On Sun, Jan 20, 2013 at 11:17:31AM +0100, Pali Rohár wrote:
> > On Sunday 20 January 2013 10:25:37 Felipe Balbi wrote:
> > > On Sun, Jan 20, 2013 at 03:58:13AM +0100, Pali Rohár wrote:
> > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > 
> > > NAK for two reasons:
> > > 
> > > a) the original Nokia kernel used a separate g_file_storage
> > > gadget to use Mass Storage mode, use that
> > > 
> > > b) there is no commit log
> > 
> > Reason why add mass storage mode to g_nokia is to avoid switching 
> > between g_{file,mass}_storage and g_nokia and to have one gadget 
> > driver for Nokia N900. It is better to have usb network and mass 
> > storage mode in one driver (and not to unload & load another).
> > 
> > I tested this patch with 3.8-rc3 kernel on Nokia N900 and usb 
> > network with mass storage mode working without problems.
> 
> Doesn't matter, in this case this is something which nokia wrote to
> carry on their Maemo/MeeGo devices so unless someone from Nokia says
> this is how they want to use nokia.c from now on, I can't simply risk
> breaking all other users for your own convenience.

Nokia is unlikely to continue linux development, sorry.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 6123 bytes --]

On Tuesday 22 January 2013 17:17:21 Felipe Balbi wrote:
> Hi,
> 
> On Tue, Jan 22, 2013 at 12:03:09PM +0100, Pali Rohár wrote:
> > On Monday 21 January 2013 09:05:06 Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Sun, Jan 20, 2013 at 11:17:31AM +0100, Pali Rohár wrote:
> > > > On Sunday 20 January 2013 10:25:37 Felipe Balbi wrote:
> > > > > On Sun, Jan 20, 2013 at 03:58:13AM +0100, Pali Rohár 
wrote:
> > > > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > > > 
> > > > > NAK for two reasons:
> > > > > 
> > > > > a) the original Nokia kernel used a separate
> > > > > g_file_storage gadget to use Mass Storage mode, use
> > > > > that
> > > > > 
> > > > > b) there is no commit log
> > > > 
> > > > Reason why add mass storage mode to g_nokia is to avoid
> > > > switching between g_{file,mass}_storage and g_nokia and
> > > > to have one gadget driver for Nokia N900. It is better
> > > > to have usb network and mass storage mode in one driver
> > > > (and not to unload & load another).
> > > > 
> > > > I tested this patch with 3.8-rc3 kernel on Nokia N900
> > > > and usb network with mass storage mode working without
> > > > problems.
> > > 
> > > Doesn't matter, in this case this is something which nokia
> > > wrote to carry on their Maemo/MeeGo devices so unless
> > > someone from Nokia says this is how they want to use
> > > nokia.c from now on, I can't simply risk breaking all
> > > other users for your own convenience.
> > 
> > Hello,
> > 
> > you may know that Nokia not working on any linux Maemo/MeeGo
> > systems anymore. And also that Nokia devices needs own
> > patched kernel. Also g_nokia gadget is for Nokia N900 and
> > this device with original Maemo 5 system which was
> > supported by Nokia is locked for patched 2.6.28 kernel.
> > More drivers in 2.6.28 was not upstreamed, so running other
> > kernels will not work without problems. And waiting what
> > Nokia say is now irrelevant, because
> 
> nonsense, many guys (including myself) have n900 booting
> mainline kernel.
> 

Hi,

this sounds good. Do you use some n900 specified patches with 
upstream kernel? There are more drivers which are not present in 
mainline (like bluetooth, camera or ssi).

To reduce divergence I created git repo for n900 patches on 
gitorious https://gitorious.org/linux-n900/linux-n900 where I 
pushing needed patched for n900 hw (now based on 3.8-rc3)

I know only few people which hacking n900 with upstream kernel. 
And all are on freenode #maemo-alternatives channel. Everybody 
with some new upstream kernel is welcome.

Btw, now I have working kernel drivers for charging battery on 
n900, so charging is solved.

> > N900 is at end-of-live cycle and Nokia not doing with linux
> > devices anymore. So I do not understand why current code
> > cannot be extended for more functionality. Because patch is
> > not signed by Nokia?
> 
> If I let your change in, you could be breaking the folks who
> are still using n900 as their daily device. Remember that
> Mass Storage access to the media, prevents phone from using
> mass storage too and the way Maemo 5 was done is that it will
> unload mass storage when that's not being used.
> 

This is not truth. I commited more patched to CSSU ke-recv maemo 
daemon (which handle pcsuite/mass storage mode and mounting 
MyDocs and SD card) and I know how it working. That daemon do not 
unload g_file_storage driver when mass storage mode is 
deactivated. Instead it will call "echo > /sys/something/lun" 
which cause to disconnect mmc device from gadget. This working 
fine, because gadget is loaded with removeable param. And because 
there are some other problems with BME and maemo kernel, you need 
to have loaded some gadget for proper wall charger detection in 
BME. And maemo upstart daemon loading g_file_storage at boot time 
with none mmc device. So this is not problem.

If you can see at my patch, it will create mass storage gadget 
with removable param too. So block device can be 
attached/deattached from gadget at runtime without problem. Also 
when gadget is loaded nothing is attached.

> > There are more developers which playing with upstream kernel
> > on Nokia N900 and trying to use some modern linux
> > distribution on it. And who using upstream kernels on N900
> > also want some additional functionality which was not in
> > 2.6.28. And having mass storage in g_nokia is usefull.
> 
> it makes no difference.
> 
> > Also you can see that this patch simply adding new composite
> > gadget to exisitng driver. Nothing is removed, so original
> > code is compatible with these changes. If somebody still
> > want (for
> 
> you could be breaking phone's own access to the memory card.
> 

No, because by default is nothing attached to gadget.

> > some reason) to switching between g_nokia and g_file_storage
> > (ops, it was renamed to g_mass_storage, so now it is broken
> > on old Nokia systems...) it is still possible.
> 
> no it's not, you can anyways make a symbolic link. Besides,
> before removing g_file_storage we waited quite some time.
> 
> > And when I looked into nokia 2.6.28 kernel, they also
> > patched g_file_storage, so I think it is incompatible with
> > upstream too.
> 
> it's patched only for strings.
> 

Not exactly, when I looked at diff there was some other code and 
comments with "hacks". But changes for this are irrelevant now...

> > So why to care about current API implementation in upstream
> > kernel (do not allow to add new functionality) which is
> > incompatible with Nokia patched kernel?
> 
> Because I don't think g_nokia needs mass storage and because I
> want to remove all gadget drivers from kernel (keep only
> function drivers).

Ok, if you remove all gadget from kernel, this will break 
compatibility for sure.

And what will be equivalent of g_nokia? Will there be some 
"universal" composite gadget (which will have support for serial 
port, mass storage and usb network)?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Felipe Balbi]

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

Hi,

On Tue, Jan 22, 2013 at 12:03:09PM +0100, Pali Rohár wrote:
> On Monday 21 January 2013 09:05:06 Felipe Balbi wrote:
> > Hi,
> > 
> > On Sun, Jan 20, 2013 at 11:17:31AM +0100, Pali Rohár wrote:
> > > On Sunday 20 January 2013 10:25:37 Felipe Balbi wrote:
> > > > On Sun, Jan 20, 2013 at 03:58:13AM +0100, Pali Rohár wrote:
> > > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > > 
> > > > NAK for two reasons:
> > > > 
> > > > a) the original Nokia kernel used a separate
> > > > g_file_storage gadget to use Mass Storage mode, use that
> > > > 
> > > > b) there is no commit log
> > > 
> > > Reason why add mass storage mode to g_nokia is to avoid
> > > switching between g_{file,mass}_storage and g_nokia and to
> > > have one gadget driver for Nokia N900. It is better to have
> > > usb network and mass storage mode in one driver (and not to
> > > unload & load another).
> > > 
> > > I tested this patch with 3.8-rc3 kernel on Nokia N900 and
> > > usb network with mass storage mode working without
> > > problems.
> > 
> > Doesn't matter, in this case this is something which nokia
> > wrote to carry on their Maemo/MeeGo devices so unless someone
> > from Nokia says this is how they want to use nokia.c from now
> > on, I can't simply risk breaking all other users for your own
> > convenience.
> > 
> 
> Hello,
> 
> you may know that Nokia not working on any linux Maemo/MeeGo 
> systems anymore. And also that Nokia devices needs own patched 
> kernel. Also g_nokia gadget is for Nokia N900 and this device 
> with original Maemo 5 system which was supported by Nokia is 
> locked for patched 2.6.28 kernel. More drivers in 2.6.28 was not 
> upstreamed, so running other kernels will not work without 
> problems. And waiting what Nokia say is now irrelevant, because 

nonsense, many guys (including myself) have n900 booting mainline
kernel.

> N900 is at end-of-live cycle and Nokia not doing with linux 
> devices anymore. So I do not understand why current code cannot 
> be extended for more functionality. Because patch is not signed 
> by Nokia?

If I let your change in, you could be breaking the folks who are still
using n900 as their daily device. Remember that Mass Storage access to
the media, prevents phone from using mass storage too and the way Maemo
5 was done is that it will unload mass storage when that's not being
used.

> There are more developers which playing with upstream kernel on 
> Nokia N900 and trying to use some modern linux distribution on 
> it. And who using upstream kernels on N900 also want some 
> additional functionality which was not in 2.6.28. And having mass 
> storage in g_nokia is usefull.

it makes no difference.

> Also you can see that this patch simply adding new composite 
> gadget to exisitng driver. Nothing is removed, so original code 
> is compatible with these changes. If somebody still want (for 

you could be breaking phone's own access to the memory card.

> some reason) to switching between g_nokia and g_file_storage (ops, 
> it was renamed to g_mass_storage, so now it is broken on old 
> Nokia systems...) it is still possible.

no it's not, you can anyways make a symbolic link. Besides, before
removing g_file_storage we waited quite some time.

> And when I looked into nokia 2.6.28 kernel, they also patched 
> g_file_storage, so I think it is incompatible with upstream too. 

it's patched only for strings.

> So why to care about current API implementation in upstream 
> kernel (do not allow to add new functionality) which is 
> incompatible with Nokia patched kernel?

Because I don't think g_nokia needs mass storage and because I want to
remove all gadget drivers from kernel (keep only function drivers).

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 2893 bytes --]

On Monday 21 January 2013 09:05:06 Felipe Balbi wrote:
> Hi,
> 
> On Sun, Jan 20, 2013 at 11:17:31AM +0100, Pali Rohár wrote:
> > On Sunday 20 January 2013 10:25:37 Felipe Balbi wrote:
> > > On Sun, Jan 20, 2013 at 03:58:13AM +0100, Pali Rohár wrote:
> > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > 
> > > NAK for two reasons:
> > > 
> > > a) the original Nokia kernel used a separate
> > > g_file_storage gadget to use Mass Storage mode, use that
> > > 
> > > b) there is no commit log
> > 
> > Reason why add mass storage mode to g_nokia is to avoid
> > switching between g_{file,mass}_storage and g_nokia and to
> > have one gadget driver for Nokia N900. It is better to have
> > usb network and mass storage mode in one driver (and not to
> > unload & load another).
> > 
> > I tested this patch with 3.8-rc3 kernel on Nokia N900 and
> > usb network with mass storage mode working without
> > problems.
> 
> Doesn't matter, in this case this is something which nokia
> wrote to carry on their Maemo/MeeGo devices so unless someone
> from Nokia says this is how they want to use nokia.c from now
> on, I can't simply risk breaking all other users for your own
> convenience.
> 

Hello,

you may know that Nokia not working on any linux Maemo/MeeGo 
systems anymore. And also that Nokia devices needs own patched 
kernel. Also g_nokia gadget is for Nokia N900 and this device 
with original Maemo 5 system which was supported by Nokia is 
locked for patched 2.6.28 kernel. More drivers in 2.6.28 was not 
upstreamed, so running other kernels will not work without 
problems. And waiting what Nokia say is now irrelevant, because 
N900 is at end-of-live cycle and Nokia not doing with linux 
devices anymore. So I do not understand why current code cannot 
be extended for more functionality. Because patch is not signed 
by Nokia?

There are more developers which playing with upstream kernel on 
Nokia N900 and trying to use some modern linux distribution on 
it. And who using upstream kernels on N900 also want some 
additional functionality which was not in 2.6.28. And having mass 
storage in g_nokia is usefull.

Also you can see that this patch simply adding new composite 
gadget to exisitng driver. Nothing is removed, so original code 
is compatible with these changes. If somebody still want (for 
some reason) to switching between g_nokia and g_file_storage (ops, 
it was renamed to g_mass_storage, so now it is broken on old 
Nokia systems...) it is still possible.

And when I looked into nokia 2.6.28 kernel, they also patched 
g_file_storage, so I think it is incompatible with upstream too. 
So why to care about current API implementation in upstream 
kernel (do not allow to add new functionality) which is 
incompatible with Nokia patched kernel?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Felipe Balbi]

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

Hi,

On Sun, Jan 20, 2013 at 11:17:31AM +0100, Pali Rohár wrote:
> On Sunday 20 January 2013 10:25:37 Felipe Balbi wrote:
> > On Sun, Jan 20, 2013 at 03:58:13AM +0100, Pali Rohár wrote:
> > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > 
> > NAK for two reasons:
> > 
> > a) the original Nokia kernel used a separate g_file_storage
> > gadget to use Mass Storage mode, use that
> > 
> > b) there is no commit log
> 
> Reason why add mass storage mode to g_nokia is to avoid switching 
> between g_{file,mass}_storage and g_nokia and to have one gadget 
> driver for Nokia N900. It is better to have usb network and mass 
> storage mode in one driver (and not to unload & load another).
> 
> I tested this patch with 3.8-rc3 kernel on Nokia N900 and usb 
> network with mass storage mode working without problems.

Doesn't matter, in this case this is something which nokia wrote to
carry on their Maemo/MeeGo devices so unless someone from Nokia says
this is how they want to use nokia.c from now on, I can't simply risk
breaking all other users for your own convenience.

On top of all that, we're working to remove all gadget drivers from
kernel and keep only function drivers.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 782 bytes --]

On Sunday 20 January 2013 10:25:37 Felipe Balbi wrote:
> On Sun, Jan 20, 2013 at 03:58:13AM +0100, Pali Rohár wrote:
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> 
> NAK for two reasons:
> 
> a) the original Nokia kernel used a separate g_file_storage
> gadget to use Mass Storage mode, use that
> 
> b) there is no commit log

Reason why add mass storage mode to g_nokia is to avoid switching 
between g_{file,mass}_storage and g_nokia and to have one gadget 
driver for Nokia N900. It is better to have usb network and mass 
storage mode in one driver (and not to unload & load another).

I tested this patch with 3.8-rc3 kernel on Nokia N900 and usb 
network with mass storage mode working without problems.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Felipe Balbi]

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

On Sun, Jan 20, 2013 at 03:58:13AM +0100, Pali Rohár wrote:
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

NAK for two reasons:

a) the original Nokia kernel used a separate g_file_storage gadget to
	use Mass Storage mode, use that

b) there is no commit log

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali Rohár]

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/usb/gadget/nokia.c |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/usb/gadget/nokia.c b/drivers/usb/gadget/nokia.c
index 661600a..56409ee 100644
--- a/drivers/usb/gadget/nokia.c
+++ b/drivers/usb/gadget/nokia.c
@@ -38,6 +38,7 @@
  * a "gcc --combine ... part1.c part2.c part3.c ... " build would.
  */
 #include "u_serial.c"
+#include "f_mass_storage.c"
 #include "f_acm.c"
 #include "f_ecm.c"
 #include "f_obex.c"
@@ -99,6 +100,17 @@ MODULE_LICENSE("GPL");
 
 /*-------------------------------------------------------------------------*/
 
+static struct fsg_module_parameters fsg_mod_data = {
+	.stall = 0,
+	.luns = 2,
+	.removable_count = 2,
+	.removable = { 1, 1, },
+};
+
+FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
+
+static struct fsg_common fsg_common;
+
 static u8 hostaddr[ETH_ALEN];
 
 static int __init nokia_bind_config(struct usb_configuration *c)
@@ -125,6 +137,11 @@ static int __init nokia_bind_config(struct usb_configuration *c)
 	if (status)
 		printk(KERN_DEBUG "could not bind ecm config\n");
 
+	status = fsg_bind_config(c->cdev, c, &fsg_common);
+	if (status)
+		printk(KERN_DEBUG "could not bind fsg config\n");
+	fsg_common_put(&fsg_common);
+
 	return status;
 }
 
@@ -148,6 +165,8 @@ static int __init nokia_bind(struct usb_composite_dev *cdev)
 {
 	struct usb_gadget	*gadget = cdev->gadget;
 	int			status;
+	void			*retp;
+	struct fsg_config	fsg_cfg;
 
 	status = gphonet_setup(cdev->gadget);
 	if (status < 0)
@@ -161,6 +180,15 @@ static int __init nokia_bind(struct usb_composite_dev *cdev)
 	if (status < 0)
 		goto err_ether;
 
+	fsg_config_from_params(&fsg_cfg, &fsg_mod_data);
+	fsg_cfg.vendor_name = "Nokia";
+	fsg_cfg.product_name = "N900";
+	retp = fsg_common_init(&fsg_common, cdev, &fsg_cfg);
+	if (IS_ERR(retp)) {
+		status = PTR_ERR(retp);
+		goto err_fsg;
+	}
+
 	status = usb_string_ids_tab(cdev, strings_dev);
 	if (status < 0)
 		goto err_usb;
@@ -190,6 +218,8 @@ static int __init nokia_bind(struct usb_composite_dev *cdev)
 	return 0;
 
 err_usb:
+	fsg_common_put(&fsg_common);
+err_fsg:
 	gether_cleanup();
 err_ether:
 	gserial_cleanup();
-- 
1.7.10.4