linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] mt76: mt7921: enable aspm by default
@ 2021-06-20  7:48 sean.wang
  2021-06-20  7:48 ` [PATCH 2/2] mt76: fix build error implicit enumeration conversion sean.wang
  2021-06-20  9:44 ` [PATCH 1/2] mt76: mt7921: enable aspm by default Lorenzo Bianconi
  0 siblings, 2 replies; 5+ messages in thread
From: sean.wang @ 2021-06-20  7:48 UTC (permalink / raw)
  To: nbd, lorenzo.bianconi
  Cc: sean.wang, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang,
	Deren.Wu, km.lin, robin.chiu, ch.yeh, posh.sun, Eric.Liang,
	Stella.Chang, jemele, yenlinlai, linux-wireless, linux-mediatek

From: Sean Wang <sean.wang@mediatek.com>

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 <sean.wang@mediatek.com>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 2/2] mt76: fix build error implicit enumeration conversion
  2021-06-20  7:48 [PATCH 1/2] mt76: mt7921: enable aspm by default sean.wang
@ 2021-06-20  7:48 ` sean.wang
  2021-07-27  0:42   ` Nathan Chancellor
  2021-06-20  9:44 ` [PATCH 1/2] mt76: mt7921: enable aspm by default Lorenzo Bianconi
  1 sibling, 1 reply; 5+ messages in thread
From: sean.wang @ 2021-06-20  7:48 UTC (permalink / raw)
  To: nbd, lorenzo.bianconi
  Cc: sean.wang, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang,
	Deren.Wu, km.lin, robin.chiu, ch.yeh, posh.sun, Eric.Liang,
	Stella.Chang, jemele, yenlinlai, linux-wireless, linux-mediatek

From: Sean Wang <sean.wang@mediatek.com>

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 <sean.wang@mediatek.com>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] mt76: mt7921: enable aspm by default
  2021-06-20  7:48 [PATCH 1/2] mt76: mt7921: enable aspm by default sean.wang
  2021-06-20  7:48 ` [PATCH 2/2] mt76: fix build error implicit enumeration conversion sean.wang
@ 2021-06-20  9:44 ` Lorenzo Bianconi
  2021-06-22  9:15   ` Kalle Valo
  1 sibling, 1 reply; 5+ messages in thread
From: Lorenzo Bianconi @ 2021-06-20  9:44 UTC (permalink / raw)
  To: sean.wang
  Cc: nbd, lorenzo.bianconi, Soul.Huang, YN.Chen, Leon.Yen,
	Eric-SY.Chang, Deren.Wu, km.lin, robin.chiu, ch.yeh, posh.sun,
	Eric.Liang, Stella.Chang, jemele, yenlinlai, linux-wireless,
	linux-mediatek

[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]

> From: Sean Wang <sean.wang@mediatek.com>
> 
> 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 <sean.wang@mediatek.com>
> ---
>  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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] mt76: mt7921: enable aspm by default
  2021-06-20  9:44 ` [PATCH 1/2] mt76: mt7921: enable aspm by default Lorenzo Bianconi
@ 2021-06-22  9:15   ` Kalle Valo
  0 siblings, 0 replies; 5+ messages in thread
From: Kalle Valo @ 2021-06-22  9:15 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: sean.wang, nbd, lorenzo.bianconi, Soul.Huang, YN.Chen, Leon.Yen,
	Eric-SY.Chang, Deren.Wu, km.lin, robin.chiu, ch.yeh, posh.sun,
	Eric.Liang, Stella.Chang, jemele, yenlinlai, linux-wireless,
	linux-mediatek

Lorenzo Bianconi <lorenzo@kernel.org> writes:

>> From: Sean Wang <sean.wang@mediatek.com>
>> 
>> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] mt76: fix build error implicit enumeration conversion
  2021-06-20  7:48 ` [PATCH 2/2] mt76: fix build error implicit enumeration conversion sean.wang
@ 2021-07-27  0:42   ` Nathan Chancellor
  0 siblings, 0 replies; 5+ messages in thread
From: Nathan Chancellor @ 2021-07-27  0:42 UTC (permalink / raw)
  To: sean.wang
  Cc: nbd, lorenzo.bianconi, Soul.Huang, YN.Chen, Leon.Yen,
	Eric-SY.Chang, Deren.Wu, km.lin, robin.chiu, ch.yeh, posh.sun,
	Eric.Liang, Stella.Chang, jemele, yenlinlai, linux-wireless,
	linux-mediatek

On Sun, Jun 20, 2021 at 03:48:07PM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> 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 <sean.wang@mediatek.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-27  0:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-20  7:48 [PATCH 1/2] mt76: mt7921: enable aspm by default sean.wang
2021-06-20  7:48 ` [PATCH 2/2] mt76: fix build error implicit enumeration conversion sean.wang
2021-07-27  0:42   ` Nathan Chancellor
2021-06-20  9:44 ` [PATCH 1/2] mt76: mt7921: enable aspm by default Lorenzo Bianconi
2021-06-22  9:15   ` Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).