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: <20211005171728.3147094-1-lucas.demarchi@intel.com>
In-Reply-To: <20211005171728.3147094-1-lucas.demarchi@intel.com>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Fri, 8 Oct 2021 00:49:44 +0900
Message-ID: <CAK7LNARbNQHPxHHp4=oLOsJLpaCv0c3NRHGSs6hAKnP3N4DHKQ@mail.gmail.com>
Subject: Re: [PATCH v3] drm/i915: remove IS_ACTIVE
To:     Lucas De Marchi <lucas.demarchi@intel.com>
Cc:     intel-gfx <intel-gfx@lists.freedesktop.org>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Dan Carpenter <dan.carpenter@oracle.com>,
        Steven Price <steven.price@arm.com>,
        Chris Wilson <chris@chris-wilson.co.uk>,
        Andrzej Hajda <a.hajda@samsung.com>,
        Jani Nikula <jani.nikula@intel.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Oct 6, 2021 at 2:21 AM Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>
> When trying to bring IS_ACTIVE to linux/kconfig.h I thought it wouldn't
> provide much value just encapsulating it in a boolean context. So I also
> added the support for handling undefined macros as the IS_ENABLED()
> counterpart. However the feedback received from Masahiro Yamada was that
> it is too ugly, not providing much value. And just wrapping in a boolean
> context is too dumb - we could simply open code it.
>
> As detailed in commit babaab2f4738 ("drm/i915: Encapsulate kconfig
> constant values inside boolean predicates"), the IS_ACTIVE macro was
> added to workaround a compilation warning. However after checking again
> our current uses of IS_ACTIVE it turned out there is only
> 1 case in which it triggers a warning in clang (due
> -Wconstant-logical-operand) and 2 in smatch. All the others
> can simply use the shorter version, without wrapping it in any macro.
>
> So here I'm dialing all the way back to simply removing the macro. That
> single case hit by clang can be changed to make the constant come first,
> so it doesn't think it's mask:
>
>         -       if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
>         +       if (CONFIG_DRM_I915_FENCE_TIMEOUT && context)
>
> As talked with Dan Carpenter, that logic will be added in smatch as
> well, so it will also stop warning about it.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Masahiro Yamada <masahiroy@kernel.org>


-- 
Best Regards
Masahiro Yamada