* [PATCH] platform/x86: intel_int0002_vgpio: Implement irq_set_wake @ 2018-09-24 14:37 Hans de Goede 2018-09-26 9:34 ` Andy Shevchenko 2019-01-31 19:47 ` Maxim Mikityanskiy 0 siblings, 2 replies; 8+ messages in thread From: Hans de Goede @ 2018-09-24 14:37 UTC (permalink / raw) To: Darren Hart, Andy Shevchenko Cc: Hans de Goede, platform-driver-x86, linux-kernel We were relying on the interrupt being shared with the ACPI SCI and the ACPI core calling irq_set_wake. But that does not always happen on Bay Trail devices, so we should do it ourselves. This fixes wake from USB not working on various Bay Trail devices. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/platform/x86/intel_int0002_vgpio.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/platform/x86/intel_int0002_vgpio.c b/drivers/platform/x86/intel_int0002_vgpio.c index 987a3b03f225..33c3489f5bc1 100644 --- a/drivers/platform/x86/intel_int0002_vgpio.c +++ b/drivers/platform/x86/intel_int0002_vgpio.c @@ -106,6 +106,21 @@ static void int0002_irq_mask(struct irq_data *data) outl(gpe_en_reg, GPE0A_EN_PORT); } +static int int0002_irq_set_wake(struct irq_data *data, unsigned int on) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + struct platform_device *pdev = to_platform_device(chip->parent); + int irq = platform_get_irq(pdev, 0); + + /* Propagate to parent irq */ + if (on) + enable_irq_wake(irq); + else + disable_irq_wake(irq); + + return 0; +} + static irqreturn_t int0002_irq(int irq, void *data) { struct gpio_chip *chip = data; @@ -128,6 +143,7 @@ static struct irq_chip int0002_irqchip = { .irq_ack = int0002_irq_ack, .irq_mask = int0002_irq_mask, .irq_unmask = int0002_irq_unmask, + .irq_set_wake = int0002_irq_set_wake, }; static int int0002_probe(struct platform_device *pdev) -- 2.19.0.rc1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] platform/x86: intel_int0002_vgpio: Implement irq_set_wake 2018-09-24 14:37 [PATCH] platform/x86: intel_int0002_vgpio: Implement irq_set_wake Hans de Goede @ 2018-09-26 9:34 ` Andy Shevchenko 2019-01-31 19:47 ` Maxim Mikityanskiy 1 sibling, 0 replies; 8+ messages in thread From: Andy Shevchenko @ 2018-09-26 9:34 UTC (permalink / raw) To: Hans de Goede Cc: Darren Hart, Andy Shevchenko, Platform Driver, Linux Kernel Mailing List On Mon, Sep 24, 2018 at 5:37 PM Hans de Goede <hdegoede@redhat.com> wrote: > > We were relying on the interrupt being shared with the ACPI SCI and the > ACPI core calling irq_set_wake. But that does not always happen on > Bay Trail devices, so we should do it ourselves. > > This fixes wake from USB not working on various Bay Trail devices. > Pushed to my review and testing queue, thanks! > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/platform/x86/intel_int0002_vgpio.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/platform/x86/intel_int0002_vgpio.c b/drivers/platform/x86/intel_int0002_vgpio.c > index 987a3b03f225..33c3489f5bc1 100644 > --- a/drivers/platform/x86/intel_int0002_vgpio.c > +++ b/drivers/platform/x86/intel_int0002_vgpio.c > @@ -106,6 +106,21 @@ static void int0002_irq_mask(struct irq_data *data) > outl(gpe_en_reg, GPE0A_EN_PORT); > } > > +static int int0002_irq_set_wake(struct irq_data *data, unsigned int on) > +{ > + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); > + struct platform_device *pdev = to_platform_device(chip->parent); > + int irq = platform_get_irq(pdev, 0); > + > + /* Propagate to parent irq */ > + if (on) > + enable_irq_wake(irq); > + else > + disable_irq_wake(irq); > + > + return 0; > +} > + > static irqreturn_t int0002_irq(int irq, void *data) > { > struct gpio_chip *chip = data; > @@ -128,6 +143,7 @@ static struct irq_chip int0002_irqchip = { > .irq_ack = int0002_irq_ack, > .irq_mask = int0002_irq_mask, > .irq_unmask = int0002_irq_unmask, > + .irq_set_wake = int0002_irq_set_wake, > }; > > static int int0002_probe(struct platform_device *pdev) > -- > 2.19.0.rc1 > -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] platform/x86: intel_int0002_vgpio: Implement irq_set_wake 2018-09-24 14:37 [PATCH] platform/x86: intel_int0002_vgpio: Implement irq_set_wake Hans de Goede 2018-09-26 9:34 ` Andy Shevchenko @ 2019-01-31 19:47 ` Maxim Mikityanskiy 2019-02-01 22:52 ` Hans de Goede 1 sibling, 1 reply; 8+ messages in thread From: Maxim Mikityanskiy @ 2019-01-31 19:47 UTC (permalink / raw) To: Hans de Goede, Andy Shevchenko Cc: Darren Hart, platform-driver-x86, linux-kernel Hi, On Mon, Sep 24, 2018 at 5:37 PM Hans de Goede <hdegoede@redhat.com> wrote: > > We were relying on the interrupt being shared with the ACPI SCI and the > ACPI core calling irq_set_wake. But that does not always happen on > Bay Trail devices, so we should do it ourselves. > > This fixes wake from USB not working on various Bay Trail devices. This patch breaks suspend on ASUS E202SA (bisecting pointed me to this patch, and if I revert it and build 4.20.5 without this patch, everything works flawlessly). This command fails with the following message: # echo mem > /sys/power/state Error while writing to stdout write_loop: Device or resource busy And here is dmesg output: [ 224.077275] PM: suspend entry (deep) [ 224.077286] PM: Syncing filesystems ... done. [ 225.495014] Freezing user space processes ... (elapsed 0.003 seconds) done. [ 225.498540] OOM killer disabled. [ 225.498543] Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done. [ 225.500693] printk: Suspending console(s) (use no_console_suspend to debug) [ 225.502793] wlp1s0: deauthenticating from 00:03:7f:12:34:56 by local choice (Reason: 3=DEAUTH_LEAVING) [ 225.535333] sd 0:0:0:0: [sda] Synchronizing SCSI cache [ 225.535882] sd 0:0:0:0: [sda] Stopping disk [ 226.969070] ACPI: EC: interrupt blocked [ 227.002156] ACPI: Preparing to enter system sleep state S3 [ 227.007890] ACPI: EC: event blocked [ 227.007895] ACPI: EC: EC stopped [ 227.007900] PM: Saving platform NVS memory [ 227.008264] Disabling non-boot CPUs ... [ 227.034114] smpboot: CPU 1 is now offline [ 227.088320] smpboot: CPU 2 is now offline [ 227.141513] smpboot: CPU 3 is now offline [ 227.147086] Enabling non-boot CPUs ... [ 227.147187] x86: Booting SMP configuration: [ 227.147190] smpboot: Booting Node 0 Processor 1 APIC 0x2 [ 227.147916] cache: parent cpu1 should not be sleeping [ 227.148354] CPU1 is up [ 227.148424] smpboot: Booting Node 0 Processor 2 APIC 0x4 [ 227.149800] cache: parent cpu2 should not be sleeping [ 227.151143] CPU2 is up [ 227.151187] smpboot: Booting Node 0 Processor 3 APIC 0x6 [ 227.152399] cache: parent cpu3 should not be sleeping [ 227.153883] CPU3 is up [ 227.154876] ACPI: EC: EC started [ 227.155282] ACPI: Waking up from system sleep state S3 [ 227.159874] ACPI: button: The lid device is not compliant to SW_LID. [ 227.169441] ACPI: EC: interrupt unblocked [ 228.236790] ACPI: EC: event unblocked [ 228.241950] rtlwifi: rtlwifi: wireless switch is on [ 228.251865] sd 0:0:0:0: [sda] Starting disk [ 228.476637] usb 1-4: reset full-speed USB device number 2 using xhci_hcd [ 228.562879] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300) [ 228.563924] ata2: SATA link down (SStatus 4 SControl 300) [ 228.564979] ata1.00: supports DRM functions and may not be fully accessible [ 228.565493] ata1.00: NCQ Send/Recv Log not supported [ 228.567649] ata1.00: supports DRM functions and may not be fully accessible [ 228.568252] ata1.00: NCQ Send/Recv Log not supported [ 228.570075] ata1.00: configured for UDMA/133 [ 228.580412] ahci 0000:00:13.0: port does not support device sleep [ 228.639349] Bluetooth: hci0: RTL: rtl: examining hci_ver=06 hci_rev=0e2f lmp_ver=06 lmp_subver=a041 [ 228.639368] Bluetooth: hci0: RTL: rtl: unknown IC info, lmp subver a041, hci rev 0e2f, hci ver 0006 [ 228.639742] acpi LNXPOWER:01: Turning OFF [ 228.640033] OOM killer enabled. [ 228.640040] Restarting tasks ... done. [ 228.795406] PM: suspend exit [ 228.800399] audit: type=1130 audit(1548962671.104:94): pid=1 uid=0 auid=4294967295 ses=4294967295 msg='unit=systemd-rfkill comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? res=success' [ 229.067206] wlp1s0: authenticate with 12:34:56:78:90:12 [ 229.067823] wlp1s0: send auth to 12:34:56:78:90:12 (try 1/3) [ 229.070955] wlp1s0: authenticated [ 229.072395] wlp1s0: associate with 12:34:56:78:90:12 (try 1/3) [ 229.074505] wlp1s0: RX AssocResp from 12:34:56:78:90:12 (capab=0x11 status=0 aid=2) [ 229.074819] wlp1s0: associated [ 233.809200] audit: type=1131 audit(1548962676.106:95): pid=1 uid=0 auid=4294967295 ses=4294967295 msg='unit=systemd-rfkill comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? res=success' The laptop doesn't go to sleep, the screen turns off, and in a couple of seconds it turns on. Please take a look at this regression. Feel free to ask me for any additional information you need. Thanks, Max > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/platform/x86/intel_int0002_vgpio.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/platform/x86/intel_int0002_vgpio.c b/drivers/platform/x86/intel_int0002_vgpio.c > index 987a3b03f225..33c3489f5bc1 100644 > --- a/drivers/platform/x86/intel_int0002_vgpio.c > +++ b/drivers/platform/x86/intel_int0002_vgpio.c > @@ -106,6 +106,21 @@ static void int0002_irq_mask(struct irq_data *data) > outl(gpe_en_reg, GPE0A_EN_PORT); > } > > +static int int0002_irq_set_wake(struct irq_data *data, unsigned int on) > +{ > + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); > + struct platform_device *pdev = to_platform_device(chip->parent); > + int irq = platform_get_irq(pdev, 0); > + > + /* Propagate to parent irq */ > + if (on) > + enable_irq_wake(irq); > + else > + disable_irq_wake(irq); > + > + return 0; > +} > + > static irqreturn_t int0002_irq(int irq, void *data) > { > struct gpio_chip *chip = data; > @@ -128,6 +143,7 @@ static struct irq_chip int0002_irqchip = { > .irq_ack = int0002_irq_ack, > .irq_mask = int0002_irq_mask, > .irq_unmask = int0002_irq_unmask, > + .irq_set_wake = int0002_irq_set_wake, > }; > > static int int0002_probe(struct platform_device *pdev) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] platform/x86: intel_int0002_vgpio: Implement irq_set_wake 2019-01-31 19:47 ` Maxim Mikityanskiy @ 2019-02-01 22:52 ` Hans de Goede 2019-02-01 23:25 ` Andy Shevchenko 2019-02-02 19:04 ` Maxim Mikityanskiy 0 siblings, 2 replies; 8+ messages in thread From: Hans de Goede @ 2019-02-01 22:52 UTC (permalink / raw) To: Maxim Mikityanskiy, Andy Shevchenko Cc: Darren Hart, platform-driver-x86, linux-kernel [-- Attachment #1: Type: text/plain, Size: 6444 bytes --] Hi, On 1/31/19 8:47 PM, Maxim Mikityanskiy wrote: > Hi, > > On Mon, Sep 24, 2018 at 5:37 PM Hans de Goede <hdegoede@redhat.com> wrote: >> >> We were relying on the interrupt being shared with the ACPI SCI and the >> ACPI core calling irq_set_wake. But that does not always happen on >> Bay Trail devices, so we should do it ourselves. >> >> This fixes wake from USB not working on various Bay Trail devices. > > This patch breaks suspend on ASUS E202SA (bisecting pointed me to this > patch, and if I revert it and build 4.20.5 without this patch, > everything works flawlessly). Thank you for the bug report, can you please test 4.20.5 with the attached patch on top? That should fix it. Once I've confirmation that this fixes things I will submit the patch upstream. Regards, Hans > > This command fails with the following message: > > # echo mem > /sys/power/state > Error while writing to stdout > write_loop: Device or resource busy > > And here is dmesg output: > > [ 224.077275] PM: suspend entry (deep) > [ 224.077286] PM: Syncing filesystems ... done. > [ 225.495014] Freezing user space processes ... (elapsed 0.003 seconds) done. > [ 225.498540] OOM killer disabled. > [ 225.498543] Freezing remaining freezable tasks ... (elapsed 0.002 > seconds) done. > [ 225.500693] printk: Suspending console(s) (use no_console_suspend to debug) > [ 225.502793] wlp1s0: deauthenticating from 00:03:7f:12:34:56 by > local choice (Reason: 3=DEAUTH_LEAVING) > [ 225.535333] sd 0:0:0:0: [sda] Synchronizing SCSI cache > [ 225.535882] sd 0:0:0:0: [sda] Stopping disk > [ 226.969070] ACPI: EC: interrupt blocked > [ 227.002156] ACPI: Preparing to enter system sleep state S3 > [ 227.007890] ACPI: EC: event blocked > [ 227.007895] ACPI: EC: EC stopped > [ 227.007900] PM: Saving platform NVS memory > [ 227.008264] Disabling non-boot CPUs ... > [ 227.034114] smpboot: CPU 1 is now offline > [ 227.088320] smpboot: CPU 2 is now offline > [ 227.141513] smpboot: CPU 3 is now offline > [ 227.147086] Enabling non-boot CPUs ... > [ 227.147187] x86: Booting SMP configuration: > [ 227.147190] smpboot: Booting Node 0 Processor 1 APIC 0x2 > [ 227.147916] cache: parent cpu1 should not be sleeping > [ 227.148354] CPU1 is up > [ 227.148424] smpboot: Booting Node 0 Processor 2 APIC 0x4 > [ 227.149800] cache: parent cpu2 should not be sleeping > [ 227.151143] CPU2 is up > [ 227.151187] smpboot: Booting Node 0 Processor 3 APIC 0x6 > [ 227.152399] cache: parent cpu3 should not be sleeping > [ 227.153883] CPU3 is up > [ 227.154876] ACPI: EC: EC started > [ 227.155282] ACPI: Waking up from system sleep state S3 > [ 227.159874] ACPI: button: The lid device is not compliant to SW_LID. > [ 227.169441] ACPI: EC: interrupt unblocked > [ 228.236790] ACPI: EC: event unblocked > [ 228.241950] rtlwifi: rtlwifi: wireless switch is on > [ 228.251865] sd 0:0:0:0: [sda] Starting disk > [ 228.476637] usb 1-4: reset full-speed USB device number 2 using xhci_hcd > [ 228.562879] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300) > [ 228.563924] ata2: SATA link down (SStatus 4 SControl 300) > [ 228.564979] ata1.00: supports DRM functions and may not be fully accessible > [ 228.565493] ata1.00: NCQ Send/Recv Log not supported > [ 228.567649] ata1.00: supports DRM functions and may not be fully accessible > [ 228.568252] ata1.00: NCQ Send/Recv Log not supported > [ 228.570075] ata1.00: configured for UDMA/133 > [ 228.580412] ahci 0000:00:13.0: port does not support device sleep > [ 228.639349] Bluetooth: hci0: RTL: rtl: examining hci_ver=06 > hci_rev=0e2f lmp_ver=06 lmp_subver=a041 > > [ 228.639368] Bluetooth: hci0: RTL: rtl: unknown IC info, lmp subver > a041, hci rev 0e2f, hci ver 0006 > [ 228.639742] acpi LNXPOWER:01: Turning OFF > [ 228.640033] OOM killer enabled. > [ 228.640040] Restarting tasks ... done. > [ 228.795406] PM: suspend exit > [ 228.800399] audit: type=1130 audit(1548962671.104:94): pid=1 uid=0 > auid=4294967295 ses=4294967295 msg='unit=systemd-rfkill comm="systemd" > exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? > res=success' > [ 229.067206] wlp1s0: authenticate with 12:34:56:78:90:12 > [ 229.067823] wlp1s0: send auth to 12:34:56:78:90:12 (try 1/3) > [ 229.070955] wlp1s0: authenticated > [ 229.072395] wlp1s0: associate with 12:34:56:78:90:12 (try 1/3) > [ 229.074505] wlp1s0: RX AssocResp from 12:34:56:78:90:12 (capab=0x11 > status=0 aid=2) > [ 229.074819] wlp1s0: associated > [ 233.809200] audit: type=1131 audit(1548962676.106:95): pid=1 uid=0 > auid=4294967295 ses=4294967295 msg='unit=systemd-rfkill comm="systemd" > exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? > res=success' > > The laptop doesn't go to sleep, the screen turns off, and in a couple > of seconds it turns on. > > Please take a look at this regression. Feel free to ask me for any > additional information you need. > > Thanks, > Max > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/platform/x86/intel_int0002_vgpio.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/drivers/platform/x86/intel_int0002_vgpio.c b/drivers/platform/x86/intel_int0002_vgpio.c >> index 987a3b03f225..33c3489f5bc1 100644 >> --- a/drivers/platform/x86/intel_int0002_vgpio.c >> +++ b/drivers/platform/x86/intel_int0002_vgpio.c >> @@ -106,6 +106,21 @@ static void int0002_irq_mask(struct irq_data *data) >> outl(gpe_en_reg, GPE0A_EN_PORT); >> } >> >> +static int int0002_irq_set_wake(struct irq_data *data, unsigned int on) >> +{ >> + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); >> + struct platform_device *pdev = to_platform_device(chip->parent); >> + int irq = platform_get_irq(pdev, 0); >> + >> + /* Propagate to parent irq */ >> + if (on) >> + enable_irq_wake(irq); >> + else >> + disable_irq_wake(irq); >> + >> + return 0; >> +} >> + >> static irqreturn_t int0002_irq(int irq, void *data) >> { >> struct gpio_chip *chip = data; >> @@ -128,6 +143,7 @@ static struct irq_chip int0002_irqchip = { >> .irq_ack = int0002_irq_ack, >> .irq_mask = int0002_irq_mask, >> .irq_unmask = int0002_irq_unmask, >> + .irq_set_wake = int0002_irq_set_wake, >> }; >> >> static int int0002_probe(struct platform_device *pdev) [-- Attachment #2: 0001-platform-x86-intel_int0002_vgpio-Only-implement-irq_.patch --] [-- Type: text/x-patch, Size: 3379 bytes --] From 0ac08d4ec8dc741777fdb58b8359e0546baab6b6 Mon Sep 17 00:00:00 2001 From: Hans de Goede <hdegoede@redhat.com> Date: Fri, 1 Feb 2019 18:05:00 +0100 Subject: [PATCH] platform/x86: intel_int0002_vgpio: Only implement irq_set_wake on Bay Trail Commit c3b8e884defa ("platform/x86: intel_int0002_vgpio: Implement irq_set_wake"), was written to fix some wakeup issues on Bay Trail (BYT) devices. We've received a bug report that this causes a suspend regression on some Cherry Trail (CHT) based devices. To fix the issues this causes on CHT devices, this commit modifies the irq_set_wake support so that we only implement irq_set_wake on BYT devices, Fixes: c3b8e884defa ("platform/x86: intel_int0002_vgpio: ... irq_set_wake") Reported-and-tested-by: Maxim Mikityanskiy <maxtram95@gmail.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/platform/x86/intel_int0002_vgpio.c | 32 ++++++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/drivers/platform/x86/intel_int0002_vgpio.c b/drivers/platform/x86/intel_int0002_vgpio.c index 4b8f7305fc8a..78787934b572 100644 --- a/drivers/platform/x86/intel_int0002_vgpio.c +++ b/drivers/platform/x86/intel_int0002_vgpio.c @@ -51,11 +51,14 @@ #define GPE0A_STS_PORT 0x420 #define GPE0A_EN_PORT 0x428 -#define ICPU(model) { X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, } +#define BAYTRAIL 0x01 +#define CHERRYTRAIL 0x02 + +#define ICPU(model, data) { X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, data} static const struct x86_cpu_id int0002_cpu_ids[] = { - ICPU(INTEL_FAM6_ATOM_SILVERMONT), /* Valleyview, Bay Trail */ - ICPU(INTEL_FAM6_ATOM_AIRMONT), /* Braswell, Cherry Trail */ + ICPU(INTEL_FAM6_ATOM_SILVERMONT, BAYTRAIL), /* Valleyview, Bay Trail */ + ICPU(INTEL_FAM6_ATOM_AIRMONT, CHERRYTRAIL), /* Braswell, Cherry Trail */ {} }; @@ -135,7 +138,7 @@ static irqreturn_t int0002_irq(int irq, void *data) return IRQ_HANDLED; } -static struct irq_chip int0002_irqchip = { +static struct irq_chip int0002_byt_irqchip = { .name = DRV_NAME, .irq_ack = int0002_irq_ack, .irq_mask = int0002_irq_mask, @@ -143,10 +146,22 @@ static struct irq_chip int0002_irqchip = { .irq_set_wake = int0002_irq_set_wake, }; +static struct irq_chip int0002_cht_irqchip = { + .name = DRV_NAME, + .irq_ack = int0002_irq_ack, + .irq_mask = int0002_irq_mask, + .irq_unmask = int0002_irq_unmask, + /* + * No set_wake, on CHT the IRQ is typically shared with the ACPI SCI + * and we don't want to mess with the ACPI SCI irq settings. + */ +}; + static int int0002_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; const struct x86_cpu_id *cpu_id; + struct irq_chip *irq_chip; struct gpio_chip *chip; int irq, ret; @@ -195,14 +210,19 @@ static int int0002_probe(struct platform_device *pdev) return ret; } - ret = gpiochip_irqchip_add(chip, &int0002_irqchip, 0, handle_edge_irq, + if (cpu_id->driver_data == BAYTRAIL) + irq_chip = &int0002_byt_irqchip; + else + irq_chip = &int0002_cht_irqchip; + + ret = gpiochip_irqchip_add(chip, irq_chip, 0, handle_edge_irq, IRQ_TYPE_NONE); if (ret) { dev_err(dev, "Error adding irqchip: %d\n", ret); return ret; } - gpiochip_set_chained_irqchip(chip, &int0002_irqchip, irq, NULL); + gpiochip_set_chained_irqchip(chip, irq_chip, irq, NULL); return 0; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] platform/x86: intel_int0002_vgpio: Implement irq_set_wake 2019-02-01 22:52 ` Hans de Goede @ 2019-02-01 23:25 ` Andy Shevchenko 2019-02-02 10:50 ` Hans de Goede 2019-02-02 19:04 ` Maxim Mikityanskiy 1 sibling, 1 reply; 8+ messages in thread From: Andy Shevchenko @ 2019-02-01 23:25 UTC (permalink / raw) To: Hans de Goede Cc: Maxim Mikityanskiy, Andy Shevchenko, Darren Hart, Platform Driver, Linux Kernel Mailing List On Sat, Feb 2, 2019 at 12:52 AM Hans de Goede <hdegoede@redhat.com> wrote: > On 1/31/19 8:47 PM, Maxim Mikityanskiy wrote: > > On Mon, Sep 24, 2018 at 5:37 PM Hans de Goede <hdegoede@redhat.com> wrote: > >> > >> We were relying on the interrupt being shared with the ACPI SCI and the > >> ACPI core calling irq_set_wake. But that does not always happen on > >> Bay Trail devices, so we should do it ourselves. > >> > >> This fixes wake from USB not working on various Bay Trail devices. > > > > This patch breaks suspend on ASUS E202SA (bisecting pointed me to this > > patch, and if I revert it and build 4.20.5 without this patch, > > everything works flawlessly). > > Thank you for the bug report, can you please test 4.20.5 with the attached > patch on top? That should fix it. Once I've confirmation that this fixes > things I will submit the patch upstream. Thanks for fast reply, Hans! I looked at the patch and would propose one modification, i.e. use INTEL_CPU_FAM6() macro instead of ICPU(). -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] platform/x86: intel_int0002_vgpio: Implement irq_set_wake 2019-02-01 23:25 ` Andy Shevchenko @ 2019-02-02 10:50 ` Hans de Goede 0 siblings, 0 replies; 8+ messages in thread From: Hans de Goede @ 2019-02-02 10:50 UTC (permalink / raw) To: Andy Shevchenko Cc: Maxim Mikityanskiy, Andy Shevchenko, Darren Hart, Platform Driver, Linux Kernel Mailing List Hi, On 2/2/19 12:25 AM, Andy Shevchenko wrote: > On Sat, Feb 2, 2019 at 12:52 AM Hans de Goede <hdegoede@redhat.com> wrote: >> On 1/31/19 8:47 PM, Maxim Mikityanskiy wrote: >>> On Mon, Sep 24, 2018 at 5:37 PM Hans de Goede <hdegoede@redhat.com> wrote: >>>> >>>> We were relying on the interrupt being shared with the ACPI SCI and the >>>> ACPI core calling irq_set_wake. But that does not always happen on >>>> Bay Trail devices, so we should do it ourselves. >>>> >>>> This fixes wake from USB not working on various Bay Trail devices. >>> >>> This patch breaks suspend on ASUS E202SA (bisecting pointed me to this >>> patch, and if I revert it and build 4.20.5 without this patch, >>> everything works flawlessly). >> >> Thank you for the bug report, can you please test 4.20.5 with the attached >> patch on top? That should fix it. Once I've confirmation that this fixes >> things I will submit the patch upstream. > > Thanks for fast reply, Hans! > > I looked at the patch and would propose one modification, i.e. use > INTEL_CPU_FAM6() macro instead of ICPU(). I just tried and I like the idea, but this does not work. The problem is that macro assumes that the driver_data being passed is always a struct and the & to take the address of the struct is part of the macro instead of passed with the argument, making it impossible to pass a simple integer value or a bunch of flags (in a bitmask). IMHO this is a shortcoming of the macro and should be fixed, moving the & to the callers of the macro. Given that this patch needs to go to stable to fix the regression I think it is best to just keep the patch as is. And then for -next, once the macro is fixed we can convert the int0002_vgpio driver to use it. Regards, Hans ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] platform/x86: intel_int0002_vgpio: Implement irq_set_wake 2019-02-01 22:52 ` Hans de Goede 2019-02-01 23:25 ` Andy Shevchenko @ 2019-02-02 19:04 ` Maxim Mikityanskiy 2019-02-03 9:43 ` Hans de Goede 1 sibling, 1 reply; 8+ messages in thread From: Maxim Mikityanskiy @ 2019-02-02 19:04 UTC (permalink / raw) To: Hans de Goede Cc: Andy Shevchenko, Darren Hart, platform-driver-x86, linux-kernel On Sat, Feb 2, 2019 at 12:52 AM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi, > > On 1/31/19 8:47 PM, Maxim Mikityanskiy wrote: > > Hi, > > > > On Mon, Sep 24, 2018 at 5:37 PM Hans de Goede <hdegoede@redhat.com> wrote: > >> > >> We were relying on the interrupt being shared with the ACPI SCI and the > >> ACPI core calling irq_set_wake. But that does not always happen on > >> Bay Trail devices, so we should do it ourselves. > >> > >> This fixes wake from USB not working on various Bay Trail devices. > > > > This patch breaks suspend on ASUS E202SA (bisecting pointed me to this > > patch, and if I revert it and build 4.20.5 without this patch, > > everything works flawlessly). > > Thank you for the bug report, can you please test 4.20.5 with the attached > patch on top? That should fix it. Once I've confirmation that this fixes > things I will submit the patch upstream. I've tested your patch against both 4.20.5 and 4.20.6, and it works fine. Thank you for such a quick fix. > Regards, > > Hans > > > > > > This command fails with the following message: > > > > # echo mem > /sys/power/state > > Error while writing to stdout > > write_loop: Device or resource busy > > > > And here is dmesg output: > > > > [ 224.077275] PM: suspend entry (deep) > > [ 224.077286] PM: Syncing filesystems ... done. > > [ 225.495014] Freezing user space processes ... (elapsed 0.003 seconds) done. > > [ 225.498540] OOM killer disabled. > > [ 225.498543] Freezing remaining freezable tasks ... (elapsed 0.002 > > seconds) done. > > [ 225.500693] printk: Suspending console(s) (use no_console_suspend to debug) > > [ 225.502793] wlp1s0: deauthenticating from 00:03:7f:12:34:56 by > > local choice (Reason: 3=DEAUTH_LEAVING) > > [ 225.535333] sd 0:0:0:0: [sda] Synchronizing SCSI cache > > [ 225.535882] sd 0:0:0:0: [sda] Stopping disk > > [ 226.969070] ACPI: EC: interrupt blocked > > [ 227.002156] ACPI: Preparing to enter system sleep state S3 > > [ 227.007890] ACPI: EC: event blocked > > [ 227.007895] ACPI: EC: EC stopped > > [ 227.007900] PM: Saving platform NVS memory > > [ 227.008264] Disabling non-boot CPUs ... > > [ 227.034114] smpboot: CPU 1 is now offline > > [ 227.088320] smpboot: CPU 2 is now offline > > [ 227.141513] smpboot: CPU 3 is now offline > > [ 227.147086] Enabling non-boot CPUs ... > > [ 227.147187] x86: Booting SMP configuration: > > [ 227.147190] smpboot: Booting Node 0 Processor 1 APIC 0x2 > > [ 227.147916] cache: parent cpu1 should not be sleeping > > [ 227.148354] CPU1 is up > > [ 227.148424] smpboot: Booting Node 0 Processor 2 APIC 0x4 > > [ 227.149800] cache: parent cpu2 should not be sleeping > > [ 227.151143] CPU2 is up > > [ 227.151187] smpboot: Booting Node 0 Processor 3 APIC 0x6 > > [ 227.152399] cache: parent cpu3 should not be sleeping > > [ 227.153883] CPU3 is up > > [ 227.154876] ACPI: EC: EC started > > [ 227.155282] ACPI: Waking up from system sleep state S3 > > [ 227.159874] ACPI: button: The lid device is not compliant to SW_LID. > > [ 227.169441] ACPI: EC: interrupt unblocked > > [ 228.236790] ACPI: EC: event unblocked > > [ 228.241950] rtlwifi: rtlwifi: wireless switch is on > > [ 228.251865] sd 0:0:0:0: [sda] Starting disk > > [ 228.476637] usb 1-4: reset full-speed USB device number 2 using xhci_hcd > > [ 228.562879] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300) > > [ 228.563924] ata2: SATA link down (SStatus 4 SControl 300) > > [ 228.564979] ata1.00: supports DRM functions and may not be fully accessible > > [ 228.565493] ata1.00: NCQ Send/Recv Log not supported > > [ 228.567649] ata1.00: supports DRM functions and may not be fully accessible > > [ 228.568252] ata1.00: NCQ Send/Recv Log not supported > > [ 228.570075] ata1.00: configured for UDMA/133 > > [ 228.580412] ahci 0000:00:13.0: port does not support device sleep > > [ 228.639349] Bluetooth: hci0: RTL: rtl: examining hci_ver=06 > > hci_rev=0e2f lmp_ver=06 lmp_subver=a041 > > > > [ 228.639368] Bluetooth: hci0: RTL: rtl: unknown IC info, lmp subver > > a041, hci rev 0e2f, hci ver 0006 > > [ 228.639742] acpi LNXPOWER:01: Turning OFF > > [ 228.640033] OOM killer enabled. > > [ 228.640040] Restarting tasks ... done. > > [ 228.795406] PM: suspend exit > > [ 228.800399] audit: type=1130 audit(1548962671.104:94): pid=1 uid=0 > > auid=4294967295 ses=4294967295 msg='unit=systemd-rfkill comm="systemd" > > exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? > > res=success' > > [ 229.067206] wlp1s0: authenticate with 12:34:56:78:90:12 > > [ 229.067823] wlp1s0: send auth to 12:34:56:78:90:12 (try 1/3) > > [ 229.070955] wlp1s0: authenticated > > [ 229.072395] wlp1s0: associate with 12:34:56:78:90:12 (try 1/3) > > [ 229.074505] wlp1s0: RX AssocResp from 12:34:56:78:90:12 (capab=0x11 > > status=0 aid=2) > > [ 229.074819] wlp1s0: associated > > [ 233.809200] audit: type=1131 audit(1548962676.106:95): pid=1 uid=0 > > auid=4294967295 ses=4294967295 msg='unit=systemd-rfkill comm="systemd" > > exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? > > res=success' > > > > The laptop doesn't go to sleep, the screen turns off, and in a couple > > of seconds it turns on. > > > > Please take a look at this regression. Feel free to ask me for any > > additional information you need. > > > > Thanks, > > Max > > > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >> --- > >> drivers/platform/x86/intel_int0002_vgpio.c | 16 ++++++++++++++++ > >> 1 file changed, 16 insertions(+) > >> > >> diff --git a/drivers/platform/x86/intel_int0002_vgpio.c b/drivers/platform/x86/intel_int0002_vgpio.c > >> index 987a3b03f225..33c3489f5bc1 100644 > >> --- a/drivers/platform/x86/intel_int0002_vgpio.c > >> +++ b/drivers/platform/x86/intel_int0002_vgpio.c > >> @@ -106,6 +106,21 @@ static void int0002_irq_mask(struct irq_data *data) > >> outl(gpe_en_reg, GPE0A_EN_PORT); > >> } > >> > >> +static int int0002_irq_set_wake(struct irq_data *data, unsigned int on) > >> +{ > >> + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); > >> + struct platform_device *pdev = to_platform_device(chip->parent); > >> + int irq = platform_get_irq(pdev, 0); > >> + > >> + /* Propagate to parent irq */ > >> + if (on) > >> + enable_irq_wake(irq); > >> + else > >> + disable_irq_wake(irq); > >> + > >> + return 0; > >> +} > >> + > >> static irqreturn_t int0002_irq(int irq, void *data) > >> { > >> struct gpio_chip *chip = data; > >> @@ -128,6 +143,7 @@ static struct irq_chip int0002_irqchip = { > >> .irq_ack = int0002_irq_ack, > >> .irq_mask = int0002_irq_mask, > >> .irq_unmask = int0002_irq_unmask, > >> + .irq_set_wake = int0002_irq_set_wake, > >> }; > >> > >> static int int0002_probe(struct platform_device *pdev) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] platform/x86: intel_int0002_vgpio: Implement irq_set_wake 2019-02-02 19:04 ` Maxim Mikityanskiy @ 2019-02-03 9:43 ` Hans de Goede 0 siblings, 0 replies; 8+ messages in thread From: Hans de Goede @ 2019-02-03 9:43 UTC (permalink / raw) To: Maxim Mikityanskiy Cc: Andy Shevchenko, Darren Hart, platform-driver-x86, linux-kernel Hi, On 2/2/19 8:04 PM, Maxim Mikityanskiy wrote: > On Sat, Feb 2, 2019 at 12:52 AM Hans de Goede <hdegoede@redhat.com> wrote: >> >> Hi, >> >> On 1/31/19 8:47 PM, Maxim Mikityanskiy wrote: >>> Hi, >>> >>> On Mon, Sep 24, 2018 at 5:37 PM Hans de Goede <hdegoede@redhat.com> wrote: >>>> >>>> We were relying on the interrupt being shared with the ACPI SCI and the >>>> ACPI core calling irq_set_wake. But that does not always happen on >>>> Bay Trail devices, so we should do it ourselves. >>>> >>>> This fixes wake from USB not working on various Bay Trail devices. >>> >>> This patch breaks suspend on ASUS E202SA (bisecting pointed me to this >>> patch, and if I revert it and build 4.20.5 without this patch, >>> everything works flawlessly). >> >> Thank you for the bug report, can you please test 4.20.5 with the attached >> patch on top? That should fix it. Once I've confirmation that this fixes >> things I will submit the patch upstream. > > I've tested your patch against both 4.20.5 and 4.20.6, and it works > fine. Thank you for such a quick fix. Thank you for the bug-report and for testing the fix. I've submitted the patch upstream now, so hopefully it will show up in a 4.20.x release soon. Regards, Hans ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-02-03 9:43 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-24 14:37 [PATCH] platform/x86: intel_int0002_vgpio: Implement irq_set_wake Hans de Goede 2018-09-26 9:34 ` Andy Shevchenko 2019-01-31 19:47 ` Maxim Mikityanskiy 2019-02-01 22:52 ` Hans de Goede 2019-02-01 23:25 ` Andy Shevchenko 2019-02-02 10:50 ` Hans de Goede 2019-02-02 19:04 ` Maxim Mikityanskiy 2019-02-03 9:43 ` Hans de Goede
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.