Re: [PATCH v3 1/3] drivers: replace printf with log macros

[Hemant Agrawal]

Hi David

On 03-07-2024 16:11, David Marchand wrote:
Hello Hemant,
On Tue, Jul 2, 2024 at 3:09 PM Hemant Agrawal <hemant.agra...@nxp.com> wrote:

[snip]

diff --git a/drivers/crypto/caam_jr/caam_jr.c b/drivers/crypto/caam_jr/caam_jr.c
index 0fa70c978f..fb9ac9cb30 100644
--- a/drivers/crypto/caam_jr/caam_jr.c
+++ b/drivers/crypto/caam_jr/caam_jr.c
@@ -461,7 +461,7 @@ caam_jr_prep_cdb(struct caam_jr_session *ses)
         }

  #if CAAM_JR_DBG
-       SEC_DUMP_DESC(cdb->sh_desc);
+       SEC_DUMP_DESC(cdb->sh_desc, stdout);
  #endif
This hunk should go to next patch.
will fix it in v4
         cdb->sh_hdr.hi.field.idlen = shared_desc_len;
[snip]

diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index 935a4f2ada..bcd19f31fb 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -282,9 +282,9 @@ dpaa_eth_dev_configure(struct rte_eth_dev *dev)
                                         dpaa_interrupt_handler,
                                         (void *)dev);
                                 if (ret == EINVAL)
-                                       printf("Failed to enable interrupt: Not 
Supported\n");
+                                       DPAA_PMD_ERR("Failed to enable interrupt: 
Not Supported\n");
                                 else
-                                       printf("Failed to enable interrupt\n");
+                                       DPAA_PMD_ERR("Failed to enable 
interrupt\n");
DPAA_PMD_ERR appends a \n, so \n should be removed here.

There are double \n in this driver prior to this patch, can you fix them too?
It is probably worth checking other NXP drivers on this topic.
will fix this in v4 and for the time being I am fixing it for dpaa set 
of drivers in 2nd patch of this series. Rest will follow as a separate 
patch.

                         }
                         dev->data->dev_conf.intr_conf.lsc = 0;
                         dev->data->dev_flags &= ~RTE_ETH_DEV_INTR_LSC;
@@ -340,7 +340,7 @@ dpaa_eth_dev_configure(struct rte_eth_dev *dev)
                         dpaa_update_link_speed(__fif->node_name, speed, 
duplex);
                 } else {
                         /* Manual autoneg - custom advertisement speed. */
-                       printf("Custom Advertisement speeds not supported\n");
+                       DPAA_PMD_ERR("Custom Advertisement speeds not 
supported\n");
Idem.
ok


                 }
         }

[snip]

diff --git a/drivers/net/dpaa/dpaa_rxtx.c b/drivers/net/dpaa/dpaa_rxtx.c
index ce4f3d6c85..c2579d65ee 100644
--- a/drivers/net/dpaa/dpaa_rxtx.c
+++ b/drivers/net/dpaa/dpaa_rxtx.c
@@ -103,7 +103,7 @@ static void dpaa_display_frame_info(const struct qm_fd *fd,
         for (ii = 0; ii < fd->length20; ii++) {
                 DISPLAY_PRINT("%02x ", ptr[ii]);
                 if (((ii + 1) % 16) == 0)
-                       printf("\n");
+                       DISPLAY_PRINT("\n");
         }
         DISPLAY_PRINT("\n");
  }
Indeed it is better to call this macro, but this macro is calling printf.
I am ok with the change as this is under a debug build cflag, but I
would prefer this debug code goes through rte_log in the future.