* [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 [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®