RE: [PATCH RESEND] wifi: mt76: mt7921e: Set memory space enable in PCI_COMMAND if unset

[Limonciello, Mario]

[Public]

> On 3/29/2023 18:24, Sean Wang wrote:
> > Hi,
> >
> > On Wed, Mar 29, 2023 at 1:18 PM Mario Limonciello
> > <mario.limonciello@xxxxxxx> wrote:
> >>
> >> When the BIOS has been configured for Fast Boot, systems with mt7921e
> >> have non-functional wifi.  Turning on Fast boot caused both bus master
> >> enable and memory space enable bits in PCI_COMMAND not to get
> configured.
> >>
> >> The mt7921 driver already sets bus master enable, but explicitly check
> >> and set memory access enable as well to fix this problem.
> >>
> >> Tested-by: Anson Tsao <anson.tsao@xxxxxxx>
> >> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> >> ---
> >> Original patch was submitted ~3 weeks ago with no comments.
> >> Link: https://lore.kernel.org/all/20230310170002.200-1-
> mario.limonciello@xxxxxxx/
> >> ---
> >>   drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 6 ++++++
> >>   1 file changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> >> index cb72ded37256..aa1a427b16c2 100644
> >> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> >> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> >> @@ -263,6 +263,7 @@ static int mt7921_pci_probe(struct pci_dev *pdev,
> >>          struct mt76_dev *mdev;
> >>          u8 features;
> >>          int ret;
> >> +       u16 cmd;
> >>
> >>          ret = pcim_enable_device(pdev);
> >>          if (ret)
> >> @@ -272,6 +273,11 @@ static int mt7921_pci_probe(struct pci_dev
> *pdev,
> >>          if (ret)
> >>                  return ret;
> >>
> >> +       pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> >> +       if (!(cmd & PCI_COMMAND_MEMORY)) {
> >> +               cmd |= PCI_COMMAND_MEMORY;
> >> +               pci_write_config_word(pdev, PCI_COMMAND, cmd);
> >> +       }
> >
> > If PCI_COMMAND_MEMORY is required in any circumstance, then we
> don't
> > need to add a conditional check and OR it with PCI_COMMAND_MEMORY.
> 
> Generally it seemed advantageous to avoid an extra PCI write if it's not
> needed.  For example that's how bus mastering works too (see
> __pci_set_master).
> 
> 
> > Also, I will try the patch on another Intel machine to see if it worked.
> 
> Thanks.

Did you get a chance to try this on an Intel system?

> 
> >
> >       Sean
> >
> >>          pci_set_master(pdev);
> >>
> >>          ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
> >> --
> >> 2.34.1
> >>