[PATCH] SAMSUNG: S5PV310: Add I2S playback support

[PATCH] SAMSUNG: S5PV310: Add I2S playback support

[Giridhar Maruthy]

This commit adds the platform device and data of
wm8994 regulator to enable the I2S playback on
S5PV310 board on linux-linaro-2.6.38.

Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org>
---
 arch/arm/mach-exynos4/mach-smdkv310.c |  167 ++++++++++++++++++++++++++++++++-
 1 files changed, 166 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-exynos4/mach-smdkv310.c b/arch/arm/mach-exynos4/mach-smdkv310.c
index 07860a5..a4fa3d9 100644
--- a/arch/arm/mach-exynos4/mach-smdkv310.c
+++ b/arch/arm/mach-exynos4/mach-smdkv310.c
@@ -15,6 +15,11 @@
 #include <linux/smsc911x.h>
 #include <linux/io.h>
 #include <linux/i2c.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/max8649.h>
+#include <linux/regulator/fixed.h>
+#include <linux/mfd/wm8994/pdata.h>
 
 #include <asm/mach/arch.h>
 #include <asm/mach-types.h>
@@ -142,9 +147,162 @@ static struct platform_device smdkv310_smsc911x = {
 	},
 };
 
+
+#if defined(CONFIG_SND_SOC_WM8994) || defined(CONFIG_SND_SOC_WM8994_MODULE)
+
+#ifdef CONFIG_REGULATOR_WM8994
+static struct regulator_consumer_supply wm8994_fixed_voltage0_supplies[] = {
+	{
+		.dev_name	= "1-001a",
+		.supply		= "AVDD2",
+	}, {
+		.dev_name	= "1-001a",
+		.supply		= "CPVDD",
+	},
+};
+
+static struct regulator_consumer_supply wm8994_fixed_voltage1_supplies[] = {
+	{
+		.dev_name	= "1-001a",
+		.supply		= "SPKVDD1",
+	}, {
+		.dev_name	= "1-001a",
+		.supply		= "SPKVDD2",
+	},
+};
+
+static struct regulator_consumer_supply wm8994_fixed_voltage2_supplies[] = {
+	{
+		.dev_name	= "1-001a",
+		.supply		= "DBVDD",
+	},
+};
+
+static struct regulator_init_data wm8994_fixed_voltage0_init_data = {
+	.constraints = {
+		.always_on = 1,
+	},
+	.num_consumer_supplies	= ARRAY_SIZE(wm8994_fixed_voltage0_supplies),
+	.consumer_supplies	= wm8994_fixed_voltage0_supplies,
+};
+
+static struct regulator_init_data wm8994_fixed_voltage1_init_data = {
+	.constraints = {
+		.always_on = 1,
+	},
+	.num_consumer_supplies	= ARRAY_SIZE(wm8994_fixed_voltage1_supplies),
+	.consumer_supplies	= wm8994_fixed_voltage1_supplies,
+};
+
+static struct regulator_init_data wm8994_fixed_voltage2_init_data = {
+	.constraints = {
+		.always_on = 1,
+	},
+	.num_consumer_supplies	= ARRAY_SIZE(wm8994_fixed_voltage2_supplies),
+	.consumer_supplies	= wm8994_fixed_voltage2_supplies,
+};
+
+static struct fixed_voltage_config wm8994_fixed_voltage0_config = {
+	.supply_name	= "VDD_1.8V",
+	.microvolts	= 1800000,
+	.gpio		= -EINVAL,
+	.init_data	= &wm8994_fixed_voltage0_init_data,
+};
+
+static struct fixed_voltage_config wm8994_fixed_voltage1_config = {
+	.supply_name	= "DC_5V",
+	.microvolts	= 5000000,
+	.gpio		= -EINVAL,
+	.init_data	= &wm8994_fixed_voltage1_init_data,
+};
+
+static struct fixed_voltage_config wm8994_fixed_voltage2_config = {
+	.supply_name	= "VDD_3.3V",
+	.microvolts	= 3300000,
+	.gpio		= -EINVAL,
+	.init_data	= &wm8994_fixed_voltage2_init_data,
+};
+
+static struct platform_device wm8994_fixed_voltage0 = {
+	.name		= "reg-fixed-voltage",
+	.id		= 0,
+	.dev		= {
+		.platform_data	= &wm8994_fixed_voltage0_config,
+	},
+};
+
+static struct platform_device wm8994_fixed_voltage1 = {
+	.name		= "reg-fixed-voltage",
+	.id		= 1,
+	.dev		= {
+		.platform_data	= &wm8994_fixed_voltage1_config,
+	},
+};
+
+static struct platform_device wm8994_fixed_voltage2 = {
+	.name		= "reg-fixed-voltage",
+	.id		= 2,
+	.dev		= {
+		.platform_data	= &wm8994_fixed_voltage2_config,
+	},
+};
+
+static struct regulator_consumer_supply wm8994_avdd1_supply = {
+	.dev_name	= "1-001a",
+	.supply		= "AVDD1",
+};
+
+static struct regulator_consumer_supply wm8994_dcvdd_supply = {
+	.dev_name	= "1-001a",
+	.supply		= "DCVDD",
+};
+
+static struct regulator_init_data wm8994_ldo1_data = {
+	.constraints	= {
+		.name		= "AVDD1",
+	},
+	.num_consumer_supplies	= 1,
+	.consumer_supplies	= &wm8994_avdd1_supply,
+};
+
+static struct regulator_init_data wm8994_ldo2_data = {
+	.constraints	= {
+		.name		= "DCVDD",
+	},
+	.num_consumer_supplies	= 1,
+	.consumer_supplies	= &wm8994_dcvdd_supply,
+};
+#endif
+
+static struct wm8994_pdata wm8994_platform_data = {
+	/* configure gpio1 function: 0x0001(Logic level input/output) */
+	.gpio_defaults[0] = 0x0001,
+	/* configure gpio3/4/5/7 function for AIF2 voice */
+	.gpio_defaults[2] = 0x8100,/*BCLK2 in*/
+	.gpio_defaults[3] = 0x8100,/*LRCLK2 in*/
+	.gpio_defaults[4] = 0x8100,/*DACDAT2 in*/
+	/* configure gpio6 function: 0x0001(Logic level input/output) */
+	.gpio_defaults[5] = 0x0001,
+	.gpio_defaults[6] = 0x0100,/*ADCDAT2 out*/
+#ifdef CONFIG_REGULATOR_WM8994
+	.ldo[0]	= { 0, NULL, &wm8994_ldo1_data },
+	.ldo[1] = { 0, NULL, &wm8994_ldo2_data },
+#endif
+};
+#endif
+
+#ifdef CONFIG_I2C_S3C2410
+#ifdef CONFIG_S3C_DEV_I2C1
 static struct i2c_board_info i2c_devs1[] __initdata = {
-	{I2C_BOARD_INFO("wm8994", 0x1a),},
+#if defined(CONFIG_SND_SOC_WM8994) || defined(CONFIG_SND_SOC_WM8994_MODULE)
+	{
+		I2C_BOARD_INFO("wm8994", 0x1a),
+		.platform_data	= &wm8994_platform_data,
+	},
+#endif
 };
