[PATCH 1/1] tty-hvsi_lib: Deletion of an unnecessary check before the function call "tty_kref_put"

[PATCH 1/1] tty-hvsi_lib: Deletion of an unnecessary check before the function call "tty_kref_put"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 21 Nov 2014 12:40:32 +0100

The tty_kref_put() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/tty/hvc/hvsi_lib.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/hvc/hvsi_lib.c b/drivers/tty/hvc/hvsi_lib.c
index 7ae6c29..a270f04 100644
--- a/drivers/tty/hvc/hvsi_lib.c
+++ b/drivers/tty/hvc/hvsi_lib.c
@@ -405,8 +405,7 @@ void hvsilib_close(struct hvsi_priv *pv, struct hvc_struct *hp)
 		hvsi_send_close(pv);
 	}
 
-	if (pv->tty)
-		tty_kref_put(pv->tty);
+	tty_kref_put(pv->tty);
 	pv->tty = NULL;
 }
 
-- 
2.1.3

[PATCH 1/1] PowerPC-83xx: Deletion of an unnecessary check before the function call "of_node_put"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 22 Nov 2014 16:18:20 +0100

The of_node_put() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/powerpc/platforms/83xx/usb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/usb.c b/arch/powerpc/platforms/83xx/usb.c
index 1ad748b..5c31d82 100644
--- a/arch/powerpc/platforms/83xx/usb.c
+++ b/arch/powerpc/platforms/83xx/usb.c
@@ -162,8 +162,7 @@ int mpc831x_usb_cfg(void)
 
 	iounmap(immap);
 
-	if (immr_node)
-		of_node_put(immr_node);
+	of_node_put(immr_node);
 
 	/* Map USB SOC space */
 	ret = of_address_to_resource(np, 0, &res);
-- 
2.1.3

[PATCH] ALSA: i2sbus: Deletion of unnecessary checks before the function call "release_and_free_resource"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 2 Dec 2014 22:50:24 +0100

The release_and_free_resource() function tests whether its argument is NULL
and then returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 sound/aoa/soundbus/i2sbus/core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
index a80d5ea..4e2b4fb 100644
--- a/sound/aoa/soundbus/i2sbus/core.c
+++ b/sound/aoa/soundbus/i2sbus/core.c
@@ -79,8 +79,7 @@ static void i2sbus_release_dev(struct device *dev)
  	if (i2sdev->out.dbdma) iounmap(i2sdev->out.dbdma);
  	if (i2sdev->in.dbdma) iounmap(i2sdev->in.dbdma);
 	for (i = aoa_resource_i2smmio; i <= aoa_resource_rxdbdma; i++)
-		if (i2sdev->allocated_resource[i])
-			release_and_free_resource(i2sdev->allocated_resource[i]);
+		release_and_free_resource(i2sdev->allocated_resource[i]);
 	free_dbdma_descriptor_ring(i2sdev, &i2sdev->out.dbdma_ring);
 	free_dbdma_descriptor_ring(i2sdev, &i2sdev->in.dbdma_ring);
 	for (i = aoa_resource_i2smmio; i <= aoa_resource_rxdbdma; i++)
