diff mbox series

[2/2] raw/cnxk_gpio: add bunch of newlines

Message ID 20231003204603.3377509-2-tduszynski@marvell.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers
Series [1/2] raw/cnxk_gpio: support multi-process mode |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Tomasz Duszynski Oct. 3, 2023, 8:46 p.m. UTC 
  Improve log output by adding some newlines.

Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
Reviewed-by: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
Tested-by: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
---
 drivers/raw/cnxk_gpio/cnxk_gpio.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Stephen Hemminger Oct. 3, 2023, 10:26 p.m. UTC | #1 
On Tue, 3 Oct 2023 22:46:03 +0200
Tomasz Duszynski <tduszynski@marvell.com> wrote:

> Improve log output by adding some newlines.
> 
> Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
> Reviewed-by: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Tested-by: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> ---
>  drivers/raw/cnxk_gpio/cnxk_gpio.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/raw/cnxk_gpio/cnxk_gpio.c b/drivers/raw/cnxk_gpio/cnxk_gpio.c
> index dcd646397e..6c4e4f5eae 100644
> --- a/drivers/raw/cnxk_gpio/cnxk_gpio.c
> +++ b/drivers/raw/cnxk_gpio/cnxk_gpio.c
> @@ -713,7 +713,7 @@ cnxk_gpio_probe(struct rte_vdev_device *dev)
>  	cnxk_gpio_format_name(name, sizeof(name));
>  	rawdev = rte_rawdev_pmd_allocate(name, sizeof(*gpiochip), rte_socket_id());
>  	if (!rawdev) {
> -		RTE_LOG(ERR, PMD, "failed to allocate %s rawdev", name);
> +		RTE_LOG(ERR, PMD, "failed to allocate %s rawdev\n", name);
>  		return -ENOMEM;
>  	}
>  
> @@ -753,7 +753,7 @@ cnxk_gpio_probe(struct rte_vdev_device *dev)
>  	snprintf(buf, sizeof(buf), "%s/gpiochip%d/base", CNXK_GPIO_CLASS_PATH, gpiochip->num);
>  	ret = cnxk_gpio_read_attr_int(buf, &gpiochip->base);
>  	if (ret) {
> -		RTE_LOG(ERR, PMD, "failed to read %s", buf);
> +		RTE_LOG(ERR, PMD, "failed to read %s\n", buf);
>  		goto out;
>  	}
>  
> @@ -761,7 +761,7 @@ cnxk_gpio_probe(struct rte_vdev_device *dev)
>  	snprintf(buf, sizeof(buf), "%s/gpiochip%d/ngpio", CNXK_GPIO_CLASS_PATH, gpiochip->num);
>  	ret = cnxk_gpio_read_attr_int(buf, &gpiochip->num_gpios);
>  	if (ret) {
> -		RTE_LOG(ERR, PMD, "failed to read %s", buf);
> +		RTE_LOG(ERR, PMD, "failed to read %s\n", buf);
>  		goto out;
>  	}
>  	gpiochip->num_queues = gpiochip->num_gpios;
> @@ -774,7 +774,7 @@ cnxk_gpio_probe(struct rte_vdev_device *dev)
>  
>  	gpiochip->gpios = rte_calloc(NULL, gpiochip->num_gpios, sizeof(struct cnxk_gpio *), 0);
>  	if (!gpiochip->gpios) {
> -		RTE_LOG(ERR, PMD, "failed to allocate gpios memory");
> +		RTE_LOG(ERR, PMD, "failed to allocate gpios memory\n");
>  		ret = -ENOMEM;
>  		goto out;
>  	}


