[flashrom] [PATCH] Add a bunch of new/tested stuff and various small changes 7

Stefan Tauner stefan.tauner at student.tuwien.ac.at
Fri Sep 2 00:16:37 CEST 2011


On Thu, 1 Sep 2011 23:20:26 +0200
Uwe Hermann <uwe at hermann-uwe.de> wrote:

> Hi,
> 
> Acked-by: Uwe Hermann <uwe at hermann-uwe.de>
> 
> but please see below for a review.

thanks! i would have preferred a review for one of the other
quadrillion patches, but thanks for your effort :)

> On Thu, Sep 01, 2011 at 01:37:48AM +0200, Stefan Tauner wrote:
> > - add P7H55-M LX to the list of supported boards
> >   although flashrom works correctly, it is marked as not ok, because flashing the
> >   vendor image will break the LAN interface.
> 
> Hm, dunno if this is what we want to do, the "board loses MAC address"
> issue happens for other boards too and is not strictly flashrom related.
> 
> Feel free to commit, that should probably be another thread discussing
> how we want to handle the "board loses MAC address" cases.

id like to add a "missing board enable" tag there anyway... a "mostly
working" category might be useful for this kind of behavior. for now i
think some inconsistency is great... more pressure to fix it in the
future! :P

> 
> > - add Zotac GeForce 8200 to the list of supported boards
> 
> "ZOTAC" as per vendor website, they use that spelling pretty
> consistently there (you correctly used "ZOTAC" in the source code already).

and there is the only place it matters imho, fixed anyway
 
> > - rename some chips that had gratuitous "probing" suffixes:
> >   - SST25VF010.REMS
> >   - SST25VF040.REMS
> >   - M25P05.RES
> >   - M25P10.RES
> 
> Please mention the reason / mail / URL for that here.

the reason is stated above: they are not needed :)
i have added this explanation(?):
  some other chip names with suffixes are needed due to lack of support
  for multiple probe functions per chip. this is explained here:
  http://www.flashrom.org/pipermail/flashrom/2011-August/007597.html

> > +	B("Shuttle",	"FH67",			1, "http://www.shuttle.eu/products/mini-pc/sh67h3/specification/", NULL),
>                                                                                                                    ^^^^
>                                                                                   I'd add "Used in the Shuttle SH67H3 barebone." (or similar) here.
> 
> And/or make the board name "FH67 (SH67H3)", but that's probably not
> such a good idea.

refrained from that due to consistency reasons. the other shuttle
boards are like the one i have added. something that should be changed,
but not by me. :)

> > +	B("Tyan",	"S2912 (Thunder n3600R)", 1, "http://www.tyan.com/product_board_detail.aspx?pid=157", NULL),
>                                                                                                               ^^^^
>                                                      Maybe add "Tested on the S2912G2NR model (without SAS)." or similar.

does not justify a comment imo. we dont comment on working boards under
normal circumstances in the wiki. what might be nice is a new field
with the url to the report. in the commit log it is ok, but having it in
the source might be better (it would not be included in normal builds).
i would prefer such a link to the vendor page links actually ;)


everything else is fixed
-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner




More information about the flashrom mailing list