@@ -323,8 +322,7 @@ static int i2sbus_add_dev(struct macio_dev *macio,
 	if (dev->out.dbdma) iounmap(dev->out.dbdma);
 	if (dev->in.dbdma) iounmap(dev->in.dbdma);
 	for (i=0;i<3;i++)
-		if (dev->allocated_resource[i])
-			release_and_free_resource(dev->allocated_resource[i]);
+		release_and_free_resource(dev->allocated_resource[i]);
 	mutex_destroy(&dev->lock);
 	kfree(dev);
 	return 0;
-- 
2.1.3

Re: [PATCH] ALSA: i2sbus: Deletion of unnecessary checks before the function call "release_and_free_resource"

[Takashi Iwai]

At Tue, 02 Dec 2014 22:55:57 +0100,
SF Markus Elfring wrote:
> 
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 2 Dec 2014 22:50:24 +0100
> 
> The release_and_free_resource() function tests whether its argument is NULL
> and then returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Thanks, applied.


Takashi

> ---
>  sound/aoa/soundbus/i2sbus/core.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
> index a80d5ea..4e2b4fb 100644
> --- a/sound/aoa/soundbus/i2sbus/core.c
> +++ b/sound/aoa/soundbus/i2sbus/core.c
> @@ -79,8 +79,7 @@ static void i2sbus_release_dev(struct device *dev)
>   	if (i2sdev->out.dbdma) iounmap(i2sdev->out.dbdma);
>   	if (i2sdev->in.dbdma) iounmap(i2sdev->in.dbdma);
>  	for (i = aoa_resource_i2smmio; i <= aoa_resource_rxdbdma; i++)
> -		if (i2sdev->allocated_resource[i])
> -			release_and_free_resource(i2sdev->allocated_resource[i]);
> +		release_and_free_resource(i2sdev->allocated_resource[i]);
>  	free_dbdma_descriptor_ring(i2sdev, &i2sdev->out.dbdma_ring);
>  	free_dbdma_descriptor_ring(i2sdev, &i2sdev->in.dbdma_ring);
>  	for (i = aoa_resource_i2smmio; i <= aoa_resource_rxdbdma; i++)
> @@ -323,8 +322,7 @@ static int i2sbus_add_dev(struct macio_dev *macio,
>  	if (dev->out.dbdma) iounmap(dev->out.dbdma);
>  	if (dev->in.dbdma) iounmap(dev->in.dbdma);
>  	for (i=0;i<3;i++)
> -		if (dev->allocated_resource[i])
> -			release_and_free_resource(dev->allocated_resource[i]);
> +		release_and_free_resource(dev->allocated_resource[i]);
>  	mutex_destroy(&dev->lock);
>  	kfree(dev);
>  	return 0;
> -- 
> 2.1.3
> 

[PATCH 9/13] ALSA: i2sbus: Delete an unnecessary check before the function call "snd_pcm_suspend_all"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 3 Jan 2015 20:43:01 +0100

The snd_pcm_suspend_all() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 sound/aoa/soundbus/i2sbus/core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
index 4e2b4fb..837ba99 100644
--- a/sound/aoa/soundbus/i2sbus/core.c
+++ b/sound/aoa/soundbus/i2sbus/core.c
@@ -381,10 +381,8 @@ static int i2sbus_suspend(struct macio_dev* dev, pm_message_t state)
 
 	list_for_each_entry(i2sdev, &control->list, item) {
 		/* Notify Alsa */
-		if (i2sdev->sound.pcm) {
-			/* Suspend PCM streams */
-			snd_pcm_suspend_all(i2sdev->sound.pcm);
-		}
+		/* Suspend PCM streams */
+		snd_pcm_suspend_all(i2sdev->sound.pcm);
 
 		/* Notify codecs */
 		list_for_each_entry(cii, &i2sdev->sound.codec_list, list) {
-- 
2.2.1

[PATCH 11/13] ALSA: Deletion of checks before the function call "iounmap"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 3 Jan 2015 22:55:54 +0100

The iounmap() function performs also input parameter validation.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 sound/aoa/soundbus/i2sbus/core.c | 13 ++++++-------
 sound/arm/aaci.c                 |  4 ++--
 sound/drivers/ml403-ac97cr.c     |  3 +--
 sound/isa/msnd/msnd_pinnacle.c   |  3 +--
 sound/parisc/harmony.c           |  4 +---
 sound/pci/ad1889.c               |  5 +----
 sound/pci/asihpi/hpioctl.c       |  6 ++----
 sound/pci/atiixp.c               |  3 +--
 sound/pci/atiixp_modem.c         |  3 +--
 sound/pci/aw2/aw2-alsa.c         |  4 +---
 sound/pci/bt87x.c                |  3 +--
 sound/pci/cs4281.c               |  6 ++----
 sound/pci/cs46xx/cs46xx_lib.c    |  4 ++--
 sound/pci/ctxfi/cthw20k1.c       |  5 +----
 sound/pci/ctxfi/cthw20k2.c       |  5 +----
 sound/pci/echoaudio/echoaudio.c  |  6 +-----
 sound/pci/hda/hda_intel.c        |  3 +--
 sound/pci/lola/lola.c            |  6 ++----
 sound/pci/mixart/mixart.c        |  7 +++----
 sound/pci/nm256/nm256.c          |  6 ++----
 sound/pci/rme9652/hdsp.c         |  4 +---
 sound/pci/rme9652/hdspm.c        |  4 +---
 sound/pci/rme9652/rme9652.c      |  3 +--
 sound/pci/sis7019.c              |  5 +----
 sound/pci/ymfpci/ymfpci_main.c   |  3 +--
 sound/ppc/pmac.c                 | 15 +++++----------
 26 files changed, 43 insertions(+), 90 deletions(-)

diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
index 4e2b4fb..7835045 100644
--- a/sound/aoa/soundbus/i2sbus/core.c
+++ b/sound/aoa/soundbus/i2sbus/core.c
@@ -74,10 +74,9 @@ static void i2sbus_release_dev(struct device *dev)
 	int i;
 
 	i2sdev = container_of(dev, struct i2sbus_dev, sound.ofdev.dev);
-
- 	if (i2sdev->intfregs) iounmap(i2sdev->intfregs);
- 	if (i2sdev->out.dbdma) iounmap(i2sdev->out.dbdma);
- 	if (i2sdev->in.dbdma) iounmap(i2sdev->in.dbdma);
+	iounmap(i2sdev->intfregs);
+	iounmap(i2sdev->out.dbdma);
+	iounmap(i2sdev->in.dbdma);
 	for (i = aoa_resource_i2smmio; i <= aoa_resource_rxdbdma; i++)
 		release_and_free_resource(i2sdev->allocated_resource[i]);
 	free_dbdma_descriptor_ring(i2sdev, &i2sdev->out.dbdma_ring);
@@ -318,9 +317,9 @@ static int i2sbus_add_dev(struct macio_dev *macio,
 			free_irq(dev->interrupts[i], dev);
 	free_dbdma_descriptor_ring(dev, &dev->out.dbdma_ring);
 	free_dbdma_descriptor_ring(dev, &dev->in.dbdma_ring);
-	if (dev->intfregs) iounmap(dev->intfregs);
-	if (dev->out.dbdma) iounmap(dev->out.dbdma);
-	if (dev->in.dbdma) iounmap(dev->in.dbdma);
+	iounmap(dev->intfregs);
+	iounmap(dev->out.dbdma);
+	iounmap(dev->in.dbdma);
 	for (i=0;i<3;i++)
 		release_and_free_resource(dev->allocated_resource[i]);
 	mutex_destroy(&dev->lock);
diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c
index 0e83a73..4140b1b 100644
--- a/sound/arm/aaci.c
+++ b/sound/arm/aaci.c
@@ -889,8 +889,8 @@ static int aaci_probe_ac97(struct aaci *aaci)
 static void aaci_free_card(struct snd_card *card)
 {
 	struct aaci *aaci = card->private_data;
-	if (aaci->base)
-		iounmap(aaci->base);
+
+	iounmap(aaci->base);
 }
 
 static struct aaci *aaci_init_card(struct amba_device *dev)
diff --git a/sound/drivers/ml403-ac97cr.c b/sound/drivers/ml403-ac97cr.c
index ec01de1..bdcb572 100644
--- a/sound/drivers/ml403-ac97cr.c
+++ b/sound/drivers/ml403-ac97cr.c
@@ -1094,8 +1094,7 @@ static int snd_ml403_ac97cr_free(struct snd_ml403_ac97cr *ml403_ac97cr)
 	if (ml403_ac97cr->capture_irq >= 0)
 		free_irq(ml403_ac97cr->capture_irq, ml403_ac97cr);
 	/* give back "port" */
-	if (ml403_ac97cr->port != NULL)
-		iounmap(ml403_ac97cr->port);
+	iounmap(ml403_ac97cr->port);
 	kfree(ml403_ac97cr);
 	PDEBUG(INIT_INFO, "free(): (done)\n");
 	return 0;
diff --git a/sound/isa/msnd/msnd_pinnacle.c b/sound/isa/msnd/msnd_pinnacle.c
index 65b3682..4c07266 100644
--- a/sound/isa/msnd/msnd_pinnacle.c
+++ b/sound/isa/msnd/msnd_pinnacle.c
@@ -627,8 +627,7 @@ static int snd_msnd_attach(struct snd_card *card)
 	return 0;
 
 err_release_region:
-	if (chip->mappedbase)
-		iounmap(chip->mappedbase);
+	iounmap(chip->mappedbase);
 	release_mem_region(chip->base, BUFFSIZE);
 	release_region(chip->io, DSP_NUMIO);
 	free_irq(chip->irq, chip);
diff --git a/sound/parisc/harmony.c b/sound/parisc/harmony.c
index 29604a2..f2350c1 100644
--- a/sound/parisc/harmony.c
+++ b/sound/parisc/harmony.c
@@ -893,9 +893,7 @@ snd_harmony_free(struct snd_harmony *h)
 	if (h->irq >= 0)
 		free_irq(h->irq, h);
 
-	if (h->iobase)
-		iounmap(h->iobase);
-
+	iounmap(h->iobase);
 	kfree(h);
 	return 0;
 }
diff --git a/sound/pci/ad1889.c b/sound/pci/ad1889.c
index 547ee30..0de3129 100644
--- a/sound/pci/ad1889.c
+++ b/sound/pci/ad1889.c
@@ -853,12 +853,9 @@ snd_ad1889_free(struct snd_ad1889 *chip)
 		free_irq(chip->irq, chip);
 
 skip_hw:
-	if (chip->iobase)
-		iounmap(chip->iobase);
-
+	iounmap(chip->iobase);
 	pci_release_regions(chip->pci);
 	pci_disable_device(chip->pci);
-
 	kfree(chip);
 	return 0;
 }
diff --git a/sound/pci/asihpi/hpioctl.c b/sound/pci/asihpi/hpioctl.c
index 72af66b..67d1133 100644
--- a/sound/pci/asihpi/hpioctl.c
+++ b/sound/pci/asihpi/hpioctl.c
@@ -541,10 +541,8 @@ void asihpi_adapter_remove(struct pci_dev *pci_dev)
 	hpi_send_recv_ex(&hm, &hr, HOWNER_KERNEL);
 
 	/* unmap PCI memory space, mapped during device init. */
-	for (idx = 0; idx < HPI_MAX_ADAPTER_MEM_SPACES; idx++) {
-		if (pci.ap_mem_base[idx])
-			iounmap(pci.ap_mem_base[idx]);
-	}
+	for (idx = 0; idx < HPI_MAX_ADAPTER_MEM_SPACES; ++idx)
+		iounmap(pci.ap_mem_base[idx]);
 
 	if (pa->irq)
 		free_irq(pa->irq, pa);
diff --git a/sound/pci/atiixp.c b/sound/pci/atiixp.c
index 9c1c445..d24188f 100644
--- a/sound/pci/atiixp.c
+++ b/sound/pci/atiixp.c
@@ -1585,8 +1585,7 @@ static int snd_atiixp_free(struct atiixp *chip)
       __hw_end:
 	if (chip->irq >= 0)
 		free_irq(chip->irq, chip);
-	if (chip->remap_addr)
-		iounmap(chip->remap_addr);
+	iounmap(chip->remap_addr);
 	pci_release_regions(chip->pci);
 	pci_disable_device(chip->pci);
 	kfree(chip);
diff --git a/sound/pci/atiixp_modem.c b/sound/pci/atiixp_modem.c
index b2f63e0..c321a97 100644
--- a/sound/pci/atiixp_modem.c
+++ b/sound/pci/atiixp_modem.c
@@ -1211,8 +1211,7 @@ static int snd_atiixp_free(struct atiixp_modem *chip)
       __hw_end:
 	if (chip->irq >= 0)
 		free_irq(chip->irq, chip);
-	if (chip->remap_addr)
-		iounmap(chip->remap_addr);
+	iounmap(chip->remap_addr);
 	pci_release_regions(chip->pci);
 	pci_disable_device(chip->pci);
 	kfree(chip);
diff --git a/sound/pci/aw2/aw2-alsa.c b/sound/pci/aw2/aw2-alsa.c
index e1cf019..8d2fee7 100644
--- a/sound/pci/aw2/aw2-alsa.c
+++ b/sound/pci/aw2/aw2-alsa.c
@@ -229,9 +229,7 @@ static int snd_aw2_dev_free(struct snd_device *device)
 	if (chip->irq >= 0)
 		free_irq(chip->irq, (void *)chip);
 	/* release the i/o ports & memory */
-	if (chip->iobase_virt)
-		iounmap(chip->iobase_virt);
-
+	iounmap(chip->iobase_virt);
 	pci_release_regions(chip->pci);
 	/* disable the PCI entry */
 	pci_disable_device(chip->pci);
diff --git a/sound/pci/bt87x.c b/sound/pci/bt87x.c
index 058b997..e82ceac 100644
--- a/sound/pci/bt87x.c
+++ b/sound/pci/bt87x.c
@@ -690,8 +690,7 @@ static int snd_bt87x_free(struct snd_bt87x *chip)
 		snd_bt87x_stop(chip);
 	if (chip->irq >= 0)
 		free_irq(chip->irq, chip);
-	if (chip->mmio)
-		iounmap(chip->mmio);
+	iounmap(chip->mmio);
 	pci_release_regions(chip->pci);
 	pci_disable_device(chip->pci);
 	kfree(chip);
diff --git a/sound/pci/cs4281.c b/sound/pci/cs4281.c
index 05a4337..ea33911 100644
--- a/sound/pci/cs4281.c
+++ b/sound/pci/cs4281.c
@@ -1316,10 +1316,8 @@ static int snd_cs4281_free(struct cs4281 *chip)
 
 	if (chip->irq >= 0)
 		free_irq(chip->irq, chip);
-	if (chip->ba0)
-		iounmap(chip->ba0);
-	if (chip->ba1)
-		iounmap(chip->ba1);
+	iounmap(chip->ba0);
+	iounmap(chip->ba1);
 	pci_release_regions(chip->pci);
 	pci_disable_device(chip->pci);
 
diff --git a/sound/pci/cs46xx/cs46xx_lib.c b/sound/pci/cs46xx/cs46xx_lib.c
index dfec84e..128bbfe 100644
--- a/sound/pci/cs46xx/cs46xx_lib.c
+++ b/sound/pci/cs46xx/cs46xx_lib.c
@@ -2949,8 +2949,8 @@ static int snd_cs46xx_free(struct snd_cs46xx *chip)
 
 	for (idx = 0; idx < 5; idx++) {
 		struct snd_cs46xx_region *region = &chip->region.idx[idx];
-		if (region->remap_addr)
-			iounmap(region->remap_addr);
+
+		iounmap(region->remap_addr);
 		release_and_free_resource(region->resource);
 	}
 
diff --git a/sound/pci/ctxfi/cthw20k1.c b/sound/pci/ctxfi/cthw20k1.c
index b425aa8..b8b0d8e 100644
--- a/sound/pci/ctxfi/cthw20k1.c
+++ b/sound/pci/ctxfi/cthw20k1.c
@@ -1985,10 +1985,7 @@ static int hw_card_shutdown(struct hw *hw)
 		free_irq(hw->irq, hw);
 
 	hw->irq	= -1;
-
-	if (hw->mem_base)
-		iounmap(hw->mem_base);
-
+	iounmap(hw->mem_base);
 	hw->mem_base = NULL;
 
 	if (hw->io_base)
diff --git a/sound/pci/ctxfi/cthw20k2.c b/sound/pci/ctxfi/cthw20k2.c
index 253899d..4e16b4d 100644
--- a/sound/pci/ctxfi/cthw20k2.c
+++ b/sound/pci/ctxfi/cthw20k2.c
@@ -2110,10 +2110,7 @@ static int hw_card_shutdown(struct hw *hw)
 		free_irq(hw->irq, hw);
 
 	hw->irq	= -1;
-
-	if (hw->mem_base)
-		iounmap(hw->mem_base);
-
+	iounmap(hw->mem_base);
 	hw->mem_base = NULL;
 
 	if (hw->io_base)
diff --git a/sound/pci/echoaudio/echoaudio.c b/sound/pci/echoaudio/echoaudio.c
index 21228ad..98d4f35 100644
--- a/sound/pci/echoaudio/echoaudio.c
+++ b/sound/pci/echoaudio/echoaudio.c
@@ -1872,12 +1872,8 @@ static int snd_echo_free(struct echoaudio *chip)
 	if (chip->comm_page)
 		snd_dma_free_pages(&chip->commpage_dma_buf);
 
-	if (chip->dsp_registers)
-		iounmap(chip->dsp_registers);
-
+	iounmap(chip->dsp_registers);
 	release_and_free_resource(chip->iores);
-
-
 	pci_disable_device(chip->pci);
 
 	/* release chip data */
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index d426a0b..a971425 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1138,8 +1138,7 @@ static int azx_free(struct azx *chip)
 		free_irq(chip->irq, (void*)chip);
 	if (chip->msi)
 		pci_disable_msi(chip->pci);
-	if (chip->remap_addr)
-		iounmap(chip->remap_addr);
+	iounmap(chip->remap_addr);
 
 	azx_free_stream_pages(chip);
 	if (chip->region_requested)
diff --git a/sound/pci/lola/lola.c b/sound/pci/lola/lola.c
index 4cf4be5..9ff60008 100644
--- a/sound/pci/lola/lola.c
+++ b/sound/pci/lola/lola.c
@@ -551,10 +551,8 @@ static void lola_free(struct lola *chip)
 	lola_free_mixer(chip);
 	if (chip->irq >= 0)
 		free_irq(chip->irq, (void *)chip);
-	if (chip->bar[0].remap_addr)
-		iounmap(chip->bar[0].remap_addr);
-	if (chip->bar[1].remap_addr)
-		iounmap(chip->bar[1].remap_addr);
+	iounmap(chip->bar[0].remap_addr);
+	iounmap(chip->bar[1].remap_addr);
 	if (chip->rb.area)
 		snd_dma_free_pages(&chip->rb);
 	pci_release_regions(chip->pci);
diff --git a/sound/pci/mixart/mixart.c b/sound/pci/mixart/mixart.c
index 1faf47e..c3a9f39 100644
--- a/sound/pci/mixart/mixart.c
+++ b/sound/pci/mixart/mixart.c
@@ -1114,10 +1114,9 @@ static int snd_mixart_free(struct mixart_mgr *mgr)
 	}
 
 	/* release the i/o ports */
-	for (i = 0; i < 2; i++) {
-		if (mgr->mem[i].virt)
-			iounmap(mgr->mem[i].virt);
-	}
+	for (i = 0; i < 2; ++i)
+		iounmap(mgr->mem[i].virt);
+
 	pci_release_regions(mgr->pci);
 
 	/* free flowarray */
diff --git a/sound/pci/nm256/nm256.c b/sound/pci/nm256/nm256.c
index 4e41a4e..3f52a44 100644
--- a/sound/pci/nm256/nm256.c
+++ b/sound/pci/nm256/nm256.c
@@ -1460,10 +1460,8 @@ static int snd_nm256_free(struct nm256 *chip)
 	if (chip->irq >= 0)
 		free_irq(chip->irq, chip);
 
-	if (chip->cport)
-		iounmap(chip->cport);
-	if (chip->buffer)
-		iounmap(chip->buffer);
+	iounmap(chip->cport);
+	iounmap(chip->buffer);
 	release_and_free_resource(chip->res_cport);
 	release_and_free_resource(chip->res_buffer);
 
diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
index cf5a6c8..fe66bcb 100644
--- a/sound/pci/rme9652/hdsp.c
+++ b/sound/pci/rme9652/hdsp.c
@@ -5309,9 +5309,7 @@ static int snd_hdsp_free(struct hdsp *hdsp)
 
 	release_firmware(hdsp->firmware);
 	vfree(hdsp->fw_uploaded);
-
-	if (hdsp->iobase)
-		iounmap(hdsp->iobase);
+	iounmap(hdsp->iobase);
 
 	if (hdsp->port)
 		pci_release_regions(hdsp->pci);
diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c
index 3342705..8109b8e 100644
--- a/sound/pci/rme9652/hdspm.c
+++ b/sound/pci/rme9652/hdspm.c
@@ -6965,9 +6965,7 @@ static int snd_hdspm_free(struct hdspm * hdspm)
 		free_irq(hdspm->irq, (void *) hdspm);
 
 	kfree(hdspm->mixer);
-
-	if (hdspm->iobase)
-		iounmap(hdspm->iobase);
+	iounmap(hdspm->iobase);
 
 	if (hdspm->port)
 		pci_release_regions(hdspm->pci);
diff --git a/sound/pci/rme9652/rme9652.c b/sound/pci/rme9652/rme9652.c
index 6521521..648911c 100644
--- a/sound/pci/rme9652/rme9652.c
+++ b/sound/pci/rme9652/rme9652.c
@@ -1756,8 +1756,7 @@ static int snd_rme9652_free(struct snd_rme9652 *rme9652)
 
 	if (rme9652->irq >= 0)
 		free_irq(rme9652->irq, (void *)rme9652);
-	if (rme9652->iobase)
-		iounmap(rme9652->iobase);
+	iounmap(rme9652->iobase);
 	if (rme9652->port)
 		pci_release_regions(rme9652->pci);
 
diff --git a/sound/pci/sis7019.c b/sound/pci/sis7019.c
index 7f6a0a0..5e9437b 100644
--- a/sound/pci/sis7019.c
+++ b/sound/pci/sis7019.c
@@ -1064,12 +1064,9 @@ static int sis_chip_free(struct sis7019 *sis)
 	if (sis->irq >= 0)
 		free_irq(sis->irq, sis);
 
-	if (sis->ioaddr)
-		iounmap(sis->ioaddr);
-
+	iounmap(sis->ioaddr);
 	pci_release_regions(sis->pci);
 	pci_disable_device(sis->pci);
-
 	sis_free_suspend(sis);
 	return 0;
 }
diff --git a/sound/pci/ymfpci/ymfpci_main.c b/sound/pci/ymfpci/ymfpci_main.c
index f5581a9..de7f06f 100644
--- a/sound/pci/ymfpci/ymfpci_main.c
+++ b/sound/pci/ymfpci/ymfpci_main.c
@@ -2246,8 +2246,7 @@ static int snd_ymfpci_free(struct snd_ymfpci *chip)
 	release_and_free_resource(chip->mpu_res);
 	release_and_free_resource(chip->fm_res);
 	snd_ymfpci_free_gameport(chip);
-	if (chip->reg_area_virt)
-		iounmap(chip->reg_area_virt);
+	iounmap(chip->reg_area_virt);
 	if (chip->work_ptr.area)
 		snd_dma_free_pages(&chip->work_ptr);
 	
diff --git a/sound/ppc/pmac.c b/sound/ppc/pmac.c
index 5a13b22..d399df4 100644
--- a/sound/ppc/pmac.c
+++ b/sound/ppc/pmac.c
@@ -867,16 +867,11 @@ static int snd_pmac_free(struct snd_pmac *chip)
 	snd_pmac_dbdma_free(chip, &chip->capture.cmd);
 	snd_pmac_dbdma_free(chip, &chip->extra_dma);
 	snd_pmac_dbdma_free(chip, &emergency_dbdma);
-	if (chip->macio_base)
-		iounmap(chip->macio_base);
-	if (chip->latch_base)
-		iounmap(chip->latch_base);
-	if (chip->awacs)
-		iounmap(chip->awacs);
-	if (chip->playback.dma)
-		iounmap(chip->playback.dma);
-	if (chip->capture.dma)
-		iounmap(chip->capture.dma);
+	iounmap(chip->macio_base);
+	iounmap(chip->latch_base);
+	iounmap(chip->awacs);
+	iounmap(chip->playback.dma);
+	iounmap(chip->capture.dma);
 
 	if (chip->node) {
 		int i;
-- 
2.2.1

Re: [PATCH 11/13] ALSA: Deletion of checks before the function call "iounmap"

[Dan Carpenter]

On Sun, Jan 04, 2015 at 02:36:01PM +0100, SF Markus Elfring wrote:
>  	/* unmap PCI memory space, mapped during device init. */
> -	for (idx = 0; idx < HPI_MAX_ADAPTER_MEM_SPACES; idx++) {
> -		if (pci.ap_mem_base[idx])
> -			iounmap(pci.ap_mem_base[idx]);
> -	}
> +	for (idx = 0; idx < HPI_MAX_ADAPTER_MEM_SPACES; ++idx)
> +		iounmap(pci.ap_mem_base[idx]);
>  

Don't do the gratuitous idx++ to ++idx changes.  You do it a couple
other places as well.  It belongs in a separate patch if you really feel
it is worth doing.  (It is not a clean up and it is not worth doing).

regards,
dan carpenter

Re: [PATCH 11/13] ALSA: Deletion of checks before the function call "iounmap"

[Dan Carpenter]

Someone was just mentioning in another thread that removing the check
from iounmap() is not portable to other arches and then I remembered
that Markus removed a bunch of these.

We should consider reverting this, perhaps?

regards,
dan carpenter

On Sun, Jan 04, 2015 at 02:36:01PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 3 Jan 2015 22:55:54 +0100
> 
> The iounmap() function performs also input parameter validation.
> Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  sound/aoa/soundbus/i2sbus/core.c | 13 ++++++-------
>  sound/arm/aaci.c                 |  4 ++--
>  sound/drivers/ml403-ac97cr.c     |  3 +--
>  sound/isa/msnd/msnd_pinnacle.c   |  3 +--
>  sound/parisc/harmony.c           |  4 +---
>  sound/pci/ad1889.c               |  5 +----
>  sound/pci/asihpi/hpioctl.c       |  6 ++----
>  sound/pci/atiixp.c               |  3 +--
>  sound/pci/atiixp_modem.c         |  3 +--
>  sound/pci/aw2/aw2-alsa.c         |  4 +---
>  sound/pci/bt87x.c                |  3 +--
>  sound/pci/cs4281.c               |  6 ++----
>  sound/pci/cs46xx/cs46xx_lib.c    |  4 ++--
>  sound/pci/ctxfi/cthw20k1.c       |  5 +----
>  sound/pci/ctxfi/cthw20k2.c       |  5 +----
>  sound/pci/echoaudio/echoaudio.c  |  6 +-----
>  sound/pci/hda/hda_intel.c        |  3 +--
>  sound/pci/lola/lola.c            |  6 ++----
>  sound/pci/mixart/mixart.c        |  7 +++----
>  sound/pci/nm256/nm256.c          |  6 ++----
>  sound/pci/rme9652/hdsp.c         |  4 +---
>  sound/pci/rme9652/hdspm.c        |  4 +---
>  sound/pci/rme9652/rme9652.c      |  3 +--
>  sound/pci/sis7019.c              |  5 +----
>  sound/pci/ymfpci/ymfpci_main.c   |  3 +--
>  sound/ppc/pmac.c                 | 15 +++++----------
>  26 files changed, 43 insertions(+), 90 deletions(-)
> 
> diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
> index 4e2b4fb..7835045 100644
> --- a/sound/aoa/soundbus/i2sbus/core.c
> +++ b/sound/aoa/soundbus/i2sbus/core.c
> @@ -74,10 +74,9 @@ static void i2sbus_release_dev(struct device *dev)
>  	int i;
>  
>  	i2sdev = container_of(dev, struct i2sbus_dev, sound.ofdev.dev);
> -
> - 	if (i2sdev->intfregs) iounmap(i2sdev->intfregs);
> - 	if (i2sdev->out.dbdma) iounmap(i2sdev->out.dbdma);
> - 	if (i2sdev->in.dbdma) iounmap(i2sdev->in.dbdma);
> +	iounmap(i2sdev->intfregs);
> +	iounmap(i2sdev->out.dbdma);
> +	iounmap(i2sdev->in.dbdma);
>  	for (i = aoa_resource_i2smmio; i <= aoa_resource_rxdbdma; i++)
>  		release_and_free_resource(i2sdev->allocated_resource[i]);
>  	free_dbdma_descriptor_ring(i2sdev, &i2sdev->out.dbdma_ring);
> @@ -318,9 +317,9 @@ static int i2sbus_add_dev(struct macio_dev *macio,
>  			free_irq(dev->interrupts[i], dev);
>  	free_dbdma_descriptor_ring(dev, &dev->out.dbdma_ring);
>  	free_dbdma_descriptor_ring(dev, &dev->in.dbdma_ring);
> -	if (dev->intfregs) iounmap(dev->intfregs);
> -	if (dev->out.dbdma) iounmap(dev->out.dbdma);
> -	if (dev->in.dbdma) iounmap(dev->in.dbdma);
> +	iounmap(dev->intfregs);
> +	iounmap(dev->out.dbdma);
> +	iounmap(dev->in.dbdma);
>  	for (i=0;i<3;i++)
>  		release_and_free_resource(dev->allocated_resource[i]);
>  	mutex_destroy(&dev->lock);
> diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c
> index 0e83a73..4140b1b 100644
> --- a/sound/arm/aaci.c
> +++ b/sound/arm/aaci.c
> @@ -889,8 +889,8 @@ static int aaci_probe_ac97(struct aaci *aaci)
>  static void aaci_free_card(struct snd_card *card)
>  {
>  	struct aaci *aaci = card->private_data;
> -	if (aaci->base)
> -		iounmap(aaci->base);
> +
> +	iounmap(aaci->base);
>  }
>  
>  static struct aaci *aaci_init_card(struct amba_device *dev)
> diff --git a/sound/drivers/ml403-ac97cr.c b/sound/drivers/ml403-ac97cr.c
> index ec01de1..bdcb572 100644
> --- a/sound/drivers/ml403-ac97cr.c
> +++ b/sound/drivers/ml403-ac97cr.c
> @@ -1094,8 +1094,7 @@ static int snd_ml403_ac97cr_free(struct snd_ml403_ac97cr *ml403_ac97cr)
>  	if (ml403_ac97cr->capture_irq >= 0)
>  		free_irq(ml403_ac97cr->capture_irq, ml403_ac97cr);
>  	/* give back "port" */
> -	if (ml403_ac97cr->port != NULL)
> -		iounmap(ml403_ac97cr->port);
> +	iounmap(ml403_ac97cr->port);
>  	kfree(ml403_ac97cr);
>  	PDEBUG(INIT_INFO, "free(): (done)\n");
>  	return 0;
> diff --git a/sound/isa/msnd/msnd_pinnacle.c b/sound/isa/msnd/msnd_pinnacle.c
> index 65b3682..4c07266 100644
> --- a/sound/isa/msnd/msnd_pinnacle.c
> +++ b/sound/isa/msnd/msnd_pinnacle.c
> @@ -627,8 +627,7 @@ static int snd_msnd_attach(struct snd_card *card)
>  	return 0;
>  
>  err_release_region:
> -	if (chip->mappedbase)
> -		iounmap(chip->mappedbase);
> +	iounmap(chip->mappedbase);
>  	release_mem_region(chip->base, BUFFSIZE);
>  	release_region(chip->io, DSP_NUMIO);
>  	free_irq(chip->irq, chip);
> diff --git a/sound/parisc/harmony.c b/sound/parisc/harmony.c
> index 29604a2..f2350c1 100644
> --- a/sound/parisc/harmony.c
> +++ b/sound/parisc/harmony.c
> @@ -893,9 +893,7 @@ snd_harmony_free(struct snd_harmony *h)
>  	if (h->irq >= 0)
>  		free_irq(h->irq, h);
>  
> -	if (h->iobase)
> -		iounmap(h->iobase);
> -
> +	iounmap(h->iobase);
>  	kfree(h);
>  	return 0;
>  }
> diff --git a/sound/pci/ad1889.c b/sound/pci/ad1889.c
> index 547ee30..0de3129 100644
> --- a/sound/pci/ad1889.c
> +++ b/sound/pci/ad1889.c
> @@ -853,12 +853,9 @@ snd_ad1889_free(struct snd_ad1889 *chip)
>  		free_irq(chip->irq, chip);
>  
>  skip_hw:
> -	if (chip->iobase)
> -		iounmap(chip->iobase);
> -
> +	iounmap(chip->iobase);
>  	pci_release_regions(chip->pci);
>  	pci_disable_device(chip->pci);
> -
>  	kfree(chip);
>  	return 0;
>  }
> diff --git a/sound/pci/asihpi/hpioctl.c b/sound/pci/asihpi/hpioctl.c
> index 72af66b..67d1133 100644
> --- a/sound/pci/asihpi/hpioctl.c
> +++ b/sound/pci/asihpi/hpioctl.c
> @@ -541,10 +541,8 @@ void asihpi_adapter_remove(struct pci_dev *pci_dev)
>  	hpi_send_recv_ex(&hm, &hr, HOWNER_KERNEL);
>  
>  	/* unmap PCI memory space, mapped during device init. */
> -	for (idx = 0; idx < HPI_MAX_ADAPTER_MEM_SPACES; idx++) {
> -		if (pci.ap_mem_base[idx])
> -			iounmap(pci.ap_mem_base[idx]);
> -	}
> +	for (idx = 0; idx < HPI_MAX_ADAPTER_MEM_SPACES; ++idx)
> +		iounmap(pci.ap_mem_base[idx]);
>  
>  	if (pa->irq)
>  		free_irq(pa->irq, pa);
> diff --git a/sound/pci/atiixp.c b/sound/pci/atiixp.c
> index 9c1c445..d24188f 100644
> --- a/sound/pci/atiixp.c
> +++ b/sound/pci/atiixp.c
> @@ -1585,8 +1585,7 @@ static int snd_atiixp_free(struct atiixp *chip)
>        __hw_end:
>  	if (chip->irq >= 0)
>  		free_irq(chip->irq, chip);
> -	if (chip->remap_addr)
> -		iounmap(chip->remap_addr);
> +	iounmap(chip->remap_addr);
>  	pci_release_regions(chip->pci);
>  	pci_disable_device(chip->pci);
>  	kfree(chip);
> diff --git a/sound/pci/atiixp_modem.c b/sound/pci/atiixp_modem.c
> index b2f63e0..c321a97 100644
> --- a/sound/pci/atiixp_modem.c
> +++ b/sound/pci/atiixp_modem.c
> @@ -1211,8 +1211,7 @@ static int snd_atiixp_free(struct atiixp_modem *chip)
>        __hw_end:
>  	if (chip->irq >= 0)
>  		free_irq(chip->irq, chip);
> -	if (chip->remap_addr)
> -		iounmap(chip->remap_addr);
> +	iounmap(chip->remap_addr);
>  	pci_release_regions(chip->pci);
>  	pci_disable_device(chip->pci);
>  	kfree(chip);
> diff --git a/sound/pci/aw2/aw2-alsa.c b/sound/pci/aw2/aw2-alsa.c
> index e1cf019..8d2fee7 100644
> --- a/sound/pci/aw2/aw2-alsa.c
> +++ b/sound/pci/aw2/aw2-alsa.c
> @@ -229,9 +229,7 @@ static int snd_aw2_dev_free(struct snd_device *device)
>  	if (chip->irq >= 0)
>  		free_irq(chip->irq, (void *)chip);
>  	/* release the i/o ports & memory */
> -	if (chip->iobase_virt)
> -		iounmap(chip->iobase_virt);
> -
> +	iounmap(chip->iobase_virt);
>  	pci_release_regions(chip->pci);
>  	/* disable the PCI entry */
>  	pci_disable_device(chip->pci);
> diff --git a/sound/pci/bt87x.c b/sound/pci/bt87x.c
> index 058b997..e82ceac 100644
> --- a/sound/pci/bt87x.c
> +++ b/sound/pci/bt87x.c
> @@ -690,8 +690,7 @@ static int snd_bt87x_free(struct snd_bt87x *chip)
>  		snd_bt87x_stop(chip);
>  	if (chip->irq >= 0)
>  		free_irq(chip->irq, chip);
> -	if (chip->mmio)
> -		iounmap(chip->mmio);
> +	iounmap(chip->mmio);
>  	pci_release_regions(chip->pci);
>  	pci_disable_device(chip->pci);
>  	kfree(chip);
> diff --git a/sound/pci/cs4281.c b/sound/pci/cs4281.c
> index 05a4337..ea33911 100644
> --- a/sound/pci/cs4281.c
> +++ b/sound/pci/cs4281.c
> @@ -1316,10 +1316,8 @@ static int snd_cs4281_free(struct cs4281 *chip)
>  
>  	if (chip->irq >= 0)
>  		free_irq(chip->irq, chip);
> -	if (chip->ba0)
> -		iounmap(chip->ba0);
> -	if (chip->ba1)
> -		iounmap(chip->ba1);
> +	iounmap(chip->ba0);
> +	iounmap(chip->ba1);
>  	pci_release_regions(chip->pci);
>  	pci_disable_device(chip->pci);
>  
> diff --git a/sound/pci/cs46xx/cs46xx_lib.c b/sound/pci/cs46xx/cs46xx_lib.c
> index dfec84e..128bbfe 100644
> --- a/sound/pci/cs46xx/cs46xx_lib.c
> +++ b/sound/pci/cs46xx/cs46xx_lib.c
> @@ -2949,8 +2949,8 @@ static int snd_cs46xx_free(struct snd_cs46xx *chip)
>  
>  	for (idx = 0; idx < 5; idx++) {
>  		struct snd_cs46xx_region *region = &chip->region.idx[idx];
> -		if (region->remap_addr)
> -			iounmap(region->remap_addr);
> +
> +		iounmap(region->remap_addr);
>  		release_and_free_resource(region->resource);
>  	}
>  
> diff --git a/sound/pci/ctxfi/cthw20k1.c b/sound/pci/ctxfi/cthw20k1.c
> index b425aa8..b8b0d8e 100644
> --- a/sound/pci/ctxfi/cthw20k1.c
> +++ b/sound/pci/ctxfi/cthw20k1.c
> @@ -1985,10 +1985,7 @@ static int hw_card_shutdown(struct hw *hw)
>  		free_irq(hw->irq, hw);
>  
>  	hw->irq	= -1;
> -
> -	if (hw->mem_base)
> -		iounmap(hw->mem_base);
> -
> +	iounmap(hw->mem_base);
>  	hw->mem_base = NULL;
>  
>  	if (hw->io_base)
> diff --git a/sound/pci/ctxfi/cthw20k2.c b/sound/pci/ctxfi/cthw20k2.c
> index 253899d..4e16b4d 100644
> --- a/sound/pci/ctxfi/cthw20k2.c
> +++ b/sound/pci/ctxfi/cthw20k2.c
> @@ -2110,10 +2110,7 @@ static int hw_card_shutdown(struct hw *hw)
>  		free_irq(hw->irq, hw);
>  
>  	hw->irq	= -1;
> -
> -	if (hw->mem_base)
> -		iounmap(hw->mem_base);
> -
> +	iounmap(hw->mem_base);
>  	hw->mem_base = NULL;
>  
>  	if (hw->io_base)
> diff --git a/sound/pci/echoaudio/echoaudio.c b/sound/pci/echoaudio/echoaudio.c
> index 21228ad..98d4f35 100644
> --- a/sound/pci/echoaudio/echoaudio.c
> +++ b/sound/pci/echoaudio/echoaudio.c
> @@ -1872,12 +1872,8 @@ static int snd_echo_free(struct echoaudio *chip)
>  	if (chip->comm_page)
>  		snd_dma_free_pages(&chip->commpage_dma_buf);
>  
> -	if (chip->dsp_registers)
> -		iounmap(chip->dsp_registers);
> -
> +	iounmap(chip->dsp_registers);
>  	release_and_free_resource(chip->iores);
> -
> -
>  	pci_disable_device(chip->pci);
>  
>  	/* release chip data */
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index d426a0b..a971425 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -1138,8 +1138,7 @@ static int azx_free(struct azx *chip)
>  		free_irq(chip->irq, (void*)chip);
>  	if (chip->msi)
>  		pci_disable_msi(chip->pci);
> -	if (chip->remap_addr)
> -		iounmap(chip->remap_addr);
> +	iounmap(chip->remap_addr);
>  
>  	azx_free_stream_pages(chip);
>  	if (chip->region_requested)
> diff --git a/sound/pci/lola/lola.c b/sound/pci/lola/lola.c
> index 4cf4be5..9ff60008 100644
> --- a/sound/pci/lola/lola.c
> +++ b/sound/pci/lola/lola.c
> @@ -551,10 +551,8 @@ static void lola_free(struct lola *chip)
>  	lola_free_mixer(chip);
>  	if (chip->irq >= 0)
>  		free_irq(chip->irq, (void *)chip);
> -	if (chip->bar[0].remap_addr)
> -		iounmap(chip->bar[0].remap_addr);
> -	if (chip->bar[1].remap_addr)
> -		iounmap(chip->bar[1].remap_addr);
> +	iounmap(chip->bar[0].remap_addr);
> +	iounmap(chip->bar[1].remap_addr);
>  	if (chip->rb.area)
>  		snd_dma_free_pages(&chip->rb);
>  	pci_release_regions(chip->pci);
> diff --git a/sound/pci/mixart/mixart.c b/sound/pci/mixart/mixart.c
> index 1faf47e..c3a9f39 100644
> --- a/sound/pci/mixart/mixart.c
> +++ b/sound/pci/mixart/mixart.c
> @@ -1114,10 +1114,9 @@ static int snd_mixart_free(struct mixart_mgr *mgr)
>  	}
>  
>  	/* release the i/o ports */
> -	for (i = 0; i < 2; i++) {
> -		if (mgr->mem[i].virt)
> -			iounmap(mgr->mem[i].virt);
> -	}
> +	for (i = 0; i < 2; ++i)
> +		iounmap(mgr->mem[i].virt);
> +
>  	pci_release_regions(mgr->pci);
>  
>  	/* free flowarray */
> diff --git a/sound/pci/nm256/nm256.c b/sound/pci/nm256/nm256.c
> index 4e41a4e..3f52a44 100644
> --- a/sound/pci/nm256/nm256.c
> +++ b/sound/pci/nm256/nm256.c
> @@ -1460,10 +1460,8 @@ static int snd_nm256_free(struct nm256 *chip)
>  	if (chip->irq >= 0)
>  		free_irq(chip->irq, chip);
>  
> -	if (chip->cport)
> -		iounmap(chip->cport);
> -	if (chip->buffer)
> -		iounmap(chip->buffer);
> +	iounmap(chip->cport);
> +	iounmap(chip->buffer);
>  	release_and_free_resource(chip->res_cport);
>  	release_and_free_resource(chip->res_buffer);
>  
> diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
> index cf5a6c8..fe66bcb 100644
> --- a/sound/pci/rme9652/hdsp.c
> +++ b/sound/pci/rme9652/hdsp.c
> @@ -5309,9 +5309,7 @@ static int snd_hdsp_free(struct hdsp *hdsp)
>  
>  	release_firmware(hdsp->firmware);
>  	vfree(hdsp->fw_uploaded);
> -
> -	if (hdsp->iobase)
> -		iounmap(hdsp->iobase);
> +	iounmap(hdsp->iobase);
>  
>  	if (hdsp->port)
>  		pci_release_regions(hdsp->pci);
> diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c
> index 3342705..8109b8e 100644
> --- a/sound/pci/rme9652/hdspm.c
> +++ b/sound/pci/rme9652/hdspm.c
> @@ -6965,9 +6965,7 @@ static int snd_hdspm_free(struct hdspm * hdspm)
>  		free_irq(hdspm->irq, (void *) hdspm);
>  
>  	kfree(hdspm->mixer);
> -
> -	if (hdspm->iobase)
> -		iounmap(hdspm->iobase);
> +	iounmap(hdspm->iobase);
>  
>  	if (hdspm->port)
>  		pci_release_regions(hdspm->pci);
> diff --git a/sound/pci/rme9652/rme9652.c b/sound/pci/rme9652/rme9652.c
> index 6521521..648911c 100644
> --- a/sound/pci/rme9652/rme9652.c
> +++ b/sound/pci/rme9652/rme9652.c
> @@ -1756,8 +1756,7 @@ static int snd_rme9652_free(struct snd_rme9652 *rme9652)
>  
>  	if (rme9652->irq >= 0)
>  		free_irq(rme9652->irq, (void *)rme9652);
> -	if (rme9652->iobase)
> -		iounmap(rme9652->iobase);
> +	iounmap(rme9652->iobase);
>  	if (rme9652->port)
>  		pci_release_regions(rme9652->pci);
>  
> diff --git a/sound/pci/sis7019.c b/sound/pci/sis7019.c
> index 7f6a0a0..5e9437b 100644
> --- a/sound/pci/sis7019.c
> +++ b/sound/pci/sis7019.c
> @@ -1064,12 +1064,9 @@ static int sis_chip_free(struct sis7019 *sis)
>  	if (sis->irq >= 0)
>  		free_irq(sis->irq, sis);
>  
> -	if (sis->ioaddr)
> -		iounmap(sis->ioaddr);
> -
> +	iounmap(sis->ioaddr);
>  	pci_release_regions(sis->pci);
>  	pci_disable_device(sis->pci);
> -
>  	sis_free_suspend(sis);
>  	return 0;
>  }
> diff --git a/sound/pci/ymfpci/ymfpci_main.c b/sound/pci/ymfpci/ymfpci_main.c
> index f5581a9..de7f06f 100644
> --- a/sound/pci/ymfpci/ymfpci_main.c
> +++ b/sound/pci/ymfpci/ymfpci_main.c
> @@ -2246,8 +2246,7 @@ static int snd_ymfpci_free(struct snd_ymfpci *chip)
>  	release_and_free_resource(chip->mpu_res);
>  	release_and_free_resource(chip->fm_res);
>  	snd_ymfpci_free_gameport(chip);
> -	if (chip->reg_area_virt)
> -		iounmap(chip->reg_area_virt);
> +	iounmap(chip->reg_area_virt);
>  	if (chip->work_ptr.area)
>  		snd_dma_free_pages(&chip->work_ptr);
>  	
> diff --git a/sound/ppc/pmac.c b/sound/ppc/pmac.c
> index 5a13b22..d399df4 100644
> --- a/sound/ppc/pmac.c
> +++ b/sound/ppc/pmac.c
> @@ -867,16 +867,11 @@ static int snd_pmac_free(struct snd_pmac *chip)
>  	snd_pmac_dbdma_free(chip, &chip->capture.cmd);
>  	snd_pmac_dbdma_free(chip, &chip->extra_dma);
>  	snd_pmac_dbdma_free(chip, &emergency_dbdma);
> -	if (chip->macio_base)
> -		iounmap(chip->macio_base);
> -	if (chip->latch_base)
> -		iounmap(chip->latch_base);
> -	if (chip->awacs)
> -		iounmap(chip->awacs);
> -	if (chip->playback.dma)
> -		iounmap(chip->playback.dma);
> -	if (chip->capture.dma)
> -		iounmap(chip->capture.dma);
> +	iounmap(chip->macio_base);
> +	iounmap(chip->latch_base);
> +	iounmap(chip->awacs);
> +	iounmap(chip->playback.dma);
> +	iounmap(chip->capture.dma);
>  
>  	if (chip->node) {
>  		int i;
> -- 
> 2.2.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/13] ALSA: Deletion of checks before the function call "iounmap"

