From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH 4/5] ALSA: hda - Fix module dependency with gfx i915 Date: Mon, 13 May 2013 14:34:54 +0200 Message-ID: References: <1368430648-2353-1-git-send-email-xingchao.wang@linux.intel.com> <1368430648-2353-5-git-send-email-xingchao.wang@linux.intel.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 704AC2625FD for ; Mon, 13 May 2013 14:34:58 +0200 (CEST) In-Reply-To: <1368430648-2353-5-git-send-email-xingchao.wang@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Wang Xingchao Cc: alsa-devel@alsa-project.org, daniel@ffwll.ch, jocelyn.li@intel.com, mengdong.lin@intel.com, intel-gfx@lists.freedesktop.org, jesse.barnes@intel.com, liam.r.girdwood@intel.com, david.henningsson@canonical.com, paulo.r.zanoni@intel.com List-Id: alsa-devel@alsa-project.org At Mon, 13 May 2013 15:37:27 +0800, Wang Xingchao wrote: > > hda_i915 has dependency on i915 module, this patch check whether > symbol exist before calling API there. If i915 module not loaded it > will try to load before use. > > Signed-off-by: Wang Xingchao You need to manage the symbols more properly. The symbols must be unreferenced upon the driver unbind. Also, I'm not sure whether module refcount of i915 is changed via symbol_get. If not, you need to keep i915 referred until unbind, which calls the corresponding module_put(). Yet another question is whether it's appropriate to call request_module_nowait(). What's wrong with a synchronized one? thanks, Takashi > --- > sound/pci/hda/hda_i915.c | 42 ++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 2 deletions(-) > > diff --git a/sound/pci/hda/hda_i915.c b/sound/pci/hda/hda_i915.c > index 00def82..edf1a08 100644 > --- a/sound/pci/hda/hda_i915.c > +++ b/sound/pci/hda/hda_i915.c > @@ -22,16 +22,54 @@ > #include > #include "hda_i915.h" > > +const char *name = "i915"; > +static void (*get_power)(const char *name); > +static void (*put_power)(const char *name); > + > /* Power well has impact on Haswell controller and codecs */ > void hda_display_power(bool enable) > { > snd_printk(KERN_INFO "HDA display power %s \n", enable ? "Enable" : "Disable"); > + > + if (!get_power || !put_power) > + return; > + > if (enable) > - i915_request_power_well("hda"); > + get_power("hda"); > else > - i915_release_power_well("hda"); > + put_power("hda"); > } > EXPORT_SYMBOL(hda_display_power); > > +static int __init hda_i915_init(void) > +{ > + struct module *i915; > + mutex_lock(&module_mutex); > + i915 = find_module(name); > + mutex_unlock(&module_mutex); > + > + if (!i915) > + request_module_nowait(name); > + > + if (!try_module_get(i915)) > + return -EFAULT; > + > + get_power = symbol_get(i915_request_power_well); > + put_power = symbol_get(i915_release_power_well); > + > + module_put(i915); > + return 0; > +} > + > +#if 0 > +static void __exit hda_i915_exit(void) > +{ > + drm_pci_exit(&driver, &i915_pci_driver); > +} > + > +module_init(hda_i915_init); > +module_exit(hda_i915_exit); > +#endif > +module_init(hda_i915_init); > MODULE_DESCRIPTION("HDA power well"); > MODULE_LICENSE("GPL"); > -- > 1.7.9.5 > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >