Patch attached.
//Peter
On 02.07.2008 01:20, Peter Stuge wrote:
flashrom: probe_flash() cleanup and remove false positive unknown .. SPI chip
Restructure probe_flash() for some readability, and now only find an unknown .. SPI chip if it is the first flash chip found in the system.
This also removes the true positive match when there is more than one flash chip and the 2nd or 3rd are unknown but I think that case is uncommon enough to warrant this improvement for the common case. The uncommon case can still use the -frc forced read.
Signed-off-by: Peter Stuge peter@stuge.se
Hm. AFAICS this collides with the cleanups Stefan wanted to do.
The evolution of flashrom is somewhat funny: 1. worked perfectly as long as only one flash chip was present, no false positives 2. multiple flash chip support was added, false generic positives for all SPI chips occured 3. cleanup/fix/bandaid for the code either improves functionality and reduces readability or the other way round 4. repeat step 3
As long as we don't perform per-bus/address probing (where appropriate), there will be no real fix for readability nor functionality.
Stefan's cleanup queue looks very promising, though. Peter, can you hold this patch for a few more days until Stefan had time to send part 2 of his cleanup stuff? I believe that will simplify the code greatly, while fixing the "unknown chip" problem permanently.
Regards, Carl-Daniel
On Wed, Jul 02, 2008 at 03:20:36AM +0200, Carl-Daniel Hailfinger wrote:
The evolution of flashrom is somewhat funny:
Too many cooks, a little bitrot and that's what you get. :)
Anyway - optimize for the common case! That means degrading in uncommon cases, which I think is fine.
As long as we don't perform per-bus/address probing (where appropriate), there will be no real fix for readability nor functionality.
-frc can allow reading at any time if the chip is the only problem.
Multiple known chips will work reliably.
Multiple chips with 2nd or 3rd unknown will not work - but this is a highly uncommon case so I do not really care.
Stefan's cleanup queue looks very promising, though. Peter, can you hold this patch for a few more days until Stefan had time to send part 2 of his cleanup stuff?
I expect it to take much longer than a few days, and thus be after 1.0, which should go out the door any day now.
I believe that will simplify the code greatly, while fixing the "unknown chip" problem permanently.
I should go post a reply in the flash bus thread. I have some thoughts.
//Peter
On 02.07.2008 04:25, Peter Stuge wrote:
On Wed, Jul 02, 2008 at 03:20:36AM +0200, Carl-Daniel Hailfinger wrote:
The evolution of flashrom is somewhat funny:
Too many cooks, a little bitrot and that's what you get. :)
<manager>we need to enstrengthen our common vision while concentrating on a customoer-focused attitude</manager> I'd really like to discuss future flashrom design once we got version 1.0 out. There are so many things flashrom could be made to do and everybody has ideas, yet we are missing a coherent design. The good thing about this is that the flashrom design certainly won't suffer from a lack of participation.
Anyway - optimize for the common case! That means degrading in uncommon cases, which I think is fine.
One could argue that two flash chips are an uncommon case, but... point taken.
As long as we don't perform per-bus/address probing (where appropriate), there will be no real fix for readability nor functionality.
-frc can allow reading at any time if the chip is the only problem.
Multiple known chips will work reliably.
Multiple chips with 2nd or 3rd unknown will not work - but this is a highly uncommon case so I do not really care.
The price we pay for this is even more special-case code.
Stefan's cleanup queue looks very promising, though. Peter, can you hold this patch for a few more days until Stefan had time to send part 2 of his cleanup stuff?
I expect it to take much longer than a few days, and thus be after 1.0, which should go out the door any day now.
No offense intended, but with all this probe special-casing the code has become a lot less readable. I look forward to the per-bus probing because it will allow us to revert most of the probing code to the state directly after multiple flash chip support went in (short, easily readable code).
I believe that will simplify the code greatly, while fixing the "unknown chip" problem permanently.
I should go post a reply in the flash bus thread. I have some thoughts.
Please do! Your input on the design is very much desired.
Regards, Carl-Daniel
On Wed, Jul 02, 2008 at 03:20:36AM +0200, Carl-Daniel Hailfinger wrote:
- cleanup/fix/bandaid for the code either improves functionality
and reduces readability or the other way round
I split my patch into two; one that only cleans up probe_flash(), and another (which is now very simple) that removes false positives (and true positives when unknown chip is 2nd or 3rd).
//Peter
-
Carl-Daniel Hailfinger -
Peter Stuge