December 2012 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] [PATCH v2 0/2] ASoC: palm27x: machine driver fixes and updates
by Mike Dunn 29 Dec '12

29 Dec '12

Hi, A fix and an update to the palm27x machine driver. Change log: v2: Patch #2: in palm27x_asoc_remove(), the call to platform_device_unregister() is replaced with snd_soc_unregister_card(). This version v2 is unsolicited; I noticed the omission today. Patch preparation error; tripping over my own patches. No new ones for a while! Thanks for your patience. Mike Dunn (2): ASoC: palm27x: fix widgets and routes in dai_link init ASoC: palm27x: register card in platform_driver probe sound/soc/pxa/palm27x.c | 38 ++++++++++---------------------------- 1 files changed, 10 insertions(+), 28 deletions(-) -- 1.7.8.6

1 2

[alsa-devel] [PATCH] ALSA: usb-audio: Add quirk for Focusrite Scarlett 18i6
by Martin Schwenke 29 Dec '12

29 Dec '12

Probing this device currently fails in snd_usb_audio_probe() because the call to snd_usb_create_mixer() fails. This is due to unknown or non-standard interface descriptor subtypes in parse_audio_unit(): usbaudio: unit 51: unexpected type 0x09 snd-usb-audio: probe of 1-8:1.0 failed with error -5 Some people are working around this by recompiling usb-audio with the call to snd_usb_create_mixer() commented out. It would be nice to avoid that. While the best idea would be to look into the mixer creation failure, a reasonable short-term solution is to use quirks to only probe the trouble-free interfaces. This allows audio and MIDI interfaces to be used without any obvious issues. Interface 0 is the main one to ignore. It contains lots of control-fu, including the unexpected interface descriptor subtypes. Interface 5 is for firmware updates and I'm not sure how to get support for this. Interface 3 is some sort of control interface that I don't understand: Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 3 bAlternateSetting 0 bNumEndpoints 0 bInterfaceClass 1 Audio bInterfaceSubClass 1 Control Device bInterfaceProtocol 0 iInterface 0 AudioControl Interface Descriptor: bLength 9 bDescriptorType 36 bDescriptorSubtype 1 (HEADER) bcdADC 1.00 wTotalLength 9 bInCollection 1 baInterfaceNr( 0) 1 Signed-off-by: Martin Schwenke <martin(a)meltin.net> --- sound/usb/quirks-table.h | 52 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h index 88d8ceb..c9dde5f 100644 --- a/sound/usb/quirks-table.h +++ b/sound/usb/quirks-table.h @@ -2882,6 +2882,58 @@ YAMAHA_DEVICE(0x7010, "UB99"), { /* + * Focusrite Scarlett 18i6 + * + * Avoid mixer creation, which otherwise fails because some of + * the interface descriptor subtypes for interface 0 are + * unknown. That should be fixed or worked-around but this at + * least allows the device to be used successfully with a DAW + * and an external mixer. See comments below about other + * ignored interfaces. + */ + USB_DEVICE(0x1235, 0x8004), + .driver_info = (unsigned long) & (const struct snd_usb_audio_quirk) { + .vendor_name = "Focusrite", + .product_name = "Scarlett 18i6", + .ifnum = QUIRK_ANY_INTERFACE, + .type = QUIRK_COMPOSITE, + .data = & (const struct snd_usb_audio_quirk[]) { + { + /* InterfaceSubClass 1 (Control Device) */ + .ifnum = 0, + .type = QUIRK_IGNORE_INTERFACE + }, + { + .ifnum = 1, + .type = QUIRK_AUDIO_STANDARD_INTERFACE + }, + { + .ifnum = 2, + .type = QUIRK_AUDIO_STANDARD_INTERFACE + }, + { + /* InterfaceSubClass 1 (Control Device) */ + .ifnum = 3, + .type = QUIRK_IGNORE_INTERFACE + }, + { + .ifnum = 4, + .type = QUIRK_MIDI_STANDARD_INTERFACE + }, + { + /* InterfaceSubClass 1 (Device Firmware Update) */ + .ifnum = 5, + .type = QUIRK_IGNORE_INTERFACE + }, + { + .ifnum = -1 + } + } + } +}, + +{ + /* * Some USB MIDI devices don't have an audio control interface, * so we have to grab MIDI streaming interfaces here. */ -- 1.7.10.4

4 5

[alsa-devel] EMU-1212m "no Audiodock - no firmware" patch
by oeai 29 Dec '12

29 Dec '12

Aloha! i've been posting about my problem earlier, but i decided to DIY the problem was that when going up from suspend the firmware for additional card (that i don't have) was trying to load, -but there's no card - sorry, i'm gonna check few more thousand times that caused my cpu to load insanely so in the patch i've changed few more strings for debugging but actually the one (but not the only) bug is here *** 728,739 **** ! if (reg & EMU_HANA_OPTION_DOCK_OFFLINE) { --- 729,740 ---- ! if (reg & !EMU_HANA_OPTION_DOCK_OFFLINE) { the thing is in the "!"EMU_HANA_OPTION_DOCK_OFFLINE -> then the firmware should be loaded. and to debug some errors if something i've put the one more else state so if no audiodock - it could continue to load config *** 777,784 **** /* Unmute all. Default is muted after a firmware load */ snd_emu1010_fpga_write(emu, EMU_HANA_UNMUTE, EMU_UNMUTE); } } ! snd_printk(KERN_INFO "emu1010: firmware thread stopping\n"); return 0; } --- 778,790 ---- /* Unmute all. Default is muted after a firmware load */ snd_emu1010_fpga_write(emu, EMU_HANA_UNMUTE, EMU_UNMUTE); } + else if ((reg & 0x3f) == 0x15) + { + snd_printk(KERN_INFO "emu1212m: trying to load config emu_hana_fpga_config"); + snd_emu1010_fpga_write(emu, EMU_HANA_FPGA_CONFIG,0x02); + } } ! snd_printk(KERN_INFO "emu1010: emu dock firmware thread stopping\n"); return 0; } *************** then i've commented the power_down for audiodock - it should be in other place, but as i'm 1212m user - it works for me - you know. *** 841,847 **** outl(0x0005a000, emu->port + HCFG); /* Disable 48Volt power to Audio Dock */ ! snd_emu1010_fpga_write(emu, EMU_HANA_DOCK_PWR, 0); /* ID, should read & 0x7f = 0x55. (Bit 7 is the IRQ bit) */ snd_emu1010_fpga_read(emu, EMU_HANA_ID, &reg); --- 847,853 ---- outl(0x0005a000, emu->port + HCFG); /* Disable 48Volt power to Audio Dock */ ! /*snd_emu1010_fpga_write(emu, EMU_HANA_DOCK_PWR, 0);*/ /* ID, should read & 0x7f = 0x55. (Bit 7 is the IRQ bit) */ snd_emu1010_fpga_read(emu, EMU_HANA_ID, &reg); *************** and here i've changed that the emu_model was firstly enabled and then ecard - since i don't know what's the ecard i thought that it is a second module of my emu card. *** 2079,2088 **** { if (emu->card_capabilities->ca_cardbus_chip) snd_emu10k1_cardbus_init(emu); ! if (emu->card_capabilities->ecard) ! snd_emu10k1_ecard_init(emu); ! else if (emu->card_capabilities->emu_model) snd_emu10k1_emu1010_init(emu); else snd_emu10k1_ptr_write(emu, AC97SLOT, 0, AC97SLOT_CNTR|AC97SLOT_LFE); snd_emu10k1_init(emu, emu->enable_ir, 1); --- 2098,2111 ---- { if (emu->card_capabilities->ca_cardbus_chip) snd_emu10k1_cardbus_init(emu); ! ! if (emu->card_capabilities->emu_model) ! { snd_emu10k1_emu1010_init(emu); + + } + else if (emu->card_capabilities->ecard) + snd_emu10k1_ecard_init(emu); else snd_emu10k1_ptr_write(emu, AC97SLOT, 0, AC97SLOT_CNTR|AC97SLOT_LFE); snd_emu10k1_init(emu, emu->enable_ir, 1); Now i still have the problem of waking up i cannot request_firmware() properly on resume and it is trying to load nothing and other changes in patch - that was trying to debug the situation so i can at least alsa-reload at anytime. also i'd like to ask if i can use the ctxfi modules - CA0110-IBG i got xfi card (it is second of 5) that actually uses as i believe the same logic - maybe a lil modified, but not so much imho. -- RA OEAI

