ASoC: SOF: Intel: fix HDA codec driver probe with multiple controllers#1697
Merged
bardliao merged 1 commit intothesofproject:release/sof-v5.0from Jan 14, 2020
Merged
Conversation
In case system has multiple HDA controllers, it can happen that same HDA codec driver is used for codecs of multiple controllers. In this case, SOF may fail to probe the HDA driver and SOF initialization fails. SOF HDA code currently relies that a call to request_module() will also run device matching logic to attach driver to the codec instance. However if driver for another HDA controller was already loaded and it already loaded the HDA codec driver, this breaks current logic in SOF. In this case the request_module() SOF does becomes a no-op and HDA Codec driver is not attached to the codec instance sitting on the HDA bus SOF is controlling. Typical scenario would be a system with both external and internal GPUs, with driver of the external GPU loaded first. Fix this by adding similar logic as is used in legacy HDA driver where an explicit device_attach() call is done after request_module(). Also add logic to propagate errors reported by device_attach() back to caller. This also works in the case where drivers are not built as modules. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
|
Issue #1672 is fixed with this PR#1697 |
|
@kv2019i , error still seen if rmmod sof_pci_dev then modprobe sof_pci_dev again. |
Collaborator
Author
@ClarexZhou Will this issue be seen on sof-dev branch, too? |
lyakh
reviewed
Jan 13, 2020
| * handle ret==0 (no driver bound) as an error, but pass | ||
| * other return codes without modification | ||
| */ | ||
| if (ret == 0) |
Collaborator
There was a problem hiding this comment.
I think checkpatch suggests to use if (!ret) in such cases
Collaborator
|
Error not seen on sof-dev, Nvidia hdmi isn't loaded on sof-dev. |
|
Can't use kmod scripts for module reload on sof-v5.0. Because snd_hda_codec_hdmi and snd_hda_intel is always in used 1 and not listed which module is using. |
Collaborator
Author
@ClarexZhou Is this a regression by this PR? |
Collaborator
|
@ClarexZhou It seems something else goes wrong in the case you get a failure: [ 199.457051] snd_hda_codec_realtek ehdaudio0D0: Unable to sync register 0x2b8000. -5 ... this is pretty bad and explains why the module unload fails. Probably good to file a separate bug about this. |
kv2019i
approved these changes
Jan 13, 2020
Collaborator
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Assumig the problem Claire raised is not a regression caused by this patch, then ok to merge this patch.
Collaborator
|
As discussed with @kv2019i, it seems, that the actual problem would be in the hda bus implementation. As far as I understand, we are dealing here with the registration of a new HDA bus instance, upon which no bus scan is initiated. And that seems to be wrong. Compare to USB, PCI. If you hot-plus a card with a new internal USB bus on it, i assume it will be scanned automatically as soon as the bus instance is registered. Shouldn't the same happen here? Obviously, such a fix would be way out of scope of this PR, which aims to provide a fix for a specific problem. |
This isn't regression from this PR. Should be there when NVidia HDMI is in use. Nvidia HDMI is using snd_hda_intel. Don't know how to unload this now. |
Collaborator
Author
|
I will merge it since the original PR has merged into topic/sof-dev branch and there is no regression to sof-v5.0 branch. |
bardliao
pushed a commit
to bardliao/linux
that referenced
this pull request
Mar 5, 2025
idpf_rx_rsc() uses skb_transport_offset(skb) while the transport header is not set yet. This triggers the following warning for CONFIG_DEBUG_NET=y builds. DEBUG_NET_WARN_ON_ONCE(!skb_transport_header_was_set(skb)) [ 69.261620] WARNING: CPU: 7 PID: 0 at ./include/linux/skbuff.h:3020 idpf_vport_splitq_napi_poll (include/linux/skbuff.h:3020) idpf [ 69.261629] Modules linked in: vfat fat dummy bridge intel_uncore_frequency_tpmi intel_uncore_frequency_common intel_vsec_tpmi idpf intel_vsec cdc_ncm cdc_eem cdc_ether usbnet mii xhci_pci xhci_hcd ehci_pci ehci_hcd libeth [ 69.261644] CPU: 7 UID: 0 PID: 0 Comm: swapper/7 Tainted: G S W 6.14.0-smp-DEV thesofproject#1697 [ 69.261648] Tainted: [S]=CPU_OUT_OF_SPEC, [W]=WARN [ 69.261650] RIP: 0010:idpf_vport_splitq_napi_poll (include/linux/skbuff.h:3020) idpf [ 69.261677] ? __warn (kernel/panic.c:242 kernel/panic.c:748) [ 69.261682] ? idpf_vport_splitq_napi_poll (include/linux/skbuff.h:3020) idpf [ 69.261687] ? report_bug (lib/bug.c:?) [ 69.261690] ? handle_bug (arch/x86/kernel/traps.c:285) [ 69.261694] ? exc_invalid_op (arch/x86/kernel/traps.c:309) [ 69.261697] ? asm_exc_invalid_op (arch/x86/include/asm/idtentry.h:621) [ 69.261700] ? __pfx_idpf_vport_splitq_napi_poll (drivers/net/ethernet/intel/idpf/idpf_txrx.c:4011) idpf [ 69.261704] ? idpf_vport_splitq_napi_poll (include/linux/skbuff.h:3020) idpf [ 69.261708] ? idpf_vport_splitq_napi_poll (drivers/net/ethernet/intel/idpf/idpf_txrx.c:3072) idpf [ 69.261712] __napi_poll (net/core/dev.c:7194) [ 69.261716] net_rx_action (net/core/dev.c:7265) [ 69.261718] ? __qdisc_run (net/sched/sch_generic.c:293) [ 69.261721] ? sched_clock (arch/x86/include/asm/preempt.h:84 arch/x86/kernel/tsc.c:288) [ 69.261726] handle_softirqs (kernel/softirq.c:561) Fixes: 3a8845a ("idpf: add RX splitq napi poll support") Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Alan Brady <alan.brady@intel.com> Cc: Joshua Hay <joshua.a.hay@intel.com> Cc: Willem de Bruijn <willemb@google.com> Acked-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> Link: https://patch.msgid.link/20250226221253.1927782-1-edumazet@google.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In case system has multiple HDA controllers, it can happen that
same HDA codec driver is used for codecs of multiple controllers.
In this case, SOF may fail to probe the HDA driver and SOF
initialization fails.
SOF HDA code currently relies that a call to request_module() will
also run device matching logic to attach driver to the codec instance.
However if driver for another HDA controller was already loaded and it
already loaded the HDA codec driver, this breaks current logic in SOF.
In this case the request_module() SOF does becomes a no-op and HDA
Codec driver is not attached to the codec instance sitting on the HDA
bus SOF is controlling. Typical scenario would be a system with both
external and internal GPUs, with driver of the external GPU loaded
first.
Fix this by adding similar logic as is used in legacy HDA driver
where an explicit device_attach() call is done after request_module().
Also add logic to propagate errors reported by device_attach() back
to caller. This also works in the case where drivers are not built
as modules.
Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com
The PR it to back port #1679 to sof-v5.0 branch. There are some conflicts when I tried to cherry-pick the commits from #1679 to release/sof-v5.0 branch. So, create this PR to make sure I didn't make mistake. @ClarexZhou Could you retest #1672 with release/sof-v5.0 + this PR?