ARM: S5PV210: Add audio support to Aquila
Subsystems (3)
PATCH 1/2 ARM: S5PV210: Add audio support to Aquila
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.
--- arch/arm/mach-s5pv210/mach-aquila.c | 177 +++++++++++++++++++++++++++++++++++ 1 files changed, 177 insertions(+), 0 deletions(-)
arch/arm/mach-s5pv210/mach-aquila.c +177 -0
@@ -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 4static 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);
Signature · 1 line
1.6.3.3
Quoted 5 lines · Chanwoo Choi
arch/arm/mach-s5pv210/mach-aquila.c +5
-0+static struct regulator_consumer_supply wm8994_fixed_voltage0_supplies[] = { + { + .dev_name ="5-001a" , + .supply ="DBVDD" , + }, {
arch/arm/mach-s5pv210/mach-aquila.c static struct max8998_platform_data aquila_max8998_pdata = { · line 416
Quoted 10 lines
arch/arm/mach-s5pv210/mach-aquila.c +10
-0+ .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.
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 562
Quoted 7 lines
arch/arm/mach-s5pv210/mach-aquila.c +7
-0+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.
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 573
Quoted 10 lines
arch/arm/mach-s5pv210/mach-aquila.c +10
-0+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?
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 584
Quoted 5 lines
arch/arm/mach-s5pv210/mach-aquila.c +5
-0+/* 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.
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 590
Quoted 5 lines
arch/arm/mach-s5pv210/mach-aquila.c +5
-0+/* 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?
Quoted 15 lines · Mark Brown
Quoted 13 lines · Chanwoo Choi
arch/arm/mach-s5pv210/mach-aquila.c +13
-0+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" , + }, + +};
Quoted 9 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ +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,
},
},
};Quoted 10 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 562
Quoted 7 lines
arch/arm/mach-s5pv210/mach-aquila.c +7
-0+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
0x1adirectly.
Ok, I will do.
Quoted 14 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 573
Quoted 11 lines
arch/arm/mach-s5pv210/mach-aquila.c +10
-0+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.
Quoted 9 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 584
Quoted 6 lines
arch/arm/mach-s5pv210/mach-aquila.c +5
-0+/* 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).
Quoted 8 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 590
Quoted 5 lines
arch/arm/mach-s5pv210/mach-aquila.c +5
-0+/* 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
Quoted 31 lines · Chanwoo Choi
Quoted 15 lines · Mark Brown
Quoted 13 lines · Chanwoo Choi
arch/arm/mach-s5pv210/mach-aquila.c +13
-0+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", + }, + +};Quoted 9 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ +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.
Quoted 11 lines
static struct
regulator_init_dataaquila_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.
Quoted 77 lines
Quoted 11 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 562
Quoted 8 lines
arch/arm/mach-s5pv210/mach-aquila.c +7
-0+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
0x1adirectly.Ok, I will do.
Quoted 14 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 573
Quoted 11 lines
arch/arm/mach-s5pv210/mach-aquila.c +10
-0+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.Quoted 9 lines
Quoted 6 lines
+ /*
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).Quoted 8 lines
Quoted 5 lines
+ /*
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
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 5 lines
MyungJoo Ham (함명주), Ph.D. Mobile Software Platform Lab, Digital Media and Communications (DMC) Business Samsung Electronics cell: 82-10-6714-2858
Quoted 10 lines · Chanwoo Choi
Quoted 5 lines
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.
Quoted 10 lines
Quoted 6 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 570
Quoted 3 lines
arch/arm/mach-s5pv210/mach-aquila.c +3
-0+/* 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).
Quoted 14 lines
Quoted 7 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 590
Quoted 5 lines
arch/arm/mach-s5pv210/mach-aquila.c +5
-0+/* 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) "
Quoted 4 lines
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.
Quoted 8 lines · Mark Brown
Quoted 5 lines · Chanwoo Choi
Quoted 5 lines
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.
Quoted 7 lines
Quoted 4 lines
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.
Quoted 6 lines · Chanwoo Choi
Quoted 2 lines · Mark Brown
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.
Quoted 11 lines
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.
Quoted 2 lines
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.
Quoted 108 lines · Chanwoo Choi
Quoted 3 lines · Mark Brown
Quoted 1 line · Chanwoo Choi
+static struct
regulator_consumer_supplywm8994_fixed_voltage0_supplies[] ={
Quoted 6 lines
Quoted 6 lines
arch/arm/mach-s5pv210/mach-aquila.c +6
-0+ { + .dev_name = "5-001a", + .supply = "DBVDD", + }, { + .dev_name = "5-001a", + .supply = "AVDD2",Quoted 8 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct max8998_platform_data aquila_max8998_pdata = {
Quoted 8 lines
arch/arm/mach-s5pv210/mach-aquila.c +8
-0+ }, { + .dev_name = "5-001a", + .supply = "CPVDD", + }, + +}; + +static struct regulator_consumer_supply wm8994_fixed_voltage1_supplies[] ={
Quoted 7 lines
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, }, }, };Quoted 10 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 562
Quoted 7 lines
arch/arm/mach-s5pv210/mach-aquila.c +7
-0+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
0x1adirectly.Ok, I will do.
Quoted 14 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 573
Quoted 11 lines
arch/arm/mach-s5pv210/mach-aquila.c +10
-0+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.Quoted 9 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 584
Quoted 6 lines
arch/arm/mach-s5pv210/mach-aquila.c +5
-0+/* 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).Quoted 8 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 590
Quoted 5 lines
arch/arm/mach-s5pv210/mach-aquila.c +5
-0+/* 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) "
Quoted 8 lines
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 <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.
Quoted 99 lines · Kukjin Kim
Quoted 98 lines · Chanwoo Choi
Quoted 3 lines · Mark Brown
Quoted 1 line · Chanwoo Choi
+static struct
regulator_consumer_supplywm8994_fixed_voltage0_supplies[] ={
Quoted 6 lines
Quoted 6 lines
arch/arm/mach-s5pv210/mach-aquila.c +6
-0+ { + .dev_name = "5-001a", + .supply = "DBVDD", + }, { + .dev_name = "5-001a", + .supply = "AVDD2",Quoted 8 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct max8998_platform_data aquila_max8998_pdata = {
Quoted 8 lines
arch/arm/mach-s5pv210/mach-aquila.c +8
-0+ }, { + .dev_name = "5-001a", + .supply = "CPVDD", + }, + +}; + +static struct regulator_consumer_supply wm8994_fixed_voltage1_supplies[] ={
Quoted 6 lines
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, }, }, };Quoted 9 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 562
Quoted 7 lines
arch/arm/mach-s5pv210/mach-aquila.c +7
-0+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
0x1adirectly.Ok, I will do.
Quoted 4 lines
Quoted 4 lines
arch/arm/mach-s5pv210/mach-aquila.c +4
-0+static void __init aquila_sound_init (void ) +{ +unsigned int gpio; +Quoted 8 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 573
Quoted 6 lines
arch/arm/mach-s5pv210/mach-aquila.c +6
-0+/* 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.Quoted 7 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 584
Quoted 5 lines
arch/arm/mach-s5pv210/mach-aquila.c +5
-0+/* 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).Quoted 7 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 590
Quoted 5 lines
arch/arm/mach-s5pv210/mach-aquila.c +5
-0+/* 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).
Quoted 9 lines
Quoted 5 lines
I will separately make the another function to initialize ADC driver.
Thank you for your comment.
Chanwoo ChoiHi,
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
Quoted 14 lines
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd._______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 115 lines · Chanwoo Choi
Quoted 5 lines · Kukjin Kim
Quoted 4 lines · Chanwoo Choi
Quoted 3 lines · Mark Brown
Quoted 1 line · Chanwoo Choi
+static struct
regulator_consumer_supplywm8994_fixed_voltage0_supplies[]=
Quoted 15 lines
Quoted 15 lines
{
Quoted 6 lines
Quoted 6 lines
arch/arm/mach-s5pv210/mach-aquila.c +6
-0+ { + .dev_name = "5-001a", + .supply = "DBVDD", + }, { + .dev_name = "5-001a", + .supply = "AVDD2",Quoted 8 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct max8998_platform_data aquila_max8998_pdata = {
Quoted 8 lines
arch/arm/mach-s5pv210/mach-aquila.c +8
-0+ }, { + .dev_name = "5-001a", + .supply = "CPVDD", + }, + +}; + +static struct regulator_consumer_supply wm8994_fixed_voltage1_supplies[]=
Quoted 10 lines
Quoted 10 lines
{
Quoted 6 lines
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 usedto
Quoted 40 lines
Quoted 40 lines
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, }, }, };Quoted 9 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 562
Quoted 7 lines
arch/arm/mach-s5pv210/mach-aquila.c +7
-0+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
0x1adirectly.Ok, I will do.
Quoted 12 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 573
Quoted 10 lines
arch/arm/mach-s5pv210/mach-aquila.c +10
-0+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?
Quoted 2 lines
Quoted 2 lines
The Aquila board have a oscillator which provide main clock to WM8994 audio codec. The oscillator provide 24MHz clock to WM8994 audio
codec
Quoted 10 lines
Quoted 10 lines
(MCLK1 pin). I set gpio setting of "
CODEC_XTAL_EN" to enable a oscillator.Quoted 7 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 584
Quoted 5 lines
arch/arm/mach-s5pv210/mach-aquila.c +5
-0+/* 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).Quoted 17 lines
Quoted 17 lines
Quoted 8 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 590
Quoted 6 lines
arch/arm/mach-s5pv210/mach-aquila.c +5
-0+/* 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).Quoted 9 lines
Quoted 5 lines
I will separately make the another function to initialize ADC driver.
Thank you for your comment.
Chanwoo ChoiHi,
How was going on?
Quoted 9 lines
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 boardThis 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 <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.
Quoted 121 lines · Kukjin Kim
Quoted 120 lines · Chanwoo Choi
Quoted 5 lines · Kukjin Kim
Quoted 4 lines · Chanwoo Choi
Quoted 3 lines · Mark Brown
Quoted 1 line · Chanwoo Choi
+static struct
regulator_consumer_supplywm8994_fixed_voltage0_supplies[]=
Quoted 15 lines
Quoted 15 lines
{
Quoted 6 lines
Quoted 6 lines
arch/arm/mach-s5pv210/mach-aquila.c +6
-0+ { + .dev_name = "5-001a", + .supply = "DBVDD", + }, { + .dev_name = "5-001a", + .supply = "AVDD2",Quoted 8 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct max8998_platform_data aquila_max8998_pdata = {
Quoted 8 lines
arch/arm/mach-s5pv210/mach-aquila.c +8
-0+ }, { + .dev_name = "5-001a", + .supply = "CPVDD", + }, + +}; + +static struct regulator_consumer_supply wm8994_fixed_voltage1_supplies[]=
Quoted 10 lines
Quoted 10 lines
{
Quoted 6 lines
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 usedto
Quoted 40 lines
Quoted 40 lines
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, }, }, };Quoted 9 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 562
Quoted 7 lines
arch/arm/mach-s5pv210/mach-aquila.c +7
-0+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
0x1adirectly.Ok, I will do.
Quoted 12 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 573
Quoted 10 lines
arch/arm/mach-s5pv210/mach-aquila.c +10
-0+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?
Quoted 2 lines
Quoted 2 lines
The Aquila board have a oscillator which provide main clock to WM8994 audio codec. The oscillator provide 24MHz clock to WM8994 audio
codec
Quoted 10 lines
Quoted 10 lines
(MCLK1 pin). I set gpio setting of "
CODEC_XTAL_EN" to enable a oscillator.Quoted 7 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 584
Quoted 5 lines
arch/arm/mach-s5pv210/mach-aquila.c +5
-0+/* 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).Quoted 16 lines
Quoted 16 lines
Quoted 7 lines
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 590
Quoted 5 lines
arch/arm/mach-s5pv210/mach-aquila.c +5
-0+/* 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).Quoted 9 lines
Quoted 5 lines
I will separately make the another function to initialize ADC driver.
Thank you for your comment.
Chanwoo ChoiHi,
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
Quoted 6 lines
Quoted 2 lines
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.
On 28/07/10 04:04, Chanwoo Choi wrote:
[snip]
Quoted 23 lines
arch/arm/mach-s5pv210/mach-aquila.c +23
-0+/* 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, + }, +}; +
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 573
Quoted 10 lines
arch/arm/mach-s5pv210/mach-aquila.c +10
-0+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.
arch/arm/mach-s5pv210/mach-aquila.c static struct i2c_board_info i2c_gpio_pmic_devs[] __initdata = { · line 578
Quoted 4 lines
arch/arm/mach-s5pv210/mach-aquila.c +4
-0+/* 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?
Quoted 33 lines · Ben Dooks
On 28/07/10 04:04, Chanwoo Choi wrote:
[snip]
Quoted 28 lines
arch/arm/mach-s5pv210/mach-aquila.c +28
-0+/* 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 */
Quoted 8 lines
Quoted 5 lines
+ 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 thecfgpin()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?
Quoted 8 lines
Quoted 5 lines
+ /* 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
Quoted 6 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html