[dpdk-dev] Pass verbose flag to kernel module

  ---
 mk/rte.module.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Hi Sergio,

2014-10-06 17:09, Sergio Gonzalez Monroy:
> --- a/mk/rte.module.mk
> +++ b/mk/rte.module.mk
> @@ -78,7 +78,7 @@ build: _postbuild
>  $(MODULE).ko: $(SRCS_LINKS)
>  	@if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi
>  	@$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR) O=$(RTE_KERNELDIR) \
> -		CROSS_COMPILE=$(CROSS)
> +		V=$(if $(V),1,0) CROSS_COMPILE=$(CROSS)

Please could you explain why it is needed?
The variable V should be inherited by the recursive make.
It's working without your patch in my test.
  

On Wed, Oct 08, 2014 at 07:05:32PM +0200, Thomas Monjalon wrote:
> Hi Sergio,
> 
> 2014-10-06 17:09, Sergio Gonzalez Monroy:
> > --- a/mk/rte.module.mk
> > +++ b/mk/rte.module.mk
> > @@ -78,7 +78,7 @@ build: _postbuild
> >  $(MODULE).ko: $(SRCS_LINKS)
> >  	@if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi
> >  	@$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR) O=$(RTE_KERNELDIR) \
> > -		CROSS_COMPILE=$(CROSS)
> > +		V=$(if $(V),1,0) CROSS_COMPILE=$(CROSS)
> 
> Please could you explain why it is needed?
> The variable V should be inherited by the recursive make.
> It's working without your patch in my test.
> 
Hi Thomas,

You are right. If you set V=1 in the commmand line it will inherit and pass it down.
In the curent framework, we do not force V to be 1, just to be defined (coud be V=y
or V=enable, etc).
This patch was just forcing the value to be 1 as it is the required value for the
kernel makefiles.

It is not a big deal and we could approach this by specifiying on the docs to be V=1
or any other way you think more appropiate?

Thanks,
Sergio

> -- 
> Thomas
  

Hi Sergio,

2014-10-09 10:37, Sergio Gonzalez Monroy:
> On Wed, Oct 08, 2014 at 07:05:32PM +0200, Thomas Monjalon wrote:
> > Hi Sergio,
> > 
> > 2014-10-06 17:09, Sergio Gonzalez Monroy:
> > > --- a/mk/rte.module.mk
> > > +++ b/mk/rte.module.mk
> > > @@ -78,7 +78,7 @@ build: _postbuild
> > >  $(MODULE).ko: $(SRCS_LINKS)
> > >  	@if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi
> > >  	@$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR) O=$(RTE_KERNELDIR) \
> > > -		CROSS_COMPILE=$(CROSS)
> > > +		V=$(if $(V),1,0) CROSS_COMPILE=$(CROSS)
> > 
> > Please could you explain why it is needed?
> > The variable V should be inherited by the recursive make.
> > It's working without your patch in my test.
> 
> You are right. If you set V=1 in the commmand line it will inherit and pass it down.
> In the curent framework, we do not force V to be 1, just to be defined (coud be V=y
> or V=enable, etc).
> This patch was just forcing the value to be 1 as it is the required value for the
> kernel makefiles.
> 
> It is not a big deal and we could approach this by specifiying on the docs to be V=1
> or any other way you think more appropiate?

Oh OK, I didn't imagine passing other value to V ;)
It's better to fix makefile than doc. So I'm OK with this patch.
  

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Sergio Gonzalez
> Monroy
> Sent: Monday, October 06, 2014 5:09 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] Pass verbose flag to kernel module
> 
> ---
>  mk/rte.module.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mk/rte.module.mk b/mk/rte.module.mk
> index c4ca3fd..bd3c596 100644
> --- a/mk/rte.module.mk
> +++ b/mk/rte.module.mk
> @@ -78,7 +78,7 @@ build: _postbuild
>  $(MODULE).ko: $(SRCS_LINKS)
>  	@if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi
>  	@$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR)
> O=$(RTE_KERNELDIR) \
> -		CROSS_COMPILE=$(CROSS)
> +		V=$(if $(V),1,0) CROSS_COMPILE=$(CROSS)
> 
>  # install module in $(RTE_OUTPUT)/kmod
>  $(RTE_OUTPUT)/kmod/$(MODULE).ko: $(MODULE).ko
> --
> 1.9.3

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
  

Sergio,

I'd like to see a v2 patch with your explanations in commit log
and with a Signed-off-by.

> > -		CROSS_COMPILE=$(CROSS)
> > +		V=$(if $(V),1,0) CROSS_COMPILE=$(CROSS)

Minor nits:
- paren is not needed for one letter variable
- V option should go to the end to keep things sorted by importance

-               CROSS_COMPILE=$(CROSS)
+               CROSS_COMPILE=$(CROSS) V=$(if $V,1,0)

> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Please Pablo, do not give your ack if there is no Signed-off-by.
It's mandatory.

Thanks
  

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, October 15, 2014 1:05 PM
> To: De Lara Guarch, Pablo; Gonzalez Monroy, Sergio
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] Pass verbose flag to kernel module
> 
> Sergio,
> 
> I'd like to see a v2 patch with your explanations in commit log and with a
> Signed-off-by.
> 
My mistake, I will be more careful to check proper comments and signed-off.
V2 on the way.

> > > -		CROSS_COMPILE=$(CROSS)
> > > +		V=$(if $(V),1,0) CROSS_COMPILE=$(CROSS)
> 
> Minor nits:
> - paren is not needed for one letter variable


I don't mind doing this but a quick grep on the current code
will show more single letters with parenthesis than without.
I was just trying to be consistent with current style.

Thanks,
Sergio

> - V option should go to the end to keep things sorted by importance
> 
> -               CROSS_COMPILE=$(CROSS)
> +               CROSS_COMPILE=$(CROSS) V=$(if $V,1,0)
> 
> > Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> 
> Please Pablo, do not give your ack if there is no Signed-off-by.
> It's mandatory.
> 
> Thanks
> --
> Thomas