[PATCH 1/3] ASoC: SAMSUNG: Fix initial return value

[PATCH 1/3] ASoC: SAMSUNG: Fix initial return value

[Seungwhan Youn]

This patch fixed intial return value to be a '0' as asuccess on
set_audio_clock_heirachy(). This avoids unintended error on initialize.

Signed-off-by: Seungwhan Youn <sw.youn@samsung.com>
---
 sound/soc/s3c24xx/smdk_spdif.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/s3c24xx/smdk_spdif.c b/sound/soc/s3c24xx/smdk_spdif.c
index c8bd904..cb658d1 100644
--- a/sound/soc/s3c24xx/smdk_spdif.c
+++ b/sound/soc/s3c24xx/smdk_spdif.c
@@ -28,7 +28,7 @@
 static int set_audio_clock_heirachy(struct platform_device *pdev)
 {
 	struct clk *fout_epll, *mout_epll, *sclk_audio0, *sclk_spdif;
-	int ret;
+	int ret = 0;
 
 	fout_epll = clk_get(NULL, "fout_epll");
 	if (IS_ERR(fout_epll)) {
-- 
1.6.2.5

[PATCH 2/3] ASoC: SAMSUNG: Remove duplicated snd_card on smdk_spdif

[Seungwhan Youn]

This patch remove duplicated snd_card defination on smdk_spdif.

Signed-off-by: Seungwhan Youn <sw.youn@samsung.com>
---
 sound/soc/s3c24xx/smdk_spdif.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/sound/soc/s3c24xx/smdk_spdif.c b/sound/soc/s3c24xx/smdk_spdif.c
index cb658d1..0e1fe60 100644
--- a/sound/soc/s3c24xx/smdk_spdif.c
+++ b/sound/soc/s3c24xx/smdk_spdif.c
@@ -152,8 +152,6 @@ static struct snd_soc_ops smdk_spdif_ops = {
 	.hw_params = smdk_hw_params,
 };
 
-static struct snd_soc_card smdk;
-
 static struct snd_soc_dai_link smdk_dai = {
 	.name = "S/PDIF",
 	.stream_name = "S/PDIF PCM Playback",
-- 
1.6.2.5

Re: [PATCH 2/3] ASoC: SAMSUNG: Remove duplicated snd_card on smdk_spdif

[Jassi Brar]

On Fri, Dec 3, 2010 at 6:32 PM, Seungwhan Youn <sw.youn@samsung.com> wrote:
> This patch remove duplicated snd_card defination on smdk_spdif.

>
> -static struct snd_soc_card smdk;
> -
>  static struct snd_soc_dai_link smdk_dai = {
>        .name = "S/PDIF",
>        .stream_name = "S/PDIF PCM Playback",

Acked-by: Jassi Brar <jassi.brar@samsung.com>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 3/3] ASoC: SAMSUNG: Match spdif source clock id and asoc platform device

[Seungwhan Youn]

This patch modify to match ids between source clock id and asoc platform
device that spdif driver can find sclk_audio0 with device id.

Signed-off-by: Seungwhan Youn <sw.youn@samsung.com>
---
 sound/soc/s3c24xx/smdk_spdif.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/s3c24xx/smdk_spdif.c b/sound/soc/s3c24xx/smdk_spdif.c
index 0e1fe60..b4ad231 100644
--- a/sound/soc/s3c24xx/smdk_spdif.c
+++ b/sound/soc/s3c24xx/smdk_spdif.c
@@ -183,7 +183,7 @@ static int __init smdk_init(void)
 	if (ret)
 		goto err2;
 
-	smdk_snd_spdif_device = platform_device_alloc("soc-audio", -1);
+	smdk_snd_spdif_device = platform_device_alloc("soc-audio", 0);
 	if (!smdk_snd_spdif_device) {
 		ret = -ENOMEM;
 		goto err2;
-- 
1.6.2.5

Re: [PATCH 3/3] ASoC: SAMSUNG: Match spdif source clock id and asoc platform device

[Jassi Brar]

On Fri, Dec 3, 2010 at 6:37 PM, Seungwhan Youn <sw.youn@samsung.com> wrote:
> This patch modify to match ids between source clock id and asoc platform
> device that spdif driver can find sclk_audio0 with device id.
>
> Signed-off-by: Seungwhan Youn <sw.youn@samsung.com>
> ---
>  sound/soc/s3c24xx/smdk_spdif.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/sound/soc/s3c24xx/smdk_spdif.c b/sound/soc/s3c24xx/smdk_spdif.c
> index 0e1fe60..b4ad231 100644
> --- a/sound/soc/s3c24xx/smdk_spdif.c
> +++ b/sound/soc/s3c24xx/smdk_spdif.c
> @@ -183,7 +183,7 @@ static int __init smdk_init(void)
>        if (ret)
>                goto err2;
>
> -       smdk_snd_spdif_device = platform_device_alloc("soc-audio", -1);
> +       smdk_snd_spdif_device = platform_device_alloc("soc-audio", 0);

I think this is not appropriate.

Maybe you should do :-
  sclk_audio0 = clk_get(NULL, "sclk_audio");
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 3/3] ASoC: SAMSUNG: Match spdif source clock id and asoc platform device

[Seungwhan Youn]

>> -       smdk_snd_spdif_device = platform_device_alloc("soc-audio", -1);
>> +       smdk_snd_spdif_device = platform_device_alloc("soc-audio", 0);
>
> I think this is not appropriate.
>
> Maybe you should do :-
>  sclk_audio0 = clk_get(NULL, "sclk_audio");

Yes, I think so, but without id, there is no way to find sclk_audio0
in current samsung s5p. Because, current sclk_audio0,1,2 is only to
distinguished by id not by name, and also all sclk_audio0's id for s5p
is '0' for now. So, I think that this modification is more batter to
modify all s5p's sclk_audio clock id to '-1'.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 3/3] ASoC: SAMSUNG: Match spdif source clock id and asoc platform device

[Jassi Brar]

On Fri, Dec 3, 2010 at 7:17 PM, Seungwhan Youn <claude.youn@gmail.com> wrote:
>>> -       smdk_snd_spdif_device = platform_device_alloc("soc-audio", -1);
>>> +       smdk_snd_spdif_device = platform_device_alloc("soc-audio", 0);
>>
>> I think this is not appropriate.
>>
>> Maybe you should do :-
>>  sclk_audio0 = clk_get(NULL, "sclk_audio");
>
> Yes, I think so, but without id, there is no way to find sclk_audio0
> in current samsung s5p. Because, current sclk_audio0,1,2 is only to
> distinguished by id not by name, and also all sclk_audio0's id for s5p
> is '0' for now. So, I think that this modification is more batter to
> modify all s5p's sclk_audio clock id to '-1'.

the platform_device soc-audio has nothing to do with a particular
audio controller, so I don't favor associating with a controller.
Good for us, SPDIF needs sclk_audio0 which is the first instance
that clk_get will come across and hence we can still get the clock.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 3/3] ASoC: SAMSUNG: Match spdif source clock id and asoc platform device

[Jassi Brar]

On Fri, Dec 3, 2010 at 7:23 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> On Fri, Dec 3, 2010 at 7:17 PM, Seungwhan Youn <claude.youn@gmail.com> wrote:
>>>> -       smdk_snd_spdif_device = platform_device_alloc("soc-audio", -1);
>>>> +       smdk_snd_spdif_device = platform_device_alloc("soc-audio", 0);
>>>
>>> I think this is not appropriate.
>>>
>>> Maybe you should do :-
>>>  sclk_audio0 = clk_get(NULL, "sclk_audio");
>>
>> Yes, I think so, but without id, there is no way to find sclk_audio0
>> in current samsung s5p. Because, current sclk_audio0,1,2 is only to
>> distinguished by id not by name, and also all sclk_audio0's id for s5p
>> is '0' for now. So, I think that this modification is more batter to
>> modify all s5p's sclk_audio clock id to '-1'.
>
> the platform_device soc-audio has nothing to do with a particular
> audio controller, so I don't favor associating with a controller.
> Good for us, SPDIF needs sclk_audio0 which is the first instance
> that clk_get will come across and hence we can still get the clock.

Ok, I see it wouldn't work as such.

Theoretically we have two parallel clocks from same source - one goes
to I2S_0 and the other to SPDIF.
So, IMO, we need to clone the sclk_audio0 clock with SPDIF specific name
and that has id as -1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 3/3] ASoC: SAMSUNG: Match spdif source clock id and asoc platform device

[Seungwhan Youn]

> Theoretically we have two parallel clocks from same source - one goes
> to I2S_0 and the other to SPDIF.
> So, IMO, we need to clone the sclk_audio0 clock with SPDIF specific name
> and that has id as -1
>

Okay, I agree whit this.
So, how do you think about this Mr. Kukjin?
Is that okay to make a clone clock of sclk_audio0 for SPDIF?

Re: [PATCH 3/3] ASoC: SAMSUNG: Match spdif source clock id and asoc platform device

[Mark Brown]

On Fri, Dec 03, 2010 at 06:37:47PM +0900, Seungwhan Youn wrote:
> This patch modify to match ids between source clock id and asoc platform
> device that spdif driver can find sclk_audio0 with device id.

Something's seriously wrong if you need to do this...

> -	smdk_snd_spdif_device = platform_device_alloc("soc-audio", -1);
> +	smdk_snd_spdif_device = platform_device_alloc("soc-audio", 0);

...the soc-audio device is a virtual device so should have no clocks
associated with it.  The driver should not be using the platform device
as an argument for clk_get() in the first place.

Re: [PATCH 3/3] ASoC: SAMSUNG: Match spdif source clock id and asoc platform device

[Seungwhan Youn]

On Fri, Dec 3, 2010 at 8:44 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Dec 03, 2010 at 06:37:47PM +0900, Seungwhan Youn wrote:
>> This patch modify to match ids between source clock id and asoc platform
>> device that spdif driver can find sclk_audio0 with device id.
>
> Something's seriously wrong if you need to do this...
>
>> -     smdk_snd_spdif_device = platform_device_alloc("soc-audio", -1);
>> +     smdk_snd_spdif_device = platform_device_alloc("soc-audio", 0);
>
> ...the soc-audio device is a virtual device so should have no clocks
> associated with it.  The driver should not be using the platform device
> as an argument for clk_get() in the first place.

I see, I can clearly understand.
Thanks for your kindly reply. :)

