| Message ID | 1412087695-10591-2-git-send-email-a.kesavan@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tuesday 30 September 2014 20:04:55 Abhilash Kesavan wrote: > --- a/arch/arm/mach-s3c64xx/irq-pm.c > +++ b/arch/arm/mach-s3c64xx/irq-pm.c > @@ -55,10 +55,10 @@ static struct irq_grp_save { > u32 mask; > } eint_grp_save[5]; > > -#ifndef CONFIG_SERIAL_SAMSUNG_UARTS > -#define SERIAL_SAMSUNG_UARTS 0 > +#ifndef CONFIG_SERIAL_SAMSUNG > +#define SERIAL_SAMSUNG_UARTS 0 > #else > -#define SERIAL_SAMSUNG_UARTS CONFIG_SERIAL_SAMSUNG_UARTS > +#define SERIAL_SAMSUNG_UARTS 4 > #endif > > static u32 irq_uart_mask[SERIAL_SAMSUNG_UARTS]; I think this won't work because now you access invalid registers on machines that have only three uarts. > diff --git a/arch/arm/plat-samsung/init.c b/arch/arm/plat-samsung/init.c > index 11fbbc2..03cafe9 100644 > --- a/arch/arm/plat-samsung/init.c > +++ b/arch/arm/plat-samsung/init.c > @@ -93,8 +93,8 @@ void __init s3c24xx_init_clocks(int xtal) > #if IS_ENABLED(CONFIG_SAMSUNG_ATAGS) > static int nr_uarts __initdata = 0; > > -#ifdef CONFIG_SERIAL_SAMSUNG_UARTS > -static struct s3c2410_uartcfg uart_cfgs[CONFIG_SERIAL_SAMSUNG_UARTS]; > +#ifdef CONFIG_SERIAL_SAMSUNG > +static struct s3c2410_uartcfg uart_cfgs[4]; > #endif > > /* s3c24xx_init_uartdevs > @@ -110,7 +110,7 @@ void __init s3c24xx_init_uartdevs(char *name, > struct s3c24xx_uart_resources *res, > struct s3c2410_uartcfg *cfg, int no) > { > -#ifdef CONFIG_SERIAL_SAMSUNG_UARTS > +#ifdef CONFIG_SERIAL_SAMSUNG > struct platform_device *platdev; > struct s3c2410_uartcfg *cfgptr = uart_cfgs; > struct s3c24xx_uart_resources *resp; Since you hardcode the number here now, you can actually drop this #ifdef. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30.09.2014 17:13, Arnd Bergmann wrote: > On Tuesday 30 September 2014 20:04:55 Abhilash Kesavan wrote: >> --- a/arch/arm/mach-s3c64xx/irq-pm.c >> +++ b/arch/arm/mach-s3c64xx/irq-pm.c >> @@ -55,10 +55,10 @@ static struct irq_grp_save { >> u32 mask; >> } eint_grp_save[5]; >> >> -#ifndef CONFIG_SERIAL_SAMSUNG_UARTS >> -#define SERIAL_SAMSUNG_UARTS 0 >> +#ifndef CONFIG_SERIAL_SAMSUNG >> +#define SERIAL_SAMSUNG_UARTS 0 >> #else >> -#define SERIAL_SAMSUNG_UARTS CONFIG_SERIAL_SAMSUNG_UARTS >> +#define SERIAL_SAMSUNG_UARTS 4 >> #endif >> >> static u32 irq_uart_mask[SERIAL_SAMSUNG_UARTS]; > > I think this won't work because now you access invalid registers > on machines that have only three uarts. Both S3C6400 and S3C6410 SoCs have 4 UART blocks. AFAICT CONFIG_SERIAL_SAMSUNG_UARTS was always set to 4 on ARCH_S3C64XX. > >> diff --git a/arch/arm/plat-samsung/init.c b/arch/arm/plat-samsung/init.c >> index 11fbbc2..03cafe9 100644 >> --- a/arch/arm/plat-samsung/init.c >> +++ b/arch/arm/plat-samsung/init.c >> @@ -93,8 +93,8 @@ void __init s3c24xx_init_clocks(int xtal) >> #if IS_ENABLED(CONFIG_SAMSUNG_ATAGS) >> static int nr_uarts __initdata = 0; >> >> -#ifdef CONFIG_SERIAL_SAMSUNG_UARTS >> -static struct s3c2410_uartcfg uart_cfgs[CONFIG_SERIAL_SAMSUNG_UARTS]; >> +#ifdef CONFIG_SERIAL_SAMSUNG >> +static struct s3c2410_uartcfg uart_cfgs[4]; Abhilash: Instead of using 4 directly, you could define a constant for it. >> #endif >> >> /* s3c24xx_init_uartdevs >> @@ -110,7 +110,7 @@ void __init s3c24xx_init_uartdevs(char *name, >> struct s3c24xx_uart_resources *res, >> struct s3c2410_uartcfg *cfg, int no) >> { >> -#ifdef CONFIG_SERIAL_SAMSUNG_UARTS >> +#ifdef CONFIG_SERIAL_SAMSUNG >> struct platform_device *platdev; >> struct s3c2410_uartcfg *cfgptr = uart_cfgs; >> struct s3c24xx_uart_resources *resp; > > Since you hardcode the number here now, you can actually drop this #ifdef. I believe what Abhilash did is correct, because this code is not needed when there is no serial support enabled. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Arnd, On Tue, Sep 30, 2014 at 8:43 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Tuesday 30 September 2014 20:04:55 Abhilash Kesavan wrote: >> --- a/arch/arm/mach-s3c64xx/irq-pm.c >> +++ b/arch/arm/mach-s3c64xx/irq-pm.c >> @@ -55,10 +55,10 @@ static struct irq_grp_save { >> u32 mask; >> } eint_grp_save[5]; >> >> -#ifndef CONFIG_SERIAL_SAMSUNG_UARTS >> -#define SERIAL_SAMSUNG_UARTS 0 >> +#ifndef CONFIG_SERIAL_SAMSUNG >> +#define SERIAL_SAMSUNG_UARTS 0 >> #else >> -#define SERIAL_SAMSUNG_UARTS CONFIG_SERIAL_SAMSUNG_UARTS >> +#define SERIAL_SAMSUNG_UARTS 4 >> #endif >> >> static u32 irq_uart_mask[SERIAL_SAMSUNG_UARTS]; > > I think this won't work because now you access invalid registers > on machines that have only three uarts. The 6400 and 6410 SoCs both have 4 uarts. Prior to this patch CONFIG_SERIAL_SAMSUNG_UARTS would have been 4 for 64xx. > >> diff --git a/arch/arm/plat-samsung/init.c b/arch/arm/plat-samsung/init.c >> index 11fbbc2..03cafe9 100644 >> --- a/arch/arm/plat-samsung/init.c >> +++ b/arch/arm/plat-samsung/init.c >> @@ -93,8 +93,8 @@ void __init s3c24xx_init_clocks(int xtal) >> #if IS_ENABLED(CONFIG_SAMSUNG_ATAGS) >> static int nr_uarts __initdata = 0; >> >> -#ifdef CONFIG_SERIAL_SAMSUNG_UARTS >> -static struct s3c2410_uartcfg uart_cfgs[CONFIG_SERIAL_SAMSUNG_UARTS]; >> +#ifdef CONFIG_SERIAL_SAMSUNG >> +static struct s3c2410_uartcfg uart_cfgs[4]; >> #endif >> >> /* s3c24xx_init_uartdevs >> @@ -110,7 +110,7 @@ void __init s3c24xx_init_uartdevs(char *name, >> struct s3c24xx_uart_resources *res, >> struct s3c2410_uartcfg *cfg, int no) >> { >> -#ifdef CONFIG_SERIAL_SAMSUNG_UARTS >> +#ifdef CONFIG_SERIAL_SAMSUNG >> struct platform_device *platdev; >> struct s3c2410_uartcfg *cfgptr = uart_cfgs; >> struct s3c24xx_uart_resources *resp; > > Since you hardcode the number here now, you can actually drop this #ifdef. Will remove it. Regards, Abhilash > > Arnd > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 30 September 2014 18:10:14 Tomasz Figa wrote: > >> @@ -110,7 +110,7 @@ void __init s3c24xx_init_uartdevs(char *name, > >> struct s3c24xx_uart_resources *res, > >> struct s3c2410_uartcfg *cfg, int no) > >> { > >> -#ifdef CONFIG_SERIAL_SAMSUNG_UARTS > >> +#ifdef CONFIG_SERIAL_SAMSUNG > >> struct platform_device *platdev; > >> struct s3c2410_uartcfg *cfgptr = uart_cfgs; > >> struct s3c24xx_uart_resources *resp; > > > > Since you hardcode the number here now, you can actually drop this #ifdef. > > I believe what Abhilash did is correct, because this code is not needed > when there is no serial support enabled. > I only added the #ifdef here because it was broken when CONFIG_SERIAL_SAMSUNG_UARTS was undefined. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30.09.2014 20:36, Arnd Bergmann wrote: > On Tuesday 30 September 2014 18:10:14 Tomasz Figa wrote: >>>> @@ -110,7 +110,7 @@ void __init s3c24xx_init_uartdevs(char *name, >>>> struct s3c24xx_uart_resources *res, >>>> struct s3c2410_uartcfg *cfg, int no) >>>> { >>>> -#ifdef CONFIG_SERIAL_SAMSUNG_UARTS >>>> +#ifdef CONFIG_SERIAL_SAMSUNG >>>> struct platform_device *platdev; >>>> struct s3c2410_uartcfg *cfgptr = uart_cfgs; >>>> struct s3c24xx_uart_resources *resp; >>> >>> Since you hardcode the number here now, you can actually drop this #ifdef. >> >> I believe what Abhilash did is correct, because this code is not needed >> when there is no serial support enabled. >> > > I only added the #ifdef here because it was broken when > CONFIG_SERIAL_SAMSUNG_UARTS was undefined. Fair enough. This isn't really that much code to really care and most (if not all) use cases have UART enabled anyway. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-s3c64xx/irq-pm.c b/arch/arm/mach-s3c64xx/irq-pm.c index ae4ea76..b20024e 100644 --- a/arch/arm/mach-s3c64xx/irq-pm.c +++ b/arch/arm/mach-s3c64xx/irq-pm.c @@ -55,10 +55,10 @@ static struct irq_grp_save { u32 mask; } eint_grp_save[5]; -#ifndef CONFIG_SERIAL_SAMSUNG_UARTS -#define SERIAL_SAMSUNG_UARTS 0 +#ifndef CONFIG_SERIAL_SAMSUNG +#define SERIAL_SAMSUNG_UARTS 0 #else -#define SERIAL_SAMSUNG_UARTS CONFIG_SERIAL_SAMSUNG_UARTS +#define SERIAL_SAMSUNG_UARTS 4 #endif static u32 irq_uart_mask[SERIAL_SAMSUNG_UARTS]; diff --git a/arch/arm/plat-samsung/init.c b/arch/arm/plat-samsung/init.c index 11fbbc2..03cafe9 100644 --- a/arch/arm/plat-samsung/init.c +++ b/arch/arm/plat-samsung/init.c @@ -93,8 +93,8 @@ void __init s3c24xx_init_clocks(int xtal) #if IS_ENABLED(CONFIG_SAMSUNG_ATAGS) static int nr_uarts __initdata = 0; -#ifdef CONFIG_SERIAL_SAMSUNG_UARTS -static struct s3c2410_uartcfg uart_cfgs[CONFIG_SERIAL_SAMSUNG_UARTS]; +#ifdef CONFIG_SERIAL_SAMSUNG +static struct s3c2410_uartcfg uart_cfgs[4]; #endif /* s3c24xx_init_uartdevs @@ -110,7 +110,7 @@ void __init s3c24xx_init_uartdevs(char *name, struct s3c24xx_uart_resources *res, struct s3c2410_uartcfg *cfg, int no) { -#ifdef CONFIG_SERIAL_SAMSUNG_UARTS +#ifdef CONFIG_SERIAL_SAMSUNG struct platform_device *platdev; struct s3c2410_uartcfg *cfgptr = uart_cfgs; struct s3c24xx_uart_resources *resp;
Remove usage of CONFIG_SERIAL_SAMSUNG_UARTS symbol from platform specific code. Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com> --- arch/arm/mach-s3c64xx/irq-pm.c | 6 +++--- arch/arm/plat-samsung/init.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)