linux-kernel - Re: [PATCH v2 1/2] arm64: dts: renesas: r9a07g044: Use SoC specific macro for CPG and RESET

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:   Wed, 8 Mar 2023 13:56:01 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Prabhakar <prabhakar.csengg@...il.com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Magnus Damm <magnus.damm@...il.com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-renesas-soc@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Biju Das <biju.das.jz@...renesas.com>,
        Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v2 1/2] arm64: dts: renesas: r9a07g044: Use SoC specific
 macro for CPG and RESET

On 31/01/2023 23:35, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> 
> Use a SoC specific macro for CPG and RESET so that we can re-use the
> RZ/G2L SoC DTSI for RZ/V2L SoC by just updating the SoC specific macro.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> ---
> v1->v2
> * No change
> ---
>  arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 254 +++++++++++----------
>  1 file changed, 129 insertions(+), 125 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> index 487536696d90..80b2332798d9 100644
> --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> @@ -1,12 +1,16 @@
>  // SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>  /*
> - * Device Tree Source for the RZ/G2L and RZ/G2LC common SoC parts
> + * Device Tree Source for the RZ/G2L, RZ/G2LC and RZ/V2L common SoC parts
>   *
>   * Copyright (C) 2021 Renesas Electronics Corp.
>   */
>  
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +#ifndef SOC_CPG_PREFIX
>  #include <dt-bindings/clock/r9a07g044-cpg.h>
> +#define SOC_CPG_PREFIX(X)	R9A07G044_ ## X
> +#endif
>  

I don't oppose it, but since I was asked on IRC about opinion, let me
share my feelings also here:

This patch #1 makes git grep difficult, reading code also a bit more
tricky. It's opposite of readability. The patchset obfuscates which
clocks will be used on r9a07g054 (so patch #2, the one which includes
other DTSI). To figure this out, I need to decode the macro for in one
file and  then open second file... but the prefix is also defined there!
Same constant in two places. So order of inclusion also matters?
Confusing code.

If the IDs and clocks are the same, maybe the constants/names in headers
should be unified?

Best regards,
Krzysztof