linux-kernel - Re: [PATCH] drm/i915/gt: Remove {} from if-else

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] [day] [month] [year] [list]

Message-ID: <adbd431d-39b7-8026-bc38-4741bc037ef4@intel.com>
Date:   Thu, 26 Oct 2023 10:03:27 +0200
From:   Karolina Stolarek <karolina.stolarek@...el.com>
To:     Soumya Negi <soumya.negi97@...il.com>
CC:     Andi Shyti <andi.shyti@...el.com>,
        Prathap Kumar Valsan <prathap.kumar.valsan@...el.com>,
        Aravind Iddamsetty <aravind.iddamsetty@...el.com>,
        <intel-gfx@...ts.freedesktop.org>,
        <dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/i915/gt: Remove {} from if-else

On 26.10.2023 06:43, Soumya Negi wrote:
> In accordance to Linux coding style(Documentation/process/4.Coding.rst),
> remove unneeded braces from if-else block as all arms of this block
> contain single statements.

I'd just keep the description simple, and say that braces are not needed
for single line statements.

The patch looks fine to me. Andi, if you decide to merge it, feel free
to add my ack.

While we're here, I wanted briefly discuss how to construct To and CC
when working on i915 code. These are not hard rules (and some developers
might disagree with me), but suggestions on how to get the right people
look at your code and reduce the noise (decided to drop maintainers;
they'll be able to join the conversation from their subscription to ML)

First of all, if you work on something in i915 that only touches this
driver, you should submit it to intel-gfx, and there's no need to
include dri-devel. You can, but that mailing list is mostly used for
changes that are either for DRM or impact other drivers.

Secondly, try to include only people who are directly involved and
potential reviewers. You can CC maintainers for bigger changes that
require their involvement, but here, it's enough to include Andi, myself
and someone who added this piece of code.

So, if it was my patch, I'd have intel-gfx in To: and Andi, Prathap and
myself in Cc:. get_maintainer.pl script might've added a lot more people
there, so I'd move away from using it, and only include developers that
are involved or interested in your work. You can always reach out to
Andi and me before sending your patches, if you have any doubts.

All the best,
Karolina

> 
> Suggested-by: Andi Shyti <andi.shyti@...el.com>
> Signed-off-by: Soumya Negi <soumya.negi97@...il.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_ggtt.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> index 1c93e84278a0..9f6f9e138532 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> @@ -226,16 +226,15 @@ static void guc_ggtt_invalidate(struct i915_ggtt *ggtt)
>   	gen8_ggtt_invalidate(ggtt);
>   
>   	list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) {
> -		if (intel_guc_tlb_invalidation_is_available(&gt->uc.guc)) {
> +		if (intel_guc_tlb_invalidation_is_available(&gt->uc.guc))
>   			guc_ggtt_ct_invalidate(gt);
> -		} else if (GRAPHICS_VER(i915) >= 12) {
> +		else if (GRAPHICS_VER(i915) >= 12)
>   			intel_uncore_write_fw(gt->uncore,
>   					      GEN12_GUC_TLB_INV_CR,
>   					      GEN12_GUC_TLB_INV_CR_INVALIDATE);
> -		} else {
> +		else
>   			intel_uncore_write_fw(gt->uncore,
>   					      GEN8_GTCR, GEN8_GTCR_INVALIDATE);
> -		}
>   	}
>   }
>