[PATCH] raw/cnxk_bphy: switch to dynamic logging

[PATCH] raw/cnxk_bphy: switch to dynamic logging

[Tomasz Duszynski]

Dynamically allocated log type is a standard approach among all drivers.
Switch to it.

Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
---
 drivers/raw/cnxk_bphy/cnxk_bphy.c          | 32 +++++++++++++---------
 drivers/raw/cnxk_bphy/cnxk_bphy_cgx.c      |  4 ++-
 drivers/raw/cnxk_bphy/cnxk_bphy_cgx.h      |  7 +++++
 drivers/raw/cnxk_bphy/cnxk_bphy_cgx_test.c | 32 ++++++++++------------
 4 files changed, 44 insertions(+), 31 deletions(-)

diff --git a/drivers/raw/cnxk_bphy/cnxk_bphy.c b/drivers/raw/cnxk_bphy/cnxk_bphy.c
index 15dbc4c1a6..af3bf97a4d 100644
--- a/drivers/raw/cnxk_bphy/cnxk_bphy.c
+++ b/drivers/raw/cnxk_bphy/cnxk_bphy.c
@@ -6,6 +6,7 @@
 #include <dev_driver.h>
 #include <rte_eal.h>
 #include <rte_lcore.h>
+#include <rte_log.h>
 #include <rte_pci.h>
 #include <rte_rawdev.h>
 #include <rte_rawdev_pmd.h>
@@ -15,6 +16,11 @@
 #include "cnxk_bphy_irq.h"
 #include "rte_pmd_bphy.h"
 
+extern int bphy_rawdev_logtype;
+
+#define BPHY_LOG(level, fmt, args...) \
+	rte_log(RTE_LOG_ ## level, bphy_rawdev_logtype, "%s(): " fmt "\n", __func__, ##args)
+
 static const struct rte_pci_id pci_bphy_map[] = {
 	{RTE_PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, PCI_DEVID_CNXK_BPHY)},
 	{
@@ -81,7 +87,7 @@ bphy_rawdev_selftest(uint16_t dev_id)
 		goto err_desc;
 	if (descs != 1) {
 		ret = -ENODEV;
-		plt_err("Wrong number of descs reported\n");
+		BPHY_LOG(ERR, "Wrong number of descs reported\n");
 		goto err_desc;
 	}
 
@@ -95,7 +101,7 @@ bphy_rawdev_selftest(uint16_t dev_id)
 
 	ret = rte_pmd_bphy_intr_init(dev_id);
 	if (ret) {
-		plt_err("intr init failed");
+		BPHY_LOG(ERR, "intr init failed");
 		return ret;
 	}
 
@@ -103,7 +109,7 @@ bphy_rawdev_selftest(uint16_t dev_id)
 
 	test = rte_zmalloc("BPHY", max_irq * sizeof(*test), 0);
 	if (test == NULL) {
-		plt_err("intr alloc failed");
+		BPHY_LOG(ERR, "intr alloc failed");
 		goto err_alloc;
 	}
 
@@ -132,7 +138,7 @@ bphy_rawdev_selftest(uint16_t dev_id)
 		}
 
 		if (ret) {
-			plt_err("intr register failed at irq %d", i);
+			BPHY_LOG(ERR, "intr register failed at irq %d", i);
 			goto err_register;
 		}
 	}
