linux-kernel - Re: [PATCH 5/6] mfd: cros_ec_dev: Check communication with ec at resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <42a823e5-ba12-2060-a2bd-44027261e26f@collabora.com>
Date:   Thu, 22 Feb 2018 10:41:57 +0100
From:   Enric Balletbo i Serra <enric.balletbo@...labora.com>
To:     Gwendal Grignou <gwendal@...omium.org>
Cc:     Lee Jones <lee.jones@...aro.org>,
        Guenter Roeck <groeck@...omium.org>,
        Benson Leung <bleung@...omium.org>,
        Linux Kernel <linux-kernel@...r.kernel.org>,
        kernel@...labora.com
Subject: Re: [PATCH 5/6] mfd: cros_ec_dev: Check communication with ec at
 resume

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@...labora.com> wrote:
>> From: Gwendal Grignou <gwendal@...omium.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@...omium.org>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@...labora.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