Hello Uwe, Thanks for your feedback.
On 09/06/2016 08:42 AM, Uwe Kleine-König wrote: > Hello Javier, > > On Mon, Aug 29, 2016 at 11:32:11AM +0200, Javier Martinez Canillas wrote: >> From: Javier Martinez Canillas <[email protected]> >> >> The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either >> built-in or as a module, use that macro instead of open coding the same. >> >> Using the macro makes the code more readable by helping abstract away some >> of the Kconfig built-in and module enable details. >> >> Signed-off-by: Javier Martinez Canillas <[email protected]> >> Signed-off-by: Javier Martinez Canillas <[email protected]> > > The following patch should do the same (untested though) and the source > looks still better: > Yes, your patch looks good to me. But is doing two things at once (using the IS_ENABLED macro and getting rid of the stub functions / ifdefery) so I would split it in two different patches. In any case, $SUBJECT was picked by Shawn already so you could post the other change on top of it if you want. > diff --git a/arch/arm/mach-imx/mach-kzm_arm11_01.c > b/arch/arm/mach-imx/mach-kzm_arm11_01.c > index 31df4361996f..48f794dbf293 100644 > --- a/arch/arm/mach-imx/mach-kzm_arm11_01.c > +++ b/arch/arm/mach-imx/mach-kzm_arm11_01.c > @@ -63,7 +63,6 @@ > */ > #define KZM_ARM11_16550 (MX31_CS4_BASE_ADDR + 0x1050) > > -#if defined(CONFIG_SERIAL_8250) || defined(CONFIG_SERIAL_8250_MODULE) > /* > * KZM-ARM11-01 has an external UART on FPGA > */ > @@ -106,37 +105,35 @@ static struct platform_device serial_device = { > > static int __init kzm_init_ext_uart(void) > { > - u8 tmp; > - > - /* > - * GPIO 1-1: external UART interrupt line > - */ > - mxc_iomux_mode(IOMUX_MODE(MX31_PIN_GPIO1_1, IOMUX_CONFIG_GPIO)); > - gpio_request(IOMUX_TO_GPIO(MX31_PIN_GPIO1_1), "ext-uart-int"); > - gpio_direction_input(IOMUX_TO_GPIO(MX31_PIN_GPIO1_1)); > - > - /* > - * Unmask UART interrupt > - */ > - tmp = __raw_readb(KZM_ARM11_IO_ADDRESS(KZM_ARM11_CTL1)); > - tmp |= 0x2; > - __raw_writeb(tmp, KZM_ARM11_IO_ADDRESS(KZM_ARM11_CTL1)); > - > - serial_platform_data[0].irq = > - gpio_to_irq(IOMUX_TO_GPIO(MX31_PIN_GPIO1_1)); > - serial8250_resources[1].start = > - gpio_to_irq(IOMUX_TO_GPIO(MX31_PIN_GPIO1_1)); > - serial8250_resources[1].end = > - gpio_to_irq(IOMUX_TO_GPIO(MX31_PIN_GPIO1_1)); > - > - return platform_device_register(&serial_device); > + if (IS_ENABLED(CONFIG_SERIAL_8250)) { > + u8 tmp; > + > + /* > + * GPIO 1-1: external UART interrupt line > + */ > + mxc_iomux_mode(IOMUX_MODE(MX31_PIN_GPIO1_1, IOMUX_CONFIG_GPIO)); > + gpio_request(IOMUX_TO_GPIO(MX31_PIN_GPIO1_1), "ext-uart-int"); > + gpio_direction_input(IOMUX_TO_GPIO(MX31_PIN_GPIO1_1)); > + > + /* > + * Unmask UART interrupt > + */ > + tmp = __raw_readb(KZM_ARM11_IO_ADDRESS(KZM_ARM11_CTL1)); > + tmp |= 0x2; > + __raw_writeb(tmp, KZM_ARM11_IO_ADDRESS(KZM_ARM11_CTL1)); > + > + serial_platform_data[0].irq = > + gpio_to_irq(IOMUX_TO_GPIO(MX31_PIN_GPIO1_1)); > + serial8250_resources[1].start = > + gpio_to_irq(IOMUX_TO_GPIO(MX31_PIN_GPIO1_1)); > + serial8250_resources[1].end = > + gpio_to_irq(IOMUX_TO_GPIO(MX31_PIN_GPIO1_1)); > + > + return platform_device_register(&serial_device); > + } else { > + return 0; > + } > } > -#else > -static inline int kzm_init_ext_uart(void) > -{ > - return 0; > -} > -#endif > > /* > * SMSC LAN9118 > @@ -178,44 +175,39 @@ static struct regulator_consumer_supply > dummy_supplies[] = { > > static int __init kzm_init_smsc9118(void) > { > - /* > - * GPIO 1-2: SMSC9118 interrupt line > - */ > - mxc_iomux_mode(IOMUX_MODE(MX31_PIN_GPIO1_2, IOMUX_CONFIG_GPIO)); > - gpio_request(IOMUX_TO_GPIO(MX31_PIN_GPIO1_2), "smsc9118-int"); > - gpio_direction_input(IOMUX_TO_GPIO(MX31_PIN_GPIO1_2)); > - > - regulator_register_fixed(0, dummy_supplies, ARRAY_SIZE(dummy_supplies)); > - > - kzm_smsc9118_resources[1].start = > - gpio_to_irq(IOMUX_TO_GPIO(MX31_PIN_GPIO1_2)); > - kzm_smsc9118_resources[1].end = > - gpio_to_irq(IOMUX_TO_GPIO(MX31_PIN_GPIO1_2)); > - > - return platform_device_register(&kzm_smsc9118_device); > + if (IS_ENABLED(CONFIG_SMSC911X)) { > + /* > + * GPIO 1-2: SMSC9118 interrupt line > + */ > + mxc_iomux_mode(IOMUX_MODE(MX31_PIN_GPIO1_2, IOMUX_CONFIG_GPIO)); > + gpio_request(IOMUX_TO_GPIO(MX31_PIN_GPIO1_2), "smsc9118-int"); > + gpio_direction_input(IOMUX_TO_GPIO(MX31_PIN_GPIO1_2)); > + > + regulator_register_fixed(0, dummy_supplies, > + ARRAY_SIZE(dummy_supplies)); > + > + kzm_smsc9118_resources[1].start = > + gpio_to_irq(IOMUX_TO_GPIO(MX31_PIN_GPIO1_2)); > + kzm_smsc9118_resources[1].end = > + gpio_to_irq(IOMUX_TO_GPIO(MX31_PIN_GPIO1_2)); > + > + return platform_device_register(&kzm_smsc9118_device); > + } else { > + return 0; > + } > } > -#else > -static inline int kzm_init_smsc9118(void) > -{ > - return 0; > -} > -#endif > > -#if defined(CONFIG_SERIAL_IMX) || defined(CONFIG_SERIAL_IMX_MODULE) > static const struct imxuart_platform_data uart_pdata __initconst = { > .flags = IMXUART_HAVE_RTSCTS, > }; > > static void __init kzm_init_imx_uart(void) > { > - imx31_add_imx_uart0(&uart_pdata); > - imx31_add_imx_uart1(&uart_pdata); > -} > -#else > -static inline void kzm_init_imx_uart(void) > -{ > + if (IS_ENABLED(CONFIG_SERIAL_IMX)) { > + imx31_add_imx_uart0(&uart_pdata); > + imx31_add_imx_uart1(&uart_pdata); > + } > } > -#endif > > static int kzm_pins[] __initdata = { > MX31_PIN_CTS1__CTS1, > > Having said that, just dropping the #ifdefs and registering the devices > even without the matching driver is fine for me, too. > > Best regards > Uwe > Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America
