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

[David Marchand <david.marchand@redhat.com>]

Hello Hemant,

On Tue, Jul 2, 2024 at 3:09 PM Hemant Agrawal <hemant.agrawal@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.

>
>         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.


>                         }
>                         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.


>                 }
>         }
>

[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.


-- 
David Marchand