[PATCH 0/4 v2] Add FSI - HDMI support V2

[PATCH 0/4 v2] Add FSI - HDMI support V2

[Kuninori Morimoto]

Dear Mark, Paul

These are FSI - HDMI prototype sound support V2

Kuninori Morimoto (4):
      fbdev: sh-mobile: Add HDMI sound type selection
      ASoC: Add sh_mobile_hdmi sound support
      ASoC: fsi-codec: Add FSI - HDMI support
      ARM: mach-shmobile: ap4evb: Add HDMI sound support

>> Paul
1st patch is depend on 
[PATCH] fbdev: sh-mobile_hdmi: remove un-necessity settings
which I sent to LinuxSH ML in 2010/08/27

>> Mark
2nd, 3rd ASoC patches are based on Mark's "for-2.6.37"

[PATCH 0/4 v2] Add FSI - HDMI support V2

[Kuninori Morimoto]

Dear Mark, Paul

These are FSI - HDMI prototype sound support V2

Kuninori Morimoto (4):
      fbdev: sh-mobile: Add HDMI sound type selection
      ASoC: Add sh_mobile_hdmi sound support
      ASoC: fsi-codec: Add FSI - HDMI support
      ARM: mach-shmobile: ap4evb: Add HDMI sound support

>> Paul
1st patch is depend on 
[PATCH] fbdev: sh-mobile_hdmi: remove un-necessity settings
which I sent to LinuxSH ML in 2010/08/27

>> Mark
2nd, 3rd ASoC patches are based on Mark's "for-2.6.37"

[PATCH 1/4 v2] fbdev: sh-mobile: Add HDMI sound type selection

[Kuninori Morimoto]

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2
 no change

 drivers/video/sh_mobile_hdmi.c |   21 ++++++++++++++++++++-
 include/video/sh_mobile_hdmi.h |   16 ++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
index afebe80..d25e348 100644
--- a/drivers/video/sh_mobile_hdmi.c
+++ b/drivers/video/sh_mobile_hdmi.c
@@ -318,6 +318,9 @@ static void sh_hdmi_video_config(struct sh_hdmi *hdmi)
  */
 static void sh_hdmi_audio_config(struct sh_hdmi *hdmi)
 {
+	u8 data;
+	struct sh_mobile_hdmi_info *pdata = hdmi->dev->platform_data;
+
 	/*
 	 * [7:4] L/R data swap control
 	 * [3:0] appropriate N[19:16]
@@ -335,7 +338,23 @@ 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
 	 */
-	hdmi_write(hdmi, 0x00, HDMI_AUDIO_SETTING_1);
+	switch (pdata->flags & HDMI_SRC_MASK) {
+	default:
+		/* FALL THROUGH */
+	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);
+		break;
+	}
+	hdmi_write(hdmi, data, HDMI_AUDIO_SETTING_1);
 
 	/* [3:0] set sending channel number for channel status */
 	hdmi_write(hdmi, 0x40, HDMI_AUDIO_SETTING_2);
diff --git a/include/video/sh_mobile_hdmi.h b/include/video/sh_mobile_hdmi.h
index 577cf18..929c2d3 100644
--- a/include/video/sh_mobile_hdmi.h
+++ b/include/video/sh_mobile_hdmi.h
@@ -14,9 +14,25 @@
 struct sh_mobile_lcdc_chan_cfg;
 struct device;
 
+/*
+ * flags format
+ *
+ * 0x0000000A
+ *
+ * A: Audio source select
+ */
+
+/* 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)
+
 struct sh_mobile_hdmi_info {
 	struct sh_mobile_lcdc_chan_cfg	*lcd_chan;
 	struct device			*lcd_dev;
+	unsigned int			 flags;
 };
 
 #endif
-- 
1.7.0.4

[PATCH 1/4 v2] fbdev: sh-mobile: Add HDMI sound type selection

[Kuninori Morimoto]

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2
 no change

 drivers/video/sh_mobile_hdmi.c |   21 ++++++++++++++++++++-
 include/video/sh_mobile_hdmi.h |   16 ++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
index afebe80..d25e348 100644
--- a/drivers/video/sh_mobile_hdmi.c
+++ b/drivers/video/sh_mobile_hdmi.c
@@ -318,6 +318,9 @@ static void sh_hdmi_video_config(struct sh_hdmi *hdmi)
  */
 static void sh_hdmi_audio_config(struct sh_hdmi *hdmi)
 {
+	u8 data;
+	struct sh_mobile_hdmi_info *pdata = hdmi->dev->platform_data;
+
 	/*
 	 * [7:4] L/R data swap control
 	 * [3:0] appropriate N[19:16]
@@ -335,7 +338,23 @@ 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
 	 */
-	hdmi_write(hdmi, 0x00, HDMI_AUDIO_SETTING_1);
+	switch (pdata->flags & HDMI_SRC_MASK) {
+	default:
+		/* FALL THROUGH */
+	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);
+		break;
+	}
+	hdmi_write(hdmi, data, HDMI_AUDIO_SETTING_1);
 
 	/* [3:0] set sending channel number for channel status */
 	hdmi_write(hdmi, 0x40, HDMI_AUDIO_SETTING_2);
diff --git a/include/video/sh_mobile_hdmi.h b/include/video/sh_mobile_hdmi.h
index 577cf18..929c2d3 100644
--- a/include/video/sh_mobile_hdmi.h
+++ b/include/video/sh_mobile_hdmi.h
@@ -14,9 +14,25 @@
 struct sh_mobile_lcdc_chan_cfg;
 struct device;
 
