Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754131AbbHHGXd (ORCPT ); Sat, 8 Aug 2015 02:23:33 -0400 Received: from mail-yk0-f179.google.com ([209.85.160.179]:35265 "EHLO mail-yk0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754086AbbHHGXb (ORCPT ); Sat, 8 Aug 2015 02:23:31 -0400 MIME-Version: 1.0 In-Reply-To: <20150807175317.GD10003@kroah.com> References: <8fcb9ff7a8e01bec53813f0702ff606bf4982943.1438844454.git.baolin.wang@linaro.org> <20150807054554.GE13200@shlinux2> <20150807175317.GD10003@kroah.com> Date: Sat, 8 Aug 2015 14:23:31 +0800 Message-ID: Subject: Re: [PATCH 2/2] gadget: Support for the usb charger framework From: Baolin Wang To: Greg KH Cc: Peter Chen , Felipe Balbi , Mark Brown , LKML , "sojka@merica.cz" , Alan Stern , "andreas@gaisler.com" , "linux-usb@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2030 Lines: 58 On 8 August 2015 at 01:53, Greg KH wrote: > On Fri, Aug 07, 2015 at 05:22:47PM +0800, Baolin Wang wrote: >> On 7 August 2015 at 17:07, Peter Chen wrote: >> > >> >> >> /** >> >> >> * struct usb_udc - describes one usb device controller @@ -127,12 >> >> >> +128,45 @@ void usb_gadget_giveback_request(struct usb_ep *ep, } >> >> >> EXPORT_SYMBOL_GPL(usb_gadget_giveback_request); >> >> >> >> >> >> +int usb_gadget_register_notify(struct usb_gadget *gadget, >> >> >> + struct notifier_block *nb) { >> >> >> + unsigned long flags; >> >> >> + int ret; >> >> >> + >> >> >> + spin_lock_irqsave(&gadget->lock, flags); >> >> > >> >> > I find you use so many spin_lock_irqsave, any reasons for that? >> >> > Why mutex_lock can't be used? >> >> > >> >> >> >> The spin_lock_irqsave() can make it as a atomic notifier, that can make sure the >> >> gadget state event can be quickly reported to the user who register a notifier >> >> on the gadget device. Is it OK? >> >> >> > >> > I don't think it is a good reason, spin_lock_irqsave is usually used for protecting >> > data which is accessed at atomic environment. >> > >> >> Yes, we want the notify process is a atomic environment which do not >> want to be interrupted by irq or other things to make the notice can >> be quickly reported to the user. > > No, this is a "slow" event, you don't need to notify anyone under atomic > context, that's crazy. > >> Also i think the notify process is less cost, thus i use the spinlock. Thanks. > > No, use a mutex please, that's the safe thing. This is not > time-critical code at all. > OK, Thanks for your comments and will fix the lock thing. > thanks, > > greg k-h -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/