linux-kernel - Re: [PATCH 7/8] thunderbolt: Add support for Intel Ice Lake

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]

Date:   Fri, 5 Jul 2019 17:44:01 +0300
From:   Yehezkel Bernat <yehezkelshb@...il.com>
To:     Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Andreas Noever <andreas.noever@...il.com>,
        Michael Jamet <michael.jamet@...el.com>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>, Lukas Wunner <lukas@...ner.de>,
        Mario Limonciello <Mario.Limonciello@...l.com>,
        Anthony Wong <anthony.wong@...onical.com>,
        linux-acpi@...r.kernel.org, raanan.avargil@...el.com
Subject: Re: [PATCH 7/8] thunderbolt: Add support for Intel Ice Lake

On Fri, Jul 5, 2019 at 12:58 PM Mika Westerberg
<mika.westerberg@...ux.intel.com> wrote:
>
> +static void icm_icl_rtd3_veto(struct tb *tb, const struct icm_pkg_header *hdr)
> +{
> +       const struct icm_icl_event_rtd3_veto *pkg =
> +               (const struct icm_icl_event_rtd3_veto *)hdr;
> +       struct icm *icm = tb_priv(tb);
> +
> +       tb_dbg(tb, "ICM rtd3 veto=0x%08x\n", pkg->veto_reason);
> +
> +       if (pkg->veto_reason) {
> +               if (!icm->veto) {
> +                       icm->veto = true;
> +                       /* Keep the domain powered while veto is in effect */
> +                       pm_runtime_get(&tb->dev);
> +               }
> +       } else {
> +               if (icm->veto) {
> +                       icm->veto = false;
> +                       /* Allow the domain suspend now */
> +                       pm_runtime_mark_last_busy(&tb->dev);
> +                       pm_runtime_put_autosuspend(&tb->dev);

Handling the removal of the veto is duplicated below. Worth introducing as a
helper function?

> +               }
> +       }
> +}
> +

...

> @@ -1853,6 +1943,18 @@ static void icm_complete(struct tb *tb)
>         if (tb->nhi->going_away)
>                 return;
>
> +       /*
> +        * If RTD3 was vetoed before we entered system suspend allow it
> +        * again now before driver ready is sent. Firmware sends a new RTD3
> +        * veto if it is still the case after we have sent it driver ready
> +        * command.
> +        */
> +       if (icm->veto) {
> +               icm->veto = false;
> +               pm_runtime_mark_last_busy(&tb->dev);
> +               pm_runtime_put_autosuspend(&tb->dev);
> +       }
> +

Here is the duplication.


> +static int nhi_suspend_power_down(struct tb *tb)
> +{
> +       int ret;
> +
> +       /*
> +        * If there is no device connected we need to perform an additional
> +        * handshake through LC mailbox and force power down before
> +        * entering D3.
> +        */
> +       ret = device_for_each_child(&tb->root_switch->dev, NULL,
> +                                   nhi_device_connected);
> +       if (!ret) {
> +               lc_mailbox_cmd(tb->nhi, LC_PREPARE_FOR_RESET);
> +               ret = lc_mailbox_cmd_complete(tb->nhi,
> +                                             LC_MAILBOX_TIMEOUT);
> +               if (ret)
> +                       return ret;
> +
> +               return nhi_power_down(tb->nhi);

Just to be sure: unforce power is done only if no device is connected?
My understanding of the comment above was that unforce power should be done
anyway (so it should be outside of this if block), and the difference between
the cases is only about the additional LC mailbox message. I guess I misread it.

> +       }
> +
> +       return 0;
> +}
> +