linux-kernel - RE: [PATCH v3] mei: vsc: Don't stop/restart mei device during system suspend/resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <MW5PR11MB57876C1C77CBF5D1B5D9F0168DF12@MW5PR11MB5787.namprd11.prod.outlook.com>
Date: Tue, 28 May 2024 00:57:42 +0000
From: "Wu, Wentong" <wentong.wu@...el.com>
To: "Winkler, Tomas" <tomas.winkler@...el.com>, "sakari.ailus@...ux.intel.com"
	<sakari.ailus@...ux.intel.com>, "gregkh@...uxfoundation.org"
	<gregkh@...uxfoundation.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>, "Yao, Hao"
	<hao.yao@...el.com>, "Chen, Jason Z" <jason.z.chen@...el.com>
Subject: RE: [PATCH v3] mei: vsc: Don't stop/restart mei device during system
 suspend/resume

> From: Winkler, Tomas <tomas.winkler@...el.com>
> > From: Wu, Wentong <wentong.wu@...el.com>
> >
> > The dynamically created mei client device (mei csi) is used as one
> > V4L2 sub device of the whole video pipeline, and the V4L2 connection
> > graph is built by software node. The mei_stop() and mei_restart() will
> > delete the old mei csi client device and create a new mei client
> > device, which will cause the software node information saved in old
> > mei csi device lost and the whole video pipeline will be broken.
> >
> > Removing mei_stop()/mei_restart() during system suspend/resume can fix
> > the issue above and won't impact hardware actual power saving logic.
> >
> > Fixes: f6085a96c973 ("mei: vsc: Unregister interrupt handler for
> > system
> > suspend")
> > Cc: stable@...r.kernel.org # for 6.8+
> > Reported-by: Hao Yao <hao.yao@...el.com>
> > Signed-off-by: Wentong Wu <wentong.wu@...el.com>
> > Reviewed-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
> > Tested-by: Jason Chen <jason.z.chen@...el.com>
> > Tested-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
> Acked-by: Tomas Winkler <tomas.winkler@...el.com>

Thanks
> >
> > ---
> > Changes since v2:
> >  - add change log which is not covered by v2, and no code change
> >
> > Changes since v1:
> >  - correct Fixes commit id in commit message, and no code change
> >
> > ---
> vX descriptions should go here

Thanks, I will follow this going forward, thanks

BR,
Wentong
> 
> 
> >  drivers/misc/mei/platform-vsc.c | 39
> > +++++++++++++--------------------
> >  1 file changed, 15 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/misc/mei/platform-vsc.c
> > b/drivers/misc/mei/platform- vsc.c index b543e6b9f3cf..1ec65d87488a
> > 100644
> > --- a/drivers/misc/mei/platform-vsc.c
> > +++ b/drivers/misc/mei/platform-vsc.c
> > @@ -399,41 +399,32 @@ static void mei_vsc_remove(struct
> > platform_device
> > *pdev)
> >
> >  static int mei_vsc_suspend(struct device *dev)  {
> > -	struct mei_device *mei_dev = dev_get_drvdata(dev);
> > -	struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
> > +	struct mei_device *mei_dev;
> > +	int ret = 0;
> >
> > -	mei_stop(mei_dev);
> > +	mei_dev = dev_get_drvdata(dev);
> > +	if (!mei_dev)
> > +		return -ENODEV;
> >
> > -	mei_disable_interrupts(mei_dev);
> > +	mutex_lock(&mei_dev->device_lock);
> >
> > -	vsc_tp_free_irq(hw->tp);
> > +	if (!mei_write_is_idle(mei_dev))
> > +		ret = -EAGAIN;
> >
> > -	return 0;
> > +	mutex_unlock(&mei_dev->device_lock);
> > +
> > +	return ret;
> >  }
> >
> >  static int mei_vsc_resume(struct device *dev)  {
> > -	struct mei_device *mei_dev = dev_get_drvdata(dev);
> > -	struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
> > -	int ret;
> > -
> > -	ret = vsc_tp_request_irq(hw->tp);
> > -	if (ret)
> > -		return ret;
> > -
> > -	ret = mei_restart(mei_dev);
> > -	if (ret)
> > -		goto err_free;
> > +	struct mei_device *mei_dev;
> >
> > -	/* start timer if stopped in suspend */
> > -	schedule_delayed_work(&mei_dev->timer_work, HZ);
> > +	mei_dev = dev_get_drvdata(dev);
> > +	if (!mei_dev)
> > +		return -ENODEV;
> >
> >  	return 0;
> > -
> > -err_free:
> > -	vsc_tp_free_irq(hw->tp);
> > -
> > -	return ret;
> >  }
> >
> >  static DEFINE_SIMPLE_DEV_PM_OPS(mei_vsc_pm_ops, mei_vsc_suspend,
> > mei_vsc_resume);
> > --
> > 2.34.1