Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Hydrabus hardware #21

Open
wants to merge 5 commits into
base: staging
Choose a base branch
from

Conversation

Baldanos
Copy link

This patch adds support for Hydrabus hardware (http://hydrabus.com/).
As it reuses the same commands as the Bus Pirate, only detection code has been changed.

This patch has been tested on Hydrabus firmware revision 8488be1e5231ca5e55c4ba193aebaf1b29abcfd1 and on Bus Pirate hw rev 3.a / firmware 6.1 and 6.2.

@urjaman
Copy link
Contributor

urjaman commented Oct 3, 2017

I found some issues with this:

  • it is missing a signoff (this one only you can do)
  • if you select spispeed=2.6M, it will complain "Hydrabus does not support lower speeds." because that speed is supported also by the bus pirate. I suggest splitting the speeds table in two...
  • the manpage needs updating

N.B. I'm also working on having serprog/libfrser support in hydrafw, but decided to look at this too...

Copy link
Contributor

@urjaman urjaman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better, but you missed a few things in splitting the spispeeds table.

N.B. When I dont find anything to complain about codewise I'll post this patchset (likely squashed to one patch) to the flashrom gerrit.

buspirate_spi.c Show resolved Hide resolved
buspirate_spi.c Outdated
msg_pinfo("It is recommended to upgrade to firmware 6.2 or newer.\n");
spispeed = 0x4;
}
if(spispeed > 0x07) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test is now unnecessary

buspirate_spi.c Outdated
@@ -424,6 +460,8 @@ int buspirate_spi_init(void)
return 1;
}

/* Hydrabus uses different speeds in the same range */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mask is now unnecessary

Baldanos added 5 commits July 11, 2018 16:01
Change-Id: I621b384dcea42688a7d67c42865c88f1b958654f
Change-Id: I6ac87251ddcb63ffd50d1bd7a5535de59eaad8b2
Signed-off-by: Baldanos <balda@balda.ch>
Change-Id: Idc0a0ffccec92e1c8eb23150dd287119425f5de8
Signed-off-by: Baldanos <balda@balda.ch>
Change-Id: I674ddd125e81c81a5c2b9dab623ba7c83db88fcd
Signed-off-by: Baldanos <balda@balda.ch>
Change-Id: I9f44a40090b7746baa4951975bf6cb2c42e381f9
@Baldanos
Copy link
Author

Baldanos commented Aug 8, 2018

Hi @urjaman !

We just made some more tests and corrected some bugs on the patch, seems to work well using BP and hydrabus.

How can I help make it into flashrom ?

I made a new branch synced on master (see https://github.com/Baldanos/flashrom), but I don't know how to change this PR to point to the new branch. Let me know if I need to close this one and open a new one.

@afiskon
Copy link

afiskon commented Oct 17, 2018

Hello,

I would very much like to see this patch in the upstream. I've tested @Baldanos branch with HydraBus and various chips and just tested once again the latest version with another chip, W25Q128.V. I can confirm that it works just fine.

Please let me know if there is anything I can do to help with this pull request.

@afiskon
Copy link

afiskon commented Oct 17, 2018

Just in case I compared two dumps, first made with HydraBus and second made with FT2232 chip. They are identical.

@bvernoux
Copy link

Could you accept this pull request ? It will be very nice to have official support of HydraBus on next version of flashrom

@fridtjof
Copy link

What's the status on this? I would very much like to use this, too :)

@bvernoux
Copy link

Could your provide an update on that Pull Request after more than 1 year I confirm it works perfectly

@bvernoux
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants