* [PATCH net] net: fealnx: fix missing pci_disable_device()
@ 2022-10-24 13:57 Yang Yingliang
2022-10-25 6:42 ` Leon Romanovsky
2022-10-25 7:47 ` Leon Romanovsky
0 siblings, 2 replies; 7+ messages in thread
From: Yang Yingliang @ 2022-10-24 13:57 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba
pci_disable_device() need be called while module exiting, switch
to use pcim_enable(), pci_disable_device() and pci_release_regions()
will be called in pcim_release() while unbinding device.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
drivers/net/ethernet/fealnx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c
index ed18450fd2cc..fb139f295b67 100644
--- a/drivers/net/ethernet/fealnx.c
+++ b/drivers/net/ethernet/fealnx.c
@@ -494,7 +494,7 @@ static int fealnx_init_one(struct pci_dev *pdev,
option = card_idx < MAX_UNITS ? options[card_idx] : 0;
- i = pci_enable_device(pdev);
+ i = pcim_enable_device(pdev);
if (i) return i;
pci_set_master(pdev);
@@ -670,7 +670,6 @@ static int fealnx_init_one(struct pci_dev *pdev,
err_out_unmap:
pci_iounmap(pdev, ioaddr);
err_out_res:
- pci_release_regions(pdev);
return err;
}
@@ -689,7 +688,6 @@ static void fealnx_remove_one(struct pci_dev *pdev)
unregister_netdev(dev);
pci_iounmap(pdev, np->mem);
free_netdev(dev);
- pci_release_regions(pdev);
} else
printk(KERN_ERR "fealnx: remove for unknown device\n");
}
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net] net: fealnx: fix missing pci_disable_device() 2022-10-24 13:57 [PATCH net] net: fealnx: fix missing pci_disable_device() Yang Yingliang @ 2022-10-25 6:42 ` Leon Romanovsky 2022-10-25 7:47 ` Leon Romanovsky 1 sibling, 0 replies; 7+ messages in thread From: Leon Romanovsky @ 2022-10-25 6:42 UTC (permalink / raw) To: Yang Yingliang; +Cc: netdev, davem, kuba On Mon, Oct 24, 2022 at 09:57:28PM +0800, Yang Yingliang wrote: > pci_disable_device() need be called while module exiting, switch > to use pcim_enable(), pci_disable_device() and pci_release_regions() > will be called in pcim_release() while unbinding device. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > drivers/net/ethernet/fealnx.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c > index ed18450fd2cc..fb139f295b67 100644 > --- a/drivers/net/ethernet/fealnx.c > +++ b/drivers/net/ethernet/fealnx.c > @@ -494,7 +494,7 @@ static int fealnx_init_one(struct pci_dev *pdev, > > option = card_idx < MAX_UNITS ? options[card_idx] : 0; > > - i = pci_enable_device(pdev); > + i = pcim_enable_device(pdev); > if (i) return i; > pci_set_master(pdev); > > @@ -670,7 +670,6 @@ static int fealnx_init_one(struct pci_dev *pdev, > err_out_unmap: > pci_iounmap(pdev, ioaddr); > err_out_res: > - pci_release_regions(pdev); > return err; > } > > @@ -689,7 +688,6 @@ static void fealnx_remove_one(struct pci_dev *pdev) > unregister_netdev(dev); > pci_iounmap(pdev, np->mem); > free_netdev(dev); > - pci_release_regions(pdev); > } else > printk(KERN_ERR "fealnx: remove for unknown device\n"); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This path is not possible. > } > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: fealnx: fix missing pci_disable_device() 2022-10-24 13:57 [PATCH net] net: fealnx: fix missing pci_disable_device() Yang Yingliang 2022-10-25 6:42 ` Leon Romanovsky @ 2022-10-25 7:47 ` Leon Romanovsky 2022-10-25 9:01 ` Yang Yingliang 1 sibling, 1 reply; 7+ messages in thread From: Leon Romanovsky @ 2022-10-25 7:47 UTC (permalink / raw) To: Yang Yingliang; +Cc: netdev, davem, kuba On Mon, Oct 24, 2022 at 09:57:28PM +0800, Yang Yingliang wrote: > pci_disable_device() need be called while module exiting, switch > to use pcim_enable(), pci_disable_device() and pci_release_regions() > will be called in pcim_release() while unbinding device. I didn't understand the description at all, maybe people in CC will. Most likely, you wanted something like this: diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c index ed18450fd2cc..78107dd4aa57 100644 --- a/drivers/net/ethernet/fealnx.c +++ b/drivers/net/ethernet/fealnx.c @@ -690,6 +690,7 @@ static void fealnx_remove_one(struct pci_dev *pdev) pci_iounmap(pdev, np->mem); free_netdev(dev); pci_release_regions(pdev); + pci_disable_device(pdev); } else printk(KERN_ERR "fealnx: remove for unknown device\n"); > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > drivers/net/ethernet/fealnx.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c > index ed18450fd2cc..fb139f295b67 100644 > --- a/drivers/net/ethernet/fealnx.c > +++ b/drivers/net/ethernet/fealnx.c > @@ -494,7 +494,7 @@ static int fealnx_init_one(struct pci_dev *pdev, > > option = card_idx < MAX_UNITS ? options[card_idx] : 0; > > - i = pci_enable_device(pdev); > + i = pcim_enable_device(pdev); > if (i) return i; > pci_set_master(pdev); > > @@ -670,7 +670,6 @@ static int fealnx_init_one(struct pci_dev *pdev, > err_out_unmap: > pci_iounmap(pdev, ioaddr); > err_out_res: > - pci_release_regions(pdev); > return err; > } > > @@ -689,7 +688,6 @@ static void fealnx_remove_one(struct pci_dev *pdev) > unregister_netdev(dev); > pci_iounmap(pdev, np->mem); > free_netdev(dev); > - pci_release_regions(pdev); > } else > printk(KERN_ERR "fealnx: remove for unknown device\n"); > } > -- > 2.25.1 > ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: fealnx: fix missing pci_disable_device() 2022-10-25 7:47 ` Leon Romanovsky @ 2022-10-25 9:01 ` Yang Yingliang 2022-10-25 10:27 ` Leon Romanovsky 0 siblings, 1 reply; 7+ messages in thread From: Yang Yingliang @ 2022-10-25 9:01 UTC (permalink / raw) To: Leon Romanovsky; +Cc: netdev, davem, kuba On 2022/10/25 15:47, Leon Romanovsky wrote: > On Mon, Oct 24, 2022 at 09:57:28PM +0800, Yang Yingliang wrote: >> pci_disable_device() need be called while module exiting, switch >> to use pcim_enable(), pci_disable_device() and pci_release_regions() >> will be called in pcim_release() while unbinding device. > I didn't understand the description at all, maybe people in CC will. > Most likely, you wanted something like this: After using pcim_enable_device(), pcim_release() will be called while unbinding device, pcim_release() calls both pci_release_regions() and pci_disable_device(). Thanks, Yang > > diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c > index ed18450fd2cc..78107dd4aa57 100644 > --- a/drivers/net/ethernet/fealnx.c > +++ b/drivers/net/ethernet/fealnx.c > @@ -690,6 +690,7 @@ static void fealnx_remove_one(struct pci_dev *pdev) > pci_iounmap(pdev, np->mem); > free_netdev(dev); > pci_release_regions(pdev); > + pci_disable_device(pdev); > } else > printk(KERN_ERR "fealnx: remove for unknown device\n"); > > >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >> --- >> drivers/net/ethernet/fealnx.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c >> index ed18450fd2cc..fb139f295b67 100644 >> --- a/drivers/net/ethernet/fealnx.c >> +++ b/drivers/net/ethernet/fealnx.c >> @@ -494,7 +494,7 @@ static int fealnx_init_one(struct pci_dev *pdev, >> >> option = card_idx < MAX_UNITS ? options[card_idx] : 0; >> >> - i = pci_enable_device(pdev); >> + i = pcim_enable_device(pdev); >> if (i) return i; >> pci_set_master(pdev); >> >> @@ -670,7 +670,6 @@ static int fealnx_init_one(struct pci_dev *pdev, >> err_out_unmap: >> pci_iounmap(pdev, ioaddr); >> err_out_res: >> - pci_release_regions(pdev); >> return err; >> } >> >> @@ -689,7 +688,6 @@ static void fealnx_remove_one(struct pci_dev *pdev) >> unregister_netdev(dev); >> pci_iounmap(pdev, np->mem); >> free_netdev(dev); >> - pci_release_regions(pdev); >> } else >> printk(KERN_ERR "fealnx: remove for unknown device\n"); >> } >> -- >> 2.25.1 >> > . ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: fealnx: fix missing pci_disable_device() 2022-10-25 9:01 ` Yang Yingliang @ 2022-10-25 10:27 ` Leon Romanovsky 2022-10-25 11:25 ` Yang Yingliang 0 siblings, 1 reply; 7+ messages in thread From: Leon Romanovsky @ 2022-10-25 10:27 UTC (permalink / raw) To: Yang Yingliang; +Cc: netdev, davem, kuba On Tue, Oct 25, 2022 at 05:01:58PM +0800, Yang Yingliang wrote: > > On 2022/10/25 15:47, Leon Romanovsky wrote: > > On Mon, Oct 24, 2022 at 09:57:28PM +0800, Yang Yingliang wrote: > > > pci_disable_device() need be called while module exiting, switch > > > to use pcim_enable(), pci_disable_device() and pci_release_regions() > > > will be called in pcim_release() while unbinding device. > > I didn't understand the description at all, maybe people in CC will. > > Most likely, you wanted something like this: > After using pcim_enable_device(), pcim_release() will be called while > unbinding > device, pcim_release() calls both pci_release_regions() and > pci_disable_device(). I'm not sure that you can mix managed with unmanaged PCI APIs. Thanks > > Thanks, > Yang > > > > diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c > > index ed18450fd2cc..78107dd4aa57 100644 > > --- a/drivers/net/ethernet/fealnx.c > > +++ b/drivers/net/ethernet/fealnx.c > > @@ -690,6 +690,7 @@ static void fealnx_remove_one(struct pci_dev *pdev) > > pci_iounmap(pdev, np->mem); > > free_netdev(dev); > > pci_release_regions(pdev); > > + pci_disable_device(pdev); > > } else > > printk(KERN_ERR "fealnx: remove for unknown device\n"); > > > > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > > > --- > > > drivers/net/ethernet/fealnx.c | 4 +--- > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c > > > index ed18450fd2cc..fb139f295b67 100644 > > > --- a/drivers/net/ethernet/fealnx.c > > > +++ b/drivers/net/ethernet/fealnx.c > > > @@ -494,7 +494,7 @@ static int fealnx_init_one(struct pci_dev *pdev, > > > option = card_idx < MAX_UNITS ? options[card_idx] : 0; > > > - i = pci_enable_device(pdev); > > > + i = pcim_enable_device(pdev); > > > if (i) return i; > > > pci_set_master(pdev); > > > @@ -670,7 +670,6 @@ static int fealnx_init_one(struct pci_dev *pdev, > > > err_out_unmap: > > > pci_iounmap(pdev, ioaddr); > > > err_out_res: > > > - pci_release_regions(pdev); > > > return err; > > > } > > > @@ -689,7 +688,6 @@ static void fealnx_remove_one(struct pci_dev *pdev) > > > unregister_netdev(dev); > > > pci_iounmap(pdev, np->mem); > > > free_netdev(dev); > > > - pci_release_regions(pdev); > > > } else > > > printk(KERN_ERR "fealnx: remove for unknown device\n"); > > > } > > > -- > > > 2.25.1 > > > > > . ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: fealnx: fix missing pci_disable_device() 2022-10-25 10:27 ` Leon Romanovsky @ 2022-10-25 11:25 ` Yang Yingliang 2022-10-25 11:48 ` Leon Romanovsky 0 siblings, 1 reply; 7+ messages in thread From: Yang Yingliang @ 2022-10-25 11:25 UTC (permalink / raw) To: Leon Romanovsky; +Cc: netdev, davem, kuba On 2022/10/25 18:27, Leon Romanovsky wrote: > On Tue, Oct 25, 2022 at 05:01:58PM +0800, Yang Yingliang wrote: >> On 2022/10/25 15:47, Leon Romanovsky wrote: >>> On Mon, Oct 24, 2022 at 09:57:28PM +0800, Yang Yingliang wrote: >>>> pci_disable_device() need be called while module exiting, switch >>>> to use pcim_enable(), pci_disable_device() and pci_release_regions() >>>> will be called in pcim_release() while unbinding device. >>> I didn't understand the description at all, maybe people in CC will. >>> Most likely, you wanted something like this: >> After using pcim_enable_device(), pcim_release() will be called while >> unbinding >> device, pcim_release() calls both pci_release_regions() and >> pci_disable_device(). > I'm not sure that you can mix managed with unmanaged PCI APIs. > > Thanks After pcim_enable_device() being called, the region_mask is set in __pci_request_region(). pcim_release() will call pci_release_region() because region_mask is set. Without device managed, the pci_release_region() and pci_disable_device() is called at end of remove() function which is called in device_remove(), and with managed, they are called in device_unbind_cleanup() which is called after device_remove(). So I think it's OK to use this management. Thanks, Yang > >> Thanks, >> Yang >>> diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c >>> index ed18450fd2cc..78107dd4aa57 100644 >>> --- a/drivers/net/ethernet/fealnx.c >>> +++ b/drivers/net/ethernet/fealnx.c >>> @@ -690,6 +690,7 @@ static void fealnx_remove_one(struct pci_dev *pdev) >>> pci_iounmap(pdev, np->mem); >>> free_netdev(dev); >>> pci_release_regions(pdev); >>> + pci_disable_device(pdev); >>> } else >>> printk(KERN_ERR "fealnx: remove for unknown device\n"); >>> >>> >>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >>>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >>>> --- >>>> drivers/net/ethernet/fealnx.c | 4 +--- >>>> 1 file changed, 1 insertion(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c >>>> index ed18450fd2cc..fb139f295b67 100644 >>>> --- a/drivers/net/ethernet/fealnx.c >>>> +++ b/drivers/net/ethernet/fealnx.c >>>> @@ -494,7 +494,7 @@ static int fealnx_init_one(struct pci_dev *pdev, >>>> option = card_idx < MAX_UNITS ? options[card_idx] : 0; >>>> - i = pci_enable_device(pdev); >>>> + i = pcim_enable_device(pdev); >>>> if (i) return i; >>>> pci_set_master(pdev); >>>> @@ -670,7 +670,6 @@ static int fealnx_init_one(struct pci_dev *pdev, >>>> err_out_unmap: >>>> pci_iounmap(pdev, ioaddr); >>>> err_out_res: >>>> - pci_release_regions(pdev); >>>> return err; >>>> } >>>> @@ -689,7 +688,6 @@ static void fealnx_remove_one(struct pci_dev *pdev) >>>> unregister_netdev(dev); >>>> pci_iounmap(pdev, np->mem); >>>> free_netdev(dev); >>>> - pci_release_regions(pdev); >>>> } else >>>> printk(KERN_ERR "fealnx: remove for unknown device\n"); >>>> } >>>> -- >>>> 2.25.1 >>>> >>> . > . ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: fealnx: fix missing pci_disable_device() 2022-10-25 11:25 ` Yang Yingliang @ 2022-10-25 11:48 ` Leon Romanovsky 0 siblings, 0 replies; 7+ messages in thread From: Leon Romanovsky @ 2022-10-25 11:48 UTC (permalink / raw) To: Yang Yingliang; +Cc: netdev, davem, kuba On Tue, Oct 25, 2022 at 07:25:48PM +0800, Yang Yingliang wrote: > > On 2022/10/25 18:27, Leon Romanovsky wrote: > > On Tue, Oct 25, 2022 at 05:01:58PM +0800, Yang Yingliang wrote: > > > On 2022/10/25 15:47, Leon Romanovsky wrote: > > > > On Mon, Oct 24, 2022 at 09:57:28PM +0800, Yang Yingliang wrote: > > > > > pci_disable_device() need be called while module exiting, switch > > > > > to use pcim_enable(), pci_disable_device() and pci_release_regions() > > > > > will be called in pcim_release() while unbinding device. > > > > I didn't understand the description at all, maybe people in CC will. > > > > Most likely, you wanted something like this: > > > After using pcim_enable_device(), pcim_release() will be called while > > > unbinding > > > device, pcim_release() calls both pci_release_regions() and > > > pci_disable_device(). > > I'm not sure that you can mix managed with unmanaged PCI APIs. > > > > Thanks > After pcim_enable_device() being called, the region_mask is set in > __pci_request_region(). pcim_release() will call pci_release_region() > because region_mask is set. > > Without device managed, the pci_release_region() and pci_disable_device() > is called at end of remove() function which is called in device_remove(), > and > with managed, they are called in device_unbind_cleanup() which is called > after device_remove(). So I think it's OK to use this management. Even so, why do you think that call in device_unbind_cleanup() is the right one for such an old driver? Thanks > > Thanks, > Yang > > > > > Thanks, > > > Yang > > > > diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c > > > > index ed18450fd2cc..78107dd4aa57 100644 > > > > --- a/drivers/net/ethernet/fealnx.c > > > > +++ b/drivers/net/ethernet/fealnx.c > > > > @@ -690,6 +690,7 @@ static void fealnx_remove_one(struct pci_dev *pdev) > > > > pci_iounmap(pdev, np->mem); > > > > free_netdev(dev); > > > > pci_release_regions(pdev); > > > > + pci_disable_device(pdev); > > > > } else > > > > printk(KERN_ERR "fealnx: remove for unknown device\n"); > > > > > > > > > > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > > > > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > > > > > --- > > > > > drivers/net/ethernet/fealnx.c | 4 +--- > > > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > > > > > diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c > > > > > index ed18450fd2cc..fb139f295b67 100644 > > > > > --- a/drivers/net/ethernet/fealnx.c > > > > > +++ b/drivers/net/ethernet/fealnx.c > > > > > @@ -494,7 +494,7 @@ static int fealnx_init_one(struct pci_dev *pdev, > > > > > option = card_idx < MAX_UNITS ? options[card_idx] : 0; > > > > > - i = pci_enable_device(pdev); > > > > > + i = pcim_enable_device(pdev); > > > > > if (i) return i; > > > > > pci_set_master(pdev); > > > > > @@ -670,7 +670,6 @@ static int fealnx_init_one(struct pci_dev *pdev, > > > > > err_out_unmap: > > > > > pci_iounmap(pdev, ioaddr); > > > > > err_out_res: > > > > > - pci_release_regions(pdev); > > > > > return err; > > > > > } > > > > > @@ -689,7 +688,6 @@ static void fealnx_remove_one(struct pci_dev *pdev) > > > > > unregister_netdev(dev); > > > > > pci_iounmap(pdev, np->mem); > > > > > free_netdev(dev); > > > > > - pci_release_regions(pdev); > > > > > } else > > > > > printk(KERN_ERR "fealnx: remove for unknown device\n"); > > > > > } > > > > > -- > > > > > 2.25.1 > > > > > > > > > . > > . ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-10-25 11:48 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-10-24 13:57 [PATCH net] net: fealnx: fix missing pci_disable_device() Yang Yingliang 2022-10-25 6:42 ` Leon Romanovsky 2022-10-25 7:47 ` Leon Romanovsky 2022-10-25 9:01 ` Yang Yingliang 2022-10-25 10:27 ` Leon Romanovsky 2022-10-25 11:25 ` Yang Yingliang 2022-10-25 11:48 ` Leon Romanovsky
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).