From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754005AbbE1OvV (ORCPT ); Thu, 28 May 2015 10:51:21 -0400 Received: from mailout3.w1.samsung.com ([210.118.77.13]:23496 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752625AbbE1OvL (ORCPT ); Thu, 28 May 2015 10:51:11 -0400 MIME-version: 1.0 Content-type: text/plain; charset=utf-8; format=flowed X-AuditID: cbfec7f5-f794b6d000001495-d0-55672b5c7989 Content-transfer-encoding: 8BIT Message-id: <55672B5B.2080606@samsung.com> Date: Thu, 28 May 2015 16:51:07 +0200 From: Krzysztof Opasiak User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 To: =?UTF-8?B?UGFsaSBSb2jDoXI=?= Cc: Felipe Balbi , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Pavel Machek , Sebastian Reichel , Aaro Koskinen , Ivaylo Dimitrov Subject: Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia References: <1422698010-2907-1-git-send-email-pali.rohar@gmail.com> <20150528074719.GF16509@pali> <556725E0.3070904@samsung.com> <20150528143105.GQ16509@pali> In-reply-to: <20150528143105.GQ16509@pali> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrBLMWRmVeSWpSXmKPExsVy+t/xy7ox2umhBls/y1useeFgcfB+vUXz 4vVsFkuuHGK3uLxrDpvFomWtzBYTT/9msrh76iibxendJQ6cHjtn3WX3OPx1IYvHplWdbB77 565h9zh+YzuTx4rV39k9Pm+SC2CP4rJJSc3JLEst0rdL4Mr4Ndel4LVvxd3t/1kaGO/bdzFy ckgImEjsufeVFcIWk7hwbz1bFyMXh5DAUkaJ9t1v2UESvAKCEj8m32PpYuTgYBaQlzhyKRsk zCxgJvHl5WGwXiGB54wS1xfwQpRrSdy9fYUNxGYRUJWYcXcfWCubgL7EvF2iIGFRgQiJ+cde M4OERQTMJV7PZQTZyiywnEliyvfrzCA1wgLeEn/W/2CGOGcRo8SpN1/AGjiB5m9qMJ7AKDAL yXGzEI6bheS4BYzMqxhFU0uTC4qT0nON9IoTc4tL89L1kvNzNzFCwv/rDsalx6wOMQpwMCrx 8CrMSQsVYk0sK67MPcQowcGsJMLbI54eKsSbklhZlVqUH19UmpNafIhRmoNFSZx35q73IUIC 6YklqdmpqQWpRTBZJg5OqQbGPTnKPL0vplUt3LI+snwf84qe0z5mSYz3wr/FGy7cuTJgu0/N r6OKUWzCxgybs/t6qzzyPwZv/336TsmMvbG7uR+EBbtbLmX6uugHu+/L0KxHJRlbnjV0m/Kb /3jcyTvH+InEspklb1eV8a6NuRluo7Mw9I3rnPv9x5Rc1oZmbtQJi5k+k0NIiaU4I9FQi7mo OBEA2EvUBXsCAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >>>> --- >>>> 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