@@ -142,12 +148,12 @@ bphy_rawdev_selftest(uint16_t dev_id)
 
 	for (i = 0; i < max_irq; i++) {
 		if (!test[i].handled_intr) {
-			plt_err("intr %u not handled", i);
+			BPHY_LOG(ERR, "intr %u not handled", i);
 			ret = -1;
 			break;
 		}
 		if (test[i].handled_data != test[i].test_data) {
-			plt_err("intr %u has wrong handler", i);
+			BPHY_LOG(ERR, "intr %u has wrong handler", i);
 			ret = -1;
 			break;
 		}
@@ -251,7 +257,7 @@ cnxk_bphy_irq_enqueue_bufs(struct rte_rawdev *dev,
 
 	/* get rid of last response if any */
 	if (qp->rsp) {
-		RTE_LOG(WARNING, PMD, "Previous response got overwritten\n");
+		BPHY_LOG(WARNING, "Previous response got overwritten");
 		rte_free(qp->rsp);
 	}
 	qp->rsp = rsp;
@@ -332,9 +338,8 @@ bphy_rawdev_probe(struct rte_pci_driver *pci_drv,
 		return 0;
 
 	if (!pci_dev->mem_resource[0].addr) {
-		plt_err("BARs have invalid values: BAR0 %p\n BAR2 %p",
-			pci_dev->mem_resource[0].addr,
-			pci_dev->mem_resource[2].addr);
+		BPHY_LOG(ERR, "BARs have invalid values: BAR0 %p\n BAR2 %p",
+			 pci_dev->mem_resource[0].addr, pci_dev->mem_resource[2].addr);
 		return -ENODEV;
 	}
 
@@ -346,7 +351,7 @@ bphy_rawdev_probe(struct rte_pci_driver *pci_drv,
 	bphy_rawdev = rte_rawdev_pmd_allocate(name, sizeof(*bphy_dev),
 					      rte_socket_id());
 	if (bphy_rawdev == NULL) {
-		plt_err("Failed to allocate rawdev");
+		BPHY_LOG(ERR, "Failed to allocate rawdev");
 		return -ENOMEM;
 	}
 
@@ -381,14 +386,14 @@ bphy_rawdev_remove(struct rte_pci_device *pci_dev)
 		return 0;
 
 	if (pci_dev == NULL) {
-		plt_err("invalid pci_dev");
+		BPHY_LOG(ERR, "invalid pci_dev");
 		return -EINVAL;
 	}
 
 	bphy_rawdev_get_name(name, pci_dev);
 	rawdev = rte_rawdev_pmd_get_named_dev(name);
 	if (rawdev == NULL) {
-		plt_err("invalid device name (%s)", name);
+		BPHY_LOG(ERR, "invalid device name (%s)", name);
 		return -EINVAL;
 	}
 
@@ -410,3 +415,4 @@ static struct rte_pci_driver cnxk_bphy_rawdev_pmd = {
 RTE_PMD_REGISTER_PCI(bphy_rawdev_pci_driver, cnxk_bphy_rawdev_pmd);
 RTE_PMD_REGISTER_PCI_TABLE(bphy_rawdev_pci_driver, pci_bphy_map);
 RTE_PMD_REGISTER_KMOD_DEP(bphy_rawdev_pci_driver, "vfio-pci");
+RTE_LOG_REGISTER_DEFAULT(bphy_rawdev_logtype, WARNING);
diff --git a/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.c b/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.c
index 2d8466ef91..9eb0da9ed4 100644
--- a/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.c
+++ b/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.c
@@ -4,6 +4,7 @@
 #include <string.h>
 
 #include <bus_pci_driver.h>
+#include <rte_log.h>
 #include <rte_rawdev.h>
 #include <rte_rawdev_pmd.h>
 
@@ -189,7 +190,7 @@ cnxk_bphy_cgx_process_buf(struct cnxk_bphy_cgx *cgx, unsigned int queue,
 
 	/* get rid of last response if any */
 	if (qp->rsp) {
-		RTE_LOG(WARNING, PMD, "Previous response got overwritten\n");
+		BPHY_CGX_LOG(WARNING, "Previous response got overwritten");
 		rte_free(qp->rsp);
 	}
 	qp->rsp = rsp;
@@ -379,3 +380,4 @@ static struct rte_pci_driver bphy_cgx_rawdev_pmd = {
 RTE_PMD_REGISTER_PCI(cnxk_bphy_cgx_rawdev_pci_driver, bphy_cgx_rawdev_pmd);
 RTE_PMD_REGISTER_PCI_TABLE(cnxk_bphy_cgx_rawdev_pci_driver, cnxk_bphy_cgx_map);
 RTE_PMD_REGISTER_KMOD_DEP(cnxk_bphy_cgx_rawdev_pci_driver, "vfio-pci");
+RTE_LOG_REGISTER_DEFAULT(bphy_cgx_rawdev_logtype, INFO);
diff --git a/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.h b/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.h
index fb6b31bf4d..fd85a2df33 100644
--- a/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.h
+++ b/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.h
@@ -5,6 +5,13 @@
 #ifndef _CNXK_BPHY_CGX_H_
 #define _CNXK_BPHY_CGX_H_
 
+#include <rte_log.h>
+
+extern int bphy_cgx_rawdev_logtype;
+
+#define BPHY_CGX_LOG(level, fmt, args...) \
+	rte_log(RTE_LOG_ ## level, bphy_cgx_rawdev_logtype, "%s(): " fmt "\n", __func__, ##args)
+
 int cnxk_bphy_cgx_dev_selftest(uint16_t dev_id);
 
 #endif /* _CNXK_BPHY_CGX_H_ */
diff --git a/drivers/raw/cnxk_bphy/cnxk_bphy_cgx_test.c b/drivers/raw/cnxk_bphy/cnxk_bphy_cgx_test.c
index a3021b4bb7..5d2a53be31 100644
--- a/drivers/raw/cnxk_bphy/cnxk_bphy_cgx_test.c
+++ b/drivers/raw/cnxk_bphy/cnxk_bphy_cgx_test.c
@@ -4,7 +4,6 @@
 #include <stdint.h>
 
 #include <rte_cycles.h>
-#include <rte_log.h>
 #include <rte_malloc.h>
 #include <rte_rawdev.h>
 
@@ -57,62 +56,61 @@ cnxk_bphy_cgx_dev_selftest(uint16_t dev_id)
 		if (ret)
 			break;
 		if (descs != 1) {
-			RTE_LOG(ERR, PMD, "Wrong number of descs reported\n");
+			BPHY_CGX_LOG(ERR, "Wrong number of descs reported");
 			ret = -ENODEV;
 			break;
 		}
 
-		RTE_LOG(INFO, PMD, "Testing queue %d\n", i);
+		BPHY_CGX_LOG(INFO, "Testing queue %d", i);
 
 		ret = rte_pmd_bphy_cgx_stop_rxtx(dev_id, i);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to stop rx/tx\n");
+			BPHY_CGX_LOG(ERR, "Failed to stop rx/tx");
 			break;
 		}
 
 		ret = rte_pmd_bphy_cgx_start_rxtx(dev_id, i);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to start rx/tx\n");
+			BPHY_CGX_LOG(ERR, "Failed to start rx/tx");
 			break;
 		}
 
 		ret = rte_pmd_bphy_cgx_set_link_state(dev_id, i, false);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to set link down\n");
+			BPHY_CGX_LOG(ERR, "Failed to set link down");
 			break;
 		}
 
 		ret = cnxk_bphy_cgx_link_cond(dev_id, i, 0);
 		if (ret != 0)
-			RTE_LOG(ERR, PMD,
-				"Timed out waiting for a link down\n");
+			BPHY_CGX_LOG(ERR, "Timed out waiting for a link down");
 
 		ret = rte_pmd_bphy_cgx_set_link_state(dev_id, i, true);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to set link up\n");
+			BPHY_CGX_LOG(ERR, "Failed to set link up");
 			break;
 		}
 
 		ret = cnxk_bphy_cgx_link_cond(dev_id, i, 1);
 		if (ret != 1)
-			RTE_LOG(ERR, PMD, "Timed out waiting for a link up\n");
+			BPHY_CGX_LOG(ERR, "Timed out waiting for a link up");
 
 		ret = rte_pmd_bphy_cgx_intlbk_enable(dev_id, i);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to enable internal lbk\n");
+			BPHY_CGX_LOG(ERR, "Failed to enable internal lbk");
 			break;
 		}
 
 		ret = rte_pmd_bphy_cgx_intlbk_disable(dev_id, i);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to disable internal lbk\n");
+			BPHY_CGX_LOG(ERR, "Failed to disable internal lbk");
 			break;
 		}
 
 		ret = rte_pmd_bphy_cgx_ptp_rx_enable(dev_id, i);
 		/* ptp not available on RPM */
 		if (ret < 0 && ret != -ENOTSUP) {
-			RTE_LOG(ERR, PMD, "Failed to enable ptp\n");
+			BPHY_CGX_LOG(ERR, "Failed to enable ptp");
 			break;
 		}
 		ret = 0;
@@ -120,27 +118,27 @@ cnxk_bphy_cgx_dev_selftest(uint16_t dev_id)
 		ret = rte_pmd_bphy_cgx_ptp_rx_disable(dev_id, i);
 		/* ptp not available on RPM */
 		if (ret < 0 && ret != -ENOTSUP) {
-			RTE_LOG(ERR, PMD, "Failed to disable ptp\n");
+			BPHY_CGX_LOG(ERR, "Failed to disable ptp");
 			break;
 		}
 		ret = 0;
 
 		ret = rte_pmd_bphy_cgx_get_supported_fec(dev_id, i, &fec);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to get supported FEC\n");
+			BPHY_CGX_LOG(ERR, "Failed to get supported FEC");
 			break;
 		}
 
 		ret = rte_pmd_bphy_cgx_set_fec(dev_id, i, fec);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to set FEC to %d\n", fec);
+			BPHY_CGX_LOG(ERR, "Failed to set FEC to %d", fec);
 			break;
 		}
 
 		fec = CNXK_BPHY_CGX_ETH_LINK_FEC_NONE;
 		ret = rte_pmd_bphy_cgx_set_fec(dev_id, i, fec);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to disable FEC\n");
