[PATCH 1/2] ARM: S5PV210: Add audio support to Aquila

[PATCH 1/2] ARM: S5PV210: Add audio support to Aquila

[Chanwoo Choi]

This patch the I2C board information for the WM8994 used in the Aquila
as audio codec and adds the I2C/I2S platform drivers. Additionlly, to
control power consumption have registerd the voltage consumer of WM8994
to the regulator framework. I initialize gpio settting relevant to
operation of audio codec. I explain following comment concering gpio
setting:
- CODEC_XTAL_EN : This gpio enables that the main clock is provided
  to operate audio codec.
- MICBIAS_EN : This gpio enable the microphone to input analog
- ADC_EN : This gpio enable the ADC device which is used to detect
  the kind of jack. (SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT)
  According to the kind of jack, an electric current is changed.

Signed-off-by : Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by : Joonyoung Shim <jy0922.shim@samsung.com>
Signed-off-by : Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-s5pv210/mach-aquila.c |  177 +++++++++++++++++++++++++++++++++++
 1 files changed, 177 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-s5pv210/mach-aquila.c b/arch/arm/mach-s5pv210/mach-aquila.c
index f0f960f..d027f9d 100644
--- a/arch/arm/mach-s5pv210/mach-aquila.c
+++ b/arch/arm/mach-s5pv210/mach-aquila.c
@@ -17,9 +17,11 @@
 #include <linux/i2c-gpio.h>
 #include <linux/i2c/mcs.h>
 #include <linux/mfd/max8998.h>
+#include <linux/mfd/wm8994/pdata.h>
 #include <linux/gpio_keys.h>
 #include <linux/input.h>
 #include <linux/gpio.h>
+#include <linux/regulator/fixed.h>
 
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
@@ -397,6 +399,120 @@ static struct max8998_platform_data aquila_max8998_pdata = {
 };
 #endif
 
