Messages in this thread Patch in this message | | | Subject | Re: [PATCH] typhoon: use request_firmware | From | Jaswinder Singh <> | Date | Wed, 30 Jul 2008 12:14:11 +0530 |
| |
Hello Dave,
On Wed, 2008-07-30 at 01:25 -0400, David Dillow wrote:
> That's fine, and makes sense. However, you need to check if you've > already loaded it (typhoon_fw != NULL), so you don't leak memory if > there is more than one NIC. Part of me feels like there should be a > mutex around loading the firmware, to avoid surprises if PCI probing > gets multi-threaded again, but a comment to that effect may suffice for > now. >
OK, fixed.
diff --git a/drivers/net/typhoon.c b/drivers/net/typhoon.c index a57941a..50f1943 100644 --- a/drivers/net/typhoon.c +++ b/drivers/net/typhoon.c @@ -2461,9 +2461,17 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) */ tp->name = pci_name(pdev); - err = typhoon_init_firmware(tp); - if (err) - goto error_out_reset; + /* + * Need to check request_firmware should be called only once + * so you don't leak memory if there is more than one NIC. + * Need to check if PCI probing gets multi-threaded as + * mutex is used while loading the firmware. + */ + if (typhoon_fw != NULL) { + err = typhoon_init_firmware(tp); + if (err) + goto error_out_reset; + } typhoon_init_interface(tp); typhoon_init_rings(tp); > Also, if not adding a mutex, then this can be folded into > typhoon_init_one(), rather than living in a separate function. >
request_firmware is using mutex.
> All in all, this is getting better, my reservations about the goal > aside. >
Without you help this was not possible :)
Thank you,
Jaswinder Singh.
| |