[Johannes Berg]

On Wed, 2016-10-26 at 15:26 +0300, Dan Carpenter wrote:
> Someone was just mentioning in another thread that removing the check
> from iounmap() is not portable to other arches and then I remembered
> that Markus removed a bunch of these.
> 
> We should consider reverting this, perhaps?

Can't we teach all architectures? Not that reverting it would be a
problem.

johannes

Re: [PATCH 11/13] ALSA: Deletion of checks before the function call "iounmap"

[Dan Carpenter]

On Wed, Oct 26, 2016 at 02:28:59PM +0200, Johannes Berg wrote:
> On Wed, 2016-10-26 at 15:26 +0300, Dan Carpenter wrote:
> > Someone was just mentioning in another thread that removing the check
> > from iounmap() is not portable to other arches and then I remembered
> > that Markus removed a bunch of these.
> > 
> > We should consider reverting this, perhaps?
> 
> Can't we teach all architectures? Not that reverting it would be a
> problem.

We probably should.  I just didn't want to suggest it, in case it sounds
like volunteering...  I'm not going to be able to do it because I'm on
the road for a bit.

regards,
dan carpenter

[PATCH] PowerPC-PCI: Delete unnecessary checks before the function call "kfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 3 Feb 2015 13:55:53 +0100

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/powerpc/platforms/cell/celleb_pci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/cell/celleb_pci.c b/arch/powerpc/platforms/cell/celleb_pci.c
index 3ce70de..9b11b5d 100644
--- a/arch/powerpc/platforms/cell/celleb_pci.c
+++ b/arch/powerpc/platforms/cell/celleb_pci.c
@@ -393,11 +393,10 @@ static int __init celleb_setup_fake_pci_device(struct device_node *node,
 
 error:
 	if (mem_init_done) {
-		if (config && *config)
+		if (config)
 			kfree(*config);
-		if (res && *res)
+		if (res)
 			kfree(*res);
-
 	} else {
 		if (config && *config) {
 			size = 256;
-- 
2.2.2

[PATCH] PowerPC-rheap: Delete an unnecessary check before the function call "kfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 3 Feb 2015 14:34:10 +0100

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/powerpc/lib/rheap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/lib/rheap.c b/arch/powerpc/lib/rheap.c
index a1060a8..69abf84 100644
--- a/arch/powerpc/lib/rheap.c
+++ b/arch/powerpc/lib/rheap.c
@@ -284,7 +284,7 @@ EXPORT_SYMBOL_GPL(rh_create);
  */
 void rh_destroy(rh_info_t * info)
 {
-	if ((info->flags & RHIF_STATIC_BLOCK) == 0 && info->block != NULL)
+	if ((info->flags & RHIF_STATIC_BLOCK) == 0)
 		kfree(info->block);
 
 	if ((info->flags & RHIF_STATIC_INFO) == 0)
-- 
2.2.2

[PATCH] macintosh: Delete an unnecessary check before the function call "of_node_put"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 4 Feb 2015 21:32:27 +0100

The of_node_put() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/macintosh/smu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/macintosh/smu.c b/drivers/macintosh/smu.c
index 10ae69b..d531f80 100644
--- a/drivers/macintosh/smu.c
+++ b/drivers/macintosh/smu.c
@@ -557,8 +557,7 @@ int __init smu_init (void)
 	return 0;
 
 fail_msg_node:
-	if (smu->msg_node)
-		of_node_put(smu->msg_node);
+	of_node_put(smu->msg_node);
 fail_db_node:
 	of_node_put(smu->db_node);
 fail_bootmem:
-- 
2.2.2

Re: [PATCH] macintosh: Delete an unnecessary check before the function call "of_node_put"

[SF Markus Elfring]

Am 04.02.2015 um 21:36 schrieb SF Markus Elfring:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 4 Feb 2015 21:32:27 +0100
> 
> The of_node_put() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/macintosh/smu.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/macintosh/smu.c b/drivers/macintosh/smu.c
> index 10ae69b..d531f80 100644
> --- a/drivers/macintosh/smu.c
> +++ b/drivers/macintosh/smu.c
> @@ -557,8 +557,7 @@ int __init smu_init (void)
>  	return 0;
>  
>  fail_msg_node:
> -	if (smu->msg_node)
> -		of_node_put(smu->msg_node);
> +	of_node_put(smu->msg_node);
>  fail_db_node:
>  	of_node_put(smu->db_node);
>  fail_bootmem:
> 

Would you like to integrate this update suggestion
into another source code repository?

Regards,
Markus

[PATCH] cxl: Delete an unnecessary check before the function call "kfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 6 Nov 2015 11:00:23 +0100

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/misc/cxl/context.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 2faa127..52e39b6 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -275,9 +275,7 @@ static void reclaim_ctx(struct rcu_head *rcu)
 	if (ctx->kernelapi)
 		kfree(ctx->mapping);
 
-	if (ctx->irq_bitmap)
-		kfree(ctx->irq_bitmap);
-
+	kfree(ctx->irq_bitmap);
 	kfree(ctx);
 }
 
-- 
2.6.2

Re: [PATCH] cxl: Delete an unnecessary check before the function call "kfree"

[Andrew Donnellan]

On 06/11/15 21:05, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 6 Nov 2015 11:00:23 +0100
>
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Thanks for picking this up - will remember in my future patches.

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

-- 
Andrew Donnellan              Software Engineer, OzLabs
andrew.donnellan@au1.ibm.com  Australia Development Lab, Canberra
+61 2 6201 8874 (work)        IBM Australia Limited

Re: [PATCH] cxl: Delete an unnecessary check before the function call "kfree"

[Ian Munsie]

Acked-by: Ian Munsie <imunsie@au1.ibm.com>

Re: cxl: Delete an unnecessary check before the function call "kfree"

[Michael Ellerman]

On Fri, 2015-06-11 at 10:05:46 UTC, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 6 Nov 2015 11:00:23 +0100
> 
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Acked-by: Ian Munsie <imunsie@au1.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/1050e689a63baffdadcd33498c

cheers

[PATCH] cxl: Delete an unnecessary check before the function call "of_node_put"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 20 Jul 2016 15:10:32 +0200

The of_node_put() function tests whether its argument is NULL
and then returns immediately.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/misc/cxl/of.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/misc/cxl/of.c b/drivers/misc/cxl/of.c
index edc4583..333256a 100644
--- a/drivers/misc/cxl/of.c
+++ b/drivers/misc/cxl/of.c
@@ -490,8 +490,7 @@ int cxl_of_probe(struct platform_device *pdev)
 		adapter->slices = 0;
 	}
 
-	if (afu_np)
-		of_node_put(afu_np);
+	of_node_put(afu_np);
 	return 0;
 }
 
-- 
2.9.2

Re: [PATCH] cxl: Delete an unnecessary check before the function call "of_node_put"

[Julia Lawall]

On Wed, 20 Jul 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 20 Jul 2016 15:10:32 +0200
>
> The of_node_put() function tests whether its argument is NULL
> and then returns immediately.
> Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/misc/cxl/of.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/misc/cxl/of.c b/drivers/misc/cxl/of.c
> index edc4583..333256a 100644
> --- a/drivers/misc/cxl/of.c
> +++ b/drivers/misc/cxl/of.c
> @@ -490,8 +490,7 @@ int cxl_of_probe(struct platform_device *pdev)
>  		adapter->slices = 0;
>  	}
>
> -	if (afu_np)
> -		of_node_put(afu_np);
> +	of_node_put(afu_np);
>  	return 0;
>  }

