Skip to contentSkip to conversation

STI problem and workaround for Dell XPS 13 (Skylake, 2016) / Broadcom BCM4350

DISCUSSION Tamas Papp 2 participants 9 messages Mar 2, 2016 – Mar 4, 2016

Subsystems (2)
ACPI ACPI component architecture (acpica)
lore
Share & tools
lore

Tamas Papp

author
#0 lore raw

Hi,

I have a Dell XPS 13 (Skylake, 2016) and wanted to get STI working, so I followed the instructions at [1]. I found that the Broadcom wireless chip is waking the machine up from STI / freeze.

A workaround is

INTERFACE=wlp58s0
ifconfig $INTERFACE down
modprobe -r brcmfmac && echo freeze > /sys/power/state && modprobe brcmfmac ifconfig $INTERFACE up

I thought I would ask whether I should report this as a bug, and if yes, where.

Best,

Tamas

[1] https://01.org/blogs/rzhang/2015/best-practice-debug-linux-suspend/hibernate-issues

Hardware info (lspci -v):

3a:00.0 Network controller: Broadcom Corporation BCM4350 802.11ac Wireless Network Adapter (rev 08)
Subsystem: Dell BCM4350 802.11ac Wireless Network Adapter
Flags: bus master, fast devsel, latency 0, IRQ 283
Memory at dc400000 (64-bit, non-prefetchable) [size=32K]
Memory at dc000000 (64-bit, non-prefetchable) [size=4M]
Capabilities: [48] Power Management version 3
Capabilities: [58] MSI: Enable+ Count=1/16 Maskable- 64bit+
Capabilities: [68] Vendor Specific Information: Len=44 <?>
Capabilities: [ac] Express Endpoint, MSI 00
Capabilities: [100] Advanced Error Reporting
Capabilities: [13c] Device Serial Number 00-00-28-ff-ff-77-c8-ff
Capabilities: [150] Power Budgeting <?>
Capabilities: [160] Virtual Channel
Capabilities: [1b0] Latency Tolerance Reporting
Capabilities: [220] #15
Capabilities: [240] L1 PM Substates
Kernel driver in use: brcmfmac
Kernel modules: brcmfmac

#1 lore raw
Quoted 8 lines · Tamas Papp

Hi,

I have a Dell XPS 13 (Skylake, 2016) and wanted to get STI working, so I followed the instructions at [1]. I found that the Broadcom wireless chip is waking the machine up from STI / freeze.

A workaround is

Quoted 8 lines

INTERFACE=wlp58s0
ifconfig $INTERFACE down
modprobe -r brcmfmac && echo freeze > /sys/power/state && modprobe brcmfmac ifconfig $INTERFACE up

I thought I would ask whether I should report this as a bug, and if yes, where.

Some device drivers such as brcmac has the ability to wake the system up, you can
disable it by echo to /proc/acpi/wakeup IMO.

author
#2 lore raw
Quoted 14 lines · yu chen
Quoted 13 lines · Tamas Papp

Hi,

I have a Dell XPS 13 (Skylake, 2016) and wanted to get STI working, so I followed the instructions at [1]. I found that the Broadcom wireless chip is waking the machine up from STI / freeze.

A workaround is

INTERFACE=wlp58s0
ifconfig $INTERFACE down
modprobe -r brcmfmac && echo freeze > /sys/power/state && modprobe brcmfmac ifconfig $INTERFACE up

Quoted 6 lines
Quoted 3 lines

I thought I would ask whether I should report this as a bug, and if yes, where.

Some device drivers such as brcmac has the ability to wake the system up, you can
disable it by echo to /proc/acpi/wakeup IMO.

That's what I attempted first, but echo to /proc/acpi/wakeup just makes the entry occur _twice_ -- what am I doing wrong?

root@tamas:~# cat /proc/acpi/wakeup | grep ena
PXSX      S4    *enabled   pci:0000:3a:00.0
PBTN      S3    *enabled   platform:PNP0C0C:00
root@tamas:~# echo PXSX > /proc/acpi/wakeup 
root@tamas:~# cat /proc/acpi/wakeup | grep ena
PXSX      S4    *enabled   pci:0000:3c:00.0
PXSX      S4    *enabled   pci:0000:3a:00.0
PBTN      S3    *enabled   platform:PNP0C0C:00
root@tamas:~# uname -r
4.5.0-040500rc6-generic

