Re: [PATCH v2] isdn: hfc_{pci,sx}: Avoid empty body if statements

From: Nathan Chancellor <natechancellor@gmail.com>
To: David Miller <davem@davemloft.net>
Cc: isdn@linux-pingi.de, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, yamada.masahiro@socionext.com
Subject: Re: [PATCH v2] isdn: hfc_{pci,sx}: Avoid empty body if statements
Date: Thu, 18 Oct 2018 17:42:51 -0700	[thread overview]
Message-ID: <20181019004251.GA28878@flashbox> (raw)
In-Reply-To: <20181018.172310.35380794084221855.davem@davemloft.net>

On Thu, Oct 18, 2018 at 05:23:10PM -0700, David Miller wrote:
> From: Nathan Chancellor <natechancellor@gmail.com>
> Date: Thu, 18 Oct 2018 17:21:17 -0700
> 
> > Thanks for the review, I went ahead and compiled with the following diff
> > on top of v2 and got no warnings from Clang, GCC, or sparse, does this
> > seem satisfactory for v3?
> 
> Well, one thing I notice.
> 

> > @@ -86,7 +86,7 @@ release_io_hfcpci(struct IsdnCardState *cs)
> >         pci_free_consistent(cs->hw.hfcpci.dev, 0x8000,
> >                             cs->hw.hfcpci.fifos, cs->hw.hfcpci.dma);
> >         cs->hw.hfcpci.fifos = NULL;
> > -       iounmap((void *)cs->hw.hfcpci.pci_io);
> > +       iounmap(cs->hw.hfcpci.pci_io);
> >  }
> 
> Driver uses iounmap().
> 
> > @@ -1692,7 +1692,7 @@ setup_hfcpci(struct IsdnCard *card)
> >                 printk(KERN_WARNING "HFC-PCI: No IRQ for PCI card found\n");
> >                 return (0);
> >         }
> > -       cs->hw.hfcpci.pci_io = (char *)(unsigned long)dev_hfcpci->resource[1].start;
> > +       cs->hw.hfcpci.pci_io = (void __iomem *)(unsigned long)dev_hfcpci->resource[1].start;
> >         printk(KERN_INFO "HiSax: HFC-PCI card manufacturer: %s card name: %s\n", id_list[i].vendor_name, id_list[i].card_name);
> 
> But does not use iomap().  You won't need any cast here if it did use
> iomap() properly.
> 
> Thanks.

So this?

diff --git a/drivers/isdn/hisax/hfc_pci.c b/drivers/isdn/hisax/hfc_pci.c
index 3dbaee8c604f..ea0e4c6de3fb 100644
--- a/drivers/isdn/hisax/hfc_pci.c
+++ b/drivers/isdn/hisax/hfc_pci.c
@@ -1692,7 +1692,7 @@ setup_hfcpci(struct IsdnCard *card)
                printk(KERN_WARNING "HFC-PCI: No IRQ for PCI card found\n");
                return (0);
        }
-       cs->hw.hfcpci.pci_io = (void __iomem *)(unsigned long)dev_hfcpci->resource[1].start;
+       cs->hw.hfcpci.pci_io = ioremap(dev_hfcpci->resource[1].start, 256);
        printk(KERN_INFO "HiSax: HFC-PCI card manufacturer: %s card name: %s\n", id_list[i].vendor_name, id_list[i].card_name);
 
        if (!cs->hw.hfcpci.pci_io) {
@@ -1716,7 +1716,6 @@ setup_hfcpci(struct IsdnCard *card)
                return 0;
        }
        pci_write_config_dword(cs->hw.hfcpci.dev, 0x80, (u32)cs->hw.hfcpci.dma);
-       cs->hw.hfcpci.pci_io = ioremap((ulong) cs->hw.hfcpci.pci_io, 256);
        printk(KERN_INFO
               "HFC-PCI: defined at mem %p fifo %p(%lx) IRQ %d HZ %d\n",
               cs->hw.hfcpci.pci_io,



Full context before the above diff should it be needed:


        cs->hw.hfcpci.pci_io = (void *)(unsigned long)dev_hfcpci->resource[1].start;
        printk(KERN_INFO "HiSax: HFC-PCI card manufacturer: %s card name: %s\n", id_list[i].vendor_name, id_list[i].card_name);

        if (!cs->hw.hfcpci.pci_io) {
                printk(KERN_WARNING "HFC-PCI: No IO-Mem for PCI card found\n");
                return (0);
        }

        /* Allocate memory for FIFOS */
        cs->hw.hfcpci.fifos = pci_alloc_consistent(cs->hw.hfcpci.dev,
                                                   0x8000, &cs->hw.hfcpci.dma);
        if (!cs->hw.hfcpci.fifos) {
                printk(KERN_WARNING "HFC-PCI: Error allocating FIFO memory!\n");
                return 0;
        }
        if (cs->hw.hfcpci.dma & 0x7fff) {
                printk(KERN_WARNING
                       "HFC-PCI: Error DMA memory not on 32K boundary (%lx)\n",
                       (u_long)cs->hw.hfcpci.dma);
                pci_free_consistent(cs->hw.hfcpci.dev, 0x8000,
                                    cs->hw.hfcpci.fifos, cs->hw.hfcpci.dma);
                return 0;
        }
        pci_write_config_dword(cs->hw.hfcpci.dev, 0x80, (u32)cs->hw.hfcpci.dma);
        cs->hw.hfcpci.pci_io = ioremap((ulong) cs->hw.hfcpci.pci_io, 256);


Thanks,
Nathan