I don't think that the call should be there at all.  The loop only exits
when afu_np is NULL.  Furthermore, the loop should not be written as a for
loop, but rather with for_each_child_of_node.

julia

Re: [PATCH] cxl: Delete an unnecessary check before the function call "of_node_put"

[Andrew Donnellan]

On 20/07/16 23:38, Julia Lawall wrote:
> I don't think that the call should be there at all.  The loop only exits
> when afu_np is NULL.  Furthermore, the loop should not be written as a for
> loop, but rather with for_each_child_of_node.

Will send a patch to fix both issues shortly.

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

[PATCH] cxl: replace loop with for_each_child_of_node(), remove unneeded of_node_put()

[Andrew Donnellan]

Rewrite the cxl_guest_init_afu() loop in cxl_of_probe() to use
for_each_child_of_node() rather than a hand-coded for loop.

Remove the useless of_node_put(afu_np) call after the loop, where it's
guaranteed that afu_np == NULL.

Reported-by: SF Markus Elfring <elfring@users.sourceforge.net>
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

---

Checked the of_node_put() with Fred, he thinks it was probably just left
over from an earlier private version of the code and we can just get rid of
it.
---
 drivers/misc/cxl/of.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/cxl/of.c b/drivers/misc/cxl/of.c
index edc4583..ec175ea 100644
--- a/drivers/misc/cxl/of.c
+++ b/drivers/misc/cxl/of.c
@@ -460,7 +460,7 @@ int cxl_of_probe(struct platform_device *pdev)
 	struct device_node *afu_np = NULL;
 	struct cxl *adapter = NULL;
 	int ret;