+			BPHY_CGX_LOG(ERR, "Failed to disable FEC");
 			break;
 		}
 	}
-- 
2.34.1

Re: [PATCH] raw/cnxk_bphy: switch to dynamic logging

[Stephen Hemminger]

On Tue, 14 Nov 2023 09:04:46 +0100
Tomasz Duszynski <tduszynski@marvell.com> wrote:

> Dynamically allocated log type is a standard approach among all drivers.
> Switch to it.
> 
> Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
> ---
>  drivers/raw/cnxk_bphy/cnxk_bphy.c          | 32 +++++++++++++---------
>  drivers/raw/cnxk_bphy/cnxk_bphy_cgx.c      |  4 ++-
>  drivers/raw/cnxk_bphy/cnxk_bphy_cgx.h      |  7 +++++
>  drivers/raw/cnxk_bphy/cnxk_bphy_cgx_test.c | 32 ++++++++++------------
>  4 files changed, 44 insertions(+), 31 deletions(-)

Good to see but there seems to be a lot of other places calling 
plt_err(), plt_info(), plt_warn(), plt_print()

RE: [EXT] Re: [PATCH] raw/cnxk_bphy: switch to dynamic logging

[Tomasz Duszynski]

>-----Original Message-----
>From: Stephen Hemminger <stephen@networkplumber.org>
>Sent: Wednesday, November 15, 2023 1:16 AM
>To: Tomasz Duszynski <tduszynski@marvell.com>
>Cc: dev@dpdk.org; Jakub Palider <jpalider@marvell.com>; Jerin Jacob Kollanukkaran
><jerinj@marvell.com>
>Subject: [EXT] Re: [PATCH] raw/cnxk_bphy: switch to dynamic logging
>
>External Email
>
>----------------------------------------------------------------------
>On Tue, 14 Nov 2023 09:04:46 +0100
>Tomasz Duszynski <tduszynski@marvell.com> wrote:
>
>> Dynamically allocated log type is a standard approach among all drivers.
>> Switch to it.
>>
>> Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
>> ---
>>  drivers/raw/cnxk_bphy/cnxk_bphy.c          | 32 +++++++++++++---------
>>  drivers/raw/cnxk_bphy/cnxk_bphy_cgx.c      |  4 ++-
>>  drivers/raw/cnxk_bphy/cnxk_bphy_cgx.h      |  7 +++++
>>  drivers/raw/cnxk_bphy/cnxk_bphy_cgx_test.c | 32
>> ++++++++++------------
>>  4 files changed, 44 insertions(+), 31 deletions(-)
>
>Good to see but there seems to be a lot of other places calling plt_err(), plt_info(),
>plt_warn(), plt_print()

