Re: [PATCH 7/8 v2] ARM OMAP2+ GPMC: calculate GPMCFCLKDIVIDER based on WAITMONITORINGTIME
Robert,
On 25/02/15 19:20, Robert Abel wrote:
> Hi Roger,
>
> On 25 Feb 2015 17:33, Roger Quadros wrote:
>> ./scripts/checkpatch.pl detects some styling errors.
> Well, there's like a million lines over 80 characters. I'm also a heathen and
> don't use an 80 character terminal either.
> I'll fix the more serious issues checkpatch finds, but some styling errors
> are even from code already in the driver.
neither do I but let's fix whatever is possible to fix
in the new code.
>
>>> #define GPMC_CONFIG1_WAIT_MON_IIME(val) ((val & 3) << 18)
>> Not caused by your patch but we can fix the typo
>>
>> GPMC_CONFIG1_WAIT_MON_IIME -> GPMC_CONFIG1_WAIT_MON_TIME
> I'd rather remove than fix. None of these defines is in active use anymore.
OK with me.
>>> +/**
>>> + * gpmc_calc_waitmonitoring_divider - calculate proper GPMCFCLKDIVIDER
>>> based on WAITMONITORINGTIME
>>> + * @wait_monitoring WAITMONITORINGTIME in ns.
>>> + * @return -1 on failure to scale, else proper divider > 0.
>>> + * @noteWAITMONITORINGTIME will be _at least_ as long as
>>> desired.
>>> + * i.e. read timings should be kept-> don't
>>> sample bus too early
>>> + * while write timings should work out as well -> data is
>>> longer on bus
>>> + */
>>> +static int gpmc_calc_waitmonitoring_divider(unsigned int wait_monitoring)
>>> +{
>>> +
>>> +int div = gpmc_ns_to_ticks(wait_monitoring);
>>> +
>>> +div += GPMC_CONFIG1_WAIT_MON_TIME_MAX - 1;
>>> +div /= GPMC_CONFIG1_WAIT_MON_TIME_MAX;
>> Sorry I didn't understand how this works. :P
>> From the TRM,
>>
>> waitmonitoringtime_ns = waitmonitoring_ticks x (gpmc_clk_div + 1) x
>> gpmc_fclk_ns
> Using that formula:
> gpmc_clk_div + 1 = ceil(ceil(waitmonitoringtime_ns / gpmc_fclk_ns) /
> waitmonitoring_ticks).
Right, we need to return div = gpmc_div + 1 to take care of the (div - 1) done
by the caller before
programming div.
I wouldn't mind if we update gpmc_calc_divider() and this function to return
the actual div value
that needs to be programmed in the register. It is upto you.
How about mentioning the above formula in a comment for benefit of future
readers?
you could also mention that reducing the formula to ticks we get.
gpmc_div + 1 = ceil(wanted_waitmonitoring_ticks/waitmonitoring_ticks)
>
> The reason I use GPMC_CONFIG1_WAIT_MON_TIME_MAX directly, is because every
> other pair is identical:
>
> We have the following cases:
> a) WAITMONITORINGTIME = 0, so div doesn't matter (we set div to 1) or
> b) WAITMONITORINGTIME = 1, with div = 1 or
> c) WAITMONITORINGTIME = 2, with div = n
>
> WAITMONITORINGTIME is never 1 with div = n and div /= 1, because that's the
> same as WAITMONITORINGTIME = 2 with div = n - 1.
> Cases a) and b) are caught using the fact that div = 0 after the division.
> Case c) is handled by assuming that WAITMONITORINGTIME will always be 2 (i.e.
> GPMC_CONFIG1_WAIT_MON_TIME_MAX) for all divs /= 0 at that point.
>
> Took me a while to realize that this works, too.
>
>>> +/*
>>> + * See if we need to change the divider for waitmonitoringtime.
>>> + *
>>> + * If DT contains sync-clk-ps for truly asynchronous accesses,
>>> + * ignore it as long as waitmonitoringtime is used.
>>> + *
>> The comment in the $subject was more easier to understand for me than the
>> above
>>
>> "Calculate GPMCFCLKDIVIDER independent of gpmc,sync-clk-ps in DT for
>> truly asynchronous accesses, i.e. both read and write asynchronous."
> Well, the comment in $subject was a general description. Here, we don't touch
> div when
> a) any access is synchronous
> b) all accesses are asynchronous, but WAITMONITORINGTIME is not used, i.e.
> WAITREADMONITORING and WAITWRITEMONITORING are both not set.
>
> So, if WAITMONITORINGTIME is not used, or any access is synchronous, we
> program the sync-clk-ps anyways, because the GPMC won't rely on it for any
> timing.
> So the comment is OK, IMHO.
OK with me.
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8 v2] ARM OMAP2+ GPMC: calculate GPMCFCLKDIVIDER based on WAITMONITORINGTIME
Robert,
On 25/02/15 19:20, Robert Abel wrote:
Hi Roger,
On 25 Feb 2015 17:33, Roger Quadros wrote:
./scripts/checkpatch.pl detects some styling errors.
Well, there's like a million lines over 80 characters. I'm also a heathen and
don't use an 80 character terminal either.
I'll fix the more serious issues checkpatch finds, but some styling errors
are even from code already in the driver.
neither do I but let's fix whatever is possible to fix
in the new code.
#define GPMC_CONFIG1_WAIT_MON_IIME(val) ((val 3) 18)
Not caused by your patch but we can fix the typo
GPMC_CONFIG1_WAIT_MON_IIME - GPMC_CONFIG1_WAIT_MON_TIME
I'd rather remove than fix. None of these defines is in active use anymore.
OK with me.
+/**
+ * gpmc_calc_waitmonitoring_divider - calculate proper GPMCFCLKDIVIDER
based on WAITMONITORINGTIME
+ * @wait_monitoring WAITMONITORINGTIME in ns.
+ * @return -1 on failure to scale, else proper divider 0.
+ * @noteWAITMONITORINGTIME will be _at least_ as long as
desired.
+ * i.e. read timings should be kept- don't
sample bus too early
+ * while write timings should work out as well - data is
longer on bus
+ */
+static int gpmc_calc_waitmonitoring_divider(unsigned int wait_monitoring)
+{
+
+int div = gpmc_ns_to_ticks(wait_monitoring);
+
+div += GPMC_CONFIG1_WAIT_MON_TIME_MAX - 1;
+div /= GPMC_CONFIG1_WAIT_MON_TIME_MAX;
Sorry I didn't understand how this works. :P
From the TRM,
waitmonitoringtime_ns = waitmonitoring_ticks x (gpmc_clk_div + 1) x
gpmc_fclk_ns
Using that formula:
gpmc_clk_div + 1 = ceil(ceil(waitmonitoringtime_ns / gpmc_fclk_ns) /
waitmonitoring_ticks).
Right, we need to return div = gpmc_div + 1 to take care of the (div - 1) done
by the caller before
programming div.
I wouldn't mind if we update gpmc_calc_divider() and this function to return
the actual div value
that needs to be programmed in the register. It is upto you.
How about mentioning the above formula in a comment for benefit of future
readers?
you could also mention that reducing the formula to ticks we get.
gpmc_div + 1 = ceil(wanted_waitmonitoring_ticks/waitmonitoring_ticks)
The reason I use GPMC_CONFIG1_WAIT_MON_TIME_MAX directly, is because every
other pair is identical:
We have the following cases:
a) WAITMONITORINGTIME = 0, so div doesn't matter (we set div to 1) or
b) WAITMONITORINGTIME = 1, with div = 1 or
c) WAITMONITORINGTIME = 2, with div = n
WAITMONITORINGTIME is never 1 with div = n and div /= 1, because that's the
same as WAITMONITORINGTIME = 2 with div = n - 1.
Cases a) and b) are caught using the fact that div = 0 after the division.
Case c) is handled by assuming that WAITMONITORINGTIME will always be 2 (i.e.
GPMC_CONFIG1_WAIT_MON_TIME_MAX) for all divs /= 0 at that point.
Took me a while to realize that this works, too.
+/*
+ * See if we need to change the divider for waitmonitoringtime.
+ *
+ * If DT contains sync-clk-ps for truly asynchronous accesses,
+ * ignore it as long as waitmonitoringtime is used.
+ *
The comment in the $subject was more easier to understand for me than the
above
Calculate GPMCFCLKDIVIDER independent of gpmc,sync-clk-ps in DT for
truly asynchronous accesses, i.e. both read and write asynchronous.
Well, the comment in $subject was a general description. Here, we don't touch
div when
a) any access is synchronous
b) all accesses are asynchronous, but WAITMONITORINGTIME is not used, i.e.
WAITREADMONITORING and WAITWRITEMONITORING are both not set.
So, if WAITMONITORINGTIME is not used, or any access is synchronous, we
program the sync-clk-ps anyways, because the GPMC won't rely on it for any
timing.
So the comment is OK, IMHO.
OK with me.
cheers,
-roger
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8 v2] ARM OMAP2+ GPMC: calculate GPMCFCLKDIVIDER based on WAITMONITORINGTIME
On 25 Feb 2015 18:20, Roger Quadros wrote: Need to patch mach-omap2/usb-tusb6010.c as well. else we get arch/arm/mach-omap2/usb-tusb6010.c: In function ‘tusb_set_async_mode’: arch/arm/mach-omap2/usb-tusb6010.c:74:2: error: too few arguments to function ‘gpmc_cs_set_timings’ In file included from arch/arm/mach-omap2/gpmc.h:14:0, from arch/arm/mach-omap2/usb-tusb6010.c:23: include/linux/omap-gpmc.h:166:12: note: declared here arch/arm/mach-omap2/usb-tusb6010.c: In function ‘tusb_set_sync_mode’: arch/arm/mach-omap2/usb-tusb6010.c:101:2: error: too few arguments to function ‘gpmc_cs_set_timings’ In file included from arch/arm/mach-omap2/gpmc.h:14:0, from arch/arm/mach-omap2/usb-tusb6010.c:23: include/linux/omap-gpmc.h:166:12: note: declared here Argh... Another one of those exported function calls :| Robert -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8 v2] ARM OMAP2+ GPMC: calculate GPMCFCLKDIVIDER based on WAITMONITORINGTIME
Hi Roger,
On 25 Feb 2015 17:33, Roger Quadros wrote:
./scripts/checkpatch.pl detects some styling errors.
Well, there's like a million lines over 80 characters. I'm also a
heathen and don't use an 80 character terminal either.
I'll fix the more serious issues checkpatch finds, but some styling
errors are even from code already in the driver.
#define GPMC_CONFIG1_WAIT_MON_IIME(val) ((val & 3) << 18)
Not caused by your patch but we can fix the typo
GPMC_CONFIG1_WAIT_MON_IIME -> GPMC_CONFIG1_WAIT_MON_TIME
I'd rather remove than fix. None of these defines is in active use anymore.
+/**
+ * gpmc_calc_waitmonitoring_divider - calculate proper GPMCFCLKDIVIDER based
on WAITMONITORINGTIME
+ * @wait_monitoring WAITMONITORINGTIME in ns.
+ * @return -1 on failure to scale, else proper divider > 0.
+ * @noteWAITMONITORINGTIME will be _at least_ as long as desired.
+ * i.e. read timings should be kept-> don't
sample bus too early
+ * while write timings should work out as well -> data is
longer on bus
+ */
+static int gpmc_calc_waitmonitoring_divider(unsigned int wait_monitoring)
+{
+
+ int div = gpmc_ns_to_ticks(wait_monitoring);
+
+ div += GPMC_CONFIG1_WAIT_MON_TIME_MAX - 1;
+ div /= GPMC_CONFIG1_WAIT_MON_TIME_MAX;
Sorry I didn't understand how this works. :P
From the TRM,
waitmonitoringtime_ns = waitmonitoring_ticks x (gpmc_clk_div + 1) x gpmc_fclk_ns
Using that formula:
gpmc_clk_div + 1 = ceil(ceil(waitmonitoringtime_ns / gpmc_fclk_ns) /
waitmonitoring_ticks).
The reason I use GPMC_CONFIG1_WAIT_MON_TIME_MAX directly, is because
every other pair is identical:
We have the following cases:
a) WAITMONITORINGTIME = 0, so div doesn't matter (we set div to 1) or
b) WAITMONITORINGTIME = 1, with div = 1 or
c) WAITMONITORINGTIME = 2, with div = n
WAITMONITORINGTIME is never 1 with div = n and div /= 1, because that's
the same as WAITMONITORINGTIME = 2 with div = n - 1.
Cases a) and b) are caught using the fact that div = 0 after the division.
Case c) is handled by assuming that WAITMONITORINGTIME will always be 2
(i.e. GPMC_CONFIG1_WAIT_MON_TIME_MAX) for all divs /= 0 at that point.
Took me a while to realize that this works, too.
+ /*
+* See if we need to change the divider for waitmonitoringtime.
+*
+* If DT contains sync-clk-ps for truly asynchronous accesses,
+* ignore it as long as waitmonitoringtime is used.
+*
The comment in the $subject was more easier to understand for me than the above
"Calculate GPMCFCLKDIVIDER independent of gpmc,sync-clk-ps in DT for
truly asynchronous accesses, i.e. both read and write asynchronous."
Well, the comment in $subject was a general description. Here, we don't
touch div when
a) any access is synchronous
b) all accesses are asynchronous, but WAITMONITORINGTIME is not used,
i.e. WAITREADMONITORING and WAITWRITEMONITORING are both not set.
So, if WAITMONITORINGTIME is not used, or any access is synchronous, we
program the sync-clk-ps anyways, because the GPMC won't rely on it for
any timing.
So the comment is OK, IMHO.
Regards,
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8 v2] ARM OMAP2+ GPMC: calculate GPMCFCLKDIVIDER based on WAITMONITORINGTIME
Robert, On 25/02/15 18:33, Roger Quadros wrote: > On 24/02/15 22:05, Robert ABEL wrote: >> The WAITMONITORINGTIME is expressed as a number of GPMC_CLK clock cycles, >> even though the access is defined as asynchronous, and no GPMC_CLK clock >> is provided to the external device. Still, GPMCFCLKDIVIDER is used as a >> divider >> for the GPMC clock, so it must be programmed to define the >> correct WAITMONITORINGTIME delay. >> >> Calculate GPMCFCLKDIVIDER independent of gpmc,sync-clk-ps in DT for >> truly asynchronous accesses, i.e. both read and write asynchronous. >> >> Signed-off-by: Robert ABEL >> --- >> arch/arm/mach-omap2/gpmc-nand.c| 17 - >> arch/arm/mach-omap2/gpmc-onenand.c | 4 +-- >> drivers/memory/omap-gpmc.c | 74 >> ++ >> include/linux/omap-gpmc.h | 2 +- >> 4 files changed, 80 insertions(+), 17 deletions(-) Need to patch mach-omap2/usb-tusb6010.c as well. else we get arch/arm/mach-omap2/usb-tusb6010.c: In function ‘tusb_set_async_mode’: arch/arm/mach-omap2/usb-tusb6010.c:74:2: error: too few arguments to function ‘gpmc_cs_set_timings’ In file included from arch/arm/mach-omap2/gpmc.h:14:0, from arch/arm/mach-omap2/usb-tusb6010.c:23: include/linux/omap-gpmc.h:166:12: note: declared here arch/arm/mach-omap2/usb-tusb6010.c: In function ‘tusb_set_sync_mode’: arch/arm/mach-omap2/usb-tusb6010.c:101:2: error: too few arguments to function ‘gpmc_cs_set_timings’ In file included from arch/arm/mach-omap2/gpmc.h:14:0, from arch/arm/mach-omap2/usb-tusb6010.c:23: include/linux/omap-gpmc.h:166:12: note: declared here cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8 v2] ARM OMAP2+ GPMC: calculate GPMCFCLKDIVIDER based on WAITMONITORINGTIME
On 24/02/15 22:05, Robert ABEL wrote:
> The WAITMONITORINGTIME is expressed as a number of GPMC_CLK clock cycles,
> even though the access is defined as asynchronous, and no GPMC_CLK clock
> is provided to the external device. Still, GPMCFCLKDIVIDER is used as a
> divider
> for the GPMC clock, so it must be programmed to define the
> correct WAITMONITORINGTIME delay.
>
> Calculate GPMCFCLKDIVIDER independent of gpmc,sync-clk-ps in DT for
> truly asynchronous accesses, i.e. both read and write asynchronous.
>
> Signed-off-by: Robert ABEL
> ---
> arch/arm/mach-omap2/gpmc-nand.c| 17 -
> arch/arm/mach-omap2/gpmc-onenand.c | 4 +--
> drivers/memory/omap-gpmc.c | 74
> ++
> include/linux/omap-gpmc.h | 2 +-
> 4 files changed, 80 insertions(+), 17 deletions(-)
>
./scripts/checkpatch.pl detects some styling errors.
> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
> index d5951b1..e863a59 100644
> --- a/arch/arm/mach-omap2/gpmc-nand.c
> +++ b/arch/arm/mach-omap2/gpmc-nand.c
> @@ -96,14 +96,6 @@ int gpmc_nand_init(struct omap_nand_platform_data
> *gpmc_nand_data,
> gpmc_nand_res[1].start = gpmc_get_client_irq(GPMC_IRQ_FIFOEVENTENABLE);
> gpmc_nand_res[2].start = gpmc_get_client_irq(GPMC_IRQ_COUNT_EVENT);
>
> - if (gpmc_t) {
> - err = gpmc_cs_set_timings(gpmc_nand_data->cs, gpmc_t);
> - if (err < 0) {
> - pr_err("omap2-gpmc: Unable to set gpmc timings: %d\n",
> err);
> - return err;
> - }
> - }
> -
> memset(, 0, sizeof(struct gpmc_settings));
> if (gpmc_nand_data->of_node)
> gpmc_read_settings_dt(gpmc_nand_data->of_node, );
> @@ -111,6 +103,15 @@ int gpmc_nand_init(struct omap_nand_platform_data
> *gpmc_nand_data,
> gpmc_set_legacy(gpmc_nand_data, );
>
> s.device_nand = true;
> +
> + if (gpmc_t) {
> + err = gpmc_cs_set_timings(gpmc_nand_data->cs, gpmc_t, );
> + if (err < 0) {
> + pr_err("omap2-gpmc: Unable to set gpmc timings: %d\n",
> err);
> + return err;
> + }
> + }
> +
> err = gpmc_cs_program_settings(gpmc_nand_data->cs, );
> if (err < 0)
> goto out_free_cs;
> diff --git a/arch/arm/mach-omap2/gpmc-onenand.c
> b/arch/arm/mach-omap2/gpmc-onenand.c
> index 53d197e..f899e77 100644
> --- a/arch/arm/mach-omap2/gpmc-onenand.c
> +++ b/arch/arm/mach-omap2/gpmc-onenand.c
> @@ -293,7 +293,7 @@ static int omap2_onenand_setup_async(void __iomem
> *onenand_base)
> if (ret < 0)
> return ret;
>
> - ret = gpmc_cs_set_timings(gpmc_onenand_data->cs, );
> + ret = gpmc_cs_set_timings(gpmc_onenand_data->cs, , _async);
> if (ret < 0)
> return ret;
>
> @@ -331,7 +331,7 @@ static int omap2_onenand_setup_sync(void __iomem
> *onenand_base, int *freq_ptr)
> if (ret < 0)
> return ret;
>
> - ret = gpmc_cs_set_timings(gpmc_onenand_data->cs, );
> + ret = gpmc_cs_set_timings(gpmc_onenand_data->cs, , _sync);
> if (ret < 0)
> return ret;
>
> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
> index a6abaf0..9cf8d21 100644
> --- a/drivers/memory/omap-gpmc.c
> +++ b/drivers/memory/omap-gpmc.c
> @@ -139,6 +139,8 @@
> #define GPMC_CONFIG1_WAIT_READ_MON (1 << 22)
> #define GPMC_CONFIG1_WAIT_WRITE_MON (1 << 21)
> #define GPMC_CONFIG1_WAIT_MON_IIME(val) ((val & 3) << 18)
Not caused by your patch but we can fix the typo
GPMC_CONFIG1_WAIT_MON_IIME -> GPMC_CONFIG1_WAIT_MON_TIME
> +/** WAITMONITORINGTIME Max Ticks */
> +#define GPMC_CONFIG1_WAIT_MON_TIME_MAX 2
> #define GPMC_CONFIG1_WAIT_PIN_SEL(val) ((val & 3) << 16)
> #define GPMC_CONFIG1_DEVICESIZE(val)((val & 3) << 12)
> #define GPMC_CONFIG1_DEVICESIZE_16 GPMC_CONFIG1_DEVICESIZE(1)
> @@ -511,13 +513,41 @@ static int set_gpmc_timing_reg(int cs, int reg, int
> st_bit, int end_bit,
> t->field, #field) < 0) \
> return -1
>
> +/**
> + * gpmc_calc_waitmonitoring_divider - calculate proper GPMCFCLKDIVIDER based
> on WAITMONITORINGTIME
> + * @wait_monitoring WAITMONITORINGTIME in ns.
> + * @return -1 on failure to scale, else proper divider > 0.
> + * @noteWAITMONITORINGTIME will be _at least_ as long as desired.
> + * i.e. read timings should be kept-> don't
> sample bus too early
> + * while write timings should work out as well -> data is
> longer on bus
> + */
> +static int gpmc_calc_waitmonitoring_divider(unsigned int wait_monitoring)
> +{
> +
> + int div = gpmc_ns_to_ticks(wait_monitoring);
> +
> + div += GPMC_CONFIG1_WAIT_MON_TIME_MAX - 1;
> + div /= GPMC_CONFIG1_WAIT_MON_TIME_MAX;
Sorry I didn't understand how this works. :P
>From the TRM,
Re: [PATCH 7/8 v2] ARM OMAP2+ GPMC: calculate GPMCFCLKDIVIDER based on WAITMONITORINGTIME
On 25 Feb 2015 18:20, Roger Quadros wrote: Need to patch mach-omap2/usb-tusb6010.c as well. else we get arch/arm/mach-omap2/usb-tusb6010.c: In function ‘tusb_set_async_mode’: arch/arm/mach-omap2/usb-tusb6010.c:74:2: error: too few arguments to function ‘gpmc_cs_set_timings’ In file included from arch/arm/mach-omap2/gpmc.h:14:0, from arch/arm/mach-omap2/usb-tusb6010.c:23: include/linux/omap-gpmc.h:166:12: note: declared here arch/arm/mach-omap2/usb-tusb6010.c: In function ‘tusb_set_sync_mode’: arch/arm/mach-omap2/usb-tusb6010.c:101:2: error: too few arguments to function ‘gpmc_cs_set_timings’ In file included from arch/arm/mach-omap2/gpmc.h:14:0, from arch/arm/mach-omap2/usb-tusb6010.c:23: include/linux/omap-gpmc.h:166:12: note: declared here Argh... Another one of those exported function calls :| Robert -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8 v2] ARM OMAP2+ GPMC: calculate GPMCFCLKDIVIDER based on WAITMONITORINGTIME
On 24/02/15 22:05, Robert ABEL wrote:
The WAITMONITORINGTIME is expressed as a number of GPMC_CLK clock cycles,
even though the access is defined as asynchronous, and no GPMC_CLK clock
is provided to the external device. Still, GPMCFCLKDIVIDER is used as a
divider
for the GPMC clock, so it must be programmed to define the
correct WAITMONITORINGTIME delay.
Calculate GPMCFCLKDIVIDER independent of gpmc,sync-clk-ps in DT for
truly asynchronous accesses, i.e. both read and write asynchronous.
Signed-off-by: Robert ABEL ra...@cit-ec.uni-bielefeld.de
---
arch/arm/mach-omap2/gpmc-nand.c| 17 -
arch/arm/mach-omap2/gpmc-onenand.c | 4 +--
drivers/memory/omap-gpmc.c | 74
++
include/linux/omap-gpmc.h | 2 +-
4 files changed, 80 insertions(+), 17 deletions(-)
./scripts/checkpatch.pl detects some styling errors.
diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index d5951b1..e863a59 100644
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -96,14 +96,6 @@ int gpmc_nand_init(struct omap_nand_platform_data
*gpmc_nand_data,
gpmc_nand_res[1].start = gpmc_get_client_irq(GPMC_IRQ_FIFOEVENTENABLE);
gpmc_nand_res[2].start = gpmc_get_client_irq(GPMC_IRQ_COUNT_EVENT);
- if (gpmc_t) {
- err = gpmc_cs_set_timings(gpmc_nand_data-cs, gpmc_t);
- if (err 0) {
- pr_err(omap2-gpmc: Unable to set gpmc timings: %d\n,
err);
- return err;
- }
- }
-
memset(s, 0, sizeof(struct gpmc_settings));
if (gpmc_nand_data-of_node)
gpmc_read_settings_dt(gpmc_nand_data-of_node, s);
@@ -111,6 +103,15 @@ int gpmc_nand_init(struct omap_nand_platform_data
*gpmc_nand_data,
gpmc_set_legacy(gpmc_nand_data, s);
s.device_nand = true;
+
+ if (gpmc_t) {
+ err = gpmc_cs_set_timings(gpmc_nand_data-cs, gpmc_t, s);
+ if (err 0) {
+ pr_err(omap2-gpmc: Unable to set gpmc timings: %d\n,
err);
+ return err;
+ }
+ }
+
err = gpmc_cs_program_settings(gpmc_nand_data-cs, s);
if (err 0)
goto out_free_cs;
diff --git a/arch/arm/mach-omap2/gpmc-onenand.c
b/arch/arm/mach-omap2/gpmc-onenand.c
index 53d197e..f899e77 100644
--- a/arch/arm/mach-omap2/gpmc-onenand.c
+++ b/arch/arm/mach-omap2/gpmc-onenand.c
@@ -293,7 +293,7 @@ static int omap2_onenand_setup_async(void __iomem
*onenand_base)
if (ret 0)
return ret;
- ret = gpmc_cs_set_timings(gpmc_onenand_data-cs, t);
+ ret = gpmc_cs_set_timings(gpmc_onenand_data-cs, t, onenand_async);
if (ret 0)
return ret;
@@ -331,7 +331,7 @@ static int omap2_onenand_setup_sync(void __iomem
*onenand_base, int *freq_ptr)
if (ret 0)
return ret;
- ret = gpmc_cs_set_timings(gpmc_onenand_data-cs, t);
+ ret = gpmc_cs_set_timings(gpmc_onenand_data-cs, t, onenand_sync);
if (ret 0)
return ret;
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index a6abaf0..9cf8d21 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -139,6 +139,8 @@
#define GPMC_CONFIG1_WAIT_READ_MON (1 22)
#define GPMC_CONFIG1_WAIT_WRITE_MON (1 21)
#define GPMC_CONFIG1_WAIT_MON_IIME(val) ((val 3) 18)
Not caused by your patch but we can fix the typo
GPMC_CONFIG1_WAIT_MON_IIME - GPMC_CONFIG1_WAIT_MON_TIME
+/** WAITMONITORINGTIME Max Ticks */
+#define GPMC_CONFIG1_WAIT_MON_TIME_MAX 2
#define GPMC_CONFIG1_WAIT_PIN_SEL(val) ((val 3) 16)
#define GPMC_CONFIG1_DEVICESIZE(val)((val 3) 12)
#define GPMC_CONFIG1_DEVICESIZE_16 GPMC_CONFIG1_DEVICESIZE(1)
@@ -511,13 +513,41 @@ static int set_gpmc_timing_reg(int cs, int reg, int
st_bit, int end_bit,
t-field, #field) 0) \
return -1
+/**
+ * gpmc_calc_waitmonitoring_divider - calculate proper GPMCFCLKDIVIDER based
on WAITMONITORINGTIME
+ * @wait_monitoring WAITMONITORINGTIME in ns.
+ * @return -1 on failure to scale, else proper divider 0.
+ * @noteWAITMONITORINGTIME will be _at least_ as long as desired.
+ * i.e. read timings should be kept- don't
sample bus too early
+ * while write timings should work out as well - data is
longer on bus
+ */
+static int gpmc_calc_waitmonitoring_divider(unsigned int wait_monitoring)
+{
+
+ int div = gpmc_ns_to_ticks(wait_monitoring);
+
+ div += GPMC_CONFIG1_WAIT_MON_TIME_MAX - 1;
+ div /= GPMC_CONFIG1_WAIT_MON_TIME_MAX;
Sorry I didn't understand how this works. :P
From the TRM,
waitmonitoringtime_ns = waitmonitoring_ticks x (gpmc_clk_div + 1) x gpmc_fclk_ns
+
+ if (div 4)
+
Re: [PATCH 7/8 v2] ARM OMAP2+ GPMC: calculate GPMCFCLKDIVIDER based on WAITMONITORINGTIME
Robert, On 25/02/15 18:33, Roger Quadros wrote: On 24/02/15 22:05, Robert ABEL wrote: The WAITMONITORINGTIME is expressed as a number of GPMC_CLK clock cycles, even though the access is defined as asynchronous, and no GPMC_CLK clock is provided to the external device. Still, GPMCFCLKDIVIDER is used as a divider for the GPMC clock, so it must be programmed to define the correct WAITMONITORINGTIME delay. Calculate GPMCFCLKDIVIDER independent of gpmc,sync-clk-ps in DT for truly asynchronous accesses, i.e. both read and write asynchronous. Signed-off-by: Robert ABEL ra...@cit-ec.uni-bielefeld.de --- arch/arm/mach-omap2/gpmc-nand.c| 17 - arch/arm/mach-omap2/gpmc-onenand.c | 4 +-- drivers/memory/omap-gpmc.c | 74 ++ include/linux/omap-gpmc.h | 2 +- 4 files changed, 80 insertions(+), 17 deletions(-) Need to patch mach-omap2/usb-tusb6010.c as well. else we get arch/arm/mach-omap2/usb-tusb6010.c: In function ‘tusb_set_async_mode’: arch/arm/mach-omap2/usb-tusb6010.c:74:2: error: too few arguments to function ‘gpmc_cs_set_timings’ In file included from arch/arm/mach-omap2/gpmc.h:14:0, from arch/arm/mach-omap2/usb-tusb6010.c:23: include/linux/omap-gpmc.h:166:12: note: declared here arch/arm/mach-omap2/usb-tusb6010.c: In function ‘tusb_set_sync_mode’: arch/arm/mach-omap2/usb-tusb6010.c:101:2: error: too few arguments to function ‘gpmc_cs_set_timings’ In file included from arch/arm/mach-omap2/gpmc.h:14:0, from arch/arm/mach-omap2/usb-tusb6010.c:23: include/linux/omap-gpmc.h:166:12: note: declared here cheers, -roger -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8 v2] ARM OMAP2+ GPMC: calculate GPMCFCLKDIVIDER based on WAITMONITORINGTIME
Hi Roger,
On 25 Feb 2015 17:33, Roger Quadros wrote:
./scripts/checkpatch.pl detects some styling errors.
Well, there's like a million lines over 80 characters. I'm also a
heathen and don't use an 80 character terminal either.
I'll fix the more serious issues checkpatch finds, but some styling
errors are even from code already in the driver.
#define GPMC_CONFIG1_WAIT_MON_IIME(val) ((val 3) 18)
Not caused by your patch but we can fix the typo
GPMC_CONFIG1_WAIT_MON_IIME - GPMC_CONFIG1_WAIT_MON_TIME
I'd rather remove than fix. None of these defines is in active use anymore.
+/**
+ * gpmc_calc_waitmonitoring_divider - calculate proper GPMCFCLKDIVIDER based
on WAITMONITORINGTIME
+ * @wait_monitoring WAITMONITORINGTIME in ns.
+ * @return -1 on failure to scale, else proper divider 0.
+ * @noteWAITMONITORINGTIME will be _at least_ as long as desired.
+ * i.e. read timings should be kept- don't
sample bus too early
+ * while write timings should work out as well - data is
longer on bus
+ */
+static int gpmc_calc_waitmonitoring_divider(unsigned int wait_monitoring)
+{
+
+ int div = gpmc_ns_to_ticks(wait_monitoring);
+
+ div += GPMC_CONFIG1_WAIT_MON_TIME_MAX - 1;
+ div /= GPMC_CONFIG1_WAIT_MON_TIME_MAX;
Sorry I didn't understand how this works. :P
From the TRM,
waitmonitoringtime_ns = waitmonitoring_ticks x (gpmc_clk_div + 1) x gpmc_fclk_ns
Using that formula:
gpmc_clk_div + 1 = ceil(ceil(waitmonitoringtime_ns / gpmc_fclk_ns) /
waitmonitoring_ticks).
The reason I use GPMC_CONFIG1_WAIT_MON_TIME_MAX directly, is because
every other pair is identical:
We have the following cases:
a) WAITMONITORINGTIME = 0, so div doesn't matter (we set div to 1) or
b) WAITMONITORINGTIME = 1, with div = 1 or
c) WAITMONITORINGTIME = 2, with div = n
WAITMONITORINGTIME is never 1 with div = n and div /= 1, because that's
the same as WAITMONITORINGTIME = 2 with div = n - 1.
Cases a) and b) are caught using the fact that div = 0 after the division.
Case c) is handled by assuming that WAITMONITORINGTIME will always be 2
(i.e. GPMC_CONFIG1_WAIT_MON_TIME_MAX) for all divs /= 0 at that point.
Took me a while to realize that this works, too.
+ /*
+* See if we need to change the divider for waitmonitoringtime.
+*
+* If DT contains sync-clk-ps for truly asynchronous accesses,
+* ignore it as long as waitmonitoringtime is used.
+*
The comment in the $subject was more easier to understand for me than the above
Calculate GPMCFCLKDIVIDER independent of gpmc,sync-clk-ps in DT for
truly asynchronous accesses, i.e. both read and write asynchronous.
Well, the comment in $subject was a general description. Here, we don't
touch div when
a) any access is synchronous
b) all accesses are asynchronous, but WAITMONITORINGTIME is not used,
i.e. WAITREADMONITORING and WAITWRITEMONITORING are both not set.
So, if WAITMONITORINGTIME is not used, or any access is synchronous, we
program the sync-clk-ps anyways, because the GPMC won't rely on it for
any timing.
So the comment is OK, IMHO.
Regards,
Robert
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
