Hi Thomas, On Tue, Apr 16, 2024 at 3:31 PM Thomas Richard <thomas.richard@xxxxxxxxxxx> wrote: > Some IOs can be needed during suspend_noirq()/resume_noirq(). > So move suspend()/resume() to noirq. > > Reviewed-by: Andi Shyti <andi.shyti@xxxxxxxxxx> > Acked-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx> > Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx> > Signed-off-by: Thomas Richard <thomas.richard@xxxxxxxxxxx> Thanks for your patch, which is now commit 86eb98127332748f ("gpio: pca953x: move suspend()/resume() to suspend_noirq()/resume_noirq()") in i2c-host/i2c/i2c-host. This patch causes regressions on e.g. Salvator-XS. s2idle: Freezing user space processes Freezing user space processes completed (elapsed 0.006 seconds) OOM killer disabled. Freezing remaining freezable tasks Freezing remaining freezable tasks completed (elapsed 0.003 seconds) sd 0:0:0:0: [sda] Synchronizing SCSI cache ata1.00: Entering standby power mode +i2c-rcar e66d8000.i2c: error -16 : 10000005 +pca953x 4-0020: Failed to sync GPIO dir registers: -16 +pca953x 4-0020: Failed to restore register map: -16 +pca953x 4-0020: PM: dpm_run_callback(): pca953x_resume_noirq returns -16 +pca953x 4-0020: PM: failed to resume async noirq: error -16 s2ram: Detected VIPT I-cache on CPU7 CPU7: Booted secondary processor 0x0000000103 [0x410fd034] CPU7 is up +i2c-rcar e66d8000.i2c: error -110 : 10000001 +pca953x 4-0020: Failed to sync GPIO dir registers: -110 +pca953x 4-0020: Failed to restore register map: -110 +pca953x 4-0020: PM: dpm_run_callback(): pca953x_resume_noirq returns -110 +pca953x 4-0020: PM: failed to resume async noirq: error -110 usb usb1: root hub lost power or was reset ... PM: suspend exit ata1: link resume succeeded after 1 retries -ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300) -sd 0:0:0:0: [sda] Starting disk -ata1.00: configured for UDMA/133 -ata1.00: Entering active power mode +ata1: SATA link down (SStatus 0 SControl 300) +ata1: link resume succeeded after 1 retries +ata1: SATA link down (SStatus 0 SControl 300) +ata1: limiting SATA link speed to <unknown> +ata1: link resume succeeded after 1 retries +ata1: SATA link down (SStatus 0 SControl 3F0) +ata1.00: disable device +ata1.00: detaching (SCSI 0:0:0:0) +sd 0:0:0:0: [sda] Synchronizing SCSI cache +sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result: hostbyte=0x04 driverbyte=DRIVER_OK When trying to read from /dev/sda afterwards: ata1: link resume succeeded after 1 retries ata1: SATA link down (SStatus 0 SControl 3F0) ata1.00: disable device ata1.00: detaching (SCSI 0:0:0:0) device offline error, dev sda, sector 0 op 0x0:(READ) flags 0x80700 phys_seg 4 prio class 0 device offline error, dev sda, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0 Buffer I/O error on dev sda, logical block 0, async page read sd 0:0:0:0: [sda] Synchronizing SCSI cache sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result: hostbyte=0x04 driverbyte=DRIVER_OK All issues above are fixed by reverting this commit. > --- a/drivers/gpio/gpio-pca953x.c > +++ b/drivers/gpio/gpio-pca953x.c > @@ -1234,7 +1234,7 @@ static void pca953x_save_context(struct pca953x_chip *chip) > regcache_cache_only(chip->regmap, true); > } > > -static int pca953x_suspend(struct device *dev) > +static int pca953x_suspend_noirq(struct device *dev) > { > struct pca953x_chip *chip = dev_get_drvdata(dev); > > @@ -1248,7 +1248,7 @@ static int pca953x_suspend(struct device *dev) > return 0; > } > > -static int pca953x_resume(struct device *dev) > +static int pca953x_resume_noirq(struct device *dev) > { > struct pca953x_chip *chip = dev_get_drvdata(dev); > int ret; > @@ -1268,7 +1268,8 @@ static int pca953x_resume(struct device *dev) > return ret; > } > > -static DEFINE_SIMPLE_DEV_PM_OPS(pca953x_pm_ops, pca953x_suspend, pca953x_resume); > +static DEFINE_NOIRQ_DEV_PM_OPS(pca953x_pm_ops, > + pca953x_suspend_noirq, pca953x_resume_noirq); > > /* convenience to stop overlong match-table lines */ > #define OF_653X(__nrgpio, __int) ((void *)(__nrgpio | PCAL653X_TYPE | __int)) > Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds