On Wed, 2019-05-29 at 12:38 -0700, Srinivas Pandruvada wrote: > On Wed, 2019-05-29 at 19:03 +0000, Yang, Hyungwoo wrote: > > > 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. > > I don't know how the driver was submitted without suspend/resume > test. > > Spilt the patch in this for bisect and the cros-ec is not in 5.2 yet. > > The first patch, is the combination of patch 1 and patch 2 excluding > cros-ec changes. > > The second patch for cros-ec only using the new API. Also run ./scripts/get_maintainer.pl on the patches to get maintainers/mailing list to send/copy. This patch probably needs to go along with cros-ec drivers pull request. Thanks, Srinivas > > Thanks, > Srinivas > > > > > > > > > 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
