Prepare for GPMC driver conversion

Reorganize gpmc-onenand initialization so that changes required for gpmc driver migration can be made smooth.

Ensuring sync read/write are disabled in onenand cannot be expected to work properly unless GPMC is setup, this has been removed.

Refactor set_async_mode & set_sync_mode functions to separate out timing calculation & actual configuration (GPMC & OneNAND side).

The last patch in this series causes onenand not to show up on my n900. I believe the problem has been there earlier too, but I just did not notice it.

Hi Tony,

Quoted 3 lines · Tony Lindgren

The last patch in this series causes onenand not to show up on my n900. I believe the problem has been there earlier too, but I just did not notice it.

Sorry for the delayed response, could reach workplace a short while ago only

Could the diff [1] be tried and check whether it resolves the issue,

Regards
Afzal

[1]

arch/arm/mach-omap2/gpmc-onenand.c +0 -0

index c8a9487..bbae674 100644

* Mohammed, Afzal <[email protected]> [120628 02:36]:

Yes that seems to do the trick, thanks! I can fold that into the breaking patch when applying.

Quoted 8 lines · Mohammed, Afzal

arch/arm/mach-omap2/gpmc-onenand.c +0 -0

index c8a9487..bbae674 100644
@@ -364,6 +364,8 @@ static int omap2_onenand_setup_async(void __iomem *onenand_base)
        struct gpmc_timings t;
        int ret;
 

arch/arm/mach-omap2/gpmc-onenand.c static int omap2_onenand_setup_async(void __iomem *onenand_base) · line 371

Quoted 5 lines

arch/arm/mach-omap2/gpmc-onenand.c +2 -0

+       omap2_onenand_set_async_mode(onenand_base);
+
        t = omap2_onenand_calc_async_timings();
 
        ret = gpmc_set_async_mode(gpmc_onenand_data->cs, &t);

Yes that seems to do the trick, thanks! I can fold that into the breaking patch when applying.

Quoted 10 lines · Tony Lindgren

Quoted 8 lines · Mohammed, Afzal

arch/arm/mach-omap2/gpmc-onenand.c +0 -0

index c8a9487..bbae674 100644
@@ -364,6 +364,8 @@ static int omap2_onenand_setup_async(void __iomem *onenand_base)
        struct gpmc_timings t;
        int ret;
 

Quoted 8 lines

arch/arm/mach-omap2/gpmc-onenand.c static int omap2_onenand_setup_async(void __iomem *onenand_base) · line 371

Quoted 5 lines

arch/arm/mach-omap2/gpmc-onenand.c +2 -0

+       omap2_onenand_set_async_mode(onenand_base);
+
        t = omap2_onenand_calc_async_timings();
 
        ret = gpmc_set_async_mode(gpmc_onenand_data->cs, &t);

Yes that seems to do the trick, thanks! I can fold that into the breaking patch when applying.

Also following needs to be removed from commit message,

"Ensuring sync read/write are disabled in onenand cannot be expected to work properly unless GPMC is setup, this has been removed."

With the above diff we are in effect disabling sync mode in onenand before GPMC has been setup in the Kernel

If you want, I can sent a new series too.

Quoted 25 lines · Mohammed, Afzal

Hi Tony,

Quoted 3 lines · Tony Lindgren

The last patch in this series causes onenand not to show up on my n900. I believe the problem has been there earlier too, but I just did not notice it.

Sorry for the delayed response, could reach workplace a short while ago only

Could the diff [1] be tried and check whether it resolves the issue,

Regards
Afzal

[1]

arch/arm/mach-omap2/gpmc-onenand.c +0 -0

index c8a9487..bbae674 100644
@@ -364,6 +364,8 @@ static int omap2_onenand_setup_async(void __iomem *onenand_base)
        struct gpmc_timings t;
        int ret;
 

arch/arm/mach-omap2/gpmc-onenand.c static int omap2_onenand_setup_async(void __iomem *onenand_base) · line 371

Quoted 5 lines

arch/arm/mach-omap2/gpmc-onenand.c +2 -0

+       omap2_onenand_set_async_mode(onenand_base);
+
        t = omap2_onenand_calc_async_timings();
 
        ret = gpmc_set_async_mode(gpmc_onenand_data->cs, &t);

Yes that seems to do the trick, thanks! I can fold that into the breaking patch when applying.

Hi Tony, Afzal,

Quoted 26 lines · Tony Lindgren

Quoted 25 lines · Mohammed, Afzal

Hi Tony,

Quoted 3 lines · Tony Lindgren

The last patch in this series causes onenand not to show up on my n900. I believe the problem has been there earlier too, but I just did not notice it.

Sorry for the delayed response, could reach workplace a short while ago only

Could the diff [1] be tried and check whether it resolves the issue,

Regards
Afzal

[1]

arch/arm/mach-omap2/gpmc-onenand.c +0 -0

index c8a9487..bbae674 100644
@@ -364,6 +364,8 @@ static int omap2_onenand_setup_async(void __iomem *onenand_base)
        struct gpmc_timings t;
        int ret;
 

Quoted 8 lines

arch/arm/mach-omap2/gpmc-onenand.c static int omap2_onenand_setup_async(void __iomem *onenand_base) · line 371

Quoted 5 lines

arch/arm/mach-omap2/gpmc-onenand.c +2 -0

+       omap2_onenand_set_async_mode(onenand_base);
+
        t = omap2_onenand_calc_async_timings();
 
        ret = gpmc_set_async_mode(gpmc_onenand_data->cs, &t);

Yes that seems to do the trick, thanks! I can fold that into the breaking patch when applying.

I am not sure what to make of this. Testing Afzal's this series along with the other gpmc-prep series [1], onenand is working fine on my 3430sdp and I see ...

[    2.792510] OneNAND driver initializing
[    2.797576] omap2-onenand omap2-onenand: initializing on CS2, phys base 0x20000000, virtual base c88c0000, freq 0 MHz

Tony, have you tried using any of the mtd kernel tests to verify OneNAND read/write is working on your n900? For example ...

# insmod mtd_pagetest.ko dev=<mtd-part-num>

_NOTE_ that above test erases the OneNAND! ;-)

Quoted 9 lines · Tony Lindgren

Quoted 5 lines · Mohammed, Afzal

Quoted 3 lines · Tony Lindgren

The last patch in this series causes onenand not to show up on my n900. I believe the problem has been there earlier too, but I just did not notice it.

Yes that seems to do the trick, thanks! I can fold that into the breaking patch when applying.

I am not sure what to make of this. Testing Afzal's this series along with the other gpmc-prep series [1], onenand is working fine on my 3430sdp and I see ...