+static struct regulator_consumer_supply wm8994_fixed_voltage0_supplies[] = {
+	{
+		.dev_name	= "5-001a",
+		.supply		= "DBVDD",
+	}, {
+		.dev_name	= "5-001a",
+		.supply		= "AVDD2",
+	}, {
+		.dev_name	= "5-001a",
+		.supply		= "CPVDD",
+	},
+
+};
+
+static struct regulator_consumer_supply wm8994_fixed_voltage1_supplies[] = {
+	{
+		.dev_name	= "5-001a",
+		.supply		= "SPKVDD1",
+	}, {
+		.dev_name	= "5-001a",
+		.supply		= "SPKVDD2",
+	},
+};
+
+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 fixed_voltage_config wm8994_fixed_voltage0_config = {
+	.supply_name	= "VCC_1.8V_PDA",
+	.microvolts	= 1800000,
+	.gpio		= -EINVAL,
+	.init_data	= &wm8994_fixed_voltage0_init_data,
+};
+
+static struct fixed_voltage_config wm8994_fixed_voltage1_config = {
+	.supply_name	= "V_BAT",
+	.microvolts	= 3700000,
+	.gpio		= -EINVAL,
+	.init_data	= &wm8994_fixed_voltage1_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 regulator_consumer_supply wm8994_avdd1_supply = {
+	.dev_name	= "5-001a",
+	.supply		= "AVDD1",
+};
+
+static struct regulator_consumer_supply wm8994_dcvdd_supply = {
+	.dev_name	= "5-001a",
+	.supply		= "DCVDD",
+};
+
+static struct regulator_init_data wm8994_ldo1_data = {
+	.constraints	= {
+		.name		= "AVDD1_3.0V",
+		.valid_ops_mask	= REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies	= 1,
+	.consumer_supplies	= &wm8994_avdd1_supply,
+};
+
+static struct regulator_init_data wm8994_ldo2_data = {
+	.constraints	= {
+		.name		= "DCVDD_1.0V",
+	},
+	.num_consumer_supplies	= 1,
+	.consumer_supplies	= &wm8994_dcvdd_supply,
+};
+
+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,
+	.gpio_defaults[3] = 0x8100,
+	.gpio_defaults[4] = 0x8100,
+	.gpio_defaults[6] = 0x0100,
+	/* configure gpio8/9/10/11 function for AIF3 BT */
+	.gpio_defaults[7] = 0x8100,
+	.gpio_defaults[8] = 0x0100,
+	.gpio_defaults[9] = 0x0100,
+	.gpio_defaults[10] = 0x0100,
+	.ldo[0]	= { S5PV210_MP03(6), NULL, &wm8994_ldo1_data },	/* XM0FRNB_2 */
+	.ldo[1]	= { 0, NULL, &wm8994_ldo2_data },
+};
+
 /* GPIO I2C PMIC */
 #define AP_I2C_GPIO_PMIC_BUS_4	4
 static struct i2c_gpio_platform_data aquila_i2c_gpio_pmic_data = {
@@ -422,6 +538,58 @@ static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = {
 #endif
 };
 
+/* GPIO I2C AP 1.8V */
+#define AP_I2C_GPIO_BUS_5	5
+static struct i2c_gpio_platform_data i2c_gpio5_data = {
+	.sda_pin	= S5PV210_MP05(3),	/* XM0ADDR_11 */
+	.scl_pin	= S5PV210_MP05(2),	/* XM0ADDR_10 */
+};
+
+static struct platform_device i2c_gpio5 = {
+	.name		= "i2c-gpio",
+	.id		= AP_I2C_GPIO_BUS_5,
+	.dev		= {
+		.platform_data	= &i2c_gpio5_data,
+	},
+};
+
+static struct i2c_board_info i2c_gpio5_devs[] __initdata = {
+	{
+		/* CS/ADDR = low 0x34 (FYI: high = 0x36) */
+		I2C_BOARD_INFO("wm8994", 0x34 >> 1),
+		.platform_data	= &wm8994_platform_data,
+	},
+};
+
+static void __init aquila_sound_init(void)
+{
+	unsigned int gpio;
+
+	/* CODEC_XTAL_EN */
+	gpio = S5PV210_GPH3(2);		/* XEINT_26 */
+	gpio_request(gpio, "CODEC_XTAL_EN");
+	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
+	s3c_gpio_setpull(gpio, S3C_GPIO_PULL_NONE);
+	gpio_direction_output(gpio, 1);
+
+	/* CLKOUT[9:8] set to 0x3(XUSBXTI) of 0xE010E000(OTHERS)
+	 * for 24MHZ
+	 */
+	writel(readl(S5P_OTHERS) | (0x3 << 8), S5P_OTHERS);
+
+	/* MICBIAS_EN */
+	gpio = S5PV210_GPJ4(2);			/* XMSMRN */
+	gpio_request(gpio, "MICBIAS_EN");
+	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
+	gpio_direction_output(gpio, 1);
+
+	/* ADC_EN */
+	gpio = S5PV210_GPJ3(2);
+	gpio_request(gpio, "ADC_EN");
+	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
+	gpio_direction_output(gpio, 1);
+}
+
 /* PMIC Power button */
 static struct gpio_keys_button aquila_gpio_keys_table[] = {
 	{
@@ -523,6 +691,10 @@ static struct platform_device *aquila_devices[] __initdata = {
 	&s5pc110_device_onenand,
 	&samsung_device_keypad,
 	&i2c_gpio10,
+	&wm8994_fixed_voltage0,
+	&wm8994_fixed_voltage1,
+	&s5pv210_device_iis0,
+	&i2c_gpio5,
 };
 
 static void __init aquila_map_io(void)
@@ -539,6 +711,11 @@ static void __init aquila_machine_init(void)
 	i2c_register_board_info(AP_I2C_GPIO_PMIC_BUS_4, i2c_gpio_pmic_devs,
 			ARRAY_SIZE(i2c_gpio_pmic_devs));
 
+	/* SOUND */
+	i2c_register_board_info(AP_I2C_GPIO_BUS_5, i2c_gpio5_devs,
+			ARRAY_SIZE(i2c_gpio5_devs));
+	aquila_sound_init();
+
 	/* FB */
 	s3c_fb_set_platdata(&aquila_lcd_pdata);
 
-- 
1.6.3.3

Re: [PATCH 1/2] ARM: S5PV210: Add audio support to Aquila

[Mark Brown]

On Wed, Jul 28, 2010 at 12:04:44PM +0900, Chanwoo Choi wrote:

> +static struct regulator_consumer_supply wm8994_fixed_voltage0_supplies[] = {
> +	{
> +		.dev_name	= "5-001a",
> +		.supply		= "DBVDD",
> +	}, {
> +		.dev_name	= "5-001a",
> +		.supply		= "AVDD2",
> +	}, {
> +		.dev_name	= "5-001a",
> +		.supply		= "CPVDD",
> +	},
> +
> +};
> +
> +static struct regulator_consumer_supply wm8994_fixed_voltage1_supplies[] = {

All these fixed voltage regulators seem a bit suspicous for a mobile
phone - I'd have expected that the supplies would all be being provided
by your PMIC except for things taken directly from the battery supply
(like the speakers tend to be, for example)?  There's no problem with
the code itself, it just looks a bit odd.

> +static struct i2c_board_info i2c_gpio5_devs[] __initdata = {
> +	{
> +		/* CS/ADDR = low 0x34 (FYI: high = 0x36) */
> +		I2C_BOARD_INFO("wm8994", 0x34 >> 1),
> +		.platform_data	= &wm8994_platform_data,
> +	},
> +};

Probably clearer for generic Linux use to specify the address as 0x1a
directly.

> +static void __init aquila_sound_init(void)
> +{
> +	unsigned int gpio;
> +
> +	/* CODEC_XTAL_EN */
> +	gpio = S5PV210_GPH3(2);		/* XEINT_26 */
> +	gpio_request(gpio, "CODEC_XTAL_EN");
> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
> +	s3c_gpio_setpull(gpio, S3C_GPIO_PULL_NONE);
> +	gpio_direction_output(gpio, 1);

Might be as well to provide some or all this stuff in your audio machine
driver?

> +	/* MICBIAS_EN */
> +	gpio = S5PV210_GPJ4(2);			/* XMSMRN */
> +	gpio_request(gpio, "MICBIAS_EN");
> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
> +	gpio_direction_output(gpio, 1);

This in particular would benefit from keeping the request of the GPIO
joined up with the driver that uses it.

> +	/* ADC_EN */
> +	gpio = S5PV210_GPJ3(2);
> +	gpio_request(gpio, "ADC_EN");
> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
> +	gpio_direction_output(gpio, 1);

I'm not sure what this does?

Re: [PATCH 1/2] ARM: S5PV210: Add audio support to Aquila

[Chanwoo Choi]

Mark Brown wrote:
> On Wed, Jul 28, 2010 at 12:04:44PM +0900, Chanwoo Choi wrote:
> 
>> +static struct regulator_consumer_supply wm8994_fixed_voltage0_supplies[] = {
>> +	{
>> +		.dev_name	= "5-001a",
>> +		.supply		= "DBVDD",
>> +	}, {
>> +		.dev_name	= "5-001a",
>> +		.supply		= "AVDD2",
>> +	}, {
>> +		.dev_name	= "5-001a",
>> +		.supply		= "CPVDD",
>> +	},
>> +
>> +};
>> +
>> +static struct regulator_consumer_supply wm8994_fixed_voltage1_supplies[] = {
> 
> All these fixed voltage regulators seem a bit suspicous for a mobile
> phone - I'd have expected that the supplies would all be being provided
> by your PMIC except for things taken directly from the battery supply
> (like the speakers tend to be, for example)?  There's no problem with
> the code itself, it just looks a bit odd.
> 

All these consumer supply of WM8994 codec connected the below regulator(VCC_1.8V) on
a circuit diagram. "VCC_1.8V" regualtor is always enabled, because it is used to many devices.
Then I haven't connected all these consumer supply of WM8994 codec to "VCC_1.8V" regulator.
I will modify that the consumer supply would be provided by PMIC.

static struct regulator_init_data aquila_buck3_data = {
	.constraints	= {
		.name		= "VCC_1.8V",
		.min_uV		= 1800000,
		.max_uV		= 1800000,
		.apply_uV	= 1,
		.state_mem	= {
			.enabled = 1,
		},
	},
};

>> +static struct i2c_board_info i2c_gpio5_devs[] __initdata = {
>> +	{
>> +		/* CS/ADDR = low 0x34 (FYI: high = 0x36) */
>> +		I2C_BOARD_INFO("wm8994", 0x34 >> 1),
>> +		.platform_data	= &wm8994_platform_data,
>> +	},
>> +};
> 
> Probably clearer for generic Linux use to specify the address as 0x1a
> directly.

Ok, I will do.

> 
>> +static void __init aquila_sound_init(void)
>> +{
>> +	unsigned int gpio;
>> +
>> +	/* CODEC_XTAL_EN */
>> +	gpio = S5PV210_GPH3(2);		/* XEINT_26 */
>> +	gpio_request(gpio, "CODEC_XTAL_EN");
>> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
>> +	s3c_gpio_setpull(gpio, S3C_GPIO_PULL_NONE);
>> +	gpio_direction_output(gpio, 1);
> 
> Might be as well to provide some or all this stuff in your audio machine
> driver?

The Aquila board have a oscillator which provide main clock to 
WM8994 audio codec. The oscillator provide 24MHz clock to WM8994 audio codec
(MCLK1 pin). I set gpio setting of "CODEC_XTAL_EN" to enable a oscillator.

> 
>> +	/* MICBIAS_EN */
>> +	gpio = S5PV210_GPJ4(2);			/* XMSMRN */
>> +	gpio_request(gpio, "MICBIAS_EN");
>> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
>> +	gpio_direction_output(gpio, 1);
> 
> This in particular would benefit from keeping the request of the GPIO
> joined up with the driver that uses it.

Ok, I will move this code to machine driver(sound/soc/s3c24xxx/aquila_wm8994.c).

>> +	/* ADC_EN */
>> +	gpio = S5PV210_GPJ3(2);
>> +	gpio_request(gpio, "ADC_EN");
>> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
>> +	gpio_direction_output(gpio, 1);
> 
> I'm not sure what this does?
> 

I explained below description about "ADC_EN" :
"ADC_EN : This gpio enable the ADC device which is used to detect
 the kind of jack. (SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT)
 According to the kind of jack, an electric current is changed.
 (Only used on Aquila board) "

When inserting the jack to Aquila board, I used ADC driver so that, detecting
the kind of jack(SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT).

I will separately make the another function to initialize ADC driver.

Thank you for your comment.
Chanwoo Choi

Re: [PATCH 1/2] ARM: S5PV210: Add audio support to Aquila

[MyungJoo Ham]

On Fri, Jul 30, 2010 at 4:15 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> Mark Brown wrote:
>> On Wed, Jul 28, 2010 at 12:04:44PM +0900, Chanwoo Choi wrote:
>>
>>> +static struct regulator_consumer_supply wm8994_fixed_voltage0_supplies[] = {
>>> +    {
>>> +            .dev_name       = "5-001a",
>>> +            .supply         = "DBVDD",
>>> +    }, {
>>> +            .dev_name       = "5-001a",
>>> +            .supply         = "AVDD2",
>>> +    }, {
>>> +            .dev_name       = "5-001a",
>>> +            .supply         = "CPVDD",
>>> +    },
>>> +
>>> +};
>>> +
>>> +static struct regulator_consumer_supply wm8994_fixed_voltage1_supplies[] = {
>>
>> All these fixed voltage regulators seem a bit suspicous for a mobile
>> phone - I'd have expected that the supplies would all be being provided
>> by your PMIC except for things taken directly from the battery supply
>> (like the speakers tend to be, for example)?  There's no problem with
>> the code itself, it just looks a bit odd.
>>
>
> All these consumer supply of WM8994 codec connected the below regulator(VCC_1.8V) on
> a circuit diagram. "VCC_1.8V" regualtor is always enabled, because it is used to many devices.
> Then I haven't connected all these consumer supply of WM8994 codec to "VCC_1.8V" regulator.
> I will modify that the consumer supply would be provided by PMIC.
>
> static struct regulator_init_data aquila_buck3_data = {
>        .constraints    = {
>                .name           = "VCC_1.8V",
>                .min_uV         = 1800000,
>                .max_uV         = 1800000,
>                .apply_uV       = 1,
>                .state_mem      = {
>                        .enabled = 1,
>                },
>        },
> };

This aquila_buck3_data.constraints would be better have ".always_on =
1" entry as this buck3 is required to be turned on at all times in
Aquila boards (even when the system is in suspend-to-mem state). This
is required especially when there are many devices physically attached
to Buck3 and some of they did not "register" as consumers to Buck3.
Buck3 might be turned off by those who are registered while
"unregistered" are still active.

>
>>> +static struct i2c_board_info i2c_gpio5_devs[] __initdata = {
>>> +    {
>>> +            /* CS/ADDR = low 0x34 (FYI: high = 0x36) */
>>> +            I2C_BOARD_INFO("wm8994", 0x34 >> 1),
>>> +            .platform_data  = &wm8994_platform_data,
>>> +    },
>>> +};
>>
>> Probably clearer for generic Linux use to specify the address as 0x1a
>> directly.
>
> Ok, I will do.
>
>>
>>> +static void __init aquila_sound_init(void)
>>> +{
>>> +    unsigned int gpio;
>>> +
>>> +    /* CODEC_XTAL_EN */
>>> +    gpio = S5PV210_GPH3(2);         /* XEINT_26 */
>>> +    gpio_request(gpio, "CODEC_XTAL_EN");
>>> +    s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
>>> +    s3c_gpio_setpull(gpio, S3C_GPIO_PULL_NONE);
>>> +    gpio_direction_output(gpio, 1);
>>
>> Might be as well to provide some or all this stuff in your audio machine
>> driver?
>
> The Aquila board have a oscillator which provide main clock to
> WM8994 audio codec. The oscillator provide 24MHz clock to WM8994 audio codec
> (MCLK1 pin). I set gpio setting of "CODEC_XTAL_EN" to enable a oscillator.
>
>>
>>> +    /* MICBIAS_EN */
>>> +    gpio = S5PV210_GPJ4(2);                 /* XMSMRN */
>>> +    gpio_request(gpio, "MICBIAS_EN");
>>> +    s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
>>> +    gpio_direction_output(gpio, 1);
>>
>> This in particular would benefit from keeping the request of the GPIO
>> joined up with the driver that uses it.
>
> Ok, I will move this code to machine driver(sound/soc/s3c24xxx/aquila_wm8994.c).
>
>>> +    /* ADC_EN */
>>> +    gpio = S5PV210_GPJ3(2);
>>> +    gpio_request(gpio, "ADC_EN");
>>> +    s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
>>> +    gpio_direction_output(gpio, 1);
>>
>> I'm not sure what this does?
>>
>
> I explained below description about "ADC_EN" :
> "ADC_EN : This gpio enable the ADC device which is used to detect
>  the kind of jack. (SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT)
>  According to the kind of jack, an electric current is changed.
>  (Only used on Aquila board) "
>
> When inserting the jack to Aquila board, I used ADC driver so that, detecting
> the kind of jack(SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT).
>
> I will separately make the another function to initialize ADC driver.
>
> Thank you for your comment.
> Chanwoo Choi
>
>
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>



-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

Re: [PATCH 1/2] ARM: S5PV210: Add audio support to Aquila

[Mark Brown]

On Fri, Jul 30, 2010 at 04:15:29PM +0900, Chanwoo Choi wrote:

> > All these fixed voltage regulators seem a bit suspicous for a mobile
> > phone - I'd have expected that the supplies would all be being provided
> > by your PMIC except for things taken directly from the battery supply
> > (like the speakers tend to be, for example)?  There's no problem with
> > the code itself, it just looks a bit odd.

> All these consumer supply of WM8994 codec connected the below regulator(VCC_1.8V) on
> a circuit diagram. "VCC_1.8V" regualtor is always enabled, because it is used to many devices.
> Then I haven't connected all these consumer supply of WM8994 codec to "VCC_1.8V" regulator.
> I will modify that the consumer supply would be provided by PMIC.

OK, good - the fact that the connection wasn't being made with the PMIC
was my main point here.

> >> +	/* CODEC_XTAL_EN */
> >> +	gpio = S5PV210_GPH3(2);		/* XEINT_26 */
> >> +	gpio_request(gpio, "CODEC_XTAL_EN");

> > Might be as well to provide some or all this stuff in your audio machine
> > driver?

> The Aquila board have a oscillator which provide main clock to 
> WM8994 audio codec. The oscillator provide 24MHz clock to WM8994 audio codec
> (MCLK1 pin). I set gpio setting of "CODEC_XTAL_EN" to enable a oscillator.

I understand what the code does.  My point is that it looks like it's
in the wrong place - it'd seem nicer (and to open up more optimisation
opportunties to manage the clock in the audio driver).

> >> +	/* ADC_EN */
> >> +	gpio = S5PV210_GPJ3(2);
> >> +	gpio_request(gpio, "ADC_EN");
> >> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
> >> +	gpio_direction_output(gpio, 1);

> > I'm not sure what this does?

> I explained below description about "ADC_EN" :
> "ADC_EN : This gpio enable the ADC device which is used to detect
>  the kind of jack. (SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT)
>  According to the kind of jack, an electric current is changed.
>  (Only used on Aquila board) "

> When inserting the jack to Aquila board, I used ADC driver so that, detecting
> the kind of jack(SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT).

> I will separately make the another function to initialize ADC driver.

So you're using an external ADC to measure the current draw on micbias
as part of your jack sense.  Again, this feels like something that ought
to be managed along with the rest of the audio code.

Re: [alsa-devel] [PATCH 1/2] ARM: S5PV210: Add audio support to Aquila

[Chanwoo Choi]

Mark Brown wrote:
> On Fri, Jul 30, 2010 at 04:15:29PM +0900, Chanwoo Choi wrote:
> 
>>> All these fixed voltage regulators seem a bit suspicous for a mobile
>>> phone - I'd have expected that the supplies would all be being provided
>>> by your PMIC except for things taken directly from the battery supply
>>> (like the speakers tend to be, for example)?  There's no problem with
>>> the code itself, it just looks a bit odd.
> 
>> All these consumer supply of WM8994 codec connected the below regulator(VCC_1.8V) on
>> a circuit diagram. "VCC_1.8V" regualtor is always enabled, because it is used to many devices.
>> Then I haven't connected all these consumer supply of WM8994 codec to "VCC_1.8V" regulator.
>> I will modify that the consumer supply would be provided by PMIC.
> 
> OK, good - the fact that the connection wasn't being made with the PMIC
> was my main point here.

Did you read writed mail about consumer supply of WM8994 by MyungJoo Ham?
He is in charge of PMIC and Power Management on Aquila board.

MyungJoo Ham is wrote : 
This aquila_buck3_data.constraints would be better have ".always_on =
1" entry as this buck3 is required to be turned on at all times in
Aquila boards (even when the system is in suspend-to-mem state). This
is required especially when there are many devices physically attached
to Buck3 and some of they did not "register" as consumers to Buck3.
Buck3 might be turned off by those who are registered while
"unregistered" are still active.

Can you tell me your opinion about the review by MyungJoo Ham?
I will respect your response.

Re: [alsa-devel] [PATCH 1/2] ARM: S5PV210: Add audio support to Aquila

[Mark Brown]

On Wed, Aug 04, 2010 at 03:16:51PM +0900, Chanwoo Choi wrote:
> Mark Brown wrote:

> > OK, good - the fact that the connection wasn't being made with the PMIC
> > was my main point here.

> Did you read writed mail about consumer supply of WM8994 by MyungJoo Ham?

I saw it, yes.

> He is in charge of PMIC and Power Management on Aquila board.

> MyungJoo Ham is wrote : 
> This aquila_buck3_data.constraints would be better have ".always_on =
> 1" entry as this buck3 is required to be turned on at all times in
> Aquila boards (even when the system is in suspend-to-mem state). This
> is required especially when there are many devices physically attached
> to Buck3 and some of they did not "register" as consumers to Buck3.
> Buck3 might be turned off by those who are registered while
> "unregistered" are still active.

> Can you tell me your opinion about the review by MyungJoo Ham?
> I will respect your response.

I think his comments are sensible.  The constraints should always try to
reflect the actual constraints of the system as closely as possible.

RE: [PATCH 1/2] ARM: S5PV210: Add audio support to Aquila

[Kukjin Kim]

Chanwoo Choi wrote:
> 
> Mark Brown wrote:
> > On Wed, Jul 28, 2010 at 12:04:44PM +0900, Chanwoo Choi wrote:
> >
> >> +static struct regulator_consumer_supply wm8994_fixed_voltage0_supplies[] =
> {
> >> +	{
> >> +		.dev_name	= "5-001a",
> >> +		.supply		= "DBVDD",
> >> +	}, {
> >> +		.dev_name	= "5-001a",
> >> +		.supply		= "AVDD2",
> >> +	}, {
> >> +		.dev_name	= "5-001a",
> >> +		.supply		= "CPVDD",
> >> +	},
> >> +
> >> +};
> >> +
> >> +static struct regulator_consumer_supply wm8994_fixed_voltage1_supplies[] =
> {
> >
> > All these fixed voltage regulators seem a bit suspicous for a mobile
> > phone - I'd have expected that the supplies would all be being provided
> > by your PMIC except for things taken directly from the battery supply
> > (like the speakers tend to be, for example)?  There's no problem with
> > the code itself, it just looks a bit odd.
> >
> 
> All these consumer supply of WM8994 codec connected the below
> regulator(VCC_1.8V) on
> a circuit diagram. "VCC_1.8V" regualtor is always enabled, because it is used to
> many devices.
> Then I haven't connected all these consumer supply of WM8994 codec to
> "VCC_1.8V" regulator.
> I will modify that the consumer supply would be provided by PMIC.
> 
> static struct regulator_init_data aquila_buck3_data = {
> 	.constraints	= {
> 		.name		= "VCC_1.8V",
> 		.min_uV		= 1800000,
> 		.max_uV		= 1800000,
> 		.apply_uV	= 1,
> 		.state_mem	= {
> 			.enabled = 1,
> 		},
> 	},
> };
> 
> >> +static struct i2c_board_info i2c_gpio5_devs[] __initdata = {
> >> +	{
> >> +		/* CS/ADDR = low 0x34 (FYI: high = 0x36) */
> >> +		I2C_BOARD_INFO("wm8994", 0x34 >> 1),
> >> +		.platform_data	= &wm8994_platform_data,
> >> +	},
> >> +};
> >
> > Probably clearer for generic Linux use to specify the address as 0x1a
> > directly.
> 
> Ok, I will do.
> 
> >
> >> +static void __init aquila_sound_init(void)
> >> +{
> >> +	unsigned int gpio;
> >> +
> >> +	/* CODEC_XTAL_EN */
> >> +	gpio = S5PV210_GPH3(2);		/* XEINT_26 */
> >> +	gpio_request(gpio, "CODEC_XTAL_EN");
> >> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
> >> +	s3c_gpio_setpull(gpio, S3C_GPIO_PULL_NONE);
> >> +	gpio_direction_output(gpio, 1);
> >
> > Might be as well to provide some or all this stuff in your audio machine
> > driver?
> 
> The Aquila board have a oscillator which provide main clock to
> WM8994 audio codec. The oscillator provide 24MHz clock to WM8994 audio codec
> (MCLK1 pin). I set gpio setting of "CODEC_XTAL_EN" to enable a oscillator.
> 
> >
> >> +	/* MICBIAS_EN */
> >> +	gpio = S5PV210_GPJ4(2);			/* XMSMRN */
> >> +	gpio_request(gpio, "MICBIAS_EN");
> >> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
> >> +	gpio_direction_output(gpio, 1);
> >
> > This in particular would benefit from keeping the request of the GPIO
> > joined up with the driver that uses it.
> 
> Ok, I will move this code to machine driver(sound/soc/s3c24xxx/aquila_wm8994.c).
> 
> >> +	/* ADC_EN */
> >> +	gpio = S5PV210_GPJ3(2);
> >> +	gpio_request(gpio, "ADC_EN");
> >> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
> >> +	gpio_direction_output(gpio, 1);
> >
> > I'm not sure what this does?
> >
> 
> I explained below description about "ADC_EN" :
> "ADC_EN : This gpio enable the ADC device which is used to detect
>  the kind of jack. (SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT)
>  According to the kind of jack, an electric current is changed.
>  (Only used on Aquila board) "
> 
> When inserting the jack to Aquila board, I used ADC driver so that, detecting
> the kind of jack(SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT).
> 
> I will separately make the another function to initialize ADC driver.
> 
> Thank you for your comment.
> Chanwoo Choi
> 
Hi,

How was going on?

Thanks.

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

Re: [PATCH 1/2] ARM: S5PV210: Add audio support to Aquila

[Chanwoo Choi]

Kukjin Kim wrote:
> Chanwoo Choi wrote:
>> Mark Brown wrote:
>>> On Wed, Jul 28, 2010 at 12:04:44PM +0900, Chanwoo Choi wrote:
>>>
>>>> +static struct regulator_consumer_supply wm8994_fixed_voltage0_supplies[] =
>> {
>>>> +	{
>>>> +		.dev_name	= "5-001a",
>>>> +		.supply		= "DBVDD",
>>>> +	}, {
>>>> +		.dev_name	= "5-001a",
>>>> +		.supply		= "AVDD2",
>>>> +	}, {
>>>> +		.dev_name	= "5-001a",
>>>> +		.supply		= "CPVDD",
>>>> +	},
>>>> +
>>>> +};
>>>> +
>>>> +static struct regulator_consumer_supply wm8994_fixed_voltage1_supplies[] =
>> {
>>> All these fixed voltage regulators seem a bit suspicous for a mobile
>>> phone - I'd have expected that the supplies would all be being provided
>>> by your PMIC except for things taken directly from the battery supply
>>> (like the speakers tend to be, for example)?  There's no problem with
>>> the code itself, it just looks a bit odd.
>>>
>> All these consumer supply of WM8994 codec connected the below
>> regulator(VCC_1.8V) on
>> a circuit diagram. "VCC_1.8V" regualtor is always enabled, because it is used to
>> many devices.
>> Then I haven't connected all these consumer supply of WM8994 codec to
>> "VCC_1.8V" regulator.
>> I will modify that the consumer supply would be provided by PMIC.
>>
>> static struct regulator_init_data aquila_buck3_data = {
>> 	.constraints	= {
>> 		.name		= "VCC_1.8V",
>> 		.min_uV		= 1800000,
>> 		.max_uV		= 1800000,
>> 		.apply_uV	= 1,
>> 		.state_mem	= {
>> 			.enabled = 1,
>> 		},
>> 	},
>> };
>>
>>>> +static struct i2c_board_info i2c_gpio5_devs[] __initdata = {
>>>> +	{
>>>> +		/* CS/ADDR = low 0x34 (FYI: high = 0x36) */
>>>> +		I2C_BOARD_INFO("wm8994", 0x34 >> 1),
>>>> +		.platform_data	= &wm8994_platform_data,
>>>> +	},
>>>> +};
>>> Probably clearer for generic Linux use to specify the address as 0x1a
>>> directly.
>> Ok, I will do.
>>
>>>> +static void __init aquila_sound_init(void)
>>>> +{
>>>> +	unsigned int gpio;
>>>> +
>>>> +	/* CODEC_XTAL_EN */
>>>> +	gpio = S5PV210_GPH3(2);		/* XEINT_26 */
>>>> +	gpio_request(gpio, "CODEC_XTAL_EN");
>>>> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
>>>> +	s3c_gpio_setpull(gpio, S3C_GPIO_PULL_NONE);
>>>> +	gpio_direction_output(gpio, 1);
>>> Might be as well to provide some or all this stuff in your audio machine
>>> driver?
>> The Aquila board have a oscillator which provide main clock to
>> WM8994 audio codec. The oscillator provide 24MHz clock to WM8994 audio codec
>> (MCLK1 pin). I set gpio setting of "CODEC_XTAL_EN" to enable a oscillator.
>>
>>>> +	/* MICBIAS_EN */
>>>> +	gpio = S5PV210_GPJ4(2);			/* XMSMRN */
>>>> +	gpio_request(gpio, "MICBIAS_EN");
>>>> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
>>>> +	gpio_direction_output(gpio, 1);
>>> This in particular would benefit from keeping the request of the GPIO
>>> joined up with the driver that uses it.
>> Ok, I will move this code to machine driver(sound/soc/s3c24xxx/aquila_wm8994.c).
>>
>>>> +	/* ADC_EN */
>>>> +	gpio = S5PV210_GPJ3(2);
>>>> +	gpio_request(gpio, "ADC_EN");
>>>> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
>>>> +	gpio_direction_output(gpio, 1);
>>> I'm not sure what this does?
>>>
>> I explained below description about "ADC_EN" :
>> "ADC_EN : This gpio enable the ADC device which is used to detect
>>  the kind of jack. (SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT)
>>  According to the kind of jack, an electric current is changed.
>>  (Only used on Aquila board) "
>>
>> When inserting the jack to Aquila board, I used ADC driver so that, detecting
>> the kind of jack(SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT).
>>
>> I will separately make the another function to initialize ADC driver.
>>
>> Thank you for your comment.
>> Chanwoo Choi
>>
> Hi,
> 
> How was going on?
> 

I did post this patch and reviewed by maintainer.
The request was reflected to following patch.
Also, I did explain the constraints of the regulator of WM8994 codec
and resend following patch. This patch include Goni and Aquila board code
related to audio.
	[PATCH] ARM: S5PV210: Add audio support to Goni and Aquila board

This is my mistake, "RESEND" word is leaved out in the subject.

Thanks,
Chanwoo Choi


> Thanks.
> 
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

RE: [PATCH 1/2] ARM: S5PV210: Add audio support to Aquila

[Kukjin Kim]

Chanwoo Choi wrote:
> Kukjin Kim wrote:
> > Chanwoo Choi wrote:
> >> Mark Brown wrote:
> >>> On Wed, Jul 28, 2010 at 12:04:44PM +0900, Chanwoo Choi wrote:
> >>>
> >>>> +static struct regulator_consumer_supply wm8994_fixed_voltage0_supplies[]
> =
> >> {
> >>>> +	{
> >>>> +		.dev_name	= "5-001a",
> >>>> +		.supply		= "DBVDD",
> >>>> +	}, {
> >>>> +		.dev_name	= "5-001a",
> >>>> +		.supply		= "AVDD2",
> >>>> +	}, {
> >>>> +		.dev_name	= "5-001a",
> >>>> +		.supply		= "CPVDD",
> >>>> +	},
> >>>> +
> >>>> +};
> >>>> +
> >>>> +static struct regulator_consumer_supply wm8994_fixed_voltage1_supplies[]
> =
> >> {
> >>> All these fixed voltage regulators seem a bit suspicous for a mobile
> >>> phone - I'd have expected that the supplies would all be being provided
> >>> by your PMIC except for things taken directly from the battery supply
> >>> (like the speakers tend to be, for example)?  There's no problem with
> >>> the code itself, it just looks a bit odd.
> >>>
> >> All these consumer supply of WM8994 codec connected the below
> >> regulator(VCC_1.8V) on
> >> a circuit diagram. "VCC_1.8V" regualtor is always enabled, because it is used
> to
> >> many devices.
> >> Then I haven't connected all these consumer supply of WM8994 codec to
> >> "VCC_1.8V" regulator.
> >> I will modify that the consumer supply would be provided by PMIC.
> >>
> >> static struct regulator_init_data aquila_buck3_data = {
> >> 	.constraints	= {
> >> 		.name		= "VCC_1.8V",
> >> 		.min_uV		= 1800000,
> >> 		.max_uV		= 1800000,
> >> 		.apply_uV	= 1,
> >> 		.state_mem	= {
> >> 			.enabled = 1,
> >> 		},
> >> 	},
> >> };
> >>
> >>>> +static struct i2c_board_info i2c_gpio5_devs[] __initdata = {
> >>>> +	{
> >>>> +		/* CS/ADDR = low 0x34 (FYI: high = 0x36) */
> >>>> +		I2C_BOARD_INFO("wm8994", 0x34 >> 1),
> >>>> +		.platform_data	= &wm8994_platform_data,
> >>>> +	},
> >>>> +};
> >>> Probably clearer for generic Linux use to specify the address as 0x1a
> >>> directly.
> >> Ok, I will do.
> >>
> >>>> +static void __init aquila_sound_init(void)
> >>>> +{
> >>>> +	unsigned int gpio;
> >>>> +
> >>>> +	/* CODEC_XTAL_EN */
> >>>> +	gpio = S5PV210_GPH3(2);		/* XEINT_26 */
> >>>> +	gpio_request(gpio, "CODEC_XTAL_EN");
> >>>> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
> >>>> +	s3c_gpio_setpull(gpio, S3C_GPIO_PULL_NONE);
> >>>> +	gpio_direction_output(gpio, 1);
> >>> Might be as well to provide some or all this stuff in your audio machine
> >>> driver?
> >> The Aquila board have a oscillator which provide main clock to
> >> WM8994 audio codec. The oscillator provide 24MHz clock to WM8994 audio
> codec
> >> (MCLK1 pin). I set gpio setting of "CODEC_XTAL_EN" to enable a oscillator.
> >>
> >>>> +	/* MICBIAS_EN */
> >>>> +	gpio = S5PV210_GPJ4(2);			/* XMSMRN */
> >>>> +	gpio_request(gpio, "MICBIAS_EN");
> >>>> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
> >>>> +	gpio_direction_output(gpio, 1);
> >>> This in particular would benefit from keeping the request of the GPIO
> >>> joined up with the driver that uses it.
> >> Ok, I will move this code to machine
> driver(sound/soc/s3c24xxx/aquila_wm8994.c).
> >>
> >>>> +	/* ADC_EN */
> >>>> +	gpio = S5PV210_GPJ3(2);
> >>>> +	gpio_request(gpio, "ADC_EN");
> >>>> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
> >>>> +	gpio_direction_output(gpio, 1);
> >>> I'm not sure what this does?
> >>>
> >> I explained below description about "ADC_EN" :
> >> "ADC_EN : This gpio enable the ADC device which is used to detect
> >>  the kind of jack. (SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT)
> >>  According to the kind of jack, an electric current is changed.
> >>  (Only used on Aquila board) "
> >>
> >> When inserting the jack to Aquila board, I used ADC driver so that, detecting
> >> the kind of jack(SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT).
> >>
> >> I will separately make the another function to initialize ADC driver.
> >>
> >> Thank you for your comment.
> >> Chanwoo Choi
> >>
> > Hi,
> >
> > How was going on?
> >
> 
> I did post this patch and reviewed by maintainer.
> The request was reflected to following patch.
> Also, I did explain the constraints of the regulator of WM8994 codec
> and resend following patch. This patch include Goni and Aquila board code
> related to audio.
> 	[PATCH] ARM: S5PV210: Add audio support to Goni and Aquila board
> 
> This is my mistake, "RESEND" word is leaved out in the subject.
> 
Hmm...so may missed. :-(

It would helpful to me if you could add patch version and changelog.

Thanks.

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

Re: [PATCH 1/2] ARM: S5PV210: Add audio support to Aquila

[Chanwoo Choi]

Kukjin Kim wrote:
> Chanwoo Choi wrote:
>> Kukjin Kim wrote:
>>> Chanwoo Choi wrote:
>>>> Mark Brown wrote:
>>>>> On Wed, Jul 28, 2010 at 12:04:44PM +0900, Chanwoo Choi wrote:
>>>>>
>>>>>> +static struct regulator_consumer_supply wm8994_fixed_voltage0_supplies[]
>> =
>>>> {
>>>>>> +	{
>>>>>> +		.dev_name	= "5-001a",
>>>>>> +		.supply		= "DBVDD",
>>>>>> +	}, {
>>>>>> +		.dev_name	= "5-001a",
>>>>>> +		.supply		= "AVDD2",
>>>>>> +	}, {
>>>>>> +		.dev_name	= "5-001a",
>>>>>> +		.supply		= "CPVDD",
>>>>>> +	},
>>>>>> +
>>>>>> +};
>>>>>> +
>>>>>> +static struct regulator_consumer_supply wm8994_fixed_voltage1_supplies[]
>> =
>>>> {
>>>>> All these fixed voltage regulators seem a bit suspicous for a mobile
>>>>> phone - I'd have expected that the supplies would all be being provided
>>>>> by your PMIC except for things taken directly from the battery supply
>>>>> (like the speakers tend to be, for example)?  There's no problem with
>>>>> the code itself, it just looks a bit odd.
>>>>>
>>>> All these consumer supply of WM8994 codec connected the below
>>>> regulator(VCC_1.8V) on
>>>> a circuit diagram. "VCC_1.8V" regualtor is always enabled, because it is used
>> to
>>>> many devices.
>>>> Then I haven't connected all these consumer supply of WM8994 codec to
>>>> "VCC_1.8V" regulator.
>>>> I will modify that the consumer supply would be provided by PMIC.
>>>>
>>>> static struct regulator_init_data aquila_buck3_data = {
>>>> 	.constraints	= {
>>>> 		.name		= "VCC_1.8V",
>>>> 		.min_uV		= 1800000,
>>>> 		.max_uV		= 1800000,
>>>> 		.apply_uV	= 1,
>>>> 		.state_mem	= {
>>>> 			.enabled = 1,
>>>> 		},
>>>> 	},
>>>> };
>>>>
>>>>>> +static struct i2c_board_info i2c_gpio5_devs[] __initdata = {
>>>>>> +	{
>>>>>> +		/* CS/ADDR = low 0x34 (FYI: high = 0x36) */
>>>>>> +		I2C_BOARD_INFO("wm8994", 0x34 >> 1),
>>>>>> +		.platform_data	= &wm8994_platform_data,
>>>>>> +	},
>>>>>> +};
>>>>> Probably clearer for generic Linux use to specify the address as 0x1a
>>>>> directly.
>>>> Ok, I will do.
>>>>
>>>>>> +static void __init aquila_sound_init(void)
>>>>>> +{
>>>>>> +	unsigned int gpio;
>>>>>> +
>>>>>> +	/* CODEC_XTAL_EN */
>>>>>> +	gpio = S5PV210_GPH3(2);		/* XEINT_26 */
>>>>>> +	gpio_request(gpio, "CODEC_XTAL_EN");
>>>>>> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
>>>>>> +	s3c_gpio_setpull(gpio, S3C_GPIO_PULL_NONE);
>>>>>> +	gpio_direction_output(gpio, 1);
>>>>> Might be as well to provide some or all this stuff in your audio machine
>>>>> driver?
>>>> The Aquila board have a oscillator which provide main clock to
>>>> WM8994 audio codec. The oscillator provide 24MHz clock to WM8994 audio
>> codec
>>>> (MCLK1 pin). I set gpio setting of "CODEC_XTAL_EN" to enable a oscillator.
>>>>
>>>>>> +	/* MICBIAS_EN */
>>>>>> +	gpio = S5PV210_GPJ4(2);			/* XMSMRN */
>>>>>> +	gpio_request(gpio, "MICBIAS_EN");
>>>>>> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
>>>>>> +	gpio_direction_output(gpio, 1);
>>>>> This in particular would benefit from keeping the request of the GPIO
>>>>> joined up with the driver that uses it.
>>>> Ok, I will move this code to machine
>> driver(sound/soc/s3c24xxx/aquila_wm8994.c).
>>>>>> +	/* ADC_EN */
>>>>>> +	gpio = S5PV210_GPJ3(2);
>>>>>> +	gpio_request(gpio, "ADC_EN");
>>>>>> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
>>>>>> +	gpio_direction_output(gpio, 1);
>>>>> I'm not sure what this does?
>>>>>
>>>> I explained below description about "ADC_EN" :
>>>> "ADC_EN : This gpio enable the ADC device which is used to detect
>>>>  the kind of jack. (SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT)
>>>>  According to the kind of jack, an electric current is changed.
>>>>  (Only used on Aquila board) "
>>>>
>>>> When inserting the jack to Aquila board, I used ADC driver so that, detecting
>>>> the kind of jack(SND_JACK_HEADPHONE/HEADSET/MECHANICAL/AVOUT).
>>>>
>>>> I will separately make the another function to initialize ADC driver.
>>>>
>>>> Thank you for your comment.
>>>> Chanwoo Choi
>>>>
>>> Hi,
>>>
>>> How was going on?
>>>
>> I did post this patch and reviewed by maintainer.
>> The request was reflected to following patch.
>> Also, I did explain the constraints of the regulator of WM8994 codec
>> and resend following patch. This patch include Goni and Aquila board code
>> related to audio.
>> 	[PATCH] ARM: S5PV210: Add audio support to Goni and Aquila board
>>
>> This is my mistake, "RESEND" word is leaved out in the subject.
>>
> Hmm...so may missed. :-(
> 
> It would helpful to me if you could add patch version and changelog.
> 

You refer to following url :
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-September/025677.html

Do you need additional info(changelog, patch version)?

Thanks

Best regards,
Chanwoo Choi.

Re: [PATCH 1/2] ARM: S5PV210: Add audio support to Aquila

[Ben Dooks]

On 28/07/10 04:04, Chanwoo Choi wrote:

[snip]


> +/* GPIO I2C AP 1.8V */
> +#define AP_I2C_GPIO_BUS_5	5
> +static struct i2c_gpio_platform_data i2c_gpio5_data = {
> +	.sda_pin	= S5PV210_MP05(3),	/* XM0ADDR_11 */
> +	.scl_pin	= S5PV210_MP05(2),	/* XM0ADDR_10 */
> +};
> +
> +static struct platform_device i2c_gpio5 = {
> +	.name		= "i2c-gpio",
> +	.id		= AP_I2C_GPIO_BUS_5,
> +	.dev		= {
> +		.platform_data	= &i2c_gpio5_data,
> +	},
> +};
> +
> +static struct i2c_board_info i2c_gpio5_devs[] __initdata = {
> +	{
> +		/* CS/ADDR = low 0x34 (FYI: high = 0x36) */
> +		I2C_BOARD_INFO("wm8994", 0x34 >> 1),
> +		.platform_data	= &wm8994_platform_data,
> +	},
> +};
> +
> +static void __init aquila_sound_init(void)
> +{
> +	unsigned int gpio;
> +
> +	/* CODEC_XTAL_EN */
> +	gpio = S5PV210_GPH3(2);		/* XEINT_26 */
> +	gpio_request(gpio, "CODEC_XTAL_EN");
> +	s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
> +	s3c_gpio_setpull(gpio, S3C_GPIO_PULL_NONE);
> +	gpio_direction_output(gpio, 1);

gpio_direction_output() should have done the cfgpin()
call.

> +	/* CLKOUT[9:8] set to 0x3(XUSBXTI) of 0xE010E000(OTHERS)
> +	 * for 24MHZ
> +	 */
> +	writel(readl(S5P_OTHERS) | (0x3 << 8), S5P_OTHERS);

Not sure if this should be being done directly? will it affect
the current clock tree?

Re: [PATCH 1/2] ARM: S5PV210: Add audio support to Aquila

[Kyungmin Park]

On Fri, Jul 30, 2010 at 7:48 AM, Ben Dooks <ben@simtec.co.uk> wrote:
> On 28/07/10 04:04, Chanwoo Choi wrote:
>
> [snip]
>
>
>> +/* GPIO I2C AP 1.8V */
>> +#define AP_I2C_GPIO_BUS_5    5
>> +static struct i2c_gpio_platform_data i2c_gpio5_data = {
>> +     .sda_pin        = S5PV210_MP05(3),      /* XM0ADDR_11 */
>> +     .scl_pin        = S5PV210_MP05(2),      /* XM0ADDR_10 */
>> +};
>> +
>> +static struct platform_device i2c_gpio5 = {
>> +     .name           = "i2c-gpio",
>> +     .id             = AP_I2C_GPIO_BUS_5,
>> +     .dev            = {
>> +             .platform_data  = &i2c_gpio5_data,
>> +     },
>> +};
>> +
>> +static struct i2c_board_info i2c_gpio5_devs[] __initdata = {
>> +     {
>> +             /* CS/ADDR = low 0x34 (FYI: high = 0x36) */
>> +             I2C_BOARD_INFO("wm8994", 0x34 >> 1),
>> +             .platform_data  = &wm8994_platform_data,
>> +     },
>> +};
>> +
>> +static void __init aquila_sound_init(void)
>> +{
>> +     unsigned int gpio;
>> +
>> +     /* CODEC_XTAL_EN */
>> +     gpio = S5PV210_GPH3(2);         /* XEINT_26 */
>> +     gpio_request(gpio, "CODEC_XTAL_EN");
>> +     s3c_gpio_cfgpin(gpio, S3C_GPIO_OUTPUT);
>> +     s3c_gpio_setpull(gpio, S3C_GPIO_PULL_NONE);
>> +     gpio_direction_output(gpio, 1);
>
> gpio_direction_output() should have done the cfgpin()
> call.

Good catch. Thanks.

I think we required that extend the current gpiolib to support the
setpull like functions. also sleep gpio pin configuration.
Marek or shim? do you have any ideas?

>
>> +     /* CLKOUT[9:8] set to 0x3(XUSBXTI) of 0xE010E000(OTHERS)
>> +      * for 24MHZ
>> +      */
>> +     writel(readl(S5P_OTHERS) | (0x3 << 8), S5P_OTHERS);
>
> Not sure if this should be being done directly? will it affect
> the current clock tree?

It's almost done by bootloader. but we make sure it's really routed
the CLKOUT to XUSBXTI.

Thank you,
Kyungmin Park

>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>