+/*
+ * flags format
+ *
+ * 0x0000000A
+ *
+ * A: Audio source select
+ */
+
+/* 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)
+
 struct sh_mobile_hdmi_info {
 	struct sh_mobile_lcdc_chan_cfg	*lcd_chan;
 	struct device			*lcd_dev;
+	unsigned int			 flags;
 };
 
 #endif
-- 
1.7.0.4

[PATCH 2/4 v2] ASoC: fsi-codec: Add FSI - HDMI support

[Kuninori Morimoto]

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2

o based on latest Mark's "for-2.6.37"

 sound/soc/sh/Kconfig    |    7 +++++
 sound/soc/sh/Makefile   |    2 +
 sound/soc/sh/fsi-hdmi.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/sh/fsi-hdmi.c

diff --git a/sound/soc/sh/Kconfig b/sound/soc/sh/Kconfig
index 52d7e8e..6b224d2 100644
--- a/sound/soc/sh/Kconfig
+++ b/sound/soc/sh/Kconfig
@@ -62,6 +62,13 @@ config SND_FSI_DA7210
 	  This option enables generic sound support for the
 	  FSI - DA7210 unit
 
+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
+
 config SND_SIU_MIGOR
 	tristate "SIU sound support on Migo-R"
 	depends on SH_MIGOR
diff --git a/sound/soc/sh/Makefile b/sound/soc/sh/Makefile
index 8a5a192..94476d4 100644
--- a/sound/soc/sh/Makefile
+++ b/sound/soc/sh/Makefile
@@ -16,9 +16,11 @@ obj-$(CONFIG_SND_SOC_SH4_SIU)	+= snd-soc-siu.o
 snd-soc-sh7760-ac97-objs	:= sh7760-ac97.o
 snd-soc-fsi-ak4642-objs		:= fsi-ak4642.o
 snd-soc-fsi-da7210-objs		:= fsi-da7210.o
+snd-soc-fsi-hdmi-objs		:= fsi-hdmi.o
 snd-soc-migor-objs		:= migor.o
 
 obj-$(CONFIG_SND_SH7760_AC97)	+= snd-soc-sh7760-ac97.o
 obj-$(CONFIG_SND_FSI_AK4642)	+= snd-soc-fsi-ak4642.o
 obj-$(CONFIG_SND_FSI_DA7210)	+= snd-soc-fsi-da7210.o
+obj-$(CONFIG_SND_FSI_HDMI)	+= snd-soc-fsi-hdmi.o
 obj-$(CONFIG_SND_SIU_MIGOR)	+= snd-soc-migor.o
diff --git a/sound/soc/sh/fsi-hdmi.c b/sound/soc/sh/fsi-hdmi.c
new file mode 100644
index 0000000..950e3e0
--- /dev/null
+++ b/sound/soc/sh/fsi-hdmi.c
@@ -0,0 +1,61 @@
+/*
+ * FSI - HDMI sound support
+ *
+ * Copyright (C) 2010 Renesas Solutions Corp.
+ * Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#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",
+	.stream_name	= "HDMI",
+	.cpu_dai_name	= "fsib-dai", /* fsi B */
+	.codec_dai_name	= "sh_mobile_hdmi-hifi",
+	.platform_name	= "sh_fsi2",
+	.codec_name	= "sh-mobile-hdmi",
+};
+
+static struct snd_soc_card fsi_soc_card  = {
+	.name		= "FSI",
+	.dai_link	= &fsi_dai_link,
+	.num_links	= 1,
+};
+
+static struct platform_device *fsi_snd_device;
+
+static int __init fsi_hdmi_init(void)
+{
+	int ret = -ENOMEM;
+
+	fsi_snd_device = platform_device_alloc("soc-audio", FSI_PORT_B);
+	if (!fsi_snd_device)
+		goto out;
+
+	platform_set_drvdata(fsi_snd_device, &fsi_soc_card);
+	ret = platform_device_add(fsi_snd_device);
+
+	if (ret)
+		platform_device_put(fsi_snd_device);
+
+out:
+	return ret;
+}
+
+static void __exit fsi_hdmi_exit(void)
+{
+	platform_device_unregister(fsi_snd_device);
+}
+
+module_init(fsi_hdmi_init);
+module_exit(fsi_hdmi_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Generic SH4 FSI-HDMI sound card");
+MODULE_AUTHOR("Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>");
-- 
1.7.0.4

[PATCH 2/4 v2] ASoC: fsi-codec: Add FSI - HDMI support

[Kuninori Morimoto]

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2

o based on latest Mark's "for-2.6.37"

 sound/soc/sh/Kconfig    |    7 +++++
 sound/soc/sh/Makefile   |    2 +
 sound/soc/sh/fsi-hdmi.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/sh/fsi-hdmi.c

diff --git a/sound/soc/sh/Kconfig b/sound/soc/sh/Kconfig
index 52d7e8e..6b224d2 100644
--- a/sound/soc/sh/Kconfig
+++ b/sound/soc/sh/Kconfig
@@ -62,6 +62,13 @@ config SND_FSI_DA7210
 	  This option enables generic sound support for the
 	  FSI - DA7210 unit
 
+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
+
 config SND_SIU_MIGOR
 	tristate "SIU sound support on Migo-R"
 	depends on SH_MIGOR
diff --git a/sound/soc/sh/Makefile b/sound/soc/sh/Makefile
index 8a5a192..94476d4 100644
--- a/sound/soc/sh/Makefile
+++ b/sound/soc/sh/Makefile
@@ -16,9 +16,11 @@ obj-$(CONFIG_SND_SOC_SH4_SIU)	+= snd-soc-siu.o
 snd-soc-sh7760-ac97-objs	:= sh7760-ac97.o
 snd-soc-fsi-ak4642-objs		:= fsi-ak4642.o
 snd-soc-fsi-da7210-objs		:= fsi-da7210.o
+snd-soc-fsi-hdmi-objs		:= fsi-hdmi.o
 snd-soc-migor-objs		:= migor.o
 
 obj-$(CONFIG_SND_SH7760_AC97)	+= snd-soc-sh7760-ac97.o
 obj-$(CONFIG_SND_FSI_AK4642)	+= snd-soc-fsi-ak4642.o
 obj-$(CONFIG_SND_FSI_DA7210)	+= snd-soc-fsi-da7210.o
+obj-$(CONFIG_SND_FSI_HDMI)	+= snd-soc-fsi-hdmi.o
 obj-$(CONFIG_SND_SIU_MIGOR)	+= snd-soc-migor.o
diff --git a/sound/soc/sh/fsi-hdmi.c b/sound/soc/sh/fsi-hdmi.c
new file mode 100644
index 0000000..950e3e0
--- /dev/null
+++ b/sound/soc/sh/fsi-hdmi.c
@@ -0,0 +1,61 @@
+/*
+ * FSI - HDMI sound support
+ *
+ * Copyright (C) 2010 Renesas Solutions Corp.
+ * Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#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",
+	.stream_name	= "HDMI",
+	.cpu_dai_name	= "fsib-dai", /* fsi B */
+	.codec_dai_name	= "sh_mobile_hdmi-hifi",
+	.platform_name	= "sh_fsi2",
+	.codec_name	= "sh-mobile-hdmi",
+};
+
+static struct snd_soc_card fsi_soc_card  = {
+	.name		= "FSI",
+	.dai_link	= &fsi_dai_link,
+	.num_links	= 1,
+};
+
+static struct platform_device *fsi_snd_device;
+
+static int __init fsi_hdmi_init(void)
+{
+	int ret = -ENOMEM;
+
+	fsi_snd_device = platform_device_alloc("soc-audio", FSI_PORT_B);
+	if (!fsi_snd_device)
+		goto out;
+
+	platform_set_drvdata(fsi_snd_device, &fsi_soc_card);
+	ret = platform_device_add(fsi_snd_device);
+
+	if (ret)
+		platform_device_put(fsi_snd_device);
+
+out:
+	return ret;
+}
+
+static void __exit fsi_hdmi_exit(void)
+{
+	platform_device_unregister(fsi_snd_device);
+}
+
+module_init(fsi_hdmi_init);
+module_exit(fsi_hdmi_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Generic SH4 FSI-HDMI sound card");
+MODULE_AUTHOR("Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>");
-- 
1.7.0.4

[PATCH 4/4 v2] ARM: mach-shmobile: ap4evb: Add HDMI sound support

[Kuninori Morimoto]

It support only 48kHz frame rate

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2
  no change

 arch/arm/mach-shmobile/board-ap4evb.c |   56 ++++++++++++++++++++++++++++++++-
 1 files changed, 55 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index 23d472f..1545277 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -515,6 +515,8 @@ static struct platform_device *qhd_devices[] __initdata = {
 /* FSI */
 #define IRQ_FSI		evt2irq(0x1840)
 #define FSIACKCR	0xE6150018
+#define FSIDIV		0xFE1F8000
+#define FSIDIV_SIZE	32
 static void fsiackcr_init(struct clk *clk)
 {
 	u32 status = __raw_readl(clk->enable_reg);
@@ -535,12 +537,58 @@ static struct clk fsiackcr_clk = {
 	.rate		= 0, /* unknown */
 };
 
+static int fsi_set_rate(int is_porta, int rate)
+{
+	struct clk *clk;
+	void __iomem *base;
+	int ret = 0;
+
+	/* set_rate is not needed if port A */
+	if (is_porta)
+		return 0;
+
+	clk = clk_get(NULL, "fsib_clk");
+	if (IS_ERR(clk))
+		return -EINVAL;
+
+	base = ioremap_nocache(FSIDIV, FSIDIV_SIZE);
+	if (!base) {
+		ret = -ENXIO;
+		goto fsi_set_rate_err;
+	}
+
+	switch (rate) {
+	case 48000:
+		clk_set_rate(clk, clk_round_rate(clk, 85428000));
+		__raw_writel(0x00070003, base + 0x8);
+		ret = SH_FSI_ACKMD_256 | SH_FSI_BPFMD_64;
+		break;
+	default:
+		pr_err("unsupported rate in FSI2 port B\n");
+		ret = -EINVAL;
+		break;
+	}
+
+	iounmap(base);
+
+fsi_set_rate_err:
+	clk_put(clk);
+
+	return ret;
+}
+
 static struct sh_fsi_platform_info fsi_info = {
 	.porta_flags = SH_FSI_BRS_INV |
 		       SH_FSI_OUT_SLAVE_MODE |
 		       SH_FSI_IN_SLAVE_MODE |
 		       SH_FSI_OFMT(PCM) |
 		       SH_FSI_IFMT(PCM),
+
+	.portb_flags = SH_FSI_BRS_INV |
+		       SH_FSI_BRM_INV |
+		       SH_FSI_LRS_INV |
+		       SH_FSI_OFMT(SPDIF),
+	.set_rate = fsi_set_rate,
 };
 
 static struct resource fsi_resources[] = {
@@ -624,6 +672,7 @@ static struct platform_device lcdc1_device = {
 static struct sh_mobile_hdmi_info hdmi_info = {
 	.lcd_chan = &sh_mobile_lcdc1_info.ch[0],
 	.lcd_dev = &lcdc1_device.dev,
+	.flags = HDMI_SRC_SPDIF,
 };
 
 static struct resource hdmi_resources[] = {
@@ -825,6 +874,7 @@ static void __init ap4evb_map_io(void)
 
 #define GPIO_PORT9CR	0xE6051009
 #define GPIO_PORT10CR	0xE605100A
+#define USCCR1		0xE6058144
 static void __init ap4evb_init(void)
 {
 	u32 srcr4;
@@ -909,7 +959,7 @@ static void __init ap4evb_init(void)
 	/* setup USB phy */
 	__raw_writew(0x8a0a, 0xE6058130);	/* USBCR2 */
 
-	/* enable FSI2 */
+	/* enable FSI2 port A (ak4643) */
 	gpio_request(GPIO_FN_FSIAIBT,	NULL);
 	gpio_request(GPIO_FN_FSIAILR,	NULL);
 	gpio_request(GPIO_FN_FSIAISLD,	NULL);
@@ -922,6 +972,10 @@ static void __init ap4evb_init(void)
 	gpio_no_direction(GPIO_PORT9CR);  /* FSIAOBT needs no direction */
 	gpio_no_direction(GPIO_PORT10CR); /* FSIAOLR needs no direction */
 
+	/* setup FSI2 port B (HDMI) */
+	gpio_request(GPIO_FN_FSIBCK, NULL);
+	__raw_writew(__raw_readw(USCCR1) & ~(1 << 6), USCCR1); /* use SPDIF */
+
 	/* set SPU2 clock to 119.6 MHz */
 	clk = clk_get(NULL, "spu_clk");
 	if (!IS_ERR(clk)) {
-- 
1.7.0.4

[PATCH 4/4 v2] ARM: mach-shmobile: ap4evb: Add HDMI sound support

[Kuninori Morimoto]

It support only 48kHz frame rate

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2
  no change

 arch/arm/mach-shmobile/board-ap4evb.c |   56 ++++++++++++++++++++++++++++++++-
 1 files changed, 55 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index 23d472f..1545277 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -515,6 +515,8 @@ static struct platform_device *qhd_devices[] __initdata = {
 /* FSI */
 #define IRQ_FSI		evt2irq(0x1840)
 #define FSIACKCR	0xE6150018
+#define FSIDIV		0xFE1F8000
+#define FSIDIV_SIZE	32
 static void fsiackcr_init(struct clk *clk)
 {
 	u32 status = __raw_readl(clk->enable_reg);
@@ -535,12 +537,58 @@ static struct clk fsiackcr_clk = {
 	.rate		= 0, /* unknown */
 };
 
+static int fsi_set_rate(int is_porta, int rate)
+{
+	struct clk *clk;
+	void __iomem *base;
+	int ret = 0;
+
+	/* set_rate is not needed if port A */
+	if (is_porta)
+		return 0;
+
+	clk = clk_get(NULL, "fsib_clk");
+	if (IS_ERR(clk))
+		return -EINVAL;
+
+	base = ioremap_nocache(FSIDIV, FSIDIV_SIZE);
+	if (!base) {
+		ret = -ENXIO;
+		goto fsi_set_rate_err;
+	}
+
+	switch (rate) {
+	case 48000:
+		clk_set_rate(clk, clk_round_rate(clk, 85428000));
+		__raw_writel(0x00070003, base + 0x8);
+		ret = SH_FSI_ACKMD_256 | SH_FSI_BPFMD_64;
+		break;
+	default:
+		pr_err("unsupported rate in FSI2 port B\n");
+		ret = -EINVAL;
+		break;
+	}
+
+	iounmap(base);
+
+fsi_set_rate_err:
+	clk_put(clk);
+
+	return ret;
+}
+
 static struct sh_fsi_platform_info fsi_info = {
 	.porta_flags = SH_FSI_BRS_INV |
 		       SH_FSI_OUT_SLAVE_MODE |
 		       SH_FSI_IN_SLAVE_MODE |
 		       SH_FSI_OFMT(PCM) |
 		       SH_FSI_IFMT(PCM),
+
+	.portb_flags = SH_FSI_BRS_INV |
+		       SH_FSI_BRM_INV |
+		       SH_FSI_LRS_INV |
+		       SH_FSI_OFMT(SPDIF),
+	.set_rate = fsi_set_rate,
 };
 
 static struct resource fsi_resources[] = {
@@ -624,6 +672,7 @@ static struct platform_device lcdc1_device = {
 static struct sh_mobile_hdmi_info hdmi_info = {
 	.lcd_chan = &sh_mobile_lcdc1_info.ch[0],
 	.lcd_dev = &lcdc1_device.dev,
+	.flags = HDMI_SRC_SPDIF,
 };
 
 static struct resource hdmi_resources[] = {
@@ -825,6 +874,7 @@ static void __init ap4evb_map_io(void)
 
 #define GPIO_PORT9CR	0xE6051009
 #define GPIO_PORT10CR	0xE605100A
+#define USCCR1		0xE6058144
 static void __init ap4evb_init(void)
 {
 	u32 srcr4;
@@ -909,7 +959,7 @@ static void __init ap4evb_init(void)
 	/* setup USB phy */
 	__raw_writew(0x8a0a, 0xE6058130);	/* USBCR2 */
 
-	/* enable FSI2 */
+	/* enable FSI2 port A (ak4643) */
 	gpio_request(GPIO_FN_FSIAIBT,	NULL);
 	gpio_request(GPIO_FN_FSIAILR,	NULL);
 	gpio_request(GPIO_FN_FSIAISLD,	NULL);
@@ -922,6 +972,10 @@ static void __init ap4evb_init(void)
 	gpio_no_direction(GPIO_PORT9CR);  /* FSIAOBT needs no direction */
 	gpio_no_direction(GPIO_PORT10CR); /* FSIAOLR needs no direction */
 
+	/* setup FSI2 port B (HDMI) */
+	gpio_request(GPIO_FN_FSIBCK, NULL);
+	__raw_writew(__raw_readw(USCCR1) & ~(1 << 6), USCCR1); /* use SPDIF */
+
 	/* set SPU2 clock to 119.6 MHz */
 	clk = clk_get(NULL, "spu_clk");
 	if (!IS_ERR(clk)) {
-- 
1.7.0.4

[PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Kuninori Morimoto]

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2

o based on Mark's latest "for-2.6.37"

 drivers/video/sh_mobile_hdmi.c |   65 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 65 insertions(+), 0 deletions(-)

diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
index d25e348..f14c58a 100644
--- a/drivers/video/sh_mobile_hdmi.c
+++ b/drivers/video/sh_mobile_hdmi.c
@@ -22,6 +22,8 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/workqueue.h>
+#include <sound/soc-dapm.h>
+#include <sound/initval.h>
 
 #include <video/sh_mobile_hdmi.h>
 #include <video/sh_mobile_lcdc.h>
@@ -222,6 +224,62 @@ static u8 hdmi_read(struct sh_hdmi *hdmi, u8 reg)
 	return ioread8(hdmi->base + reg);
 }
 
+/************************************************************************
+
+
+			HDMI sound
+
+
+************************************************************************/
+static unsigned int sh_hdmi_snd_read(struct snd_soc_codec *codec,
+				     unsigned int reg)
+{
+	struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
+
+	return hdmi_read(hdmi, reg);
+}
+
+static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
+			     unsigned int reg,
+			     unsigned int value)
+{
+	struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
+
+	hdmi_write(hdmi, value, reg);
+	return 0;
+}
+
+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,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+	},
+};
+
+static int sh_hdmi_snd_probe(struct snd_soc_codec *codec)
+{
+	dev_info(codec->dev, "SH Mobile HDMI Audio Codec");
+
+	return 0;
+}
+
+static struct snd_soc_codec_driver soc_codec_dev_sh_hdmi = {
+	.probe		= sh_hdmi_snd_probe,
+	.read		= sh_hdmi_snd_read,
+	.write		= sh_hdmi_snd_write,
+};
+
+/************************************************************************
+
+
+			HDMI video
+
+
+************************************************************************/
 /* 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");
@@ -901,6 +964,8 @@ static int __exit sh_hdmi_remove(struct platform_device *pdev)
 	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	int irq = platform_get_irq(pdev, 0);
 
+	snd_soc_unregister_codec(&pdev->dev);
+
 	pdata->lcd_chan->board_cfg.display_on = NULL;
 	pdata->lcd_chan->board_cfg.display_off = NULL;
 	pdata->lcd_chan->board_cfg.board_data = NULL;
-- 
1.7.0.4

[PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Kuninori Morimoto]

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2

o based on Mark's latest "for-2.6.37"

 drivers/video/sh_mobile_hdmi.c |   65 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 65 insertions(+), 0 deletions(-)

diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
index d25e348..f14c58a 100644
--- a/drivers/video/sh_mobile_hdmi.c
+++ b/drivers/video/sh_mobile_hdmi.c
@@ -22,6 +22,8 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/workqueue.h>
+#include <sound/soc-dapm.h>
+#include <sound/initval.h>
 
 #include <video/sh_mobile_hdmi.h>
 #include <video/sh_mobile_lcdc.h>
@@ -222,6 +224,62 @@ static u8 hdmi_read(struct sh_hdmi *hdmi, u8 reg)
 	return ioread8(hdmi->base + reg);
 }
 
+/************************************************************************
+
+
+			HDMI sound
+
+
+************************************************************************/
+static unsigned int sh_hdmi_snd_read(struct snd_soc_codec *codec,
+				     unsigned int reg)
+{
+	struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
+
+	return hdmi_read(hdmi, reg);
+}
+
+static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
+			     unsigned int reg,
+			     unsigned int value)
+{
+	struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
+
+	hdmi_write(hdmi, value, reg);
+	return 0;
+}
+
+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,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+	},
+};
+
+static int sh_hdmi_snd_probe(struct snd_soc_codec *codec)
+{
+	dev_info(codec->dev, "SH Mobile HDMI Audio Codec");
+
+	return 0;
+}
+
+static struct snd_soc_codec_driver soc_codec_dev_sh_hdmi = {
+	.probe		= sh_hdmi_snd_probe,
+	.read		= sh_hdmi_snd_read,
+	.write		= sh_hdmi_snd_write,
+};
+
+/************************************************************************
+
+
+			HDMI video
+
+
+************************************************************************/
 /* 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");
@@ -901,6 +964,8 @@ static int __exit sh_hdmi_remove(struct platform_device *pdev)
 	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	int irq = platform_get_irq(pdev, 0);
 
+	snd_soc_unregister_codec(&pdev->dev);
+
 	pdata->lcd_chan->board_cfg.display_on = NULL;
 	pdata->lcd_chan->board_cfg.display_off = NULL;
 	pdata->lcd_chan->board_cfg.board_data = NULL;
-- 
1.7.0.4

Re: [alsa-devel] [PATCH 0/4 v2] Add FSI - HDMI support V2

[Liam Girdwood]

On Tue, 2010-08-31 at 14:45 +0900, Kuninori Morimoto wrote:
> Dear Mark, Paul
> 
> These are FSI - HDMI prototype sound support V2
> 
> Kuninori Morimoto (4):
>       fbdev: sh-mobile: Add HDMI sound type selection
>       ASoC: Add sh_mobile_hdmi sound support
>       ASoC: fsi-codec: Add FSI - HDMI support
>       ARM: mach-shmobile: ap4evb: Add HDMI sound support
> 
> >> Paul
> 1st patch is depend on 
> [PATCH] fbdev: sh-mobile_hdmi: remove un-necessity settings
> which I sent to LinuxSH ML in 2010/08/27
> 
> >> Mark
> 2nd, 3rd ASoC patches are based on Mark's "for-2.6.37"

ASoC parts:-

Acked-by: Liam Girdwood <lrg@slimlogic.co.uk>
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [alsa-devel] [PATCH 0/4 v2] Add FSI - HDMI support V2

[Liam Girdwood]

On Tue, 2010-08-31 at 14:45 +0900, Kuninori Morimoto wrote:
> Dear Mark, Paul
> 
> These are FSI - HDMI prototype sound support V2
> 
> Kuninori Morimoto (4):
>       fbdev: sh-mobile: Add HDMI sound type selection
>       ASoC: Add sh_mobile_hdmi sound support
>       ASoC: fsi-codec: Add FSI - HDMI support
>       ARM: mach-shmobile: ap4evb: Add HDMI sound support
> 
> >> Paul
> 1st patch is depend on 
> [PATCH] fbdev: sh-mobile_hdmi: remove un-necessity settings
> which I sent to LinuxSH ML in 2010/08/27
> 
> >> Mark
> 2nd, 3rd ASoC patches are based on Mark's "for-2.6.37"

ASoC parts:-

Acked-by: Liam Girdwood <lrg@slimlogic.co.uk>
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 1/4 v2] fbdev: sh-mobile: Add HDMI sound type selection

[Mark Brown]

On Tue, Aug 31, 2010 at 02:46:41PM +0900, Kuninori Morimoto wrote:
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Applied, thanks.

Re: [PATCH 1/4 v2] fbdev: sh-mobile: Add HDMI sound type selection

[Mark Brown]

On Tue, Aug 31, 2010 at 02:46:41PM +0900, Kuninori Morimoto wrote:
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Applied, thanks.

Re: [PATCH 2/4 v2] ASoC: fsi-codec: Add FSI - HDMI support

[Mark Brown]

On Tue, Aug 31, 2010 at 02:46:53PM +0900, Kuninori Morimoto wrote:
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Applied, thanks.

Re: [PATCH 2/4 v2] ASoC: fsi-codec: Add FSI - HDMI support

[Mark Brown]

On Tue, Aug 31, 2010 at 02:46:53PM +0900, Kuninori Morimoto wrote:
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Applied, thanks.

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Mark Brown]

On Tue, Aug 31, 2010 at 02:47:07PM +0900, Kuninori Morimoto wrote:
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Applied, thanks.

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Mark Brown]

On Tue, Aug 31, 2010 at 02:47:07PM +0900, Kuninori Morimoto wrote:
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Applied, thanks.

Re: [PATCH 4/4 v2] ARM: mach-shmobile: ap4evb: Add HDMI sound

[Mark Brown]

On Tue, Aug 31, 2010 at 02:46:58PM +0900, Kuninori Morimoto wrote:
> It support only 48kHz frame rate
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

I can apply this if required but it seems to make sense for it to go via
the SH tree.

Re: [PATCH 4/4 v2] ARM: mach-shmobile: ap4evb: Add HDMI sound support

[Mark Brown]

On Tue, Aug 31, 2010 at 02:46:58PM +0900, Kuninori Morimoto wrote:
> It support only 48kHz frame rate
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

I can apply this if required but it seems to make sense for it to go via
the SH tree.

Re: [PATCH 1/4 v2] fbdev: sh-mobile: Add HDMI sound type selection

[Guennadi Liakhovetski]

Hi Morimoto-san

On Tue, 31 Aug 2010, Kuninori Morimoto wrote:

> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> v1 -> v2
>  no change
> 
>  drivers/video/sh_mobile_hdmi.c |   21 ++++++++++++++++++++-
>  include/video/sh_mobile_hdmi.h |   16 ++++++++++++++++
>  2 files changed, 36 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
> index afebe80..d25e348 100644
> --- a/drivers/video/sh_mobile_hdmi.c
> +++ b/drivers/video/sh_mobile_hdmi.c
> @@ -318,6 +318,9 @@ static void sh_hdmi_video_config(struct sh_hdmi *hdmi)
>   */
>  static void sh_hdmi_audio_config(struct sh_hdmi *hdmi)
>  {
> +	u8 data;
> +	struct sh_mobile_hdmi_info *pdata = hdmi->dev->platform_data;
> +
>  	/*
>  	 * [7:4] L/R data swap control
>  	 * [3:0] appropriate N[19:16]
> @@ -335,7 +338,23 @@ 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
>  	 */
> -	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;)

> +	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.

> +		break;
> +	}
> +	hdmi_write(hdmi, data, HDMI_AUDIO_SETTING_1);
>  
>  	/* [3:0] set sending channel number for channel status */
>  	hdmi_write(hdmi, 0x40, HDMI_AUDIO_SETTING_2);
> diff --git a/include/video/sh_mobile_hdmi.h b/include/video/sh_mobile_hdmi.h
> index 577cf18..929c2d3 100644
> --- a/include/video/sh_mobile_hdmi.h
> +++ b/include/video/sh_mobile_hdmi.h
> @@ -14,9 +14,25 @@
>  struct sh_mobile_lcdc_chan_cfg;
>  struct device;
>  
> +/*
> + * flags format
> + *
> + * 0x0000000A
> + *
> + * A: Audio source select
> + */
> +
> +/* 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.

> +
>  struct sh_mobile_hdmi_info {
>  	struct sh_mobile_lcdc_chan_cfg	*lcd_chan;
>  	struct device			*lcd_dev;
> +	unsigned int			 flags;
>  };
>  
>  #endif
> -- 
> 1.7.0.4

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 1/4 v2] fbdev: sh-mobile: Add HDMI sound type selection

[Guennadi Liakhovetski]

Hi Morimoto-san

On Tue, 31 Aug 2010, Kuninori Morimoto wrote:

> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> v1 -> v2
>  no change
> 
>  drivers/video/sh_mobile_hdmi.c |   21 ++++++++++++++++++++-
>  include/video/sh_mobile_hdmi.h |   16 ++++++++++++++++
>  2 files changed, 36 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
> index afebe80..d25e348 100644
> --- a/drivers/video/sh_mobile_hdmi.c
> +++ b/drivers/video/sh_mobile_hdmi.c
> @@ -318,6 +318,9 @@ static void sh_hdmi_video_config(struct sh_hdmi *hdmi)
>   */
>  static void sh_hdmi_audio_config(struct sh_hdmi *hdmi)
>  {
> +	u8 data;
> +	struct sh_mobile_hdmi_info *pdata = hdmi->dev->platform_data;
> +
>  	/*
>  	 * [7:4] L/R data swap control
>  	 * [3:0] appropriate N[19:16]
> @@ -335,7 +338,23 @@ 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
>  	 */
> -	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;)

> +	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.

> +		break;
> +	}
> +	hdmi_write(hdmi, data, HDMI_AUDIO_SETTING_1);
>  
>  	/* [3:0] set sending channel number for channel status */
>  	hdmi_write(hdmi, 0x40, HDMI_AUDIO_SETTING_2);
> diff --git a/include/video/sh_mobile_hdmi.h b/include/video/sh_mobile_hdmi.h
> index 577cf18..929c2d3 100644
> --- a/include/video/sh_mobile_hdmi.h
> +++ b/include/video/sh_mobile_hdmi.h
> @@ -14,9 +14,25 @@
>  struct sh_mobile_lcdc_chan_cfg;
>  struct device;
>  
> +/*
> + * flags format
> + *
> + * 0x0000000A
> + *
> + * A: Audio source select
> + */
> +
> +/* 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.

> +
>  struct sh_mobile_hdmi_info {
>  	struct sh_mobile_lcdc_chan_cfg	*lcd_chan;
>  	struct device			*lcd_dev;
> +	unsigned int			 flags;
>  };
>  
>  #endif
> -- 
> 1.7.0.4

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Guennadi Liakhovetski]

On Tue, 31 Aug 2010, Kuninori Morimoto wrote:

> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> v1 -> v2
> 
> o based on Mark's latest "for-2.6.37"
> 
>  drivers/video/sh_mobile_hdmi.c |   65 ++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 65 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
> index d25e348..f14c58a 100644
> --- a/drivers/video/sh_mobile_hdmi.c
> +++ b/drivers/video/sh_mobile_hdmi.c
> @@ -22,6 +22,8 @@
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  #include <linux/workqueue.h>
> +#include <sound/soc-dapm.h>
> +#include <sound/initval.h>
>  
>  #include <video/sh_mobile_hdmi.h>
>  #include <video/sh_mobile_lcdc.h>
> @@ -222,6 +224,62 @@ static u8 hdmi_read(struct sh_hdmi *hdmi, u8 reg)
>  	return ioread8(hdmi->base + reg);
>  }
>  
> +/************************************************************************
> +
> +
> +			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
 */

?

> +static unsigned int sh_hdmi_snd_read(struct snd_soc_codec *codec,
> +				     unsigned int reg)
> +{
> +	struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
> +
> +	return hdmi_read(hdmi, reg);
> +}
> +
> +static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
> +			     unsigned int reg,
> +			     unsigned int value)
> +{
> +	struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
> +
> +	hdmi_write(hdmi, value, reg);
> +	return 0;
> +}

Are these two actually needed? As long as you don't have a register cache 
- no need for these?

> +
> +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.

> +		.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.

> +	},
> +};
> +
> +static int sh_hdmi_snd_probe(struct snd_soc_codec *codec)
> +{
> +	dev_info(codec->dev, "SH Mobile HDMI Audio Codec");
> +
> +	return 0;
> +}
> +
> +static struct snd_soc_codec_driver soc_codec_dev_sh_hdmi = {
> +	.probe		= sh_hdmi_snd_probe,
> +	.read		= sh_hdmi_snd_read,
> +	.write		= sh_hdmi_snd_write,
> +};
> +
> +/************************************************************************
> +
> +
> +			HDMI video
> +
> +
> +************************************************************************/

See above - 7 lines seem to be an overkill to me.

>  /* 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);
 

to the error path.

Besides, I think, this will not link without CONFIG_SND_SOC.

> @@ -901,6 +964,8 @@ static int __exit sh_hdmi_remove(struct platform_device *pdev)
>  	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	int irq = platform_get_irq(pdev, 0);
>  
> +	snd_soc_unregister_codec(&pdev->dev);
> +
>  	pdata->lcd_chan->board_cfg.display_on = NULL;
>  	pdata->lcd_chan->board_cfg.display_off = NULL;
>  	pdata->lcd_chan->board_cfg.board_data = NULL;
> -- 
> 1.7.0.4

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Guennadi Liakhovetski]

On Tue, 31 Aug 2010, Kuninori Morimoto wrote:

> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> v1 -> v2
> 
> o based on Mark's latest "for-2.6.37"
> 
>  drivers/video/sh_mobile_hdmi.c |   65 ++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 65 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
> index d25e348..f14c58a 100644
> --- a/drivers/video/sh_mobile_hdmi.c
> +++ b/drivers/video/sh_mobile_hdmi.c
> @@ -22,6 +22,8 @@
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  #include <linux/workqueue.h>
> +#include <sound/soc-dapm.h>
> +#include <sound/initval.h>
>  
>  #include <video/sh_mobile_hdmi.h>
>  #include <video/sh_mobile_lcdc.h>
> @@ -222,6 +224,62 @@ static u8 hdmi_read(struct sh_hdmi *hdmi, u8 reg)
>  	return ioread8(hdmi->base + reg);
>  }
>  
> +/************************************************************************
> +
> +
> +			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
 */

?

> +static unsigned int sh_hdmi_snd_read(struct snd_soc_codec *codec,
> +				     unsigned int reg)
> +{
> +	struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
> +
> +	return hdmi_read(hdmi, reg);
> +}
> +
> +static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
> +			     unsigned int reg,
> +			     unsigned int value)
> +{
> +	struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
> +
> +	hdmi_write(hdmi, value, reg);
> +	return 0;
> +}

Are these two actually needed? As long as you don't have a register cache 
- no need for these?

> +
> +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.

> +		.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.

> +	},
> +};
> +
> +static int sh_hdmi_snd_probe(struct snd_soc_codec *codec)
> +{
> +	dev_info(codec->dev, "SH Mobile HDMI Audio Codec");
> +
> +	return 0;
> +}
> +
> +static struct snd_soc_codec_driver soc_codec_dev_sh_hdmi = {
> +	.probe		= sh_hdmi_snd_probe,
> +	.read		= sh_hdmi_snd_read,
> +	.write		= sh_hdmi_snd_write,
> +};
> +
> +/************************************************************************
> +
> +
> +			HDMI video
> +
> +
> +************************************************************************/

