[PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Chris Rorvick]

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(-)

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;
 	}
 
-- 
2.10.1

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Luca Coelho]

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.

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Paul Bolle]

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

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Paul Bolle]

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

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Chris Rorvick]

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

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Chris Rorvick]

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

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Chris Rorvick]

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

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Luca Coelho]

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.

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Luca Coelho]

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.

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Luca Coelho]

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.

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Luca Coelho]

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!

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Paul Bolle]

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

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Luca Coelho]

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.

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Luca Coelho]

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?

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Paul Bolle]

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

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Luca Coelho]

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.

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Luca Coelho]

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!

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Chris Rorvick]

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> wrot=
e:
> > > > This is not coming from the NIC itself, but from the platform's ACP=
I
> > > > 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. =C3=82 This also matches your
> description that, at runtime, you got something different than the pure
> dump. =C3=82 If you follow these DOM*, LIM*, TIM* symbols, you'll probabl=
y
> 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 Sto=
re
    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

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Chris Rorvick]

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

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Paul Bolle]

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

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Paul Bolle]

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

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Chris Rorvick]

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

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Luca Coelho]

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.

Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning

[Luca Coelho]

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. ;)

[PATCH] iwlwifi: pcie: fix SPLC structure parsing

[Luca Coelho]

From: Luca Coelho <luciano.coelho@intel.com>

The 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 <chris@rorvick.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---

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_WIFI	BIT(1)
-#define SPL_DOMAINTYPE_WIGIG	BIT(2)
-#define SPL_DOMAINTYPE_RFEM	BIT(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 = &splx->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 = &splc->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 = &data_pkg->package.elements[0];
+		if (domain->integer.value == ACPI_SPLC_DOMAIN_WIFI)
+			break;
+
+		data_pkg = NULL;
 	}
 
-	domain_type = &limits->package.elements[0];
-	power_limit = &limits->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 = &data_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(&pdev->dev);
@@ -593,23 +609,24 @@ static void set_dflt_pwr_limit(struct iwl_trans *trans, struct pci_dev *pdev)
 	}
 
 	/* Get the method's handle */
-	status = acpi_get_handle(pxsx_handle, (acpi_string)SPL_METHOD, &handle);
+	status = acpi_get_handle(pxsx_handle, (acpi_string)ACPI_SPLC_METHOD,
+				 &handle);
 	if (ACPI_FAILURE(status)) {
-		IWL_DEBUG_INFO(trans, "SPL method not found\n");
+		IWL_DEBUG_INFO(trans, "SPLC method not found\n");
 		return;
 	}
 
 	/* Call SPLC with no arguments */
-	status = acpi_evaluate_object(handle, NULL, NULL, &splx);
+	status = acpi_evaluate_object(handle, NULL, NULL, &splc);
 	if (ACPI_FAILURE(status)) {
 		IWL_ERR(trans, "SPLC invocation failed (0x%x)\n", status);
 		return;
 	}
 
-	trans->dflt_pwr_limit = splx_get_pwr_limit(trans, splx.pointer);
+	trans->dflt_pwr_limit = splc_get_pwr_limit(trans, splc.pointer);
 	IWL_DEBUG_INFO(trans, "Default power limit set to %lld\n",
 		       trans->dflt_pwr_limit);
-	kfree(splx.pointer);
+	kfree(splc.pointer);
 }
 
 #else /* CONFIG_ACPI */
-- 
2.9.3

Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing

[Paul Bolle]

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 <pebolle@tiscali.nl>

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!


Paul Bolle

Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing

[Luca Coelho]

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 <pebolle@tiscali.nl>
> 
> 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 Coelho]

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 <pebolle-IWqWACnzNjzz+pZb47iToQ@public.gmane.org>
> 
> 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

