Robert, On 16/02/15 17:49, Robert ABEL wrote: > WAITMONITORINGTIME is expressed in GPMC_CLK cycles (even for asynchronous accesses). > However the driver currently converts them to GPMC_FCLK cycles, thus waitmonitoringtime in dt > had to be predivided by divider as a workaround. This patch fixes the issue by reading the Can you please point out which DT had used this pre-divided workaround? We will have to fix those then. > current GPMCFCLKDIVIDER and adjusting the divisor for WAITMONITORINGTIME accordingly. > > Signed-off-by: Robert ABEL <rabel@xxxxxxxxxxxxxxxxxxxxxxx> > --- > arch/arm/mach-omap2/gpmc.c | 78 +++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 64 insertions(+), 14 deletions(-) > > diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c Can you please rebase your patches on top of v3.19? gpmc.c has been moved to drivers/memory/omap-gpmc.c > index bae4a20..4fa5ff1 100644 > --- a/arch/arm/mach-omap2/gpmc.c > +++ b/arch/arm/mach-omap2/gpmc.c > @@ -113,6 +113,9 @@ > */ > #define GPMC_NR_IRQ 2 > > +#define GPMC_FCLK 0 > +#define GPMC_CLK 1 > + enum gpmc_clksel { GPMC_CLKSEL_FCLK, GPMC_CLKSEL_CLK, }; So all the below occurrences of "unsigned int clk_sel" become "enum gpmc_clksel". > struct gpmc_client_irq { > unsigned irq; > u32 bitmask; > @@ -208,16 +211,55 @@ static unsigned long gpmc_get_fclk_period(void) > return rate; > } > > -static unsigned int gpmc_ns_to_ticks(unsigned int time_ns) > +/** > + * gpmc_get_clk_period - get period of selected clk in ps > + * @cs : affected chip select > + * @clk_sel: either one of GPMC_CLK or GPMC_FCLK > + * > + * GPMC_CS_CONFIG1 GPMCFCLKDIVIDER for cs has to be setup > + * prior to calling this function with GPMC_CLK. > + * > + */ > +static unsigned long gpmc_get_clk_period(int cs, unsigned int clk_sel) > +{ > + > + unsigned long tick_ps = gpmc_get_fclk_period(); > + u32 l; > + int div; > + > + switch (clk_sel) { > + case GPMC_CLK: > + /* get current clk divider */ > + l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1); > + div = (l & 0x03) + 1; > + /* get GPMC_CLK period */ > + tick_ps *= div; > + break; > + case GPMC_FCLK: > + /* FALL-THROUGH */ > + default: > + break; > + } > + > + return tick_ps; > + > +} > + > +static unsigned int gpmc_ns_to_clk_ticks(unsigned int time_ns, int cs, unsigned int clk_sel) > { > unsigned long tick_ps; > > /* Calculate in picosecs to yield more exact results */ > - tick_ps = gpmc_get_fclk_period(); > + tick_ps = gpmc_get_clk_period(cs, clk_sel); > > return (time_ns * 1000 + tick_ps - 1) / tick_ps; > } > > +static unsigned int gpmc_ns_to_ticks(unsigned int time_ns) > +{ > + return gpmc_ns_to_clk_ticks(time_ns, 0, GPMC_FCLK); This function should have been unchanged since we're dealing with GPMC_FCLK and it was using gpmc_get_fclk_period(). > +} > + > static unsigned int gpmc_ps_to_ticks(unsigned int time_ps) > { > unsigned long tick_ps; > @@ -280,10 +322,10 @@ static void gpmc_cs_bool_timings(int cs, const struct gpmc_bool_timings *p) > > #ifdef DEBUG > static int set_gpmc_timing_reg(int cs, int reg, int st_bit, int end_bit, > - int time, const char *name) > + int time, unsigned int clk_sel, const char *name) > #else > static int set_gpmc_timing_reg(int cs, int reg, int st_bit, int end_bit, > - int time) > + int time, unsigned int clk_sel) > #endif > { > u32 l; > @@ -292,7 +334,7 @@ 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 = gpmc_ns_to_clk_ticks(time, cs, clk_sel); > nr_bits = end_bit - st_bit + 1; > if (ticks >= 1 << nr_bits) { > #ifdef DEBUG > @@ -307,7 +349,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: %-17s: %3d ticks, %3lu ns (was %3i ticks) %3d ns\n", > - cs, name, ticks, gpmc_get_fclk_period() * ticks / 1000, > + cs, name, ticks, gpmc_get_clk_period(cs, clk_sel) * ticks / 1000, > (l >> st_bit) & mask, time); > #endif > l &= ~(mask << st_bit); > @@ -320,12 +362,19 @@ static int set_gpmc_timing_reg(int cs, int reg, int st_bit, int end_bit, > #ifdef DEBUG > #define GPMC_SET_ONE(reg, st, end, field) \ > if (set_gpmc_timing_reg(cs, (reg), (st), (end), \ > - t->field, #field) < 0) \ > + t->field, GPMC_FCLK, #field) < 0) \ > return -1 > +#define GPMC_SET_ONE_C(reg, st, end, field, clk_sel) \ Let's call this GPMC_SET_ONE_CLKSEL() > + if (set_gpmc_timing_reg(cs, (reg), (st), (end), \ > + t->field, clk_sel, #field) < 0) \ > + return -1 > #else > #define GPMC_SET_ONE(reg, st, end, field) \ > - if (set_gpmc_timing_reg(cs, (reg), (st), (end), t->field) < 0) \ > + if (set_gpmc_timing_reg(cs, (reg), (st), (end), t->field, GPMC_FCLK) < 0) \ > return -1 > +#define GPMC_SET_ONE_C(reg, st, end, field, clk_sel) \ > + if (set_gpmc_timing_reg(cs, (reg), (st), (end), t->field, clk_sel) < 0) \ > + return -1 > #endif > > int gpmc_calc_divider(unsigned int sync_clk) > @@ -374,22 +423,23 @@ int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t) > GPMC_SET_ONE(GPMC_CS_CONFIG6, 0, 3, bus_turnaround); > GPMC_SET_ONE(GPMC_CS_CONFIG6, 8, 11, cycle2cycle_delay); > > - GPMC_SET_ONE(GPMC_CS_CONFIG1, 18, 19, wait_monitoring); > - GPMC_SET_ONE(GPMC_CS_CONFIG1, 25, 26, clk_activation); > - > if (gpmc_capability & GPMC_HAS_WR_DATA_MUX_BUS) > GPMC_SET_ONE(GPMC_CS_CONFIG6, 16, 19, wr_data_mux_bus); > if (gpmc_capability & GPMC_HAS_WR_ACCESS) > GPMC_SET_ONE(GPMC_CS_CONFIG6, 24, 28, wr_access); > > l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1); > + l &= ~0x03; > + l |= (div - 1); > + gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, l); > + > + GPMC_SET_ONE_C(GPMC_CS_CONFIG1, 18, 19, wait_monitoring, GPMC_CLK); > + GPMC_SET_ONE(GPMC_CS_CONFIG1, 25, 26, clk_activation); > + > #ifdef DEBUG > printk(KERN_INFO "GPMC CS%d CLK period is %lu ns (div %d)\n", > cs, (div * gpmc_get_fclk_period()) / 1000, div); > #endif > - l &= ~0x03; > - l |= (div - 1); > - gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, l); > > gpmc_cs_bool_timings(cs, &t->bool_timings); > > You will also need to correct gpmc_cs_show_timings() to show the correct timing for "wait-monitoring-ns" based on GPMC_CLK. cheers, -roger -- 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