Messages in this thread | | | Date | Sat, 19 Jun 2010 08:38:41 -0400 | From | Bob Copeland <> | Subject | Re: [PATCH v2] ath5k: disable ASPM |
| |
On Sat, Jun 19, 2010 at 10:49:34AM +0300, Maxim Levitsky wrote: > How this patch?
Looks fine to me. Some nitpicking below but feel free to add my
Acked-by: Bob Copeland <me@bobcopeland.com>
to this or a later version.
> Signed-off-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi> > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
I believe maintaining the s-o-b is usually reserved for the case when the patch only has minor edits; here your patch while same in spirit has some 30 fewer lines. Maybe should use "From:" or mention Jussi in the commit log to maintain attribution instead?
> + /* Disable PCIE ASPM L0S. It is never enabled by windows driver */ > + pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S); > +
Would be good to expand this comment as to why (just copy another sentence from the commit log or so).
> ret = pci_enable_device(pdev); > if (ret) { > dev_err(&pdev->dev, "can't enable device\n"); > @@ -722,6 +726,8 @@ static int ath5k_pci_resume(struct device *dev) > struct ieee80211_hw *hw = pci_get_drvdata(pdev); > struct ath5k_softc *sc = hw->priv; > > + pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S); > +
Did you test that it did get lost over suspend/resume? A glance at pci_save_pcie_state seems to indicate it's saved.
-- Bob Copeland %% www.bobcopeland.com
| |