| Message ID | 9b704807383f3048898944d2b9cb74e6b4e8d83d.1624174954.git.objelf@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Felix Fietkau |
| 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.
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);