Re: [PATCH 1/5] ARM: OMAP2+: gpmc: Print error message in set_gpmc_timing_reg()
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 Signed-off-by: Sekhar Nori --- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] ARM: OMAP2+: gpmc: Print error message in set_gpmc_timing_reg()
* Roger Quadros [141029 01:51]: > On 10/29/2014 12:23 AM, Tony Lindgren wrote: > > * Roger Quadros [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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] ARM: OMAP2+: gpmc: Print error message in set_gpmc_timing_reg()
On 10/29/2014 12:23 AM, Tony Lindgren wrote: > * Roger Quadros [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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] ARM: OMAP2+: gpmc: Print error message in set_gpmc_timing_reg()
* Roger Quadros [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
> Signed-off-by: Sekhar Nori
> ---
> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 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
Signed-off-by: Sekhar Nori
---
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
