August 2011 - Alsa-devel - mailman.alsa-project.org

Re: [alsa-devel] [PATCH] ASoC: soc-jack: Fix checking return value of request_any_context_irq
by Mark Brown 15 Aug '11

15 Aug '11

On Sat, Aug 13, 2011 at 07:15:01PM +0800, Axel Lin wrote: > request_any_context_irq() returns a negative value on failure. > On success, it returns either IRQC_IS_HARDIRQ or IRQC_IS_NESTED. Applied, thanks.

1 0

[alsa-devel] [PATCH 0/4] RME RayDAT/AIO - enable period size of 32 samples
by Adrian Knoth 15 Aug '11

15 Aug '11

Hi! Here is a series of patches that enable period sizes of 32 samples on RME RayDAT and AIO cards. It also adds 8192 samples per period for MADI and AES cards, this mode was present in the original driver but got lost during the merge. Cheers Adrian Knoth (4): ALSA: hdspm - Allow for 8192 period size on RME MADI and AES cards ALSA: hdspm - Set period_bytes_min to 32 * 4 for new RME cards ALSA: hdspm - Reorder period sizes according to their bit representation ALSA: hdspm - Enable 32 samples/period on RME RayDAT/AIO sound/pci/rme9652/hdspm.c | 50 ++++++++++++++++++++++++++++++++++++-------- 1 files changed, 41 insertions(+), 9 deletions(-) -- 1.7.5.4

2 18

Re: [alsa-devel] [PATCH] ASoC: sta32x: shortcut the for loop to get ir and mcs
by Johannes Stezenbach 15 Aug '11

15 Aug '11

On Sun, Aug 14, 2011 at 11:31:04AM +0800, Axel Lin wrote: > There is exactly one match or no match at all during the for loop iteration, > thus we can break from the for loop once a match is found. > > Signed-off-by: Axel Lin <axel.lin(a)gmail.com> Looks good. Acked-by: Johannes Stezenbach <js(a)sig21.net> Thanks, Johannes > --- > sound/soc/codecs/sta32x.c | 8 ++++++-- > 1 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c > index fbd7eb9..3d155f5 100644 > --- a/sound/soc/codecs/sta32x.c > +++ b/sound/soc/codecs/sta32x.c > @@ -524,13 +524,17 @@ static int sta32x_hw_params(struct snd_pcm_substream *substream, > rate = params_rate(params); > pr_debug("rate: %u\n", rate); > for (i = 0; i < ARRAY_SIZE(interpolation_ratios); i++) > - if (interpolation_ratios[i].fs == rate) > + if (interpolation_ratios[i].fs == rate) { > ir = interpolation_ratios[i].ir; > + break; > + } > if (ir < 0) > return -EINVAL; > for (i = 0; mclk_ratios[ir][i].ratio; i++) > - if (mclk_ratios[ir][i].ratio * rate == sta32x->mclk) > + if (mclk_ratios[ir][i].ratio * rate == sta32x->mclk) { > mcs = mclk_ratios[ir][i].mcs; > + break; > + } > if (mcs < 0) > return -EINVAL; > > -- > 1.7.4.1 > > > >

1 0

[alsa-devel] Questions about the alsa-driver-unstable tarballs
by David Henningsson 15 Aug '11

15 Aug '11

Hi Takashi, 1) there seems to be a build failure with current alsa-driver-unstable-snapshot [1] : ...as alsa-kernel/soc/Makefile references a mxs/ directory, but this directory is not present in the soc/ directory. 2) it seems like the .bz2 version of alsa-driver-snapshot has disappeared, is this intentional? 3) Could you explain the difference between alsa-driver-snapshot and alsa-driver-unstable-snapshot, and how the alsa-kernel.git, alsa-driver.git and sound-2.6.git come together to form the two different tarballs? Thanks! -- David Henningsson, Canonical Ltd. http://launchpad.net/~diwic [1] http://ftp.kernel.org/pub/linux/kernel/people/tiwai/alsa/alsa-driver/alsa-d…

