[U-Boot-Users] CFI flash polling for AMD/SPANSION?

So we have a patch to CFI flash to use DQ7/#data polling instead of DQ6/toggle. There seems to have been some discussion from time to time regard this and several people have reported empirical evidence about how DQ7 appears to work better for them on some systems.
I was hoping to understand what the issues are with getting such a change accepted.
- k

Hi Kumar,
On Tuesday 27 November 2007, Kumar Gala wrote:
So we have a patch to CFI flash to use DQ7/#data polling instead of DQ6/toggle. There seems to have been some discussion from time to time regard this and several people have reported empirical evidence about how DQ7 appears to work better for them on some systems.
I have to admit, that I don't remember those discussions. Could you please refresh my memory? And why exactly are you implementing this change? What problem does it fix?
I was hoping to understand what the issues are with getting such a change accepted.
If your patch fixes a problem, then chances are quite good. :)
BTW: What does Linux MTD do? DQ7/data or DQ6/toggle? (I'm just too lazy to check myself :))
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================

On Nov 27, 2007, at 12:41 PM, Stefan Roese wrote:
Hi Kumar,
On Tuesday 27 November 2007, Kumar Gala wrote:
So we have a patch to CFI flash to use DQ7/#data polling instead of DQ6/toggle. There seems to have been some discussion from time to time regard this and several people have reported empirical evidence about how DQ7 appears to work better for them on some systems.
I have to admit, that I don't remember those discussions. Could you please refresh my memory? And why exactly are you implementing this change? What problem does it fix?
http://search.gmane.org/?query=DQ7&group=gmane.comp.boot-loaders.u-boot
I'm told that higher frequencies we have issues using the toggle method on 85xx/86xx parts from freescale. But use DQ7 at those freq seems to work reliable for the flash parts we have on some of the boards.
I was hoping to understand what the issues are with getting such a change accepted.
If your patch fixes a problem, then chances are quite good. :)
BTW: What does Linux MTD do? DQ7/data or DQ6/toggle? (I'm just too lazy to check myself :))
It looks like it just uses DQ6/toggle if I'm reading the code correctly.
- k

In message 9CAC8A3B-89FA-44C3-B554-A53C79A0003D@kernel.crashing.org you wrote:
I'm told that higher frequencies we have issues using the toggle method on 85xx/86xx parts from freescale. But use DQ7 at those freq
Given the fact that the CFI code has absolutely no knowledge about which processor it is running on, this means one or both of two things:
1) There are issues with certain versions of certain toolchains which happen to be used with the processors in question
2) The processors in question are more likely to have problems with unexpected out-of-order scheduling / instruction reordering etc.
Please note that the current CFI driver accesses the flash by plain pointer references, in some cases even without using "volatile". This is supposed to cause problems sooner or later.
I think the CFI driver needs some basic rework to get ridof such pointer accesses and use proper accessor functions/macros instead to make sure we have all the memory barriers etc. we may need.
seems to work reliable for the flash parts we have on some of the boards.
If the DQ7 method works better, we could compare the code. Check for example for missing volatiles - but even better fix the code to use correct accessors.
It looks like it just uses DQ6/toggle if I'm reading the code correctly.
Note that Linux calls a (map)->write() function to access the flash.
Best regards,
Wolfgang Denk

On Nov 27, 2007, at 5:03 PM, Wolfgang Denk wrote:
In message <9CAC8A3B-89FA-44C3-B554- A53C79A0003D@kernel.crashing.org> you wrote:
I'm told that higher frequencies we have issues using the toggle method on 85xx/86xx parts from freescale. But use DQ7 at those freq
Given the fact that the CFI code has absolutely no knowledge about which processor it is running on, this means one or both of two things:
- There are issues with certain versions of certain toolchains which
happen to be used with the processors in question
- The processors in question are more likely to have problems with
unexpected out-of-order scheduling / instruction reordering etc.
Please note that the current CFI driver accesses the flash by plain pointer references, in some cases even without using "volatile". This is supposed to cause problems sooner or later.
I think the CFI driver needs some basic rework to get ridof such pointer accesses and use proper accessor functions/macros instead to make sure we have all the memory barriers etc. we may need.
Good to know. I'll see if I can get a test case that fails and play with fixing up the accessors to have barriers and the such.
seems to work reliable for the flash parts we have on some of the boards.
If the DQ7 method works better, we could compare the code. Check for example for missing volatiles - but even better fix the code to use correct accessors.
Agreed. Hopefully I can get a testcase from the people that have seen this to know if I'm actually fixing the code by adding proper accessors.
It looks like it just uses DQ6/toggle if I'm reading the code correctly.
Note that Linux calls a (map)->write() function to access the flash.
I came across the following comment:
* Note that anything more complicated than checking if no bits are toggling * (including checking DQ5 for an error status) is tricky to get working * correctly and is therefore not done (particulary with interleaved chips * as each chip must be checked independantly of the others).
- k

On Wed, 28 Nov 2007 00:03:48 +0100 Wolfgang Denk wd@denx.de wrote:
I think the CFI driver needs some basic rework to get ridof such pointer accesses and use proper accessor functions/macros instead to make sure we have all the memory barriers etc. we may need.
I happen to have a patch lying around which does exactly that, in addition to a few other cleanups.
Stefan, if you could push out all the stuff you have pending to the CFI custodian tree, I can rebase my stuff on top of that and submit for review.
It looks like it just uses DQ6/toggle if I'm reading the code correctly.
Note that Linux calls a (map)->write() function to access the flash.
Only if CONFIG_MTD_COMPLEX_MAPPINGS=y. Otherwise, it uses __raw_write[bwl].
Haavard

Hi Haavard,
On Wednesday 28 November 2007, Haavard Skinnemoen wrote:
I think the CFI driver needs some basic rework to get ridof such pointer accesses and use proper accessor functions/macros instead to make sure we have all the memory barriers etc. we may need.
I happen to have a patch lying around which does exactly that, in addition to a few other cleanups.
Great.
Stefan, if you could push out all the stuff you have pending to the CFI custodian tree, I can rebase my stuff on top of that and submit for review.
I suggest that we wait a few days more, till the new Makefile rework etc is in place. I'll then merge the master repo into the CFI custodian repo and push the pending changes in (mainly the non-CFI/JEDEC support). After this you can rebase your patch against the CFI repo. I suspect that this will happen sometime next week.
What do you think? Does this sound like a plan or what? :)
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================

On Wed, 28 Nov 2007 16:35:56 +0100 Stefan Roese sr@denx.de wrote:
What do you think? Does this sound like a plan or what? :)
Absolutely. Please drop me a note when you're ready.
Haavard

On Wednesday 28 November 2007, Haavard Skinnemoen wrote:
What do you think? Does this sound like a plan or what? :)
Absolutely. Please drop me a note when you're ready.
Ready for updates. The u-boot-cfi-flash custodian repo waits for your new shiny patches... :)
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
participants (4)
-
Haavard Skinnemoen
-
Kumar Gala
-
Stefan Roese
-
Wolfgang Denk