[Paul Bolle]

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 0000:3a:00.0: U iwl_pcie_prepare_card_hw iwl_trans_prepare_card_hw enter
<7>[  767.691362] iwlwifi 0000:3a:00.0: U iwl_pcie_set_hw_ready hardware ready
<7>[  767.692127] iwlwifi 0000:3a:00.0: U iwl_request_firmware attempting to load firmware 'iwlwifi-8000C-24.ucode'
<7>[  767.692322] iwlwifi 0000:3a:00.0: U splc_get_pwr_limit No element for the WiFi domain returned by the SPLC method.
<7>[  767.692324] iwlwifi 0000:3a:00.0: U set_dflt_pwr_limit Default power limit set to 0
<4>[  767.692672] iwlwifi 0000:3a:00.0: Direct firmware load for iwlwifi-8000C-24.ucode failed with error -2
<7>[  767.692674] iwlwifi 0000:3a:00.0: U iwl_request_firmware attempting to load firmware 'iwlwifi-8000C-23.ucode'
<4>[  767.692683] iwlwifi 0000:3a:00.0: Direct firmware load for iwlwifi-8000C-23.ucode failed with error -2
<7>[  767.692684] iwlwifi 0000:3a:00.0: U iwl_request_firmware attempting to load firmware 'iwlwifi-8000C-22.ucode'
<7>[  767.693267] iwlwifi 0000:3a:00.0: U iwl_req_fw_callback Loaded firmware file 'iwlwifi-8000C-22.ucode' (2120860 bytes).
<7>[  767.693271] iwlwifi 0000:3a:00.0: U iwl_parse_tlv_firmware Found debug memory segment: 0
<7>[  767.693272] iwlwifi 0000:3a:00.0: U iwl_parse_tlv_firmware Found debug memory segment: 1
<7>[  767.693274] iwlwifi 0000:3a:00.0: U iwl_parse_tlv_firmware Found debug memory segment: 2
<7>[  767.693276] iwlwifi 0000:3a:00.0: U iwl_parse_tlv_firmware unknown TLV: 48
<7>[  767.693278] iwlwifi 0000:3a:00.0: U iwl_parse_tlv_firmware GSCAN is supported but capabilities TLV is unavailable
<6>[  767.693829] iwlwifi 0000:3a:00.0: loaded firmware version 22.361476.0 op_mode iwlmvm
<6>[  767.747133] iwlwifi 0000:3a:00.0: Detected Intel(R) Dual Band Wireless AC 8260, REV=0x208
<7>[  767.747141] iwlwifi 0000:3a:00.0: U iwl_pcie_prepare_card_hw iwl_trans_prepare_card_hw enter
<7>[  767.747168] iwlwifi 0000:3a:00.0: U iwl_pcie_set_hw_ready hardware ready
<7>[  767.749221] iwlwifi 0000:3a:00.0: U iwl_pcie_apm_init Init card's basic functions
<6>[  767.749257] iwlwifi 0000:3a:00.0: L1 Enabled - LTR Disabled
<7>[  767.749855] iwlwifi 0000:3a:00.0: U iwl_pcie_prepare_card_hw iwl_trans_prepare_card_hw enter
<7>[  767.749870] iwlwifi 0000:3a:00.0: U iwl_pcie_set_hw_ready hardware ready
<7>[  767.749878] iwlwifi 0000:3a:00.0: U iwl_pcie_apm_init Init card's basic functions
<6>[  767.749896] iwlwifi 0000:3a:00.0: L1 Enabled - LTR Disabled
<7>[  767.749906] iwlwifi 0000:3a:00.0: U iwl_mvm_nic_config Radio type=0x0-0x2-0x1
<7>[  767.750608] iwlwifi 0000:3a:00.0: U iwl_pcie_nic_init Enabling shadow registers in device
<7>[  767.750624] iwlwifi 0000:3a:00.0: U iwl_pcie_rsa_race_bug_wa can't access the RSA semaphore it is write protected
<7>[  767.810446] iwlwifi 0000:3a:00.0: I iwl_mvm_rx_mfuart_notif MFUART: installed ver: 0x12000415, external ver: 0x12000415, status: 0x00010080, duration: 0x00000006
<7>[  767.810861] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command BT_CONFIG
<7>[  767.810862] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command BT_CONFIG
<7>[  767.810984] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command BT_CONFIG
<7>[  767.811023] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.811025] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811098] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811102] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.811103] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811212] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811222] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.811223] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811318] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811326] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.811327] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811451] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811459] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.811460] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811555] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811564] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.811565] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811672] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811681] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.811682] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811742] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811746] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.811747] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811837] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811842] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.811843] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811944] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.811952] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.811953] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.812011] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.812016] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.812017] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.812073] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.812077] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.812077] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.812142] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.812146] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.812147] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.812201] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.812204] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.812205] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.812276] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.812281] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command NVM_ACCESS_CMD
<7>[  767.812281] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.812344] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command NVM_ACCESS_CMD
<7>[  767.812366] iwlwifi 0000:3a:00.0: U iwl_nvm_check_version device EEPROM VER=0xe56, CALIB=0xff
<7>[  767.812368] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command TX_ANT_CONFIGURATION_CMD
<7>[  767.812369] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command TX_ANT_CONFIGURATION_CMD
<7>[  767.812422] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command TX_ANT_CONFIGURATION_CMD
<7>[  767.812425] iwlwifi 0000:3a:00.0: U iwl_send_phy_cfg_cmd Sending Phy CFG command: 0x330018
<7>[  767.812426] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Attempting to send sync command PHY_CONFIGURATION_CMD
<7>[  767.812427] iwlwifi 0000:3a:00.0: U iwl_pcie_send_hcmd_sync Setting HCMD_ACTIVE for command PHY_CONFIGURATION_CMD
<7>[  767.812480] iwlwifi 0000:3a:00.0: I iwl_pcie_hcmd_complete Clearing HCMD_ACTIVE for command PHY_CONFIGURATION_CMD
<7>[  767.871057] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 1 , Size: 3684
<7>[  767.871065] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 2 , Size: 4
<7>[  767.871071] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 5 , Size: 2056
<7>[  767.871108] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 5 , Size: 2056
<7>[  767.871263] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 5 , Size: 2056
<7>[  767.871441] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 5 , Size: 2056
<7>[  767.871607] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 5 , Size: 2056
<7>[  767.871782] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 5 , Size: 2056
<7>[  767.871814] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 5 , Size: 2056
<7>[  767.871955] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 4 , Size: 1552
<7>[  767.872084] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 4 , Size: 1552
<7>[  767.872216] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 4 , Size: 1552
<7>[  767.872346] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 4 , Size: 1552
<7>[  767.872478] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 4 , Size: 1552
<7>[  767.872622] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 4 , Size: 1552
<7>[  767.872764] iwlwifi 0000:3a:00.0: I iwl_phy_db_set_section iwl_phy_db_set_section(282): [PHYDB]SET: Type 4 , Size: 1552
<7>[  767.872871] iwlwifi 0000:3a:00.0: U _iwl_trans_pcie_stop_device DEVICE_ENABLED bit was set and is now cleared
<7>[  767.872961] iwlwifi 0000:3a:00.0: U iwl_pcie_apm_stop Stop card, put in low power state
<7>[  767.872980] iwlwifi 0000:3a:00.0: U iwl_pcie_apm_stop_master stop master
<7>[  767.877089] iwlwifi 0000:3a:00.0: U iwl_pcie_prepare_card_hw iwl_trans_prepare_card_hw enter
<7>[  767.877119] iwlwifi 0000:3a:00.0: U iwl_pcie_set_hw_ready hardware ready
<6>[  767.905185] iwlwifi 0000:3a:00.0 wlp58s0: renamed from wlan0

Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing

