2007-11-23 10:15:19

by Bryan Wu

[permalink] [raw]
Subject: [PATCH 1/1] [MTD/NAND]: Add Blackfin BF52x on-chip NAND Flash controller driver support in bf5xx_nand driver

From: Michael Hennerich <[email protected]>

Signed-off-by: Michael Hennerich <[email protected]>
Signed-off-by: Bryan Wu <[email protected]>
---
drivers/mtd/nand/Kconfig | 2 +-
drivers/mtd/nand/bf5xx_nand.c | 20 ++++++++++++++++++--
2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 246d451..bf0e9b0 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -93,7 +93,7 @@ config MTD_NAND_AU1550

config MTD_NAND_BF5XX
tristate "Blackfin on-chip NAND Flash Controller driver"
- depends on BF54x && MTD_NAND
+ depends on (BF54x || BF52x) && MTD_NAND
help
This enables the Blackfin on-chip NAND flash controller

diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
index 1657ecd..542850c 100644
--- a/drivers/mtd/nand/bf5xx_nand.c
+++ b/drivers/mtd/nand/bf5xx_nand.c
@@ -74,7 +74,22 @@ static int hardware_ecc = 1;
static int hardware_ecc;
#endif

-static unsigned short bfin_nfc_pin_req[] = {P_NAND_CE, P_NAND_RB, 0};
+static unsigned short bfin_nfc_pin_req[] =
+ {P_NAND_CE,
+ P_NAND_RB,
+ P_NAND_D0,
+ P_NAND_D1,
+ P_NAND_D2,
+ P_NAND_D3,
+ P_NAND_D4,
+ P_NAND_D5,
+ P_NAND_D6,
+ P_NAND_D7,
+ P_NAND_WE,
+ P_NAND_RE,
+ P_NAND_CLE,
+ P_NAND_ALE,
+ 0};

/*
* Data structures for bf5xx nand flash controller driver
@@ -507,12 +522,13 @@ static int bf5xx_nand_dma_init(struct bf5xx_nand_info *info)

init_completion(&info->dma_completion);

+#ifdef CONFIG_BF54x
/* Setup DMAC1 channel mux for NFC which shared with SDH */
val = bfin_read_DMAC1_PERIMUX();
val &= 0xFFFE;
bfin_write_DMAC1_PERIMUX(val);
SSYNC();
-
+#endif
/* Request NFC DMA channel */
ret = request_dma(CH_NFC, "BF5XX NFC driver");
if (ret < 0) {
--
1.5.3.4


2007-11-23 10:19:51

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH 1/1] [MTD/NAND]: Add Blackfin BF52x on-chip NAND Flash controller driver support in bf5xx_nand driver


On Fri, 2007-11-23 at 18:14 +0800, Bryan Wu wrote:
>
> +#ifdef CONFIG_BF54x
> /* Setup DMAC1 channel mux for NFC which shared with SDH */
> val = bfin_read_DMAC1_PERIMUX();
> val &= 0xFFFE;
> bfin_write_DMAC1_PERIMUX(val);
> SSYNC();
> -
> +#endif

You can't build a multiplatform kernel which runs on BF52x and BF54x?
And want to make it harder to do so in future?

--
dwmw2

2007-11-23 14:25:39

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH 1/1] [MTD/NAND]: Add Blackfin BF52x on-chip NAND Flash controller driver support in bf5xx_nand driver

On Nov 23, 2007 6:19 PM, David Woodhouse <[email protected]> wrote:
>
> On Fri, 2007-11-23 at 18:14 +0800, Bryan Wu wrote:
> >
> > +#ifdef CONFIG_BF54x
> > /* Setup DMAC1 channel mux for NFC which shared with SDH */
> > val = bfin_read_DMAC1_PERIMUX();
> > val &= 0xFFFE;
> > bfin_write_DMAC1_PERIMUX(val);
> > SSYNC();
> > -
> > +#endif
>
> You can't build a multiplatform kernel which runs on BF52x and BF54x?

