[PATCH 2/3]NAND: OMAP: Fixing omap nand driver, compiled as module

[Vimal Singh]

On Wed, Nov 11, 2009 at 12:26 AM, Tony Lindgren <tony at atomide.com> wrote:
> * Artem Bityutskiy <dedekind1 at gmail.com> [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 <vimalsingh at ti.com>
>> > 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 <vimalsingh at ti.com>
>> > ---
>> >  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.

-- 
Regards,
Vimal Singh