| Message ID | 2784471.e9J7NaK4W3@iron-maiden (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drivers: Follow the indentation coding standard on printks | expand |
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 <bilbao@vt.edu> > --- > 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
> --- 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
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.
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
On Thu, 08 Jul 2021 23:25:37 +0200 Rolf Eike Beer <eike-kernel@sf-tec.de> 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?
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
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 <deller@gmx.de> 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
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); /*
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 <bilbao@vt.edu> --- 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(-)