* [PATCH] usb: gadget: goku_udc: Fix error path
@ 2010-10-03 14:10 Rahul Ruikar
2010-10-03 16:17 ` Dan Carpenter
2010-10-04 10:46 ` Sergei Shtylyov
0 siblings, 2 replies; 5+ messages in thread
From: Rahul Ruikar @ 2010-10-03 14:10 UTC (permalink / raw)
To: David Brownell, Greg Kroah-Hartman, Dan Carpenter, nm127
Cc: linux-usb, linux-kernel, Rahul Ruikar
call put_device() when device_register() fails.
Signed-off-by: Rahul Ruikar <rahul.ruikar@gmail.com>
---
drivers/usb/gadget/goku_udc.c | 7 ++++++-
drivers/usb/gadget/goku_udc.h | 3 ++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/goku_udc.c b/drivers/usb/gadget/goku_udc.c
index 1088d08..080779b 100644
--- a/drivers/usb/gadget/goku_udc.c
+++ b/drivers/usb/gadget/goku_udc.c
@@ -1847,8 +1847,13 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
/* done */
the_controller = dev;
retval = device_register(&dev->gadget.dev);
- if (retval == 0)
+ if (retval != 0) {
+ dev_reg_status = 2;
+ goto done;
+ } else {
+ dev_reg_status = 1;
return 0;
+ }
done:
if (dev)
diff --git a/drivers/usb/gadget/goku_udc.h b/drivers/usb/gadget/goku_udc.h
index 566cb23..c27b9e2 100644
--- a/drivers/usb/gadget/goku_udc.h
+++ b/drivers/usb/gadget/goku_udc.h
@@ -251,7 +251,8 @@ struct goku_udc {
got_region:1,
req_config:1,
configured:1,
- enabled:1;
+ enabled:1,
+ dev_reg_status:2;
/* pci state used to access those endpoints */
struct pci_dev *pdev;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] usb: gadget: goku_udc: Fix error path
2010-10-03 14:10 [PATCH] usb: gadget: goku_udc: Fix error path Rahul Ruikar
@ 2010-10-03 16:17 ` Dan Carpenter
2010-10-04 10:46 ` Sergei Shtylyov
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2010-10-03 16:17 UTC (permalink / raw)
To: Rahul Ruikar
Cc: David Brownell, Greg Kroah-Hartman, nm127, linux-usb,
linux-kernel
On Sun, Oct 03, 2010 at 07:40:16PM +0530, Rahul Ruikar wrote:
> call put_device() when device_register() fails.
>
Huh? I don't see any calls to put_device() in your patch.
regards,
dan carpenter
> Signed-off-by: Rahul Ruikar <rahul.ruikar@gmail.com>
> ---
> drivers/usb/gadget/goku_udc.c | 7 ++++++-
> drivers/usb/gadget/goku_udc.h | 3 ++-
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/goku_udc.c b/drivers/usb/gadget/goku_udc.c
> index 1088d08..080779b 100644
> --- a/drivers/usb/gadget/goku_udc.c
> +++ b/drivers/usb/gadget/goku_udc.c
> @@ -1847,8 +1847,13 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> /* done */
> the_controller = dev;
> retval = device_register(&dev->gadget.dev);
> - if (retval == 0)
> + if (retval != 0) {
> + dev_reg_status = 2;
> + goto done;
> + } else {
> + dev_reg_status = 1;
> return 0;
> + }
>
> done:
> if (dev)
> diff --git a/drivers/usb/gadget/goku_udc.h b/drivers/usb/gadget/goku_udc.h
> index 566cb23..c27b9e2 100644
> --- a/drivers/usb/gadget/goku_udc.h
> +++ b/drivers/usb/gadget/goku_udc.h
> @@ -251,7 +251,8 @@ struct goku_udc {
> got_region:1,
> req_config:1,
> configured:1,
> - enabled:1;
> + enabled:1,
> + dev_reg_status:2;
>
> /* pci state used to access those endpoints */
> struct pci_dev *pdev;
> --
> 1.7.2.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] usb: gadget: goku_udc: Fix error path
2010-10-03 14:10 [PATCH] usb: gadget: goku_udc: Fix error path Rahul Ruikar
2010-10-03 16:17 ` Dan Carpenter
@ 2010-10-04 10:46 ` Sergei Shtylyov
2010-10-04 10:50 ` Rahul Ruikar
1 sibling, 1 reply; 5+ messages in thread
From: Sergei Shtylyov @ 2010-10-04 10:46 UTC (permalink / raw)
To: Rahul Ruikar
Cc: David Brownell, Greg Kroah-Hartman, Dan Carpenter, nm127,
linux-usb, linux-kernel
Hello.
On 03-10-2010 18:10, Rahul Ruikar wrote:
> call put_device() when device_register() fails.
> Signed-off-by: Rahul Ruikar<rahul.ruikar@gmail.com>
[...]
> diff --git a/drivers/usb/gadget/goku_udc.c b/drivers/usb/gadget/goku_udc.c
> index 1088d08..080779b 100644
> --- a/drivers/usb/gadget/goku_udc.c
> +++ b/drivers/usb/gadget/goku_udc.c
> @@ -1847,8 +1847,13 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> /* done */
> the_controller = dev;
> retval = device_register(&dev->gadget.dev);
> - if (retval == 0)
> + if (retval != 0) {
> + dev_reg_status = 2;
> + goto done;
> + } else {
> + dev_reg_status = 1;
Did you mean 'dev->dev_reg_status'?
> return 0;
> + }
>
> done:
> if (dev)
> diff --git a/drivers/usb/gadget/goku_udc.h b/drivers/usb/gadget/goku_udc.h
> index 566cb23..c27b9e2 100644
> --- a/drivers/usb/gadget/goku_udc.h
> +++ b/drivers/usb/gadget/goku_udc.h
> @@ -251,7 +251,8 @@ struct goku_udc {
> got_region:1,
> req_config:1,
> configured:1,
> - enabled:1;
> + enabled:1,
> + dev_reg_status:2;
I don't see where this new field is read in your patch...
WBR, Sergei
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] usb: gadget: goku_udc: Fix error path
2010-10-04 10:46 ` Sergei Shtylyov
@ 2010-10-04 10:50 ` Rahul Ruikar
0 siblings, 0 replies; 5+ messages in thread
From: Rahul Ruikar @ 2010-10-04 10:50 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: David Brownell, Greg Kroah-Hartman, Dan Carpenter, nm127,
linux-usb, linux-kernel
Sergei,
Please ignore this particular patch.. I resubmitted this in another mail
Thanks,
- Rahul Ruikar
On 4 October 2010 16:16, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> Hello.
>
> On 03-10-2010 18:10, Rahul Ruikar wrote:
>
>> call put_device() when device_register() fails.
>
>> Signed-off-by: Rahul Ruikar<rahul.ruikar@gmail.com>
>
> [...]
>>
>> diff --git a/drivers/usb/gadget/goku_udc.c b/drivers/usb/gadget/goku_udc.c
>> index 1088d08..080779b 100644
>> --- a/drivers/usb/gadget/goku_udc.c
>> +++ b/drivers/usb/gadget/goku_udc.c
>> @@ -1847,8 +1847,13 @@ static int goku_probe(struct pci_dev *pdev, const
>> struct pci_device_id *id)
>> /* done */
>> the_controller = dev;
>> retval = device_register(&dev->gadget.dev);
>> - if (retval == 0)
>> + if (retval != 0) {
>> + dev_reg_status = 2;
>> + goto done;
>> + } else {
>> + dev_reg_status = 1;
>
> Did you mean 'dev->dev_reg_status'?
>
>> return 0;
>> + }
>>
>> done:
>> if (dev)
>> diff --git a/drivers/usb/gadget/goku_udc.h b/drivers/usb/gadget/goku_udc.h
>> index 566cb23..c27b9e2 100644
>> --- a/drivers/usb/gadget/goku_udc.h
>> +++ b/drivers/usb/gadget/goku_udc.h
>> @@ -251,7 +251,8 @@ struct goku_udc {
>> got_region:1,
>> req_config:1,
>> configured:1,
>> - enabled:1;
>> + enabled:1,
>> + dev_reg_status:2;
>
> I don't see where this new field is read in your patch...
>
> WBR, Sergei
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] usb: gadget: goku_udc: Fix error path
2010-10-05 13:20 ` Greg KH
@ 2010-10-05 16:55 ` Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2010-10-05 16:55 UTC (permalink / raw)
To: Greg KH; +Cc: Rahul Ruikar, David Brownell, nm127, linux-usb, linux-kernel
This is based on an initial patch by Rahul Ruikar.
The goku_remove() function can be called before device_register() so it
can call device_unregister() improperly. Also if the call to
device_register() fails we need to call put_device().
As I was changing the error handling in goku_probe(), I noticed that
the label was "done" but actually if the function succeeds we return
earlier. I renamed the error path to "err" instead of "done."
Reported-by: Rahul Ruikar <rahul.ruikar@gmail.com>
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/usb/gadget/goku_udc.c b/drivers/usb/gadget/goku_udc.c
index 1088d08..7232018 100644
--- a/drivers/usb/gadget/goku_udc.c
+++ b/drivers/usb/gadget/goku_udc.c
@@ -1744,7 +1744,8 @@ static void goku_remove(struct pci_dev *pdev)
pci_resource_len (pdev, 0));
if (dev->enabled)
pci_disable_device(pdev);
- device_unregister(&dev->gadget.dev);
+ if (dev->registered)
+ device_unregister(&dev->gadget.dev);
pci_set_drvdata(pdev, NULL);
dev->regs = NULL;
@@ -1774,7 +1775,7 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (!pdev->irq) {
printk(KERN_ERR "Check PCI %s IRQ setup!\n", pci_name(pdev));
retval = -ENODEV;
- goto done;
+ goto err;
}
/* alloc, and start init */
@@ -1782,7 +1783,7 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (dev == NULL){
pr_debug("enomem %s\n", pci_name(pdev));
retval = -ENOMEM;
- goto done;
+ goto err;
}
spin_lock_init(&dev->lock);
@@ -1800,7 +1801,7 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
retval = pci_enable_device(pdev);
if (retval < 0) {
DBG(dev, "can't enable, %d\n", retval);
- goto done;
+ goto err;
}
dev->enabled = 1;
@@ -1809,7 +1810,7 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (!request_mem_region(resource, len, driver_name)) {
DBG(dev, "controller already in use\n");
retval = -EBUSY;
- goto done;
+ goto err;
}
dev->got_region = 1;
@@ -1817,7 +1818,7 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (base == NULL) {
DBG(dev, "can't map memory\n");
retval = -EFAULT;
- goto done;
+ goto err;
}
dev->regs = (struct goku_udc_regs __iomem *) base;
@@ -1833,7 +1834,7 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
driver_name, dev) != 0) {
DBG(dev, "request interrupt %d failed\n", pdev->irq);
retval = -EBUSY;
- goto done;
+ goto err;
}
dev->got_irq = 1;
if (use_dma)
@@ -1844,13 +1845,16 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
create_proc_read_entry(proc_node_name, 0, NULL, udc_proc_read, dev);
#endif
- /* done */
the_controller = dev;
retval = device_register(&dev->gadget.dev);
- if (retval == 0)
- return 0;
+ if (retval) {
+ put_device(&dev->gadget.dev);
+ goto err;
+ }
+ dev->registered = 1;
+ return 0;
-done:
+err:
if (dev)
goku_remove (pdev);
return retval;
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-10-05 16:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-03 14:10 [PATCH] usb: gadget: goku_udc: Fix error path Rahul Ruikar
2010-10-03 16:17 ` Dan Carpenter
2010-10-04 10:46 ` Sergei Shtylyov
2010-10-04 10:50 ` Rahul Ruikar
-- strict thread matches above, loose matches on Subject: below --
2010-10-03 19:59 [RESEND/PATCH] " Rahul Ruikar
2010-10-04 12:22 ` Dan Carpenter
2010-10-05 13:20 ` Greg KH
2010-10-05 16:55 ` [PATCH] " Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox