staging: bcm2835-audio: refactor code

PATCH In review Aishwarya Pant 2 participants 4 messages Mar 6, 2017

Subsystems (3)

Staging Broadcom BCM2711/BCM2835 Arm architecture Broadcom BCM281XX/BCM11XXX/BCM216XX Arm architecture

lore

Share & tools

lore

Aishwarya Pant

author

2017-03-06 05:17 UTC #0 lore raw

PATCH 1/1 staging: bcm2835-audio: refactor code

This patch contains the following fixes:
-Merge two if conditions into one
-Remove initialization of unused semaphores alsa_stream->buffers_update_sem and alsa_stream->control_sem -Modify goto label err_free_mem to kfree vchi_instance if vchi_connect fails; add label err_init to add debug log and return error value (pre-existing functionality)

Signed-off-by Aishwarya Pant <[email protected]>

---
 drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c   | 8 ++------
 drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 9 ++++++---
 2 files changed, 8 insertions(+), 9 deletions(-)

drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c +2 -6

index d60d702..32b3793 100644
@@ -122,10 +122,8 @@ static int snd_bcm2835_playback_open_generic(
 	audio_info("Alsa open (%d)\n", substream->number);
 	idx = substream->number;
 
-	if (spdif && chip->opened) {
-		err = -EBUSY;
-		goto out;
-	} else if (!spdif && (chip->opened & (1 << idx))) {
+	if ((spdif && chip->opened) ||
+	    (!spdif && (chip->opened & (1 << idx))))	{
 		err = -EBUSY;
 		goto out;
 	}

@@ -156,8 +154,6 @@ static int snd_bcm2835_playback_open_generic(
 	alsa_stream->substream = substream;
 	alsa_stream->idx = idx;
 
-	sema_init(&alsa_stream->buffers_update_sem, 0);
-	sema_init(&alsa_stream->control_sem, 0);
 	spin_lock_init(&alsa_stream->lock);
 
 	err = bcm2835_audio_open(alsa_stream);

drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c +6 -3

index df1f3dd..557ba8e 100644
@@ -402,7 +402,7 @@ static int bcm2835_audio_open_connection(struct bcm2835_alsa_stream *alsa_stream
 		instance->alsa_stream = alsa_stream;
 		alsa_stream->instance = instance;
 		ret = 0; // xxx todo -1;
-		goto err_free_mem;
+		goto err_init;
 	}
 
 	/* Initialize and create a VCHI connection */

@@ -413,7 +413,7 @@ static int bcm2835_audio_open_connection(struct bcm2835_alsa_stream *alsa_stream
 				__func__, ret);
 
 			ret = -EIO;
-			goto err_free_mem;
+			goto err_init;
 		}
 		ret = vchi_connect(NULL, 0, vchi_instance);
 		if (ret) {

@@ -441,9 +441,12 @@ static int bcm2835_audio_open_connection(struct bcm2835_alsa_stream *alsa_stream
 
 	LOG_DBG(" success !\n");
 	ret = 0;
+err_init:
+	LOG_DBG(" .. OUT\n");
+	return ret;
 err_free_mem:
 	LOG_DBG(" .. OUT\n");
-
+	kfree(vchi_instance);
 	return ret;
 }

Signature · 1 line

2.7.4

Julia Lawall

2017-03-06 06:41 UTC #1 lore raw

Quoted 7 lines · Aishwarya Pant

This patch contains the following fixes:
(quoted diff) +0 -2
-Merge two if conditions into one
-Remove initialization of unused semaphores
alsa_stream->buffers_update_sem and alsa_stream->control_sem -Modify goto label err_free_mem to kfree vchi_instance if vchi_connect fails; add label err_init to add debug log and return error value (pre-existing functionality)

These changes appear to be completely unrelated. There should be three patches.

julia

Quoted 79 lines

Signed-off-by Aishwarya Pant <[email protected]>

---
 drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c   | 8 ++------
 drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 9 ++++++---
 2 files changed, 8 insertions(+), 9 deletions(-)

drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c +2 -6

index d60d702..32b3793 100644
@@ -122,10 +122,8 @@ static int snd_bcm2835_playback_open_generic(
 	audio_info("Alsa open (%d)\n", substream->number);
 	idx = substream->number;
 
-	if (spdif && chip->opened) {
-		err = -EBUSY;
-		goto out;
-	} else if (!spdif && (chip->opened & (1 << idx))) {
+	if ((spdif && chip->opened) ||
+	    (!spdif && (chip->opened & (1 << idx))))	{
 		err = -EBUSY;
 		goto out;
 	}

@@ -156,8 +154,6 @@ static int snd_bcm2835_playback_open_generic(
 	alsa_stream->substream = substream;
 	alsa_stream->idx = idx;
 
-	sema_init(&alsa_stream->buffers_update_sem, 0);
-	sema_init(&alsa_stream->control_sem, 0);
 	spin_lock_init(&alsa_stream->lock);
 
 	err = bcm2835_audio_open(alsa_stream);

drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c +6 -4

index df1f3dd..557ba8e 100644
@@ -402,7 +402,7 @@ static int bcm2835_audio_open_connection(struct bcm2835_alsa_stream *alsa_stream
 		instance->alsa_stream = alsa_stream;
 		alsa_stream->instance = instance;
 		ret = 0; // xxx todo -1;
-		goto err_free_mem;
+		goto err_init;
 	}
 
 	/* Initialize and create a VCHI connection */

@@ -413,7 +413,7 @@ static int bcm2835_audio_open_connection(struct bcm2835_alsa_stream *alsa_stream
 				__func__, ret);
 
 			ret = -EIO;
-			goto err_free_mem;
+			goto err_init;
 		}
 		ret = vchi_connect(NULL, 0, vchi_instance);
 		if (ret) {

@@ -441,9 +441,12 @@ static int bcm2835_audio_open_connection(struct bcm2835_alsa_stream *alsa_stream
 
 	LOG_DBG(" success !\n");
 	ret = 0;
+err_init:
+	LOG_DBG(" .. OUT\n");
+	return ret;
 err_free_mem:
 	LOG_DBG(" .. OUT\n");
-
+	kfree(vchi_instance);
 	return ret;
 }
 
--

2.7.4

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170306051742.GA21192%40aishwarya.
For more options, visit https://groups.google.com/d/optout.

Aishwarya Pant

author

2017-03-06 07:42 UTC #2 lore raw

Quoted 4 lines · Julia Lawall

On Mon, 6 Mar 2017, Aishwarya Pant wrote:

Quoted 10 lines

Quoted 7 lines
This patch contains the following fixes:
(quoted diff) +0 -2
-Merge two if conditions into one
-Remove initialization of unused semaphores
alsa_stream->buffers_update_sem and alsa_stream->control_sem -Modify goto label err_free_mem to kfree vchi_instance if vchi_connect fails; add label err_init to add debug log and return error value (pre-existing functionality)
These changes appear to be completely unrelated. There should be three patches.

These changes don't build upon each other, so they should be sent as three patches (and not a patchset) right?

Quoted 82 lines

julia

Quoted 79 lines

Signed-off-by Aishwarya Pant <[email protected]>

---
 drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c   | 8 ++------
 drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 9 ++++++---
 2 files changed, 8 insertions(+), 9 deletions(-)

drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c +2 -6

index d60d702..32b3793 100644
@@ -122,10 +122,8 @@ static int snd_bcm2835_playback_open_generic(
 	audio_info("Alsa open (%d)\n", substream->number);
 	idx = substream->number;
 
-	if (spdif && chip->opened) {
-		err = -EBUSY;
-		goto out;
-	} else if (!spdif && (chip->opened & (1 << idx))) {
+	if ((spdif && chip->opened) ||
+	    (!spdif && (chip->opened & (1 << idx))))	{
 		err = -EBUSY;
 		goto out;
 	}

@@ -156,8 +154,6 @@ static int snd_bcm2835_playback_open_generic(
 	alsa_stream->substream = substream;
 	alsa_stream->idx = idx;
 
-	sema_init(&alsa_stream->buffers_update_sem, 0);
-	sema_init(&alsa_stream->control_sem, 0);
 	spin_lock_init(&alsa_stream->lock);
 
 	err = bcm2835_audio_open(alsa_stream);

drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c +6 -4

index df1f3dd..557ba8e 100644
@@ -402,7 +402,7 @@ static int bcm2835_audio_open_connection(struct bcm2835_alsa_stream *alsa_stream
 		instance->alsa_stream = alsa_stream;
 		alsa_stream->instance = instance;
 		ret = 0; // xxx todo -1;
-		goto err_free_mem;
+		goto err_init;
 	}
 
 	/* Initialize and create a VCHI connection */

@@ -413,7 +413,7 @@ static int bcm2835_audio_open_connection(struct bcm2835_alsa_stream *alsa_stream
 				__func__, ret);
 
 			ret = -EIO;
-			goto err_free_mem;
+			goto err_init;
 		}
 		ret = vchi_connect(NULL, 0, vchi_instance);
 		if (ret) {

@@ -441,9 +441,12 @@ static int bcm2835_audio_open_connection(struct bcm2835_alsa_stream *alsa_stream
 
 	LOG_DBG(" success !\n");
 	ret = 0;
+err_init:
+	LOG_DBG(" .. OUT\n");
+	return ret;
 err_free_mem:
 	LOG_DBG(" .. OUT\n");
-
+	kfree(vchi_instance);
 	return ret;
 }
 
--

2.7.4

Julia Lawall

2017-03-06 11:45 UTC #3 lore raw

Quoted 16 lines · Aishwarya Pant

Quoted 4 lines · Julia Lawall
On Mon, 6 Mar 2017, Aishwarya Pant wrote:
Quoted 10 lines
Quoted 7 lines
This patch contains the following fixes:
(quoted diff) +0 -2
-Merge two if conditions into one
-Remove initialization of unused semaphores
alsa_stream->buffers_update_sem and alsa_stream->control_sem -Modify goto label err_free_mem to kfree vchi_instance if vchi_connect fails; add label err_init to add debug log and return error value (pre-existing functionality)
These changes appear to be completely unrelated. There should be three patches.

Quoted 2 lines

These changes don't build upon each other, so they should be sent as three patches (and not a patchset) right?

For changes in the same file, Greg will prefer to have a series. Then there is no ambiguity about the order in which they should be applied. For changes in separate files, just sending them separately is better.

julia

Quoted 91 lines

Quoted 76 lines
julia
Quoted 72 lines
Signed-off-by Aishwarya Pant <[email protected]>
---
 drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c   | 8 ++------
 drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 9 ++++++---
 2 files changed, 8 insertions(+), 9 deletions(-)
drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c +2 -6
index d60d702..32b3793 100644
@@ -122,10 +122,8 @@ static int snd_bcm2835_playback_open_generic(
 	audio_info("Alsa open (%d)\n", substream->number);
 	idx = substream->number;
 
-	if (spdif && chip->opened) {
-		err = -EBUSY;
-		goto out;
-	} else if (!spdif && (chip->opened & (1 << idx))) {
+	if ((spdif && chip->opened) ||
+	    (!spdif && (chip->opened & (1 << idx))))	{
 		err = -EBUSY;
 		goto out;
 	}
@@ -156,8 +154,6 @@ static int snd_bcm2835_playback_open_generic(
 	alsa_stream->substream = substream;
 	alsa_stream->idx = idx;
 
-	sema_init(&alsa_stream->buffers_update_sem, 0);
-	sema_init(&alsa_stream->control_sem, 0);
 	spin_lock_init(&alsa_stream->lock);
 
 	err = bcm2835_audio_open(alsa_stream);
drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c +6 -4
index df1f3dd..557ba8e 100644
@@ -402,7 +402,7 @@ static int bcm2835_audio_open_connection(struct bcm2835_alsa_stream *alsa_stream
 		instance->alsa_stream = alsa_stream;
 		alsa_stream->instance = instance;
 		ret = 0; // xxx todo -1;
-		goto err_free_mem;
+		goto err_init;
 	}
 
 	/* Initialize and create a VCHI connection */
@@ -413,7 +413,7 @@ static int bcm2835_audio_open_connection(struct bcm2835_alsa_stream *alsa_stream
 				__func__, ret);
 
 			ret = -EIO;
-			goto err_free_mem;
+			goto err_init;
 		}
 		ret = vchi_connect(NULL, 0, vchi_instance);
 		if (ret) {
@@ -441,9 +441,12 @@ static int bcm2835_audio_open_connection(struct bcm2835_alsa_stream *alsa_stream
 
 	LOG_DBG(" success !\n");
 	ret = 0;
+err_init:
+	LOG_DBG(" .. OUT\n");
+	return ret;
 err_free_mem:
 	LOG_DBG(" .. OUT\n");
-
+	kfree(vchi_instance);
 	return ret;
 }
 
--
2.7.4
Quoted 7 lines
Quoted 7 lines
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170306051742.GA21192%40aishwarya.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170306074212.GA19258%40aishwarya.
For more options, visit https://groups.google.com/d/optout.