2021-06-20 07:52:29

by Sean Wang

[permalink] [raw]
Subject: [PATCH 1/2] mt76: mt7921: enable aspm by default

From: Sean Wang <[email protected]>

mt7921 is mainly used in NB, CE and IoT application where battery life is
much concerned so the patch enabled PCIe ASPM by default to shut off the
clocks related PCIe as much as possible when MT7921 is either in suspend
state or in runtime pm to lower power consumption.

We still leave disable aspm as an option with module_param for users to
disable ASPM if necessary.

Signed-off-by: Sean Wang <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
index c3905bcab360..33782e1ee312 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
@@ -17,6 +17,10 @@ static const struct pci_device_id mt7921_pci_device_table[] = {
{ },
};

+static bool mt7921_disable_aspm;
+module_param_named(disable_aspm, mt7921_disable_aspm, bool, 0644);
+MODULE_PARM_DESC(disable_aspm, "disable PCI ASPM support");
+
static void
mt7921_rx_poll_complete(struct mt76_dev *mdev, enum mt76_rxq_id q)
{
@@ -132,7 +136,8 @@ static int mt7921_pci_probe(struct pci_dev *pdev,
if (ret)
goto err_free_pci_vec;

- mt76_pci_disable_aspm(pdev);
+ if (mt7921_disable_aspm)
+ mt76_pci_disable_aspm(pdev);

mdev = mt76_alloc_device(&pdev->dev, sizeof(*dev), &mt7921_ops,
&drv_ops);
--
2.25.1


2021-06-20 07:52:29

by Sean Wang

[permalink] [raw]
Subject: [PATCH 2/2] mt76: fix build error implicit enumeration conversion

From: Sean Wang <[email protected]>

drivers/net/wireless/mediatek/mt76/mt7915/mcu.c:114:10: error: implicit
conversion from enumeration type 'enum mt76_cipher_type' to different
enumeration type 'enum mcu_cipher_type' [-Werror,-Wenum-conversion]
return MT_CIPHER_NONE;
~~~~~~ ^~~~~~~~~~~~~~

drivers/net/wireless/mediatek/mt76/mt7921/mcu.c:114:10: error: implicit
conversion from enumeration type 'enum mt76_cipher_type' to different
enumeration type 'enum mcu_cipher_type' [-Werror,-Wenum-conversion]
return MT_CIPHER_NONE;
~~~~~~ ^~~~~~~~~~~~~~

Fixes: c368362c36d3 ("mt76: fix iv and CCMP header insertion")
Signed-off-by: Sean Wang <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt7915/mcu.c | 4 ++--
drivers/net/wireless/mediatek/mt76/mt7915/mcu.h | 3 ++-
drivers/net/wireless/mediatek/mt76/mt7921/mcu.c | 4 ++--
drivers/net/wireless/mediatek/mt76/mt7921/mcu.h | 3 ++-
4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
index 863aa18b3024..c2e537a9c1dc 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
@@ -111,7 +111,7 @@ mt7915_mcu_get_cipher(int cipher)
case WLAN_CIPHER_SUITE_SMS4:
return MCU_CIPHER_WAPI;
default:
- return MT_CIPHER_NONE;
+ return MCU_CIPHER_NONE;
}
}

@@ -1201,7 +1201,7 @@ mt7915_mcu_sta_key_tlv(struct mt7915_sta *msta, struct sk_buff *skb,
u8 cipher;

cipher = mt7915_mcu_get_cipher(key->cipher);
- if (cipher == MT_CIPHER_NONE)
+ if (cipher == MCU_CIPHER_NONE)
return -EOPNOTSUPP;

sec_key = &sec->key[0];
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.h b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.h
index edd3ba3a0c2d..5b9b425bd836 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.h
@@ -1073,7 +1073,8 @@ enum {
};

enum mcu_cipher_type {
- MCU_CIPHER_WEP40 = 1,
+ MCU_CIPHER_NONE,
+ MCU_CIPHER_WEP40,
MCU_CIPHER_WEP104,
MCU_CIPHER_WEP128,
MCU_CIPHER_TKIP,
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
index c2c4dc196802..81633be09e90 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
@@ -111,7 +111,7 @@ mt7921_mcu_get_cipher(int cipher)
case WLAN_CIPHER_SUITE_SMS4:
return MCU_CIPHER_WAPI;
default:
- return MT_CIPHER_NONE;
+ return MCU_CIPHER_NONE;
}
}

