[PATCH v1 0/2] fix race condition in rsnd_ssi_pointer_update

[PATCH v1 0/2] fix race condition in rsnd_ssi_pointer_update

[jiada_wang]

From: Jiada Wang <jiada_wang@mentor.com>

This patch set aims to fix the race condition in rsnd_ssi_pointer_update,
between set of byte_pos and wrap it around when new buffer starts.
 
Jiada Wang (2):
  ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update
  ASoC: rsnd: ssi: remove unnesessary period_pos

 sound/soc/sh/rcar/ssi.c |   25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

-- 
1.7.9.5

[PATCH v1 0/2] fix race condition in rsnd_ssi_pointer_update

[jiada_wang]

From: Jiada Wang <jiada_wang@mentor.com>

This patch set aims to fix the race condition in rsnd_ssi_pointer_update,
between set of byte_pos and wrap it around when new buffer starts.
 
Jiada Wang (2):
  ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update
  ASoC: rsnd: ssi: remove unnesessary period_pos

 sound/soc/sh/rcar/ssi.c |   25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

-- 
1.7.9.5

[PATCH v1 1/2] ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update

[jiada_wang]

From: Jiada Wang <jiada_wang@mentor.com>

Currently there is race condition between set of byte_pos and wrap
it around when new buffer starts. If .pointer is called in-between
it will result in inconsistent pointer position be returned
from .pointer callback.