See above - 7 lines seem to be an overkill to me.

>  /* 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);
 

to the error path.

Besides, I think, this will not link without CONFIG_SND_SOC.

> @@ -901,6 +964,8 @@ static int __exit sh_hdmi_remove(struct platform_device *pdev)
>  	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	int irq = platform_get_irq(pdev, 0);
>  
> +	snd_soc_unregister_codec(&pdev->dev);
> +
>  	pdata->lcd_chan->board_cfg.display_on = NULL;
>  	pdata->lcd_chan->board_cfg.display_off = NULL;
>  	pdata->lcd_chan->board_cfg.board_data = NULL;
> -- 
> 1.7.0.4

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 2/4 v2] ASoC: fsi-codec: Add FSI - HDMI support

[Guennadi Liakhovetski]

On Tue, 31 Aug 2010, Kuninori Morimoto wrote:

> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> v1 -> v2
> 
> o based on latest Mark's "for-2.6.37"
> 
>  sound/soc/sh/Kconfig    |    7 +++++
>  sound/soc/sh/Makefile   |    2 +
>  sound/soc/sh/fsi-hdmi.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 70 insertions(+), 0 deletions(-)
>  create mode 100644 sound/soc/sh/fsi-hdmi.c
> 
> diff --git a/sound/soc/sh/Kconfig b/sound/soc/sh/Kconfig
> index 52d7e8e..6b224d2 100644
> --- a/sound/soc/sh/Kconfig
> +++ b/sound/soc/sh/Kconfig
> @@ -62,6 +62,13 @@ config SND_FSI_DA7210
>  	  This option enables generic sound support for the
>  	  FSI - DA7210 unit
>  
> +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.

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?

>  config SND_SIU_MIGOR
>  	tristate "SIU sound support on Migo-R"
>  	depends on SH_MIGOR
> diff --git a/sound/soc/sh/Makefile b/sound/soc/sh/Makefile
> index 8a5a192..94476d4 100644
> --- a/sound/soc/sh/Makefile
> +++ b/sound/soc/sh/Makefile
> @@ -16,9 +16,11 @@ obj-$(CONFIG_SND_SOC_SH4_SIU)	+= snd-soc-siu.o
>  snd-soc-sh7760-ac97-objs	:= sh7760-ac97.o
>  snd-soc-fsi-ak4642-objs		:= fsi-ak4642.o
>  snd-soc-fsi-da7210-objs		:= fsi-da7210.o
> +snd-soc-fsi-hdmi-objs		:= fsi-hdmi.o
>  snd-soc-migor-objs		:= migor.o
>  
>  obj-$(CONFIG_SND_SH7760_AC97)	+= snd-soc-sh7760-ac97.o
>  obj-$(CONFIG_SND_FSI_AK4642)	+= snd-soc-fsi-ak4642.o
>  obj-$(CONFIG_SND_FSI_DA7210)	+= snd-soc-fsi-da7210.o
> +obj-$(CONFIG_SND_FSI_HDMI)	+= snd-soc-fsi-hdmi.o
>  obj-$(CONFIG_SND_SIU_MIGOR)	+= snd-soc-migor.o
> diff --git a/sound/soc/sh/fsi-hdmi.c b/sound/soc/sh/fsi-hdmi.c
> new file mode 100644
> index 0000000..950e3e0
> --- /dev/null
> +++ b/sound/soc/sh/fsi-hdmi.c
> @@ -0,0 +1,61 @@
> +/*
> + * FSI - HDMI sound support
> + *
> + * Copyright (C) 2010 Renesas Solutions Corp.
> + * Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + */
> +
> +#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?

