| Message ID | 20161010071943.4717-1-chris@rorvick.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Luca Coelho |
| Headers | show |
Hi, On Mon, 2016-10-10 at 02:19 -0500, Chris Rorvick wrote: > Commit bcb079a14d75 ("iwlwifi: pcie: retrieve and parse ACPI power > limitations") looks for a specific structure in the ACPI tables for > setting the default power limit. The data returned for at least some > dual band chipsets is not recognized, though. For example, the AC 8260 > reports the following: This is not coming from the NIC itself, but from the platform's ACPI tables. Can you tell us which platform you are using? > Name (SPLX, Package (0x04) > { > Zero, > Package (0x03) > { > 0, > 1200, > 1000 > }, > Package (0x03) > { > 0, > 1200, > 1000 > }, > Package (0x03) > { > 0, > 1200, > 1000 > } > }) This is not the structure that we are expecting. We expect this: Name (SPLX, Package (0x02) { Zero, Package (0x03) { 0x07, <value>, <value> } }) ...as you correctly pointed out. The data in the structure you have is not for WiFi (actually I don't think 0 is a valid value, but I'll double-check). > The current logic expects exactly two elements in the outer package, > causing the above to be ignored and the power limit unset. > > Despite the interface being fully functional after initialization, the > above condition is reported as an error. Knock the message down to a > warning and provide better context for understanding its consequence. Reducing this to a warning is an easy way to reduce the verbosity of the problem, but I think the correct thing to do would be to accept multiple entries and ignore the ones that don't have the WIFI marker. And only type-check the WIFI ones. There are other things that look a bit inconsistent in this code... I'll try to find the official ACPI table definitions for this entries to make sure it's correct. > Signed-off-by: Chris Rorvick <chris@rorvick.com> > --- > drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c > index 78cf9a7..19b531f 100644 > --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c > +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c > @@ -540,7 +540,7 @@ static u64 splx_get_pwr_limit(struct iwl_trans *trans, union acpi_object *splx) > 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"); > + IWL_WARN(trans, "Unsupported splx structure, not limiting WiFi power\n"); > return 0; > } If this is really bothering you, I guess I could apply this patch for now. But as I said, this is not solving the actual problem. -- Cheers, Luca.
Hi Luca, On Mon, 2016-10-10 at 17:02 +0300, Luca Coelho wrote: > On Mon, 2016-10-10 at 02:19 -0500, Chris Rorvick wrote: > This is not coming from the NIC itself, but from the platform's ACPI > tables. Can you tell us which platform you are using? On my machine I'm seeing the same error as Chris. So what exactly do you mean with "platform" here? > > Name (SPLX, Package (0x04) > > { > > Zero, > > Package (0x03) > > { > > 0, > > 1200, > > 1000 > > }, > > Package (0x03) > > { > > 0, > > 1200, > > 1000 > > }, > > Package (0x03) > > { > > 0, > > 1200, > > 1000 > > } > > }) > > This is not the structure that we are expecting. We expect this: > > Name (SPLX, Package (0x02) > { > Zero, > Package (0x03) > { > 0x07, > <value>, > <value> > } > }) > > ...as you correctly pointed out. The data in the structure you have is > not for WiFi (actually I don't think 0 is a valid value, but I'll > double-check). For what it's worth, on my machine I have twenty (!) SPLX entries, all reading: Name (SPLX, Package (0x04) { Zero, Package (0x03) { 0x80000000, 0x80000000, 0x80000000 }, Package (0x03) { 0x80000000, 0x80000000, 0x80000000 }, Package (0x03) { 0x80000000, 0x80000000, 0x80000000 } }) > There are other things that look a bit inconsistent in this code... > I'll try to find the official ACPI table definitions for this entries > to make sure it's correct. When I looked into this error, some time ago, I searched around a bit for documentation on this splx stuff. Sadly, commit bcb079a14d75 ("iwlwifi: pcie: retrieve and parse ACPI power limitations") provides very few clues and my searches turned up nothing useful. So a pointer or two would be really appreciated. > > --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c > > +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c > > @@ -540,7 +540,7 @@ static u64 splx_get_pwr_limit(struct iwl_trans *trans, union acpi_object *splx) > > 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"); > > + IWL_WARN(trans, "Unsupported splx structure, not limiting WiFi power\n"); > > return 0; > > } > > If this is really bothering you, I guess I could apply this patch for > now. But as I said, this is not solving the actual problem. Bikeshedding: I think IWL_INFO() is more appropriate, as info doesn't imply one needs to act on this message, while warn does imply that action is needed. Thanks, Paul Bolle
Hi Luca, I didn't receive your email so I'll try to respond via Paul's. On Tue, Oct 11, 2016 at 5:11 AM, Paul Bolle <pebolle@tiscali.nl> wrote: >> This is not coming from the NIC itself, but from the platform's ACPI >> tables. Can you tell us which platform you are using? Interesting. I'm running a Dell XPS 13 9350. I replaced the factory-provided Broadcom card with an AC 8260. I can update the commit log to reflect this. >> There are other things that look a bit inconsistent in this code... >> I'll try to find the official ACPI table definitions for this entries >> to make sure it's correct. > > When I looked into this error, some time ago, I searched around a bit > for documentation on this splx stuff. Sadly, commit bcb079a14d75 > ("iwlwifi: pcie: retrieve and parse ACPI power limitations") provides > very few clues and my searches turned up nothing useful. So a pointer > or two would be really appreciated. Ditto. >> If this is really bothering you, I guess I could apply this patch for >> now. But as I said, this is not solving the actual problem. > > Bikeshedding: I think IWL_INFO() is more appropriate, as info doesn't > imply one needs to act on this message, while warn does imply that > action is needed. Agreed. I still think making this a warning is appropriate, but it seems pretty clear this is not an error. This has nothing to do with how much it bothers me. An error tells the user something needs to be fixed, but in this case the interface is working fine. Making it a warning with an improved message will result in fewer people wasting their time. Thanks! Chris
On Tue, Oct 11, 2016 at 9:09 AM, Chris Rorvick <chris@rorvick.com> wrote: > I didn't receive your email so I'll try to respond via Paul's. >>> If this is really bothering you, I guess I could apply this patch for >>> now. But as I said, this is not solving the actual problem. >> >> Bikeshedding: I think IWL_INFO() is more appropriate, as info doesn't >> imply one needs to act on this message, while warn does imply that >> action is needed. > > Agreed. I still think making this a warning is appropriate, but it > seems pretty clear this is not an error. This has nothing to do with > how much it bothers me. An error tells the user something needs to be > fixed, but in this case the interface is working fine. Making it a > warning with an improved message will result in fewer people wasting > their time. I found your original email on lkml.org... should have looked there in the first place! Yes, if there is a fix for the underlying issue then that is obviously preferred. When I investigated this I saw several reports spanning at least a few distros and kernel versions with at least some concluding "this is normal". Again, thanks! Chris
On Tue, Oct 11, 2016 at 5:11 AM, Paul Bolle <pebolle@tiscali.nl> wrote: > For what it's worth, on my machine I have twenty (!) SPLX entries, all > reading: > Name (SPLX, Package (0x04) > { > Zero, > Package (0x03) > { > 0x80000000, > 0x80000000, > 0x80000000 > }, > > Package (0x03) > { > 0x80000000, > 0x80000000, > 0x80000000 > }, > > Package (0x03) > { > 0x80000000, > 0x80000000, > 0x80000000 > } > }) I actually see exactly the same on my Dell XPS 13 (9350) when I use acpidump, etc. I typed the entry I included in the commit log by hand based on what the driver gets back from the SPLC method (I added a function to dump the returned object.) Chris
Hi Paul, On Tue, 2016-10-11 at 12:11 +0200, Paul Bolle wrote: > On Mon, 2016-10-10 at 17:02 +0300, Luca Coelho wrote: > > On Mon, 2016-10-10 at 02:19 -0500, Chris Rorvick wrote: > > This is not coming from the NIC itself, but from the platform's ACPI > > tables. Can you tell us which platform you are using? > > > On my machine I'm seeing the same error as Chris. So what exactly do > you mean with "platform" here? By "platform" I meant the PC you are using. The ACPI table is created by the OEM, so different PCs have different tables. > > > > Name (SPLX, Package (0x04) > > > { > > > Zero, > > > Package (0x03) > > > { > > > 0, > > > 1200, > > > 1000 > > > }, > > > Package (0x03) > > > { > > > 0, > > > 1200, > > > 1000 > > > }, > > > Package (0x03) > > > { > > > 0, > > > 1200, > > > 1000 > > > } > > > }) > > > > > > This is not the structure that we are expecting. We expect this: > > > > Name (SPLX, Package (0x02) > > { > > Zero, > > Package (0x03) > > { > > 0x07, > > <value>, > > <value> > > } > > }) > > > > ...as you correctly pointed out. The data in the structure you have is > > not for WiFi (actually I don't think 0 is a valid value, but I'll > > double-check). > > > For what it's worth, on my machine I have twenty (!) SPLX entries, all > reading: > Name (SPLX, Package (0x04) > { > Zero, > Package (0x03) > { > 0x80000000, > 0x80000000, > 0x80000000 > }, > > Package (0x03) > { > 0x80000000, > 0x80000000, > 0x80000000 > }, > > Package (0x03) > { > 0x80000000, > 0x80000000, > 0x80000000 > } > }) Thanks. So this is another case where the first value doesn't match what we are expecting and we should just ignore that. > > There are other things that look a bit inconsistent in this code... > > I'll try to find the official ACPI table definitions for this entries > > to make sure it's correct. > > > When I looked into this error, some time ago, I searched around a bit > for documentation on this splx stuff. Sadly, commit bcb079a14d75 > ("iwlwifi: pcie: retrieve and parse ACPI power limitations") provides > very few clues and my searches turned up nothing useful. So a pointer > or two would be really appreciated. Yeah, I looked into that commit too and there's not much there. I'll try to find the documentation and, if I can, I'll share it with you. > > > --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c > > > +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c > > > @@ -540,7 +540,7 @@ static u64 splx_get_pwr_limit(struct iwl_trans *trans, union acpi_object *splx) > > > 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"); > > > + IWL_WARN(trans, "Unsupported splx structure, not limiting WiFi power\n"); > > > return 0; > > > } > > > > > > If this is really bothering you, I guess I could apply this patch for > > now. But as I said, this is not solving the actual problem. > > > Bikeshedding: I think IWL_INFO() is more appropriate, as info doesn't > imply one needs to act on this message, while warn does imply that > action is needed. Right, but in fact, the code considers that if the SPLX method exists, it must return a value iwlwifi can understand, thus the error. That assumption is wrong, so we should just ignore entries that don't match and continue without printing anything out (as would happen if the splx method were not even there). In any case, I will rework this code, so I'd prefer if we skip this patch entirely. -- Cheers, Luca.
Hi Chris, On Tue, 2016-10-11 at 09:09 -0500, Chris Rorvick wrote: > On Tue, Oct 11, 2016 at 5:11 AM, Paul Bolle <pebolle@tiscali.nl> wrote: > > > This is not coming from the NIC itself, but from the platform's ACPI > > > tables. Can you tell us which platform you are using? > > > Interesting. I'm running a Dell XPS 13 9350. I replaced the > factory-provided Broadcom card with an AC 8260. I can update the > commit log to reflect this. Okay, so this makes sense. Those entries are probably formatted for the Broadcom card, which the iwlwifi driver obviously doesn't understand. The best we can do, as I already said, is to ignore values we don't understand. I will also check what is the correct procedure in such cases, because it is possible, in theory, that the format *matches* but applies only to another device. > > > If this is really bothering you, I guess I could apply this patch for > > > now. But as I said, this is not solving the actual problem. > > > > > > Bikeshedding: I think IWL_INFO() is more appropriate, as info doesn't > > imply one needs to act on this message, while warn does imply that > > action is needed. > > > Agreed. I still think making this a warning is appropriate, but it > seems pretty clear this is not an error. This has nothing to do with > how much it bothers me. An error tells the user something needs to be > fixed, but in this case the interface is working fine. Making it a > warning with an improved message will result in fewer people wasting > their time. Yes, so I'll try to stop wasting people's timing by trying to do the correct thing without bothering the user at all. :) Thanks for pointing this all out! -- Cheers, Luca.
Luca, On Wed, 2016-10-12 at 09:11 +0300, Luca Coelho wrote: > By "platform" I meant the PC you are using. The ACPI table is created > by the OEM, so different PCs have different tables. Like Chris I use a Dell XPS 13 (9350), but mine came with an AC 8260 out of it's assembly plant: Detected Intel(R) Dual Band Wireless AC 8260, REV=0x208 > In any case, I will rework this code, so I'd prefer if we skip this > patch entirely. Feel free to prod me for testing whatever you come up with. Paul Bolle
On Tue, 2016-10-11 at 23:32 -0500, Chris Rorvick wrote: > On Tue, Oct 11, 2016 at 5:11 AM, Paul Bolle <pebolle@tiscali.nl> wrote: > > For what it's worth, on my machine I have twenty (!) SPLX entries, all > > reading: > > Name (SPLX, Package (0x04) > > { > > Zero, > > Package (0x03) > > { > > 0x80000000, > > 0x80000000, > > 0x80000000 > > }, > > > > Package (0x03) > > { > > 0x80000000, > > 0x80000000, > > 0x80000000 > > }, > > > > Package (0x03) > > { > > 0x80000000, > > 0x80000000, > > 0x80000000 > > } > > }) > > > I actually see exactly the same on my Dell XPS 13 (9350) when I use > acpidump, etc. I typed the entry I included in the commit log by hand > based on what the driver gets back from the SPLC method (I added a > function to dump the returned object.) Okay... Actually this is a structure in the BIOS and the actual method we call is SPLC. The SPLC method may return one item from this table, or something entirely different, possible one of the three values depending on a configuration option or so. Can you to find and send me the actual SPLC method that we call, from your BIOS? -- Cheers, Luca.
On Wed, 2016-10-12 at 15:24 +0300, Luca Coelho wrote: > Okay... Actually this is a structure in the BIOS and the actual method > we call is SPLC. The SPLC method may return one item from this table, > or something entirely different, possible one of the three values > depending on a configuration option or so. > > Can you to find and send me the actual SPLC method that we call, from > your BIOS? It seems Chris and I basically have identical setups, so I'll answer. There are 20 SPLC methods in the BIOS. The first reads Method (SPLC, 0, Serialized) { DerefOf (SPLX [One]) [Zero] = DOM1 /* \DOM1 */ DerefOf (SPLX [One]) [One] = LIM1 /* \LIM1 */ DerefOf (SPLX [One]) [0x02] = TIM1 /* \TIM1 */ DerefOf (SPLX [0x02]) [Zero] = DOM2 /* \DOM2 */ DerefOf (SPLX [0x02]) [One] = LIM2 /* \LIM2 */ DerefOf (SPLX [0x02]) [0x02] = TIM2 /* \TIM2 */ DerefOf (SPLX [0x03]) [Zero] = DOM3 /* \DOM3 */ DerefOf (SPLX [0x03]) [One] = LIM3 /* \LIM3 */ DerefOf (SPLX [0x03]) [0x02] = TIM3 /* \TIM3 */ Return (SPLX) /* \_SB_.PCI0.RP01.PXSX.SPLX */ } The only difference is in the last comment. Ie, RP01 is increased until it reaches RP20. (The machine has 20 PCI devices according to lspci. I have no clue how to match that RPxx number to the 20 devices showing up in lspci, sorry.) Paul Bolle
On Wed, 2016-10-12 at 14:36 +0200, Paul Bolle wrote: > On Wed, 2016-10-12 at 15:24 +0300, Luca Coelho wrote: > > Okay... Actually this is a structure in the BIOS and the actual method > > we call is SPLC. The SPLC method may return one item from this table, > > or something entirely different, possible one of the three values > > depending on a configuration option or so. > > > > Can you to find and send me the actual SPLC method that we call, from > > your BIOS? > > > It seems Chris and I basically have identical setups, so I'll answer. Thanks! Yeah, I implied any of you two. ;) > There are 20 SPLC methods in the BIOS. The first reads > Method (SPLC, 0, Serialized) > { > DerefOf (SPLX [One]) [Zero] = DOM1 /* \DOM1 */ > DerefOf (SPLX [One]) [One] = LIM1 /* \LIM1 */ > DerefOf (SPLX [One]) [0x02] = TIM1 /* \TIM1 */ > DerefOf (SPLX [0x02]) [Zero] = DOM2 /* \DOM2 */ > DerefOf (SPLX [0x02]) [One] = LIM2 /* \LIM2 */ > DerefOf (SPLX [0x02]) [0x02] = TIM2 /* \TIM2 */ > DerefOf (SPLX [0x03]) [Zero] = DOM3 /* \DOM3 */ > DerefOf (SPLX [0x03]) [One] = LIM3 /* \LIM3 */ > DerefOf (SPLX [0x03]) [0x02] = TIM3 /* \TIM3 */ > Return (SPLX) /* \_SB_.PCI0.RP01.PXSX.SPLX */ > } > > The only difference is in the last comment. Ie, RP01 is increased until > it reaches RP20. (The machine has 20 PCI devices according to lspci. I > have no clue how to match that RPxx number to the 20 devices showing up > in lspci, sorry.) No problem, these BIOSes are usually quite cryptic. :) But what you're saying makes sense. They have added the SPLC method to all PCI root- ports (which is what RP stands for here). And, the values in the SPLX structs are being changed here, to DOM1, LIM1, TIM1 etc., before being returned. This also matches your description that, at runtime, you got something different than the pure dump. If you follow these DOM*, LIM*, TIM* symbols, you'll probably end up getting the values you observed at runtime. Basically this tells me that indeed 3 "structs" are being returned (as your dumps already showed). And, according to the specs that I found (which unfortunately are confidential, so I can't share) this is correct and the driver code is broken. I'll send you a patch for testing soon. Thanks for all the help! -- Cheers, Luca.
Hi Luca, FYI, It seems that Google does not like your email as I'm not receiving any of your messages in gmail. Some responses below: On Wed, 2016-10-12 at 15:24 +0300, Luca Coelho wrote: > Hi Chris, > On Tue, 2016-10-11 at 09:09 -0500, Chris Rorvick wrote: > > On Tue, Oct 11, 2016 at 5:11 AM, Paul Bolle <pebolle [at] tiscali> wrote: > > > > This is not coming from the NIC itself, but from the platform's ACPI > > > > tables. Can you tell us which platform you are using? > > > > Interesting. I'm running a Dell XPS 13 9350. I replaced the > > factory-provided Broadcom card with an AC 8260. I can update the > > commit log to reflect this. > > Okay, so this makes sense. Those entries are probably formatted for > the Broadcom card, which the iwlwifi driver obviously doesn't > understand. The best we can do, as I already said, is to ignore values > we don't understand. This may already be apparent, but Dell sells two versions of the 9350: one with the Broadcom adapter and one with the AC 8260. I just happened to find the former version at a deep discount at Costco so decided to chance it. Turns out the Broadcom card is not so good even with new kernels so I upgraded. Anyway, since Paul is seeing the same issue I don't think the values are intended to be Broadcom-specific. On Wed, 2016-10-12 at 17:21 +0300, Luca Coelho wrote: > And, the values in the SPLX structs are being changed here, to DOM1, > LIM1, TIM1 etc., before being returned. Â This also matches your > description that, at runtime, you got something different than the pure > dump. Â If you follow these DOM*, LIM*, TIM* symbols, you'll probably > end up getting the values you observed at runtime. Probably not important, but it seems that there is some additional indirection. The only values I'm seeing associated with those symbols are 8 and 16: $ grep -e 'DOM[0-9]' -e 'LIM[0-9]' -e 'TIM[0-9]' dsdt.dsl | grep -v Store DOM1, 8, LIM1, 16, TIM1, 16, DOM2, 8, LIM2, 16, TIM2, 16, DOM3, 8, LIM3, 16, TIM3, 16, > I'll send you a patch for testing soon. I will keep an eye on the list archive, thanks! Chris
On Wed, 2016-10-12 at 12:50 -0500, Chris Rorvick wrote: > This may already be apparent, but Dell sells two versions of the 9350: > one with the Broadcom adapter and one with the AC 8260. Off topic, for most readers: my version (with the AC 8260) came with Ubuntu preinstalled. Perhaps Chris' version came preinstalled with something else? Thanks, Paul Bolle
On Wed, Oct 12, 2016 at 1:05 PM, Paul Bolle <pebolle@tiscali.nl> wrote: > On Wed, 2016-10-12 at 12:50 -0500, Chris Rorvick wrote: >> This may already be apparent, but Dell sells two versions of the 9350: >> one with the Broadcom adapter and one with the AC 8260. > > Off topic, for most readers: my version (with the AC 8260) came with > Ubuntu preinstalled. Perhaps Chris' version came preinstalled with > something else? Exactly. Mine came with Windows 10 and the Broadcom adapter, while the "Developer Edition" comes preloaded with Ubuntu and has the AC 8260. The Broadcom adapter has been supported since 4.4 but still seems to have issues. For example, I had it working at home but I was not able to connect to a friend's AT&T U-Verse wireless gateway; something was failing with -EBUSY. I ordered the AC 8260 and it works fine after the upgrade. Chris
On Wed, 2016-10-12 at 12:50 -0500, Chris Rorvick wrote: > Hi Luca, > > FYI, It seems that Google does not like your email as I'm not > receiving any of your messages in gmail. Some responses below: That's odd. It works for me. Replied privately to try to sort this out. > On Wed, 2016-10-12 at 15:24 +0300, Luca Coelho wrote: > > Hi Chris, > > On Tue, 2016-10-11 at 09:09 -0500, Chris Rorvick wrote: > > > On Tue, Oct 11, 2016 at 5:11 AM, Paul Bolle <pebolle [at] tiscali> wrote: > > > > > This is not coming from the NIC itself, but from the platform's ACPI > > > > > tables. Can you tell us which platform you are using? > > > > > > > > > Interesting. I'm running a Dell XPS 13 9350. I replaced the > > > factory-provided Broadcom card with an AC 8260. I can update the > > > commit log to reflect this. > > > > > > Okay, so this makes sense. Those entries are probably formatted for > > the Broadcom card, which the iwlwifi driver obviously doesn't > > understand. The best we can do, as I already said, is to ignore values > > we don't understand. > > > This may already be apparent, but Dell sells two versions of the 9350: > one with the Broadcom adapter and one with the AC 8260. I just > happened to find the former version at a deep discount at Costco so > decided to chance it. Turns out the Broadcom card is not so good even > with new kernels so I upgraded. Anyway, since Paul is seeing the same > issue I don't think the values are intended to be Broadcom-specific. Right, this is not for Broadcom. I found out that this is something Intel specifies as part of the entire platform's ACPI recommendations. > On Wed, 2016-10-12 at 17:21 +0300, Luca Coelho wrote: > > And, the values in the SPLX structs are being changed here, to DOM1, > > LIM1, TIM1 etc., before being returned. Â This also matches your > > description that, at runtime, you got something different than the pure > > dump. Â If you follow these DOM*, LIM*, TIM* symbols, you'll probably > > end up getting the values you observed at runtime. > > > Probably not important, but it seems that there is some additional > indirection. The only values I'm seeing associated with those symbols > are 8 and 16: > > $ grep -e 'DOM[0-9]' -e 'LIM[0-9]' -e 'TIM[0-9]' dsdt.dsl | grep -v Store > DOM1, 8, > LIM1, 16, > TIM1, 16, > DOM2, 8, > LIM2, 16, > TIM2, 16, > DOM3, 8, > LIM3, 16, > TIM3, 16, Yeah, there are often many levels of indirection. These settings can be also tied to the configuration that is reachable by the user in the BIOS setup, so you never know. The easiest way is probably to run the ASL with acpiexec and execute the method... > > I'll send you a patch for testing soon. > > > I will keep an eye on the list archive, thanks! I'll ping you from my Intel address and provide you with a link to the patch, to make it easier for you. ;) -- Cheers, Luca.
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c index 78cf9a7..19b531f 100644 --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c @@ -540,7 +540,7 @@ static u64 splx_get_pwr_limit(struct iwl_trans *trans, union acpi_object *splx) 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"); + IWL_WARN(trans, "Unsupported splx structure, not limiting WiFi power\n"); return 0; }
Commit bcb079a14d75 ("iwlwifi: pcie: retrieve and parse ACPI power limitations") looks for a specific structure in the ACPI tables for setting the default power limit. The data returned for at least some dual band chipsets is not recognized, though. For example, the AC 8260 reports the following: Name (SPLX, Package (0x04) { Zero, Package (0x03) { 0, 1200, 1000 }, Package (0x03) { 0, 1200, 1000 }, Package (0x03) { 0, 1200, 1000 } }) The current logic expects exactly two elements in the outer package, causing the above to be ignored and the power limit unset. Despite the interface being fully functional after initialization, the above condition is reported as an error. Knock the message down to a warning and provide better context for understanding its consequence. Signed-off-by: Chris Rorvick <chris@rorvick.com> --- drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)