Attention is currently required from: Felix Singer, Thomas Heijligen, Angel Pons, Steve Markgraf.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71801 )
Change subject: programmer: Add bitbanging programmer driver for Linux libgpiod ......................................................................
Patch Set 4: Code-Review+1
(7 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/71801/comment/a333903b_67f8cf6e PS4, Line 9: old smartphone This is cool! :)
https://review.coreboot.org/c/flashrom/+/71801/comment/a9680929_3e85fee5 PS4, Line 17: All arguments are required. I think this can be mentioned in the manpage too.
https://review.coreboot.org/c/flashrom/+/71801/comment/9c368954_14d50915 PS4, Line 19: Reading of a 2048 kB flash chip on a Qualcomm MSM8916 SoC @800 MHz: I suggest a little re-phrase of this, just to make clear what was the testing scenario that you ran. Maybe:
Tested by reading of a 2048 kB flash chip on a Qualcomm MSM8916 SoC @800 MHz, ran the following command: <exact command line you ran> Output: Found GigaDevice flash chip ....
If you tested other scenarios (write, erase, verify) it would be great to mention here too. Thanks!
Patchset:
PS3:
What I had missed so far is that there is a second build system. […]
You are right that we have two build systems [still], and those are make and meson. You actually covered them both in your patch! just one small comment left (see my comment in README). `test_build.sh` is a script used for CI, and the script is running builds + tests for both build systems. Yes, you need to add new programmer to `test_build.sh` into both `make_programmer_opts` and `meson_programmer_opts` and once you add it, Jenkins will start building your patch after you push it to Gerrit. As you see, every time you push a patchset, Jenkins responds with +1 (or -1) and gives a link to build logs. If by any chance you get -1, you can look into the logs why.
The bit I am not sure about: how to install libgpiod for CI environment. It would be great if Thomas and Felix could comment on this? Thank you!
Patchset:
PS4: Thomas, Felix, it would be great if you could have a look, thanks!
File MAINTAINERS:
https://review.coreboot.org/c/flashrom/+/71801/comment/750588f1_e50262dc PS4, Line 137: LINUX GPIOD We keep this in abc order, so you need to swap with previous entry (LINUX MTD).
File README:
https://review.coreboot.org/c/flashrom/+/71801/comment/8e8381b9_3c1748a8 PS4, Line 76: libgpiod-dev (if you want support for Linux GPIO devices) Could you please add this info to meson doc too? Here: Documentation/building.md Thank you!