2 3

[alsa-devel] [PATCH] ALSA: ac97: Add HP Compaq dc5100 SFF(PT003AW) to Headphone Jack Sense whitelist
by Dan Chen 15 Aug '11

15 Aug '11

Hi, Attached please find a patch against sound-2.6 master HEAD fixing inaudible audio in the bug reporter's system by unmuting 'Headphone Jack Sense'. To make it permanent, his PCI SSID is added to the whitelist. Please consider applying. Cheers, -Dan

2 1

[alsa-devel] [PATCH] ASoC: Disable pulls on WM8994 AIF2 when starting it
by Mark Brown 15 Aug '11

15 Aug '11

Pull control is availalbe for WM8994 AIF2, generally disabled as part of the GPIO configuration in order to save power after system startup. As on newer devices in the series there is no GPIO functionality on these pins this will happen less naturally so have the driver disable the pulls as the AIF is probed. Signed-off-by: Mark Brown <broonie(a)opensource.wolfsonmicro.com> --- sound/soc/codecs/wm8994.c | 16 ++++++++++++++++ 1 files changed, 16 insertions(+), 0 deletions(-) diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c index b63c4c8..f328369 100644 --- a/sound/soc/codecs/wm8994.c +++ b/sound/soc/codecs/wm8994.c @@ -2498,6 +2498,21 @@ static int wm8994_set_tristate(struct snd_soc_dai *codec_dai, int tristate) return snd_soc_update_bits(codec, reg, mask, val); } +static int wm8994_aif2_probe(struct snd_soc_dai *dai) +{ + struct snd_soc_codec *codec = dai->codec; + + /* Disable the pulls on the AIF if we're using it to save power. */ + snd_soc_update_bits(codec, WM8994_GPIO_3, + WM8994_GPN_PU | WM8994_GPN_PD, 0); + snd_soc_update_bits(codec, WM8994_GPIO_4, + WM8994_GPN_PU | WM8994_GPN_PD, 0); + snd_soc_update_bits(codec, WM8994_GPIO_5, + WM8994_GPN_PU | WM8994_GPN_PD, 0); + + return 0; +} + #define WM8994_RATES SNDRV_PCM_RATE_8000_96000 #define WM8994_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\ @@ -2565,6 +2580,7 @@ static struct snd_soc_dai_driver wm8994_dai[] = { .rates = WM8994_RATES, .formats = WM8994_FORMATS, }, + .probe = wm8994_aif2_probe, .ops = &wm8994_aif2_dai_ops, }, { -- 1.7.5.4

1 0

[alsa-devel] Investigating ALC662 3stack noise
by Miguel Freitas 14 Aug '11

14 Aug '11

Hi developers, I'm trying to fix an annoying noise i get from my D945GCLF2D Intel Motherboard, but only from Surround/clfe outputs, not from Front jack. Since i'm running out of ideas i thought people here might be able to suggest some pointers. The problem is: whenever I change from 2ch to 6ch Channel Mode in mixer I get the background noise in my speakers. In 2ch mode there is no noise but that doesn't help at all since I want that pin as output. I fail to admit Intel or Realtek would do such a horrible engineering job (Intel is happy to say in their datasheet the motherboard has 95dB S/N ratio). It's more like we did connect something wrong is our driver, or failed to change some required settings, i don't know. What I've tried so far: 1) I removed the cable to front case connectors guessing noise might be coming from there. Nothing. 2) The motherboard/ALC662 assembly is obviously 3stack kind. I double-checked all settings in patch_realtek.c against ALC662 datasheet. Changing NID 0x1A from input to output, unmuting and connecting it to idx 0x01 (surround) seems like the right thing to me, afterall, I do get sound as expected from this port. 3) To eliminate the possibility that the noise was coming from the ALC662 itself (but from a different port) I changed NID 0x0d (surround) to mute idx 01, which is the connection from NID 0x0b (that big mixer with Mic, Line, Beeper, CD etc). The leaves just DAC PCM-2 connected and the noise is still there. 4) Muting/lowering volume for Surround (DAC PCM-2 / NID 0x03) also has no effect on noise. 5) Thinking that some external amplifier on motherboard might need to be disabled I've played with EAPD and GPIOs 0,1. I can clearly see at least GPIO 1 is routed on the motherboard, although no idea if it is just ends in a testpoint pad or that could be a via to some internal layer. Different settings of EAPD/GPIOs pins made no difference to noise. So that is pretty much the point i'm now. I'm starting to consider soldering another jack to unused Port-A (surround) pins... but that would be kind of rude, don't you think so? :-) Any ideas are very welcomed... regards, Miguel