Yeah, apparently my script did not catch everything. Thanks for pointing this out. 

Re: [PATCH] raw/cnxk_bphy: switch to dynamic logging

[David Marchand]

Hello,

On Tue, Nov 14, 2023 at 9:05 AM Tomasz Duszynski <tduszynski@marvell.com> wrote:
[snip]
> @@ -15,6 +16,11 @@
>  #include "cnxk_bphy_irq.h"
>  #include "rte_pmd_bphy.h"
>
> +extern int bphy_rawdev_logtype;
> +
> +#define BPHY_LOG(level, fmt, args...) \
> +       rte_log(RTE_LOG_ ## level, bphy_rawdev_logtype, "%s(): " fmt "\n", __func__, ##args)
> +
>  static const struct rte_pci_id pci_bphy_map[] = {
>         {RTE_PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, PCI_DEVID_CNXK_BPHY)},
>         {
> @@ -81,7 +87,7 @@ bphy_rawdev_selftest(uint16_t dev_id)
>                 goto err_desc;
>         if (descs != 1) {
>                 ret = -ENODEV;
> -               plt_err("Wrong number of descs reported\n");
> +               BPHY_LOG(ERR, "Wrong number of descs reported\n");

I think it is the only occurence in this patch, please remove trailing
\n since BPHY_LOG appends one.
Thanks.


-- 
David Marchand

[PATCH v2] raw/cnxk_bphy: switch to dynamic logging

[Tomasz Duszynski]

Dynamically allocated log type is a standard approach among all drivers.
Switch to it.

Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
---
v2:
- cleanup remaining plt_ log calls

 drivers/raw/cnxk_bphy/cnxk_bphy.c          | 31 +++++++++++----------
 drivers/raw/cnxk_bphy/cnxk_bphy.h          | 15 ++++++++++
 drivers/raw/cnxk_bphy/cnxk_bphy_cgx.c      |  4 ++-
 drivers/raw/cnxk_bphy/cnxk_bphy_cgx.h      |  7 +++++
 drivers/raw/cnxk_bphy/cnxk_bphy_cgx_test.c | 32 ++++++++++------------
 drivers/raw/cnxk_bphy/cnxk_bphy_irq.c      |  3 +-
 drivers/raw/cnxk_bphy/rte_pmd_bphy.h       |  6 ++--
 7 files changed, 61 insertions(+), 37 deletions(-)
 create mode 100644 drivers/raw/cnxk_bphy/cnxk_bphy.h

diff --git a/drivers/raw/cnxk_bphy/cnxk_bphy.c b/drivers/raw/cnxk_bphy/cnxk_bphy.c
index 15dbc4c1a6..b6830530d7 100644
--- a/drivers/raw/cnxk_bphy/cnxk_bphy.c
+++ b/drivers/raw/cnxk_bphy/cnxk_bphy.c
@@ -12,6 +12,7 @@

 #include <roc_api.h>

+#include "cnxk_bphy.h"
 #include "cnxk_bphy_irq.h"
 #include "rte_pmd_bphy.h"

@@ -81,21 +82,21 @@ bphy_rawdev_selftest(uint16_t dev_id)
 		goto err_desc;
 	if (descs != 1) {
 		ret = -ENODEV;
-		plt_err("Wrong number of descs reported\n");
+		BPHY_LOG(ERR, "Wrong number of descs reported");
 		goto err_desc;
 	}

 	ret = rte_pmd_bphy_npa_pf_func_get(dev_id, &pf_func);
 	if (ret || pf_func == 0)
-		plt_warn("NPA pf_func is invalid");
+		BPHY_LOG(WARNING, "NPA pf_func is invalid");

 	ret = rte_pmd_bphy_sso_pf_func_get(dev_id, &pf_func);
 	if (ret || pf_func == 0)
-		plt_warn("SSO pf_func is invalid");
+		BPHY_LOG(WARNING, "SSO pf_func is invalid");

 	ret = rte_pmd_bphy_intr_init(dev_id);
 	if (ret) {
-		plt_err("intr init failed");
+		BPHY_LOG(ERR, "intr init failed");
 		return ret;
 	}

@@ -103,7 +104,7 @@ bphy_rawdev_selftest(uint16_t dev_id)

 	test = rte_zmalloc("BPHY", max_irq * sizeof(*test), 0);
 	if (test == NULL) {
-		plt_err("intr alloc failed");
+		BPHY_LOG(ERR, "intr alloc failed");
 		goto err_alloc;
 	}

@@ -132,7 +133,7 @@ bphy_rawdev_selftest(uint16_t dev_id)
 		}

 		if (ret) {
-			plt_err("intr register failed at irq %d", i);
+			BPHY_LOG(ERR, "intr register failed at irq %d", i);
 			goto err_register;
 		}
 	}
@@ -142,12 +143,12 @@ bphy_rawdev_selftest(uint16_t dev_id)

 	for (i = 0; i < max_irq; i++) {
 		if (!test[i].handled_intr) {
-			plt_err("intr %u not handled", i);
+			BPHY_LOG(ERR, "intr %u not handled", i);
 			ret = -1;
 			break;
 		}
 		if (test[i].handled_data != test[i].test_data) {
-			plt_err("intr %u has wrong handler", i);
+			BPHY_LOG(ERR, "intr %u has wrong handler", i);
 			ret = -1;
 			break;
 		}
@@ -251,7 +252,7 @@ cnxk_bphy_irq_enqueue_bufs(struct rte_rawdev *dev,

 	/* get rid of last response if any */
 	if (qp->rsp) {
-		RTE_LOG(WARNING, PMD, "Previous response got overwritten\n");
+		BPHY_LOG(WARNING, "Previous response got overwritten");
 		rte_free(qp->rsp);
 	}
 	qp->rsp = rsp;
@@ -332,9 +333,8 @@ bphy_rawdev_probe(struct rte_pci_driver *pci_drv,
 		return 0;

 	if (!pci_dev->mem_resource[0].addr) {
-		plt_err("BARs have invalid values: BAR0 %p\n BAR2 %p",
-			pci_dev->mem_resource[0].addr,
-			pci_dev->mem_resource[2].addr);
+		BPHY_LOG(ERR, "BARs have invalid values: BAR0 %p\n BAR2 %p",
+			 pci_dev->mem_resource[0].addr, pci_dev->mem_resource[2].addr);
 		return -ENODEV;
 	}

@@ -346,7 +346,7 @@ bphy_rawdev_probe(struct rte_pci_driver *pci_drv,
 	bphy_rawdev = rte_rawdev_pmd_allocate(name, sizeof(*bphy_dev),
 					      rte_socket_id());
 	if (bphy_rawdev == NULL) {
-		plt_err("Failed to allocate rawdev");
+		BPHY_LOG(ERR, "Failed to allocate rawdev");
 		return -ENOMEM;
 	}

@@ -381,14 +381,14 @@ bphy_rawdev_remove(struct rte_pci_device *pci_dev)
 		return 0;

 	if (pci_dev == NULL) {
-		plt_err("invalid pci_dev");
+		BPHY_LOG(ERR, "invalid pci_dev");
 		return -EINVAL;
 	}

 	bphy_rawdev_get_name(name, pci_dev);
 	rawdev = rte_rawdev_pmd_get_named_dev(name);
 	if (rawdev == NULL) {
-		plt_err("invalid device name (%s)", name);
+		BPHY_LOG(ERR, "invalid device name (%s)", name);
 		return -EINVAL;
 	}

@@ -410,3 +410,4 @@ static struct rte_pci_driver cnxk_bphy_rawdev_pmd = {
 RTE_PMD_REGISTER_PCI(bphy_rawdev_pci_driver, cnxk_bphy_rawdev_pmd);
 RTE_PMD_REGISTER_PCI_TABLE(bphy_rawdev_pci_driver, pci_bphy_map);
 RTE_PMD_REGISTER_KMOD_DEP(bphy_rawdev_pci_driver, "vfio-pci");
+RTE_LOG_REGISTER_DEFAULT(bphy_rawdev_logtype, WARNING);
diff --git a/drivers/raw/cnxk_bphy/cnxk_bphy.h b/drivers/raw/cnxk_bphy/cnxk_bphy.h
new file mode 100644
index 0000000000..2acc1f98d9
--- /dev/null
+++ b/drivers/raw/cnxk_bphy/cnxk_bphy.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2023 Marvell.
+ */
+
+#ifndef _CNXK_BPHY_H_
+#define _CNXK_BPHY_H_
+
+#include <rte_log.h>
+
+extern int bphy_rawdev_logtype;
+
+#define BPHY_LOG(level, fmt, args...) \
+	rte_log(RTE_LOG_ ## level, bphy_rawdev_logtype, "%s(): " fmt "\n", __func__, ##args)
+
+#endif /* _CNXK_BPHY_H_ */
diff --git a/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.c b/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.c
index 2d8466ef91..9eb0da9ed4 100644
--- a/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.c
+++ b/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.c
@@ -4,6 +4,7 @@
 #include <string.h>

 #include <bus_pci_driver.h>
+#include <rte_log.h>
 #include <rte_rawdev.h>
 #include <rte_rawdev_pmd.h>

@@ -189,7 +190,7 @@ cnxk_bphy_cgx_process_buf(struct cnxk_bphy_cgx *cgx, unsigned int queue,

 	/* get rid of last response if any */
 	if (qp->rsp) {
-		RTE_LOG(WARNING, PMD, "Previous response got overwritten\n");
+		BPHY_CGX_LOG(WARNING, "Previous response got overwritten");
 		rte_free(qp->rsp);
 	}
 	qp->rsp = rsp;
@@ -379,3 +380,4 @@ static struct rte_pci_driver bphy_cgx_rawdev_pmd = {
 RTE_PMD_REGISTER_PCI(cnxk_bphy_cgx_rawdev_pci_driver, bphy_cgx_rawdev_pmd);
 RTE_PMD_REGISTER_PCI_TABLE(cnxk_bphy_cgx_rawdev_pci_driver, cnxk_bphy_cgx_map);
 RTE_PMD_REGISTER_KMOD_DEP(cnxk_bphy_cgx_rawdev_pci_driver, "vfio-pci");
+RTE_LOG_REGISTER_DEFAULT(bphy_cgx_rawdev_logtype, INFO);
diff --git a/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.h b/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.h
index fb6b31bf4d..fd85a2df33 100644
--- a/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.h
+++ b/drivers/raw/cnxk_bphy/cnxk_bphy_cgx.h
@@ -5,6 +5,13 @@
 #ifndef _CNXK_BPHY_CGX_H_
 #define _CNXK_BPHY_CGX_H_

+#include <rte_log.h>
+
+extern int bphy_cgx_rawdev_logtype;
+
+#define BPHY_CGX_LOG(level, fmt, args...) \
+	rte_log(RTE_LOG_ ## level, bphy_cgx_rawdev_logtype, "%s(): " fmt "\n", __func__, ##args)
+
 int cnxk_bphy_cgx_dev_selftest(uint16_t dev_id);

 #endif /* _CNXK_BPHY_CGX_H_ */
diff --git a/drivers/raw/cnxk_bphy/cnxk_bphy_cgx_test.c b/drivers/raw/cnxk_bphy/cnxk_bphy_cgx_test.c
index a3021b4bb7..5d2a53be31 100644
--- a/drivers/raw/cnxk_bphy/cnxk_bphy_cgx_test.c
+++ b/drivers/raw/cnxk_bphy/cnxk_bphy_cgx_test.c
@@ -4,7 +4,6 @@
 #include <stdint.h>

 #include <rte_cycles.h>
-#include <rte_log.h>
 #include <rte_malloc.h>
 #include <rte_rawdev.h>

@@ -57,62 +56,61 @@ cnxk_bphy_cgx_dev_selftest(uint16_t dev_id)
 		if (ret)
 			break;
 		if (descs != 1) {
-			RTE_LOG(ERR, PMD, "Wrong number of descs reported\n");
+			BPHY_CGX_LOG(ERR, "Wrong number of descs reported");
 			ret = -ENODEV;
 			break;
 		}

-		RTE_LOG(INFO, PMD, "Testing queue %d\n", i);
+		BPHY_CGX_LOG(INFO, "Testing queue %d", i);

 		ret = rte_pmd_bphy_cgx_stop_rxtx(dev_id, i);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to stop rx/tx\n");
+			BPHY_CGX_LOG(ERR, "Failed to stop rx/tx");
 			break;
 		}

 		ret = rte_pmd_bphy_cgx_start_rxtx(dev_id, i);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to start rx/tx\n");
+			BPHY_CGX_LOG(ERR, "Failed to start rx/tx");
 			break;
 		}

 		ret = rte_pmd_bphy_cgx_set_link_state(dev_id, i, false);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to set link down\n");
+			BPHY_CGX_LOG(ERR, "Failed to set link down");
 			break;
 		}

 		ret = cnxk_bphy_cgx_link_cond(dev_id, i, 0);
 		if (ret != 0)
-			RTE_LOG(ERR, PMD,
-				"Timed out waiting for a link down\n");
+			BPHY_CGX_LOG(ERR, "Timed out waiting for a link down");

 		ret = rte_pmd_bphy_cgx_set_link_state(dev_id, i, true);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to set link up\n");
+			BPHY_CGX_LOG(ERR, "Failed to set link up");
 			break;
 		}

 		ret = cnxk_bphy_cgx_link_cond(dev_id, i, 1);
 		if (ret != 1)
-			RTE_LOG(ERR, PMD, "Timed out waiting for a link up\n");
+			BPHY_CGX_LOG(ERR, "Timed out waiting for a link up");

 		ret = rte_pmd_bphy_cgx_intlbk_enable(dev_id, i);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to enable internal lbk\n");
+			BPHY_CGX_LOG(ERR, "Failed to enable internal lbk");
 			break;
 		}

 		ret = rte_pmd_bphy_cgx_intlbk_disable(dev_id, i);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to disable internal lbk\n");
+			BPHY_CGX_LOG(ERR, "Failed to disable internal lbk");
 			break;
 		}

 		ret = rte_pmd_bphy_cgx_ptp_rx_enable(dev_id, i);
 		/* ptp not available on RPM */
 		if (ret < 0 && ret != -ENOTSUP) {
-			RTE_LOG(ERR, PMD, "Failed to enable ptp\n");
+			BPHY_CGX_LOG(ERR, "Failed to enable ptp");
 			break;
 		}
 		ret = 0;
@@ -120,27 +118,27 @@ cnxk_bphy_cgx_dev_selftest(uint16_t dev_id)
 		ret = rte_pmd_bphy_cgx_ptp_rx_disable(dev_id, i);
 		/* ptp not available on RPM */
 		if (ret < 0 && ret != -ENOTSUP) {
-			RTE_LOG(ERR, PMD, "Failed to disable ptp\n");
+			BPHY_CGX_LOG(ERR, "Failed to disable ptp");
 			break;
 		}
 		ret = 0;

 		ret = rte_pmd_bphy_cgx_get_supported_fec(dev_id, i, &fec);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to get supported FEC\n");
+			BPHY_CGX_LOG(ERR, "Failed to get supported FEC");
 			break;
 		}

 		ret = rte_pmd_bphy_cgx_set_fec(dev_id, i, fec);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to set FEC to %d\n", fec);
+			BPHY_CGX_LOG(ERR, "Failed to set FEC to %d", fec);
 			break;
 		}

 		fec = CNXK_BPHY_CGX_ETH_LINK_FEC_NONE;
 		ret = rte_pmd_bphy_cgx_set_fec(dev_id, i, fec);
 		if (ret) {
-			RTE_LOG(ERR, PMD, "Failed to disable FEC\n");
+			BPHY_CGX_LOG(ERR, "Failed to disable FEC");
 			break;
 		}
 	}
diff --git a/drivers/raw/cnxk_bphy/cnxk_bphy_irq.c b/drivers/raw/cnxk_bphy/cnxk_bphy_irq.c
index b424d6127d..3dbd4a9c8a 100644
--- a/drivers/raw/cnxk_bphy/cnxk_bphy_irq.c
+++ b/drivers/raw/cnxk_bphy/cnxk_bphy_irq.c
@@ -8,6 +8,7 @@

 #include <roc_api.h>

+#include "cnxk_bphy.h"
 #include "cnxk_bphy_irq.h"

 static struct bphy_device *
@@ -87,7 +88,7 @@ cnxk_bphy_intr_unregister(uint16_t dev_id, int irq_num)
 	if (bphy_dev->irq_chip)
 		roc_bphy_intr_clear(bphy_dev->irq_chip, irq_num);
 	else
-		plt_err("Missing irq chip");
+		BPHY_LOG(ERR, "Missing irq chip");
 }

 struct cnxk_bphy_mem *
