Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CAPM=9txyMmW1DWhS--SuYQu4qDK1GPzgHJwxbAfhHT=hUsPODA@mail.gmail.com>
 <CAHk-=wiP+S4K3yU+FqgGmF+EgcSgk04Eerf3gE-6qObicy346w@mail.gmail.com>
In-Reply-To: <CAHk-=wiP+S4K3yU+FqgGmF+EgcSgk04Eerf3gE-6qObicy346w@mail.gmail.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Thu, 15 Oct 2020 11:42:22 -0700
Message-ID: <CAHk-=wgYNzhoYuUBsBcFq1TFA5C+Bie-2uzZkF2McO9joXxW0A@mail.gmail.com>
Subject: Re: [git pull] drm next pull for 5.10-rc1
To:     Dave Airlie <airlied@gmail.com>, Eryk Brol <eryk.brol@amd.com>
Cc:     Daniel Vetter <daniel.vetter@ffwll.ch>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Oct 15, 2020 at 10:51 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Thanks, looks good to me [..]

Uhhuh. I already pushed things out, but my clang build (which I don't
do between each merge) shows a problem:

  drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:650:8:
  warning: logical not is only applied to the left hand side of this
comparison [-Wlogical-not-parentheses]
                && !params[i].clock_force_enable == DSC_CLK_FORCE_DEFAULT) {
                   ^                             ~~

and I think clang is entirely right to complain about that code.

Yes, the code may be correct, but even if it's correct, that's a
really odd way to write things.

Anyway, what it does is:

   !params[i].clock_force_enable

turns 0 into 1, and anything that isn't 0 into 0.

And DSC_CLK_FORCE_DEFAULT has a value of 0, so what that line actually means is

  (params[i].clock_force_enable == 0) == 0

which obviously is

  params[i].clock_force_enable != 0

which in this case is the same as

  params[i].clock_force_enable != DSC_CLK_FORCE_DEFAULT

which may be what the code actually meant to do.

So I suspect it does what it meant to do, but only because
DSC_CLK_FORCE_DEFAULT also happens to be 0, which also acts as a
boolean 'false'.

But it's also possible that the '!' is a left-over, and the code
actually _meant_ to do the exact reverse of that. I have no idea.

This odd code was introduced by commit 0749ddeb7d6c ("drm/amd/display:
Add DSC force disable to dsc_clock_en debugfs entry"), and can we
please agree to not write this kind of obfuscated C code?

               Linus