@@ -619,7 +619,7 @@ mt7921_mcu_sta_key_tlv(struct mt7921_sta *msta, struct sk_buff *skb,
u8 cipher;

cipher = mt7921_mcu_get_cipher(key->cipher);
- if (cipher == MT_CIPHER_NONE)
+ if (cipher == MCU_CIPHER_NONE)
return -EOPNOTSUPP;

sec_key = &sec->key[0];
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.h b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.h
index d76cf8f8dfdf..3334afd8aea9 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.h
@@ -199,7 +199,8 @@ struct sta_rec_sec {
} __packed;

enum mcu_cipher_type {
- MCU_CIPHER_WEP40 = 1,
+ MCU_CIPHER_NONE,
+ MCU_CIPHER_WEP40,
MCU_CIPHER_WEP104,
MCU_CIPHER_WEP128,
MCU_CIPHER_TKIP,
--
2.25.1

2021-06-20 09:50:14

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH 1/2] mt76: mt7921: enable aspm by default

> From: Sean Wang <[email protected]>
>
> mt7921 is mainly used in NB, CE and IoT application where battery life is
> much concerned so the patch enabled PCIe ASPM by default to shut off the
> clocks related PCIe as much as possible when MT7921 is either in suspend
> state or in runtime pm to lower power consumption.
>
> We still leave disable aspm as an option with module_param for users to
> disable ASPM if necessary.

instead of adding a module parameter (deprecated), why not just enable it by
default for mt7921 and add a debugfs knob to flip it?

Regards,
Lorenzo

>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> index c3905bcab360..33782e1ee312 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> @@ -17,6 +17,10 @@ static const struct pci_device_id mt7921_pci_device_table[] = {
> { },
> };
>
> +static bool mt7921_disable_aspm;
> +module_param_named(disable_aspm, mt7921_disable_aspm, bool, 0644);
> +MODULE_PARM_DESC(disable_aspm, "disable PCI ASPM support");
> +
> static void
> mt7921_rx_poll_complete(struct mt76_dev *mdev, enum mt76_rxq_id q)
> {
> @@ -132,7 +136,8 @@ static int mt7921_pci_probe(struct pci_dev *pdev,
> if (ret)
> goto err_free_pci_vec;
>
> - mt76_pci_disable_aspm(pdev);
> + if (mt7921_disable_aspm)
> + mt76_pci_disable_aspm(pdev);
>
> mdev = mt76_alloc_device(&pdev->dev, sizeof(*dev), &mt7921_ops,
> &drv_ops);
> --
> 2.25.1
>


Attachments:
(No filename) (1.72 kB)
signature.asc (235.00 B)
Download all attachments

2021-06-22 09:15:55

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/2] mt76: mt7921: enable aspm by default

Lorenzo Bianconi <[email protected]> writes:

>> From: Sean Wang <[email protected]>
>>
>> mt7921 is mainly used in NB, CE and IoT application where battery life is
>> much concerned so the patch enabled PCIe ASPM by default to shut off the
>> clocks related PCIe as much as possible when MT7921 is either in suspend
>> state or in runtime pm to lower power consumption.
>>
>> We still leave disable aspm as an option with module_param for users to
>> disable ASPM if necessary.
>
> instead of adding a module parameter (deprecated)

Why is a module parameter deprecated? I have not heard about that.

> , why not just enable it by default for mt7921 and add a debugfs knob
> to flip it?

debugfs is not ideal for this kind of hardware configuration as debugfs
can be disabled. My preference is to use a module parameter.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-07-27 00:46:12

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH 2/2] mt76: fix build error implicit enumeration conversion

On Sun, Jun 20, 2021 at 03:48:07PM +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> drivers/net/wireless/mediatek/mt76/mt7915/mcu.c:114:10: error: implicit
> conversion from enumeration type 'enum mt76_cipher_type' to different
> enumeration type 'enum mcu_cipher_type' [-Werror,-Wenum-conversion]
> return MT_CIPHER_NONE;
> ~~~~~~ ^~~~~~~~~~~~~~
>
> drivers/net/wireless/mediatek/mt76/mt7921/mcu.c:114:10: error: implicit
> conversion from enumeration type 'enum mt76_cipher_type' to different
> enumeration type 'enum mcu_cipher_type' [-Werror,-Wenum-conversion]
> return MT_CIPHER_NONE;
> ~~~~~~ ^~~~~~~~~~~~~~
>
> Fixes: c368362c36d3 ("mt76: fix iv and CCMP header insertion")
> Signed-off-by: Sean Wang <[email protected]>

Reviewed-by: Nathan Chancellor <[email protected]>

It would be nice if this could be added to 5.14-rc at some point in the
cycle as this shows up in clang builds for allmodconfig for various
architectures and I still do not see it in -next.

> ---
> drivers/net/wireless/mediatek/mt76/mt7915/mcu.c | 4 ++--
> drivers/net/wireless/mediatek/mt76/mt7915/mcu.h | 3 ++-
> drivers/net/wireless/mediatek/mt76/mt7921/mcu.c | 4 ++--
> drivers/net/wireless/mediatek/mt76/mt7921/mcu.h | 3 ++-
> 4 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> index 863aa18b3024..c2e537a9c1dc 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> @@ -111,7 +111,7 @@ mt7915_mcu_get_cipher(int cipher)
> case WLAN_CIPHER_SUITE_SMS4:
> return MCU_CIPHER_WAPI;
> default:
> - return MT_CIPHER_NONE;
> + return MCU_CIPHER_NONE;
> }
> }
>
> @@ -1201,7 +1201,7 @@ mt7915_mcu_sta_key_tlv(struct mt7915_sta *msta, struct sk_buff *skb,
> u8 cipher;
>
> cipher = mt7915_mcu_get_cipher(key->cipher);
> - if (cipher == MT_CIPHER_NONE)
> + if (cipher == MCU_CIPHER_NONE)
> return -EOPNOTSUPP;
>
> sec_key = &sec->key[0];
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.h b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.h
> index edd3ba3a0c2d..5b9b425bd836 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.h
> @@ -1073,7 +1073,8 @@ enum {
> };
>
> enum mcu_cipher_type {
> - MCU_CIPHER_WEP40 = 1,
> + MCU_CIPHER_NONE,
> + MCU_CIPHER_WEP40,
> MCU_CIPHER_WEP104,
> MCU_CIPHER_WEP128,
> MCU_CIPHER_TKIP,
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
> index c2c4dc196802..81633be09e90 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
> @@ -111,7 +111,7 @@ mt7921_mcu_get_cipher(int cipher)
> case WLAN_CIPHER_SUITE_SMS4:
> return MCU_CIPHER_WAPI;
> default:
> - return MT_CIPHER_NONE;
> + return MCU_CIPHER_NONE;
> }
> }
>
> @@ -619,7 +619,7 @@ mt7921_mcu_sta_key_tlv(struct mt7921_sta *msta, struct sk_buff *skb,
> u8 cipher;
>
> cipher = mt7921_mcu_get_cipher(key->cipher);
> - if (cipher == MT_CIPHER_NONE)
> + if (cipher == MCU_CIPHER_NONE)
> return -EOPNOTSUPP;
>
> sec_key = &sec->key[0];
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.h b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.h
> index d76cf8f8dfdf..3334afd8aea9 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.h
> @@ -199,7 +199,8 @@ struct sta_rec_sec {
> } __packed;
>
> enum mcu_cipher_type {
> - MCU_CIPHER_WEP40 = 1,
> + MCU_CIPHER_NONE,
> + MCU_CIPHER_WEP40,
> MCU_CIPHER_WEP104,
> MCU_CIPHER_WEP128,
> MCU_CIPHER_TKIP,
> --
> 2.25.1