Hi Afzal,
Looks good! Some minor comments ...
On 05/01/2012 07:19 AM, Afzal Mohammed wrote:
> Convert GPMC code to driver. Boards using GPMC should provide driver
> with type of configuration, timing, CS. Platform devices would then be
> created for each connected peripheral (details also to be passed by
> board so that it reaches respective driver). And GPMC driver would
> populate memory resource details for the connected peripheral driver.
> Boards should inform gpmc driver with platform data destined for
> peripheral driver. gpmc driver will provide the same information to
> peripheral driver.
>
> A peripheral connected to GPMC can have multiple address spaces using
> different chip select. Hence GPMC driver has been provided capability
> to create platform device for peripheral using mutiple CS. The
> peripheral that made it necessary was tusb6010.
>
> Interrupts of GPMC are presented to drivers of connected peripherals
> as resource. A fictitious interrupt controller chip handles these
> interrupts at GPMC hardware level. Clients can use normal interrupt
> APIs. Platform information of peripheral passed to GPMC driver should
> indicate interrupts to be used via flags.
>
> Driver is capable of configuring waitpin, waitpin details has to be
> provided per CS. Wait pin has been considered as exclusive resource
> as multiple peripherals should not using the same pin, at the same
> it is valid for mutiple CS to use same waitpin provided they are
> a part of single peripheral (eg. tusb6010)
>
> An exported symbol for reconfiguring GPMC settings has been provided.
> OneNAND is the one that neccessitated this.
>
> Acquiring CS# for NAND is done on a few boards. It means, depending
> on bootloader to embed this information. Probably CS# being used can
> be set in the Kernel, and acquiring it can be removed. If ever this
> capbility is needed, GPMC driver has to be made aware of handling it.
>
> Modifications has been made keeping in mind that the driver would
> have to move to driver folder. This explains requirement of clk_prd
> field; even though clk_prd variable is not necessary as
> gpmc_get_fclk_period is present in the same file as of now, this will
> help in moving the driver easily to drivers folder.
>
> Code related to GPMC clock may have to continue live in platform
> folders as input clock is beyond the control of GPMC and calculating
> timing for the peripheral may need other helpers. This explains
> presence of 'gpmc_cs_calc_divider' along with 'gpmc_calc_divider',
> both doing same work, latter meant to go with driver, former for
> calculation in platform code.
>
> Thanks to Vaibhav Hiremath & Jonathan Hunter on their various good
> suggestions which resulted in improving the code.
>
> Cc: Vaibhav Hiremath <hvaibhav@xxxxxx>
> Cc: Jon Hunter <jon-hunter@xxxxxx>
> Signed-off-by: Afzal Mohammed <afzal@xxxxxx>
> ---
> arch/arm/mach-omap2/gpmc.c | 877 ++++++++++++++++++++++++++++----
> arch/arm/plat-omap/include/plat/gpmc.h | 93 +++-
> 2 files changed, 872 insertions(+), 98 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index 580e684..12916f3 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -14,8 +14,11 @@
> */
> #undef DEBUG
>
> +#include <linux/platform_device.h>
> +
> #include <linux/irq.h>
> #include <linux/kernel.h>
> +#include <linux/slab.h>
> #include <linux/init.h>
> #include <linux/err.h>
> #include <linux/clk.h>
> @@ -53,6 +56,45 @@
> #define GPMC_CS0_OFFSET 0x60
> #define GPMC_CS_SIZE 0x30
>
> +/* GPMC register bits */
> +#define GPMC_CONFIG1_TIMEPARAGRANULARITY BIT(4)
> +#define GPMC_CONFIG1_DEVICETYPE_NAND GPMC_CONFIG1_DEVICETYPE(0x2)
> +#define GPMC_CONFIG1_WAIT_PIN_SEL_MASK GPMC_CONFIG1_WAIT_PIN_SEL(0x3)
> +#define GPMC_CONFIG1_WAIT_MON_TIME(val) ((val & 0x3) << 18)
> +#define GPMC_CONFIG1_WRITEMULTIPLE BIT(28)
> +#define GPMC_CONFIG1_READMULTIPLE BIT(30)
> +#define GPMC_CONFIG1_WRAPBURST BIT(31)
> +#define GPMC_CONFIG_WAITPIN_POLARITY_SHIFT 0x8
> +#define GPMC_CONFIG1_WAITPIN_MONITOR_TIME(val) ((val & 0x3) << 18)
> +#define GPMC_CONFIG1_WAITPIN_MONITOR_TIME_1 \
> + GPMC_CONFIG1_WAITPIN_MONITOR_TIME(0x1)
> +#define GPMC_CONFIG1_WAITPIN_MONITOR_TIME_2 \
> + GPMC_CONFIG1_WAITPIN_MONITOR_TIME(0x2)
> +#define GPMC_CONFIG1_CLOCKACTIVATION_TIME(val) ((val & 0x3) << 25)
> +#define GPMC_CONFIG1_CLOCKACTIVATION_TIME_1 \
> + GPMC_CONFIG1_CLOCKACTIVATION_TIME(0x1)
> +#define GPMC_CONFIG1_CLOCKACTIVATION_TIME_2 \
> + GPMC_CONFIG1_CLOCKACTIVATION_TIME(0x2)
> +
> +#define GPMC_CONFIG2_CSEXTRADELAY BIT(7)
> +
> +#define GPMC_CONFIG3_ADVEXTRADELAY BIT(7)
> +
> +#define GPMC_CONFIG4_OEEXTRADELAY BIT(7)
> +#define GPMC_CONFIG4_WEEXTRADELAY BIT(23)
> +
> +#define GPMC_CONFIG6_CYCLE2CYCLEDIFFCSEN BIT(6)
> +#define GPMC_CONFIG6_CYCLE2CYCLESAMECSEN BIT(7)
> +
> +#define GPMC_IRQ_BIT_FIFOEVENT BIT(0)
> +#define GPMC_IRQ_BIT_TERMINALCOUNT BIT(1)
> +
> +#define GPMC_WAITPIN_IDX0 0x0
> +#define GPMC_WAITPIN_IDX1 0x1
> +#define GPMC_WAITPIN_IDX2 0x2
> +#define GPMC_WAITPIN_IDX3 0x3
> +#define GPMC_NR_WAITPIN 0x4
How about an enum here? Also OMAP4 only have 3 wait pins and so the
number is device dependent.
> #define GPMC_MEM_START 0x00000000
> #define GPMC_MEM_END 0x3FFFFFFF
> #define BOOT_ROM_SPACE 0x100000 /* 1MB */
> @@ -64,6 +106,55 @@
> #define ENABLE_PREFETCH (0x1 << 7)
> #define DMA_MPU_MODE 2
>
> +#define DRIVER_NAME "omap-gpmc"
> +
> +#define GPMC_NR_IRQ 2
Why has this been changed to 2? It was 6 in the previous version. As we
discussed before the number of IRQs should be detected based upon GPMC
IP version.
> +
> +#define HIGH 1
> +#define LOW -1
> +
> +struct gpmc_device {
> + char *name;
> + int id;
> + void *pdata;
> + unsigned pdata_size;
> + struct resource *per_res;
> + unsigned per_res_cnt;
> + struct resource *gpmc_res;
> + unsigned gpmc_res_cnt;
> + bool have_waitpin;
> + unsigned waitpin;
> + int waitpin_polarity;
I think that it make more sense to have the wait-pin information part of
the gpmc_cs_data structure for the following reasons ...
1. If a device can use multiple CS, then it could use multiple wait signals.
2. Eventually, all board information needs to move to device tree. By
having it in the pdata it will be easier to migrate to device tree.
It may be nice to have "have_waitpin" be an integer too, that indicates
if wait is used for both read and write or just one or the other.
Also, any reason why waitpin_polarity is an int? I see you define LOW as
-1, but I not sure why LOW cannot be 0 as 0 is programmed into the
register for an active low wait signal.
> +};
> +
> +struct gpmc_irq {
> + unsigned irq;
> + u32 bitmask;
> +};
> +
> +struct gpmc {
> + struct device *dev;
> + unsigned long clk_prd;
> + void __iomem *io_base;
> + unsigned long phys_base;
> + u32 memsize;
> + unsigned cs_map;
> + int ecc_used;
> + spinlock_t mem_lock;
> + struct resource mem_root;
> + struct resource cs_mem[GPMC_CS_NUM];
> + /* XXX: Or allocate dynamically ? */
> + struct gpmc_device device[GPMC_CS_NUM];
> + unsigned num_device;
> + unsigned master_irq;
> + unsigned irq_start;
> + struct gpmc_irq irq[GPMC_NR_IRQ];
> + struct irq_chip irq_chip;
> + bool wp;
> + unsigned waitpin_map;
> + unsigned revision;
> +};
> +
> /* Structure to save gpmc cs context */
> struct gpmc_cs_config {
> u32 config1;
> @@ -91,58 +182,50 @@ struct omap3_gpmc_regs {
> struct gpmc_cs_config cs_context[GPMC_CS_NUM];
> };
>
> -static struct resource gpmc_mem_root;
> -static struct resource gpmc_cs_mem[GPMC_CS_NUM];
> -static DEFINE_SPINLOCK(gpmc_mem_lock);
> -static unsigned int gpmc_cs_map; /* flag for cs which are initialized */
> -static int gpmc_ecc_used = -EINVAL; /* cs using ecc engine */
> -
> -static void __iomem *gpmc_base;
> -
> static struct clk *gpmc_l3_clk;
>
> -static irqreturn_t gpmc_handle_irq(int irq, void *dev);
> +static struct gpmc *gpmc;
>
> static void gpmc_write_reg(int idx, u32 val)
> {
> - __raw_writel(val, gpmc_base + idx);
> + writel(val, gpmc->io_base + idx);
> }
>
> static u32 gpmc_read_reg(int idx)
> {
> - return __raw_readl(gpmc_base + idx);
> + return readl(gpmc->io_base + idx);
> }
>
> static void gpmc_cs_write_byte(int cs, int idx, u8 val)
> {
> void __iomem *reg_addr;
>
> - reg_addr = gpmc_base + GPMC_CS0_OFFSET + (cs * GPMC_CS_SIZE) + idx;
> - __raw_writeb(val, reg_addr);
> + reg_addr = gpmc->io_base + GPMC_CS0_OFFSET + (cs * GPMC_CS_SIZE) + idx;
> + writeb(val, reg_addr);
> }
>
> static u8 gpmc_cs_read_byte(int cs, int idx)
> {
> void __iomem *reg_addr;
>
> - reg_addr = gpmc_base + GPMC_CS0_OFFSET + (cs * GPMC_CS_SIZE) + idx;
> - return __raw_readb(reg_addr);
> + reg_addr = gpmc->io_base + GPMC_CS0_OFFSET + (cs * GPMC_CS_SIZE) + idx;
> + return readb(reg_addr);
> }
>
> void gpmc_cs_write_reg(int cs, int idx, u32 val)
> {
> void __iomem *reg_addr;
>
> - reg_addr = gpmc_base + GPMC_CS0_OFFSET + (cs * GPMC_CS_SIZE) + idx;
> - __raw_writel(val, reg_addr);
> + reg_addr = gpmc->io_base + GPMC_CS0_OFFSET + (cs * GPMC_CS_SIZE) + idx;
> + writel(val, reg_addr);
> }
>
> u32 gpmc_cs_read_reg(int cs, int idx)
> {
> void __iomem *reg_addr;
>
> - reg_addr = gpmc_base + GPMC_CS0_OFFSET + (cs * GPMC_CS_SIZE) + idx;
> - return __raw_readl(reg_addr);
> + reg_addr = gpmc->io_base + GPMC_CS0_OFFSET + (cs * GPMC_CS_SIZE) + idx;
> + return readl(reg_addr);
> }
>
> /* TODO: Add support for gpmc_fck to clock framework and use it */
> @@ -207,7 +290,8 @@ static int set_gpmc_timing_reg(int cs, int reg, int st_bit, int end_bit,
> if (time == 0)
> ticks = 0;
> else
> - ticks = gpmc_ns_to_ticks(time);
> + ticks = (time * 1000 + gpmc->clk_prd - 1) / gpmc->clk_prd;
> +
> nr_bits = end_bit - st_bit + 1;
> if (ticks >= 1 << nr_bits) {
> #ifdef DEBUG
> @@ -222,7 +306,7 @@ static int set_gpmc_timing_reg(int cs, int reg, int st_bit, int end_bit,
> #ifdef DEBUG
> printk(KERN_INFO
> "GPMC CS%d: %-10s: %3d ticks, %3lu ns (was %3i ticks) %3d ns\n",
> - cs, name, ticks, gpmc_get_fclk_period() * ticks / 1000,
> + cs, name, ticks, gpmc->clk_prd * ticks / 1000,
> (l >> st_bit) & mask, time);
> #endif
> l &= ~(mask << st_bit);
> @@ -243,6 +327,21 @@ static int set_gpmc_timing_reg(int cs, int reg, int st_bit, int end_bit,
> return -1
> #endif
>
> +int gpmc_calc_divider(unsigned int sync_clk)
> +{
> + int div;
> + u32 l;
> +
> + l = sync_clk + (gpmc->clk_prd - 1);
> + div = l / gpmc->clk_prd;
> + if (div > 4)
> + return -1;
> + if (div <= 0)
> + div = 1;
> +
> + return div;
> +}
> +
> int gpmc_cs_calc_divider(int cs, unsigned int sync_clk)
> {
> int div;
> @@ -258,12 +357,53 @@ int gpmc_cs_calc_divider(int cs, unsigned int sync_clk)
> return div;
> }
>
> +static void gpmc_cs_onoff_timings(int cs, const struct gpmc_onoff_timings *p)
> +{
> + u32 l;
> +
> + l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG2);
> + if (p->cs_extra_delay)
> + l |= GPMC_CONFIG2_CSEXTRADELAY;
> + else
> + l &= ~GPMC_CONFIG2_CSEXTRADELAY;
> + gpmc_cs_write_reg(cs, GPMC_CS_CONFIG2, l);
> +
> + l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG3);
> + if (p->adv_extra_delay)
> + l |= GPMC_CONFIG3_ADVEXTRADELAY;
> + else
> + l &= ~GPMC_CONFIG3_ADVEXTRADELAY;
> + gpmc_cs_write_reg(cs, GPMC_CS_CONFIG3, l);
> +
> + l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG4);
> + if (p->oe_extra_delay)
> + l |= GPMC_CONFIG4_OEEXTRADELAY;
> + else
> + l &= ~GPMC_CONFIG4_OEEXTRADELAY;
> + if (p->we_extra_delay)
> + l |= GPMC_CONFIG4_WEEXTRADELAY;
> + else
> + l &= ~GPMC_CONFIG4_WEEXTRADELAY;
> + gpmc_cs_write_reg(cs, GPMC_CS_CONFIG4, l);
> +
> + l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG6);
> + if (p->cycle2cyclesamecsen)
> + l |= GPMC_CONFIG6_CYCLE2CYCLESAMECSEN;
> + else
> + l &= ~GPMC_CONFIG6_CYCLE2CYCLESAMECSEN;
> + if (p->cycle2cyclediffcsen)
> + l |= GPMC_CONFIG6_CYCLE2CYCLEDIFFCSEN;
> + else
> + l &= ~GPMC_CONFIG6_CYCLE2CYCLEDIFFCSEN;
> + gpmc_cs_write_reg(cs, GPMC_CS_CONFIG6, l);
> +}
> +
> int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t)
> {
> int div;
> u32 l;
>
> - div = gpmc_cs_calc_divider(cs, t->sync_clk);
> + div = gpmc_calc_divider(t->sync_clk);
> if (div < 0)
> return -1;
>
> @@ -286,7 +426,10 @@ int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t)
>
> GPMC_SET_ONE(GPMC_CS_CONFIG5, 24, 27, page_burst_access);
>
> - if (cpu_is_omap34xx()) {
> + GPMC_SET_ONE(GPMC_CS_CONFIG6, 8, 11, cycle2cycle_delay);
> + GPMC_SET_ONE(GPMC_CS_CONFIG6, 0, 3, busturnaround);
> +
> + if (gpmc->revision >= 5) {
> GPMC_SET_ONE(GPMC_CS_CONFIG6, 16, 19, wr_data_mux_bus);
> GPMC_SET_ONE(GPMC_CS_CONFIG6, 24, 28, wr_access);
> }
> @@ -298,13 +441,15 @@ int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t)
> if (l & (GPMC_CONFIG1_READTYPE_SYNC | GPMC_CONFIG1_WRITETYPE_SYNC)) {
> #ifdef DEBUG
> printk(KERN_INFO "GPMC CS%d CLK period is %lu ns (div %d)\n",
> - cs, (div * gpmc_get_fclk_period()) / 1000, div);
> + cs, (div * gpmc->clk_prd) / 1000, div);
> #endif
> l &= ~0x03;
> l |= (div - 1);
> gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, l);
> }
>
> + gpmc_cs_onoff_timings(cs, &t->control);
> +
> return 0;
> }
>
> @@ -332,7 +477,7 @@ static void gpmc_cs_disable_mem(int cs)
> gpmc_cs_write_reg(cs, GPMC_CS_CONFIG7, l);
> }
>
> -static void gpmc_cs_get_memconf(int cs, u32 *base, u32 *size)
> +static __devinit void gpmc_cs_get_memconf(int cs, u32 *base, u32 *size)
> {
> u32 l;
> u32 mask;
> @@ -351,23 +496,23 @@ static int gpmc_cs_mem_enabled(int cs)
> return l & GPMC_CONFIG7_CSVALID;
> }
>
> -int gpmc_cs_set_reserved(int cs, int reserved)
> +static int gpmc_cs_set_reserved(int cs, int reserved)
> {
> if (cs > GPMC_CS_NUM)
> return -ENODEV;
>
> - gpmc_cs_map &= ~(1 << cs);
> - gpmc_cs_map |= (reserved ? 1 : 0) << cs;
> + gpmc->cs_map &= ~(1 << cs);
> + gpmc->cs_map |= (reserved ? 1 : 0) << cs;
>
> return 0;
> }
>
> -int gpmc_cs_reserved(int cs)
> +static int gpmc_cs_reserved(int cs)
> {
> if (cs > GPMC_CS_NUM)
> return -ENODEV;
>
> - return gpmc_cs_map & (1 << cs);
> + return gpmc->cs_map & (1 << cs);
> }
>
> static unsigned long gpmc_mem_align(unsigned long size)
> @@ -384,24 +529,25 @@ static unsigned long gpmc_mem_align(unsigned long size)
> return size;
> }
>
> -static int gpmc_cs_insert_mem(int cs, unsigned long base, unsigned long size)
> +static __devinit
> +int gpmc_cs_insert_mem(int cs, unsigned long base, unsigned long size)
> {
> - struct resource *res = &gpmc_cs_mem[cs];
> + struct resource *res = &gpmc->cs_mem[cs];
> int r;
>
> size = gpmc_mem_align(size);
> - spin_lock(&gpmc_mem_lock);
> + spin_lock(&gpmc->mem_lock);
> res->start = base;
> res->end = base + size - 1;
> - r = request_resource(&gpmc_mem_root, res);
> - spin_unlock(&gpmc_mem_lock);
> + r = request_resource(&gpmc->mem_root, res);
> + spin_unlock(&gpmc->mem_lock);
>
> return r;
> }
>
> int gpmc_cs_request(int cs, unsigned long size, unsigned long *base)
> {
> - struct resource *res = &gpmc_cs_mem[cs];
> + struct resource *res = &gpmc->cs_mem[cs];
> int r = -1;
>
> if (cs > GPMC_CS_NUM)
> @@ -411,7 +557,7 @@ int gpmc_cs_request(int cs, unsigned long size, unsigned long *base)
> if (size > (1 << GPMC_SECTION_SHIFT))
> return -ENOMEM;
>
> - spin_lock(&gpmc_mem_lock);
> + spin_lock(&gpmc->mem_lock);
> if (gpmc_cs_reserved(cs)) {
> r = -EBUSY;
> goto out;
> @@ -419,7 +565,7 @@ int gpmc_cs_request(int cs, unsigned long size, unsigned long *base)
> if (gpmc_cs_mem_enabled(cs))
> r = adjust_resource(res, res->start & ~(size - 1), size);
> if (r < 0)
> - r = allocate_resource(&gpmc_mem_root, res, size, 0, ~0,
> + r = allocate_resource(&gpmc->mem_root, res, size, 0, ~0,
> size, NULL, NULL);
> if (r < 0)
> goto out;
> @@ -428,24 +574,24 @@ int gpmc_cs_request(int cs, unsigned long size, unsigned long *base)
> *base = res->start;
> gpmc_cs_set_reserved(cs, 1);
> out:
> - spin_unlock(&gpmc_mem_lock);
> + spin_unlock(&gpmc->mem_lock);
> return r;
> }
> EXPORT_SYMBOL(gpmc_cs_request);
>
> void gpmc_cs_free(int cs)
> {
> - spin_lock(&gpmc_mem_lock);
> + spin_lock(&gpmc->mem_lock);
> if (cs >= GPMC_CS_NUM || cs < 0 || !gpmc_cs_reserved(cs)) {
> printk(KERN_ERR "Trying to free non-reserved GPMC CS%d\n", cs);
> BUG();
> - spin_unlock(&gpmc_mem_lock);
> + spin_unlock(&gpmc->mem_lock);
> return;
> }
> gpmc_cs_disable_mem(cs);
> - release_resource(&gpmc_cs_mem[cs]);
> + release_resource(&gpmc->cs_mem[cs]);
> gpmc_cs_set_reserved(cs, 0);
> - spin_unlock(&gpmc_mem_lock);
> + spin_unlock(&gpmc->mem_lock);
> }
> EXPORT_SYMBOL(gpmc_cs_free);
>
> @@ -668,7 +814,7 @@ int gpmc_prefetch_reset(int cs)
> }
> EXPORT_SYMBOL(gpmc_prefetch_reset);
>
> -static void __init gpmc_mem_init(void)
> +static __devinit void gpmc_mem_init(void)
> {
> int cs;
> unsigned long boot_rom_space = 0;
> @@ -680,8 +826,8 @@ static void __init gpmc_mem_init(void)
> /* In apollon the CS0 is mapped as 0x0000 0000 */
> if (machine_is_omap_apollon())
> boot_rom_space = 0;
> - gpmc_mem_root.start = GPMC_MEM_START + boot_rom_space;
> - gpmc_mem_root.end = GPMC_MEM_END;
> + gpmc->mem_root.start = GPMC_MEM_START + boot_rom_space;
> + gpmc->mem_root.end = GPMC_MEM_END;
>
> /* Reserve all regions that has been set up by bootloader */
> for (cs = 0; cs < GPMC_CS_NUM; cs++) {
> @@ -697,26 +843,15 @@ static void __init gpmc_mem_init(void)
>
> static int __init gpmc_init(void)
> {
> - u32 l, irq;
> - int cs, ret = -EINVAL;
> - int gpmc_irq;
> + int ret = -EINVAL;
> char *ck = NULL;
>
> if (cpu_is_omap24xx()) {
> ck = "core_l3_ck";
> - if (cpu_is_omap2420())
> - l = OMAP2420_GPMC_BASE;
> - else
> - l = OMAP34XX_GPMC_BASE;
> - gpmc_irq = INT_34XX_GPMC_IRQ;
> } else if (cpu_is_omap34xx()) {
> ck = "gpmc_fck";
> - l = OMAP34XX_GPMC_BASE;
> - gpmc_irq = INT_34XX_GPMC_IRQ;
> } else if (cpu_is_omap44xx()) {
> ck = "gpmc_ck";
> - l = OMAP44XX_GPMC_BASE;
> - gpmc_irq = OMAP44XX_IRQ_GPMC;
> }
>
> if (WARN_ON(!ck))
> @@ -728,53 +863,607 @@ static int __init gpmc_init(void)
> BUG();
> }
>
> - gpmc_base = ioremap(l, SZ_4K);
> - if (!gpmc_base) {
> - clk_put(gpmc_l3_clk);
> - printk(KERN_ERR "Could not get GPMC register memory\n");
> - BUG();
> + clk_enable(gpmc_l3_clk);
> +
> + return 0;
> +}
> +postcore_initcall(gpmc_init);
> +
> +static inline int gpmc_waitpin_is_reserved(struct gpmc *gpmc, unsigned waitpin)
> +{
> + return gpmc->waitpin_map & (0x1 << waitpin);
> +}
> +
> +static inline void gpmc_reserve_waitpin(struct gpmc *gpmc, unsigned waitpin)
> +{
> + gpmc->waitpin_map &= ~(0x1 << waitpin);
> + gpmc->waitpin_map |= (0x1 << waitpin);
> +}
> +
> +static int gpmc_waitpin_request(struct gpmc *gpmc, unsigned waitpin)
> +{
> + if (!(waitpin < GPMC_NR_WAITPIN))
> + return -ENODEV;
> +
> + if (gpmc_waitpin_is_reserved(gpmc, waitpin))
> + return -EBUSY;
> + else
> + gpmc_reserve_waitpin(gpmc, waitpin);
> +
> + return 0;
> +}
> +
> +static int gpmc_setup_waitpin(struct gpmc *gpmc, struct gpmc_device *gd)
> +{
> + int ret;
> +
> + if (!gd->have_waitpin)
> + return 0;
> +
> + ret = gpmc_waitpin_request(gpmc, gd->waitpin);
> + if (IS_ERR_VALUE(ret)) {
> + dev_err(gpmc->dev, "waitpin %u reserved\n", gd->waitpin);
> + return ret;
> + } else if (gd->waitpin_polarity) {
> + u32 l = gpmc_read_reg(GPMC_CONFIG);
> + u32 shift = gd->waitpin + GPMC_CONFIG_WAITPIN_POLARITY_SHIFT;
> +
> + if (gd->waitpin_polarity == HIGH)
> + l |= 1 << shift;
> + else
> + l &= ~(1 << shift);
> +
> + gpmc_write_reg(GPMC_CONFIG, l);
> }
> + return 0;
> +}
>
> - clk_enable(gpmc_l3_clk);
> +static int gpmc_setup_cs_waitpin(struct gpmc *gpmc, struct gpmc_device *gd,
> + unsigned cs, unsigned conf)
> +{
> + u32 l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
> + unsigned idx = ~0x0;
> + int polarity = 0;
>
> - l = gpmc_read_reg(GPMC_REVISION);
> - printk(KERN_INFO "GPMC revision %d.%d\n", (l >> 4) & 0x0f, l & 0x0f);
> - /* Set smart idle mode and automatic L3 clock gating */
> - l = gpmc_read_reg(GPMC_SYSCONFIG);
> - l &= 0x03 << 3;
> - l |= (0x02 << 3) | (1 << 0);
> - gpmc_write_reg(GPMC_SYSCONFIG, l);
> - gpmc_mem_init();
> + switch (conf & GPMC_WAITPIN_MASK) {
> + case GPMC_WAITPIN_0:
> + idx = GPMC_WAITPIN_IDX0;
> + break;
> + case GPMC_WAITPIN_1:
> + idx = GPMC_WAITPIN_IDX1;
> + break;
> + case GPMC_WAITPIN_2:
> + idx = GPMC_WAITPIN_IDX2;
> + break;
> + case GPMC_WAITPIN_3:
> + idx = GPMC_WAITPIN_IDX3;
> + break;
> + /* no waitpin */
> + case 0:
> + break;
> + default:
> + dev_err(gpmc->dev, "multiple waitpins selected on CS:%u\n", cs);
> + return -EINVAL;
> + break;
> + }
>
> - /* initalize the irq_chained */
> - irq = OMAP_GPMC_IRQ_BASE;
> - for (cs = 0; cs < GPMC_CS_NUM; cs++) {
> - irq_set_chip_and_handler(irq, &dummy_irq_chip,
> - handle_simple_irq);
> - set_irq_flags(irq, IRQF_VALID);
> - irq++;
> + switch (conf & GPMC_WAITPIN_POLARITY_MASK) {
> + case GPMC_WAITPIN_ACTIVE_LOW:
> + polarity = LOW;
> + break;
> + case GPMC_WAITPIN_ACTIVE_HIGH:
> + polarity = HIGH;
> + break;
> + /* no waitpin */
> + case 0:
> + break;
> + default:
> + dev_err(gpmc->dev, "waitpin polarity set to low & high\n");
> + return -EINVAL;
> + break;
> }
>
> - ret = request_irq(gpmc_irq, gpmc_handle_irq, IRQF_SHARED, "gpmc", NULL);
> - if (ret)
> - pr_err("gpmc: irq-%d could not claim: err %d\n",
> - gpmc_irq, ret);
> - return ret;
> + if (idx != ~0x0) {
> + if (gd->have_waitpin) {
> + if (gd->waitpin != idx ||
> + gd->waitpin_polarity != polarity) {
> + dev_err(gpmc->dev, "error: conflict: waitpin %u with polarity %d on device %s.%d\n",
> + gd->waitpin, gd->waitpin_polarity,
> + gd->name, gd->id);
> + return -EBUSY;
> + }
> + } else {
> + gd->have_waitpin = true;
> + gd->waitpin = idx;
> + gd->waitpin_polarity = polarity;
> + }
I am not sure about the above code. What happens if a device has
multiple CS signals and uses multiple wait signals?
I am also not sure why gpmc_setup_cs_waitpin and gpmc_setup_waitpin
cannot be combined. I think that it would be a fair assumption to make
that anyone using the waitpin does so inconjunction with the CS (I think
that they would have too).
However, if there is a reason for splitting it up this way please
explain why.
> +
> + l &= ~GPMC_CONFIG1_WAIT_PIN_SEL_MASK;
> + l |= GPMC_CONFIG1_WAIT_PIN_SEL(idx);
> + gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, l);
> + } else if (polarity) {
> + dev_err(gpmc->dev, "error: waitpin polarity specified with out wait pin number on device %s.%d\n",
> + gd->name, gd->id);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static void gpmc_setup_cs_config(struct gpmc *gpmc, unsigned cs, unsigned conf)
> +{
> + u32 l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
> +
> + l &= ~(GPMC_CONFIG1_TIMEPARAGRANULARITY |
> + GPMC_CONFIG1_MUXADDDATA |
> + GPMC_CONFIG1_DEVICETYPE(~0) |
> + GPMC_CONFIG1_DEVICESIZE(~0) |
> + GPMC_CONFIG1_WAIT_WRITE_MON |
> + GPMC_CONFIG1_WAIT_READ_MON |
> + GPMC_CONFIG1_PAGE_LEN(~0) |
> + GPMC_CONFIG1_WRITETYPE_SYNC |
> + GPMC_CONFIG1_WRITEMULTIPLE |
> + GPMC_CONFIG1_READTYPE_SYNC |
> + GPMC_CONFIG1_READMULTIPLE |
> + GPMC_CONFIG1_WRAPBURST |
> + GPMC_CONFIG1_WAITPIN_MONITOR_TIME(~0) |
> + GPMC_CONFIG1_CLOCKACTIVATION_TIME(~0));
> +
> + if (conf & GPMC_TIMEPARAGRANULARITY)
> + l |= GPMC_CONFIG1_TIMEPARAGRANULARITY;
> + if (conf & GPMC_MUXADDDATA)
> + l |= GPMC_CONFIG1_MUXADDDATA;
> + if (conf & GPMC_DEVICETYPE_NAND)
> + l |= GPMC_CONFIG1_DEVICETYPE_NAND;
> + if (conf & GPMC_DEVICESIZE_16)
> + l |= GPMC_CONFIG1_DEVICESIZE_16;
> + if (conf & GPMC_WAIT_WRITE_MON)
> + l |= GPMC_CONFIG1_WAIT_WRITE_MON;
> + if (conf & GPMC_WAIT_READ_MON)
> + l |= GPMC_CONFIG1_WAIT_READ_MON;
> + if (conf & GPMC_PAGE_LEN_16)
> + l |= GPMC_CONFIG1_PAGE_LEN_16;
> + else if (conf & GPMC_PAGE_LEN_8)
> + l |= GPMC_CONFIG1_PAGE_LEN_8;
> + if (conf & GPMC_WRITETYPE_SYNC)
> + l |= GPMC_CONFIG1_WRITETYPE_SYNC;
> + if (conf & GPMC_WRITEMULTIPLE)
> + l |= GPMC_CONFIG1_WRITEMULTIPLE;
> + if (conf & GPMC_READTYPE_SYNC)
> + l |= GPMC_CONFIG1_READTYPE_SYNC;
> + if (conf & GPMC_READMULTIPLE)
> + l |= GPMC_CONFIG1_READMULTIPLE;
> + if (conf & GPMC_WRAPBURST)
> + l |= GPMC_CONFIG1_WRAPBURST;
> + if (conf & GPMC_WAITPIN_MONITOR_TIME_1)
> + l |= GPMC_CONFIG1_WAITPIN_MONITOR_TIME_1;
> + else if (conf & GPMC_WAITPIN_MONITOR_TIME_2)
> + l |= GPMC_CONFIG1_WAITPIN_MONITOR_TIME_2;
> + if (conf & GPMC_CLOCKACTIVATION_TIME_1)
> + l |= GPMC_CONFIG1_CLOCKACTIVATION_TIME_1;
> + else if (conf & GPMC_CLOCKACTIVATION_TIME_2)
> + l |= GPMC_CONFIG1_CLOCKACTIVATION_TIME_2;
> +
> + gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, l);
> +
> + if (conf & GPMC_WRITEPROTECT)
> + gpmc->wp = true;
> +}
> +
> +static int gpmc_setup_cs_nonres(struct gpmc *gpmc,
> + struct gpmc_device *gd, struct gpmc_cs_data *cs)
The name of this function is not 100% clear to me. What do you mean by
"nonres"?
> +{
> + int ret;
> +
> + /* some boards rely on bootloader for configuration */
> + if (cs->have_config) {
> + gpmc_setup_cs_config(gpmc, cs->cs, cs->config);
> + ret = gpmc_setup_cs_waitpin(gpmc, gd, cs->cs, cs->config);
> + if (IS_ERR_VALUE(ret)) {
> + dev_err(gpmc->dev, "error: waitpin on CS %d\n", cs->cs);
> + return ret;
> + }
> + } else
> + dev_warn(gpmc->dev, "config not present for CS: %d\n", cs->cs);
> +
> + if (cs->timing) {
> + ret = gpmc_cs_set_timings(cs->cs, cs->timing);
> + if (IS_ERR_VALUE(ret)) {
> + dev_err(gpmc->dev, "error: timing on CS: %d\n", cs->cs);
> + return ret;
> + }
> + } else
> + dev_warn(gpmc->dev, "timing not present for CS: %u\n", cs->cs);
> +
> + return 0;
> +}
> +
> +static int gpmc_match_device(struct gpmc *gpmc, char *name, int id)
> +{
> + int i;
> + struct gpmc_device *gd;
> +
> + for (i = 0, gd = gpmc->device; i < gpmc->num_device; i++, gd++)
> + if (!strcmp(gd->name, name) && gd->id == id)
> + return i;
> +
> + return -ENOENT;
> +}
> +
> +int gpmc_cs_reconfigure(char *name, int id, struct gpmc_cs_data *cs)
What scenario is this function used in? May be worth adding a comment
about the function.
> +{
> + int i;
> +
> + i = gpmc_match_device(gpmc, name, id);
> + if (IS_ERR_VALUE(i)) {
> + dev_err(gpmc->dev, "no device %s.%d to configure\n", name, id);
> + return i;
> + }
> +
> + i = gpmc_setup_cs_nonres(gpmc, gpmc->device + i, cs);
> + if (IS_ERR_VALUE(i)) {
> + dev_err(gpmc->dev, "error: configure device %s.%d\n", name, id);
> + return i;
> + }
> +
> + return gpmc_setup_waitpin(gpmc, gpmc->device + i);
> +
> +}
> +EXPORT_SYMBOL_GPL(gpmc_cs_reconfigure);
> +
> +static inline unsigned int gpmc_bit_to_irq(unsigned bitmask)
> +{
> + if (bitmask & GPMC_IRQ_BIT_FIFOEVENT)
> + return GPMC_IRQ_FIFOEVENTENABLE;
> + else if (bitmask & GPMC_IRQ_BIT_TERMINALCOUNT)
> + return GPMC_IRQ_COUNT_EVENT;
> + else
> + return 0;
> +}
> +
> +static __devinit int gpmc_setup_cs_irq(struct gpmc *gpmc,
> + struct gpmc_cs_data *cs, struct resource *res)
> +{
> + int i, n;
> +
> + for (i = 0, n = 0; i < GPMC_NR_IRQ; i++)
> + if (gpmc_bit_to_irq(gpmc->irq[i].bitmask) & cs->irq_config) {
> + res[n].start = res[n].end = gpmc->irq[i].irq;
> + res[n].flags = IORESOURCE_IRQ;
> + dev_info(gpmc->dev, "irq %u on CS %d\n",
> + res[n].start, cs->cs);
> + n++;
> + }
> +
> + return n;
> +}
> +
> +static __devinit int gpmc_setup_cs_mem(struct gpmc *gpmc,
> + struct gpmc_cs_data *cs, struct resource *res)
> +{
> + unsigned long start;
> + int ret;
> +
> + ret = gpmc_cs_request(cs->cs, cs->mem_size, &start);
> + if (IS_ERR_VALUE(ret)) {
> + dev_err(gpmc->dev, "error: gpmc request on CS: %d\n", cs->cs);
> + return ret;
> + }
> +
> + res->start = start + cs->mem_offset;
> + res->end = res->start + cs->mem_size - 1;
> + res->flags = IORESOURCE_MEM;
> +
> + dev_info(gpmc->dev, "memory 0x%x-0x%x on CS %d\n", res->start,
> + res->end, cs->cs);
> +
> + return 1;
> +}
> +
> +static __devinit int gpmc_setup_cs(struct gpmc *gpmc, struct gpmc_device *gd,
> + struct gpmc_cs_data *cs, struct resource *res)
> +{
> + int num, ret;
> +
> + num = gpmc_setup_cs_mem(gpmc, cs, res);
> + if (IS_ERR_VALUE(num))
> + return num;
> +
> + ret = gpmc_setup_cs_nonres(gpmc, gd, cs);
> + if (IS_ERR_VALUE(ret))
> + return ret;
> +
> + num += gpmc_setup_cs_irq(gpmc, cs, res + num);
> +
> + return num;
> +}
> +
> +static __devinit int gpmc_setup_device(struct gpmc *gpmc,
> + struct gpmc_device *gd, struct gpmc_device_pdata *gdp)
> +{
> + int i, n, ret;
> + struct gpmc_cs_data *cs;
> +
> + for (i = 0, n = gdp->num_cs, cs = gdp->cs_data;
> + i < gdp->num_cs; i++, cs++)
> + n += hweight32(cs->irq_config);
As you know I am not a fan of these overloaded for-loops as I find them
hard to read ;-)
Why not ...
n = gdp->num_cs;
cs = gdp->cs_data;
/* Calculate total number of irqs used by device */
for (i = 0, i < gdp->num_cs; i++)
n += hweight32(cs[i].irq_flags & GPMC_IRQ_MASK);
> + gd->gpmc_res = devm_kzalloc(gpmc->dev, sizeof(*gd->gpmc_res) * n,
> + GFP_KERNEL);
> + if (gd->gpmc_res == NULL) {
> + dev_err(gpmc->dev, "error: memory allocation\n");
> + return -ENOMEM;
> + }
> +
> + for (i = 0, cs = gdp->cs_data, gd->gpmc_res_cnt = 0;
> + i < gdp->num_cs; cs++, i++) {
By the way, if you make the above change, you can also remove "cs =
gdp->cs_data" from this for-loop as it is already initialised.
> + ret = gpmc_setup_cs(gpmc, gd, cs,
> + gd->gpmc_res + gd->gpmc_res_cnt);
> + if (IS_ERR_VALUE(ret) ||
> + IS_ERR_VALUE(gpmc_setup_waitpin(gpmc, gd))) {
May be consider moving gpmc_setup_waitpin into gpmc_setup_cs as it makes
sense that it is part of the cs setup.
> + dev_err(gpmc->dev, "error: setup for %s\n", gdp->name);
> + devm_kfree(gpmc->dev, gd->gpmc_res);
> + gd->gpmc_res = NULL;
> + gd->gpmc_res_cnt = 0;
> + return -EINVAL;
> + } else
> + gd->gpmc_res_cnt += ret;
> + }
> +
> + gd->name = gdp->name;
> + gd->id = gdp->id;
> + gd->pdata = gdp->pdata;
> + gd->pdata_size = gdp->pdata_size;
> + gd->per_res = gdp->per_res;
> + gd->per_res_cnt = gdp->per_res_cnt;
> +
> + return 0;
> +}
> +
> +static __devinit
> +struct platform_device *gpmc_create_device(struct gpmc *gpmc,
> + struct gpmc_device *p)
> +{
> + int num = p->per_res_cnt + p->gpmc_res_cnt;
> + struct resource *res;
> + struct platform_device *pdev;
> +
> + res = devm_kzalloc(gpmc->dev, sizeof(struct resource) * num,
> + GFP_KERNEL);
> + if (!res) {
> + dev_err(gpmc->dev, "error: allocating memory\n");
> + return NULL;
> + }
> +
> + memcpy((char *)res, (const char *) p->gpmc_res,
> + sizeof(struct resource) * p->gpmc_res_cnt);
> + memcpy((char *)(res + p->gpmc_res_cnt), (const char *)p->per_res,
> + sizeof(struct resource) * p->per_res_cnt);
> +
> + pdev = platform_device_register_resndata(gpmc->dev, p->name, p->id,
> + res, num, p->pdata, p->pdata_size);
> +
> + devm_kfree(gpmc->dev, res);
> +
> + return pdev;
> }
> -postcore_initcall(gpmc_init);
>
> static irqreturn_t gpmc_handle_irq(int irq, void *dev)
> {
> - u8 cs;
> + int i;
> + u32 regval;
> + struct gpmc *gpmc = dev;
> +
> + regval = gpmc_read_reg(GPMC_IRQSTATUS);
>
> - /* check cs to invoke the irq */
> - cs = ((gpmc_read_reg(GPMC_PREFETCH_CONFIG1)) >> CS_NUM_SHIFT) & 0x7;
> - if (OMAP_GPMC_IRQ_BASE+cs <= OMAP_GPMC_IRQ_END)
> - generic_handle_irq(OMAP_GPMC_IRQ_BASE+cs);
> + for (i = 0; i < GPMC_NR_IRQ; i++)
> + if (regval & gpmc->irq[i].bitmask)
> + generic_handle_irq(gpmc->irq[i].irq);
> +
> + gpmc_write_reg(GPMC_IRQSTATUS, regval);
>
> return IRQ_HANDLED;
> }
>
> +static int gpmc_irq_endis(struct gpmc *gpmc, unsigned irq, bool endis)
> +{
> + int i;
> + u32 regval;
> +
> + for (i = 0; i < GPMC_NR_IRQ; i++)
> + if (irq == gpmc->irq[i].irq) {
> + regval = gpmc_read_reg(GPMC_IRQENABLE);
> + if (endis)
> + regval |= gpmc->irq[i].bitmask;
> + else
> + regval &= ~gpmc->irq[i].bitmask;
> + gpmc_write_reg(GPMC_IRQENABLE, regval);
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static void gpmc_irq_disable(struct irq_data *p)
> +{
> + gpmc_irq_endis(irq_data_get_irq_chip_data(p), p->irq, false);
> +}
> +
> +static void gpmc_irq_enable(struct irq_data *p)
> +{
> + gpmc_irq_endis(irq_data_get_irq_chip_data(p), p->irq, true);
> +}
> +
> +static void gpmc_irq_noop(struct irq_data *data) { }
> +
> +static unsigned int gpmc_irq_noop_ret(struct irq_data *data) { return 0; }
> +
> +static __devinit int gpmc_setup_irq(struct gpmc *gpmc)
> +{
> + int i;
> + u32 regval;
> +
> + if (!gpmc->master_irq)
> + return -EINVAL;
> +
> + gpmc->irq_start = irq_alloc_descs(-1, 0, GPMC_NR_IRQ, 0);
> + if (IS_ERR_VALUE(gpmc->irq_start)) {
> + dev_err(gpmc->dev, "irq_alloc_descs failed\n");
> + return gpmc->irq_start;
> + }
> +
> + gpmc->irq_chip.name = "gpmc";
> + gpmc->irq_chip.irq_startup = gpmc_irq_noop_ret;
> + gpmc->irq_chip.irq_enable = gpmc_irq_enable;
> + gpmc->irq_chip.irq_disable = gpmc_irq_disable;
> + gpmc->irq_chip.irq_shutdown = gpmc_irq_noop;
> + gpmc->irq_chip.irq_ack = gpmc_irq_noop;
> + gpmc->irq_chip.irq_mask = gpmc_irq_noop;
> + gpmc->irq_chip.irq_unmask = gpmc_irq_noop;
> +
> + gpmc->irq[0].bitmask = GPMC_IRQ_BIT_FIFOEVENT;
> + gpmc->irq[1].bitmask = GPMC_IRQ_BIT_TERMINALCOUNT;
> +
> + for (i = 0; i < GPMC_NR_IRQ; i++) {
> + gpmc->irq[i].irq = gpmc->irq_start + i;
> + irq_set_chip_and_handler(gpmc->irq[i].irq,
> + &gpmc->irq_chip, handle_simple_irq);
> + irq_set_chip_data(gpmc->irq[i].irq, gpmc);
> + set_irq_flags(gpmc->irq[i].irq, IRQF_VALID | IRQF_NOAUTOEN);
> + }
> +
> + /* Disable interrupts */
> + gpmc_write_reg(GPMC_IRQENABLE, 0);
> +
> + /* clear interrupts */
> + regval = gpmc_read_reg(GPMC_IRQSTATUS);
> + gpmc_write_reg(GPMC_IRQSTATUS, regval);
> +
> + return request_irq(gpmc->master_irq, gpmc_handle_irq, IRQF_SHARED,
> + "gpmc", gpmc);
> +}
> +
> +static __devinit void gpmc_setup_writeprotect(struct gpmc *gpmc)
> +{
> + u32 l;
> +
> + l = gpmc_read_reg(GPMC_CONFIG);
> + if (gpmc->wp == true) {
> + l &= ~GPMC_CONFIG_WRITEPROTECT;
> + dev_info(gpmc->dev, "write protect enabled\n");
> + } else
> + l |= GPMC_CONFIG_WRITEPROTECT;
> + gpmc_write_reg(GPMC_CONFIG, l);
> +}
> +
> +static __devinit int gpmc_probe(struct platform_device *pdev)
> +{
> + u32 l;
> + int i;
> + int ret = 0;
> + struct resource *res = NULL;
> + struct gpmc_pdata *gp = dev_get_platdata(&pdev->dev);
> + struct gpmc_device_pdata **gdq = NULL;
> + struct gpmc_device *gd = NULL;
> +
> + gpmc = devm_kzalloc(&pdev->dev, sizeof(struct gpmc), GFP_KERNEL);
> + if (gpmc == NULL)
> + return -ENOMEM;
> +
> + gpmc->dev = &pdev->dev;
> + gpmc->clk_prd = gp->clk_prd;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (res == NULL)
> + return -ENOENT;
> +
> + gpmc->phys_base = res->start;
> + gpmc->memsize = resource_size(res);
> +
> + gpmc->io_base = devm_request_and_ioremap(gpmc->dev, res);
> + if (!gpmc->io_base)
> + return -EADDRNOTAVAIL;
> +
> + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (res == NULL)
> + dev_warn(gpmc->dev, "Failed to get resource: irq\n");
> + else
> + gpmc->master_irq = res->start;
> +
> + if (IS_ERR_VALUE(gpmc_setup_irq(gpmc)))
> + dev_warn(gpmc->dev, "gpmc_setup_irq failed\n");
> +
> + gpmc->ecc_used = -EINVAL;
> + spin_lock_init(&gpmc->mem_lock);
> + platform_set_drvdata(pdev, gpmc);
> +
> + l = gpmc_read_reg(GPMC_REVISION);
> + gpmc->revision = (l >> 4) & 0xf;
> + dev_info(gpmc->dev, "GPMC revision %u.%u\n", gpmc->revision, l & 0x0f);
> +
> + gpmc_mem_init();
> +
> + for (i = 0, gdq = gp->device_pdata, gd = gpmc->device;
> + (i < gp->num_device) && (*gdq); i++, gdq++) {
You have num_devices now and so do you really need the "&& (*gdq)"?
Seems redundant.
> + ret = gpmc_setup_device(gpmc, gd, *gdq);
gdp[i]
> + if (IS_ERR_VALUE(ret))
> + dev_err(gpmc->dev, "gpmc setup on %s failed\n",
> + (*gdq)->name);
> + else
> + gd++;
> + }
> + gpmc->num_device = gd - gpmc->device;
> +
> + gpmc_setup_writeprotect(gpmc);
Write protect is a pin and there is only one. Like the waitpins and CS
signals this needs to be associated with a device. It would make sense
that this is associated with the cs data.
> +
> + for (i = 0, gd = gpmc->device; i < gpmc->num_device; i++, gd++)
> + if (IS_ERR(gpmc_create_device(gpmc, gd)))
> + dev_err(gpmc->dev, "device creation on %s failed\n",
> + gd->name);
> +
> + return 0;
> +}
> +
> +static __devexit int gpmc_free_irq(struct gpmc *gpmc)
> +{
> + int i;
> +
> + if (gpmc->master_irq)
> + free_irq(gpmc->master_irq, gpmc);
> +
> + for (i = 0; i < GPMC_NR_IRQ; i++) {
> + irq_set_handler(gpmc->irq[i].irq, NULL);
> + irq_set_chip(gpmc->irq[i].irq, &no_irq_chip);
> + irq_set_chip_data(gpmc->irq[i].irq, NULL);
> + irq_modify_status(gpmc->irq[i].irq, 0, 0);
> + }
> +
> + irq_free_descs(gpmc->irq_start, GPMC_NR_IRQ);
> +
> + return 0;
> +}
> +
> +static __devexit int gpmc_remove(struct platform_device *pdev)
> +{
> + struct gpmc *gpmc = platform_get_drvdata(pdev);
> +
> + platform_set_drvdata(pdev, NULL);
> + gpmc_free_irq(gpmc);
> +
> + return 0;
> +}
> +
> +static struct platform_driver gpmc_driver = {
> + .probe = gpmc_probe,
> + .remove = __devexit_p(gpmc_remove),
> + .driver = {
> + .name = DRIVER_NAME,
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +module_platform_driver(gpmc_driver);
> +
> #ifdef CONFIG_ARCH_OMAP3
> static struct omap3_gpmc_regs gpmc_context;
>
> @@ -854,10 +1543,10 @@ int gpmc_enable_hwecc(int cs, int mode, int dev_width, int ecc_size)
> unsigned int val;
>
> /* check if ecc module is in used */
> - if (gpmc_ecc_used != -EINVAL)
> + if (gpmc->ecc_used != -EINVAL)
> return -EINVAL;
>
> - gpmc_ecc_used = cs;
> + gpmc->ecc_used = cs;
>
> /* clear ecc and enable bits */
> val = ((0x00000001<<8) | 0x00000001);
> @@ -905,7 +1594,7 @@ int gpmc_calculate_ecc(int cs, const u_char *dat, u_char *ecc_code)
> {
> unsigned int val = 0x0;
>
> - if (gpmc_ecc_used != cs)
> + if (gpmc->ecc_used != cs)
> return -EINVAL;
>
> /* read ecc result */
> @@ -915,7 +1604,7 @@ int gpmc_calculate_ecc(int cs, const u_char *dat, u_char *ecc_code)
> /* P2048o, P1024o, P512o, P256o, P2048e, P1024e, P512e, P256e */
> *ecc_code++ = ((val >> 8) & 0x0f) | ((val >> 20) & 0xf0);
>
> - gpmc_ecc_used = -EINVAL;
> + gpmc->ecc_used = -EINVAL;
> return 0;
> }
> EXPORT_SYMBOL_GPL(gpmc_calculate_ecc);
> diff --git a/arch/arm/plat-omap/include/plat/gpmc.h b/arch/arm/plat-omap/include/plat/gpmc.h
> index 1527929..2eedd99 100644
> --- a/arch/arm/plat-omap/include/plat/gpmc.h
> +++ b/arch/arm/plat-omap/include/plat/gpmc.h
> @@ -11,6 +11,44 @@
> #ifndef __OMAP2_GPMC_H
> #define __OMAP2_GPMC_H
>
> +/* configuration flags */
> +#define GPMC_WRITEPROTECT BIT(0)
> +#define GPMC_TIMEPARAGRANULARITY BIT(1)
> +#define GPMC_MUXADDDATA BIT(2)
> +#define GPMC_DEVICETYPE_NOR BIT(3)
> +#define GPMC_DEVICETYPE_NAND BIT(4)
> +#define GPMC_DEVICESIZE_8 BIT(5)
> +#define GPMC_DEVICESIZE_16 BIT(6)
> +#define GPMC_WAIT_WRITE_MON BIT(7)
> +#define GPMC_WAIT_READ_MON BIT(8)
> +#define GPMC_PAGE_LEN_4 BIT(9)
> +#define GPMC_PAGE_LEN_8 BIT(10)
> +#define GPMC_PAGE_LEN_16 BIT(11)
> +#define GPMC_CLOCKACTIVATIONTIME_0 BIT(12)
> +#define GPMC_CLOCKACTIVATIONTIME_1 BIT(13)
> +#define GPMC_CLOCKACTIVATIONTIME_2 BIT(14)
> +#define GPMC_WRITETYPE_SYNC BIT(15)
> +#define GPMC_WRITEMULTIPLE BIT(16)
> +#define GPMC_READTYPE_SYNC BIT(17)
> +#define GPMC_READMULTIPLE BIT(18)
> +#define GPMC_WRAPBURST BIT(19)
> +#define GPMC_WAITPIN_0 BIT(20)
> +#define GPMC_WAITPIN_1 BIT(21)
> +#define GPMC_WAITPIN_2 BIT(22)
> +#define GPMC_WAITPIN_3 BIT(23)
> +#define GPMC_WAITPIN_MASK (GPMC_WAITPIN_0 | GPMC_WAITPIN_1 | \
> + GPMC_WAITPIN_2 | GPMC_WAITPIN_3)
> +#define GPMC_WAITPIN_ACTIVE_LOW BIT(24)
> +#define GPMC_WAITPIN_ACTIVE_HIGH BIT(25)
> +#define GPMC_WAITPIN_POLARITY_MASK (GPMC_WAITPIN_ACTIVE_LOW | \
> + GPMC_WAITPIN_ACTIVE_HIGH)
> +#define GPMC_WAITPIN_MONITOR_TIME_1 BIT(26)
> +#define GPMC_WAITPIN_MONITOR_TIME_2 BIT(27)
> +#define GPMC_CLOCKACTIVATION_TIME_1 BIT(28)
> +#define GPMC_CLOCKACTIVATION_TIME_2 BIT(29)
> +#define GPMC_READTYPE_ASYNC BIT(30)
> +#define GPMC_WRITETYPE_ASYNC BIT(31)
> +
Please keep in mind that eventually this has to all be moved into device
tree and so at that point these configuration flags will have to be
re-worked. However, just a FYI.
Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
