[alsa-devel] [PATCH 1/3] ASoC: SAMSUNG: Fix initial return value
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)) {
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",
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
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;
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");
- 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'.
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.
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
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?
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.
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
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.
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.
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.
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.
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
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.
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
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
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.
participants (4)
-
Jassi Brar -
Mark Brown -
Seungwhan Youn -
Seungwhan Youn