Messages in this thread | | | Date | Tue, 2 Apr 2024 21:32:28 +0300 | Subject | Re: [Intel-wired-lan] [PATCH 1/3] e1000e: Remove redundant runtime resume for ethtool_ops | From | "naamax.meir" <> |
| |
On 3/26/2024 00:29, Bjorn Helgaas wrote: > From: Bjorn Helgaas <bhelgaas@google.com> > > e60b22c5b7e5 ("e1000e: fix accessing to suspended device") added > ethtool_ops.begin() and .complete(), which used pm_runtime_get_sync() to > resume suspended devices before any ethtool_ops callback and allow suspend > after it completed. > > 3ef672ab1862 ("e1000e: ethtool unnecessarily takes device out of RPM > suspend") removed ethtool_ops.begin() and .complete() and instead did > pm_runtime_get_sync() only in the individual ethtool_ops callbacks that > access device registers. > > Subsequently, f32a21376573 ("ethtool: runtime-resume netdev parent before > ethtool ioctl ops") added pm_runtime_get_sync() in the dev_ethtool() path, > so the device is resumed before *any* ethtool_ops callback, as it was > before 3ef672ab1862. > > Remove most runtime resumes from ethtool_ops, which are now redundant > because the resume has already been done by dev_ethtool(). This is > essentially a revert of 3ef672ab1862 ("e1000e: ethtool unnecessarily takes > device out of RPM suspend"). > > There are a couple subtleties: > > - Prior to 3ef672ab1862, the device was resumed only for the duration of > a single ethtool callback. 3ef672ab1862 changed e1000_set_phys_id() so > the device was resumed for ETHTOOL_ID_ACTIVE and remained resumed until > a subsequent callback for ETHTOOL_ID_INACTIVE. Preserve that part of > 3ef672ab1862 so the device will not be runtime suspended while in the > ETHTOOL_ID_ACTIVE state. > > - 3ef672ab1862 added "if (!pm_runtime_suspended())" in before reading the > STATUS register in e1000_get_settings(). This was racy and is now > unnecessary because dev_ethtool() has resumed the device already, so > revert that. > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > --- > drivers/net/ethernet/intel/e1000e/ethtool.c | 62 ++------------------- > 1 file changed, 6 insertions(+), 56 deletions(-)
Tested-by: Naama Meir <naamax.meir@linux.intel.com>
| |