[v1] wifi: mt76: mt7921e: Set memory space enable in PCI_COMMAND if unset

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

Posted by Mario Limonciello 1 year, 5 months ago

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@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
Original patch was submitted ~3 weeks ago with no comments.
Link: https://lore.kernel.org/all/20230310170002.200-1-mario.limonciello@amd.com/
---
 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);
+	}
 	pci_set_master(pdev);
 
 	ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
-- 
2.34.1

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

Posted by Sean Wang 1 year, 5 months ago

Hi,

On Wed, Mar 29, 2023 at 1:18 PM Mario Limonciello
<mario.limonciello@amd.com> 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@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> Original patch was submitted ~3 weeks ago with no comments.
> Link: https://lore.kernel.org/all/20230310170002.200-1-mario.limonciello@amd.com/
> ---
>  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.
Also, I will try the patch on another Intel machine to see if it worked.

     Sean

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

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

Posted by Limonciello, Mario 1 year, 5 months ago

On 3/29/2023 18:24, Sean Wang wrote:
> Hi,
> 
> On Wed, Mar 29, 2023 at 1:18 PM Mario Limonciello
> <mario.limonciello@amd.com> 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@amd.com>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> Original patch was submitted ~3 weeks ago with no comments.
>> Link: https://lore.kernel.org/all/20230310170002.200-1-mario.limonciello@amd.com/
>> ---
>>   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.

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

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

Posted by Limonciello, Mario 1 year, 5 months ago

[Public]

> On 3/29/2023 18:24, Sean Wang wrote:
> > Hi,
> >
> > On Wed, Mar 29, 2023 at 1:18 PM Mario Limonciello
> > <mario.limonciello@amd.com> 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@amd.com>
> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> >> ---
> >> Original patch was submitted ~3 weeks ago with no comments.
> >> Link: https://lore.kernel.org/all/20230310170002.200-1-
> mario.limonciello@amd.com/
> >> ---
> >>   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
> >>

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

Posted by Sean Wang 1 year, 5 months ago

On Mon, Apr 3, 2023 at 6:42 AM Limonciello, Mario
<Mario.Limonciello@amd.com> wrote:
>
> [Public]
>
> > On 3/29/2023 18:24, Sean Wang wrote:
> > > Hi,
> > >
> > > On Wed, Mar 29, 2023 at 1:18 PM Mario Limonciello
> > > <mario.limonciello@amd.com> 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@amd.com>
> > >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > >> ---
> > >> Original patch was submitted ~3 weeks ago with no comments.
> > >> Link: https://lore.kernel.org/all/20230310170002.200-1-
> > mario.limonciello@amd.com/
> > >> ---
> > >>   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?

Hi,

Sorry for the late response. We have tested the related Intel platform
and it worked fine. You can add the tag from me like
Acked-by: Sean Wang <sean.wang@mediatek.com>


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

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

Posted by Mario Limonciello 1 year, 5 months ago

On 4/9/23 19:57, Sean Wang wrote:
> On Mon, Apr 3, 2023 at 6:42 AM Limonciello, Mario
> <Mario.Limonciello@amd.com> wrote:
>> [Public]
>>
>>> On 3/29/2023 18:24, Sean Wang wrote:
>>>> Hi,
>>>>
>>>> On Wed, Mar 29, 2023 at 1:18 PM Mario Limonciello
>>>> <mario.limonciello@amd.com> 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@amd.com>
>>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>>> ---
>>>>> Original patch was submitted ~3 weeks ago with no comments.
>>>>> Link: https://lore.kernel.org/all/20230310170002.200-1-
>>> mario.limonciello@amd.com/
>>>>> ---
>>>>>    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?
> Hi,
>
> Sorry for the late response. We have tested the related Intel platform
> and it worked fine. You can add the tag from me like
> Acked-by: Sean Wang <sean.wang@mediatek.com>
>
Thanks!

Felix, Lorenzo, or Ryder can you guys please pick this up?

Thanks,