Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
On Thu, 2016-10-13 at 14:55 +0200, Paul Bolle wrote: > On Thu, 2016-10-13 at 15:44 +0300, Luca Coelho wrote: > > Even though there is apparently something wrong with this part of the > > ACPI table on you laptop, since it doesn't match our specifications. > > In any case, it's mostly harmless. > > > Would a correct implementation by Dell have any benefits for the users > of these laptops? In other words: should I bother somehow contacting > Dell and point them to this discussion in order to have them fix this? This value provides a way for the OEM to fine tune the power budget of the device. This is (usually) used to prevent parts of the platform from getting too hot. We have a certain default that is good enough for most cases. If Dell didn't want to set proper limits for this device, it probably means that it is not a concern. Dell does set these values correctly for other platforms. So, I guess it's up to you if you want to clarify this with them. This could be some default "blank" values when they don't want to change them. -- Luca.
Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
On Thu, 2016-10-13 at 08:56 -0500, Chris Rorvick wrote: > Hi Luca, > > > On Thu, 2016-10-13 at 13:21 +0300, Luca Coelho wrote: > > Could you please give this a spin? I have tested it with some handmade > > ACPI tables in QEMU and it seems to work fine now. > > > Tested-by: Chris Rorvick> > I think the debug output looks as expected, see below for the first 20 > lines or so. And, more importantly, everything seems to be working! > :-) Yes, you got exactly what was expected. Thanks for testing! -- Luca.
Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
Hi Luca, > On Thu, 2016-10-13 at 13:21 +0300, Luca Coelho wrote: > Could you please give this a spin? I have tested it with some handmade > ACPI tables in QEMU and it seems to work fine now. Tested-by: Chris RorvickI think the debug output looks as expected, see below for the first 20 lines or so. And, more importantly, everything seems to be working! :-) Thanks! Chris == iwlwifi :3a:00.0: L1 Enabled - LTR Disabled iwlwifi: module verification failed: signature and/or required key missing - tainting kernel Intel(R) Wireless WiFi driver for Linux Copyright(c) 2003- 2015 Intel Corporation iwlwifi :3a:00.0: U iwl_pcie_prepare_card_hw iwl_trans_prepare_card_hw enter iwlwifi :3a:00.0: U iwl_pcie_set_hw_ready hardware ready iwlwifi :3a:00.0: U iwl_request_firmware attempting to load firmware 'iwlwifi-8000C-24.ucode' iwlwifi :3a:00.0: Direct firmware load for iwlwifi-8000C-24.ucode failed with error -2 iwlwifi :3a:00.0: U iwl_request_firmware attempting to load firmware 'iwlwifi-8000C-23.ucode' iwlwifi :3a:00.0: Direct firmware load for iwlwifi-8000C-23.ucode failed with error -2 iwlwifi :3a:00.0: U iwl_request_firmware attempting to load firmware 'iwlwifi-8000C-22.ucode' iwlwifi :3a:00.0: Direct firmware load for iwlwifi-8000C-22.ucode failed with error -2 iwlwifi :3a:00.0: U iwl_request_firmware attempting to load firmware 'iwlwifi-8000C-21.ucode' iwlwifi :3a:00.0: U iwl_req_fw_callback Loaded firmware file 'iwlwifi-8000C-21.ucode' (2394060 bytes). iwlwifi :3a:00.0: U iwl_parse_tlv_firmware unknown TLV: 48 iwlwifi :3a:00.0: U iwl_parse_tlv_firmware GSCAN is supported but capabilities TLV is unavailable iwlwifi :3a:00.0: U splc_get_pwr_limit No element for the WiFi domain returned by the SPLC method. iwlwifi :3a:00.0: U set_dflt_pwr_limit Default power limit set to 0 iwlwifi :3a:00.0: loaded firmware version 21.302800.0 op_mode iwlmvm iwlwifi :3a:00.0: Detected Intel(R) Dual Band Wireless AC 8260, REV=0x208
Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
On Thu, 2016-10-13 at 15:44 +0300, Luca Coelho wrote: > Even though there is apparently something wrong with this part of the > ACPI table on you laptop, since it doesn't match our specifications. > In any case, it's mostly harmless. Would a correct implementation by Dell have any benefits for the users of these laptops? In other words: should I bother somehow contacting Dell and point them to this discussion in order to have them fix this? Paul Bolle
Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
On Thu, 2016-10-13 at 14:36 +0200, Paul Bolle wrote: > On Thu, 2016-10-13 at 14:30 +0300, Luca Coelho wrote: > > I forgot to say... could you load the iwlwifi module with debug=0x01 > > (module parameter), so we can see the messages the driver is printing > > when it doesn't find a proper structure? > > > That makes a lot of noise! Here's the first 100 lines or so, that > apparently are generated directly after modprobing iwlwifi. > > After these 100 lines there's a ten second gap (I guess it took me ten > seconds to actually use the wifi). I assume you don't care about that > part of the debug messages. > > Have fun! Thanks, Paul! Yeah, this is the "INFO" debugging level and it is a sort of fallback bucket when there is nothing more specific. Sorry for that. > <7>[ 767.691342] iwlwifi :3a:00.0: U iwl_pcie_prepare_card_hw > iwl_trans_prepare_card_hw enter > <7>[ 767.691362] iwlwifi :3a:00.0: U iwl_pcie_set_hw_ready hardware ready > <7>[ 767.692127] iwlwifi :3a:00.0: U iwl_request_firmware attempting to > load firmware 'iwlwifi-8000C-24.ucode' > <7>[ 767.692322] iwlwifi :3a:00.0: U splc_get_pwr_limit No element for > the WiFi domain returned by the SPLC method. This is the line I was looking for. :) So everything looks fine now. Even though there is apparently something wrong with this part of the ACPI table on you laptop, since it doesn't match our specifications. In any case, it's mostly harmless. Thanks for the help! -- Cheers, Luca.
Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
On Thu, 2016-10-13 at 14:30 +0300, Luca Coelho wrote: > I forgot to say... could you load the iwlwifi module with debug=0x01 > (module parameter), so we can see the messages the driver is printing > when it doesn't find a proper structure? That makes a lot of noise! Here's the first 100 lines or so, that apparently are generated directly after modprobing iwlwifi. After these 100 lines there's a ten second gap (I guess it took me ten seconds to actually use the wifi). I assume you don't care about that part of the debug messages. Have fun! Paul Bolle <7>[ 767.691342] iwlwifi :3a:00.0: U iwl_pcie_prepare_card_hw iwl_trans_prepare_card_hw enter <7>[ 767.691362] iwlwifi :3a:00.0: U iwl_pcie_set_hw_ready hardware ready <7>[ 767.692127] iwlwifi :3a:00.0: U iwl_request_firmware attempting to load firmware 'iwlwifi-8000C-24.ucode' <7>[ 767.692322] iwlwifi :3a:00.0: U splc_get_pwr_limit No element for the WiFi domain returned by the SPLC method. <7>[ 767.692324] iwlwifi :3a:00.0: U set_dflt_pwr_limit Default power limit set to 0 <4>[ 767.692672] iwlwifi :3a:00.0: Direct firmware load for iwlwifi-8000C-24.ucode failed with error -2 <7>[ 767.692674] iwlwifi :3a:00.0: U iwl_request_firmware attempting to load firmware 'iwlwifi-8000C-23.ucode' <4>[ 767.692683] iwlwifi :3a:00.0: Direct firmware load for iwlwifi-8000C-23.ucode failed with error -2 <7>[ 767.692684] iwlwifi :3a:00.0: U iwl_request_firmware attempting to load firmware 'iwlwifi-8000C-22.ucode' <7>[ 767.693267] iwlwifi :3a:00.0: U iwl_req_fw_callback Loaded firmware file 'iwlwifi-8000C-22.ucode' (2120860 bytes). <7>[ 767.693271] iwlwifi :3a:00.0: U iwl_parse_tlv_firmware Found debug memory segment: 0 <7>[ 767.693272] iwlwifi :3a:00.0: U iwl_parse_tlv_firmware Found debug memory segment: 1 <7>[ 767.693274] iwlwifi :3a:00.0: U iwl_parse_tlv_firmware Found debug memory segment: 2 <7>[ 767.693276] iwlwifi :3a:00.0: U iwl_parse_tlv_firmware unknown TLV: 48 <7>[ 767.693278] iwlwifi :3a:00.0: U iwl_parse_tlv_firmware GSCAN is supported but capabilities TLV is unavailable <6>[ 767.693829] iwlwifi :3a:00.0: loaded firmware version 22.361476.0 op_mode iwlmvm <6>[ 767.747133] iwlwifi :3a:00.0: Detected Intel(R) Dual Band Wireless AC 8260, REV=0x208 <7>[ 767.747141] iwlwifi :3a:00.0: U iwl_pcie_prepare_card_hw iwl_trans_prepare_card_hw enter <7>[ 767.747168] iwlwifi :3a:00.0: U iwl_pcie_set_hw_ready hardware ready <7>[ 767.749221] iwlwifi :3a:00.0: U iwl_pcie_apm_init Init card's basic functions <6>[ 767.749257] iwlwifi :3a:00.0: L1 Enabled - LTR Disabled <7>[ 767.749855] iwlwifi :3a:00.0: U iwl_pcie_prepare_card_hw iwl_trans_prepare_card_hw enter <7>[ 767.749870] iwlwifi :3a:00.0: U iwl_pcie_set_hw_ready hardware ready <7>[ 767.749878] iwlwifi :3a:00.0: U iwl_pcie_apm_init Init card's basic functions <6>[ 767.749896] iwlwifi :3a:00.0: L1 Enabled - LTR Disabled <7>[ 767.749906] iwlwifi :3a:00.0: U iwl_mvm_nic_config Radio type=0x0-0x2-0x1 <7>[ 767.750608] iwlwifi :3a:00.0: U iwl_pcie_nic_init Enabling shadow registers in device <7>[ 767.750624] iwlwifi :3a:00.0: U iwl_pcie_rsa_race_bug_wa can't access the RSA semaphore it is write protected <7>[ 767.810446] iwlwifi :3a:00.0: I iwl_mvm_rx_mfuart_notif MFUART: installed ver: 0x12000415, external ver: 0x12000415, status: 0x00010080, duration: 0x0006 <7>[ 767.810861] iwlwifi :3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command BT_CONFIG <7>[ 767.810862] iwlwifi :3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command BT_CONFIG <7>[ 767.810984] iwlwifi :3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command BT_CONFIG <7>[ 767.811023] iwlwifi :3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD <7>[ 767.811025] iwlwifi :3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD <7>[ 767.811098] iwlwifi :3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD <7>[ 767.811102] iwlwifi :3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD <7>[ 767.811103] iwlwifi :3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD <7>[ 767.811212] iwlwifi :3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD <7>[ 767.811222] iwlwifi :3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD <7>[ 767.811223] iwlwifi :3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD <7>[ 767.811318] iwlwifi :3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD <7>[ 767.811326] iwlwifi :3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD <7>[ 767.811327] iwlwifi :3a:00.0: U
Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
On Thu, 2016-10-13 at 13:27 +0200, Paul Bolle wrote: > Luca, > > On Thu, 2016-10-13 at 13:21 +0300, Luca Coelho wrote: > > Could you please give this a spin? I have tested it with some handmade > > ACPI tables in QEMU and it seems to work fine now. > > > Tested-by: Paul Bolle> > Not that this test was worth a lot: it builds cleanly (on top of > 4.8.1), the error at boot is gone, obviously, and wifi still works (as > you're reading a message that was sent out over wifi). And I haven't > even tested this on another machine than my XPS 13 (9350). Thanks for testing! I forgot to say... could you load the iwlwifi module with debug=0x01 (module parameter), so we can see the messages the driver is printing when it doesn't find a proper structure? -- Luca.
Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
Luca, On Thu, 2016-10-13 at 13:21 +0300, Luca Coelho wrote: > Could you please give this a spin? I have tested it with some handmade > ACPI tables in QEMU and it seems to work fine now. Tested-by: Paul BolleNot that this test was worth a lot: it builds cleanly (on top of 4.8.1), the error at boot is gone, obviously, and wifi still works (as you're reading a message that was sent out over wifi). And I haven't even tested this on another machine than my XPS 13 (9350). Thanks! Paul Bolle
[PATCH] iwlwifi: pcie: fix SPLC structure parsing
From: Luca CoelhoThe SPLC data parsing is too restrictive and was not trying find the correct element for WiFi. This causes problems with some BIOSes where the SPLC method exists, but doesn't have a WiFi entry on the first element of the list. The domain type values are also incorrect according to the specification. Fix this by complying with the actual specification. Additionally, replace all occurrences of SPLX to SPLC, since SPLX is only a structure internal to the ACPI tables, and may not even exist. Fixes: bcb079a14d75 ("iwlwifi: pcie: retrieve and parse ACPI power limitations") Reported-by: Chris Rorvick Signed-off-by: Luca Coelho --- Paul, Chris, Could you please give this a spin? I have tested it with some handmade ACPI tables in QEMU and it seems to work fine now. Thanks! drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 79 --- 1 file changed, 48 insertions(+), 31 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c index 001be40..da4810f 100644 --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c @@ -541,48 +541,64 @@ static const struct pci_device_id iwl_hw_card_ids[] = { MODULE_DEVICE_TABLE(pci, iwl_hw_card_ids); #ifdef CONFIG_ACPI -#define SPL_METHOD "SPLC" -#define SPL_DOMAINTYPE_MODULE BIT(0) -#define SPL_DOMAINTYPE_WIFIBIT(1) -#define SPL_DOMAINTYPE_WIGIG BIT(2) -#define SPL_DOMAINTYPE_RFEMBIT(3) +#define ACPI_SPLC_METHOD "SPLC" +#define ACPI_SPLC_DOMAIN_WIFI (0x07) -static u64 splx_get_pwr_limit(struct iwl_trans *trans, union acpi_object *splx) +static u64 splc_get_pwr_limit(struct iwl_trans *trans, union acpi_object *splc) { - union acpi_object *limits, *domain_type, *power_limit; - - if (splx->type != ACPI_TYPE_PACKAGE || - splx->package.count != 2 || - splx->package.elements[0].type != ACPI_TYPE_INTEGER || - splx->package.elements[0].integer.value != 0) { - IWL_ERR(trans, "Unsupported splx structure\n"); + union acpi_object *data_pkg, *dflt_pwr_limit; + int i; + + /* We need at least two elemets, one for the revision and one +* for the data itself. Also check that the revision is +* supported (currently only revision 0). + */ + if (splc->type != ACPI_TYPE_PACKAGE || + splc->package.count < 2 || + splc->package.elements[0].type != ACPI_TYPE_INTEGER || + splc->package.elements[0].integer.value != 0) { + IWL_DEBUG_INFO(trans, + "Unsupported structure returned by the SPLC method. Ignoring.\n"); return 0; } - limits = >package.elements[1]; - if (limits->type != ACPI_TYPE_PACKAGE || - limits->package.count < 2 || - limits->package.elements[0].type != ACPI_TYPE_INTEGER || - limits->package.elements[1].type != ACPI_TYPE_INTEGER) { - IWL_ERR(trans, "Invalid limits element\n"); - return 0; + /* loop through all the packages to find the one for WiFi */ + for (i = 1; i < splc->package.count; i++) { + union acpi_object *domain; + + data_pkg = >package.elements[i]; + + /* Skip anything that is not a package with the right +* amount of elements (i.e. at least 2 integers). +*/ + if (data_pkg->type != ACPI_TYPE_PACKAGE || + data_pkg->package.count < 2 || + data_pkg->package.elements[0].type != ACPI_TYPE_INTEGER || + data_pkg->package.elements[1].type != ACPI_TYPE_INTEGER) + continue; + + domain = _pkg->package.elements[0]; + if (domain->integer.value == ACPI_SPLC_DOMAIN_WIFI) + break; + + data_pkg = NULL; } - domain_type = >package.elements[0]; - power_limit = >package.elements[1]; - if (!(domain_type->integer.value & SPL_DOMAINTYPE_WIFI)) { - IWL_DEBUG_INFO(trans, "WiFi power is not limited\n"); + if (!data_pkg) { + IWL_DEBUG_INFO(trans, + "No element for the WiFi domain returned by the SPLC method.\n"); return 0; } - return power_limit->integer.value; + dflt_pwr_limit = _pkg->package.elements[1]; + return dflt_pwr_limit->integer.value; } static void set_dflt_pwr_limit(struct iwl_trans *trans, struct pci_dev *pdev) { acpi_handle pxsx_handle; acpi_handle handle; - struct acpi_buffer splx = {ACPI_ALLOCATE_BUFFER, NULL}; + struct acpi_buffer splc = {ACPI_ALLOCATE_BUFFER, NULL}; acpi_status status; pxsx_handle = ACPI_HANDLE(>dev); @@ -593,23 +609,24 @@