From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932227AbbE1O16 (ORCPT ); Thu, 28 May 2015 10:27:58 -0400 Received: from mailout3.w1.samsung.com ([210.118.77.13]:22074 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753773AbbE1O1t (ORCPT ); Thu, 28 May 2015 10:27:49 -0400 MIME-version: 1.0 Content-type: text/plain; charset=utf-8; format=flowed X-AuditID: cbfec7f5-f794b6d000001495-fd-556725e2a6d9 Content-transfer-encoding: 8BIT Message-id: <556725E0.3070904@samsung.com> Date: Thu, 28 May 2015 16:27:44 +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=?= , Felipe Balbi , Greg Kroah-Hartman Cc: 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> In-reply-to: <20150528074719.GF16509@pali> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrNLMWRmVeSWpSXmKPExsVy+t/xa7qPVNNDDe6eErdY88LB4uD9eovm xevZLJZcOcRucXnXHDaLRctamS0mnv7NZHH31FE2i9O7Sxw4PXbOusvucfjrQhaPTas62Tz2 z13D7nH8xnYmjxWrv7N7fN4kF8AexWWTkpqTWZZapG+XwJXxYusxpoJ9bhVX1u9jbGDstuxi 5OSQEDCRaF1xlhXCFpO4cG89WxcjF4eQwFJGiV+vL4EleAUEJX5MvsfSxcjBwSwgL3HkUjZI mFnATOLLy8OsEPXPGSX2zV3HBlLDK6Alsf+GAUgNi4CqxIMNL1lBwmwC+hLzdomChEUFIiTm H3vNDNIqItDOKLH6wRSwOcwC1xkleruWMoFUCQt4S/xZ/4MZxBYSSJY4Mnc/2D2cQPN37PjL OoFRYBaS82YhnDcLyXkLGJlXMYqmliYXFCel5xrpFSfmFpfmpesl5+duYoREwdcdjEuPWR1i FOBgVOLhVZiTFirEmlhWXJl7iFGCg1lJhLdHPD1UiDclsbIqtSg/vqg0J7X4EKM0B4uSOO/M Xe9DhATSE0tSs1NTC1KLYLJMHJxSDYyXO/qz5J/mB1Y9MWz9msBp/yIuSdPH3XQOz4tnDXe+ veP7vGzfBS/t7Hd1M66HvtANn2jXMv13ihlLfnPB1aOf17j6qEduOs7xsU6kNG2xVvevt7rc qqsEcw+/uyzxsWp7y8mI6SvSX7PrqKkZZDC1VgT/tvqh6y73NEnIp/1f8ZFEnrYnqkosxRmJ hlrMRcWJAOmhn6l+AgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics