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	IronPort-SDR: bQSBZuRwfbOb0UYyBKPWEBkTIcdJCj2um8sCU2Y5s4J2Ak7yoMp89cV1mNPfNXUX1UVKKEAbVF fauarzohyOMA== IronPort-SDR: D2FN8SdVWG2Oq9emMrzEkqi6cS9vfAQxnslmgkuzRLNYfVdY+L+EOSmzy/+yuD1QqmSxElzx4f mVnesqn0IQ6Q== From: Bruce Richardson <bruce.richardson@intel.com> To: dev@dpdk.org Cc: Bruce Richardson <bruce.richardson@intel.com>, Guy Kaneti <guyk@marvell.com>, Luca Boccassi <bluca@debian.org>, Rosen Xu <rosen.xu@intel.com>, Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> Date: Mon, 2 Nov 2020 17:45:00 +0000 Message-Id: <20201102174507.1085128-2-bruce.richardson@intel.com> In-Reply-To: <20201102174507.1085128-1-bruce.richardson@intel.com> References: <20201102174507.1085128-1-bruce.richardson@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [PATCH 1/8] regex/octeontx2: fix unnecessary name override Precedence: list Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org>
Series	fix driver filenames in the docs \| [0/8] fix driver filenames in the docs [1/8] regex/octeontx2: fix unnecessary name override [2/8] doc: fix driver names in compression devices guide [3/8] doc: fix driver names in crypto devices guide [4/8] doc: fix driver names in event devices guide [5/8] doc: fix driver names in NIC devices guide [6/8] doc: fix driver names in regex devices guide [7/8] doc: fix driver names in vDPA devices guide [8/8] doc: fix driver names in programmers guide

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')