From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756879AbcIVMxX (ORCPT ); Thu, 22 Sep 2016 08:53:23 -0400 Received: from mga04.intel.com ([192.55.52.120]:65162 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751586AbcIVMxU (ORCPT ); Thu, 22 Sep 2016 08:53:20 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,378,1470726000"; d="scan'208";a="1060468226" From: Felipe Balbi To: Baolin Wang , Greg KH Cc: Badhri Jagan Sridharan , Mark Brown , USB , LKML Subject: Re: [PATCH v2] usb: gadget: Add uevent to notify userspace In-Reply-To: References: <20160922122334.GA2301@kroah.com> User-Agent: Notmuch/0.22.1+63~g994277e (https://notmuchmail.org) Emacs/25.1.3 (x86_64-pc-linux-gnu) Date: Thu, 22 Sep 2016 15:53:14 +0300 Message-ID: <878tuk6qpx.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Baolin Wang writes: >>> static const struct usb_gadget_driver configfs_driver_template = { >>> .bind = configfs_composite_bind, >>> .unbind = configfs_composite_unbind, >>> >>> +#ifdef CONFIG_USB_CONFIGFS_UEVENT >>> + .setup = configfs_setup, >>> + .reset = configfs_disconnect, >>> + .disconnect = configfs_disconnect, >>> +#else >>> .setup = composite_setup, >>> .reset = composite_disconnect, >>> .disconnect = composite_disconnect, >>> +#endif nope, this is quite wrong. >>> @@ -1453,6 +1556,10 @@ static struct config_group *gadgets_make( >>> gi->composite.gadget_driver.function = kstrdup(name, GFP_KERNEL); >>> gi->composite.name = gi->composite.gadget_driver.function; >>> >>> +#ifdef CONFIG_USB_CONFIGFS_UEVENT >>> + INIT_WORK(&gi->work, configfs_work); >>> +#endif >> >> This is just way too ugly, please make it so there are no #ifdefs in the >> .c files. >> >> Or, as others said, why is this a build option at all, why would you not >> always want this enabled if you are relying on it all of the time? > > Sometimes userspace does not need the notification, it is not all the > time. Anyway I will remove the macro if you still insist on that. what's wrong with the sysfs we already have for this? -- balbi