Best,

Tamas

#3 lore raw
Quoted 26 lines · Tamas Papp
Quoted 14 lines · yu chen
Quoted 13 lines · Tamas Papp

Hi,

I have a Dell XPS 13 (Skylake, 2016) and wanted to get STI working, so I followed the instructions at [1]. I found that the Broadcom wireless chip is waking the machine up from STI / freeze.

A workaround is

INTERFACE=wlp58s0
ifconfig $INTERFACE down
modprobe -r brcmfmac && echo freeze > /sys/power/state && modprobe brcmfmac ifconfig $INTERFACE up

Quoted 6 lines
Quoted 3 lines

I thought I would ask whether I should report this as a bug, and if yes, where.

Some device drivers such as brcmac has the ability to wake the system up, you can
disable it by echo to /proc/acpi/wakeup IMO.

That's what I attempted first, but echo to /proc/acpi/wakeup just makes the entry occur _twice_ -- what am I doing wrong?

Quoted 11 lines
root@tamas:~# cat /proc/acpi/wakeup | grep ena
PXSX      S4    *enabled   pci:0000:3a:00.0
PBTN      S3    *enabled   platform:PNP0C0C:00
root@tamas:~# echo PXSX > /proc/acpi/wakeup
root@tamas:~# cat /proc/acpi/wakeup | grep ena
PXSX      S4    *enabled   pci:0000:3c:00.0
PXSX      S4    *enabled   pci:0000:3a:00.0
PBTN      S3    *enabled   platform:PNP0C0C:00
root@tamas:~# uname -r
4.5.0-040500rc6-generic

Currently the 0000:3c and 0000:3a might have the same pnp object name PXSX, unfortunately the 'echo' will find the first math device in the wakable list, and maybe
0000:3c was probed before 0000:3a, so... maybe this is a weakness.

Maybe you can disabled the wakable feature directly in device sysfs, for example,
echo 0 > /sys/devices/pci0000:00/0000:3a:00.0/power/wakeup, you might need to find out where 0000:3a:00.0 is.

author
#4 lore raw
Quoted 9 lines · yu chen

Currently the 0000:3c and 0000:3a might have the same pnp object name PXSX, unfortunately the 'echo' will find the first math device in the wakable list, and maybe
0000:3c was probed before 0000:3a, so... maybe this is a weakness.

Maybe you can disabled the wakable feature directly in device sysfs, for example,
echo 0 > /sys/devices/pci0000:00/0000:3a:00.0/power/wakeup, you might need to find out where 0000:3a:00.0 is.

I tried that, but the module has problems after wakeup. I have to remove and insert it again with modprobe.

Still, this is an OK workaround, thanks for the help. I still don't know if I should file this as a bug, and if yes, where.

Best,

Tamas

#5 lore raw
Quoted 12 lines · Tamas Papp
Quoted 9 lines · yu chen

Currently the 0000:3c and 0000:3a might have the same pnp object name PXSX, unfortunately the 'echo' will find the first math device in the wakable list, and maybe
0000:3c was probed before 0000:3a, so... maybe this is a weakness.

Maybe you can disabled the wakable feature directly in device sysfs, for example,
echo 0 > /sys/devices/pci0000:00/0000:3a:00.0/power/wakeup, you might need to find out where 0000:3a:00.0 is.

Quoted 6 lines

I tried that, but the module has problems after wakeup. I have to remove and insert it again with modprobe.

Still, this is an OK workaround, thanks for the help. I still don't know if I should file this as a bug, and if yes, where.

Does the following fix make sense for you:

cat /proc/acpi/wakeup
PXSX:0 S4 *enabled pci:0000:3a:00.0
then you need to echo PXSX:0 to disable it,
and for 3c, it would be PXSX:1

#6 lore raw
Quoted 19 lines · yu chen
Quoted 12 lines · Tamas Papp
Quoted 9 lines · yu chen

