2021-07-08 15:24:29

by Carlos Bilbao

[permalink] [raw]
Subject: [PATCH] drivers: net: Follow the indentation coding standard on printks

Fix indentation of printks that start at the beginning of the line. Change this
for the right number of space characters, or tabs if the file uses them.

Signed-off-by: Carlos Bilbao <[email protected]>
---
drivers/net/ethernet/dec/tulip/de4x5.c | 2 +-
drivers/net/sb1000.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/dec/tulip/de4x5.c b/drivers/net/ethernet/dec/tulip/de4x5.c
index b125d7faefdf..155cfe8800cd 100644
--- a/drivers/net/ethernet/dec/tulip/de4x5.c
+++ b/drivers/net/ethernet/dec/tulip/de4x5.c
@@ -3169,7 +3169,7 @@ dc2114x_autoconf(struct net_device *dev)

default:
lp->tcount++;
-printk("Huh?: media:%02x\n", lp->media);
+ printk("Huh?: media:%02x\n", lp->media);
lp->media = INIT;
break;
}
diff --git a/drivers/net/sb1000.c b/drivers/net/sb1000.c
index e88af978f63c..54a7c7613434 100644
--- a/drivers/net/sb1000.c
+++ b/drivers/net/sb1000.c
@@ -760,7 +760,7 @@ sb1000_rx(struct net_device *dev)

insw(ioaddr, (unsigned short*) st, 1);
#ifdef XXXDEBUG
-printk("cm0: received: %02x %02x\n", st[0], st[1]);
+ printk("cm0: received: %02x %02x\n", st[0], st[1]);
#endif /* XXXDEBUG */
lp->rx_frames++;

@@ -805,7 +805,7 @@ printk("cm0: received: %02x %02x\n", st[0], st[1]);
/* get data length */
insw(ioaddr, buffer, NewDatagramHeaderSize / 2);
#ifdef XXXDEBUG
-printk("cm0: IP identification: %02x%02x fragment offset: %02x%02x\n", buffer[30], buffer[31], buffer[32], buffer[33]);
+ printk("cm0: IP identification: %02x%02x fragment offset: %02x%02x\n", buffer[30], buffer[31], buffer[32], buffer[33]);
#endif /* XXXDEBUG */
if (buffer[0] != NewDatagramHeaderSkip) {
if (sb1000_debug > 1)
--
2.25.1




2021-07-08 16:44:32

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] drivers: net: Follow the indentation coding standard on printks

On Thu, 2021-07-08 at 11:23 -0400, Carlos Bilbao wrote:
> Fix indentation of printks that start at the beginning of the line. Change this
> for the right number of space characters, or tabs if the file uses them.

The tulip and sb1000 code are from the 1990's.
Likely this doesn't need touching, but if you really want to:

> diff --git a/drivers/net/ethernet/dec/tulip/de4x5.c b/drivers/net/ethernet/dec/tulip/de4x5.c
[]
> @@ -3169,7 +3169,7 @@ dc2114x_autoconf(struct net_device *dev)
> ?
>
> ?????default:
> ? lp->tcount++;
> -printk("Huh?: media:%02x\n", lp->media);
> + printk("Huh?: media:%02x\n", lp->media);

There should be a KERN_<LEVEL> here like KERN_WARNING/KERN_NOTICE

> diff --git a/drivers/net/sb1000.c b/drivers/net/sb1000.c
[]
> @@ -760,7 +760,7 @@ sb1000_rx(struct net_device *dev)
> ?
>
> ? insw(ioaddr, (unsigned short*) st, 1);
> ?#ifdef XXXDEBUG
> -printk("cm0: received: %02x %02x\n", st[0], st[1]);
> + printk("cm0: received: %02x %02x\n", st[0], st[1]);
> ?#endif /* XXXDEBUG */

XXXDEBUG isn't defined anywhere so these could just be deleted.

