[1/5] ARM: OMAP2+: gpmc: Print error message in set_gpmc_timing_reg()

Simplify set_gpmc_timing_reg() and always print error message
if the requested timing cannot be achieved due to a too fast
GPMC functional clock, irrespective if whether DEBUG is defined
or not. This should help us debug timing configuration issues,
which were otherwise simply not being displayed in the kernel log.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
 arch/arm/mach-omap2/gpmc.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

* Roger Quadros <rogerq@ti.com> [141021 05:43]:
> Simplify set_gpmc_timing_reg() and always print error message
> if the requested timing cannot be achieved due to a too fast
> GPMC functional clock, irrespective if whether DEBUG is defined
> or not. This should help us debug timing configuration issues,
> which were otherwise simply not being displayed in the kernel log.

I think some newer versions of GPMC have a divider in the
GPMC_CONFIG regs somewhere but we're not currently using it.
Probably does not affect this patch, just FYI.

Regards,

Tony
 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> ---
>  arch/arm/mach-omap2/gpmc.c | 23 ++++++-----------------
>  1 file changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index 5fa3755..45f680f 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -283,13 +283,8 @@ static void gpmc_cs_bool_timings(int cs, const struct gpmc_bool_timings *p)
>  			   p->cycle2cyclediffcsen);
>  }
>  
> -#ifdef DEBUG
>  static int set_gpmc_timing_reg(int cs, int reg, int st_bit, int end_bit,
>  			       int time, const char *name)
> -#else
> -static int set_gpmc_timing_reg(int cs, int reg, int st_bit, int end_bit,
> -			       int time)
> -#endif
>  {
>  	u32 l;
>  	int ticks, mask, nr_bits;
> @@ -299,15 +294,15 @@ static int set_gpmc_timing_reg(int cs, int reg, int st_bit, int end_bit,
>  	else
>  		ticks = gpmc_ns_to_ticks(time);
>  	nr_bits = end_bit - st_bit + 1;
> -	if (ticks >= 1 << nr_bits) {
> -#ifdef DEBUG
> -		printk(KERN_INFO "GPMC CS%d: %-10s* %3d ns, %3d ticks >= %d\n",
> -				cs, name, time, ticks, 1 << nr_bits);
> -#endif
> +	mask = (1 << nr_bits) - 1;
> +
> +	if (ticks > mask) {
> +		pr_err("%s: GPMC error! CS%d: %s: %d ns, %d ticks > %d\n",
> +		       __func__, cs, name, time, ticks, mask);
> +
>  		return -1;
>  	}
>  
> -	mask = (1 << nr_bits) - 1;
>  	l = gpmc_cs_read_reg(cs, reg);
>  #ifdef DEBUG
>  	printk(KERN_INFO
> @@ -322,16 +317,10 @@ static int set_gpmc_timing_reg(int cs, int reg, int st_bit, int end_bit,
>  	return 0;
>  }
>  
> -#ifdef DEBUG
>  #define GPMC_SET_ONE(reg, st, end, field) \
>  	if (set_gpmc_timing_reg(cs, (reg), (st), (end),		\
>  			t->field, #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) \
> -		return -1
> -#endif
>  
>  int gpmc_calc_divider(unsigned int sync_clk)
>  {
> -- 
> 1.8.3.2
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 10/29/2014 12:23 AM, Tony Lindgren wrote:
> * Roger Quadros <rogerq@ti.com> [141021 05:43]:
>> Simplify set_gpmc_timing_reg() and always print error message
>> if the requested timing cannot be achieved due to a too fast
>> GPMC functional clock, irrespective if whether DEBUG is defined
>> or not. This should help us debug timing configuration issues,
>> which were otherwise simply not being displayed in the kernel log.
> 
> I think some newer versions of GPMC have a divider in the
> GPMC_CONFIG regs somewhere but we're not currently using it.
> Probably does not affect this patch, just FYI.

Right, we don't use it. In the future it could be a possibility that the GPMC
driver scales the clock as necessary by using the GPMC_FCLK divider
to accommodate slower devices. But then again, who needs slower devices? ;)

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

* Roger Quadros <rogerq@ti.com> [141029 01:51]:
> On 10/29/2014 12:23 AM, Tony Lindgren wrote:
> > * Roger Quadros <rogerq@ti.com> [141021 05:43]:
> >> Simplify set_gpmc_timing_reg() and always print error message
> >> if the requested timing cannot be achieved due to a too fast
> >> GPMC functional clock, irrespective if whether DEBUG is defined
> >> or not. This should help us debug timing configuration issues,
> >> which were otherwise simply not being displayed in the kernel log.
> > 
> > I think some newer versions of GPMC have a divider in the
> > GPMC_CONFIG regs somewhere but we're not currently using it.
> > Probably does not affect this patch, just FYI.
> 
> Right, we don't use it. In the future it could be a possibility that the GPMC
> driver scales the clock as necessary by using the GPMC_FCLK divider
> to accommodate slower devices. But then again, who needs slower devices? ;)

I think some devices need such slow timings that we're already
hitting the issue with 200MHz L3 on 37xx connected to a SMSC
LAN9220 at least. With LAN9221 this is not an issue with faster
timings. Anyways, I think the issue is out of the way now with
LAN9220 and GPMC_FCLK divider support can be added later on as
needed.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tuesday 21 October 2014 06:11 PM, Roger Quadros wrote:
> Simplify set_gpmc_timing_reg() and always print error message
> if the requested timing cannot be achieved due to a too fast
> GPMC functional clock, irrespective if whether DEBUG is defined
> or not. This should help us debug timing configuration issues,
> which were otherwise simply not being displayed in the kernel log.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> ---

Thanks for this patch.
Its helpful in tweaking NAND signal timing, while trying to squeeze 
read/write performance especially with new NAND devices.


with regards, pekon

------------------------
Powered by BigRock.com

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html