> +
> +static struct snd_soc_dai_link fsi_dai_link = {
> +	.name		= "HDMI",
> +	.stream_name	= "HDMI",
> +	.cpu_dai_name	= "fsib-dai", /* fsi B */
> +	.codec_dai_name	= "sh_mobile_hdmi-hifi",
> +	.platform_name	= "sh_fsi2",
> +	.codec_name	= "sh-mobile-hdmi",
> +};
> +
> +static struct snd_soc_card fsi_soc_card  = {
> +	.name		= "FSI",
> +	.dai_link	= &fsi_dai_link,
> +	.num_links	= 1,
> +};
> +
> +static struct platform_device *fsi_snd_device;
> +
> +static int __init fsi_hdmi_init(void)
> +{
> +	int ret = -ENOMEM;
> +
> +	fsi_snd_device = platform_device_alloc("soc-audio", FSI_PORT_B);
> +	if (!fsi_snd_device)
> +		goto out;
> +
> +	platform_set_drvdata(fsi_snd_device, &fsi_soc_card);
> +	ret = platform_device_add(fsi_snd_device);
> +
> +	if (ret)
> +		platform_device_put(fsi_snd_device);
> +
> +out:
> +	return ret;
> +}
> +
> +static void __exit fsi_hdmi_exit(void)
> +{
> +	platform_device_unregister(fsi_snd_device);
> +}
> +
> +module_init(fsi_hdmi_init);
> +module_exit(fsi_hdmi_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Generic SH4 FSI-HDMI sound card");
> +MODULE_AUTHOR("Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>");
> -- 
> 1.7.0.4

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 2/4 v2] ASoC: fsi-codec: Add FSI - HDMI support