[Luca Coelho]

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 0000:3a:00.0: U iwl_pcie_prepare_card_hw iwl_trans_prepare_card_hw enter
> <7>[  767.691362] iwlwifi 0000:3a:00.0: U iwl_pcie_set_hw_ready hardware ready
> <7>[  767.692127] iwlwifi 0000:3a:00.0: U iwl_request_firmware attempting to load firmware 'iwlwifi-8000C-24.ucode'
> <7>[  767.692322] iwlwifi 0000: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

[Luca Coelho]

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 0000:3a:00.0: U iwl_pcie_prepare_card_hw iwl_trans_prepare_card_hw enter
> <7>[  767.691362] iwlwifi 0000:3a:00.0: U iwl_pcie_set_hw_ready hardware ready
> <7>[  767.692127] iwlwifi 0000:3a:00.0: U iwl_request_firmware attempting to load firmware 'iwlwifi-8000C-24.ucode'
> <7>[  767.692322] iwlwifi 0000: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

[Paul Bolle]

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

[Paul Bolle]

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

[Chris Rorvick]

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 <chris@rorvick.com>

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!
:-)

Thanks!

Chris

==========

iwlwifi 0000: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 0000:3a:00.0: U iwl_pcie_prepare_card_hw iwl_trans_prepare_card_hw enter
iwlwifi 0000:3a:00.0: U iwl_pcie_set_hw_ready hardware ready
iwlwifi 0000:3a:00.0: U iwl_request_firmware attempting to load
firmware 'iwlwifi-8000C-24.ucode'
iwlwifi 0000:3a:00.0: Direct firmware load for iwlwifi-8000C-24.ucode
failed with error -2
iwlwifi 0000:3a:00.0: U iwl_request_firmware attempting to load
firmware 'iwlwifi-8000C-23.ucode'
iwlwifi 0000:3a:00.0: Direct firmware load for iwlwifi-8000C-23.ucode
failed with error -2
iwlwifi 0000:3a:00.0: U iwl_request_firmware attempting to load
firmware 'iwlwifi-8000C-22.ucode'
iwlwifi 0000:3a:00.0: Direct firmware load for iwlwifi-8000C-22.ucode
failed with error -2
iwlwifi 0000:3a:00.0: U iwl_request_firmware attempting to load
firmware 'iwlwifi-8000C-21.ucode'
iwlwifi 0000:3a:00.0: U iwl_req_fw_callback Loaded firmware file
'iwlwifi-8000C-21.ucode' (2394060 bytes).
iwlwifi 0000:3a:00.0: U iwl_parse_tlv_firmware unknown TLV: 48
iwlwifi 0000:3a:00.0: U iwl_parse_tlv_firmware GSCAN is supported but
capabilities TLV is unavailable
iwlwifi 0000:3a:00.0: U splc_get_pwr_limit No element for the WiFi
domain returned by the SPLC method.
iwlwifi 0000:3a:00.0: U set_dflt_pwr_limit Default power limit set to 0
iwlwifi 0000:3a:00.0: loaded firmware version 21.302800.0 op_mode iwlmvm
iwlwifi 0000:3a:00.0: Detected Intel(R) Dual Band Wireless AC 8260, REV=0x208

Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing

