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

[Hemant Agrawal <hemant.agrawal@oss.nxp.com>]

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