* [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).