diff mbox series

[RFC,1/5] usb: core: hcd: Modularize HCD stop configuration in usb_stop_hcd()

Message ID 20210824105302.25382-2-kishon@ti.com
State Superseded
Headers show
Series Fix cold plugged USB device on certain PCIe USB cards | expand

Commit Message

Kishon Vijay Abraham I Aug. 24, 2021, 10:52 a.m. UTC
No functional change. Since configuration to stop HCD is invoked from
multiple places, group all of them in usb_stop_hcd().

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/usb/core/hcd.c | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

Comments

Greg KH Aug. 24, 2021, 1:06 p.m. UTC | #1
On Tue, Aug 24, 2021 at 04:22:58PM +0530, Kishon Vijay Abraham I wrote:
> No functional change. Since configuration to stop HCD is invoked from
> multiple places, group all of them in usb_stop_hcd().
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/usb/core/hcd.c | 42 +++++++++++++++++++++++++-----------------
>  1 file changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 0f8b7c93310e..c036ba5311b3 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2760,6 +2760,29 @@ static void usb_put_invalidate_rhdev(struct usb_hcd *hcd)
>  	usb_put_dev(rhdev);
>  }
>  
> +/**
> + * usb_stop_hcd - Halt the HCD
> + * @hcd: the usb_hcd that has to be halted
> + *
> + * Stop the timer and invoke ->stop() callback on the HCD
> + */
> +static void usb_stop_hcd(struct usb_hcd *hcd)
> +{
> +	if (!hcd)
> +		return;

That's impossible to hit, so no need to check for it, right?

thanks,

greg k-h
Kishon Vijay Abraham I Aug. 24, 2021, 3:18 p.m. UTC | #2
Hi Greg,

On 24/08/21 6:36 pm, Greg Kroah-Hartman wrote:
> On Tue, Aug 24, 2021 at 04:22:58PM +0530, Kishon Vijay Abraham I wrote:
>> No functional change. Since configuration to stop HCD is invoked from
>> multiple places, group all of them in usb_stop_hcd().
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  drivers/usb/core/hcd.c | 42 +++++++++++++++++++++++++-----------------
>>  1 file changed, 25 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 0f8b7c93310e..c036ba5311b3 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -2760,6 +2760,29 @@ static void usb_put_invalidate_rhdev(struct usb_hcd *hcd)
>>  	usb_put_dev(rhdev);
>>  }
>>  
>> +/**
>> + * usb_stop_hcd - Halt the HCD
>> + * @hcd: the usb_hcd that has to be halted
>> + *
>> + * Stop the timer and invoke ->stop() callback on the HCD
>> + */
>> +static void usb_stop_hcd(struct usb_hcd *hcd)
>> +{
>> +	if (!hcd)
>> +		return;
> 
> That's impossible to hit, so no need to check for it, right?

Patch 3 of this series adds support for registering roothub of shared
HCD. So after that patch there can be a case where shared_hcd is NULL.
The other option would be to check for non-null value in hcd and then
invoke usb_stop_hcd().

Thanks
Kishon
Greg KH Aug. 26, 2021, 11:14 a.m. UTC | #3
On Tue, Aug 24, 2021 at 08:48:45PM +0530, Kishon Vijay Abraham I wrote:
> Hi Greg,

> 

> On 24/08/21 6:36 pm, Greg Kroah-Hartman wrote:

> > On Tue, Aug 24, 2021 at 04:22:58PM +0530, Kishon Vijay Abraham I wrote:

> >> No functional change. Since configuration to stop HCD is invoked from

> >> multiple places, group all of them in usb_stop_hcd().

> >>

> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

> >> ---

> >>  drivers/usb/core/hcd.c | 42 +++++++++++++++++++++++++-----------------

> >>  1 file changed, 25 insertions(+), 17 deletions(-)

> >>

> >> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c

> >> index 0f8b7c93310e..c036ba5311b3 100644

> >> --- a/drivers/usb/core/hcd.c

> >> +++ b/drivers/usb/core/hcd.c

> >> @@ -2760,6 +2760,29 @@ static void usb_put_invalidate_rhdev(struct usb_hcd *hcd)

> >>  	usb_put_dev(rhdev);

> >>  }

> >>  

> >> +/**

> >> + * usb_stop_hcd - Halt the HCD

> >> + * @hcd: the usb_hcd that has to be halted

> >> + *

> >> + * Stop the timer and invoke ->stop() callback on the HCD

> >> + */

> >> +static void usb_stop_hcd(struct usb_hcd *hcd)

> >> +{

> >> +	if (!hcd)

> >> +		return;

> > 

> > That's impossible to hit, so no need to check for it, right?

> 

> Patch 3 of this series adds support for registering roothub of shared

> HCD. So after that patch there can be a case where shared_hcd is NULL.

> The other option would be to check for non-null value in hcd and then

> invoke usb_stop_hcd().


Then add the check when you need it please.

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 0f8b7c93310e..c036ba5311b3 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2760,6 +2760,29 @@  static void usb_put_invalidate_rhdev(struct usb_hcd *hcd)
 	usb_put_dev(rhdev);
 }
 
+/**
+ * usb_stop_hcd - Halt the HCD
+ * @hcd: the usb_hcd that has to be halted
+ *
+ * Stop the timer and invoke ->stop() callback on the HCD
+ */
+static void usb_stop_hcd(struct usb_hcd *hcd)
+{
+	if (!hcd)
+		return;
+
+	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);
+}
+
 /**
  * usb_add_hcd - finish generic HCD structure initialization and register
  * @hcd: the usb_hcd structure to initialize
@@ -2946,13 +2969,7 @@  int usb_add_hcd(struct usb_hcd *hcd,
 	return retval;
 
 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);
+	usb_stop_hcd(hcd);
 err_hcd_driver_start:
 	if (usb_hcd_is_primary_hcd(hcd) && hcd->irq > 0)
 		free_irq(irqnum, hcd);
@@ -3022,16 +3039,7 @@  void usb_remove_hcd(struct usb_hcd *hcd)
 	 * interrupt occurs), but usb_hcd_poll_rh_status() won't invoke
 	 * the hub_status_data() callback.
 	 */
-	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);
+	usb_stop_hcd(hcd);
 
 	if (usb_hcd_is_primary_hcd(hcd)) {
 		if (hcd->irq > 0)