On Sun, 13 Mar 2016 14:30:19 +0100
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2...@gmx.net> wrote:
> On 05.03.2016 22:42, Stefan Tauner wrote:
> > Tested mainboards:
> > OK:
> > - ASRock Fatal1ty 970 Performance and P4i65G
> > Reported by anonymous email message ID:
> > 932677687262b1300eaf14260999d9262...@guerrillamail.com
> > The latter actually had a tested board enable already.
> >
> > Flash chips:
> > - GigaDevice GD25VQ41B to PREW (+PREW)
> > Reported by David Hendricks
> > - Winbond W39V040FB to PREW (+EW)
> > Reported by fjed on IRC
> >
> > Miscellaneous:
> > - Change PCI IDs of "MS-6577 (Xenon)" board enable.
> > The previous IDs contained the on-board display adapter which is
> > disabled when a dedicated graphics card is installed.
> > - Add a note to the README how to overcome the clang warning if only a
> > single programmer is enabled.
> > - Fix some typo and manpage problems found by lintian
> > - r1920 introduced some explicit calls to pkg-config instead of
> > $(PKG_CONFIG).
> > This patch corrects that.
> > - Add some overrides to the Makefile in case someone/something sets
> > variables like CPPFLAGS or LDFLAGS as command line parameters
>
> I couldn't find that in the patch.
This was already committed separately in r1931, thanks.
> > Signed-off-by: Stefan Tauner <stefan.tau...@alumni.tuwien.ac.at>
> > Acked-by: Stefan Tauner <stefan.tau...@alumni.tuwien.ac.at>
> > ---
> > Makefile | 34 ++++++++++++++---------------
> > README | 6 ++++++
> > board_enable.c | 2 +-
> > flashchips.c | 66
> > ++++++++++++++++++++++++++++-----------------------------
> > flashrom.8.tmpl | 38 ++++++++++++++++++---------------
> > pickit2_spi.c | 2 +-
> > print.c | 4 ++--
> > sb600spi.c | 2 +-
> > 8 files changed, 82 insertions(+), 72 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 927105d..d9a70b3 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -661,22 +661,22 @@ override CONFIG_CH341A_SPI = no
> > override CONFIG_DEDIPROG = no
> > endif
> > ifeq ($(CONFIG_ENABLE_LIBPCI_PROGRAMMERS), no)
> > -override CONFIG_INTERNAL = no
> > -override CONFIG_NIC3COM = no
> > -override CONFIG_GFXNVIDIA = no
> > -override CONFIG_SATASII = no
> > -override CONFIG_ATAHPT = no
> > -override CONFIG_ATAVIA = no
> > -override CONFIG_ATAPROMISE = no
> > -override CONFIG_IT8212 = no
> > -override CONFIG_DRKAISER = no
> > -override CONFIG_NICREALTEK = no
> > -override CONFIG_NICNATSEMI = no
> > -override CONFIG_NICINTEL = no
> > -override CONFIG_NICINTEL_SPI = no
> > -override CONFIG_NICINTEL_EEPROM = no
> > -override CONFIG_OGP_SPI = no
> > -override CONFIG_SATAMV = no
> > +override CONFIG_INTERNAL = no
> > +override CONFIG_NIC3COM = no
> > +override CONFIG_GFXNVIDIA = no
> > +override CONFIG_SATASII = no
> > +override CONFIG_ATAHPT = no
> > +override CONFIG_ATAVIA = no
> > +override CONFIG_ATAPROMISE = no
> > +override CONFIG_IT8212 = no
> > +override CONFIG_DRKAISER = no
> > +override CONFIG_NICREALTEK = no
> > +override CONFIG_NICNATSEMI = no
> > +override CONFIG_NICINTEL = no
> > +override CONFIG_NICINTEL_SPI = no
> > +override CONFIG_NICINTEL_EEPROM = no
> > +override CONFIG_OGP_SPI = no
> > +override CONFIG_SATAMV = no
>
> Zero changes except whitespace. The changelog claims the overrides changed.
No that's not related I think. The whitespace is something I noticed
but forgot in the review. The override was something Uwe/Debian has
problems with... see above/r1931.
>
> > endif
> >
> > # Bitbanging SPI infrastructure, default off unless needed.
> > diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl
> > index cb77e46..5de735b 100644
> > --- a/flashrom.8.tmpl
> > +++ b/flashrom.8.tmpl
> > @@ -1,9 +1,11 @@
> > .\" Load the www device when using groff; provide a fallback for groff's
> > MTO macro that formats email addresses.
> > .ie \n[.g] \
> > . mso www.tmac
> > -.el \
> > -. de MTO \\$2 \(la\\$1 \(ra\\$3
> > +.el \{
> > +. de MTO
> > + \\$2 \(la\\$1 \(ra\\$3 \
> > . .
> > +.\}
> > .\" Create wrappers for .MTO and .URL that print only text on systems w/o
> > groff or if not outputting to a HTML
> > .\" device. To that end we need to distinguish HTML output on groff from
> > other configurations first.
> > .nr groffhtml 0
>
> I have to trust you on this.
That one was found by lintian and reproduced locally (one has to enable
warnings explicitly for groff). I have also discussed that in #groff or
something and they said it was obviously an error. And I said: "Sure.
Obvious." and left :P
> > @@ -684,13 +680,18 @@ size (padding to 32 kB is required).
> > .IP
> > This is the first programmer module in flashrom that does not provide
> > access to NOR flash chips but EEPROMs
> > mounted on gigabit Ethernet cards based on Intel's 82580 NIC. Because
> > EEPROMs normally do not announce their
> > -size nor allow to be identified, the controller relies on correct size
> > values written to predefined addresses
> > -within the chip. Flashrom follows this scheme but assumes the minimum size
> > of 16 kB (128 kb) if an unprogrammed
> > -EEPROM/card is detected. Intel specifies following EEPROMs to be
> > compatible: Atmel AT25128, AT25256, Micron (ST)
> > -M95128, M95256 and OnSemi (Catalyst) CAT25CS128.
> > +size nor allow themselves to be identified, the controller relies on
> > correct size values written to predefined
> > +addresses within the chip. Flashrom follows this scheme but assumes the
> > minimum size of 16 kB (128 kb) if an
> > +unprogrammed EEPROM/card is detected. Intel specifies following EEPROMs to
> > be compatible:
> > +Atmel AT25128, AT25256, Micron (ST) M95128, M95256 and OnSemi (Catalyst)
> > CAT25CS128.
>
> Sorry, -ENOPARSE. Does this mean we try to determine chip size by
> reading predefined locations of the EEPROM where size is customarily stored?
We don't - the hardware does and reports it back to us. This was a
simple grammar fix (found by and required for lintian).
> > .SS
> > .BR "ft2232_spi " programmer
> > .IP
> > +This module supports various programmers based on FTDI
> > FT2232/FT4232H/FT232H chips including the DLP Design
> > +DLP-USB1232H, openbiosprog-spi, Amontec JTAGkey/JTAGkey-tiny/JTAGkey-2,
> > Dangerous Prototypes Bus Blaster,
> > +Olimex ARM-USB-TINY/-H, Olimex ARM-USB-OCD/-H, OpenMoko Neo1973 Debug
> > board (V2+), TIAO/DIYGADGET USB
> > +Multi-Protocol Adapter (TUMPA), TUMPA Lite, GOEPEL PicoTAP and Google
> > Servo v1/v2.
> > +.sp
> > An optional parameter specifies the controller
> > type and channel/interface/port it should support. For that you have to
> > use the
> > .sp
> > @@ -981,7 +985,7 @@ Please note that the linux_spi driver only works on
> > Linux.
> > .BR "mstarddc_spi " programmer
> > .IP
> > The Display Data Channel (DDC) is an I2C bus present on VGA and DVI
> > connectors, that allows exchanging
> > -informations between a computer and attached displays. Its most common
> > uses are getting display capabilities
> > +information between a computer and attached displays. Its most common uses
> > are getting display capabilities
> > through EDID (at I2C address 0x50) and sending commands to the display
> > using the DDC/CI protocol (at address
> > 0x37). On displays driven by MSTAR SoCs, it is also possible to access the
> > SoC firmware flash (connected to
> > the Soc through another SPI bus) using an In-System Programming (ISP)
> > port, usually at address 0x49.
> > @@ -1016,7 +1020,7 @@ Example that does not reset the display at the end of
> > the operation:
> > .sp
> > .B " flashrom \-p mstarddc_spi:dev=/dev/i2c-1:49,noreset=1
> > .sp
> > -Please note that sending the reset command is also inhibited in the event
> > an error occured during the operation.
> > +Please note that sending the reset command is also inhibited in the event
> > an error occurred during the operation.
>
> Please note that sending the reset command is also inhibited if an error
> occurred during the operation.
k
> > To send the reset command afterwards, you can simply run flashrom once
> > more, in chip probe mode (not specifying
> > an operation), without the
> > .B noreset
> > diff --git a/pickit2_spi.c b/pickit2_spi.c
> > index f1f60a2..f6aa676 100644
> > --- a/pickit2_spi.c
> > +++ b/pickit2_spi.c
> > @@ -400,7 +400,7 @@ static int pickit2_shutdown(void *data)
> >
> > int pickit2_spi_init(void)
> > {
> > - unsigned int usedevice = 0; // FIXME: allow to select one of multiple
> > devices
> > + unsigned int usedevice = 0; // FIXME: allows one to select one of
> > multiple devices
>
> The original sentence has grammar problems, but I wouldn't endorse the
> replacement either.
> "Allow selecting one of multiple devices" would have correct grammar and
> style.
I couldn't care less... :) but this is required to make lintian shut up.
Changed to your suggestion.
--
Kind regards/Mit freundlichen Grüßen, Stefan Tauner
_______________________________________________
flashrom mailing list
flashrom@flashrom.org
https://www.flashrom.org/mailman/listinfo/flashrom
