Re: [PATCH 2/7] wifi: rtlwifi: Convert to use PCIe capability accessors

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

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

On Fri, 17 Nov 2023, Bjorn Helgaas wrote:

> On Fri, Nov 17, 2023 at 11:44:20AM +0200, Ilpo Järvinen wrote:
> > The rtlwifi driver accesses PCIe capabilities through custom config
> > offsets.
> > 
> > Convert the accesses to use the normal PCIe capability accessors.
> > pcibridge_pciehdr_offset in the struct mp_adapter becomes unused after
> > the conversion and can be removed.
> 
> More good stuff.  I guess patch [1/7] was specifically for the RMW
> things, and this one is for the rest?

Yes, I wanted to separate them because of the Fixes tag.

> > @@ -219,7 +220,7 @@ static void rtl_pci_disable_aspm(struct ieee80211_hw *hw)
> >  	}
> >  
> >  	/*for promising device will in L0 state after an I/O. */
> > -	pci_read_config_byte(rtlpci->pdev, 0x80, &tmp_u1b);
> > +	pcie_capability_read_word(rtlpci->pdev, PCI_EXP_LNKCTL, &tmp_u1b);
> >  
> >  	/*Set corresponding value. */
> >  	aspmlevel |= BIT(0) | BIT(1);
> 
> I guess this is PCI_EXP_LNKCTL_ASPM_L0S | PCI_EXP_LNKCTL_ASPM_L1?

I'll change it too. There was just so much to cleanup I started to miss 
things even this obvious :-(.

Also, I was not at all sure if that read from LNKCTL is really trying to 
achieve. The comment sounds like it's trying to ensure the dev is in L0
but why it cares? These drivers do so odd things :-).

> There's also a similar u_pcibridge_aspmsetting mask in
> rtl_pci_enable_aspm().

Yes, but I'll put that into 1/7 since it's related to the change made 
there.

> And some scary looking stuff in rtl_pci_get_amd_l1_patch().
> And platform_enable_dma64().  No clue what either of those does.

Those elude me as well.


-- 
 i.