public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: jon-hunter@ti•com (Jon Hunter)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v6 07/10] ARM: OMAP2+: gpmc: generic timing calculation
Date: Mon, 27 Aug 2012 15:30:13 -0500	[thread overview]
Message-ID: <503BD8D5.5060400@ti.com> (raw)
In-Reply-To: <C8443D0743D26F4388EA172BF4E2A7A93E9BA63D@DBDE01.ent.ti.com>

Hi Afzal,

On 08/27/2012 05:37 AM, Mohammed, Afzal wrote:
> On Thu, Aug 23, 2012 at 08:28:44, Hunter, Jon wrote:

[snip]

>> I understand that this is based upon how timings for onenand and tusb
>> are being calculated today, but I am not sure that this is the way to go
>> for all devices. Personally, I would like to see us get away from how
>> those devices are calculating timings for any new device.
> 
> You cannot do away with many of the those, as logically they
> are right. Eg. read data should be available at access time,
> assuming a zero data hold time, we can very well derive an
> expression as,

I am not saying that we do away with them for current devices, just
maintain them as is.

> read de-assertion time (oe_off) = access time plus 1 gpmc clock,
> 
> and this is what the existing calculations do, and also the
> generic routine. There could be other constraints, but this
> certainly should be one (I do realize that oe_off could be
> optimized to be less than access time, by relying on read
> hold time, then again effect of it would be in similar way
> for different peripherals, but let's forget about
> optimization in the beginning)
> 
>> In general, I am concerned that we are abstracting the timings further
>> from the actual register fields. For example, the timings parameter
>> "t_ceasu" is described "address setup to CS valid" which is not
>> incorrect but this value is really just programming the CSONTIME field
>> and so why not call this cs_on?
> 
> Timing fields of struct gpmc_device_timings are selected such
> that they should be bindable by DT. At least one of the peripheral
> datasheet has these fields.

Right, but these are not applicable to every device and so I worry this
could be confusing. However, more documentation may help clear this up.

> If user knows timings in terms of
> gpmc values, he can directly use struct gpmc_timings, but
> then all the values should be updated in struct gpmc_timings.
> User should not update some of the values in terms of
> peripheral timings, others in terms of gpmc timings, that
> would make things complex and does not seem the right way
> to me.
> 
> cs_on and other gpmc aware timings could be binded by DT, not as
> peripheral timing, but as gpmc timing.
> 
> Bindings for peripheral DT timings should be something that
> can be obtained from peripheral datasheet, here accidentally
> it is same as gpmc timings, but not most timings are this way.
> Also there could be other constraints that can come for cs_on,
> even though I have not come across any till now.
> 
>>
>> So although this may consolidate how the timings are calculated today, I
>> am concerned it will be confusing to add timings for a new device. At
>> least if I am calculating timings, I am taking the timing information
>> for the device and translating that to the how I need to program the
>> gpmc register fields.
> 
> If I am not wrong, GPMC IP has been present for around a
> decade, and so far I have not come across any generic time
> calculation method that can be applied to all peripherals.

Yes not an easy problem to solve :-(

> Getting to work same peripheral for a different gpmc
> frequency is another problem.
> 
> Here we are trying to generalize based on the understanding of
> gpmc & peripheral timings, as well as by a kind of reverse
> engineering without most of the hardware or datasheet. Think of
> this as an attempt to create one, let it evolve and become a
> robust one. If a new user want to add a peripheral, let him add
> support, input timings are selected such that it is found in
> peripheral datasheet, if that does not work, let him try
> to add any new timing field or alter existing expression
> as he deems to be proper and verify that it does not break
> others. And I can help provided I am not heavily loaded
> with other works.

So long as it is maintainable ;-)

> And at least for initial users, they are expected to have
> some grasp on how to calculate timings, such a user will
> not be much worried about your 3 concerns above, anyway as
> of now they need to have a good grasp on it.

I would consider myself to be an initial user and I am concerned,
doesn't that count?

An example, would be the following where you have 4 timing parameters
for access time. You need to dig through the code to understand how
these are being used.

+	u32     t_aa;		/* access time from ADV assertion */
+	u32     t_iaa;		/* initial access time */
+	u32     t_oe;		/* access time from OE assertion */
+	u32     t_ce;		/* access time from CS asertion */

> Meanwhile I will try to document more.

Yes more documentation is definitely needed.

Cheers
Jon

  reply	other threads:[~2012-08-27 20:30 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-21 10:41 [PATCH v6 00/10] OMAP-GPMC: generic time calc, prepare for driver Afzal Mohammed
2012-08-21 10:45 ` [PATCH v6 01/10] ARM: OMAP2+: nand: unify init functions Afzal Mohammed
2012-08-21 11:37   ` Igor Grinberg
2012-08-21 10:45 ` [PATCH v6 02/10] ARM: OMAP2+: gpmc: handle additional timings Afzal Mohammed
2012-08-21 10:45 ` [PATCH v6 03/10] ARM: OMAP2+: onenand: refactor for clarity Afzal Mohammed
2012-08-21 10:45 ` [PATCH v6 04/10] ARM: OMAP2+: GPMC: Remove unused OneNAND get_freq() platform function Afzal Mohammed
2012-08-21 10:45 ` [PATCH v6 05/10] ARM: OMAP2+: gpmc: find features by ip rev check Afzal Mohammed
2012-08-22  2:08   ` Jon Hunter
2012-08-21 10:45 ` [PATCH v6 06/10] ARM: OMAP2+: gpmc: remove cs# in sync clk div calc Afzal Mohammed
2012-08-22  2:11   ` Jon Hunter
2012-08-21 10:45 ` [PATCH v6 07/10] ARM: OMAP2+: gpmc: generic timing calculation Afzal Mohammed
2012-08-23  2:58   ` Jon Hunter
2012-08-24 19:54     ` Tony Lindgren
2012-08-27 11:46       ` Mohammed, Afzal
2012-08-27 10:37     ` Mohammed, Afzal
2012-08-27 20:30       ` Jon Hunter [this message]
2012-08-28 12:21         ` Mohammed, Afzal
2012-08-21 10:45 ` [PATCH v6 08/10] ARM: OMAP2+: onenand: " Afzal Mohammed
2012-08-21 10:46 ` [PATCH v6 09/10] ARM: OMAP2+: smc91x: " Afzal Mohammed
2012-08-21 10:46 ` [PATCH v6 10/10] ARM: OMAP2+: tusb6010: " Afzal Mohammed
2012-08-24 19:46   ` Tony Lindgren
2012-08-27  8:34     ` Mohammed, Afzal
2012-09-03  5:34       ` Mohammed, Afzal
2012-09-06  7:39         ` Mohammed, Afzal
2012-09-06 20:43           ` Tony Lindgren
2012-09-11 18:46             ` Tony Lindgren
2012-09-12  9:50               ` Mohammed, Afzal
2012-09-14 10:20                 ` Mohammed, Afzal
2012-09-17  8:39                   ` Mohammed, Afzal
2012-09-17 22:50                     ` Tony Lindgren
2012-09-17 23:10                       ` Tony Lindgren
2012-09-19 13:43                         ` Mohammed, Afzal
2012-09-07  0:15           ` Paul Walmsley
2012-08-27 12:16 ` [PATCH v6 00/10] OMAP-GPMC: generic time calc, prepare for driver Daniel Mack
2012-08-27 12:38   ` Mohammed, Afzal
2012-08-27 13:30     ` Daniel Mack
2012-08-27 14:01       ` Mohammed, Afzal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=503BD8D5.5060400@ti.com \
    --to=jon-hunter@ti$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox