śr., 27 sty 2021 o 23:02 Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> napisał(a): > On 1/27/21 1:18 PM, Marcin Ślusarz wrote: > > śr., 27 sty 2021 o 18:28 Pierre-Louis Bossart > > <pierre-louis.bossart@xxxxxxxxxxxxxxx> napisał(a): > >>> Weird, I can't reproduce this problem with my self-compiled kernel :/ > >>> I don't even see soundwire modules loaded in. Manually loading them of course > >>> doesn't do much. > >>> > >>> Previously I could boot into the "faulty" kernel by using "recovery mode", but > >>> I can't do that anymore - it crashes too. > >>> > >>> Maybe there's some kind of race and this bug depends on some specific > >>> ordering of events? > >> > >> missing Kconfig? > >> You need CONFIG_SOUNDWIRE and CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE > >> selected to enter this sdw_intel_acpi_scan() routine. > > > > It was a PEBKAC, but a slightly different one. I won't bore you with > > (embarrassing) details ;). > > > > I reproduced the problem, tested both your and Rafael's patches > > and the kernel still crashes, with the same stack trace. > > (Yes, I'm sure I booted the right kernel :) > > > > Why "recovery mode" stopped working (or worked previously) is still a mystery. > > > > Thanks Marcin for the information. If you have a consistent failure > that's better to some extent. > > Maybe a bit of explanation of what this routine tries to do: > when SoundWire is enabled in a system, we need to have the following > pattern in the DSDT: > > Scope (_SB.PCI0) > { > Device (HDAS) > { > Name (_ADR, 0x001F0003) // _ADR: Address > } > > > Scope (HDAS) > { > Device (SNDW) > { > Name (_ADR, 0x40000000) // _ADR: Address > > The only thing the code does is to walk through the children and check > if the valid _ADR 0x40000000 is found. > > You don't have SoundWire in your device so there should not be any > children found. I don't see anything in the DSDT that looks like > _SB.PCI0.HDAS.<something>, so in theory we should not even enter the > callback. > > The error happens in acpi_bus_get_device(), after we read the adr but > before we check it, so wondering if we shouldn't revert the checks. Can > you try the diff below? I am not sure why there is a crash and we should > root-cause this issue, just trying to triangulate what is happening. > > diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c > index cabdadb09a1b..6bc87a682fb3 100644 > --- a/drivers/soundwire/intel_init.c > +++ b/drivers/soundwire/intel_init.c > @@ -369,13 +369,6 @@ static acpi_status sdw_intel_acpi_cb(acpi_handle > handle, u32 level, > if (ACPI_FAILURE(status)) > return AE_OK; /* keep going */ > > - if (acpi_bus_get_device(handle, &adev)) { > - pr_err("%s: Couldn't find ACPI handle\n", __func__); > - return AE_NOT_FOUND; > - } > - > - info->handle = handle; > - > /* > * On some Intel platforms, multiple children of the HDAS > * device can be found, but only one of them is the SoundWire > @@ -386,6 +379,13 @@ static acpi_status sdw_intel_acpi_cb(acpi_handle > handle, u32 level, > if (FIELD_GET(GENMASK(31, 28), adr) != SDW_LINK_TYPE) > return AE_OK; /* keep going */ > > + if (acpi_bus_get_device(handle, &adev)) { > + pr_err("%s: Couldn't find ACPI handle\n", __func__); > + return AE_NOT_FOUND; > + } > + > + info->handle = handle; > + > /* device found, stop namespace walk */ > return AE_CTRL_TERMINATE; > } still the same crash