Re: [PATCH] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi
On Wed, May 29, 2019 at 1:55 AM Hans de Goede wrote: > > Hi João, > > On 5/28/19 11:22 PM, João Paulo Rechi Vita wrote: > > On Mon, May 20, 2019 at 11:28 PM Hans de Goede wrote: > >> > >> Commit 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will > >> handle the display off hotkey") causes the backlight to be permanently off > >> on various EeePC laptop models using the eeepc-wmi driver (Asus EeePC > >> 1015BX, Asus EeePC 1025C). > >> > >> The asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL) call added > >> by that commit is made conditional in this commit and only enabled in > >> the quirk_entry structs in the asus-nb-wmi driver fixing the broken > >> display / backlight on various EeePC laptop models. > >> > >> Cc: João Paulo Rechi Vita > >> Fixes: 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will > >> handle the display off hotkey") > >> Signed-off-by: Hans de Goede > >> --- > >> drivers/platform/x86/asus-nb-wmi.c | 8 > >> drivers/platform/x86/asus-wmi.c| 2 +- > >> drivers/platform/x86/asus-wmi.h| 1 + > >> 3 files changed, 10 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/platform/x86/asus-nb-wmi.c > >> b/drivers/platform/x86/asus-nb-wmi.c > >> index b6f2ff95c3ed..59f3a37a44d7 100644 > >> --- a/drivers/platform/x86/asus-nb-wmi.c > >> +++ b/drivers/platform/x86/asus-nb-wmi.c > >> @@ -78,10 +78,12 @@ static bool asus_q500a_i8042_filter(unsigned char > >> data, unsigned char str, > >> > >> static struct quirk_entry quirk_asus_unknown = { > >> .wapf = 0, > >> + .wmi_backlight_set_devstate = true, > >> }; > >> > >> static struct quirk_entry quirk_asus_q500a = { > >> .i8042_filter = asus_q500a_i8042_filter, > >> + .wmi_backlight_set_devstate = true, > >> }; > >> > >> /* > >> @@ -92,26 +94,32 @@ static struct quirk_entry quirk_asus_q500a = { > >> static struct quirk_entry quirk_asus_x55u = { > >> .wapf = 4, > >> .wmi_backlight_power = true, > >> + .wmi_backlight_set_devstate = true, > >> .no_display_toggle = true, > >> }; > >> > >> static struct quirk_entry quirk_asus_wapf4 = { > >> .wapf = 4, > >> + .wmi_backlight_set_devstate = true, > >> }; > >> > >> static struct quirk_entry quirk_asus_x200ca = { > >> .wapf = 2, > >> + .wmi_backlight_set_devstate = true, > >> }; > >> > >> static struct quirk_entry quirk_asus_ux303ub = { > >> .wmi_backlight_native = true, > >> + .wmi_backlight_set_devstate = true, > >> }; > >> > >> static struct quirk_entry quirk_asus_x550lb = { > >> + .wmi_backlight_set_devstate = true, > >> .xusb2pr = 0x01D9, > >> }; > >> > >> static struct quirk_entry quirk_asus_forceals = { > >> + .wmi_backlight_set_devstate = true, > >> .wmi_force_als_set = true, > >> }; > >> > >> diff --git a/drivers/platform/x86/asus-wmi.c > >> b/drivers/platform/x86/asus-wmi.c > >> index ee1fa93708ec..a66e99500c12 100644 > >> --- a/drivers/platform/x86/asus-wmi.c > >> +++ b/drivers/platform/x86/asus-wmi.c > >> @@ -2131,7 +2131,7 @@ static int asus_wmi_add(struct platform_device *pdev) > >> err = asus_wmi_backlight_init(asus); > >> if (err && err != -ENODEV) > >> goto fail_backlight; > >> - } else > >> + } else if (asus->driver->quirks->wmi_backlight_set_devstate) > >> err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, > >> NULL); > >> > >> status = wmi_install_notify_handler(asus->driver->event_guid, > >> diff --git a/drivers/platform/x86/asus-wmi.h > >> b/drivers/platform/x86/asus-wmi.h > >> index 6c1311f4b04d..57a79bddb286 100644 > >> --- a/drivers/platform/x86/asus-wmi.h > >> +++ b/drivers/platform/x86/asus-wmi.h > >> @@ -44,6 +44,7 @@ struct quirk_entry { > >> bool store_backlight_power; > >> bool wmi_backlight_power; > >> bool wmi_backlight_native; > >> + bool wmi_backlight_set_devstate; > > > &g
Re: [PATCH] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi
On Mon, May 20, 2019 at 11:28 PM Hans de Goede wrote: > > Commit 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will > handle the display off hotkey") causes the backlight to be permanently off > on various EeePC laptop models using the eeepc-wmi driver (Asus EeePC > 1015BX, Asus EeePC 1025C). > > The asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL) call added > by that commit is made conditional in this commit and only enabled in > the quirk_entry structs in the asus-nb-wmi driver fixing the broken > display / backlight on various EeePC laptop models. > > Cc: João Paulo Rechi Vita > Fixes: 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will handle > the display off hotkey") > Signed-off-by: Hans de Goede > --- > drivers/platform/x86/asus-nb-wmi.c | 8 > drivers/platform/x86/asus-wmi.c| 2 +- > drivers/platform/x86/asus-wmi.h| 1 + > 3 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/asus-nb-wmi.c > b/drivers/platform/x86/asus-nb-wmi.c > index b6f2ff95c3ed..59f3a37a44d7 100644 > --- a/drivers/platform/x86/asus-nb-wmi.c > +++ b/drivers/platform/x86/asus-nb-wmi.c > @@ -78,10 +78,12 @@ static bool asus_q500a_i8042_filter(unsigned char data, > unsigned char str, > > static struct quirk_entry quirk_asus_unknown = { > .wapf = 0, > + .wmi_backlight_set_devstate = true, > }; > > static struct quirk_entry quirk_asus_q500a = { > .i8042_filter = asus_q500a_i8042_filter, > + .wmi_backlight_set_devstate = true, > }; > > /* > @@ -92,26 +94,32 @@ static struct quirk_entry quirk_asus_q500a = { > static struct quirk_entry quirk_asus_x55u = { > .wapf = 4, > .wmi_backlight_power = true, > + .wmi_backlight_set_devstate = true, > .no_display_toggle = true, > }; > > static struct quirk_entry quirk_asus_wapf4 = { > .wapf = 4, > + .wmi_backlight_set_devstate = true, > }; > > static struct quirk_entry quirk_asus_x200ca = { > .wapf = 2, > + .wmi_backlight_set_devstate = true, > }; > > static struct quirk_entry quirk_asus_ux303ub = { > .wmi_backlight_native = true, > + .wmi_backlight_set_devstate = true, > }; > > static struct quirk_entry quirk_asus_x550lb = { > + .wmi_backlight_set_devstate = true, > .xusb2pr = 0x01D9, > }; > > static struct quirk_entry quirk_asus_forceals = { > + .wmi_backlight_set_devstate = true, > .wmi_force_als_set = true, > }; > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c > index ee1fa93708ec..a66e99500c12 100644 > --- a/drivers/platform/x86/asus-wmi.c > +++ b/drivers/platform/x86/asus-wmi.c > @@ -2131,7 +2131,7 @@ static int asus_wmi_add(struct platform_device *pdev) > err = asus_wmi_backlight_init(asus); > if (err && err != -ENODEV) > goto fail_backlight; > - } else > + } else if (asus->driver->quirks->wmi_backlight_set_devstate) > err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, > NULL); > > status = wmi_install_notify_handler(asus->driver->event_guid, > diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h > index 6c1311f4b04d..57a79bddb286 100644 > --- a/drivers/platform/x86/asus-wmi.h > +++ b/drivers/platform/x86/asus-wmi.h > @@ -44,6 +44,7 @@ struct quirk_entry { > bool store_backlight_power; > bool wmi_backlight_power; > bool wmi_backlight_native; > + bool wmi_backlight_set_devstate; Wouldn't it be better to add this field to struct asus_wmi_driver instead, and set it in asus_nb_wmi_driver only? This way we wouldn't need to make sure it is present in all quirk entries from this driver, current and future. I've tested both the original patch and my suggestion above and in both cases the "turn off backlight" hotkey continued to work fine on a machine where asus-nb-wmi is used (I don't have access to any machine using the eeepc driver). > bool wmi_force_als_set; > int wapf; > /* > -- > 2.21.0 >
[PATCH] Bluetooth: Add new 13d3:3501 QCA_ROME device
Without the QCA ROME setup routine this adapter fails to establish a SCO connection. T: Bus=01 Lev=01 Prnt=01 Port=04 Cnt=01 Dev#= 2 Spd=12 MxCh= 0 D: Ver= 1.10 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs= 1 P: Vendor=13d3 ProdID=3501 Rev=00.01 C: #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA I: If#=0x0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb I: If#=0x1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb Signed-off-by: João Paulo Rechi Vita --- drivers/bluetooth/btusb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index 7db48ae65cd2..ff44622ea10d 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -280,6 +280,7 @@ static const struct usb_device_id blacklist_table[] = { { USB_DEVICE(0x04ca, 0x3016), .driver_info = BTUSB_QCA_ROME }, { USB_DEVICE(0x04ca, 0x301a), .driver_info = BTUSB_QCA_ROME }, { USB_DEVICE(0x13d3, 0x3496), .driver_info = BTUSB_QCA_ROME }, + { USB_DEVICE(0x13d3, 0x3501), .driver_info = BTUSB_QCA_ROME }, /* Broadcom BCM2035 */ { USB_DEVICE(0x0a5c, 0x2009), .driver_info = BTUSB_BCM92035 }, -- 2.20.1
[PATCH] Bluetooth: Add new 13d3:3491 QCA_ROME device
Without the QCA ROME setup routine this adapter fails to establish a SCO connection. T: Bus=01 Lev=01 Prnt=01 Port=08 Cnt=01 Dev#= 2 Spd=12 MxCh= 0 D: Ver= 1.10 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs= 1 P: Vendor=13d3 ProdID=3491 Rev=00.01 C: #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA I: If#=0x0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb I: If#=0x1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb Signed-off-by: João Paulo Rechi Vita --- drivers/bluetooth/btusb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index ff44622ea10d..4c9f11766e82 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -279,6 +279,7 @@ static const struct usb_device_id blacklist_table[] = { { USB_DEVICE(0x04ca, 0x3015), .driver_info = BTUSB_QCA_ROME }, { USB_DEVICE(0x04ca, 0x3016), .driver_info = BTUSB_QCA_ROME }, { USB_DEVICE(0x04ca, 0x301a), .driver_info = BTUSB_QCA_ROME }, + { USB_DEVICE(0x13d3, 0x3491), .driver_info = BTUSB_QCA_ROME }, { USB_DEVICE(0x13d3, 0x3496), .driver_info = BTUSB_QCA_ROME }, { USB_DEVICE(0x13d3, 0x3501), .driver_info = BTUSB_QCA_ROME }, -- 2.20.1
[PATCH] Bluetooth: Ignore CC events not matching the last HCI command
This commit makes the kernel not send the next queued HCI command until a command complete arrives for the last HCI command sent to the controller. This change avoids a problem with some buggy controllers (seen on two SKUs of QCA9377) that send an extra command complete event for the previous command after the kernel had already sent a new HCI command to the controller. The problem was reproduced when starting an active scanning procedure, where an extra command complete event arrives for the LE_SET_RANDOM_ADDR command. When this happends the kernel ends up not processing the command complete for the following commmand, LE_SET_SCAN_PARAM, and ultimately behaving as if a passive scanning procedure was being performed, when in fact controller is performing an active scanning procedure. This makes it impossible to discover BLE devices as no device found events are sent to userspace. This problem is reproducible on 100% of the attempts on the affected controllers. The extra command complete event can be seen at timestamp 27.420131 on the btmon logs bellow. Bluetooth monitor ver 5.50 = Note: Linux version 5.0.0+ (x86_64) 0.352340 = Note: Bluetooth subsystem version 2.22 0.352343 = New Index: 80:C5:F2:8F:87:84 (Primary,USB,hci0) [hci0] 0.352344 = Open Index: 80:C5:F2:8F:87:84 [hci0] 0.352345 = Index Info: 80:C5:F2:8F:87:84 (Qualcomm) [hci0] 0.352346 @ MGMT Open: bluetoothd (privileged) version 1.14 {0x0001} 0.352347 @ MGMT Open: btmon (privileged) version 1.14 {0x0002} 0.352366 @ MGMT Open: btmgmt (privileged) version 1.14{0x0003} 27.302164 @ MGMT Command: Start Discovery (0x0023) plen 1 {0x0003} [hci0] 27.302310 Address type: 0x06 LE Public LE Random < HCI Command: LE Set Random Address (0x08|0x0005) plen 6 #1 [hci0] 27.302496 Address: 15:60:F2:91:B2:24 (Non-Resolvable) > HCI Event: Command Complete (0x0e) plen 4 #2 [hci0] 27.419117 LE Set Random Address (0x08|0x0005) ncmd 1 Status: Success (0x00) < HCI Command: LE Set Scan Parameters (0x08|0x000b) plen 7 #3 [hci0] 27.419244 Type: Active (0x01) Interval: 11.250 msec (0x0012) Window: 11.250 msec (0x0012) Own address type: Random (0x01) Filter policy: Accept all advertisement (0x00) > HCI Event: Command Complete (0x0e) plen 4 #4 [hci0] 27.420131 LE Set Random Address (0x08|0x0005) ncmd 1 Status: Success (0x00) < HCI Command: LE Set Scan Enable (0x08|0x000c) plen 2 #5 [hci0] 27.420259 Scanning: Enabled (0x01) Filter duplicates: Enabled (0x01) > HCI Event: Command Complete (0x0e) plen 4 #6 [hci0] 27.420969 LE Set Scan Parameters (0x08|0x000b) ncmd 1 Status: Success (0x00) > HCI Event: Command Complete (0x0e) plen 4 #7 [hci0] 27.421983 LE Set Scan Enable (0x08|0x000c) ncmd 1 Status: Success (0x00) @ MGMT Event: Command Complete (0x0001) plen 4{0x0003} [hci0] 27.422059 Start Discovery (0x0023) plen 1 Status: Success (0x00) Address type: 0x06 LE Public LE Random @ MGMT Event: Discovering (0x0013) plen 2 {0x0003} [hci0] 27.422067 Address type: 0x06 LE Public LE Random Discovery: Enabled (0x01) @ MGMT Event: Discovering (0x0013) plen 2 {0x0002} [hci0] 27.422067 Address type: 0x06 LE Public LE Random Discovery: Enabled (0x01) @ MGMT Event: Discovering (0x0013) plen 2 {0x0001} [hci0] 27.422067 Address type: 0x06 LE Public LE Random Discovery: Enabled (0x01) Signed-off-by: João Paulo Rechi Vita --- include/net/bluetooth/hci_core.h | 1 + net/bluetooth/hci_core.c | 5 + net/bluetooth/hci_event.c| 3 +++ 3 files changed, 9 insertions(+) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 094e61e07030..85bed4e916d3 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -364,6 +364,7 @@ struct hci_dev { struct sk_buff_head cmd_q; struct sk_buff *sent_cmd; + __u8sent_cmd_pending_cc; struct mutexreq_lock; wait_queue_head_t req_wait_q; diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index d6b2540ba7f8..37893b0c6077 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -4380,9 +4380,13 @@ void hci_req_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status, if (test_bit(HCI_INIT, >flags) && opcode == HCI_OP_RESET) hci_resend_last(hdev); + bt_dev_err(hdev, + "u
[PATCH 0/2] Quirk to enable QCA9377 to discover BLE devices
As reported previously on [1], it is currently not possible to discover BLE devices with QCA9377 controllers. When trying to start an active scanning procedure with this controller, three commands are queued, LE_SET_RANDOM_ADDR, LE_SET_SCAN_PARAM and LE_SET_SCAN_ENABLE. After the first command is sent to the controller, a command complete event for it is received, and the second command is sent, an extra command complete for the first command is received. At this point the kernel sends the next command and fails to process the command complete event for the LE_SET_SCAN_PARAM command, because when it arrives it does not match the last command that was sent. This makes hdev->le_scan_type never be updated and the kernel behaves as if a passive scanning procedure was being performed, thus no device found events are sent to userspace. [1] https://www.spinics.net/lists/linux-bluetooth/msg79102.html I have received no replies on the previous report and on further attempts to contact the QCA addresses that have submitted Bluetooth firmware blobs to linux-firmware upstream. This series avoids the problem described above, but I believe ideally the controller should not be sending this extra command complete event. I'm not 100% sure if the approach taken here is the best way to work around this problem in the kernel, as I am not super familiar with the HCI layer. I'll be happy to hear suggestions of better approaches. Full logs from btmon can be found bellow this message, and the extra command complete event can be seen at timestamp 27.420131. Best regards, João Paulo Rechi Vita (2): Bluetooth: Create new HCI_QUIRK_WAIT_FOR_MATCHING_CC Bluetooth: Set HCI_QUIRK_WAIT_FOR_MATCHING_CC for QCA9377 drivers/bluetooth/btusb.c| 9 + include/net/bluetooth/hci.h | 4 include/net/bluetooth/hci_core.h | 1 + net/bluetooth/hci_core.c | 3 +++ net/bluetooth/hci_event.c| 4 5 files changed, 21 insertions(+) -- 2.20.1 Bluetooth monitor ver 5.50 = Note: Linux version 5.0.0+ (x86_64) 0.352340 = Note: Bluetooth subsystem version 2.22 0.352343 = New Index: 80:C5:F2:8F:87:84 (Primary,USB,hci0) [hci0] 0.352344 = Open Index: 80:C5:F2:8F:87:84 [hci0] 0.352345 = Index Info: 80:C5:F2:8F:87:84 (Qualcomm) [hci0] 0.352346 @ MGMT Open: bluetoothd (privileged) version 1.14 {0x0001} 0.352347 @ MGMT Open: btmon (privileged) version 1.14 {0x0002} 0.352366 @ MGMT Open: btmgmt (privileged) version 1.14{0x0003} 27.302164 @ MGMT Command: Start Discovery (0x0023) plen 1 {0x0003} [hci0] 27.302310 Address type: 0x06 LE Public LE Random < HCI Command: LE Set Random Address (0x08|0x0005) plen 6 #1 [hci0] 27.302496 Address: 15:60:F2:91:B2:24 (Non-Resolvable) > HCI Event: Command Complete (0x0e) plen 4 #2 [hci0] 27.419117 LE Set Random Address (0x08|0x0005) ncmd 1 Status: Success (0x00) < HCI Command: LE Set Scan Parameters (0x08|0x000b) plen 7 #3 [hci0] 27.419244 Type: Active (0x01) Interval: 11.250 msec (0x0012) Window: 11.250 msec (0x0012) Own address type: Random (0x01) Filter policy: Accept all advertisement (0x00) > HCI Event: Command Complete (0x0e) plen 4 #4 [hci0] 27.420131 LE Set Random Address (0x08|0x0005) ncmd 1 Status: Success (0x00) < HCI Command: LE Set Scan Enable (0x08|0x000c) plen 2 #5 [hci0] 27.420259 Scanning: Enabled (0x01) Filter duplicates: Enabled (0x01) > HCI Event: Command Complete (0x0e) plen 4 #6 [hci0] 27.420969 LE Set Scan Parameters (0x08|0x000b) ncmd 1 Status: Success (0x00) > HCI Event: Command Complete (0x0e) plen 4 #7 [hci0] 27.421983 LE Set Scan Enable (0x08|0x000c) ncmd 1 Status: Success (0x00) @ MGMT Event: Command Complete (0x0001) plen 4{0x0003} [hci0] 27.422059 Start Discovery (0x0023) plen 1 Status: Success (0x00) Address type: 0x06 LE Public LE Random @ MGMT Event: Discovering (0x0013) plen 2 {0x0003} [hci0] 27.422067 Address type: 0x06 LE Public LE Random Discovery: Enabled (0x01) @ MGMT Event: Discovering (0x0013) plen 2 {0x0002} [hci0] 27.422067 Address type: 0x06 LE Public LE Random Discovery: Enabled (0x01) @ MGMT Event: Discovering (0x0013) plen 2 {0x0001} [hci0] 27.422067 Address type: 0x06 LE Public LE Random Discovery: Enabled (0x01) > HCI Event: LE Meta Event (0x3e) plen 39 #8 [hci0] 27.434978 LE Advertising Report (0x02) Num reports: 1 Event type: Connectable undirected - ADV_IND (0x00)
Re: X450LCP lost abillity to turn the screen off
On Mon, Feb 11, 2019 at 6:31 PM Marcos Paulo de Souza wrote: > > Hello João, > > On 2/11/19 5:14 PM, João Paulo Rechi Vita wrote: > > Hello Marcos, > > > > On Sun, Feb 10, 2019 at 5:05 PM Marcos Paulo de Souza > > wrote: > >> > >> > >> > >> On 2/10/19 9:45 PM, Andy Shevchenko wrote: > >>> On Sun, Feb 10, 2019 at 9:24 PM Marcos Paulo de Souza > >>> wrote: > >>>> > >>>> Hi, > >>>> > >>>> Since 5.0.0-rc4 I vefiried that my ASUS laptop > >>> > >>> Can you be more specific, what model, BIOS version, etc (also would be > >>> nice to have dmi strings from it, I guess dmidecode tool would help). > >> > >> dmidecode attached. > >> > >>>> cannot turn the screen of > >>>> anymore. There were several commits in 5.0 merge window touching this > >>>> functionality like: > >>>> > >>>> 71b12beaf12f platform/x86: asus-nb-wmi: Drop mapping of 0x33 and 0x34 > >>>> scan codes > >>>> b3f2f3799a97 platform/x86: asus-nb-wmi: Map 0x35 to KEY_SCREENLOCK > >>>> 78f3ac76d9e5 platform/x86: asus-wmi: Tell the EC the OS will handle > >>>> the display off hotkey > >>>> > >>> > >>> Can you bisect or just try to revert one-by-one from above and see > >>> which one is a culprit? > >> > >> I already did some primary analysis, and it seems the commit 3f2f3799a97 > >> maps the x035 (which is Alt+f7 in my laptop) to SCREENLOCK, which is > >> wrong because alt+f7 should be Screen Toggle. I will try to revert this > >> commit, or remap to KEY_DISPLAYTOGGLE or KEY_DISPLAY_OFF, and test if it > >> works. > >> > > > > User-space does not act on KEY_DISPLAYTOGGLE / KEY_DISPLAY_OFF, these > > values should be used when the hardware is turning the screen > > back-light ON and OFF. According to Asus BIOS engineers, the > > back-light used to be driven by the hardware, but they have changed to > > the this new approach of telling the OS to drive the back-light for a > > while now (no specific dates or BIOS / windows driver versions were > > shared). They we actually surprised when we told the that some > > machines still have a working implementation (and selected by default > > unless told otherwise) of the old behavior, which sounds like it is > > the case for the machine you have at hand. > > > > The new behavior, as defined in their spec is to only notify the OS of > > the keypress with 0x35, and have the OS "close" the screen, with the > > screen being "opened" on mouse or keyboard activity. This closely > > matches the screen lock behavior on Linux platforms, so we are mapping > > it to KEY_SCREENLOCK in the kernel, and it then gets mapped to > > XF86ScreenSaver by xkeyboard-config, and finally gnome-settings-daemon > > uses it as a lock screen shortcut (look for "screensaver" in > > plugins/media-keys/shortcuts-list.h on the gnome-settings-daemon > > repository). > > Interesting. > > > > >> But yes, I'll do my best to track the problem ASAP at my side. Please > >> let me know if I can provide any additional information. > >> > > > > You can check what is being sent by the kernel with evtest, and what > > is being sent by X with "xinput test " (and you can find > > the device id with "xinput list"). And you can re-map it without > > having to rebuild the kernel using udev's hwdb. But simply re-mapping > > should not change anything, since userspace does not act on > > KEY_DISPLAYTOGGLE / KEY_DISPLAY_OFF. If you want to switch back to the > > old behavior you need to revert "78f3ac76d9e5 platform/x86: asus-wmi: > > Tell the EC the OS will handle the display off hotkey". > > I tried reverting the patch and only recompiling/reinstalling the > platform/x86 modules, but the problem still happens. My next step will > be testing agains't 4.20, since my machine was working with 4.12, so I > might try the major releases first. > So maybe your desktop environment (KDE) acts on KEY_DISPLAYTOGGLE / KEY_DISPLAY_OFF and this is the only reason why this was working in the first place? It would be sad to find out different DEs behave differently in this situation, but IMO include/uapi/linux/input-event-codes.h is not super clear about whether userspace should act on these values or they are just intended to notify userspace of a change so desktop notifications (like an OSD) can be sho
Re: X450LCP lost abillity to turn the screen off
Hello Marcos, On Sun, Feb 10, 2019 at 5:05 PM Marcos Paulo de Souza wrote: > > > > On 2/10/19 9:45 PM, Andy Shevchenko wrote: > > On Sun, Feb 10, 2019 at 9:24 PM Marcos Paulo de Souza > > wrote: > >> > >> Hi, > >> > >> Since 5.0.0-rc4 I vefiried that my ASUS laptop > > > > Can you be more specific, what model, BIOS version, etc (also would be > > nice to have dmi strings from it, I guess dmidecode tool would help). > > dmidecode attached. > > >> cannot turn the screen of > >> anymore. There were several commits in 5.0 merge window touching this > >> functionality like: > >> > >> 71b12beaf12f platform/x86: asus-nb-wmi: Drop mapping of 0x33 and 0x34 > >> scan codes > >> b3f2f3799a97 platform/x86: asus-nb-wmi: Map 0x35 to KEY_SCREENLOCK > >> 78f3ac76d9e5 platform/x86: asus-wmi: Tell the EC the OS will handle the > >> display off hotkey > >> > > > > Can you bisect or just try to revert one-by-one from above and see > > which one is a culprit? > > I already did some primary analysis, and it seems the commit 3f2f3799a97 > maps the x035 (which is Alt+f7 in my laptop) to SCREENLOCK, which is > wrong because alt+f7 should be Screen Toggle. I will try to revert this > commit, or remap to KEY_DISPLAYTOGGLE or KEY_DISPLAY_OFF, and test if it > works. > User-space does not act on KEY_DISPLAYTOGGLE / KEY_DISPLAY_OFF, these values should be used when the hardware is turning the screen back-light ON and OFF. According to Asus BIOS engineers, the back-light used to be driven by the hardware, but they have changed to the this new approach of telling the OS to drive the back-light for a while now (no specific dates or BIOS / windows driver versions were shared). They we actually surprised when we told the that some machines still have a working implementation (and selected by default unless told otherwise) of the old behavior, which sounds like it is the case for the machine you have at hand. The new behavior, as defined in their spec is to only notify the OS of the keypress with 0x35, and have the OS "close" the screen, with the screen being "opened" on mouse or keyboard activity. This closely matches the screen lock behavior on Linux platforms, so we are mapping it to KEY_SCREENLOCK in the kernel, and it then gets mapped to XF86ScreenSaver by xkeyboard-config, and finally gnome-settings-daemon uses it as a lock screen shortcut (look for "screensaver" in plugins/media-keys/shortcuts-list.h on the gnome-settings-daemon repository). > But yes, I'll do my best to track the problem ASAP at my side. Please > let me know if I can provide any additional information. > You can check what is being sent by the kernel with evtest, and what is being sent by X with "xinput test " (and you can find the device id with "xinput list"). And you can re-map it without having to rebuild the kernel using udev's hwdb. But simply re-mapping should not change anything, since userspace does not act on KEY_DISPLAYTOGGLE / KEY_DISPLAY_OFF. If you want to switch back to the old behavior you need to revert "78f3ac76d9e5 platform/x86: asus-wmi: Tell the EC the OS will handle the display off hotkey". That being said, I believe it would be more productive to figure out why your userspace stack is not reacting to 0x35 / XF86ScreenSaver and fix that. Which window manager / graphical desktop environment are you using? As a final note, from your dmidecode output I see you are on BIOS version X450LCP.207, and there is version 208 available for download on Asus website. I'm curious to know if it changes the old behavior (with the patches you listed reverted), but I'm not responsible if a BIOS update breaks your machine in any way, so just do it if you this is something you are comfortable with and understand and assume all the risks yourself. We have been reporting machines with the old behavior back to Asus, but I don't know what they are doing with that information, if anything. I'm adding your machine with the old BIOS version to the list, so if you test the new BIOS let me know so I can add that as well. But please don't feel any pressure to update the BIOS if this is something you would not do otherwise. Best regards, -- João Paulo Rechi Vita
Re: [PATCH 3/3] iwlwifi: Load firmware exclusively for Intel WiFi
usb 1-7: new full-speed USB device number 7 using xhci_hcd Jan 09 16:54:32 endless kernel: usb 1-7: New USB device found, idVendor=8087, idProduct=0a2b, bcdDevice= 0.01 Jan 09 16:54:32 endless kernel: usb 1-7: New USB device strings: Mfr=0, Product=0, SerialNumber=0 Jan 09 16:54:32 endless kernel: Bluetooth: hci0: Bootloader revision 0.0 build 2 week 52 2014 Jan 09 16:54:32 endless kernel: Bluetooth: hci0: Device revision is 5 Jan 09 16:54:32 endless kernel: Bluetooth: hci0: Secure boot is enabled Jan 09 16:54:32 endless kernel: Bluetooth: hci0: OTP lock is enabled Jan 09 16:54:32 endless kernel: Bluetooth: hci0: API lock is enabled Jan 09 16:54:32 endless kernel: Bluetooth: hci0: Debug lock is disabled Jan 09 16:54:32 endless kernel: Bluetooth: hci0: Minimum firmware build 1 week 10 2014 Jan 09 16:54:32 endless kernel: Bluetooth: hci0: Found device firmware: intel/ibt-11-5.sfi Jan 09 16:54:34 endless kernel: Bluetooth: hci0: Waiting for firmware download to complete Jan 09 16:54:34 endless kernel: Bluetooth: hci0: Firmware loaded in 1820173 usecs Jan 09 16:54:34 endless kernel: Bluetooth: hci0: Waiting for device to boot Jan 09 16:54:34 endless kernel: Bluetooth: hci0: Device booted in 11761 usecs Jan 09 16:54:34 endless kernel: Bluetooth: hci0: Found Intel DDC parameters: intel/ibt-11-5.ddc Jan 09 16:54:34 endless kernel: Bluetooth: hci0: Applying Intel DDC parameters completed Jan 09 16:54:34 endless kernel: Bluetooth: RFCOMM TTY layer initialized Jan 09 16:54:34 endless kernel: Bluetooth: RFCOMM socket layer initialized Jan 09 16:54:34 endless kernel: Bluetooth: RFCOMM ver 1.11 -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH AUTOSEL 4.20 008/117] platform/x86: asus-wmi: Tell the EC the OS will handle the display off hotkey
Hello Sasha, On Tue, Jan 8, 2019 at 11:26 AM Sasha Levin wrote: > > From: João Paulo Rechi Vita > > [ Upstream commit 78f3ac76d9e5219589718b9e4733bee21627b3f5 ] > > In the past, Asus firmwares would change the panel backlight directly > through the EC when the display off hotkey (Fn+F7) was pressed, and > only notify the OS of such change, with 0x33 when the LCD was ON and > 0x34 when the LCD was OFF. These are currently mapped to > KEY_DISPLAYTOGGLE and KEY_DISPLAY_OFF, respectively. > > Most recently the EC on Asus most machines lost ability to toggle the > LCD backlight directly, but unless the OS informs the firmware it is > going to handle the display toggle hotkey events, the firmware still > tries change the brightness through the EC, to no effect. The end result > is a long list (at Endless we counted 11) of Asus laptop models where > the display toggle hotkey does not perform any action. Our firmware > engineers contacts at Asus were surprised that there were still machines > out there with the old behavior. > > Calling WMNB(ASUS_WMI_DEVID_BACKLIGHT==0x00050011, 2) on the _WDG device > tells the firmware that it should let the OS handle the display toggle > event, in which case it will simply notify the OS of a key press with > 0x35, as shown by the DSDT excerpts bellow. > > Scope (_SB) > { > (...) > > Device (ATKD) > { > (...) > > Name (_WDG, Buffer (0x28) > { > /* */ 0xD0, 0x5E, 0x84, 0x97, 0x6D, 0x4E, 0xDE, 0x11, > /* 0008 */ 0x8A, 0x39, 0x08, 0x00, 0x20, 0x0C, 0x9A, 0x66, > /* 0010 */ 0x4E, 0x42, 0x01, 0x02, 0x35, 0xBB, 0x3C, 0x0B, > /* 0018 */ 0xC2, 0xE3, 0xED, 0x45, 0x91, 0xC2, 0x4C, 0x5A, > /* 0020 */ 0x6D, 0x19, 0x5D, 0x1C, 0xFF, 0x00, 0x01, 0x08 > }) > Method (WMNB, 3, Serialized) > { > CreateDWordField (Arg2, Zero, IIA0) > CreateDWordField (Arg2, 0x04, IIA1) > Local0 = (Arg1 & 0x) > > (...) > > If ((Local0 == 0x53564544)) > { > (...) > > If ((IIA0 == 0x00050011)) > { > If ((IIA1 == 0x02)) > { > ^^PCI0.SBRG.EC0.SPIN (0x72, One) > ^^PCI0.SBRG.EC0.BLCT = One > } > > Return (One) > } > } > (...) > } > (...) > } > (...) > } > (...) > > Scope (_SB.PCI0.SBRG.EC0) > { > (...) > > Name (BLCT, Zero) > > (...) > > Method (_Q10, 0, NotSerialized) // _Qxx: EC Query > { > If ((BLCT == Zero)) > { > Local0 = One > Local0 = RPIN (0x72) > Local0 ^= One > SPIN (0x72, Local0) > If (ATKP) > { > Local0 = (0x34 - Local0) > ATKD.IANE (Local0) > } > } > ElseIf ((BLCT == One)) > { > If (ATKP) > { > ATKD.IANE (0x35) > } > } > } > (...) > } > > Signed-off-by: João Paulo Rechi Vita > Signed-off-by: Andy Shevchenko > Signed-off-by: Sasha Levin I am not entirely sure this is linux-stable material. This patch makes the "turn off the display backlight" hotkey work on some Asus machines where, without this patch, the key would simply do nothing. It seems to me this is more of a new feature support than a bug fix. That said, if you or Andy think this should go to stable after this short explanation, I'll not object it. Thanks and best regards, -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH 3/3] iwlwifi: Load firmware exclusively for Intel WiFi
Hello Marcel, On Wed, Dec 5, 2018 at 11:27 AM João Paulo Rechi Vita wrote: > > Hello Marcel, > > On Fri, Nov 9, 2018 at 4:36 PM João Paulo Rechi Vita > wrote: > > > > Hello Marcel, > > > > On Thu, Nov 8, 2018 at 11:49 PM Marcel Holtmann wrote: > > > > > > our hardware teams from the Bluetooth and WiFi side really need to look > > > at this. > > Were you able to get attention from the hardware teams with the logs > I've provided? Are there any news or an idea of when / if we can > expect this to be fixed in firmware? If not, do you have suggestions > for an alternative solution? > Sorry to bother you again with this, but I'd really like to figure out some way forward here. Did you get any feedback from the hardware teams? Otherwise, I understand having an inter-dependency between the wifi and bt kernel modules is not desirable, so do you have any suggestion on how to solve this without adding this dependency? -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH] ACPI / battery: Fix reporting "Not charging" when capacity is 100%
On Sun, Nov 11, 2018 at 4:22 AM Pavel Machek wrote: > > On Sun 2018-11-11 12:57:12, Hans de Goede wrote: > > Hi, > > > > On 11/7/18 5:53 AM, Daniel Drake wrote: > > >On Mon, Nov 5, 2018 at 1:19 AM Pavel Machek wrote: > > >>Plus, I don't think "100% charge" is right test for "battery full". At > > >>least on thinkpads, there's configuration option, and it is common > > >>_not_ to charge batterry above 95% or so (to increase its lifetime). > > > > > >Hans also touched on this area in his response: > > > > > >>As for this kernel-side fix I do not believe that fixing thus in > > >>the kernel is the right thing to do. We try to stay away from > > >>heuristics using full_charge_capacity in the kernel since that > > >>is not really reliable / deterministic. > > > > > >I'm not fully convinced by this argument though. > > > > > >The ACPI spec is not very clear on what conditions you should apply to > > >decide when the battery is full. Instead, ACPI seems to provide a > > >pretty decent amount of data, and the decision about whether to > > >interpret that as "battery full" is left for consumers. > > > > Right, but in this case the "discharging" status bit is explicitly > > set, to me it feels wrong to report "full", when the firmware > > is reporting "discharging" IMHO, at best we are "not charging" > > (on AC, below the threshold where a new charge cycle starts) and > > that is what we are currently reporting. > > > > Anu heurstics to decide that "not charging" is close enough to full > > to report it as full to the user belongs in userspace IMHO. > > > > Anyways this ultimately is Rafael's call. If Rafael is ok with this > > patch then I would like to see Pavel's comment addressed and otherwise > > it is fine with me. > > If we can get to an agreement on this I'll send a v2 without division. > > Note that we will still often get the case where a laptop is charged, > > reports full, is unplugged for 5 minutes and then replugged and then > > reports a capacity of 97% combined with "not charging", so we will > > still need to fix userspace to handle this. > Yes, I agree that should be addressed in userspace, as it is pretty much a policy decision. > For the record, I don't think I'm okay with this. > > There's nothing special about 100% charge. > I don't agree there is nothing special about 100% charge. There is a separate state to represent battery full for a reason, which is the user wanting to know when their battery is 100% charged and not being discharged. > This changes userland ABI and I don't think it has good enough reasons to do that. > This only changes which state will be reported when the battery is 100% charged and not discharging, it does not introduce / remove any values. I don't think that is considered ABI change, and on other hardware like the Dell Latitude 5480, /sys/class/power_supply//status already reports "Full" under these conditions. I still believe it is a bug that makes the ABI inconsistent across different hardware. -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH] ACPI / battery: Fix reporting "Not charging" when capacity is 100%
On Sun, Nov 11, 2018 at 4:22 AM Pavel Machek wrote: > > On Sun 2018-11-11 12:57:12, Hans de Goede wrote: > > Hi, > > > > On 11/7/18 5:53 AM, Daniel Drake wrote: > > >On Mon, Nov 5, 2018 at 1:19 AM Pavel Machek wrote: > > >>Plus, I don't think "100% charge" is right test for "battery full". At > > >>least on thinkpads, there's configuration option, and it is common > > >>_not_ to charge batterry above 95% or so (to increase its lifetime). > > > > > >Hans also touched on this area in his response: > > > > > >>As for this kernel-side fix I do not believe that fixing thus in > > >>the kernel is the right thing to do. We try to stay away from > > >>heuristics using full_charge_capacity in the kernel since that > > >>is not really reliable / deterministic. > > > > > >I'm not fully convinced by this argument though. > > > > > >The ACPI spec is not very clear on what conditions you should apply to > > >decide when the battery is full. Instead, ACPI seems to provide a > > >pretty decent amount of data, and the decision about whether to > > >interpret that as "battery full" is left for consumers. > > > > Right, but in this case the "discharging" status bit is explicitly > > set, to me it feels wrong to report "full", when the firmware > > is reporting "discharging" IMHO, at best we are "not charging" > > (on AC, below the threshold where a new charge cycle starts) and > > that is what we are currently reporting. > > > > Anu heurstics to decide that "not charging" is close enough to full > > to report it as full to the user belongs in userspace IMHO. > > > > Anyways this ultimately is Rafael's call. If Rafael is ok with this > > patch then I would like to see Pavel's comment addressed and otherwise > > it is fine with me. > > If we can get to an agreement on this I'll send a v2 without division. > > Note that we will still often get the case where a laptop is charged, > > reports full, is unplugged for 5 minutes and then replugged and then > > reports a capacity of 97% combined with "not charging", so we will > > still need to fix userspace to handle this. > Yes, I agree that should be addressed in userspace, as it is pretty much a policy decision. > For the record, I don't think I'm okay with this. > > There's nothing special about 100% charge. > I don't agree there is nothing special about 100% charge. There is a separate state to represent battery full for a reason, which is the user wanting to know when their battery is 100% charged and not being discharged. > This changes userland ABI and I don't think it has good enough reasons to do that. > This only changes which state will be reported when the battery is 100% charged and not discharging, it does not introduce / remove any values. I don't think that is considered ABI change, and on other hardware like the Dell Latitude 5480, /sys/class/power_supply//status already reports "Full" under these conditions. I still believe it is a bug that makes the ABI inconsistent across different hardware. -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH] ACPI / battery: Fix reporting "Not charging" when capacity is 100%
On Mon, Nov 5, 2018 at 1:19 AM Pavel Machek wrote: > > On Fri 2018-11-02 23:57:32, João Paulo Rechi Vita wrote: > > Commit 19fffc8450d4378580a8f019b195c4617083176f fixed reporting > > "Discharging" on some machines when AC was connected but the battery was > > not charging. But now on these machines the battery status is reported > > as "Not charging" even when the battery is fully charged. > > > > This commit takes the battery capacity into consideration when checking > > if "Not charging" should be returned and "Full" is returned when the > > capacity is 100%. > > > > Signed-off-by: João Paulo Rechi Vita > > We have people trying to modify this and it caused regressions in > MATE, IIRC. > Do you have any pointers for further information? > Plus, I don't think "100% charge" is right test for "battery full". At > least on thinkpads, there's configuration option, and it is common > _not_ to charge batterry above 95% or so (to increase its lifetime). > This will only affect machines where the firmware wrongly reports the battery state as discharging, which the commit mentioned on this commit message fixed so we now report it as not charging. From your comment it does not sound like thinkpads are in this category. In any case deciding to report battery full for any percentage other than 100% is a policy decision, which is normally left out of the kernel. > > > >* was plugged in and the device thus did not start a new charge > > cycle. > >*/ > > if ((battery_ac_is_broken || power_supply_is_system_supplied()) && > > - battery->rate_now == 0) > > + battery->rate_now == 0) { > > + if (battery->capacity_now && battery->full_charge_capacity && > > + battery->capacity_now / battery->full_charge_capacity == > > 1) > > + return POWER_SUPPLY_STATUS_FULL; > > Division? Really? > If you look further down in acpi_battery_get_property, that is how the capacity property is calculated. Do you have a better suggestion? -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH] ACPI / battery: Fix reporting "Not charging" when capacity is 100%
On Mon, Nov 5, 2018 at 1:19 AM Pavel Machek wrote: > > On Fri 2018-11-02 23:57:32, João Paulo Rechi Vita wrote: > > Commit 19fffc8450d4378580a8f019b195c4617083176f fixed reporting > > "Discharging" on some machines when AC was connected but the battery was > > not charging. But now on these machines the battery status is reported > > as "Not charging" even when the battery is fully charged. > > > > This commit takes the battery capacity into consideration when checking > > if "Not charging" should be returned and "Full" is returned when the > > capacity is 100%. > > > > Signed-off-by: João Paulo Rechi Vita > > We have people trying to modify this and it caused regressions in > MATE, IIRC. > Do you have any pointers for further information? > Plus, I don't think "100% charge" is right test for "battery full". At > least on thinkpads, there's configuration option, and it is common > _not_ to charge batterry above 95% or so (to increase its lifetime). > This will only affect machines where the firmware wrongly reports the battery state as discharging, which the commit mentioned on this commit message fixed so we now report it as not charging. From your comment it does not sound like thinkpads are in this category. In any case deciding to report battery full for any percentage other than 100% is a policy decision, which is normally left out of the kernel. > > > >* was plugged in and the device thus did not start a new charge > > cycle. > >*/ > > if ((battery_ac_is_broken || power_supply_is_system_supplied()) && > > - battery->rate_now == 0) > > + battery->rate_now == 0) { > > + if (battery->capacity_now && battery->full_charge_capacity && > > + battery->capacity_now / battery->full_charge_capacity == > > 1) > > + return POWER_SUPPLY_STATUS_FULL; > > Division? Really? > If you look further down in acpi_battery_get_property, that is how the capacity property is calculated. Do you have a better suggestion? -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH] ACPI / battery: Fix reporting "Not charging" when capacity is 100%
Hello Hans, thanks for your quick response, On Sat, Nov 3, 2018 at 4:28 AM Hans de Goede wrote: > > Hi, > > On 03-11-18 07:57, João Paulo Rechi Vita wrote: > > Commit 19fffc8450d4378580a8f019b195c4617083176f fixed reporting > > "Discharging" on some machines when AC was connected but the battery was > > not charging. But now on these machines the battery status is reported > > as "Not charging" even when the battery is fully charged. > > > > This commit takes the battery capacity into consideration when checking > > if "Not charging" should be returned and "Full" is returned when the > > capacity is 100%. > > > > Signed-off-by: João Paulo Rechi Vita > > acpi_battery_handle_discharging() only gets called if the > ACPI_BATTERY_STATE_DISCHARGING > bit is set by the firmware in that case if we are not actually discharging > returning POWER_SUPPLY_STATUS_NOT_CHARGING is the only correct thing to do, > we should never return POWER_SUPPLY_STATUS_FULL when the > ACPI_BATTERY_STATE_DISCHARGING bit is set. > I understand the reason behind your original patch, but can you elaborate on why would it be wrong to return POWER_SUPPLY_STATUS_FULL if the battery is full and we know it is not actually discharging? IIUC that is the state returned on hw / firmware that behaves correctly when there is AC power and the battery has 100% of charge. > I was about to point you to the upower bug for upower not > handling POWER_SUPPLY_STATUS_NOT_CHARGING as well as it > could atm, but I see you've already found that and are working > on fixing that. That is great, thank you. > Yes, I'm trying to fix this problem, but it touches different points across the stack and there are different possible approaches for how to present the "Not charging" state to the user. This patch is not necessarily part of, although closely related to, the bigger problem of better handling "Not charging". It actually addresses the problem that the kernel returns "Not charging" when AC is present and the battery is fully charged, which seems wrong to me, despite of "Not charging" not being strictly defined anywhere (at least that I know of). > As for this kernel-side fix I do not believe that fixing thus in > the kernel is the right thing to do. We try to stay away from > heuristics using full_charge_capacity in the kernel since that > is not really reliable / deterministic. > At first I thought that returning POWER_SUPPLY_STATUS_FULL when the battery is full and AC is present was deterministic enough to have it in the kernel. But I don't have any prior experience working with this kind of hardware, so I may very well be wrong on this assumption. It would be great to get some extra clarification though. Thanks, -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH] ACPI / battery: Fix reporting "Not charging" when capacity is 100%
Hello Hans, thanks for your quick response, On Sat, Nov 3, 2018 at 4:28 AM Hans de Goede wrote: > > Hi, > > On 03-11-18 07:57, João Paulo Rechi Vita wrote: > > Commit 19fffc8450d4378580a8f019b195c4617083176f fixed reporting > > "Discharging" on some machines when AC was connected but the battery was > > not charging. But now on these machines the battery status is reported > > as "Not charging" even when the battery is fully charged. > > > > This commit takes the battery capacity into consideration when checking > > if "Not charging" should be returned and "Full" is returned when the > > capacity is 100%. > > > > Signed-off-by: João Paulo Rechi Vita > > acpi_battery_handle_discharging() only gets called if the > ACPI_BATTERY_STATE_DISCHARGING > bit is set by the firmware in that case if we are not actually discharging > returning POWER_SUPPLY_STATUS_NOT_CHARGING is the only correct thing to do, > we should never return POWER_SUPPLY_STATUS_FULL when the > ACPI_BATTERY_STATE_DISCHARGING bit is set. > I understand the reason behind your original patch, but can you elaborate on why would it be wrong to return POWER_SUPPLY_STATUS_FULL if the battery is full and we know it is not actually discharging? IIUC that is the state returned on hw / firmware that behaves correctly when there is AC power and the battery has 100% of charge. > I was about to point you to the upower bug for upower not > handling POWER_SUPPLY_STATUS_NOT_CHARGING as well as it > could atm, but I see you've already found that and are working > on fixing that. That is great, thank you. > Yes, I'm trying to fix this problem, but it touches different points across the stack and there are different possible approaches for how to present the "Not charging" state to the user. This patch is not necessarily part of, although closely related to, the bigger problem of better handling "Not charging". It actually addresses the problem that the kernel returns "Not charging" when AC is present and the battery is fully charged, which seems wrong to me, despite of "Not charging" not being strictly defined anywhere (at least that I know of). > As for this kernel-side fix I do not believe that fixing thus in > the kernel is the right thing to do. We try to stay away from > heuristics using full_charge_capacity in the kernel since that > is not really reliable / deterministic. > At first I thought that returning POWER_SUPPLY_STATUS_FULL when the battery is full and AC is present was deterministic enough to have it in the kernel. But I don't have any prior experience working with this kind of hardware, so I may very well be wrong on this assumption. It would be great to get some extra clarification though. Thanks, -- João Paulo Rechi Vita http://about.me/jprvita
[PATCH] ACPI / battery: Fix reporting "Not charging" when capacity is 100%
Commit 19fffc8450d4378580a8f019b195c4617083176f fixed reporting "Discharging" on some machines when AC was connected but the battery was not charging. But now on these machines the battery status is reported as "Not charging" even when the battery is fully charged. This commit takes the battery capacity into consideration when checking if "Not charging" should be returned and "Full" is returned when the capacity is 100%. Signed-off-by: João Paulo Rechi Vita --- drivers/acpi/battery.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index cb97b6105f52..82e194290f01 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -217,8 +217,12 @@ static int acpi_battery_handle_discharging(struct acpi_battery *battery) * was plugged in and the device thus did not start a new charge cycle. */ if ((battery_ac_is_broken || power_supply_is_system_supplied()) && - battery->rate_now == 0) + battery->rate_now == 0) { + if (battery->capacity_now && battery->full_charge_capacity && + battery->capacity_now / battery->full_charge_capacity == 1) + return POWER_SUPPLY_STATUS_FULL; return POWER_SUPPLY_STATUS_NOT_CHARGING; + } return POWER_SUPPLY_STATUS_DISCHARGING; } -- 2.19.1
[PATCH] ACPI / battery: Fix reporting "Not charging" when capacity is 100%
Commit 19fffc8450d4378580a8f019b195c4617083176f fixed reporting "Discharging" on some machines when AC was connected but the battery was not charging. But now on these machines the battery status is reported as "Not charging" even when the battery is fully charged. This commit takes the battery capacity into consideration when checking if "Not charging" should be returned and "Full" is returned when the capacity is 100%. Signed-off-by: João Paulo Rechi Vita --- drivers/acpi/battery.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index cb97b6105f52..82e194290f01 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -217,8 +217,12 @@ static int acpi_battery_handle_discharging(struct acpi_battery *battery) * was plugged in and the device thus did not start a new charge cycle. */ if ((battery_ac_is_broken || power_supply_is_system_supplied()) && - battery->rate_now == 0) + battery->rate_now == 0) { + if (battery->capacity_now && battery->full_charge_capacity && + battery->capacity_now / battery->full_charge_capacity == 1) + return POWER_SUPPLY_STATUS_FULL; return POWER_SUPPLY_STATUS_NOT_CHARGING; + } return POWER_SUPPLY_STATUS_DISCHARGING; } -- 2.19.1
[PATCH 2/3] asus-nb-wmi: Map 0x35 to KEY_SCREENLOCK
When the OS registers to handle events from the display off hotkey the EC will send a notification with 0x35 for every key press, independent of the backlight state. The behavior of this key on Windows, with the ATKACPI driver from Asus installed, is turning off the backlight of all connected displays with a fading effect, and any cursor input or key press turning the backlight back on. The key press or cursor input that wakes up the display is also passed through to the application under the cursor or under focus. The key that matches this behavior the closest is KEY_SCREENLOCK. Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-nb-wmi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c index db2af09067db..5a00a7665f9b 100644 --- a/drivers/platform/x86/asus-nb-wmi.c +++ b/drivers/platform/x86/asus-nb-wmi.c @@ -444,6 +444,7 @@ static const struct key_entry asus_nb_wmi_keymap[] = { { KE_KEY, 0x32, { KEY_MUTE } }, { KE_KEY, 0x33, { KEY_DISPLAYTOGGLE } }, /* LCD on */ { KE_KEY, 0x34, { KEY_DISPLAY_OFF } }, /* LCD off */ + { KE_KEY, 0x35, { KEY_SCREENLOCK } }, { KE_KEY, 0x40, { KEY_PREVIOUSSONG } }, { KE_KEY, 0x41, { KEY_NEXTSONG } }, { KE_KEY, 0x43, { KEY_STOPCD } }, /* Stop/Eject */ -- 2.19.1
[PATCH 3/3] asus-nb-wmi: Drop mapping of 0x33 and 0x34 scan codes
According to Asus firmware engineers, the meaning of these codes is only to notify the OS that the screen brightness has been turned on/off by the EC. This does not match the meaning of KEY_DISPLAYTOGGLE / KEY_DISPLAY_OFF, where userspace is expected to change the display brightness. Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-nb-wmi.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c index 5a00a7665f9b..b6f2ff95c3ed 100644 --- a/drivers/platform/x86/asus-nb-wmi.c +++ b/drivers/platform/x86/asus-nb-wmi.c @@ -442,8 +442,6 @@ static const struct key_entry asus_nb_wmi_keymap[] = { { KE_KEY, 0x30, { KEY_VOLUMEUP } }, { KE_KEY, 0x31, { KEY_VOLUMEDOWN } }, { KE_KEY, 0x32, { KEY_MUTE } }, - { KE_KEY, 0x33, { KEY_DISPLAYTOGGLE } }, /* LCD on */ - { KE_KEY, 0x34, { KEY_DISPLAY_OFF } }, /* LCD off */ { KE_KEY, 0x35, { KEY_SCREENLOCK } }, { KE_KEY, 0x40, { KEY_PREVIOUSSONG } }, { KE_KEY, 0x41, { KEY_NEXTSONG } }, -- 2.19.1
[PATCH 2/3] asus-nb-wmi: Map 0x35 to KEY_SCREENLOCK
When the OS registers to handle events from the display off hotkey the EC will send a notification with 0x35 for every key press, independent of the backlight state. The behavior of this key on Windows, with the ATKACPI driver from Asus installed, is turning off the backlight of all connected displays with a fading effect, and any cursor input or key press turning the backlight back on. The key press or cursor input that wakes up the display is also passed through to the application under the cursor or under focus. The key that matches this behavior the closest is KEY_SCREENLOCK. Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-nb-wmi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c index db2af09067db..5a00a7665f9b 100644 --- a/drivers/platform/x86/asus-nb-wmi.c +++ b/drivers/platform/x86/asus-nb-wmi.c @@ -444,6 +444,7 @@ static const struct key_entry asus_nb_wmi_keymap[] = { { KE_KEY, 0x32, { KEY_MUTE } }, { KE_KEY, 0x33, { KEY_DISPLAYTOGGLE } }, /* LCD on */ { KE_KEY, 0x34, { KEY_DISPLAY_OFF } }, /* LCD off */ + { KE_KEY, 0x35, { KEY_SCREENLOCK } }, { KE_KEY, 0x40, { KEY_PREVIOUSSONG } }, { KE_KEY, 0x41, { KEY_NEXTSONG } }, { KE_KEY, 0x43, { KEY_STOPCD } }, /* Stop/Eject */ -- 2.19.1
[PATCH 3/3] asus-nb-wmi: Drop mapping of 0x33 and 0x34 scan codes
According to Asus firmware engineers, the meaning of these codes is only to notify the OS that the screen brightness has been turned on/off by the EC. This does not match the meaning of KEY_DISPLAYTOGGLE / KEY_DISPLAY_OFF, where userspace is expected to change the display brightness. Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-nb-wmi.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c index 5a00a7665f9b..b6f2ff95c3ed 100644 --- a/drivers/platform/x86/asus-nb-wmi.c +++ b/drivers/platform/x86/asus-nb-wmi.c @@ -442,8 +442,6 @@ static const struct key_entry asus_nb_wmi_keymap[] = { { KE_KEY, 0x30, { KEY_VOLUMEUP } }, { KE_KEY, 0x31, { KEY_VOLUMEDOWN } }, { KE_KEY, 0x32, { KEY_MUTE } }, - { KE_KEY, 0x33, { KEY_DISPLAYTOGGLE } }, /* LCD on */ - { KE_KEY, 0x34, { KEY_DISPLAY_OFF } }, /* LCD off */ { KE_KEY, 0x35, { KEY_SCREENLOCK } }, { KE_KEY, 0x40, { KEY_PREVIOUSSONG } }, { KE_KEY, 0x41, { KEY_NEXTSONG } }, -- 2.19.1
[PATCH 0/3] Fix display off hotkey on Asus machines
Asus laptops have a hotkey function on the F7 key to turn the display backlight OFF, labeled with a screen with a X inside, as shown on https://dlcdnimgs.asus.com/websites/global/products/Xep1ZcSY8dyWXK11/images/keyboard.png This hotkey worked on very few Asus models, where the EC acts on the backlight and input events are generated only to notify userspace of the backlight status. On these machines the first hotkey press turns the display backlight OFF and notifies the OS with 0x34, which asus-nb-wmi forwards to userspace as KEY_DISPLAY_OFF, and a second press turns it back ON and notifies the OS with 0x33, which asus-nb-wmi forwards to userspace as KEY_DISPLAYTOGGLE. No other keys turn the display backlight back ON, but their input is forwarded normally to the application under focus. But for the majority of models, the EC actually does not act on the backlight and we simply get a 0x33 notification every time the key is pressed, or alternating values of 0x33 / 0x34. We have confirmed this behavior on the following models: E203NAS, GL553VE, X441NC, X441UVK, X541UVK, X555DG, X555UB, X555UQ, X560UD, X570ZD and X705FD, and the DSDT on these machines and the working one (only confirmed on N552VW) is the same for the query involved here. After trying to get information from Asus for quite some time on how this works on Windows, we finally recently got a contact that was able to give us a definitive answer and a specification for this feature. According this contact the 0x33 / 0x34 is an old behavior and all newer machines should only be notifying the OS with 0x35 instead, as newer ECs don't control the backlight anymore. When a 0x35 notification is received the OS should act on the display backlight. From the spec (machine-translated from Chinese): 1.4 Fn+F7 Function introduction After the user presses Fn + F7, the screen will be closed through the Windows API. The user will immediately open the screen with the mouse and keyboard. The API used can be found in the Sample URL: https://code.msdn.microsoft.com/windowsdesktop/Coalescable-Timer-Sample-d9da954c BIOS implementation Increase Notify code LCD On: 0x33 (display OSD) LCD Off: 0x34 (display OSD) LCD Switch: 0x35 (using API switch) The behavior on Windows with the ATKACPI driver from Asus installed matches what is described above, with the hotkey turning OFF the backlight of all connected displays with a fading effect, and any cursor input or key press turning the backlight back ON. The key press or cursor input is passed through to the application under focus or under the cursor. With this information from the spec, a simple analysis of the DSDT (pasted on the first commit of this series) shows that in order to have the firmware notify the OS with 0x35 and have the OS act on the backlight, we need to call WMNB(ASUS_WMI_DEVID_BACKLIGHT==0x00050011, 2) on the _WDG device. Then we can simply map that scan code to the appropriate key code in the driver. In include/uapi/linux/input-event-codes.h KEY_DISPLAY_OFF is defined as "display device to off state", but it is not actually handled by userspace on a GNOME+Xorg stack. There are also KEY_SCREENSAVER and KEY_SCREENLOCK / KEY_COFFEE. The former seems to be ignored by userspace as well (and its value is higher than 255 so it can't be handled by Xorg IIUC), and the later is mapped to XF86ScreenSaver by Xorg and triggers the lock screen action on GNOME. KEY_SCREENLOCK seems to be what closest matches the behavior described above in a Xorg world. I am not sure if any of this changes with Wayland, so any clarifications in that regard would be greatly appreciated. João Paulo Rechi Vita (3): asus-wmi: Tell the EC the OS will handle the display off hotkey asus-nb-wmi: Map 0x35 to KEY_SCREENLOCK asus-nb-wmi: Drop mapping of 0x33 and 0x34 scan codes drivers/platform/x86/asus-nb-wmi.c | 3 +-- drivers/platform/x86/asus-wmi.c| 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) -- 2.19.1
[PATCH 1/3] asus-wmi: Tell the EC the OS will handle the display off hotkey
In the past, Asus firmwares would change the panel backlight directly through the EC when the display off hotkey (Fn+F7) was pressed, and only notify the OS of such change, with 0x33 when the LCD was ON and 0x34 when the LCD was OFF. These are currently mapped to KEY_DISPLAYTOGGLE and KEY_DISPLAY_OFF, respectively. Most recently the EC on Asus most machines lost ability to toggle the LCD backlight directly, but unless the OS informs the firmware it is going to handle the display toggle hotkey events, the firmware still tries change the brightness through the EC, to no effect. The end result is a long list (at Endless we counted 11) of Asus laptop models where the display toggle hotkey does not perform any action. Our firmware engineers contacts at Asus were surprised that there were still machines out there with the old behavior. Calling WMNB(ASUS_WMI_DEVID_BACKLIGHT==0x00050011, 2) on the _WDG device tells the firmware that it should let the OS handle the display toggle event, in which case it will simply notify the OS of a key press with 0x35, as shown by the DSDT excerpts bellow. Scope (_SB) { (...) Device (ATKD) { (...) Name (_WDG, Buffer (0x28) { /* */ 0xD0, 0x5E, 0x84, 0x97, 0x6D, 0x4E, 0xDE, 0x11, /* 0008 */ 0x8A, 0x39, 0x08, 0x00, 0x20, 0x0C, 0x9A, 0x66, /* 0010 */ 0x4E, 0x42, 0x01, 0x02, 0x35, 0xBB, 0x3C, 0x0B, /* 0018 */ 0xC2, 0xE3, 0xED, 0x45, 0x91, 0xC2, 0x4C, 0x5A, /* 0020 */ 0x6D, 0x19, 0x5D, 0x1C, 0xFF, 0x00, 0x01, 0x08 }) Method (WMNB, 3, Serialized) { CreateDWordField (Arg2, Zero, IIA0) CreateDWordField (Arg2, 0x04, IIA1) Local0 = (Arg1 & 0x) (...) If ((Local0 == 0x53564544)) { (...) If ((IIA0 == 0x00050011)) { If ((IIA1 == 0x02)) { ^^PCI0.SBRG.EC0.SPIN (0x72, One) ^^PCI0.SBRG.EC0.BLCT = One } Return (One) } } (...) } (...) } (...) } (...) Scope (_SB.PCI0.SBRG.EC0) { (...) Name (BLCT, Zero) (...) Method (_Q10, 0, NotSerialized) // _Qxx: EC Query { If ((BLCT == Zero)) { Local0 = One Local0 = RPIN (0x72) Local0 ^= One SPIN (0x72, Local0) If (ATKP) { Local0 = (0x34 - Local0) ATKD.IANE (Local0) } } ElseIf ((BLCT == One)) { If (ATKP) { ATKD.IANE (0x35) } } } (...) } Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-wmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index b52b192a4f16..c25b946bb602 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -2150,7 +2150,8 @@ static int asus_wmi_add(struct platform_device *pdev) err = asus_wmi_backlight_init(asus); if (err && err != -ENODEV) goto fail_backlight; - } + } else + err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL); status = wmi_install_notify_handler(asus->driver->event_guid, asus_wmi_notify, asus); -- 2.19.1
[PATCH 0/3] Fix display off hotkey on Asus machines
Asus laptops have a hotkey function on the F7 key to turn the display backlight OFF, labeled with a screen with a X inside, as shown on https://dlcdnimgs.asus.com/websites/global/products/Xep1ZcSY8dyWXK11/images/keyboard.png This hotkey worked on very few Asus models, where the EC acts on the backlight and input events are generated only to notify userspace of the backlight status. On these machines the first hotkey press turns the display backlight OFF and notifies the OS with 0x34, which asus-nb-wmi forwards to userspace as KEY_DISPLAY_OFF, and a second press turns it back ON and notifies the OS with 0x33, which asus-nb-wmi forwards to userspace as KEY_DISPLAYTOGGLE. No other keys turn the display backlight back ON, but their input is forwarded normally to the application under focus. But for the majority of models, the EC actually does not act on the backlight and we simply get a 0x33 notification every time the key is pressed, or alternating values of 0x33 / 0x34. We have confirmed this behavior on the following models: E203NAS, GL553VE, X441NC, X441UVK, X541UVK, X555DG, X555UB, X555UQ, X560UD, X570ZD and X705FD, and the DSDT on these machines and the working one (only confirmed on N552VW) is the same for the query involved here. After trying to get information from Asus for quite some time on how this works on Windows, we finally recently got a contact that was able to give us a definitive answer and a specification for this feature. According this contact the 0x33 / 0x34 is an old behavior and all newer machines should only be notifying the OS with 0x35 instead, as newer ECs don't control the backlight anymore. When a 0x35 notification is received the OS should act on the display backlight. From the spec (machine-translated from Chinese): 1.4 Fn+F7 Function introduction After the user presses Fn + F7, the screen will be closed through the Windows API. The user will immediately open the screen with the mouse and keyboard. The API used can be found in the Sample URL: https://code.msdn.microsoft.com/windowsdesktop/Coalescable-Timer-Sample-d9da954c BIOS implementation Increase Notify code LCD On: 0x33 (display OSD) LCD Off: 0x34 (display OSD) LCD Switch: 0x35 (using API switch) The behavior on Windows with the ATKACPI driver from Asus installed matches what is described above, with the hotkey turning OFF the backlight of all connected displays with a fading effect, and any cursor input or key press turning the backlight back ON. The key press or cursor input is passed through to the application under focus or under the cursor. With this information from the spec, a simple analysis of the DSDT (pasted on the first commit of this series) shows that in order to have the firmware notify the OS with 0x35 and have the OS act on the backlight, we need to call WMNB(ASUS_WMI_DEVID_BACKLIGHT==0x00050011, 2) on the _WDG device. Then we can simply map that scan code to the appropriate key code in the driver. In include/uapi/linux/input-event-codes.h KEY_DISPLAY_OFF is defined as "display device to off state", but it is not actually handled by userspace on a GNOME+Xorg stack. There are also KEY_SCREENSAVER and KEY_SCREENLOCK / KEY_COFFEE. The former seems to be ignored by userspace as well (and its value is higher than 255 so it can't be handled by Xorg IIUC), and the later is mapped to XF86ScreenSaver by Xorg and triggers the lock screen action on GNOME. KEY_SCREENLOCK seems to be what closest matches the behavior described above in a Xorg world. I am not sure if any of this changes with Wayland, so any clarifications in that regard would be greatly appreciated. João Paulo Rechi Vita (3): asus-wmi: Tell the EC the OS will handle the display off hotkey asus-nb-wmi: Map 0x35 to KEY_SCREENLOCK asus-nb-wmi: Drop mapping of 0x33 and 0x34 scan codes drivers/platform/x86/asus-nb-wmi.c | 3 +-- drivers/platform/x86/asus-wmi.c| 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) -- 2.19.1
[PATCH 1/3] asus-wmi: Tell the EC the OS will handle the display off hotkey
In the past, Asus firmwares would change the panel backlight directly through the EC when the display off hotkey (Fn+F7) was pressed, and only notify the OS of such change, with 0x33 when the LCD was ON and 0x34 when the LCD was OFF. These are currently mapped to KEY_DISPLAYTOGGLE and KEY_DISPLAY_OFF, respectively. Most recently the EC on Asus most machines lost ability to toggle the LCD backlight directly, but unless the OS informs the firmware it is going to handle the display toggle hotkey events, the firmware still tries change the brightness through the EC, to no effect. The end result is a long list (at Endless we counted 11) of Asus laptop models where the display toggle hotkey does not perform any action. Our firmware engineers contacts at Asus were surprised that there were still machines out there with the old behavior. Calling WMNB(ASUS_WMI_DEVID_BACKLIGHT==0x00050011, 2) on the _WDG device tells the firmware that it should let the OS handle the display toggle event, in which case it will simply notify the OS of a key press with 0x35, as shown by the DSDT excerpts bellow. Scope (_SB) { (...) Device (ATKD) { (...) Name (_WDG, Buffer (0x28) { /* */ 0xD0, 0x5E, 0x84, 0x97, 0x6D, 0x4E, 0xDE, 0x11, /* 0008 */ 0x8A, 0x39, 0x08, 0x00, 0x20, 0x0C, 0x9A, 0x66, /* 0010 */ 0x4E, 0x42, 0x01, 0x02, 0x35, 0xBB, 0x3C, 0x0B, /* 0018 */ 0xC2, 0xE3, 0xED, 0x45, 0x91, 0xC2, 0x4C, 0x5A, /* 0020 */ 0x6D, 0x19, 0x5D, 0x1C, 0xFF, 0x00, 0x01, 0x08 }) Method (WMNB, 3, Serialized) { CreateDWordField (Arg2, Zero, IIA0) CreateDWordField (Arg2, 0x04, IIA1) Local0 = (Arg1 & 0x) (...) If ((Local0 == 0x53564544)) { (...) If ((IIA0 == 0x00050011)) { If ((IIA1 == 0x02)) { ^^PCI0.SBRG.EC0.SPIN (0x72, One) ^^PCI0.SBRG.EC0.BLCT = One } Return (One) } } (...) } (...) } (...) } (...) Scope (_SB.PCI0.SBRG.EC0) { (...) Name (BLCT, Zero) (...) Method (_Q10, 0, NotSerialized) // _Qxx: EC Query { If ((BLCT == Zero)) { Local0 = One Local0 = RPIN (0x72) Local0 ^= One SPIN (0x72, Local0) If (ATKP) { Local0 = (0x34 - Local0) ATKD.IANE (Local0) } } ElseIf ((BLCT == One)) { If (ATKP) { ATKD.IANE (0x35) } } } (...) } Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-wmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index b52b192a4f16..c25b946bb602 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -2150,7 +2150,8 @@ static int asus_wmi_add(struct platform_device *pdev) err = asus_wmi_backlight_init(asus); if (err && err != -ENODEV) goto fail_backlight; - } + } else + err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL); status = wmi_install_notify_handler(asus->driver->event_guid, asus_wmi_notify, asus); -- 2.19.1
[PATCH 2/2] rfkill: Create rfkill-none LED trigger
Creates a new trigger rfkill-none, as a complement to rfkill-any, which drives LEDs when any radio is enabled. The new trigger is meant to turn a LED ON whenever all radios are OFF, and turn it OFF otherwise. Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- net/rfkill/core.c | 27 --- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/net/rfkill/core.c b/net/rfkill/core.c index 6d64d14f4b0a..07235520b00f 100644 --- a/net/rfkill/core.c +++ b/net/rfkill/core.c @@ -178,6 +178,7 @@ static void rfkill_led_trigger_unregister(struct rfkill *rfkill) } static struct led_trigger rfkill_any_led_trigger; +static struct led_trigger rfkill_none_led_trigger; static struct work_struct rfkill_global_led_trigger_work; static void rfkill_global_led_trigger_worker(struct work_struct *work) @@ -195,6 +196,8 @@ static void rfkill_global_led_trigger_worker(struct work_struct *work) mutex_unlock(_global_mutex); led_trigger_event(_any_led_trigger, brightness); + led_trigger_event(_none_led_trigger, brightness == LED_OFF ? + LED_FULL : LED_OFF); } static void rfkill_global_led_trigger_event(void) @@ -202,22 +205,32 @@ static void rfkill_global_led_trigger_event(void) schedule_work(_global_led_trigger_work); } -static void rfkill_global_led_trigger_activate(struct led_classdev *led_cdev) -{ - rfkill_global_led_trigger_event(); -} - static int rfkill_global_led_trigger_register(void) { + int ret; + INIT_WORK(_global_led_trigger_work, rfkill_global_led_trigger_worker); + rfkill_any_led_trigger.name = "rfkill-any"; - rfkill_any_led_trigger.activate = rfkill_global_led_trigger_activate; - return led_trigger_register(_any_led_trigger); + ret = led_trigger_register(_any_led_trigger); + if (ret) + return ret; + + rfkill_none_led_trigger.name = "rfkill-none"; + ret = led_trigger_register(_none_led_trigger); + if (ret) + led_trigger_unregister(_any_led_trigger); + else + /* Delay activation until all global triggers are registered */ + rfkill_global_led_trigger_event(); + + return ret; } static void rfkill_global_led_trigger_unregister(void) { + led_trigger_unregister(_none_led_trigger); led_trigger_unregister(_any_led_trigger); cancel_work_sync(_global_led_trigger_work); } -- 2.17.0
[PATCH 2/2] rfkill: Create rfkill-none LED trigger
Creates a new trigger rfkill-none, as a complement to rfkill-any, which drives LEDs when any radio is enabled. The new trigger is meant to turn a LED ON whenever all radios are OFF, and turn it OFF otherwise. Signed-off-by: João Paulo Rechi Vita --- net/rfkill/core.c | 27 --- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/net/rfkill/core.c b/net/rfkill/core.c index 6d64d14f4b0a..07235520b00f 100644 --- a/net/rfkill/core.c +++ b/net/rfkill/core.c @@ -178,6 +178,7 @@ static void rfkill_led_trigger_unregister(struct rfkill *rfkill) } static struct led_trigger rfkill_any_led_trigger; +static struct led_trigger rfkill_none_led_trigger; static struct work_struct rfkill_global_led_trigger_work; static void rfkill_global_led_trigger_worker(struct work_struct *work) @@ -195,6 +196,8 @@ static void rfkill_global_led_trigger_worker(struct work_struct *work) mutex_unlock(_global_mutex); led_trigger_event(_any_led_trigger, brightness); + led_trigger_event(_none_led_trigger, brightness == LED_OFF ? + LED_FULL : LED_OFF); } static void rfkill_global_led_trigger_event(void) @@ -202,22 +205,32 @@ static void rfkill_global_led_trigger_event(void) schedule_work(_global_led_trigger_work); } -static void rfkill_global_led_trigger_activate(struct led_classdev *led_cdev) -{ - rfkill_global_led_trigger_event(); -} - static int rfkill_global_led_trigger_register(void) { + int ret; + INIT_WORK(_global_led_trigger_work, rfkill_global_led_trigger_worker); + rfkill_any_led_trigger.name = "rfkill-any"; - rfkill_any_led_trigger.activate = rfkill_global_led_trigger_activate; - return led_trigger_register(_any_led_trigger); + ret = led_trigger_register(_any_led_trigger); + if (ret) + return ret; + + rfkill_none_led_trigger.name = "rfkill-none"; + ret = led_trigger_register(_none_led_trigger); + if (ret) + led_trigger_unregister(_any_led_trigger); + else + /* Delay activation until all global triggers are registered */ + rfkill_global_led_trigger_event(); + + return ret; } static void rfkill_global_led_trigger_unregister(void) { + led_trigger_unregister(_none_led_trigger); led_trigger_unregister(_any_led_trigger); cancel_work_sync(_global_led_trigger_work); } -- 2.17.0
[PATCH] platform/x86: asus-wmi: Fix NULL pointer dereference
Do not perform the rfkill cleanup routine when (asus->driver->wlan_ctrl_by_user && ashs_present()) is true, since nothing is registered with the rfkill subsystem in that case. Doing so leads to the following kernel NULL pointer dereference: BUG: unable to handle kernel NULL pointer dereference at (null) IP: [] __mutex_lock_slowpath+0x98/0x120 PGD 1a3aa8067 PUD 1a3b3d067 PMD 0 Oops: 0002 [#1] PREEMPT SMP Modules linked in: bnep ccm binfmt_misc uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_core hid_a4tech videodev x86_pkg_temp_thermal intel_powerclamp coretemp ath3k btusb btrtl btintel bluetooth kvm_intel snd_hda_codec_hdmi kvm snd_hda_codec_realtek snd_hda_codec_generic irqbypass crc32c_intel arc4 i915 snd_hda_intel snd_hda_codec ath9k ath9k_common ath9k_hw ath i2c_algo_bit snd_hwdep mac80211 ghash_clmulni_intel snd_hda_core snd_pcm snd_timer cfg80211 ehci_pci xhci_pci drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm xhci_hcd ehci_hcd asus_nb_wmi(-) asus_wmi sparse_keymap r8169 rfkill mxm_wmi serio_raw snd mii mei_me lpc_ich i2c_i801 video soundcore mei i2c_smbus wmi i2c_core mfd_core CPU: 3 PID: 3275 Comm: modprobe Not tainted 4.9.34-gentoo #34 Hardware name: ASUSTeK COMPUTER INC. K56CM/K56CM, BIOS K56CM.206 08/21/2012 task: 8801a639ba00 task.stack: c900014cc000 RIP: 0010:[] [] __mutex_lock_slowpath+0x98/0x120 RSP: 0018:c900014cfce0 EFLAGS: 00010282 RAX: RBX: 8801a54315b0 RCX: c100 RDX: 0001 RSI: RDI: 8801a54315b4 RBP: c900014cfd30 R08: R09: 0002 R10: R11: R12: 8801a54315b4 R13: 8801a639ba00 R14: R15: 8801a54315b8 FS: 7faa254fb700() GS:8801aef8() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 0001a3b1b000 CR4: 001406e0 Stack: 8801a54315b8 814733ae c900014cfd28 8146a28c 8801a54315b0 8801a54315b0 8801a66f3820 c900014cfd48 816c73e7 Call Trace: [] ? acpi_ut_release_mutex+0x5d/0x61 [] ? acpi_ns_get_node+0x49/0x52 [] mutex_lock+0x17/0x30 [] asus_rfkill_hotplug+0x24/0x1a0 [asus_wmi] [] asus_wmi_rfkill_exit+0x61/0x150 [asus_wmi] [] asus_wmi_remove+0x61/0xb0 [asus_wmi] [] platform_drv_remove+0x28/0x40 [] __device_release_driver+0xa1/0x160 [] device_release_driver+0x23/0x30 [] bus_remove_device+0xfd/0x170 [] device_del+0x139/0x270 [] platform_device_del+0x28/0x90 [] platform_device_unregister+0x12/0x30 [] asus_wmi_unregister_driver+0x19/0x30 [asus_wmi] [] asus_nb_wmi_exit+0x10/0xf26 [asus_nb_wmi] [] SyS_delete_module+0x192/0x270 [] ? exit_to_usermode_loop+0x92/0xa0 [] entry_SYSCALL_64_fastpath+0x13/0x94 Code: e8 5e 30 00 00 8b 03 83 f8 01 0f 84 93 00 00 00 48 8b 43 10 4c 8d 7b 08 48 89 63 10 41 be ff ff ff ff 4c 89 3c 24 48 89 44 24 08 <48> 89 20 4c 89 6c 24 10 eb 1d 4c 89 e7 49 c7 45 08 02 00 00 00 RIP [] __mutex_lock_slowpath+0x98/0x120 RSP CR2: ---[ end trace 8d484233fa7cb512 ]--- note: modprobe[3275] exited with preempt_count 2 https://bugzilla.kernel.org/show_bug.cgi?id=196467 Reported-by: red.f0...@gmail.com Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/platform/x86/asus-wmi.c | 23 +-- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index ef87e78ca772..3d523ca64694 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -163,6 +163,16 @@ MODULE_LICENSE("GPL"); static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL }; +static bool ashs_present(void) +{ + int i = 0; + while (ashs_ids[i]) { + if (acpi_dev_found(ashs_ids[i++])) + return true; + } + return false; +} + struct bios_args { u32 arg0; u32 arg1; @@ -1025,6 +1035,9 @@ static int asus_new_rfkill(struct asus_wmi *asus, static void asus_wmi_rfkill_exit(struct asus_wmi *asus) { + if (asus->driver->wlan_ctrl_by_user && ashs_present()) + return; + asus_unregister_rfkill_notifier(asus, "\\_SB.PCI0.P0P5"); asus_unregister_rfkill_notifier(asus, "\\_SB.PCI0.P0P6"); asus_unregister_rfkill_notifier(asus, "\\_SB.PCI0.P0P7"); @@ -2120,16 +2133,6 @@ static int asus_wmi_fan_init(struct asus_wmi *asus) return 0; } -static bool ashs_present(void) -{ - int i = 0; - while (ashs_ids[i]) { - if (acpi_dev_found(ashs_ids[i++])) - return true; - } - return false; -} - /* * WMI Driver */ -- 2.17.0
[PATCH] platform/x86: asus-wmi: Fix NULL pointer dereference
Do not perform the rfkill cleanup routine when (asus->driver->wlan_ctrl_by_user && ashs_present()) is true, since nothing is registered with the rfkill subsystem in that case. Doing so leads to the following kernel NULL pointer dereference: BUG: unable to handle kernel NULL pointer dereference at (null) IP: [] __mutex_lock_slowpath+0x98/0x120 PGD 1a3aa8067 PUD 1a3b3d067 PMD 0 Oops: 0002 [#1] PREEMPT SMP Modules linked in: bnep ccm binfmt_misc uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_core hid_a4tech videodev x86_pkg_temp_thermal intel_powerclamp coretemp ath3k btusb btrtl btintel bluetooth kvm_intel snd_hda_codec_hdmi kvm snd_hda_codec_realtek snd_hda_codec_generic irqbypass crc32c_intel arc4 i915 snd_hda_intel snd_hda_codec ath9k ath9k_common ath9k_hw ath i2c_algo_bit snd_hwdep mac80211 ghash_clmulni_intel snd_hda_core snd_pcm snd_timer cfg80211 ehci_pci xhci_pci drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm xhci_hcd ehci_hcd asus_nb_wmi(-) asus_wmi sparse_keymap r8169 rfkill mxm_wmi serio_raw snd mii mei_me lpc_ich i2c_i801 video soundcore mei i2c_smbus wmi i2c_core mfd_core CPU: 3 PID: 3275 Comm: modprobe Not tainted 4.9.34-gentoo #34 Hardware name: ASUSTeK COMPUTER INC. K56CM/K56CM, BIOS K56CM.206 08/21/2012 task: 8801a639ba00 task.stack: c900014cc000 RIP: 0010:[] [] __mutex_lock_slowpath+0x98/0x120 RSP: 0018:c900014cfce0 EFLAGS: 00010282 RAX: RBX: 8801a54315b0 RCX: c100 RDX: 0001 RSI: RDI: 8801a54315b4 RBP: c900014cfd30 R08: R09: 0002 R10: R11: R12: 8801a54315b4 R13: 8801a639ba00 R14: R15: 8801a54315b8 FS: 7faa254fb700() GS:8801aef8() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 0001a3b1b000 CR4: 001406e0 Stack: 8801a54315b8 814733ae c900014cfd28 8146a28c 8801a54315b0 8801a54315b0 8801a66f3820 c900014cfd48 816c73e7 Call Trace: [] ? acpi_ut_release_mutex+0x5d/0x61 [] ? acpi_ns_get_node+0x49/0x52 [] mutex_lock+0x17/0x30 [] asus_rfkill_hotplug+0x24/0x1a0 [asus_wmi] [] asus_wmi_rfkill_exit+0x61/0x150 [asus_wmi] [] asus_wmi_remove+0x61/0xb0 [asus_wmi] [] platform_drv_remove+0x28/0x40 [] __device_release_driver+0xa1/0x160 [] device_release_driver+0x23/0x30 [] bus_remove_device+0xfd/0x170 [] device_del+0x139/0x270 [] platform_device_del+0x28/0x90 [] platform_device_unregister+0x12/0x30 [] asus_wmi_unregister_driver+0x19/0x30 [asus_wmi] [] asus_nb_wmi_exit+0x10/0xf26 [asus_nb_wmi] [] SyS_delete_module+0x192/0x270 [] ? exit_to_usermode_loop+0x92/0xa0 [] entry_SYSCALL_64_fastpath+0x13/0x94 Code: e8 5e 30 00 00 8b 03 83 f8 01 0f 84 93 00 00 00 48 8b 43 10 4c 8d 7b 08 48 89 63 10 41 be ff ff ff ff 4c 89 3c 24 48 89 44 24 08 <48> 89 20 4c 89 6c 24 10 eb 1d 4c 89 e7 49 c7 45 08 02 00 00 00 RIP [] __mutex_lock_slowpath+0x98/0x120 RSP CR2: ---[ end trace 8d484233fa7cb512 ]--- note: modprobe[3275] exited with preempt_count 2 https://bugzilla.kernel.org/show_bug.cgi?id=196467 Reported-by: red.f0...@gmail.com Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-wmi.c | 23 +-- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index ef87e78ca772..3d523ca64694 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -163,6 +163,16 @@ MODULE_LICENSE("GPL"); static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL }; +static bool ashs_present(void) +{ + int i = 0; + while (ashs_ids[i]) { + if (acpi_dev_found(ashs_ids[i++])) + return true; + } + return false; +} + struct bios_args { u32 arg0; u32 arg1; @@ -1025,6 +1035,9 @@ static int asus_new_rfkill(struct asus_wmi *asus, static void asus_wmi_rfkill_exit(struct asus_wmi *asus) { + if (asus->driver->wlan_ctrl_by_user && ashs_present()) + return; + asus_unregister_rfkill_notifier(asus, "\\_SB.PCI0.P0P5"); asus_unregister_rfkill_notifier(asus, "\\_SB.PCI0.P0P6"); asus_unregister_rfkill_notifier(asus, "\\_SB.PCI0.P0P7"); @@ -2120,16 +2133,6 @@ static int asus_wmi_fan_init(struct asus_wmi *asus) return 0; } -static bool ashs_present(void) -{ - int i = 0; - while (ashs_ids[i]) { - if (acpi_dev_found(ashs_ids[i++])) - return true; - } - return false; -} - /* * WMI Driver */ -- 2.17.0
[PATCH] platform/x86: asus-wireless: Fix format specifier
From: João Paulo Rechi Vita <jprv...@gmail.com> u64 should be printed with %llx instead of %x and cast to uint. Signed-off-by: João Paulo Rechi Vita <jprv...@gmail.com> --- drivers/platform/x86/asus-wireless.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c index f086469ea740..6afd011de9e5 100644 --- a/drivers/platform/x86/asus-wireless.c +++ b/drivers/platform/x86/asus-wireless.c @@ -72,7 +72,7 @@ static u64 asus_wireless_method(acpi_handle handle, const char *method, acpi_handle_err(handle, "Failed to eval method %s, param %#x (%d)\n", method, param, s); - acpi_handle_debug(handle, "%s returned %#x\n", method, (uint) ret); + acpi_handle_debug(handle, "%s returned %#llx\n", method, ret); return ret; } -- 2.17.0
[PATCH] platform/x86: asus-wireless: Fix format specifier
From: João Paulo Rechi Vita u64 should be printed with %llx instead of %x and cast to uint. Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-wireless.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c index f086469ea740..6afd011de9e5 100644 --- a/drivers/platform/x86/asus-wireless.c +++ b/drivers/platform/x86/asus-wireless.c @@ -72,7 +72,7 @@ static u64 asus_wireless_method(acpi_handle handle, const char *method, acpi_handle_err(handle, "Failed to eval method %s, param %#x (%d)\n", method, param, s); - acpi_handle_debug(handle, "%s returned %#x\n", method, (uint) ret); + acpi_handle_debug(handle, "%s returned %#llx\n", method, ret); return ret; } -- 2.17.0
[PATCH 1/2] rfkill: Rename rfkill_any_led_trigger* functions
Rename these functions to rfkill_global_led_trigger*, as they are going to be extended to handle another global rfkill led trigger. This commit does not change any functionality. Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- net/rfkill/core.c | 47 --- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/net/rfkill/core.c b/net/rfkill/core.c index 59d0eb960275..6d64d14f4b0a 100644 --- a/net/rfkill/core.c +++ b/net/rfkill/core.c @@ -178,9 +178,9 @@ static void rfkill_led_trigger_unregister(struct rfkill *rfkill) } static struct led_trigger rfkill_any_led_trigger; -static struct work_struct rfkill_any_work; +static struct work_struct rfkill_global_led_trigger_work; -static void rfkill_any_led_trigger_worker(struct work_struct *work) +static void rfkill_global_led_trigger_worker(struct work_struct *work) { enum led_brightness brightness = LED_OFF; struct rfkill *rfkill; @@ -197,28 +197,29 @@ static void rfkill_any_led_trigger_worker(struct work_struct *work) led_trigger_event(_any_led_trigger, brightness); } -static void rfkill_any_led_trigger_event(void) +static void rfkill_global_led_trigger_event(void) { - schedule_work(_any_work); + schedule_work(_global_led_trigger_work); } -static void rfkill_any_led_trigger_activate(struct led_classdev *led_cdev) +static void rfkill_global_led_trigger_activate(struct led_classdev *led_cdev) { - rfkill_any_led_trigger_event(); + rfkill_global_led_trigger_event(); } -static int rfkill_any_led_trigger_register(void) +static int rfkill_global_led_trigger_register(void) { - INIT_WORK(_any_work, rfkill_any_led_trigger_worker); + INIT_WORK(_global_led_trigger_work, + rfkill_global_led_trigger_worker); rfkill_any_led_trigger.name = "rfkill-any"; - rfkill_any_led_trigger.activate = rfkill_any_led_trigger_activate; + rfkill_any_led_trigger.activate = rfkill_global_led_trigger_activate; return led_trigger_register(_any_led_trigger); } -static void rfkill_any_led_trigger_unregister(void) +static void rfkill_global_led_trigger_unregister(void) { led_trigger_unregister(_any_led_trigger); - cancel_work_sync(_any_work); + cancel_work_sync(_global_led_trigger_work); } #else static void rfkill_led_trigger_event(struct rfkill *rfkill) @@ -234,16 +235,16 @@ static inline void rfkill_led_trigger_unregister(struct rfkill *rfkill) { } -static void rfkill_any_led_trigger_event(void) +static void rfkill_global_led_trigger_event(void) { } -static int rfkill_any_led_trigger_register(void) +static int rfkill_global_led_trigger_register(void) { return 0; } -static void rfkill_any_led_trigger_unregister(void) +static void rfkill_global_led_trigger_unregister(void) { } #endif /* CONFIG_RFKILL_LEDS */ @@ -354,7 +355,7 @@ static void rfkill_set_block(struct rfkill *rfkill, bool blocked) spin_unlock_irqrestore(>lock, flags); rfkill_led_trigger_event(rfkill); - rfkill_any_led_trigger_event(); + rfkill_global_led_trigger_event(); if (prev != curr) rfkill_event(rfkill); @@ -535,7 +536,7 @@ bool rfkill_set_hw_state(struct rfkill *rfkill, bool blocked) spin_unlock_irqrestore(>lock, flags); rfkill_led_trigger_event(rfkill); - rfkill_any_led_trigger_event(); + rfkill_global_led_trigger_event(); if (rfkill->registered && prev != blocked) schedule_work(>uevent_work); @@ -579,7 +580,7 @@ bool rfkill_set_sw_state(struct rfkill *rfkill, bool blocked) schedule_work(>uevent_work); rfkill_led_trigger_event(rfkill); - rfkill_any_led_trigger_event(); + rfkill_global_led_trigger_event(); return blocked; } @@ -629,7 +630,7 @@ void rfkill_set_states(struct rfkill *rfkill, bool sw, bool hw) schedule_work(>uevent_work); rfkill_led_trigger_event(rfkill); - rfkill_any_led_trigger_event(); + rfkill_global_led_trigger_event(); } } EXPORT_SYMBOL(rfkill_set_states); @@ -1046,7 +1047,7 @@ int __must_check rfkill_register(struct rfkill *rfkill) #endif } - rfkill_any_led_trigger_event(); + rfkill_global_led_trigger_event(); rfkill_send_events(rfkill, RFKILL_OP_ADD); mutex_unlock(_global_mutex); @@ -1079,7 +1080,7 @@ void rfkill_unregister(struct rfkill *rfkill) mutex_lock(_global_mutex); rfkill_send_events(rfkill, RFKILL_OP_DEL); list_del_init(>node); - rfkill_any_led_trigger_event(); + rfkill_global_led_trigger_event(); mutex_unlock(_global_mutex); rfkill_led_trigger_unregister(rfkill); @@ -1332,7 +1333,7 @@ static int __init rfkill_init(void) if (error) goto error_misc; -
[PATCH 1/2] rfkill: Rename rfkill_any_led_trigger* functions
Rename these functions to rfkill_global_led_trigger*, as they are going to be extended to handle another global rfkill led trigger. This commit does not change any functionality. Signed-off-by: João Paulo Rechi Vita --- net/rfkill/core.c | 47 --- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/net/rfkill/core.c b/net/rfkill/core.c index 59d0eb960275..6d64d14f4b0a 100644 --- a/net/rfkill/core.c +++ b/net/rfkill/core.c @@ -178,9 +178,9 @@ static void rfkill_led_trigger_unregister(struct rfkill *rfkill) } static struct led_trigger rfkill_any_led_trigger; -static struct work_struct rfkill_any_work; +static struct work_struct rfkill_global_led_trigger_work; -static void rfkill_any_led_trigger_worker(struct work_struct *work) +static void rfkill_global_led_trigger_worker(struct work_struct *work) { enum led_brightness brightness = LED_OFF; struct rfkill *rfkill; @@ -197,28 +197,29 @@ static void rfkill_any_led_trigger_worker(struct work_struct *work) led_trigger_event(_any_led_trigger, brightness); } -static void rfkill_any_led_trigger_event(void) +static void rfkill_global_led_trigger_event(void) { - schedule_work(_any_work); + schedule_work(_global_led_trigger_work); } -static void rfkill_any_led_trigger_activate(struct led_classdev *led_cdev) +static void rfkill_global_led_trigger_activate(struct led_classdev *led_cdev) { - rfkill_any_led_trigger_event(); + rfkill_global_led_trigger_event(); } -static int rfkill_any_led_trigger_register(void) +static int rfkill_global_led_trigger_register(void) { - INIT_WORK(_any_work, rfkill_any_led_trigger_worker); + INIT_WORK(_global_led_trigger_work, + rfkill_global_led_trigger_worker); rfkill_any_led_trigger.name = "rfkill-any"; - rfkill_any_led_trigger.activate = rfkill_any_led_trigger_activate; + rfkill_any_led_trigger.activate = rfkill_global_led_trigger_activate; return led_trigger_register(_any_led_trigger); } -static void rfkill_any_led_trigger_unregister(void) +static void rfkill_global_led_trigger_unregister(void) { led_trigger_unregister(_any_led_trigger); - cancel_work_sync(_any_work); + cancel_work_sync(_global_led_trigger_work); } #else static void rfkill_led_trigger_event(struct rfkill *rfkill) @@ -234,16 +235,16 @@ static inline void rfkill_led_trigger_unregister(struct rfkill *rfkill) { } -static void rfkill_any_led_trigger_event(void) +static void rfkill_global_led_trigger_event(void) { } -static int rfkill_any_led_trigger_register(void) +static int rfkill_global_led_trigger_register(void) { return 0; } -static void rfkill_any_led_trigger_unregister(void) +static void rfkill_global_led_trigger_unregister(void) { } #endif /* CONFIG_RFKILL_LEDS */ @@ -354,7 +355,7 @@ static void rfkill_set_block(struct rfkill *rfkill, bool blocked) spin_unlock_irqrestore(>lock, flags); rfkill_led_trigger_event(rfkill); - rfkill_any_led_trigger_event(); + rfkill_global_led_trigger_event(); if (prev != curr) rfkill_event(rfkill); @@ -535,7 +536,7 @@ bool rfkill_set_hw_state(struct rfkill *rfkill, bool blocked) spin_unlock_irqrestore(>lock, flags); rfkill_led_trigger_event(rfkill); - rfkill_any_led_trigger_event(); + rfkill_global_led_trigger_event(); if (rfkill->registered && prev != blocked) schedule_work(>uevent_work); @@ -579,7 +580,7 @@ bool rfkill_set_sw_state(struct rfkill *rfkill, bool blocked) schedule_work(>uevent_work); rfkill_led_trigger_event(rfkill); - rfkill_any_led_trigger_event(); + rfkill_global_led_trigger_event(); return blocked; } @@ -629,7 +630,7 @@ void rfkill_set_states(struct rfkill *rfkill, bool sw, bool hw) schedule_work(>uevent_work); rfkill_led_trigger_event(rfkill); - rfkill_any_led_trigger_event(); + rfkill_global_led_trigger_event(); } } EXPORT_SYMBOL(rfkill_set_states); @@ -1046,7 +1047,7 @@ int __must_check rfkill_register(struct rfkill *rfkill) #endif } - rfkill_any_led_trigger_event(); + rfkill_global_led_trigger_event(); rfkill_send_events(rfkill, RFKILL_OP_ADD); mutex_unlock(_global_mutex); @@ -1079,7 +1080,7 @@ void rfkill_unregister(struct rfkill *rfkill) mutex_lock(_global_mutex); rfkill_send_events(rfkill, RFKILL_OP_DEL); list_del_init(>node); - rfkill_any_led_trigger_event(); + rfkill_global_led_trigger_event(); mutex_unlock(_global_mutex); rfkill_led_trigger_unregister(rfkill); @@ -1332,7 +1333,7 @@ static int __init rfkill_init(void) if (error) goto error_misc; - error = rfkill_an
Re: RTL8723BE performance regression
On Tue, May 8, 2018 at 1:37 AM, Pkshih <pks...@realtek.com> wrote: > On Mon, 2018-05-07 at 14:49 -0700, João Paulo Rechi Vita wrote: >> On Tue, May 1, 2018 at 10:58 PM, Pkshih <pks...@realtek.com> wrote: >> > On Wed, 2018-05-02 at 05:44 +, Pkshih wrote: >> >> >> >> > -Original Message- >> >> > From: João Paulo Rechi Vita [mailto:jprv...@gmail.com] >> >> > Sent: Wednesday, May 02, 2018 6:41 AM >> >> > To: Larry Finger >> >> > Cc: Steve deRosier; 莊彥宣; Pkshih; Birming Chiu; Shaofu; Steven Ting; >> >> > Chaoming_Li; Kalle Valo; >> >> > linux-wireless; Network Development; LKML; Daniel Drake; João Paulo >> >> > Rechi Vita; linux@endless >> m.c >> >> om >> >> > Subject: Re: RTL8723BE performance regression >> >> > >> >> > On Tue, Apr 3, 2018 at 7:51 PM, Larry Finger >> >> > <larry.fin...@lwfinger.net> wrote: >> >> > > On 04/03/2018 09:37 PM, João Paulo Rechi Vita wrote: >> >> > >> >> >> > >> On Tue, Apr 3, 2018 at 7:28 PM, Larry Finger >> >> > >> <larry.fin...@lwfinger.net> >> >> > >> wrote: >> >> > >> >> >> > >> (...) >> >> > >> >> >> > >>> As the antenna selection code changes affected your first >> >> > >>> bisection, do >> >> > >>> you >> >> > >>> have one of those HP laptops with only one antenna and the incorrect >> >> > >>> coding >> >> > >>> in the FUSE? >> >> > >> >> >> > >> >> >> > >> Yes, that is why I've been passing ant_sel=1 during my tests -- this >> >> > >> was needed to achieve a good performance in the past, before this >> >> > >> regression. I've also opened the laptop chassis and confirmed the >> >> > >> antenna cable is plugged to the connector labeled with "1" on the >> >> > >> card. >> >> > >> >> >> > >>> If so, please make sure that you still have the same signal >> >> > >>> strength for good and bad cases. I have tried to keep the driver >> >> > >>> and the >> >> > >>> btcoex code in sync, but there may be some combinations of antenna >> >> > >>> configuration and FUSE contents that cause the code to fail. >> >> > >>> >> >> > >> >> >> > >> What is the recommended way to monitor the signal strength? >> >> > > >> >> > > >> >> > > The btcoex code is developed for multiple platforms by a different >> >> > > group >> >> > > than the Linux driver. I think they made a change that caused ant_sel >> >> > > to >> >> > > switch from 1 to 2. At least numerous comments at >> >> > > github.com/lwfinger/rtlwifi_new claimed they needed to make that >> >> > > change. >> >> > > >> >> > > Mhy recommended method is to verify the wifi device name with "iw >> >> > > dev". Then >> >> > > using that device >> >> > > >> >> > > sudo iw dev scan | egrep "SSID|signal" >> >> > > >> >> > >> >> > I have confirmed that the performance regression is indeed tied to >> >> > signal strength: on the good cases signal was between -16 and -8 dBm, >> >> > whereas in bad cases signal was always between -50 to - 40 dBm. I've >> >> > also switched to testing bandwidth in controlled LAN environment using >> >> > iperf3, as suggested by Steve deRosier, with the DUT being the only >> >> > machine connected to the 2.4 GHz radio and the machine running the >> >> > iperf3 server connected via ethernet. >> >> > >> >> >> >> We have new experimental results in commit af8a41cccf8f46 ("rtlwifi: >> >> cleanup >> >> 8723be ant_sel definition"). You can use the above commit and do the same >> >> experiments (with ant_sel=0, 1 and 2) in your side, and then share your >> >> results. >> >> Since performance is tied to signal strength, you can only share signal >> >> streng
Re: RTL8723BE performance regression
On Tue, May 8, 2018 at 1:37 AM, Pkshih wrote: > On Mon, 2018-05-07 at 14:49 -0700, João Paulo Rechi Vita wrote: >> On Tue, May 1, 2018 at 10:58 PM, Pkshih wrote: >> > On Wed, 2018-05-02 at 05:44 +, Pkshih wrote: >> >> >> >> > -Original Message- >> >> > From: João Paulo Rechi Vita [mailto:jprv...@gmail.com] >> >> > Sent: Wednesday, May 02, 2018 6:41 AM >> >> > To: Larry Finger >> >> > Cc: Steve deRosier; 莊彥宣; Pkshih; Birming Chiu; Shaofu; Steven Ting; >> >> > Chaoming_Li; Kalle Valo; >> >> > linux-wireless; Network Development; LKML; Daniel Drake; João Paulo >> >> > Rechi Vita; linux@endless >> m.c >> >> om >> >> > Subject: Re: RTL8723BE performance regression >> >> > >> >> > On Tue, Apr 3, 2018 at 7:51 PM, Larry Finger >> >> > wrote: >> >> > > On 04/03/2018 09:37 PM, João Paulo Rechi Vita wrote: >> >> > >> >> >> > >> On Tue, Apr 3, 2018 at 7:28 PM, Larry Finger >> >> > >> >> >> > >> wrote: >> >> > >> >> >> > >> (...) >> >> > >> >> >> > >>> As the antenna selection code changes affected your first >> >> > >>> bisection, do >> >> > >>> you >> >> > >>> have one of those HP laptops with only one antenna and the incorrect >> >> > >>> coding >> >> > >>> in the FUSE? >> >> > >> >> >> > >> >> >> > >> Yes, that is why I've been passing ant_sel=1 during my tests -- this >> >> > >> was needed to achieve a good performance in the past, before this >> >> > >> regression. I've also opened the laptop chassis and confirmed the >> >> > >> antenna cable is plugged to the connector labeled with "1" on the >> >> > >> card. >> >> > >> >> >> > >>> If so, please make sure that you still have the same signal >> >> > >>> strength for good and bad cases. I have tried to keep the driver >> >> > >>> and the >> >> > >>> btcoex code in sync, but there may be some combinations of antenna >> >> > >>> configuration and FUSE contents that cause the code to fail. >> >> > >>> >> >> > >> >> >> > >> What is the recommended way to monitor the signal strength? >> >> > > >> >> > > >> >> > > The btcoex code is developed for multiple platforms by a different >> >> > > group >> >> > > than the Linux driver. I think they made a change that caused ant_sel >> >> > > to >> >> > > switch from 1 to 2. At least numerous comments at >> >> > > github.com/lwfinger/rtlwifi_new claimed they needed to make that >> >> > > change. >> >> > > >> >> > > Mhy recommended method is to verify the wifi device name with "iw >> >> > > dev". Then >> >> > > using that device >> >> > > >> >> > > sudo iw dev scan | egrep "SSID|signal" >> >> > > >> >> > >> >> > I have confirmed that the performance regression is indeed tied to >> >> > signal strength: on the good cases signal was between -16 and -8 dBm, >> >> > whereas in bad cases signal was always between -50 to - 40 dBm. I've >> >> > also switched to testing bandwidth in controlled LAN environment using >> >> > iperf3, as suggested by Steve deRosier, with the DUT being the only >> >> > machine connected to the 2.4 GHz radio and the machine running the >> >> > iperf3 server connected via ethernet. >> >> > >> >> >> >> We have new experimental results in commit af8a41cccf8f46 ("rtlwifi: >> >> cleanup >> >> 8723be ant_sel definition"). You can use the above commit and do the same >> >> experiments (with ant_sel=0, 1 and 2) in your side, and then share your >> >> results. >> >> Since performance is tied to signal strength, you can only share signal >> >> strength. >> >> >> > >> > Please pay attention to cold reboot once ant_sel is changed. >
Re: RTL8723BE performance regression
On Tue, May 1, 2018 at 10:58 PM, Pkshih <pks...@realtek.com> wrote: > On Wed, 2018-05-02 at 05:44 +, Pkshih wrote: >> >> > -Original Message----- >> > From: João Paulo Rechi Vita [mailto:jprv...@gmail.com] >> > Sent: Wednesday, May 02, 2018 6:41 AM >> > To: Larry Finger >> > Cc: Steve deRosier; 莊彥宣; Pkshih; Birming Chiu; Shaofu; Steven Ting; >> > Chaoming_Li; Kalle Valo; >> > linux-wireless; Network Development; LKML; Daniel Drake; João Paulo Rechi >> > Vita; linux@endlessm.c >> om >> > Subject: Re: RTL8723BE performance regression >> > >> > On Tue, Apr 3, 2018 at 7:51 PM, Larry Finger <larry.fin...@lwfinger.net> >> > wrote: >> > > On 04/03/2018 09:37 PM, João Paulo Rechi Vita wrote: >> > >> >> > >> On Tue, Apr 3, 2018 at 7:28 PM, Larry Finger <larry.fin...@lwfinger.net> >> > >> wrote: >> > >> >> > >> (...) >> > >> >> > >>> As the antenna selection code changes affected your first bisection, do >> > >>> you >> > >>> have one of those HP laptops with only one antenna and the incorrect >> > >>> coding >> > >>> in the FUSE? >> > >> >> > >> >> > >> Yes, that is why I've been passing ant_sel=1 during my tests -- this >> > >> was needed to achieve a good performance in the past, before this >> > >> regression. I've also opened the laptop chassis and confirmed the >> > >> antenna cable is plugged to the connector labeled with "1" on the >> > >> card. >> > >> >> > >>> If so, please make sure that you still have the same signal >> > >>> strength for good and bad cases. I have tried to keep the driver and >> > >>> the >> > >>> btcoex code in sync, but there may be some combinations of antenna >> > >>> configuration and FUSE contents that cause the code to fail. >> > >>> >> > >> >> > >> What is the recommended way to monitor the signal strength? >> > > >> > > >> > > The btcoex code is developed for multiple platforms by a different group >> > > than the Linux driver. I think they made a change that caused ant_sel to >> > > switch from 1 to 2. At least numerous comments at >> > > github.com/lwfinger/rtlwifi_new claimed they needed to make that change. >> > > >> > > Mhy recommended method is to verify the wifi device name with "iw dev". >> > > Then >> > > using that device >> > > >> > > sudo iw dev scan | egrep "SSID|signal" >> > > >> > >> > I have confirmed that the performance regression is indeed tied to >> > signal strength: on the good cases signal was between -16 and -8 dBm, >> > whereas in bad cases signal was always between -50 to - 40 dBm. I've >> > also switched to testing bandwidth in controlled LAN environment using >> > iperf3, as suggested by Steve deRosier, with the DUT being the only >> > machine connected to the 2.4 GHz radio and the machine running the >> > iperf3 server connected via ethernet. >> > >> >> We have new experimental results in commit af8a41cccf8f46 ("rtlwifi: cleanup >> 8723be ant_sel definition"). You can use the above commit and do the same >> experiments (with ant_sel=0, 1 and 2) in your side, and then share your >> results. >> Since performance is tied to signal strength, you can only share signal >> strength. >> > > Please pay attention to cold reboot once ant_sel is changed. > I've tested the commit mentioned above and it fixes the problem on top of v4.16 (in addition to the latest wireless-drivers-next also been fixed as it already contains such commit). On v4.15, we also need the following commits before "af8a41cccf8f rtlwifi: cleanup 8723be ant_sel definition" to have a good performance again: 874e837d67d0 rtlwifi: fill FW version and subversion a44709bba70f rtlwifi: btcoex: Add power_on_setting routine 40d9dd4f1c5d rtlwifi: btcoex: Remove global variables from btcoex Surprisingly, it seems forcing ant_sel=1 is not needed anymore on these machines, as the shown by the numbers bellow (ant_sel=0 means that actually no parameter was passed to the module). I have powered off the machine and done a cold boot for every test. It seems something have changed in the antenna auto-selection code since v4.11, the latest point where I could confirm we definitely need to force ant_sel=1. I've been trying to understand what causes this difference, but haven't made progress on that so far, so any suggestions are appreciated (we are trying to decide if we can confidently drop the downstream DMI quirks for these specific machines). w-d-n ant_sel=0: -14.00 dBm, 69.5 Mbps -> good w-d-n ant_sel=1: -10.00 dBm, 41.1 Mbps -> good w-d-n ant_sel=2: -44.00 dBm, 607 kbps -> bad v4.16 ant_sel=0: -12.00 dBm, 63.0 Mbps -> good v4.16 ant_sel=1: - 8.00 dBm, 69.0 Mbps -> good v4.16 ant_sel=2: -50.00 dBm, 224 kbps -> bad v4.15 ant_sel=0: - 8.00 dBm, 33.0 Mbps -> good v4.15 ant_sel=1: -10.00 dBm, 38.1 Mbps -> good v4.15 ant_sel=2: -48.00 dBm, 206 kbps -> bad -- João Paulo Rechi Vita http://about.me/jprvita
Re: RTL8723BE performance regression
On Tue, May 1, 2018 at 10:58 PM, Pkshih wrote: > On Wed, 2018-05-02 at 05:44 +, Pkshih wrote: >> >> > -Original Message----- >> > From: João Paulo Rechi Vita [mailto:jprv...@gmail.com] >> > Sent: Wednesday, May 02, 2018 6:41 AM >> > To: Larry Finger >> > Cc: Steve deRosier; 莊彥宣; Pkshih; Birming Chiu; Shaofu; Steven Ting; >> > Chaoming_Li; Kalle Valo; >> > linux-wireless; Network Development; LKML; Daniel Drake; João Paulo Rechi >> > Vita; linux@endlessm.c >> om >> > Subject: Re: RTL8723BE performance regression >> > >> > On Tue, Apr 3, 2018 at 7:51 PM, Larry Finger >> > wrote: >> > > On 04/03/2018 09:37 PM, João Paulo Rechi Vita wrote: >> > >> >> > >> On Tue, Apr 3, 2018 at 7:28 PM, Larry Finger >> > >> wrote: >> > >> >> > >> (...) >> > >> >> > >>> As the antenna selection code changes affected your first bisection, do >> > >>> you >> > >>> have one of those HP laptops with only one antenna and the incorrect >> > >>> coding >> > >>> in the FUSE? >> > >> >> > >> >> > >> Yes, that is why I've been passing ant_sel=1 during my tests -- this >> > >> was needed to achieve a good performance in the past, before this >> > >> regression. I've also opened the laptop chassis and confirmed the >> > >> antenna cable is plugged to the connector labeled with "1" on the >> > >> card. >> > >> >> > >>> If so, please make sure that you still have the same signal >> > >>> strength for good and bad cases. I have tried to keep the driver and >> > >>> the >> > >>> btcoex code in sync, but there may be some combinations of antenna >> > >>> configuration and FUSE contents that cause the code to fail. >> > >>> >> > >> >> > >> What is the recommended way to monitor the signal strength? >> > > >> > > >> > > The btcoex code is developed for multiple platforms by a different group >> > > than the Linux driver. I think they made a change that caused ant_sel to >> > > switch from 1 to 2. At least numerous comments at >> > > github.com/lwfinger/rtlwifi_new claimed they needed to make that change. >> > > >> > > Mhy recommended method is to verify the wifi device name with "iw dev". >> > > Then >> > > using that device >> > > >> > > sudo iw dev scan | egrep "SSID|signal" >> > > >> > >> > I have confirmed that the performance regression is indeed tied to >> > signal strength: on the good cases signal was between -16 and -8 dBm, >> > whereas in bad cases signal was always between -50 to - 40 dBm. I've >> > also switched to testing bandwidth in controlled LAN environment using >> > iperf3, as suggested by Steve deRosier, with the DUT being the only >> > machine connected to the 2.4 GHz radio and the machine running the >> > iperf3 server connected via ethernet. >> > >> >> We have new experimental results in commit af8a41cccf8f46 ("rtlwifi: cleanup >> 8723be ant_sel definition"). You can use the above commit and do the same >> experiments (with ant_sel=0, 1 and 2) in your side, and then share your >> results. >> Since performance is tied to signal strength, you can only share signal >> strength. >> > > Please pay attention to cold reboot once ant_sel is changed. > I've tested the commit mentioned above and it fixes the problem on top of v4.16 (in addition to the latest wireless-drivers-next also been fixed as it already contains such commit). On v4.15, we also need the following commits before "af8a41cccf8f rtlwifi: cleanup 8723be ant_sel definition" to have a good performance again: 874e837d67d0 rtlwifi: fill FW version and subversion a44709bba70f rtlwifi: btcoex: Add power_on_setting routine 40d9dd4f1c5d rtlwifi: btcoex: Remove global variables from btcoex Surprisingly, it seems forcing ant_sel=1 is not needed anymore on these machines, as the shown by the numbers bellow (ant_sel=0 means that actually no parameter was passed to the module). I have powered off the machine and done a cold boot for every test. It seems something have changed in the antenna auto-selection code since v4.11, the latest point where I could confirm we definitely need to force ant_sel=1. I've been trying to understand what causes this difference, but haven't made progress on that so far, so any suggestions are appreciated (we are trying to decide if we can confidently drop the downstream DMI quirks for these specific machines). w-d-n ant_sel=0: -14.00 dBm, 69.5 Mbps -> good w-d-n ant_sel=1: -10.00 dBm, 41.1 Mbps -> good w-d-n ant_sel=2: -44.00 dBm, 607 kbps -> bad v4.16 ant_sel=0: -12.00 dBm, 63.0 Mbps -> good v4.16 ant_sel=1: - 8.00 dBm, 69.0 Mbps -> good v4.16 ant_sel=2: -50.00 dBm, 224 kbps -> bad v4.15 ant_sel=0: - 8.00 dBm, 33.0 Mbps -> good v4.15 ant_sel=1: -10.00 dBm, 38.1 Mbps -> good v4.15 ant_sel=2: -48.00 dBm, 206 kbps -> bad -- João Paulo Rechi Vita http://about.me/jprvita
[RFC PATCH 2/3] Revert "rtlwifi: fill FW version and subversion"
This reverts commit 874e837d67d0db179c9771f38fd21df07c703e93. This drastically affects the upload performance and signal strenght on the HP 240 G4 laptop, as shown by the results bellow: Without this change: $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal signal: -42.00 dBm $ iperf3 -c 192.168.1.254 Connecting to host 192.168.1.254, port 5201 [ 4] local 192.168.1.253 port 39678 connected to 192.168.1.254 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 735 KBytes 6.02 Mbits/sec1 1.41 KBytes [ 4] 1.00-2.00 sec 274 KBytes 2.25 Mbits/sec1 1.41 KBytes [ 4] 2.00-3.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 3.00-4.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 4.00-5.00 sec 0.00 Bytes 0.00 bits/sec1 28.3 KBytes [ 4] 5.00-6.00 sec 423 KBytes 3.47 Mbits/sec3 41.0 KBytes [ 4] 6.00-7.00 sec 840 KBytes 6.88 Mbits/sec0 58.0 KBytes [ 4] 7.00-8.00 sec 830 KBytes 6.79 Mbits/sec1 1.41 KBytes [ 4] 8.00-9.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 9.00-10.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 4] 0.00-10.00 sec 3.03 MBytes 2.54 Mbits/sec7 sender [ 4] 0.00-10.00 sec 2.88 MBytes 2.41 Mbits/sec receiver iperf Done. With this change: $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal signal: -14.00 dBm $ iperf3 -c 192.168.1.254 Connecting to host 192.168.1.254, port 5201 [ 4] local 192.168.1.253 port 59380 connected to 192.168.1.254 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 4.63 MBytes 38.8 Mbits/sec0194 KBytes [ 4] 1.00-2.00 sec 4.58 MBytes 38.4 Mbits/sec0273 KBytes [ 4] 2.00-3.00 sec 5.05 MBytes 42.3 Mbits/sec0332 KBytes [ 4] 3.00-4.00 sec 4.98 MBytes 41.8 Mbits/sec0393 KBytes [ 4] 4.00-5.00 sec 4.76 MBytes 39.9 Mbits/sec0434 KBytes [ 4] 5.00-6.00 sec 4.85 MBytes 40.7 Mbits/sec0434 KBytes [ 4] 6.00-7.00 sec 3.96 MBytes 33.2 Mbits/sec0464 KBytes [ 4] 7.00-8.00 sec 4.74 MBytes 39.8 Mbits/sec0481 KBytes [ 4] 8.00-9.00 sec 4.22 MBytes 35.4 Mbits/sec0508 KBytes [ 4] 9.00-10.00 sec 4.09 MBytes 34.3 Mbits/sec0564 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 4] 0.00-10.00 sec 45.9 MBytes 38.5 Mbits/sec0 sender [ 4] 0.00-10.00 sec 45.0 MBytes 37.7 Mbits/sec receiver iperf Done. Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c | 2 -- drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c | 2 -- 2 files changed, 4 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c index 63874512598b..a2eca669873b 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c @@ -141,8 +141,6 @@ int rtl88e_download_fw(struct ieee80211_hw *hw, return 1; pfwheader = (struct rtlwifi_firmware_header *)rtlhal->pfirmware; - rtlhal->fw_version = le16_to_cpu(pfwheader->version); - rtlhal->fw_subversion = pfwheader->subversion; pfwdata = rtlhal->pfirmware; fwsize = rtlhal->fwsize; RT_TRACE(rtlpriv, COMP_FW, DBG_DMESG, diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c index 521039c5d4ce..0d1ebc861720 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c @@ -200,8 +200,6 @@ int rtl8723_download_fw(struct ieee80211_hw *hw, return 1; pfwheader = (struct rtlwifi_firmware_header *)rtlhal->pfirmware; - rtlhal->fw_version = le16_to_cpu(pfwheader->version); - rtlhal->fw_subversion = pfwheader->subversion; pfwdata = rtlhal->pfirmware; fwsize = rtlhal->fwsize; -- 2.17.0
[RFC PATCH 2/3] Revert "rtlwifi: fill FW version and subversion"
This reverts commit 874e837d67d0db179c9771f38fd21df07c703e93. This drastically affects the upload performance and signal strenght on the HP 240 G4 laptop, as shown by the results bellow: Without this change: $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal signal: -42.00 dBm $ iperf3 -c 192.168.1.254 Connecting to host 192.168.1.254, port 5201 [ 4] local 192.168.1.253 port 39678 connected to 192.168.1.254 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 735 KBytes 6.02 Mbits/sec1 1.41 KBytes [ 4] 1.00-2.00 sec 274 KBytes 2.25 Mbits/sec1 1.41 KBytes [ 4] 2.00-3.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 3.00-4.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 4.00-5.00 sec 0.00 Bytes 0.00 bits/sec1 28.3 KBytes [ 4] 5.00-6.00 sec 423 KBytes 3.47 Mbits/sec3 41.0 KBytes [ 4] 6.00-7.00 sec 840 KBytes 6.88 Mbits/sec0 58.0 KBytes [ 4] 7.00-8.00 sec 830 KBytes 6.79 Mbits/sec1 1.41 KBytes [ 4] 8.00-9.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 9.00-10.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 4] 0.00-10.00 sec 3.03 MBytes 2.54 Mbits/sec7 sender [ 4] 0.00-10.00 sec 2.88 MBytes 2.41 Mbits/sec receiver iperf Done. With this change: $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal signal: -14.00 dBm $ iperf3 -c 192.168.1.254 Connecting to host 192.168.1.254, port 5201 [ 4] local 192.168.1.253 port 59380 connected to 192.168.1.254 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 4.63 MBytes 38.8 Mbits/sec0194 KBytes [ 4] 1.00-2.00 sec 4.58 MBytes 38.4 Mbits/sec0273 KBytes [ 4] 2.00-3.00 sec 5.05 MBytes 42.3 Mbits/sec0332 KBytes [ 4] 3.00-4.00 sec 4.98 MBytes 41.8 Mbits/sec0393 KBytes [ 4] 4.00-5.00 sec 4.76 MBytes 39.9 Mbits/sec0434 KBytes [ 4] 5.00-6.00 sec 4.85 MBytes 40.7 Mbits/sec0434 KBytes [ 4] 6.00-7.00 sec 3.96 MBytes 33.2 Mbits/sec0464 KBytes [ 4] 7.00-8.00 sec 4.74 MBytes 39.8 Mbits/sec0481 KBytes [ 4] 8.00-9.00 sec 4.22 MBytes 35.4 Mbits/sec0508 KBytes [ 4] 9.00-10.00 sec 4.09 MBytes 34.3 Mbits/sec0564 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 4] 0.00-10.00 sec 45.9 MBytes 38.5 Mbits/sec0 sender [ 4] 0.00-10.00 sec 45.0 MBytes 37.7 Mbits/sec receiver iperf Done. Signed-off-by: João Paulo Rechi Vita --- drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c | 2 -- drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c | 2 -- 2 files changed, 4 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c index 63874512598b..a2eca669873b 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c @@ -141,8 +141,6 @@ int rtl88e_download_fw(struct ieee80211_hw *hw, return 1; pfwheader = (struct rtlwifi_firmware_header *)rtlhal->pfirmware; - rtlhal->fw_version = le16_to_cpu(pfwheader->version); - rtlhal->fw_subversion = pfwheader->subversion; pfwdata = rtlhal->pfirmware; fwsize = rtlhal->fwsize; RT_TRACE(rtlpriv, COMP_FW, DBG_DMESG, diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c index 521039c5d4ce..0d1ebc861720 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c @@ -200,8 +200,6 @@ int rtl8723_download_fw(struct ieee80211_hw *hw, return 1; pfwheader = (struct rtlwifi_firmware_header *)rtlhal->pfirmware; - rtlhal->fw_version = le16_to_cpu(pfwheader->version); - rtlhal->fw_subversion = pfwheader->subversion; pfwdata = rtlhal->pfirmware; fwsize = rtlhal->fwsize; -- 2.17.0
[RFC PATCH 1/3] rtlwifi: btcoex: Don't init antenna position to main port
This partially reverts commit 40d9dd4f1c5dcd0d4a2a1f0efcd225c9c4b36d6f, which does not only remove global variables from btcoex, as described on its commit message, but also does a few other things -- in particular, setting the default antenna position to BTC_ANTENNA_AT_MAIN_PORT. This drastically affects the upload performance and signal strenght on the HP 240 G4 laptop, as shown by the results bellow: Without this change: $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal signal: -42.00 dBm $ iperf3 -c 192.168.1.254 Connecting to host 192.168.1.254, port 5201 [ 4] local 192.168.1.253 port 39678 connected to 192.168.1.254 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 735 KBytes 6.02 Mbits/sec1 1.41 KBytes [ 4] 1.00-2.00 sec 274 KBytes 2.25 Mbits/sec1 1.41 KBytes [ 4] 2.00-3.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 3.00-4.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 4.00-5.00 sec 0.00 Bytes 0.00 bits/sec1 28.3 KBytes [ 4] 5.00-6.00 sec 423 KBytes 3.47 Mbits/sec3 41.0 KBytes [ 4] 6.00-7.00 sec 840 KBytes 6.88 Mbits/sec0 58.0 KBytes [ 4] 7.00-8.00 sec 830 KBytes 6.79 Mbits/sec1 1.41 KBytes [ 4] 8.00-9.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 9.00-10.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 4] 0.00-10.00 sec 3.03 MBytes 2.54 Mbits/sec7 sender [ 4] 0.00-10.00 sec 2.88 MBytes 2.41 Mbits/sec receiver iperf Done. With this change: $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal signal: -14.00 dBm $ iperf3 -c 192.168.1.254 Connecting to host 192.168.1.254, port 5201 [ 4] local 192.168.1.253 port 59380 connected to 192.168.1.254 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 4.63 MBytes 38.8 Mbits/sec0194 KBytes [ 4] 1.00-2.00 sec 4.58 MBytes 38.4 Mbits/sec0273 KBytes [ 4] 2.00-3.00 sec 5.05 MBytes 42.3 Mbits/sec0332 KBytes [ 4] 3.00-4.00 sec 4.98 MBytes 41.8 Mbits/sec0393 KBytes [ 4] 4.00-5.00 sec 4.76 MBytes 39.9 Mbits/sec0434 KBytes [ 4] 5.00-6.00 sec 4.85 MBytes 40.7 Mbits/sec0434 KBytes [ 4] 6.00-7.00 sec 3.96 MBytes 33.2 Mbits/sec0464 KBytes [ 4] 7.00-8.00 sec 4.74 MBytes 39.8 Mbits/sec0481 KBytes [ 4] 8.00-9.00 sec 4.22 MBytes 35.4 Mbits/sec0508 KBytes [ 4] 9.00-10.00 sec 4.09 MBytes 34.3 Mbits/sec0564 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 4] 0.00-10.00 sec 45.9 MBytes 38.5 Mbits/sec0 sender [ 4] 0.00-10.00 sec 45.0 MBytes 37.7 Mbits/sec receiver iperf Done. Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c index b026e80940a4..86182b917c92 100644 --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c @@ -1388,9 +1388,6 @@ bool exhalbtc_bind_bt_coex_withadapter(void *adapter) ant_num = rtl_get_hwpg_ant_num(rtlpriv); exhalbtc_set_ant_num(rtlpriv, BT_COEX_ANT_TYPE_PG, ant_num); - /* set default antenna position to main port */ - btcoexist->board_info.btdm_ant_pos = BTC_ANTENNA_AT_MAIN_PORT; - single_ant_path = rtl_get_hwpg_single_ant_path(rtlpriv); exhalbtc_set_single_ant_path(btcoexist, single_ant_path); -- 2.17.0
[RFC PATCH 1/3] rtlwifi: btcoex: Don't init antenna position to main port
This partially reverts commit 40d9dd4f1c5dcd0d4a2a1f0efcd225c9c4b36d6f, which does not only remove global variables from btcoex, as described on its commit message, but also does a few other things -- in particular, setting the default antenna position to BTC_ANTENNA_AT_MAIN_PORT. This drastically affects the upload performance and signal strenght on the HP 240 G4 laptop, as shown by the results bellow: Without this change: $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal signal: -42.00 dBm $ iperf3 -c 192.168.1.254 Connecting to host 192.168.1.254, port 5201 [ 4] local 192.168.1.253 port 39678 connected to 192.168.1.254 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 735 KBytes 6.02 Mbits/sec1 1.41 KBytes [ 4] 1.00-2.00 sec 274 KBytes 2.25 Mbits/sec1 1.41 KBytes [ 4] 2.00-3.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 3.00-4.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 4.00-5.00 sec 0.00 Bytes 0.00 bits/sec1 28.3 KBytes [ 4] 5.00-6.00 sec 423 KBytes 3.47 Mbits/sec3 41.0 KBytes [ 4] 6.00-7.00 sec 840 KBytes 6.88 Mbits/sec0 58.0 KBytes [ 4] 7.00-8.00 sec 830 KBytes 6.79 Mbits/sec1 1.41 KBytes [ 4] 8.00-9.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 9.00-10.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 4] 0.00-10.00 sec 3.03 MBytes 2.54 Mbits/sec7 sender [ 4] 0.00-10.00 sec 2.88 MBytes 2.41 Mbits/sec receiver iperf Done. With this change: $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal signal: -14.00 dBm $ iperf3 -c 192.168.1.254 Connecting to host 192.168.1.254, port 5201 [ 4] local 192.168.1.253 port 59380 connected to 192.168.1.254 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 4.63 MBytes 38.8 Mbits/sec0194 KBytes [ 4] 1.00-2.00 sec 4.58 MBytes 38.4 Mbits/sec0273 KBytes [ 4] 2.00-3.00 sec 5.05 MBytes 42.3 Mbits/sec0332 KBytes [ 4] 3.00-4.00 sec 4.98 MBytes 41.8 Mbits/sec0393 KBytes [ 4] 4.00-5.00 sec 4.76 MBytes 39.9 Mbits/sec0434 KBytes [ 4] 5.00-6.00 sec 4.85 MBytes 40.7 Mbits/sec0434 KBytes [ 4] 6.00-7.00 sec 3.96 MBytes 33.2 Mbits/sec0464 KBytes [ 4] 7.00-8.00 sec 4.74 MBytes 39.8 Mbits/sec0481 KBytes [ 4] 8.00-9.00 sec 4.22 MBytes 35.4 Mbits/sec0508 KBytes [ 4] 9.00-10.00 sec 4.09 MBytes 34.3 Mbits/sec0564 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 4] 0.00-10.00 sec 45.9 MBytes 38.5 Mbits/sec0 sender [ 4] 0.00-10.00 sec 45.0 MBytes 37.7 Mbits/sec receiver iperf Done. Signed-off-by: João Paulo Rechi Vita --- drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c index b026e80940a4..86182b917c92 100644 --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c @@ -1388,9 +1388,6 @@ bool exhalbtc_bind_bt_coex_withadapter(void *adapter) ant_num = rtl_get_hwpg_ant_num(rtlpriv); exhalbtc_set_ant_num(rtlpriv, BT_COEX_ANT_TYPE_PG, ant_num); - /* set default antenna position to main port */ - btcoexist->board_info.btdm_ant_pos = BTC_ANTENNA_AT_MAIN_PORT; - single_ant_path = rtl_get_hwpg_single_ant_path(rtlpriv); exhalbtc_set_single_ant_path(btcoexist, single_ant_path); -- 2.17.0
[RFC PATCH 3/3] rtlwifi: btcoex: Always use 2ant-functions for RTL8723BE
This partially reverts commit 7937f02d1953952a6eaf626b175ea9db5541e699, which not only hooked external functions for newer ICs, as described in its commit message, but also changed the behavior for RTL8723BE depending on the value of board_info.btdm_ant_num. When board_info.btdm_ant_num == 1, 7937f02d19 changes the codepath to use a whole different set of functions ex_btc8723b1ant_*, instead of the ex_btc8723b2ant_* that were used before it. This drastically affects the upload performance and signal strenght on the HP 240 G4 laptop, as shown by the results bellow: Without this change: $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal signal: -42.00 dBm $ iperf3 -c 192.168.1.254 Connecting to host 192.168.1.254, port 5201 [ 4] local 192.168.1.253 port 39678 connected to 192.168.1.254 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 735 KBytes 6.02 Mbits/sec1 1.41 KBytes [ 4] 1.00-2.00 sec 274 KBytes 2.25 Mbits/sec1 1.41 KBytes [ 4] 2.00-3.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 3.00-4.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 4.00-5.00 sec 0.00 Bytes 0.00 bits/sec1 28.3 KBytes [ 4] 5.00-6.00 sec 423 KBytes 3.47 Mbits/sec3 41.0 KBytes [ 4] 6.00-7.00 sec 840 KBytes 6.88 Mbits/sec0 58.0 KBytes [ 4] 7.00-8.00 sec 830 KBytes 6.79 Mbits/sec1 1.41 KBytes [ 4] 8.00-9.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 9.00-10.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 4] 0.00-10.00 sec 3.03 MBytes 2.54 Mbits/sec7 sender [ 4] 0.00-10.00 sec 2.88 MBytes 2.41 Mbits/sec receiver iperf Done. With this change: $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal signal: -14.00 dBm $ iperf3 -c 192.168.1.254 Connecting to host 192.168.1.254, port 5201 [ 4] local 192.168.1.253 port 59380 connected to 192.168.1.254 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 4.63 MBytes 38.8 Mbits/sec0194 KBytes [ 4] 1.00-2.00 sec 4.58 MBytes 38.4 Mbits/sec0273 KBytes [ 4] 2.00-3.00 sec 5.05 MBytes 42.3 Mbits/sec0332 KBytes [ 4] 3.00-4.00 sec 4.98 MBytes 41.8 Mbits/sec0393 KBytes [ 4] 4.00-5.00 sec 4.76 MBytes 39.9 Mbits/sec0434 KBytes [ 4] 5.00-6.00 sec 4.85 MBytes 40.7 Mbits/sec0434 KBytes [ 4] 6.00-7.00 sec 3.96 MBytes 33.2 Mbits/sec0464 KBytes [ 4] 7.00-8.00 sec 4.74 MBytes 39.8 Mbits/sec0481 KBytes [ 4] 8.00-9.00 sec 4.22 MBytes 35.4 Mbits/sec0508 KBytes [ 4] 9.00-10.00 sec 4.09 MBytes 34.3 Mbits/sec0564 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 4] 0.00-10.00 sec 45.9 MBytes 38.5 Mbits/sec0 sender [ 4] 0.00-10.00 sec 45.0 MBytes 37.7 Mbits/sec receiver iperf Done. Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- .../realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 62 --- 1 file changed, 12 insertions(+), 50 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c index 86182b917c92..a862b5efdf55 100644 --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c @@ -1452,10 +1452,7 @@ void exhalbtc_init_hw_config(struct btc_coexist *btcoexist, bool wifi_only) else if (btcoexist->board_info.btdm_ant_num == 1) ex_btc8821a1ant_init_hwconfig(btcoexist, wifi_only); } else if (IS_HARDWARE_TYPE_8723B(btcoexist->adapter)) { - if (btcoexist->board_info.btdm_ant_num == 2) - ex_btc8723b2ant_init_hwconfig(btcoexist); - else if (btcoexist->board_info.btdm_ant_num == 1) - ex_btc8723b1ant_init_hwconfig(btcoexist, wifi_only); + ex_btc8723b2ant_init_hwconfig(btcoexist); } else if (IS_HARDWARE_TYPE_8723A(btcoexist->adapter)) { /* 8723A has no this function */ } else if (IS_HARDWARE_TYPE_8192E(btcoexist->adapter)) { @@ -1481,10 +1478,7 @@ void exhalbtc_init_coex_dm(struct btc_coexist *btcoexist) else if (btcoexist->board_info.btdm_ant_num == 1) ex_btc8821a1ant_init_coex_dm(btcoexist); } else if (IS_HARDWARE_TYPE_8723B(btcoexist->adapter)) { - if (btcoexist->board_info.btdm_ant_num == 2) - ex_btc8723b2ant_init_coex_dm(btcoexist); -
[RFC PATCH 3/3] rtlwifi: btcoex: Always use 2ant-functions for RTL8723BE
This partially reverts commit 7937f02d1953952a6eaf626b175ea9db5541e699, which not only hooked external functions for newer ICs, as described in its commit message, but also changed the behavior for RTL8723BE depending on the value of board_info.btdm_ant_num. When board_info.btdm_ant_num == 1, 7937f02d19 changes the codepath to use a whole different set of functions ex_btc8723b1ant_*, instead of the ex_btc8723b2ant_* that were used before it. This drastically affects the upload performance and signal strenght on the HP 240 G4 laptop, as shown by the results bellow: Without this change: $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal signal: -42.00 dBm $ iperf3 -c 192.168.1.254 Connecting to host 192.168.1.254, port 5201 [ 4] local 192.168.1.253 port 39678 connected to 192.168.1.254 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 735 KBytes 6.02 Mbits/sec1 1.41 KBytes [ 4] 1.00-2.00 sec 274 KBytes 2.25 Mbits/sec1 1.41 KBytes [ 4] 2.00-3.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 3.00-4.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 4.00-5.00 sec 0.00 Bytes 0.00 bits/sec1 28.3 KBytes [ 4] 5.00-6.00 sec 423 KBytes 3.47 Mbits/sec3 41.0 KBytes [ 4] 6.00-7.00 sec 840 KBytes 6.88 Mbits/sec0 58.0 KBytes [ 4] 7.00-8.00 sec 830 KBytes 6.79 Mbits/sec1 1.41 KBytes [ 4] 8.00-9.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 9.00-10.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 4] 0.00-10.00 sec 3.03 MBytes 2.54 Mbits/sec7 sender [ 4] 0.00-10.00 sec 2.88 MBytes 2.41 Mbits/sec receiver iperf Done. With this change: $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal signal: -14.00 dBm $ iperf3 -c 192.168.1.254 Connecting to host 192.168.1.254, port 5201 [ 4] local 192.168.1.253 port 59380 connected to 192.168.1.254 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 4.63 MBytes 38.8 Mbits/sec0194 KBytes [ 4] 1.00-2.00 sec 4.58 MBytes 38.4 Mbits/sec0273 KBytes [ 4] 2.00-3.00 sec 5.05 MBytes 42.3 Mbits/sec0332 KBytes [ 4] 3.00-4.00 sec 4.98 MBytes 41.8 Mbits/sec0393 KBytes [ 4] 4.00-5.00 sec 4.76 MBytes 39.9 Mbits/sec0434 KBytes [ 4] 5.00-6.00 sec 4.85 MBytes 40.7 Mbits/sec0434 KBytes [ 4] 6.00-7.00 sec 3.96 MBytes 33.2 Mbits/sec0464 KBytes [ 4] 7.00-8.00 sec 4.74 MBytes 39.8 Mbits/sec0481 KBytes [ 4] 8.00-9.00 sec 4.22 MBytes 35.4 Mbits/sec0508 KBytes [ 4] 9.00-10.00 sec 4.09 MBytes 34.3 Mbits/sec0564 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 4] 0.00-10.00 sec 45.9 MBytes 38.5 Mbits/sec0 sender [ 4] 0.00-10.00 sec 45.0 MBytes 37.7 Mbits/sec receiver iperf Done. Signed-off-by: João Paulo Rechi Vita --- .../realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 62 --- 1 file changed, 12 insertions(+), 50 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c index 86182b917c92..a862b5efdf55 100644 --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c @@ -1452,10 +1452,7 @@ void exhalbtc_init_hw_config(struct btc_coexist *btcoexist, bool wifi_only) else if (btcoexist->board_info.btdm_ant_num == 1) ex_btc8821a1ant_init_hwconfig(btcoexist, wifi_only); } else if (IS_HARDWARE_TYPE_8723B(btcoexist->adapter)) { - if (btcoexist->board_info.btdm_ant_num == 2) - ex_btc8723b2ant_init_hwconfig(btcoexist); - else if (btcoexist->board_info.btdm_ant_num == 1) - ex_btc8723b1ant_init_hwconfig(btcoexist, wifi_only); + ex_btc8723b2ant_init_hwconfig(btcoexist); } else if (IS_HARDWARE_TYPE_8723A(btcoexist->adapter)) { /* 8723A has no this function */ } else if (IS_HARDWARE_TYPE_8192E(btcoexist->adapter)) { @@ -1481,10 +1478,7 @@ void exhalbtc_init_coex_dm(struct btc_coexist *btcoexist) else if (btcoexist->board_info.btdm_ant_num == 1) ex_btc8821a1ant_init_coex_dm(btcoexist); } else if (IS_HARDWARE_TYPE_8723B(btcoexist->adapter)) { - if (btcoexist->board_info.btdm_ant_num == 2) - ex_btc8723b2ant_init_coex_dm(btcoexist); - else if (btcoexist-&g
Re: RTL8723BE performance regression
On Tue, Apr 3, 2018 at 7:51 PM, Larry Finger <larry.fin...@lwfinger.net> wrote: > On 04/03/2018 09:37 PM, João Paulo Rechi Vita wrote: >> >> On Tue, Apr 3, 2018 at 7:28 PM, Larry Finger <larry.fin...@lwfinger.net> >> wrote: >> >> (...) >> >>> As the antenna selection code changes affected your first bisection, do >>> you >>> have one of those HP laptops with only one antenna and the incorrect >>> coding >>> in the FUSE? >> >> >> Yes, that is why I've been passing ant_sel=1 during my tests -- this >> was needed to achieve a good performance in the past, before this >> regression. I've also opened the laptop chassis and confirmed the >> antenna cable is plugged to the connector labeled with "1" on the >> card. >> >>> If so, please make sure that you still have the same signal >>> strength for good and bad cases. I have tried to keep the driver and the >>> btcoex code in sync, but there may be some combinations of antenna >>> configuration and FUSE contents that cause the code to fail. >>> >> >> What is the recommended way to monitor the signal strength? > > > The btcoex code is developed for multiple platforms by a different group > than the Linux driver. I think they made a change that caused ant_sel to > switch from 1 to 2. At least numerous comments at > github.com/lwfinger/rtlwifi_new claimed they needed to make that change. > > Mhy recommended method is to verify the wifi device name with "iw dev". Then > using that device > > sudo iw dev scan | egrep "SSID|signal" > I have confirmed that the performance regression is indeed tied to signal strength: on the good cases signal was between -16 and -8 dBm, whereas in bad cases signal was always between -50 to - 40 dBm. I've also switched to testing bandwidth in controlled LAN environment using iperf3, as suggested by Steve deRosier, with the DUT being the only machine connected to the 2.4 GHz radio and the machine running the iperf3 server connected via ethernet. Using those two tests (iperf3 and signal strength) I've dug deeper into the culprit I had found previously, commit 7937f02d1953, reverting it partially and testing the resulting driver, to isolate which change was causing the problem. Besides "hooking up external functions for newer ICs", as described by the commit message, that commit also added code to decided whether ex_btc8723b1ant_*() or ex_btc8723b2ant_*() functions should be used in halbtcoutsrc.c, depending on the value of board_info.btdm_ant_num, whereas before that commit ex_btc8723b2ant_*() were always used. Reverting to always using ex_btc8723b2ant_*() functions fixes the regression on v4.15. I've also tried to bisect between v4.15..v4.16 to find what else was causing problems there, as the changes mentioned above on top of v4.16 did not solve the problem. The bisect pointed to "874e837d67d0 rtlwifi: fill FW version and subversion", only but reverting it plus the changes mentioned above also didn't yield good results. That's when I decided to get a bit creative: starting on v4.16 I first applied the changes to have ex_btc8723b2ant_*() always being used, as mentioned above, then reverted every commit between v4.15..v4.16 affecting drivers/net/wireless/realtek/rtlwifi/, and verified the resulting kernel had a good performance. Then I started trimming down the history and testing along the way, to reduce to the minimum set of changes that had to be reverted in order to restore the good performance. In addition to the ex_btc8723b2ant_*() changes and reverting "874e837d67d0 rtlwifi: fill FW version and subversion", I've also had to remove the following lines from drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c, which were introduced by "40d9dd4f1c5d rtlwifi: btcoex: Remove global variables from btcoex", in order to restore the upload performance and signal strength. /* set default antenna position to main port */ btcoexist->board_info.btdm_ant_pos = BTC_ANTENNA_AT_MAIN_PORT; These are the results I've got on v4.16 (similarly on wireless-drivers-next-for-davem-2018-03-29 or v4.15): $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal signal: -42.00 dBm $ iperf3 -c 192.168.1.254 Connecting to host 192.168.1.254, port 5201 [ 4] local 192.168.1.253 port 39678 connected to 192.168.1.254 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 735 KBytes 6.02 Mbits/sec1 1.41 KBytes [ 4] 1.00-2.00 sec 274 KBytes 2.25 Mbits/sec1 1.41 KBytes [ 4] 2.00-3.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 3.00-4.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes
Re: RTL8723BE performance regression
On Tue, Apr 3, 2018 at 7:51 PM, Larry Finger wrote: > On 04/03/2018 09:37 PM, João Paulo Rechi Vita wrote: >> >> On Tue, Apr 3, 2018 at 7:28 PM, Larry Finger >> wrote: >> >> (...) >> >>> As the antenna selection code changes affected your first bisection, do >>> you >>> have one of those HP laptops with only one antenna and the incorrect >>> coding >>> in the FUSE? >> >> >> Yes, that is why I've been passing ant_sel=1 during my tests -- this >> was needed to achieve a good performance in the past, before this >> regression. I've also opened the laptop chassis and confirmed the >> antenna cable is plugged to the connector labeled with "1" on the >> card. >> >>> If so, please make sure that you still have the same signal >>> strength for good and bad cases. I have tried to keep the driver and the >>> btcoex code in sync, but there may be some combinations of antenna >>> configuration and FUSE contents that cause the code to fail. >>> >> >> What is the recommended way to monitor the signal strength? > > > The btcoex code is developed for multiple platforms by a different group > than the Linux driver. I think they made a change that caused ant_sel to > switch from 1 to 2. At least numerous comments at > github.com/lwfinger/rtlwifi_new claimed they needed to make that change. > > Mhy recommended method is to verify the wifi device name with "iw dev". Then > using that device > > sudo iw dev scan | egrep "SSID|signal" > I have confirmed that the performance regression is indeed tied to signal strength: on the good cases signal was between -16 and -8 dBm, whereas in bad cases signal was always between -50 to - 40 dBm. I've also switched to testing bandwidth in controlled LAN environment using iperf3, as suggested by Steve deRosier, with the DUT being the only machine connected to the 2.4 GHz radio and the machine running the iperf3 server connected via ethernet. Using those two tests (iperf3 and signal strength) I've dug deeper into the culprit I had found previously, commit 7937f02d1953, reverting it partially and testing the resulting driver, to isolate which change was causing the problem. Besides "hooking up external functions for newer ICs", as described by the commit message, that commit also added code to decided whether ex_btc8723b1ant_*() or ex_btc8723b2ant_*() functions should be used in halbtcoutsrc.c, depending on the value of board_info.btdm_ant_num, whereas before that commit ex_btc8723b2ant_*() were always used. Reverting to always using ex_btc8723b2ant_*() functions fixes the regression on v4.15. I've also tried to bisect between v4.15..v4.16 to find what else was causing problems there, as the changes mentioned above on top of v4.16 did not solve the problem. The bisect pointed to "874e837d67d0 rtlwifi: fill FW version and subversion", only but reverting it plus the changes mentioned above also didn't yield good results. That's when I decided to get a bit creative: starting on v4.16 I first applied the changes to have ex_btc8723b2ant_*() always being used, as mentioned above, then reverted every commit between v4.15..v4.16 affecting drivers/net/wireless/realtek/rtlwifi/, and verified the resulting kernel had a good performance. Then I started trimming down the history and testing along the way, to reduce to the minimum set of changes that had to be reverted in order to restore the good performance. In addition to the ex_btc8723b2ant_*() changes and reverting "874e837d67d0 rtlwifi: fill FW version and subversion", I've also had to remove the following lines from drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c, which were introduced by "40d9dd4f1c5d rtlwifi: btcoex: Remove global variables from btcoex", in order to restore the upload performance and signal strength. /* set default antenna position to main port */ btcoexist->board_info.btdm_ant_pos = BTC_ANTENNA_AT_MAIN_PORT; These are the results I've got on v4.16 (similarly on wireless-drivers-next-for-davem-2018-03-29 or v4.15): $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal signal: -42.00 dBm $ iperf3 -c 192.168.1.254 Connecting to host 192.168.1.254, port 5201 [ 4] local 192.168.1.253 port 39678 connected to 192.168.1.254 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 735 KBytes 6.02 Mbits/sec1 1.41 KBytes [ 4] 1.00-2.00 sec 274 KBytes 2.25 Mbits/sec1 1.41 KBytes [ 4] 2.00-3.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 3.00-4.00 sec 0.00 Bytes 0.00 bits/sec0 1.41 KBytes [ 4] 4.00-5.00 sec 0.00 Bytes 0.00 bits/sec1 28.3
[PATCH] Bluetooth: Add a new 13d3:3496 QCA_ROME device
Without this patch we can't establish a SCO connection with this adapter. This adapter is named "IMC Networks" under lsusb. T: Bus=01 Lev=01 Prnt=01 Port=07 Cnt=02 Dev#= 3 Spd=12 MxCh= 0 D: Ver= 1.10 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs= 1 P: Vendor=13d3 ProdID=3496 Rev= 0.01 C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb E: Ad=81(I) Atr=03(Int.) MxPS= 16 Ivl=1ms E: Ad=82(I) Atr=02(Bulk) MxPS= 64 Ivl=0ms E: Ad=02(O) Atr=02(Bulk) MxPS= 64 Ivl=0ms I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb E: Ad=83(I) Atr=01(Isoc) MxPS= 0 Ivl=1ms E: Ad=03(O) Atr=01(Isoc) MxPS= 0 Ivl=1ms I: If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb E: Ad=83(I) Atr=01(Isoc) MxPS= 9 Ivl=1ms E: Ad=03(O) Atr=01(Isoc) MxPS= 9 Ivl=1ms I: If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb E: Ad=83(I) Atr=01(Isoc) MxPS= 17 Ivl=1ms E: Ad=03(O) Atr=01(Isoc) MxPS= 17 Ivl=1ms I: If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb E: Ad=83(I) Atr=01(Isoc) MxPS= 25 Ivl=1ms E: Ad=03(O) Atr=01(Isoc) MxPS= 25 Ivl=1ms I: If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb E: Ad=83(I) Atr=01(Isoc) MxPS= 33 Ivl=1ms E: Ad=03(O) Atr=01(Isoc) MxPS= 33 Ivl=1ms I: If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb E: Ad=83(I) Atr=01(Isoc) MxPS= 49 Ivl=1ms E: Ad=03(O) Atr=01(Isoc) MxPS= 49 Ivl=1ms Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/bluetooth/btusb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index b8812fee04b2..7094e59f6e58 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -277,6 +277,7 @@ static const struct usb_device_id blacklist_table[] = { { USB_DEVICE(0x04ca, 0x3015), .driver_info = BTUSB_QCA_ROME }, { USB_DEVICE(0x04ca, 0x3016), .driver_info = BTUSB_QCA_ROME }, { USB_DEVICE(0x04ca, 0x301a), .driver_info = BTUSB_QCA_ROME }, + { USB_DEVICE(0x13d3, 0x3496), .driver_info = BTUSB_QCA_ROME }, /* Broadcom BCM2035 */ { USB_DEVICE(0x0a5c, 0x2009), .driver_info = BTUSB_BCM92035 }, -- 2.17.0
[PATCH] Bluetooth: Add a new 13d3:3496 QCA_ROME device
Without this patch we can't establish a SCO connection with this adapter. This adapter is named "IMC Networks" under lsusb. T: Bus=01 Lev=01 Prnt=01 Port=07 Cnt=02 Dev#= 3 Spd=12 MxCh= 0 D: Ver= 1.10 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs= 1 P: Vendor=13d3 ProdID=3496 Rev= 0.01 C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb E: Ad=81(I) Atr=03(Int.) MxPS= 16 Ivl=1ms E: Ad=82(I) Atr=02(Bulk) MxPS= 64 Ivl=0ms E: Ad=02(O) Atr=02(Bulk) MxPS= 64 Ivl=0ms I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb E: Ad=83(I) Atr=01(Isoc) MxPS= 0 Ivl=1ms E: Ad=03(O) Atr=01(Isoc) MxPS= 0 Ivl=1ms I: If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb E: Ad=83(I) Atr=01(Isoc) MxPS= 9 Ivl=1ms E: Ad=03(O) Atr=01(Isoc) MxPS= 9 Ivl=1ms I: If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb E: Ad=83(I) Atr=01(Isoc) MxPS= 17 Ivl=1ms E: Ad=03(O) Atr=01(Isoc) MxPS= 17 Ivl=1ms I: If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb E: Ad=83(I) Atr=01(Isoc) MxPS= 25 Ivl=1ms E: Ad=03(O) Atr=01(Isoc) MxPS= 25 Ivl=1ms I: If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb E: Ad=83(I) Atr=01(Isoc) MxPS= 33 Ivl=1ms E: Ad=03(O) Atr=01(Isoc) MxPS= 33 Ivl=1ms I: If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb E: Ad=83(I) Atr=01(Isoc) MxPS= 49 Ivl=1ms E: Ad=03(O) Atr=01(Isoc) MxPS= 49 Ivl=1ms Signed-off-by: João Paulo Rechi Vita --- drivers/bluetooth/btusb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index b8812fee04b2..7094e59f6e58 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -277,6 +277,7 @@ static const struct usb_device_id blacklist_table[] = { { USB_DEVICE(0x04ca, 0x3015), .driver_info = BTUSB_QCA_ROME }, { USB_DEVICE(0x04ca, 0x3016), .driver_info = BTUSB_QCA_ROME }, { USB_DEVICE(0x04ca, 0x301a), .driver_info = BTUSB_QCA_ROME }, + { USB_DEVICE(0x13d3, 0x3496), .driver_info = BTUSB_QCA_ROME }, /* Broadcom BCM2035 */ { USB_DEVICE(0x0a5c, 0x2009), .driver_info = BTUSB_BCM92035 }, -- 2.17.0
[PATCH v2] platform/x86: asus-wireless: Fix NULL pointer dereference
When the module is removed the led workqueue is destroyed in the remove callback, before the led device is unregistered from the led subsystem. This leads to a NULL pointer derefence when the led device is unregistered automatically later as part of the module removal cleanup. Bellow is the backtrace showing the problem. BUG: unable to handle kernel NULL pointer dereference at (null) IP: __queue_work+0x8c/0x410 PGD 0 P4D 0 Oops: [#1] SMP NOPTI Modules linked in: ccm edac_mce_amd kvm_amd kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64 joydev crypto_simd asus_nb_wmi glue_helper uvcvideo snd_hda_codec_conexant snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_intel asus_wmi snd_hda_codec cryptd snd_hda_core sparse_keymap videobuf2_vmalloc arc4 videobuf2_memops snd_hwdep input_leds videobuf2_v4l2 ath9k psmouse videobuf2_core videodev ath9k_common snd_pcm ath9k_hw media fam15h_power ath k10temp snd_timer mac80211 i2c_piix4 r8169 mii mac_hid cfg80211 asus_wireless(-) snd soundcore wmi shpchp 8250_dw ip_tables x_tables amdkfd amd_iommu_v2 amdgpu radeon chash i2c_algo_bit drm_kms_helper syscopyarea serio_raw sysfillrect sysimgblt fb_sys_fops ahci ttm libahci drm video CPU: 3 PID: 2177 Comm: rmmod Not tainted 4.15.0-5-generic #6+dev94.b4287e5bem1-Endless Hardware name: ASUSTeK COMPUTER INC. X555DG/X555DG, BIOS 5.011 05/05/2015 RIP: 0010:__queue_work+0x8c/0x410 RSP: 0018:be8cc249fcd8 EFLAGS: 00010086 RAX: 992ac6810800 RBX: RCX: 0008 RDX: RSI: 0008 RDI: 992ac6400e18 RBP: be8cc249fd18 R08: 992ac6400db0 R09: R10: 0040 R11: 992ac6400dd8 R12: 2000 R13: 992abd762e00 R14: 992abd763e38 R15: 0001ebe0 FS: 7f318203e700() GS:992aced8() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 0001c720e000 CR4: 001406e0 Call Trace: queue_work_on+0x38/0x40 led_state_set+0x2c/0x40 [asus_wireless] led_set_brightness_nopm+0x14/0x40 led_set_brightness+0x37/0x60 led_trigger_set+0xfc/0x1d0 led_classdev_unregister+0x32/0xd0 devm_led_classdev_release+0x11/0x20 release_nodes+0x109/0x1f0 devres_release_all+0x3c/0x50 device_release_driver_internal+0x16d/0x220 driver_detach+0x3f/0x80 bus_remove_driver+0x55/0xd0 driver_unregister+0x2c/0x40 acpi_bus_unregister_driver+0x15/0x20 asus_wireless_driver_exit+0x10/0xb7c [asus_wireless] SyS_delete_module+0x1da/0x2b0 entry_SYSCALL_64_fastpath+0x24/0x87 RIP: 0033:0x7f3181b65fd7 RSP: 002b:7ffe74bcbe18 EFLAGS: 0206 ORIG_RAX: 00b0 RAX: ffda RBX: RCX: 7f3181b65fd7 RDX: 000a RSI: 0800 RDI: 555ea2559258 RBP: 555ea25591f0 R08: 7ffe74bcad91 R09: 000a R10: R11: 0206 R12: 0003 R13: 7ffe74bcae00 R14: R15: 555ea25591f0 Code: 01 00 00 02 0f 85 7d 01 00 00 48 63 45 d4 48 c7 c6 00 f4 fa 87 49 8b 9d 08 01 00 00 48 03 1c c6 4c 89 f7 e8 87 fb ff ff 48 85 c0 <48> 8b 3b 0f 84 c5 01 00 00 48 39 f8 0f 84 bc 01 00 00 48 89 c7 RIP: __queue_work+0x8c/0x410 RSP: be8cc249fcd8 CR2: ---[ end trace 7aa4f4a232e9c39c ]--- Unregistering the led device on the remove callback before destroying the workqueue avoids this problem. https://bugzilla.kernel.org/show_bug.cgi?id=196097 Reported-by: Dun Hum <bitter.ta...@gmx.com> Cc: sta...@vger.kernel.org Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/platform/x86/asus-wireless.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c index 343e12547660..b8e35a8d65cf 100644 --- a/drivers/platform/x86/asus-wireless.c +++ b/drivers/platform/x86/asus-wireless.c @@ -181,8 +181,10 @@ static int asus_wireless_remove(struct acpi_device *adev) { struct asus_wireless_data *data = acpi_driver_data(adev); - if (data->wq) + if (data->wq) { + devm_led_classdev_unregister(>dev, >led); destroy_workqueue(data->wq); + } return 0; } -- 2.17.0
[PATCH v2] platform/x86: asus-wireless: Fix NULL pointer dereference
When the module is removed the led workqueue is destroyed in the remove callback, before the led device is unregistered from the led subsystem. This leads to a NULL pointer derefence when the led device is unregistered automatically later as part of the module removal cleanup. Bellow is the backtrace showing the problem. BUG: unable to handle kernel NULL pointer dereference at (null) IP: __queue_work+0x8c/0x410 PGD 0 P4D 0 Oops: [#1] SMP NOPTI Modules linked in: ccm edac_mce_amd kvm_amd kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64 joydev crypto_simd asus_nb_wmi glue_helper uvcvideo snd_hda_codec_conexant snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_intel asus_wmi snd_hda_codec cryptd snd_hda_core sparse_keymap videobuf2_vmalloc arc4 videobuf2_memops snd_hwdep input_leds videobuf2_v4l2 ath9k psmouse videobuf2_core videodev ath9k_common snd_pcm ath9k_hw media fam15h_power ath k10temp snd_timer mac80211 i2c_piix4 r8169 mii mac_hid cfg80211 asus_wireless(-) snd soundcore wmi shpchp 8250_dw ip_tables x_tables amdkfd amd_iommu_v2 amdgpu radeon chash i2c_algo_bit drm_kms_helper syscopyarea serio_raw sysfillrect sysimgblt fb_sys_fops ahci ttm libahci drm video CPU: 3 PID: 2177 Comm: rmmod Not tainted 4.15.0-5-generic #6+dev94.b4287e5bem1-Endless Hardware name: ASUSTeK COMPUTER INC. X555DG/X555DG, BIOS 5.011 05/05/2015 RIP: 0010:__queue_work+0x8c/0x410 RSP: 0018:be8cc249fcd8 EFLAGS: 00010086 RAX: 992ac6810800 RBX: RCX: 0008 RDX: RSI: 0008 RDI: 992ac6400e18 RBP: be8cc249fd18 R08: 992ac6400db0 R09: R10: 0040 R11: 992ac6400dd8 R12: 2000 R13: 992abd762e00 R14: 992abd763e38 R15: 0001ebe0 FS: 7f318203e700() GS:992aced8() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 0001c720e000 CR4: 001406e0 Call Trace: queue_work_on+0x38/0x40 led_state_set+0x2c/0x40 [asus_wireless] led_set_brightness_nopm+0x14/0x40 led_set_brightness+0x37/0x60 led_trigger_set+0xfc/0x1d0 led_classdev_unregister+0x32/0xd0 devm_led_classdev_release+0x11/0x20 release_nodes+0x109/0x1f0 devres_release_all+0x3c/0x50 device_release_driver_internal+0x16d/0x220 driver_detach+0x3f/0x80 bus_remove_driver+0x55/0xd0 driver_unregister+0x2c/0x40 acpi_bus_unregister_driver+0x15/0x20 asus_wireless_driver_exit+0x10/0xb7c [asus_wireless] SyS_delete_module+0x1da/0x2b0 entry_SYSCALL_64_fastpath+0x24/0x87 RIP: 0033:0x7f3181b65fd7 RSP: 002b:7ffe74bcbe18 EFLAGS: 0206 ORIG_RAX: 00b0 RAX: ffda RBX: RCX: 7f3181b65fd7 RDX: 000a RSI: 0800 RDI: 555ea2559258 RBP: 555ea25591f0 R08: 7ffe74bcad91 R09: 000a R10: R11: 0206 R12: 0003 R13: 7ffe74bcae00 R14: R15: 555ea25591f0 Code: 01 00 00 02 0f 85 7d 01 00 00 48 63 45 d4 48 c7 c6 00 f4 fa 87 49 8b 9d 08 01 00 00 48 03 1c c6 4c 89 f7 e8 87 fb ff ff 48 85 c0 <48> 8b 3b 0f 84 c5 01 00 00 48 39 f8 0f 84 bc 01 00 00 48 89 c7 RIP: __queue_work+0x8c/0x410 RSP: be8cc249fcd8 CR2: ---[ end trace 7aa4f4a232e9c39c ]--- Unregistering the led device on the remove callback before destroying the workqueue avoids this problem. https://bugzilla.kernel.org/show_bug.cgi?id=196097 Reported-by: Dun Hum Cc: sta...@vger.kernel.org Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-wireless.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c index 343e12547660..b8e35a8d65cf 100644 --- a/drivers/platform/x86/asus-wireless.c +++ b/drivers/platform/x86/asus-wireless.c @@ -181,8 +181,10 @@ static int asus_wireless_remove(struct acpi_device *adev) { struct asus_wireless_data *data = acpi_driver_data(adev); - if (data->wq) + if (data->wq) { + devm_led_classdev_unregister(>dev, >led); destroy_workqueue(data->wq); + } return 0; } -- 2.17.0
Re: [PATCH] platform/x86: asus-wireless: Fix NULL pointer dereference
On Sat, Apr 7, 2018 at 8:50 AM, Darren Hart <dvh...@infradead.org> wrote: > On Fri, Apr 06, 2018 at 10:37:29PM -0700, João Paulo Rechi Vita wrote: >> When the module is removed the led workqueue is destroyed in the remove >> callback, before the led device is unregistered from the led subsystem. >> >> This leads to a NULL pointer derefence when the led device is >> unregistered automatically later as part of the module removal cleanup. >> Bellow is the backtrace showing the problem. >> > > Thanks João Paulo, > ... > >> Unregistering the led device on the remove callback before destroying the >> workqueue avoids this problem. >> >> https://bugzilla.kernel.org/show_bug.cgi?id=196097 >> >> Reported-by: Dun Hum <bitter.ta...@gmx.com> >> Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> >> --- >> drivers/platform/x86/asus-wireless.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/platform/x86/asus-wireless.c >> b/drivers/platform/x86/asus-wireless.c >> index 343e12547660..ecd715c82de5 100644 >> --- a/drivers/platform/x86/asus-wireless.c >> +++ b/drivers/platform/x86/asus-wireless.c >> @@ -181,6 +181,7 @@ static int asus_wireless_remove(struct acpi_device *adev) >> { >> struct asus_wireless_data *data = acpi_driver_data(adev); >> >> + devm_led_classdev_unregister(>dev, >led); >> if (data->wq) >> destroy_workqueue(data->wq); >> return 0; > > asus_wireless_add only calls devm_led_classdev_register() iff the workqueue is > successfully created. It seems like it would make sense to move the > devm_led_classdev_unregister() call within the 'if (data->wq)' condition > block. > I agree. I didn't do it this way initially as I believe the call to devm_led_classdev_unregister() would be harmless in any case, but it indeed best to avoid it if we can. > This should also cc stable. > Adding to v2. -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH] platform/x86: asus-wireless: Fix NULL pointer dereference
On Sat, Apr 7, 2018 at 8:50 AM, Darren Hart wrote: > On Fri, Apr 06, 2018 at 10:37:29PM -0700, João Paulo Rechi Vita wrote: >> When the module is removed the led workqueue is destroyed in the remove >> callback, before the led device is unregistered from the led subsystem. >> >> This leads to a NULL pointer derefence when the led device is >> unregistered automatically later as part of the module removal cleanup. >> Bellow is the backtrace showing the problem. >> > > Thanks João Paulo, > ... > >> Unregistering the led device on the remove callback before destroying the >> workqueue avoids this problem. >> >> https://bugzilla.kernel.org/show_bug.cgi?id=196097 >> >> Reported-by: Dun Hum >> Signed-off-by: João Paulo Rechi Vita >> --- >> drivers/platform/x86/asus-wireless.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/platform/x86/asus-wireless.c >> b/drivers/platform/x86/asus-wireless.c >> index 343e12547660..ecd715c82de5 100644 >> --- a/drivers/platform/x86/asus-wireless.c >> +++ b/drivers/platform/x86/asus-wireless.c >> @@ -181,6 +181,7 @@ static int asus_wireless_remove(struct acpi_device *adev) >> { >> struct asus_wireless_data *data = acpi_driver_data(adev); >> >> + devm_led_classdev_unregister(>dev, >led); >> if (data->wq) >> destroy_workqueue(data->wq); >> return 0; > > asus_wireless_add only calls devm_led_classdev_register() iff the workqueue is > successfully created. It seems like it would make sense to move the > devm_led_classdev_unregister() call within the 'if (data->wq)' condition > block. > I agree. I didn't do it this way initially as I believe the call to devm_led_classdev_unregister() would be harmless in any case, but it indeed best to avoid it if we can. > This should also cc stable. > Adding to v2. -- João Paulo Rechi Vita http://about.me/jprvita
[PATCH] platform/x86: asus-wireless: Fix NULL pointer dereference
When the module is removed the led workqueue is destroyed in the remove callback, before the led device is unregistered from the led subsystem. This leads to a NULL pointer derefence when the led device is unregistered automatically later as part of the module removal cleanup. Bellow is the backtrace showing the problem. BUG: unable to handle kernel NULL pointer dereference at (null) IP: __queue_work+0x8c/0x410 PGD 0 P4D 0 Oops: [#1] SMP NOPTI Modules linked in: ccm edac_mce_amd kvm_amd kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64 joydev crypto_simd asus_nb_wmi glue_helper uvcvideo snd_hda_codec_conexant snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_intel asus_wmi snd_hda_codec cryptd snd_hda_core sparse_keymap videobuf2_vmalloc arc4 videobuf2_memops snd_hwdep input_leds videobuf2_v4l2 ath9k psmouse videobuf2_core videodev ath9k_common snd_pcm ath9k_hw media fam15h_power ath k10temp snd_timer mac80211 i2c_piix4 r8169 mii mac_hid cfg80211 asus_wireless(-) snd soundcore wmi shpchp 8250_dw ip_tables x_tables amdkfd amd_iommu_v2 amdgpu radeon chash i2c_algo_bit drm_kms_helper syscopyarea serio_raw sysfillrect sysimgblt fb_sys_fops ahci ttm libahci drm video CPU: 3 PID: 2177 Comm: rmmod Not tainted 4.15.0-5-generic #6+dev94.b4287e5bem1-Endless Hardware name: ASUSTeK COMPUTER INC. X555DG/X555DG, BIOS 5.011 05/05/2015 RIP: 0010:__queue_work+0x8c/0x410 RSP: 0018:be8cc249fcd8 EFLAGS: 00010086 RAX: 992ac6810800 RBX: RCX: 0008 RDX: RSI: 0008 RDI: 992ac6400e18 RBP: be8cc249fd18 R08: 992ac6400db0 R09: R10: 0040 R11: 992ac6400dd8 R12: 2000 R13: 992abd762e00 R14: 992abd763e38 R15: 0001ebe0 FS: 7f318203e700() GS:992aced8() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 0001c720e000 CR4: 001406e0 Call Trace: queue_work_on+0x38/0x40 led_state_set+0x2c/0x40 [asus_wireless] led_set_brightness_nopm+0x14/0x40 led_set_brightness+0x37/0x60 led_trigger_set+0xfc/0x1d0 led_classdev_unregister+0x32/0xd0 devm_led_classdev_release+0x11/0x20 release_nodes+0x109/0x1f0 devres_release_all+0x3c/0x50 device_release_driver_internal+0x16d/0x220 driver_detach+0x3f/0x80 bus_remove_driver+0x55/0xd0 driver_unregister+0x2c/0x40 acpi_bus_unregister_driver+0x15/0x20 asus_wireless_driver_exit+0x10/0xb7c [asus_wireless] SyS_delete_module+0x1da/0x2b0 entry_SYSCALL_64_fastpath+0x24/0x87 RIP: 0033:0x7f3181b65fd7 RSP: 002b:7ffe74bcbe18 EFLAGS: 0206 ORIG_RAX: 00b0 RAX: ffda RBX: RCX: 7f3181b65fd7 RDX: 000a RSI: 0800 RDI: 555ea2559258 RBP: 555ea25591f0 R08: 7ffe74bcad91 R09: 000a R10: R11: 0206 R12: 0003 R13: 7ffe74bcae00 R14: R15: 555ea25591f0 Code: 01 00 00 02 0f 85 7d 01 00 00 48 63 45 d4 48 c7 c6 00 f4 fa 87 49 8b 9d 08 01 00 00 48 03 1c c6 4c 89 f7 e8 87 fb ff ff 48 85 c0 <48> 8b 3b 0f 84 c5 01 00 00 48 39 f8 0f 84 bc 01 00 00 48 89 c7 RIP: __queue_work+0x8c/0x410 RSP: be8cc249fcd8 CR2: ---[ end trace 7aa4f4a232e9c39c ]--- Unregistering the led device on the remove callback before destroying the workqueue avoids this problem. https://bugzilla.kernel.org/show_bug.cgi?id=196097 Reported-by: Dun Hum <bitter.ta...@gmx.com> Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/platform/x86/asus-wireless.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c index 343e12547660..ecd715c82de5 100644 --- a/drivers/platform/x86/asus-wireless.c +++ b/drivers/platform/x86/asus-wireless.c @@ -181,6 +181,7 @@ static int asus_wireless_remove(struct acpi_device *adev) { struct asus_wireless_data *data = acpi_driver_data(adev); + devm_led_classdev_unregister(>dev, >led); if (data->wq) destroy_workqueue(data->wq); return 0; -- 2.16.3
[PATCH] platform/x86: asus-wireless: Fix NULL pointer dereference
When the module is removed the led workqueue is destroyed in the remove callback, before the led device is unregistered from the led subsystem. This leads to a NULL pointer derefence when the led device is unregistered automatically later as part of the module removal cleanup. Bellow is the backtrace showing the problem. BUG: unable to handle kernel NULL pointer dereference at (null) IP: __queue_work+0x8c/0x410 PGD 0 P4D 0 Oops: [#1] SMP NOPTI Modules linked in: ccm edac_mce_amd kvm_amd kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64 joydev crypto_simd asus_nb_wmi glue_helper uvcvideo snd_hda_codec_conexant snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_intel asus_wmi snd_hda_codec cryptd snd_hda_core sparse_keymap videobuf2_vmalloc arc4 videobuf2_memops snd_hwdep input_leds videobuf2_v4l2 ath9k psmouse videobuf2_core videodev ath9k_common snd_pcm ath9k_hw media fam15h_power ath k10temp snd_timer mac80211 i2c_piix4 r8169 mii mac_hid cfg80211 asus_wireless(-) snd soundcore wmi shpchp 8250_dw ip_tables x_tables amdkfd amd_iommu_v2 amdgpu radeon chash i2c_algo_bit drm_kms_helper syscopyarea serio_raw sysfillrect sysimgblt fb_sys_fops ahci ttm libahci drm video CPU: 3 PID: 2177 Comm: rmmod Not tainted 4.15.0-5-generic #6+dev94.b4287e5bem1-Endless Hardware name: ASUSTeK COMPUTER INC. X555DG/X555DG, BIOS 5.011 05/05/2015 RIP: 0010:__queue_work+0x8c/0x410 RSP: 0018:be8cc249fcd8 EFLAGS: 00010086 RAX: 992ac6810800 RBX: RCX: 0008 RDX: RSI: 0008 RDI: 992ac6400e18 RBP: be8cc249fd18 R08: 992ac6400db0 R09: R10: 0040 R11: 992ac6400dd8 R12: 2000 R13: 992abd762e00 R14: 992abd763e38 R15: 0001ebe0 FS: 7f318203e700() GS:992aced8() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 0001c720e000 CR4: 001406e0 Call Trace: queue_work_on+0x38/0x40 led_state_set+0x2c/0x40 [asus_wireless] led_set_brightness_nopm+0x14/0x40 led_set_brightness+0x37/0x60 led_trigger_set+0xfc/0x1d0 led_classdev_unregister+0x32/0xd0 devm_led_classdev_release+0x11/0x20 release_nodes+0x109/0x1f0 devres_release_all+0x3c/0x50 device_release_driver_internal+0x16d/0x220 driver_detach+0x3f/0x80 bus_remove_driver+0x55/0xd0 driver_unregister+0x2c/0x40 acpi_bus_unregister_driver+0x15/0x20 asus_wireless_driver_exit+0x10/0xb7c [asus_wireless] SyS_delete_module+0x1da/0x2b0 entry_SYSCALL_64_fastpath+0x24/0x87 RIP: 0033:0x7f3181b65fd7 RSP: 002b:7ffe74bcbe18 EFLAGS: 0206 ORIG_RAX: 00b0 RAX: ffda RBX: RCX: 7f3181b65fd7 RDX: 000a RSI: 0800 RDI: 555ea2559258 RBP: 555ea25591f0 R08: 7ffe74bcad91 R09: 000a R10: R11: 0206 R12: 0003 R13: 7ffe74bcae00 R14: R15: 555ea25591f0 Code: 01 00 00 02 0f 85 7d 01 00 00 48 63 45 d4 48 c7 c6 00 f4 fa 87 49 8b 9d 08 01 00 00 48 03 1c c6 4c 89 f7 e8 87 fb ff ff 48 85 c0 <48> 8b 3b 0f 84 c5 01 00 00 48 39 f8 0f 84 bc 01 00 00 48 89 c7 RIP: __queue_work+0x8c/0x410 RSP: be8cc249fcd8 CR2: ---[ end trace 7aa4f4a232e9c39c ]--- Unregistering the led device on the remove callback before destroying the workqueue avoids this problem. https://bugzilla.kernel.org/show_bug.cgi?id=196097 Reported-by: Dun Hum Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-wireless.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c index 343e12547660..ecd715c82de5 100644 --- a/drivers/platform/x86/asus-wireless.c +++ b/drivers/platform/x86/asus-wireless.c @@ -181,6 +181,7 @@ static int asus_wireless_remove(struct acpi_device *adev) { struct asus_wireless_data *data = acpi_driver_data(adev); + devm_led_classdev_unregister(>dev, >led); if (data->wq) destroy_workqueue(data->wq); return 0; -- 2.16.3
Re: RTL8723BE performance regression
On Tue, Apr 3, 2018 at 7:28 PM, Larry Finger <larry.fin...@lwfinger.net> wrote: (...) > As the antenna selection code changes affected your first bisection, do you > have one of those HP laptops with only one antenna and the incorrect coding > in the FUSE? Yes, that is why I've been passing ant_sel=1 during my tests -- this was needed to achieve a good performance in the past, before this regression. I've also opened the laptop chassis and confirmed the antenna cable is plugged to the connector labeled with "1" on the card. > If so, please make sure that you still have the same signal > strength for good and bad cases. I have tried to keep the driver and the > btcoex code in sync, but there may be some combinations of antenna > configuration and FUSE contents that cause the code to fail. > What is the recommended way to monitor the signal strength? Thanks for such a quick reply, -- João Paulo Rechi Vita http://about.me/jprvita
Re: RTL8723BE performance regression
On Tue, Apr 3, 2018 at 7:28 PM, Larry Finger wrote: (...) > As the antenna selection code changes affected your first bisection, do you > have one of those HP laptops with only one antenna and the incorrect coding > in the FUSE? Yes, that is why I've been passing ant_sel=1 during my tests -- this was needed to achieve a good performance in the past, before this regression. I've also opened the laptop chassis and confirmed the antenna cable is plugged to the connector labeled with "1" on the card. > If so, please make sure that you still have the same signal > strength for good and bad cases. I have tried to keep the driver and the > btcoex code in sync, but there may be some combinations of antenna > configuration and FUSE contents that cause the code to fail. > What is the recommended way to monitor the signal strength? Thanks for such a quick reply, -- João Paulo Rechi Vita http://about.me/jprvita
RTL8723BE performance regression
Hello, I've been trying to track a performance regression on the RTL8723BE WiFi adapter, which mainly affects the upload bandwidth (although we can see a decreased download performance as well, the effect on upload is more drastic). This was first reported by users after upgrading from our 4.11-based kernel to our 4.13-based kernel, but also confirmed to affect our development branch (4.15-based kernel) and wireless-drivers-next at the wireless-drivers-next-for-davem-2018-03-29 tag. This is happening on an HP laptop that needs rtl8723be.ant_sel=1 (and all the following tests have been made with that param). My first bisect attempt pointed me to the following commit: bcd37f4a0831 rtlwifi: btcoex: 23b 2ant: let bt transmit when hw initialisation done Which I later found to be already fixed by a33fcba6ec01 rtlwifi: btcoexist: Fix breakage of ant_sel for rtl8723be. That fix is already included in v4.15 though (and our dev branch as well), so I did a second bisect, now cherry-picking a33fcba6ec01 at every step, and it pointed me to the following commit: 7937f02d1953 rtlwifi: btcoex: hook external functions for newer chips Reverting that commit on top of our development branch fixes the problem, but on top of v4.15 I get mixed results: a few times getting a good upload performance (~5-6Mbps) but most of the time just getting ~1-1.5Mpbs (which is still better than the 0.0 then test failure I've gotten on most bad points of the bisect). Bisecting the downstream patches we carry on top of v4.15 (we base our kernel on Ubuntu's, so there are quite a few downstream changes) did not bring any clarity, as at all bisect points (plus reverting 7937f02d1953) the performance was good, so probably there was some other difference in the resulting kernels from my initial revert of that patch on top of v4.15 and each step during the bisect. I've experimented a bit with fwlps=0, but it did not bring any conclusive results either. I'll try to look at other things that may have changed (configuration perhaps?), but I don't have a clear plan yet. Have you seen anything similar, or have any other ideas or suggestions to track this problem? Even without crystal clear results, it looks like 7937f02d1953 is having a negative impact on the RTL8723BE performance, so perhaps it is worth reverting it and reworking it a later point? This are the results (testing with speedtest.net) I got at some key points: VersionCommitPingDownUp v4.11a351e9b1225.445.99 v4.11a351e9b131 17.025.89 v4.13569dbb8174 14.080.00 v4.13569dbb8261 8.41 0.00 v4.15+revert d8a5b801923.861.41 v4.15+revert d8a5b80189 18.691.39 Best regards, -- João Paulo Rechi Vita http://about.me/jprvita
RTL8723BE performance regression
Hello, I've been trying to track a performance regression on the RTL8723BE WiFi adapter, which mainly affects the upload bandwidth (although we can see a decreased download performance as well, the effect on upload is more drastic). This was first reported by users after upgrading from our 4.11-based kernel to our 4.13-based kernel, but also confirmed to affect our development branch (4.15-based kernel) and wireless-drivers-next at the wireless-drivers-next-for-davem-2018-03-29 tag. This is happening on an HP laptop that needs rtl8723be.ant_sel=1 (and all the following tests have been made with that param). My first bisect attempt pointed me to the following commit: bcd37f4a0831 rtlwifi: btcoex: 23b 2ant: let bt transmit when hw initialisation done Which I later found to be already fixed by a33fcba6ec01 rtlwifi: btcoexist: Fix breakage of ant_sel for rtl8723be. That fix is already included in v4.15 though (and our dev branch as well), so I did a second bisect, now cherry-picking a33fcba6ec01 at every step, and it pointed me to the following commit: 7937f02d1953 rtlwifi: btcoex: hook external functions for newer chips Reverting that commit on top of our development branch fixes the problem, but on top of v4.15 I get mixed results: a few times getting a good upload performance (~5-6Mbps) but most of the time just getting ~1-1.5Mpbs (which is still better than the 0.0 then test failure I've gotten on most bad points of the bisect). Bisecting the downstream patches we carry on top of v4.15 (we base our kernel on Ubuntu's, so there are quite a few downstream changes) did not bring any clarity, as at all bisect points (plus reverting 7937f02d1953) the performance was good, so probably there was some other difference in the resulting kernels from my initial revert of that patch on top of v4.15 and each step during the bisect. I've experimented a bit with fwlps=0, but it did not bring any conclusive results either. I'll try to look at other things that may have changed (configuration perhaps?), but I don't have a clear plan yet. Have you seen anything similar, or have any other ideas or suggestions to track this problem? Even without crystal clear results, it looks like 7937f02d1953 is having a negative impact on the RTL8723BE performance, so perhaps it is worth reverting it and reworking it a later point? This are the results (testing with speedtest.net) I got at some key points: VersionCommitPingDownUp v4.11a351e9b1225.445.99 v4.11a351e9b131 17.025.89 v4.13569dbb8174 14.080.00 v4.13569dbb8261 8.41 0.00 v4.15+revert d8a5b801923.861.41 v4.15+revert d8a5b80189 18.691.39 Best regards, -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH 2/2] HID: multitouch: Support Asus T304UA media keys
On Thu, Aug 3, 2017 at 2:24 AM, Jiri Kosina <ji...@kernel.org> wrote: > On Wed, 2 Aug 2017, Benjamin Tissoires wrote: > >> Sorry for the delay. I was at GUADEC the whole past week and couldn't >> get much kernel work done. I was thinking a little bit about this series >> though. Patch 1 is fine, but patch 2 is a little bit more of an issue. >> Ideally, I'd like to keep hid-multitouch from having too many vendor >> specific code, but it looks like this is the easier way to handle things >> here. >> >> I guess the proper way of solving this situation would be to merge the >> generic windows 8 code from hid-multitouch into hid-input so that other >> drivers can benefit from it, but this is going to be a lot of work and >> -ETIME. > > Yes, I actually have this on the list of things I'd eventually like to > look into one day ... but we shouldn't let this block any further > development. > > I have applied the series now to for-4.14/multitouch (fixing up the > 0xff310076 -> HID_VD_ASUS_CUSTOM_MEDIA_KEYS constant in the condition as a > followup). > Thanks! -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH 2/2] HID: multitouch: Support Asus T304UA media keys
On Thu, Aug 3, 2017 at 2:24 AM, Jiri Kosina wrote: > On Wed, 2 Aug 2017, Benjamin Tissoires wrote: > >> Sorry for the delay. I was at GUADEC the whole past week and couldn't >> get much kernel work done. I was thinking a little bit about this series >> though. Patch 1 is fine, but patch 2 is a little bit more of an issue. >> Ideally, I'd like to keep hid-multitouch from having too many vendor >> specific code, but it looks like this is the easier way to handle things >> here. >> >> I guess the proper way of solving this situation would be to merge the >> generic windows 8 code from hid-multitouch into hid-input so that other >> drivers can benefit from it, but this is going to be a lot of work and >> -ETIME. > > Yes, I actually have this on the list of things I'd eventually like to > look into one day ... but we shouldn't let this block any further > development. > > I have applied the series now to for-4.14/multitouch (fixing up the > 0xff310076 -> HID_VD_ASUS_CUSTOM_MEDIA_KEYS constant in the condition as a > followup). > Thanks! -- João Paulo Rechi Vita http://about.me/jprvita
[PATCH v3] iwlwifi: Demote messages about fw flags size to info
These messages are not reporting a real error, just the fact that the firmware knows about more flags than the driver. Currently these messages are presented to the user during boot if there is no bootsplash covering the console, even when booting the kernel with "quiet". Demoting it to the warn level helps having a clean boot process. Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- v2 changes: - Set to warn level instead of info v3 changes: - Fix commit message typo drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c index be466a074c1d..c98a254bbd4d 100644 --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c @@ -461,9 +461,9 @@ static int iwl_set_ucode_api_flags(struct iwl_drv *drv, const u8 *data, int i; if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_API, 32)) { - IWL_ERR(drv, - "api flags index %d larger than supported by driver\n", - api_index); + IWL_WARN(drv, +"api flags index %d larger than supported by driver\n", +api_index); /* don't return an error so we can load FW that has more bits */ return 0; } @@ -485,9 +485,9 @@ static int iwl_set_ucode_capabilities(struct iwl_drv *drv, const u8 *data, int i; if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_CAPA, 32)) { - IWL_ERR(drv, - "capa flags index %d larger than supported by driver\n", - api_index); + IWL_WARN(drv, +"capa flags index %d larger than supported by driver\n", +api_index); /* don't return an error so we can load FW that has more bits */ return 0; } -- 2.13.2
[PATCH v3] iwlwifi: Demote messages about fw flags size to info
These messages are not reporting a real error, just the fact that the firmware knows about more flags than the driver. Currently these messages are presented to the user during boot if there is no bootsplash covering the console, even when booting the kernel with "quiet". Demoting it to the warn level helps having a clean boot process. Signed-off-by: João Paulo Rechi Vita --- v2 changes: - Set to warn level instead of info v3 changes: - Fix commit message typo drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c index be466a074c1d..c98a254bbd4d 100644 --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c @@ -461,9 +461,9 @@ static int iwl_set_ucode_api_flags(struct iwl_drv *drv, const u8 *data, int i; if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_API, 32)) { - IWL_ERR(drv, - "api flags index %d larger than supported by driver\n", - api_index); + IWL_WARN(drv, +"api flags index %d larger than supported by driver\n", +api_index); /* don't return an error so we can load FW that has more bits */ return 0; } @@ -485,9 +485,9 @@ static int iwl_set_ucode_capabilities(struct iwl_drv *drv, const u8 *data, int i; if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_CAPA, 32)) { - IWL_ERR(drv, - "capa flags index %d larger than supported by driver\n", - api_index); + IWL_WARN(drv, +"capa flags index %d larger than supported by driver\n", +api_index); /* don't return an error so we can load FW that has more bits */ return 0; } -- 2.13.2
[PATCH v2] iwlwifi: Demote messages about fw flags size to info
These messages are not reporting a real error, just the fact that the firmware knows about more flags then the driver. Currently these messages are presented to the user during boot if there is no bootsplash covering the console, even when booting the kernel with "quiet". Demoting it to the warn level helps having a clean boot process. Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- v2 changes: - Set to warn level instead of info drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c index be466a074c1d..c98a254bbd4d 100644 --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c @@ -461,9 +461,9 @@ static int iwl_set_ucode_api_flags(struct iwl_drv *drv, const u8 *data, int i; if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_API, 32)) { - IWL_ERR(drv, - "api flags index %d larger than supported by driver\n", - api_index); + IWL_WARN(drv, +"api flags index %d larger than supported by driver\n", +api_index); /* don't return an error so we can load FW that has more bits */ return 0; } @@ -485,9 +485,9 @@ static int iwl_set_ucode_capabilities(struct iwl_drv *drv, const u8 *data, int i; if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_CAPA, 32)) { - IWL_ERR(drv, - "capa flags index %d larger than supported by driver\n", - api_index); + IWL_WARN(drv, +"capa flags index %d larger than supported by driver\n", +api_index); /* don't return an error so we can load FW that has more bits */ return 0; } -- 2.13.2
[PATCH v2] iwlwifi: Demote messages about fw flags size to info
These messages are not reporting a real error, just the fact that the firmware knows about more flags then the driver. Currently these messages are presented to the user during boot if there is no bootsplash covering the console, even when booting the kernel with "quiet". Demoting it to the warn level helps having a clean boot process. Signed-off-by: João Paulo Rechi Vita --- v2 changes: - Set to warn level instead of info drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c index be466a074c1d..c98a254bbd4d 100644 --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c @@ -461,9 +461,9 @@ static int iwl_set_ucode_api_flags(struct iwl_drv *drv, const u8 *data, int i; if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_API, 32)) { - IWL_ERR(drv, - "api flags index %d larger than supported by driver\n", - api_index); + IWL_WARN(drv, +"api flags index %d larger than supported by driver\n", +api_index); /* don't return an error so we can load FW that has more bits */ return 0; } @@ -485,9 +485,9 @@ static int iwl_set_ucode_capabilities(struct iwl_drv *drv, const u8 *data, int i; if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_CAPA, 32)) { - IWL_ERR(drv, - "capa flags index %d larger than supported by driver\n", - api_index); + IWL_WARN(drv, +"capa flags index %d larger than supported by driver\n", +api_index); /* don't return an error so we can load FW that has more bits */ return 0; } -- 2.13.2
Re: [PATCH] iwlwifi: Demote messages about fw flags size to info
Hello Luca, On Mon, Jul 24, 2017 at 4:01 AM, Coelho, Luciano <luciano.coe...@intel.com> wrote: > On Fri, 2017-07-21 at 07:51 -0700, João Paulo Rechi Vita wrote: (...) >> Currently these messages are presented to the user during boot if there >> is no bootsplash covering the console, sometimes even if the boot splash >> is enabled but has not started yet by the time this message is shown. >> I should have provided another piece of information here: all of this happens even when booting with the 'quiet' kernel parameter. > This specific case is harmless, but I'd rather keep this message as an > error, because in other situations it could lead to unexpected > behavioir, so I prefer to keep it very visible. > > I see your point, and I understand the purpose of these messages. I'm wondering if perhaps having them at the warning level would give them enough visibility, while still keeping a clean boot process to the end user. If so, I can send an updated patch. Thanks for your reply and for pointing to the fix for the root cause of that message. Cheers, .......... João Paulo Rechi Vita | +1.415.851.5778 | Endless
Re: [PATCH] iwlwifi: Demote messages about fw flags size to info
Hello Luca, On Mon, Jul 24, 2017 at 4:01 AM, Coelho, Luciano wrote: > On Fri, 2017-07-21 at 07:51 -0700, João Paulo Rechi Vita wrote: (...) >> Currently these messages are presented to the user during boot if there >> is no bootsplash covering the console, sometimes even if the boot splash >> is enabled but has not started yet by the time this message is shown. >> I should have provided another piece of information here: all of this happens even when booting with the 'quiet' kernel parameter. > This specific case is harmless, but I'd rather keep this message as an > error, because in other situations it could lead to unexpected > behavioir, so I prefer to keep it very visible. > > I see your point, and I understand the purpose of these messages. I'm wondering if perhaps having them at the warning level would give them enough visibility, while still keeping a clean boot process to the end user. If so, I can send an updated patch. Thanks for your reply and for pointing to the fix for the root cause of that message. Cheers, ...... João Paulo Rechi Vita | +1.415.851.5778 | Endless
[PATCH 1/2] HID: multitouch: Support HID_GD_WIRELESS_RADIO_CTLS
Some keyboard + touchpad devices have the Microsoft Win8 Wireless Radio Controls extensions Usage Page define in the touchpad report descriptor, so we need to support them in this driver. Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/hid/hid-multitouch.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index aff20f4b6d97..152d33120a55 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -922,7 +922,8 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, field->application != HID_DG_PEN && field->application != HID_DG_TOUCHPAD && field->application != HID_GD_KEYBOARD && - field->application != HID_CP_CONSUMER_CONTROL) + field->application != HID_CP_CONSUMER_CONTROL && + field->application != HID_GD_WIRELESS_RADIO_CTLS) return -1; /* @@ -1133,6 +1134,9 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi) case HID_CP_CONSUMER_CONTROL: suffix = "Consumer Control"; break; + case HID_GD_WIRELESS_RADIO_CTLS: + suffix = "Wireless Radio Control"; + break; default: suffix = "UNKNOWN"; break; -- 2.13.2
[PATCH 2/2] HID: multitouch: Support Asus T304UA media keys
The Asus T304UA convertible sports a magnetic detachable keyboard with touchpad, which is connected over USB. Most of the keyboard hotkeys are exposed through the same USB interface as the touchpad, defined in the report descriptor as follows: 0x06, 0x31, 0xFF, // Usage Page (Vendor Defined 0xFF31) 0x09, 0x76,// Usage (0x76) 0xA1, 0x01,// Collection (Application) 0x05, 0xFF,// Usage Page (Reserved 0xFF) 0x85, 0x5A,// Report ID (90) 0x19, 0x00,// Usage Minimum (0x00) 0x2A, 0xFF, 0x00, // Usage Maximum (0xFF) 0x15, 0x00,// Logical Minimum (0) 0x26, 0xFF, 0x00, // Logical Maximum (255) 0x75, 0x08,// Report Size (8) 0x95, 0x0F,// Report Count (15) 0xB1, 0x02,// Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) 0x05, 0xFF,// Usage Page (Reserved 0xFF) 0x85, 0x5A,// Report ID (90) 0x19, 0x00,// Usage Minimum (0x00) 0x2A, 0xFF, 0x00, // Usage Maximum (0xFF) 0x15, 0x00,// Logical Minimum (0) 0x26, 0xFF, 0x00, // Logical Maximum (255) 0x75, 0x08,// Report Size (8) 0x95, 0x02,// Report Count (2) 0x81, 0x02,// Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) 0xC0, // End Collection This UsagePage is declared as a variable, but we need to treat it as an array to be able to map each Usage we care about to its corresponding input key. Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/hid/hid-ids.h| 1 + drivers/hid/hid-multitouch.c | 44 +++- include/linux/hid.h | 2 ++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 3d911bfd91cf..6b7f9707076e 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -176,6 +176,7 @@ #define USB_DEVICE_ID_ASUSTEK_LCM 0x1726 #define USB_DEVICE_ID_ASUSTEK_LCM2 0x175b #define USB_DEVICE_ID_ASUSTEK_T100_KEYBOARD0x17e0 +#define USB_DEVICE_ID_ASUSTEK_T304_KEYBOARD0x184a #define USB_DEVICE_ID_ASUSTEK_I2C_KEYBOARD 0x8585 #define USB_DEVICE_ID_ASUSTEK_I2C_TOUCHPAD 0x0101 #define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD1 0x1854 diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 152d33120a55..6b3de7b01571 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -72,6 +72,7 @@ MODULE_LICENSE("GPL"); #define MT_QUIRK_FIX_CONST_CONTACT_ID BIT(14) #define MT_QUIRK_TOUCH_SIZE_SCALINGBIT(15) #define MT_QUIRK_STICKY_FINGERSBIT(16) +#define MT_QUIRK_ASUS_CUSTOM_UPBIT(17) #define MT_INPUTMODE_TOUCHSCREEN 0x02 #define MT_INPUTMODE_TOUCHPAD 0x03 @@ -169,6 +170,7 @@ static void mt_post_parse(struct mt_device *td); #define MT_CLS_GENERALTOUCH_TWOFINGERS 0x0108 #define MT_CLS_GENERALTOUCH_PWT_TENFINGERS 0x0109 #define MT_CLS_LG 0x010a +#define MT_CLS_ASUS0x010b #define MT_CLS_VTL 0x0110 #define MT_CLS_GOOGLE 0x0111 @@ -290,6 +292,10 @@ static struct mt_class mt_classes[] = { MT_QUIRK_IGNORE_DUPLICATES | MT_QUIRK_HOVERING | MT_QUIRK_CONTACT_CNT_ACCURATE }, + { .name = MT_CLS_ASUS, + .quirks = MT_QUIRK_ALWAYS_VALID | + MT_QUIRK_CONTACT_CNT_ACCURATE | + MT_QUIRK_ASUS_CUSTOM_UP }, { .name = MT_CLS_VTL, .quirks = MT_QUIRK_ALWAYS_VALID | MT_QUIRK_CONTACT_CNT_ACCURATE | @@ -905,6 +911,8 @@ static int mt_touch_input_configured(struct hid_device *hdev, return 0; } +#define mt_map_key_clear(c)hid_map_usage_clear(hi, usage, bit, \ + max, EV_KEY, (c)) static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, struct hid_field *field, struct hid_usage *usage, unsigned long **bit, int *max) @@ -923,10 +931,35 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, field->application != HID_DG_TOUCHPAD && field->application != HID_GD_KEYBOARD && field->application != HID_CP_CONSUMER_CONTROL && - field->application != HID_GD_WIRELESS_RADIO_CTLS) + field->application != HID_GD_WIRELESS_RADIO_CTLS && + !(field->application == HID_VD_ASUS_CUSTOM_MEDIA_KEYS && + td->mtclass.quirks & MT_QUIRK_ASUS_CUSTOM_UP)) return -1; /* +* Some Asus keyboard+touchpad devices have the hotkeys defined in the +* touchpad report descriptor. We need to treat these as an array t
[PATCH 1/2] HID: multitouch: Support HID_GD_WIRELESS_RADIO_CTLS
Some keyboard + touchpad devices have the Microsoft Win8 Wireless Radio Controls extensions Usage Page define in the touchpad report descriptor, so we need to support them in this driver. Signed-off-by: João Paulo Rechi Vita --- drivers/hid/hid-multitouch.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index aff20f4b6d97..152d33120a55 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -922,7 +922,8 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, field->application != HID_DG_PEN && field->application != HID_DG_TOUCHPAD && field->application != HID_GD_KEYBOARD && - field->application != HID_CP_CONSUMER_CONTROL) + field->application != HID_CP_CONSUMER_CONTROL && + field->application != HID_GD_WIRELESS_RADIO_CTLS) return -1; /* @@ -1133,6 +1134,9 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi) case HID_CP_CONSUMER_CONTROL: suffix = "Consumer Control"; break; + case HID_GD_WIRELESS_RADIO_CTLS: + suffix = "Wireless Radio Control"; + break; default: suffix = "UNKNOWN"; break; -- 2.13.2
[PATCH 2/2] HID: multitouch: Support Asus T304UA media keys
The Asus T304UA convertible sports a magnetic detachable keyboard with touchpad, which is connected over USB. Most of the keyboard hotkeys are exposed through the same USB interface as the touchpad, defined in the report descriptor as follows: 0x06, 0x31, 0xFF, // Usage Page (Vendor Defined 0xFF31) 0x09, 0x76,// Usage (0x76) 0xA1, 0x01,// Collection (Application) 0x05, 0xFF,// Usage Page (Reserved 0xFF) 0x85, 0x5A,// Report ID (90) 0x19, 0x00,// Usage Minimum (0x00) 0x2A, 0xFF, 0x00, // Usage Maximum (0xFF) 0x15, 0x00,// Logical Minimum (0) 0x26, 0xFF, 0x00, // Logical Maximum (255) 0x75, 0x08,// Report Size (8) 0x95, 0x0F,// Report Count (15) 0xB1, 0x02,// Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) 0x05, 0xFF,// Usage Page (Reserved 0xFF) 0x85, 0x5A,// Report ID (90) 0x19, 0x00,// Usage Minimum (0x00) 0x2A, 0xFF, 0x00, // Usage Maximum (0xFF) 0x15, 0x00,// Logical Minimum (0) 0x26, 0xFF, 0x00, // Logical Maximum (255) 0x75, 0x08,// Report Size (8) 0x95, 0x02,// Report Count (2) 0x81, 0x02,// Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) 0xC0, // End Collection This UsagePage is declared as a variable, but we need to treat it as an array to be able to map each Usage we care about to its corresponding input key. Signed-off-by: João Paulo Rechi Vita --- drivers/hid/hid-ids.h| 1 + drivers/hid/hid-multitouch.c | 44 +++- include/linux/hid.h | 2 ++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 3d911bfd91cf..6b7f9707076e 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -176,6 +176,7 @@ #define USB_DEVICE_ID_ASUSTEK_LCM 0x1726 #define USB_DEVICE_ID_ASUSTEK_LCM2 0x175b #define USB_DEVICE_ID_ASUSTEK_T100_KEYBOARD0x17e0 +#define USB_DEVICE_ID_ASUSTEK_T304_KEYBOARD0x184a #define USB_DEVICE_ID_ASUSTEK_I2C_KEYBOARD 0x8585 #define USB_DEVICE_ID_ASUSTEK_I2C_TOUCHPAD 0x0101 #define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD1 0x1854 diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 152d33120a55..6b3de7b01571 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -72,6 +72,7 @@ MODULE_LICENSE("GPL"); #define MT_QUIRK_FIX_CONST_CONTACT_ID BIT(14) #define MT_QUIRK_TOUCH_SIZE_SCALINGBIT(15) #define MT_QUIRK_STICKY_FINGERSBIT(16) +#define MT_QUIRK_ASUS_CUSTOM_UPBIT(17) #define MT_INPUTMODE_TOUCHSCREEN 0x02 #define MT_INPUTMODE_TOUCHPAD 0x03 @@ -169,6 +170,7 @@ static void mt_post_parse(struct mt_device *td); #define MT_CLS_GENERALTOUCH_TWOFINGERS 0x0108 #define MT_CLS_GENERALTOUCH_PWT_TENFINGERS 0x0109 #define MT_CLS_LG 0x010a +#define MT_CLS_ASUS0x010b #define MT_CLS_VTL 0x0110 #define MT_CLS_GOOGLE 0x0111 @@ -290,6 +292,10 @@ static struct mt_class mt_classes[] = { MT_QUIRK_IGNORE_DUPLICATES | MT_QUIRK_HOVERING | MT_QUIRK_CONTACT_CNT_ACCURATE }, + { .name = MT_CLS_ASUS, + .quirks = MT_QUIRK_ALWAYS_VALID | + MT_QUIRK_CONTACT_CNT_ACCURATE | + MT_QUIRK_ASUS_CUSTOM_UP }, { .name = MT_CLS_VTL, .quirks = MT_QUIRK_ALWAYS_VALID | MT_QUIRK_CONTACT_CNT_ACCURATE | @@ -905,6 +911,8 @@ static int mt_touch_input_configured(struct hid_device *hdev, return 0; } +#define mt_map_key_clear(c)hid_map_usage_clear(hi, usage, bit, \ + max, EV_KEY, (c)) static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, struct hid_field *field, struct hid_usage *usage, unsigned long **bit, int *max) @@ -923,10 +931,35 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, field->application != HID_DG_TOUCHPAD && field->application != HID_GD_KEYBOARD && field->application != HID_CP_CONSUMER_CONTROL && - field->application != HID_GD_WIRELESS_RADIO_CTLS) + field->application != HID_GD_WIRELESS_RADIO_CTLS && + !(field->application == HID_VD_ASUS_CUSTOM_MEDIA_KEYS && + td->mtclass.quirks & MT_QUIRK_ASUS_CUSTOM_UP)) return -1; /* +* Some Asus keyboard+touchpad devices have the hotkeys defined in the +* touchpad report descriptor. We need to treat these as an array to +* map usages to input keys.
[PATCH 0/2] HID: multitouch: Support Asus T304UA media keys
The Asus T304UA convertible sports a magnetic detachable keyboard with touchpad, which is connected over USB. Most of the keyboard hotkeys are exposed through the same USB interface as the touchpad. The mapping defined here is the same present in hid-asus, but when using hid-asus to drive this touchpad, we lose the multitouch funcionality. In my experiments, when trying to add mark this USB id with QUIRK_IS_MULTITOUCH in hid-asus, both the keyboard and touchpad stop responding. I would be happy to re-work this series if some guidance is provided on how these kind of mappings should be implemented (is it ok to have vendor-specific mappings in hid-multitouch? how can we keep the mt funcionality if using hid-asus? how can we bing hid-asus only to the to USB interface?). Bellow is the output from usb-devices, where If#=0 exports the keyboard and If#=1 exports the touchpad and media keys. T: Bus=01 Lev=01 Prnt=01 Port=07 Cnt=03 Dev#= 4 Spd=12 MxCh= 0 D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 P: Vendor=0b05 ProdID=184a Rev=11.02 S: Product=ASUS Touchpad and G-sensor Keyboard C: #Ifs= 2 Cfg#= 1 Atr=a0 MxPwr=100mA I: If#= 0 Alt= 0 #EPs= 1 Cls=03(HID ) Sub=01 Prot=01 Driver=usbhid I: If#= 1 Alt= 0 #EPs= 1 Cls=03(HID ) Sub=01 Prot=02 Driver=usbhid João Paulo Rechi Vita (2): HID: multitouch: Support HID_GD_WIRELESS_RADIO_CTLS HID: multitouch: Support Asus T304UA media keys drivers/hid/hid-ids.h| 1 + drivers/hid/hid-multitouch.c | 48 +++- include/linux/hid.h | 2 ++ 3 files changed, 50 insertions(+), 1 deletion(-) -- 2.13.2
[PATCH 0/2] HID: multitouch: Support Asus T304UA media keys
The Asus T304UA convertible sports a magnetic detachable keyboard with touchpad, which is connected over USB. Most of the keyboard hotkeys are exposed through the same USB interface as the touchpad. The mapping defined here is the same present in hid-asus, but when using hid-asus to drive this touchpad, we lose the multitouch funcionality. In my experiments, when trying to add mark this USB id with QUIRK_IS_MULTITOUCH in hid-asus, both the keyboard and touchpad stop responding. I would be happy to re-work this series if some guidance is provided on how these kind of mappings should be implemented (is it ok to have vendor-specific mappings in hid-multitouch? how can we keep the mt funcionality if using hid-asus? how can we bing hid-asus only to the to USB interface?). Bellow is the output from usb-devices, where If#=0 exports the keyboard and If#=1 exports the touchpad and media keys. T: Bus=01 Lev=01 Prnt=01 Port=07 Cnt=03 Dev#= 4 Spd=12 MxCh= 0 D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 P: Vendor=0b05 ProdID=184a Rev=11.02 S: Product=ASUS Touchpad and G-sensor Keyboard C: #Ifs= 2 Cfg#= 1 Atr=a0 MxPwr=100mA I: If#= 0 Alt= 0 #EPs= 1 Cls=03(HID ) Sub=01 Prot=01 Driver=usbhid I: If#= 1 Alt= 0 #EPs= 1 Cls=03(HID ) Sub=01 Prot=02 Driver=usbhid João Paulo Rechi Vita (2): HID: multitouch: Support HID_GD_WIRELESS_RADIO_CTLS HID: multitouch: Support Asus T304UA media keys drivers/hid/hid-ids.h| 1 + drivers/hid/hid-multitouch.c | 48 +++- include/linux/hid.h | 2 ++ 3 files changed, 50 insertions(+), 1 deletion(-) -- 2.13.2
[PATCH] iwlwifi: Demote messages about fw flags size to info
These messages are not reporting a real error, just the fact that the firmware knows about more flags then the driver. Currently these messages are presented to the user during boot if there is no bootsplash covering the console, sometimes even if the boot splash is enabled but has not started yet by the time this message is shown. Demoting it to the info level helps having a clean boot process. Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c index 6fdb5921e17f..557acd43d705 100644 --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c @@ -487,9 +487,9 @@ static int iwl_set_ucode_api_flags(struct iwl_drv *drv, const u8 *data, int i; if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_API, 32)) { - IWL_ERR(drv, - "api flags index %d larger than supported by driver\n", - api_index); + IWL_INFO(drv, +"api flags index %d larger than supported by driver\n", +api_index); /* don't return an error so we can load FW that has more bits */ return 0; } @@ -511,9 +511,9 @@ static int iwl_set_ucode_capabilities(struct iwl_drv *drv, const u8 *data, int i; if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_CAPA, 32)) { - IWL_ERR(drv, - "capa flags index %d larger than supported by driver\n", - api_index); + IWL_INFO(drv, +"capa flags index %d larger than supported by driver\n", +api_index); /* don't return an error so we can load FW that has more bits */ return 0; } -- 2.13.2
[PATCH] iwlwifi: Demote messages about fw flags size to info
These messages are not reporting a real error, just the fact that the firmware knows about more flags then the driver. Currently these messages are presented to the user during boot if there is no bootsplash covering the console, sometimes even if the boot splash is enabled but has not started yet by the time this message is shown. Demoting it to the info level helps having a clean boot process. Signed-off-by: João Paulo Rechi Vita --- drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c index 6fdb5921e17f..557acd43d705 100644 --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c @@ -487,9 +487,9 @@ static int iwl_set_ucode_api_flags(struct iwl_drv *drv, const u8 *data, int i; if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_API, 32)) { - IWL_ERR(drv, - "api flags index %d larger than supported by driver\n", - api_index); + IWL_INFO(drv, +"api flags index %d larger than supported by driver\n", +api_index); /* don't return an error so we can load FW that has more bits */ return 0; } @@ -511,9 +511,9 @@ static int iwl_set_ucode_capabilities(struct iwl_drv *drv, const u8 *data, int i; if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_CAPA, 32)) { - IWL_ERR(drv, - "capa flags index %d larger than supported by driver\n", - api_index); + IWL_INFO(drv, +"capa flags index %d larger than supported by driver\n", +api_index); /* don't return an error so we can load FW that has more bits */ return 0; } -- 2.13.2
[PATCH] platform/x86/acer-wmi: Detect RF Button capability
If a machine reports a RF Button in the communication button device bitmap, we need to remove it before calling Get Device Status otherwise it will return the "Undefined device" (0xE2) error code. Although this may be a BIOS bug, we don't really need to get or set the RF Button status. The status indicator LED embedded in the button is controlled by firmware logic, depending on the status of the wireless radios present on the machine (WiFi || WWAN). This commit fixes the wireless status indicator LED on the Acer TravelMate P648-G2-MG, and cleans the following message from the kernel log: "Get Current Device Status failed: 0xe2 - 0x0". Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/platform/x86/acer-wmi.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c index 79fa5ab3fd00..3b381178039b 100644 --- a/drivers/platform/x86/acer-wmi.c +++ b/drivers/platform/x86/acer-wmi.c @@ -149,6 +149,8 @@ struct event_return_value { #define ACER_WMID3_GDS_THREEG (1<<6) /* 3G */ #define ACER_WMID3_GDS_WIMAX (1<<7) /* WiMAX */ #define ACER_WMID3_GDS_BLUETOOTH (1<<11) /* BT */ +#define ACER_WMID3_GDS_RFBTN (1<<14) /* RF Button */ + #define ACER_WMID3_GDS_TOUCHPAD(1<<1) /* Touchpad */ /* Hotkey Customized Setting and Acer Application Status. @@ -221,6 +223,7 @@ struct hotkey_function_type_aa { #define ACER_CAP_BRIGHTNESS(1<<3) #define ACER_CAP_THREEG(1<<4) #define ACER_CAP_ACCEL (1<<5) +#define ACER_CAP_RFBTN (1<<6) #define ACER_CAP_ANY (0x) /* @@ -1264,6 +1267,10 @@ static void __init type_aa_dmi_decode(const struct dmi_header *header, void *d) interface->capability |= ACER_CAP_THREEG; if (type_aa->commun_func_bitmap & ACER_WMID3_GDS_BLUETOOTH) interface->capability |= ACER_CAP_BLUETOOTH; + if (type_aa->commun_func_bitmap & ACER_WMID3_GDS_RFBTN) { + interface->capability |= ACER_CAP_RFBTN; + commun_func_bitmap &= ~ACER_WMID3_GDS_RFBTN; + } commun_fn_key_number = type_aa->commun_fn_key_number; } -- 2.11.0
[PATCH] platform/x86/acer-wmi: Detect RF Button capability
If a machine reports a RF Button in the communication button device bitmap, we need to remove it before calling Get Device Status otherwise it will return the "Undefined device" (0xE2) error code. Although this may be a BIOS bug, we don't really need to get or set the RF Button status. The status indicator LED embedded in the button is controlled by firmware logic, depending on the status of the wireless radios present on the machine (WiFi || WWAN). This commit fixes the wireless status indicator LED on the Acer TravelMate P648-G2-MG, and cleans the following message from the kernel log: "Get Current Device Status failed: 0xe2 - 0x0". Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/acer-wmi.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c index 79fa5ab3fd00..3b381178039b 100644 --- a/drivers/platform/x86/acer-wmi.c +++ b/drivers/platform/x86/acer-wmi.c @@ -149,6 +149,8 @@ struct event_return_value { #define ACER_WMID3_GDS_THREEG (1<<6) /* 3G */ #define ACER_WMID3_GDS_WIMAX (1<<7) /* WiMAX */ #define ACER_WMID3_GDS_BLUETOOTH (1<<11) /* BT */ +#define ACER_WMID3_GDS_RFBTN (1<<14) /* RF Button */ + #define ACER_WMID3_GDS_TOUCHPAD(1<<1) /* Touchpad */ /* Hotkey Customized Setting and Acer Application Status. @@ -221,6 +223,7 @@ struct hotkey_function_type_aa { #define ACER_CAP_BRIGHTNESS(1<<3) #define ACER_CAP_THREEG(1<<4) #define ACER_CAP_ACCEL (1<<5) +#define ACER_CAP_RFBTN (1<<6) #define ACER_CAP_ANY (0x) /* @@ -1264,6 +1267,10 @@ static void __init type_aa_dmi_decode(const struct dmi_header *header, void *d) interface->capability |= ACER_CAP_THREEG; if (type_aa->commun_func_bitmap & ACER_WMID3_GDS_BLUETOOTH) interface->capability |= ACER_CAP_BLUETOOTH; + if (type_aa->commun_func_bitmap & ACER_WMID3_GDS_RFBTN) { + interface->capability |= ACER_CAP_RFBTN; + commun_func_bitmap &= ~ACER_WMID3_GDS_RFBTN; + } commun_fn_key_number = type_aa->commun_fn_key_number; } -- 2.11.0
[PATCHv2 0/2] platform/x86: asus-wmi: Substitute quirk_no_rfkill with a generic fix
The quirk avoids the conflicting usage of ASUS_WMI_DEVID_WLAN_LED (0x00010002) by both asus-wireless and asus-wmi on machines where the BIOS can't set and record the wlan status (wlan_ctrl_by_user in asus-wmi.c). At the moment we have six models quirked upstream, but another fifteen downstream, and we expect this list to continue growing. This series makes use of ASUS_WMI_DSTS_USER_BIT plus the status of the ASHS device to skip asus_wmi_rfkill_init(), instead of static DMI-based quirks. João Paulo Rechi Vita (2): platform/x86: asus-wmi: Detect quirk_no_rfkill from the DSDT platform/x86: asus-wmi: Remove quirk_no_rfkill drivers/platform/x86/asus-nb-wmi.c | 49 ++ drivers/platform/x86/asus-wmi.c| 22 + drivers/platform/x86/asus-wmi.h| 1 - 3 files changed, 19 insertions(+), 53 deletions(-) -- 2.11.0
[PATCHv2 0/2] platform/x86: asus-wmi: Substitute quirk_no_rfkill with a generic fix
The quirk avoids the conflicting usage of ASUS_WMI_DEVID_WLAN_LED (0x00010002) by both asus-wireless and asus-wmi on machines where the BIOS can't set and record the wlan status (wlan_ctrl_by_user in asus-wmi.c). At the moment we have six models quirked upstream, but another fifteen downstream, and we expect this list to continue growing. This series makes use of ASUS_WMI_DSTS_USER_BIT plus the status of the ASHS device to skip asus_wmi_rfkill_init(), instead of static DMI-based quirks. João Paulo Rechi Vita (2): platform/x86: asus-wmi: Detect quirk_no_rfkill from the DSDT platform/x86: asus-wmi: Remove quirk_no_rfkill drivers/platform/x86/asus-nb-wmi.c | 49 ++ drivers/platform/x86/asus-wmi.c| 22 + drivers/platform/x86/asus-wmi.h| 1 - 3 files changed, 19 insertions(+), 53 deletions(-) -- 2.11.0
[PATCHv2 2/2] platform/x86: asus-wmi: Remove quirk_no_rfkill
With the detection introduced in the previou patche, we don't need these static DMI-based quirks anymore. This reverts the following commits: 56a37a72002b "asus-wmi: Add quirk_no_rfkill_wapf4 for the Asus X456UA" a961a285b479 "asus-wmi: Add quirk_no_rfkill_wapf4 for the Asus X456UF" 6b7ff2af5286 "asus-wmi: Add quirk_no_rfkill for the Asus Z550MA" 02db9ff7af18 "asus-wmi: Add quirk_no_rfkill for the Asus U303LB" 2d735244b798 "asus-wmi: Add quirk_no_rfkill for the Asus N552VW" a977e59c0c67 "asus-wmi: Create quirk for airplane_mode LED" Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/platform/x86/asus-nb-wmi.c | 49 ++ drivers/platform/x86/asus-wmi.c| 5 +--- drivers/platform/x86/asus-wmi.h| 1 - 3 files changed, 3 insertions(+), 52 deletions(-) diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c index 5be4783e40d4..dea98ffb6f60 100644 --- a/drivers/platform/x86/asus-nb-wmi.c +++ b/drivers/platform/x86/asus-nb-wmi.c @@ -103,15 +103,6 @@ static struct quirk_entry quirk_asus_x200ca = { .wapf = 2, }; -static struct quirk_entry quirk_no_rfkill = { - .no_rfkill = true, -}; - -static struct quirk_entry quirk_no_rfkill_wapf4 = { - .wapf = 4, - .no_rfkill = true, -}; - static struct quirk_entry quirk_asus_ux303ub = { .wmi_backlight_native = true, }; @@ -194,7 +185,7 @@ static const struct dmi_system_id asus_quirks[] = { DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), DMI_MATCH(DMI_PRODUCT_NAME, "X456UA"), }, - .driver_data = _no_rfkill_wapf4, + .driver_data = _asus_wapf4, }, { .callback = dmi_matched, @@ -203,7 +194,7 @@ static const struct dmi_system_id asus_quirks[] = { DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), DMI_MATCH(DMI_PRODUCT_NAME, "X456UF"), }, - .driver_data = _no_rfkill_wapf4, + .driver_data = _asus_wapf4, }, { .callback = dmi_matched, @@ -369,42 +360,6 @@ static const struct dmi_system_id asus_quirks[] = { }, { .callback = dmi_matched, - .ident = "ASUSTeK COMPUTER INC. X555UB", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), - DMI_MATCH(DMI_PRODUCT_NAME, "X555UB"), - }, - .driver_data = _no_rfkill, - }, - { - .callback = dmi_matched, - .ident = "ASUSTeK COMPUTER INC. N552VW", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), - DMI_MATCH(DMI_PRODUCT_NAME, "N552VW"), - }, - .driver_data = _no_rfkill, - }, - { - .callback = dmi_matched, - .ident = "ASUSTeK COMPUTER INC. U303LB", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), - DMI_MATCH(DMI_PRODUCT_NAME, "U303LB"), - }, - .driver_data = _no_rfkill, - }, - { - .callback = dmi_matched, - .ident = "ASUSTeK COMPUTER INC. Z550MA", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), - DMI_MATCH(DMI_PRODUCT_NAME, "Z550MA"), - }, - .driver_data = _no_rfkill, - }, - { - .callback = dmi_matched, .ident = "ASUSTeK COMPUTER INC. UX303UB", .matches = { DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index 8499d3ae4257..8fe5890bf539 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -2111,10 +2111,7 @@ static int asus_wmi_add(struct platform_device *pdev) if (result & (ASUS_WMI_DSTS_PRESENCE_BIT | ASUS_WMI_DSTS_USER_BIT)) asus->driver->wlan_ctrl_by_user = 1; - if (asus->driver->wlan_ctrl_by_user && ashs_present()) - asus->driver->quirks->no_rfkill = 1; - - if (!asus->driver->quirks->no_rfkill) { + if (!(asus->driver->wlan_ctrl_by_user && ashs_present())) { err = asus_wmi_rfkill_init(asus); if (err) goto fail_rfkill; diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platfor
[PATCHv2 2/2] platform/x86: asus-wmi: Remove quirk_no_rfkill
With the detection introduced in the previou patche, we don't need these static DMI-based quirks anymore. This reverts the following commits: 56a37a72002b "asus-wmi: Add quirk_no_rfkill_wapf4 for the Asus X456UA" a961a285b479 "asus-wmi: Add quirk_no_rfkill_wapf4 for the Asus X456UF" 6b7ff2af5286 "asus-wmi: Add quirk_no_rfkill for the Asus Z550MA" 02db9ff7af18 "asus-wmi: Add quirk_no_rfkill for the Asus U303LB" 2d735244b798 "asus-wmi: Add quirk_no_rfkill for the Asus N552VW" a977e59c0c67 "asus-wmi: Create quirk for airplane_mode LED" Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-nb-wmi.c | 49 ++ drivers/platform/x86/asus-wmi.c| 5 +--- drivers/platform/x86/asus-wmi.h| 1 - 3 files changed, 3 insertions(+), 52 deletions(-) diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c index 5be4783e40d4..dea98ffb6f60 100644 --- a/drivers/platform/x86/asus-nb-wmi.c +++ b/drivers/platform/x86/asus-nb-wmi.c @@ -103,15 +103,6 @@ static struct quirk_entry quirk_asus_x200ca = { .wapf = 2, }; -static struct quirk_entry quirk_no_rfkill = { - .no_rfkill = true, -}; - -static struct quirk_entry quirk_no_rfkill_wapf4 = { - .wapf = 4, - .no_rfkill = true, -}; - static struct quirk_entry quirk_asus_ux303ub = { .wmi_backlight_native = true, }; @@ -194,7 +185,7 @@ static const struct dmi_system_id asus_quirks[] = { DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), DMI_MATCH(DMI_PRODUCT_NAME, "X456UA"), }, - .driver_data = _no_rfkill_wapf4, + .driver_data = _asus_wapf4, }, { .callback = dmi_matched, @@ -203,7 +194,7 @@ static const struct dmi_system_id asus_quirks[] = { DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), DMI_MATCH(DMI_PRODUCT_NAME, "X456UF"), }, - .driver_data = _no_rfkill_wapf4, + .driver_data = _asus_wapf4, }, { .callback = dmi_matched, @@ -369,42 +360,6 @@ static const struct dmi_system_id asus_quirks[] = { }, { .callback = dmi_matched, - .ident = "ASUSTeK COMPUTER INC. X555UB", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), - DMI_MATCH(DMI_PRODUCT_NAME, "X555UB"), - }, - .driver_data = _no_rfkill, - }, - { - .callback = dmi_matched, - .ident = "ASUSTeK COMPUTER INC. N552VW", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), - DMI_MATCH(DMI_PRODUCT_NAME, "N552VW"), - }, - .driver_data = _no_rfkill, - }, - { - .callback = dmi_matched, - .ident = "ASUSTeK COMPUTER INC. U303LB", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), - DMI_MATCH(DMI_PRODUCT_NAME, "U303LB"), - }, - .driver_data = _no_rfkill, - }, - { - .callback = dmi_matched, - .ident = "ASUSTeK COMPUTER INC. Z550MA", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), - DMI_MATCH(DMI_PRODUCT_NAME, "Z550MA"), - }, - .driver_data = _no_rfkill, - }, - { - .callback = dmi_matched, .ident = "ASUSTeK COMPUTER INC. UX303UB", .matches = { DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index 8499d3ae4257..8fe5890bf539 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -2111,10 +2111,7 @@ static int asus_wmi_add(struct platform_device *pdev) if (result & (ASUS_WMI_DSTS_PRESENCE_BIT | ASUS_WMI_DSTS_USER_BIT)) asus->driver->wlan_ctrl_by_user = 1; - if (asus->driver->wlan_ctrl_by_user && ashs_present()) - asus->driver->quirks->no_rfkill = 1; - - if (!asus->driver->quirks->no_rfkill) { + if (!(asus->driver->wlan_ctrl_by_user && ashs_present())) { err = asus_wmi_rfkill_init(asus); if (err) goto fail_rfkill; diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h ind
[PATCHv2 1/2] platform/x86: asus-wmi: Detect quirk_no_rfkill from the DSDT
Some Asus laptops that have an airplane-mode indicator LED, also have the WMI WLAN user bit set, and the following bits in their DSDT: Scope (_SB) { (...) Device (ATKD) { (...) Method (WMNB, 3, Serialized) { (...) If (LEqual (IIA0, 0x00010002)) { OWGD (IIA1) Return (One) } } } } So when asus-wmi uses ASUS_WMI_DEVID_WLAN_LED (0x00010002) to store the wlan state, it drives the airplane-mode indicator LED (through the call to OWGD) in an inverted fashion: the LED is ON when airplane mode is OFF (since wlan is ON), and vice-versa. This commit skips registering RFKill switches at all for these laptops, to allow the asus-wireless driver to drive the airplane mode LED correctly through the ASHS ACPI device. Relying on the presence of ASHS and ASUS_WMI_DSTS_USER_BIT avoids adding DMI-based quirks for at least 21 different laptops. Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/platform/x86/asus-wmi.c | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index 43cb680adbb4..8499d3ae4257 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -159,6 +159,8 @@ MODULE_LICENSE("GPL"); #define USB_INTEL_XUSB2PR 0xD0 #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31 +static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL }; + struct bios_args { u32 arg0; u32 arg1; @@ -2051,6 +2053,16 @@ static int asus_wmi_fan_init(struct asus_wmi *asus) return 0; } +static bool ashs_present(void) +{ + int i = 0; + while (ashs_ids[i]) { + if (acpi_dev_found(ashs_ids[i++])) + return true; + } + return false; +} + /* * WMI Driver */ @@ -2095,6 +2107,13 @@ static int asus_wmi_add(struct platform_device *pdev) if (err) goto fail_leds; + asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_WLAN, ); + if (result & (ASUS_WMI_DSTS_PRESENCE_BIT | ASUS_WMI_DSTS_USER_BIT)) + asus->driver->wlan_ctrl_by_user = 1; + + if (asus->driver->wlan_ctrl_by_user && ashs_present()) + asus->driver->quirks->no_rfkill = 1; + if (!asus->driver->quirks->no_rfkill) { err = asus_wmi_rfkill_init(asus); if (err) @@ -2134,10 +2153,6 @@ static int asus_wmi_add(struct platform_device *pdev) if (err) goto fail_debugfs; - asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_WLAN, ); - if (result & (ASUS_WMI_DSTS_PRESENCE_BIT | ASUS_WMI_DSTS_USER_BIT)) - asus->driver->wlan_ctrl_by_user = 1; - return 0; fail_debugfs: -- 2.11.0
[PATCHv2 1/2] platform/x86: asus-wmi: Detect quirk_no_rfkill from the DSDT
Some Asus laptops that have an airplane-mode indicator LED, also have the WMI WLAN user bit set, and the following bits in their DSDT: Scope (_SB) { (...) Device (ATKD) { (...) Method (WMNB, 3, Serialized) { (...) If (LEqual (IIA0, 0x00010002)) { OWGD (IIA1) Return (One) } } } } So when asus-wmi uses ASUS_WMI_DEVID_WLAN_LED (0x00010002) to store the wlan state, it drives the airplane-mode indicator LED (through the call to OWGD) in an inverted fashion: the LED is ON when airplane mode is OFF (since wlan is ON), and vice-versa. This commit skips registering RFKill switches at all for these laptops, to allow the asus-wireless driver to drive the airplane mode LED correctly through the ASHS ACPI device. Relying on the presence of ASHS and ASUS_WMI_DSTS_USER_BIT avoids adding DMI-based quirks for at least 21 different laptops. Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-wmi.c | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index 43cb680adbb4..8499d3ae4257 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -159,6 +159,8 @@ MODULE_LICENSE("GPL"); #define USB_INTEL_XUSB2PR 0xD0 #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31 +static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL }; + struct bios_args { u32 arg0; u32 arg1; @@ -2051,6 +2053,16 @@ static int asus_wmi_fan_init(struct asus_wmi *asus) return 0; } +static bool ashs_present(void) +{ + int i = 0; + while (ashs_ids[i]) { + if (acpi_dev_found(ashs_ids[i++])) + return true; + } + return false; +} + /* * WMI Driver */ @@ -2095,6 +2107,13 @@ static int asus_wmi_add(struct platform_device *pdev) if (err) goto fail_leds; + asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_WLAN, ); + if (result & (ASUS_WMI_DSTS_PRESENCE_BIT | ASUS_WMI_DSTS_USER_BIT)) + asus->driver->wlan_ctrl_by_user = 1; + + if (asus->driver->wlan_ctrl_by_user && ashs_present()) + asus->driver->quirks->no_rfkill = 1; + if (!asus->driver->quirks->no_rfkill) { err = asus_wmi_rfkill_init(asus); if (err) @@ -2134,10 +2153,6 @@ static int asus_wmi_add(struct platform_device *pdev) if (err) goto fail_debugfs; - asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_WLAN, ); - if (result & (ASUS_WMI_DSTS_PRESENCE_BIT | ASUS_WMI_DSTS_USER_BIT)) - asus->driver->wlan_ctrl_by_user = 1; - return 0; fail_debugfs: -- 2.11.0
Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID
On 7 February 2017 at 18:37, Andy Shevchenko <andy.shevche...@gmail.com> wrote: > On Tue, Feb 7, 2017 at 11:44 PM, João Paulo Rechi Vita > <jprv...@gmail.com> wrote: >> On 4 February 2017 at 10:02, Andy Shevchenko <andy.shevche...@gmail.com> >> wrote: >>> On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita <jprv...@gmail.com> >>> wrote: >>>> On 27 January 2017 at 10:26, Andy Shevchenko <andy.shevche...@gmail.com> >>>> wrote: >>>>> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita >>>>> <jprv...@gmail.com> wrote: >>> >>>>>> +static const struct acpi_device_id device_ids[] = { >>>>>> + {"ATK4001", 0}, >>>>>> + {"ATK4002", 0}, >>>>> >>>>> ...and use it as a parameter here. >>>>> >>>> >>>> I'm not exactly sure how to do that, as driver_data is a >>>> kernel_ulong_t. Can you please elaborate a bit more? >>> >>> {"ATK4001", (kernel_ulong_t)atk4001_id_params}, >>> >> >> The code you suggested: >> >> static const struct hswc_params atk4001_id_params = {0x0, 0x1, 0x2}; >> static const struct acpi_device_id device_ids[] = { >>{"ATK4001", (kernel_ulong_t)atk4001_id_params}, >>{"", 0}, >> }; >> >> does not compile: "drivers/platform/x86/asus-wireless.c:44:2: error: >> aggregate value used where an integer was expected", so I guess you >> meant the address of that struct (_id_params), right? I don't >> see any way how we could have the actual data in .driver_data. > > Yes, you are right. I kept in mind array when was suggesting this. > >> Even after moving the parameters in the driver_data field of >> device_ids[], I'm not able retrieve them with acpi_match_device(), >> because the "struct device" from the "struct acpi_device" that comes >> as an input parameter in asus_wireless_add() does not have a acpi >> companion device associated with it, so acpi_match_device() returns >> NULL. I still need to manually loop through device_ids[] in order to >> retrieve the .driver_data associated with the HID, and I see the same >> pattern in the i2c-scmi driver. > > And what prevents us to set companion device? > Assuming this in the scope of a platform driver (genuine question, as I don't see anything under drivers/platform/ doing so), nothing prevents us to set it. But when doing so, acpi_match_device() still fails, because acpi_get_first_physical_node() returns a different "struct dev *" in acpi_primary_dev_companion(). -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID
On 7 February 2017 at 18:37, Andy Shevchenko wrote: > On Tue, Feb 7, 2017 at 11:44 PM, João Paulo Rechi Vita > wrote: >> On 4 February 2017 at 10:02, Andy Shevchenko >> wrote: >>> On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita >>> wrote: >>>> On 27 January 2017 at 10:26, Andy Shevchenko >>>> wrote: >>>>> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita >>>>> wrote: >>> >>>>>> +static const struct acpi_device_id device_ids[] = { >>>>>> + {"ATK4001", 0}, >>>>>> + {"ATK4002", 0}, >>>>> >>>>> ...and use it as a parameter here. >>>>> >>>> >>>> I'm not exactly sure how to do that, as driver_data is a >>>> kernel_ulong_t. Can you please elaborate a bit more? >>> >>> {"ATK4001", (kernel_ulong_t)atk4001_id_params}, >>> >> >> The code you suggested: >> >> static const struct hswc_params atk4001_id_params = {0x0, 0x1, 0x2}; >> static const struct acpi_device_id device_ids[] = { >>{"ATK4001", (kernel_ulong_t)atk4001_id_params}, >>{"", 0}, >> }; >> >> does not compile: "drivers/platform/x86/asus-wireless.c:44:2: error: >> aggregate value used where an integer was expected", so I guess you >> meant the address of that struct (_id_params), right? I don't >> see any way how we could have the actual data in .driver_data. > > Yes, you are right. I kept in mind array when was suggesting this. > >> Even after moving the parameters in the driver_data field of >> device_ids[], I'm not able retrieve them with acpi_match_device(), >> because the "struct device" from the "struct acpi_device" that comes >> as an input parameter in asus_wireless_add() does not have a acpi >> companion device associated with it, so acpi_match_device() returns >> NULL. I still need to manually loop through device_ids[] in order to >> retrieve the .driver_data associated with the HID, and I see the same >> pattern in the i2c-scmi driver. > > And what prevents us to set companion device? > Assuming this in the scope of a platform driver (genuine question, as I don't see anything under drivers/platform/ doing so), nothing prevents us to set it. But when doing so, acpi_match_device() still fails, because acpi_get_first_physical_node() returns a different "struct dev *" in acpi_primary_dev_companion(). -- João Paulo Rechi Vita http://about.me/jprvita
[PATCHv2] platform/x86: asus-wireless: Use per-HID HSWC parameters
Some Asus machines use 0x4/0x5 as their LED on/off values, while others use 0x0/0x1, as shown in the DSDT excerpts below. Luckily it seems this behavior is tied to different HIDs, after looking at 44 DSDTs from different Asus models. Another small difference is that a few of them call GWBL instead of OWGS, and SWBL instead of OWGD. That does not seem to make a difference for asus-wireless, and is additional reasoning to not try to call these methods directly. Device (ASHS) | Device (ASHS) { | { Name (_HID, "ATK4002") | Name (_HID, "ATK4001") Method (HSWC, 1, Serialized)| Method (HSWC, 1, Serialized) { | { If ((Arg0 < 0x02)) | If ((Arg0 < 0x02)) { | { OWGD (Arg0) | OWGD (Arg0) Return (One)| Return (One) } | } If ((Arg0 == 0x02)) | { | If ((Arg0 == 0x02)) Local0 = OWGS ()| { If (Local0) | Return (OWGS ()) { | } Return (0x05) | } | If ((Arg0 == 0x03)) Else| { { | Return (0xFF) Return (0x04) | } } | } | If ((Arg0 == 0x80)) If ((Arg0 == 0x03)) | { { |Return (One) Return (0xFF) | } } | } If ((Arg0 == 0x04)) | Method (_STA, 0, NotSerialized) { | { OWGD (Zero) | If ((MSOS () >= OSW8)) Return (One)| { } | Return (0x0F) If ((Arg0 == 0x05)) | } { | Else OWGD (One) | { Return (One)| Return (Zero) } | } If ((Arg0 == 0x80)) | } { | } Return (One)| } | } | Method (_STA, 0, NotSerialized) | { | If ((MSOS () >= OSW8)) | { | Return (0x0F) | } | Else| { | Return (Zero) | } | } | } | Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/platform/x86/asus-wireless.c | 57 ++-- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c index a7ee18383146..f3796164329e 100644 --- a/drivers/platform/x86/asus-wireless.c +++ b/drivers/platform/x86/asus-wireless.c @@ -17,19 +17,41 @@ #include #include -#define ASUS_WIRELESS_LED_STATUS 0x2 -#define ASUS_WIRELESS_LED_OFF 0x4 -#define ASUS_WIRELESS_LED_ON 0x5 +struct hswc_params { + u8 on; + u8 off; + u8 status; +}; struct asus_wireless_data { struct input_dev *idev; struct acpi_device *adev; + const struct hswc_params *hswc_params; struct workqueue_struct *wq; struct work_struct led_work; struct led_classdev led; int led_state; }; +static const struct hswc_params atk4001_id_params = { + .on = 0x0, + .off = 0x1, + .status = 0x2, +}; + +static const struct hswc_params atk4002_id_params = { + .on = 0x5, + .off = 0x4, + .status = 0x2, +}; + +static const struct acpi_device_id device_ids[] = { + {"ATK4001", (kernel_ulong_t)_id_params}, + {"ATK4002", (kernel_ulong_t)_id_params}, + {"", 0}, +}; +MODULE_DEVICE_TABLE(acpi, device_ids); + static u64 asus_wireless_method(acpi_handle handle, const char *method, int param) { @@ -61,8 +83,8 @@ static enum led_brightness led_state_get(struct led_classdev *led) data = container_of(led, struct asus_wireless_data, led); s = asus_wireless_method(acpi_device_handle(data->adev), "HSWC", -ASUS_WIRELESS_LED_STATUS); - if (s == ASUS_WIRELESS_LED_ON) +
[PATCHv2] platform/x86: asus-wireless: Use per-HID HSWC parameters
Some Asus machines use 0x4/0x5 as their LED on/off values, while others use 0x0/0x1, as shown in the DSDT excerpts below. Luckily it seems this behavior is tied to different HIDs, after looking at 44 DSDTs from different Asus models. Another small difference is that a few of them call GWBL instead of OWGS, and SWBL instead of OWGD. That does not seem to make a difference for asus-wireless, and is additional reasoning to not try to call these methods directly. Device (ASHS) | Device (ASHS) { | { Name (_HID, "ATK4002") | Name (_HID, "ATK4001") Method (HSWC, 1, Serialized)| Method (HSWC, 1, Serialized) { | { If ((Arg0 < 0x02)) | If ((Arg0 < 0x02)) { | { OWGD (Arg0) | OWGD (Arg0) Return (One)| Return (One) } | } If ((Arg0 == 0x02)) | { | If ((Arg0 == 0x02)) Local0 = OWGS ()| { If (Local0) | Return (OWGS ()) { | } Return (0x05) | } | If ((Arg0 == 0x03)) Else| { { | Return (0xFF) Return (0x04) | } } | } | If ((Arg0 == 0x80)) If ((Arg0 == 0x03)) | { { |Return (One) Return (0xFF) | } } | } If ((Arg0 == 0x04)) | Method (_STA, 0, NotSerialized) { | { OWGD (Zero) | If ((MSOS () >= OSW8)) Return (One)| { } | Return (0x0F) If ((Arg0 == 0x05)) | } { | Else OWGD (One) | { Return (One)| Return (Zero) } | } If ((Arg0 == 0x80)) | } { | } Return (One)| } | } | Method (_STA, 0, NotSerialized) | { | If ((MSOS () >= OSW8)) | { | Return (0x0F) | } | Else| { | Return (Zero) | } | } | } | Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-wireless.c | 57 ++-- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c index a7ee18383146..f3796164329e 100644 --- a/drivers/platform/x86/asus-wireless.c +++ b/drivers/platform/x86/asus-wireless.c @@ -17,19 +17,41 @@ #include #include -#define ASUS_WIRELESS_LED_STATUS 0x2 -#define ASUS_WIRELESS_LED_OFF 0x4 -#define ASUS_WIRELESS_LED_ON 0x5 +struct hswc_params { + u8 on; + u8 off; + u8 status; +}; struct asus_wireless_data { struct input_dev *idev; struct acpi_device *adev; + const struct hswc_params *hswc_params; struct workqueue_struct *wq; struct work_struct led_work; struct led_classdev led; int led_state; }; +static const struct hswc_params atk4001_id_params = { + .on = 0x0, + .off = 0x1, + .status = 0x2, +}; + +static const struct hswc_params atk4002_id_params = { + .on = 0x5, + .off = 0x4, + .status = 0x2, +}; + +static const struct acpi_device_id device_ids[] = { + {"ATK4001", (kernel_ulong_t)_id_params}, + {"ATK4002", (kernel_ulong_t)_id_params}, + {"", 0}, +}; +MODULE_DEVICE_TABLE(acpi, device_ids); + static u64 asus_wireless_method(acpi_handle handle, const char *method, int param) { @@ -61,8 +83,8 @@ static enum led_brightness led_state_get(struct led_classdev *led) data = container_of(led, struct asus_wireless_data, led); s = asus_wireless_method(acpi_device_handle(data->adev), "HSWC", -ASUS_WIRELESS_LED_STATUS); - if (s == ASUS_WIRELESS_LED_ON) +
Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID
On 4 February 2017 at 10:02, Andy Shevchenko <andy.shevche...@gmail.com> wrote: > On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita <jprv...@gmail.com> > wrote: >> On 27 January 2017 at 10:26, Andy Shevchenko <andy.shevche...@gmail.com> >> wrote: >>> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita >>> <jprv...@gmail.com> wrote: > >>>> +static const struct acpi_device_id device_ids[] = { >>>> + {"ATK4001", 0}, >>>> + {"ATK4002", 0}, >>> >>> ...and use it as a parameter here. >>> >> >> I'm not exactly sure how to do that, as driver_data is a >> kernel_ulong_t. Can you please elaborate a bit more? > > {"ATK4001", (kernel_ulong_t)atk4001_id_params}, > The code you suggested: static const struct hswc_params atk4001_id_params = {0x0, 0x1, 0x2}; static const struct acpi_device_id device_ids[] = { {"ATK4001", (kernel_ulong_t)atk4001_id_params}, {"", 0}, }; does not compile: "drivers/platform/x86/asus-wireless.c:44:2: error: aggregate value used where an integer was expected", so I guess you meant the address of that struct (_id_params), right? I don't see any way how we could have the actual data in .driver_data. Even after moving the parameters in the driver_data field of device_ids[], I'm not able retrieve them with acpi_match_device(), because the "struct device" from the "struct acpi_device" that comes as an input parameter in asus_wireless_add() does not have a acpi companion device associated with it, so acpi_match_device() returns NULL. I still need to manually loop through device_ids[] in order to retrieve the .driver_data associated with the HID, and I see the same pattern in the i2c-scmi driver. I'm sending the next revision for feedback, please advise if I'm missing any detail, or if you want this implemented differently. -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID
On 4 February 2017 at 10:02, Andy Shevchenko wrote: > On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita > wrote: >> On 27 January 2017 at 10:26, Andy Shevchenko >> wrote: >>> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita >>> wrote: > >>>> +static const struct acpi_device_id device_ids[] = { >>>> + {"ATK4001", 0}, >>>> + {"ATK4002", 0}, >>> >>> ...and use it as a parameter here. >>> >> >> I'm not exactly sure how to do that, as driver_data is a >> kernel_ulong_t. Can you please elaborate a bit more? > > {"ATK4001", (kernel_ulong_t)atk4001_id_params}, > The code you suggested: static const struct hswc_params atk4001_id_params = {0x0, 0x1, 0x2}; static const struct acpi_device_id device_ids[] = { {"ATK4001", (kernel_ulong_t)atk4001_id_params}, {"", 0}, }; does not compile: "drivers/platform/x86/asus-wireless.c:44:2: error: aggregate value used where an integer was expected", so I guess you meant the address of that struct (_id_params), right? I don't see any way how we could have the actual data in .driver_data. Even after moving the parameters in the driver_data field of device_ids[], I'm not able retrieve them with acpi_match_device(), because the "struct device" from the "struct acpi_device" that comes as an input parameter in asus_wireless_add() does not have a acpi companion device associated with it, so acpi_match_device() returns NULL. I still need to manually loop through device_ids[] in order to retrieve the .driver_data associated with the HID, and I see the same pattern in the i2c-scmi driver. I'm sending the next revision for feedback, please advise if I'm missing any detail, or if you want this implemented differently. -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH 7/8] asus-wireless: Export ids list
On 4 February 2017 at 10:35, Andy Shevchenko <andy.shevche...@gmail.com> wrote: > On Wed, Feb 1, 2017 at 2:23 PM, João Paulo Rechi Vita <jprv...@gmail.com> > wrote: >> On 27 January 2017 at 10:36, Andy Shevchenko <andy.shevche...@gmail.com> >> wrote: >>> On Thu, Jan 26, 2017 at 5:30 PM, João Paulo Rechi Vita >>> <jprv...@gmail.com> wrote: > > Fill commit message, btw. > >>>> Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> > >>>> -static const struct acpi_device_id device_ids[] = { >>>> - {"ATK4001", 0}, >>>> - {"ATK4002", 0}, >>>> - {"", 0}, >>>> -}; >>>> -MODULE_DEVICE_TABLE(acpi, device_ids); >>>> +MODULE_DEVICE_TABLE(acpi, asus_wireless_ids); >>>> >>> >>> No, Don't do this. >>> >> >> Why? > > Table is a property of certain driver. You make it visible to parts > that non need it. > Moreover, you may here the list itself non-explicit, which reduces > readability and understandability worse. > > If you would like to maintain a list of devices in two > (semi-)independent modules, it would be not good looking in any case, > either you make a hard dependency (if they already are it's okay to > just export a function which helps you to find an ID in the list), or > copy it in both modules. > > I need to check this as well. > Currently these modules do not depend on each other. There are machines which only need asus-wmi, and not asus-wireless, and there may be machines in the future which will only need asus-wireless and not asus-wmi (not really seen any in that situation, but it is a possibility), so I don't think adding a dependency here is the right thing. Duplicating code is not good, so I was trying to avoid it, but maybe there isn't really any better way in this case. >>>> static u64 asus_wireless_method(acpi_handle handle, const char *method, >>>> int param) >>>> @@ -130,8 +127,8 @@ static int asus_wireless_add(struct acpi_device *adev) >>>> adev->driver_data = data; >>>> >>>> hid = acpi_device_hid(adev); >>>> - for (i = 0; strcmp(device_ids[i].id, ""); i++) { >>> >>> This is wrong. >>> >>>> - if (!strcmp(device_ids[i].id, hid)) { >>>> + for (i = 0; strcmp(asus_wireless_ids[i].id, ""); i++) { >>> >>> This is too. >>> >>> Potential infinite loop. >>> >>> On top of that seems you just introduced this by previous patches and >>> changing here. >>> Often it means you need to reconsider how you actually split the >>> series on logical pieces. >>> >> >> Can you please elaborate a bit more? > > The original code relies on "" in the first parameter which basically > can be NULL. This fragile. > But this is part subject to change in a sequential patch. > >> All this change does is to change >> the name of the variable being iterated in the loop. As you said, the >> loop was introduced in a previous series, and you didn't spot any >> problems there. > > If I didn't spot it doesn't mean there is no issues, right? ;) > >> I don't think it makes sense to also rename the >> variable in that other series, since I'm only renaming it because I'm >> moving it to a header so it can be used by asus-wmi.c as well. > > The main problem with the above is introduction something that you are > changing soon later. > Ok, I should probably have sent this as RFC, as it was actually the case. But since I'm not going to export the ids list anymore, this patch will be dropped from the next revision. -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH 7/8] asus-wireless: Export ids list
On 4 February 2017 at 10:35, Andy Shevchenko wrote: > On Wed, Feb 1, 2017 at 2:23 PM, João Paulo Rechi Vita > wrote: >> On 27 January 2017 at 10:36, Andy Shevchenko >> wrote: >>> On Thu, Jan 26, 2017 at 5:30 PM, João Paulo Rechi Vita >>> wrote: > > Fill commit message, btw. > >>>> Signed-off-by: João Paulo Rechi Vita > >>>> -static const struct acpi_device_id device_ids[] = { >>>> - {"ATK4001", 0}, >>>> - {"ATK4002", 0}, >>>> - {"", 0}, >>>> -}; >>>> -MODULE_DEVICE_TABLE(acpi, device_ids); >>>> +MODULE_DEVICE_TABLE(acpi, asus_wireless_ids); >>>> >>> >>> No, Don't do this. >>> >> >> Why? > > Table is a property of certain driver. You make it visible to parts > that non need it. > Moreover, you may here the list itself non-explicit, which reduces > readability and understandability worse. > > If you would like to maintain a list of devices in two > (semi-)independent modules, it would be not good looking in any case, > either you make a hard dependency (if they already are it's okay to > just export a function which helps you to find an ID in the list), or > copy it in both modules. > > I need to check this as well. > Currently these modules do not depend on each other. There are machines which only need asus-wmi, and not asus-wireless, and there may be machines in the future which will only need asus-wireless and not asus-wmi (not really seen any in that situation, but it is a possibility), so I don't think adding a dependency here is the right thing. Duplicating code is not good, so I was trying to avoid it, but maybe there isn't really any better way in this case. >>>> static u64 asus_wireless_method(acpi_handle handle, const char *method, >>>> int param) >>>> @@ -130,8 +127,8 @@ static int asus_wireless_add(struct acpi_device *adev) >>>> adev->driver_data = data; >>>> >>>> hid = acpi_device_hid(adev); >>>> - for (i = 0; strcmp(device_ids[i].id, ""); i++) { >>> >>> This is wrong. >>> >>>> - if (!strcmp(device_ids[i].id, hid)) { >>>> + for (i = 0; strcmp(asus_wireless_ids[i].id, ""); i++) { >>> >>> This is too. >>> >>> Potential infinite loop. >>> >>> On top of that seems you just introduced this by previous patches and >>> changing here. >>> Often it means you need to reconsider how you actually split the >>> series on logical pieces. >>> >> >> Can you please elaborate a bit more? > > The original code relies on "" in the first parameter which basically > can be NULL. This fragile. > But this is part subject to change in a sequential patch. > >> All this change does is to change >> the name of the variable being iterated in the loop. As you said, the >> loop was introduced in a previous series, and you didn't spot any >> problems there. > > If I didn't spot it doesn't mean there is no issues, right? ;) > >> I don't think it makes sense to also rename the >> variable in that other series, since I'm only renaming it because I'm >> moving it to a header so it can be used by asus-wmi.c as well. > > The main problem with the above is introduction something that you are > changing soon later. > Ok, I should probably have sent this as RFC, as it was actually the case. But since I'm not going to export the ids list anymore, this patch will be dropped from the next revision. -- João Paulo Rechi Vita http://about.me/jprvita
[PATCHv3] platform/x86: asus-wireless: Fix identation
Fix identation problem introduced when this driver was first merged into the kernel. Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/platform/x86/asus-wireless.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c index 9f31bc1a47d0..a7ee18383146 100644 --- a/drivers/platform/x86/asus-wireless.c +++ b/drivers/platform/x86/asus-wireless.c @@ -76,8 +76,7 @@ static void led_state_update(struct work_struct *work) data->led_state); } -static void led_state_set(struct led_classdev *led, - enum led_brightness value) +static void led_state_set(struct led_classdev *led, enum led_brightness value) { struct asus_wireless_data *data; -- 2.11.0
[PATCHv3] platform/x86: asus-wireless: Fix identation
Fix identation problem introduced when this driver was first merged into the kernel. Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-wireless.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c index 9f31bc1a47d0..a7ee18383146 100644 --- a/drivers/platform/x86/asus-wireless.c +++ b/drivers/platform/x86/asus-wireless.c @@ -76,8 +76,7 @@ static void led_state_update(struct work_struct *work) data->led_state); } -static void led_state_set(struct led_classdev *led, - enum led_brightness value) +static void led_state_set(struct led_classdev *led, enum led_brightness value) { struct asus_wireless_data *data; -- 2.11.0
[PATCH] acpica: Fix double-free in acpi_ns_repair_CID()
When acpi_ns_repair_CID() is called for a _CID which returns a package of strings, it calls acpi_ns_repair_HID() for each of the package elements. acpi_ns_repair_HID() calls acpi_ut_remove_reference() on the original object, but acpi_ns_repair_CID() calls it again on return, leading to a double free. This problem was seen on a Acer TravelMate P449-G2-MG. Thanks to Daniel Drake for helping investigating this problem. Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/acpi/acpica/nsrepair2.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/acpi/acpica/nsrepair2.c b/drivers/acpi/acpica/nsrepair2.c index d5336122486b..c429c8eca476 100644 --- a/drivers/acpi/acpica/nsrepair2.c +++ b/drivers/acpi/acpica/nsrepair2.c @@ -411,8 +411,6 @@ acpi_ns_repair_CID(struct acpi_evaluate_info *info, (*element_ptr)->common.reference_count = original_ref_count; - - acpi_ut_remove_reference(original_element); } element_ptr++; -- 2.11.0
[PATCH] acpica: Fix double-free in acpi_ns_repair_CID()
When acpi_ns_repair_CID() is called for a _CID which returns a package of strings, it calls acpi_ns_repair_HID() for each of the package elements. acpi_ns_repair_HID() calls acpi_ut_remove_reference() on the original object, but acpi_ns_repair_CID() calls it again on return, leading to a double free. This problem was seen on a Acer TravelMate P449-G2-MG. Thanks to Daniel Drake for helping investigating this problem. Signed-off-by: João Paulo Rechi Vita --- drivers/acpi/acpica/nsrepair2.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/acpi/acpica/nsrepair2.c b/drivers/acpi/acpica/nsrepair2.c index d5336122486b..c429c8eca476 100644 --- a/drivers/acpi/acpica/nsrepair2.c +++ b/drivers/acpi/acpica/nsrepair2.c @@ -411,8 +411,6 @@ acpi_ns_repair_CID(struct acpi_evaluate_info *info, (*element_ptr)->common.reference_count = original_ref_count; - - acpi_ut_remove_reference(original_element); } element_ptr++; -- 2.11.0
[PATCH] asus-wireless: Fix identation
Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> --- drivers/platform/x86/asus-wireless.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c index 9f31bc1a47d0..a7ee18383146 100644 --- a/drivers/platform/x86/asus-wireless.c +++ b/drivers/platform/x86/asus-wireless.c @@ -76,8 +76,7 @@ static void led_state_update(struct work_struct *work) data->led_state); } -static void led_state_set(struct led_classdev *led, - enum led_brightness value) +static void led_state_set(struct led_classdev *led, enum led_brightness value) { struct asus_wireless_data *data; -- 2.11.0
[PATCH] asus-wireless: Fix identation
Signed-off-by: João Paulo Rechi Vita --- drivers/platform/x86/asus-wireless.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c index 9f31bc1a47d0..a7ee18383146 100644 --- a/drivers/platform/x86/asus-wireless.c +++ b/drivers/platform/x86/asus-wireless.c @@ -76,8 +76,7 @@ static void led_state_update(struct work_struct *work) data->led_state); } -static void led_state_set(struct led_classdev *led, - enum led_brightness value) +static void led_state_set(struct led_classdev *led, enum led_brightness value) { struct asus_wireless_data *data; -- 2.11.0
Re: [PATCH 1/1] asus-wireless: Fix identation
On 27 January 2017 at 10:28, Andy Shevchenko <andy.shevche...@gmail.com> wrote: > On Thu, Jan 26, 2017 at 4:25 PM, João Paulo Rechi Vita > <jprv...@gmail.com> wrote: >> Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> >> --- >> drivers/platform/x86/asus-wireless.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/platform/x86/asus-wireless.c >> b/drivers/platform/x86/asus-wireless.c >> index 9f31bc1a47d0..c9b5ac152cf1 100644 >> --- a/drivers/platform/x86/asus-wireless.c >> +++ b/drivers/platform/x86/asus-wireless.c >> @@ -77,7 +77,7 @@ static void led_state_update(struct work_struct *work) >> } >> >> static void led_state_set(struct led_classdev *led, >> - enum led_brightness value) >> + enum led_brightness value) > > I would see benefit if it would be one line. > Yes, I fixed this in a hurry and missed it actually fits in one line. Thanks for catching it! I'll reply with an updated patch. -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH 1/1] asus-wireless: Fix identation
On 27 January 2017 at 10:28, Andy Shevchenko wrote: > On Thu, Jan 26, 2017 at 4:25 PM, João Paulo Rechi Vita > wrote: >> Signed-off-by: João Paulo Rechi Vita >> --- >> drivers/platform/x86/asus-wireless.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/platform/x86/asus-wireless.c >> b/drivers/platform/x86/asus-wireless.c >> index 9f31bc1a47d0..c9b5ac152cf1 100644 >> --- a/drivers/platform/x86/asus-wireless.c >> +++ b/drivers/platform/x86/asus-wireless.c >> @@ -77,7 +77,7 @@ static void led_state_update(struct work_struct *work) >> } >> >> static void led_state_set(struct led_classdev *led, >> - enum led_brightness value) >> + enum led_brightness value) > > I would see benefit if it would be one line. > Yes, I fixed this in a hurry and missed it actually fits in one line. Thanks for catching it! I'll reply with an updated patch. -- João Paulo Rechi Vita http://about.me/jprvita
Re: [PATCH 7/8] asus-wireless: Export ids list
On 27 January 2017 at 10:36, Andy Shevchenko <andy.shevche...@gmail.com> wrote: > On Thu, Jan 26, 2017 at 5:30 PM, João Paulo Rechi Vita > <jprv...@gmail.com> wrote: >> Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com> >> --- >> drivers/platform/x86/asus-wireless.c | 15 ++- >> drivers/platform/x86/asus-wireless.h | 10 ++ >> 2 files changed, 16 insertions(+), 9 deletions(-) >> create mode 100644 drivers/platform/x86/asus-wireless.h >> >> diff --git a/drivers/platform/x86/asus-wireless.c >> b/drivers/platform/x86/asus-wireless.c >> index 5a238fad35fb..fa28613688b4 100644 >> --- a/drivers/platform/x86/asus-wireless.c >> +++ b/drivers/platform/x86/asus-wireless.c >> @@ -17,6 +17,8 @@ >> #include >> #include >> >> +#include "asus-wireless.h" >> + >> #define ASUS_WIRELESS_LED_STATUS 0x2 >> >> struct hswc_params { >> @@ -40,12 +42,7 @@ static const struct hswc_params id_params[] = { >> { 0x5, 0x4 }, >> }; >> >> -static const struct acpi_device_id device_ids[] = { >> - {"ATK4001", 0}, >> - {"ATK4002", 0}, >> - {"", 0}, >> -}; >> -MODULE_DEVICE_TABLE(acpi, device_ids); >> +MODULE_DEVICE_TABLE(acpi, asus_wireless_ids); >> > > No, Don't do this. > Why? >> static u64 asus_wireless_method(acpi_handle handle, const char *method, >> int param) >> @@ -130,8 +127,8 @@ static int asus_wireless_add(struct acpi_device *adev) >> adev->driver_data = data; >> >> hid = acpi_device_hid(adev); >> - for (i = 0; strcmp(device_ids[i].id, ""); i++) { > > This is wrong. > >> - if (!strcmp(device_ids[i].id, hid)) { >> + for (i = 0; strcmp(asus_wireless_ids[i].id, ""); i++) { > > This is too. > > Potential infinite loop. > > On top of that seems you just introduced this by previous patches and > changing here. > Often it means you need to reconsider how you actually split the > series on logical pieces. > Can you please elaborate a bit more? All this change does is to change the name of the variable being iterated in the loop. As you said, the loop was introduced in a previous series, and you didn't spot any problems there. I don't think it makes sense to also rename the variable in that other series, since I'm only renaming it because I'm moving it to a header so it can be used by asus-wmi.c as well. >> + if (!strcmp(asus_wireless_ids[i].id, hid)) { >> data->hswc_params = _params[i]; >> break; >> } >> @@ -179,7 +176,7 @@ static int asus_wireless_remove(struct acpi_device *adev) >> static struct acpi_driver asus_wireless_driver = { >> .name = "Asus Wireless Radio Control Driver", >> .class = "hotkey", >> - .ids = device_ids, >> + .ids = asus_wireless_ids, >> .ops = { >> .add = asus_wireless_add, >> .remove = asus_wireless_remove, >> diff --git a/drivers/platform/x86/asus-wireless.h >> b/drivers/platform/x86/asus-wireless.h >> new file mode 100644 >> index ..10a345863da6 >> --- /dev/null >> +++ b/drivers/platform/x86/asus-wireless.h >> @@ -0,0 +1,10 @@ >> +#ifndef _ASUS_WIRELESS_H_ >> +#define _ASUS_WIRELESS_H_ >> + >> +const struct acpi_device_id asus_wireless_ids[] = { >> + {"ATK4001", 0}, >> + {"ATK4002", 0}, >> + {"", 0}, >> +}; >> + >> +#endif /* !_ASUS_WIRELESS_H_ */ >> -- >> 2.11.0 >> > > > > -- > With Best Regards, > Andy Shevchenko -- João Paulo Rechi Vita http://about.me/jprvita