1 0

[alsa-devel] [PATCH] ALSA: snd_usb_caiaq: track submitted output urbs
by Daniel Mack 14 Aug '11

14 Aug '11

The snd_usb_caiaq driver currently assumes that output urbs are serviced in time and doesn't track when and whether they are given back by the USB core. That usually works fine, but due to temporary limitations of the XHCI stack, we faced that urbs were submitted more than once with this approach. As it's no good practice to fire and forget urbs anyway, this patch introduces a proper bit mask to track which requests have been submitted and given back. That alone however doesn't make the driver work in case the host controller is broken and doesn't give back urbs at all, and the output stream will stop once all pre-allocated output urbs are consumed. But it does prevent crashes of the controller stack in such cases. See http://bugzilla.kernel.org/show_bug.cgi?id=40702 for more details. Signed-off-by: Daniel Mack <zonque(a)gmail.com> Reported-and-tested-by: Matej Laitl <matej(a)laitl.cz> Cc: Sarah Sharp <sarah.a.sharp(a)linux.intel.com> Cc: stable(a)kernel.org --- sound/usb/caiaq/audio.c | 31 +++++++++++++++++++++++++++---- sound/usb/caiaq/device.h | 1 + 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/sound/usb/caiaq/audio.c b/sound/usb/caiaq/audio.c index aa52b3e..2cf87f5 100644 --- a/sound/usb/caiaq/audio.c +++ b/sound/usb/caiaq/audio.c @@ -139,8 +139,12 @@ static void stream_stop(struct snd_usb_caiaqdev *dev) for (i = 0; i < N_URBS; i++) { usb_kill_urb(dev->data_urbs_in[i]); - usb_kill_urb(dev->data_urbs_out[i]); + + if (test_bit(i, &dev->outurb_active_mask)) + usb_kill_urb(dev->data_urbs_out[i]); } + + dev->outurb_active_mask = 0; } static int snd_usb_caiaq_substream_open(struct snd_pcm_substream *substream) @@ -612,8 +616,8 @@ static void read_completed(struct urb *urb) { struct snd_usb_caiaq_cb_info *info = urb->context; struct snd_usb_caiaqdev *dev; - struct urb *out; - int frame, len, send_it = 0, outframe = 0; + struct urb *out = NULL; + int i, frame, len, send_it = 0, outframe = 0; size_t offset = 0; if (urb->status || !info) @@ -624,7 +628,17 @@ static void read_completed(struct urb *urb) if (!dev->streaming) return; - out = dev->data_urbs_out[info->index]; + /* find an unused output urb that is unused */ + for (i = 0; i < N_URBS; i++) + if (test_and_set_bit(i, &dev->outurb_active_mask) == 0) { + out = dev->data_urbs_out[i]; + break; + } + + if (!out) { + log("Unable to find an output urb to use\n"); + goto requeue; + } /* read the recently received packet and send back one which has * the same layout */ @@ -655,8 +669,12 @@ static void read_completed(struct urb *urb) out->number_of_packets = outframe; out->transfer_flags = URB_ISO_ASAP; usb_submit_urb(out, GFP_ATOMIC); + } else { + struct snd_usb_caiaq_cb_info *oinfo = out->context; + clear_bit(oinfo->index, &dev->outurb_active_mask); } +requeue: /* re-submit inbound urb */ for (frame = 0; frame < FRAMES_PER_URB; frame++) { urb->iso_frame_desc[frame].offset = BYTES_PER_FRAME * frame; @@ -678,6 +696,8 @@ static void write_completed(struct urb *urb) dev->output_running = 1; wake_up(&dev->prepare_wait_queue); } + + clear_bit(info->index, &dev->outurb_active_mask); } static struct urb **alloc_urbs(struct snd_usb_caiaqdev *dev, int dir, int *ret) @@ -829,6 +849,9 @@ int snd_usb_caiaq_audio_init(struct snd_usb_caiaqdev *dev) if (!dev->data_cb_info) return -ENOMEM; + dev->outurb_active_mask = 0; + BUILD_BUG_ON(N_URBS > (sizeof(dev->outurb_active_mask) * 8)); + for (i = 0; i < N_URBS; i++) { dev->data_cb_info[i].dev = dev; dev->data_cb_info[i].index = i; diff --git a/sound/usb/caiaq/device.h b/sound/usb/caiaq/device.h index b2b3101..3f9c633 100644 --- a/sound/usb/caiaq/device.h +++ b/sound/usb/caiaq/device.h @@ -96,6 +96,7 @@ struct snd_usb_caiaqdev { int input_panic, output_panic, warned; char *audio_in_buf, *audio_out_buf; unsigned int samplerates, bpp; + unsigned long outurb_active_mask; struct snd_pcm_substream *sub_playback[MAX_STREAMS]; struct snd_pcm_substream *sub_capture[MAX_STREAMS]; -- 1.7.6