diff --git a/drivers/raw/cnxk_bphy/rte_pmd_bphy.h b/drivers/raw/cnxk_bphy/rte_pmd_bphy.h
index 9d8d028558..ebb85bd869 100644
--- a/drivers/raw/cnxk_bphy/rte_pmd_bphy.h
+++ b/drivers/raw/cnxk_bphy/rte_pmd_bphy.h
@@ -2,8 +2,8 @@
  * Copyright(C) 2021 Marvell.
  */

-#ifndef _CNXK_BPHY_H_
-#define _CNXK_BPHY_H_
+#ifndef _RTE_PMD_CNXK_BPHY_H_
+#define _RTE_PMD_CNXK_BPHY_H_

 #include <stdint.h>

@@ -889,4 +889,4 @@ int rte_pmd_bphy_sso_pf_func_get_rmt(uint16_t *pf_func);
 }
 #endif

-#endif /* _CNXK_BPHY_H_ */
+#endif /* _RTE_PMD_CNXK_BPHY_H_ */
--
2.34.1

Re: [PATCH v2] raw/cnxk_bphy: switch to dynamic logging

[Jerin Jacob]

On Mon, Nov 27, 2023 at 11:42 PM Tomasz Duszynski
<tduszynski@marvell.com> wrote:
>
> Dynamically allocated log type is a standard approach among all drivers.
> Switch to it.
>
> Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
> ---
> v2:
> - cleanup remaining plt_ log calls

