* [PATCH 5/6] mfd: cros_ec_dev: Check communication with ec at resume
@ 2018-02-19 22:41 Enric Balletbo i Serra
2018-02-22 2:20 ` Gwendal Grignou
0 siblings, 1 reply; 5+ messages in thread
From: Enric Balletbo i Serra @ 2018-02-19 22:41 UTC (permalink / raw)
To: Lee Jones; +Cc: groeck, gwendal, bleung, linux-kernel, kernel
From: Gwendal Grignou <gwendal@chromium.org>
Send dummy command to EC at resume time, wait for status.
If EC loses that command and return a status for the
pre-suspend command, we will not interpret that status as an
answer for an important command.
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
drivers/mfd/cros_ec_dev.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index fc98e0564061..c901839317ae 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -553,6 +553,16 @@ static __maybe_unused int ec_device_suspend(struct device *dev)
static __maybe_unused int ec_device_resume(struct device *dev)
{
struct cros_ec_dev *ec = dev_get_drvdata(dev);
+ char msg[sizeof(struct ec_response_get_version) +
+ sizeof(CROS_EC_DEV_VERSION)];
+ int ret;
+
+ /* Be sure the communication with the EC is reestablished */
+ ret = ec_get_version(ec, msg, sizeof(msg));
+ if (ret < 0) {
+ dev_err(ec->ec_dev->dev, "No EC response at resume: %d\n", ret);
+ return 0;
+ }
lb_resume(ec);
--
2.16.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 5/6] mfd: cros_ec_dev: Check communication with ec at resume
2018-02-19 22:41 [PATCH 5/6] mfd: cros_ec_dev: Check communication with ec at resume Enric Balletbo i Serra
@ 2018-02-22 2:20 ` Gwendal Grignou
2018-02-22 9:41 ` Enric Balletbo i Serra
0 siblings, 1 reply; 5+ messages in thread
From: Gwendal Grignou @ 2018-02-22 2:20 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Lee Jones, Guenter Roeck, Benson Leung, Linux Kernel, kernel
On Mon, Feb 19, 2018 at 2:41 PM, Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
> From: Gwendal Grignou <gwendal@chromium.org>
This patch is not needed anymore. It was added to
>
> Send dummy command to EC at resume time, wait for status.
> If EC loses that command and return a status for the
> pre-suspend command, we will not interpret that status as an
> answer for an important command.
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Hold on for this commit. We have only used it in 3.14 kernels. It
looks like the issue has been addressed by a more elegant solution at
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/0225a9de5971b99770344c04e0eba5b35ca68f71
> ---
> drivers/mfd/cros_ec_dev.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index fc98e0564061..c901839317ae 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -553,6 +553,16 @@ static __maybe_unused int ec_device_suspend(struct device *dev)
> static __maybe_unused int ec_device_resume(struct device *dev)
> {
> struct cros_ec_dev *ec = dev_get_drvdata(dev);
> + char msg[sizeof(struct ec_response_get_version) +
> + sizeof(CROS_EC_DEV_VERSION)];
> + int ret;
> +
> + /* Be sure the communication with the EC is reestablished */
> + ret = ec_get_version(ec, msg, sizeof(msg));
> + if (ret < 0) {
> + dev_err(ec->ec_dev->dev, "No EC response at resume: %d\n", ret);
> + return 0;
Besides, that looks wrong, we should try to resume the lightbar.
> + }
>
> lb_resume(ec);
>
> --
> 2.16.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5/6] mfd: cros_ec_dev: Check communication with ec at resume
2018-02-22 2:20 ` Gwendal Grignou
@ 2018-02-22 9:41 ` Enric Balletbo i Serra
2018-02-23 9:01 ` Gwendal Grignou
0 siblings, 1 reply; 5+ messages in thread
From: Enric Balletbo i Serra @ 2018-02-22 9:41 UTC (permalink / raw)
To: Gwendal Grignou
Cc: Lee Jones, Guenter Roeck, Benson Leung, Linux Kernel, kernel
Hi Gwendal,
On 22/02/18 03:20, Gwendal Grignou wrote:
> On Mon, Feb 19, 2018 at 2:41 PM, Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
>> From: Gwendal Grignou <gwendal@chromium.org>
> This patch is not needed anymore. It was added to
>>
>> Send dummy command to EC at resume time, wait for status.
>> If EC loses that command and return a status for the
>> pre-suspend command, we will not interpret that status as an
>> answer for an important command.
>>
>> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Hold on for this commit. We have only used it in 3.14 kernels. It
> looks like the issue has been addressed by a more elegant solution at
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/0225a9de5971b99770344c04e0eba5b35ca68f71
Just to make sure.
I don't see this patch reverted in current chromeos-4.4, seems that both are
applied. The patch is still there [1]. So, to double check, can I remove this
patch from this series and add only the one that moves the system sleep pm ops
to late?
[1]
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.4/drivers/platform/chrome/cros_ec_dev.c#782
>> ---
>> drivers/mfd/cros_ec_dev.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
>> index fc98e0564061..c901839317ae 100644
>> --- a/drivers/mfd/cros_ec_dev.c
>> +++ b/drivers/mfd/cros_ec_dev.c
>> @@ -553,6 +553,16 @@ static __maybe_unused int ec_device_suspend(struct device *dev)
>> static __maybe_unused int ec_device_resume(struct device *dev)
>> {
>> struct cros_ec_dev *ec = dev_get_drvdata(dev);
>> + char msg[sizeof(struct ec_response_get_version) +
>> + sizeof(CROS_EC_DEV_VERSION)];
>> + int ret;
>> +
>> + /* Be sure the communication with the EC is reestablished */
>> + ret = ec_get_version(ec, msg, sizeof(msg));
>> + if (ret < 0) {
>> + dev_err(ec->ec_dev->dev, "No EC response at resume: %d\n", ret);
>> + return 0;
> Besides, that looks wrong, we should try to resume the lightbar.
>> + }
>>
>> lb_resume(ec);
>>
>> --
>> 2.16.1
>>
Thanks,
Enric
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5/6] mfd: cros_ec_dev: Check communication with ec at resume
2018-02-22 9:41 ` Enric Balletbo i Serra
@ 2018-02-23 9:01 ` Gwendal Grignou
2018-02-23 9:04 ` Enric Balletbo i Serra
0 siblings, 1 reply; 5+ messages in thread
From: Gwendal Grignou @ 2018-02-23 9:01 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Gwendal Grignou, Lee Jones, Guenter Roeck, Benson Leung,
Linux Kernel, kernel
On Thu, Feb 22, 2018 at 1:41 AM, Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
> Hi Gwendal,
>
> On 22/02/18 03:20, Gwendal Grignou wrote:
>> On Mon, Feb 19, 2018 at 2:41 PM, Enric Balletbo i Serra
>> <enric.balletbo@collabora.com> wrote:
>>> From: Gwendal Grignou <gwendal@chromium.org>
>> This patch is not needed anymore. It was added to
>>>
>>> Send dummy command to EC at resume time, wait for status.
>>> If EC loses that command and return a status for the
>>> pre-suspend command, we will not interpret that status as an
>>> answer for an important command.
>>>
>>> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> Hold on for this commit. We have only used it in 3.14 kernels. It
>> looks like the issue has been addressed by a more elegant solution at
>> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/0225a9de5971b99770344c04e0eba5b35ca68f71
>
> Just to make sure.
>
> I don't see this patch reverted in current chromeos-4.4, seems that both are
> applied. The patch is still there [1]. So, to double check, can I remove this
> patch from this series and add only the one that moves the system sleep pm ops
> to late?
You're right, it is still in chromeos-4.4. If it is still really
needed, we will apply it later.
Thanks,
Gwendal.
>
> [1]
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.4/drivers/platform/chrome/cros_ec_dev.c#782
>
>>> ---
>>> drivers/mfd/cros_ec_dev.c | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
>>> index fc98e0564061..c901839317ae 100644
>>> --- a/drivers/mfd/cros_ec_dev.c
>>> +++ b/drivers/mfd/cros_ec_dev.c
>>> @@ -553,6 +553,16 @@ static __maybe_unused int ec_device_suspend(struct device *dev)
>>> static __maybe_unused int ec_device_resume(struct device *dev)
>>> {
>>> struct cros_ec_dev *ec = dev_get_drvdata(dev);
>>> + char msg[sizeof(struct ec_response_get_version) +
>>> + sizeof(CROS_EC_DEV_VERSION)];
>>> + int ret;
>>> +
>>> + /* Be sure the communication with the EC is reestablished */
>>> + ret = ec_get_version(ec, msg, sizeof(msg));
>>> + if (ret < 0) {
>>> + dev_err(ec->ec_dev->dev, "No EC response at resume: %d\n", ret);
>>> + return 0;
>> Besides, that looks wrong, we should try to resume the lightbar.
>>> + }
>>>
>>> lb_resume(ec);
>>>
>>> --
>>> 2.16.1
>>>
>
> Thanks,
> Enric
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5/6] mfd: cros_ec_dev: Check communication with ec at resume
2018-02-23 9:01 ` Gwendal Grignou
@ 2018-02-23 9:04 ` Enric Balletbo i Serra
0 siblings, 0 replies; 5+ messages in thread
From: Enric Balletbo i Serra @ 2018-02-23 9:04 UTC (permalink / raw)
To: Gwendal Grignou
Cc: Gwendal Grignou, Lee Jones, Guenter Roeck, Benson Leung,
Linux Kernel, kernel
Hi Gwendal,
On 23/02/18 10:01, Gwendal Grignou wrote:
> On Thu, Feb 22, 2018 at 1:41 AM, Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
>> Hi Gwendal,
>>
>> On 22/02/18 03:20, Gwendal Grignou wrote:
>>> On Mon, Feb 19, 2018 at 2:41 PM, Enric Balletbo i Serra
>>> <enric.balletbo@collabora.com> wrote:
>>>> From: Gwendal Grignou <gwendal@chromium.org>
>>> This patch is not needed anymore. It was added to
>>>>
>>>> Send dummy command to EC at resume time, wait for status.
>>>> If EC loses that command and return a status for the
>>>> pre-suspend command, we will not interpret that status as an
>>>> answer for an important command.
>>>>
>>>> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
>>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>> Hold on for this commit. We have only used it in 3.14 kernels. It
>>> looks like the issue has been addressed by a more elegant solution at
>>> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/0225a9de5971b99770344c04e0eba5b35ca68f71
>>
>> Just to make sure.
>>
>> I don't see this patch reverted in current chromeos-4.4, seems that both are
>> applied. The patch is still there [1]. So, to double check, can I remove this
>> patch from this series and add only the one that moves the system sleep pm ops
>> to late?
> You're right, it is still in chromeos-4.4. If it is still really
> needed, we will apply it later.
> Thanks,
>
Sounds good, I'll resend another version removing this patch and adding the
other. Let's leave this patch in standby.
Enric
> Gwendal.
>>
>> [1]
>> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.4/drivers/platform/chrome/cros_ec_dev.c#782
>>
>>>> ---
>>>> drivers/mfd/cros_ec_dev.c | 10 ++++++++++
>>>> 1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
>>>> index fc98e0564061..c901839317ae 100644
>>>> --- a/drivers/mfd/cros_ec_dev.c
>>>> +++ b/drivers/mfd/cros_ec_dev.c
>>>> @@ -553,6 +553,16 @@ static __maybe_unused int ec_device_suspend(struct device *dev)
>>>> static __maybe_unused int ec_device_resume(struct device *dev)
>>>> {
>>>> struct cros_ec_dev *ec = dev_get_drvdata(dev);
>>>> + char msg[sizeof(struct ec_response_get_version) +
>>>> + sizeof(CROS_EC_DEV_VERSION)];
>>>> + int ret;
>>>> +
>>>> + /* Be sure the communication with the EC is reestablished */
>>>> + ret = ec_get_version(ec, msg, sizeof(msg));
>>>> + if (ret < 0) {
>>>> + dev_err(ec->ec_dev->dev, "No EC response at resume: %d\n", ret);
>>>> + return 0;
>>> Besides, that looks wrong, we should try to resume the lightbar.
>>>> + }
>>>>
>>>> lb_resume(ec);
>>>>
>>>> --
>>>> 2.16.1
>>>>
>>
>> Thanks,
>> Enric
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-02-23 9:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-19 22:41 [PATCH 5/6] mfd: cros_ec_dev: Check communication with ec at resume Enric Balletbo i Serra
2018-02-22 2:20 ` Gwendal Grignou
2018-02-22 9:41 ` Enric Balletbo i Serra
2018-02-23 9:01 ` Gwendal Grignou
2018-02-23 9:04 ` Enric Balletbo i Serra
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.