RE: gpmc generic retime function (subject was RE: [PATCH v5 3/3] ARM: OMAP2+: onenand: prepare for gpmc driver migration)

2012-08-21 Thread Mohammed, Afzal
Hi Jon,

On Fri, Aug 17, 2012 at 20:32:34, Hunter, Jon wrote:

  And we have been able to create such a function. Below is an implementation
  that has been made for handling asynchronous timings. It has been tested for
  OneNAND  SMSC on OMAP3EVM (rev G  C) with [1-4]. OneNAND was tested using
  [5] (OMAP3EVM OneNAND works in async mode)  SMSC using [6] (mainline does
  not have a timing calculation for smsc911x)
 
 Are you able to verify that the timing calculated by this function are
 identical? May be some more details on exactly how you tested this would
 be good.

Yes, it was verified.

A new driver preparation series has been posted,
http://marc.info/?l=linux-omapm=134554573104116w=2
that includes generic timing calculation method.

The new series mentions how timing values were validated. There are a
couple of timing parameters that would vary as mentioned in the above
mentioned mail, but these I don't expect to create problems as this is 
more inline with gpmc peripheral timings. And both of the these has
been verified that it would not create problem with peripheral
functionality. One was tested directly (we_on related for OneNAND) and
other indirectly (adv_rd(wr)_off on SMSC 9220 for SMSC 91C96).

Reason for doing so was that quirks required to handle these specific
cases could be avoided, otherwise new peripheral timing data would be
required and it would be difficult to achieve DT bindings (when DT
happens) for these kind of fixup timings.