[Guennadi Liakhovetski]

On Tue, 31 Aug 2010, Kuninori Morimoto wrote:

> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> v1 -> v2
> 
> o based on latest Mark's "for-2.6.37"
> 
>  sound/soc/sh/Kconfig    |    7 +++++
>  sound/soc/sh/Makefile   |    2 +
>  sound/soc/sh/fsi-hdmi.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 70 insertions(+), 0 deletions(-)
>  create mode 100644 sound/soc/sh/fsi-hdmi.c
> 
> diff --git a/sound/soc/sh/Kconfig b/sound/soc/sh/Kconfig
> index 52d7e8e..6b224d2 100644
> --- a/sound/soc/sh/Kconfig
> +++ b/sound/soc/sh/Kconfig
> @@ -62,6 +62,13 @@ config SND_FSI_DA7210
>  	  This option enables generic sound support for the
>  	  FSI - DA7210 unit
>  
> +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.

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?

>  config SND_SIU_MIGOR
>  	tristate "SIU sound support on Migo-R"
>  	depends on SH_MIGOR
> diff --git a/sound/soc/sh/Makefile b/sound/soc/sh/Makefile
> index 8a5a192..94476d4 100644
> --- a/sound/soc/sh/Makefile
> +++ b/sound/soc/sh/Makefile
> @@ -16,9 +16,11 @@ obj-$(CONFIG_SND_SOC_SH4_SIU)	+= snd-soc-siu.o
>  snd-soc-sh7760-ac97-objs	:= sh7760-ac97.o
>  snd-soc-fsi-ak4642-objs		:= fsi-ak4642.o
>  snd-soc-fsi-da7210-objs		:= fsi-da7210.o
> +snd-soc-fsi-hdmi-objs		:= fsi-hdmi.o
>  snd-soc-migor-objs		:= migor.o
>  
>  obj-$(CONFIG_SND_SH7760_AC97)	+= snd-soc-sh7760-ac97.o
>  obj-$(CONFIG_SND_FSI_AK4642)	+= snd-soc-fsi-ak4642.o
>  obj-$(CONFIG_SND_FSI_DA7210)	+= snd-soc-fsi-da7210.o
> +obj-$(CONFIG_SND_FSI_HDMI)	+= snd-soc-fsi-hdmi.o
>  obj-$(CONFIG_SND_SIU_MIGOR)	+= snd-soc-migor.o
> diff --git a/sound/soc/sh/fsi-hdmi.c b/sound/soc/sh/fsi-hdmi.c
> new file mode 100644
> index 0000000..950e3e0
> --- /dev/null
> +++ b/sound/soc/sh/fsi-hdmi.c
> @@ -0,0 +1,61 @@
> +/*
> + * FSI - HDMI sound support
> + *
> + * Copyright (C) 2010 Renesas Solutions Corp.
> + * Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + */
> +
> +#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?