Claude
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 3/3] ASoC: SAMSUNG: Match spdif source clock id and asoc platform device

[Jassi Brar]

On Fri, Dec 3, 2010 at 8:44 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:

>> This patch modify to match ids between source clock id and asoc platform
>> device that spdif driver can find sclk_audio0 with device id.
>
> Something's seriously wrong if you need to do this...

It's necessary evil ATM.
There is no access to SPDIF device in machine code and we need to
get/set the clock.
So the soc-audio is used as a place-holder.

Re: [PATCH 3/3] ASoC: SAMSUNG: Match spdif source clock id and asoc platform device

[Mark Brown]

On Sun, Dec 05, 2010 at 12:40:01AM +0900, Jassi Brar wrote:
> On Fri, Dec 3, 2010 at 8:44 PM, Mark Brown

> > Something's seriously wrong if you need to do this...

> It's necessary evil ATM.
> There is no access to SPDIF device in machine code and we need to
> get/set the clock.
> So the soc-audio is used as a place-holder.

In order to do this you need to tell the clock framework about the ASoC
virtual device - once you're doing stuff like that you may as well just
fix the S/PDIF driver to let you do what you need.

Re: [PATCH 3/3] ASoC: SAMSUNG: Match spdif source clock id and asoc platform device

[Jassi Brar]