-	int slice, slice_ok;
+	int slice = 0, slice_ok = 0;
 
 	pr_devel("in %s\n", __func__);
 
@@ -476,13 +476,13 @@ int cxl_of_probe(struct platform_device *pdev)
 	}
 
 	/* init afu */
-	slice_ok = 0;
-	for (afu_np = NULL, slice = 0; (afu_np = of_get_next_child(np, afu_np)); slice++) {
+	for_each_child_of_node(np, afu_np) {
 		if ((ret = cxl_guest_init_afu(adapter, slice, afu_np)))
 			dev_err(&pdev->dev, "AFU %i failed to initialise: %i\n",
 				slice, ret);
 		else
 			slice_ok++;
+		slice++;
 	}
 
 	if (slice_ok == 0) {
@@ -490,8 +490,6 @@ int cxl_of_probe(struct platform_device *pdev)
 		adapter->slices = 0;
 	}
 
-	if (afu_np)
-		of_node_put(afu_np);
 	return 0;
 }
 
-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

Re: [PATCH] cxl: replace loop with for_each_child_of_node(), remove unneeded of_node_put()

[Frederic Barrat]

Le 29/07/2016 à 05:55, Andrew Donnellan a écrit :
> Rewrite the cxl_guest_init_afu() loop in cxl_of_probe() to use
> for_each_child_of_node() rather than a hand-coded for loop.
>
> Remove the useless of_node_put(afu_np) call after the loop, where it's
> guaranteed that afu_np == NULL.
>
> Reported-by: SF Markus Elfring <elfring@users.sourceforge.net>
> Reported-by: Julia Lawall <julia.lawall@lip6.fr>
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>


