* [PATCH 1/5 v2] fbdev: sh_mobile_hdmi: modify noisy comment out
2010-09-09 2:46 [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2 Kuninori Morimoto
@ 2010-09-09 2:47 ` Kuninori Morimoto
2010-09-09 2:48 ` [PATCH 2/5 v2] fbdev: sh_mobile_hdmi: modify flags name to more specific Kuninori Morimoto
` (7 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Kuninori Morimoto @ 2010-09-09 2:47 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA, Guennadi, Liam Girdwood
This patch solve below report from Guennadi
1)
> - hdmi_write(hdmi, 0x00, HDMI_AUDIO_SETTING_1);
> + switch (pdata->flags & HDMI_SRC_MASK) {
> + default:
> + /* FALL THROUGH */
I'm not sure I like the capitalisation here - no reason to shout;)
2)
> +/************************************************************************
> +
> +
> + HDMI sound
> +
> +
> +************************************************************************/
I don't think this comment deserves 7 lines of text, besides breaking the
multiline comment style. If you think, one line like
/* HDMI sound */
is not enough how about just
/*
* HDMI sound
*/
3)
> +/************************************************************************
> +
> +
> + HDMI video
> +
> +
> +************************************************************************/
See above - 7 lines seem to be an overkill to me.
Reported-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2
# I didn't care this on V1 patches.
o add Guennadi's mail on log area
drivers/video/sh_mobile_hdmi.c | 21 +++++++--------------
1 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
index 16187d6..0acd850 100644
--- a/drivers/video/sh_mobile_hdmi.c
+++ b/drivers/video/sh_mobile_hdmi.c
@@ -224,13 +224,9 @@ static u8 hdmi_read(struct sh_hdmi *hdmi, u8 reg)
return ioread8(hdmi->base + reg);
}
-/************************************************************************
-
-
- HDMI sound
-
-
-************************************************************************/
+/*
+ * HDMI sound
+ */
static unsigned int sh_hdmi_snd_read(struct snd_soc_codec *codec,
unsigned int reg)
{
@@ -273,13 +269,10 @@ static struct snd_soc_codec_driver soc_codec_dev_sh_hdmi = {
.write = sh_hdmi_snd_write,
};
-/************************************************************************
-
-
- HDMI video
-
+/*
+ * HDMI video
+ */
-************************************************************************/
/* External video parameter settings */
static void hdmi_external_video_param(struct sh_hdmi *hdmi)
{
@@ -398,7 +391,7 @@ static void sh_hdmi_audio_config(struct sh_hdmi *hdmi)
*/
switch (pdata->flags & HDMI_SRC_MASK) {
default:
- /* FALL THROUGH */
+ /* fall through */
case HDMI_SRC_I2S:
data = (0x0 << 3);
break;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/5 v2] fbdev: sh_mobile_hdmi: modify flags name to more specific
2010-09-09 2:46 [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2 Kuninori Morimoto
2010-09-09 2:47 ` [PATCH 1/5 v2] fbdev: sh_mobile_hdmi: modify noisy comment out Kuninori Morimoto
@ 2010-09-09 2:48 ` Kuninori Morimoto
2010-09-09 2:48 ` [PATCH 3/5 v2] fbdev: sh_mobile_hdmi: modify snd_soc_dai_driver settings Kuninori Morimoto
` (6 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Kuninori Morimoto @ 2010-09-09 2:48 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA, Guennadi, Liam Girdwood
This patch solve below report from Guennadi
1)
> +/* 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)
I would be more specific with these macro names, i.e., include "AUDIO" or
"SND" or something similar in them, e.g., HDMI_AUDIO_SRC_I2S.
2)
> + case HDMI_SRC_I2S:
> + data = (0x0 << 3);
> + break;
> + case HDMI_SRC_SPDIF:
> + data = (0x1 << 3);
> + break;
> + case HDMI_SRC_DSD:
> + data = (0x2 << 3);
> + break;
> + case HDMI_SRC_HBR:
> + data = (0x3 << 3);
In all above cases parenthesis are superfluous.
Reported-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2
o add Guennadi's mail on log area
o remove noisy parenthesis
drivers/video/sh_mobile_hdmi.c | 18 +++++++++---------
include/video/sh_mobile_hdmi.h | 10 +++++-----
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
index 0acd850..beb04ef 100644
--- a/drivers/video/sh_mobile_hdmi.c
+++ b/drivers/video/sh_mobile_hdmi.c
@@ -389,20 +389,20 @@ 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:
- data = (0x0 << 3);
+ case HDMI_SND_SRC_I2S:
+ data = 0x0 << 3;
break;
- case HDMI_SRC_SPDIF:
- data = (0x1 << 3);
+ case HDMI_SND_SRC_SPDIF:
+ data = 0x1 << 3;
break;
- case HDMI_SRC_DSD:
- data = (0x2 << 3);
+ case HDMI_SND_SRC_DSD:
+ data = 0x2 << 3;
break;
- case HDMI_SRC_HBR:
- data = (0x3 << 3);
+ case HDMI_SND_SRC_HBR:
+ data = 0x3 << 3;
break;
}
hdmi_write(hdmi, data, HDMI_AUDIO_SETTING_1);
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/5 v2] fbdev: sh_mobile_hdmi: modify snd_soc_dai_driver settings
2010-09-09 2:46 [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2 Kuninori Morimoto
2010-09-09 2:47 ` [PATCH 1/5 v2] fbdev: sh_mobile_hdmi: modify noisy comment out Kuninori Morimoto
2010-09-09 2:48 ` [PATCH 2/5 v2] fbdev: sh_mobile_hdmi: modify flags name to more specific Kuninori Morimoto
@ 2010-09-09 2:48 ` Kuninori Morimoto
2010-09-09 2:48 ` [PATCH 4/5 v2] fbdev: sh_mobile_hdmi: add new label for sound error path Kuninori Morimoto
` (5 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Kuninori Morimoto @ 2010-09-09 2:48 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA, Guennadi, Liam Girdwood
This patch solve below report from Guennadi
> +static struct snd_soc_dai_driver sh_hdmi_dai = {
> + .name = "sh_mobile_hdmi-hifi",
> + .playback = {
> + .stream_name = "Playback",
> + .channels_min = 1,
Can it actually do mono? Maybe at probe time you could look at audio flags
from your previous patch and, e.g., for SPDIF set channels_min to 2?
> + .channels_max = 2,
That's the "smallest max," yes. With some other interfaces (I2S, DSD) it
can support up to 8 channels...
> + .rates = SNDRV_PCM_RATE_8000_48000,
Hm, in the datasheet I see supported frequencies 32kHz to 192kHz. And if
you promise support for multiple frequencies, don't you want to implement
.hw_params? Besides, not all of these frequencies will be available,
depending on your supplied clock and your willingness to implement
downsampling.
Reported-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2
o add Guennadi's mail on log area
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 beb04ef..a2cb492 100644
--- a/drivers/video/sh_mobile_hdmi.c
+++ b/drivers/video/sh_mobile_hdmi.c
@@ -249,9 +249,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,
},
};
--
1.7.0.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/5 v2] fbdev: sh_mobile_hdmi: add new label for sound error path
2010-09-09 2:46 [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2 Kuninori Morimoto
` (2 preceding siblings ...)
2010-09-09 2:48 ` [PATCH 3/5 v2] fbdev: sh_mobile_hdmi: modify snd_soc_dai_driver settings Kuninori Morimoto
@ 2010-09-09 2:48 ` Kuninori Morimoto
2010-09-09 2:48 ` Kuninori Morimoto
` (4 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Kuninori Morimoto @ 2010-09-09 2:48 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA, Guennadi, Liam Girdwood
This patch solve below report from Guennadi
> /* External video parameter settings */
> static void hdmi_external_video_param(struct sh_hdmi *hdmi)
> {
> @@ -804,6 +862,11 @@ static int __init sh_hdmi_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> + ret = snd_soc_register_codec(&pdev->dev,
> + &soc_codec_dev_sh_hdmi, &sh_hdmi_dai, 1);
> + if (ret < 0)
> + goto egetclk;
> +
> hdmi->dev = &pdev->dev;
>
> hdmi->hdmi_clk = clk_get(&pdev->dev, "ick");
NAK. This breaks the error path and has to be fixed. Firstly, please, use
a new label like "esndreg," secondly, you have to add
clk_disable(hdmi->hdmi_clk);
erate:
clk_put(hdmi->hdmi_clk);
egetclk:
+ snd_soc_unregister_codec(&pdev->dev);
+esndreg:
mutex_destroy(&hdmi->mutex);
kfree(hdmi);
Reported-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2
o add Guennadi's mail on log area
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 a2cb492..ef989d9 100644
--- a/drivers/video/sh_mobile_hdmi.c
+++ b/drivers/video/sh_mobile_hdmi.c
@@ -967,7 +967,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;
@@ -1054,6 +1054,8 @@ eclkenable:
erate:
clk_put(hdmi->hdmi_clk);
egetclk:
+ snd_soc_unregister_codec(&pdev->dev);
+esndreg:
kfree(hdmi);
return ret;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/5 v2] fbdev: sh_mobile_hdmi: add new label for sound error path
2010-09-09 2:46 [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2 Kuninori Morimoto
` (3 preceding siblings ...)
2010-09-09 2:48 ` [PATCH 4/5 v2] fbdev: sh_mobile_hdmi: add new label for sound error path Kuninori Morimoto
@ 2010-09-09 2:48 ` Kuninori Morimoto
2010-09-09 2:48 ` [PATCH 5/5 v2] ASoC: fsi-hdmi: remove unneeded header Kuninori Morimoto
` (3 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Kuninori Morimoto @ 2010-09-09 2:48 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA, Guennadi, Liam Girdwood
This patch solve below report from Guennadi
> /* External video parameter settings */
> static void hdmi_external_video_param(struct sh_hdmi *hdmi)
> {
> @@ -804,6 +862,11 @@ static int __init sh_hdmi_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> + ret = snd_soc_register_codec(&pdev->dev,
> + &soc_codec_dev_sh_hdmi, &sh_hdmi_dai, 1);
> + if (ret < 0)
> + goto egetclk;
> +
> hdmi->dev = &pdev->dev;
>
> hdmi->hdmi_clk = clk_get(&pdev->dev, "ick");
NAK. This breaks the error path and has to be fixed. Firstly, please, use
a new label like "esndreg," secondly, you have to add
clk_disable(hdmi->hdmi_clk);
erate:
clk_put(hdmi->hdmi_clk);
egetclk:
+ snd_soc_unregister_codec(&pdev->dev);
+esndreg:
mutex_destroy(&hdmi->mutex);
kfree(hdmi);
Reported-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2
o add Guennadi's mail on log area
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 a2cb492..ef989d9 100644
--- a/drivers/video/sh_mobile_hdmi.c
+++ b/drivers/video/sh_mobile_hdmi.c
@@ -967,7 +967,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;
@@ -1054,6 +1054,8 @@ eclkenable:
erate:
clk_put(hdmi->hdmi_clk);
egetclk:
+ snd_soc_unregister_codec(&pdev->dev);
+esndreg:
kfree(hdmi);
return ret;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 5/5 v2] ASoC: fsi-hdmi: remove unneeded header
2010-09-09 2:46 [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2 Kuninori Morimoto
` (4 preceding siblings ...)
2010-09-09 2:48 ` Kuninori Morimoto
@ 2010-09-09 2:48 ` Kuninori Morimoto
2010-09-09 6:32 ` [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2 Guennadi Liakhovetski
` (2 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Kuninori Morimoto @ 2010-09-09 2:48 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA, Guennadi, Liam Girdwood
This patch solve below report from Guennadi.
But I didn't remove #include <sound/sh_fsi.h>.
Because it have FSI_PORT_B define which is used on this file.
> +#include <linux/platform_device.h>
> +#include <sound/sh_fsi.h>
> +#include <video/sh_mobile_hdmi.h>
Now that everything is done with strings - do you still need these
headers?
Reported-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2
o add Guennadi's mail on log area
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",
--
1.7.0.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2
2010-09-09 2:46 [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2 Kuninori Morimoto
` (5 preceding siblings ...)
2010-09-09 2:48 ` [PATCH 5/5 v2] ASoC: fsi-hdmi: remove unneeded header Kuninori Morimoto
@ 2010-09-09 6:32 ` Guennadi Liakhovetski
2010-09-09 7:07 ` Kuninori Morimoto
2010-09-09 7:30 ` [PATCH 6/5] fbdev: sh_mobile_hdmi: Add select SND_SOC to Kconfig Kuninori Morimoto
2010-09-10 11:29 ` [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2 Guennadi Liakhovetski
8 siblings, 1 reply; 13+ messages in thread
From: Guennadi Liakhovetski @ 2010-09-09 6:32 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: Linux-ALSA, Mark Brown, Liam Girdwood
Hello Morimoto-san
Thanks
for your patches! But what about this my comment:
<quote>
Besides, I think, this will not link without CONFIG_SND_SOC.
</quote>
? Or is it wrong? If this is right, it would be better to either add a
"select SND_SOC" to FB_SH_MOBILE_HDMI Kconfig entry, or put calls to
snd_soc_(un)register_codec (and all the HDMI audio code) under "#ifdef
CONFIG_SND_SOC." The letter is, probably, less elegant.
Thanks
Guennadi
On Thu, 9 Sep 2010, Kuninori Morimoto wrote:
>
> Dear Mark, Liam, Guennadi
>
> These are ALSA V2 bug fix patches which are reported by Guennadi.
>
> Kuninori Morimoto (5):
> fbdev: sh_mobile_hdmi: modify noisy comment out
> fbdev: sh_mobile_hdmi: modify 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
>
> To Mark.
> Please care above patch independently from these patches.
> [alsa-devel] [PATCH 5/5] ASoC: fsi-ak4642: modiry platform_name
>
> These series are for ALSA side patches.
> I will send SH side V2 patches soon.
>
> Main diff v1 <==> v2 is
> I added Guennadi's report mail to log area on each patches,
> and care more.
> But these series still didn't care above.
> I added reasons.
>
> -- Guennadi ------------------------------------------
> > +config SND_FSI_HDMI
> > + bool "FSI-HDMI sound support"
> > + depends on SND_SOC_SH4_FSI && FB_SH_MOBILE_HDMI
> > + help
> > + This option enables generic sound support for the
> > + FSI - HDMI unit
> > +
>
> "bool" means, if someone is linking the whole ASoC into the kernel, they
> will not be able to build this as a module. Not a big deal, but you're
> stealing some freedom from the user.
> -------------------------------------------------------
>
> understand.
> But the config which use "bool" for FSI-XXX is not only FSI-HDMI.
> So, I will care your indication as "new function patch" in future.
>
>
> -- Guennadi ------------------------------------------
> With this config option you will have 3 SND_SOC_SH4_FSI implementations in
> the Kconfig, all selectable independently. Do you really think it makes
> sense and would work, if someone were to select more than one of those
> options at the same time?
> -------------------------------------------------------
>
> Yes. I created it for independently.
> For example, you can select FSI-AK4642 and FSI-HDMI in same time,
> and both works well for me on AP4 board now.
>
> But I guess, if you select FSI-DA7210 and FSI-HDMI,
> small patch which change FSIA <-> FSIB is needed.
>
> FSI-AK4642 : FSI-A
> FSI-DA7210 : FSI-B
> FSI-HDMI : FSI-B
>
> I think that it should be going to enable the selection of
> "FSI-A" and "FSI-B" in the future.
> But now, no way, no idea.
> It is my task
>
> By the way, about HDMI sound,
> I know that there are some HDMI monitors which can not use sound.
> Now I'm debuging about it.
>
> -- Guennadi ------------------------------------------
> > + .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
>
> Again, this I am not sure about. The datasheet says 16 to 32 bit are
> possible, but then I only see configuration for 16 to 24 bits, but in any
> case, I think, you'd want to implement .hw_params to support non-default
> formats.
> -------------------------------------------------------
>
> Yes. I should modify this .formats.
> But I can not select, test, support 32bit in current environment.
> So, please put it to my TODO list.
>
> And yes. .hw_params implementation is important for advanced support.
> But now, current HDMI sound support is still prototype (I didn't indicate though).
> So. please put it to my TODO list too.
>
> Best regards
> --
> Kuninori Morimoto
>
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2
2010-09-09 6:32 ` [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2 Guennadi Liakhovetski
@ 2010-09-09 7:07 ` Kuninori Morimoto
2010-09-09 7:17 ` Guennadi Liakhovetski
0 siblings, 1 reply; 13+ messages in thread
From: Kuninori Morimoto @ 2010-09-09 7:07 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: Linux-ALSA, Mark Brown, Liam Girdwood
Dear Guennadi
> <quote>
> Besides, I think, this will not link without CONFIG_SND_SOC.
> </quote>
>
> ? Or is it wrong? If this is right, it would be better to either add a
> "select SND_SOC" to FB_SH_MOBILE_HDMI Kconfig entry, or put calls to
> snd_soc_(un)register_codec (and all the HDMI audio code) under "#ifdef
> CONFIG_SND_SOC." The letter is, probably, less elegant.
Oh sorry. I didn't explain about it.
Yes.
My 1st idea was same as yours (add SND_SOC to Kconfig).
But I couldn't decide which is better.
I send "add select SND_SOC to Kconfig" patch.
OK ?
Best regards
--
Kuninori Morimoto
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2
2010-09-09 7:07 ` Kuninori Morimoto
@ 2010-09-09 7:17 ` Guennadi Liakhovetski
0 siblings, 0 replies; 13+ messages in thread
From: Guennadi Liakhovetski @ 2010-09-09 7:17 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: Linux-ALSA, Mark Brown, Liam Girdwood
On Thu, 9 Sep 2010, Kuninori Morimoto wrote:
>
> Dear Guennadi
>
> > <quote>
> > Besides, I think, this will not link without CONFIG_SND_SOC.
> > </quote>
> >
> > ? Or is it wrong? If this is right, it would be better to either add a
> > "select SND_SOC" to FB_SH_MOBILE_HDMI Kconfig entry, or put calls to
> > snd_soc_(un)register_codec (and all the HDMI audio code) under "#ifdef
> > CONFIG_SND_SOC." The letter is, probably, less elegant.
>
> Oh sorry. I didn't explain about it.
> Yes.
> My 1st idea was same as yours (add SND_SOC to Kconfig).
> But I couldn't decide which is better.
>
> I send "add select SND_SOC to Kconfig" patch.
> OK ?
Suits me, thanks.
Regards
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 6/5] fbdev: sh_mobile_hdmi: Add select SND_SOC to Kconfig
2010-09-09 2:46 [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2 Kuninori Morimoto
` (6 preceding siblings ...)
2010-09-09 6:32 ` [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2 Guennadi Liakhovetski
@ 2010-09-09 7:30 ` Kuninori Morimoto
2010-09-10 11:29 ` [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2 Guennadi Liakhovetski
8 siblings, 0 replies; 13+ messages in thread
From: Kuninori Morimoto @ 2010-09-09 7:30 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA, Guennadi, Liam Girdwood
This patch solve compile error when .config doesn't have
CONFIG_SND_SOC which is needed for HDMI sound.
It was reported by Guennadi as follows
Besides, I think, this will not link without CONFIG_SND_SOC.
Reported-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
drivers/video/Kconfig | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 8b31fdf..43e90b8 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -1919,6 +1919,7 @@ config FB_SH_MOBILE_HDMI
tristate "SuperH Mobile HDMI controller support"
depends on FB_SH_MOBILE_LCDC
select FB_MODE_HELPERS
+ select SND_SOC
---help---
Driver for the on-chip SH-Mobile HDMI controller.
--
1.7.0.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2
2010-09-09 2:46 [PATCH 0/5 v2] fbdev: sh_mobile_hdmi: bug fix patches V2 Kuninori Morimoto
` (7 preceding siblings ...)
2010-09-09 7:30 ` [PATCH 6/5] fbdev: sh_mobile_hdmi: Add select SND_SOC to Kconfig Kuninori Morimoto
@ 2010-09-10 11:29 ` Guennadi Liakhovetski
2010-09-10 15:08 ` Mark Brown
8 siblings, 1 reply; 13+ messages in thread
From: Guennadi Liakhovetski @ 2010-09-10 11:29 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: Linux-ALSA, Mark Brown, Liam Girdwood
On Thu, 9 Sep 2010, Kuninori Morimoto wrote:
>
> Dear Mark, Liam, Guennadi
Hi Mark
Feel free to add my
Reviewed-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
to these 6 patches.
Thanks
Guennadi
>
> These are ALSA V2 bug fix patches which are reported by Guennadi.
>
> Kuninori Morimoto (5):
> fbdev: sh_mobile_hdmi: modify noisy comment out
> fbdev: sh_mobile_hdmi: modify 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
>
> To Mark.
> Please care above patch independently from these patches.
> [alsa-devel] [PATCH 5/5] ASoC: fsi-ak4642: modiry platform_name
>
> These series are for ALSA side patches.
> I will send SH side V2 patches soon.
>
> Main diff v1 <==> v2 is
> I added Guennadi's report mail to log area on each patches,
> and care more.
> But these series still didn't care above.
> I added reasons.
>
> -- Guennadi ------------------------------------------
> > +config SND_FSI_HDMI
> > + bool "FSI-HDMI sound support"
> > + depends on SND_SOC_SH4_FSI && FB_SH_MOBILE_HDMI
> > + help
> > + This option enables generic sound support for the
> > + FSI - HDMI unit
> > +
>
> "bool" means, if someone is linking the whole ASoC into the kernel, they
> will not be able to build this as a module. Not a big deal, but you're
> stealing some freedom from the user.
> -------------------------------------------------------
>
> understand.
> But the config which use "bool" for FSI-XXX is not only FSI-HDMI.
> So, I will care your indication as "new function patch" in future.
>
>
> -- Guennadi ------------------------------------------
> With this config option you will have 3 SND_SOC_SH4_FSI implementations in
> the Kconfig, all selectable independently. Do you really think it makes
> sense and would work, if someone were to select more than one of those
> options at the same time?
> -------------------------------------------------------
>
> Yes. I created it for independently.
> For example, you can select FSI-AK4642 and FSI-HDMI in same time,
> and both works well for me on AP4 board now.
>
> But I guess, if you select FSI-DA7210 and FSI-HDMI,
> small patch which change FSIA <-> FSIB is needed.
>
> FSI-AK4642 : FSI-A
> FSI-DA7210 : FSI-B
> FSI-HDMI : FSI-B
>
> I think that it should be going to enable the selection of
> "FSI-A" and "FSI-B" in the future.
> But now, no way, no idea.
> It is my task
>
> By the way, about HDMI sound,
> I know that there are some HDMI monitors which can not use sound.
> Now I'm debuging about it.
>
> -- Guennadi ------------------------------------------
> > + .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
>
> Again, this I am not sure about. The datasheet says 16 to 32 bit are
> possible, but then I only see configuration for 16 to 24 bits, but in any
> case, I think, you'd want to implement .hw_params to support non-default
> formats.
> -------------------------------------------------------
>
> Yes. I should modify this .formats.
> But I can not select, test, support 32bit in current environment.
> So, please put it to my TODO list.
>
> And yes. .hw_params implementation is important for advanced support.
> But now, current HDMI sound support is still prototype (I didn't indicate though).
> So. please put it to my TODO list too.
>
> Best regards
> --
> Kuninori Morimoto
>
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 13+ messages in thread