Currently the 0000:3c and 0000:3a might have the same pnp object name PXSX, unfortunately the 'echo' will find the first math device in the wakable list, and maybe
0000:3c was probed before 0000:3a, so... maybe this is a weakness.

Maybe you can disabled the wakable feature directly in device sysfs, for example,
echo 0 > /sys/devices/pci0000:00/0000:3a:00.0/power/wakeup, you might need to find out where 0000:3a:00.0 is.

Quoted 6 lines

I tried that, but the module has problems after wakeup. I have to remove and insert it again with modprobe.

Still, this is an OK workaround, thanks for the help. I still don't know if I should file this as a bug, and if yes, where.

Quoted 6 lines

Does the following fix make sense for you:

cat /proc/acpi/wakeup
PXSX:0 S4 *enabled pci:0000:3a:00.0
then you need to echo PXSX:0 to disable it,
and for 3c, it would be PXSX:1

plz find attached the trival patch for it

drivers/acpi/proc.c +22 -9
Index: linux_for_test/drivers/acpi/proc.c
===================================================================
@@ -33,10 +33,14 @@
 
 		if (!dev->wakeup.flags.valid)
 			continue;
-
-		seq_printf(seq, "%s\t  S%d\t",
-			   dev->pnp.bus_id,
-			   (u32) dev->wakeup.sleep_state);
+		if (dev->pnp.bus_sub_id >= 0)
+			seq_printf(seq, "%s:%d\t  S%d\t",
+				    dev->pnp.bus_id, dev->pnp.bus_sub_id,
+				    (u32) dev->wakeup.sleep_state);
+		else
+			seq_printf(seq, "%s\t  S%d\t",
+				    dev->pnp.bus_id,
+				    (u32) dev->wakeup.sleep_state);
 
 		mutex_lock(&dev->physical_node_lock);
 