> +
> +static struct snd_soc_dai_link fsi_dai_link = {
> +	.name		= "HDMI",
> +	.stream_name	= "HDMI",
> +	.cpu_dai_name	= "fsib-dai", /* fsi B */
> +	.codec_dai_name	= "sh_mobile_hdmi-hifi",
> +	.platform_name	= "sh_fsi2",
> +	.codec_name	= "sh-mobile-hdmi",
> +};
> +
> +static struct snd_soc_card fsi_soc_card  = {
> +	.name		= "FSI",
> +	.dai_link	= &fsi_dai_link,
> +	.num_links	= 1,
> +};
> +
> +static struct platform_device *fsi_snd_device;
> +
> +static int __init fsi_hdmi_init(void)
> +{
> +	int ret = -ENOMEM;
> +
> +	fsi_snd_device = platform_device_alloc("soc-audio", FSI_PORT_B);
> +	if (!fsi_snd_device)
> +		goto out;
> +
> +	platform_set_drvdata(fsi_snd_device, &fsi_soc_card);
> +	ret = platform_device_add(fsi_snd_device);
> +
> +	if (ret)
> +		platform_device_put(fsi_snd_device);
> +
> +out:
> +	return ret;
> +}
> +
> +static void __exit fsi_hdmi_exit(void)
> +{
> +	platform_device_unregister(fsi_snd_device);
> +}
> +
> +module_init(fsi_hdmi_init);
> +module_exit(fsi_hdmi_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Generic SH4 FSI-HDMI sound card");
> +MODULE_AUTHOR("Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>");
> -- 
> 1.7.0.4

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 1/4 v2] fbdev: sh-mobile: Add HDMI sound type selection

[Kuninori Morimoto]

Dear Guennadi

Thank you for checking patch !!

But ALSA side patches are already applied to Mark's tree.
So, I will send bug fix patch to ALSA ML
(add new label for snd_soc_unregister_codec etc...)

and send v2 patches to SH ML

EC No. W                  # public

Best regards
--
Kuninori Morimoto
 

Re: [PATCH 1/4 v2] fbdev: sh-mobile: Add HDMI sound type selection

[Kuninori Morimoto]

Dear Guennadi

Thank you for checking patch !!

But ALSA side patches are already applied to Mark's tree.
So, I will send bug fix patch to ALSA ML
(add new label for snd_soc_unregister_codec etc...)

and send v2 patches to SH ML

EC No. W                  # public

Best regards
--
Kuninori Morimoto

Re: [PATCH 4/4 v2] ARM: mach-shmobile: ap4evb: Add HDMI sound support

[Guennadi Liakhovetski]

On Tue, 31 Aug 2010, Kuninori Morimoto wrote:

