On Wed, May 29, 2019 at 1:01 AM Hyungwoo Yang <hyungwoo.yang@xxxxxxxxx> wrote:
>
> Currently, in suspend() and resume(), ishtp client drivers are using
> driver_data to get "struct ishtp_cl_device" object which is set by
> bus driver. It's wrong since the driver_data should not be owned bus.
> driver_data should be owned by the corresponding ishtp client driver.
> Due to this, some ishtp client driver like cros_ec_ishtp which
> uses its driver_data doesn't work correctly.
>
> So instead of using driver_data to get "struct ishtp_cl_device", since
> "struct device" is embedded in "struct ishtp_cl_device", we introduce
> a helper function that uses container_of() method.
>
> Signed-off-by: Hyungwoo Yang <hyungwoo.yang@xxxxxxxxx>
> ---
> drivers/hid/intel-ish-hid/ishtp-hid-client.c | 4 ++--
> drivers/hid/intel-ish-hid/ishtp/bus.c | 14 ++++++++++++++
> drivers/platform/chrome/cros_ec_ishtp.c | 4 ++--
> include/linux/intel-ish-client-if.h | 1 +
> 4 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> index 56777a4..19102a3 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> @@ -899,7 +899,7 @@ static int hid_ishtp_cl_reset(struct ishtp_cl_device *cl_device)
> */
> static int hid_ishtp_cl_suspend(struct device *device)
> {
> - struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
> + struct ishtp_cl_device *cl_device = ishtp_dev_to_cl_device(device);
> struct ishtp_cl *hid_ishtp_cl = ishtp_get_drvdata(cl_device);
> struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
>
> @@ -920,7 +920,7 @@ static int hid_ishtp_cl_suspend(struct device *device)
> */
> static int hid_ishtp_cl_resume(struct device *device)
> {
> - struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
> + struct ishtp_cl_device *cl_device = ishtp_dev_to_cl_device(device);
> struct ishtp_cl *hid_ishtp_cl = ishtp_get_drvdata(cl_device);
> struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
>
> diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
> index fb8ca12..5beea44 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/bus.c
> +++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
> @@ -648,6 +648,20 @@ void *ishtp_get_drvdata(struct ishtp_cl_device *cl_device)
> EXPORT_SYMBOL(ishtp_get_drvdata);
>
> /**
> + * ishtp_dev_to_cl_device() - get ishtp_cl_device instance from device instance
> + * @device: device instance
> + *
> + * Get ish_cl_device instance which embeds device instance in it.
> + *
> + * Return: pointer to ishtp_cl_device instance
> + */
> +struct ishtp_cl_device *ishtp_dev_to_cl_device(struct device *device)
> +{
> + return container_of(device, struct ishtp_cl_device, dev);
Should we reuse to_ishtp_cl_device from the top of bus.c?
> +}
> +EXPORT_SYMBOL(ishtp_dev_to_cl_device);
> +
> +/**
> * ishtp_bus_new_client() - Create a new client
> * @dev: ISHTP device instance
> *
> diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c
> index e504d25..430731c 100644
> --- a/drivers/platform/chrome/cros_ec_ishtp.c
> +++ b/drivers/platform/chrome/cros_ec_ishtp.c
> @@ -707,7 +707,7 @@ static int cros_ec_ishtp_reset(struct ishtp_cl_device *cl_device)
> */
> static int __maybe_unused cros_ec_ishtp_suspend(struct device *device)
> {
> - struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
> + struct ishtp_cl_device *cl_device = ishtp_dev_to_cl_device(device);
> struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
>
> @@ -722,7 +722,7 @@ static int __maybe_unused cros_ec_ishtp_suspend(struct device *device)
> */
> static int __maybe_unused cros_ec_ishtp_resume(struct device *device)
> {
> - struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
> + struct ishtp_cl_device *cl_device = ishtp_dev_to_cl_device(device);
> struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
>
> diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
> index 16255c2..0d6b4bc 100644
> --- a/include/linux/intel-ish-client-if.h
> +++ b/include/linux/intel-ish-client-if.h
> @@ -103,6 +103,7 @@ int ishtp_register_event_cb(struct ishtp_cl_device *device,
> void ishtp_get_device(struct ishtp_cl_device *cl_dev);
> void ishtp_set_drvdata(struct ishtp_cl_device *cl_device, void *data);
> void *ishtp_get_drvdata(struct ishtp_cl_device *cl_device);
> +struct ishtp_cl_device *ishtp_dev_to_cl_device(struct device *dev);
> int ishtp_register_event_cb(struct ishtp_cl_device *device,
> void (*read_cb)(struct ishtp_cl_device *));
> struct ishtp_fw_client *ishtp_fw_cl_get_client(struct ishtp_device *dev,
> --
> 1.9.1
>
