From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tony Lindgren Subject: Re: [PATCH 2/3]NAND: OMAP: Fixing omap nand driver, compiled as module Date: Wed, 11 Nov 2009 10:43:18 -0800 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-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mho-02-ewr.mailhop.org ([204.13.248.72]:56659 "EHLO mho-02-ewr.mailhop.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757814AbZKKSnT (ORCPT ); Wed, 11 Nov 2009 13:43:19 -0500 Content-Disposition: inline In-Reply-To: Sender: linux-omap-owner@vger.kernel.org List-Id: linux-omap@vger.kernel.org To: Vimal Singh Cc: Artem Bityutskiy , Linux MTD , linux-omap@vger.kernel.org * Vimal Singh [091110 20:46]: > On Wed, Nov 11, 2009 at 12:26 AM, Tony Lindgren wr= ote: > > * 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'= =2E.. 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 a= s 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 > >> > --- > >> > =A0arch/arm/mach-omap2/gpmc.c | =A0 =A02 ++ > >> > =A0drivers/mtd/nand/omap2.c =A0 | =A0 =A05 ++++- > >> > =A02 files changed, 6 insertions(+), 1 deletions(-) > >> > > >> > diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gp= mc.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 va= l) > >> > =A0 =A0 reg_addr =3D gpmc_base + GPMC_CS0 + (cs * GPMC_CS_SIZE) = + idx; > >> > =A0 =A0 __raw_writel(val, reg_addr); > >> > =A0} > >> > +EXPORT_SYMBOL(gpmc_cs_write_reg); > >> > > >> > =A0u32 gpmc_cs_read_reg(int cs, int idx) > >> > =A0{ > >> > @@ -96,6 +97,7 @@ u32 gpmc_cs_read_reg(int cs, int idx) > >> > =A0 =A0 reg_addr =3D gpmc_base + GPMC_CS0 + (cs * GPMC_CS_SIZE) = + idx; > >> > =A0 =A0 return __raw_readl(reg_addr); > >> > =A0} > >> > +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 shou= ld > >> 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 ther= e > > 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-smc91= x.c, > > and that's clearly the way to go. > > > > So instead of trying to add back the same old hacks, how about rath= er > > 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. >=20 > Ok, I'll look into these and will try to do something generic. Thanks! Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" i= n the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html