> @@ -805,7 +805,7 @@ printk("cm0: received: %02x %02x\n", st[0], st[1]);
> ? /* get data length */
> ? insw(ioaddr, buffer, NewDatagramHeaderSize / 2);
> ?#ifdef XXXDEBUG
> -printk("cm0: IP identification: %02x%02x fragment offset: %02x%02x\n", buffer[30], buffer[31], buffer[32], buffer[33]);
> + printk("cm0: IP identification: %02x%02x fragment offset: %02x%02x\n", buffer[30], buffer[31], buffer[32], buffer[33]);
> ?#endif /* XXXDEBUG */
> ? if (buffer[0] != NewDatagramHeaderSkip) {
> ? if (sb1000_debug > 1)


2021-07-08 17:34:55

by Carlos Bilbao

[permalink] [raw]
Subject: [PATCH net-next v2] drivers: net: Follow the indentation coding standard on printks

Fix indentation of printks that start at the beginning of the line. Change this
for the right number of space characters, or tabs if the file uses them.

Signed-off-by: Carlos Bilbao <[email protected]>
---
Changelog:
v2 - Remove the printks inside XXXDEBUG
---
diff --git a/drivers/net/ethernet/dec/tulip/de4x5.c b/drivers/net/ethernet/dec/tulip/de4x5.c
index b125d7faefdf..0d8ddfdd5c09 100644
--- a/drivers/net/ethernet/dec/tulip/de4x5.c
+++ b/drivers/net/ethernet/dec/tulip/de4x5.c
@@ -3169,7 +3169,7 @@ dc2114x_autoconf(struct net_device *dev)

default:
lp->tcount++;
-printk("Huh?: media:%02x\n", lp->media);
+ printk(KERN_NOTICE "Huh?: media:%02x\n", lp->media);
lp->media = INIT;
break;
}
diff --git a/drivers/net/ethernet/dec/tulip/de4x5.c.rej b/drivers/net/ethernet/dec/tulip/de4x5.c.rej
new file mode 100644
index 000000000000..949b9902b0bc
--- /dev/null
+++ b/drivers/net/ethernet/dec/tulip/de4x5.c.rej
@@ -0,0 +1,11 @@
+--- drivers/net/ethernet/dec/tulip/de4x5.c
++++ drivers/net/ethernet/dec/tulip/de4x5.c
+@@ -3169,7 +3169,7 @@ dc2114x_autoconf(struct net_device *dev)
+
+ default:
+ lp->tcount++;
+-printk("Huh?: media:%02x\n", lp->media);
++ printk("Huh?: media:%02x\n", lp->media);
+ lp->media = INIT;
+ break;
+ }
diff --git a/drivers/net/sb1000.c b/drivers/net/sb1000.c
index e88af978f63c..a7a6bd7ef015 100644
--- a/drivers/net/sb1000.c
+++ b/drivers/net/sb1000.c
@@ -759,9 +759,6 @@ sb1000_rx(struct net_device *dev)
ioaddr = dev->base_addr;

insw(ioaddr, (unsigned short*) st, 1);
-#ifdef XXXDEBUG
-printk("cm0: received: %02x %02x\n", st[0], st[1]);
-#endif /* XXXDEBUG */
lp->rx_frames++;

/* decide if it is a good or bad frame */
@@ -804,9 +801,6 @@ printk("cm0: received: %02x %02x\n", st[0], st[1]);
if (st[0] & 0x40) {
/* get data length */
insw(ioaddr, buffer, NewDatagramHeaderSize / 2);
-#ifdef XXXDEBUG
-printk("cm0: IP identification: %02x%02x fragment offset: %02x%02x\n", buffer[30], buffer[31], buffer[32], buffer[33]);
-#endif /* XXXDEBUG */
if (buffer[0] != NewDatagramHeaderSkip) {
if (sb1000_debug > 1)
printk(KERN_WARNING "%s: new datagram header skip error: "
--
2.25.1



2021-07-08 17:40:21

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH net-next v2] drivers: net: Follow the indentation coding standard on printks

On Thu, 2021-07-08 at 13:33 -0400, Carlos Bilbao wrote:
> Fix indentation of printks that start at the beginning of the line. Change this
> for the right number of space characters, or tabs if the file uses them.

You are touching 2 different drivers and this should really be
2 separate patches.

> diff --git a/drivers/net/ethernet/dec/tulip/de4x5.c.rej b/drivers/net/ethernet/dec/tulip/de4x5.c.rej
[]
> +++ b/drivers/net/ethernet/dec/tulip/de4x5.c.rej
> @@ -0,0 +1,11 @@
> +--- drivers/net/ethernet/dec/tulip/de4x5.c
> ++++ drivers/net/ethernet/dec/tulip/de4x5.c
> +@@ -3169,7 +3169,7 @@ dc2114x_autoconf(struct net_device *dev)
> +
> + default:
> + lp->tcount++;
> +-printk("Huh?: media:%02x\n", lp->media);
> ++ printk("Huh?: media:%02x\n", lp->media);
> + lp->media = INIT;
> + break;
> + }

This is an interdiff that should not be part of this change.


2021-07-08 17:45:33

by Carlos Bilbao

[permalink] [raw]
Subject: Re: [PATCH net-next v2] drivers: net: Follow the indentation coding standard on printks

Hello Joe,

I apologize for the mess, I will send two different patches for the drivers now.

thanks,
Carlos.



2021-07-08 17:50:28

by Carlos Bilbao

[permalink] [raw]
Subject: [PATCH net-next v2] drivers: ethernet: tulip: Fix indentation of printk

Fix indentation of printk that starts at the beginning of the line and does
not have a KERN_<LEVEL>.

Signed-off-by: Carlos Bilbao <[email protected]>
---
drivers/net/ethernet/dec/tulip/de4x5.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/dec/tulip/de4x5.c b/drivers/net/ethernet/dec/tulip/de4x5.c
index b125d7faefdf..0d8ddfdd5c09 100644
--- a/drivers/net/ethernet/dec/tulip/de4x5.c
+++ b/drivers/net/ethernet/dec/tulip/de4x5.c
@@ -3169,7 +3169,7 @@ dc2114x_autoconf(struct net_device *dev)

default:
lp->tcount++;
-printk("Huh?: media:%02x\n", lp->media);
+ printk(KERN_NOTICE "Huh?: media:%02x\n", lp->media);
lp->media = INIT;
break;
}
--
2.25.1