On Mon, Dec 6, 2010 at 9:46 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Sun, Dec 05, 2010 at 12:40:01AM +0900, Jassi Brar wrote:

>> It's necessary evil ATM.
>> There is no access to SPDIF device in machine code and we need to
>> get/set the clock.
>> So the soc-audio is used as a place-holder.
>
> In order to do this you need to tell the clock framework about the ASoC
> virtual device - once you're doing stuff like that you may as well just
> fix the S/PDIF driver to let you do what you need.

Assuming I understand you correctly, the problem still remains of having the
S/PDIF driver to setup clock hierarchies -- which is the job of the
board init code, IMHO.
So until we have proper clock subsystem and board init code to handle
such scenarios
we have to do that in ASoC machine driver in such hackish way.

Re: [PATCH 3/3] ASoC: SAMSUNG: Match spdif source clock id and asoc platform device

[Mark Brown]

On Tue, Dec 07, 2010 at 11:10:22AM +0900, Jassi Brar wrote:
> On Mon, Dec 6, 2010 at 9:46 PM, Mark Brown

> > In order to do this you need to tell the clock framework about the ASoC
> > virtual device - once you're doing stuff like that you may as well just
> > fix the S/PDIF driver to let you do what you need.

> Assuming I understand you correctly, the problem still remains of having the
> S/PDIF driver to setup clock hierarchies -- which is the job of the
> board init code, IMHO.
> So until we have proper clock subsystem and board init code to handle
> such scenarios
> we have to do that in ASoC machine driver in such hackish way.