There are some hardware difference between BF52x and BF54x. We have to do this.

> And want to make it harder to do so in future?
>
Any idea to make it easier? providing a specific hardware init hook
function from each platform board files?

Regards,
-Bryan

> --
> dwmw2
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2007-11-23 21:53:20

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH 1/1] [MTD/NAND]: Add Blackfin BF52x on-chip NAND Flash controller driver support in bf5xx_nand driver

On Fri, 23 Nov 2007 22:25:29 +0800
"Bryan Wu" <[email protected]> wrote:

> On Nov 23, 2007 6:19 PM, David Woodhouse <[email protected]> wrote:
> >
> > On Fri, 2007-11-23 at 18:14 +0800, Bryan Wu wrote:
> > >
> > > +#ifdef CONFIG_BF54x
> > > /* Setup DMAC1 channel mux for NFC which shared with SDH
> > > */ val = bfin_read_DMAC1_PERIMUX();
> > > val &= 0xFFFE;
> > > bfin_write_DMAC1_PERIMUX(val);
> > > SSYNC();
> > > -
> > > +#endif
> >
> > You can't build a multiplatform kernel which runs on BF52x and
> > BF54x?
>
> There are some hardware difference between BF52x and BF54x. We have
> to do this.
>

well does it need to be an #ifdef, or can it be a runtime if() ?

--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2007-11-23 22:04:38

by Robin Getz

[permalink] [raw]
Subject: Re: [PATCH 1/1] [MTD/NAND]: Add Blackfin BF52x on-chip NAND Flash controller driver support in bf5xx_nand driver

On Fri 23 Nov 2007 16:52, Arjan van de Ven pondered:
> On Fri, 23 Nov 2007 22:25:29 +0800
> "Bryan Wu" <[email protected]> wrote:
>
> > On Nov 23, 2007 6:19 PM, David Woodhouse <[email protected]> wrote:
> > >
> > > On Fri, 2007-11-23 at 18:14 +0800, Bryan Wu wrote:
> > > >
> > > > +#ifdef CONFIG_BF54x
> > > > /* Setup DMAC1 channel mux for NFC which shared with SDH
> > > > */ val = bfin_read_DMAC1_PERIMUX();
> > > > val &= 0xFFFE;
> > > > bfin_write_DMAC1_PERIMUX(val);
> > > > SSYNC();
> > > > -
> > > > +#endif
> > >
> > > You can't build a multiplatform kernel which runs on BF52x and
> > > BF54x?
> >
> > There are some hardware difference between BF52x and BF54x. We have
> > to do this.
> >
>
> well does it need to be an #ifdef, or can it be a runtime if() ?

It could be a runtime if() but we don't currently have the is_mach() all set
up properly today.

This is because on most systems that Blackfin ships on - memory is the
dominate cost of the system, and end users don't want to take the either the
storage (flash) hit of having code they don't use, or the run time (DRAM)
overhead. They are fine with compiling 2 kernels for two platforms if it
means things are cheaper. :)

That being said, we still need to go back, and add things properly - and just
let gcc optimise things away if it is not used - c code is more maintainable
than all the ifdefs we have today.

This is the goal - it will just take a little bit to get there.

-Robin

2007-11-24 06:43:46

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH 1/1] [MTD/NAND]: Add Blackfin BF52x on-chip NAND Flash controller driver support in bf5xx_nand driver


On Fri, 2007-11-23 at 17:04 -0500, Robin Getz wrote:
> It could be a runtime if() but we don't currently have the is_mach() all set
> up properly today.
>
> This is because on most systems that Blackfin ships on - memory is the
> dominate cost of the system, and end users don't want to take the either the
> storage (flash) hit of having code they don't use, or the run time (DRAM)
> overhead. They are fine with compiling 2 kernels for two platforms if it
> means things are cheaper. :)
>
> That being said, we still need to go back, and add things properly - and just
> let gcc optimise things away if it is not used - c code is more maintainable
> than all the ifdefs we have today.
>
> This is the goal - it will just take a little bit to get there.