Applied to dpdk-next-net-mrvl/for-main. Thanks

Re: [PATCH v2] raw/cnxk_bphy: switch to dynamic logging

[Thomas Monjalon]

06/12/2023 15:09, Jerin Jacob:
> On Mon, Nov 27, 2023 at 11:42 PM Tomasz Duszynski
> <tduszynski@marvell.com> wrote:
> >
> > Dynamically allocated log type is a standard approach among all drivers.
> > Switch to it.
> >
> > Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
> > ---
> > v2:
> > - cleanup remaining plt_ log calls
> 
> Applied to dpdk-next-net-mrvl/for-main. Thanks

Sorry, a patch from Stephen has been merged first in the main tree.

The main difference is on log registration:
Stephen's patch:
	RTE_LOG_REGISTER_SUFFIX(cnxk_logtype_bphy, bphy, INFO);
Tomasz' patch:
	RTE_LOG_REGISTER_DEFAULT(bphy_rawdev_logtype, WARNING);
	RTE_LOG_REGISTER_DEFAULT(bphy_cgx_rawdev_logtype, INFO);

There are also many leftovers that were missed in Stephen's patch
(plt_err, plt_warn).

Please could you send a rebased patch to fix what was merged?

RE: [EXT] Re: [PATCH v2] raw/cnxk_bphy: switch to dynamic logging

