diff mbox series

[1/8] regex/octeontx2: fix unnecessary name override

Message ID 20201102174507.1085128-2-bruce.richardson@intel.com (mailing list archive)
State Rejected, archived
Delegated to: David Marchand
Headers show
Series fix driver filenames in the docs | expand

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Bruce Richardson Nov. 2, 2020, 5:45 p.m. UTC
Since the device class is now part of the standard filenames, it's no
longer needed in name overrides in specific drivers.

Fixes: a20b2c01a7a1 ("build: standardize component names and defines")

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/regex/octeontx2/meson.build | 1 -
 1 file changed, 1 deletion(-)

Comments

David Marchand Nov. 2, 2020, 7:48 p.m. UTC | #1
On Mon, Nov 2, 2020 at 6:45 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> Since the device class is now part of the standard filenames, it's no
> longer needed in name overrides in specific drivers.
>
> Fixes: a20b2c01a7a1 ("build: standardize component names and defines")
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Equivalent to https://patchwork.dpdk.org/patch/81961/ but maybe better
to skip setting fmt_name when it is the same as the default value.
Thomas Monjalon Nov. 3, 2020, 12:30 a.m. UTC | #2
02/11/2020 20:48, David Marchand:
> On Mon, Nov 2, 2020 at 6:45 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > Since the device class is now part of the standard filenames, it's no
> > longer needed in name overrides in specific drivers.
> >
> > Fixes: a20b2c01a7a1 ("build: standardize component names and defines")
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> Equivalent to https://patchwork.dpdk.org/patch/81961/ but maybe better
> to skip setting fmt_name when it is the same as the default value.
[...]
> > -name = 'octeontx2_regex'

But it is not the same?

The name will default to "octeontx2", which is fine.
But the fmt_name should not take this default.
I believe fmt_name should be "octeontx2_regex" as I did in my patch.
David Marchand Nov. 3, 2020, 8:19 a.m. UTC | #3
On Tue, Nov 3, 2020 at 1:30 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > -name = 'octeontx2_regex'
>
> But it is not the same?
>
> The name will default to "octeontx2", which is fine.
> But the fmt_name should not take this default.
> I believe fmt_name should be "octeontx2_regex" as I did in my patch.

fmt_name is only for maintaining config compat.
This driver is new to 20.11.
We can drop fmt_name too.
Thomas Monjalon Nov. 3, 2020, 9:06 a.m. UTC | #4
03/11/2020 09:19, David Marchand:
> On Tue, Nov 3, 2020 at 1:30 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > -name = 'octeontx2_regex'
> >
> > But it is not the same?
> >
> > The name will default to "octeontx2", which is fine.
> > But the fmt_name should not take this default.
> > I believe fmt_name should be "octeontx2_regex" as I did in my patch.
> 
> fmt_name is only for maintaining config compat.
> This driver is new to 20.11.
> We can drop fmt_name too.

If we don't set fmt_name, it defaults to "name", "octeontx2" here.
What is the consequence in compat definitions?
David Marchand Nov. 3, 2020, 9:19 a.m. UTC | #5
On Tue, Nov 3, 2020 at 10:06 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 03/11/2020 09:19, David Marchand:
> > On Tue, Nov 3, 2020 at 1:30 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > > -name = 'octeontx2_regex'
> > >
> > > But it is not the same?
> > >
> > > The name will default to "octeontx2", which is fine.
> > > But the fmt_name should not take this default.
> > > I believe fmt_name should be "octeontx2_regex" as I did in my patch.
> >
> > fmt_name is only for maintaining config compat.
> > This driver is new to 20.11.
> > We can drop fmt_name too.
>
> If we don't set fmt_name, it defaults to "name", "octeontx2" here.
> What is the consequence in compat definitions?

