[PATCH 3/3] Add FSI-AK464x sound support for ms7724se

[PATCH 3/3] Add FSI-AK464x sound support for ms7724se

[Kuninori Morimoto]

Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
---
 arch/sh/boards/Kconfig                   |    9 +++
 arch/sh/boards/mach-se/7724/Makefile     |    3 +-
 arch/sh/boards/mach-se/7724/fsi-ak464x.c |   90 ++++++++++++++++++++++++++++++
 arch/sh/boards/mach-se/7724/setup.c      |   90 ++++++++++++++++++++++++++++++
 4 files changed, 191 insertions(+), 1 deletions(-)
 create mode 100644 arch/sh/boards/mach-se/7724/fsi-ak464x.c

diff --git a/arch/sh/boards/Kconfig b/arch/sh/boards/Kconfig
index db04c85..44f4c8b 100644
--- a/arch/sh/boards/Kconfig
+++ b/arch/sh/boards/Kconfig
@@ -55,6 +55,15 @@ config SH_7724_SOLUTION_ENGINE
 	  Select 7724 SolutionEngine if configuring for a Hitachi SH7724
 	  evaluation board.
 
+config SND_SH4_FSI_AK464X
+	bool "FSI-AK464X sound support"
+	depends on SND_SOC_SH4_FSI
+	depends on SH_7724_SOLUTION_ENGINE
+	select SND_SOC_AK464X
+	help
+	  This option enables generic sound support for the
+	  FSI - AK464x unit of the SH4.
+
 config SH_7751_SOLUTION_ENGINE
 	bool "SolutionEngine7751"
 	select SOLUTION_ENGINE
diff --git a/arch/sh/boards/mach-se/7724/Makefile b/arch/sh/boards/mach-se/7724/Makefile
index 349cbd6..e0f5366 100644
--- a/arch/sh/boards/mach-se/7724/Makefile
+++ b/arch/sh/boards/mach-se/7724/Makefile
@@ -7,4 +7,5 @@
 #
 #
 
