Re: [PATCH 3/4] omap: move detection of NAND CS to common-board-devices
Hello! On May 4, 2011, at 2:38 AM, Mike Rapoport wrote: > On 05/04/11 07:10, Oleg Drokin wrote: >> Ok, so here's a simple patch to save everyone trouble, I guess. >> >> Though on the other hand I can imagine that perhaps including this generic >> common-board-devices.c >> might not be desirable for people that don't use anything from that file. > Since the common-board-devices.c has TWL initialization I doubt there would a > board that does not use it at all... Actually the twl init you have is somewhat inflexible. E.g. on the Nook Color the 1st i2c bus has the twl and some other devices, but I don't see how to put more than just the twl on the first bus with your common code. Bye, Oleg-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] omap: move detection of NAND CS to common-board-devices
* Mike Rapoport [110504 10:13]:
> On 05/04/11 09:46, Tony Lindgren wrote:
> > * Mike Rapoport [110504 09:35]:
> >> On 05/04/11 07:10, Oleg Drokin wrote:
> >>> Ok, so here's a simple patch to save everyone trouble, I guess.
> >>>
> >>> Though on the other hand I can imagine that perhaps including this
> >>> generic common-board-devices.c
> >>> might not be desirable for people that don't use anything from that file.
> >>
> >> Since the common-board-devices.c has TWL initialization I doubt there
> >> would a
> >> board that does not use it at all...
> >>
> >>> Would it be a better idea to split it to a file-per-feature?
> >>
> >> Splitting the common-board-devices into a file-per-feature will diminish
> >> its
> >> added value, IMO.
> >> We can either continue to use #ifdef CONFIG_SOMETHING in both
> >> common-board-devices.[ch] as your fix proposes or just drop #ifdefs and
> >> inlines
> >> from the header.
> >> Tony, what is your preference?
> >
> > We should consider the code size too.. Maybe see if you can make them
> > weak instead of the ifdefs?
>
> Unless I completely misunderstand how weak works, we'll still have ifdefs in
> .c
> file, i.e.
>
> common-board-devices.h:
>
> void __omap_nand_flash_init(int opts, struct mtd_partition *parts, int
> n_parts)
> {
> }
>
> void omap_nand_flash_init(int opts, struct mtd_partition *parts, int n_parts)
> __attribute__((weak, alias("__omap_nand_flash_init")));
>
> common-board-devices.c:
> #if defined(CONFIG_MTD_NAND_OMAP2) || defined(CONFIG_MTD_NAND_OMAP2_MODULE)
> void omap_nand_flash_init(int opts, struct mtd_partition *parts, int n_parts)
> {
> ...
> }
> #endif
>
> Yet, we can keep the ifdefs only in common-board-devices.c and get rid if
> inlines in the header.
> Also, all the code in common-board-devices.c is __init, so it's eventually
> dropped in runtime
Sounds OK to me for the simple platform init functions used for most
common devices.
For anything more complex, we can have a separate file with ifdef in the
header and Makefile compiling it in only as selected.
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] omap: move detection of NAND CS to common-board-devices
On 05/04/11 09:46, Tony Lindgren wrote:
> * Mike Rapoport [110504 09:35]:
>> On 05/04/11 07:10, Oleg Drokin wrote:
>>> Ok, so here's a simple patch to save everyone trouble, I guess.
>>>
>>> Though on the other hand I can imagine that perhaps including this generic
>>> common-board-devices.c
>>> might not be desirable for people that don't use anything from that file.
>>
>> Since the common-board-devices.c has TWL initialization I doubt there would a
>> board that does not use it at all...
>>
>>> Would it be a better idea to split it to a file-per-feature?
>>
>> Splitting the common-board-devices into a file-per-feature will diminish its
>> added value, IMO.
>> We can either continue to use #ifdef CONFIG_SOMETHING in both
>> common-board-devices.[ch] as your fix proposes or just drop #ifdefs and
>> inlines
>> from the header.
>> Tony, what is your preference?
>
> We should consider the code size too.. Maybe see if you can make them
> weak instead of the ifdefs?
Unless I completely misunderstand how weak works, we'll still have ifdefs in .c
file, i.e.
common-board-devices.h:
void __omap_nand_flash_init(int opts, struct mtd_partition *parts, int n_parts)
{
}
void omap_nand_flash_init(int opts, struct mtd_partition *parts, int n_parts)
__attribute__((weak, alias("__omap_nand_flash_init")));
common-board-devices.c:
#if defined(CONFIG_MTD_NAND_OMAP2) || defined(CONFIG_MTD_NAND_OMAP2_MODULE)
void omap_nand_flash_init(int opts, struct mtd_partition *parts, int n_parts)
{
...
}
#endif
Yet, we can keep the ifdefs only in common-board-devices.c and get rid if
inlines in the header.
Also, all the code in common-board-devices.c is __init, so it's eventually
dropped in runtime
> Tony
--
Sincerely yours,
Mike.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] omap: move detection of NAND CS to common-board-devices
* Mike Rapoport [110504 09:35]: > On 05/04/11 07:10, Oleg Drokin wrote: > > Ok, so here's a simple patch to save everyone trouble, I guess. > > > > Though on the other hand I can imagine that perhaps including this generic > > common-board-devices.c > > might not be desirable for people that don't use anything from that file. > > Since the common-board-devices.c has TWL initialization I doubt there would a > board that does not use it at all... > > > Would it be a better idea to split it to a file-per-feature? > > Splitting the common-board-devices into a file-per-feature will diminish its > added value, IMO. > We can either continue to use #ifdef CONFIG_SOMETHING in both > common-board-devices.[ch] as your fix proposes or just drop #ifdefs and > inlines > from the header. > Tony, what is your preference? We should consider the code size too.. Maybe see if you can make them weak instead of the ifdefs? Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] omap: move detection of NAND CS to common-board-devices
On 05/04/11 07:10, Oleg Drokin wrote: > Ok, so here's a simple patch to save everyone trouble, I guess. > > Though on the other hand I can imagine that perhaps including this generic > common-board-devices.c > might not be desirable for people that don't use anything from that file. Since the common-board-devices.c has TWL initialization I doubt there would a board that does not use it at all... > Would it be a better idea to split it to a file-per-feature? Splitting the common-board-devices into a file-per-feature will diminish its added value, IMO. We can either continue to use #ifdef CONFIG_SOMETHING in both common-board-devices.[ch] as your fix proposes or just drop #ifdefs and inlines from the header. Tony, what is your preference? -- Sincerely yours, Mike. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] omap: move detection of NAND CS to common-board-devices
Ok, so here's a simple patch to save everyone trouble, I guess. Though on the other hand I can imagine that perhaps including this generic common-board-devices.c might not be desirable for people that don't use anything from that file. Would it be a better idea to split it to a file-per-feature? compile-fix.diff Description: Binary data
Re: [PATCH 3/4] omap: move detection of NAND CS to common-board-devices
Hello!
This patch breaks compile for me:
On Apr 24, 2011, at 6:09 PM, Mike Rapoport wrote:
> --- a/arch/arm/mach-omap2/common-board-devices.c
> +++ b/arch/arm/mach-omap2/common-board-devices.c
> @@ -29,6 +29,7 @@
...
> +void __init omap_nand_flash_init(int options, struct mtd_partition *parts,
> + int nr_parts)
> +{
...
> +}
> diff --git a/arch/arm/mach-omap2/common-board-devices.h
> b/arch/arm/mach-omap2/common-board-devices.h
> index 0ec3e07..ca03abf 100644
> --- a/arch/arm/mach-omap2/common-board-devices.h
> +++ b/arch/arm/mach-omap2/common-board-devices.h
> @@ -39,4 +40,13 @@ static inline void omap_ads7846_init(int bus_num,
> }
> #endif
>
> +#if defined(CONFIG_MTD_NAND_OMAP2) || defined(CONFIG_MTD_NAND_OMAP2_MODULE)
> +void omap_nand_flash_init(int opts, struct mtd_partition *parts, int
> n_parts);
> +#else
> +static inline void omap_nand_flash_init(int opts, struct mtd_partition
> *parts,
> + int nr_parts)
> +{
> +}
arch/arm/mach-omap2/common-board-devices.c:113: error: redefinition of
'omap_nand_flash_init'
arch/arm/mach-omap2/common-board-devices.h:46: note: previous definition of
'omap_nand_flash_init' was here
I don't have CONFIG_MTD_NAND_OMAP2 defined of course as there is no NAND flash
on my board.
Bye,
Oleg
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] omap: move detection of NAND CS to common-board-devices
and reduce amount of copy/paste
Signed-off-by: Mike Rapoport
---
arch/arm/mach-omap2/board-devkit8000.c | 43 +-
arch/arm/mach-omap2/board-omap3beagle.c| 45 +--
arch/arm/mach-omap2/board-omap3touchbook.c | 45 +--
arch/arm/mach-omap2/board-overo.c | 42 +
arch/arm/mach-omap2/common-board-devices.c | 42 ++
arch/arm/mach-omap2/common-board-devices.h | 10 ++
6 files changed, 60 insertions(+), 167 deletions(-)
diff --git a/arch/arm/mach-omap2/board-devkit8000.c
b/arch/arm/mach-omap2/board-devkit8000.c
index 983f44b..e7dc057 100644
--- a/arch/arm/mach-omap2/board-devkit8000.c
+++ b/arch/arm/mach-omap2/board-devkit8000.c
@@ -97,13 +97,6 @@ static struct mtd_partition devkit8000_nand_partitions[] = {
},
};
-static struct omap_nand_platform_data devkit8000_nand_data = {
- .options= NAND_BUSWIDTH_16,
- .parts = devkit8000_nand_partitions,
- .nr_parts = ARRAY_SIZE(devkit8000_nand_partitions),
- .dma_channel= -1, /* disable DMA in OMAP NAND driver */
-};
-
static struct omap2_hsmmc_info mmc[] = {
{
.mmc= 1,
@@ -516,39 +509,6 @@ static struct platform_device *devkit8000_devices[]
__initdata = {
&omap_dm9000_dev,
};
-static void __init devkit8000_flash_init(void)
-{
- u8 cs = 0;
- u8 nandcs = GPMC_CS_NUM + 1;
-
- /* find out the chip-select on which NAND exists */
- while (cs < GPMC_CS_NUM) {
- u32 ret = 0;
- ret = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
-
- if ((ret & 0xC00) == 0x800) {
- printk(KERN_INFO "Found NAND on CS%d\n", cs);
- if (nandcs > GPMC_CS_NUM)
- nandcs = cs;
- }
- cs++;
- }
-
- if (nandcs > GPMC_CS_NUM) {
- printk(KERN_INFO "NAND: Unable to find configuration "
-"in GPMC\n ");
- return;
- }
-
- if (nandcs < GPMC_CS_NUM) {
- devkit8000_nand_data.cs = nandcs;
-
- printk(KERN_INFO "Registering NAND on CS%d\n", nandcs);
- if (gpmc_nand_init(&devkit8000_nand_data) < 0)
- printk(KERN_ERR "Unable to register NAND device\n");
- }
-}
-
static struct omap_musb_board_data musb_board_data = {
.interface_type = MUSB_INTERFACE_ULPI,
.mode = MUSB_OTG,
@@ -740,7 +700,8 @@ static void __init devkit8000_init(void)
usb_musb_init(&musb_board_data);
usbhs_init(&usbhs_bdata);
- devkit8000_flash_init();
+ omap_nand_flash_init(NAND_BUSWIDTH_16, devkit8000_nand_partitions,
+ARRAY_SIZE(devkit8000_nand_partitions));
/* Ensure SDRC pins are mux'd for self-refresh */
omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT);
diff --git a/arch/arm/mach-omap2/board-omap3beagle.c
b/arch/arm/mach-omap2/board-omap3beagle.c
index 13a1664..ce3bc2d 100644
--- a/arch/arm/mach-omap2/board-omap3beagle.c
+++ b/arch/arm/mach-omap2/board-omap3beagle.c
@@ -174,15 +174,6 @@ static struct mtd_partition omap3beagle_nand_partitions[]
= {
},
};
-static struct omap_nand_platform_data omap3beagle_nand_data = {
- .options= NAND_BUSWIDTH_16,
- .parts = omap3beagle_nand_partitions,
- .nr_parts = ARRAY_SIZE(omap3beagle_nand_partitions),
- .dma_channel= -1, /* disable DMA in OMAP NAND driver */
- .nand_setup = NULL,
- .dev_ready = NULL,
-};
-
/* DSS */
static int beagle_enable_dvi(struct omap_dss_device *dssdev)
@@ -542,39 +533,6 @@ static struct platform_device *omap3_beagle_devices[]
__initdata = {
&keys_gpio,
};
-static void __init omap3beagle_flash_init(void)
-{
- u8 cs = 0;
- u8 nandcs = GPMC_CS_NUM + 1;
-
- /* find out the chip-select on which NAND exists */
- while (cs < GPMC_CS_NUM) {
- u32 ret = 0;
- ret = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
-
- if ((ret & 0xC00) == 0x800) {
- printk(KERN_INFO "Found NAND on CS%d\n", cs);
- if (nandcs > GPMC_CS_NUM)
- nandcs = cs;
- }
- cs++;
- }
-
- if (nandcs > GPMC_CS_NUM) {
- printk(KERN_INFO "NAND: Unable to find configuration "
-"in GPMC\n ");
- return;
- }
-
- if (nandcs < GPMC_CS_NUM) {
- omap3beagle_nand_data.cs = nandcs;
-
- printk(KERN_INFO "Registering NAND on CS%d\n", nandcs);
- if (gpmc_nand_init(&omap3beagle_nand_data) < 0)
- printk(KERN_ERR "Unable to register NAND device\n"
