Re: [PATCH 3/5] bnx2x: Add RTE_LIBRTE_BNX2X_REG_ACCESS config option

From: Harish Patil <harish.patil@qlogic.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Sony Chacko <sony.chacko@qlogic.com>
Subject: Re: [PATCH 3/5] bnx2x: Add RTE_LIBRTE_BNX2X_REG_ACCESS config option
Date: Sat, 7 Nov 2015 03:08:36 +0000	[thread overview]
Message-ID: <D262A6A6.EB730%harish.patil@qlogic.com> (raw)
In-Reply-To: <6673500.WzqtfFB45U@xps13>

>2015-10-23 22:28, Harish Patil:
>> >2015-09-11 17:42, Rasesh Mody:
>> >> From: Harish Patil <harish.patil@qlogic.com>
>> >> --- a/config/common_linuxapp
>> >> +++ b/config/common_linuxapp
>> >> @@ -220,6 +220,7 @@ CONFIG_RTE_LIBRTE_BNX2X_DEBUG_INIT=n
>> >>  CONFIG_RTE_LIBRTE_BNX2X_DEBUG_RX=n
>> >>  CONFIG_RTE_LIBRTE_BNX2X_DEBUG_TX=n
>> >>  CONFIG_RTE_LIBRTE_BNX2X_MF_SUPPORT=n
>> >> +CONFIG_RTE_LIBRTE_BNX2X_REG_ACCESS=n
>> >
>> >Why do you need this option?
>>
>> Currently logging of register accesses is using (wrongly) the generic
>> driver debug option of PMD_DRV_LOG (which gets enabled if
>> CONFIG_RTE_LIBRTE_BNX2X_DEBUG=y).
>> This floods the console due to excessive printing of register
>> reads/writes. So creating a new debug option in order to capture only
>>the
>> register read/writes (if required) and also does not interfere with the
>> informational/user-level debug.
>
>You mean there is not enough log levels?
>
>#define RTE_LOG_EMERG    1U  /**< System is unusable.               */
>#define RTE_LOG_ALERT    2U  /**< Action must be taken immediately. */
>#define RTE_LOG_CRIT     3U  /**< Critical conditions.              */
>#define RTE_LOG_ERR      4U  /**< Error conditions.                 */
>#define RTE_LOG_WARNING  5U  /**< Warning conditions.               */
>#define RTE_LOG_NOTICE   6U  /**< Normal but significant condition. */
>#define RTE_LOG_INFO     7U  /**< Informational.                    */
>#define RTE_LOG_DEBUG    8U  /**< Debug-level messages.             */
>
>Either other "debug" logs can be put as INFO level, or
>you need a level 9 for verbose debug.
>But I don't really understand what is the problem of a verbose debug.
>
>It is not critical for the acceptance of this patch. Just wanted to
>discuss
>it as we have many config options to tune the debug.
>
>Please, at least, use the DEBUG word in the option name as done for
>CONFIG_RTE_LIBRTE_BNX2X_DEBUG_RX.
>

Currently PMD_DRV_LOG(DEBUG,..) is used all over the places and converting
that into INFO/NOTICE levels results in lot of changes.
Even if we convert other debug logs as INFO level logs, since level 8 is
the default debug level,  the user would have to edit config file to
prevent flooding of the messages on the console. So I guess adding a debug
option without needing additional level would suffice for now. I shall
rename it as CONFIG_RTE_LIBRTE_BNX2X_DEBUG_PERIODIC and include all
periodic events like statistics, register R/Ws etc under this debug.

Thanks,
Harish

________________________________

This message and any attached documents contain information from the sending company or its parent company(s), subsidiaries, divisions or branch offices that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.