| Message ID | 1280850203-5532-1-git-send-email-segooon@gmail.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Aug 3, 2010 at 5:43 PM, Kulikov Vasiliy <segooon@gmail.com> wrote: > IRQ and resource[] may not have correct values until > after PCI hotplug setup occurs at pci_enable_device() time. > > The semantic match that finds this problem is as follows: > > // <smpl> > @@ > identifier x; > identifier request ~= "pci_request.*|pci_resource.*"; > @@ > > ( > * x->irq > | > * x->resource > | > * request(x, ...) > ) > ... > *pci_enable_device(x) > // </smpl> > > Signed-off-by: Kulikov Vasiliy <segooon@gmail.com> Good catch. Acked-by: Gertjan van Wingerde <gwingerde@gmail.com> > --- > drivers/net/wireless/rt2x00/rt2x00pci.c | 21 ++++++++++----------- > 1 files changed, 10 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/wireless/rt2x00/rt2x00pci.c b/drivers/net/wireless/rt2x00/rt2x00pci.c > index 19b262e..63c2cc4 100644 > --- a/drivers/net/wireless/rt2x00/rt2x00pci.c > +++ b/drivers/net/wireless/rt2x00/rt2x00pci.c > @@ -240,16 +240,16 @@ int rt2x00pci_probe(struct pci_dev *pci_dev, const struct pci_device_id *id) > struct rt2x00_dev *rt2x00dev; > int retval; > > - retval = pci_request_regions(pci_dev, pci_name(pci_dev)); > + retval = pci_enable_device(pci_dev); > if (retval) { > - ERROR_PROBE("PCI request regions failed.\n"); > + ERROR_PROBE("Enable device failed.\n"); > return retval; > } > > - retval = pci_enable_device(pci_dev); > + retval = pci_request_regions(pci_dev, pci_name(pci_dev)); > if (retval) { > - ERROR_PROBE("Enable device failed.\n"); > - goto exit_release_regions; > + ERROR_PROBE("PCI request regions failed.\n"); > + goto exit_disable_device; > } > > pci_set_master(pci_dev); > @@ -260,14 +260,14 @@ int rt2x00pci_probe(struct pci_dev *pci_dev, const struct pci_device_id *id) > if (dma_set_mask(&pci_dev->dev, DMA_BIT_MASK(32))) { > ERROR_PROBE("PCI DMA not supported.\n"); > retval = -EIO; > - goto exit_disable_device; > + goto exit_release_regions; > } > > hw = ieee80211_alloc_hw(sizeof(struct rt2x00_dev), ops->hw); > if (!hw) { > ERROR_PROBE("Failed to allocate hardware.\n"); > retval = -ENOMEM; > - goto exit_disable_device; > + goto exit_release_regions; > } > > pci_set_drvdata(pci_dev, hw); > @@ -300,13 +300,12 @@ exit_free_reg: > exit_free_device: > ieee80211_free_hw(hw); > > -exit_disable_device: > - if (retval != -EBUSY) > - pci_disable_device(pci_dev); > - > exit_release_regions: > pci_release_regions(pci_dev); > > +exit_disable_device: > + pci_disable_device(pci_dev); > + > pci_set_drvdata(pci_dev, NULL); > > return retval; > -- > 1.7.0.4 > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 3, 2010 at 6:14 PM, Gertjan van Wingerde <gwingerde@gmail.com> wrote: > On Tue, Aug 3, 2010 at 5:43 PM, Kulikov Vasiliy <segooon@gmail.com> wrote: >> IRQ and resource[] may not have correct values until >> after PCI hotplug setup occurs at pci_enable_device() time. >> >> The semantic match that finds this problem is as follows: >> >> // <smpl> >> @@ >> identifier x; >> identifier request ~= "pci_request.*|pci_resource.*"; >> @@ >> >> ( >> * x->irq >> | >> * x->resource >> | >> * request(x, ...) >> ) >> ... >> *pci_enable_device(x) >> // </smpl> >> >> Signed-off-by: Kulikov Vasiliy <segooon@gmail.com> > > Good catch. > > Acked-by: Gertjan van Wingerde <gwingerde@gmail.com> Acked-by: Ivo van Doorn <IvDoorn@gmail.com> >> --- >> drivers/net/wireless/rt2x00/rt2x00pci.c | 21 ++++++++++----------- >> 1 files changed, 10 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/net/wireless/rt2x00/rt2x00pci.c b/drivers/net/wireless/rt2x00/rt2x00pci.c >> index 19b262e..63c2cc4 100644 >> --- a/drivers/net/wireless/rt2x00/rt2x00pci.c >> +++ b/drivers/net/wireless/rt2x00/rt2x00pci.c >> @@ -240,16 +240,16 @@ int rt2x00pci_probe(struct pci_dev *pci_dev, const struct pci_device_id *id) >> struct rt2x00_dev *rt2x00dev; >> int retval; >> >> - retval = pci_request_regions(pci_dev, pci_name(pci_dev)); >> + retval = pci_enable_device(pci_dev); >> if (retval) { >> - ERROR_PROBE("PCI request regions failed.\n"); >> + ERROR_PROBE("Enable device failed.\n"); >> return retval; >> } >> >> - retval = pci_enable_device(pci_dev); >> + retval = pci_request_regions(pci_dev, pci_name(pci_dev)); >> if (retval) { >> - ERROR_PROBE("Enable device failed.\n"); >> - goto exit_release_regions; >> + ERROR_PROBE("PCI request regions failed.\n"); >> + goto exit_disable_device; >> } >> >> pci_set_master(pci_dev); >> @@ -260,14 +260,14 @@ int rt2x00pci_probe(struct pci_dev *pci_dev, const struct pci_device_id *id) >> if (dma_set_mask(&pci_dev->dev, DMA_BIT_MASK(32))) { >> ERROR_PROBE("PCI DMA not supported.\n"); >> retval = -EIO; >> - goto exit_disable_device; >> + goto exit_release_regions; >> } >> >> hw = ieee80211_alloc_hw(sizeof(struct rt2x00_dev), ops->hw); >> if (!hw) { >> ERROR_PROBE("Failed to allocate hardware.\n"); >> retval = -ENOMEM; >> - goto exit_disable_device; >> + goto exit_release_regions; >> } >> >> pci_set_drvdata(pci_dev, hw); >> @@ -300,13 +300,12 @@ exit_free_reg: >> exit_free_device: >> ieee80211_free_hw(hw); >> >> -exit_disable_device: >> - if (retval != -EBUSY) >> - pci_disable_device(pci_dev); >> - >> exit_release_regions: >> pci_release_regions(pci_dev); >> >> +exit_disable_device: >> + pci_disable_device(pci_dev); >> + >> pci_set_drvdata(pci_dev, NULL); >> >> return retval; >> -- >> 1.7.0.4 >> >> > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/rt2x00/rt2x00pci.c b/drivers/net/wireless/rt2x00/rt2x00pci.c index 19b262e..63c2cc4 100644 --- a/drivers/net/wireless/rt2x00/rt2x00pci.c +++ b/drivers/net/wireless/rt2x00/rt2x00pci.c @@ -240,16 +240,16 @@ int rt2x00pci_probe(struct pci_dev *pci_dev, const struct pci_device_id *id) struct rt2x00_dev *rt2x00dev; int retval; - retval = pci_request_regions(pci_dev, pci_name(pci_dev)); + retval = pci_enable_device(pci_dev); if (retval) { - ERROR_PROBE("PCI request regions failed.\n"); + ERROR_PROBE("Enable device failed.\n"); return retval; } - retval = pci_enable_device(pci_dev); + retval = pci_request_regions(pci_dev, pci_name(pci_dev)); if (retval) { - ERROR_PROBE("Enable device failed.\n"); - goto exit_release_regions; + ERROR_PROBE("PCI request regions failed.\n"); + goto exit_disable_device; } pci_set_master(pci_dev); @@ -260,14 +260,14 @@ int rt2x00pci_probe(struct pci_dev *pci_dev, const struct pci_device_id *id) if (dma_set_mask(&pci_dev->dev, DMA_BIT_MASK(32))) { ERROR_PROBE("PCI DMA not supported.\n"); retval = -EIO; - goto exit_disable_device; + goto exit_release_regions; } hw = ieee80211_alloc_hw(sizeof(struct rt2x00_dev), ops->hw); if (!hw) { ERROR_PROBE("Failed to allocate hardware.\n"); retval = -ENOMEM; - goto exit_disable_device; + goto exit_release_regions; } pci_set_drvdata(pci_dev, hw); @@ -300,13 +300,12 @@ exit_free_reg: exit_free_device: ieee80211_free_hw(hw); -exit_disable_device: - if (retval != -EBUSY) - pci_disable_device(pci_dev); - exit_release_regions: pci_release_regions(pci_dev); +exit_disable_device: + pci_disable_device(pci_dev); + pci_set_drvdata(pci_dev, NULL); return retval;
IRQ and resource[] may not have correct values until after PCI hotplug setup occurs at pci_enable_device() time. The semantic match that finds this problem is as follows: // <smpl> @@ identifier x; identifier request ~= "pci_request.*|pci_resource.*"; @@ ( * x->irq | * x->resource | * request(x, ...) ) ... *pci_enable_device(x) // </smpl> Signed-off-by: Kulikov Vasiliy <segooon@gmail.com> --- drivers/net/wireless/rt2x00/rt2x00pci.c | 21 ++++++++++----------- 1 files changed, 10 insertions(+), 11 deletions(-)