+#endif
+#endif
 
 static struct platform_device *smdkv310_devices[] __initdata = {
 	&s3c_device_hsmmc0,
@@ -158,6 +316,13 @@ static struct platform_device *smdkv310_devices[] __initdata = {
 	&exynos4_device_i2s0,
 	&exynos4_device_pd[PD_MFC],
 	&exynos4_device_pd[PD_G3D],
+#if (defined(CONFIG_SND_SOC_WM8994) || \
+		defined(CONFIG_SND_SOC_WM8994_MODULE)) && \
+		defined(CONFIG_REGULATOR_WM8994)
+	&wm8994_fixed_voltage0,
+	&wm8994_fixed_voltage1,
+	&wm8994_fixed_voltage2,
+#endif
 	&exynos4_device_pd[PD_LCD0],
 	&exynos4_device_pd[PD_LCD1],
 	&exynos4_device_pd[PD_CAM],
-- 
1.7.1

Re: [PATCH] SAMSUNG: S5PV310: Add I2S playback support

[Mark Brown]

On Tue, Mar 08, 2011 at 04:10:52PM +0530, Giridhar Maruthy wrote:

> +#ifdef CONFIG_REGULATOR_WM8994
> +static struct regulator_consumer_supply wm8994_fixed_voltage0_supplies[] = {
> +	{
> +		.dev_name	= "1-001a",
> +		.supply		= "AVDD2",

This ifdef looks wrong - you're testing for the WM8994 regulators but
the things inside the define are fixed voltage regulators.  Probably
best to just make the whole lot depend on CONFIG_REGULATOR and then
select the regulator drivers if that's enabled:

	select REGULATOR_WM8994 if REGULATOR
	select REGULATOR_FIXED if REGULATOR

(just doing Kconfig symbols from memory, I might be misremembering.)

> +#ifdef CONFIG_I2C_S3C2410
> +#ifdef CONFIG_S3C_DEV_I2C1

The above will break if someone builds I2C modular - there's a separate
_MODULE define for that.  Though TBH I'm not sure all this ifdefery is
really buying much.

Re: [PATCH] SAMSUNG: S5PV310: Add I2S playback support

[Giridhar Bachalli Maruthy]

Hi Mark,

Thanks for your comments.

On 8 March 2011 17:07, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
>
> On Tue, Mar 08, 2011 at 04:10:52PM +0530, Giridhar Maruthy wrote:
>
> > +#ifdef CONFIG_REGULATOR_WM8994
> > +static struct regulator_consumer_supply wm8994_fixed_voltage0_supplies[] = {
> > +     {
> > +             .dev_name       = "1-001a",
> > +             .supply         = "AVDD2",
>
> This ifdef looks wrong - you're testing for the WM8994 regulators but
> the things inside the define are fixed voltage regulators.  Probably
> best to just make the whole lot depend on CONFIG_REGULATOR and then
> select the regulator drivers if that's enabled:
>
>        select REGULATOR_WM8994 if REGULATOR
>        select REGULATOR_FIXED if REGULATOR
>
> (just doing Kconfig symbols from memory, I might be misremembering.)
>
Ok. I will move these configs to the Kconfig file[1] where we select
WM8994 codec.

Since WM8994 codec requires CONFIG_REGULATOR to be defined, would it
be appropriate to select REGULATOR also in [1] and remove the ifdef
CONFIG_REGULATOR?

[1] sound/soc/samsung/Kconfig

> > +#ifdef CONFIG_I2C_S3C2410
> > +#ifdef CONFIG_S3C_DEV_I2C1
>
> The above will break if someone builds I2C modular - there's a separate
> _MODULE define for that.  Though TBH I'm not sure all this ifdefery is
> really buying much.
Yes, you are right. I will get rid of this ifdef.

Regards,
Giridhar

Re: [PATCH] SAMSUNG: S5PV310: Add I2S playback support

[Mark Brown]

On Wed, Mar 09, 2011 at 03:50:18PM +0530, Giridhar Bachalli Maruthy wrote:

> Since WM8994 codec requires CONFIG_REGULATOR to be defined, would it
> be appropriate to select REGULATOR also in [1] and remove the ifdef
> CONFIG_REGULATOR?

What makes you say that CONFIG_REGULATOR is required?

Re: [PATCH] SAMSUNG: S5PV310: Add I2S playback support

[Giridhar Bachalli Maruthy]

Hi Mark

On 9 March 2011 15:54, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Mar 09, 2011 at 03:50:18PM +0530, Giridhar Bachalli Maruthy wrote:
>
>> Since WM8994 codec requires CONFIG_REGULATOR to be defined, would it
>> be appropriate to select REGULATOR also in [1] and remove the ifdef
>> CONFIG_REGULATOR?
>
> What makes you say that CONFIG_REGULATOR is required?
>

You are right...CONFIG_REGULATOR is not required, my mistake.

This patch will be useful when CONFIG_REGULATOR is defined.
I will resubmit the patch after necessary rework.

Regards,
Giridhar

RE: [PATCH] SAMSUNG: S5PV310: Add I2S playback support

[Kukjin Kim]

Giridhar Maruthy wrote:
> 
> This commit adds the platform device and data of
> wm8994 regulator to enable the I2S playback on
> S5PV310 board on linux-linaro-2.6.38.
> 
> Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org>
> ---

(snip)

> +#if defined(CONFIG_SND_SOC_WM8994) ||
defined(CONFIG_SND_SOC_WM8994_MODULE)
> +
> +#ifdef CONFIG_REGULATOR_WM8994
> +static struct regulator_consumer_supply wm8994_fixed_voltage0_supplies[]
= {
> +	{

(snip)

> +};
> +#endif

If required above, please use like following.

+#endif /* CONFIG_REGULATOR_WM8994 */

> +
> +static struct wm8994_pdata wm8994_platform_data = {
> +	/* configure gpio1 function: 0x0001(Logic level input/output) */
> +	.gpio_defaults[0] = 0x0001,
> +	/* configure gpio3/4/5/7 function for AIF2 voice */
> +	.gpio_defaults[2] = 0x8100,/*BCLK2 in*/
> +	.gpio_defaults[3] = 0x8100,/*LRCLK2 in*/
> +	.gpio_defaults[4] = 0x8100,/*DACDAT2 in*/
> +	/* configure gpio6 function: 0x0001(Logic level input/output) */
> +	.gpio_defaults[5] = 0x0001,
> +	.gpio_defaults[6] = 0x0100,/*ADCDAT2 out*/
> +#ifdef CONFIG_REGULATOR_WM8994
> +	.ldo[0]	= { 0, NULL, &wm8994_ldo1_data },
> +	.ldo[1] = { 0, NULL, &wm8994_ldo2_data },
> +#endif
> +};
> +#endif

Same as above.

> +
> +#ifdef CONFIG_I2C_S3C2410
> +#ifdef CONFIG_S3C_DEV_I2C1

Do we _really_ S3C_DEV_I2C1?
Basically, already selected above in mach-exynos4/Kconfig.

Of course, no need I2C_S3C2410 also here.

>  static struct i2c_board_info i2c_devs1[] __initdata = {
> -	{I2C_BOARD_INFO("wm8994", 0x1a),},
> +#if defined(CONFIG_SND_SOC_WM8994) ||
defined(CONFIG_SND_SOC_WM8994_MODULE)
> +	{
> +		I2C_BOARD_INFO("wm8994", 0x1a),
> +		.platform_data	= &wm8994_platform_data,
> +	},
> +#endif
>  };
> +#endif
> +#endif
> 
>  static struct platform_device *smdkv310_devices[] __initdata = {
>  	&s3c_device_hsmmc0,
> @@ -158,6 +316,13 @@ static struct platform_device *smdkv310_devices[]
> __initdata = {
>  	&exynos4_device_i2s0,
>  	&exynos4_device_pd[PD_MFC],
>  	&exynos4_device_pd[PD_G3D],
> +#if (defined(CONFIG_SND_SOC_WM8994) || \
> +		defined(CONFIG_SND_SOC_WM8994_MODULE)) && \
> +		defined(CONFIG_REGULATOR_WM8994)
> +	&wm8994_fixed_voltage0,
> +	&wm8994_fixed_voltage1,
> +	&wm8994_fixed_voltage2,
> +#endif
>  	&exynos4_device_pd[PD_LCD0],
>  	&exynos4_device_pd[PD_LCD1],
>  	&exynos4_device_pd[PD_CAM],
> --

Please make sure that ifdefs has no dependencies and no problem before
re-submitting.
I think, as Mark said, have to check whether need or not.


Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Re: [PATCH] SAMSUNG: S5PV310: Add I2S playback support

[Seungwhan Youn]

Hi,

On Tue, Mar 8, 2011 at 7:40 PM, Giridhar Maruthy
<giridhar.maruthy@linaro.org> wrote:
> This commit adds the platform device and data of
> wm8994 regulator to enable the I2S playback on
> S5PV310 board on linux-linaro-2.6.38.
>
> Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org>
> ---
>  arch/arm/mach-exynos4/mach-smdkv310.c |  167 ++++++++++++++++++++++++++++++++-
>  1 files changed, 166 insertions(+), 1 deletions(-)
>

I'm so sad about this patch is pretty much same as my previous patch,
"ARM: SMDKC210: Add regulator settings for WM8994
audio"(http://git.kernel.org/?p=linux/kernel/git/kki_ap/linux-2.6-samsung.git;a=commitdiff;h=1307c0dac85bc5e5f26de4f615040a2b06eb64ac),
and there is no comment about this source comes from in your patch.
But, I think that you just _forgot_ to write this comment and you will
add it into your re-worked patch. ;)

Thanks,
Claude.

Re: [PATCH] SAMSUNG: S5PV310: Add I2S playback support

[Giridhar Bachalli Maruthy]

Hi Claude,

On 10 March 2011 15:38, Seungwhan Youn <claude.youn@gmail.com> wrote:
> Hi,
>
> On Tue, Mar 8, 2011 at 7:40 PM, Giridhar Maruthy
> <giridhar.maruthy@linaro.org> wrote:
>> This commit adds the platform device and data of
>> wm8994 regulator to enable the I2S playback on
>> S5PV310 board on linux-linaro-2.6.38.
>>
>> Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org>
>> ---
>>  arch/arm/mach-exynos4/mach-smdkv310.c |  167 ++++++++++++++++++++++++++++++++-
>>  1 files changed, 166 insertions(+), 1 deletions(-)
>>
>
> I'm so sad about this patch is pretty much same as my previous patch,
> "ARM: SMDKC210: Add regulator settings for WM8994
> audio"(http://git.kernel.org/?p=linux/kernel/git/kki_ap/linux-2.6-samsung.git;a=commitdiff;h=1307c0dac85bc5e5f26de4f615040a2b06eb64ac),
> and there is no comment about this source comes from in your patch.
> But, I think that you just _forgot_ to write this comment and you will
> add it into your re-worked patch. ;)
>
> Thanks,
> Claude.
>
Thanks for the link. Actually I had completely missed that out.
I will be adding appropriate _Signed-off-by_ when I resubmit the patch.

Regards,
Giridhar