[    2.792510] OneNAND driver initializing
[    2.797576] omap2-onenand omap2-onenand: initializing on CS2, phys base 0x20000000, virtual base c88c0000, freq 0 MHz
[    2.808929] OneNAND Manufacturer: Samsung (0xec)
[    2.808990] Muxed OneNAND 256MB 1.8V 16-bit (0x40)
[    2.814208] OneNAND version = 0x002c

The above change seems to imply that Tony's n900 is dependent on the bootloader settings and not those being set by the kernel. Ideally, we should not need to set the async mode

Numonyx. The gpmc-onenand.c only has one set of settings that it is using for all devices. However, it would appear that at least the async settings are not working for the Numonyx. Therefore, may be we need to get a dump of Tony's n900 settings and make

Kernel can't handle gpmc by itself, may be resetting onenand is a solution, as it seems bootloader is leaving it in sync mode.

Hi Tony, Jon,

Quoted 14 lines · Jon

Quoted 9 lines · Tony Lindgren

Quoted 5 lines · Mohammed, Afzal

Quoted 3 lines · Tony Lindgren

The last patch in this series causes onenand not to show up on my n900. I believe the problem has been there earlier too, but I just did not notice it.

Yes that seems to do the trick, thanks! I can fold that into the breaking patch when applying.

I am not sure what to make of this. Testing Afzal's this series along with the other gpmc-prep series [1], onenand is working fine on my 3430sdp and I see ...

Quoted 8 lines

[    2.792510] OneNAND driver initializing
[    2.797576] omap2-onenand omap2-onenand: initializing on CS2, phys base 0x20000000, virtual base c88c0000, freq 0 MHz
[    2.808929] OneNAND Manufacturer: Samsung (0xec)
[    2.808990] Muxed OneNAND 256MB 1.8V 16-bit (0x40)
[    2.814208] OneNAND version = 0x002c

The above change seems to imply that Tony's n900 is dependent on the bootloader settings and not those being set by the kernel. Ideally, we should not need to set the async mode

In one of the Samsumg onenand datasheet, it was mentioned, (seems Numonyx datasheet is under NDA)

[A] "If device is accessed synchronously, while set to asynchronous read mode, it is possible to read out first data without problems"

It seems even though above may not imply the below, chances are that below would be true

[B] "If device is accessed asynchronously, while set to synchronous read mode, it will not be possible to read out data"

And B seems logical(to me); if onenand is set to synchronous mode, it would expect clocks to transfer data which may not happen with asynchronous operation from gpmc.

Here the problem with Tony's n900 (expecting it to use one of mach-id in n8x0, hence flag sync read) board may be that bootloader (could not find uboot handling these machines) before handing over control to Kernel, sets it to synchronous mode. And then if we apply B, we can expect what we are seeing here.

Assuming above is a explanation to what Tony is observing on n900, perhaps, resetting onenand may help Kernel work even without the proposed diff. There is a gpio pin field used in n8x0, if that is connected to reset of onenand, may be it can achieve what we want without the diff.

Quoted 3 lines

Numonyx. The gpmc-onenand.c only has one set of settings that it is using for all devices. However, it would appear that at least the async settings are not working for the Numonyx. Therefore, may be we need to get a dump of Tony's n900 settings and make

I have a different opinion, even with the existing code, with the default timings for onenand, Numonyx is working in async mode, reason being that frequency is being obtained with read operation executed in async mode (later based on this frequency, code calculates sync timings & then set to sync mode)

This change indeed has brought to our notice one instance where Kernel can't handle gpmc by itself, may be resetting onenand is a solution, as it seems bootloader is leaving it in sync mode.

Quoted 5 lines · Afzal

I have a different opinion, even with the existing code, with the default timings for onenand, Numonyx is working in async mode, reason being that frequency is being obtained with read operation executed in async mode (later based on this frequency, code calculates sync timings & then set to sync mode)

I don't follow "frequency is being obtained with read operation executed in async mode". Can you give a code reference?

[    2.792510] OneNAND driver initializing
[    2.797576] omap2-onenand omap2-onenand: initializing on CS2, phys base 0x20000000, virtual base c88c0000, freq 0 MHz
[    2.808929] OneNAND Manufacturer: Samsung (0xec)
[    2.808990] Muxed OneNAND 256MB 1.8V 16-bit (0x40)
[    2.814208] OneNAND version = 0x002c

The above change seems to imply that Tony's n900 is dependent on the bootloader settings and not those being set by the kernel. Ideally, we should not need to set the async mode in the onenand before we set the onenand timings in the gpmc (per Afzal's changelog comment). This appears backwards.

The other thing to note is that the 3430sdp has samsung onenand where as the n900 has Numonyx. The gpmc-onenand.c only has one set of settings that it is using for all devices. However, it would appear that at least the async settings are not working for the Numonyx. Therefore, may be we need to get a dump of Tony's n900 settings and make sure the right settings are being used for the appropriate board.

These onenand settings are really killing us. I don't want us to have to spend alot of time re-calculating this stuff but the way it has been written to begin with is not driver friendly. I really wonder if we need to have some sort of callback for the onenand timings from the driver. It is ugly, but the alternative is that someone needs to sit down and re-calculate all the timings again to get them into a driver friendly format. Furthermore, it seems that onenand is no longer available from the likes of samsung and numonyx (micron) so it is hard to justify re-calculating everything again. I am not even sure if we have all the datasheets!

Let me know your thoughts.

Quoted 4 lines · Jon Hunter

The above change seems to imply that Tony's n900 is dependent on the bootloader settings and not those being set by the kernel. Ideally, we should not need to set the async mode in the onenand before we set the onenand timings in the gpmc (per Afzal's changelog comment). This appears backwards.

That should not be the case, I'm more likely to believe in Afzal's explanation.

Hi Tony,

Quoted 8 lines · Tony Lindgren

Quoted 4 lines · Jon Hunter

The above change seems to imply that Tony's n900 is dependent on the bootloader settings and not those being set by the kernel. Ideally, we should not need to set the async mode in the onenand before we set the onenand timings in the gpmc (per Afzal's changelog comment). This appears backwards.

That should not be the case, I'm more likely to believe in Afzal's explanation.

Not sure whether you are fine with fixing up this patch with added diff

Assuming inferences so far is not wrong, right now this patch with the added diff would be perfectly fine.

Problem would happen when we are at a stage to do gpmc reset using hwmod [seems miles to go before I sleep {or read gpmc hwmod reset} ;)]. If bootloader left onenand configured in sync mode, to switch onenand to async mode, first configuring gpmc to sync mode would be required & for that we need frequency information from onenand and to get that information from onenand, gpmc has to be configured for sync mode and to configure gpmc to sync mode ....

