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

From: Paul Bolle
Date: Tue Oct 11 2016 - 06:11:56 EST


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