No driver should be using the PMD logtype. It should always be using a dynamically
allocated log type.
Tomasz Duszynski Oct. 4, 2023, 8:42 p.m. UTC | #2 
>-----Original Message-----
>From: Stephen Hemminger <stephen@networkplumber.org>
>Sent: Wednesday, October 4, 2023 12:26 AM
>To: Tomasz Duszynski <tduszynski@marvell.com>
>Cc: dev@dpdk.org; Jakub Palider <jpalider@marvell.com>; Jerin Jacob Kollanukkaran
><jerinj@marvell.com>; thomas@monjalon.net
>Subject: [EXT] Re: [PATCH 2/2] raw/cnxk_gpio: add bunch of newlines
>
>External Email
>
>----------------------------------------------------------------------
>On Tue, 3 Oct 2023 22:46:03 +0200
>Tomasz Duszynski <tduszynski@marvell.com> wrote:
>
>> Improve log output by adding some newlines.
>>
>> Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
>> Reviewed-by: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
>> Tested-by: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
>> ---
>>  drivers/raw/cnxk_gpio/cnxk_gpio.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/raw/cnxk_gpio/cnxk_gpio.c
>> b/drivers/raw/cnxk_gpio/cnxk_gpio.c
>> index dcd646397e..6c4e4f5eae 100644
>> --- a/drivers/raw/cnxk_gpio/cnxk_gpio.c
>> +++ b/drivers/raw/cnxk_gpio/cnxk_gpio.c
>> @@ -713,7 +713,7 @@ cnxk_gpio_probe(struct rte_vdev_device *dev)
>>  	cnxk_gpio_format_name(name, sizeof(name));
>>  	rawdev = rte_rawdev_pmd_allocate(name, sizeof(*gpiochip), rte_socket_id());
>>  	if (!rawdev) {
>> -		RTE_LOG(ERR, PMD, "failed to allocate %s rawdev", name);
>> +		RTE_LOG(ERR, PMD, "failed to allocate %s rawdev\n", name);
>>  		return -ENOMEM;
>>  	}
>>
>> @@ -753,7 +753,7 @@ cnxk_gpio_probe(struct rte_vdev_device *dev)
>>  	snprintf(buf, sizeof(buf), "%s/gpiochip%d/base", CNXK_GPIO_CLASS_PATH, gpiochip->num);
>>  	ret = cnxk_gpio_read_attr_int(buf, &gpiochip->base);
>>  	if (ret) {
>> -		RTE_LOG(ERR, PMD, "failed to read %s", buf);
>> +		RTE_LOG(ERR, PMD, "failed to read %s\n", buf);
>>  		goto out;
>>  	}
>>
>> @@ -761,7 +761,7 @@ cnxk_gpio_probe(struct rte_vdev_device *dev)
>>  	snprintf(buf, sizeof(buf), "%s/gpiochip%d/ngpio", CNXK_GPIO_CLASS_PATH, gpiochip->num);
>>  	ret = cnxk_gpio_read_attr_int(buf, &gpiochip->num_gpios);
>>  	if (ret) {
>> -		RTE_LOG(ERR, PMD, "failed to read %s", buf);
>> +		RTE_LOG(ERR, PMD, "failed to read %s\n", buf);
>>  		goto out;
>>  	}
>>  	gpiochip->num_queues = gpiochip->num_gpios; @@ -774,7 +774,7 @@
>> cnxk_gpio_probe(struct rte_vdev_device *dev)
>>
>>  	gpiochip->gpios = rte_calloc(NULL, gpiochip->num_gpios, sizeof(struct cnxk_gpio *), 0);
>>  	if (!gpiochip->gpios) {
>> -		RTE_LOG(ERR, PMD, "failed to allocate gpios memory");
>> +		RTE_LOG(ERR, PMD, "failed to allocate gpios memory\n");
>>  		ret = -ENOMEM;
>>  		goto out;
>>  	}
>
>
>No driver should be using the PMD logtype. It should always be using a dynamically allocated log
>type.

Even though it uses old-fashioned logging that change could still
improve output for some users out there. 

Anyway, if you don't want any extra commit noise let's abandon that and these improvements will show up in dynamic logging patch.
Stephen Hemminger Oct. 5, 2023, 2:54 a.m. UTC | #3 
On Wed, 4 Oct 2023 20:42:47 +0000
Tomasz Duszynski <tduszynski@marvell.com> wrote:

> >
> >No driver should be using the PMD logtype. It should always be using a dynamically allocated log
> >type.  
> 
> Even though it uses old-fashioned logging that change could still
> improve output for some users out there. 
> 
> Anyway, if you don't want any extra commit noise let's abandon that and these improvements will show up in dynamic logging patch. 

