From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753794AbbCSBr6 (ORCPT ); Wed, 18 Mar 2015 21:47:58 -0400 Received: from mail-bn1bon0138.outbound.protection.outlook.com ([157.56.111.138]:18456 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750804AbbCSBrz (ORCPT ); Wed, 18 Mar 2015 21:47:55 -0400 Date: Thu, 19 Mar 2015 09:46:51 +0800 From: Peter Chen To: Roger Quadros CC: , , , , , , , , Subject: Re: [RFC][PATCH 1/9] usb: hcd: Introduce usb_start/stop_hcd() Message-ID: <20150319014650.GA1476@shlinux2> References: <1426686963-11613-1-git-send-email-rogerq@ti.com> <1426686963-11613-2-git-send-email-rogerq@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <1426686963-11613-2-git-send-email-rogerq@ti.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 Authentication-Results: spf=fail (sender IP is 192.88.168.50) smtp.mailfrom=peter.chen@freescale.com; vger.kernel.org; dkim=none (message not signed) header.d=none; X-Forefront-Antispam-Report: CIP:192.88.168.50;CTRY:US;IPV:NLI;EFV:NLI;BMV:1;SFV:NSPM;SFS:(10019020)(6009001)(339900001)(189002)(51704005)(199003)(92566002)(104016003)(33716001)(33656002)(97756001)(110136001)(105606002)(50466002)(46102003)(106466001)(50986999)(87936001)(54356999)(85426001)(47776003)(6806004)(77156002)(23726002)(62966003)(77096005)(2950100001)(86362001)(83506001)(19580395003)(19580405001)(46406003);DIR:OUT;SFP:1102;SCL:1;SRVR:BY1PR0301MB1221;H:tx30smr01.am.freescale.net;FPR:;SPF:Fail;MLV:sfv;MX:1;A:1;LANG:en; X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BY1PR0301MB1221; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(5005006)(5002010);SRVR:BY1PR0301MB1221;BCL:0;PCL:0;RULEID:;SRVR:BY1PR0301MB1221; X-Forefront-PRVS: 052017CAF1 X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Mar 2015 01:47:49.6913 (UTC) X-MS-Exchange-CrossTenant-Id: 710a03f5-10f6-4d38-9ff4-a80b81da590d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=710a03f5-10f6-4d38-9ff4-a80b81da590d;Ip=[192.88.168.50];Helo=[tx30smr01.am.freescale.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR0301MB1221 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Mar 18, 2015 at 03:55:55PM +0200, Roger Quadros wrote: > To support OTG we want a mechanism to start and stop > the HCD from the OTG state machine. Add usb_start_hcd() > and usb_stop_hcd(). Hi Roger, You may not need to create another pair of hcd APIs for doing it, you can use usb_add_hcd/usb_remove_hcd directly, it is safer and cleaner. The chipidea uses it for both ID role switch use case and OTG FSM use case. Peter > > Signed-off-by: Roger Quadros > --- > drivers/usb/core/hcd.c | 147 ++++++++++++++++++++++++++++++++---------------- > include/linux/usb/hcd.h | 2 + > 2 files changed, 100 insertions(+), 49 deletions(-) > > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c > index 45a915c..e28bd9d 100644 > --- a/drivers/usb/core/hcd.c > +++ b/drivers/usb/core/hcd.c > @@ -2613,7 +2613,76 @@ static void usb_put_invalidate_rhdev(struct usb_hcd *hcd) > } > > /** > - * usb_add_hcd - finish generic HCD structure initialization and register > + * usb_start_hcd - start the HCD > + * @hcd: the usb_hcd structure to initialize > + * > + * calls the drivers start() routine, registers the root hub > + * and creates the USB sysfs attributes. > + */ > +int usb_start_hcd(struct usb_hcd *hcd) > +{ > + int retval; > + struct usb_device *rhdev = hcd->self.root_hub; > + > + if (hcd->state != HC_STATE_HALT) { > + dev_err(hcd->self.controller, "not starting a running HCD\n"); > + return -EINVAL; > + } > + > + hcd->state = HC_STATE_RUNNING; > + retval = hcd->driver->start(hcd); > + if (retval < 0) { > + dev_err(hcd->self.controller, "startup error %d\n", retval); > + hcd->state = HC_STATE_HALT; > + return retval; > + } > + > + /* starting here, usbcore will pay attention to this root hub */ > + if ((retval = register_root_hub(hcd)) != 0) > + goto err_register_root_hub; > + > + retval = sysfs_create_group(&rhdev->dev.kobj, &usb_bus_attr_group); > + if (retval < 0) { > + pr_err("Cannot register USB bus sysfs attributes: %d\n", > + retval); > + goto error_create_attr_group; > + } > + if (hcd->uses_new_polling && HCD_POLL_RH(hcd)) > + usb_hcd_poll_rh_status(hcd); > + > + return retval; > + > +error_create_attr_group: > + clear_bit(HCD_FLAG_RH_RUNNING, &hcd->flags); > + if (HC_IS_RUNNING(hcd->state)) > + hcd->state = HC_STATE_QUIESCING; > + spin_lock_irq(&hcd_root_hub_lock); > + hcd->rh_registered = 0; > + spin_unlock_irq(&hcd_root_hub_lock); > + > +#ifdef CONFIG_PM > + cancel_work_sync(&hcd->wakeup_work); > +#endif > + mutex_lock(&usb_bus_list_lock); > + usb_disconnect(&rhdev); /* Sets rhdev to NULL */ > + mutex_unlock(&usb_bus_list_lock); > +err_register_root_hub: > + hcd->rh_pollable = 0; > + clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); > + del_timer_sync(&hcd->rh_timer); > + hcd->driver->stop(hcd); > + hcd->state = HC_STATE_HALT; > + > + /* In case the HCD restarted the timer, stop it again. */ > + clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); > + del_timer_sync(&hcd->rh_timer); > + > + return retval; > +} > +EXPORT_SYMBOL_GPL(usb_start_hcd); > + > +/** > + * usb_add_hcd - finish generic HCD structure initialization and register. > * @hcd: the usb_hcd structure to initialize > * @irqnum: Interrupt line to allocate > * @irqflags: Interrupt type flags > @@ -2757,50 +2826,12 @@ int usb_add_hcd(struct usb_hcd *hcd, > goto err_request_irq; > } > > - hcd->state = HC_STATE_RUNNING; > - retval = hcd->driver->start(hcd); > - if (retval < 0) { > - dev_err(hcd->self.controller, "startup error %d\n", retval); > + retval = usb_start_hcd(hcd); > + if (retval) > goto err_hcd_driver_start; > - } > - > - /* starting here, usbcore will pay attention to this root hub */ > - if ((retval = register_root_hub(hcd)) != 0) > - goto err_register_root_hub; > - > - retval = sysfs_create_group(&rhdev->dev.kobj, &usb_bus_attr_group); > - if (retval < 0) { > - printk(KERN_ERR "Cannot register USB bus sysfs attributes: %d\n", > - retval); > - goto error_create_attr_group; > - } > - if (hcd->uses_new_polling && HCD_POLL_RH(hcd)) > - usb_hcd_poll_rh_status(hcd); > - > - return retval; > > -error_create_attr_group: > - clear_bit(HCD_FLAG_RH_RUNNING, &hcd->flags); > - if (HC_IS_RUNNING(hcd->state)) > - hcd->state = HC_STATE_QUIESCING; > - spin_lock_irq(&hcd_root_hub_lock); > - hcd->rh_registered = 0; > - spin_unlock_irq(&hcd_root_hub_lock); > + return 0; > > -#ifdef CONFIG_PM > - cancel_work_sync(&hcd->wakeup_work); > -#endif > - mutex_lock(&usb_bus_list_lock); > - usb_disconnect(&rhdev); /* Sets rhdev to NULL */ > - mutex_unlock(&usb_bus_list_lock); > -err_register_root_hub: > - hcd->rh_pollable = 0; > - clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); > - del_timer_sync(&hcd->rh_timer); > - hcd->driver->stop(hcd); > - hcd->state = HC_STATE_HALT; > - clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); > - del_timer_sync(&hcd->rh_timer); > err_hcd_driver_start: > if (usb_hcd_is_primary_hcd(hcd) && hcd->irq > 0) > free_irq(irqnum, hcd); > @@ -2830,18 +2861,20 @@ err_phy: > EXPORT_SYMBOL_GPL(usb_add_hcd); > > /** > - * usb_remove_hcd - shutdown processing for generic HCDs > - * @hcd: the usb_hcd structure to remove > - * Context: !in_interrupt() > + * usb_stop_hcd - stop the HCD > + * @hcd: the usb_hcd structure to initialize > * > - * Disconnects the root hub, then reverses the effects of usb_add_hcd(), > - * invoking the HCD's stop() method. > + * removes the USB sysfs attributes, disconnects the root hub > + * and calls the driver's stop() routine. > */ > -void usb_remove_hcd(struct usb_hcd *hcd) > +void usb_stop_hcd(struct usb_hcd *hcd) > { > struct usb_device *rhdev = hcd->self.root_hub; > > - dev_info(hcd->self.controller, "remove, state %x\n", hcd->state); > + if (hcd->state == HC_STATE_HALT) { > + dev_err(hcd->self.controller, "not stopping a halted HCD\n"); > + return; > + } > > usb_get_dev(rhdev); > sysfs_remove_group(&rhdev->dev.kobj, &usb_bus_attr_group); > @@ -2888,6 +2921,22 @@ void usb_remove_hcd(struct usb_hcd *hcd) > /* In case the HCD restarted the timer, stop it again. */ > clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); > del_timer_sync(&hcd->rh_timer); > +} > +EXPORT_SYMBOL_GPL(usb_stop_hcd); > + > +/** > + * usb_remove_hcd - shutdown processing for generic HCDs > + * @hcd: the usb_hcd structure to remove > + * Context: !in_interrupt() > + * > + * Disconnects the root hub, then reverses the effects of usb_add_hcd(), > + * invoking the HCD's stop() method. > + */ > +void usb_remove_hcd(struct usb_hcd *hcd) > +{ > + dev_info(hcd->self.controller, "remove, state %x\n", hcd->state); > + > + usb_stop_hcd(hcd); > > if (usb_hcd_is_primary_hcd(hcd)) { > if (hcd->irq > 0) > diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h > index 68b1e83..12aaf4c 100644 > --- a/include/linux/usb/hcd.h > +++ b/include/linux/usb/hcd.h > @@ -433,6 +433,8 @@ extern void usb_put_hcd(struct usb_hcd *hcd); > extern int usb_hcd_is_primary_hcd(struct usb_hcd *hcd); > extern int usb_add_hcd(struct usb_hcd *hcd, > unsigned int irqnum, unsigned long irqflags); > +extern int usb_start_hcd(struct usb_hcd *hcd); > +extern void usb_stop_hcd(struct usb_hcd *hcd); > extern void usb_remove_hcd(struct usb_hcd *hcd); > extern int usb_hcd_find_raw_port_number(struct usb_hcd *hcd, int port1); > > -- > 2.1.0 > -- Best Regards, Peter Chen From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Chen Subject: Re: [RFC][PATCH 1/9] usb: hcd: Introduce usb_start/stop_hcd() Date: Thu, 19 Mar 2015 09:46:51 +0800 Message-ID: <20150319014650.GA1476@shlinux2> References: <1426686963-11613-1-git-send-email-rogerq@ti.com> <1426686963-11613-2-git-send-email-rogerq@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Return-path: Content-Disposition: inline In-Reply-To: <1426686963-11613-2-git-send-email-rogerq@ti.com> Sender: linux-kernel-owner@vger.kernel.org To: Roger Quadros Cc: gregkh@linuxfoundation.org, balbi@ti.com, stern@rowland.harvard.edu, dan.j.williams@intel.com, jun.li@freescale.com, mathias.nyman@linux.intel.com, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org List-Id: linux-omap@vger.kernel.org On Wed, Mar 18, 2015 at 03:55:55PM +0200, Roger Quadros wrote: > To support OTG we want a mechanism to start and stop > the HCD from the OTG state machine. Add usb_start_hcd() > and usb_stop_hcd(). Hi Roger, You may not need to create another pair of hcd APIs for doing it, you can use usb_add_hcd/usb_remove_hcd directly, it is safer and cleaner. The chipidea uses it for both ID role switch use case and OTG FSM use case. Peter > > Signed-off-by: Roger Quadros > --- > drivers/usb/core/hcd.c | 147 ++++++++++++++++++++++++++++++++---------------- > include/linux/usb/hcd.h | 2 + > 2 files changed, 100 insertions(+), 49 deletions(-) > > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c > index 45a915c..e28bd9d 100644 > --- a/drivers/usb/core/hcd.c > +++ b/drivers/usb/core/hcd.c > @@ -2613,7 +2613,76 @@ static void usb_put_invalidate_rhdev(struct usb_hcd *hcd) > } > > /** > - * usb_add_hcd - finish generic HCD structure initialization and register > + * usb_start_hcd - start the HCD > + * @hcd: the usb_hcd structure to initialize > + * > + * calls the drivers start() routine, registers the root hub > + * and creates the USB sysfs attributes. > + */ > +int usb_start_hcd(struct usb_hcd *hcd) > +{ > + int retval; > + struct usb_device *rhdev = hcd->self.root_hub; > + > + if (hcd->state != HC_STATE_HALT) { > + dev_err(hcd->self.controller, "not starting a running HCD\n"); > + return -EINVAL; > + } > + > + hcd->state = HC_STATE_RUNNING; > + retval = hcd->driver->start(hcd); > + if (retval < 0) { > + dev_err(hcd->self.controller, "startup error %d\n", retval); > + hcd->state = HC_STATE_HALT; > + return retval; > + } > + > + /* starting here, usbcore will pay attention to this root hub */ > + if ((retval = register_root_hub(hcd)) != 0) > + goto err_register_root_hub; > + > + retval = sysfs_create_group(&rhdev->dev.kobj, &usb_bus_attr_group); > + if (retval < 0) { > + pr_err("Cannot register USB bus sysfs attributes: %d\n", > + retval); > + goto error_create_attr_group; > + } > + if (hcd->uses_new_polling && HCD_POLL_RH(hcd)) > + usb_hcd_poll_rh_status(hcd); > + > + return retval; > + > +error_create_attr_group: > + clear_bit(HCD_FLAG_RH_RUNNING, &hcd->flags); > + if (HC_IS_RUNNING(hcd->state)) > + hcd->state = HC_STATE_QUIESCING; > + spin_lock_irq(&hcd_root_hub_lock); > + hcd->rh_registered = 0; > + spin_unlock_irq(&hcd_root_hub_lock); > + > +#ifdef CONFIG_PM > + cancel_work_sync(&hcd->wakeup_work); > +#endif > + mutex_lock(&usb_bus_list_lock); > + usb_disconnect(&rhdev); /* Sets rhdev to NULL */ > + mutex_unlock(&usb_bus_list_lock); > +err_register_root_hub: > + hcd->rh_pollable = 0; > + clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); > + del_timer_sync(&hcd->rh_timer); > + hcd->driver->stop(hcd); > + hcd->state = HC_STATE_HALT; > + > + /* In case the HCD restarted the timer, stop it again. */ > + clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); > + del_timer_sync(&hcd->rh_timer); > + > + return retval; > +} > +EXPORT_SYMBOL_GPL(usb_start_hcd); > + > +/** > + * usb_add_hcd - finish generic HCD structure initialization and register. > * @hcd: the usb_hcd structure to initialize > * @irqnum: Interrupt line to allocate > * @irqflags: Interrupt type flags > @@ -2757,50 +2826,12 @@ int usb_add_hcd(struct usb_hcd *hcd, > goto err_request_irq; > } > > - hcd->state = HC_STATE_RUNNING; > - retval = hcd->driver->start(hcd); > - if (retval < 0) { > - dev_err(hcd->self.controller, "startup error %d\n", retval); > + retval = usb_start_hcd(hcd); > + if (retval) > goto err_hcd_driver_start; > - } > - > - /* starting here, usbcore will pay attention to this root hub */ > - if ((retval = register_root_hub(hcd)) != 0) > - goto err_register_root_hub; > - > - retval = sysfs_create_group(&rhdev->dev.kobj, &usb_bus_attr_group); > - if (retval < 0) { > - printk(KERN_ERR "Cannot register USB bus sysfs attributes: %d\n", > - retval); > - goto error_create_attr_group; > - } > - if (hcd->uses_new_polling && HCD_POLL_RH(hcd)) > - usb_hcd_poll_rh_status(hcd); > - > - return retval; > > -error_create_attr_group: > - clear_bit(HCD_FLAG_RH_RUNNING, &hcd->flags); > - if (HC_IS_RUNNING(hcd->state)) > - hcd->state = HC_STATE_QUIESCING; > - spin_lock_irq(&hcd_root_hub_lock); > - hcd->rh_registered = 0; > - spin_unlock_irq(&hcd_root_hub_lock); > + return 0; > > -#ifdef CONFIG_PM > - cancel_work_sync(&hcd->wakeup_work); > -#endif > - mutex_lock(&usb_bus_list_lock); > - usb_disconnect(&rhdev); /* Sets rhdev to NULL */ > - mutex_unlock(&usb_bus_list_lock); > -err_register_root_hub: > - hcd->rh_pollable = 0; > - clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); > - del_timer_sync(&hcd->rh_timer); > - hcd->driver->stop(hcd); > - hcd->state = HC_STATE_HALT; > - clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); > - del_timer_sync(&hcd->rh_timer); > err_hcd_driver_start: > if (usb_hcd_is_primary_hcd(hcd) && hcd->irq > 0) > free_irq(irqnum, hcd); > @@ -2830,18 +2861,20 @@ err_phy: > EXPORT_SYMBOL_GPL(usb_add_hcd); > > /** > - * usb_remove_hcd - shutdown processing for generic HCDs > - * @hcd: the usb_hcd structure to remove > - * Context: !in_interrupt() > + * usb_stop_hcd - stop the HCD > + * @hcd: the usb_hcd structure to initialize > * > - * Disconnects the root hub, then reverses the effects of usb_add_hcd(), > - * invoking the HCD's stop() method. > + * removes the USB sysfs attributes, disconnects the root hub > + * and calls the driver's stop() routine. > */ > -void usb_remove_hcd(struct usb_hcd *hcd) > +void usb_stop_hcd(struct usb_hcd *hcd) > { > struct usb_device *rhdev = hcd->self.root_hub; > > - dev_info(hcd->self.controller, "remove, state %x\n", hcd->state); > + if (hcd->state == HC_STATE_HALT) { > + dev_err(hcd->self.controller, "not stopping a halted HCD\n"); > + return; > + } > > usb_get_dev(rhdev); > sysfs_remove_group(&rhdev->dev.kobj, &usb_bus_attr_group); > @@ -2888,6 +2921,22 @@ void usb_remove_hcd(struct usb_hcd *hcd) > /* In case the HCD restarted the timer, stop it again. */ > clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); > del_timer_sync(&hcd->rh_timer); > +} > +EXPORT_SYMBOL_GPL(usb_stop_hcd); > + > +/** > + * usb_remove_hcd - shutdown processing for generic HCDs > + * @hcd: the usb_hcd structure to remove > + * Context: !in_interrupt() > + * > + * Disconnects the root hub, then reverses the effects of usb_add_hcd(), > + * invoking the HCD's stop() method. > + */ > +void usb_remove_hcd(struct usb_hcd *hcd) > +{ > + dev_info(hcd->self.controller, "remove, state %x\n", hcd->state); > + > + usb_stop_hcd(hcd); > > if (usb_hcd_is_primary_hcd(hcd)) { > if (hcd->irq > 0) > diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h > index 68b1e83..12aaf4c 100644 > --- a/include/linux/usb/hcd.h > +++ b/include/linux/usb/hcd.h > @@ -433,6 +433,8 @@ extern void usb_put_hcd(struct usb_hcd *hcd); > extern int usb_hcd_is_primary_hcd(struct usb_hcd *hcd); > extern int usb_add_hcd(struct usb_hcd *hcd, > unsigned int irqnum, unsigned long irqflags); > +extern int usb_start_hcd(struct usb_hcd *hcd); > +extern void usb_stop_hcd(struct usb_hcd *hcd); > extern void usb_remove_hcd(struct usb_hcd *hcd); > extern int usb_hcd_find_raw_port_number(struct usb_hcd *hcd, int port1); > > -- > 2.1.0 > -- Best Regards, Peter Chen