For now I suspect you could at least define machine_is_bf52x() and
machine_is_bf54x() which are hard-coded to either zero or one according
to the configuration, and at least you wouldn't need to add ifdefs to
drivers.

--
dwmw2

2007-11-24 07:15:29

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH 1/1] [MTD/NAND]: Add Blackfin BF52x on-chip NAND Flash controller driver support in bf5xx_nand driver

On Nov 24, 2007 2:43 PM, David Woodhouse <[email protected]> wrote:
>
> On Fri, 2007-11-23 at 17:04 -0500, Robin Getz wrote:
> > It could be a runtime if() but we don't currently have the is_mach() all set
> > up properly today.
> >
> > This is because on most systems that Blackfin ships on - memory is the
> > dominate cost of the system, and end users don't want to take the either the
> > storage (flash) hit of having code they don't use, or the run time (DRAM)
> > overhead. They are fine with compiling 2 kernels for two platforms if it
> > means things are cheaper. :)
> >
> > That being said, we still need to go back, and add things properly - and just
> > let gcc optimise things away if it is not used - c code is more maintainable
> > than all the ifdefs we have today.
> >
> > This is the goal - it will just take a little bit to get there.
>
> For now I suspect you could at least define machine_is_bf52x() and
> machine_is_bf54x() which are hard-coded to either zero or one according
> to the configuration, and at least you wouldn't need to add ifdefs to
> drivers.
>

We got some plan to do this, but maybe cpu_is_bf52x() and
cpu_is_bf54x() are better.
Thanks.

-Bryan

2007-11-24 13:11:19

by Robin Getz

[permalink] [raw]
Subject: Re: [PATCH 1/1] [MTD/NAND]: Add Blackfin BF52x on-chip NAND Flash controller driver support in bf5xx_nand driver

On Sat 24 Nov 2007 02:15, Bryan Wu pondered:
> On Nov 24, 2007 2:43 PM, David Woodhouse <[email protected]> wrote:
> >
> > On Fri, 2007-11-23 at 17:04 -0500, Robin Getz wrote:
> > > It could be a runtime if() but we don't currently have the is_mach() all
> > > set up properly today.
> > >
> > > This is because on most systems that Blackfin ships on - memory is the
> > > dominate cost of the system, and end users don't want to take the either
> > > the storage (flash) hit of having code they don't use, or the run time
> > > (DRAM) overhead. They are fine with compiling 2 kernels for two
> > > platforms if it means things are cheaper. :)
> > >
> > > That being said, we still need to go back, and add things properly - and
> > > just let gcc optimise things away if it is not used - c code is more
> > > maintainable than all the ifdefs we have today.
> > >
> > > This is the goal - it will just take a little bit to get there.
> >
> > For now I suspect you could at least define machine_is_bf52x() and
> > machine_is_bf54x() which are hard-coded to either zero or one according
> > to the configuration, and at least you wouldn't need to add ifdefs to
> > drivers.
> >
>
> We got some plan to do this, but maybe cpu_is_bf52x() and
> cpu_is_bf54x() are better.

Which was one of the reasons I think we were waiting/confused. There doesn't
seem to be any consistancy between:

cpu_is_

which I assume refers to specific processor/chipset/SoC implementation?
cpu_is_ixp42x
cpu_is_ixp43x
cpu_is_ixp46x

mach_is_

which I assume refers to a specific platform:

mach_is_dreamcast
mach_is_hp6xx
mach_is_r7780mp
mach_is_r7780rp

machine_is_xxxx

which I assume refers to a specific platform:

machine_is_jornada56x
machine_is_jornada720
machine_is_nokia770

So, we should use cpu_is_ as Bryan suggested?

-Robin