3 8

[alsa-devel] Fwd: Request for HW auto-recognition: HP Pavilion dv7 with STAC92xx
by Nicolas Maître 27 Dec '12

27 Dec '12

Hi, At Wed, 12 Dec 2012 18:08:25 +0100, Tomas Pospisek wrote: > > Hello, > > I have a HP Pavilion dv7 laptop (LS066EA#UUZ), with: > > # cat /proc/asound/card0/codec* | grep Codec > Codec: IDT 92HD81B1X5 > Codec: Intel CougarPoint HDMI > > when play sound without any tweaks, I get a very weak sound output. If I > want to have "beats audio", that is subwoofer and all speakers, I need to > set: > > # cat /etc/modprobe.d/alsa-base.conf > options snd-hda-intel model=hp-dv7-4000 > > Could the snd_hda_intel alsa driver be updated to recognize and setup the > driver accordingly? > > alsa-info.sh output here: > > > http://www.alsa-project.org/db/?f=54ea874f43f12c1572aff3ee3113dc4b0ad0b398 I'm not quite sure that I have the exact same hardware, but I think so. I have 5 speakers (rear left and right, front left and right + subwoofer) Here is my alsa-info: https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/1056669/+attachm… In my case, model=hp-dv7-4000 doesn't fully solve the problem. The subwoofer works but it is not mapped as such. After some tests, it seems that the alsa's speakers rear left and rear right are mapped to the subwoofer. The front ones are alright. With the model=hp-dv5 that I'm using, the subwoofer isnt recognized at all, but the rear speakers are correctly mapped. Also, with hp-dv7-4000, the internal microphone is still used when a micro jack is plugged in. That's not the case with hp-dv5. If you think there's a configuration issue on my side, please let me know, but I think hp-dv5 is actually a better default choice if I have to choose between both. Kind regards, -- nimai

4 7

Re: [alsa-devel] [PATCH 001/001] ASoC: Replace max98090 Device Driver
by Mark Brown 27 Dec '12

27 Dec '12

On Thu, Dec 13, 2012 at 11:19:29AM -0800, Jerry Wong wrote: For legibility please submit future revisions as a two stage patch series, one removing the old driver and another adding the new one. > +/* codec platform data */ > +struct max98090_pdata { > + > + int irq; This should be passed in using the normal mechanism provided by the bus, not custom implemented. > + int power_over_performance; This doesn't look like platform data to me, it looks like something that should be configured dynamically at runtime. > + /* Equalizers for DAC */ > + struct max98090_eq_cfg *eq_cfg; > + unsigned int eq_cfgcnt; Coefficients should be configured using SND_SOC_BYTES(). Some older drivers use this method but it's deprecated. > + { 0xE0, 0x00 }, /* E0 */ > + { 0xE1, 0x00 }, /* E1 */ > + { 0xE2, 0x00 }, /* E2 */ Do these number-only registers actually exist in the published register map or are they undefined? If they're undefined and not used by software there's no need to include them here. > + switch (reg) { > + case M98090_REG_01_IRQ_STATUS: > + case M98090_REG_02_JACK_STATUS: > + case M98090_REG_FF_REV_ID: Including the register number in the constants seems to rather defeat the point of having symbolic names for the registers; it means people still have to remember the register numbers. > +static int max98090_get_enab_tlv(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) What's the relevance of _tlv here? It looks like these are normal get and sets. > +static const char * max98090_dsts_text[] = > + { "None", "Left", "Right", "Left and Right" }; > + > +static const struct soc_enum max98090_dsts_enum = > + SOC_ENUM_SINGLE(M98090_REG_1A_LVL_SIDETONE, M98090_DSTS_SHIFT, > + ARRAY_SIZE(max98090_dsts_text), max98090_dsts_text); > +static const struct snd_kcontrol_new max98090_snd_controls[] = { > + SOC_SINGLE("DMIC MIC Left Enable", M98090_REG_13_MIC_CFG1, > + M98090_DIGMICL_SHIFT, M98090_DIGMICL_NUM - 1, 0), > + SOC_SINGLE("DMIC MIC Right Enable", M98090_REG_13_MIC_CFG1, > + M98090_DIGMICR_SHIFT, M98090_DIGMICR_NUM - 1, 0), This looks like it should be DAPM. > + SOC_ENUM_EXT("External MIC Mux", max98090_extmic_mux_enum, > + max98090_extmic_mux_get, max98090_extmic_mux_set), Likewise. > + SOC_ENUM("Digital Sidetone Source", max98090_dsts_enum), DAPM. > + SOC_SINGLE_EXT_TLV("Digital Sidetone Level Volume", > + M98090_REG_1A_LVL_SIDETONE, M98090_DVST_SHIFT, > + M98090_DVST_NUM - 1, 1, max98090_get_enab_tlv, > + max98090_put_enab_tlv, max98090_micboost_tlv), Level Volume? > + SOC_SINGLE_TLV("Digital Gain Volume", M98090_REG_27_DAI_LVL, > + M98090_DAI_LVL_DVG_SHIFT, M98090_DAI_LVL_DVG_NUM - 1, 0, > + max98090_dvg_tlv), Gain Volume? > + SOC_SINGLE("Digital EQ 3 Band Enable", M98090_REG_41_DSP_EQ_EN, > + M98090_EQ3BANDEN_SHIFT, M98090_EQ3BANDEN_NUM - 1, 0), Switch. > + SOC_SINGLE("DAC HP Playback Performance Mode", M98090_REG_43_DAC_CFG, > + M98090_DAC_PERFMODE_SHIFT, M98090_DAC_PERFMODE_NUM - 1, 0), > + SOC_SINGLE("DAC High Performance Mode", M98090_REG_43_DAC_CFG, > + M98090_DACHP_SHIFT, M98090_DACHP_NUM - 1, 0), These should either be enumerations with real names of Switches if it's just an on/off (looks like the former). > + SOC_SINGLE_TLV("Headphone Left Volume", M98090_REG_2C_LVL_HP_LEFT, > + M98090_HPVOLL_SHIFT, M98090_HPVOLL_NUM - 1, 0, > + max98090_hp_tlv), > + SOC_SINGLE_TLV("Headphone Right Volume", M98090_REG_2D_LVL_HP_RIGHT, > + M98090_HPVOLR_SHIFT, M98090_HPVOLR_NUM - 1, 0, > + max98090_hp_tlv), Should be a single stereo control (and similarly for the other output controls). > + SOC_SINGLE("Quick Setup System Clock", M98090_REG_04_QCFG_SYS_CLK, > + M98090_CLK_ALL_SHIFT, M98090_CLK_ALL_NUM - 1, 0), What do these controls do? > + if (val >= 1) { > + if (w->reg == M98090_REG_10_LVL_MIC1) > + max98090->mic1pre = val - 1; /* Update for volatile */ > + else > + max98090->mic2pre = val - 1; /* Update for volatile */ > + } Use braces consistently within an if statemeent - if you use them for the outer one use them for the inner one too. > +static const struct snd_soc_dapm_route max98090_dapm_routes[] = { > + > + {"MIC1 Input", NULL, "MIC1"}, > + {"MIC2 Input", NULL, "MIC2"}, > + {"MIC1 Input", NULL, "MICBIAS"}, > + {"MIC2 Input", NULL, "MICBIAS"}, Unless this chip is *very* unusual I expect the bias is something that's connected externally and therefore these connections should be being made by the machine driver. > + > + /* Check for user calculated MI and NI ratios */ > + for (i = 0; i < ARRAY_SIZE(user_pclk_rates); i++) { > + if ((user_pclk_rates[i] == max98090->sysclk) && > + (user_lrclk_rates[i] == max98090->lrclk)) { > + dev_info(codec->dev, > + "Found user supported PCLK to LRCLK rates\n"); > + dev_info(codec->dev, "i %d ni %lld mi %lld\n", > + i, ni_value[i], mi_value[i]); dev_dbg() would be fine but dev_info() is too loud. > +/* > + * This accommodates an inverted logic in the MAX98090 chip > + * for Bit Clock Invert (BCI). The inverted logic is only seen > + * for the case of TDM mode. The remaining cases have normal logic. > + */ > + if (max98090->tdm_slots > 1) { > + regval ^= M98090_DAI_BCI_MASK; > + } Coding style, the indentation is inconsistent here and no braces for single line if () statements. > + if (max98090->jack_state == M98090_JACK_STATE_HEADSET) { > + /* > + * Set to normal bias level. > + */ > + snd_soc_update_bits(codec, M98090_REG_12_MIC_BIAS, > + M98090_MBVSEL_MASK, M98090_MBVSEL_2V4); > + > + snd_soc_update_bits(codec, M98090_REG_3E_PWR_EN_IN, > + M98090_PWR_MBEN_MASK, M98090_PWR_MBEN_MASK); > + } This looks suspicous, why does the headset state have any bearing here? > +static int max98090_dai_digital_mute(struct snd_soc_dai *codec_dai, int mute) > +{ > + struct snd_soc_codec *codec = codec_dai->codec; > + int regval; > + > + dev_info(codec->dev, "max98090_dai_digital_mute\n"); Drop this, it's not conveying any useful information. > +static void max98090_dmic_switch(struct snd_soc_codec *codec, int state) > +{ I have no idea what this function is supposed to do... > +static void max98090_headset_button_event(struct snd_soc_codec *codec) > +{ > + dev_info(codec->dev, "max98090_headset_button_event\n"); > +} Delete this, it doesn't do anything except log. > + switch (reg & (M98090_LSNS_MASK | M98090_JKSNS_MASK)) { > + case M98090_LSNS_MASK | M98090_JKSNS_MASK: > + { > + dev_info(codec->dev, "No Headset Detected\n"); > + > + max98090->jack_state = M98090_JACK_STATE_NO_HEADSET; > + > + max98090_set_bias_level(codec, SND_SOC_BIAS_STANDBY); > + > + snd_soc_dapm_disable_pin(&codec->dapm, "HPL"); > + snd_soc_dapm_disable_pin(&codec->dapm, "HPR"); > + snd_soc_dapm_force_enable_pin(&codec->dapm, "SPKL"); > + snd_soc_dapm_force_enable_pin(&codec->dapm, "SPKR"); > + snd_soc_dapm_disable_pin(&codec->dapm, "MIC1"); > + snd_soc_dapm_disable_pin(&codec->dapm, "MIC2"); > + snd_soc_dapm_force_enable_pin(&codec->dapm, "DMIC1"); > + snd_soc_dapm_force_enable_pin(&codec->dapm, "DMIC2"); > + max98090_dmic_switch(codec, 1); > + > + break; > + } This doesn't look obviously sensible... calling set_bias_level() outside of DAPM is going to get overridden in pretty short order, none of the DAPM pin setting looks like a good idea (especially not forcing on the speakers whenever there's no headset) and there's no DAPM sync to make any of this take effect. I'm a bit confused about the intended effect. I also can't see any interaction with soc-jack... What is this code actually intended to do? > + dev_info(codec->dev, "***** max98090_interrupt *****\n"); > + There's lots of these noisy, uninformative prints at info level throughout the driver. > + /* Send work to be scheduled */ > + if (active & M98090_IRQ_CLD_MASK) { > + dev_info(codec->dev, "M98090_IRQ_CLD_MASK\n"); > + } This doesn't actually schedule anything... > + ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_I2C); > + if (ret != 0) { > + dev_err(codec->dev, "Failed to set cache I/O: %d\n", ret); > + return ret; > + } SND_SOC_REGMAP. > + if ( (request_threaded_irq(pdata->irq, NULL, > + max98090_interrupt, IRQF_TRIGGER_FALLING, > + "max98090_interrupt", codec)) < 0) { > + dev_info(codec->dev, "request_irq failed\n"); > + } > + /* > + * Clear any old interrupts. Coding style and it's not terribly helpful to hide the return code from users. > + /* Turn on VCM bandgap reference */ > + snd_soc_write(codec, M98090_REG_42_BIAS_CNTL, > + M98090_VCM_MODE_MASK); This shouldn't be managed at runtime? > +/* > + * Driver revision > + */ > +#define MAX98090_REVISION "0.01.00" The kernel already has perfectly good versioning.

1 0

[alsa-devel] [PATCH 1/2] ASoC: wm8985: Refactor set_pll code to avoid gcc warnings
by Fabio Estevam 27 Dec '12

27 Dec '12

From: Fabio Estevam <fabio.estevam(a)freescale.com> Refactor set_pll code to avoid the following warnings: sound/soc/codecs/wm8985.c:852:50: warning: 'pll_div.k' may be used uninitialized in this function sound/soc/codecs/wm8985.c:849:9: warning: 'pll_div.n' may be used uninitialized in this function sound/soc/codecs/wm8985.c:848:23: warning: 'pll_div.div2' may be used uninitialized in this function Do the same as in commit 86ce6c9a (ASoC: WM8804: Refactor set_pll code to avoid GCC warnings). Signed-off-by: Fabio Estevam <fabio.estevam(a)freescale.com> --- sound/soc/codecs/wm8985.c | 43 ++++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/sound/soc/codecs/wm8985.c b/sound/soc/codecs/wm8985.c index ab37826..dd6ce3b 100644 --- a/sound/soc/codecs/wm8985.c +++ b/sound/soc/codecs/wm8985.c @@ -830,33 +830,30 @@ static int wm8985_set_pll(struct snd_soc_dai *dai, int pll_id, struct pll_div pll_div; codec = dai->codec; - if (freq_in && freq_out) { + if (!freq_in || !freq_out) { + /* disable the PLL */ + snd_soc_update_bits(codec, WM8985_POWER_MANAGEMENT_1, + WM8985_PLLEN_MASK, 0); + } else { ret = pll_factors(&pll_div, freq_out * 4 * 2, freq_in); if (ret) return ret; - } - /* disable the PLL before reprogramming it */ - snd_soc_update_bits(codec, WM8985_POWER_MANAGEMENT_1, - WM8985_PLLEN_MASK, 0); - - if (!freq_in || !freq_out) - return 0; - - /* set PLLN and PRESCALE */ - snd_soc_write(codec, WM8985_PLL_N, - (pll_div.div2 << WM8985_PLL_PRESCALE_SHIFT) - | pll_div.n); - /* set PLLK */ - snd_soc_write(codec, WM8985_PLL_K_3, pll_div.k & 0x1ff); - snd_soc_write(codec, WM8985_PLL_K_2, (pll_div.k >> 9) & 0x1ff); - snd_soc_write(codec, WM8985_PLL_K_1, (pll_div.k >> 18)); - /* set the source of the clock to be the PLL */ - snd_soc_update_bits(codec, WM8985_CLOCK_GEN_CONTROL, - WM8985_CLKSEL_MASK, WM8985_CLKSEL); - /* enable the PLL */ - snd_soc_update_bits(codec, WM8985_POWER_MANAGEMENT_1, - WM8985_PLLEN_MASK, WM8985_PLLEN); + /* set PLLN and PRESCALE */ + snd_soc_write(codec, WM8985_PLL_N, + (pll_div.div2 << WM8985_PLL_PRESCALE_SHIFT) + | pll_div.n); + /* set PLLK */ + snd_soc_write(codec, WM8985_PLL_K_3, pll_div.k & 0x1ff); + snd_soc_write(codec, WM8985_PLL_K_2, (pll_div.k >> 9) & 0x1ff); + snd_soc_write(codec, WM8985_PLL_K_1, (pll_div.k >> 18)); + /* set the source of the clock to be the PLL */ + snd_soc_update_bits(codec, WM8985_CLOCK_GEN_CONTROL, + WM8985_CLKSEL_MASK, WM8985_CLKSEL); + /* enable the PLL */ + snd_soc_update_bits(codec, WM8985_POWER_MANAGEMENT_1, + WM8985_PLLEN_MASK, WM8985_PLLEN); + } return 0; } -- 1.7.9.5

2 3

Re: [alsa-devel] [PATCH V5 2/2] ASoC: SAMSUNG: Add DT support for i2s
by Padma Venkat 27 Dec '12

27 Dec '12

Hi, On Sat, Dec 22, 2012 at 12:32 AM, Kukjin Kim <kgene.kim(a)samsung.com> wrote: > Padma Venkat wrote: >> >> Hi, >> >> On Wed, Dec 19, 2012 at 10:39 PM, Mark Brown >> <broonie(a)opensource.wolfsonmicro.com> wrote: >> > On Wed, Dec 19, 2012 at 01:24:14PM +0000, Grant Likely wrote: >> >> On Thu, 13 Dec 2012 16:12:53 +0530, Padmavathi Venna >> <padma.v(a)samsung.com> wrote: >> > >> >> > +- compatible : "samsung,samsung-i2s" >> > >> >> Isn't that kind of redundant? :-) >> > >> >> The format of the compatible strings should be "<vendor>,<part- >> number>-i2s". >> >> Please be specific about the part number that you're doing the binding >> >> for. For example; use "samsung,exynos4210-i2s" instead of >> "samsung,exynos-i2s". >> > >> > There are actually versioned IPs here (where the versions are used >> > publically in a few places) but it's not clearly documented which is >> > which. It would be reasonable to use the IP versions here I think. >> >> Samsung has three i2s drivers one for s3c24xx, one for s3c2412 and one >> for rest of the platforms. The above mentioned other platforms has >> Version 3/4/5 of i2s controllers. This dt binding is for for the i2s > > Where is the version defined such as 3, 4, 5? So, what is the > "sound/soc/Samsung/s3c-i2s-v2.[ch]"? Versions 3, 4, 5 are defined in dev-audio.c file of corresponding platforms. s3c-i2s-v2 is used in s3c2412 platform. > >> driver that has support for Version 3/4/5 of i2s controller. So >> "samsung,i2s-v5" is okay as compatible name? Please suggest me. >> > I agree with using version here but we need some consensus about that. > > - Kukjin > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo(a)vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Thanks Padma

2 2

[alsa-devel] [PATCH] ASoC: cs42l52: Convert to devm_input_allocate_device()
by Axel Lin 27 Dec '12

27 Dec '12

Signed-off-by: Axel Lin <axel.lin(a)ingics.com> --- sound/soc/codecs/cs42l52.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sound/soc/codecs/cs42l52.c b/sound/soc/codecs/cs42l52.c index 99bb1c6..73a8049 100644 --- a/sound/soc/codecs/cs42l52.c +++ b/sound/soc/codecs/cs42l52.c @@ -1040,7 +1040,7 @@ static void cs42l52_init_beep(struct snd_soc_codec *codec) struct cs42l52_private *cs42l52 = snd_soc_codec_get_drvdata(codec); int ret; - cs42l52->beep = input_allocate_device(); + cs42l52->beep = devm_input_allocate_device(codec->dev); if (!cs42l52->beep) { dev_err(codec->dev, "Failed to allocate beep device\n"); return; @@ -1061,7 +1061,6 @@ static void cs42l52_init_beep(struct snd_soc_codec *codec) ret = input_register_device(cs42l52->beep); if (ret != 0) { - input_free_device(cs42l52->beep); cs42l52->beep = NULL; dev_err(codec->dev, "Failed to register beep device\n"); } @@ -1078,7 +1077,6 @@ static void cs42l52_free_beep(struct snd_soc_codec *codec) struct cs42l52_private *cs42l52 = snd_soc_codec_get_drvdata(codec); device_remove_file(codec->dev, &dev_attr_beep); - input_unregister_device(cs42l52->beep); cancel_work_sync(&cs42l52->beep_work); cs42l52->beep = NULL; -- 1.7.9.5

2 1

Re: [alsa-devel] [PATCH 1/2] ASoC: core: fix the memory leak in case of device_add() failure
by Mark Brown 27 Dec '12

27 Dec '12

On Wed, Dec 26, 2012 at 12:56:05AM +0800, Chuansheng Liu wrote: > > After called device_initialize(), even device_add() returns > error, we still need use the put_device() to release the reference > to call rtd_release(), which will do the free() action. Applied both, thanks.

1 0

[alsa-devel] [PATCH] ASoC: cs42l52: Catch no-match case in cs42l52_get_clk
by Axel Lin 24 Dec '12

24 Dec '12

In the case of no-match, return -EINVAL instead of 0. Since we assign i to ret in the for loop, ret always less than ARRAY_SIZE(clk_map_table). Thus remove the boundary checking for ret. Signed-off-by: Axel Lin <axel.lin(a)ingics.com> --- sound/soc/codecs/cs42l52.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sound/soc/codecs/cs42l52.c b/sound/soc/codecs/cs42l52.c index 99bb1c6..9811a54 100644 --- a/sound/soc/codecs/cs42l52.c +++ b/sound/soc/codecs/cs42l52.c @@ -737,7 +737,7 @@ static const struct cs42l52_clk_para clk_map_table[] = { static int cs42l52_get_clk(int mclk, int rate) { - int i, ret = 0; + int i, ret = -EINVAL; u_int mclk1, mclk2 = 0; for (i = 0; i < ARRAY_SIZE(clk_map_table); i++) { @@ -749,8 +749,6 @@ static int cs42l52_get_clk(int mclk, int rate) } } } - if (ret > ARRAY_SIZE(clk_map_table)) - return -EINVAL; return ret; } -- 1.7.9.5

3 2