| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20090608113813.GA24067@atomide.com>
Date: Mon, 8 Jun 2009 04:38:15 -0700
From: Tony Lindgren <tony@...mide.com>
To: "Singh, Vimal"
<IMCEAEX-_O=TI_OU=BD_CN=RECIPIENTS_CN=X0094262@...e86.itg.ti.com>
Cc: "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
"dwmw2@...radead.org" <dwmw2@...radead.org>,
"dedekind@...radead.org" <dedekind@...radead.org>,
"david-b@...bell.net" <david-b@...bell.net>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>
Subject: Re: [PATCH] [MTD] [NAND] Add prefetch and dma support for omap2/3
NAND driver
* Singh, Vimal <IMCEAEX-_O=TI_OU=BD_CN=RECIPIENTS_CN=X0094262@...e86.itg.ti.com> [090605 05:00]:
>
>
> On Thu, Jun 4, 2009 at 8:58 PM, Tony Lindgren<tony@...mide.com> wrote:
> > * vimal singh <vimalsingh@...com> [090604 02:34]:
> >> On Wed, Jun 3, 2009 at 10:03 PM, Tony Lindgren <tony@...mide.com> wrote:
> >> > * Singh, Vimal
> >> <IMCEAEX-_O=TI_OU=BD_CN=RECIPIENTS_CN=X0094262@...e86.itg.ti.com> [090602
> >> 23:46]:
> >> >>
> >> >> On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren <tony@...mide.com> wrote:
> >> >> > * vimal singh <vimalsingh@...com> [090602 05:40]:
> >> >> >> This patch adds prefetch support to access nand flash in both mpu and dma
> >> mode.
> >> >> >> This patch also adds 8-bit nand support (omap_read/write_buf8).
> >> >> >> Prefetch can be used for both 8- and 16-bit devices.
> >> >> >
> >> >> > This should be reviewed on the linux-omap@...r.kernel.org list for sure.
> >> >> > One other comment below.
> >> >> >
> >> >> >> Signed-off-by: Vimal Singh <vimalsingh@...com>
> >> >> >> ---
> >> >> >> I prepared this patch on top of "OMAP2 / OMAP3 NAND driver" patch:
> >> >> >> http://lists.infradead.org/pipermail/linux-mtd/2009-May/025562.html
> >> >> >>
> >> >> >> ---
> >> >> >> arch/arm/mach-omap2/gpmc.c | 102 ++++++++++
> >> >> >> arch/arm/plat-omap/include/mach/gpmc.h | 4
> >> >> >> drivers/mtd/nand/Kconfig | 17 +
> >> >> >> drivers/mtd/nand/omap2.c | 308
> >> ++++++++++++++++++++++++++++++++-
> >> >> >> 4 files changed, 422 insertions(+), 9 deletions(-)
> >> >> >>
> >> >> >> Index: mtd-2.6/arch/arm/mach-omap2/gpmc.c
> >> >> >> ===================================================================
> >> >> >> --- mtd-2.6.orig/arch/arm/mach-omap2/gpmc.c
> >> >> >> +++ mtd-2.6/arch/arm/mach-omap2/gpmc.c
> >> >> >> @@ -54,6 +54,12 @@
> >> >> >> #define GPMC_CHUNK_SHIFT 24 /* 16 MB */
> >> >> >> #define GPMC_SECTION_SHIFT 28 /* 128 MB */
> >> >> >>
> >> >> >> +#ifdef CONFIG_MTD_NAND_OMAP_PREFETCH
> >> >> >> +#define CS_NUM_SHIFT 24
> >> >> >> +#define ENABLE_PREFETCH 7
> >> >> >> +#define DMA_MPU_MODE 2
> >> >> >> +#endif
> >> >> >> +
> >> >> >> static struct resource gpmc_mem_root;
> >> >> >> static struct resource gpmc_cs_mem[GPMC_CS_NUM];
> >> >> >> static DEFINE_SPINLOCK(gpmc_mem_lock);
> >> >> >> @@ -383,6 +389,99 @@ void gpmc_cs_free(int cs)
> >> >> >> }
> >> >> >> EXPORT_SYMBOL(gpmc_cs_free);
> >> >> >>
> >> >> >> +#ifdef CONFIG_MTD_NAND_OMAP_PREFETCH
> >> >> >> +/**
> >> >> >> + * gpmc_prefetch_init - configures default configuration for prefetch
> >> engine
> >> >> >> + */
> >> >> >> +static void gpmc_prefetch_init(void)
> >> >> >> +{
> >> >> >> + /* Setting the default threshold to 64 */
> >> >> >> + gpmc_write_reg(GPMC_PREFETCH_CONTROL, 0x0);
> >> >> >> + gpmc_write_reg(GPMC_PREFETCH_CONFIG1, 0x40 << 8);
> >> >> >> + gpmc_write_reg(GPMC_PREFETCH_CONFIG2, 0x0);
> >> >> >> +}
> >> >> >
> >> >> > Why would you want to have NAND specific init code int gpmc.c?
> >> >> >
> >> >> > The purpose if gpmc.c is to provide access to configuring the
> >> >> > General Purpose Memory Controller (GPMC). You should just provide
> >> >> > functions in gpmc.c for the platform init code to use, and then
> >> >> > the drivers can stay platform independent.
> >> >>
> >> >> In my understanding, this 'prefetch' engine is part of GPMC itself, it is a
> >> >> kind of feature provided by GPMC which can be utilized by NAND driver.
> >> >> So, to me, it makes sens to get initialized prefetch by GPMC itself so that
> >> >> NAND driver can use it.
> >> >
> >> > But it should not have a dependency to NAND.
> >>
> >> This engine, in GPMC, is dedicated for NAND devices only.
> >>
> >> >
> >> >> Another reason was that all read / write to GPMC register are done by
> >> >> functions 'gpmc_read_reg' / 'gpmc_write_reg', which have been made
> >> >> 'static' in nature.
> >> >
> >> > That's why you need to provide a generic function in gpmc.c to enable
> >> > prefetch that the platform code for any driver can use.
> >>
> >> Exactly, and whenever a platform code uses gpmc init call, gpmc initializes
> >> this engine too. Since prefetch engine is the part of GPMC, IMHOP, it should
> >> get initialized as part of GPMC initialization.
> >
> > No, the driver needing it should initialize it. There should not be any
> > ifdefs needed in the gpmc.c for that.
>
> Are you suggesting to move gpmc prefetch specific code to 'drivers/mtd/nand/omap2.c'?
> This can be done but then, we will be accessing gpmc registers outside gpmc.c. Is that ok?
Well I'm thinking that you just basically need something like this in
gpmc.c:
int gpmc_prefetch_request(int cs);
int gpmc_prefetch_enable(int cs);
int gpmc_prefetch_reset(int cs);
...
And gpmc_prefetch_request would then return an error if already taken.
No need then to tinker with the GPMC prefetch registers in the NAND
driver.
> >
> >> And then there are prefetch start, stop and status functions calls have been
> >> provided to be used by NAND driver.
> >
> > These should really be done in the platform init code for the NAND driver.
> > The NAND driver really should be generic, not omap specific.
>
> Are you talking about different omap versions or omap itself, I am bit confused. This NAND driver is
> omap specific only.
I guess I'm talking in generic terms. What's under drivers/* should in the long
run be device independent code, and the platform specific code should be under
arch/arm/*omap*.
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists