[v2,2/2] arm: imx: imx8m: soc: replace ifdef by IS_ENABLED()
diff mbox series

Message ID 20250515133035.815572-2-dario.binacchi@amarulasolutions.com
State New
Headers show
Series
  • [v2,1/2] arm: imx: imx8m: soc: fix the macro name
Related show

Commit Message

Dario Binacchi May 15, 2025, 1:30 p.m. UTC 
Standardize on using the IS_ENABLED macro.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

---

(no changes since v1)

 arch/arm/mach-imx/imx8m/soc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

'Thomas Petazzoni' via Amarula Linux May 15, 2025, 1:40 p.m. UTC | #1 
Hi Dario,

On 5/15/25 3:30 PM, Dario Binacchi wrote:
> Standardize on using the IS_ENABLED macro.
> 
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> 
> ---
> 
> (no changes since v1)
> 
>   arch/arm/mach-imx/imx8m/soc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
> index 806adcf145fa..6c53555d22bf 100644
> --- a/arch/arm/mach-imx/imx8m/soc.c
> +++ b/arch/arm/mach-imx/imx8m/soc.c
> @@ -791,7 +791,7 @@ int boot_mode_getprisec(void)
>   #endif
>   
>   #if defined(CONFIG_IMX8MN) || defined(CONFIG_IMX8MP)

You can even do those two as well :)

and all the others in this file :) I would recommend to only do for 
those that start with CONFIG_ which are Kconfig symbols. E.g. 
PHYS_SDRAM_2_SIZE isn't, so I don't think you can use IS_ENABLED.

The easiest way to check everything is fine is to compile before and 
after patching the file and checking that the output is bit to bit 
identical (you may need to change the SOURCE_DATE_EPOCH to guarantee 
that though?

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
Tom Rini May 15, 2025, 2:38 p.m. UTC | #2 
On Thu, May 15, 2025 at 03:40:15PM +0200, Quentin Schulz wrote:
> Hi Dario,
> 
> On 5/15/25 3:30 PM, Dario Binacchi wrote:
> > Standardize on using the IS_ENABLED macro.
> > 
> > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > 
> > ---
> > 
> > (no changes since v1)
> > 
> >   arch/arm/mach-imx/imx8m/soc.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
> > index 806adcf145fa..6c53555d22bf 100644
> > --- a/arch/arm/mach-imx/imx8m/soc.c
> > +++ b/arch/arm/mach-imx/imx8m/soc.c
> > @@ -791,7 +791,7 @@ int boot_mode_getprisec(void)
> >   #endif
> >   #if defined(CONFIG_IMX8MN) || defined(CONFIG_IMX8MP)
> 
> You can even do those two as well :)
> 
> and all the others in this file :) I would recommend to only do for those
> that start with CONFIG_ which are Kconfig symbols. E.g. PHYS_SDRAM_2_SIZE
> isn't, so I don't think you can use IS_ENABLED.
> 
> The easiest way to check everything is fine is to compile before and after
> patching the file and checking that the output is bit to bit identical (you
> may need to change the SOURCE_DATE_EPOCH to guarantee that though?

A change like that would just be code churn and I'd rather not see it.
Especially since yes, you *can* get away with IS_ENABLED(FOO) and the
kernel does it in a few places, but you shouldn't do that in U-Boot as
it's more likely to be an error (you wanted IS_ENABLED(CONFIG_FOO)) than
a helpful case of:
        if (IS_ENABLED(FOO)) { ... }

Patch
diff mbox series 

diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index 806adcf145fa..6c53555d22bf 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -791,7 +791,7 @@  int boot_mode_getprisec(void)
 #endif
 
 #if defined(CONFIG_IMX8MN) || defined(CONFIG_IMX8MP)
-#ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
+#if IS_ENABLED(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION)
 #define IMG_CNTN_SET1_OFFSET	GENMASK(22, 19)
 unsigned long arch_spl_mmc_get_uboot_raw_sector(struct mmc *mmc,
 						unsigned long raw_sect)