> On Wed, 2019-05-29 at 07:21 +0000, Yang, Hyungwoo wrote: > > > On Tue, 2019-05-28 at 14:36 -0700, Hyungwoo Yang wrote: > > > > > > What was symptom or problem you try to address? Is there any crash > > > or bug occurred? Does it happen with the mainline kernel? > > > > I've added the detail in commit message in v2. Basically due to wrong > > usage of driver_data of ishtp client device, we see kernel crash. > > Currently driver_data is set by bus driver which is wrong since > > driver_data should be owned by corresponding device driver. Right now, > > we see kernel crash during suspend() of cros_ec_ishtp. Yes, it happens > > with the mainline kernel since cros_ec_ishtp is already upstreamed. > Technically this driver is not mainline. It will go in 5.3. > > The problem is cros_ec ish driver is overriding driver_data " > client_data->ec_dev = ec_dev; > dev->driver_data = ec_dev; > " > The client drivers own the driver data in its "struct ishtp_cl_device *" not the struct device *. No. still driver_data in "struct device" should be owned by its device driver. So there's no problem here since cros_ec_ish driver is owner of the device. > > As far as I can see the purpose of this is to get device pointer for debug purpose only. It's not for debug purpose and most importantly driver_data in "struct device" is used by its child in ec_device_probe() So wrong usage of driver_data should be corrected. > > I think you can remove the dev->driver_data assignment and simply replace > > dev_*(dev, > > to > dev_*(ec_dev->dev, > > Thanks, > Srinivas Thanks, Hyungwoo
