Received-SPF: pass (google.com: domain of linux-kernel+bounces-187535-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: Sakari Ailus <sakari.ailus@linux.intel.com>
CC: "Winkler, Tomas" <tomas.winkler@intel.com>, "gregkh@linuxfoundation.org"
	<gregkh@linuxfoundation.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "Yao, Hao" <hao.yao@intel.com>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>, "Chen, Jason Z"
	<jason.z.chen@intel.com>
Subject: RE: [PATCH] mei: vsc: Don't stop/restart mei device during system
 suspend/resume
Thread-Topic: [PATCH] mei: vsc: Don't stop/restart mei device during system
 suspend/resume
Thread-Index: AQHapzEUtjJdNJsaK0i6Uv4C/F0UKbGkrjmAgAAljEA=
Date: Thu, 23 May 2024 13:02:13 +0000
Message-ID: <MW5PR11MB5787B1488ECC68EB6A5F44F08DF42@MW5PR11MB5787.namprd11.prod.outlook.com>
References: <20240516015400.3281634-1-wentong.wu@intel.com>
 <Zk8eP2-UEPSxv42v@kekkonen.localdomain>
In-Reply-To: <Zk8eP2-UEPSxv42v@kekkonen.localdomain>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?7WbzB80usrPO+Ms4NBDmHlie0L6DijJ3la6Z2e6tsFRof0nku8OPsckK5IwG?=
 =?us-ascii?Q?NL3PIibH/+cNw00VexQnJ43+mA53VrugVsWGgB6WNagFVU3rpb2DSBdsB6ta?=
 =?us-ascii?Q?l3OgyrCYRpLtiFs+0/zHUd5wwAnt8Q6Vh7Xi2vgzmufKRpiYCuEla9MYDpff?=
 =?us-ascii?Q?RcfY0r1HWoYe95cClJojWrZjZaxu/NX28K4m4IGkENBtokswXNhJsLzQl3HQ?=
 =?us-ascii?Q?sp4LVXjIW6yqtWvvxdcqxali7vkitBQ4qDQYMqnbbwZiL7D1tPa6dWT7ZKTn?=
 =?us-ascii?Q?yYLXVUbCyEV3kdgWSzRHHvihXlZDjmYShRIg7MTCGe1gK8JHJdyfIIK+sIuB?=
 =?us-ascii?Q?1Uzj61i8Qz4ZrXbsMfKUDzsrayijxGEJe8BEknerZ3FJg2QEGpbRdMtsf2ZI?=
 =?us-ascii?Q?wCsqRPG9af04IE7znYlxkMMLA43jc+BLiajI1qZn5ys9fjOAyedJWh1rlbor?=
 =?us-ascii?Q?AGLbvAQGVP7VJMwBZ5J/uzAKbmEErl1T+CLSIYWWAIm6d5QVeDh/5ZwPPKik?=
 =?us-ascii?Q?hvzonkw0xFL+RSN6BH4QnALGzgxnt4kTeqZ8RgYOP1iALXhkg4+qtPm4pEV3?=
 =?us-ascii?Q?Yl22H58Co+N+xtFDonSelFjfXi0cQvqpowWlhcxMyv+kjBSxIfKC1/SD7jB9?=
 =?us-ascii?Q?nLMegHLswNKgN9ztPuWaH/x65us4+uDUe9DubUkKM6JnIYLHR4j1ASaQaRjM?=
 =?us-ascii?Q?lmLhJkZGTpeSgFa21SAaxBh2TqtdB65CoFgoz1HasLj6+fqlb36z+hnl5gV2?=
 =?us-ascii?Q?qtyinVrZJwd5msf/womcInlCTZrVrpi/uHIoIrNYOvfQJt33gVoVcKKLdmaB?=
 =?us-ascii?Q?PeavGTOCz8X7UHcqFYSy/+OX+ECHE31Gvpa49qoGkKhz7W0XXphPfI1kq58Z?=
 =?us-ascii?Q?DYE9aLpxUN6kNnzAjx3eUJbFlmKnuhHPHzwADVd0t+LrTn+DTT3UI1gjQ2DU?=
 =?us-ascii?Q?nI2DusRWfXJtIzNjB2mPuNRAdDvuzLH/F5JqDQZ+OO6ykAeq1UvkPQDPkaHD?=
 =?us-ascii?Q?JnHlpDpUocWYhfWqIu4IblLKeg3AZde2HarsNPFt2ZBSOornntOeoZaR3GpT?=
 =?us-ascii?Q?Y+uA1FO2rg9bgl2CSkARXHIL7aFeySnif29m5N1xHuwCIMQowe0UjUb6goLU?=
 =?us-ascii?Q?ykMe2T2VxMqqrl0xd23RIr/CiHHDJsdIPWlhotkPrrtk6vYeB6TnNh9G56uI?=
 =?us-ascii?Q?Ozz1tpgk4XQLkL2ZLg0FbGya2pFsTFx2tPFW3OLtjnS7XThL8IcO4cfZAGX0?=
 =?us-ascii?Q?plVyIB/Ql8ErWwPUu3Br31pBWgBvzPuozmdBeY6wein+RT+VGkBlHWhAVll+?=
 =?us-ascii?Q?LoOHEYIJv49i42sX17Nn4aFOzpP/0DPsmEExS7QPu3VHIiVHpxRresrJCUXA?=
 =?us-ascii?Q?elEuuLAmx8Mhi8Lh8KOcad353QFlAl4NB4Ea0MTBstn0PKcULHlSUJHnSrMx?=
 =?us-ascii?Q?aopyMguNzfVaWrRFCRDjNAbz5XqinlqlT/yKkOux7G60E+BlJT/+yTuGWdiP?=
 =?us-ascii?Q?q9rcYjjlLrAASVwoKibNSVmmNNgDwEl3lJ1qPuNLstG9KKB0gktT8I7Yowh5?=
 =?us-ascii?Q?KptjzWmlBJk5rwoaLQS360UQnnMhXLIfzdYLZi6z?=
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: 34a33dd7-f9b3-4b29-6c26-08dc7b2890ac
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 May 2024 13:02:13.5828
 (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: WlgYhsL3ZvSTTwmfdQtgviBo9Y+hdKcrF4GN9NgOMm7q4RXYzE2P/HCH/N3Gi+/D/crSipw4j3ad7IUJX87NxA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR11MB7355
X-OriginatorOrg: intel.com

> From: Sakari Ailus <sakari.ailus@linux.intel.com>
>=20
> Hi Wentong,
>=20
> Thanks for the patch. I thought something like this would indeed have bee=
n
> possible.

Hi Sakari,

Thanks for your review.

>=20
> On Thu, May 16, 2024 at 09:54:00AM +0800, Wentong Wu wrote:
> > 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: 386a766c4169 ("mei: Add MEI hardware support for IVSC device")
>=20
> I think this should be instead:
>=20
> Fixes: f6085a96c973 ("mei: vsc: Unregister interrupt handler for system
> suspend")
>=20
> As this fix depends on the previous not-quite-as-good fix.

Agree, v2 patch will address this. Thanks

BR,
Wentong
>=20
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Tested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>=20
> > 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>
> > Tested-by: Jason Chen <jason.z.chen@intel.com>
> > ---
> >  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);
>=20
> --
> Kind regards,
>=20
> Sakari Ailus