Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758845Ab1ENUa0 (ORCPT ); Sat, 14 May 2011 16:30:26 -0400 Received: from palinux.external.hp.com ([192.25.206.14]:45131 "EHLO mail.parisc-linux.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754152Ab1ENUaY (ORCPT ); Sat, 14 May 2011 16:30:24 -0400 Date: Sat, 14 May 2011 14:30:23 -0600 From: Grant Grundler To: Joe Perches Cc: Tobias Ringstrom , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH net-next 2/4] tulip: Convert printks to netdev_ Message-ID: <20110514203023.GA30466@parisc-linux.org> References: <84a86a5b89eac3aec88427c8afd09591bd5b0cfc.1304733889.git.joe@perches.com> <20110512040906.GB8885@parisc-linux.org> <1305176371.6124.29.camel@Joe-Laptop> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1305176371.6124.29.camel@Joe-Laptop> X-Home-Page: http://www.parisc-linux.org/ User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 17318 Lines: 441 On Wed, May 11, 2011 at 09:59:31PM -0700, Joe Perches wrote: > On Wed, 2011-05-11 at 22:09 -0600, Grant Grundler wrote: > > Some additional clean ups to consider in a future patch: > > o replace "if (skb == NULL)" with "if (!skb)" > > Hey Grant. > > I generally don't change those. > While I prefer (!ptr), others prefer explicit comparisons. > I do have a script that does the conversion though. > (de4x5.c is a mess and I didn't change it) > > Here's the output with hoisted assigns from if too. > (and a spelling fix I noticed) LGTM. :) Please repost with Signed-off-by, my "Acked-By: Grant Grundler " and davem can apply. I can't test these at the moment because I haven't figured out how to update my parisc machine (parisc was dropped from debian testing). I'm trying to build a gentoo chroot today though. We'll see. thanks, grant > > drivers/net/tulip/de2104x.c | 3 ++- > drivers/net/tulip/dmfe.c | 11 ++++++----- > drivers/net/tulip/eeprom.c | 6 +++--- > drivers/net/tulip/interrupt.c | 18 +++++++++--------- > drivers/net/tulip/timer.c | 2 +- > drivers/net/tulip/tulip_core.c | 15 +++++++++------ > drivers/net/tulip/uli526x.c | 15 +++++++-------- > drivers/net/tulip/winbond-840.c | 14 ++++++++------ > drivers/net/tulip/xircom_cb.c | 12 ++++++------ > 9 files changed, 51 insertions(+), 45 deletions(-) > > diff --git a/drivers/net/tulip/de2104x.c b/drivers/net/tulip/de2104x.c > index e2f6923..45a4843 100644 > --- a/drivers/net/tulip/de2104x.c > +++ b/drivers/net/tulip/de2104x.c > @@ -2170,7 +2170,8 @@ static int de_resume (struct pci_dev *pdev) > goto out; > if (!netif_running(dev)) > goto out_attach; > - if ((retval = pci_enable_device(pdev))) { > + retval = pci_enable_device(pdev); > + if (retval) { > netdev_err(dev, "pci_enable_device failed in resume\n"); > goto out; > } > diff --git a/drivers/net/tulip/dmfe.c b/drivers/net/tulip/dmfe.c > index 4685127..588d6b4 100644 > --- a/drivers/net/tulip/dmfe.c > +++ b/drivers/net/tulip/dmfe.c > @@ -400,7 +400,7 @@ static int __devinit dmfe_init_one (struct pci_dev *pdev, > > /* Init network device */ > dev = alloc_etherdev(sizeof(*db)); > - if (dev == NULL) > + if (!dev) > return -ENOMEM; > SET_NETDEV_DEV(dev, &pdev->dev); > > @@ -1009,10 +1009,10 @@ static void dmfe_rx_packet(struct DEVICE *dev, struct dmfe_board_info * db) > db->dm910x_chk_mode = 3; > } else { > /* Good packet, send to upper layer */ > - /* Shorst packet used new SKB */ > + /* Short packet used new SKB */ > if ((rxlen < RX_COPY_SIZE) && > - ((newskb = dev_alloc_skb(rxlen + 2)) > - != NULL)) { > + (newskb = > + dev_alloc_skb(rxlen + 2))) { > > skb = newskb; > /* size less than COPY_SIZE, allocate a rxlen SKB */ > @@ -1561,7 +1561,8 @@ static void allocate_rx_buffer(struct dmfe_board_info *db) > rxptr = db->rx_insert_ptr; > > while(db->rx_avail_cnt < RX_DESC_CNT) { > - if ( ( skb = dev_alloc_skb(RX_ALLOC_SIZE) ) == NULL ) > + skb = dev_alloc_skb(RX_ALLOC_SIZE); > + if (!skb) > break; > rxptr->rx_skb_ptr = skb; /* FIXME (?) */ > rxptr->rdes2 = cpu_to_le32( pci_map_single(db->pdev, skb->data, > diff --git a/drivers/net/tulip/eeprom.c b/drivers/net/tulip/eeprom.c > index fa5eee9..a11eb73 100644 > --- a/drivers/net/tulip/eeprom.c > +++ b/drivers/net/tulip/eeprom.c > @@ -123,7 +123,7 @@ static void __devinit tulip_build_fake_mediatable(struct tulip_private *tp) > tp->mtable = kmalloc(sizeof(struct mediatable) + > sizeof(struct medialeaf), GFP_KERNEL); > > - if (tp->mtable == NULL) > + if (!tp->mtable) > return; /* Horrible, impossible failure. */ > > tp->mtable->defaultmedia = 0x800; > @@ -192,7 +192,7 @@ void __devinit tulip_parse_eeprom(struct net_device *dev) > break; > } > } > - if (eeprom_fixups[i].name == NULL) { /* No fixup found. */ > + if (!eeprom_fixups[i].name) { /* No fixup found. */ > pr_info("%s: Old style EEPROM with no media selection information\n", > dev->name); > return; > @@ -230,7 +230,7 @@ subsequent_board: > mtable = kmalloc(sizeof(struct mediatable) + > count * sizeof(struct medialeaf), > GFP_KERNEL); > - if (mtable == NULL) > + if (!mtable) > return; /* Horrible, impossible failure. */ > last_mediatable = tp->mtable = mtable; > mtable->defaultmedia = media; > diff --git a/drivers/net/tulip/interrupt.c b/drivers/net/tulip/interrupt.c > index 5350d75..1a1bff5 100644 > --- a/drivers/net/tulip/interrupt.c > +++ b/drivers/net/tulip/interrupt.c > @@ -68,12 +68,12 @@ int tulip_refill_rx(struct net_device *dev) > /* Refill the Rx ring buffers. */ > for (; tp->cur_rx - tp->dirty_rx > 0; tp->dirty_rx++) { > entry = tp->dirty_rx % RX_RING_SIZE; > - if (tp->rx_buffers[entry].skb == NULL) { > + if (!tp->rx_buffers[entry].skb) { > struct sk_buff *skb; > dma_addr_t mapping; > > skb = tp->rx_buffers[entry].skb = dev_alloc_skb(PKT_BUF_SZ); > - if (skb == NULL) > + if (!skb) > break; > > mapping = pci_map_single(tp->pdev, skb->data, PKT_BUF_SZ, > @@ -205,7 +205,7 @@ int tulip_poll(struct napi_struct *napi, int budget) > /* Check if the packet is long enough to accept without copying > to a minimally-sized skbuff. */ > if (pkt_len < tulip_rx_copybreak && > - (skb = dev_alloc_skb(pkt_len + 2)) != NULL) { > + (skb = dev_alloc_skb(pkt_len + 2))) { > skb_reserve(skb, 2); /* 16 byte align the IP header */ > pci_dma_sync_single_for_cpu(tp->pdev, > tp->rx_buffers[entry].mapping, > @@ -311,7 +311,7 @@ int tulip_poll(struct napi_struct *napi, int budget) > tulip_refill_rx(dev); > > /* If RX ring is not full we are out of memory. */ > - if (tp->rx_buffers[tp->dirty_rx % RX_RING_SIZE].skb == NULL) > + if (!tp->rx_buffers[tp->dirty_rx % RX_RING_SIZE].skb) > goto oom; > > /* Remove us from polling list and enable RX intr. */ > @@ -334,10 +334,10 @@ int tulip_poll(struct napi_struct *napi, int budget) > > not_done: > if (tp->cur_rx - tp->dirty_rx > RX_RING_SIZE/2 || > - tp->rx_buffers[tp->dirty_rx % RX_RING_SIZE].skb == NULL) > + !tp->rx_buffers[tp->dirty_rx % RX_RING_SIZE].skb) > tulip_refill_rx(dev); > > - if (tp->rx_buffers[tp->dirty_rx % RX_RING_SIZE].skb == NULL) > + if (!tp->rx_buffers[tp->dirty_rx % RX_RING_SIZE].skb) > goto oom; > > return work_done; > @@ -431,7 +431,7 @@ static int tulip_rx(struct net_device *dev) > /* Check if the packet is long enough to accept without copying > to a minimally-sized skbuff. */ > if (pkt_len < tulip_rx_copybreak && > - (skb = dev_alloc_skb(pkt_len + 2)) != NULL) { > + (skb = dev_alloc_skb(pkt_len + 2))) { > skb_reserve(skb, 2); /* 16 byte align the IP header */ > pci_dma_sync_single_for_cpu(tp->pdev, > tp->rx_buffers[entry].mapping, > @@ -591,7 +591,7 @@ irqreturn_t tulip_interrupt(int irq, void *dev_instance) > break; /* It still has not been Txed */ > > /* Check for Rx filter setup frames. */ > - if (tp->tx_buffers[entry].skb == NULL) { > + if (!tp->tx_buffers[entry].skb) { > /* test because dummy frames not mapped */ > if (tp->tx_buffers[entry].mapping) > pci_unmap_single(tp->pdev, > @@ -775,7 +775,7 @@ irqreturn_t tulip_interrupt(int irq, void *dev_instance) > > /* check if the card is in suspend mode */ > entry = tp->dirty_rx % RX_RING_SIZE; > - if (tp->rx_buffers[entry].skb == NULL) { > + if (!tp->rx_buffers[entry].skb) { > if (tulip_debug > 1) > dev_warn(&dev->dev, > "in rx suspend mode: (%lu) (tp->cur_rx = %u, ttimer = %d, rx = %d) go/stay in suspend mode\n", > diff --git a/drivers/net/tulip/timer.c b/drivers/net/tulip/timer.c > index 2017faf..afc5445 100644 > --- a/drivers/net/tulip/timer.c > +++ b/drivers/net/tulip/timer.c > @@ -43,7 +43,7 @@ void tulip_media_task(struct work_struct *work) > default: { > struct medialeaf *mleaf; > unsigned char *p; > - if (tp->mtable == NULL) { /* No EEPROM info, use generic code. */ > + if (!tp->mtable) { /* No EEPROM info, use generic code. */ > /* Not much that can be done. > Assume this a generic MII or SYM transceiver. */ > next_tick = 60*HZ; > diff --git a/drivers/net/tulip/tulip_core.c b/drivers/net/tulip/tulip_core.c > index 82f8764..0ab2465 100644 > --- a/drivers/net/tulip/tulip_core.c > +++ b/drivers/net/tulip/tulip_core.c > @@ -384,7 +384,7 @@ static void tulip_up(struct net_device *dev) > > /* Allow selecting a default media. */ > i = 0; > - if (tp->mtable == NULL) > + if (!tp->mtable) > goto media_picked; > if (dev->if_port) { > int looking_for = tulip_media_cap[dev->if_port] & MediaIsMII ? 11 : > @@ -642,7 +642,7 @@ static void tulip_init_ring(struct net_device *dev) > use skb_reserve() to align the IP header! */ > struct sk_buff *skb = dev_alloc_skb(PKT_BUF_SZ); > tp->rx_buffers[i].skb = skb; > - if (skb == NULL) > + if (!skb) > break; > mapping = pci_map_single(tp->pdev, skb->data, > PKT_BUF_SZ, PCI_DMA_FROMDEVICE); > @@ -728,7 +728,7 @@ static void tulip_clean_tx_ring(struct tulip_private *tp) > } > > /* Check for Tx filter setup frames. */ > - if (tp->tx_buffers[entry].skb == NULL) { > + if (!tp->tx_buffers[entry].skb) { > /* test because dummy frames not mapped */ > if (tp->tx_buffers[entry].mapping) > pci_unmap_single(tp->pdev, > @@ -821,7 +821,7 @@ static void tulip_free_ring (struct net_device *dev) > for (i = 0; i < TX_RING_SIZE; i++) { > struct sk_buff *skb = tp->tx_buffers[i].skb; > > - if (skb != NULL) { > + if (skb) { > pci_unmap_single(tp->pdev, tp->tx_buffers[i].mapping, > skb->len, PCI_DMA_TODEVICE); > dev_kfree_skb (skb); > @@ -1900,12 +1900,15 @@ static int tulip_resume(struct pci_dev *pdev) > if (!netif_running(dev)) > return 0; > > - if ((retval = pci_enable_device(pdev))) { > + retval = pci_enable_device(pdev); > + if (retval) { > pr_err("pci_enable_device failed in resume\n"); > return retval; > } > > - if ((retval = request_irq(dev->irq, tulip_interrupt, IRQF_SHARED, dev->name, dev))) { > + retval = request_irq(dev->irq, tulip_interrupt, IRQF_SHARED, > + dev->name, dev); > + if (retval) { > pr_err("request_irq failed in resume\n"); > return retval; > } > diff --git a/drivers/net/tulip/uli526x.c b/drivers/net/tulip/uli526x.c > index 9e63f40..a6f6a9e 100644 > --- a/drivers/net/tulip/uli526x.c > +++ b/drivers/net/tulip/uli526x.c > @@ -286,7 +286,7 @@ static int __devinit uli526x_init_one (struct pci_dev *pdev, > > /* Init network device */ > dev = alloc_etherdev(sizeof(*db)); > - if (dev == NULL) > + if (!dev) > return -ENOMEM; > SET_NETDEV_DEV(dev, &pdev->dev); > > @@ -324,14 +324,12 @@ static int __devinit uli526x_init_one (struct pci_dev *pdev, > > /* Allocate Tx/Rx descriptor memory */ > db->desc_pool_ptr = pci_alloc_consistent(pdev, sizeof(struct tx_desc) * DESC_ALL_CNT + 0x20, &db->desc_pool_dma_ptr); > - if(db->desc_pool_ptr == NULL) > - { > + if (!db->desc_pool_ptr) { > err = -ENOMEM; > goto err_out_nomem; > } > db->buf_pool_ptr = pci_alloc_consistent(pdev, TX_BUF_ALLOC * TX_DESC_CNT + 4, &db->buf_pool_dma_ptr); > - if(db->buf_pool_ptr == NULL) > - { > + if (!db->buf_pool_ptr) { > err = -ENOMEM; > goto err_out_nomem; > } > @@ -404,7 +402,7 @@ err_out_nomem: > pci_free_consistent(pdev, sizeof(struct tx_desc) * DESC_ALL_CNT + 0x20, > db->desc_pool_ptr, db->desc_pool_dma_ptr); > > - if(db->buf_pool_ptr != NULL) > + if (db->buf_pool_ptr) > pci_free_consistent(pdev, TX_BUF_ALLOC * TX_DESC_CNT + 4, > db->buf_pool_ptr, db->buf_pool_dma_ptr); > err_out_disable: > @@ -844,7 +842,7 @@ static void uli526x_rx_packet(struct net_device *dev, struct uli526x_board_info > /* Good packet, send to upper layer */ > /* Shorst packet used new SKB */ > if ((rxlen < RX_COPY_SIZE) && > - (((new_skb = dev_alloc_skb(rxlen + 2)) != NULL))) { > + (new_skb = dev_alloc_skb(rxlen + 2))) { > skb = new_skb; > /* size less than COPY_SIZE, allocate a rxlen SKB */ > skb_reserve(skb, 2); /* 16byte align */ > @@ -1440,7 +1438,8 @@ static void allocate_rx_buffer(struct uli526x_board_info *db) > rxptr = db->rx_insert_ptr; > > while(db->rx_avail_cnt < RX_DESC_CNT) { > - if ( ( skb = dev_alloc_skb(RX_ALLOC_SIZE) ) == NULL ) > + skb = dev_alloc_skb(RX_ALLOC_SIZE); > + if (!skb) > break; > rxptr->rx_skb_ptr = skb; /* FIXME (?) */ > rxptr->rdes2 = cpu_to_le32(pci_map_single(db->pdev, > diff --git a/drivers/net/tulip/winbond-840.c b/drivers/net/tulip/winbond-840.c > index 862eadf..a957e72 100644 > --- a/drivers/net/tulip/winbond-840.c > +++ b/drivers/net/tulip/winbond-840.c > @@ -647,7 +647,8 @@ static int netdev_open(struct net_device *dev) > if (debug > 1) > netdev_dbg(dev, "w89c840_open() irq %d\n", dev->irq); > > - if((i=alloc_ringdesc(dev))) > + i = alloc_ringdesc(dev); > + if (i) > goto out_err; > > spin_lock_irq(&np->lock); > @@ -817,7 +818,7 @@ static void init_rxtx_rings(struct net_device *dev) > for (i = 0; i < RX_RING_SIZE; i++) { > struct sk_buff *skb = dev_alloc_skb(np->rx_buf_sz); > np->rx_skbuff[i] = skb; > - if (skb == NULL) > + if (!skb) > break; > np->rx_addr[i] = pci_map_single(np->pci_dev,skb->data, > np->rx_buf_sz,PCI_DMA_FROMDEVICE); > @@ -1231,7 +1232,7 @@ static int netdev_rx(struct net_device *dev) > /* Check if the packet is long enough to accept without copying > to a minimally-sized skbuff. */ > if (pkt_len < rx_copybreak && > - (skb = dev_alloc_skb(pkt_len + 2)) != NULL) { > + (skb = dev_alloc_skb(pkt_len + 2))) { > skb_reserve(skb, 2); /* 16 byte align the IP header */ > pci_dma_sync_single_for_cpu(np->pci_dev,np->rx_addr[entry], > np->rx_skbuff[entry]->len, > @@ -1269,10 +1270,10 @@ static int netdev_rx(struct net_device *dev) > for (; np->cur_rx - np->dirty_rx > 0; np->dirty_rx++) { > struct sk_buff *skb; > entry = np->dirty_rx % RX_RING_SIZE; > - if (np->rx_skbuff[entry] == NULL) { > + if (!np->rx_skbuff[entry]) { > skb = dev_alloc_skb(np->rx_buf_sz); > np->rx_skbuff[entry] = skb; > - if (skb == NULL) > + if (!skb) > break; /* Better luck next round. */ > np->rx_addr[entry] = pci_map_single(np->pci_dev, > skb->data, > @@ -1618,7 +1619,8 @@ static int w840_resume (struct pci_dev *pdev) > if (netif_device_present(dev)) > goto out; /* device not suspended */ > if (netif_running(dev)) { > - if ((retval = pci_enable_device(pdev))) { > + retval = pci_enable_device(pdev); > + if (retval) { > dev_err(&dev->dev, > "pci_enable_device failed in resume\n"); > goto out; > diff --git a/drivers/net/tulip/xircom_cb.c b/drivers/net/tulip/xircom_cb.c > index 988b8eb..1cb7208 100644 > --- a/drivers/net/tulip/xircom_cb.c > +++ b/drivers/net/tulip/xircom_cb.c > @@ -230,12 +230,12 @@ static int __devinit xircom_probe(struct pci_dev *pdev, const struct pci_device_ > > /* Allocate the send/receive buffers */ > private->rx_buffer = pci_alloc_consistent(pdev,8192,&private->rx_dma_handle); > - if (private->rx_buffer == NULL) { > + if (!private->rx_buffer) { > pr_err("%s: no memory for rx buffer\n", __func__); > goto rx_buf_fail; > } > private->tx_buffer = pci_alloc_consistent(pdev,8192,&private->tx_dma_handle); > - if (private->tx_buffer == NULL) { > + if (!private->tx_buffer) { > pr_err("%s: no memory for tx buffer\n", __func__); > goto tx_buf_fail; > } > @@ -546,8 +546,8 @@ static void setup_descriptors(struct xircom_private *card) > u32 address; > int i; > > - BUG_ON(card->rx_buffer == NULL); > - BUG_ON(card->tx_buffer == NULL); > + BUG_ON(!card->rx_buffer); > + BUG_ON(!card->tx_buffer); > > /* Receive descriptors */ > memset(card->rx_buffer, 0, 128); /* clear the descriptors */ > @@ -1086,7 +1086,7 @@ investigate_read_descriptor(struct net_device *dev, struct xircom_private *card, > } > > skb = dev_alloc_skb(pkt_len + 2); > - if (skb == NULL) { > + if (!skb) { > dev->stats.rx_dropped++; > goto out; > } > @@ -1125,7 +1125,7 @@ investigate_write_descriptor(struct net_device *dev, > } > #endif > if (status > 0) { /* bit 31 is 0 when done */ > - if (card->tx_skb[descnr]!=NULL) { > + if (card->tx_skb[descnr]) { > dev->stats.tx_bytes += card->tx_skb[descnr]->len; > dev_kfree_skb_irq(card->tx_skb[descnr]); > } > > > o in general, HW doesn't return signed integer values. Where possible, > > I prefer to see "unsigned int status;" and "if (status)". > > That one is yours to change if you want. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/