Quoted 11 lines · Afzal

Not sure whether you are fine with fixing up this patch with added diff

Assuming inferences so far is not wrong, right now this patch with the added diff would be perfectly fine.

Problem would happen when we are at a stage to do gpmc reset using hwmod [seems miles to go before I sleep {or read gpmc hwmod reset} ;)]. If bootloader left onenand configured in sync mode, to switch onenand to async mode, first configuring gpmc to sync mode would be required & for that we need frequency information from onenand and to get that information from onenand, gpmc has to be configured for sync mode and to configure gpmc to sync mode ....

You are concerned about hwmod performing a reset of the gpmc during boot? We should be able to use the HWMOD_INIT_NO_RESET flag to prevent this. Would that work?

Hi Jon,

Quoted 14 lines · Jon

Quoted 11 lines · Afzal

Not sure whether you are fine with fixing up this patch with added diff

Assuming inferences so far is not wrong, right now this patch with the added diff would be perfectly fine.

Problem would happen when we are at a stage to do gpmc reset using hwmod [seems miles to go before I sleep {or read gpmc hwmod reset} ;)]. If bootloader left onenand configured in sync mode, to switch onenand to async mode, first configuring gpmc to sync mode would be required & for that we need frequency information from onenand and to get that information from onenand, gpmc has to be configured for sync mode and to configure gpmc to sync mode ....

Quoted 3 lines

You are concerned about hwmod performing a reset of the gpmc during boot? We should be able to use the HWMOD_INIT_NO_RESET flag to prevent this. Would that work?

Yes that will work in the short term, the reason I am trying to avoid no reset flag in the long term is to prevent this board support being broken, please refer Paul's requirement [1] in accepting gpmc hwmod patch

So we have 2 options here ...

1. Use the HWMOD_INIT_NO_RESET for now and your updated version of this patch
2. See if there is a gpio available to control the OneNAND reset on the n900.

Do you agree? Any other options?

Quoted 7 lines · Jon Hunter

[    2.792510] OneNAND driver initializing
[    2.797576] omap2-onenand omap2-onenand: initializing on CS2, phys base 0x20000000, virtual base c88c0000, freq 0 MHz
[    2.808929] OneNAND Manufacturer: Samsung (0xec)
[    2.808990] Muxed OneNAND 256MB 1.8V 16-bit (0x40)
[    2.814208] OneNAND version = 0x002c

Quoted 4 lines

The above change seems to imply that Tony's n900 is dependent on the bootloader settings and not those being set by the kernel. Ideally, we should not need to set the async mode in the onenand before we set the onenand timings in the gpmc (per Afzal's changelog comment). This appears backwards.

That should not be the case, I'm more likely to believe in Afzal's explanation.

Quoted 5 lines

The other thing to note is that the 3430sdp has samsung onenand where as the n900 has Numonyx. The gpmc-onenand.c only has one set of settings that it is using for all devices. However, it would appear that at least the async settings are not working for the Numonyx. Therefore, may be we need to get a dump of Tony's n900 settings and make sure the right settings are being used for the appropriate board.

Hmm I would assume the n900 onenand settings are the most tested settings we have. And AFAIK have also been tested with L3 frequency scaling. So let's assume they're mostly right.

Quoted 10 lines

These onenand settings are really killing us. I don't want us to have to spend alot of time re-calculating this stuff but the way it has been written to begin with is not driver friendly. I really wonder if we need to have some sort of callback for the onenand timings from the driver. It is ugly, but the alternative is that someone needs to sit down and re-calculate all the timings again to get them into a driver friendly format. Furthermore, it seems that onenand is no longer available from the likes of samsung and numonyx (micron) so it is hard to justify re-calculating everything again. I am not even sure if we have all the datasheets!

Let me know your thoughts.

I don't think we should spend much time working on the recalculations. Let's rather use these known working cases as examples. If they don't work, it's likely that adding any new devices won't work either.

For the timings, we should allow specifying either cycles or time values in the data struct. And always just use the cycle value directly if specified, and othewise use the time value. We may be able to limit the registers where we need to allow either cycle or time value depending on the device futher.

In general, I doubt that we can come up with better calculations. The existing code pretty well already follows the device spec timings. And using cycle values for some registers is the right thing to do according to the connected device specs no matter what the frequency is. In those cases converting from time values to cycles does not make sense.

Quoted 6 lines · Tony Lindgren

In general, I doubt that we can come up with better calculations. The existing code pretty well already follows the device spec timings. And using cycle values for some registers is the right thing to do according to the connected device specs no matter what the frequency is. In those cases converting from time values to cycles does not make sense.

Ok agree, but the problem here is how to provide the timings to the driver. The onenand code is doing a lot of rounding based upon the gpmc clock before it presents the timings (in nano-seconds) to the gpmc function to calculate the final timings and program the gpmc chip-select. So therefore I think that we have the following options ...

1. The simplest is to continue using a global variable for storing the gpmc f-clk handle and have the OneNAND timings calculated prior to probing the gpmc driver.

2. Provide some sort of "retime" callback that the gpmc driver can call at probe time to calculate the timings.

Quoted 15 lines · Jon Hunter

Quoted 6 lines · Tony Lindgren

In general, I doubt that we can come up with better calculations. The existing code pretty well already follows the device spec timings. And using cycle values for some registers is the right thing to do according to the connected device specs no matter what the frequency is. In those cases converting from time values to cycles does not make sense.

Ok agree, but the problem here is how to provide the timings to the driver. The onenand code is doing a lot of rounding based upon the gpmc clock before it presents the timings (in nano-seconds) to the gpmc function to calculate the final timings and program the gpmc chip-select. So therefore I think that we have the following options ...

Quoted 3 lines

1. The simplest is to continue using a global variable for storing the gpmc f-clk handle and have the OneNAND timings calculated prior to probing the gpmc driver.

Well we should not expose gpmc fck handle to the drivers..

Quoted 2 lines

2. Provide some sort of "retime" callback that the gpmc driver can call at probe time to calculate the timings.

Yes how about the gpmc using driver code registers itself with the gpmc code and also registers it's retime function with the gpmc? That way the gpmc fck stays inside the gpmc code, and the driver specific retime function should be able to do the calculation based on driver clocks. The retime function needs to have also a pointer to driver private data for it's clocks etc.

It seems this retime function may need to be called by the gpmc code when L3 changes, and the driver code if the driver is switching between runtime and idle clocks like tusb6010 for example does.

Quoted 2 lines · Jon Hunter

