From: Neil Horman <nhorman@tuxdriver.com>
To: Shreyansh Jain <shreyansh.jain@nxp.com>
Cc: Thomas Monjalon <thomas.monjalon@6wind.com>,
"david.marchand@6wind.com" <david.marchand@6wind.com>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 1/3] eal/drivers: prefix driver REGISTER macros with EAL
Date: Mon, 10 Oct 2016 08:56:24 -0400 [thread overview]
Message-ID: <20161010125624.GA19423@hmsreliant.think-freely.org> (raw)
In-Reply-To: <DB5PR0401MB2054FAD862EBAD3373F4B27890D90@DB5PR0401MB2054.eurprd04.prod.outlook.com>
On Sat, Oct 08, 2016 at 01:00:59PM +0000, Shreyansh Jain wrote:
> Hi Thomas,
>
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Friday, October 07, 2016 7:22 PM
> > To: Shreyansh Jain <shreyansh.jain@nxp.com>
> > Cc: david.marchand@6wind.com; dev@dpdk.org
> > Subject: Re: [PATCH 1/3] eal/drivers: prefix driver REGISTER macros with EAL
> >
> > 2016-10-07 19:11, Shreyansh Jain:
> > > --- a/mk/internal/rte.compile-pre.mk
> > > +++ b/mk/internal/rte.compile-pre.mk
> > > @@ -87,7 +87,7 @@ endif
> > > PMDINFO_GEN = $(RTE_SDK_BIN)/app/dpdk-pmdinfogen $@ $@.pmd.c
> > > PMDINFO_CC = $(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@.pmd.o $@.pmd.c
> > > PMDINFO_LD = $(CROSS)ld $(LDFLAGS) -r -o $@.o $@.pmd.o $@
> > > -PMDINFO_TO_O = if grep -q 'DRIVER_REGISTER_.*(.*)' $<; then \
> > > +PMDINFO_TO_O = if grep 'EAL_REGISTER_.*(.*)' $<; then \
> > > echo "$(if $V,$(PMDINFO_GEN), PMDINFO $@.pmd.c)" && \
> > > $(PMDINFO_GEN) && \
> > > echo "$(if $V,$(PMDINFO_CC), CC $@.pmd.o)" && \
> > >
> > > --->8---
> > > CC eal_pci_vfio.o
> > > PMDINFO eal_pci_vfio.o.pmd.c
> > > /bin/sh: 1:
> > > /home/shreyansh/build/DPDK/02_dpdk/x86_64-native-linuxapp-gcc/app/dpdk-
> > pmdinfogen:
> > > not found
> > > /home/shreyansh/build/DPDK/02_dpdk/mk/internal/rte.compile-pre.mk:138:
> > > recipe for target 'eal_pci_vfio.o' failed
> > > --->8---
> > >
> > > I don't think PMDINFO should be running on eal_pci_vfio file. Isn't it?
> >
> > Every files are scanned for the pattern.
>
> Sorry, I should have been clearer in my statement.
> I meant, I didn't think eal_pci_vfio.o had anything of interest for the PMD tool and hence the mk files would have skipped over it in absence of a match.
> I understand that PMDINFO would run on all files.
>
Thats incorrect, the Makefile does a REGEX search for appropriate registration
macros that imply the need for pmdinfo to run. If its running on an
inappropriate file its because your new macros inadvertently match the current
regex, hence my suggestion that the regex should be tuned to be more specific
Neil
> >
> > > Is it because EAL_REGISTER_* is matching EAL_REGISTER_TAILQ like macro
> > > as well?
> >
> > Probably.
> > That's another argument in favor of good prefixes.
> > I think you should use RTE_DRIVER_REGISTER_ or better, RTE_PMD_REGISTER_
>
> I thought of EAL_PMD_REGISTER_* but dropped it because for PCI_TABLE like macros, the name got really long (EAL_PMD_REGISTER_PCI_TABLE()).
> Anyways, I will use RTE_PMD_REGISTER_* now and send another version.
>
> > >
> > > I am not very well versed with how PMDINFO is linking with the build
> > > system so any hints/insight into this would be really helpful.
> > >
> > > One I get more clarity on this, I will push a new version of this patch.
> >
> > Thanks
>
> -
> Shreyansh
>
next prev parent reply other threads:[~2016-10-10 12:56 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-07 13:33 Shreyansh Jain
2016-10-07 13:33 ` [dpdk-dev] [PATCH 2/3] eal/drivers: prefix VDEV driver REGISTER macro " Shreyansh Jain
2016-10-07 13:33 ` [dpdk-dev] [PATCH 3/3] eal/drivers: prefix driver param " Shreyansh Jain
2016-10-07 13:41 ` [dpdk-dev] [PATCH 1/3] eal/drivers: prefix driver REGISTER macros " Shreyansh Jain
2016-10-07 13:51 ` Thomas Monjalon
2016-10-07 14:18 ` Neil Horman
2016-10-08 13:01 ` Shreyansh Jain
2016-10-08 13:00 ` Shreyansh Jain
2016-10-10 12:56 ` Neil Horman [this message]
2016-10-11 6:36 ` Shreyansh Jain
2016-10-11 7:27 ` Thomas Monjalon
2016-10-11 13:38 ` Neil Horman
2016-10-11 13:57 ` Thomas Monjalon
2016-10-11 14:57 ` Neil Horman
2016-10-12 4:29 ` Shreyansh Jain
2016-10-07 13:45 ` Thomas Monjalon
2016-10-08 12:47 ` Shreyansh Jain
2016-10-17 13:09 ` Neil Horman
2016-10-08 18:01 ` [dpdk-dev] [PATCH v1] drivers: prefix driver REGISTER macro with RTE EAL Shreyansh Jain
2016-10-08 18:02 ` Shreyansh Jain
2016-10-08 18:05 ` [dpdk-dev] [PATCH v2] drivers: prefix driver REGISTER macro with RTE PMD Shreyansh Jain
2016-10-08 20:21 ` Thomas Monjalon
2016-10-09 15:12 ` Shreyansh Jain
2016-10-09 19:50 ` Thomas Monjalon
2016-10-10 5:11 ` Shreyansh Jain
2016-10-10 5:16 ` Shreyansh Jain
2016-10-10 5:43 ` [dpdk-dev] [PATCH v3] " Shreyansh Jain
2016-10-13 23:45 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161010125624.GA19423@hmsreliant.think-freely.org \
--to=nhorman@tuxdriver.com \
--cc=david.marchand@6wind.com \
--cc=dev@dpdk.org \
--cc=shreyansh.jain@nxp.com \
--cc=thomas.monjalon@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).