diff mbox

[1/3] omap gpmc : add support of setting CYCLE2CYCLEDELAY and BUSTURNAROUND

Message ID 1352220277-4251-1-git-send-email-matthieu.castet@parrot.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matthieu CASTET Nov. 6, 2012, 4:44 p.m. UTC
Signed-off-by: Matthieu CASTET <matthieu.castet@parrot.com>
---
 arch/arm/mach-omap2/gpmc.c             |    7 ++++++-
 arch/arm/plat-omap/include/plat/gpmc.h |    2 ++
 2 files changed, 8 insertions(+), 1 deletion(-)

Comments

Hunter, Jon Nov. 6, 2012, 5:23 p.m. UTC | #1
On 11/06/2012 10:44 AM, Matthieu CASTET wrote:
> Signed-off-by: Matthieu CASTET <matthieu.castet@parrot.com>

I think you need to have something in the changelog for the patch
describing why this change is needed and what device this has been
tested on.

> ---
>  arch/arm/mach-omap2/gpmc.c             |    7 ++++++-
>  arch/arm/plat-omap/include/plat/gpmc.h |    2 ++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index 8ab1e1b..3957ffc 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -333,8 +333,13 @@ int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t)
>  
>  	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)
> +	if (gpmc_capability & GPMC_HAS_WR_ACCESS) {
> +		/* XXX check on which hardware it is supported */

I understand that you may not have all the documentation but lets fix
this now.

> +		GPMC_SET_ONE(GPMC_CS_CONFIG6,  0,  3, busturnaround);
> +		GPMC_SET_ONE(GPMC_CS_CONFIG6,  8, 11, cycle2cycledelay);

Looking at the documentation for OMAP2420, OMAP3430, OMAP4430 and
OMAP5430, the above fields are present and same size location for all
OMAP devices. So this does not need to be done under the HAS_WR_ACCESS
field test. In fact, I believe that Afzal was going to add these fields
in a patch and was doing it for all devices [1].

>  		GPMC_SET_ONE(GPMC_CS_CONFIG6, 24, 28, wr_access);
> +	}
>  
>  	/* caller is expected to have initialized CONFIG1 to cover
>  	 * at least sync vs async
> diff --git a/arch/arm/plat-omap/include/plat/gpmc.h b/arch/arm/plat-omap/include/plat/gpmc.h
> index 2e6e259..34ca454 100644
> --- a/arch/arm/plat-omap/include/plat/gpmc.h
> +++ b/arch/arm/plat-omap/include/plat/gpmc.h
> @@ -131,6 +131,8 @@ struct gpmc_timings {
>  	/* The following are only on OMAP3430 */
>  	u16 wr_access;		/* WRACCESSTIME */
>  	u16 wr_data_mux_bus;	/* WRDATAONADMUXBUS */
> +	u16 cycle2cycledelay;	/* CYCLE2CYCLEDELAY */
> +	u16 busturnaround;		/* BUSTURNAROUND */

So you should be able to move these out of OMAP3430 specific as they are
generic.

Cheers
Jon

[1] http://marc.info/?l=linux-omap&m=134037095705900&w=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
Matthieu CASTET Nov. 6, 2012, 6 p.m. UTC | #2
Jon Hunter a écrit :
> On 11/06/2012 10:44 AM, Matthieu CASTET wrote:
>>  
>>  	/* caller is expected to have initialized CONFIG1 to cover
>>  	 * at least sync vs async
>> diff --git a/arch/arm/plat-omap/include/plat/gpmc.h b/arch/arm/plat-omap/include/plat/gpmc.h
>> index 2e6e259..34ca454 100644
>> --- a/arch/arm/plat-omap/include/plat/gpmc.h
>> +++ b/arch/arm/plat-omap/include/plat/gpmc.h
>> @@ -131,6 +131,8 @@ struct gpmc_timings {
>>  	/* The following are only on OMAP3430 */
>>  	u16 wr_access;		/* WRACCESSTIME */
>>  	u16 wr_data_mux_bus;	/* WRDATAONADMUXBUS */
>> +	u16 cycle2cycledelay;	/* CYCLE2CYCLEDELAY */
>> +	u16 busturnaround;		/* BUSTURNAROUND */
> 
> So you should be able to move these out of OMAP3430 specific as they are
> generic.
Thanks for the quick review.

I will post another patch, unless this is already done in  Afzal patch (Is there
a tree where I can get Afzal pending patches ?)