2021-07-08 17:53:54

by Carlos Bilbao

[permalink] [raw]
Subject: [PATCH net-next v2] drivers: net: Remove undefined XXXDEBUG on driver sb1000

XXXDEBUG isn't defined anywhere so these can be deleted from this file.

Signed-off-by: Carlos Bilbao <[email protected]>
---
drivers/net/sb1000.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/drivers/net/sb1000.c b/drivers/net/sb1000.c
index e88af978f63c..a7a6bd7ef015 100644
--- a/drivers/net/sb1000.c
+++ b/drivers/net/sb1000.c
@@ -759,9 +759,6 @@ sb1000_rx(struct net_device *dev)
ioaddr = dev->base_addr;

insw(ioaddr, (unsigned short*) st, 1);
-#ifdef XXXDEBUG
-printk("cm0: received: %02x %02x\n", st[0], st[1]);
-#endif /* XXXDEBUG */
lp->rx_frames++;

/* decide if it is a good or bad frame */
@@ -804,9 +801,6 @@ printk("cm0: received: %02x %02x\n", st[0], st[1]);
if (st[0] & 0x40) {
/* get data length */
insw(ioaddr, buffer, NewDatagramHeaderSize / 2);
-#ifdef XXXDEBUG
-printk("cm0: IP identification: %02x%02x fragment offset: %02x%02x\n", buffer[30], buffer[31], buffer[32], buffer[33]);
-#endif /* XXXDEBUG */
if (buffer[0] != NewDatagramHeaderSkip) {
if (sb1000_debug > 1)
printk(KERN_WARNING "%s: new datagram header skip error: "
--
2.25.1



2021-07-08 18:00:37

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v2] drivers: ethernet: tulip: Fix indentation of printk

On Thu, Jul 08, 2021 at 01:48:24PM -0400, Carlos Bilbao wrote:
> Fix indentation of printk that starts at the beginning of the line and does
> not have a KERN_<LEVEL>.
>
> Signed-off-by: Carlos Bilbao <[email protected]>
> ---
> drivers/net/ethernet/dec/tulip/de4x5.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/dec/tulip/de4x5.c b/drivers/net/ethernet/dec/tulip/de4x5.c
> index b125d7faefdf..0d8ddfdd5c09 100644
> --- a/drivers/net/ethernet/dec/tulip/de4x5.c
> +++ b/drivers/net/ethernet/dec/tulip/de4x5.c
> @@ -3169,7 +3169,7 @@ dc2114x_autoconf(struct net_device *dev)
>
> default:
> lp->tcount++;
> -printk("Huh?: media:%02x\n", lp->media);
> + printk(KERN_NOTICE "Huh?: media:%02x\n", lp->media);
> lp->media = INIT;
> break;
> }

Since this is a network driver, and you have a net_device structure,
the best practice is to use

netdev_notice(dev, "Huh?: media:%02x\n", lp->media);

You could go through this driver and change all printk() to
netdev_dbg(), netdev_err(), netdev_info etc. The advantage of these
calls is that they make it clear which network interface is outputting
the message.

Other subsystems have similar calls. If there are not subsystem
specific print functions, but you have a struct device, it is best to
use dev_err(), dev_dbg(), dev_info() etc. These functions will make it
clear which device is printing the message.

Andrew