2. Provide some sort of "retime" callback that the gpmc driver can call at probe time to calculate the timings.

Yes how about the gpmc using driver code registers itself with the gpmc code and also registers it's retime function with the gpmc? That way the gpmc fck stays inside the gpmc code, and the driver specific retime function should be able to do the calculation based on driver clocks. The retime function needs to have also a pointer to driver private data for it's clocks etc.

It seems this retime function may need to be called by the gpmc code when L3 changes, and the driver code if the driver is switching between runtime and idle clocks like tusb6010 for example does.

Hi Tony,

Quoted 10 lines · Tony Lindgren

Quoted 2 lines · Jon Hunter

2. Provide some sort of "retime" callback that the gpmc driver can call at probe time to calculate the timings.

Yes how about the gpmc using driver code registers itself with the gpmc code and also registers it's retime function with the gpmc? That way the gpmc fck stays inside the gpmc code, and the driver specific retime function should be able to do the calculation based on driver clocks. The retime function needs to have also a pointer to driver private data for it's clocks etc.

Sorry, not sure whether I follow you properly, based on what has been understood, will try to rephrase,

All the present gpmc timing calculation done in arch/arm/mach-omap2/gpmc-* to be moved to gpmc driver. And all the peripheral drivers using gpmc, i.e like smsc911x, tusb6010 needs to register their retime function with gpmc driver. And gpmc driver will invoke these registered retime function, when clock frequency changes.

But wouldn't it need changes in the existing drivers like smsc911x that are used by multiple architectures with gpmc specific registration (put under a macro ?). We will be having gpmc driver code that contains knowledge about peripheral timing calculation, seems there is no way out of this. Peripheral agnostic gpmc code may not happen it seems

In that case gpmc driver probe would have to be relieved of the task of setting up gpmc timings as we have to wait till peripheral drivers register their callback, right ?, seems in that case no timing information needs (or can be) to be passed from DT

Quoted 3 lines

It seems this retime function may need to be called by the gpmc code when L3 changes, and the driver code if the driver is switching between runtime and idle clocks like tusb6010 for example does.

I believe you are referring to tusb6010_platform_retime(), other than during initial setup, i.e. in tusb6010_setup_interface(), it is not invoked. tusb6010_platform_retime() is an exported symbol, unless I am missing something it is not invoked other than during initial setup. Did find this function in tusb6010.c, it is commented out, may be this was present earlier ?

Quoted 17 lines · Mohammed, Afzal

Quoted 5 lines · Tony Lindgren

Yes how about the gpmc using driver code registers itself with the gpmc code and also registers it's retime function with the gpmc? That way the gpmc fck stays inside the gpmc code, and the driver specific retime function should be able to do the calculation based on driver clocks. The retime function needs to have also a pointer to driver private data for it's clocks etc.

Sorry, not sure whether I follow you properly, based on what has been understood, will try to rephrase,

All the present gpmc timing calculation done in arch/arm/mach-omap2/gpmc-* to be moved to gpmc driver. And all the peripheral drivers using gpmc, i.e like smsc911x, tusb6010 needs to register their retime function with gpmc driver. And gpmc driver will invoke these registered retime function, when clock frequency changes.

Quoted 5 lines

But wouldn't it need changes in the existing drivers like smsc911x that are used by multiple architectures with gpmc specific registration (put under a macro ?). We will be having gpmc driver code that contains knowledge about peripheral timing calculation, seems there is no way out of this. Peripheral agnostic gpmc code may not happen it seems

It depends. For some drivers scaling both gpmc clock and the device clock can happen, like with tusb6010 for example. But the smsc911x does not know about the clocks.. So to additional changes to the driver would be required to if device clocks need scaling. But for now, we should be able to do it at gpmc level with the retime function, or just disable DFS for those clocks if not supported.

Quoted 4 lines

In that case gpmc driver probe would have to be relieved of the task of setting up gpmc timings as we have to wait till peripheral drivers register their callback, right ?, seems in that case no timing information needs (or can be) to be passed from DT

Well we should pass all the gpmc timing information from DT. And then the driver also still needs to register it's retime function with gpmc.

I have a doubt whether we are talking about the same thing, presently our main issue is in eliminating the necessity of peripheral specific functions like gpmc_onenand_init, tusb_setup_interface (which calls tusb6010_platform_retime), etc., which calculate gpmc timings by processing peripheral specific timings over gpmc clock period and these processing were required before gpmc driver probe gets invoked as gpmc driver needed timings which gpmc can understand to configure gpmc.

During boot time, gpmc driver needs to know timings in terms of gpmc parameters to configure gpmc, what I unable to understand from your proposal is how gpmc driver knows which retime function to invoke (so that gpmc timings can be calculated based on the type of peripheral) to calculate gpmc timings. Or do you want a DT field specifying type of peripheral connected and so that at probe time the relevant retime can be invoked ?

Initially I thought you were suggesting that all peripheral drivers connected to gpmc should register their retime function (where function will be an exported symbol - part of gpmc code) and invoke the respective retime as part of peripheral driver probe, hence relieve gpmc driver of doing it, even though retime would be present as part of gpmc code. But seems that is not what you were suggesting.

By "we should be able to do it at gpmc level", I am unable to understand what you are suggesting.

Quoted 7 lines

Quoted 4 lines

In that case gpmc driver probe would have to be relieved of the task of setting up gpmc timings as we have to wait till peripheral drivers register their callback, right ?, seems in that case no timing information needs (or can be) to be passed from DT

Well we should pass all the gpmc timing information from DT. And then the driver also still needs to register it's retime function with gpmc.

So timing information that would be passed from DT should be for exact gpmc timings like cs_on, cs_off etc., right ?, in that case should we manually calculate (like as now done by Kernel in gpmc-onenand.c etc) it by having the knowledge of connected peripheral & gpmc frequency at boot time and update it in DT ?, as we can't apply retime on it as we don't know the connected peripheral in gpmc driver. Or do you want another field through DT to decide retime that is to be used, then I think passing timing from DT would not be needed

Quoted 2 lines · Mohammed, Afzal

