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@xxxxxxxxx>
> > > > ---
> > > >
> > > > 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
