linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] USB: ohci-at91: add a reset function to fix race condition
@ 2012-05-09  8:48 Nicolas Ferre
  2012-05-09 17:22 ` Sergei Shtylyov
  2012-05-09 17:45 ` Alan Stern
  0 siblings, 2 replies; 4+ messages in thread
From: Nicolas Ferre @ 2012-05-09  8:48 UTC (permalink / raw)
  To: linux-arm-kernel

A possible race condition appears because we are not initializing
the ohci->regs before calling usb_hcd_request_irqs().
We move the call to ohci_init() in hcd->driver->reset() instead of
hcd->driver->start() to fix this.
This was experienced when we share the same IRQ line between OHCI and EHCI
controllers.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Tested-by: Christian Eggers <christian.eggers@kathrein.de>
Cc: stable <stable@vger.kernel.org>
---
 drivers/usb/host/ohci-at91.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 13ebeca..55d3d64 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -223,7 +223,7 @@ static void __devexit usb_hcd_at91_remove(struct usb_hcd *hcd,
 /*-------------------------------------------------------------------------*/
 
 static int __devinit
-ohci_at91_start (struct usb_hcd *hcd)
+ohci_at91_reset (struct usb_hcd *hcd)
 {
 	struct at91_usbh_data	*board = hcd->self.controller->platform_data;
 	struct ohci_hcd		*ohci = hcd_to_ohci (hcd);
@@ -233,6 +233,14 @@ ohci_at91_start (struct usb_hcd *hcd)
 		return ret;
 
 	ohci->num_ports = board->ports;
+	return 0;
+}
+
+static int __devinit
+ohci_at91_start (struct usb_hcd *hcd)
+{
+	struct ohci_hcd		*ohci = hcd_to_ohci (hcd);
+	int			ret;
 
 	if ((ret = ohci_run(ohci)) < 0) {
 		err("can't start %s", hcd->self.bus_name);
@@ -418,6 +426,7 @@ static const struct hc_driver ohci_at91_hc_driver = {
 	/*
 	 * basic lifecycle operations
 	 */
+	.reset =		ohci_at91_reset,
 	.start =		ohci_at91_start,
 	.stop =			ohci_stop,
 	.shutdown =		ohci_shutdown,
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH] USB: ohci-at91: add a reset function to fix race condition
  2012-05-09  8:48 [PATCH] USB: ohci-at91: add a reset function to fix race condition Nicolas Ferre
@ 2012-05-09 17:22 ` Sergei Shtylyov
  2012-05-10  7:38   ` Nicolas Ferre
  2012-05-09 17:45 ` Alan Stern
  1 sibling, 1 reply; 4+ messages in thread
From: Sergei Shtylyov @ 2012-05-09 17:22 UTC (permalink / raw)
  To: linux-arm-kernel

Hello.

On 09-05-2012 12:48, Nicolas Ferre wrote:

> A possible race condition appears because we are not initializing
> the ohci->regs before calling usb_hcd_request_irqs().
> We move the call to ohci_init() in hcd->driver->reset() instead of
> hcd->driver->start() to fix this.
> This was experienced when we share the same IRQ line between OHCI and EHCI
> controllers.

> Signed-off-by: Nicolas Ferre<nicolas.ferre@atmel.com>
> Tested-by: Christian Eggers<christian.eggers@kathrein.de>
> Cc: stable<stable@vger.kernel.org>
> ---
>   drivers/usb/host/ohci-at91.c |   11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)

> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index 13ebeca..55d3d64 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -223,7 +223,7 @@ static void __devexit usb_hcd_at91_remove(struct usb_hcd *hcd,
>   /*-------------------------------------------------------------------------*/
>
>   static int __devinit
> -ohci_at91_start (struct usb_hcd *hcd)
> +ohci_at91_reset (struct usb_hcd *hcd)

    Have you run the patch thru scripts/checkpatch.pl? There should be no 
space between function name and '('.

>   {
>   	struct at91_usbh_data	*board = hcd->self.controller->platform_data;
>   	struct ohci_hcd		*ohci = hcd_to_ohci (hcd);
> @@ -233,6 +233,14 @@ ohci_at91_start (struct usb_hcd *hcd)
>   		return ret;
>
>   	ohci->num_ports = board->ports;
> +	return 0;
> +}
> +
> +static int __devinit
> +ohci_at91_start (struct usb_hcd *hcd)

    Same here.

> +{
> +	struct ohci_hcd		*ohci = hcd_to_ohci (hcd);

    And here.

WBR, Sergei

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] USB: ohci-at91: add a reset function to fix race condition
  2012-05-09  8:48 [PATCH] USB: ohci-at91: add a reset function to fix race condition Nicolas Ferre
  2012-05-09 17:22 ` Sergei Shtylyov
@ 2012-05-09 17:45 ` Alan Stern
  1 sibling, 0 replies; 4+ messages in thread
From: Alan Stern @ 2012-05-09 17:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 9 May 2012, Nicolas Ferre wrote:

> A possible race condition appears because we are not initializing
> the ohci->regs before calling usb_hcd_request_irqs().
> We move the call to ohci_init() in hcd->driver->reset() instead of
> hcd->driver->start() to fix this.
> This was experienced when we share the same IRQ line between OHCI and EHCI
> controllers.
> 
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> Tested-by: Christian Eggers <christian.eggers@kathrein.de>
> Cc: stable <stable@vger.kernel.org>

Acked-by: Alan Stern <stern@rowland.harvard.edu>

> ---
>  drivers/usb/host/ohci-at91.c |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index 13ebeca..55d3d64 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -223,7 +223,7 @@ static void __devexit usb_hcd_at91_remove(struct usb_hcd *hcd,
>  /*-------------------------------------------------------------------------*/
>  
>  static int __devinit
> -ohci_at91_start (struct usb_hcd *hcd)
> +ohci_at91_reset (struct usb_hcd *hcd)
>  {
>  	struct at91_usbh_data	*board = hcd->self.controller->platform_data;
>  	struct ohci_hcd		*ohci = hcd_to_ohci (hcd);
> @@ -233,6 +233,14 @@ ohci_at91_start (struct usb_hcd *hcd)
>  		return ret;
>  
>  	ohci->num_ports = board->ports;
> +	return 0;
> +}
> +
> +static int __devinit
> +ohci_at91_start (struct usb_hcd *hcd)
> +{
> +	struct ohci_hcd		*ohci = hcd_to_ohci (hcd);
> +	int			ret;
>  
>  	if ((ret = ohci_run(ohci)) < 0) {
>  		err("can't start %s", hcd->self.bus_name);
> @@ -418,6 +426,7 @@ static const struct hc_driver ohci_at91_hc_driver = {
>  	/*
>  	 * basic lifecycle operations
>  	 */
> +	.reset =		ohci_at91_reset,
>  	.start =		ohci_at91_start,
>  	.stop =			ohci_stop,
>  	.shutdown =		ohci_shutdown,

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] USB: ohci-at91: add a reset function to fix race condition
  2012-05-09 17:22 ` Sergei Shtylyov
@ 2012-05-10  7:38   ` Nicolas Ferre
  0 siblings, 0 replies; 4+ messages in thread
From: Nicolas Ferre @ 2012-05-10  7:38 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/09/2012 07:22 PM, Sergei Shtylyov :
> Hello.
> 
> On 09-05-2012 12:48, Nicolas Ferre wrote:
> 
>> A possible race condition appears because we are not initializing
>> the ohci->regs before calling usb_hcd_request_irqs().
>> We move the call to ohci_init() in hcd->driver->reset() instead of
>> hcd->driver->start() to fix this.
>> This was experienced when we share the same IRQ line between OHCI and
>> EHCI
>> controllers.
> 
>> Signed-off-by: Nicolas Ferre<nicolas.ferre@atmel.com>
>> Tested-by: Christian Eggers<christian.eggers@kathrein.de>
>> Cc: stable<stable@vger.kernel.org>
>> ---
>>   drivers/usb/host/ohci-at91.c |   11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
>> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
>> index 13ebeca..55d3d64 100644
>> --- a/drivers/usb/host/ohci-at91.c
>> +++ b/drivers/usb/host/ohci-at91.c
>> @@ -223,7 +223,7 @@ static void __devexit usb_hcd_at91_remove(struct
>> usb_hcd *hcd,
>>  
>> /*-------------------------------------------------------------------------*/
>>
>>
>>   static int __devinit
>> -ohci_at91_start (struct usb_hcd *hcd)
>> +ohci_at91_reset (struct usb_hcd *hcd)
> 
>    Have you run the patch thru scripts/checkpatch.pl? There should be no
> space between function name and '('.
>
>>   {
>>       struct at91_usbh_data    *board =
>> hcd->self.controller->platform_data;
>>       struct ohci_hcd        *ohci = hcd_to_ohci (hcd);
>> @@ -233,6 +233,14 @@ ohci_at91_start (struct usb_hcd *hcd)
>>           return ret;
>>
>>       ohci->num_ports = board->ports;
>> +    return 0;
>> +}
>> +
>> +static int __devinit
>> +ohci_at91_start (struct usb_hcd *hcd)
> 
>    Same here.
> 
>> +{
>> +    struct ohci_hcd        *ohci = hcd_to_ohci (hcd);
> 
>    And here.

Yes, I have run checkpatch.pl. But I also know that I have to conform to
existing code and the history of the file that I am touching.
This file is using this convention as well as
drivers/usb/host/ohci-hcd.c and several others dealing with USB. So I
kept this style in my patch.

Best regards,
-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-05-10  7:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-09  8:48 [PATCH] USB: ohci-at91: add a reset function to fix race condition Nicolas Ferre
2012-05-09 17:22 ` Sergei Shtylyov
2012-05-10  7:38   ` Nicolas Ferre
2012-05-09 17:45 ` Alan Stern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).