On Sat, 04 Jun 2011 11:37:27 +0200
Carl-Daniel Hailfinger <[email protected]> wrote:

> Am 04.06.2011 10:55 schrieb Stefan Tauner:
> > diff --git a/internal.c b/internal.c
> > index c9f62c1..bbcdc5b 100644
> > --- a/internal.c
> > +++ b/internal.c
> > @@ -124,7 +124,7 @@ int register_superio(struct superio s)
> >  
> >  #endif
> >  
> > -int is_laptop = 0;
> > +int is_laptop = -1;
> >   
> 
> This changes the logic. With this change, flashrom assumes that every
> machine is a laptop, and that will bite us everywhere dmidecode is
> unavailable.
> Please review your changes below in light of that.
yes. i forgot about the dependency on dmidecode.
your last sentence seems to indicate a problem with the hunk below, but
i dont see it (assuming that we leave is_laptop = 0 the default).

> >  int laptop_ok = 0;
> >  
> >  int internal_init(void)
> > @@ -222,11 +222,19 @@ int internal_init(void)
> >  
> >     /* Warn if a non-whitelisted laptop is detected. */
> >     if (is_laptop && !laptop_ok) {
> > -           
> > msg_perr("========================================================================\n"
> > -                    "WARNING! You seem to be running flashrom on an 
> > unsupported laptop.\n"
> > -                    "Laptops, notebooks and netbooks are difficult to 
> > support and we recommend\n"
> > -                    "to use the vendor flashing utility. The embedded 
> > controller (EC) in these\n"
> > -                    "machines often interacts badly with flashing.\n"
> > +           
> > msg_perr("========================================================================\n");
> > +           if (is_laptop == 1) {
> > +                   msg_perr("WARNING! You seem to be running flashrom on 
> > an unsupported laptop.\n");
> > +           } else {
> > +                   msg_perr("WARNING! You may be running flashrom on an 
> > unsupported laptop. We could\n"
> > +                            "not detect this for sure because your vendor 
> > has not setup the SMBIOS\n"
> > +                            "tables correctly. You can enforce execution 
> > by adding\n"
> > +                            "'-p internal:laptop=force_I_want_a_brick' to 
> > the command line, but\n"
> > +                            "please read the following warning if you are 
> > not sure.\n\n");
> > +           }
> > +           msg_perr("Laptops, notebooks and netbooks are difficult to 
> > support and we\n"
> > +                    "recommend to use the vendor flashing utility. The 
> > embedded controller\n"
> > +                    "(EC) in these machines often interacts badly with 
> > flashing.\n"
> >                      "See http://www.flashrom.org/Laptops for details.\n\n"
> >                      "If flash is shared with the EC, erase is guaranteed 
> > to brick your laptop\n"
> >                      "and write may brick your laptop.\n"
> > @@ -234,6 +242,7 @@ int internal_init(void)
> >                      "failure and sudden poweroff.\n"
> >                      "You have been warned.\n"
> >                      
> > "========================================================================\n");
> > +
> >             if (force_laptop) {
> >                     msg_perr("Proceeding anyway because user specified "
> >                              "laptop=force_I_want_a_brick\n");

rest is clear and will be dealt with.
thanks for the quick review.
-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner

_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to