> It support only 48kHz frame rate
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> v1 -> v2
>   no change
> 
>  arch/arm/mach-shmobile/board-ap4evb.c |   56 ++++++++++++++++++++++++++++++++-
>  1 files changed, 55 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
> index 23d472f..1545277 100644
> --- a/arch/arm/mach-shmobile/board-ap4evb.c
> +++ b/arch/arm/mach-shmobile/board-ap4evb.c
> @@ -515,6 +515,8 @@ static struct platform_device *qhd_devices[] __initdata = {
>  /* FSI */
>  #define IRQ_FSI		evt2irq(0x1840)
>  #define FSIACKCR	0xE6150018
> +#define FSIDIV		0xFE1F8000
> +#define FSIDIV_SIZE	32

32 bytes? There are only 2 registers 4 bytes each in FSIDIV?

>  static void fsiackcr_init(struct clk *clk)
>  {
>  	u32 status = __raw_readl(clk->enable_reg);
> @@ -535,12 +537,58 @@ static struct clk fsiackcr_clk = {
>  	.rate		= 0, /* unknown */
>  };
>  
> +static int fsi_set_rate(int is_porta, int rate)
> +{
> +	struct clk *clk;
> +	void __iomem *base;
> +	int ret = 0;

No need to initialise ret.

> +
> +	/* set_rate is not needed if port A */
> +	if (is_porta)
> +		return 0;
> +
> +	clk = clk_get(NULL, "fsib_clk");
> +	if (IS_ERR(clk))
> +		return -EINVAL;
> +
> +	base = ioremap_nocache(FSIDIV, FSIDIV_SIZE);
> +	if (!base) {
> +		ret = -ENXIO;
> +		goto fsi_set_rate_err;
> +	}
> +
> +	switch (rate) {
> +	case 48000:
> +		clk_set_rate(clk, clk_round_rate(clk, 85428000));
> +		__raw_writel(0x00070003, base + 0x8);

Hm, these two registers seem to be a perfect candidate for the clock API?

> +		ret = SH_FSI_ACKMD_256 | SH_FSI_BPFMD_64;
> +		break;
> +	default:
> +		pr_err("unsupported rate in FSI2 port B\n");
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	iounmap(base);
> +
> +fsi_set_rate_err:
> +	clk_put(clk);
> +
> +	return ret;
> +}
> +
>  static struct sh_fsi_platform_info fsi_info = {
>  	.porta_flags = SH_FSI_BRS_INV |
>  		       SH_FSI_OUT_SLAVE_MODE |
>  		       SH_FSI_IN_SLAVE_MODE |
>  		       SH_FSI_OFMT(PCM) |
>  		       SH_FSI_IFMT(PCM),
> +
> +	.portb_flags = SH_FSI_BRS_INV |
> +		       SH_FSI_BRM_INV |
> +		       SH_FSI_LRS_INV |
> +		       SH_FSI_OFMT(SPDIF),
> +	.set_rate = fsi_set_rate,
>  };
>  
>  static struct resource fsi_resources[] = {
> @@ -624,6 +672,7 @@ static struct platform_device lcdc1_device = {
>  static struct sh_mobile_hdmi_info hdmi_info = {
>  	.lcd_chan = &sh_mobile_lcdc1_info.ch[0],
>  	.lcd_dev = &lcdc1_device.dev,
> +	.flags = HDMI_SRC_SPDIF,
>  };
>  
>  static struct resource hdmi_resources[] = {
> @@ -825,6 +874,7 @@ static void __init ap4evb_map_io(void)
>  
>  #define GPIO_PORT9CR	0xE6051009
>  #define GPIO_PORT10CR	0xE605100A
> +#define USCCR1		0xE6058144
>  static void __init ap4evb_init(void)
>  {
>  	u32 srcr4;
> @@ -909,7 +959,7 @@ static void __init ap4evb_init(void)
>  	/* setup USB phy */
>  	__raw_writew(0x8a0a, 0xE6058130);	/* USBCR2 */
>  
> -	/* enable FSI2 */
> +	/* enable FSI2 port A (ak4643) */
>  	gpio_request(GPIO_FN_FSIAIBT,	NULL);
>  	gpio_request(GPIO_FN_FSIAILR,	NULL);
>  	gpio_request(GPIO_FN_FSIAISLD,	NULL);
> @@ -922,6 +972,10 @@ static void __init ap4evb_init(void)
>  	gpio_no_direction(GPIO_PORT9CR);  /* FSIAOBT needs no direction */
>  	gpio_no_direction(GPIO_PORT10CR); /* FSIAOLR needs no direction */
>  
> +	/* setup FSI2 port B (HDMI) */
> +	gpio_request(GPIO_FN_FSIBCK, NULL);
> +	__raw_writew(__raw_readw(USCCR1) & ~(1 << 6), USCCR1); /* use SPDIF */
> +
>  	/* set SPU2 clock to 119.6 MHz */
>  	clk = clk_get(NULL, "spu_clk");
>  	if (!IS_ERR(clk)) {
> -- 
> 1.7.0.4

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 4/4 v2] ARM: mach-shmobile: ap4evb: Add HDMI sound support

[Guennadi Liakhovetski]

On Tue, 31 Aug 2010, Kuninori Morimoto wrote:

> It support only 48kHz frame rate
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> v1 -> v2
>   no change
> 
>  arch/arm/mach-shmobile/board-ap4evb.c |   56 ++++++++++++++++++++++++++++++++-
>  1 files changed, 55 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
> index 23d472f..1545277 100644
> --- a/arch/arm/mach-shmobile/board-ap4evb.c
> +++ b/arch/arm/mach-shmobile/board-ap4evb.c
> @@ -515,6 +515,8 @@ static struct platform_device *qhd_devices[] __initdata = {
>  /* FSI */
>  #define IRQ_FSI		evt2irq(0x1840)
>  #define FSIACKCR	0xE6150018
> +#define FSIDIV		0xFE1F8000
> +#define FSIDIV_SIZE	32

32 bytes? There are only 2 registers 4 bytes each in FSIDIV?

>  static void fsiackcr_init(struct clk *clk)
>  {
>  	u32 status = __raw_readl(clk->enable_reg);
> @@ -535,12 +537,58 @@ static struct clk fsiackcr_clk = {
>  	.rate		= 0, /* unknown */
>  };
>  
> +static int fsi_set_rate(int is_porta, int rate)
> +{
> +	struct clk *clk;
> +	void __iomem *base;
> +	int ret = 0;

No need to initialise ret.

> +
> +	/* set_rate is not needed if port A */
> +	if (is_porta)
> +		return 0;
> +
> +	clk = clk_get(NULL, "fsib_clk");
> +	if (IS_ERR(clk))
> +		return -EINVAL;
> +
> +	base = ioremap_nocache(FSIDIV, FSIDIV_SIZE);
> +	if (!base) {
> +		ret = -ENXIO;
> +		goto fsi_set_rate_err;
> +	}
> +
> +	switch (rate) {
> +	case 48000:
> +		clk_set_rate(clk, clk_round_rate(clk, 85428000));
> +		__raw_writel(0x00070003, base + 0x8);

Hm, these two registers seem to be a perfect candidate for the clock API?

> +		ret = SH_FSI_ACKMD_256 | SH_FSI_BPFMD_64;
> +		break;
> +	default:
> +		pr_err("unsupported rate in FSI2 port B\n");
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	iounmap(base);
> +
> +fsi_set_rate_err:
> +	clk_put(clk);
> +
> +	return ret;
> +}
> +
>  static struct sh_fsi_platform_info fsi_info = {
>  	.porta_flags = SH_FSI_BRS_INV |
>  		       SH_FSI_OUT_SLAVE_MODE |
>  		       SH_FSI_IN_SLAVE_MODE |
>  		       SH_FSI_OFMT(PCM) |
>  		       SH_FSI_IFMT(PCM),
> +
> +	.portb_flags = SH_FSI_BRS_INV |
> +		       SH_FSI_BRM_INV |
> +		       SH_FSI_LRS_INV |
> +		       SH_FSI_OFMT(SPDIF),
> +	.set_rate = fsi_set_rate,
>  };
>  
>  static struct resource fsi_resources[] = {
> @@ -624,6 +672,7 @@ static struct platform_device lcdc1_device = {
>  static struct sh_mobile_hdmi_info hdmi_info = {
>  	.lcd_chan = &sh_mobile_lcdc1_info.ch[0],
>  	.lcd_dev = &lcdc1_device.dev,
> +	.flags = HDMI_SRC_SPDIF,
>  };
>  
>  static struct resource hdmi_resources[] = {
> @@ -825,6 +874,7 @@ static void __init ap4evb_map_io(void)
>  
>  #define GPIO_PORT9CR	0xE6051009
>  #define GPIO_PORT10CR	0xE605100A
> +#define USCCR1		0xE6058144
>  static void __init ap4evb_init(void)
>  {
>  	u32 srcr4;
> @@ -909,7 +959,7 @@ static void __init ap4evb_init(void)
>  	/* setup USB phy */
>  	__raw_writew(0x8a0a, 0xE6058130);	/* USBCR2 */
>  
> -	/* enable FSI2 */
> +	/* enable FSI2 port A (ak4643) */
>  	gpio_request(GPIO_FN_FSIAIBT,	NULL);
>  	gpio_request(GPIO_FN_FSIAILR,	NULL);
>  	gpio_request(GPIO_FN_FSIAISLD,	NULL);
> @@ -922,6 +972,10 @@ static void __init ap4evb_init(void)
>  	gpio_no_direction(GPIO_PORT9CR);  /* FSIAOBT needs no direction */
>  	gpio_no_direction(GPIO_PORT10CR); /* FSIAOLR needs no direction */
>  
> +	/* setup FSI2 port B (HDMI) */
> +	gpio_request(GPIO_FN_FSIBCK, NULL);
> +	__raw_writew(__raw_readw(USCCR1) & ~(1 << 6), USCCR1); /* use SPDIF */
> +
>  	/* set SPU2 clock to 119.6 MHz */
>  	clk = clk_get(NULL, "spu_clk");
>  	if (!IS_ERR(clk)) {
> -- 
> 1.7.0.4

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Mark Brown]

On Mon, Sep 06, 2010 at 10:27:56AM +0200, Guennadi Liakhovetski wrote:
> On Tue, 31 Aug 2010, Kuninori Morimoto wrote:

> > +static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
> > +			     unsigned int reg,
> > +			     unsigned int value)
> > +{
> > +	struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
> > +
> > +	hdmi_write(hdmi, value, reg);
> > +	return 0;
> > +}

> Are these two actually needed? As long as you don't have a register cache 
> - no need for these?

Something needs to translate the ASoC register I/O functions into what
the HDMI layer code is expecting.

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Mark Brown]

On Mon, Sep 06, 2010 at 10:27:56AM +0200, Guennadi Liakhovetski wrote:
> On Tue, 31 Aug 2010, Kuninori Morimoto wrote:

> > +static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
> > +			     unsigned int reg,
> > +			     unsigned int value)
> > +{
> > +	struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
> > +
> > +	hdmi_write(hdmi, value, reg);
> > +	return 0;
> > +}

> Are these two actually needed? As long as you don't have a register cache 
> - no need for these?

Something needs to translate the ASoC register I/O functions into what
the HDMI layer code is expecting.

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Kuninori Morimoto]

Dear 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. .hw_params implementation is needed for advanced support.
I will send its patch in future.

>> +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
(snip)
> 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.
If you select FSI-DA7210 and FSI-AK4642, small patch which change
FSIA <-> FSIB is needed for now.

>> +	switch (rate) {
>> +	case 48000:
>> +		clk_set_rate(clk, clk_round_rate(clk, 85428000));
>> +		__raw_writel(0x00070003, base + 0x8);
>
>Hm, these two registers seem to be a perfect candidate for the clock API?

? I think so. why ?

Best regards
--
Kuninori Morimoto
 

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Kuninori Morimoto]

Dear 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. .hw_params implementation is needed for advanced support.
I will send its patch in future.

>> +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
(snip)
> 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.
If you select FSI-DA7210 and FSI-AK4642, small patch which change
FSIA <-> FSIB is needed for now.

>> +	switch (rate) {
>> +	case 48000:
>> +		clk_set_rate(clk, clk_round_rate(clk, 85428000));
>> +		__raw_writel(0x00070003, base + 0x8);
>
>Hm, these two registers seem to be a perfect candidate for the clock API?

? I think so. why ?

Best regards
--
Kuninori Morimoto

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Guennadi Liakhovetski]

On Mon, 6 Sep 2010, Mark Brown wrote:

> On Mon, Sep 06, 2010 at 10:27:56AM +0200, Guennadi Liakhovetski wrote:
> > On Tue, 31 Aug 2010, Kuninori Morimoto wrote:
> 
> > > +static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
> > > +			     unsigned int reg,
> > > +			     unsigned int value)
> > > +{
> > > +	struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
> > > +
> > > +	hdmi_write(hdmi, value, reg);
> > > +	return 0;
> > > +}
> 
> > Are these two actually needed? As long as you don't have a register cache 
> > - no need for these?
> 
> Something needs to translate the ASoC register I/O functions into what
> the HDMI layer code is expecting.

AFAICS, with ->reg_cache_size = 0 the ASoC core will not attempt to call 
them.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Guennadi Liakhovetski]

On Mon, 6 Sep 2010, Mark Brown wrote:

> On Mon, Sep 06, 2010 at 10:27:56AM +0200, Guennadi Liakhovetski wrote:
> > On Tue, 31 Aug 2010, Kuninori Morimoto wrote:
> 
> > > +static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
> > > +			     unsigned int reg,
> > > +			     unsigned int value)
> > > +{
> > > +	struct sh_hdmi *hdmi = snd_soc_codec_get_drvdata(codec);
> > > +
> > > +	hdmi_write(hdmi, value, reg);
> > > +	return 0;
> > > +}
> 
> > Are these two actually needed? As long as you don't have a register cache 
> > - no need for these?
> 
> Something needs to translate the ASoC register I/O functions into what
> the HDMI layer code is expecting.

AFAICS, with ->reg_cache_size = 0 the ASoC core will not attempt to call 
them.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Mark Brown]

On Tue, Sep 07, 2010 at 09:11:28AM +0200, Guennadi Liakhovetski wrote:
> On Mon, 6 Sep 2010, Mark Brown wrote:

> > > > +static int sh_hdmi_snd_write(struct snd_soc_codec *codec,

> > > Are these two actually needed? As long as you don't have a register cache 
> > > - no need for these?

> > Something needs to translate the ASoC register I/O functions into what
> > the HDMI layer code is expecting.

> AFAICS, with ->reg_cache_size = 0 the ASoC core will not attempt to call 
> them.

Could you please be more explicit here?  Register I/O needs to happen
somehow...

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Mark Brown]

On Tue, Sep 07, 2010 at 09:11:28AM +0200, Guennadi Liakhovetski wrote:
> On Mon, 6 Sep 2010, Mark Brown wrote:

> > > > +static int sh_hdmi_snd_write(struct snd_soc_codec *codec,

> > > Are these two actually needed? As long as you don't have a register cache 
> > > - no need for these?

> > Something needs to translate the ASoC register I/O functions into what
> > the HDMI layer code is expecting.

> AFAICS, with ->reg_cache_size = 0 the ASoC core will not attempt to call 
> them.

Could you please be more explicit here?  Register I/O needs to happen
somehow...

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Guennadi Liakhovetski]

On Tue, 7 Sep 2010, Mark Brown wrote:

> On Tue, Sep 07, 2010 at 09:11:28AM +0200, Guennadi Liakhovetski wrote:
> > On Mon, 6 Sep 2010, Mark Brown wrote:
> 
> > > > > +static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
> 
> > > > Are these two actually needed? As long as you don't have a register cache 
> > > > - no need for these?
> 
> > > Something needs to translate the ASoC register I/O functions into what
> > > the HDMI layer code is expecting.
> 
> > AFAICS, with ->reg_cache_size = 0 the ASoC core will not attempt to call 
> > them.
> 
> Could you please be more explicit here?  Register I/O needs to happen
> somehow...

Sorry, maybe I am missing something, but my understanding is, that the 
ASoC core knows nothing about codec's specific register layout, so, the 
core itself cannot initiate any register IO. So, I presume, there can be 
only two instances, that can do that - the codec driver itself and some 
user-space (debugging) programs. The driver doesn't use cached register 
accesses, so, it can access the registers directly, and it doesn't have to 
provide an ability to the user-space to access registers - if it chooses 
so. So, I don't see, who should be trying to use generic ASoC register 
access routines here.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Guennadi Liakhovetski]

On Tue, 7 Sep 2010, Mark Brown wrote:

> On Tue, Sep 07, 2010 at 09:11:28AM +0200, Guennadi Liakhovetski wrote:
> > On Mon, 6 Sep 2010, Mark Brown wrote:
> 
> > > > > +static int sh_hdmi_snd_write(struct snd_soc_codec *codec,
> 
> > > > Are these two actually needed? As long as you don't have a register cache 
> > > > - no need for these?
> 
> > > Something needs to translate the ASoC register I/O functions into what
> > > the HDMI layer code is expecting.
> 
> > AFAICS, with ->reg_cache_size = 0 the ASoC core will not attempt to call 
> > them.
> 
> Could you please be more explicit here?  Register I/O needs to happen
> somehow...

Sorry, maybe I am missing something, but my understanding is, that the 
ASoC core knows nothing about codec's specific register layout, so, the 
core itself cannot initiate any register IO. So, I presume, there can be 
only two instances, that can do that - the codec driver itself and some 
user-space (debugging) programs. The driver doesn't use cached register 
accesses, so, it can access the registers directly, and it doesn't have to 
provide an ability to the user-space to access registers - if it chooses 
so. So, I don't see, who should be trying to use generic ASoC register 
access routines here.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Mark Brown]

On Tue, Sep 07, 2010 at 11:56:34AM +0200, Guennadi Liakhovetski wrote:
> On Tue, 7 Sep 2010, Mark Brown wrote:

> > Could you please be more explicit here?  Register I/O needs to happen
> > somehow...

> Sorry, maybe I am missing something, but my understanding is, that the 
> ASoC core knows nothing about codec's specific register layout, so, the 
> core itself cannot initiate any register IO. So, I presume, there can be 
> only two instances, that can do that - the codec driver itself and some 
> user-space (debugging) programs. The driver doesn't use cached register 
> accesses, so, it can access the registers directly, and it doesn't have to 
> provide an ability to the user-space to access registers - if it chooses 
> so. So, I don't see, who should be trying to use generic ASoC register 
> access routines here.

All register I/O, cached or not, is supposed to go through the register
I/O operations.  Any cache is abstraced away within those operations.
Since the driver presumably needs do do some I/O (even if only in the
hw_params() you have identified as missing) I would expect it to provide
register I/O functionality.

Re: [PATCH 2/4 v2] ASoC: Add sh_mobile_hdmi sound support

[Mark Brown]

On Tue, Sep 07, 2010 at 11:56:34AM +0200, Guennadi Liakhovetski wrote:
> On Tue, 7 Sep 2010, Mark Brown wrote:

> > Could you please be more explicit here?  Register I/O needs to happen
> > somehow...

> Sorry, maybe I am missing something, but my understanding is, that the 
> ASoC core knows nothing about codec's specific register layout, so, the 
> core itself cannot initiate any register IO. So, I presume, there can be 
> only two instances, that can do that - the codec driver itself and some 
> user-space (debugging) programs. The driver doesn't use cached register 
> accesses, so, it can access the registers directly, and it doesn't have to 
> provide an ability to the user-space to access registers - if it chooses 
> so. So, I don't see, who should be trying to use generic ASoC register 
> access routines here.

All register I/O, cached or not, is supposed to go through the register
I/O operations.  Any cache is abstraced away within those operations.
Since the driver presumably needs do do some I/O (even if only in the
hw_params() you have identified as missing) I would expect it to provide
register I/O functionality.

Re: [PATCH 4/4 v2] ARM: mach-shmobile: ap4evb: Add HDMI sound support

[Kuninori Morimoto]

Dear Guennadi

> > +	switch (rate) {
> > +	case 48000:
> > +		clk_set_rate(clk, clk_round_rate(clk, 85428000));
> > +		__raw_writel(0x00070003, base + 0x8);
> 
> Hm, these two registers seem to be a perfect candidate for the clock API?

Do you mean I should use new struct clk for this operation
like fsiackcr_clk ?

Best regards
--
Kuninori Morimoto
 

Re: [PATCH 4/4 v2] ARM: mach-shmobile: ap4evb: Add HDMI sound support

[Guennadi Liakhovetski]

On Thu, 9 Sep 2010, Kuninori Morimoto wrote:

> 
> Dear Guennadi
> 
> > > +	switch (rate) {
> > > +	case 48000:
> > > +		clk_set_rate(clk, clk_round_rate(clk, 85428000));
> > > +		__raw_writel(0x00070003, base + 0x8);
> > 
> > Hm, these two registers seem to be a perfect candidate for the clock API?
> 
> Do you mean I should use new struct clk for this operation
> like fsiackcr_clk ?

Yes, I meant using the clock API, as defined in include/linux/sh_clk.h.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/