2021-07-08 13:12:28

by Carlos Bilbao

[permalink] [raw]
Subject: [PATCH] drivers: 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/atm/eni.c | 2 +-
drivers/atm/iphase.c | 2 +-
drivers/atm/suni.c | 4 ++--
drivers/atm/zatm.c | 8 ++++----
drivers/net/ethernet/dec/tulip/de4x5.c | 2 +-
drivers/net/sb1000.c | 4 ++--
drivers/parisc/iosapic.c | 4 ++--
drivers/parisc/sba_iommu.c | 2 +-
8 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/atm/eni.c b/drivers/atm/eni.c
index 422753d52244..6d10fd62ba7e 100644
--- a/drivers/atm/eni.c
+++ b/drivers/atm/eni.c
@@ -1456,7 +1456,7 @@ static int start_tx(struct atm_dev *dev)

static void foo(void)
{
-printk(KERN_INFO
+ printk(KERN_INFO
"tx_complete=%d,dma_complete=%d,queued=%d,requeued=%d,sub=%d,\n"
"backlogged=%d,rx_enqueued=%d,rx_dequeued=%d,putting=%d,pushed=%d\n",
tx_complete,dma_complete,queued,requeued,submitted,backlogged,
diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c
index bc8e8d9f176b..65bb700cd5af 100644
--- a/drivers/atm/iphase.c
+++ b/drivers/atm/iphase.c
@@ -1246,7 +1246,7 @@ static void rx_intr(struct atm_dev *dev)
((iadev->rx_pkt_cnt - iadev->rx_tmp_cnt) == 0)) {
for (i = 1; i <= iadev->num_rx_desc; i++)
free_desc(dev, i);
-printk("Test logic RUN!!!!\n");
+ printk("Test logic RUN!!!!\n");
writew( ~(RX_FREEQ_EMPT|RX_EXCP_RCVD),iadev->reass_reg+REASS_MASK_REG);
iadev->rxing = 1;
}
diff --git a/drivers/atm/suni.c b/drivers/atm/suni.c
index 21e5acc766b8..149605cdb859 100644
--- a/drivers/atm/suni.c
+++ b/drivers/atm/suni.c
@@ -328,8 +328,8 @@ static int suni_start(struct atm_dev *dev)
timer_setup(&poll_timer, suni_hz, 0);
poll_timer.expires = jiffies+HZ;
#if 0
-printk(KERN_DEBUG "[u] p=0x%lx,n=0x%lx\n",(unsigned long) poll_timer.list.prev,
- (unsigned long) poll_timer.list.next);
+ printk(KERN_DEBUG "[u] p=0x%lx,n=0x%lx\n",(unsigned long) poll_timer.list.prev,
+ (unsigned long) poll_timer.list.next);
#endif
add_timer(&poll_timer);
}
diff --git a/drivers/atm/zatm.c b/drivers/atm/zatm.c
index cf5fffcf98a1..4fb89ed47311 100644
--- a/drivers/atm/zatm.c
+++ b/drivers/atm/zatm.c
@@ -380,7 +380,7 @@ static void poll_rx(struct atm_dev *dev,int mbx)
pos = zatm_dev->mbx_start[mbx];
cells = here[0] & uPD98401_AAL5_SIZE;
#if 0
-printk("RX IND: 0x%x, 0x%x, 0x%x, 0x%x\n",here[0],here[1],here[2],here[3]);
+ printk("RX IND: 0x%x, 0x%x, 0x%x, 0x%x\n",here[0],here[1],here[2],here[3]);
{
unsigned long *x;
printk("POOL: 0x%08x, 0x%08x\n",zpeekl(zatm_dev,
@@ -403,14 +403,14 @@ EVENT("error code 0x%x/0x%x\n",(here[3] & uPD98401_AAL5_ES) >>
skb = ((struct rx_buffer_head *) bus_to_virt(here[2]))->skb;
__net_timestamp(skb);
#if 0
-printk("[-3..0] 0x%08lx 0x%08lx 0x%08lx 0x%08lx\n",((unsigned *) skb->data)[-3],
+ printk("[-3..0] 0x%08lx 0x%08lx 0x%08lx 0x%08lx\n",((unsigned *) skb->data)[-3],
((unsigned *) skb->data)[-2],((unsigned *) skb->data)[-1],
((unsigned *) skb->data)[0]);
#endif
EVENT("skb 0x%lx, here 0x%lx\n",(unsigned long) skb,
(unsigned long) here);
#if 0
-printk("dummy: 0x%08lx, 0x%08lx\n",dummy[0],dummy[1]);
+ printk("dummy: 0x%08lx, 0x%08lx\n",dummy[0],dummy[1]);
#endif
size = error ? 0 : ntohs(((__be16 *) skb->data)[cells*
ATM_CELL_PAYLOAD/sizeof(u16)-3]);
@@ -664,7 +664,7 @@ static int do_tx(struct sk_buff *skb)
EVENT("dsc (0x%lx)\n",(unsigned long) dsc,0);
}
else {
-printk("NONONONOO!!!!\n");
+ printk("NONONONOO!!!!\n");
dsc = NULL;
#if 0
u32 *put;
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)
diff --git a/drivers/parisc/iosapic.c b/drivers/parisc/iosapic.c
index 8a3b0c3a1e92..5d27c23e6429 100644
--- a/drivers/parisc/iosapic.c
+++ b/drivers/parisc/iosapic.c
@@ -633,7 +633,7 @@ static void iosapic_unmask_irq(struct irq_data *d)
printk("\n");
}

-printk("iosapic_enable_irq(): sel ");
+ printk("iosapic_enable_irq(): sel ");
{
struct iosapic_info *isp = vi->iosapic;

@@ -642,7 +642,7 @@ printk("iosapic_enable_irq(): sel ");
printk(" %x", d1);
}
}
-printk("\n");
+ printk("\n");
#endif

/*
diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
index dce4cdf786cd..c3381facdfc5 100644
--- a/drivers/parisc/sba_iommu.c
+++ b/drivers/parisc/sba_iommu.c
@@ -1550,7 +1550,7 @@ static void sba_hw_init(struct sba_device *sba_dev)


#if 0
-printk("sba_hw_init(): mem_boot 0x%x 0x%x 0x%x 0x%x\n", PAGE0->mem_boot.hpa,
+ printk("sba_hw_init(): mem_boot 0x%x 0x%x 0x%x 0x%x\n", PAGE0->mem_boot.hpa,
PAGE0->mem_boot.spa, PAGE0->mem_boot.pad, PAGE0->mem_boot.cl_class);

/*
--
2.25.1




2021-07-08 14:41:58

by Greg Kroah-Hartman

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

On Thu, Jul 08, 2021 at 09:10:01AM -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.
>
> Signed-off-by: Carlos Bilbao <[email protected]>
> ---
> drivers/atm/eni.c | 2 +-
> drivers/atm/iphase.c | 2 +-
> drivers/atm/suni.c | 4 ++--
> drivers/atm/zatm.c | 8 ++++----
> drivers/net/ethernet/dec/tulip/de4x5.c | 2 +-
> drivers/net/sb1000.c | 4 ++--
> drivers/parisc/iosapic.c | 4 ++--
> drivers/parisc/sba_iommu.c | 2 +-
> 8 files changed, 14 insertions(+), 14 deletions(-)

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch did many different things all at once, making it difficult
to review. All Linux kernel patches need to only do one thing at a
time. If you need to do multiple things (such as clean up all coding
style issues in a file/driver), do it in a sequence of patches, each
one doing only one thing. This will make it easier to review the
patches to ensure that they are correct, and to help alleviate any
merge issues that larger patches can cause.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

2021-07-08 14:55:19

by Andrew Lunn

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

> --- a/drivers/atm/suni.c
> +++ b/drivers/atm/suni.c
> @@ -328,8 +328,8 @@ static int suni_start(struct atm_dev *dev)
> timer_setup(&poll_timer, suni_hz, 0);
> poll_timer.expires = jiffies+HZ;
> #if 0
> -printk(KERN_DEBUG "[u] p=0x%lx,n=0x%lx\n",(unsigned long) poll_timer.list.prev,
> - (unsigned long) poll_timer.list.next);
> + printk(KERN_DEBUG "[u] p=0x%lx,n=0x%lx\n",(unsigned long) poll_timer.list.prev,
> + (unsigned long) poll_timer.list.next);

Why not use DPRINTK(), defined at the start of suni.c?

Andrew

2021-07-08 15:09:41

by Carlos Bilbao

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

Hello,

> - Your patch did many different things all at once, making it difficult
> to review. All Linux kernel patches need to only do one thing at a
> time. If you need to do multiple things (such as clean up all coding

Greg, I am going to divide the patch in three parts, for atm/, net/ and parisc/.

>
> Why not use DPRINTK(), defined at the start of suni.c?
>
> Andrew

Thanks for the idea Andrew, I will make a new version of the net/ patch.

thanks,
Carlos.


2021-07-08 15:21:03

by Carlos Bilbao

[permalink] [raw]
Subject: [PATCH] drivers: atm: 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: Replaced printk for DPRINTK on suni.c
---
drivers/atm/eni.c | 2 +-
drivers/atm/iphase.c | 2 +-
drivers/atm/suni.c | 4 ++--
drivers/atm/zatm.c | 8 ++++----
4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/atm/eni.c b/drivers/atm/eni.c
index 422753d52244..6d10fd62ba7e 100644
--- a/drivers/atm/eni.c
+++ b/drivers/atm/eni.c
@@ -1456,7 +1456,7 @@ static int start_tx(struct atm_dev *dev)

static void foo(void)
{
-printk(KERN_INFO
+ printk(KERN_INFO
"tx_complete=%d,dma_complete=%d,queued=%d,requeued=%d,sub=%d,\n"
"backlogged=%d,rx_enqueued=%d,rx_dequeued=%d,putting=%d,pushed=%d\n",
tx_complete,dma_complete,queued,requeued,submitted,backlogged,
diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c
index bc8e8d9f176b..65bb700cd5af 100644
--- a/drivers/atm/iphase.c
+++ b/drivers/atm/iphase.c
@@ -1246,7 +1246,7 @@ static void rx_intr(struct atm_dev *dev)
((iadev->rx_pkt_cnt - iadev->rx_tmp_cnt) == 0)) {
for (i = 1; i <= iadev->num_rx_desc; i++)
free_desc(dev, i);
-printk("Test logic RUN!!!!\n");
+ printk("Test logic RUN!!!!\n");
writew( ~(RX_FREEQ_EMPT|RX_EXCP_RCVD),iadev->reass_reg+REASS_MASK_REG);
iadev->rxing = 1;
}
diff --git a/drivers/atm/suni.c b/drivers/atm/suni.c
index 21e5acc766b8..149605cdb859 100644
--- a/drivers/atm/suni.c
+++ b/drivers/atm/suni.c
@@ -328,8 +328,8 @@ static int suni_start(struct atm_dev *dev)
timer_setup(&poll_timer, suni_hz, 0);
poll_timer.expires = jiffies+HZ;
#if 0
-printk(KERN_DEBUG "[u] p=0x%lx,n=0x%lx\n",(unsigned long) poll_timer.list.prev,
- (unsigned long) poll_timer.list.next);
+ DPRINTK("[u] p=0x%lx,n=0x%lx\n",(unsigned long) poll_timer.list.prev,
+ (unsigned long) poll_timer.list.next);
#endif
add_timer(&poll_timer);
}
diff --git a/drivers/atm/zatm.c b/drivers/atm/zatm.c
index cf5fffcf98a1..4fb89ed47311 100644
--- a/drivers/atm/zatm.c
+++ b/drivers/atm/zatm.c
@@ -380,7 +380,7 @@ static void poll_rx(struct atm_dev *dev,int mbx)
pos = zatm_dev->mbx_start[mbx];
cells = here[0] & uPD98401_AAL5_SIZE;
#if 0
-printk("RX IND: 0x%x, 0x%x, 0x%x, 0x%x\n",here[0],here[1],here[2],here[3]);
+ printk("RX IND: 0x%x, 0x%x, 0x%x, 0x%x\n",here[0],here[1],here[2],here[3]);
{
unsigned long *x;
printk("POOL: 0x%08x, 0x%08x\n",zpeekl(zatm_dev,
@@ -403,14 +403,14 @@ EVENT("error code 0x%x/0x%x\n",(here[3] & uPD98401_AAL5_ES) >>
skb = ((struct rx_buffer_head *) bus_to_virt(here[2]))->skb;
__net_timestamp(skb);
#if 0
-printk("[-3..0] 0x%08lx 0x%08lx 0x%08lx 0x%08lx\n",((unsigned *) skb->data)[-3],
+ printk("[-3..0] 0x%08lx 0x%08lx 0x%08lx 0x%08lx\n",((unsigned *) skb->data)[-3],
((unsigned *) skb->data)[-2],((unsigned *) skb->data)[-1],
((unsigned *) skb->data)[0]);
#endif
EVENT("skb 0x%lx, here 0x%lx\n",(unsigned long) skb,
(unsigned long) here);
#if 0
-printk("dummy: 0x%08lx, 0x%08lx\n",dummy[0],dummy[1]);
+ printk("dummy: 0x%08lx, 0x%08lx\n",dummy[0],dummy[1]);
#endif
size = error ? 0 : ntohs(((__be16 *) skb->data)[cells*
ATM_CELL_PAYLOAD/sizeof(u16)-3]);
@@ -664,7 +664,7 @@ static int do_tx(struct sk_buff *skb)
EVENT("dsc (0x%lx)\n",(unsigned long) dsc,0);
}
else {
-printk("NONONONOO!!!!\n");
+ printk("NONONONOO!!!!\n");
dsc = NULL;
#if 0
u32 *put;
--
2.25.1



2021-07-08 15:50:28

by Andrew Lunn

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

On Thu, Jul 08, 2021 at 11:19:19AM -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.
>
> Signed-off-by: Carlos Bilbao <[email protected]>
> ---
> Changelog: Replaced printk for DPRINTK on suni.c
> ---
> drivers/atm/eni.c | 2 +-
> drivers/atm/iphase.c | 2 +-
> drivers/atm/suni.c | 4 ++--
> drivers/atm/zatm.c | 8 ++++----
> 4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/atm/eni.c b/drivers/atm/eni.c
> index 422753d52244..6d10fd62ba7e 100644
> --- a/drivers/atm/eni.c
> +++ b/drivers/atm/eni.c
> @@ -1456,7 +1456,7 @@ static int start_tx(struct atm_dev *dev)
>
> static void foo(void)
> {
> -printk(KERN_INFO
> + printk(KERN_INFO
> "tx_complete=%d,dma_complete=%d,queued=%d,requeued=%d,sub=%d,\n"
> "backlogged=%d,rx_enqueued=%d,rx_dequeued=%d,putting=%d,pushed=%d\n",
> tx_complete,dma_complete,queued,requeued,submitted,backlogged,

Did you just blindly fix the warning, or look at the surrounding code
and think a bit?

There is the comment:

/* may become useful again when tuning things */

What does git log show? When was the last tuning? When was the last
serious change made to this driver which was not an automated/manual
cleanup? My guess is, you need to go back to at least 2005.

So maybe it is time to remove this #if 0 code?

Please also read

https://www.kernel.org/doc/Documentation/process/submitting-patches.rst

and

https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html

Andrew

2021-07-08 17:08:17

by Carlos Bilbao

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

Hello,

>
> So maybe it is time to remove this #if 0 code?
>

I agree Andrew. It is indeed very old code. I will just remove this #if 0 and
send a new version of this patch.

thanks,
Carlos.




2021-07-08 17:24:21

by Carlos Bilbao

[permalink] [raw]
Subject: [PATCH net-next v2] drivers: atm: 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:
- Replaced printk for DPRINTK on suni.c
- Directly removed the #if 0 printks.
---





2021-07-08 17:29:43

by Carlos Bilbao

[permalink] [raw]
Subject: [PATCH net-next v2] drivers: atm: 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. Also
directly remove the printks inside #if 0.

Signed-off-by: Carlos Bilbao <[email protected]>
---
Changelog:
- Replaced printk for DPRINTK on suni.c
- Directly removed the #if 0 printks.
---
drivers/atm/eni.c | 23 -----------------------
drivers/atm/iphase.c | 4 ++--
drivers/atm/suni.c | 4 ----
drivers/atm/zatm.c | 22 +---------------------
4 files changed, 3 insertions(+), 50 deletions(-)

diff --git a/drivers/atm/eni.c b/drivers/atm/eni.c
index 422753d52244..d766500e91a2 100644
--- a/drivers/atm/eni.c
+++ b/drivers/atm/eni.c
@@ -1448,25 +1448,6 @@ static int start_tx(struct atm_dev *dev)
return 0;
}

-
-/*--------------------------------- common ----------------------------------*/
-
-
-#if 0 /* may become useful again when tuning things */
-
-static void foo(void)
-{
-printk(KERN_INFO
- "tx_complete=%d,dma_complete=%d,queued=%d,requeued=%d,sub=%d,\n"
- "backlogged=%d,rx_enqueued=%d,rx_dequeued=%d,putting=%d,pushed=%d\n",
- tx_complete,dma_complete,queued,requeued,submitted,backlogged,
- rx_enqueued,rx_dequeued,putting,pushed);
-if (eni_boards) printk(KERN_INFO "loss: %ld\n",ENI_DEV(eni_boards)->lost);
-}
-
-#endif
-
-
static void bug_int(struct atm_dev *dev,unsigned long reason)
{
DPRINTK(">bug_int\n");
@@ -1509,9 +1490,6 @@ static irqreturn_t eni_int(int irq,void *dev_id)
if (reason & MID_SUNI_INT) {
EVENT("SUNI int\n",0,0);
dev->phy->interrupt(dev);
-#if 0
- foo();
-#endif
}
spin_lock(&eni_dev->lock);
eni_dev->events |= reason;
@@ -1901,7 +1879,6 @@ static void eni_close(struct atm_vcc *vcc)
kfree(ENI_VCC(vcc));
vcc->dev_data = NULL;
clear_bit(ATM_VF_ADDR,&vcc->flags);
- /*foo();*/
}


diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c
index bc8e8d9f176b..8a3ee51e6c9f 100644
--- a/drivers/atm/iphase.c
+++ b/drivers/atm/iphase.c
@@ -1246,8 +1246,8 @@ static void rx_intr(struct atm_dev *dev)
((iadev->rx_pkt_cnt - iadev->rx_tmp_cnt) == 0)) {
for (i = 1; i <= iadev->num_rx_desc; i++)
free_desc(dev, i);
-printk("Test logic RUN!!!!\n");
- writew( ~(RX_FREEQ_EMPT|RX_EXCP_RCVD),iadev->reass_reg+REASS_MASK_REG);
+ printk("Test logic RUN!!!!\n");
+ writew( ~(RX_FREEQ_EMPT|RX_EXCP_RCVD),iadev->reass_reg+REASS_MASK_REG);
iadev->rxing = 1;
}
IF_EVENT(printk("Rx intr status: RX_FREEQ_EMPT %08x\n", status);)
diff --git a/drivers/atm/suni.c b/drivers/atm/suni.c
index 21e5acc766b8..eaafcfc3b229 100644
--- a/drivers/atm/suni.c
+++ b/drivers/atm/suni.c
@@ -327,10 +327,6 @@ static int suni_start(struct atm_dev *dev)
if (first) {
timer_setup(&poll_timer, suni_hz, 0);
poll_timer.expires = jiffies+HZ;
-#if 0
-printk(KERN_DEBUG "[u] p=0x%lx,n=0x%lx\n",(unsigned long) poll_timer.list.prev,
- (unsigned long) poll_timer.list.next);
-#endif
add_timer(&poll_timer);
}
return 0;
diff --git a/drivers/atm/zatm.c b/drivers/atm/zatm.c
index cf5fffcf98a1..a487fcb9282d 100644
--- a/drivers/atm/zatm.c
+++ b/drivers/atm/zatm.c
@@ -379,18 +379,6 @@ static void poll_rx(struct atm_dev *dev,int mbx)
if (((pos += 16) & 0xffff) == zatm_dev->mbx_end[mbx])
pos = zatm_dev->mbx_start[mbx];
cells = here[0] & uPD98401_AAL5_SIZE;
-#if 0
-printk("RX IND: 0x%x, 0x%x, 0x%x, 0x%x\n",here[0],here[1],here[2],here[3]);
-{
-unsigned long *x;
- printk("POOL: 0x%08x, 0x%08x\n",zpeekl(zatm_dev,
- zatm_dev->pool_base),
- zpeekl(zatm_dev,zatm_dev->pool_base+1));
- x = (unsigned long *) here[2];
- printk("[0..3] = 0x%08lx, 0x%08lx, 0x%08lx, 0x%08lx\n",
- x[0],x[1],x[2],x[3]);
-}
-#endif
error = 0;
if (here[3] & uPD98401_AAL5_ERR) {
error = (here[3] & uPD98401_AAL5_ES) >>
@@ -402,16 +390,8 @@ EVENT("error code 0x%x/0x%x\n",(here[3] & uPD98401_AAL5_ES) >>
uPD98401_AAL5_ES_SHIFT,error);
skb = ((struct rx_buffer_head *) bus_to_virt(here[2]))->skb;
__net_timestamp(skb);
-#if 0
-printk("[-3..0] 0x%08lx 0x%08lx 0x%08lx 0x%08lx\n",((unsigned *) skb->data)[-3],
- ((unsigned *) skb->data)[-2],((unsigned *) skb->data)[-1],
- ((unsigned *) skb->data)[0]);
-#endif
EVENT("skb 0x%lx, here 0x%lx\n",(unsigned long) skb,
(unsigned long) here);
-#if 0
-printk("dummy: 0x%08lx, 0x%08lx\n",dummy[0],dummy[1]);
-#endif
size = error ? 0 : ntohs(((__be16 *) skb->data)[cells*
ATM_CELL_PAYLOAD/sizeof(u16)-3]);
EVENT("got skb 0x%lx, size %d\n",(unsigned long) skb,size);
@@ -664,7 +644,7 @@ static int do_tx(struct sk_buff *skb)
EVENT("dsc (0x%lx)\n",(unsigned long) dsc,0);
}
else {
-printk("NONONONOO!!!!\n");
+ printk("NONONONOO!!!!\n");
dsc = NULL;
#if 0
u32 *put;
--
2.25.1




2021-07-08 18:17:40

by Andrew Lunn

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

> diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c
> index bc8e8d9f176b..8a3ee51e6c9f 100644
> --- a/drivers/atm/iphase.c
> +++ b/drivers/atm/iphase.c
> @@ -1246,8 +1246,8 @@ static void rx_intr(struct atm_dev *dev)
> ((iadev->rx_pkt_cnt - iadev->rx_tmp_cnt) == 0)) {
> for (i = 1; i <= iadev->num_rx_desc; i++)
> free_desc(dev, i);
> -printk("Test logic RUN!!!!\n");
> - writew( ~(RX_FREEQ_EMPT|RX_EXCP_RCVD),iadev->reass_reg+REASS_MASK_REG);
> + printk("Test logic RUN!!!!\n");
> + writew( ~(RX_FREEQ_EMPT|RX_EXCP_RCVD),iadev->reass_reg+REASS_MASK_REG);
> iadev->rxing = 1;
> }

It looks like you turned a set of spaces into a tab for the writew()
line. Please don't make such whitespace changes in the same patch as
other changes.

Yes, lots of things to learn before you get your first patch accepted...

>> +++ b/drivers/atm/zatm.c
> @@ -664,7 +644,7 @@ static int do_tx(struct sk_buff *skb)
> EVENT("dsc (0x%lx)\n",(unsigned long) dsc,0);
> }
> else {
> -printk("NONONONOO!!!!\n");
> + printk("NONONONOO!!!!\n");
> dsc = NULL;

This seems like an error message. So maybe

pr_err("NONONONOO!!!!\n");

These ATM drivers don't appear to be netdev drivers. There does not
appear to be a struct device available, so you have to fall back to
pr_err(), pr_info() etc.

Andrew

2021-07-08 21:34:32

by Rolf Eike Beer

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

Am Donnerstag, 8. Juli 2021, 15:10:01 CEST schrieb Carlos Bilbao:
> 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.

> diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c
> index bc8e8d9f176b..65bb700cd5af 100644
> --- a/drivers/atm/iphase.c
> +++ b/drivers/atm/iphase.c
> @@ -1246,7 +1246,7 @@ static void rx_intr(struct atm_dev *dev)
> ((iadev->rx_pkt_cnt - iadev->rx_tmp_cnt) == 0)) {
> for (i = 1; i <= iadev->num_rx_desc; i++)
> free_desc(dev, i);
> -printk("Test logic RUN!!!!\n");
> + printk("Test logic RUN!!!!\n");
> writew(
> ~(RX_FREEQ_EMPT|RX_EXCP_RCVD),iadev->reass_reg+REASS_MASK_REG);
> iadev->rxing = 1;
> }

