| Message ID | 9b704807383f3048898944d2b9cb74e6b4e8d83d.1624174954.git.objelf@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] mt76: mt7921: enable aspm by default | expand |
> 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 >
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
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
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);