$ git reset --hard origin/main
HEAD is now at 30cf171352 app/regex: add job context
$ ninja-build -C build >/dev/null 2>&1 && ls -rt
build/drivers/*regex*octeo*.so.21.0 |tail -1; grep OCTEONTX2
build/rte_build_config.h  |grep REG
build/drivers/librte_regex_octeontx2_regex.so.21.0
#define RTE_LIBRTE_OCTEONTX2_REGEX_PMD 1
#define RTE_REGEX_OCTEONTX2_REGEX 1

$ git reset --hard origin/main
HEAD is now at 30cf171352 app/regex: add job context
$ git pw patch apply 81961
Applying: regex/octeontx2: fix driver name
$ ninja-build -C build >/dev/null 2>&1 && ls -rt
build/drivers/*regex*octeo*.so.21.0 |tail -1; grep OCTEONTX2
build/rte_build_config.h  |grep REG
build/drivers/librte_regex_octeontx2.so.21.0
#define RTE_LIBRTE_OCTEONTX2_REGEX_PMD 1
#define RTE_REGEX_OCTEONTX2 1

$ git reset --hard origin/main
HEAD is now at 30cf171352 app/regex: add job context
$ git pw patch apply --no-deps 83425
Applying: regex/octeontx2: fix unnecessary name override
$ ninja-build -C build >/dev/null 2>&1 && ls -rt
build/drivers/*regex*octeo*.so.21.0 |tail -1; grep OCTEONTX2
build/rte_build_config.h  |grep REG
build/drivers/librte_regex_octeontx2.so.21.0
#define RTE_REGEX_OCTEONTX2 1
David Marchand Nov. 3, 2020, 9:45 a.m. UTC | #6
On Tue, Nov 3, 2020 at 10:19 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Tue, Nov 3, 2020 at 10:06 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > 03/11/2020 09:19, David Marchand:
> > > On Tue, Nov 3, 2020 at 1:30 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > > > -name = 'octeontx2_regex'
> > > >
> > > > But it is not the same?
> > > >
> > > > The name will default to "octeontx2", which is fine.
> > > > But the fmt_name should not take this default.
> > > > I believe fmt_name should be "octeontx2_regex" as I did in my patch.
> > >
> > > fmt_name is only for maintaining config compat.
> > > This driver is new to 20.11.
> > > We can drop fmt_name too.
> >
> > If we don't set fmt_name, it defaults to "name", "octeontx2" here.
> > What is the consequence in compat definitions?
>

Ok, got it, the problem is when we disable the net/octeontx2 driver.
Your patch correctly sets a RTE_LIBRTE_OCTEONTX2_REGEX_PMD compat
option that is unused but that does not overwrite the
RTE_LIBRTE_OCTEONTX2_PMD compat option (which indicates the presence
of the net/octeontx2 driver).
Bruce Richardson Nov. 3, 2020, 10:27 a.m. UTC | #7
On Tue, Nov 03, 2020 at 10:45:37AM +0100, David Marchand wrote:
> On Tue, Nov 3, 2020 at 10:19 AM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > On Tue, Nov 3, 2020 at 10:06 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > 03/11/2020 09:19, David Marchand:
> > > > On Tue, Nov 3, 2020 at 1:30 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > > > > -name = 'octeontx2_regex'
> > > > >
> > > > > But it is not the same?
> > > > >
> > > > > The name will default to "octeontx2", which is fine.
> > > > > But the fmt_name should not take this default.
> > > > > I believe fmt_name should be "octeontx2_regex" as I did in my patch.
> > > >
> > > > fmt_name is only for maintaining config compat.
> > > > This driver is new to 20.11.
> > > > We can drop fmt_name too.
> > >
> > > If we don't set fmt_name, it defaults to "name", "octeontx2" here.
> > > What is the consequence in compat definitions?
> >
> 
> Ok, got it, the problem is when we disable the net/octeontx2 driver.
> Your patch correctly sets a RTE_LIBRTE_OCTEONTX2_REGEX_PMD compat
> option that is unused but that does not overwrite the
> RTE_LIBRTE_OCTEONTX2_PMD compat option (which indicates the presence
> of the net/octeontx2 driver).
> 
Yes, I forgot about compatibility macro settings. Given the context of this
patchset, I was instead just looking at the library filesnames.

I'll drop this patch from v2, and you can take Thomas' instead.

/Bruce
diff mbox series

Patch

diff --git a/drivers/regex/octeontx2/meson.build b/drivers/regex/octeontx2/meson.build
index 398a981c07..0ab32f7788 100644
--- a/drivers/regex/octeontx2/meson.build
+++ b/drivers/regex/octeontx2/meson.build
@@ -38,7 +38,6 @@  foreach flag: extra_flags
 	endif
 endforeach
 
-name = 'octeontx2_regex'
 deps += ['bus_pci', 'common_octeontx2', 'regexdev']
 
 includes += include_directories('../../common/octeontx2')