On Fri, 01 Jul 2011 00:00:58 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Am 30.06.2011 02:13 schrieb Stefan Tauner:
On Wed, 29 Jun 2011 15:06:00 -0700 David Hendricks dhendrix@google.com wrote:
On Mon, Jun 27, 2011 at 8:33 PM, Stefan Tauner stefan.tauner@student.tuwien.ac.at wrote:
We don't optimize for compilers. The ICH SPI code was donated to flashrom, and I'm happy we got it. The authors already rewrote most of it (compared to the initial submission) before I merged it, and back then the flashrom source code was not exactly a thing of beauty. Since then, quite a few cleanups have happened, but people tended to leave ICH SPI alone because the code was a bit more complex than the rest of flashrom. ICH SPI has quite a few refactoring opportunities: There are various helper functions which could be used more than once...
working on it if you have something in mind specifically, just drop it off in my corner. ;)
but changes should be in their on commit though anyway and so we can postpone it.
Yes. However, using the better style for the new code would make it easy to just copy it over to the old code in a later commit.
parser exception. the current patch does move the fill/read loops used equivalently in ich7_run_opcode and ich9_run_opcode into two functions which are called instead. much easier for review and testing. it can then be optimized/de-obfuscated later.
On Mon, Jun 27, 2011 at 8:33 PM, Stefan Tauner < stefan.tauner@student.tuwien.ac.at> wrote:
+static uint8_t ich_fill_data(const uint8_t *data, uint8_t len, int reg0_off)
As mentioned on IRC, the caller tends to pass in a "len" value > 255, e.g. 1 block of 4096 bytes or more. Since len is handled as an int in the caller, it should probably get passed in as an int here.
inderdaad. i would have expected the compiler to warn in such cases. obviously i am too used to my usual embedded makefile with ~20 -W terms :)
Side note: Do we want a release mode Makefile switch which activates all warnings under the sun?
i would call it a developing makefile and would love it. the only danger of such a thing is that you begin to rely on it and stop thinking for yourself - i am all for it therefore! :)
please see 9/10 and 10/10 for my proposed fixes of this (and other problems like the possibility of setting SME )