This patch increments buffer pointer atomically to avoid this issue.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Reviewed-by: Takashi Sakamoto <takashi.sakamoto@miraclelinux.com>
---
 sound/soc/sh/rcar/ssi.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index fece1e5f..cbf3bf3 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -446,25 +446,29 @@ static bool rsnd_ssi_pointer_update(struct rsnd_mod *mod,
 				    int byte)
 {
 	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
+	bool ret = false;
+	int byte_pos;
 
-	ssi->byte_pos += byte;
+	byte_pos = ssi->byte_pos + byte;
 
-	if (ssi->byte_pos >= ssi->next_period_byte) {
+	if (byte_pos >= ssi->next_period_byte) {
 		struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
 
 		ssi->period_pos++;
 		ssi->next_period_byte += ssi->byte_per_period;
 
 		if (ssi->period_pos >= runtime->periods) {
-			ssi->byte_pos = 0;
+			byte_pos = 0;
 			ssi->period_pos = 0;
 			ssi->next_period_byte = ssi->byte_per_period;
 		}
 
-		return true;
+		ret = true;
 	}
 
-	return false;
+	WRITE_ONCE(ssi->byte_pos, byte_pos);
+
+	return ret;
 }
 
 /*
@@ -838,7 +842,7 @@ static int rsnd_ssi_pointer(struct rsnd_mod *mod,
 	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
 	struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
 
-	*pointer = bytes_to_frames(runtime, ssi->byte_pos);
+	*pointer = bytes_to_frames(runtime, READ_ONCE(ssi->byte_pos));
 
 	return 0;
 }
-- 
1.7.9.5

[PATCH v1 1/2] ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update

[jiada_wang]

From: Jiada Wang <jiada_wang@mentor.com>

Currently there is race condition between set of byte_pos and wrap
it around when new buffer starts. If .pointer is called in-between
it will result in inconsistent pointer position be returned
from .pointer callback.

This patch increments buffer pointer atomically to avoid this issue.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Reviewed-by: Takashi Sakamoto <takashi.sakamoto@miraclelinux.com>
---
 sound/soc/sh/rcar/ssi.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index fece1e5f..cbf3bf3 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -446,25 +446,29 @@ static bool rsnd_ssi_pointer_update(struct rsnd_mod *mod,
 				    int byte)
 {
 	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
+	bool ret = false;
+	int byte_pos;
 
-	ssi->byte_pos += byte;
+	byte_pos = ssi->byte_pos + byte;
 
-	if (ssi->byte_pos >= ssi->next_period_byte) {
+	if (byte_pos >= ssi->next_period_byte) {
 		struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
 
 		ssi->period_pos++;
 		ssi->next_period_byte += ssi->byte_per_period;
 
 		if (ssi->period_pos >= runtime->periods) {
-			ssi->byte_pos = 0;
+			byte_pos = 0;
 			ssi->period_pos = 0;
 			ssi->next_period_byte = ssi->byte_per_period;
 		}
 
-		return true;
+		ret = true;
 	}
 
-	return false;
+	WRITE_ONCE(ssi->byte_pos, byte_pos);
+
+	return ret;
 }
 
 /*
@@ -838,7 +842,7 @@ static int rsnd_ssi_pointer(struct rsnd_mod *mod,
 	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
 	struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
 
-	*pointer = bytes_to_frames(runtime, ssi->byte_pos);
+	*pointer = bytes_to_frames(runtime, READ_ONCE(ssi->byte_pos));
 
 	return 0;
 }
-- 
1.7.9.5

Re: [PATCH v1 1/2] ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update

[Kuninori Morimoto]

Hi Jiada

Thank you for your patch

> Currently there is race condition between set of byte_pos and wrap
> it around when new buffer starts. If .pointer is called in-between
> it will result in inconsistent pointer position be returned
> from .pointer callback.
> 
> This patch increments buffer pointer atomically to avoid this issue.
> 
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> Reviewed-by: Takashi Sakamoto <takashi.sakamoto@miraclelinux.com>
> ---

You using playback with PIO mode ?
Because this function is no longer used on DMA mode

Best regards
---
Kuninori Morimoto

Re: [PATCH v1 1/2] ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update

[Kuninori Morimoto]

Hi Jiada

Thank you for your patch

> Currently there is race condition between set of byte_pos and wrap
> it around when new buffer starts. If .pointer is called in-between
> it will result in inconsistent pointer position be returned
> from .pointer callback.
> 
> This patch increments buffer pointer atomically to avoid this issue.
> 
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> Reviewed-by: Takashi Sakamoto <takashi.sakamoto@miraclelinux.com>
> ---

You using playback with PIO mode ?
Because this function is no longer used on DMA mode

Best regards
---
Kuninori Morimoto

Re: [PATCH v1 1/2] ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update

[Jiada Wang]

Hello Morimoto-san

On 12/07/2017 01:45 AM, Kuninori Morimoto wrote:
> Hi Jiada
>
> Thank you for your patch
>
>> Currently there is race condition between set of byte_pos and wrap
>> it around when new buffer starts. If .pointer is called in-between
>> it will result in inconsistent pointer position be returned
>> from .pointer callback.
>>
>> This patch increments buffer pointer atomically to avoid this issue.
>>
>> Signed-off-by: Jiada Wang<jiada_wang@mentor.com>
>> Reviewed-by: Takashi Sakamoto<takashi.sakamoto@miraclelinux.com>
>> ---
> You using playback with PIO mode ?
> Because this function is no longer used on DMA mode
No, we are using rcar sound in DMA mode,
our original patch resolves the issue in core.c for both PIO & DMA mode.

but with your commit a97a06c ("ASoC: rsnd: cleanup pointer related code"),
DMA mode no longer has the race condition issue,
so I ported our fix patch to only address the issue in PIO mode

Thanks,
Jiada
> Best regards
> ---
> Kuninori Morimoto

Re: [PATCH v1 1/2] ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update

[Jiada Wang]

Hello Morimoto-san

On 12/07/2017 01:45 AM, Kuninori Morimoto wrote:
> Hi Jiada
>
> Thank you for your patch
>
>> Currently there is race condition between set of byte_pos and wrap
>> it around when new buffer starts. If .pointer is called in-between
>> it will result in inconsistent pointer position be returned
>> from .pointer callback.
>>
>> This patch increments buffer pointer atomically to avoid this issue.
>>
>> Signed-off-by: Jiada Wang<jiada_wang@mentor.com>
>> Reviewed-by: Takashi Sakamoto<takashi.sakamoto@miraclelinux.com>
>> ---
> You using playback with PIO mode ?
> Because this function is no longer used on DMA mode
No, we are using rcar sound in DMA mode,
our original patch resolves the issue in core.c for both PIO & DMA mode.

but with your commit a97a06c ("ASoC: rsnd: cleanup pointer related code"),
DMA mode no longer has the race condition issue,
so I ported our fix patch to only address the issue in PIO mode

Thanks,
Jiada
> Best regards
> ---
> Kuninori Morimoto

Re: [PATCH v1 1/2] ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update

[Kuninori Morimoto]

Hi Jiada

> >> Currently there is race condition between set of byte_pos and wrap
> >> it around when new buffer starts. If .pointer is called in-between
> >> it will result in inconsistent pointer position be returned
> >> from .pointer callback.
> >>
> >> This patch increments buffer pointer atomically to avoid this issue.
> >>
> >> Signed-off-by: Jiada Wang<jiada_wang@mentor.com>
> >> Reviewed-by: Takashi Sakamoto<takashi.sakamoto@miraclelinux.com>
> >> ---
> > You using playback with PIO mode ?
> > Because this function is no longer used on DMA mode
> No, we are using rcar sound in DMA mode,
> our original patch resolves the issue in core.c for both PIO & DMA mode.
> 
> but with your commit a97a06c ("ASoC: rsnd: cleanup pointer related code"),
> DMA mode no longer has the race condition issue,
> so I ported our fix patch to only address the issue in PIO mode

Thanks. Nice to know.

Best regards
---
Kuninori Morimoto

Re: [PATCH v1 1/2] ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update

[Kuninori Morimoto]

Hi Jiada

> >> Currently there is race condition between set of byte_pos and wrap
> >> it around when new buffer starts. If .pointer is called in-between
> >> it will result in inconsistent pointer position be returned
> >> from .pointer callback.
> >>
> >> This patch increments buffer pointer atomically to avoid this issue.
> >>
> >> Signed-off-by: Jiada Wang<jiada_wang@mentor.com>
> >> Reviewed-by: Takashi Sakamoto<takashi.sakamoto@miraclelinux.com>
> >> ---
> > You using playback with PIO mode ?
> > Because this function is no longer used on DMA mode
> No, we are using rcar sound in DMA mode,
> our original patch resolves the issue in core.c for both PIO & DMA mode.
> 
> but with your commit a97a06c ("ASoC: rsnd: cleanup pointer related code"),
> DMA mode no longer has the race condition issue,
> so I ported our fix patch to only address the issue in PIO mode

Thanks. Nice to know.

Best regards
---
Kuninori Morimoto

Applied "ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update" to the asoc tree

[Mark Brown]

The patch

   ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 33f801366bdf3f8b67dfe325b84f4051a090d01e Mon Sep 17 00:00:00 2001
From: Jiada Wang <jiada_wang@mentor.com>
Date: Thu, 7 Dec 2017 22:15:38 -0800
Subject: [PATCH] ASoC: rsnd: ssi: fix race condition in
 rsnd_ssi_pointer_update

Currently there is race condition between set of byte_pos and wrap
it around when new buffer starts. If .pointer is called in-between
it will result in inconsistent pointer position be returned
from .pointer callback.

This patch increments buffer pointer atomically to avoid this issue.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Reviewed-by: Takashi Sakamoto <takashi.sakamoto@miraclelinux.com>
Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sh/rcar/ssi.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index fece1e5f582f..cbf3bf312d23 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -446,25 +446,29 @@ static bool rsnd_ssi_pointer_update(struct rsnd_mod *mod,
 				    int byte)
 {
 	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
+	bool ret = false;
+	int byte_pos;
 
-	ssi->byte_pos += byte;
+	byte_pos = ssi->byte_pos + byte;
 
-	if (ssi->byte_pos >= ssi->next_period_byte) {
+	if (byte_pos >= ssi->next_period_byte) {
 		struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
 
 		ssi->period_pos++;
 		ssi->next_period_byte += ssi->byte_per_period;
 
 		if (ssi->period_pos >= runtime->periods) {
-			ssi->byte_pos = 0;
+			byte_pos = 0;
 			ssi->period_pos = 0;
 			ssi->next_period_byte = ssi->byte_per_period;
 		}
 
-		return true;
+		ret = true;
 	}
 
-	return false;
+	WRITE_ONCE(ssi->byte_pos, byte_pos);
+
+	return ret;
 }
 
 /*
@@ -838,7 +842,7 @@ static int rsnd_ssi_pointer(struct rsnd_mod *mod,
 	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
 	struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
 
-	*pointer = bytes_to_frames(runtime, ssi->byte_pos);
+	*pointer = bytes_to_frames(runtime, READ_ONCE(ssi->byte_pos));
 
 	return 0;
 }
-- 
2.15.1

Applied "ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update" to the asoc tree

[Mark Brown]

The patch

   ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 33f801366bdf3f8b67dfe325b84f4051a090d01e Mon Sep 17 00:00:00 2001
From: Jiada Wang <jiada_wang@mentor.com>
Date: Thu, 7 Dec 2017 22:15:38 -0800
Subject: [PATCH] ASoC: rsnd: ssi: fix race condition in
 rsnd_ssi_pointer_update

Currently there is race condition between set of byte_pos and wrap
it around when new buffer starts. If .pointer is called in-between
it will result in inconsistent pointer position be returned
from .pointer callback.

This patch increments buffer pointer atomically to avoid this issue.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Reviewed-by: Takashi Sakamoto <takashi.sakamoto@miraclelinux.com>
Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sh/rcar/ssi.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index fece1e5f582f..cbf3bf312d23 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -446,25 +446,29 @@ static bool rsnd_ssi_pointer_update(struct rsnd_mod *mod,
 				    int byte)
 {
 	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
+	bool ret = false;
+	int byte_pos;
 
-	ssi->byte_pos += byte;
+	byte_pos = ssi->byte_pos + byte;
 
-	if (ssi->byte_pos >= ssi->next_period_byte) {
+	if (byte_pos >= ssi->next_period_byte) {
 		struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
 
 		ssi->period_pos++;
 		ssi->next_period_byte += ssi->byte_per_period;
 
 		if (ssi->period_pos >= runtime->periods) {
-			ssi->byte_pos = 0;
+			byte_pos = 0;
 			ssi->period_pos = 0;
 			ssi->next_period_byte = ssi->byte_per_period;
 		}
 
-		return true;
+		ret = true;
 	}
 
-	return false;
+	WRITE_ONCE(ssi->byte_pos, byte_pos);
+
+	return ret;
 }
 
 /*
@@ -838,7 +842,7 @@ static int rsnd_ssi_pointer(struct rsnd_mod *mod,
 	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
 	struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
 
-	*pointer = bytes_to_frames(runtime, ssi->byte_pos);
+	*pointer = bytes_to_frames(runtime, READ_ONCE(ssi->byte_pos));
 
 	return 0;
 }
-- 
2.15.1

Re: Applied "ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update" to the asoc tree

[Takashi Sakamoto]

Hi Mark,

On Dec 9 2017 03:52, Mark Brown wrote:
> The patch
> 
>     ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update
> 
> has been applied to the asoc tree at
> 
>     https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

The applied patches are in v1 patchset, but we have v2 patchset already:
http://mailman.alsa-project.org/pipermail/alsa-devel/2017-December/128970.html

Would you please replace the applied patches with the renewed ones? 
There're slight differences between them.

> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.
> 
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
> 
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
> 
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
> 
> Thanks,
> Mark
> 
>>From 33f801366bdf3f8b67dfe325b84f4051a090d01e Mon Sep 17 00:00:00 2001
> From: Jiada Wang <jiada_wang@mentor.com>
> Date: Thu, 7 Dec 2017 22:15:38 -0800
> Subject: [PATCH] ASoC: rsnd: ssi: fix race condition in
>   rsnd_ssi_pointer_update
> 
> Currently there is race condition between set of byte_pos and wrap
> it around when new buffer starts. If .pointer is called in-between
> it will result in inconsistent pointer position be returned
> from .pointer callback.
> 
> This patch increments buffer pointer atomically to avoid this issue.
> 
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> Reviewed-by: Takashi Sakamoto <takashi.sakamoto@miraclelinux.com>
> Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   sound/soc/sh/rcar/ssi.c | 16 ++++++++++------
>   1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
> index fece1e5f582f..cbf3bf312d23 100644
> --- a/sound/soc/sh/rcar/ssi.c
> +++ b/sound/soc/sh/rcar/ssi.c
> @@ -446,25 +446,29 @@ static bool rsnd_ssi_pointer_update(struct rsnd_mod *mod,
>   				    int byte)
>   {
>   	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
> +	bool ret = false;
> +	int byte_pos;
>   
> -	ssi->byte_pos += byte;
> +	byte_pos = ssi->byte_pos + byte;
>   
> -	if (ssi->byte_pos >= ssi->next_period_byte) {
> +	if (byte_pos >= ssi->next_period_byte) {
>   		struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
>   
>   		ssi->period_pos++;
>   		ssi->next_period_byte += ssi->byte_per_period;
>   
>   		if (ssi->period_pos >= runtime->periods) {
> -			ssi->byte_pos = 0;
> +			byte_pos = 0;
>   			ssi->period_pos = 0;
>   			ssi->next_period_byte = ssi->byte_per_period;
>   		}
>   
> -		return true;
> +		ret = true;
>   	}
>   
> -	return false;
> +	WRITE_ONCE(ssi->byte_pos, byte_pos);
> +
> +	return ret;
>   }
>   
>   /*
> @@ -838,7 +842,7 @@ static int rsnd_ssi_pointer(struct rsnd_mod *mod,
>   	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
>   	struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
>   
> -	*pointer = bytes_to_frames(runtime, ssi->byte_pos);
> +	*pointer = bytes_to_frames(runtime, READ_ONCE(ssi->byte_pos));
>   
>   	return 0;
>   }


Thanks

Takashi Sakamoto

Re: Applied "ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update" to the asoc tree

[Takashi Sakamoto]

Hi Mark,

On Dec 9 2017 03:52, Mark Brown wrote:
> The patch
> 
>     ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update
> 
> has been applied to the asoc tree at
> 
>     https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

The applied patches are in v1 patchset, but we have v2 patchset already:
http://mailman.alsa-project.org/pipermail/alsa-devel/2017-December/128970.html

Would you please replace the applied patches with the renewed ones? 
There're slight differences between them.

> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.
> 
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
> 
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
> 
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
> 
> Thanks,
> Mark
> 
>>From 33f801366bdf3f8b67dfe325b84f4051a090d01e Mon Sep 17 00:00:00 2001
> From: Jiada Wang <jiada_wang@mentor.com>
> Date: Thu, 7 Dec 2017 22:15:38 -0800
> Subject: [PATCH] ASoC: rsnd: ssi: fix race condition in
>   rsnd_ssi_pointer_update
> 
> Currently there is race condition between set of byte_pos and wrap
> it around when new buffer starts. If .pointer is called in-between
> it will result in inconsistent pointer position be returned
> from .pointer callback.
> 
> This patch increments buffer pointer atomically to avoid this issue.
> 
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> Reviewed-by: Takashi Sakamoto <takashi.sakamoto@miraclelinux.com>
> Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   sound/soc/sh/rcar/ssi.c | 16 ++++++++++------
>   1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
> index fece1e5f582f..cbf3bf312d23 100644
> --- a/sound/soc/sh/rcar/ssi.c
> +++ b/sound/soc/sh/rcar/ssi.c
> @@ -446,25 +446,29 @@ static bool rsnd_ssi_pointer_update(struct rsnd_mod *mod,
>   				    int byte)
>   {
>   	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
> +	bool ret = false;
> +	int byte_pos;
>   
> -	ssi->byte_pos += byte;
> +	byte_pos = ssi->byte_pos + byte;
>   
> -	if (ssi->byte_pos >= ssi->next_period_byte) {
> +	if (byte_pos >= ssi->next_period_byte) {
>   		struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
>   
>   		ssi->period_pos++;
>   		ssi->next_period_byte += ssi->byte_per_period;
>   
>   		if (ssi->period_pos >= runtime->periods) {
> -			ssi->byte_pos = 0;
> +			byte_pos = 0;
>   			ssi->period_pos = 0;
>   			ssi->next_period_byte = ssi->byte_per_period;
>   		}
>   
> -		return true;
> +		ret = true;
>   	}
>   
> -	return false;
> +	WRITE_ONCE(ssi->byte_pos, byte_pos);
> +
> +	return ret;
>   }
>   
>   /*
> @@ -838,7 +842,7 @@ static int rsnd_ssi_pointer(struct rsnd_mod *mod,
>   	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
>   	struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
>   
> -	*pointer = bytes_to_frames(runtime, ssi->byte_pos);
> +	*pointer = bytes_to_frames(runtime, READ_ONCE(ssi->byte_pos));
>   
>   	return 0;
>   }


Thanks

Takashi Sakamoto

Re: Applied "ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update" to the asoc tree

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 423 bytes --]

On Sat, Dec 09, 2017 at 02:22:50PM +0900, Takashi Sakamoto wrote:
> On Dec 9 2017 03:52, Mark Brown wrote:

> The applied patches are in v1 patchset, but we have v2 patchset already:
> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-December/128970.html

> Would you please replace the applied patches with the renewed ones? There're
> slight differences between them.

As far as I can tell what's applied is v2?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Applied "ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update" to the asoc tree

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 423 bytes --]

On Sat, Dec 09, 2017 at 02:22:50PM +0900, Takashi Sakamoto wrote:
> On Dec 9 2017 03:52, Mark Brown wrote:

> The applied patches are in v1 patchset, but we have v2 patchset already:
> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-December/128970.html

> Would you please replace the applied patches with the renewed ones? There're
> slight differences between them.

As far as I can tell what's applied is v2?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Applied "ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update" to the asoc tree

[Jiada Wang]

Hi Mark

Only the first patch in
https://www.spinics.net/lists/alsa-devel/msg71021.html
was applied, but the second patch "ASoC: rsnd: ssi: remove unnesessary 
period_pos"
was missed

Could you please have a look


Thanks,
Jiada

On 12/11/2017 03:38 AM, Mark Brown wrote:
> On Sat, Dec 09, 2017 at 02:22:50PM +0900, Takashi Sakamoto wrote:
>> On Dec 9 2017 03:52, Mark Brown wrote:
>> The applied patches are in v1 patchset, but we have v2 patchset already:
>> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-December/128970.html
>> Would you please replace the applied patches with the renewed ones? There're
>> slight differences between them.
> As far as I can tell what's applied is v2?

Re: Applied "ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update" to the asoc tree

[Jiada Wang]

Hi Mark

Only the first patch in
https://www.spinics.net/lists/alsa-devel/msg71021.html
was applied, but the second patch "ASoC: rsnd: ssi: remove unnesessary 
period_pos"
was missed

Could you please have a look


Thanks,
Jiada

On 12/11/2017 03:38 AM, Mark Brown wrote:
> On Sat, Dec 09, 2017 at 02:22:50PM +0900, Takashi Sakamoto wrote:
>> On Dec 9 2017 03:52, Mark Brown wrote:
>> The applied patches are in v1 patchset, but we have v2 patchset already:
>> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-December/128970.html
>> Would you please replace the applied patches with the renewed ones? There're
>> slight differences between them.
> As far as I can tell what's applied is v2?

Re: Applied "ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update" to the asoc tree

[Takashi Sakamoto]

Hi Jiada,

On Jan 9 2018 14:42, Jiada Wang wrote:
> Only the first patch in
> https://www.spinics.net/lists/alsa-devel/msg71021.html
> was applied, but the second patch "ASoC: rsnd: ssi: remove unnesessary 
> period_pos"
> was missed
> 
> Could you please have a look

I can see all of them were applied to Mark's for-next branch[1].

  - 33f801366bdf ('ASoC: rsnd: ssi: fix race condition in 
rsnd_ssi_pointer_update')[2]
  - 2e2d53da81af ('ASoC: rsnd: ssi: remove unnesessary period_pos')[3]

No worries to us.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/log/?h=for-next
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/sound/soc/sh?h=for-next&id=33f801366bdf3f8b67dfe325b84f4051a090d01e
[3] 
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/sound/soc/sh?h=for-next&id=2e2d53da81af6b2222c6b4e025a5d01b37b4449b


Regards

Takashi Sakamoto

Re: Applied "ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update" to the asoc tree

[Jiada Wang]

Hi Sakamoto-san


On 01/08/2018 10:53 PM, Takashi Sakamoto wrote:
> Hi Jiada,
>
> On Jan 9 2018 14:42, Jiada Wang wrote:
>> Only the first patch in
>> https://www.spinics.net/lists/alsa-devel/msg71021.html
>> was applied, but the second patch "ASoC: rsnd: ssi: remove 
>> unnesessary period_pos"
>> was missed
>>
>> Could you please have a look
>
> I can see all of them were applied to Mark's for-next branch[1].
>
>  - 33f801366bdf ('ASoC: rsnd: ssi: fix race condition in 
> rsnd_ssi_pointer_update')[2]
>  - 2e2d53da81af ('ASoC: rsnd: ssi: remove unnesessary period_pos')[3]
>
> No worries to us.

Thanks,

Jiada
>
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/log/?h=for-next
> [2] 
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/sound/soc/sh?h=for-next&id=33f801366bdf3f8b67dfe325b84f4051a090d01e
> [3] 
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/sound/soc/sh?h=for-next&id=2e2d53da81af6b2222c6b4e025a5d01b37b4449b
>
>
> Regards
>
> Takashi Sakamoto

Re: Applied "ASoC: rsnd: ssi: fix race condition in rsnd_ssi_pointer_update" to the asoc tree

[Jiada Wang]

Hi Sakamoto-san


On 01/08/2018 10:53 PM, Takashi Sakamoto wrote:
> Hi Jiada,
>
> On Jan 9 2018 14:42, Jiada Wang wrote:
>> Only the first patch in
>> https://www.spinics.net/lists/alsa-devel/msg71021.html
>> was applied, but the second patch "ASoC: rsnd: ssi: remove 
>> unnesessary period_pos"
>> was missed
>>
>> Could you please have a look
>
> I can see all of them were applied to Mark's for-next branch[1].
>
>  - 33f801366bdf ('ASoC: rsnd: ssi: fix race condition in 
> rsnd_ssi_pointer_update')[2]
>  - 2e2d53da81af ('ASoC: rsnd: ssi: remove unnesessary period_pos')[3]
>
> No worries to us.

Thanks,

Jiada
>
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/log/?h=for-next
> [2] 
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/sound/soc/sh?h=for-next&id=33f801366bdf3f8b67dfe325b84f4051a090d01e
> [3] 
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/sound/soc/sh?h=for-next&id=2e2d53da81af6b2222c6b4e025a5d01b37b4449b
>
>
> Regards
>
> Takashi Sakamoto

[PATCH v2 2/2] ASoC: rsnd: ssi: remove unnesessary period_pos

[jiada_wang]

From: Jiada Wang <jiada_wang@mentor.com>

period_pos can always be calculated by byte_pos and
byte_per_period, there is no reason to maintain this
variable in rsnd_dai_stream.
Further more, if the passed 'byte' amount to
rsnd_ssi_pointer_update() is more than byte_per_period.
the calculation of next_period_byte isn't correct.

This patch removes period_pos from rsnd_ssi and calculates
next_period_byte with consideration of actual byte_pos value.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 sound/soc/sh/rcar/ssi.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index cbf3bf3..f212024 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -80,7 +80,6 @@ struct rsnd_ssi {
 	unsigned int usrcnt;
 
 	int byte_pos;
-	int period_pos;
 	int byte_per_period;
 	int next_period_byte;
 };
@@ -421,7 +420,6 @@ static void rsnd_ssi_pointer_init(struct rsnd_mod *mod,
 	struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
 
 	ssi->byte_pos		= 0;
-	ssi->period_pos		= 0;
 	ssi->byte_per_period	= runtime->period_size *
 				  runtime->channels *
 				  samples_to_bytes(runtime, 1);
@@ -453,13 +451,12 @@ static bool rsnd_ssi_pointer_update(struct rsnd_mod *mod,
 
 	if (byte_pos >= ssi->next_period_byte) {
 		struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
+		int period_pos = byte_pos / ssi->byte_per_period;
 
-		ssi->period_pos++;
-		ssi->next_period_byte += ssi->byte_per_period;
+		ssi->next_period_byte = (period_pos + 1) * ssi->byte_per_period;
 
-		if (ssi->period_pos >= runtime->periods) {
+		if (period_pos >= runtime->periods) {
 			byte_pos = 0;
-			ssi->period_pos = 0;
 			ssi->next_period_byte = ssi->byte_per_period;
 		}
 
-- 
1.7.9.5

[PATCH v2 2/2] ASoC: rsnd: ssi: remove unnesessary period_pos

[jiada_wang]

From: Jiada Wang <jiada_wang@mentor.com>

period_pos can always be calculated by byte_pos and
byte_per_period, there is no reason to maintain this
variable in rsnd_dai_stream.
Further more, if the passed 'byte' amount to
rsnd_ssi_pointer_update() is more than byte_per_period.
the calculation of next_period_byte isn't correct.

This patch removes period_pos from rsnd_ssi and calculates
next_period_byte with consideration of actual byte_pos value.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 sound/soc/sh/rcar/ssi.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index cbf3bf3..f212024 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -80,7 +80,6 @@ struct rsnd_ssi {
 	unsigned int usrcnt;
 
 	int byte_pos;
-	int period_pos;
 	int byte_per_period;
 	int next_period_byte;
 };
@@ -421,7 +420,6 @@ static void rsnd_ssi_pointer_init(struct rsnd_mod *mod,
 	struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
 
 	ssi->byte_pos		= 0;
-	ssi->period_pos		= 0;
 	ssi->byte_per_period	= runtime->period_size *
 				  runtime->channels *
 				  samples_to_bytes(runtime, 1);
@@ -453,13 +451,12 @@ static bool rsnd_ssi_pointer_update(struct rsnd_mod *mod,
 
 	if (byte_pos >= ssi->next_period_byte) {
 		struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
+		int period_pos = byte_pos / ssi->byte_per_period;
 
-		ssi->period_pos++;
-		ssi->next_period_byte += ssi->byte_per_period;
+		ssi->next_period_byte = (period_pos + 1) * ssi->byte_per_period;
 
-		if (ssi->period_pos >= runtime->periods) {
+		if (period_pos >= runtime->periods) {
 			byte_pos = 0;
-			ssi->period_pos = 0;
 			ssi->next_period_byte = ssi->byte_per_period;
 		}
 
-- 
1.7.9.5

Re: [PATCH v2 2/2] ASoC: rsnd: ssi: remove unnesessary period_pos

[Kuninori Morimoto]

Hi Jiada

> Further more, if the passed 'byte' amount to
> rsnd_ssi_pointer_update() is more than byte_per_period.
> the calculation of next_period_byte isn't correct.

Is it really happen ??

Basically, I have no objection about this patch,
but this explanation is very strange for me...

Best regards
---
Kuninori Morimoto

Re: [PATCH v2 2/2] ASoC: rsnd: ssi: remove unnesessary period_pos

[Kuninori Morimoto]

Hi Jiada

> Further more, if the passed 'byte' amount to
> rsnd_ssi_pointer_update() is more than byte_per_period.
> the calculation of next_period_byte isn't correct.

Is it really happen ??

Basically, I have no objection about this patch,
but this explanation is very strange for me...

Best regards
---
Kuninori Morimoto

Re: [PATCH v2 2/2] ASoC: rsnd: ssi: remove unnesessary period_pos

[Jiada Wang]

Hi Morimoto-san

On 12/07/2017 01:58 AM, Kuninori Morimoto wrote:
> Hi Jiada
>
>> Further more, if the passed 'byte' amount to
>> rsnd_ssi_pointer_update() is more than byte_per_period.
>> the calculation of next_period_byte isn't correct.
> Is it really happen ??
>
> Basically, I have no objection about this patch,
> but this explanation is very strange for me...
No, I didn't see the issue,
but the implementation of rsnd_ssi_pointer_update(), behaves like
it knows all caller will always pass 'byte' no larger than byte_per_period,
without any check internally.

I am ok to remove this explanation from commit message,
what do you think?

Thanks,
Jiada
> Best regards
> ---
> Kuninori Morimoto

Re: [PATCH v2 2/2] ASoC: rsnd: ssi: remove unnesessary period_pos

[Jiada Wang]

Hi Morimoto-san

On 12/07/2017 01:58 AM, Kuninori Morimoto wrote:
> Hi Jiada
>
>> Further more, if the passed 'byte' amount to
>> rsnd_ssi_pointer_update() is more than byte_per_period.
>> the calculation of next_period_byte isn't correct.
> Is it really happen ??
>
> Basically, I have no objection about this patch,
> but this explanation is very strange for me...
No, I didn't see the issue,
but the implementation of rsnd_ssi_pointer_update(), behaves like
it knows all caller will always pass 'byte' no larger than byte_per_period,
without any check internally.

I am ok to remove this explanation from commit message,
what do you think?

Thanks,
Jiada
> Best regards
> ---
> Kuninori Morimoto

Re: [PATCH v2 2/2] ASoC: rsnd: ssi: remove unnesessary period_pos

[Kuninori Morimoto]

Hi Jiada

Thank you for your feedback

> >> Further more, if the passed 'byte' amount to
> >> rsnd_ssi_pointer_update() is more than byte_per_period.
> >> the calculation of next_period_byte isn't correct.
> > Is it really happen ??
> >
> > Basically, I have no objection about this patch,
> > but this explanation is very strange for me...
> No, I didn't see the issue,
> but the implementation of rsnd_ssi_pointer_update(), behaves like
> it knows all caller will always pass 'byte' no larger than byte_per_period,
> without any check internally.
> 
> I am ok to remove this explanation from commit message,
> what do you think?

This function is used from PIO mode only now, and "byte" is sizeof(u32)
(Its size was "byte_per_period" when DMA mode used).
This "Further more" case never happen.
Removing from commit message is better for reader, IMO.

Best regards
---
Kuninori Morimoto

Re: [PATCH v2 2/2] ASoC: rsnd: ssi: remove unnesessary period_pos

[Kuninori Morimoto]

Hi Jiada

Thank you for your feedback

> >> Further more, if the passed 'byte' amount to
> >> rsnd_ssi_pointer_update() is more than byte_per_period.
> >> the calculation of next_period_byte isn't correct.
> > Is it really happen ??
> >
> > Basically, I have no objection about this patch,
> > but this explanation is very strange for me...
> No, I didn't see the issue,
> but the implementation of rsnd_ssi_pointer_update(), behaves like
> it knows all caller will always pass 'byte' no larger than byte_per_period,
> without any check internally.
> 
> I am ok to remove this explanation from commit message,
> what do you think?

This function is used from PIO mode only now, and "byte" is sizeof(u32)
(Its size was "byte_per_period" when DMA mode used).
This "Further more" case never happen.
Removing from commit message is better for reader, IMO.

Best regards
---
Kuninori Morimoto

Applied "ASoC: rsnd: ssi: remove unnesessary period_pos" to the asoc tree

[Mark Brown]

The patch

   ASoC: rsnd: ssi: remove unnesessary period_pos

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 2e2d53da81af6b2222c6b4e025a5d01b37b4449b Mon Sep 17 00:00:00 2001
From: Jiada Wang <jiada_wang@mentor.com>
Date: Thu, 7 Dec 2017 22:15:39 -0800
Subject: [PATCH] ASoC: rsnd: ssi: remove unnesessary period_pos

period_pos can always be calculated by byte_pos and
byte_per_period, there is no reason to maintain this
variable in rsnd_dai_stream.

This patch removes period_pos from rsnd_ssi and calculates
next_period_byte with consideration of actual byte_pos value.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sh/rcar/ssi.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index cbf3bf312d23..f21202429000 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -80,7 +80,6 @@ struct rsnd_ssi {
 	unsigned int usrcnt;
 
 	int byte_pos;
-	int period_pos;
 	int byte_per_period;
 	int next_period_byte;
 };
@@ -421,7 +420,6 @@ static void rsnd_ssi_pointer_init(struct rsnd_mod *mod,
 	struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
 
 	ssi->byte_pos		= 0;
-	ssi->period_pos		= 0;
 	ssi->byte_per_period	= runtime->period_size *
 				  runtime->channels *
 				  samples_to_bytes(runtime, 1);
@@ -453,13 +451,12 @@ static bool rsnd_ssi_pointer_update(struct rsnd_mod *mod,
 
 	if (byte_pos >= ssi->next_period_byte) {
 		struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
+		int period_pos = byte_pos / ssi->byte_per_period;
 
-		ssi->period_pos++;
-		ssi->next_period_byte += ssi->byte_per_period;
+		ssi->next_period_byte = (period_pos + 1) * ssi->byte_per_period;
 
-		if (ssi->period_pos >= runtime->periods) {
+		if (period_pos >= runtime->periods) {
 			byte_pos = 0;
-			ssi->period_pos = 0;
 			ssi->next_period_byte = ssi->byte_per_period;
 		}
 
-- 
2.15.1

Applied "ASoC: rsnd: ssi: remove unnesessary period_pos" to the asoc tree

[Mark Brown]

The patch

   ASoC: rsnd: ssi: remove unnesessary period_pos

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 2e2d53da81af6b2222c6b4e025a5d01b37b4449b Mon Sep 17 00:00:00 2001
From: Jiada Wang <jiada_wang@mentor.com>
Date: Thu, 7 Dec 2017 22:15:39 -0800
Subject: [PATCH] ASoC: rsnd: ssi: remove unnesessary period_pos

period_pos can always be calculated by byte_pos and
byte_per_period, there is no reason to maintain this
variable in rsnd_dai_stream.

This patch removes period_pos from rsnd_ssi and calculates
next_period_byte with consideration of actual byte_pos value.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sh/rcar/ssi.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index cbf3bf312d23..f21202429000 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -80,7 +80,6 @@ struct rsnd_ssi {
 	unsigned int usrcnt;
 
 	int byte_pos;
-	int period_pos;
 	int byte_per_period;
 	int next_period_byte;
 };
@@ -421,7 +420,6 @@ static void rsnd_ssi_pointer_init(struct rsnd_mod *mod,
 	struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
 
 	ssi->byte_pos		= 0;
-	ssi->period_pos		= 0;
 	ssi->byte_per_period	= runtime->period_size *
 				  runtime->channels *
 				  samples_to_bytes(runtime, 1);
@@ -453,13 +451,12 @@ static bool rsnd_ssi_pointer_update(struct rsnd_mod *mod,
 
 	if (byte_pos >= ssi->next_period_byte) {
 		struct snd_pcm_runtime *runtime = rsnd_io_to_runtime(io);
+		int period_pos = byte_pos / ssi->byte_per_period;
 
-		ssi->period_pos++;
-		ssi->next_period_byte += ssi->byte_per_period;
+		ssi->next_period_byte = (period_pos + 1) * ssi->byte_per_period;
 
-		if (ssi->period_pos >= runtime->periods) {
+		if (period_pos >= runtime->periods) {
 			byte_pos = 0;
-			ssi->period_pos = 0;
 			ssi->next_period_byte = ssi->byte_per_period;
 		}
 
-- 
2.15.1