diff mbox series

[07/11] net/txgbe: move wrapper to base driver

Message ID 20240907145433.1479091-8-david.marchand@redhat.com (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series Use RTE_LOG_LINE in drivers |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

David Marchand Sept. 7, 2024, 2:54 p.m. UTC 
BP_LOG() is only used in the base driver.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/txgbe/base/txgbe_osdep.h | 8 ++++++++
 drivers/net/txgbe/txgbe_logs.h       | 7 -------
 2 files changed, 8 insertions(+), 7 deletions(-)

Comments

Jiawen Wu Sept. 9, 2024, 6:18 a.m. UTC | #1 
On  Sat, Sep 7, 2024 10:54 PM, David Marchand wrote:
> BP_LOG() is only used in the base driver.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/net/txgbe/base/txgbe_osdep.h | 8 ++++++++
>  drivers/net/txgbe/txgbe_logs.h       | 7 -------
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/txgbe/base/txgbe_osdep.h b/drivers/net/txgbe/base/txgbe_osdep.h
> index 62d16a6abb..91c8abf12e 100644
> --- a/drivers/net/txgbe/base/txgbe_osdep.h
> +++ b/drivers/net/txgbe/base/txgbe_osdep.h
> @@ -28,6 +28,14 @@
>  #define TMZ_VADDR(mz)  ((mz)->addr)
>  #define TDEV_NAME(eth_dev)  ((eth_dev)->device->name)
> 
> +extern int txgbe_logtype_bp;
> +#define RTE_LOGTYPE_TXGBE_BP txgbe_logtype_bp
> +#define BP_LOG(fmt, ...) \
> +	RTE_LOG(DEBUG, TXGBE_BP, \
> +		"[%"PRIu64".%"PRIu64"]%s(%d): " fmt, \
> +		usec_stamp() / 1000000, usec_stamp() % 1000000, \
> +		__func__, __LINE__, ## __VA_ARGS__)
> +
>  #define ASSERT(x) do {			\
>  	if (!(x))			\
>  		PMD_DRV_LOG(ERR, "TXGBE: %d", x);	\
> diff --git a/drivers/net/txgbe/txgbe_logs.h b/drivers/net/txgbe/txgbe_logs.h
> index 74f49ab9ef..b5a5a9233f 100644
> --- a/drivers/net/txgbe/txgbe_logs.h
> +++ b/drivers/net/txgbe/txgbe_logs.h
> @@ -51,11 +51,4 @@ extern int txgbe_logtype_tx_free;
>  #define DEBUGOUT(fmt, args...)    PMD_DRV_LOG(DEBUG, fmt, ##args)
>  #define PMD_INIT_FUNC_TRACE()     PMD_DRV_LOG(DEBUG, ">>")
> 
> -extern int txgbe_logtype_bp;
> -#define BP_LOG(fmt, args...) \
> -	rte_log(RTE_LOG_DEBUG, txgbe_logtype_bp, \
> -		"[%"PRIu64".%"PRIu64"]%s(%d): " fmt, \
> -		usec_stamp() / 1000000, usec_stamp() % 1000000, \
> -		__func__, __LINE__, ##args)
> -
>  #endif /* _TXGBE_LOGS_H_ */
> --
> 2.46.0
> 

Hi,

Does this have to change? It looks a little weird.
David Marchand Sept. 9, 2024, 6:50 a.m. UTC | #2 
On Mon, Sep 9, 2024 at 8:18 AM Jiawen Wu <jiawenwu@trustnetic.com> wrote:
>
> On  Sat, Sep 7, 2024 10:54 PM, David Marchand wrote:
> > BP_LOG() is only used in the base driver.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  drivers/net/txgbe/base/txgbe_osdep.h | 8 ++++++++
> >  drivers/net/txgbe/txgbe_logs.h       | 7 -------
> >  2 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/txgbe/base/txgbe_osdep.h b/drivers/net/txgbe/base/txgbe_osdep.h
> > index 62d16a6abb..91c8abf12e 100644
> > --- a/drivers/net/txgbe/base/txgbe_osdep.h
> > +++ b/drivers/net/txgbe/base/txgbe_osdep.h
> > @@ -28,6 +28,14 @@
> >  #define TMZ_VADDR(mz)  ((mz)->addr)
> >  #define TDEV_NAME(eth_dev)  ((eth_dev)->device->name)
> >
> > +extern int txgbe_logtype_bp;
> > +#define RTE_LOGTYPE_TXGBE_BP txgbe_logtype_bp
> > +#define BP_LOG(fmt, ...) \
> > +     RTE_LOG(DEBUG, TXGBE_BP, \
> > +             "[%"PRIu64".%"PRIu64"]%s(%d): " fmt, \
> > +             usec_stamp() / 1000000, usec_stamp() % 1000000, \
> > +             __func__, __LINE__, ## __VA_ARGS__)
> > +
> >  #define ASSERT(x) do {                       \
> >       if (!(x))                       \
> >               PMD_DRV_LOG(ERR, "TXGBE: %d", x);       \
> > diff --git a/drivers/net/txgbe/txgbe_logs.h b/drivers/net/txgbe/txgbe_logs.h
> > index 74f49ab9ef..b5a5a9233f 100644
> > --- a/drivers/net/txgbe/txgbe_logs.h
> > +++ b/drivers/net/txgbe/txgbe_logs.h
> > @@ -51,11 +51,4 @@ extern int txgbe_logtype_tx_free;
> >  #define DEBUGOUT(fmt, args...)    PMD_DRV_LOG(DEBUG, fmt, ##args)
> >  #define PMD_INIT_FUNC_TRACE()     PMD_DRV_LOG(DEBUG, ">>")
> >
> > -extern int txgbe_logtype_bp;
> > -#define BP_LOG(fmt, args...) \
> > -     rte_log(RTE_LOG_DEBUG, txgbe_logtype_bp, \
> > -             "[%"PRIu64".%"PRIu64"]%s(%d): " fmt, \
> > -             usec_stamp() / 1000000, usec_stamp() % 1000000, \
> > -             __func__, __LINE__, ##args)
> > -
> >  #endif /* _TXGBE_LOGS_H_ */
> > --
> > 2.46.0
> >
>
> Hi,
>
> Does this have to change? It looks a little weird.

Can you be more specific about the part that you don't like?

There is a change in behavior, I agree: messages would now be prefixed
with the logtype, here TXGBE_BP.
I can revert this part if you prefer.

As for moving the macro, in all other drivers in DPDK, the osdep.h
header provides helpers for the base driver code, so it is the right
location.
Jiawen Wu Sept. 9, 2024, 7:23 a.m. UTC | #3 
On Mon, Sep 9, 2024 2:51 PM, David Marchand wrote:
> On Mon, Sep 9, 2024 at 8:18 AM Jiawen Wu <jiawenwu@trustnetic.com> wrote:
> >
> > On  Sat, Sep 7, 2024 10:54 PM, David Marchand wrote:
> > > BP_LOG() is only used in the base driver.
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > >  drivers/net/txgbe/base/txgbe_osdep.h | 8 ++++++++
> > >  drivers/net/txgbe/txgbe_logs.h       | 7 -------
> > >  2 files changed, 8 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/net/txgbe/base/txgbe_osdep.h b/drivers/net/txgbe/base/txgbe_osdep.h
> > > index 62d16a6abb..91c8abf12e 100644
> > > --- a/drivers/net/txgbe/base/txgbe_osdep.h
> > > +++ b/drivers/net/txgbe/base/txgbe_osdep.h
> > > @@ -28,6 +28,14 @@
> > >  #define TMZ_VADDR(mz)  ((mz)->addr)
> > >  #define TDEV_NAME(eth_dev)  ((eth_dev)->device->name)
> > >
> > > +extern int txgbe_logtype_bp;
> > > +#define RTE_LOGTYPE_TXGBE_BP txgbe_logtype_bp
> > > +#define BP_LOG(fmt, ...) \
> > > +     RTE_LOG(DEBUG, TXGBE_BP, \
> > > +             "[%"PRIu64".%"PRIu64"]%s(%d): " fmt, \
> > > +             usec_stamp() / 1000000, usec_stamp() % 1000000, \
> > > +             __func__, __LINE__, ## __VA_ARGS__)
> > > +
> > >  #define ASSERT(x) do {                       \
> > >       if (!(x))                       \
> > >               PMD_DRV_LOG(ERR, "TXGBE: %d", x);       \
> > > diff --git a/drivers/net/txgbe/txgbe_logs.h b/drivers/net/txgbe/txgbe_logs.h
> > > index 74f49ab9ef..b5a5a9233f 100644
> > > --- a/drivers/net/txgbe/txgbe_logs.h
> > > +++ b/drivers/net/txgbe/txgbe_logs.h
> > > @@ -51,11 +51,4 @@ extern int txgbe_logtype_tx_free;
> > >  #define DEBUGOUT(fmt, args...)    PMD_DRV_LOG(DEBUG, fmt, ##args)
> > >  #define PMD_INIT_FUNC_TRACE()     PMD_DRV_LOG(DEBUG, ">>")
> > >
> > > -extern int txgbe_logtype_bp;
> > > -#define BP_LOG(fmt, args...) \
> > > -     rte_log(RTE_LOG_DEBUG, txgbe_logtype_bp, \
> > > -             "[%"PRIu64".%"PRIu64"]%s(%d): " fmt, \
> > > -             usec_stamp() / 1000000, usec_stamp() % 1000000, \
> > > -             __func__, __LINE__, ##args)
> > > -
> > >  #endif /* _TXGBE_LOGS_H_ */
> > > --
> > > 2.46.0
> > >
> >
> > Hi,
> >
> > Does this have to change? It looks a little weird.
> 
> Can you be more specific about the part that you don't like?

Just for defining log macro in other .h file.

> There is a change in behavior, I agree: messages would now be prefixed
> with the logtype, here TXGBE_BP.
> I can revert this part if you prefer.
> 
> As for moving the macro, in all other drivers in DPDK, the osdep.h
> header provides helpers for the base driver code, so it is the right
> location.

Acked-by: Jiawen Wu <jiawenwu@trustnetic.com>
diff mbox series

Patch

diff --git a/drivers/net/txgbe/base/txgbe_osdep.h b/drivers/net/txgbe/base/txgbe_osdep.h
index 62d16a6abb..91c8abf12e 100644
--- a/drivers/net/txgbe/base/txgbe_osdep.h
+++ b/drivers/net/txgbe/base/txgbe_osdep.h
@@ -28,6 +28,14 @@ 
 #define TMZ_VADDR(mz)  ((mz)->addr)
 #define TDEV_NAME(eth_dev)  ((eth_dev)->device->name)
 
+extern int txgbe_logtype_bp;
+#define RTE_LOGTYPE_TXGBE_BP txgbe_logtype_bp
+#define BP_LOG(fmt, ...) \
+	RTE_LOG(DEBUG, TXGBE_BP, \
+		"[%"PRIu64".%"PRIu64"]%s(%d): " fmt, \
+		usec_stamp() / 1000000, usec_stamp() % 1000000, \
+		__func__, __LINE__, ## __VA_ARGS__)
+
 #define ASSERT(x) do {			\
 	if (!(x))			\
 		PMD_DRV_LOG(ERR, "TXGBE: %d", x);	\
diff --git a/drivers/net/txgbe/txgbe_logs.h b/drivers/net/txgbe/txgbe_logs.h
index 74f49ab9ef..b5a5a9233f 100644
--- a/drivers/net/txgbe/txgbe_logs.h
+++ b/drivers/net/txgbe/txgbe_logs.h
@@ -51,11 +51,4 @@  extern int txgbe_logtype_tx_free;
 #define DEBUGOUT(fmt, args...)    PMD_DRV_LOG(DEBUG, fmt, ##args)
 #define PMD_INIT_FUNC_TRACE()     PMD_DRV_LOG(DEBUG, ">>")
 
-extern int txgbe_logtype_bp;
-#define BP_LOG(fmt, args...) \
-	rte_log(RTE_LOG_DEBUG, txgbe_logtype_bp, \
-		"[%"PRIu64".%"PRIu64"]%s(%d): " fmt, \
-		usec_stamp() / 1000000, usec_stamp() % 1000000, \
-		__func__, __LINE__, ##args)
-
 #endif /* _TXGBE_LOGS_H_ */