-obj-y	 := setup.o irq.o
\ No newline at end of file
+obj-y	 := setup.o irq.o
+obj-$(CONFIG_SND_SH4_FSI_AK464X) += fsi-ak464x.o
diff --git a/arch/sh/boards/mach-se/7724/fsi-ak464x.c b/arch/sh/boards/mach-se/7724/fsi-ak464x.c
new file mode 100644
index 0000000..bab118e
--- /dev/null
+++ b/arch/sh/boards/mach-se/7724/fsi-ak464x.c
@@ -0,0 +1,90 @@
+/*
+ * FSI-AK464x sound support for ms7724se
+ *
+ * Copyright (C) 2009 Renesas Solutions Corp.
+ * Kuninori Morimoto <morimoto.kuninori@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/module.h>
+#include <linux/moduleparam.h>
+#include <linux/platform_device.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
+#include <asm/io.h>
+
+#include <sound/fsi.h>
+#include <../sound/soc/codecs/ak464x.h>
+
+static int machine_init(struct snd_soc_codec *codec)
+{
+	snd_soc_dapm_sync(codec);
+	return 0;
+}
+
+static struct snd_soc_dai_link fsi_dai_link = {
+	.name		= "AK464x",
+	.stream_name	= "AK464x",
+	.cpu_dai	= &fsi_soc_dai[0],	/* fsi */
+	.codec_dai	= &ak464x_dai,
+	.init		= machine_init,
+	.ops		= NULL,
+};
+
+static struct snd_soc_card fsi_soc_card  = {
+	.name		= "SH4 I2S (FSI)",
+	.platform	= &fsi_soc_platform,
+	.dai_link	= &fsi_dai_link,
+	.num_links	= 1,
+};
+
+struct ak464x_setup_data ak464x_setup = {
+	.i2c_bus	= 0,
+	.i2c_address	= 0x12, /* 0x13 */
+};
+
+static struct snd_soc_device fsi_snd_devdata = {
+	.card		= &fsi_soc_card,
+	.codec_dev	= &soc_codec_dev_ak464x,
+	.codec_data	= &ak464x_setup,
+};
+
+static struct platform_device *fsi_snd_device;
+
+static int __init fsi_ak464x_init(void)
+{
+	int ret;
+
+	ret = -ENOMEM;
+	fsi_snd_device = platform_device_alloc("soc-audio", -1);
+	if (!fsi_snd_device)
+		goto out;
+
+	platform_set_drvdata(fsi_snd_device,
+			     &fsi_snd_devdata);
+	fsi_snd_devdata.dev = &fsi_snd_device->dev;
+	ret = platform_device_add(fsi_snd_device);
+
+	if (ret)
+		platform_device_put(fsi_snd_device);
+
+out:
+	return ret;
+}
+
+static void __exit fsi_ak464x_exit(void)
+{
+	platform_device_unregister(fsi_snd_device);
+}
+
+module_init(fsi_ak464x_init);
+module_exit(fsi_ak464x_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Generic SH4 FSI-AK464x sound card");
+MODULE_AUTHOR("Kuninori Morimoto <morimoto.kuninori@renesas.com>");
diff --git a/arch/sh/boards/mach-se/7724/setup.c b/arch/sh/boards/mach-se/7724/setup.c
index 9162081..0099a79 100644
--- a/arch/sh/boards/mach-se/7724/setup.c
+++ b/arch/sh/boards/mach-se/7724/setup.c
@@ -22,11 +22,13 @@
 #include <linux/usb/r8a66597.h>
 #include <video/sh_mobile_lcdc.h>
 #include <media/sh_mobile_ceu.h>
+#include <sound/fsi.h>
 #include <asm/io.h>
 #include <asm/heartbeat.h>
 #include <asm/sh_eth.h>
 #include <asm/clock.h>
 #include <asm/sh_keysc.h>
+#include <asm/dma-sh.h>
 #include <cpu/sh7724.h>
 #include <mach-se/mach/se7724.h>
 
@@ -246,6 +248,65 @@ static struct platform_device ceu1_device = {
 	},
 };
 
