[2/7] wifi: rtlwifi: Convert to use PCIe capability accessors
Commit Message
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.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/net/wireless/realtek/rtlwifi/pci.c | 30 ++++++++--------------
drivers/net/wireless/realtek/rtlwifi/pci.h | 1 -
2 files changed, 11 insertions(+), 20 deletions(-)
Comments
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?
> @@ -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?
There's also a similar u_pcibridge_aspmsetting mask in
rtl_pci_enable_aspm().
And some scary looking stuff in rtl_pci_get_amd_l1_patch(). And
platform_enable_dma64(). No clue what either of those does.
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.
@@ -64,7 +64,7 @@ static void _rtl_pci_update_default_setting(struct ieee80211_hw *hw)
struct rtl_ps_ctl *ppsc = rtl_psc(rtl_priv(hw));
struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
u8 pcibridge_vendor = pcipriv->ndis_adapter.pcibridge_vendor;
- u8 init_aspm;
+ u16 init_aspm;
ppsc->reg_rfps_level = 0;
ppsc->support_aspm = false;
@@ -151,9 +151,10 @@ static void _rtl_pci_update_default_setting(struct ieee80211_hw *hw)
/* toshiba aspm issue, toshiba will set aspm selfly
* so we should not set aspm in driver
*/
- pci_read_config_byte(rtlpci->pdev, 0x80, &init_aspm);
+ pcie_capability_read_word(rtlpci->pdev, PCI_EXP_LNKCTL, &init_aspm);
if (rtlpriv->rtlhal.hw_type == HARDWARE_TYPE_RTL8192SE &&
- init_aspm == 0x43)
+ ((u8)init_aspm) == (PCI_EXP_LNKCTL_ASPM_L0S |
+ PCI_EXP_LNKCTL_ASPM_L1 | PCI_EXP_LNKCTL_CCC))
ppsc->support_aspm = false;
}
@@ -201,7 +202,7 @@ static void rtl_pci_disable_aspm(struct ieee80211_hw *hw)
/*Retrieve original configuration settings. */
u8 linkctrl_reg = pcipriv->ndis_adapter.linkctrl_reg;
u16 aspmlevel = 0;
- u8 tmp_u1b = 0;
+ u16 tmp_u1b = 0;
if (!ppsc->support_aspm)
return;
@@ -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);
@@ -363,14 +364,9 @@ static void rtl_pci_get_linkcontrol_field(struct ieee80211_hw *hw)
{
struct rtl_pci_priv *pcipriv = rtl_pcipriv(hw);
struct rtl_pci *rtlpci = rtl_pcidev(pcipriv);
- u8 capabilityoffset = pcipriv->ndis_adapter.pcibridge_pciehdr_offset;
- u8 linkctrl_reg;
- u8 num4bbytes;
-
- num4bbytes = (capabilityoffset + 0x10) / 4;
+ u16 linkctrl_reg;
- /*Read Link Control Register */
- pci_read_config_byte(rtlpci->pdev, (num4bbytes << 2), &linkctrl_reg);
+ pcie_capability_read_word(rtlpci->pdev, PCI_EXP_LNKCTL, &linkctrl_reg);
pcipriv->ndis_adapter.pcibridge_linkctrlreg = linkctrl_reg;
}
@@ -391,9 +387,8 @@ static void rtl_pci_parse_configuration(struct pci_dev *pdev,
rtl_dbg(rtlpriv, COMP_INIT, DBG_TRACE, "Link Control Register =%x\n",
pcipriv->ndis_adapter.linkctrl_reg);
- pci_read_config_byte(pdev, 0x98, &tmp);
- tmp |= BIT(4);
- pci_write_config_byte(pdev, 0x98, tmp);
+ pcie_capability_set_word(pdev, PCI_EXP_DEVCTL2,
+ PCI_EXP_DEVCTL2_COMP_TMOUT_DIS);
tmp = 0x17;
pci_write_config_byte(pdev, 0x70f, tmp);
@@ -2029,8 +2024,6 @@ static bool _rtl_pci_find_adapter(struct pci_dev *pdev,
PCI_SLOT(bridge_pdev->devfn);
pcipriv->ndis_adapter.pcibridge_funcnum =
PCI_FUNC(bridge_pdev->devfn);
- pcipriv->ndis_adapter.pcibridge_pciehdr_offset =
- pci_pcie_cap(bridge_pdev);
rtl_pci_get_linkcontrol_field(hw);
@@ -2049,12 +2042,11 @@ static bool _rtl_pci_find_adapter(struct pci_dev *pdev,
pdev->vendor, pcipriv->ndis_adapter.linkctrl_reg);
rtl_dbg(rtlpriv, COMP_INIT, DBG_DMESG,
- "pci_bridge busnumber:devnumber:funcnumber:vendor:pcie_cap:link_ctl_reg:amd %d:%d:%d:%x:%x:%x:%x\n",
+ "pci_bridge busnumber:devnumber:funcnumber:vendor:link_ctl_reg:amd %d:%d:%d:%x:%x:%x\n",
pcipriv->ndis_adapter.pcibridge_busnum,
pcipriv->ndis_adapter.pcibridge_devnum,
pcipriv->ndis_adapter.pcibridge_funcnum,
pcibridge_vendors[pcipriv->ndis_adapter.pcibridge_vendor],
- pcipriv->ndis_adapter.pcibridge_pciehdr_offset,
pcipriv->ndis_adapter.pcibridge_linkctrlreg,
pcipriv->ndis_adapter.amd_l1_patch);
@@ -236,7 +236,6 @@ struct mp_adapter {
u16 pcibridge_vendorid;
u16 pcibridge_deviceid;
- u8 pcibridge_pciehdr_offset;
u8 pcibridge_linkctrlreg;
bool amd_l1_patch;