[PATCH] wifi: iwlwifi: fix system commands group ordering

[PATCH] wifi: iwlwifi: fix system commands group ordering

[Emmanuel Grumbach]

From: Miri Korenblit <miriam.rachel.korenblit@intel.com>

The commands should be sorted inside the group definition.
Fix the ordering so we won't get following warning:
WARN_ON(iwl_cmd_groups_verify_sorted(trans_cfg))

Fixes: b6e3d1ba4fcf ("wifi: iwlwifi: mvm: implement new firmware API for statistics")
Change-Id: I0112bd7da4559a8e69e84cd8f55ba8e6fe8252cb
Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
index fef86a8b4163..1627b2f819db 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
@@ -550,9 +550,9 @@ static const struct iwl_hcmd_names iwl_mvm_system_names[] = {
 	HCMD_NAME(RFI_CONFIG_CMD),
 	HCMD_NAME(RFI_GET_FREQ_TABLE_CMD),
 	HCMD_NAME(SYSTEM_FEATURES_CONTROL_CMD),
-	HCMD_NAME(RFI_DEACTIVATE_NOTIF),
 	HCMD_NAME(SYSTEM_STATISTICS_CMD),
 	HCMD_NAME(SYSTEM_STATISTICS_END_NOTIF),
+	HCMD_NAME(RFI_DEACTIVATE_NOTIF),
 };
 
 /* Please keep this array *SORTED* by hex value.
-- 
2.41.0

[PATCH v2] wifi: iwlwifi: fix system commands group ordering

[Emmanuel Grumbach]

From: Miri Korenblit <miriam.rachel.korenblit@intel.com>

The commands should be sorted inside the group definition.
Fix the ordering so we won't get following warning:
WARN_ON(iwl_cmd_groups_verify_sorted(trans_cfg))

Fixes: b6e3d1ba4fcf ("wifi: iwlwifi: mvm: implement new firmware API for statistics")
Change-Id: I18f7b57c7c565537e3d97f8c7ff83f0cc8e4709c
Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
v2: remove ChangeId
---
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
index fef86a8b4163..1627b2f819db 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
@@ -550,9 +550,9 @@ static const struct iwl_hcmd_names iwl_mvm_system_names[] = {
 	HCMD_NAME(RFI_CONFIG_CMD),
 	HCMD_NAME(RFI_GET_FREQ_TABLE_CMD),
 	HCMD_NAME(SYSTEM_FEATURES_CONTROL_CMD),
-	HCMD_NAME(RFI_DEACTIVATE_NOTIF),
 	HCMD_NAME(SYSTEM_STATISTICS_CMD),
 	HCMD_NAME(SYSTEM_STATISTICS_END_NOTIF),
+	HCMD_NAME(RFI_DEACTIVATE_NOTIF),
 };
 
 /* Please keep this array *SORTED* by hex value.
-- 
2.41.0

[PATCH v2] wifi: iwlwifi: fix system commands group ordering

[Emmanuel Grumbach]

From: Miri Korenblit <miriam.rachel.korenblit@intel.com>

The commands should be sorted inside the group definition.
Fix the ordering so we won't get following warning:
WARN_ON(iwl_cmd_groups_verify_sorted(trans_cfg))

Fixes: b6e3d1ba4fcf ("wifi: iwlwifi: mvm: implement new firmware API for statistics")
Change-Id: I18f7b57c7c565537e3d97f8c7ff83f0cc8e4709c
Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
v2: remove ChangeId
---
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
index fef86a8b4163..1627b2f819db 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
@@ -550,9 +550,9 @@ static const struct iwl_hcmd_names iwl_mvm_system_names[] = {
 	HCMD_NAME(RFI_CONFIG_CMD),
 	HCMD_NAME(RFI_GET_FREQ_TABLE_CMD),
 	HCMD_NAME(SYSTEM_FEATURES_CONTROL_CMD),
-	HCMD_NAME(RFI_DEACTIVATE_NOTIF),
 	HCMD_NAME(SYSTEM_STATISTICS_CMD),
 	HCMD_NAME(SYSTEM_STATISTICS_END_NOTIF),
+	HCMD_NAME(RFI_DEACTIVATE_NOTIF),
 };
 
 /* Please keep this array *SORTED* by hex value.
-- 
2.41.0

[PATCH v3] wifi: iwlwifi: fix system commands group ordering

[Emmanuel Grumbach]

From: Miri Korenblit <miriam.rachel.korenblit@intel.com>

The commands should be sorted inside the group definition.
Fix the ordering so we won't get following warning:
WARN_ON(iwl_cmd_groups_verify_sorted(trans_cfg))

Fixes: b6e3d1ba4fcf ("wifi: iwlwifi: mvm: implement new firmware API for statistics")
Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
v3: remove changeId for real...
---
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
index fef86a8b4163..1627b2f819db 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
@@ -550,9 +550,9 @@ static const struct iwl_hcmd_names iwl_mvm_system_names[] = {
 	HCMD_NAME(RFI_CONFIG_CMD),
 	HCMD_NAME(RFI_GET_FREQ_TABLE_CMD),
 	HCMD_NAME(SYSTEM_FEATURES_CONTROL_CMD),
-	HCMD_NAME(RFI_DEACTIVATE_NOTIF),
 	HCMD_NAME(SYSTEM_STATISTICS_CMD),
 	HCMD_NAME(SYSTEM_STATISTICS_END_NOTIF),
+	HCMD_NAME(RFI_DEACTIVATE_NOTIF),
 };
 
 /* Please keep this array *SORTED* by hex value.
-- 
2.41.0

Re: [PATCH v3] wifi: iwlwifi: fix system commands group ordering

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 1971 bytes --]

On Sun, Nov 12, 2023 at 12:09:44PM +0200, Emmanuel Grumbach wrote:
> From: Miri Korenblit <miriam.rachel.korenblit@intel.com>
> 
> The commands should be sorted inside the group definition.
> Fix the ordering so we won't get following warning:
> WARN_ON(iwl_cmd_groups_verify_sorted(trans_cfg))
> 
> Fixes: b6e3d1ba4fcf ("wifi: iwlwifi: mvm: implement new firmware API for statistics")
> Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

Hi Emmanuel,

As this fixes a regression tracked by regzbot, let this be known by
regzbot by adding missing tags:

Link: https://lore.kernel.org/regressions/2fa930bb-54dd-4942-a88d-05a47c8e9731@gmail.com/
Link: https://lore.kernel.org/linux-wireless/CAHk-=wix6kqQ5vHZXjOPpZBfM7mMm9bBZxi2Jh7XnaKCqVf94w@mail.gmail.com/

> ---
> v3: remove changeId for real...
> ---
>  drivers/net/wireless/intel/iwlwifi/mvm/ops.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
> index fef86a8b4163..1627b2f819db 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
> @@ -550,9 +550,9 @@ static const struct iwl_hcmd_names iwl_mvm_system_names[] = {
>  	HCMD_NAME(RFI_CONFIG_CMD),
>  	HCMD_NAME(RFI_GET_FREQ_TABLE_CMD),
>  	HCMD_NAME(SYSTEM_FEATURES_CONTROL_CMD),
> -	HCMD_NAME(RFI_DEACTIVATE_NOTIF),
>  	HCMD_NAME(SYSTEM_STATISTICS_CMD),
>  	HCMD_NAME(SYSTEM_STATISTICS_END_NOTIF),
> +	HCMD_NAME(RFI_DEACTIVATE_NOTIF),
>  };
>  
>  /* Please keep this array *SORTED* by hex value.

This fix is confirmed by one of original reporter and another tester, so:

Tested-by: Niklāvs Koļesņikovs <pinkflames.linux@gmail.com>
Tested-by: Damian Tometzki <damian@riscv-rocks.de>

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v4] wifi: iwlwifi: fix system commands group ordering

[Emmanuel Grumbach]

From: Miri Korenblit <miriam.rachel.korenblit@intel.com>

The commands should be sorted inside the group definition.
Fix the ordering so we won't get following warning:
WARN_ON(iwl_cmd_groups_verify_sorted(trans_cfg))

Link: https://lore.kernel.org/regressions/2fa930bb-54dd-4942-a88d-05a47c8e9731@gmail.com/
Link: https://lore.kernel.org/linux-wireless/CAHk-=wix6kqQ5vHZXjOPpZBfM7mMm9bBZxi2Jh7XnaKCqVf94w@mail.gmail.com/
Fixes: b6e3d1ba4fcf ("wifi: iwlwifi: mvm: implement new firmware API for statistics")
Tested-by: Niklāvs Koļesņikovs <pinkflames.linux@gmail.com>
Tested-by: Damian Tometzki <damian@riscv-rocks.de>
Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
v3: remove ChangeId
v4: add the required tested-by and link tags
---
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
index fef86a8b4163..1627b2f819db 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
@@ -550,9 +550,9 @@ static const struct iwl_hcmd_names iwl_mvm_system_names[] = {
 	HCMD_NAME(RFI_CONFIG_CMD),
 	HCMD_NAME(RFI_GET_FREQ_TABLE_CMD),
 	HCMD_NAME(SYSTEM_FEATURES_CONTROL_CMD),
-	HCMD_NAME(RFI_DEACTIVATE_NOTIF),
 	HCMD_NAME(SYSTEM_STATISTICS_CMD),
 	HCMD_NAME(SYSTEM_STATISTICS_END_NOTIF),
+	HCMD_NAME(RFI_DEACTIVATE_NOTIF),
 };
 
 /* Please keep this array *SORTED* by hex value.
-- 
2.41.0

Re: [PATCH v4] wifi: iwlwifi: fix system commands group ordering

[Kalle Valo]

Emmanuel Grumbach <emmanuel.grumbach@intel.com> writes:

> From: Miri Korenblit <miriam.rachel.korenblit@intel.com>
>
> The commands should be sorted inside the group definition.
> Fix the ordering so we won't get following warning:
> WARN_ON(iwl_cmd_groups_verify_sorted(trans_cfg))
>
> Link: https://lore.kernel.org/regressions/2fa930bb-54dd-4942-a88d-05a47c8e9731@gmail.com/
> Link: https://lore.kernel.org/linux-wireless/CAHk-=wix6kqQ5vHZXjOPpZBfM7mMm9bBZxi2Jh7XnaKCqVf94w@mail.gmail.com/
> Fixes: b6e3d1ba4fcf ("wifi: iwlwifi: mvm: implement new firmware API for statistics")
> Tested-by: Niklāvs Koļesņikovs <pinkflames.linux@gmail.com>
> Tested-by: Damian Tometzki <damian@riscv-rocks.de>
> Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

Linus, I suspect it will take a while before we make a new wireless pull
request (there's currently just one commit in the wireless tree). So if
you want to take this directly to your tree please go ahead, otherwise
you get it via the normal route in a week or two. Just let us know what
you prefer.

Acked-by: Kalle Valo <kvalo@kernel.org>

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v4] wifi: iwlwifi: fix system commands group ordering

[Linus Torvalds]

On Sun, 12 Nov 2023 at 10:54, Kalle Valo <kvalo@kernel.org> wrote:
>
> Linus, I suspect it will take a while before we make a new wireless pull
> request (there's currently just one commit in the wireless tree). So if
> you want to take this directly to your tree please go ahead, otherwise
> you get it via the normal route in a week or two. Just let us know what
> you prefer.
>
> Acked-by: Kalle Valo <kvalo@kernel.org>

Ok, I just tested it in my private tree, and it fixes the WARN_ON() as
expected, so I'll apply it for real.

However, now that I don't have that big warning in there, I do note
another iwlwifi issue that is new to this meger window:

  debugfs: Directory 'iwlmvm' with parent 'netdev:wlo2' already present!
  iwlwifi 0000:45:00.0: Failed to create debugfs directory under netdev:wlo2
  debugfs: Directory 'iwlmvm' with parent 'netdev:wlo2' already present!
  iwlwifi 0000:45:00.0: Failed to create debugfs directory under netdev:wlo2

and looking at my system logs, this is new.

It looks like it is probably due to

  c36235acb34f ("wifi: iwlwifi: mvm: rework debugfs handling")
  e9dd25550770 ("wifi: iwlwifi: mvm: add a per-link debugfs")

but that's just from looking at the patches (ie no bisection or any
real effort).

This is after the system has come up, so I assume it's when
networkmanager or whatever actually is setting that wireless thing up.

Again: I actually don't *use* the WiFi on this machine - but from a
quick check it does seem to work. So this is just an annoyance and a
sign that somebody didn't do all the details right (possibly triggered
by odd user land behavior, of course, but still...)

                Linus

Re: [PATCH v4] wifi: iwlwifi: fix system commands group ordering

[Emmanuel Grumbach]

On Sun, Nov 12, 2023 at 9:39 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Sun, 12 Nov 2023 at 10:54, Kalle Valo <kvalo@kernel.org> wrote:
> >
> > Linus, I suspect it will take a while before we make a new wireless pull
> > request (there's currently just one commit in the wireless tree). So if
> > you want to take this directly to your tree please go ahead, otherwise
> > you get it via the normal route in a week or two. Just let us know what
> > you prefer.
> >
> > Acked-by: Kalle Valo <kvalo@kernel.org>
>
> Ok, I just tested it in my private tree, and it fixes the WARN_ON() as
> expected, so I'll apply it for real.

Good to know - thanks.

>
> However, now that I don't have that big warning in there, I do note
> another iwlwifi issue that is new to this meger window:
>
>   debugfs: Directory 'iwlmvm' with parent 'netdev:wlo2' already present!
>   iwlwifi 0000:45:00.0: Failed to create debugfs directory under netdev:wlo2
>   debugfs: Directory 'iwlmvm' with parent 'netdev:wlo2' already present!
>   iwlwifi 0000:45:00.0: Failed to create debugfs directory under netdev:wlo2
>
> and looking at my system logs, this is new.
>
> It looks like it is probably due to
>
>   c36235acb34f ("wifi: iwlwifi: mvm: rework debugfs handling")
>   e9dd25550770 ("wifi: iwlwifi: mvm: add a per-link debugfs")
>
> but that's just from looking at the patches (ie no bisection or any
> real effort).

Don't bother, we'll take a look.

>
> This is after the system has come up, so I assume it's when
> networkmanager or whatever actually is setting that wireless thing up.

Most likely the network-manager brings the device up several times or something
like that, but let's not blame them before we check what happens internally.

>
> Again: I actually don't *use* the WiFi on this machine - but from a
> quick check it does seem to work. So this is just an annoyance and a
> sign that somebody didn't do all the details right (possibly triggered
> by odd user land behavior, of course, but still...)
>

Oh, we know the rules :)

>                 Linus

Re: [PATCH v4] wifi: iwlwifi: fix system commands group ordering

[Emmanuel Grumbach]

>
> >
> > However, now that I don't have that big warning in there, I do note
> > another iwlwifi issue that is new to this meger window:
> >
> >   debugfs: Directory 'iwlmvm' with parent 'netdev:wlo2' already present!
> >   iwlwifi 0000:45:00.0: Failed to create debugfs directory under netdev:wlo2
> >   debugfs: Directory 'iwlmvm' with parent 'netdev:wlo2' already present!
> >   iwlwifi 0000:45:00.0: Failed to create debugfs directory under netdev:wlo2
> >
> > and looking at my system logs, this is new.
> >
> > It looks like it is probably due to
> >
> >   c36235acb34f ("wifi: iwlwifi: mvm: rework debugfs handling")
> >   e9dd25550770 ("wifi: iwlwifi: mvm: add a per-link debugfs")
> >
> > but that's just from looking at the patches (ie no bisection or any
> > real effort).
>
> Don't bother, we'll take a look.

Miri sent me fixes that we have in our internal tree that have not yet
been published.
The fixes were made during the merge window.
I can send them here, but I think I'd prefer to let those patches go
through the regular
pipes. Also, I'd prefer the people who usually interact with the trees
handle that. Gregory
was OOO today and that's why I chimed in. Miri is still ramping up.
If it's really urgent, I can send the fixes though.

>
> >
> > This is after the system has come up, so I assume it's when
> > networkmanager or whatever actually is setting that wireless thing up.
>
> Most likely the network-manager brings the device up several times or something
> like that, but let's not blame them before we check what happens internally.
>
> >
> > Again: I actually don't *use* the WiFi on this machine - but from a
> > quick check it does seem to work. So this is just an annoyance and a
> > sign that somebody didn't do all the details right (possibly triggered
> > by odd user land behavior, of course, but still...)
> >
>
> Oh, we know the rules :)
>
> >                 Linus

Re: [PATCH v4] wifi: iwlwifi: fix system commands group ordering

[Ben Greear]

On 11/12/23 11:53 AM, Emmanuel Grumbach wrote:
> On Sun, Nov 12, 2023 at 9:39 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> On Sun, 12 Nov 2023 at 10:54, Kalle Valo <kvalo@kernel.org> wrote:
>>>
>>> Linus, I suspect it will take a while before we make a new wireless pull
>>> request (there's currently just one commit in the wireless tree). So if
>>> you want to take this directly to your tree please go ahead, otherwise
>>> you get it via the normal route in a week or two. Just let us know what
>>> you prefer.
>>>
>>> Acked-by: Kalle Valo <kvalo@kernel.org>
>>
>> Ok, I just tested it in my private tree, and it fixes the WARN_ON() as
>> expected, so I'll apply it for real.
> 
> Good to know - thanks.
> 
>>
>> However, now that I don't have that big warning in there, I do note
>> another iwlwifi issue that is new to this meger window:
>>
>>    debugfs: Directory 'iwlmvm' with parent 'netdev:wlo2' already present!
>>    iwlwifi 0000:45:00.0: Failed to create debugfs directory under netdev:wlo2
>>    debugfs: Directory 'iwlmvm' with parent 'netdev:wlo2' already present!
>>    iwlwifi 0000:45:00.0: Failed to create debugfs directory under netdev:wlo2
>>
>> and looking at my system logs, this is new.
>>
>> It looks like it is probably due to
>>
>>    c36235acb34f ("wifi: iwlwifi: mvm: rework debugfs handling")
>>    e9dd25550770 ("wifi: iwlwifi: mvm: add a per-link debugfs")
>>
>> but that's just from looking at the patches (ie no bisection or any
>> real effort).
> 
> Don't bother, we'll take a look.
> 
>>
>> This is after the system has come up, so I assume it's when
>> networkmanager or whatever actually is setting that wireless thing up.
> 
> Most likely the network-manager brings the device up several times or something
> like that, but let's not blame them before we check what happens internally.

The debugfs warnings are seen all of my systems, no network manager for me, but
I am bringing stations up and down a lot.

Thanks,
Ben



-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com