+/* FSI */
+/*
+ * FSI-A use external clock which came from ak464x.
+ * So, we should change parent of fsi
+ */
+#define FCLKACR		0xa4150008
+static void fsimck_init(struct clk *clk)
+{
+	u32 status = ctrl_inl(clk->enable_reg);
+
+	/* use external clock */
+	status &= ~0x000000ff;
+	status |= 0x00000080;
+	ctrl_outl(status, clk->enable_reg);
+}
+
+static struct clk_ops fsimck_clk_ops = {
+	.init = fsimck_init,
+};
+
+static struct clk fsimcka_clk = {
+	.name		= "fsimcka_clk",
+	.id		= -1,
+	.ops		= &fsimck_clk_ops,
+	.enable_reg	= (void __iomem *)FCLKACR,
+	.rate		= 0, /* unknown */
+};
+
+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),
+};
+
+static struct resource fsi_resources[] = {
+	[0] = {
+		.name	= "FSI",
+		.start	= 0xFE3C0000,
+		.end	= 0xFE3C021d,
+		.flags	= IORESOURCE_MEM,
+	},
+	[1] = {
+		.start  = 108,
+		.flags  = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device fsi_device = {
+	.name		= "sh_fsi",
+	.id		= 0,
+	.num_resources	= ARRAY_SIZE(fsi_resources),
+	.resource	= fsi_resources,
+	.dev	= {
+		.platform_data	= &fsi_info,
+	},
+};
+
 /* KEYSC in SoC (Needs SW33-2 set to ON) */
 static struct sh_keysc_info keysc_info = {
 	.mode = SH_KEYSC_MODE_1,
@@ -351,6 +412,7 @@ static struct platform_device *ms7724se_devices[] __initdata = {
 	&keysc_device,
 	&sh_eth_device,
 	&sh7724_usb0_host_device,
+	&fsi_device,
 };
 
 #define EEPROM_OP   0xBA206000
@@ -418,11 +480,13 @@ static void __init sh_eth_init(void)
 static int __init devices_setup(void)
 {
 	u16 sw = ctrl_inw(SW4140); /* select camera, monitor */
+	struct clk *fsia_clk;
 
 	/* Reset Release */
 	ctrl_outw(ctrl_inw(FPGA_OUT) &
 		  ~((1 << 1)  | /* LAN */
 		    (1 << 6)  | /* VIDEO DAC */
+		    (1 << 7)  | /* AK4643 */
 		    (1 << 12) | /* USB0 */
 		    (1 << 14)), /* RMII */
 		  FPGA_OUT);
@@ -558,6 +622,32 @@ static int __init devices_setup(void)
 	gpio_request(GPIO_FN_KEYOUT1,     NULL);
 	gpio_request(GPIO_FN_KEYOUT0,     NULL);
 
+	/* enable FSI */
+	gpio_request(GPIO_FN_FSIMCKB,    NULL);
+	gpio_request(GPIO_FN_FSIMCKA,    NULL);
+	gpio_request(GPIO_FN_FSIOASD,    NULL);
+	gpio_request(GPIO_FN_FSIIABCK,   NULL);
+	gpio_request(GPIO_FN_FSIIALRCK,  NULL);
+	gpio_request(GPIO_FN_FSIOABCK,   NULL);
+	gpio_request(GPIO_FN_FSIOALRCK,  NULL);
+	gpio_request(GPIO_FN_CLKAUDIOAO, NULL);
+	gpio_request(GPIO_FN_FSIIBSD,    NULL);
+	gpio_request(GPIO_FN_FSIOBSD,    NULL);
+	gpio_request(GPIO_FN_FSIIBBCK,   NULL);
+	gpio_request(GPIO_FN_FSIIBLRCK,  NULL);
+	gpio_request(GPIO_FN_FSIOBBCK,   NULL);
+	gpio_request(GPIO_FN_FSIOBLRCK,  NULL);
+	gpio_request(GPIO_FN_CLKAUDIOBO, NULL);
+	gpio_request(GPIO_FN_FSIIASD,    NULL);
+
+	/* change parent of FSI A */
+	fsia_clk = clk_get(NULL, "fsia_clk");
+	clk_register(&fsimcka_clk);
+	clk_set_parent(fsia_clk, &fsimcka_clk);
+	clk_set_rate(fsia_clk, 11000);
+	clk_set_rate(&fsimcka_clk, 11000);
+	clk_put(fsia_clk);
+
 	/*
 	 * enable SH-Eth
 	 *
-- 
1.6.0.4

Re: [PATCH 3/3] Add FSI-AK464x sound support for ms7724se

[Mark Brown]

On Wed, Aug 19, 2009 at 08:25:28PM +0900, Kuninori Morimoto wrote:

>  arch/sh/boards/Kconfig                   |    9 +++
>  arch/sh/boards/mach-se/7724/Makefile     |    3 +-
>  arch/sh/boards/mach-se/7724/fsi-ak464x.c |   90 ++++++++++++++++++++++++++++++

the machine driver itself should be under sound/soc/sh.

> +static int machine_init(struct snd_soc_codec *codec)
> +{
> +	snd_soc_dapm_sync(codec);
> +	return 0;
> +}

This should just be removed - the core will do this for you.

> +	/* enable FSI */
> +	gpio_request(GPIO_FN_FSIMCKB,    NULL);
> +	gpio_request(GPIO_FN_FSIMCKA,    NULL);
> +	gpio_request(GPIO_FN_FSIOASD,    NULL);
> +	gpio_request(GPIO_FN_FSIIABCK,   NULL);
> +	gpio_request(GPIO_FN_FSIIALRCK,  NULL);
> +	gpio_request(GPIO_FN_FSIOABCK,   NULL);
> +	gpio_request(GPIO_FN_FSIOALRCK,  NULL);
> +	gpio_request(GPIO_FN_CLKAUDIOAO, NULL);
> +	gpio_request(GPIO_FN_FSIIBSD,    NULL);
> +	gpio_request(GPIO_FN_FSIOBSD,    NULL);
> +	gpio_request(GPIO_FN_FSIIBBCK,   NULL);
> +	gpio_request(GPIO_FN_FSIIBLRCK,  NULL);
> +	gpio_request(GPIO_FN_FSIOBBCK,   NULL);
> +	gpio_request(GPIO_FN_FSIOBLRCK,  NULL);
> +	gpio_request(GPIO_FN_CLKAUDIOBO, NULL);
> +	gpio_request(GPIO_FN_FSIIASD,    NULL);

Is this something that the FSI driver should do for itself?  It looks
like there's no pin options here, just fixed functions for FSI A or B so
replicating per-board seems redundant.  On the other hand, if it's
idiomatic for SH better keep it this way.

Re: [PATCH 3/3] Add FSI-AK464x sound support for ms7724se

[Magnus Damm]

Hi Mark,

Thanks for your feedback!

On Wed, Aug 19, 2009 at 9:00 PM, Mark
Brown<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Aug 19, 2009 at 08:25:28PM +0900, Kuninori Morimoto wrote:
>> +     /* enable FSI */
>> +     gpio_request(GPIO_FN_FSIMCKB,    NULL);
>> +     gpio_request(GPIO_FN_FSIMCKA,    NULL);
>> +     gpio_request(GPIO_FN_FSIOASD,    NULL);
>> +     gpio_request(GPIO_FN_FSIIABCK,   NULL);
>> +     gpio_request(GPIO_FN_FSIIALRCK,  NULL);
>> +     gpio_request(GPIO_FN_FSIOABCK,   NULL);
>> +     gpio_request(GPIO_FN_FSIOALRCK,  NULL);
>> +     gpio_request(GPIO_FN_CLKAUDIOAO, NULL);
>> +     gpio_request(GPIO_FN_FSIIBSD,    NULL);
>> +     gpio_request(GPIO_FN_FSIOBSD,    NULL);
>> +     gpio_request(GPIO_FN_FSIIBBCK,   NULL);
>> +     gpio_request(GPIO_FN_FSIIBLRCK,  NULL);
>> +     gpio_request(GPIO_FN_FSIOBBCK,   NULL);
>> +     gpio_request(GPIO_FN_FSIOBLRCK,  NULL);
>> +     gpio_request(GPIO_FN_CLKAUDIOBO, NULL);
>> +     gpio_request(GPIO_FN_FSIIASD,    NULL);
>
> Is this something that the FSI driver should do for itself?  It looks
> like there's no pin options here, just fixed functions for FSI A or B so
> replicating per-board seems redundant.  On the other hand, if it's
> idiomatic for SH better keep it this way.

These GPIO_FN_ values are processor specific, sh7724 in this case. I
suspect we will see the FSI block reused in future processors so
keeping processor-specific bits out of the driver itself may be a good
plan. Long term I'd be more than happy to pass along processors
specific GPIOs to each driver, but at this point I don't know any
clean and standard way to do that.

Cheers,

/ magnus

Re: [PATCH 3/3] Add FSI-AK464x sound support for ms7724se

[Mark Brown]

On Wed, Aug 19, 2009 at 10:46:34PM +0900, Magnus Damm wrote:
> On Wed, Aug 19, 2009 at 9:00 PM, Mark
> Brown<broonie@opensource.wolfsonmicro.com> wrote:

> >> +     gpio_request(GPIO_FN_FSIMCKA,    NULL);

> > Is this something that the FSI driver should do for itself?  It looks
> > like there's no pin options here, just fixed functions for FSI A or B so
> > replicating per-board seems redundant.  On the other hand, if it's
> > idiomatic for SH better keep it this way.

> These GPIO_FN_ values are processor specific, sh7724 in this case. I
> suspect we will see the FSI block reused in future processors so
> keeping processor-specific bits out of the driver itself may be a good
> plan. Long term I'd be more than happy to pass along processors
> specific GPIOs to each driver, but at this point I don't know any
> clean and standard way to do that.

Is it possible to build kernels for more than one processor?  The way
the code looks that's not the case...

Re: [PATCH 3/3] Add FSI-AK464x sound support for ms7724se

[Magnus Damm]

On Wed, Aug 19, 2009 at 11:02 PM, Mark
Brown<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Aug 19, 2009 at 10:46:34PM +0900, Magnus Damm wrote:
>> On Wed, Aug 19, 2009 at 9:00 PM, Mark
>> Brown<broonie@opensource.wolfsonmicro.com> wrote:
>
>> >> +     gpio_request(GPIO_FN_FSIMCKA,    NULL);
>
>> > Is this something that the FSI driver should do for itself?  It looks
>> > like there's no pin options here, just fixed functions for FSI A or B so
>> > replicating per-board seems redundant.  On the other hand, if it's
>> > idiomatic for SH better keep it this way.
>
>> These GPIO_FN_ values are processor specific, sh7724 in this case. I
>> suspect we will see the FSI block reused in future processors so
>> keeping processor-specific bits out of the driver itself may be a good
>> plan. Long term I'd be more than happy to pass along processors
>> specific GPIOs to each driver, but at this point I don't know any
>> clean and standard way to do that.
>
> Is it possible to build kernels for more than one processor?  The way
> the code looks that's not the case...

At this point on SuperH we first select processor and then board(s)
after that. So the kernel is configured to run on one specific
processor model. But the FSI block may show up in other processors and
the pinmux configuration will most likely be different at that point.
We usually keep the pinmux configuration in the board setup code and
the let the driver "just work" and never touch any GPIOs unless the
driver comes with GPIO support.

Does it make sense? =)

/ magnus

Re: [PATCH 3/3] Add FSI-AK464x sound support for ms7724se

[Mark Brown]

On Wed, Aug 19, 2009 at 11:18:13PM +0900, Magnus Damm wrote:

> At this point on SuperH we first select processor and then board(s)
> after that. So the kernel is configured to run on one specific
> processor model. But the FSI block may show up in other processors and
> the pinmux configuration will most likely be different at that point.
> We usually keep the pinmux configuration in the board setup code and
> the let the driver "just work" and never touch any GPIOs unless the
> driver comes with GPIO support.

Hrm, OK.  In that case it's a bit of an odd way of doing pinmux
configuration but that's fine.

Re: [alsa-devel] [PATCH 3/3] Add FSI-AK464x sound support for ms7724se

[Kuninori Morimoto]

Dear Mark, pHilipp, Magnus

Thank you for a lot of advice/comments

> Should this driver also be able to drive AK4641? I'm asking because I

I checked datasheet.
My codec is AK4643, and it is compatible with AK4642.
And AK4642 is not compatible AK4641 (^^;

I will re-name it AK464x -> AK4642.
Can you agree with me ?

> +#define PREALLOC_BUFFER		(32 * 1024)
> +#define PREALLOC_BUFFER_MAX	(32 * 1024)
>
> Is it worth letting machines override these in the platform data or is
> there no point?

do you mean it should be possible to change size ?
If so, is it good as future tasks ?
This time, 32k is good for me.

> If possible please try to avoid using a global variable like this.
> Future processors may come with multiple FSI blocks.
>
> Right now I'm quite happy that the CEU driver does not use global

hmm... I can agree with you.
But this time, is it good as future tasks ?
I will fix this problem if I can get free time

>  arch/sh/boards/mach-se/7724/Makefile     |    3 +-
>  arch/sh/boards/mach-se/7724/fsi-ak464x.c |   90 ++++++++++++++++++++++++++++++
>
> the machine driver itself should be under sound/soc/sh.

if fsi-ak464x.c is in arch/sh/boards/mach-se/7724/
it works, and it say
------------
AK4642 Audio Codec 0.0.1
asoc: AK4642 <-> FSI0 mapping ok
ALSA device list:
  #0: FSI (AK4642)
------------

but when I tried to move it to sound/soc/sh,
then it be doesn't work. it say
------------
ALSA device list:
  No soundcards found.
------------

there is no "AK4642 Audio Codec 0.0.1" print,
so, snd_soc_codec_device :: probe isn't called.
can you give me advice ?

Best regards
--
Kuninori Morimoto

Re: [alsa-devel] [PATCH 3/3] Add FSI-AK464x sound support for

[Mark Brown]

On Thu, Aug 20, 2009 at 04:46:15PM +0900, Kuninori Morimoto wrote:

> Dear Mark, pHilipp, Magnus

It's a bit easier to follow things if you reply to individual mails
rather than stitching things together into a singel mail like this.

> > Should this driver also be able to drive AK4641? I'm asking because I

> I checked datasheet.
> My codec is AK4643, and it is compatible with AK4642.
> And AK4642 is not compatible AK4641 (^^;

> I will re-name it AK464x -> AK4642.
> Can you agree with me ?

Yes, that sounds lkike a good approach.  You can add entries to the I2C
device ID table for both ak4642 and ak4643 so it's clear that both
devices are supported.

> > +#define PREALLOC_BUFFER		(32 * 1024)
> > +#define PREALLOC_BUFFER_MAX	(32 * 1024)

> > Is it worth letting machines override these in the platform data or is
> > there no point?

> do you mean it should be possible to change size ?
> If so, is it good as future tasks ?
> This time, 32k is good for me.

It's OK for you but is it going to be the right choice for every system?

> > If possible please try to avoid using a global variable like this.
> > Future processors may come with multiple FSI blocks.

> > Right now I'm quite happy that the CEU driver does not use global

> hmm... I can agree with you.
> But this time, is it good as future tasks ?
> I will fix this problem if I can get free time

> >  arch/sh/boards/mach-se/7724/Makefile     |    3 +-
> >  arch/sh/boards/mach-se/7724/fsi-ak464x.c |   90 ++++++++++++++++++++++++++++++

> > the machine driver itself should be under sound/soc/sh.

> if fsi-ak464x.c is in arch/sh/boards/mach-se/7724/
> it works, and it say

You will be able to load the driver, yes, this is a coding style sort of
issue.  Placing machine drivers outside of ASoC will create maintinance
problems since the ASoC APIs change relatively often.

> but when I tried to move it to sound/soc/sh,
> then it be doesn't work. it say
> ------------
> ALSA device list:
>   No soundcards found.
> ------------

This needn't be anything to worry about - cards can be started at any
time, this is just a snapshot of the cards that were ready at this point
during system startup.

> there is no "AK4642 Audio Codec 0.0.1" print,
> so, snd_soc_codec_device :: probe isn't called.
> can you give me advice ?

If you could repost with the driver under sound/soc/sh it may be
possible to identify where the problem lies.

Re: [alsa-devel] [PATCH 3/3] Add FSI-AK464x sound support for ms7724se

[Kuninori Morimoto]

Dear Mark

Thank you very much

> > do you mean it should be possible to change size ?
> > If so, is it good as future tasks ?
> > This time, 32k is good for me.
> 
> It's OK for you but is it going to be the right choice for every system?

I'm not sure.
But this patch is start point for us.
So, I would like to fix it if we find trouble.
OK ?

> > there is no "AK4642 Audio Codec 0.0.1" print,
> > so, snd_soc_codec_device :: probe isn't called.
> > can you give me advice ?
> 
> If you could repost with the driver under sound/soc/sh it may be
> possible to identify where the problem lies.

Thank you. I could fix it.
The criminal was my miss.
Thank you.

Best regards
--
Kuninori Morimoto
 

Re: [alsa-devel] [PATCH 3/3] Add FSI-AK464x sound support for

[Mark Brown]

On Thu, Aug 20, 2009 at 08:48:12PM +0900, Kuninori Morimoto wrote:

> I'm not sure.
> But this patch is start point for us.
> So, I would like to fix it if we find trouble.
> OK ?

Yes, that's OK - there's a few other issues like the capture support
that will need to be addressed in the future so it wouldn't be the only
thing.