2 1

[alsa-devel] No sound with HDA ATI HDMI since kernel 3.0
by Yannick Palanque 14 Aug '11

14 Aug '11

Hello, First, I'm sorry for my english is awkward and I'm not sure if I'm at the right place. I use Linux amd64 and have two sound "cards": Motherboard: 0 [Intel ]: HDA-Intel - HDA Intel HDA Intel at 0xfe9f8000 irq 47 Graphic card: 1 [HDMI ]: HDA-Intel - HDA ATI HDMI HDA ATI HDMI at 0xfeaec000 irq 48 (lspci ouput and alsa-info.txt attached) I have no problem with 2.6.39.3. With kernel 3.0 and 3.0.1, I have no sound with HDMI output. VLC 1.1.3 gives me no sound (I don't see any error message). MPlayer SVN-r31918 (with correct -ao alsa:device=hw=1.3 option, it works with 2.6.39.3) gives me no sound and I see the audio position in seconds on status line going approximatively 6 times faster than normal (in fact, video is 6 times faster, too). I attached dmesg output for kernels 2.6.39.3 and 3.0.1. For both kernels, I have the line hda-intel: IRQ timing workaround is activated for card #1. Suggest a bigger bdl_pos_adj. I tried modifying parameters bdl_pos_adj and position_fix, because I read that some people tried this, when I searched for this problem, but only 2.6.39.3 worked, with default setup (see attached tests.txt file). I also tried to replace the files in sound/pci/hda/* from 2.6.39.3 (I read that about Debian bug 635362 in this very list) but it doesn't work better. I'm only a modest user and I don't know what to do best, but I would be delighted if I could help to solve what I believe to be a bug on a specific hardware. Cordially, -- Yannick Palanque

2 3

Re: [alsa-devel] Please review rt5631 driver on alsa 1.0.24.
by Mark Brown 14 Aug '11

14 Aug '11

