* [PATCH] bus: fix code unused
[not found] <8>
@ 2022-05-12 12:19 ` taoyunxiang
2022-05-16 7:55 ` David Marchand
0 siblings, 1 reply; 4+ messages in thread
From: taoyunxiang @ 2022-05-12 12:19 UTC (permalink / raw)
To: dev; +Cc: taoyunxiang
The return value of pci_plug in pci_common.c
will always be int vaule, can not be NULL.
We could use not 0 to check it.
Author: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
Signed-off-by: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
---
lib/eal/common/eal_common_dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/eal/common/eal_common_dev.c b/lib/eal/common/eal_common_dev.c
index 148a23830a..99677bae58 100644
--- a/lib/eal/common/eal_common_dev.c
+++ b/lib/eal/common/eal_common_dev.c
@@ -143,7 +143,7 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev)
if (ret)
goto err_devarg;
- if (da->bus->plug == NULL) {
+ if (da->bus->plug != 0) {
RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
da->bus->name);
ret = -ENOTSUP;
--
2.12.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bus: fix code unused
2022-05-12 12:19 ` [PATCH] bus: fix code unused taoyunxiang
@ 2022-05-16 7:55 ` David Marchand
2022-05-17 1:54 ` taoyunxiang
0 siblings, 1 reply; 4+ messages in thread
From: David Marchand @ 2022-05-16 7:55 UTC (permalink / raw)
To: taoyunxiang; +Cc: dev
On Mon, May 16, 2022 at 9:38 AM taoyunxiang
<taoyunxiang@cmss.chinamobile.com> wrote:
>
> The return value of pci_plug in pci_common.c
> will always be int vaule, can not be NULL.
> We could use not 0 to check it.
I don't see the relation between patch and commitlog.
Please clarify what is the issue you want to fix.
>
> Author: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
> Signed-off-by: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
>
> ---
> lib/eal/common/eal_common_dev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/eal/common/eal_common_dev.c b/lib/eal/common/eal_common_dev.c
> index 148a23830a..99677bae58 100644
> --- a/lib/eal/common/eal_common_dev.c
> +++ b/lib/eal/common/eal_common_dev.c
> @@ -143,7 +143,7 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev)
> if (ret)
> goto err_devarg;
>
> - if (da->bus->plug == NULL) {
> + if (da->bus->plug != 0) {
The current (before patch) check is correct: it is allowed that a bus
does not support hotplug.
Inverting this check as you propose breaks hotplug.
> RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
> da->bus->name);
> ret = -ENOTSUP;
--
David Marchand
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Re: [PATCH] bus: fix code unused
2022-05-16 7:55 ` David Marchand
@ 2022-05-17 1:54 ` taoyunxiang
2022-05-18 12:07 ` David Marchand
0 siblings, 1 reply; 4+ messages in thread
From: taoyunxiang @ 2022-05-17 1:54 UTC (permalink / raw)
To: David Marchand; +Cc: dev
Hi David ,
Thanks for your review, see the bellows.
--------------
taoyunxiang@cmss.chinamobile.com
>On Mon, May 16, 2022 at 9:38 AM taoyunxiang
><taoyunxiang@cmss.chinamobile.com> wrote:
>>
>> The return value of pci_plug in pci_common.c
>> will always be int vaule, can not be NULL.
>> We could use not 0 to check it.
>
>I don't see the relation between patch and commitlog.
>Please clarify what is the issue you want to fix.
I think we have some code unused, so fix it.
>
>
>>
>> Author: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
>> Signed-off-by: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
>>
>> ---
>> lib/eal/common/eal_common_dev.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/eal/common/eal_common_dev.c b/lib/eal/common/eal_common_dev.c
>> index 148a23830a..99677bae58 100644
>> --- a/lib/eal/common/eal_common_dev.c
>> +++ b/lib/eal/common/eal_common_dev.c
>> @@ -143,7 +143,7 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev)
>> if (ret)
>> goto err_devarg;
>>
>> - if (da->bus->plug == NULL) {
>> + if (da->bus->plug != 0) {
>
>The current (before patch) check is correct: it is allowed that a bus
>does not support hotplug.
>Inverting this check as you propose breaks hotplug.
"da->bus->plug" will call pci_plug and pci_probe_all_drivers in pci_common.c , is it right ?
The pci_probe_all_drivers will never return NULL, so the check and related code will not go throuth,
no matter the plug is ok or not.
>
>
>> RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
>> da->bus->name);
>> ret = -ENOTSUP;
>
>
>--
>David Marchand
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Re: [PATCH] bus: fix code unused
2022-05-17 1:54 ` taoyunxiang
@ 2022-05-18 12:07 ` David Marchand
0 siblings, 0 replies; 4+ messages in thread
From: David Marchand @ 2022-05-18 12:07 UTC (permalink / raw)
To: taoyunxiang; +Cc: dev
On Tue, May 17, 2022 at 3:54 AM taoyunxiang@cmss.chinamobile.com
<taoyunxiang@cmss.chinamobile.com> wrote:
> >> @@ -143,7 +143,7 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev)
> >> if (ret)
> >> goto err_devarg;
> >>
> >> - if (da->bus->plug == NULL) {
> >> + if (da->bus->plug != 0) {
>
>
>
> >The current (before patch) check is correct: it is allowed that a bus
> >does not support hotplug.
> >Inverting this check as you propose breaks hotplug.
>
> "da->bus->plug" will call pci_plug and pci_probe_all_drivers in pci_common.c , is it right ?
Checking for da->bus->plug value is different from calling da->bus->plug(dev).
>
> The pci_probe_all_drivers will never return NULL, so the check and related code will not go throuth,
> no matter the plug is ok or not.
This check is needed, and is correct in its current form.
Thanks.
--
David Marchand
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-19 15:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <8>
2022-05-12 12:19 ` [PATCH] bus: fix code unused taoyunxiang
2022-05-16 7:55 ` David Marchand
2022-05-17 1:54 ` taoyunxiang
2022-05-18 12:07 ` David Marchand
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).