From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mho-02-ewr.mailhop.org ([204.13.248.72]) by bombadil.infradead.org with esmtps (Exim 4.69 #1 (Red Hat Linux)) id 1N8IAA-0006G6-8X for linux-mtd@lists.infradead.org; Wed, 11 Nov 2009 18:43:30 +0000 Date: Wed, 11 Nov 2009 10:43:18 -0800 From: Tony Lindgren To: Vimal Singh Subject: Re: [PATCH 2/3]NAND: OMAP: Fixing omap nand driver, compiled as module Message-ID: <20091111184318.GD24837@atomide.com> References: <1257862989.21596.763.camel@localhost> <20091110185621.GB23952@atomide.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Cc: linux-omap@vger.kernel.org, Linux MTD , Artem Bityutskiy List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , * Vimal Singh [091110 20:46]: > On Wed, Nov 11, 2009 at 12:26 AM, Tony Lindgren wrote: > > * Artem Bityutskiy [091110 06:22]: > >> On Fri, 2009-10-30 at 14:57 +0530, Vimal Singh wrote: > >> > Last time I forgot to 'git add' for 'arch/arm/mach-omap2/gpmc.c'... My bad. > >> > Correct patch is below. > >> > > >> > -vimal > >> > > >> > > >> > From: Vimal Singh > >> > Date: Fri, 30 Oct 2009 14:54:29 +0530 > >> > Subject: [PATCH] NAND: OMAP: Fixing omap nand driver, compiled as module > >> > > >> > Removing OMAP NAND driver, when loaded as a module, gives error and > >> > does not get success. This fixes this and makes driver loadable and > >> > removable run time. > >> > > >> > Signed-off-by: Vimal Singh > >> > --- > >> >  arch/arm/mach-omap2/gpmc.c |    2 ++ > >> >  drivers/mtd/nand/omap2.c   |    5 ++++- > >> >  2 files changed, 6 insertions(+), 1 deletions(-) > >> > > >> > diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c > >> > index 1587682..1d10b7b 100644 > >> > --- a/arch/arm/mach-omap2/gpmc.c > >> > +++ b/arch/arm/mach-omap2/gpmc.c > >> > @@ -88,6 +88,7 @@ void gpmc_cs_write_reg(int cs, int idx, u32 val) > >> >     reg_addr = gpmc_base + GPMC_CS0 + (cs * GPMC_CS_SIZE) + idx; > >> >     __raw_writel(val, reg_addr); > >> >  } > >> > +EXPORT_SYMBOL(gpmc_cs_write_reg); > >> > > >> >  u32 gpmc_cs_read_reg(int cs, int idx) > >> >  { > >> > @@ -96,6 +97,7 @@ u32 gpmc_cs_read_reg(int cs, int idx) > >> >     reg_addr = gpmc_base + GPMC_CS0 + (cs * GPMC_CS_SIZE) + idx; > >> >     return __raw_readl(reg_addr); > >> >  } > >> > +EXPORT_SYMBOL(gpmc_cs_read_reg); > >> > >> You should get Tony's ack for this. I do not know the code, but on > >> surface it looks strange. Exporting so low-level functions is bad in > >> general, IMO. These function should either be inlined, or you should > >> invent better abstraction, so that you would not need to ever call these > >> functions from omap2.c. > > > > NAK. We don't want the drivers to tinker with these registers > > directly. And really, the drivers should be platform independent. > > > > This seems like a quick hack to add back the missing functionality > > we threw out of the linux-omap tree. It was thrown out because there > > were the same cut and paste hacks duplicated all over the place > > tinkering with the GPMC registers directly. > > > > We've fixed a lot of this by creating gpmc-onenand.c and gpmc-smc91x.c, > > and that's clearly the way to go. > > > > So instead of trying to add back the same old hacks, how about rather > > spend that time to create something that we can use for all boards > > using GPMC? > > > > To me it looks like platform init like this should be done in a > > generic way in arch/arm/mach-omap2/gpmc-nand.c the same way we have > > gpmc-onenand.c and gpmc-smc91x.c. > > > > Also, you should calculate the GPMC timings dynamically as they > > can change based on the L3 frequency. Just take a look at the > > gpmc-onenand.c and gpmc-smc91x.c. > > Ok, I'll look into these and will try to do something generic. Thanks! Tony