On Fri, Aug 12, 2011 at 05:31:40PM +0800, johnnyhsu wrote: > Dear Mr. Brown and Mr. Girdwood, > > This patch is made against sound-2.6.git for-next. > Because the git send-email can't work in my PC, I send the patch by Outlook. > Hope it won't make error. No, this doesn't work at all - Outlook mangles your patch so it can't be applied and text like this at the top of the mail means it's not in the format the tools expect. There's quite a few problems, overall my main comment is the same as the last time - please look at how other drivers do things and try to fit in with the general style. There's a lot of code in here which doesn't look like normal ASoC code with no hint as to why. > +#define RT5631_VERSION "0.01 alsa 1.0.24" The kernel has a perfectly good versioning system, remove this. > +static const u16 rt5631_reg[0x80]; > +static int timesofbclk = 32; No global variables. > +static void snd_soc_write_index(struct snd_soc_codec *codec, > + unsigned int reg, unsigned int value) All your snd_soc_ functions should have some driver specific name, unless they really are generic in which case they should be added to the subsystem code rather than kept in your driver. This one doesn't appear to be generic. > +{ > + snd_soc_write(codec, RT5631_INDEX_ADD, reg); > + snd_soc_write(codec, RT5631_INDEX_DATA, value); > + > + return; > +} The return statement is pointless here. > +static void snd_soc_update_bits_index(struct snd_soc_codec *codec, > + unsigned int reg, unsigned int mask, unsigned int value) > +{ > + unsigned int reg_val; > + > + if (!mask) > + return; > + > + if (mask != 0xffff) { > + reg_val = snd_soc_read_index(codec, reg); > + reg_val &= ~mask; > + reg_val |= (value & mask); > + snd_soc_write_index(codec, reg, reg_val); > + } else { > + snd_soc_write_index(codec, reg, value); > + } > + > + return; > +} Looking at this I'd expect regular snd_soc_update_bits() would work just fine for you. > +/* > + * speaker channel volume 0dB by default > + * Headphone channel volume 0dB by default > + * AXO1/AXO2 channel volume 0dB by default > + * Mic1/Mic2 boost 40dB by default > + * Speaker AMP ratio gain is 1.44X > + * enable HP zero cross > + * change Mic1 & mic2 to differential mode > + */ As I said in reply to your original mail: | This sort of stuff is not suitable for mainline either, the driver | should just use chip defaults for audio paths and leave the use case up | to the application layer. | Please compare your driver with the other CODEC drivers and make sure it | is following a simmilar style to them. > +static const char *rt5631_mic_boost[] = {"Bypass", "+20db", "+24db", > "+30db", > + "+35db", "+40db", "+44db", "+50db", "+52db"}; This should be TLV dB information. > +static const char *rt5631_hpl_source_sel[] = {"LEFT HPVOL", "LEFT DAC"}; > +static const char *rt5631_hpr_source_sel[] = {"RIGHT HPVOL", "RIGHT DAC"}; > +static const char *rt5631_eq_sel[] = {"NORMAL", "CLUB", "DANCE", "LIVE", > "POP", > + "ROCK", "OPPO", "TREBLE", "BASS"}; The general style is not all caps. > +static const struct soc_enum rt5631_enum[] = { Don't use arrays of enums, they're hard to read and error prone. > +static int spk_event(struct snd_soc_dapm_widget *w, > + struct snd_kcontrol *kcontrol, int event) > +{ > + struct snd_soc_codec *codec = w->codec; > + static int spkl_out_enable, spkr_out_enable; > + > + switch (event) { > + case SND_SOC_DAPM_POST_PMU: > + if (!spkl_out_enable && !strcmp(w->name, "SPKL Amp")) { DAPM will take care of tracking if the widget is already enabled or disabled. > + snd_soc_update_bits(codec, RT5631_PWR_MANAG_ADD1, > + PWR_CLASS_D, PWR_CLASS_D); I suspect you want a supply widget. > +static void hp_depop_mode2_onebit(struct snd_soc_codec *codec, int enable) > +{ > + unsigned int soft_vol, hp_zc; This function is rather abstruse in name and code terms, it should be clarified. > +static void hp_mute_unmute_depop_onebit(struct snd_soc_codec *codec, int Similarly here and with the other depop functions, they could all could use rather more blank lines and comments. > + if (enable) { > + snd_soc_write_index(codec, RT5631_SPK_INTL_CTRL, 0x303e); > + snd_soc_update_bits(codec, RT5631_PWR_MANAG_ADD3, > + PWR_CHARGE_PUMP | PWR_HP_L_AMP | PWR_HP_R_AMP, > + PWR_CHARGE_PUMP | PWR_HP_L_AMP | PWR_HP_R_AMP); I rather suspect there should be some supply widgets in here. > + case SND_SOC_DAPM_POST_PMU: > + /* > + * If microphone is stereo, need not copy ADC channel > + * If mic1 is used, copy ADC left to right > + * If mic2 is used, copy ADC right to left > + */ Expose the routes via DAPM and leave the control up to the user. > +static int auxo1_event(struct snd_soc_dapm_widget *w, > + struct snd_kcontrol *kcontrol, int event) > +{ > + struct snd_soc_codec *codec = w->codec; > + static bool aux1_en; > + > + switch (event) { > + case SND_SOC_DAPM_PRE_PMD: > + if (aux1_en) { > + snd_soc_update_bits(codec, RT5631_MONO_AXO_1_2_VOL, > + RT_L_MUTE, RT_L_MUTE); > + aux1_en = false; Again, DAPM will keep track of power for you. I'm not clear why you're managing mutes here rather than letting the user control them. > +/** > + * config_common_power - control all common power of codec system > + * @pmu: power up or not > + */ > +static int config_common_power(struct snd_soc_codec *codec, bool pmu) This should just be inline in set_bias_level(). Though... > + if (pmu) { > + snd_soc_update_bits(codec, RT5631_PWR_MANAG_ADD1, > + PWR_MAIN_I2S_EN | PWR_DAC_REF, > + PWR_MAIN_I2S_EN | PWR_DAC_REF); > + mux_val = snd_soc_read(codec, RT5631_SPK_MONO_HP_OUT_CTRL); > + if (!(mux_val & HP_L_MUX_SEL_DAC_L)) > + snd_soc_update_bits(codec, RT5631_PWR_MANAG_ADD1, > + PWR_DAC_L_TO_MIXER, PWR_DAC_L_TO_MIXER); > + if (!(mux_val & HP_R_MUX_SEL_DAC_R)) > + snd_soc_update_bits(codec, RT5631_PWR_MANAG_ADD1, > + PWR_DAC_R_TO_MIXER, PWR_DAC_R_TO_MIXER); > + if (rt5631->pll_used_flag) > + snd_soc_update_bits(codec, RT5631_PWR_MANAG_ADD2, > + PWR_PLL, PWR_PLL); ..this all looks rather like it should be managed via DAPM. > +static int adc_event(struct snd_soc_dapm_widget *w, > + struct snd_kcontrol *kcontrol, int event) > +{ > + struct snd_soc_codec *codec = w->codec; > + static bool pmu; This isn't going to work if you've got more than one device in the system. > + > + switch (event) { > + case SND_SOC_DAPM_POST_PMD: > + if (pmu) { > + config_common_power(codec, false); > + pmu = false; > + } Again, everything is refcounted for you. Though I'm not entirely clear what this is supposed to do it looks rather like you may want to use a supply widget. In general please try to work with the subsystem rather than layering stuff on top of it - it makes the code more complicated and harder to follow. > +static int rt5631_add_widgets(struct snd_soc_codec *codec) > +{ > + struct snd_soc_dapm_context *dapm = &codec->dapm; > + > + snd_soc_dapm_new_controls(dapm, rt5631_dapm_widgets, > + ARRAY_SIZE(rt5631_dapm_widgets)); > + snd_soc_dapm_add_routes(dapm, audio_map, ARRAY_SIZE(audio_map)); Just point to the tables from the driver, no need to do this manually. > +static const struct pll_div codec_slave_pll_div[] = { > + {256000, 2048000, 0x46f0}, > + {256000, 4096000, 0x3ea0}, > + {352800, 5644800, 0x3ea0}, > + {512000, 8192000, 0x3ea0}, Try to keep the indentation consistent within the table. > +static ssize_t rt5631_index_reg_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + #define IDX_REG_FMT "%02x: %04x\n" > + #define IDX_REG_LEN 9 Ick, don't indent #defines and don't put them in the middle of functions. > + cnt += sprintf(buf, "RT5631 index register\n"); > + for (i = 0; i < 0x55; i++) { > + if (cnt + IDX_REG_LEN >= PAGE_SIZE - 1) > + break; > + val = snd_soc_read_index(codec, i); > + if (!val) > + continue; > + cnt += sprintf(buf + cnt, IDX_REG_FMT, i, val); > + } Use the subsystem, please. > +EXPORT_SYMBOL_GPL(rt5631_dai); No, don't do this. There's no need for anyone to reference the DAI. > + case SND_SOC_BIAS_OFF: > + snd_soc_update_bits(codec, RT5631_SPK_OUT_VOL, > + RT_L_MUTE | RT_R_MUTE, RT_L_MUTE | RT_R_MUTE); > + snd_soc_update_bits(codec, RT5631_HP_OUT_VOL, > + RT_L_MUTE | RT_R_MUTE, RT_L_MUTE | RT_R_MUTE); This looks very suspicous, it'll mess with whatever the user has configured for the mutes. This applies to quite a few places in the driver > + snd_soc_add_controls(codec, rt5631_snd_controls, > + ARRAY_SIZE(rt5631_snd_controls)); Again, use the tables. > + pr_info("RT5631 initial ok!\n"); The subsystem does a perfectly good job of announcing things, and if you do need to do prints they should use dev_ prints. > +static int rt5631_resume(struct snd_soc_codec *codec) > +{ > + struct rt5631_priv *rt5631 = snd_soc_codec_get_drvdata(codec); > + > + snd_soc_update_bits(codec, RT5631_PWR_MANAG_ADD3, > + PWR_VREF | PWR_MAIN_BIAS, PWR_VREF | PWR_MAIN_BIAS); > + schedule_timeout_uninterruptible(msecs_to_jiffies(110)); > + snd_soc_update_bits(codec, RT5631_PWR_MANAG_ADD3, > + PWR_FAST_VREF_CTRL, PWR_FAST_VREF_CTRL); > + rt5631_reg_init(codec); > + > + /* power off ClassD auto Recovery */ > + if (rt5631->codec_version) > + snd_soc_update_bits(codec, RT5631_INT_ST_IRQ_CTRL_2, > + 0x2000, 0x2000); > + else > + snd_soc_update_bits(codec, RT5631_INT_ST_IRQ_CTRL_2, > + 0x2000, 0); This looks like it should share code with the probe. > +/* > + * detect short current for mic1 > + */ > +int rt5631_ext_mic_detect(struct snd_soc_codec *codec) > +{ > + int det; > + > + snd_soc_update_bits(codec, RT5631_MIC_CTRL_2, > + MICBIAS1_S_C_DET_MASK, MICBIAS1_S_C_DET_ENA); > + det = snd_soc_read(codec, RT5631_INT_ST_IRQ_CTRL_2) & 0x0001; > + snd_soc_update_bits(codec, RT5631_INT_ST_IRQ_CTRL_2, 0x0001, > 0x00001); > + > + return det; > +} > +EXPORT_SYMBOL_GPL(rt5631_ext_mic_detect); It looks like you want to write something that slots into soc-jack. > + pr_info("RT5631 Audio Codec %s\n", RT5631_VERSION); This is needlessly chatty. > +struct i2c_driver rt5631_i2c_driver = { > + .driver = { > + .name = "rt5631-codec", > + .owner = THIS_MODULE, No need for a -codec, the device only does one thing. > +/* global definition */ > +#define RT_L_MUTE (0x1 << 15) > +#define RT_R_MUTE (0x1 << 7) Most of the constants need namespacing.

1 0