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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a687cad5-6cc9-3db0-c8fa-a959dd70347a@linux.intel.com>
Date:   Mon, 20 Nov 2023 10:54:45 +0200 (EET)
From:   Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
cc:     "John W. Linville" <linville@...driver.com>,
        Kalle Valo <kvalo@...nel.org>,
        Larry Finger <Larry.Finger@...inger.net>,
        linux-wireless@...r.kernel.org, Ping-Ke Shih <pkshih@...ltek.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/7] wifi: rtlwifi: Convert to use PCIe capability
 accessors

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.