On Fri, 01 Jul 2011 00:00:58 +0200 Carl-Daniel Hailfinger <[email protected]> wrote:
> Am 30.06.2011 02:13 schrieb Stefan Tauner: > > On Wed, 29 Jun 2011 15:06:00 -0700 > > David Hendricks <[email protected]> wrote: > > > >> On Mon, Jun 27, 2011 at 8:33 PM, Stefan Tauner > >> <[email protected]> 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 < > >> [email protected]> 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 ) > > > -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