It may be that what's needed in the S/PDIF driver is to provide a way to
get the device for the S/PDIF controller so that the machine driver can
fiddle with it.  Actually, you can do that anyway as the device will be
associated with the DAI - the machine driver can just look at dai->dev
to get the struct device for the S/PDIF controller.

Re: [PATCH 1/3] ASoC: SAMSUNG: Fix initial return value

[Jassi Brar]

On Fri, Dec 3, 2010 at 6:27 PM, Seungwhan Youn <sw.youn@samsung.com> wrote:
> This patch fixed intial return value to be a '0' as asuccess on
> set_audio_clock_heirachy(). This avoids unintended error on initialize.
>
> Signed-off-by: Seungwhan Youn <sw.youn@samsung.com>
> ---
>  sound/soc/s3c24xx/smdk_spdif.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/sound/soc/s3c24xx/smdk_spdif.c b/sound/soc/s3c24xx/smdk_spdif.c
> index c8bd904..cb658d1 100644
> --- a/sound/soc/s3c24xx/smdk_spdif.c
> +++ b/sound/soc/s3c24xx/smdk_spdif.c
> @@ -28,7 +28,7 @@
>  static int set_audio_clock_heirachy(struct platform_device *pdev)
>  {
>        struct clk *fout_epll, *mout_epll, *sclk_audio0, *sclk_spdif;
> -       int ret;
> +       int ret = 0;
>
>        fout_epll = clk_get(NULL, "fout_epll");
>        if (IS_ERR(fout_epll)) {

Acked-by: Jassi Brar <jassi.brar@samsung.com>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/3] ASoC: SAMSUNG: Fix initial return value

[Mark Brown]

On Fri, Dec 03, 2010 at 06:27:33PM +0900, Seungwhan Youn wrote:
> This patch fixed intial return value to be a '0' as asuccess on
> set_audio_clock_heirachy(). This avoids unintended error on initialize.
> 
> Signed-off-by: Seungwhan Youn <sw.youn@samsung.com>
> ---
>  sound/soc/s3c24xx/smdk_spdif.c |    2 +-

Could you regenerate against current for-2.6.38 please?  The move to the
Samsung directory means this won't apply any more.

Re: [PATCH 1/3] ASoC: SAMSUNG: Fix initial return value

[Seungwhan Youn]

On Sat, Dec 4, 2010 at 1:42 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Dec 03, 2010 at 06:27:33PM +0900, Seungwhan Youn wrote:
>> This patch fixed intial return value to be a '0' as asuccess on
>> set_audio_clock_heirachy(). This avoids unintended error on initialize.
>>
>> Signed-off-by: Seungwhan Youn <sw.youn@samsung.com>
>> ---
>>  sound/soc/s3c24xx/smdk_spdif.c |    2 +-
>
> Could you regenerate against current for-2.6.38 please?  The move to the
> Samsung directory means this won't apply any more.

Of course, I do. :-)

Thanks.
Claude
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/3] ASoC: SAMSUNG: Fix initial return value

[Seungwhan Youn]

On Sat, Dec 4, 2010 at 1:42 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Dec 03, 2010 at 06:27:33PM +0900, Seungwhan Youn wrote:
>> This patch fixed intial return value to be a '0' as asuccess on
>> set_audio_clock_heirachy(). This avoids unintended error on initialize.
>>
>> Signed-off-by: Seungwhan Youn <sw.youn@samsung.com>
>> ---
>>  sound/soc/s3c24xx/smdk_spdif.c |    2 +-
>
> Could you regenerate against current for-2.6.38 please?  The move to the
> Samsung directory means this won't apply any more.

If there are something more needed to accept these 2 patches for
2.6.37, please let me know.

BRs,
Claude
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/3] ASoC: SAMSUNG: Fix initial return value

[Mark Brown]

On Wed, Dec 08, 2010 at 10:00:04AM +0900, Seungwhan Youn wrote:

> If there are something more needed to accept these 2 patches for
> 2.6.37, please let me know.

You'd need to at least resubmit against 2.6.37 but especially with the
second patch I'm not sure there's any urgency - the second patch is just
a minor code cleanup and has no practical effect.