[PATCH v3] wifi: iwlwifi: pcie: fix the order of scanning iwl_dev_info_table

[PATCH v3] wifi: iwlwifi: pcie: fix the order of scanning iwl_dev_info_table

[Aiden Leong]

Fix a bug introduced by:
commit 32ed101aa140 ("iwlwifi: convert all Qu with Jf devices to the new
 config table"), so now we pick the FIRST matching config.

Signed-off-by: Aiden Leong <aiden.leong@aibsd.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 99768d6a6032..05764eef15a7 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
@@ -1456,7 +1456,7 @@ iwl_pci_find_dev_info(u16 device, u16 subsystem_device,
 	if (!num_devices)
 		return NULL;
 
-	for (i = num_devices - 1; i >= 0; i--) {
+	for (i = 0; i < num_devices; i++) {
 		const struct iwl_dev_info *dev_info = &iwl_dev_info_table[i];
 
 		if (dev_info->device != (u16)IWL_CFG_ANY &&
-- 
2.39.0

Re: [PATCH v3] wifi: iwlwifi: pcie: fix the order of scanning iwl_dev_info_table

[Greenman, Gregory]

On Fri, 2023-01-20 at 01:56 +0800, Aiden Leong wrote:
> Fix a bug introduced by:
> commit 32ed101aa140 ("iwlwifi: convert all Qu with Jf devices to the new
>  config table"), so now we pick the FIRST matching config.
> 
> Signed-off-by: Aiden Leong <aiden.leong@aibsd.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 99768d6a6032..05764eef15a7 100644
> --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> @@ -1456,7 +1456,7 @@ iwl_pci_find_dev_info(u16 device, u16 subsystem_device,
>         if (!num_devices)
>                 return NULL;
>  
> -       for (i = num_devices - 1; i >= 0; i--) {
> +       for (i = 0; i < num_devices; i++) {
>                 const struct iwl_dev_info *dev_info = &iwl_dev_info_table[i];
>  
>                 if (dev_info->device != (u16)IWL_CFG_ANY &&

It failed or internal testing, so it's more complicated. To traverse this table
from the beginning to the end requires some changes to the table itself and the 
"goto" wasn't omitted by a mistake, but for a reason...
For the device that you have (device id 0x4DF0, sub-device id 0x0244, right?)
is it enough to have the first fix (disable tx_with_siso_diversity)?

Re: [PATCH v3] wifi: iwlwifi: pcie: fix the order of scanning iwl_dev_info_table

[Aiden Leong]

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

On Wednesday, February 8, 2023 1:44:39 AM CST Greenman, Gregory wrote:
> On Fri, 2023-01-20 at 01:56 +0800, Aiden Leong wrote:
> 
> > Fix a bug introduced by:
> > commit 32ed101aa140 ("iwlwifi: convert all Qu with Jf devices to the new
> >  config table"), so now we pick the FIRST matching config.
> > 
> > Signed-off-by: Aiden Leong <aiden.leong@aibsd.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
> > 99768d6a6032..05764eef15a7 100644
> > --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> > +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> > @@ -1456,7 +1456,7 @@ iwl_pci_find_dev_info(u16 device, u16
> > subsystem_device,
 if (!num_devices)
> >                 return NULL;
> >  
> > -       for (i = num_devices - 1; i >= 0; i--) {
> > +       for (i = 0; i < num_devices; i++) {
> >                 const struct iwl_dev_info *dev_info =
> > &iwl_dev_info_table[i];
 
> >                 if (dev_info->device != (u16)IWL_CFG_ANY &&
> 
> 
> It failed or internal testing, so it's more complicated. To traverse this
> table
 from the beginning to the end requires some changes to the table
> itself and the "goto" wasn't omitted by a mistake, but for a reason...
> For the device that you have (device id 0x4DF0, sub-device id 0x0244,
> right?)
 is it enough to have the first fix (disable
> tx_with_siso_diversity)?

Hi Gregory,
That's exactly why I put a warning in previous emails.
My opinion will be a little different than yours in this situation.
1. We SHOULD traverse this table from top to bottom to keep our source tree as 
clean as possible.
2. One simple option is to reverse every config items in this table so the 
logic keep the same.
3. Your team(I assume Luca Coelho is your colleague) may need to provide 
further explaination about the `goto` line, since each change in kernel should 
have a reason.
4. 0x4DF0, 0x0244 is correct. The question is: Will Intel release products 
with same pid+subID but differenct STEP/RF_TYPE/RF_ID etc? If so, pid+subID 
won't be enough.

To sum up, there will be three patches:
1. This patch still fixes the BUG introduced by the `goto` change.
2. Patch 2 should be [PATCH 1/2] in previous email.
3. Patch 3 reverses every items in this table. Your team can fine-tune the 
order of each items. I won't submit this patch.

If you like my ideas, please merge patch1&2 along with another ident fix patch.

BTW, it has been a month since the first email. I'd appreciate if you reply 
soon.

Cheers,
Aiden

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3] wifi: iwlwifi: pcie: fix the order of scanning iwl_dev_info_table

[Aiden Leong]

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

On Wednesday, February 8, 2023 5:14:50 AM CST Aiden Leong wrote:
> On Wednesday, February 8, 2023 1:44:39 AM CST Greenman, Gregory wrote:
> > On Fri, 2023-01-20 at 01:56 +0800, Aiden Leong wrote:
> > > Fix a bug introduced by:
> > > commit 32ed101aa140 ("iwlwifi: convert all Qu with Jf devices to the new
> > > 
> > >  config table"), so now we pick the FIRST matching config.
> > > 
> > > Signed-off-by: Aiden Leong <aiden.leong@aibsd.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
> 
> > > 99768d6a6032..05764eef15a7 100644
> > > --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> > > +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> > > @@ -1456,7 +1456,7 @@ iwl_pci_find_dev_info(u16 device, u16
> > > subsystem_device,
> 
>  if (!num_devices)
> 
> > >                 return NULL;
> > > 
> > > -       for (i = num_devices - 1; i >= 0; i--) {
> > > +       for (i = 0; i < num_devices; i++) {
> > > 
> > >                 const struct iwl_dev_info *dev_info =
> > > 
> > > &iwl_dev_info_table[i];
> > > 
> > >                 if (dev_info->device != (u16)IWL_CFG_ANY &&
> > 
> > It failed or internal testing, so it's more complicated. To traverse this
> > table
> 
>  from the beginning to the end requires some changes to the table
> 
> > itself and the "goto" wasn't omitted by a mistake, but for a reason...
> > For the device that you have (device id 0x4DF0, sub-device id 0x0244,
> > right?)
> 
>  is it enough to have the first fix (disable
> 
> > tx_with_siso_diversity)?
> 
> Hi Gregory,
> That's exactly why I put a warning in previous emails.
> My opinion will be a little different than yours in this situation.
> 1. We SHOULD traverse this table from top to bottom to keep our source tree
> as clean as possible.
> 2. One simple option is to reverse every config items in this table so the
> logic keep the same.
> 3. Your team(I assume Luca Coelho is your colleague) may need to provide
> further explaination about the `goto` line, since each change in kernel
> should have a reason.
> 4. 0x4DF0, 0x0244 is correct. The question is: Will Intel release products
> with same pid+subID but differenct STEP/RF_TYPE/RF_ID etc? If so, pid+subID
> won't be enough.
> 
> To sum up, there will be three patches:
> 1. This patch still fixes the BUG introduced by the `goto` change.
> 2. Patch 2 should be [PATCH 1/2] in previous email.
> 3. Patch 3 reverses every items in this table. Your team can fine-tune the
> order of each items. I won't submit this patch.
> 
> If you like my ideas, please merge patch1&2 along with another ident fix
> patch.
> 
> BTW, it has been a month since the first email. I'd appreciate if you reply
> soon.
> 
> Cheers,
> Aiden

Hi Gregory,

PING

You should let us know if you are not actively maintaining the community part 
of the driver. If you are only working on the close source firmware, we should 
have someone else do the open source job.
We should not waste our life for months on such a small patch.

Have a nice day,
Aiden

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3] wifi: iwlwifi: pcie: fix the order of scanning iwl_dev_info_table

[Greenman, Gregory]

On Fri, 2023-03-10 at 13:14 +0800, Aiden Leong wrote:
> On Wednesday, February 8, 2023 5:14:50 AM CST Aiden Leong wrote:
> > On Wednesday, February 8, 2023 1:44:39 AM CST Greenman, Gregory wrote:
> > > On Fri, 2023-01-20 at 01:56 +0800, Aiden Leong wrote:
> > > > Fix a bug introduced by:
> > > > commit 32ed101aa140 ("iwlwifi: convert all Qu with Jf devices to the new
> > > > 
> > > >  config table"), so now we pick the FIRST matching config.
> > > > 
> > > > Signed-off-by: Aiden Leong <aiden.leong@aibsd.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
> > 
> > > > 99768d6a6032..05764eef15a7 100644
> > > > --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> > > > +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> > > > @@ -1456,7 +1456,7 @@ iwl_pci_find_dev_info(u16 device, u16
> > > > subsystem_device,
> > 
> >  if (!num_devices)
> > 
> > > >                 return NULL;
> > > > 
> > > > -       for (i = num_devices - 1; i >= 0; i--) {
> > > > +       for (i = 0; i < num_devices; i++) {
> > > > 
> > > >                 const struct iwl_dev_info *dev_info =
> > > > 
> > > > &iwl_dev_info_table[i];
> > > > 
> > > >                 if (dev_info->device != (u16)IWL_CFG_ANY &&
> > > 
> > > It failed or internal testing, so it's more complicated. To traverse this
> > > table
> > 
> >  from the beginning to the end requires some changes to the table
> > 
> > > itself and the "goto" wasn't omitted by a mistake, but for a reason...
> > > For the device that you have (device id 0x4DF0, sub-device id 0x0244,
> > > right?)
> > 
> >  is it enough to have the first fix (disable
> > 
> > > tx_with_siso_diversity)?
> > 
> > Hi Gregory,
> > That's exactly why I put a warning in previous emails.
> > My opinion will be a little different than yours in this situation.
> > 1. We SHOULD traverse this table from top to bottom to keep our source tree
> > as clean as possible.
> > 2. One simple option is to reverse every config items in this table so the
> > logic keep the same.
> > 3. Your team(I assume Luca Coelho is your colleague) may need to provide
> > further explaination about the `goto` line, since each change in kernel
> > should have a reason.
> > 4. 0x4DF0, 0x0244 is correct. The question is: Will Intel release products
> > with same pid+subID but differenct STEP/RF_TYPE/RF_ID etc? If so, pid+subID
> > won't be enough.
> > 
> > To sum up, there will be three patches:
> > 1. This patch still fixes the BUG introduced by the `goto` change.
> > 2. Patch 2 should be [PATCH 1/2] in previous email.
> > 3. Patch 3 reverses every items in this table. Your team can fine-tune the
> > order of each items. I won't submit this patch.
> > 
> > If you like my ideas, please merge patch1&2 along with another ident fix
> > patch.
> > 
> > BTW, it has been a month since the first email. I'd appreciate if you reply
> > soon.
> > 
> > Cheers,
> > Aiden
> 
> Hi Gregory,
> 
> PING
> 
> You should let us know if you are not actively maintaining the community part 
> of the driver. If you are only working on the close source firmware, we should 
> have someone else do the open source job.
> We should not waste our life for months on such a small patch.
> 
> Have a nice day,
> Aiden

Hi,

You’re coming across as rather accusatory and demanding. I’d appreciate if you could 
tone it down a bit. Regarding the table order, we’ve made a decision in the code way
back to walk the table from the back – that may not match your personal expectation of 
“clean”, but that’s really your problem, not ours.
Also, we cannot comment on future product releases in general.

If you’re willing to work with us to fix the issue you’re encountering within the
framework of how the driver is written now, I can give you a patch with more logs
to understand why your second patch doesn't fix the issue.

Regards,
Gregory

Re: [PATCH v3] wifi: iwlwifi: pcie: fix the order of scanning iwl_dev_info_table

[Aiden Leong]

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

On Sunday, March 12, 2023 5:47:07 PM CST Greenman, Gregory wrote:
> On Fri, 2023-03-10 at 13:14 +0800, Aiden Leong wrote:
> 
> > On Wednesday, February 8, 2023 5:14:50 AM CST Aiden Leong wrote:
> > 
> > > On Wednesday, February 8, 2023 1:44:39 AM CST Greenman, Gregory wrote:
> > > 
> > > > On Fri, 2023-01-20 at 01:56 +0800, Aiden Leong wrote:
> > > > 
> > > > > Fix a bug introduced by:
> > > > > commit 32ed101aa140 ("iwlwifi: convert all Qu with Jf devices to the
> > > > > new
> > > > > 
> > > > >  config table"), so now we pick the FIRST matching config.
> > > > > 
> > > > > Signed-off-by: Aiden Leong <aiden.leong@aibsd.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
> > > 
> > > 
> > > > > 99768d6a6032..05764eef15a7 100644
> > > > > --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> > > > > +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> > > > > @@ -1456,7 +1456,7 @@ iwl_pci_find_dev_info(u16 device, u16
> > > > > subsystem_device,
> > > 
> > > 
> > >  if (!num_devices)
> > > 
> > > 
> > > > >                 return NULL;
> > > > > 
> > > > > -       for (i = num_devices - 1; i >= 0; i--) {
> > > > > +       for (i = 0; i < num_devices; i++) {
> > > > > 
> > > > >                 const struct iwl_dev_info *dev_info =
> > > > > 
> > > > > &iwl_dev_info_table[i];
> > > > > 
> > > > >                 if (dev_info->device != (u16)IWL_CFG_ANY &&
> > > > 
> > > > 
> > > > It failed or internal testing, so it's more complicated. To traverse
> > > > this
> > > > table
> > > 
> > > 
> > >  from the beginning to the end requires some changes to the table
> > > 
> > > 
> > > > itself and the "goto" wasn't omitted by a mistake, but for a
> > > > reason...
> > > > For the device that you have (device id 0x4DF0, sub-device id 0x0244,
> > > > right?)
> > > 
> > > 
> > >  is it enough to have the first fix (disable
> > > 
> > > 
> > > > tx_with_siso_diversity)?
> > > 
> > > 
> > > Hi Gregory,
> > > That's exactly why I put a warning in previous emails.
> > > My opinion will be a little different than yours in this situation.
> > > 1. We SHOULD traverse this table from top to bottom to keep our source
> > > tree
 as clean as possible.
> > > 2. One simple option is to reverse every config items in this table so
> > > the
> > > logic keep the same.
> > > 3. Your team(I assume Luca Coelho is your colleague) may need to
> > > provide
> > > further explaination about the `goto` line, since each change in kernel
> > > should have a reason.
> > > 4. 0x4DF0, 0x0244 is correct. The question is: Will Intel release
> > > products
> > > with same pid+subID but differenct STEP/RF_TYPE/RF_ID etc? If so,
> > > pid+subID
 won't be enough.
> > > 
> > > To sum up, there will be three patches:
> > > 1. This patch still fixes the BUG introduced by the `goto` change.
> > > 2. Patch 2 should be [PATCH 1/2] in previous email.
> > > 3. Patch 3 reverses every items in this table. Your team can fine-tune
> > > the
> > > order of each items. I won't submit this patch.
> > > 
> > > If you like my ideas, please merge patch1&2 along with another ident
> > > fix
> > > patch.
> > > 
> > > BTW, it has been a month since the first email. I'd appreciate if you
> > > reply
 soon.
> > > 
> > > Cheers,
> > > Aiden
> > 
> > 
> > Hi Gregory,
> > 
> > PING
> > 
> > You should let us know if you are not actively maintaining the community
> > part 
 of the driver. If you are only working on the close source
> > firmware, we should have someone else do the open source job.
> > We should not waste our life for months on such a small patch.
> > 
> > Have a nice day,
> > Aiden
> 
> 
> Hi,
> 
> You’re coming across as rather accusatory and demanding. I’d appreciate if
> you could 
 tone it down a bit. Regarding the table order, we’ve made a
> decision in the code way back to walk the table from the back – that may
> not match your personal expectation of “clean”, but that’s really your
> problem, not ours.
> Also, we cannot comment on future product releases in general.
> 
> If you’re willing to work with us to fix the issue you’re encountering
> within the
 framework of how the driver is written now, I can give you a
> patch with more logs to understand why your second patch doesn't fix the
> issue.
> 
> Regards,
> Gregory

So I'm the bad guy now?
Fine.
That's funny!

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3] wifi: iwlwifi: pcie: fix the order of scanning iwl_dev_info_table

[Aiden Leong]

It has been a year since the first time I submitted a patch on AX101.

We didn't have a nice conversation due to many reasons and 
misunderstandings.

Since the device is working well now, I believe this might be a 
relatively proper timing to

explain a bit more about the whole story.

Previous emails are important any more, so I just write on top.

This is absolutely not any sort of complaint but just a simple and 
hopefully friendly additional information.


Let's get start.

The story begins when we traverse a configuration look-up table of a 
long list of devices.

E.g.

     if we have devices [a1, a2, a3] of series A, [b1, b2, b3] of series B.

     we may have future devices a4, b4, so we have fall back config for 
series A and B

Then the table should be like this: [a1, a2, a3, A(fallback), b1, b2, 
b3, B(fallback)].

The original correct version was:

     for (i = 0; i < ARRAY_SIZE(iwl_dev_info_table); i++) {
         const struct iwl_dev_info *dev_info = &iwl_dev_info_table[i];
         if ((dev_info->device == (u16)IWL_CFG_ANY ||
              ...
             goto found;
         }
     }

It's a simple top-down search and break with `goto found`. So far so good.


Then there was not 1 but 3 refactorings. The former one introduced a bug 
and made a chaos

in the config table as well.

commit 32ed101aa140 ("iwlwifi: convert all Qu with Jf devices to the new 
 >  config table")

Refactor version 1 wrong code:

     for (i = 0; i < ARRAY_SIZE(iwl_dev_info_table); i++) {
         const struct iwl_dev_info *dev_info = &iwl_dev_info_table[i];
         if ((dev_info->device == (u16)IWL_CFG_ANY ||
              ...
         }
     }

It removed `goto found` with no reason, so the last match will always be 
the A(fallback) or B(fallback).

This patch was commited on Mar 27, 2020

Again, the table was NEVER reversed.


Then the second refactor was commited on Oct 28, 2021

commit 0a1f96d571c8 (iwlwifi: pcie: refactor dev_info lookup)

Refactor version 2 code:

     for (i = 0; i < ARRAY_SIZE(iwl_dev_info_table); i++) {
         const struct iwl_dev_info *dev_info = &iwl_dev_info_table[i];
         if (dev_info->device != (u16)IWL_CFG_ANY &&
             dev_info->device != device)
             continue;
         ...
         ret = dev_info;
     }

It was a good refactoring but he didn't notice the bug introduced by the 
former patch.


Then the third refactor was commited on Nov 23, 2021

commit fe785f56ad58 (iwlwifi: pcie: fix constant-conversion warning)

Refactor version 3 code:

     for (i = num_devices - 1; i >= 0; i--) {
         const struct iwl_dev_info *dev_info = &iwl_dev_info_table[i];

         if (dev_info->device != (u16)IWL_CFG_ANY &&
             dev_info->device != device)
             continue;
         ...

         return dev_info;
     }

     return NULL;

This time somebody found the weirdness: why do we traverse a table 
top-down but logically down-top?

Unfortunately again, he didn't find the bug buried deep in the git 
history years ago.


It's now Feb 3, 2024. 4 years later after the first wrong patch.

The config table `iwl_dev_info_table` is still mixed with fallback-last 
and fallback-first entries.

[a1, a2, a3, A, b1, b2, b3, B, C, c1, c2, c3]

Some devices works. Some devices don't work. Some issues are reported. 
Some are not.

That's why my patch was not even possible to pass Intel's mysterious 
internal testing process.


I'm not opinionated on top-down or down-top traverse, the 
big-little-endian story,

but we need to write logically correct code.


Aiden Leong



On 2023/3/12 17:47, Greenman, Gregory wrote:

> On Fri, 2023-03-10 at 13:14 +0800, Aiden Leong wrote:
>> On Wednesday, February 8, 2023 5:14:50 AM CST Aiden Leong wrote:
>>> On Wednesday, February 8, 2023 1:44:39 AM CST Greenman, Gregory wrote:
>>>> On Fri, 2023-01-20 at 01:56 +0800, Aiden Leong wrote:
>>>>> Fix a bug introduced by:
>>>>> commit 32ed101aa140 ("iwlwifi: convert all Qu with Jf devices to the new
>>>>>
>>>>>   config table"), so now we pick the FIRST matching config.
>>>>>
>>>>> Signed-off-by: Aiden Leong <aiden.leong@aibsd.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
>>>
>>>>> 99768d6a6032..05764eef15a7 100644
>>>>> --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
>>>>> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
>>>>> @@ -1456,7 +1456,7 @@ iwl_pci_find_dev_info(u16 device, u16
>>>>> subsystem_device,
>>>   if (!num_devices)
>>>
>>>>>                  return NULL;
>>>>>
>>>>> -       for (i = num_devices - 1; i >= 0; i--) {
>>>>> +       for (i = 0; i < num_devices; i++) {
>>>>>
>>>>>                  const struct iwl_dev_info *dev_info =
>>>>>
>>>>> &iwl_dev_info_table[i];
>>>>>
>>>>>                  if (dev_info->device != (u16)IWL_CFG_ANY &&
>>>> It failed or internal testing, so it's more complicated. To traverse this
>>>> table
>>>   from the beginning to the end requires some changes to the table
>>>
>>>> itself and the "goto" wasn't omitted by a mistake, but for a reason...
>>>> For the device that you have (device id 0x4DF0, sub-device id 0x0244,
>>>> right?)
>>>   is it enough to have the first fix (disable
>>>
>>>> tx_with_siso_diversity)?
>>> Hi Gregory,
>>> That's exactly why I put a warning in previous emails.
>>> My opinion will be a little different than yours in this situation.
>>> 1. We SHOULD traverse this table from top to bottom to keep our source tree
>>> as clean as possible.
>>> 2. One simple option is to reverse every config items in this table so the
>>> logic keep the same.
>>> 3. Your team(I assume Luca Coelho is your colleague) may need to provide
>>> further explaination about the `goto` line, since each change in kernel
>>> should have a reason.
>>> 4. 0x4DF0, 0x0244 is correct. The question is: Will Intel release products
>>> with same pid+subID but differenct STEP/RF_TYPE/RF_ID etc? If so, pid+subID
>>> won't be enough.
>>>
>>> To sum up, there will be three patches:
>>> 1. This patch still fixes the BUG introduced by the `goto` change.
>>> 2. Patch 2 should be [PATCH 1/2] in previous email.
>>> 3. Patch 3 reverses every items in this table. Your team can fine-tune the
>>> order of each items. I won't submit this patch.
>>>
>>> If you like my ideas, please merge patch1&2 along with another ident fix
>>> patch.
>>>
>>> BTW, it has been a month since the first email. I'd appreciate if you reply
>>> soon.
>>>
>>> Cheers,
>>> Aiden
>> Hi Gregory,
>>
>> PING
>>
>> You should let us know if you are not actively maintaining the community part
>> of the driver. If you are only working on the close source firmware, we should
>> have someone else do the open source job.
>> We should not waste our life for months on such a small patch.
>>
>> Have a nice day,
>> Aiden
> Hi,
>
> You’re coming across as rather accusatory and demanding. I’d appreciate if you could
> tone it down a bit. Regarding the table order, we’ve made a decision in the code way
> back to walk the table from the back – that may not match your personal expectation of
> “clean”, but that’s really your problem, not ours.
> Also, we cannot comment on future product releases in general.
>
> If you’re willing to work with us to fix the issue you’re encountering within the
> framework of how the driver is written now, I can give you a patch with more logs
> to understand why your second patch doesn't fix the issue.
>
> Regards,
> Gregory