| Message ID | 20241122094224.226773-1-stanislaw.gruszka@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9c69c8286754c61f95ea9189b2b5a794bdb07fed |
| Headers | show |
| Series | spi: Fix acpi deferred irq probe | expand |
Hi, On 22-Nov-24 10:42 AM, Stanislaw Gruszka wrote: > When probing spi device take care of deferred probe of ACPI irq gpio > similar like for OF/DT case. > > From practical standpoint this fixes issue with vsc-tp driver on > Dell XP 9340 laptop, which try to request interrupt with spi->irq > equal to -EPROBE_DEFER and fail to probe with the following error: > > vsc-tp spi-INTC10D0:00: probe with driver vsc-tp failed with error -22 > > Suggested-by: Hans de Goede <hdegoede@redhat.com> > Fixes: 33ada67da352 ("ACPI / spi: attach GPIO IRQ from ACPI description to SPI device") > Cc: stable@vger.kernel.org > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > drivers/spi/spi.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index c1dad30a4528..0f3e6e2c2474 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -424,6 +424,16 @@ static int spi_probe(struct device *dev) > spi->irq = 0; > } > > + if (has_acpi_companion(dev) && spi->irq < 0) { > + struct acpi_device *adev = to_acpi_device_node(dev->fwnode); > + > + spi->irq = acpi_dev_gpio_irq_get(adev, 0); > + if (spi->irq == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + if (spi->irq < 0) > + spi->irq = 0; > + } > + > ret = dev_pm_domain_attach(dev, true); > if (ret) > return ret; > @@ -2869,9 +2879,6 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr, > acpi_set_modalias(adev, acpi_device_hid(adev), spi->modalias, > sizeof(spi->modalias)); > > - if (spi->irq < 0) > - spi->irq = acpi_dev_gpio_irq_get(adev, 0); > - > acpi_device_set_enumerated(adev); > > adev->power.flags.ignore_parent = true;
Hello, owning a Dell XPS 9320 and struggling to make the freshly merged IPU6 support to work on it, I was about to send a call for help on the media ML, when I eventually spotted your observations on Redhat's bugzilla ([0]), and this corresponding patch. On Fri Nov 22, 2024 at 10:42 AM CET, Stanislaw Gruszka wrote: > When probing spi device take care of deferred probe of ACPI irq gpio > similar like for OF/DT case. > > From practical standpoint this fixes issue with vsc-tp driver on > Dell XP 9340 laptop, which try to request interrupt with spi->irq > equal to -EPROBE_DEFER and fail to probe with the following error: > > vsc-tp spi-INTC10D0:00: probe with driver vsc-tp failed with error -22 > > Suggested-by: Hans de Goede <hdegoede@redhat.com> > Fixes: 33ada67da352 ("ACPI / spi: attach GPIO IRQ from ACPI description to SPI device") > Cc: stable@vger.kernel.org > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> [...] I systematically observe this issue (probe failure with -22) on each boot, and reached the same intermediate conclusion (IRQ failing to register, and spi->irq value being -EPROBE_DEFER). I can confirm that this patch makes the vsc-tp -22 error disappear on my machine, and that I have now /sys/devices/platform/intel_vsc. Unfortunately, I now encounter a new issue preventing the camera to work (ipu6 still fails with -EPROBE_DEFER, I now have ipu_bridge_get_ivsc_csi_dev failing while searching for child device intel_vsc-92335fcf-3203-4472-af93-7b4453ac29da). I am not familiar enough neither with ACPI nor the exact pipeline involved to explain this yet, but if you can affirm that is is unrelated to your change, feel free to take my: Tested-by: Alexis Lothoré <alexis.lothore@bootlin.com> # Dell XPS9320, ov01a10 Thanks, [0] https://bugzilla.redhat.com/show_bug.cgi?id=2324683#c11
Hi Alexis, On 24-Nov-24 8:23 PM, Alexis Lothoré wrote: > Hello, > > owning a Dell XPS 9320 and struggling to make the freshly merged IPU6 > support to work on it, I was about to send a call for help on the media ML, > when I eventually spotted your observations on Redhat's bugzilla ([0]), and > this corresponding patch. > > On Fri Nov 22, 2024 at 10:42 AM CET, Stanislaw Gruszka wrote: >> When probing spi device take care of deferred probe of ACPI irq gpio >> similar like for OF/DT case. >> >> From practical standpoint this fixes issue with vsc-tp driver on >> Dell XP 9340 laptop, which try to request interrupt with spi->irq >> equal to -EPROBE_DEFER and fail to probe with the following error: >> >> vsc-tp spi-INTC10D0:00: probe with driver vsc-tp failed with error -22 >> >> Suggested-by: Hans de Goede <hdegoede@redhat.com> >> Fixes: 33ada67da352 ("ACPI / spi: attach GPIO IRQ from ACPI description to SPI device") >> Cc: stable@vger.kernel.org >> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> > > [...] > > I systematically observe this issue (probe failure with -22) on each boot, > and reached the same intermediate conclusion (IRQ failing to register, and > spi->irq value being -EPROBE_DEFER). > I can confirm that this patch makes the vsc-tp -22 error disappear on my > machine, and that I have now /sys/devices/platform/intel_vsc. > > Unfortunately, I now encounter a new issue preventing the camera to work > (ipu6 still fails with -EPROBE_DEFER, I now have > ipu_bridge_get_ivsc_csi_dev failing while searching for child device > intel_vsc-92335fcf-3203-4472-af93-7b4453ac29da). This sounds like you may not have the actual MEI driver enabled or that it is not binding. Do you have both CONFIG_INTEL_MEI_VSC_HW and CONFIG_INTEL_MEI_VSC enabled? And do you get a driver symlink under /sys/devices/platform/intel_vsc indicating that a driver has bound to it ? If not any related messages in dmesg ? If yes what is the output of: ls /sys/bus/mei/devices and of: ls -l /sys/bus/mei/devices/*/driver ? Regards, Hans
Hi Hans, thanks a lot for the the help ! On Mon Nov 25, 2024 at 11:05 AM CET, Hans de Goede wrote: > Hi Alexis, > > On 24-Nov-24 8:23 PM, Alexis Lothoré wrote: > > Hello, [...] > > I systematically observe this issue (probe failure with -22) on each boot, > > and reached the same intermediate conclusion (IRQ failing to register, and > > spi->irq value being -EPROBE_DEFER). > > I can confirm that this patch makes the vsc-tp -22 error disappear on my > > machine, and that I have now /sys/devices/platform/intel_vsc. > > > > Unfortunately, I now encounter a new issue preventing the camera to work > > (ipu6 still fails with -EPROBE_DEFER, I now have > > ipu_bridge_get_ivsc_csi_dev failing while searching for child device > > intel_vsc-92335fcf-3203-4472-af93-7b4453ac29da). > > This sounds like you may not have the actual MEI driver enabled or > that it is not binding. You were right, it looks like I have been missing CONFIG_INTEL_MEI_VSC. My config is comming from the archlinux kernel, there may be a miss here. > Do you have both CONFIG_INTEL_MEI_VSC_HW and CONFIG_INTEL_MEI_VSC enabled? So now with this change, I still have no success with ipu loading, because of new errors on vsc-tp, but those errors have actually changed: $ dmesg|grep vsc [ 8.594501] vsc-tp spi-INTC1094:00: wait rom failed ret: -110 [ 8.594506] intel_vsc intel_vsc: hw_reset failed ret = -110 [ 9.138269] vsc-tp spi-INTC1094:00: wait rom failed ret: -110 [ 9.138287] intel_vsc intel_vsc: hw_reset failed ret = -110 [ 9.678712] vsc-tp spi-INTC1094:00: wait rom failed ret: -110 [ 9.678729] intel_vsc intel_vsc: hw_reset failed ret = -110 [ 9.678750] intel_vsc intel_vsc: reset: reached maximal consecutive resets: disabling the device [ 9.678755] intel_vsc intel_vsc: reset failed ret = -19 [ 9.678758] intel_vsc intel_vsc: link layer initialization failed. [ 9.678761] intel_vsc intel_vsc: error -ENODEV: init hw failed I have seen some mentions of this -110 error in the many redhat bugzilla issues you have been helping with, I'll check more thoroughly if some hints and/or patches have emerged from there. For the record, I am doing my tests with the current Archlinux kernel (6.12.1-arch1), with those 3 patches on top: "mei: vsc: Do not re-enable interrupt from vsc_tp_reset()" "media: intel/ipu6: do not handle interrupts when device is disabled" "spi: Fix acpi deferred irq probe" > And do you get a driver symlink under /sys/devices/platform/intel_vsc > indicating that a driver has bound to it ? With the updated config: no, but I guess the dmesg output above explains it. > If not any related messages in dmesg ? > > If yes what is the output of: > > ls /sys/bus/mei/devices With the updated config: 0000:00:16.0-082ee5a7-7c25-470a-9643-0c06f0466ea1 -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-082ee5a7-7c25-470a-9643-0c06f0466ea1 0000:00:16.0-309dcde8-ccb1-4062-8f78-600115a34327 -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-309dcde8-ccb1-4062-8f78-600115a34327 0000:00:16.0-3c4852d6-d47b-4f46-b05e-b5edc1aa440e -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-3c4852d6-d47b-4f46-b05e-b5edc1aa440e 0000:00:16.0-42b3ce2f-bd9f-485a-96ae-26406230b1ff -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-42b3ce2f-bd9f-485a-96ae-26406230b1ff 0000:00:16.0-4fcc395c-a9e5-4647-bc68-47bad7cc6bd3 -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-4fcc395c-a9e5-4647-bc68-47bad7cc6bd3 0000:00:16.0-55213584-9a29-4916-badf-0fb7ed682aeb -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-55213584-9a29-4916-badf-0fb7ed682aeb 0000:00:16.0-5565a099-7fe2-45c1-a22b-d7e9dfea9a2e -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-5565a099-7fe2-45c1-a22b-d7e9dfea9a2e 0000:00:16.0-6861ec7b-d07a-4673-856c-7f22b4d55769 -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-6861ec7b-d07a-4673-856c-7f22b4d55769 0000:00:16.0-8c2f4425-77d6-4755-aca3-891fdbc66a58 -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-8c2f4425-77d6-4755-aca3-891fdbc66a58 0000:00:16.0-8e6a6715-9abc-4043-88ef-9e39c6f63e0f -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-8e6a6715-9abc-4043-88ef-9e39c6f63e0f 0000:00:16.0-b638ab7e-94e2-4ea2-a552-d1c54b627f04 -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-b638ab7e-94e2-4ea2-a552-d1c54b627f04 0000:00:16.0-cea154ea-8ff5-4f94-9290-0bb7355a34db -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-cea154ea-8ff5-4f94-9290-0bb7355a34db 0000:00:16.0-dba4d603-d7ed-4931-8823-17ad585705d5 -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-dba4d603-d7ed-4931-8823-17ad585705d5 0000:00:16.0-dd17041c-09ea-4b17-a271-5b989867ec65 -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-dd17041c-09ea-4b17-a271-5b989867ec65 0000:00:16.0-fbf6fcf1-96cf-4e2e-a6a6-1bab8cbe36b1 -> ../../../devices/pci0000:00/0000:00:16.0/0000:00:16.0-fbf6fcf1-96cf-4e2e-a6a6-1bab8cbe36b1 > > and of: > > ls -l /sys/bus/mei/devices/*/driver No driver bound to any MEI device Sorry for the thread hijack, that's totally fine for me to continue the discussions elsewhere if relevant. Thanks, Alexis
Hi, On 25-Nov-24 1:34 PM, Alexis Lothoré wrote: > Hi Hans, thanks a lot for the the help ! > > On Mon Nov 25, 2024 at 11:05 AM CET, Hans de Goede wrote: >> Hi Alexis, >> >> On 24-Nov-24 8:23 PM, Alexis Lothoré wrote: >>> Hello, > [...] > >>> I systematically observe this issue (probe failure with -22) on each boot, >>> and reached the same intermediate conclusion (IRQ failing to register, and >>> spi->irq value being -EPROBE_DEFER). >>> I can confirm that this patch makes the vsc-tp -22 error disappear on my >>> machine, and that I have now /sys/devices/platform/intel_vsc. >>> >>> Unfortunately, I now encounter a new issue preventing the camera to work >>> (ipu6 still fails with -EPROBE_DEFER, I now have >>> ipu_bridge_get_ivsc_csi_dev failing while searching for child device >>> intel_vsc-92335fcf-3203-4472-af93-7b4453ac29da). >> >> This sounds like you may not have the actual MEI driver enabled or >> that it is not binding. > > You were right, it looks like I have been missing CONFIG_INTEL_MEI_VSC. My > config is comming from the archlinux kernel, there may be a miss here. > >> Do you have both CONFIG_INTEL_MEI_VSC_HW and CONFIG_INTEL_MEI_VSC enabled? > > So now with this change, I still have no success with ipu loading, because > of new errors on vsc-tp, but those errors have actually changed: > > $ dmesg|grep vsc > [ 8.594501] vsc-tp spi-INTC1094:00: wait rom failed ret: -110 > [ 8.594506] intel_vsc intel_vsc: hw_reset failed ret = -110 > [ 9.138269] vsc-tp spi-INTC1094:00: wait rom failed ret: -110 > [ 9.138287] intel_vsc intel_vsc: hw_reset failed ret = -110 > [ 9.678712] vsc-tp spi-INTC1094:00: wait rom failed ret: -110 > [ 9.678729] intel_vsc intel_vsc: hw_reset failed ret = -110 > [ 9.678750] intel_vsc intel_vsc: reset: reached maximal consecutive resets: disabling the device > [ 9.678755] intel_vsc intel_vsc: reset failed ret = -19 > [ 9.678758] intel_vsc intel_vsc: link layer initialization failed. > [ 9.678761] intel_vsc intel_vsc: error -ENODEV: init hw failed > > I have seen some mentions of this -110 error in the many redhat bugzilla > issues you have been helping with, I'll check more thoroughly if some hints > and/or patches have emerged from there. Right. So this is a problem where the VSC chip is unresponsive which we still do not fully understand. power-cycling the laptop; or sometimes a power-cycle + if things still don't work a reboot after the power-cycle clears this. > For the record, I am doing my tests with the current Archlinux kernel > (6.12.1-arch1), with those 3 patches on top: > > "mei: vsc: Do not re-enable interrupt from vsc_tp_reset()" > "media: intel/ipu6: do not handle interrupts when device is disabled" > "spi: Fix acpi deferred irq probe" You may also want to throw these 2 into the mix. I've some hope that those will avoid the VSC chip getting stuck, requiring a power-cycle again (they do not help to "unstuck" the chip): https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=5c5d8eb8af06df615e8b1dc88e5847196c846045 https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=2481af79671a6603fce201cbbc48f31e488e9fae >> And do you get a driver symlink under /sys/devices/platform/intel_vsc >> indicating that a driver has bound to it ? > > With the updated config: no, but I guess the dmesg output above explains it. Right, the dmesg explains that. > Sorry for the thread hijack, that's totally fine for me to continue the > discussions elsewhere if relevant. I have changed the topic to reflect the new somewhat offtopic discussions. Another place to discuss the -110 error would be: https://bugzilla.redhat.com/show_bug.cgi?id=2316918 Regards, Hans
On 11/25/24 13:44, Hans de Goede wrote: > Hi, > > On 25-Nov-24 1:34 PM, Alexis Lothoré wrote: >> Hi Hans, thanks a lot for the the help ! >> >> On Mon Nov 25, 2024 at 11:05 AM CET, Hans de Goede wrote: >>> Hi Alexis, >>> >>> On 24-Nov-24 8:23 PM, Alexis Lothoré wrote: >>>> Hello, >> [...] >> >>>> I systematically observe this issue (probe failure with -22) on each boot, >>>> and reached the same intermediate conclusion (IRQ failing to register, and >>>> spi->irq value being -EPROBE_DEFER). >>>> I can confirm that this patch makes the vsc-tp -22 error disappear on my >>>> machine, and that I have now /sys/devices/platform/intel_vsc. >>>> >>>> Unfortunately, I now encounter a new issue preventing the camera to work >>>> (ipu6 still fails with -EPROBE_DEFER, I now have >>>> ipu_bridge_get_ivsc_csi_dev failing while searching for child device >>>> intel_vsc-92335fcf-3203-4472-af93-7b4453ac29da). >>> >>> This sounds like you may not have the actual MEI driver enabled or >>> that it is not binding. >> >> You were right, it looks like I have been missing CONFIG_INTEL_MEI_VSC. My >> config is comming from the archlinux kernel, there may be a miss here. >> >>> Do you have both CONFIG_INTEL_MEI_VSC_HW and CONFIG_INTEL_MEI_VSC enabled? >> >> So now with this change, I still have no success with ipu loading, because >> of new errors on vsc-tp, but those errors have actually changed: >> >> $ dmesg|grep vsc >> [ 8.594501] vsc-tp spi-INTC1094:00: wait rom failed ret: -110 >> [ 8.594506] intel_vsc intel_vsc: hw_reset failed ret = -110 >> [ 9.138269] vsc-tp spi-INTC1094:00: wait rom failed ret: -110 >> [ 9.138287] intel_vsc intel_vsc: hw_reset failed ret = -110 >> [ 9.678712] vsc-tp spi-INTC1094:00: wait rom failed ret: -110 >> [ 9.678729] intel_vsc intel_vsc: hw_reset failed ret = -110 >> [ 9.678750] intel_vsc intel_vsc: reset: reached maximal consecutive resets: disabling the device >> [ 9.678755] intel_vsc intel_vsc: reset failed ret = -19 >> [ 9.678758] intel_vsc intel_vsc: link layer initialization failed. >> [ 9.678761] intel_vsc intel_vsc: error -ENODEV: init hw failed >> >> I have seen some mentions of this -110 error in the many redhat bugzilla >> issues you have been helping with, I'll check more thoroughly if some hints >> and/or patches have emerged from there. > > Right. So this is a problem where the VSC chip is unresponsive which we > still do not fully understand. power-cycling the laptop; or sometimes > a power-cycle + if things still don't work a reboot after the power-cycle > clears this. > >> For the record, I am doing my tests with the current Archlinux kernel >> (6.12.1-arch1), with those 3 patches on top: >> >> "mei: vsc: Do not re-enable interrupt from vsc_tp_reset()" >> "media: intel/ipu6: do not handle interrupts when device is disabled" >> "spi: Fix acpi deferred irq probe" > > You may also want to throw these 2 into the mix. I've some hope that > those will avoid the VSC chip getting stuck, requiring a power-cycle > again (they do not help to "unstuck" the chip): > > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=5c5d8eb8af06df615e8b1dc88e5847196c846045 > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=2481af79671a6603fce201cbbc48f31e488e9fae ACK, I'll do the tests with those as well, and check if some power cycles help as well. > >>> And do you get a driver symlink under /sys/devices/platform/intel_vsc >>> indicating that a driver has bound to it ? >> >> With the updated config: no, but I guess the dmesg output above explains it. > > Right, the dmesg explains that. > >> Sorry for the thread hijack, that's totally fine for me to continue the >> discussions elsewhere if relevant. > > I have changed the topic to reflect the new somewhat offtopic discussions. > > Another place to discuss the -110 error would be: > > https://bugzilla.redhat.com/show_bug.cgi?id=2316918 ACK, I'll perform the tests mentioned above and report back to this issue. Many thanks again for your support ! Alexis
On Fri, 22 Nov 2024 10:42:24 +0100, Stanislaw Gruszka wrote: > When probing spi device take care of deferred probe of ACPI irq gpio > similar like for OF/DT case. > > >From practical standpoint this fixes issue with vsc-tp driver on > Dell XP 9340 laptop, which try to request interrupt with spi->irq > equal to -EPROBE_DEFER and fail to probe with the following error: > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: Fix acpi deferred irq probe commit: 9c69c8286754c61f95ea9189b2b5a794bdb07fed All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index c1dad30a4528..0f3e6e2c2474 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -424,6 +424,16 @@ static int spi_probe(struct device *dev) spi->irq = 0; } + if (has_acpi_companion(dev) && spi->irq < 0) { + struct acpi_device *adev = to_acpi_device_node(dev->fwnode); + + spi->irq = acpi_dev_gpio_irq_get(adev, 0); + if (spi->irq == -EPROBE_DEFER) + return -EPROBE_DEFER; + if (spi->irq < 0) + spi->irq = 0; + } + ret = dev_pm_domain_attach(dev, true); if (ret) return ret; @@ -2869,9 +2879,6 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr, acpi_set_modalias(adev, acpi_device_hid(adev), spi->modalias, sizeof(spi->modalias)); - if (spi->irq < 0) - spi->irq = acpi_dev_gpio_irq_get(adev, 0); - acpi_device_set_enumerated(adev); adev->power.flags.ignore_parent = true;
When probing spi device take care of deferred probe of ACPI irq gpio similar like for OF/DT case. From practical standpoint this fixes issue with vsc-tp driver on Dell XP 9340 laptop, which try to request interrupt with spi->irq equal to -EPROBE_DEFER and fail to probe with the following error: vsc-tp spi-INTC10D0:00: probe with driver vsc-tp failed with error -22 Suggested-by: Hans de Goede <hdegoede@redhat.com> Fixes: 33ada67da352 ("ACPI / spi: attach GPIO IRQ from ACPI description to SPI device") Cc: stable@vger.kernel.org Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> --- drivers/spi/spi.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)