Matthieu
--
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
Hunter, Jon Nov. 6, 2012, 8:34 p.m. UTC | #3
On 11/06/2012 12:00 PM, Matthieu CASTET wrote:
> Jon Hunter a écrit :
>> On 11/06/2012 10:44 AM, Matthieu CASTET wrote:
>>>  
>>>  	/* caller is expected to have initialized CONFIG1 to cover
>>>  	 * at least sync vs async
>>> diff --git a/arch/arm/plat-omap/include/plat/gpmc.h b/arch/arm/plat-omap/include/plat/gpmc.h
>>> index 2e6e259..34ca454 100644
>>> --- a/arch/arm/plat-omap/include/plat/gpmc.h
>>> +++ b/arch/arm/plat-omap/include/plat/gpmc.h
>>> @@ -131,6 +131,8 @@ struct gpmc_timings {
>>>  	/* The following are only on OMAP3430 */
>>>  	u16 wr_access;		/* WRACCESSTIME */
>>>  	u16 wr_data_mux_bus;	/* WRDATAONADMUXBUS */
>>> +	u16 cycle2cycledelay;	/* CYCLE2CYCLEDELAY */
>>> +	u16 busturnaround;		/* BUSTURNAROUND */
>>
>> So you should be able to move these out of OMAP3430 specific as they are
>> generic.
> Thanks for the quick review.
> 
> I will post another patch, unless this is already done in  Afzal patch (Is there
> a tree where I can get Afzal pending patches ?)

Afzal keeps his kernel tree on gitorious [1]. However, I am not sure
what Afzal's plans are for the remaining patches not yet merged and
which branch you should look at (I have a lot of problems viewing
anything on gitorious these days).

Afzal, let us know how you prefer to handle this.

Cheers
Jon

