Received-SPF: pass (google.com: domain of linux-kernel+bounces-191595-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: "Wu, Wentong" <wentong.wu@intel.com>
To: "Winkler, Tomas" <tomas.winkler@intel.com>, "sakari.ailus@linux.intel.com"
	<sakari.ailus@linux.intel.com>, "gregkh@linuxfoundation.org"
	<gregkh@linuxfoundation.org>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>, "Yao, Hao"
	<hao.yao@intel.com>, "Chen, Jason Z" <jason.z.chen@intel.com>
Subject: RE: [PATCH v3] mei: vsc: Don't stop/restart mei device during system
 suspend/resume
Thread-Topic: [PATCH v3] mei: vsc: Don't stop/restart mei device during system
 suspend/resume
Thread-Index: AQHasC/Q78OsVQzYU0uj61RhUE6AtrGrhy0AgABMHjA=
Date: Tue, 28 May 2024 00:57:42 +0000
Message-ID: <MW5PR11MB57876C1C77CBF5D1B5D9F0168DF12@MW5PR11MB5787.namprd11.prod.outlook.com>
References: <20240527123835.522384-1-wentong.wu@intel.com>
 <PH7PR11MB7605CC4C866718D28B5393CEE5F02@PH7PR11MB7605.namprd11.prod.outlook.com>
In-Reply-To: <PH7PR11MB7605CC4C866718D28B5393CEE5F02@PH7PR11MB7605.namprd11.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?YHAA7DctRK3ElYI409VvCSmWKwfTXhHRJtMHYNKYiWmBG6zLyrRdR9uRGRGV?=
 =?us-ascii?Q?uXis7or7vnXU6IHWqKWwAhVL0iFbqYDNB3iE2zWSTqceer6HansXDkG2iSiJ?=
 =?us-ascii?Q?4Ok+omdzAsiu/r+Gupekd+bRkhgKuWy2ursH3jlaCXzdyTsuKm04uJ/iWlBX?=
 =?us-ascii?Q?MsMcwt4gs3dUV9AaC4Kp8Gj/244EreRI2690ABan6gSxNtfXWqJVkE7gwU+d?=
 =?us-ascii?Q?qDjw3+873f6ol6SOmSDiZ2rUrqg34UOEvEcLU3qePqpigmMT8qkM+xe7KOzD?=
 =?us-ascii?Q?Rb53og3TRf9tcezx6o4CSgwFK9HVSexRa3w5fahyo+6Gz4mR+wkofL4PJu7A?=
 =?us-ascii?Q?DjqvVQ8qAISOZVQvpliB10o2MJwDCg0rMxn9taa3DA0QWq1nN1DOjuMSMwZh?=
 =?us-ascii?Q?+XiXFtnaJkqjkrzzRd6vb98khtwX+ibz18JOgndfAOgJQqNJmwkoWVFbZ0yc?=
 =?us-ascii?Q?jPLNqhkqJFzoXaYjFNxMAESN4y7MNhOdy20BZXNRHeaYpHZ6n+6mc9AJVEWW?=
 =?us-ascii?Q?DiQ8s8CtXTW0gEf35BX2yv+XE74ycMe5Kuo+Ivk1XiPiZnljOHRXNrkiE+aP?=
 =?us-ascii?Q?EeoQknV03niwIZxY27BVr/MybtmT4WCS7y9N4U2c/natgejjihnBF6JqY22/?=
 =?us-ascii?Q?FrkBH1R3O2uy4LMkKCpo4dGVn+zgfpyJhL+dVMl8IA1Jz/9pbsY4WeDEBvGu?=
 =?us-ascii?Q?pPIyxx7G+q9EfuqBTS8d2iIMRKcJC/ILUH0ofZ7MrlXPm2eBg7LdPOquBrfo?=
 =?us-ascii?Q?+UeMlYC2VEdDoZIsI7bokvMlM3rdx1MktXtAln7HccvUYSi5qcAHtMyukT1s?=
 =?us-ascii?Q?14s3mN2CnmyjIHyZyR8LIb8SbEPGQXAT7e/h4Ayjj8bflR+TzFc/Rkt4nE3z?=
 =?us-ascii?Q?s+wumRoTHZXKDmnFOMxpmkAaMbTy8d6D4y7Sn5GamTtZWCH2QLJuSn58WIXG?=
 =?us-ascii?Q?aKKuI6y9uX1JqiA3nEuGG11ZJe4IILRhy/xnW/SS66rDTRX880+odCnusHB4?=
 =?us-ascii?Q?CLbtRJy1r98MkySXjnzqLbxaA03bej49LlWeH2dFeYCBXHCYM27GLpNaIMVS?=
 =?us-ascii?Q?CdqD7tmKpX4YOBGqbR0htwaoenY3PdgwdMQOkWsKFcxnDZFLkqCCkZG/DfH9?=
 =?us-ascii?Q?LCDFA/7qtVxB9/m7PZGoCsVXWwVRM2cF35ZTPdDjOcuOv1GEtdQ9r0xEPgg7?=
 =?us-ascii?Q?fg93kRI/BP33IeulHJdOnA3cSdYNENvmnSEO05LkYGn2ONJj3XZ4rRNu3ecd?=
 =?us-ascii?Q?yqFPZNXAFqNf1OnYrUp0rpuE8ipYLQ/CdodaHZ7p6a1BO3rQhE8sQeENuw62?=
 =?us-ascii?Q?EL/fzrvZi3nSsc1XYvWaQZII3U7TYk4+nK09Ienm/O1wvHa1Bd5B+qa4Mf32?=
 =?us-ascii?Q?e3bclq5y23Oo25oHZ2aO1MO3jSQqWu9KdzgctyGszQCI7sruvHfX6H7KwjRx?=
 =?us-ascii?Q?taYf0ATe4H5XN6AbBBB7Z0yuUop1qk53pvKQ2oB6fUM8bModyU7z1gBJZBPk?=
 =?us-ascii?Q?mtoLSvxlkT3X1pBf0R8UOffRSLXNHQhI1kXAFfbSFOChmm4ZLeK7WxrxAr65?=
 =?us-ascii?Q?xKYIz4lzaCaKoz0ObCu/qdhiP9YFXQOTqlLPYrDf?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: MW5PR11MB5787.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c4aeb456-572e-4b7b-c87e-08dc7eb12e0f
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 May 2024 00:57:42.6496
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: W9uw95Ay4MrtIZBrd7Ru44aaYkTnkVf48rh6JD45lBiiQumBModg78W8mfPPIhnAPF/PSUwsCn8tldr6ZreNSA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB8686
X-OriginatorOrg: intel.com

> From: Winkler, Tomas <tomas.winkler@intel.com>
> > From: Wu, Wentong <wentong.wu@intel.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@vger.kernel.org # for 6.8+
> > Reported-by: Hao Yao <hao.yao@intel.com>
> > Signed-off-by: Wentong Wu <wentong.wu@intel.com>
> > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Tested-by: Jason Chen <jason.z.chen@intel.com>
> > Tested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Acked-by: Tomas Winkler <tomas.winkler@intel.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
>=20
>=20
> >  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 =3D dev_get_drvdata(dev);
> > -	struct mei_vsc_hw *hw =3D mei_dev_to_vsc_hw(mei_dev);
> > +	struct mei_device *mei_dev;
> > +	int ret =3D 0;
> >
> > -	mei_stop(mei_dev);
> > +	mei_dev =3D 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 =3D -EAGAIN;
> >
> > -	return 0;
> > +	mutex_unlock(&mei_dev->device_lock);
> > +
> > +	return ret;
> >  }
> >
> >  static int mei_vsc_resume(struct device *dev)  {
> > -	struct mei_device *mei_dev =3D dev_get_drvdata(dev);
> > -	struct mei_vsc_hw *hw =3D mei_dev_to_vsc_hw(mei_dev);
> > -	int ret;
> > -
> > -	ret =3D vsc_tp_request_irq(hw->tp);
> > -	if (ret)
> > -		return ret;
> > -
> > -	ret =3D 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 =3D 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