Initially I thought you were suggesting that all peripheral drivers connected to gpmc should register their retime function (where

Yes that's what I was suggesting.

Hi Tony,

Quoted 4 lines · Tony Lindgren

Quoted 2 lines · Mohammed, Afzal

Initially I thought you were suggesting that all peripheral drivers connected to gpmc should register their retime function (where

Quoted 1 line

Yes that's what I was suggesting.

To understand you better, peripheral drivers above refers to driver created out of present gpmc platform init code like gpmc-smsc911x.c. You were not referring to drivers/net/ethernet/smsc/smsc911x.c, right ?

Quoted 8 lines · Mohammed, Afzal

I have a doubt whether we are talking about the same thing, presently our main issue is in eliminating the necessity of peripheral specific functions like gpmc_onenand_init, tusb_setup_interface (which calls tusb6010_platform_retime), etc., which calculate gpmc timings by processing peripheral specific timings over gpmc clock period and these processing were required before gpmc driver probe gets invoked as gpmc driver needed timings which gpmc can understand to configure gpmc.

Right. The issue is that both the gpmc clock and the peripheral clock may change, and both cause the need to reprogram the gpmc timings.

Quoted 2 lines

By "we should be able to do it at gpmc level", I am unable to understand what you are suggesting.

Right, gpmc should be able to handle things alone with the registered retime function for smsc911x, where the driver does not even know about the bus. With DT, the platform init code gpmc-smsc911x.c should become a driver that registers with gpmc and provides the retime function.

Quoted 9 lines

So timing information that would be passed from DT should be for exact gpmc timings like cs_on, cs_off etc., right ?, in that case should we manually calculate (like as now done by Kernel in gpmc-onenand.c etc) it by having the knowledge of connected peripheral & gpmc frequency at boot time and update it in DT ?, as we can't apply retime on it as we don't know the connected peripheral in gpmc driver. Or do you want another field through DT to decide retime that is to be used, then I think passing timing from DT would not be needed

The timings values should be passed to gpmc from DT. We need to be able to pass both cycle and time based values. If no cycle based value is passed, the time based value should be used. This is because some peripheral timings can be cycle based, while others can be time based.

The peripheral driver can register it's retime function with gpmc and get a cookie back that allows getting the DT provided timings from gpmc. And after that the initial timings can be set.

Hi Tony,

Quoted 11 lines · Tony Lindgren

Quoted 8 lines · Mohammed, Afzal

I have a doubt whether we are talking about the same thing, presently our main issue is in eliminating the necessity of peripheral specific functions like gpmc_onenand_init, tusb_setup_interface (which calls tusb6010_platform_retime), etc., which calculate gpmc timings by processing peripheral specific timings over gpmc clock period and these processing were required before gpmc driver probe gets invoked as gpmc driver needed timings which gpmc can understand to configure gpmc.

Quoted 2 lines

Right. The issue is that both the gpmc clock and the peripheral clock may change, and both cause the need to reprogram the gpmc timings.

Presently bigger issue that I am facing w.r.t driver conversion is the requirement of peripheral specific gpmc timing calculation before probe. I believe currently in mainline runtime gpmc clock changes are not handled

Quoted 7 lines

Quoted 2 lines

By "we should be able to do it at gpmc level", I am unable to understand what you are suggesting.

Right, gpmc should be able to handle things alone with the registered retime function for smsc911x, where the driver does not even know about the bus. With DT, the platform init code gpmc-smsc911x.c should become a driver that registers with gpmc and provides the retime function.

So then we would be having two devices & drivers to represent gpmc peripheral like smsc911x, one existing ethernet driver and other one for handling gpmc timings, right ?. And with DT, so we need two nodes to represent a gpmc peripheral ?

Quoted 10 lines

Quoted 9 lines

So timing information that would be passed from DT should be for exact gpmc timings like cs_on, cs_off etc., right ?, in that case should we manually calculate (like as now done by Kernel in gpmc-onenand.c etc) it by having the knowledge of connected peripheral & gpmc frequency at boot time and update it in DT ?, as we can't apply retime on it as we don't know the connected peripheral in gpmc driver. Or do you want another field through DT to decide retime that is to be used, then I think passing timing from DT would not be needed

Quoted 5 lines

The timings values should be passed to gpmc from DT. We need to be able to pass both cycle and time based values. If no cycle based value is passed, the time based value should be used. This is because some peripheral timings can be cycle based, while others can be time based.

If we can describe gpmc timings purely based on time and cycles field for all the peripherals, can we not remove all the retime functions like timing calculation done in gpmc-onenand.c ?

Quoted 3 lines

The peripheral driver can register it's retime function with gpmc and get a cookie back that allows getting the DT provided timings from gpmc. And after that the initial timings can be set.

If timings peripheral timings can be fully contained in driver, should we try to pass the same timings translated in terms of gpmc timings through DT ?, and how do we get equivalent gpmc timings to update DT, manually calculate similar to platform init code ?, or I misunderstood you

Sorry to trouble you with more questions, I wanted to understand the way you want to deal with the issue.

Quoted 6 lines · Mohammed, Afzal

On Thu, Jul 05, 2012 at 16:25:35, Tony Lindgren wrote:

Presently bigger issue that I am facing w.r.t driver conversion is the requirement of peripheral specific gpmc timing calculation before probe. I believe currently in mainline runtime gpmc clock changes are not handled

Hmm I don't follow, why can't the timings be set during the peripheral specific driver probe?

Quoted 9 lines

Quoted 3 lines

The peripheral driver can register it's retime function with gpmc and get a cookie back that allows getting the DT provided timings from gpmc. And after that the initial timings can be set.

If timings peripheral timings can be fully contained in driver, should we try to pass the same timings translated in terms of gpmc timings through DT ?, and how do we get equivalent gpmc timings to update DT, manually calculate similar to platform init code ?, or I misunderstood you

Well yes the timings should be passed via devicetree in a gpmc specific

format. But the peripheral specific retime function still needs to be also registered for peripherals that need it.

Hi Tony,

Could not respond you earlier as was sick

Quoted 8 lines · Tony Lindgren

Quoted 6 lines · Mohammed, Afzal

On Thu, Jul 05, 2012 at 16:25:35, Tony Lindgren wrote:

Presently bigger issue that I am facing w.r.t driver conversion is the requirement of peripheral specific gpmc timing calculation before probe. I believe currently in mainline runtime gpmc clock changes are not handled

Quoted 2 lines

Hmm I don't follow, why can't the timings be set during the peripheral specific driver probe?

My viewpoint while making the above comment was with gpmc driver series that was posted, in that it was required to do peripheral specific timings calculation before probe. With your suggested peripheral specific driver for the purpose of handling retime, it is not a problem, a solution on how to integrate it has to be found though.

Quoted 9 lines

Quoted 9 lines

Quoted 3 lines

The peripheral driver can register it's retime function with gpmc and get a cookie back that allows getting the DT provided timings from gpmc. And after that the initial timings can be set.

If timings peripheral timings can be fully contained in driver, should we try to pass the same timings translated in terms of gpmc timings through DT ?, and how do we get equivalent gpmc timings to update DT, manually calculate similar to platform init code ?, or I misunderstood you

Quoted 1 line

Well yes the timings should be passed via devicetree in a gpmc specific

I assume with the above, you were referring to peripherals that does not have retime function.

Quoted 2 lines

format. But the peripheral specific retime function still needs to be also registered for peripherals that need it.

For the peripherals requiring retime, we cannot (as otherwise whatever retime does would have to be manually done based on the knowledge of boot time gpmc clock period to calculate gpmc timings to be fed to DT) pass gpmc timings via device tree, right ?

Quoted 5 lines · Mohammed, Afzal

Quoted 2 lines · Tony Lindgren

format. But the peripheral specific retime function still needs to be also registered for peripherals that need it.

Quoted 4 lines

For the peripherals requiring retime, we cannot (as otherwise whatever retime does would have to be manually done based on the knowledge of boot time gpmc clock period to calculate gpmc timings to be fed to DT) pass gpmc timings via device tree, right ?

We can still do it when the connected peripheral probe registers with gpmc.

Hi Tony,

Quoted 6 lines · Tony Lindgren

Quoted 5 lines · Mohammed, Afzal

Quoted 2 lines · Tony Lindgren

format. But the peripheral specific retime function still needs to be also registered for peripherals that need it.

Quoted 7 lines

Quoted 4 lines

For the peripherals requiring retime, we cannot (as otherwise whatever retime does would have to be manually done based on the knowledge of boot time gpmc clock period to calculate gpmc timings to be fed to DT) pass gpmc timings via device tree, right ?

We can still do it when the connected peripheral probe registers with gpmc.

We can, but would it be feasible practically ?, gpmc timings to update in DT for a such a peripheral (requiring retime) can be found out only by manual calculation similar to the way done in retime function (based on peripheral's timings and boot time gpmc clock period), correct ?, Also wouldn't this make it necessary to know gpmc clock period at boot time for properly updating gpmc timing entry in DT ?

And in this case, we are going to register retime function, so instead of relying on DT to provide gpmc timings for such a peripheral, won't it be better to make use of retime that is already registered ?

Quoted 10 lines · Mohammed, Afzal

Quoted 9 lines · Tony Lindgren

Quoted 4 lines · Mohammed, Afzal

For the peripherals requiring retime, we cannot (as otherwise whatever retime does would have to be manually done based on the knowledge of boot time gpmc clock period to calculate gpmc timings to be fed to DT) pass gpmc timings via device tree, right ?

We can still do it when the connected peripheral probe registers with gpmc.

Quoted 6 lines

We can, but would it be feasible practically ?, gpmc timings to update in DT for a such a peripheral (requiring retime) can be found out only by manual calculation similar to the way done in retime function (based on peripheral's timings and boot time gpmc clock period), correct ?, Also wouldn't this make it necessary to know gpmc clock period at boot time for properly updating gpmc timing entry in DT ?

The gpmc clock period can be returned to the connected peripheral when it's registering. Well basically we can call the retime function upon registering and pass the gpmc clock period.

Quoted 4 lines

And in this case, we are going to register retime function, so instead of relying on DT to provide gpmc timings for such a peripheral, won't it be better to make use of retime that is already registered ?

No we need to pass the timings from device tree as they may be different for similar boards. For example, different level shifters used on similar boards may affect the timings, although the retime function can be the same.

Hi Tony,

Quoted 9 lines · Tony Lindgren

Quoted 8 lines · Mohammed, Afzal

Quoted 7 lines · Tony Lindgren

Quoted 4 lines · Mohammed, Afzal

For the peripherals requiring retime, we cannot (as otherwise whatever retime does would have to be manually done based on the knowledge of boot time gpmc clock period to calculate gpmc timings to be fed to DT) pass gpmc timings via device tree, right ?

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

We can still do it when the connected peripheral probe registers with gpmc.

Were you actually referring to updating kernel view of device tree nodes, and not the device tree file (not sure whether it is really possible) ?

Quoted 7 lines

Quoted 6 lines

We can, but would it be feasible practically ?, gpmc timings to update in DT for a such a peripheral (requiring retime) can be found out only by manual calculation similar to the way done in retime function (based on peripheral's timings and boot time gpmc clock period), correct ?, Also wouldn't this make it necessary to know gpmc clock period at boot time for properly updating gpmc timing entry in DT ?

Quoted 3 lines

The gpmc clock period can be returned to the connected peripheral when it's registering. Well basically we can call the retime function upon registering and pass the gpmc clock period.

Won't this lead to the necessity of particular driver load order problem ?, As per the above, to return gpmc clock period to the connected peripheral, we need to ensure that gpmc driver is probed before peripheral driver.

And in that case how can gpmc driver rely on DT as gpmc timings for the peripheral requiring retime would not yet be available as peripheral driver is not yet probed, seems like a circular dependency.

Quoted 8 lines

Quoted 3 lines

And in this case, we are going to register retime function, so instead of relying on DT to provide gpmc timings for such a peripheral, won't it be better to make use of retime that is already registered ?

No we need to pass the timings from device tree as they may be different for similar boards. For example, different level shifters used on similar boards may affect the timings, although the retime function can be the same.

Unless I am missing something, could not see this scenario taken care in the existing retime functions, did see one comment for smc91x, but seems in that case Kernel doesn't do any timing configuration and leave timings as configured by bootloader

From a high-level I think that the goal should be ...

Hi Afzal,

Quoted 15 lines · Afzal

Hi Tony,

Quoted 11 lines · Tony Lindgren

Quoted 10 lines · Mohammed, Afzal

Quoted 9 lines · Tony Lindgren

Quoted 4 lines · Mohammed, Afzal

For the peripherals requiring retime, we cannot (as otherwise whatever retime does would have to be manually done based on the knowledge of boot time gpmc clock period to calculate gpmc timings to be fed to DT) pass gpmc timings via device tree, right ?

We can still do it when the connected peripheral probe registers with gpmc.

Quoted 2 lines

Were you actually referring to updating kernel view of device tree nodes, and not the device tree file (not sure whether it is really possible) ?

I believe that Tony is suggesting performing the retime at probe time. In the case of OneNand you would simply call the onenand_set_async_mode/sync_mode from within the probe as a retime function.

Quoted 11 lines

Quoted 10 lines

Quoted 6 lines

We can, but would it be feasible practically ?, gpmc timings to update in DT for a such a peripheral (requiring retime) can be found out only by manual calculation similar to the way done in retime function (based on peripheral's timings and boot time gpmc clock period), correct ?, Also wouldn't this make it necessary to know gpmc clock period at boot time for properly updating gpmc timing entry in DT ?

The gpmc clock period can be returned to the connected peripheral when it's registering. Well basically we can call the retime function upon registering and pass the gpmc clock period.

Quoted 3 lines

Won't this lead to the necessity of particular driver load order problem ?, As per the above, to return gpmc clock period to the connected peripheral, we need to ensure that gpmc driver is probed before peripheral driver.

I think it is more like when probing the gpmc, the retime function for the connected peripheral is called passing the gpmc clock freq.

Quoted 3 lines

And in that case how can gpmc driver rely on DT as gpmc timings for the peripheral requiring retime would not yet be available as peripheral driver is not yet probed, seems like a circular dependency.

The DT node should simply have the information required by the retime function or gpmc timings themselves if available. In the case of OneNAND async mode you have a bunch of constants such as ...

const int t_cer = 15;
const int t_avdp = 12;

const int t_cez  = 20; /* max of t_cez, t_oez */
const int t_ds   = 30;
const int t_wpl  = 40;
const int t_wph  = 30

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 at compile time.

Quoted 9 lines

Quoted 8 lines

Quoted 3 lines

And in this case, we are going to register retime function, so instead of relying on DT to provide gpmc timings for such a peripheral, won't it be better to make use of retime that is already registered ?

No we need to pass the timings from device tree as they may be different for similar boards. For example, different level shifters used on similar boards may affect the timings, although the retime function can be the same.

Quoted 4 lines

Unless I am missing something, could not see this scenario taken care in the existing retime functions, did see one comment for smc91x, but seems in that case Kernel doesn't do any timing configuration and leave timings as configured by bootloader

I think that we just need to adapt the current functions that our calculating the timings so that ...

1. We can call from them within the gpmc probe to setup the timings versus having the peripherals program the gpmc priory to probe.
2. Any static timing information needed by the retime function is part of the platform data passed to the gpmc probe and therefore can also be read from DT.

From a high-level I think that the goal should be ...

gpmc_probe

	--> request CS
	--> calls retime function to calculate gpmc timing (optional)
	--> configures CS
	--> registers peripheral device

Quoted 18 lines · Jon Hunter

Hi Afzal,

Quoted 15 lines · Afzal

Hi Tony,

Quoted 11 lines · Tony Lindgren

Quoted 10 lines · Mohammed, Afzal

Quoted 9 lines · Tony Lindgren

Quoted 4 lines · Mohammed, Afzal

For the peripherals requiring retime, we cannot (as otherwise whatever retime does would have to be manually done based on the knowledge of boot time gpmc clock period to calculate gpmc timings to be fed to DT) pass gpmc timings via device tree, right ?

We can still do it when the connected peripheral probe registers with gpmc.

Quoted 7 lines

Quoted 2 lines

Were you actually referring to updating kernel view of device tree nodes, and not the device tree file (not sure whether it is really possible) ?

I believe that Tony is suggesting performing the retime at probe time. In the case of OneNand you would simply call the onenand_set_async_mode/sync_mode from within the probe as a retime function.

Yes there's no need to do it earlier.

Quoted 11 lines

Quoted 11 lines

Quoted 10 lines

Quoted 6 lines

We can, but would it be feasible practically ?, gpmc timings to update in DT for a such a peripheral (requiring retime) can be found out only by manual calculation similar to the way done in retime function (based on peripheral's timings and boot time gpmc clock period), correct ?, Also wouldn't this make it necessary to know gpmc clock period at boot time for properly updating gpmc timing entry in DT ?

The gpmc clock period can be returned to the connected peripheral when it's registering. Well basically we can call the retime function upon registering and pass the gpmc clock period.

Quoted 6 lines

Quoted 3 lines

Won't this lead to the necessity of particular driver load order problem ?, As per the above, to return gpmc clock period to the connected peripheral, we need to ensure that gpmc driver is probed before peripheral driver.

I think it is more like when probing the gpmc, the retime function for the connected peripheral is called passing the gpmc clock freq.

Right, there's no need to do any of that at gpmc probe time. And the module dependencies take care of the load ordrer as gpmc_cs_request() is exported.

Quoted 10 lines

Quoted 3 lines

And in that case how can gpmc driver rely on DT as gpmc timings for the peripheral requiring retime would not yet be available as peripheral driver is not yet probed, seems like a circular dependency.

The DT node should simply have the information required by the retime function or gpmc timings themselves if available. In the case of OneNAND async mode you have a bunch of constants such as ...

const int t_cer = 15;
const int t_avdp = 12;

Quoted 8 lines

const int t_cez  = 20; /* max of t_cez, t_oez */
const int t_ds   = 30;
const int t_wpl  = 40;
const int t_wph  = 30

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 at compile time.

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.

Quoted 14 lines

Quoted 9 lines

Quoted 8 lines

Quoted 3 lines

And in this case, we are going to register retime function, so instead of relying on DT to provide gpmc timings for such a peripheral, won't it be better to make use of retime that is already registered ?

No we need to pass the timings from device tree as they may be different for similar boards. For example, different level shifters used on similar boards may affect the timings, although the retime function can be the same.

Quoted 4 lines

Unless I am missing something, could not see this scenario taken care in the existing retime functions, did see one comment for smc91x, but seems in that case Kernel doesn't do any timing configuration and leave timings as configured by bootloader

Quoted 8 lines

I think that we just need to adapt the current functions that our calculating the timings so that ...

1. We can call from them within the gpmc probe to setup the timings versus having the peripherals program the gpmc priory to probe.
2. Any static timing information needed by the retime function is part of the platform data passed to the gpmc probe and therefore can also be read from DT.

Yup, and also:

3. Disable frequency scaling for L3 if no retime function is specified

In that case we may have a generic default function that just sets the boot time values and disables the L3 scaling.

Quoted 7 lines

From a high-level I think that the goal should be ...

gpmc_probe

	--> request CS
	--> calls retime function to calculate gpmc timing (optional)
	--> configures CS
	--> registers peripheral device

Yes with few additions.. Connected peripheral probe requests CS from gpmc with the optional retime function pointer passed as a parameter. After gpmc code has determined the CS is available, it calls the optional retime function before returning back to the connected peripheral probe.

So how about the following with a bit more details:

gpmc_probe

	--> just sets up gpmc resources then idles itself

connected peripheral probe

	--> calls gpmc_cs_request() with gpmc timings from DT and an
	    optional retime function as a parameter
	--> gpmc_cs_request() allocates the CS
	--> gpmc_cs_request() calls the optional retime function and
	    if not specified, just sets the DT timings and disables
	    L3 DFS
	--> gpmc_cs_request() returns to connected peripheral probe

Regards,

Tony

Quoted 5 lines · Jon Hunter

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.

Quoted 7 lines

From a high-level I think that the goal should be ...

gpmc_probe

	--> request CS
	--> calls retime function to calculate gpmc timing (optional)
	--> configures CS
	--> registers peripheral device

Yes with few additions.. Connected peripheral probe requests CS from gpmc with the optional retime function pointer passed as a parameter. After gpmc code has determined the CS is available, it calls the optional retime function before returning back to the connected peripheral probe.

So how about the following with a bit more details:

gpmc_probe

	--> just sets up gpmc resources then idles itself

connected peripheral probe

	--> calls gpmc_cs_request() with gpmc timings from DT and an
	    optional retime function as a parameter
	--> gpmc_cs_request() allocates the CS
	--> gpmc_cs_request() calls the optional retime function and
	    if not specified, just sets the DT timings and disables
	    L3 DFS
	--> gpmc_cs_request() returns to connected peripheral probe

Hi Tony, Jon,

Quoted 10 lines · Tony Lindgren

Quoted 5 lines · Jon Hunter

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 */

        bool    mux;            /* address & data muxed */
        bool    tusb;           /* 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, dev_t->rd_cycle,
                        gpmc_t.cs_rd_off * 1000 + dev_t->cs_highz);
        temp = max_t(u32, temp,
                        gpmc_t.oe_off * 1000 + dev_t->oe_highz);
        if (tusb) {
                temp = max_t(u32, temp,
                        (gpmc_t.cs_rd_off + gpmc_ticks_to_ns(1)) * 1000);
                temp = max_t(u32, temp, gpmc_t.cs_rd_off * 1000 + 7000);
        }
        gpmc_t.rd_cycle = gpmc_round_ns_to_ticks(temp / 1000);

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

        /* we_on */
        if (mux)
                temp = gpmc_t.adv_wr_off * 1000 + dev_t->adv_add_hold;
        else
                temp = dev_t->we_setup;
        if (tusb)
                temp = max_t(u32,
                        (gpmc_t.adv_wr_off + gpmc_ticks_to_ns(1)) * 1000, temp);
        gpmc_t.we_on = gpmc_round_ns_to_ticks(temp / 1000);

        gpmc_t.wr_data_mux_bus = gpmc_t.we_on;

        /* we_off */
        temp = max_t(u32, gpmc_t.we_on * 1000 + dev_t->wr_pulse,
                        gpmc_t.we_on * 1000 + dev_t->wr_data_setup);
        if (tusb) {
                temp = max_t(u32, temp,
                                (gpmc_t.we_on + gpmc_ticks_to_ns(1)) * 1000);
                temp = max_t(u32, temp, gpmc_t.we_on * 1000 + 300);
        }
        gpmc_t.we_off = gpmc_round_ns_to_ticks(temp / 1000);

        gpmc_t.cs_wr_off = gpmc_round_ns_to_ticks((gpmc_t.we_off * 1000 +
                                dev_t->wr_high) / 1000);

        /* wr_cycle */
        temp = max_t(u32, dev_t->wr_cycle,
                        gpmc_t.cs_wr_off * 1000 + dev_t->cs_highz);
        temp = max_t(u32, temp,
                        gpmc_t.we_off * 1000 + dev_t->we_highz);
        if (tusb) {
                temp = max_t(u32, temp,
                        (gpmc_t.cs_wr_off + gpmc_ticks_to_ns(1)) * 1000);
                temp = max_t(u32, temp, gpmc_t.cs_wr_off * 1000 + 7000);
        }
        gpmc_t.wr_cycle = gpmc_round_ns_to_ticks(temp / 1000);

        return gpmc_t;
}

Quoted 18 lines

Quoted 7 lines

From a high-level I think that the goal should be ...

gpmc_probe

	--> request CS
	--> calls retime function to calculate gpmc timing (optional)
	--> configures CS
	--> registers peripheral device

Yes with few additions.. Connected peripheral probe requests CS from gpmc with the optional retime function pointer passed as a parameter. After gpmc code has determined the CS is available, it calls the optional retime function before returning back to the connected peripheral probe.

So how about the following with a bit more details:

gpmc_probe

	--> just sets up gpmc resources then idles itself

Quoted 8 lines

connected peripheral probe

	--> calls gpmc_cs_request() with gpmc timings from DT and an
	    optional retime function as a parameter
	--> gpmc_cs_request() allocates the CS
	--> gpmc_cs_request() calls the optional retime function and
	    if not specified, just sets the DT timings and disables
	    L3 DFS
	--> gpmc_cs_request() returns to connected peripheral probe

Once we are able to use the generic retime function as mentioned above, instead of requiring additional driver for each peripheral that has retime and its effect on complicating DT conversion, we can have the gpmc driver organization similar to the one that was posted earlier[4].

With DT, we can pass peripheral timings that has been generalized as in gpmc_device_timings structure. In addition to timings, we would need to have fields for specifying whether multiplexed or not ("mux" in struct gpmc_device_timings), asynchronous or synchronous.

With the generic retime function, driver could do as follows,

gpmc_probe
-> setup_device

    -> setup_cs
      ->setup_cs_config_timing
		if (gpmc_timings)
			gpmc_cs_set_timings(gpmc_t);
		else if (gpmc_device_timings) {
			gpmc_t = gpmc_calc_timings();
			gpmc_cs_set_timings(cs, &gpmc_t);
		}

In the above, addition to already posted driver series [4] would be the "else if" portions.

For core DFS, we could register a notifier that would invoke,

	gpmc_t = gpmc_calc_timings(); /* defined above */
	gpmc_cs_set-timings(cs, &gpmc_t);

enabling us to have the gpmc timings adjusted.

Please let me know your comments on the above. Currently I am working on achieving generalized timing for synchronous operations too so that gpmc timings can be calculated at runtime using a single generic retime function for all kinds of operations like mux, sync/async etc.

Quoted 5 lines

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.

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().

Objective of this series is to make things easy for GPMC driver conversion series by separating out more things from driver conversion series.

This series,
1. Unifies NAND platform initialization functions
2. Prepares OneNAND platform code for gpmc driver migration
3. Handles additional timings in Kernel

This series is based on 3.5-rc1 & made on top of
[PATCH v2 00/10] Prepare for GPMC driver conversion (w.r.t MTD) {http://www.mail-archive.com/linux-omap at vger.kernel.org/msg70096.html}

These changes has been tested with omap3evm & beagle board. Relevant GPMC peripherals that got tested by this were NAND (beagle) and OneNAND (using local patches for omap3evm)