This looks like leftover debug code and probably should just be deleted.

> diff --git a/drivers/atm/suni.c b/drivers/atm/suni.c
> index 21e5acc766b8..149605cdb859 100644
> --- a/drivers/atm/suni.c
> +++ b/drivers/atm/suni.c
> @@ -328,8 +328,8 @@ static int suni_start(struct atm_dev *dev)
> timer_setup(&poll_timer, suni_hz, 0);
> poll_timer.expires = jiffies+HZ;
> #if 0
> -printk(KERN_DEBUG "[u] p=0x%lx,n=0x%lx\n",(unsigned long)
> poll_timer.list.prev, - (unsigned long) poll_timer.list.next);
> + printk(KERN_DEBUG "[u] p=0x%lx,n=0x%lx\n",(unsigned long)
> poll_timer.list.prev, + (unsigned long) poll_timer.list.next);
> #endif
> add_timer(&poll_timer);
> }

This should be converted to pr_debug() and the #if 0 can be removed. Or the
whole thing should likely just be removed, this looks like dead debug code.

> diff --git a/drivers/atm/zatm.c b/drivers/atm/zatm.c
> index cf5fffcf98a1..4fb89ed47311 100644
> --- a/drivers/atm/zatm.c
> +++ b/drivers/atm/zatm.c
> @@ -380,7 +380,7 @@ static void poll_rx(struct atm_dev *dev,int mbx)
> pos = zatm_dev->mbx_start[mbx];
> cells = here[0] & uPD98401_AAL5_SIZE;
> #if 0
> -printk("RX IND: 0x%x, 0x%x, 0x%x, 0x%x\n",here[0],here[1],here[2],here[3]);
> + printk("RX IND: 0x%x, 0x%x, 0x%x,
> 0x%x\n",here[0],here[1],here[2],here[3]);
> {
> unsigned long *x;
> printk("POOL: 0x%08x, 0x%08x\n",zpeekl(zatm_dev,

This is lacking a loglevel, as well as the following prints. It should be
converted to pr_debug(). The indentation of the following lines should be
fixed, too.

> @@ -403,14 +403,14 @@ EVENT("error code 0x%x/0x%x\n",(here[3] &
> uPD98401_AAL5_ES) >> skb = ((struct rx_buffer_head *)
> bus_to_virt(here[2]))->skb;
> __net_timestamp(skb);
> #if 0
> -printk("[-3..0] 0x%08lx 0x%08lx 0x%08lx 0x%08lx\n",((unsigned *)
> skb->data)[-3],
> + printk("[-3..0] 0x%08lx 0x%08lx 0x%08lx
> 0x%08lx\n",((unsigned *) skb->data)[-3],
> ((unsigned *) skb->data)[-2],((unsigned *) skb->data)[-1],
> ((unsigned *) skb->data)[0]);
> #endif

These as well. But this doesn't make sense, the format string says %lx, but
the casts are to unsigned, so I suspect this would spit warnings if enabled.

> EVENT("skb 0x%lx, here 0x%lx\n",(unsigned long) skb,
> (unsigned long) here);
> #if 0
> -printk("dummy: 0x%08lx, 0x%08lx\n",dummy[0],dummy[1]);
> + printk("dummy: 0x%08lx, 0x%08lx\n",dummy[0],dummy[1]);
> #endif
> size = error ? 0 : ntohs(((__be16 *) skb->data)[cells*
> ATM_CELL_PAYLOAD/sizeof(u16)-3]);

Same here.

> @@ -664,7 +664,7 @@ static int do_tx(struct sk_buff *skb)
> EVENT("dsc (0x%lx)\n",(unsigned long) dsc,0);
> }
> else {
> -printk("NONONONOO!!!!\n");
> + printk("NONONONOO!!!!\n");
> dsc = NULL;
> #if 0
> u32 *put;

And this should give something more useful, at least showing the driver name
or something like that.

> 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;
> }

That should be netdev_something, like netdev_dbg() or netdev_warn().

> 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

Here as well.

> --- a/drivers/parisc/iosapic.c
> +++ b/drivers/parisc/iosapic.c
> @@ -633,7 +633,7 @@ static void iosapic_unmask_irq(struct irq_data *d)
> printk("\n");
> }
>
> -printk("iosapic_enable_irq(): sel ");
> + printk("iosapic_enable_irq(): sel ");
> {
> struct iosapic_info *isp = vi->iosapic;
>
> @@ -642,7 +642,7 @@ printk("iosapic_enable_irq(): sel ");
> printk(" %x", d1);
> }
> }
> -printk("\n");
> + printk("\n");
> #endif
>
> /*

This is also debug code. It is basically unchanged since it has been imported
into git. So it may be time to remove the whole block. Helge?

> diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
> index dce4cdf786cd..c3381facdfc5 100644
> --- a/drivers/parisc/sba_iommu.c
> +++ b/drivers/parisc/sba_iommu.c
> @@ -1550,7 +1550,7 @@ static void sba_hw_init(struct sba_device *sba_dev)
>
>
> #if 0
> -printk("sba_hw_init(): mem_boot 0x%x 0x%x 0x%x 0x%x\n",
> PAGE0->mem_boot.hpa,
> + printk("sba_hw_init(): mem_boot 0x%x 0x%x 0x%x
> 0x%x\n", PAGE0->mem_boot.hpa, PAGE0->mem_boot.spa, PAGE0->mem_boot.pad,
> PAGE0->mem_boot.cl_class);
>
> /*

This is equally old. It should be either also removed, also this seems at
least worth as documentation. Maybe just switch it to pr_debug() or
dev_debug() while fixing the indentation.

Eike


Attachments:
signature.asc (201.00 B)
This is a digitally signed message part.

2021-07-09 00:28:23

by Stephen Hemminger

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

On Thu, 08 Jul 2021 23:25:37 +0200
Rolf Eike Beer <[email protected]> wrote:

> Am Donnerstag, 8. Juli 2021, 15:10:01 CEST schrieb Carlos Bilbao:
> > 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.
>
> > diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c

Does anyone still have Linux ATM devices?


Attachments:
(No filename) (849.00 B)
OpenPGP digital signature

2021-07-09 06:46:33

by Helge Deller

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

On 7/8/21 11:25 PM, Rolf Eike Beer wrote:
> Am Donnerstag, 8. Juli 2021, 15:10:01 CEST schrieb Carlos Bilbao:
>> 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.
> [...]
>> --- a/drivers/parisc/iosapic.c
>> +++ b/drivers/parisc/iosapic.c
>> @@ -633,7 +633,7 @@ static void iosapic_unmask_irq(struct irq_data *d)
>> printk("\n");
>> }
>>
>> -printk("iosapic_enable_irq(): sel ");
>> + printk("iosapic_enable_irq(): sel ");
>> {
>> struct iosapic_info *isp = vi->iosapic;
>>
>> @@ -642,7 +642,7 @@ printk("iosapic_enable_irq(): sel ");
>> printk(" %x", d1);
>> }
>> }
>> -printk("\n");
>> + printk("\n");
>> #endif
>>
>> /*
>
> This is also debug code. It is basically unchanged since it has been imported
> into git. So it may be time to remove the whole block. Helge?

I'd prefer to clean it proper up and keep it.


>> diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
>> index dce4cdf786cd..c3381facdfc5 100644
>> --- a/drivers/parisc/sba_iommu.c
>> +++ b/drivers/parisc/sba_iommu.c
>> @@ -1550,7 +1550,7 @@ static void sba_hw_init(struct sba_device *sba_dev)
>>
>>
>> #if 0
>> -printk("sba_hw_init(): mem_boot 0x%x 0x%x 0x%x 0x%x\n",
>> PAGE0->mem_boot.hpa,
>> + printk("sba_hw_init(): mem_boot 0x%x 0x%x 0x%x
>> 0x%x\n", PAGE0->mem_boot.hpa, PAGE0->mem_boot.spa, PAGE0->mem_boot.pad,
>> PAGE0->mem_boot.cl_class);
>>
>> /*
>
> This is equally old. It should be either also removed, also this seems at
> least worth as documentation. Maybe just switch it to pr_debug() or
> dev_debug() while fixing the indentation.

Yes, I'll clean it up too.

@Carlos:
Instead of just removing or fixing the indentation, I'll fix it for both parisc
drivers. Unless you want to try...

Helge

2021-07-09 10:32:05

by Carlos Bilbao

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

Helge,

I would like to finish what I started and take care of this, yes.

thanks,
Carlos.

> On Jul 9, 2021, at 2:43 AM, Helge Deller <[email protected]> wrote:
>
> On 7/8/21 11:25 PM, Rolf Eike Beer wrote:
>> Am Donnerstag, 8. Juli 2021, 15:10:01 CEST schrieb Carlos Bilbao:
>>> 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.
>> [...]
>>> --- a/drivers/parisc/iosapic.c
>>> +++ b/drivers/parisc/iosapic.c
>>> @@ -633,7 +633,7 @@ static void iosapic_unmask_irq(struct irq_data *d)
>>> printk("\n");
>>> }
>>> -printk("iosapic_enable_irq(): sel ");
>>> + printk("iosapic_enable_irq(): sel ");
>>> {
>>> struct iosapic_info *isp = vi->iosapic;
>>> @@ -642,7 +642,7 @@ printk("iosapic_enable_irq(): sel ");
>>> printk(" %x", d1);
>>> }
>>> }
>>> -printk("\n");
>>> + printk("\n");
>>> #endif
>>> /*
>> This is also debug code. It is basically unchanged since it has been imported
>> into git. So it may be time to remove the whole block. Helge?
>
> I'd prefer to clean it proper up and keep it.
>
>
>>> diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
>>> index dce4cdf786cd..c3381facdfc5 100644
>>> --- a/drivers/parisc/sba_iommu.c
>>> +++ b/drivers/parisc/sba_iommu.c
>>> @@ -1550,7 +1550,7 @@ static void sba_hw_init(struct sba_device *sba_dev)
>>> #if 0
>>> -printk("sba_hw_init(): mem_boot 0x%x 0x%x 0x%x 0x%x\n",
>>> PAGE0->mem_boot.hpa,
>>> + printk("sba_hw_init(): mem_boot 0x%x 0x%x 0x%x
>>> 0x%x\n", PAGE0->mem_boot.hpa, PAGE0->mem_boot.spa, PAGE0->mem_boot.pad,
>>> PAGE0->mem_boot.cl_class);
>>> /*
>> This is equally old. It should be either also removed, also this seems at
>> least worth as documentation. Maybe just switch it to pr_debug() or
>> dev_debug() while fixing the indentation.
>
> Yes, I'll clean it up too.
>
> @Carlos:
> Instead of just removing or fixing the indentation, I'll fix it for both parisc
> drivers. Unless you want to try...
>
> Helge