Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755529AbdCTQZb (ORCPT ); Mon, 20 Mar 2017 12:25:31 -0400 Received: from mail-yw0-f174.google.com ([209.85.161.174]:34812 "EHLO mail-yw0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754154AbdCTQYV (ORCPT ); Mon, 20 Mar 2017 12:24:21 -0400 MIME-Version: 1.0 In-Reply-To: <20170320160622.5s2oqdyluvanjp2u@sirena.org.uk> References: <20170320035831.10762-1-kai.heng.feng@canonical.com> <20170320150845.7rltrhycdkk2mza4@sirena.org.uk> <20170320160622.5s2oqdyluvanjp2u@sirena.org.uk> From: Kai-Heng Feng Date: Tue, 21 Mar 2017 00:23:53 +0800 Message-ID: Subject: Re: [PATCH v3] ASoC: rt286: fix headphone click/crack noise on Dell XPS 9343 I2S mode To: Mark Brown Cc: Liam Girdwood , Bard Liao , Oder Chiou , alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2142 Lines: 57 On Tue, Mar 21, 2017 at 12:06 AM, Mark Brown wrote: > On Mon, Mar 20, 2017 at 03:46:13PM +0000, Kai-Heng Feng wrote: >> On Mon, Mar 20, 2017 at 11:08 PM Mark Brown wrote: > >> > As covered in SubmittingPatches this should come after the ---, it >> > doesn't need to end up in the changelogs. > >> Do you mean >> https://github.com/git/git/blob/master/Documentation/SubmittingPatches#L197 >> ? >> I didn't find any hard rules regarding this, but I'll keep it in mind. > > As it says there "...and inserted automatically following the three dash > line". I saw iteration changelog in git log all over the place, maybe add a rule section for each subsystem? > >> > > SND_SOC_DAPM_POST_PMD: + case SND_SOC_DAPM_POST_PMU: + >> > > snd_soc_write(codec, RT286_SET_AMP_GAIN_HPO, AMP_OUT_MUTE); + >> > > break; + case SND_SOC_DAPM_PRE_PMU: > > Please fix your mail client not to completely mangle quoted patches when > replying. Okay. I was toying with Google Inbox. > >> > To repeat what I said last time: > >> > | After power up we mute the amplifier? That's worthy of a >> > comment... > >> IIUC, HPO Power's _POST_PMU is triggered right before power down >> (_PRE_PMD), hence it's pretty logical to mute the amplifier at this >> stage. I can't quite see anything wrong here. > > No, that is not the case - I'm not sure what would lead you to believe > that it is. _POST_PMU is triggered as the last step of powering up the > widget as the name might suggest. Has this code been tested at all? I had the same thought originally, but printk under each case suggests otherwise - _POST_PMU is triggered not right after _PRE_PMU but right before _PRE_PMD. And yes, the patch was tested on a real machine. > >> So no I didn't ignore your comment, I simply misinterpreted what you >> meant. Because of the logical assumption, I thought you were talking >> the unmute part in _PRE_PMU, which I did add in the changelog. > > You didn't reply to my review comment and you sent the same code again. > That looks an awful lot like being ignored. Fair enough, I thought changelog is sufficient.