From mboxrd@z Thu Jan 1 00:00:00 1970 From: kgene.kim@samsung.com (Kukjin Kim) Date: Fri, 30 Jul 2010 11:23:58 +0900 Subject: [PATCH 1/6] S5PC110: Move OneNAND platform device to plat-s5p In-Reply-To: References: <20100728101138.GA21329@july> <006f01cb2eb5$3a53cd80$aefb6880$%kim@samsung.com> Message-ID: <016301cb2f8e$46856580$d3903080$%kim@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Kyungmin Park wrote: > > On Thu, Jul 29, 2010 at 9:30 AM, Kukjin Kim wrote: > > Kyungmin Park wrote: > >> > > Hi, > > > > Cc'ed Ben Dooks. > > > >> From: Kyungmin Park > >> > >> To share the same platform device for s5pc210 > >> > >> Signed-off-by: Kyungmin Park > >> --- > >> ?arch/arm/mach-s5pv210/Kconfig ? ? ? ? ? ?| ? ?5 --- > >> ?arch/arm/mach-s5pv210/Makefile ? ? ? ? ? | ? ?1 - > >> ?arch/arm/mach-s5pv210/dev-onenand.c ? ? ?| ? 50 > > ---------------------------- > >> ?arch/arm/mach-s5pv210/include/mach/map.h | ? ?3 -- > >> ?arch/arm/plat-s5p/Kconfig ? ? ? ? ? ? ? ?| ? ?5 +++ > >> ?arch/arm/plat-s5p/Makefile ? ? ? ? ? ? ? | ? ?5 ++- > >> ?arch/arm/plat-s5p/dev-onenand.c ? ? ? ? ?| ? 53 > >> ++++++++++++++++++++++++++++++ > >> ?7 files changed, 62 insertions(+), 60 deletions(-) > >> ?delete mode 100644 arch/arm/mach-s5pv210/dev-onenand.c > >> ?create mode 100644 arch/arm/plat-s5p/dev-onenand.c > > > > In future, please check your git configuration so that can come out as > > follows. > > It's just 'rename(move)' not 'delete and create'...It can help to us that it > > can see more easily whether something changed. > > It used the lower version of git. I will update it. > > > > arch/arm/{mach-s5pv210 => plat-s5p}/dev-onenand.c | ? ?5 ++++- > > ?1 files changed, 4 insertions(+), 1 deletions(-) > > ?rename arch/arm/{mach-s5pv210 => plat-s5p}/dev-onenand.c (91%) > > > > diff --git a/arch/arm/mach-s5pv210/dev-onenand.c > > b/arch/arm/plat-s5p/dev-onenand.c > > similarity index 91% > > rename from arch/arm/mach-s5pv210/dev-onenand.c > > rename to arch/arm/plat-s5p/dev-onenand.c > > index 34997b7..08b3a3f 100644 > > --- a/arch/arm/mach-s5pv210/dev-onenand.c > > +++ b/arch/arm/plat-s5p/dev-onenand.c > > > > > >> > >> diff --git a/arch/arm/mach-s5pv210/Kconfig b/arch/arm/mach-s5pv210/Kconfig > >> index 0761eac..96f4d9b 100644 > >> --- a/arch/arm/mach-s5pv210/Kconfig > >> +++ b/arch/arm/mach-s5pv210/Kconfig > >> @@ -62,11 +62,6 @@ config MACH_GONI > >> ? ? ? ? Machine support for Samsung GONI board > >> ? ? ? ? S5PC110(MCP) is one of package option of S5PV210 > >> > >> -config S5PC110_DEV_ONENAND > >> - ? ? bool > >> - ? ? help > >> - ? ? ? Compile in platform device definition for OneNAND1 controller > >> - > >> ?config MACH_SMDKV210 > >> ? ? ? bool "SMDKV210" > >> ? ? ? select CPU_S5PV210 > >> diff --git a/arch/arm/mach-s5pv210/Makefile > > b/arch/arm/mach-s5pv210/Makefile > >> index 30be9a6..6a6dea1 100644 > >> --- a/arch/arm/mach-s5pv210/Makefile > >> +++ b/arch/arm/mach-s5pv210/Makefile > >> @@ -26,7 +26,6 @@ obj-$(CONFIG_MACH_GONI) ? ? ? ? ? ? += mach-goni.o > >> > >> ?obj-y ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?+= dev-audio.o > >> ?obj-$(CONFIG_S3C64XX_DEV_SPI) ? ? ? ?+= dev-spi.o > >> -obj-$(CONFIG_S5PC110_DEV_ONENAND) += dev-onenand.o > >> > >> ?obj-$(CONFIG_S5PV210_SETUP_FB_24BPP) += setup-fb-24bpp.o > >> ?obj-$(CONFIG_S5PV210_SETUP_I2C1) ? ? += setup-i2c1.o > >> diff --git a/arch/arm/mach-s5pv210/dev-onenand.c > > b/arch/arm/mach-s5pv210/dev- > >> onenand.c > >> deleted file mode 100644 > >> index 34997b7..0000000 > >> --- a/arch/arm/mach-s5pv210/dev-onenand.c > >> +++ /dev/null > >> @@ -1,50 +0,0 @@ > >> -/* > >> - * linux/arch/arm/mach-s5pv210/dev-onenand.c > >> - * > >> - * ?Copyright (c) 2008-2010 Samsung Electronics > >> - * ?Kyungmin Park > >> - * > >> - * S5PC110 series device definition for OneNAND devices > >> - * > >> - * This program is free software; you can redistribute it and/or modify > >> - * it under the terms of the GNU General Public License version 2 as > >> - * published by the Free Software Foundation. > >> - */ > >> - > >> -#include > >> -#include > >> -#include > >> -#include > >> - > >> -#include > >> -#include > >> - > >> -static struct resource s5pc110_onenand_resources[] = { > >> - ? ? [0] = { > >> - ? ? ? ? ? ? .start ?= S5PC110_PA_ONENAND, > >> - ? ? ? ? ? ? .end ? ?= S5PC110_PA_ONENAND + SZ_128K - 1, > >> - ? ? ? ? ? ? .flags ?= IORESOURCE_MEM, > >> - ? ? }, > >> - ? ? [1] = { > >> - ? ? ? ? ? ? .start ?= S5PC110_PA_ONENAND_DMA, > >> - ? ? ? ? ? ? .end ? ?= S5PC110_PA_ONENAND_DMA + SZ_2K - 1, > >> - ? ? ? ? ? ? .flags ?= IORESOURCE_MEM, > >> - ? ? }, > >> -}; > >> - > >> -struct platform_device s5pc110_device_onenand = { > >> - ? ? .name ? ? ? ? ? = "s5pc110-onenand", > >> - ? ? .id ? ? ? ? ? ? = -1, > >> - ? ? .num_resources ?= ARRAY_SIZE(s5pc110_onenand_resources), > >> - ? ? .resource ? ? ? = s5pc110_onenand_resources, > >> -}; > >> - > >> -void s5pc110_onenand_set_platdata(struct onenand_platform_data *pdata) > >> -{ > >> - ? ? struct onenand_platform_data *pd; > >> - > >> - ? ? pd = kmemdup(pdata, sizeof(struct onenand_platform_data), > >> GFP_KERNEL); > >> - ? ? if (!pd) > >> - ? ? ? ? ? ? printk(KERN_ERR "%s: no memory for platform data\n", > >> __func__); > >> - ? ? s5pc110_device_onenand.dev.platform_data = pd; > >> -} > >> diff --git a/arch/arm/mach-s5pv210/include/mach/map.h b/arch/arm/mach- > >> s5pv210/include/mach/map.h > >> index 34eb168..3a44e1e 100644 > >> --- a/arch/arm/mach-s5pv210/include/mach/map.h > >> +++ b/arch/arm/mach-s5pv210/include/mach/map.h > >> @@ -16,9 +16,6 @@ > >> ?#include > >> ?#include > >> > >> -#define S5PC110_PA_ONENAND ? (0xB0000000) > >> -#define S5PC110_PA_ONENAND_DMA ? ? ? (0xB0600000) > >> - > >> ?#define S5PV210_PA_CHIPID ? ?(0xE0000000) > >> ?#define S5P_PA_CHIPID ? ? ? ? ? ? ? ?S5PV210_PA_CHIPID > >> > >> diff --git a/arch/arm/plat-s5p/Kconfig b/arch/arm/plat-s5p/Kconfig > >> index 907ac63..314f7d1 100644 > >> --- a/arch/arm/plat-s5p/Kconfig > >> +++ b/arch/arm/plat-s5p/Kconfig > >> @@ -31,3 +31,8 @@ config S5P_EXT_INT > >> ? ? ? help > >> ? ? ? ? Use the external interrupts (other than GPIO interrupts.) > >> ? ? ? ? Note: Do not choose this for S5P6440. > >> + > >> +config S5PC110_DEV_ONENAND > > > > As I said, S5P_DEV_ONENAND is better in here. > No, I don't want to break the current board use the S5PC110_DEV_ONENAND. Hmm...you can use S5PC110_xxx even though change the prefix in there. > > Before request the S5P_* prefix. first make rules. In your Please use s5p_xxx for easily reading and avoiding confusing in plat-s5p. > explanation. it can move to the plat-samsung. No. > What's the definition of s5p. just two s5p series have the same IP. > then can be used the plat-s5p? Yes, it's simple...it can help to avoid duplicate code. > > So I suggest that (the number means the priority and naming rules.) > 1. use the first SoC chip name. > 2. two s5p series or more have the same IP. then use the plat-s5p > 3. If driver support s3c and s5p series then use the plat-samsung. e.g., RTC. In this case, I don't agree with you... > > So it's reasonable to use S5PC110_DEV_ONENAND name. > > > > >> + ? ? bool > >> + ? ? help > >> + ? ? ? Compile in platform device definition for OneNAND controller > >> diff --git a/arch/arm/plat-s5p/Makefile b/arch/arm/plat-s5p/Makefile > >> index 39c242b..c49617e 100644 > >> --- a/arch/arm/plat-s5p/Makefile > >> +++ b/arch/arm/plat-s5p/Makefile > >> @@ -12,9 +12,12 @@ obj- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? := > >> > >> ?# Core files > >> > >> -obj-y ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?+= dev-uart.o > >> ?obj-y ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?+= cpu.o > >> ?obj-y ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?+= clock.o > >> ?obj-y ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?+= irq.o > >> ?obj-$(CONFIG_S5P_EXT_INT) ? ?+= irq-eint.o > >> > >> +# Device files > >> + > >> +obj-y ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?+= dev-uart.o > >> +obj-$(CONFIG_S5PC110_DEV_ONENAND) ? ?+= dev-onenand.o > >> diff --git a/arch/arm/plat-s5p/dev-onenand.c > > b/arch/arm/plat-s5p/dev-onenand.c > >> new file mode 100644 > >> index 0000000..08b3a3f > >> --- /dev/null > >> +++ b/arch/arm/plat-s5p/dev-onenand.c > >> @@ -0,0 +1,53 @@ > >> +/* > >> + * linux/arch/arm/plat-s5p/dev-onenand.c > >> + * > >> + * ?Copyright (c) 2008-2010 Samsung Electronics > >> + * ?Kyungmin Park > >> + * > >> + * S5PC110 series device definition for OneNAND devices > > > > I thinks...this is for S5P SoCs such as S5PC210 and S5PC110 > > ...even though S5P6440 does not have OneNAND device... > > > >> + * > >> + * This program is free software; you can redistribute it and/or modify > >> + * it under the terms of the GNU General Public License version 2 as > >> + * published by the Free Software Foundation. > >> + */ > >> + > >> +#include > >> +#include > >> +#include > >> +#include > >> + > >> +#include > >> +#include > >> + > >> +#define S5PC110_PA_ONENAND ? ? ? ? ? (0xB0000000) > >> +#define S5PC110_PA_ONENAND_DMA ? ? ? ? ? ? ? (0xB0600000) > > > > Please move to mach/map.h for compatibilities with other SoCs. > > Can you explain the why use the hard-coded value in clock framework? It's different case...because the address can be changed in other APs. > it's only used that at there. it's same rule. it's only used here. > Hmm...you misunderstood :-( I didn't say like that...please see below. --- mach/map.h #define S5PC110_PA_ONENAND (0xB0000000) #define S5PC110_PA_ONENAND_DMA (0xB0600000) ... #define S5P_PA_ONENAND S5PC110_PA_ONENAND #define S5P_PA_ONENAND_DMA S5PC110_PA_ONENAND_DMA --- Used above method on other cases. And it can be used even if the address differs on each AP. > Do you want to hard-coded the value at resource directly? > No... I wonder...if the address differs on each AP, how can you support it...maybe adding ifdef in there? > As I suggested, make rules first and discuss it. > Please use above method in there...of course if there is more reasonable reason, the rule can be changed. > > > > #define S5PC110_PA_ONENAND ? ? ? ? ? ? ?(0xB0000000) > > ... > > #define S5P_PA_ONENAD ? ? ? ? ? S5PC110_PA_ONENAND > > ... > > > >> + > >> +static struct resource s5pc110_onenand_resources[] = { > > > > s5p_onenand_resources... > > > >> + ? ? [0] = { > >> + ? ? ? ? ? ? .start ?= S5PC110_PA_ONENAND, > >> + ? ? ? ? ? ? .end ? ?= S5PC110_PA_ONENAND + SZ_128K - 1, > > > > Here, S5P_PA_xxx is better... > > > >> + ? ? ? ? ? ? .flags ?= IORESOURCE_MEM, > >> + ? ? }, > >> + ? ? [1] = { > >> + ? ? ? ? ? ? .start ?= S5PC110_PA_ONENAND_DMA, > >> + ? ? ? ? ? ? .end ? ?= S5PC110_PA_ONENAND_DMA + SZ_2K - 1, > >> + ? ? ? ? ? ? .flags ?= IORESOURCE_MEM, > >> + ? ? }, > >> +}; > >> + > >> +struct platform_device s5pc110_device_onenand = { > >> + ? ? .name ? ? ? ? ? = "s5pc110-onenand", > >> + ? ? .id ? ? ? ? ? ? = -1, > >> + ? ? .num_resources ?= ARRAY_SIZE(s5pc110_onenand_resources), > >> + ? ? .resource ? ? ? = s5pc110_onenand_resources, > >> +}; > >> + > >> +void s5pc110_onenand_set_platdata(struct onenand_platform_data *pdata) > >> +{ > >> + ? ? struct onenand_platform_data *pd; > >> + > >> + ? ? pd = kmemdup(pdata, sizeof(struct onenand_platform_data), > >> GFP_KERNEL); > >> + ? ? if (!pd) > >> + ? ? ? ? ? ? printk(KERN_ERR "%s: no memory for platform data\n", > >> __func__); > >> + ? ? s5pc110_device_onenand.dev.platform_data = pd; > > > > s3c_set_platdata(pdata, sizeof(struct onenand_platform_data), > > &s5p_device_onenand); > > I just remove it. there's no user. and no need to setup for platform. > > Thank you, > Kyungmin Park > > Thanks. Best regards, Kgene. -- Kukjin Kim , Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.