On Jul 4, 2011, at 8:08 PM, Mike Frysinger wrote: > in general, this seems to go in a good direction. certainly better than > previous attempts. there are some issues with this patch though. > > can you split up independent issues into independent patches ?
Yes, I should have done that to begin with. > the > flashverify command stuff for example should be a patch by itself (and > further > discussed since i'm pretty sure it breaks existing behavior). As you said, this should have been done separately, however I will give a quick overview of what I did: The "flashmem" command as it is in the codebase programs the flash and then verifies it. I made the verify step optional using the command line parameter "noverify", and added an additional "flashverify" command. I don't think it breaks any existing behavior as everything behaves the same as the codebase if you don't add the "noverify" parameter to flashmem. > then have one > patch which simply adds the new stub enable/disable/write_start/cls fields. > then one which adds the spi bus driver, and finally one which adds the spi > flash driver. I assume I can just submit each of these patches to this same thread? > > the style in a bunch of new code is slightly broken: > - dont use tabs ... use 4 spaces to indent > - no trailing whitespace on lines > - no trailing new lines in files > - func calls are like: > foo (arg, arg, arg); > - dont use inline var decls like: > for (int i ....... > - use space in between operators: > i == 0 > i >= 0 > you call printf() in a bunch of places. these all need to be changed to > urj_log() and such. No problem cleaning up the formatting. > with the spi bus driver, if the 3 main pins arent specified, then just try to > grab pins with the relevant name. so if people dont specify MOSI, then > default to the signal name "MOSI". Good idea. > i'm thinking we should abstract out the cfi structures a bit rather than > trying to stick spi flash info into cfi structs. Well, some SPI flashes can provide much of the same identification and geometry information as a CFI. So, it seemed like a good idea to use the same common structs. That said, I have not implemented any code to read that information because the specific device I was working with at the time did not provide this information. It also just seemed easier as it didn't break any other code by using this struct. > > i think you changed the binary verify code to only work with msbin's. the > current logic treats files as raw binaries. I don't think I broke anything here; however, I don't have any msbin files to test with. Everything I work with is all raw binary. What I did was pull all of the verification code out of urf_flashmsbin and created a function called urj_flashverifymsbin. Then I pulled all of the verification code out of urj_flashmem and created a function called urj_flashverify. > > and of course, by now, the code will need to be rebased onto the current code > base as a bunch of things have changed. No problem. I try to keep everything up to date anyway. > -mike
smime.p7s
Description: S/MIME cryptographic signature
------------------------------------------------------------------------------ All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2
_______________________________________________ UrJTAG-development mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/urjtag-development