Thanks!

Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>

Re: [PATCH] cxl: replace loop with for_each_child_of_node(), remove unneeded of_node_put()

[walter harms]

Am 29.07.2016 05:55, schrieb Andrew Donnellan:
> Rewrite the cxl_guest_init_afu() loop in cxl_of_probe() to use
> for_each_child_of_node() rather than a hand-coded for loop.
> 
> Remove the useless of_node_put(afu_np) call after the loop, where it's
> guaranteed that afu_np == NULL.
> 
> Reported-by: SF Markus Elfring <elfring@users.sourceforge.net>
> Reported-by: Julia Lawall <julia.lawall@lip6.fr>
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> 
> ---
> 
> Checked the of_node_put() with Fred, he thinks it was probably just left
> over from an earlier private version of the code and we can just get rid of
> it.
> ---
>  drivers/misc/cxl/of.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/misc/cxl/of.c b/drivers/misc/cxl/of.c
> index edc4583..ec175ea 100644
> --- a/drivers/misc/cxl/of.c
> +++ b/drivers/misc/cxl/of.c
> @@ -460,7 +460,7 @@ int cxl_of_probe(struct platform_device *pdev)
>  	struct device_node *afu_np = NULL;
>  	struct cxl *adapter = NULL;
>  	int ret;
> -	int slice, slice_ok;
> +	int slice = 0, slice_ok = 0;
>  
>  	pr_devel("in %s\n", __func__);
>  
> @@ -476,13 +476,13 @@ int cxl_of_probe(struct platform_device *pdev)
>  	}
>  
>  	/* init afu */
> -	slice_ok = 0;
> -	for (afu_np = NULL, slice = 0; (afu_np = of_get_next_child(np, afu_np)); slice++) {
> +	for_each_child_of_node(np, afu_np) {
>  		if ((ret = cxl_guest_init_afu(adapter, slice, afu_np)))
>  			dev_err(&pdev->dev, "AFU %i failed to initialise: %i\n",
>  				slice, ret);
>  		else
>  			slice_ok++;
> +		slice++;
>  	}

while you are here ..
you could move the assign out of the condition..

ret = cxl_guest_init_afu(adapter, slice, afu_np);
if (ret) ....

just my 2 cents,

re,
 wh

>  
>  	if (slice_ok == 0) {
> @@ -490,8 +490,6 @@ int cxl_of_probe(struct platform_device *pdev)
>  		adapter->slices = 0;
>  	}
>  
> -	if (afu_np)
> -		of_node_put(afu_np);
>  	return 0;
>  }
>  

