Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752498AbdFUPJF (ORCPT ); Wed, 21 Jun 2017 11:09:05 -0400 Received: from mail-lf0-f65.google.com ([209.85.215.65]:35981 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751711AbdFUPJD (ORCPT ); Wed, 21 Jun 2017 11:09:03 -0400 From: Daniel Vetter To: Intel Graphics Development Cc: Chris Wilson , Daniel Vetter , Jaroslav Kysela , Takashi Iwai , Guneshwor Singh , Hardik T Shah , Julia Lawall , Vinod Koul , "Subhransu S. Prusty" , Libin Yang , linux-kernel@vger.kernel.org Subject: [PATCH] BUG-REPORT: snd-hda: hacked-together EPROBE_DEFER support Date: Wed, 21 Jun 2017 17:08:54 +0200 Message-Id: <1498057734-14622-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.5.5 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4748 Lines: 152 So back when the i915 power well support landed in commit 99a2008d0b32d72dfc2a54e7be1eb698dd2e3bd6 Author: Wang Xingchao Date: Thu May 30 22:07:10 2013 +0800 ALSA: hda - Add power-welll support for haswell HDA the logic to handle the cross-module depencies was hand-rolled using a async work item, and that just doesn't work. The correct way to handle cross-module deps is either: - request_module + failing when the other module isn't there OR - failing the module load with EPROBE_DEFER. You can't mix them, if you do then the entire load path just busy-spins blowing through cpu cycles forever with no way to stop this. snd-hda-intel does mix it, because the hda codec drivers are loaded using request_module, but the i915 depency is handled using PROBE_DEFER (or well, should be, but I haven't found any code at all). This is a major pain when trying to debug i915 load failures. This patch here is a horrible hackish attempt at somewhat correctly wriing EPROBE_DEFER through. Stuff that's missing: - Check all the other places where load errors are conveniently dropped on the floor. - Also fix up the firmware_cb path. - Drop the debug noise I've left in to make it clear this isn't anything for merging. Cheers, Daniel Cc: Jaroslav Kysela Cc: Takashi Iwai Cc: "GitAuthor: Daniel Vetter" Cc: Guneshwor Singh Cc: Hardik T Shah Cc: Julia Lawall Cc: Vinod Koul Cc: "Subhransu S. Prusty" Cc: Libin Yang Cc: linux-kernel@vger.kernel.org --- drivers/base/dd.c | 2 ++ sound/pci/hda/hda_bind.c | 6 +++--- sound/pci/hda/hda_controller.c | 8 +++++++- sound/pci/hda/hda_intel.c | 13 +++++++++---- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 4882f06d12df..842bc8782124 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -17,6 +17,8 @@ * This file is released under the GPLv2 */ +#define DEBUG + #include #include #include diff --git a/sound/pci/hda/hda_bind.c b/sound/pci/hda/hda_bind.c index 6efadbfb3fe3..0bc164a17493 100644 --- a/sound/pci/hda/hda_bind.c +++ b/sound/pci/hda/hda_bind.c @@ -253,7 +253,7 @@ static int codec_bind_generic(struct hda_codec *codec) request_codec_module(codec); if (codec_probed(codec)) return 0; - return -ENODEV; + return -EPROBE_DEFER; } #if IS_ENABLED(CONFIG_SND_HDA_GENERIC) @@ -289,8 +289,8 @@ int snd_hda_codec_configure(struct hda_codec *codec) codec_bind_module(codec); if (!codec->preset) { err = codec_bind_generic(codec); - if (err < 0) { - codec_err(codec, "Unable to bind the codec\n"); + if (WARN_ON(err < 0)) { + codec_err(codec, "Unable to bind the codec, err=%i\n", err); goto error; } } diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c index 3715a5725613..4b4262c72327 100644 --- a/sound/pci/hda/hda_controller.c +++ b/sound/pci/hda/hda_controller.c @@ -1337,9 +1337,15 @@ EXPORT_SYMBOL_GPL(azx_probe_codecs); /* configure each codec instance */ int azx_codec_configure(struct azx *chip) { + int ret; + struct hda_codec *codec; list_for_each_codec(codec, &chip->bus) { - snd_hda_codec_configure(codec); + ret = snd_hda_codec_configure(codec); + if (ret) { + printk("bailing real hard %i\n", ret); + return ret; + } } return 0; } diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 07ea7f48aa01..8241387cc8ca 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -1649,7 +1649,8 @@ static void azx_check_snoop_available(struct azx *chip) static void azx_probe_work(struct work_struct *work) { struct hda_intel *hda = container_of(work, struct hda_intel, probe_work); - azx_probe_continue(&hda->chip); + + WARN_ON(1); } static int default_bdl_pos_adj(struct azx *chip) @@ -2158,7 +2159,6 @@ static int azx_probe(struct pci_dev *pci, azx_firmware_cb); if (err < 0) goto out_free; - schedule_probe = false; /* continued in azx_firmware_cb() */ } #endif /* CONFIG_SND_HDA_PATCH_LOADER */ @@ -2167,8 +2167,13 @@ static int azx_probe(struct pci_dev *pci, dev_err(card->dev, "Haswell/Broadwell HDMI/DP must build in CONFIG_SND_HDA_I915\n"); #endif - if (schedule_probe) - schedule_work(&hda->probe_work); + if (schedule_probe) { + err = azx_probe_continue(chip); + if (err) { + printk("hit the right error return finally! err=%i\n", err); + goto out_free; + } + } dev++; if (chip->disabled) -- 2.5.5