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 for Android: free password hash cracker in your pocket

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

Message-ID: <CA+CmpXu27CfWeEN2F6YrtVzHTQtas6KyTk6dz4jaeo8LmckHWQ@mail.gmail.com>
Date:   Fri, 5 Jul 2019 18:02:23 +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 <raanan.avargil@...el.com>
Subject: Re: [PATCH 7/8] thunderbolt: Add support for Intel Ice Lake

On Fri, Jul 5, 2019 at 5:51 PM Mika Westerberg
<mika.westerberg@...ux.intel.com> wrote:
>
> > > +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.
>
> nhi_power_down() should be only called if no device was connected so it
> should be in correct place. I can try to clarify the comment a bit,
> though.

Maybe adding the word "both" ("to perform both an additional") will make it
clearer. Maybe removing the "additional" (which to my ears sounds like "an
additional operation besides the normal one, to unforce power") is enough.
Again, your call. I'm not sure it's strictly needed, maybe it's just me.

Thanks!