[1] http://gitorious.org/x0148406-public/linux-kernel
--
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
Afzal Mohammed Nov. 7, 2012, 8:58 a.m. UTC | #4
KyBUb255LCBEYW5pZWwNCg0KSGksDQoNCk9uIFdlZCwgTm92IDA3LCAyMDEyIGF0IDAyOjA0OjAz
LCBIdW50ZXIsIEpvbiB3cm90ZToNCj4gT24gMTEvMDYvMjAxMiAxMjowMCBQTSwgTWF0dGhpZXUg
Q0FTVEVUIHdyb3RlOg0KDQo+ID4gSSB3aWxsIHBvc3QgYW5vdGhlciBwYXRjaCwgdW5sZXNzIHRo
aXMgaXMgYWxyZWFkeSBkb25lIGluICBBZnphbCBwYXRjaCAoSXMgdGhlcmUNCj4gPiBhIHRyZWUg
d2hlcmUgSSBjYW4gZ2V0IEFmemFsIHBlbmRpbmcgcGF0Y2hlcyA/KQ0KDQo+IEFmemFsIGtlZXBz
IGhpcyBrZXJuZWwgdHJlZSBvbiBnaXRvcmlvdXMgWzFdLiBIb3dldmVyLCBJIGFtIG5vdCBzdXJl
DQo+IHdoYXQgQWZ6YWwncyBwbGFucyBhcmUgZm9yIHRoZSByZW1haW5pbmcgcGF0Y2hlcyBub3Qg
eWV0IG1lcmdlZCBhbmQNCj4gd2hpY2ggYnJhbmNoIHlvdSBzaG91bGQgbG9vayBhdCAoSSBoYXZl
IGEgbG90IG9mIHByb2JsZW1zIHZpZXdpbmcNCj4gYW55dGhpbmcgb24gZ2l0b3Jpb3VzIHRoZXNl
IGRheXMpLg0KPiANCj4gQWZ6YWwsIGxldCB1cyBrbm93IGhvdyB5b3UgcHJlZmVyIHRvIGhhbmRs
ZSB0aGlzLg0KDQpNeSBpbml0aWFsIHBsYW4gd2FzIHRvIGRvIHRpbWluZyByZWxhdGVkIGNoYW5n
ZXMgZmlyc3QgYW5kIHRoZW4gZHQNCmNvbnZlcnNpb24uIEJ1dCBhdCB0aGUgcHJlc2VudCBtb21l
bnQsIGl0IHNlZW1zIGhpZ2hlciBwcmlvcml0eSBoYXMNCnRvIGJlIGdpdmVuIGZvciBkdCBjb252
ZXJzaW9uIG92ZXIgdGltaW5nIGNoYW5nZXMgKGl0IGludm9sdmVzDQp1c2luZyBnZW5lcmljIHRp
bWluZyByb3V0aW5lIGZvciBhbGwgcmVxdWlyZWQgZ3BtYyBwZXJpcGhlcmFscywNCmhhbmRsaW5n
IGFkZGl0aW9uYWwgdGltaW5ncyBpbmNsdXNpdmUgb2YgJHN1YmplY3QpIGFuZCBoZW5jZSB0aW1p
bmcNCnJlbGF0ZWQgY2hhbmdlcyB3ZXJlIHB1dCBpbiBzdXNwZW5kZWQgYW5pbWF0aW9uIGZvciBu
b3cuDQoNCkFuZCBEYW5pZWwgaGFzIHN0YXJ0ZWQgd29ya2luZyBvbiBncG1jIGR0LiBMZXQgdXMg
dGFrZSBUb255J3MNCm9waW5pb24gb24gaG93IHRvIGRlYWwgd2l0aCB0aGlzLCBUb255ID8NCg0K
Rm9sbG93aW5nIGFyZSB0aGUgcGVuZGluZyBjaGFuZ2VzIHcuci50IHRpbWluZyBhdmFpbGFibGUg
QFsxXQ0KKHBsZWFzZSBub3RlIHRoYXQgdGhlc2Ugd291bGQgaGF2ZSB0byBiZSByZWJhc2VkIG92
ZXIgYnJhbmNoL3RhZw0Kc3BlY2lmaWVkIGJ5IFRvbnkgaW4gcmVwbHkgdG8gTWF0dGhpZXUncyBw
YXRjaCAzLzMpDQphLiBkNDJlYWZhIEFSTTogT01BUDIrOiBuYW5kOiByZW1vdmUgcmVkdW5kYW50
IHJvdW5kaW5nDQpiLiBmMjI5YWJhIEFSTTogT01BUDIrOiBncG1jOiBoYW5kbGUgYWRkaXRpb25h
bCB0aW1pbmdzDQpjLiAxNGNiYjg3IEFSTTogT01BUDIrOiBncG1jOiBnZW5lcmljIHRpbWluZyBj
YWxjdWxhdGlvbg0KZC4gOTgzMDI2NCBBUk06IE9NQVAyKzogb25lbmFuZDogZ2VuZXJpYyB0aW1p
bmcgY2FsY3VsYXRpb24NCmUuIDVmMzNlYTUgQVJNOiBPTUFQMis6IHNtYzkxeDogZ2VuZXJpYyB0
aW1pbmcgY2FsY3VsYXRpb24NCmYuIDk4NzYwMjAgQVJNOiBPTUFQMis6IHR1c2I2MDEwOiBnZW5l
cmljIHRpbWluZyBjYWxjdWxhdGlvbiAoSEVBRCkNCg0KJ2InIGlzIGEgc3VwZXJzZXQgb2YgJHN1
YmplY3QNCg0KT3JpZ2luYWxseSAnYScgYW5kICdiJyB3YXMgcGFydCBvZiBncG1jIGNsZWFudXAg
c2VyaWVzIGZvciBjb21tb24NCnpJbWFnZSwgdGhlbiBUb255IHJlcXVlc3RlZCBmb3IgbWluaW1h
bCBjaGFuZ2VzIGZvciBpdCwgaGVuY2UNCidhJyAmICdiJyB3YXMgbGVmdCBvdXQgaW4gdGhlIHB1
bGwgcmVxdWVzdCAocGlja2VkIHVwIGJ5IFRvbnkpLA0Kc28gdGhhdCBncG1jIGNvbW1vbiB6SW1h
Z2UgY2xlYW51cCBzZXJpZXMgd291bGQgbm90IGNyZWF0ZSBhbnkNCnRpbWluZyByZWxhdGVkIGlz
c3Vlcy4NCg0KT25lIHRoaW5nIHRvIG5vdGUgaXMgdGhhdCBjeWNsZTJjeWNsZWRlbGF5IGFuZCBi
dXN0dXJuYXJvdW5kIGZpZWxkJ3MNCndvdWxkIGdldCB6ZXJvZWQgb3V0IHdpdGggJHN1YmplY3Qg
cGF0Y2ggZm9yIHRob3NlIHBlcmlwaGVyYWwncyB0aGF0DQpjYWxsIGdwbWNfY3Nfc2V0X3RpbWlu
Z3MoKS4gSWYgdGhlcmUgYXJlIGFueSBib2FyZHMgc2lsZW50bHkNCmRlcGVuZGluZyBvbiBib290
bG9hZGVyIHNldHRpbmcgdGhlc2UgdmFsdWVzLCBpdCBtYXkgYmUgYWZmZWN0ZWQsIHNvDQp0aGlz
IGNoYW5nZSB3b3VsZCBuZWVkIHRvIGJlIHZlcmlmaWVkIGZvciBleGlzdGluZyBib2FyZHMgaW4g
bWFpbmxpbmUuDQoNClBlcmhhcHMgJ2InIChmb3IgJHN1YmplY3QgcGF0Y2gpIGNhbiBiZSB0YWtl
biBhaGVhZCBpZiBUb255IGlzDQpoYXBweSB3aXRoIGl0Lg0KDQpBbmQgcmVnYXJkaW5nIHBhdGNo
ZXMgMiAmIDMgb2YgTWF0dGhpZXUncyBzZXJpZXMgdGhhdCBjYWxjdWxhdGUNCnRpbWluZ3MsIEkg
d2FzIHdvbmRlcmluZyB3aGV0aGVyIGdlbmVyaWMgdGltaW5nIHJvdXRpbmUgKGMpIGNhbg0KbGVh
cm4gZnJvbSBpdCBhcyB3ZWxsIGFzIHZpY2UtdmVyc2EuIEFsc28gd2l0aCBncG1jIGNvbW1vbiB6
SW1hZ2UNCnNlcmllcywgb21hcC1uYW5kIGRyaXZlciBkb2VzIG5vdCBoYXZlIGFjY2VzcyB0byB0
aW1pbmcgcmVsYXRlZA0KZ3BtYyBmdW5jdGlvbnMsIGEgbmV3IGdwbWMgZnVuY3Rpb24gd291bGQg
aGF2ZSB0byBiZSBleHBvcnRlZA0KdGhhdCB0cmFuc2xhdGVzIG9uZmkgdGltaW5ncyB0byBncG1j
IChvciBlZHVjYXRlIGdlbmVyaWMgdGltaW5nDQpyb3V0aW5lIHdpdGggb25maSB0cmFuc2xhdGlv
biB0b28gPykNCg0KUmVnYXJkcw0KQWZ6YWwNCg0KW2FdIGdpdDovL2dpdG9yaW91cy5vcmcveDAx
NDg0MDYtcHVibGljL2xpbnV4LWtlcm5lbC5naXQgZ3BtYy10aW1pbmcNCg0K
--
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
Tony Lindgren Nov. 7, 2012, 9:40 p.m. UTC | #5
* Mohammed, Afzal <afzal@ti.com> [121107 01:00]:
> + Tony, Daniel
> 
> Hi,
> 
> On Wed, Nov 07, 2012 at 02:04:03, Hunter, Jon wrote:
> > On 11/06/2012 12:00 PM, Matthieu CASTET wrote:
> 
> > > I will post another patch, unless this is already done in  Afzal patch (Is there
> > > a tree where I can get Afzal pending patches ?)
> 
> > Afzal keeps his kernel tree on gitorious [1]. However, I am not sure
> > what Afzal's plans are for the remaining patches not yet merged and
> > which branch you should look at (I have a lot of problems viewing
> > anything on gitorious these days).
> > 
> > Afzal, let us know how you prefer to handle this.
> 
> My initial plan was to do timing related changes first and then dt
> conversion. But at the present moment, it seems higher priority has
> to be given for dt conversion over timing changes (it involves
> using generic timing routine for all required gpmc peripherals,
> handling additional timings inclusive of $subject) and hence timing
> related changes were put in suspended animation for now.
> 
> And Daniel has started working on gpmc dt. Let us take Tony's
> opinion on how to deal with this, Tony ?

