[alsa-devel] [PATCH 0/5] ASoC: sh_mobile_hdmi: bug fix patches
Dear Mark, Liam, Guennadi
These are bug fix patches for sh_mobile_hdmi.
Kuninori Morimoto (5): fbdev: sh_mobile_hdmi: modify HDMI flags name to more specific fbdev: sh_mobile_hdmi: modify snd_soc_dai_driver settings fbdev: sh_mobile_hdmi: add new label for sound error path ASoC: fsi-hdmi: remove unneeded header ASoC: fsi-ak4642: modiry platform_name
1/5 - 4/5 bugs are reported by Guennadi. And 5/5 is needed if Paul's genesis tree are merged to Mark's "for-2.6.37"
Best regards -- Kuninori Morimoto
Reported-by: Guennadi Liakhovetski g.liakhovetski@gmx.de Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- drivers/video/sh_mobile_hdmi.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c index 5e855cc..d51f751 100644 --- a/drivers/video/sh_mobile_hdmi.c +++ b/drivers/video/sh_mobile_hdmi.c @@ -253,9 +253,12 @@ static struct snd_soc_dai_driver sh_hdmi_dai = { .name = "sh_mobile_hdmi-hifi", .playback = { .stream_name = "Playback", - .channels_min = 1, - .channels_max = 2, - .rates = SNDRV_PCM_RATE_8000_48000, + .channels_min = 2, + .channels_max = 8, + .rates = SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | + SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 | + SNDRV_PCM_RATE_96000 | SNDRV_PCM_RATE_176400 | + SNDRV_PCM_RATE_192000, .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE, }, };
Reported-by: Guennadi Liakhovetski g.liakhovetski@gmx.de Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- drivers/video/sh_mobile_hdmi.c | 10 +++++----- include/video/sh_mobile_hdmi.h | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c index 16187d6..5e855cc 100644 --- a/drivers/video/sh_mobile_hdmi.c +++ b/drivers/video/sh_mobile_hdmi.c @@ -396,19 +396,19 @@ static void sh_hdmi_audio_config(struct sh_hdmi *hdmi) * [6:5] set required down sampling rate if required * [4:3] set required audio source */ - switch (pdata->flags & HDMI_SRC_MASK) { + switch (pdata->flags & HDMI_SND_SRC_MASK) { default: /* FALL THROUGH */ - case HDMI_SRC_I2S: + case HDMI_SND_SRC_I2S: data = (0x0 << 3); break; - case HDMI_SRC_SPDIF: + case HDMI_SND_SRC_SPDIF: data = (0x1 << 3); break; - case HDMI_SRC_DSD: + case HDMI_SND_SRC_DSD: data = (0x2 << 3); break; - case HDMI_SRC_HBR: + case HDMI_SND_SRC_HBR: data = (0x3 << 3); break; } diff --git a/include/video/sh_mobile_hdmi.h b/include/video/sh_mobile_hdmi.h index 929c2d3..1e1aa54 100644 --- a/include/video/sh_mobile_hdmi.h +++ b/include/video/sh_mobile_hdmi.h @@ -23,11 +23,11 @@ struct device; */
/* Audio source select */ -#define HDMI_SRC_MASK (0xF << 0) -#define HDMI_SRC_I2S (0 << 0) /* default */ -#define HDMI_SRC_SPDIF (1 << 0) -#define HDMI_SRC_DSD (2 << 0) -#define HDMI_SRC_HBR (3 << 0) +#define HDMI_SND_SRC_MASK (0xF << 0) +#define HDMI_SND_SRC_I2S (0 << 0) /* default */ +#define HDMI_SND_SRC_SPDIF (1 << 0) +#define HDMI_SND_SRC_DSD (2 << 0) +#define HDMI_SND_SRC_HBR (3 << 0)
struct sh_mobile_hdmi_info { struct sh_mobile_lcdc_chan_cfg *lcd_chan;
On Tue, 7 Sep 2010, Kuninori Morimoto wrote:
Reported-by: Guennadi Liakhovetski g.liakhovetski@gmx.de Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
drivers/video/sh_mobile_hdmi.c | 10 +++++----- include/video/sh_mobile_hdmi.h | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c index 16187d6..5e855cc 100644 --- a/drivers/video/sh_mobile_hdmi.c +++ b/drivers/video/sh_mobile_hdmi.c @@ -396,19 +396,19 @@ static void sh_hdmi_audio_config(struct sh_hdmi *hdmi) * [6:5] set required down sampling rate if required * [4:3] set required audio source */
- switch (pdata->flags & HDMI_SRC_MASK) {
- switch (pdata->flags & HDMI_SND_SRC_MASK) { default: /* FALL THROUGH */
- case HDMI_SRC_I2S:
- case HDMI_SND_SRC_I2S: data = (0x0 << 3);
While at it you could as well remove these parenthesis - in all 4 cases, of course.
Thanks Guennadi
break;
- case HDMI_SRC_SPDIF:
- case HDMI_SND_SRC_SPDIF: data = (0x1 << 3); break;
- case HDMI_SRC_DSD:
- case HDMI_SND_SRC_DSD: data = (0x2 << 3); break;
- case HDMI_SRC_HBR:
- case HDMI_SND_SRC_HBR: data = (0x3 << 3); break; }
diff --git a/include/video/sh_mobile_hdmi.h b/include/video/sh_mobile_hdmi.h index 929c2d3..1e1aa54 100644 --- a/include/video/sh_mobile_hdmi.h +++ b/include/video/sh_mobile_hdmi.h @@ -23,11 +23,11 @@ struct device; */
/* Audio source select */ -#define HDMI_SRC_MASK (0xF << 0) -#define HDMI_SRC_I2S (0 << 0) /* default */ -#define HDMI_SRC_SPDIF (1 << 0) -#define HDMI_SRC_DSD (2 << 0) -#define HDMI_SRC_HBR (3 << 0) +#define HDMI_SND_SRC_MASK (0xF << 0) +#define HDMI_SND_SRC_I2S (0 << 0) /* default */ +#define HDMI_SND_SRC_SPDIF (1 << 0) +#define HDMI_SND_SRC_DSD (2 << 0) +#define HDMI_SND_SRC_HBR (3 << 0)
struct sh_mobile_hdmi_info { struct sh_mobile_lcdc_chan_cfg *lcd_chan; -- 1.7.0.4
--- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
Reported-by: Guennadi Liakhovetski g.liakhovetski@gmx.de Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- drivers/video/sh_mobile_hdmi.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c index 5e855cc..d51f751 100644 --- a/drivers/video/sh_mobile_hdmi.c +++ b/drivers/video/sh_mobile_hdmi.c @@ -253,9 +253,12 @@ static struct snd_soc_dai_driver sh_hdmi_dai = { .name = "sh_mobile_hdmi-hifi", .playback = { .stream_name = "Playback", - .channels_min = 1, - .channels_max = 2, - .rates = SNDRV_PCM_RATE_8000_48000, + .channels_min = 2, + .channels_max = 8, + .rates = SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | + SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 | + SNDRV_PCM_RATE_96000 | SNDRV_PCM_RATE_176400 | + SNDRV_PCM_RATE_192000, .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE, }, };
Reported-by: Guennadi Liakhovetski g.liakhovetski@gmx.de Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- drivers/video/sh_mobile_hdmi.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c index d51f751..a2d6f7b 100644 --- a/drivers/video/sh_mobile_hdmi.c +++ b/drivers/video/sh_mobile_hdmi.c @@ -974,7 +974,7 @@ static int __init sh_hdmi_probe(struct platform_device *pdev) ret = snd_soc_register_codec(&pdev->dev, &soc_codec_dev_sh_hdmi, &sh_hdmi_dai, 1); if (ret < 0) - goto egetclk; + goto esndreg;
hdmi->dev = &pdev->dev;
@@ -1061,6 +1061,8 @@ eclkenable: erate: clk_put(hdmi->hdmi_clk); egetclk: + snd_soc_unregister_codec(&pdev->dev); +esndreg: kfree(hdmi);
return ret;
Hello Morimoto-san
On Tue, 7 Sep 2010, Kuninori Morimoto wrote:
Reported-by: Guennadi Liakhovetski g.liakhovetski@gmx.de Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
You have not addressed all of my comments to your sh_mobile_hdmi.c patch and you have not explained, why you do not address them. On the whole, I am still in favour of reverting that original patch and redoing it.
Thanks Guennadi
drivers/video/sh_mobile_hdmi.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c index d51f751..a2d6f7b 100644 --- a/drivers/video/sh_mobile_hdmi.c +++ b/drivers/video/sh_mobile_hdmi.c @@ -974,7 +974,7 @@ static int __init sh_hdmi_probe(struct platform_device *pdev) ret = snd_soc_register_codec(&pdev->dev, &soc_codec_dev_sh_hdmi, &sh_hdmi_dai, 1); if (ret < 0)
goto egetclk;
goto esndreg;hdmi->dev = &pdev->dev;
@@ -1061,6 +1061,8 @@ eclkenable: erate: clk_put(hdmi->hdmi_clk); egetclk:
- snd_soc_unregister_codec(&pdev->dev);
+esndreg: kfree(hdmi);
return ret;
1.7.0.4
--- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
On Tue, Sep 07, 2010 at 09:13:57AM +0200, Guennadi Liakhovetski wrote:
On Tue, 7 Sep 2010, Kuninori Morimoto wrote:
Reported-by: Guennadi Liakhovetski g.liakhovetski@gmx.de Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
You have not addressed all of my comments to your sh_mobile_hdmi.c patch
Please do this Morimoto-san - if you wish to leave things for addressing on a subsequent patch series then just note that in your covering mail or similar.
and you have not explained, why you do not address them. On the whole, I am still in favour of reverting that original patch and redoing it.
It would seem like we'd make more progress if the two of you could both work on incrementally fixing the problems with the current code rather than blocking its inclusion totally.
Reported-by: Guennadi Liakhovetski g.liakhovetski@gmx.de Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi-hdmi.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/sound/soc/sh/fsi-hdmi.c b/sound/soc/sh/fsi-hdmi.c index 950e3e0..fcda149 100644 --- a/sound/soc/sh/fsi-hdmi.c +++ b/sound/soc/sh/fsi-hdmi.c @@ -11,7 +11,6 @@
#include <linux/platform_device.h> #include <sound/sh_fsi.h> -#include <video/sh_mobile_hdmi.h>
static struct snd_soc_dai_link fsi_dai_link = { .name = "HDMI",
On Tue, 7 Sep 2010, Kuninori Morimoto wrote:
Reported-by: Guennadi Liakhovetski g.liakhovetski@gmx.de Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
sound/soc/sh/fsi-hdmi.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/sound/soc/sh/fsi-hdmi.c b/sound/soc/sh/fsi-hdmi.c index 950e3e0..fcda149 100644 --- a/sound/soc/sh/fsi-hdmi.c +++ b/sound/soc/sh/fsi-hdmi.c @@ -11,7 +11,6 @@
#include <linux/platform_device.h> #include <sound/sh_fsi.h>
Is this header needed?
Thanks Guennadi
-#include <video/sh_mobile_hdmi.h>
static struct snd_soc_dai_link fsi_dai_link = { .name = "HDMI", -- 1.7.0.4
--- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi-ak4642.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/sh/fsi-ak4642.c b/sound/soc/sh/fsi-ak4642.c index 53836ca..a7d2686 100644 --- a/sound/soc/sh/fsi-ak4642.c +++ b/sound/soc/sh/fsi-ak4642.c @@ -32,7 +32,7 @@ static struct snd_soc_dai_link fsi_dai_link = { .cpu_dai_name = "fsia-dai", /* fsi A */ .codec_dai_name = "ak4642-hifi", #ifdef CONFIG_MACH_AP4EVB - .platform_name = "sh_fsi2.0", + .platform_name = "sh_fsi2", .codec_name = "ak4642-codec.0-0013", #else .platform_name = "sh_fsi.0",
On Tue, 2010-09-07 at 15:46 +0900, Kuninori Morimoto wrote:
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
sound/soc/sh/fsi-ak4642.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/sh/fsi-ak4642.c b/sound/soc/sh/fsi-ak4642.c index 53836ca..a7d2686 100644 --- a/sound/soc/sh/fsi-ak4642.c +++ b/sound/soc/sh/fsi-ak4642.c @@ -32,7 +32,7 @@ static struct snd_soc_dai_link fsi_dai_link = { .cpu_dai_name = "fsia-dai", /* fsi A */ .codec_dai_name = "ak4642-hifi", #ifdef CONFIG_MACH_AP4EVB
- .platform_name = "sh_fsi2.0",
- .platform_name = "sh_fsi2", .codec_name = "ak4642-codec.0-0013",
#else .platform_name = "sh_fsi.0",
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
On Wed, Sep 08, 2010 at 12:08:50PM +0100, Liam Girdwood wrote:
On Tue, 2010-09-07 at 15:46 +0900, Kuninori Morimoto wrote:
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
#ifdef CONFIG_MACH_AP4EVB
- .platform_name = "sh_fsi2.0",
- .platform_name = "sh_fsi2", .codec_name = "ak4642-codec.0-0013",
#else .platform_name = "sh_fsi.0",
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
Applied, thanks.
participants (4)
-
Guennadi Liakhovetski -
Kuninori Morimoto -
Liam Girdwood -
Mark Brown