* [PATCH -next] tty: moxa: add missing pci_disable_device()
@ 2022-09-23 9:25 ruanjinjie
2022-09-23 9:56 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: ruanjinjie @ 2022-09-23 9:25 UTC (permalink / raw)
To: gregkh, jirislaby, linux-kernel; +Cc: ruanjinjie
Driver should call pci_disable_device() if it returns from
moxa_pci_probe() with error.
Meanwhile, the driver calls pci_enable_device() in
moxa_pci_probe(), but never calls pci_disable_device() during removal.
Signed-off-by: ruanjinjie <ruanjinjie@huawei.com>
---
drivers/tty/moxa.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/moxa.c b/drivers/tty/moxa.c
index 35b6fddf0341..9174b64ea2db 100644
--- a/drivers/tty/moxa.c
+++ b/drivers/tty/moxa.c
@@ -1239,7 +1239,7 @@ static int moxa_pci_probe(struct pci_dev *pdev,
retval = pci_enable_device(pdev);
if (retval) {
dev_err(&pdev->dev, "can't enable pci device\n");
- goto err;
+ return retval;
}
for (i = 0; i < MAX_BOARDS; i++)
@@ -1300,6 +1300,7 @@ static int moxa_pci_probe(struct pci_dev *pdev,
err_reg:
pci_release_region(pdev, 2);
err:
+ pci_disable_device(pdev);
return retval;
}
@@ -1310,6 +1311,7 @@ static void moxa_pci_remove(struct pci_dev *pdev)
moxa_board_deinit(brd);
pci_release_region(pdev, 2);
+ pci_disable_device(pdev);
}
static struct pci_driver moxa_pci_driver = {
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH -next] tty: moxa: add missing pci_disable_device()
2022-09-23 9:25 [PATCH -next] tty: moxa: add missing pci_disable_device() ruanjinjie
@ 2022-09-23 9:56 ` Greg KH
2022-09-23 10:12 ` Ruan Jinjie
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2022-09-23 9:56 UTC (permalink / raw)
To: ruanjinjie; +Cc: jirislaby, linux-kernel
On Fri, Sep 23, 2022 at 05:25:30PM +0800, ruanjinjie wrote:
> Driver should call pci_disable_device() if it returns from
> moxa_pci_probe() with error.
Why?
That is not a normal thing to do, as you can disable other PCI devices
attached to it, right?
> Meanwhile, the driver calls pci_enable_device() in
> moxa_pci_probe(), but never calls pci_disable_device() during removal.
And is that really a problem?
>
> Signed-off-by: ruanjinjie <ruanjinjie@huawei.com>
> ---
> drivers/tty/moxa.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Do you have this hardware to test with?
How did you find this issue?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] tty: moxa: add missing pci_disable_device()
2022-09-23 9:56 ` Greg KH
@ 2022-09-23 10:12 ` Ruan Jinjie
2022-09-23 11:25 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Ruan Jinjie @ 2022-09-23 10:12 UTC (permalink / raw)
To: Greg KH; +Cc: jirislaby, linux-kernel
On 2022/9/23 17:56, Greg KH wrote:
> On Fri, Sep 23, 2022 at 05:25:30PM +0800, ruanjinjie wrote:
>> Driver should call pci_disable_device() if it returns from
>> moxa_pci_probe() with error.
>
> Why?
>
> That is not a normal thing to do, as you can disable other PCI devices
> attached to it, right?
>
>> Meanwhile, the driver calls pci_enable_device() in
>> moxa_pci_probe(), but never calls pci_disable_device() during removal.
>
> And is that really a problem?
>
>>
>> Signed-off-by: ruanjinjie <ruanjinjie@huawei.com>
>> ---
>> drivers/tty/moxa.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> Do you have this hardware to test with?
>
> How did you find this issue?
>
We use static analysis via coccinelle to find the above issue. The
command we use is below:
spatch -I include -timeout 60 -very_quiet -sp_file
pci_disable_device_missing.cocci drivers/tty/moxa.c
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] tty: moxa: add missing pci_disable_device()
2022-09-23 10:12 ` Ruan Jinjie
@ 2022-09-23 11:25 ` Greg KH
2022-09-24 0:58 ` Ruan Jinjie
2022-09-24 2:21 ` Ruan Jinjie
0 siblings, 2 replies; 6+ messages in thread
From: Greg KH @ 2022-09-23 11:25 UTC (permalink / raw)
To: Ruan Jinjie; +Cc: jirislaby, linux-kernel
On Fri, Sep 23, 2022 at 06:12:03PM +0800, Ruan Jinjie wrote:
>
>
> On 2022/9/23 17:56, Greg KH wrote:
> > On Fri, Sep 23, 2022 at 05:25:30PM +0800, ruanjinjie wrote:
> >> Driver should call pci_disable_device() if it returns from
> >> moxa_pci_probe() with error.
> >
> > Why?
> >
> > That is not a normal thing to do, as you can disable other PCI devices
> > attached to it, right?
> >
> >> Meanwhile, the driver calls pci_enable_device() in
> >> moxa_pci_probe(), but never calls pci_disable_device() during removal.
> >
> > And is that really a problem?
> >
> >>
> >> Signed-off-by: ruanjinjie <ruanjinjie@huawei.com>
> >> ---
> >> drivers/tty/moxa.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > Do you have this hardware to test with?
> >
> > How did you find this issue?
> >
>
> We use static analysis via coccinelle to find the above issue. The
> command we use is below:
>
> spatch -I include -timeout 60 -very_quiet -sp_file
> pci_disable_device_missing.cocci drivers/tty/moxa.c
Please test with real hardware and look up what pci_disable_device()
does.
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] tty: moxa: add missing pci_disable_device()
2022-09-23 11:25 ` Greg KH
@ 2022-09-24 0:58 ` Ruan Jinjie
2022-09-24 2:21 ` Ruan Jinjie
1 sibling, 0 replies; 6+ messages in thread
From: Ruan Jinjie @ 2022-09-24 0:58 UTC (permalink / raw)
To: Greg KH; +Cc: jirislaby, linux-kernel
On 2022/9/23 19:25, Greg KH wrote:
> On Fri, Sep 23, 2022 at 06:12:03PM +0800, Ruan Jinjie wrote:
>>
>>
>> On 2022/9/23 17:56, Greg KH wrote:
>>> On Fri, Sep 23, 2022 at 05:25:30PM +0800, ruanjinjie wrote:
>>>> Driver should call pci_disable_device() if it returns from
>>>> moxa_pci_probe() with error.
>>>
>>> Why?
>>>
>>> That is not a normal thing to do, as you can disable other PCI devices
>>> attached to it, right?
>>>
>>>> Meanwhile, the driver calls pci_enable_device() in
>>>> moxa_pci_probe(), but never calls pci_disable_device() during removal.
>>>
>>> And is that really a problem?
>>>
>>>>
>>>> Signed-off-by: ruanjinjie <ruanjinjie@huawei.com>
>>>> ---
>>>> drivers/tty/moxa.c | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> Do you have this hardware to test with?
>>>
>>> How did you find this issue?
>>>
>>
>> We use static analysis via coccinelle to find the above issue. The
>> command we use is below:
>>
>> spatch -I include -timeout 60 -very_quiet -sp_file
>> pci_disable_device_missing.cocci drivers/tty/moxa.c
>
> Please test with real hardware and look up what pci_disable_device()
> does.
Thank you very much for your sincere advice! we will deeply experiment
with this issue.
>
> greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] tty: moxa: add missing pci_disable_device()
2022-09-23 11:25 ` Greg KH
2022-09-24 0:58 ` Ruan Jinjie
@ 2022-09-24 2:21 ` Ruan Jinjie
1 sibling, 0 replies; 6+ messages in thread
From: Ruan Jinjie @ 2022-09-24 2:21 UTC (permalink / raw)
To: Greg KH; +Cc: jirislaby, linux-kernel
On 2022/9/23 19:25, Greg KH wrote:
> On Fri, Sep 23, 2022 at 06:12:03PM +0800, Ruan Jinjie wrote:
>>
>>
>> On 2022/9/23 17:56, Greg KH wrote:
>>> On Fri, Sep 23, 2022 at 05:25:30PM +0800, ruanjinjie wrote:
>>>> Driver should call pci_disable_device() if it returns from
>>>> moxa_pci_probe() with error.
>>>
>>> Why?
>>>
>>> That is not a normal thing to do, as you can disable other PCI devices
>>> attached to it, right?
I think pci_enable_device and pci_disable_device should be paired. If
there are other PCI devices, they will not be disabled because they are
disabled only when reference counting enable_cnt is reduced to 0.
When these devices are initialized, pci_enable_device is called and
reference counting enable_cnt is incremented. Therefore, when the
current device invokes pci_disable_device, other devices are working
normally and have not invoke pci_disable_device. As a result, the value
of enable_cnt is not 0, and the device will not be really disabled.
There's no problem with that.
On the other hand, if there are no other PCI devices attached to it, and
this driver do not call pci_disable_device() if it returns from
moxa_pci_probe() with error or during removal, the device will never be
disabled.
>>>
>>>> Meanwhile, the driver calls pci_enable_device() in
>>>> moxa_pci_probe(), but never calls pci_disable_device() during removal.
>>>
>>> And is that really a problem?
>>>
>>>>
>>>> Signed-off-by: ruanjinjie <ruanjinjie@huawei.com>
>>>> ---
>>>> drivers/tty/moxa.c | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> Do you have this hardware to test with?
>>>
>>> How did you find this issue?
>>>
>>
>> We use static analysis via coccinelle to find the above issue. The
>> command we use is below:
>>
>> spatch -I include -timeout 60 -very_quiet -sp_file
>> pci_disable_device_missing.cocci drivers/tty/moxa.c
>
> Please test with real hardware and look up what pci_disable_device()
> does.
>
> greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-24 2:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-23 9:25 [PATCH -next] tty: moxa: add missing pci_disable_device() ruanjinjie
2022-09-23 9:56 ` Greg KH
2022-09-23 10:12 ` Ruan Jinjie
2022-09-23 11:25 ` Greg KH
2022-09-24 0:58 ` Ruan Jinjie
2022-09-24 2:21 ` Ruan Jinjie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®