@@ -96,15 +100,23 @@
 				size_t count, loff_t * ppos)
 {
 	struct list_head *node, *next;
-	char strbuf[5];
-	char str[5] = "";
+	char strbuf[7];
+	char str[7] = "";
+	int sub_id = -1;
+	char *split;
 
-	if (count > 4)
-		count = 4;
+	if (count > 6)
+		count = 6;
 
 	if (copy_from_user(strbuf, buffer, count))
 		return -EFAULT;
 	strbuf[count] = '\0';
+	split = strstr(strbuf, ":");
+	if (split) {
+		*split = '\0';
+		split++;
+		sscanf(split, "%d", &sub_id);
+	}
 	sscanf(strbuf, "%s", str);
 
 	mutex_lock(&acpi_device_lock);

@@ -114,7 +126,8 @@
 		if (!dev->wakeup.flags.valid)
 			continue;
 
-		if (!strncmp(dev->pnp.bus_id, str, 4)) {
+		if (!strncmp(dev->pnp.bus_id, str, 4) &&
+		    (sub_id == -1 || sub_id == dev->pnp.bus_sub_id)) {
 			if (device_can_wakeup(&dev->dev)) {
 				bool enable = !device_may_wakeup(&dev->dev);
 				device_set_wakeup_enable(&dev->dev, enable);
drivers/acpi/scan.c +29 -1
Index: linux_for_test/drivers/acpi/scan.c
===================================================================
@@ -607,6 +607,34 @@
 	put_device(&adev->dev);
 }
 
+#define MAX_SAME_NAME	100
+static void _wake_device_add(struct acpi_device *device)
+{
+	struct list_head *node, *next;
+	int max_idx = -1;
+
+	list_for_each_safe(node, next, &acpi_wakeup_device_list) {
+		struct acpi_device *dev =
+		    container_of(node, struct acpi_device, wakeup_list);
+
+		if (!dev->wakeup.flags.valid)
+			continue;
+
+		if (!strncmp(dev->pnp.bus_id, device->pnp.bus_id, 4)) {
+			if (dev->pnp.bus_sub_id == -1)
+				dev->pnp.bus_sub_id = 0;
+			if (dev->pnp.bus_sub_id > max_idx)
+				max_idx = dev->pnp.bus_sub_id;
+		}
+	}
+
+	if (max_idx >= MAX_SAME_NAME)
+		pr_err("Too many devices of the same name.\n");
+
+	device->pnp.bus_sub_id = (max_idx >= 0) ? (max_idx + 1) : max_idx;
+	list_add_tail(&device->wakeup_list, &acpi_wakeup_device_list);
+}
+
 int acpi_device_add(struct acpi_device *device,
 		    void (*release)(struct device *))
 {

@@ -671,7 +699,7 @@
 		list_add_tail(&device->node, &device->parent->children);
 
 	if (device->wakeup.flags.valid)
-		list_add_tail(&device->wakeup_list, &acpi_wakeup_device_list);
+		_wake_device_add(device);
 	mutex_unlock(&acpi_device_lock);
 
 	if (device->parent)
include/acpi/acpi_bus.h +1 -0
Index: linux_for_test/include/acpi/acpi_bus.h
===================================================================
@@ -241,6 +241,7 @@
 
 struct acpi_device_pnp {
 	acpi_bus_id bus_id;		/* Object name */
+	int bus_sub_id;			/* Sub object index */
 	struct acpi_pnp_type type;	/* ID type */
 	acpi_bus_address bus_address;	/* _ADR */
 	char *unique_id;		/* _UID */
author
#7 lore raw
In reply to yu chen
Quoted 19 lines · yu chen
Quoted 12 lines · Tamas Papp
Quoted 9 lines · yu chen

Currently the 0000:3c and 0000:3a might have the same pnp object name PXSX, unfortunately the 'echo' will find the first math device in the wakable list, and maybe
0000:3c was probed before 0000:3a, so... maybe this is a weakness.

Maybe you can disabled the wakable feature directly in device sysfs, for example,
echo 0 > /sys/devices/pci0000:00/0000:3a:00.0/power/wakeup, you might need to find out where 0000:3a:00.0 is.

Quoted 6 lines

I tried that, but the module has problems after wakeup. I have to remove and insert it again with modprobe.

Still, this is an OK workaround, thanks for the help. I still don't know if I should file this as a bug, and if yes, where.

Quoted 6 lines

Does the following fix make sense for you:

cat /proc/acpi/wakeup
PXSX:0 S4 *enabled pci:0000:3a:00.0
then you need to echo PXSX:0 to disable it,
and for 3c, it would be PXSX:1

Sorry, I am confused --- is this a proposed new syntax for /proc/acpi/wakeup? I guess this could work. BTW, for kernel 4.4.3, I don't need any fix (that's what I am using now), it works out of the box, I just need to fiddle with the broadcom module in 4.5.0-rc6.

Best,

Tamas

#8 lore raw
Quoted 28 lines · Tamas Papp
Quoted 19 lines · yu chen
Quoted 12 lines · Tamas Papp
Quoted 9 lines · yu chen

Currently the 0000:3c and 0000:3a might have the same pnp object name PXSX, unfortunately the 'echo' will find the first math device in the wakable list, and maybe
0000:3c was probed before 0000:3a, so... maybe this is a weakness.

Maybe you can disabled the wakable feature directly in device sysfs, for example,
echo 0 > /sys/devices/pci0000:00/0000:3a:00.0/power/wakeup, you might need to find out where 0000:3a:00.0 is.

Quoted 6 lines

I tried that, but the module has problems after wakeup. I have to remove and insert it again with modprobe.

Still, this is an OK workaround, thanks for the help. I still don't know if I should file this as a bug, and if yes, where.

Quoted 6 lines

Does the following fix make sense for you:

cat /proc/acpi/wakeup
PXSX:0 S4 *enabled pci:0000:3a:00.0
then you need to echo PXSX:0 to disable it,
and for 3c, it would be PXSX:1

Quoted 4 lines

Sorry, I am confused --- is this a proposed new syntax for /proc/acpi/wakeup? I guess this could work. BTW, for kernel 4.4.3, I don't need any fix (that's what I am using now), it works out of the box, I just need to fiddle with the broadcom module in 4.5.0-rc6.

It appends index suffix to their orinal name if there's a conflict.
Maybe in 4.4.3 broadcom is not a wakable by default, although the duplicate
naming problem should always be there.