But if this causes any problems (which I don't expect), then we will
have to fallback to the quirks that I wanted to avoid.

 Do you think that there is any value in making the tusb member a u32
 dev_type and then set it too GPMC_DEVICE_TUSB then this could be used
 for other devices in the future too if needed?

 Would it be possible to create a sub-function called
 gpmc_calc_timings_tusb() and put all these if (tusb) statements in
 there? Or maybe a generic function called gpmc_calc_timings_prepare().

Usage of a tusb check was something that I really wanted to avoid as
that was making generic timing calculation function peripheral Gnostic.
With the newly posted series, tusb field has been removed.

Regards
Afzal
--
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: gpmc generic retime function (subject was RE: [PATCH v5 3/3] ARM: OMAP2+: onenand: prepare for gpmc driver migration)

2012-08-17 Thread Jon Hunter
Hi Afzal,

Sorry for the delay, I have been out of the office.

On 08/06/2012 08:38 AM, Mohammed, Afzal wrote:
 Hi Tony, Jon,
 
 On Wed, Jul 11, 2012 at 12:17:25, Tony Lindgren wrote:
 * Jon Hunter jon-hun...@ti.com [120710 10:20]:
 
 The DT node should simply have the information required by the retime
 function or gpmc timings themselves if available. In the case of OneNAND
 
 These can be stored in the DT and then translated to gpmc timings at
 runtime. DT should only store static timing or clock information known
 
 Yup. And the format of the timing data in DT should be standardized so
 the only differences for each connected peripheral is the retime function.
 
 If we are able to achieve a generic retime function applicable to all
 peripherals then we don't need wrapper layer for retime handling or two
 linux devices and drivers (one the existing and the other to handle retime)
 to represent a single physical gpmc peripheral device (for DT conversion).
 Then handling core frequency scaling and DT conversion would be easier.
 We were trying to create such a retime function that would be generic so
 as to handle different types of gpmc peripherals.

Sounds like a much better approach!

 And we have been able to create such a function. Below is an implementation
 that has been made for handling asynchronous timings. It has been tested for
 OneNAND  SMSC on OMAP3EVM (rev G  C) with [1-4]. OneNAND was tested using
 [5] (OMAP3EVM OneNAND works in async mode)  SMSC using [6] (mainline does
 not have a timing calculation for smsc911x)

Are you able to verify that the timing calculated by this function are
identical? May be some more details on exactly how you tested this would
be good.

 It was difficult to squeeze tusb6010 timing calculation into generic timing
 calculation, hence a boolean tusb has been used. This is what I could
 achieve based on existing retime for tusb6010 and for lack of tusb6010
 timing specifications.
 
 8---
 
 /* Device timings in picoseconds */
 struct gpmc_device_timings {
 u32 cs_setup;   /* CS setup time */
 u32 adv_setup;  /* ADV setup time */
 u32 adv_rd_off; /* ADV read off time */
 u32 adv_add_hold;   /* address hold time */
 u32 oe_setup;   /* OE setup time */
 u32 adv_access; /* access time from ADV assertion */
 u32 rd_access;  /* read access time */
 u32 oe_access;  /* access time from OE assertion */
 u32 cs_access;  /* access time from CS asertion */
 u32 rd_cycle;   /* read cycle time */
 u32 cs_highz;   /* CS deassertion to high Z */
 u32 oe_highz;   /* OE deassertion to high Z */
 u32 adv_wr_off; /* ADV write off time */
 u32 we_setup;   /* WE setup time */
 u32 wr_pulse;   /* write assertion time */
 u32 wr_data_setup;  /* data setup time from write assertion */
 u32 wr_high;/* write deassertion time */
 u32 we_highz;   /* WE deassertion to high Z */
 u32 wr_cycle;   /* write cycle time */
 
 boolmux;/* address  data muxed */
 booltusb;   /* peripheral is tusb6010 */

Do you think that there is any value in making the tusb member a u32
dev_type and then set it too GPMC_DEVICE_TUSB then this could be used
for other devices in the future too if needed?

 };
 
 struct gpmc_timings gpmc_calc_timings(struct gpmc_device_timings *dev_t)
 {
 struct gpmc_timings gpmc_t;
 bool mux = dev_t-mux;
 bool tusb = dev_t-tusb;
 u32 temp;
 
 memset(gpmc_t, 0, sizeof(gpmc_t));
 
 /* cs_on */
 gpmc_t.cs_on = gpmc_round_ns_to_ticks(dev_t-cs_setup / 1000);
 
 /* adv_on */
 temp = dev_t-adv_setup;
 if (tusb)
 temp = max_t(u32,
 (gpmc_t.cs_on + gpmc_ticks_to_ns(1)) * 1000, temp);
 gpmc_t.adv_on = gpmc_round_ns_to_ticks(temp / 1000);

Would it be possible to create a sub-function called
gpmc_calc_timings_tusb() and put all these if (tusb) statements in
there? Or maybe a generic function called gpmc_calc_timings_prepare().

For the above case could have ...

void gpmc_calc_timings_prepare(struct gpmc_device_timings *dev_t)
{
if (dev_t-tusb) {
dev_t-adv_on = max_t(u32,
(gpmc_t.cs_on + gpmc_ticks_to_ns(1)) * 1000,
dev_t-adv_setup);
...
} else {
dev_t-adv_on = dev_t-adv_setup;
...
}
}

And then in the gpmc_calc_timings() you would just have ...

 gpmc_t.adv_on = gpmc_round_ns_to_ticks(dev_t-adv_on / 1000);

 /* adv_rd_off */
 temp = dev_t-adv_rd_off;
 if (tusb)
 temp = max_t(u32,
 

gpmc generic retime function (subject was RE: [PATCH v5 3/3] ARM: OMAP2+: onenand: prepare for gpmc driver migration)

2012-08-06 Thread Mohammed, Afzal
Hi Tony, Jon,

On Wed, Jul 11, 2012 at 12:17:25, Tony Lindgren wrote:
 * Jon Hunter jon-hun...@ti.com [120710 10:20]:

  The DT node should simply have the information required by the retime
  function or gpmc timings themselves if available. In the case of OneNAND

  These can be stored in the DT and then translated to gpmc timings at
  runtime. DT should only store static timing or clock information known

 Yup. And the format of the timing data in DT should be standardized so
 the only differences for each connected peripheral is the retime function.

If we are able to achieve a generic retime function applicable to all
peripherals then we don't need wrapper layer for retime handling or two
linux devices and drivers (one the existing and the other to handle retime)
to represent a single physical gpmc peripheral device (for DT conversion).
Then handling core frequency scaling and DT conversion would be easier.
We were trying to create such a retime function that would be generic so
as to handle different types of gpmc peripherals.

And we have been able to create such a function. Below is an implementation
that has been made for handling asynchronous timings. It has been tested for
OneNAND  SMSC on OMAP3EVM (rev G  C) with [1-4]. OneNAND was tested using
[5] (OMAP3EVM OneNAND works in async mode)  SMSC using [6] (mainline does
not have a timing calculation for smsc911x)

It was difficult to squeeze tusb6010 timing calculation into generic timing
calculation, hence a boolean tusb has been used. This is what I could
achieve based on existing retime for tusb6010 and for lack of tusb6010
timing specifications.

8---

/* Device timings in picoseconds */
struct gpmc_device_timings {
u32 cs_setup;   /* CS setup time */
u32 adv_setup;  /* ADV setup time */
u32 adv_rd_off; /* ADV read off time */
u32 adv_add_hold;   /* address hold time */
u32 oe_setup;   /* OE setup time */
u32 adv_access; /* access time from ADV assertion */
u32 rd_access;  /* read access time */
u32 oe_access;  /* access time from OE assertion */
u32 cs_access;  /* access time from CS asertion */
u32 rd_cycle;   /* read cycle time */
u32 cs_highz;   /* CS deassertion to high Z */
u32 oe_highz;   /* OE deassertion to high Z */
u32 adv_wr_off; /* ADV write off time */
u32 we_setup;   /* WE setup time */
u32 wr_pulse;   /* write assertion time */
u32 wr_data_setup;  /* data setup time from write assertion */
u32 wr_high;/* write deassertion time */
u32 we_highz;   /* WE deassertion to high Z */
u32 wr_cycle;   /* write cycle time */

boolmux;/* address  data muxed */
booltusb;   /* peripheral is tusb6010 */
};

struct gpmc_timings gpmc_calc_timings(struct gpmc_device_timings *dev_t)
{
struct gpmc_timings gpmc_t;
bool mux = dev_t-mux;
bool tusb = dev_t-tusb;
u32 temp;

memset(gpmc_t, 0, sizeof(gpmc_t));

/* cs_on */
gpmc_t.cs_on = gpmc_round_ns_to_ticks(dev_t-cs_setup / 1000);

/* adv_on */
temp = dev_t-adv_setup;
if (tusb)
temp = max_t(u32,
(gpmc_t.cs_on + gpmc_ticks_to_ns(1)) * 1000, temp);
gpmc_t.adv_on = gpmc_round_ns_to_ticks(temp / 1000);

/* adv_rd_off */
temp = dev_t-adv_rd_off;
if (tusb)
temp = max_t(u32,
(gpmc_t.adv_on + gpmc_ticks_to_ns(1)) * 1000, temp);
gpmc_t.adv_rd_off = gpmc_round_ns_to_ticks(temp / 1000);

/* oe_on */
if (mux)
temp = gpmc_t.adv_rd_off * 1000 + dev_t-adv_add_hold;
else
temp = dev_t-oe_setup;
if (tusb)
temp = max_t(u32,
(gpmc_t.adv_rd_off + gpmc_ticks_to_ns(1)) * 1000, temp);
gpmc_t.oe_on = gpmc_round_ns_to_ticks(temp / 1000);

/* access */
temp = max_t(u32, dev_t-rd_access,
gpmc_t.oe_on * 1000 + dev_t-oe_access);
temp = max_t(u32, temp,
gpmc_t.cs_on * 1000 + dev_t-cs_access);
temp = max_t(u32, temp,
gpmc_t.adv_on * 1000 + dev_t-adv_access);
if (tusb) {
temp = max_t(u32, temp,
(gpmc_t.oe_on + gpmc_ticks_to_ns(1)) * 1000);
temp = max_t(u32, temp, gpmc_t.oe_on * 1000 + 300);
}
gpmc_t.access = gpmc_round_ns_to_ticks(temp / 1000);

gpmc_t.oe_off = gpmc_t.access + gpmc_ticks_to_ns(1);
gpmc_t.cs_rd_off = gpmc_t.oe_off;

/* rd_cycle */
temp = max_t(u32,