Re: [PATCH] cxl: replace loop with for_each_child_of_node(), remove unneeded of_node_put()

[Julia Lawall]

On Fri, 29 Jul 2016, walter harms wrote:

>
>
> Am 29.07.2016 05:55, schrieb Andrew Donnellan:
> > Rewrite the cxl_guest_init_afu() loop in cxl_of_probe() to use
> > for_each_child_of_node() rather than a hand-coded for loop.
> >
> > Remove the useless of_node_put(afu_np) call after the loop, where it's
> > guaranteed that afu_np == NULL.
> >
> > Reported-by: SF Markus Elfring <elfring@users.sourceforge.net>
> > Reported-by: Julia Lawall <julia.lawall@lip6.fr>
> > Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> >
> > ---
> >
> > Checked the of_node_put() with Fred, he thinks it was probably just left
> > over from an earlier private version of the code and we can just get rid of
> > it.
> > ---
> >  drivers/misc/cxl/of.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/misc/cxl/of.c b/drivers/misc/cxl/of.c
> > index edc4583..ec175ea 100644
> > --- a/drivers/misc/cxl/of.c
> > +++ b/drivers/misc/cxl/of.c
> > @@ -460,7 +460,7 @@ int cxl_of_probe(struct platform_device *pdev)
> >  	struct device_node *afu_np = NULL;
> >  	struct cxl *adapter = NULL;
> >  	int ret;
> > -	int slice, slice_ok;
> > +	int slice = 0, slice_ok = 0;
> >
> >  	pr_devel("in %s\n", __func__);
> >
> > @@ -476,13 +476,13 @@ int cxl_of_probe(struct platform_device *pdev)
> >  	}
> >
> >  	/* init afu */
> > -	slice_ok = 0;
> > -	for (afu_np = NULL, slice = 0; (afu_np = of_get_next_child(np, afu_np)); slice++) {
> > +	for_each_child_of_node(np, afu_np) {
> >  		if ((ret = cxl_guest_init_afu(adapter, slice, afu_np)))
> >  			dev_err(&pdev->dev, "AFU %i failed to initialise: %i\n",
> >  				slice, ret);
> >  		else
> >  			slice_ok++;
> > +		slice++;
> >  	}
>
> while you are here ..
> you could move the assign out of the condition..
>
> ret = cxl_guest_init_afu(adapter, slice, afu_np);
> if (ret) ....

Yes, please.

julia


>
> just my 2 cents,
>
> re,
>  wh
>
> >
> >  	if (slice_ok == 0) {
> > @@ -490,8 +490,6 @@ int cxl_of_probe(struct platform_device *pdev)
> >  		adapter->slices = 0;
> >  	}
> >
> > -	if (afu_np)
> > -		of_node_put(afu_np);
> >  	return 0;
> >  }
> >
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] cxl: replace loop with for_each_child_of_node(), remove unneeded of_node_put()