[Chris Rorvick]

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 <chris-FAYdYWbMHyxBDgjK7y7TUQ@public.gmane.org>

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!
:-)

Thanks!

Chris

==========

iwlwifi 0000: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 0000:3a:00.0: U iwl_pcie_prepare_card_hw iwl_trans_prepare_card_hw enter
iwlwifi 0000:3a:00.0: U iwl_pcie_set_hw_ready hardware ready
iwlwifi 0000:3a:00.0: U iwl_request_firmware attempting to load
firmware 'iwlwifi-8000C-24.ucode'
iwlwifi 0000:3a:00.0: Direct firmware load for iwlwifi-8000C-24.ucode
failed with error -2
iwlwifi 0000:3a:00.0: U iwl_request_firmware attempting to load
firmware 'iwlwifi-8000C-23.ucode'
iwlwifi 0000:3a:00.0: Direct firmware load for iwlwifi-8000C-23.ucode
failed with error -2
iwlwifi 0000:3a:00.0: U iwl_request_firmware attempting to load
firmware 'iwlwifi-8000C-22.ucode'
iwlwifi 0000:3a:00.0: Direct firmware load for iwlwifi-8000C-22.ucode
failed with error -2
iwlwifi 0000:3a:00.0: U iwl_request_firmware attempting to load
firmware 'iwlwifi-8000C-21.ucode'
iwlwifi 0000:3a:00.0: U iwl_req_fw_callback Loaded firmware file
'iwlwifi-8000C-21.ucode' (2394060 bytes).
iwlwifi 0000:3a:00.0: U iwl_parse_tlv_firmware unknown TLV: 48
iwlwifi 0000:3a:00.0: U iwl_parse_tlv_firmware GSCAN is supported but
capabilities TLV is unavailable
iwlwifi 0000:3a:00.0: U splc_get_pwr_limit No element for the WiFi
domain returned by the SPLC method.
iwlwifi 0000:3a:00.0: U set_dflt_pwr_limit Default power limit set to 0
iwlwifi 0000:3a:00.0: loaded firmware version 21.302800.0 op_mode iwlmvm
iwlwifi 0000:3a:00.0: Detected Intel(R) Dual Band Wireless AC 8260, REV=0x208

Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing

[Luca Coelho]

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 <chris@rorvick.com>
> 
> 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

[Luca Coelho]

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 <chris@rorvick.com>
> 
> 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!

Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing

[Luca Coelho]

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

[Luca Coelho]

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.