[Tomasz Duszynski]

>-----Original Message-----
>From: Thomas Monjalon <thomas@monjalon.net>
>Sent: Wednesday, February 21, 2024 3:00 AM
>To: Tomasz Duszynski <tduszynski@marvell.com>
>Cc: dev@dpdk.org; Jakub Palider <jpalider@marvell.com>; Jerin Jacob <jerinj@marvell.com>;
>david.marchand@redhat.com; stephen@networkplumber.org; Jerin Jacob <jerinjacobk@gmail.com>
>Subject: [EXT] Re: [PATCH v2] raw/cnxk_bphy: switch to dynamic logging
>
>External Email
>
>----------------------------------------------------------------------
>06/12/2023 15:09, Jerin Jacob:
>> On Mon, Nov 27, 2023 at 11:42 PM Tomasz Duszynski
>> <tduszynski@marvell.com> wrote:
>> >
>> > Dynamically allocated log type is a standard approach among all drivers.
>> > Switch to it.
>> >
>> > Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
>> > ---
>> > v2:
>> > - cleanup remaining plt_ log calls
>>
>> Applied to dpdk-next-net-mrvl/for-main. Thanks
>
>Sorry, a patch from Stephen has been merged first in the main tree.
>
>The main difference is on log registration:
>Stephen's patch:
>	RTE_LOG_REGISTER_SUFFIX(cnxk_logtype_bphy, bphy, INFO); Tomasz' patch:
>	RTE_LOG_REGISTER_DEFAULT(bphy_rawdev_logtype, WARNING);
>	RTE_LOG_REGISTER_DEFAULT(bphy_cgx_rawdev_logtype, INFO);
>
>There are also many leftovers that were missed in Stephen's patch (plt_err, plt_warn).
>
>Please could you send a rebased patch to fix what was merged?
>

Okay, I'll clean it up.