Submitter | David Hendricks |
---|---|
Date | 2015-08-02 22:09:40 |
Message ID | <CAAAYX_irhAtcjOBxgOvR+aBSY9DS7dqOEEmJANyPznFa62CoTA@mail.gmail.com> |
Download | mbox | patch |
Permalink | /patch/4312/ |
State | Accepted |
Headers | show |
Comments
On Sun, 2015-08-02 at 15:09 -0700, David Hendricks wrote:
> Tested with SF100 V:5.1.5 and SF600 V:7.1.4 (with subsequent patches)
Where are these subsequent patches of which you speak? I have SF600
testers who would be happy to try them out...
On Mon, Aug 3, 2015 at 4:50 PM, David Woodhouse <dwmw2@infradead.org> wrote: > On Sun, 2015-08-02 at 15:09 -0700, David Hendricks wrote: > > Tested with SF100 V:5.1.5 and SF600 V:7.1.4 (with subsequent patches) > > Where are these subsequent patches of which you speak? I have SF600 > testers who would be happy to try them out... > I was just referring to the earlier patches that you and Stefan had posted earlier (based off Simon Glass's work): http://www.flashrom.org/pipermail/flashrom/2015-July/013759.html <insert rant about SCM and code review process>
On Tue, 2015-08-04 at 00:37 -0700, David Hendricks wrote: > > > On Mon, Aug 3, 2015 at 4:50 PM, David Woodhouse <dwmw2@infradead.org> > wrote: > > On Sun, 2015-08-02 at 15:09 -0700, David Hendricks wrote: > > > Tested with SF100 V:5.1.5 and SF600 V:7.1.4 (with subsequent > > patches) > > > > Where are these subsequent patches of which you speak? I have SF600 > > testers who would be happy to try them out... > I was just referring to the earlier patches that you and Stefan had > posted earlier (based off Simon Glass's work): > http://www.flashrom.org/pipermail/flashrom/2015-July/013759.html Hm. I could have sworn I'd tried simply disabling the call to dediprog_device_init(), and my SF600Plus tester (FW 7.1.1) still told me it wasn't working — or that it was working for reads but not erase/writes. Am I missing something more that your patch does? > <insert rant about SCM and code review process> Yeah. FWIW my set of patches is at http://git.infradead.org/users/dwmw2/flashrom.git — it's not *impossible* to work sanely, even if you are stuck with a project which for some reason is still stuck in the 20th century with Subversion.
On Sun, 2015-08-02 at 15:09 -0700, David Hendricks wrote: > > Tested with SF100 V:5.1.5 and SF600 V:7.1.4 (with subsequent patches) > Signed-off-by: David Hendricks <dhendrix@chromium.org> Can you push a git tree somewhere with those 'subsequent patches' please? I know you said they're just the ones I posted, but I could have sworn I'd *tried* what you're doing in this patch, and my testers said it didn't work (I only have SF100 locally). http://git.infradead.org/users/dwmw2/flashrom.git has my version of those 'subsequent patches' — can you confirm that your patch works on top of that?
Voilà! https://chromium-review.googlesource.com/#/c/293924/ Freed of the shackles of Subversion and Patchwork, maybe now I can get some real work done on this thing. On Fri, Aug 14, 2015 at 2:09 AM, David Woodhouse <dwmw2@infradead.org> wrote: > On Sun, 2015-08-02 at 15:09 -0700, David Hendricks wrote: > > > > Tested with SF100 V:5.1.5 and SF600 V:7.1.4 (with subsequent patches) > > Signed-off-by: David Hendricks <dhendrix@chromium.org> > > Can you push a git tree somewhere with those 'subsequent patches' > please? I know you said they're just the ones I posted, but I could > have sworn I'd *tried* what you're doing in this patch, and my testers > said it didn't work (I only have SF100 locally). > > http://git.infradead.org/users/dwmw2/flashrom.git has my version of > those 'subsequent patches' — can you confirm that your patch works on > top of that? > > -- > David Woodhouse Open Source Technology Centre > David.Woodhouse@intel.com Intel Corporation > > _______________________________________________ > flashrom mailing list > flashrom@flashrom.org > http://www.flashrom.org/mailman/listinfo/flashrom >
On Sun, 2 Aug 2015 15:09:40 -0700 David Hendricks <dhendrix@chromium.org> wrote: > As per e-mail with Dediprog, command 0x0B (which is not listed in the > command spec) is used to set voltage level on older Dediprogs. SF100 > V6.0.0 and newer as well as all SF600 programmers do not support it. > > This patch renames dediprog_device_init() to something more > appropriate and adds comments for clarity, and only runs it > conditionally if we cannot query the devicestring during init. > > Tested with SF100 V:5.1.5 and SF600 V:7.1.4 (with subsequent patches) > Signed-off-by: David Hendricks <dhendrix@chromium.org> As I said on gerrit, I would really like to know what this really does. There is another function to change the SPI VCC so this is probably something else...
Patch
Index: dediprog.c =================================================================== --- dediprog.c (revision 1896) +++ dediprog.c (working copy) @@ -75,6 +75,7 @@ CMD_READ_PROG_INFO = 0x08, CMD_SET_VCC = 0x09, CMD_SET_STANDALONE = 0x0A, + CMD_SET_VOLTAGE = 0x0B, /* Only in firmware older than 6.0.0 */ CMD_GET_BUTTON = 0x11, CMD_GET_UID = 0x12, CMD_SET_CS = 0x14, @@ -579,13 +580,15 @@ return 0; } -static int dediprog_device_init(void) +/* Only use dediprog_set_voltage on SF100 programmers with firmware older + * than V6.0.0. Newer programmers (including all SF600s) do not support it. */ +static int dediprog_set_voltage(void) { int ret; char buf[0x1]; memset(buf, 0, sizeof(buf)); - ret = usb_control_msg(dediprog_handle, REQTYPE_OTHER_IN, 0x0B, 0x0, 0x0, + ret = usb_control_msg(dediprog_handle, REQTYPE_OTHER_IN, CMD_SET_VOLTAGE, 0x0, 0x0, buf, 0x1, DEFAULT_TIMEOUT); if (ret < 0) {
As per e-mail with Dediprog, command 0x0B (which is not listed in the command spec) is used to set voltage level on older Dediprogs. SF100 V6.0.0 and newer as well as all SF600 programmers do not support it. This patch renames dediprog_device_init() to something more appropriate and adds comments for clarity, and only runs it conditionally if we cannot query the devicestring during init. Tested with SF100 V:5.1.5 and SF600 V:7.1.4 (with subsequent patches) Signed-off-by: David Hendricks <dhendrix@chromium.org> msg_perr("Command A failed (%s)!\n", usb_strerror()); @@ -595,6 +598,7 @@ msg_perr("Unexpected response to init!\n"); return 1; } + return 0; } @@ -853,11 +857,15 @@ if (register_shutdown(dediprog_shutdown, NULL)) return 1; - /* Perform basic setup. */ - if (dediprog_device_init()) - return 1; - if (dediprog_check_devicestring()) - return 1; + /* Try reading the devicestring. If that fails and the device is old + * (FW < 6.0.0) then we need to try the "set voltage" command and then + * attempt to read the devicestring again. */ + if (dediprog_check_devicestring()) { + if (dediprog_set_voltage()) + return 1; + if (dediprog_check_devicestring()) + return 1; + } /* Set all possible LEDs as soon as possible to indicate activity. * Because knowing the firmware version is required to set the LEDs correctly we need to this after