[Michael Ellerman]

Andrew Donnellan <andrew.donnellan@au1.ibm.com> writes:

> Rewrite the cxl_guest_init_afu() loop in cxl_of_probe() to use
> for_each_child_of_node() rather than a hand-coded for loop.
>
> Remove the useless of_node_put(afu_np) call after the loop, where it's
> guaranteed that afu_np == NULL.
>
> Reported-by: SF Markus Elfring <elfring@users.sourceforge.net>
> Reported-by: Julia Lawall <julia.lawall@lip6.fr>
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>
> ---
>
> Checked the of_node_put() with Fred, he thinks it was probably just left
> over from an earlier private version of the code and we can just get rid of
> it.

But who does keep a reference on the device_node? I can't see it anywhere. Which
means in theory the device_node can be freed out from under you.

You have a reference for afu_np as part of for_each_child_of_node(), but it's
dropped as soon as you go around the loop.

The typical pattern would be that cxl_guest_init_afu() takes an additional
reference once it's done all its setup and can't fail.

That way at the end of the loop when the loop construct has dropped all
references, the nodes you actually init'ed have their reference count
incremented by 1.

cheers

Re: [PATCH] cxl: replace loop with for_each_child_of_node(), remove unneeded of_node_put()

[Frederic Barrat]

Le 29/07/2016 à 13:38, Michael Ellerman a écrit :
> But who does keep a reference on the device_node? I can't see it anywhere. Which
> means in theory the device_node can be freed out from under you.
>
> You have a reference for afu_np as part of for_each_child_of_node(), but it's
> dropped as soon as you go around the loop.
>
> The typical pattern would be that cxl_guest_init_afu() takes an additional
> reference once it's done all its setup and can't fail.
>
> That way at the end of the loop when the loop construct has dropped all
> references, the nodes you actually init'ed have their reference count
> incremented by 1.


We don't keep a reference on the AFU device_node. Once we've read the 
config of the AFU, the AFU device_node is never accessed again. So I 
guess it's possible (though unexpected) that it's freed from under us, 
but it should not affect the driver.

The AFU is really dependent on the adapter itself, which is one level up 
in the device tree, and for which we create a device through 
of_platform_device_create(). The properties under the AFU device node 
are read directly from the PCI config space in the bare-metal case, 
where the cxl adapter is a PCI device.

Do we really have a problem here?

   Fred

Re: cxl: replace loop with for_each_child_of_node(), remove unneeded of_node_put()

[Michael Ellerman]

On Fri, 2016-29-07 at 03:55:34 UTC, Andrew Donnellan wrote:
> Rewrite the cxl_guest_init_afu() loop in cxl_of_probe() to use
> for_each_child_of_node() rather than a hand-coded for loop.
> 
> Remove the useless of_node_put(afu_np) call after the loop, where it's
> guaranteed that afu_np == NULL.
> 
> Reported-by: SF Markus Elfring <elfring@users.sourceforge.net>
> Reported-by: Julia Lawall <julia.lawall@lip6.fr>
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/735840b44bcc998e2574faf63d1aaa

cheers