Up to you to figure out the ordering.
 
> Following are the pending changes w.r.t timing available @[1]
> (please note that these would have to be rebased over branch/tag
> specified by Tony in reply to Matthieu's patch 3/3)
> a. d42eafa ARM: OMAP2+: nand: remove redundant rounding
> b. f229aba ARM: OMAP2+: gpmc: handle additional timings
> c. 14cbb87 ARM: OMAP2+: gpmc: generic timing calculation
> d. 9830264 ARM: OMAP2+: onenand: generic timing calculation
> e. 5f33ea5 ARM: OMAP2+: smc91x: generic timing calculation
> f. 9876020 ARM: OMAP2+: tusb6010: generic timing calculation (HEAD)
> 
> 'b' is a superset of $subject
> 
> Originally 'a' and 'b' was part of gpmc cleanup series for common
> zImage, then Tony requested for minimal changes for it, hence
> 'a' & 'b' was left out in the pull request (picked up by Tony),
> so that gpmc common zImage cleanup series would not create any
> timing related issues.

Maybe send pull requests for the ones that are ready to go?

They should be done against what we have in clean-up probably, so
omap-for-v3.8/cleanup-headers-prepare-multiplatform-v3 or against
omap-for-v3.8/cleanup-headers-gpmc it that merges easily with
the cleanup branch.
 
> One thing to note is that cycle2cycledelay and busturnaround field's
> would get zeroed out with $subject patch for those peripheral's that
> call gpmc_cs_set_timings(). If there are any boards silently
> depending on bootloader setting these values, it may be affected, so
> this change would need to be verified for existing boards in mainline.
> 
> Perhaps 'b' (for $subject patch) can be taken ahead if Tony is
> happy with it.
> 
> And regarding patches 2 & 3 of Matthieu's series that calculate
> timings, I was wondering whether generic timing routine (c) can
> learn from it as well as vice-versa. Also with gpmc common zImage
> series, omap-nand driver does not have access to timing related
> gpmc functions, a new gpmc function would have to be exported
> that translates onfi timings to gpmc (or educate generic timing
> routine with onfi translation too ?)

Right, once we enable CONFIG_ARCH_MULTIPLATFORM, drivers trying
to include <mach/*.h> or <plat/*.h> files will fail to build.

Regards,

Tony
 
> [a] git://gitorious.org/x0148406-public/linux-kernel.git gpmc-timing
--
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
Afzal Mohammed Nov. 8, 2012, 12:54 p.m. UTC | #6
Hi Tony,

On Thu, Nov 08, 2012 at 03:10:14, Tony Lindgren wrote:
> * Mohammed, Afzal <afzal@ti.com> [121107 01:00]:


> > And Daniel has started working on gpmc dt. Let us take Tony's

> > opinion on how to deal with this, Tony ?


> Up to you to figure out the ordering.


> Maybe send pull requests for the ones that are ready to go?


Pending timing related patches,

a. ARM: OMAP2+: nand: remove redundant rounding
b. ARM: OMAP2+: gpmc: handle additional timings
c. ARM: OMAP2+: gpmc: generic timing calculation
d. ARM: OMAP2+: onenand: generic timing calculation
e. ARM: OMAP2+: smc91x: generic timing calculation
f. ARM: OMAP2+: tusb6010: generic timing calculation

can be divided to two categories,

1. timing cleanup (a,b)
2. migrate to generic timing routine (c-f)

'1' has been tested on omap3evm (also with the help of a local patch
adding onenand support).

'2' has been verified on omap3evm onenand async operation (support for
omap3evm onenand is not present in mainline) and by simulation. These
changes could not be validated on boards supported in mainline that
make use of runtime timing calculation as I do not have those boards.

I will be sending pull request containing both sets 1 & 2, please let
me know in case you have a different opinion.

Regards
Afzal
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 8ab1e1b..3957ffc 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -333,8 +333,13 @@  int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t)
 
 	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)
+	if (gpmc_capability & GPMC_HAS_WR_ACCESS) {
+		/* XXX check on which hardware it is supported */
+		GPMC_SET_ONE(GPMC_CS_CONFIG6,  0,  3, busturnaround);
+		GPMC_SET_ONE(GPMC_CS_CONFIG6,  8, 11, cycle2cycledelay);
+
 		GPMC_SET_ONE(GPMC_CS_CONFIG6, 24, 28, wr_access);
+	}
 
 	/* caller is expected to have initialized CONFIG1 to cover
 	 * at least sync vs async
diff --git a/arch/arm/plat-omap/include/plat/gpmc.h b/arch/arm/plat-omap/include/plat/gpmc.h
index 2e6e259..34ca454 100644
--- a/arch/arm/plat-omap/include/plat/gpmc.h
+++ b/arch/arm/plat-omap/include/plat/gpmc.h
@@ -131,6 +131,8 @@  struct gpmc_timings {
 	/* The following are only on OMAP3430 */
 	u16 wr_access;		/* WRACCESSTIME */
 	u16 wr_data_mux_bus;	/* WRDATAONADMUXBUS */
+	u16 cycle2cycledelay;	/* CYCLE2CYCLEDELAY */
+	u16 busturnaround;		/* BUSTURNAROUND */
 };
 
 struct gpmc_nand_regs {