Keep it for now, but soon RTE_LOGTYPE_PMD will be marked deprecated.
Tomasz Duszynski Oct. 5, 2023, 7:36 a.m. UTC | #4 
>-----Original Message-----
>From: Stephen Hemminger <stephen@networkplumber.org>
>Sent: Thursday, October 5, 2023 4:55 AM
>To: Tomasz Duszynski <tduszynski@marvell.com>
>Cc: dev@dpdk.org; Jakub Palider <jpalider@marvell.com>; Jerin Jacob Kollanukkaran
><jerinj@marvell.com>; thomas@monjalon.net
>Subject: Re: [EXT] Re: [PATCH 2/2] raw/cnxk_gpio: add bunch of newlines
>
>On Wed, 4 Oct 2023 20:42:47 +0000
>Tomasz Duszynski <tduszynski@marvell.com> wrote:
>
>> >
>> >No driver should be using the PMD logtype. It should always be using
>> >a dynamically allocated log type.
>>
>> Even though it uses old-fashioned logging that change could still
>> improve output for some users out there.
>>
>> Anyway, if you don't want any extra commit noise let's abandon that and these improvements will
>show up in dynamic logging patch.
>
>Keep it for now, but soon RTE_LOGTYPE_PMD will be marked deprecated.

Okay. Will respin that and send separate patch that uses dynamic logging instead.
diff mbox series

Patch

diff --git a/drivers/raw/cnxk_gpio/cnxk_gpio.c b/drivers/raw/cnxk_gpio/cnxk_gpio.c
index dcd646397e..6c4e4f5eae 100644
--- a/drivers/raw/cnxk_gpio/cnxk_gpio.c
+++ b/drivers/raw/cnxk_gpio/cnxk_gpio.c
@@ -713,7 +713,7 @@  cnxk_gpio_probe(struct rte_vdev_device *dev)
 	cnxk_gpio_format_name(name, sizeof(name));
 	rawdev = rte_rawdev_pmd_allocate(name, sizeof(*gpiochip), rte_socket_id());
 	if (!rawdev) {
-		RTE_LOG(ERR, PMD, "failed to allocate %s rawdev", name);
+		RTE_LOG(ERR, PMD, "failed to allocate %s rawdev\n", name);
 		return -ENOMEM;
 	}
 
@@ -753,7 +753,7 @@  cnxk_gpio_probe(struct rte_vdev_device *dev)
 	snprintf(buf, sizeof(buf), "%s/gpiochip%d/base", CNXK_GPIO_CLASS_PATH, gpiochip->num);
 	ret = cnxk_gpio_read_attr_int(buf, &gpiochip->base);
 	if (ret) {
-		RTE_LOG(ERR, PMD, "failed to read %s", buf);
+		RTE_LOG(ERR, PMD, "failed to read %s\n", buf);
 		goto out;
 	}
 
@@ -761,7 +761,7 @@  cnxk_gpio_probe(struct rte_vdev_device *dev)
 	snprintf(buf, sizeof(buf), "%s/gpiochip%d/ngpio", CNXK_GPIO_CLASS_PATH, gpiochip->num);
 	ret = cnxk_gpio_read_attr_int(buf, &gpiochip->num_gpios);
 	if (ret) {
-		RTE_LOG(ERR, PMD, "failed to read %s", buf);
+		RTE_LOG(ERR, PMD, "failed to read %s\n", buf);
 		goto out;
 	}
 	gpiochip->num_queues = gpiochip->num_gpios;
@@ -774,7 +774,7 @@  cnxk_gpio_probe(struct rte_vdev_device *dev)
 
 	gpiochip->gpios = rte_calloc(NULL, gpiochip->num_gpios, sizeof(struct cnxk_gpio *), 0);
 	if (!gpiochip->gpios) {
-		RTE_LOG(ERR, PMD, "failed to allocate gpios memory");
+		RTE_LOG(ERR, PMD, "failed to allocate gpios memory\n");
 		ret = -ENOMEM;
 		goto out;
 	}