Search Linux Wireless

Re: [PATCH 2/2] iwlwifi: pcie: add support for AX101NGW

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023/1/5 14:21, Greenman, Gregory wrote:
On Mon, 2023-01-02 at 22:00 +0800, Aiden Leong wrote:
1. Please have a look at the second commit:
https://lore.kernel.org/linux-wireless/iwlwifi.20200323151304.ec4f463bde60.I14e9146a99621ff11ce50bc746a4b88af508fee0@changeid/

Find the `goto found;` statement.

The logic was to find the FIRST match then break with `goto found`. The
refactor code removed the `goto` statement which is incorrect.


2. Let's go back to the first commit:
https://lore.kernel.org/linux-wireless/iwlwifi.20211024165252.abd85e1391cb.I7681fe90735044cc1c59f120e8591b7ac125535d@changeid/

  > We don't want to change the semantics ("most generic entry must come
first")

That `semantics` was mislead by the previous commit.


On 2023/1/2 21:35, Greenman, Gregory wrote:
On Mon, 2023-01-02 at 10:40 +0800, Aiden Leong wrote:
Revert:
commit 3f7320428fa4 ("iwlwifi: pcie: simplify iwl_pci_find_dev_info()")

A bug was introduced by:
commit 32ed101aa140 ("iwlwifi: convert all Qu with Jf devices to the new
config table"),
where a goto statement was removed.
Not sure I undestand what problem reversing the "for" loop solves.

Signed-off-by: Aiden Leong <aiden.leong@xxxxxxxxx>
---
Notice:
Please run further tests before merging. I'm NOT familiar with device
drivers.
---
   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 a46df1320372..5d74adbd49cf 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
@@ -1461,7 +1461,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 &&
So, the fix itself is correct (thanks for fixing it!).
The commit message should be a bit changed, the "goto" removal is not relevant
in the current code since now this for loop is in a separate function. Also, all
wifi commits should have prefix "wifi: " to separate them from the rest of netdev.
Tell me if you want to resend this commit or I can fix it by myself.

I'm a kernel newbie. Thank you for your patience.

I'd like to resend the commit so I can learn more about how to do it right.

The title of the patchset will be "wifi: iwlwifi: pcie: add support for AX101NGW",

and the commit message will be "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."

Would it be all right?


Have a nice day!

Aiden Leong




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux