[PATCH 1/1] Introduce ERROR_FATAL in chipset enables

[Tadas Slotkus]

Currently we are continuing even if chipset enable has failed.
Some chipset enables do exits, so to kill those exits I have
prepared one sneaky check :) Errors are negative there, so I
have chosen this constant.

Signed-off-by: Tadas Slotkus 
---
 chipset_enable.c |    4 ++++
 flash.h          |    3 ++-
 internal.c       |    3 ++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/chipset_enable.c b/chipset_enable.c
index 687682a..f6c5d81 100644
--- a/chipset_enable.c
+++ b/chipset_enable.c
@@ -1289,6 +1289,10 @@ int chipset_flash_enable(void)
    		msg_pinfo("OK.\n");
    	else if (ret == ERROR_NONFATAL)
    		msg_pinfo("PROBLEMS, continuing anyway\n");
+		else if (ret == ERROR_FATAL) {
+			msg_perr("FATAL ERROR!\n")
+			return ret;
+		}
    }

s = flashbuses_to_text(buses_supported);
diff --git a/flash.h b/flash.h
index 3d47a16..7386ecb 100644
--- a/flash.h
+++ b/flash.h
@@ -224,8 +224,9 @@ int write_buf_to_file(unsigned char *buf, unsigned long size, const char *filena
 #define OK 0
 #define NT 1    /* Not tested */

-/* Something happened that shouldn't happen, but we can go on */
+/* Something happened that shouldn't happen, but we can go on, or not */
 #define ERROR_NONFATAL 0x100
+#define ERROR_FATAL -0xEE

/* cli_output.c */
 /* Let gcc and clang check for correct printf-style format strings. */
diff --git a/internal.c b/internal.c
index 846cf1e..46836b7 100644
--- a/internal.c
+++ b/internal.c
@@ -268,7 +268,8 @@ int internal_init(void)
    if (ret == -2) {
    	msg_perr("WARNING: No chipset found. Flash detection "
    		 "will most likely fail.\n");
-	}
+	} else if (ret == ERROR_FATAL)
+		return ret;

#if defined(__i386__) || defined(__x86_64__)
    /* Probe unconditionally for IT87* LPC->SPI translation and for

[PATCH 1/2] Kill exit() in enable_flash_sb400

[Tadas Slotkus]

Replace it by newly created return mechanism.

Signed-off-by: Tadas Slotkus 
---
 chipset_enable.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/chipset_enable.c b/chipset_enable.c
index f6c5d81..ee005af 100644
--- a/chipset_enable.c
+++ b/chipset_enable.c
@@ -861,7 +861,7 @@ static int enable_flash_sb400(struct pci_dev *dev, const char *name)

if (!smbusdev) {
    	msg_perr("ERROR: SMBus device not found. Aborting.\n");
-		exit(1);
+		return ERROR_FATAL;
    }

/* Enable some SMBus stuff. */

[PATCH 2/2] Kill exit() in enable_flash_ich_dc

[Tadas Slotkus]

Replace it by newly created return mechanism.

Signed-off-by: Tadas Slotkus 
---
 chipset_enable.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/chipset_enable.c b/chipset_enable.c
index ee005af..ca4f245 100644
--- a/chipset_enable.c
+++ b/chipset_enable.c
@@ -343,10 +343,7 @@ static int enable_flash_ich_dc(struct pci_dev *dev, const char *name)
    	msg_perr("Error: fwh_idsel= specified, but no value given.\n");
 idsel_garbage_out:	
    	free(idsel);
-		/* FIXME: Return failure here once internal_init() starts
-		 * to care about the return value of the chipset enable.
-		 */
-		exit(1);
+		return ERROR_FATAL;
    }
    free(idsel);

@@ -468,6 +465,8 @@ static int enable_flash_ich_dc_spi(struct pci_dev *dev, const char *name,

/* Enable Flash Writes */
    ret = enable_flash_ich_dc(dev, name);
+	if (ret == ERROR_FATAL)
+		return ret;

/* Get physical address of Root Complex Register Block */
    tmp = pci_read_long(dev, 0xf0) & 0xffffc000;

[PATCH 1/2] Kill exit() in enable_flash_sb400

[Tadas Slotkus]

> Replace it by newly created return mechanism.
> Signed-off-by: Tadas Slotkus
>
> chipset_enable.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/chipset_enable.c b/chipset_enable.c
> index f6c5d81..ee005af 100644
> --- a/chipset_enable.c
> +++ b/chipset_enable.c
> @@ -861,7 +861,7 @@ static int enable_flash_sb400(struct pci_dev *dev, const char *name)
>
> if (!smbusdev) {
>    	msg_perr("ERROR: SMBus device not found. Aborting.\n");
> exit(1);
> return ERROR_FATAL;
>
> }
>
> /* Enable some SMBus stuff. */

Thanks to Uwe, this is now applicable for review.

[PATCH 1/2] Kill exit() in enable_flash_sb400

[Uwe Hermann]

On Sun, Sep 04, 2011 at 05:30:27PM +0300, Tadas Slotkus wrote:
> > exit(1);
> > return ERROR_FATAL;
> >
> > }
> >
> > /* Enable some SMBus stuff. */
>
> Thanks to Uwe, this is now applicable for review.

Thanks, r1429 together with [PATCH 2/2].

Uwe.

[PATCH] Introduce ERROR_FATAL in chipset enables[updated]

[Tadas Slotkus]

Currently we are continuing even if chipset enable has failed.
Some chipset enables do exits, so to kill those exits I have
prepared one sneaky check :) Errors are negative there, so I
have chosen this constant.

Signed-off-by: Tadas Slotkus 
---
 chipset_enable.c |    4 ++++
 flash.h          |    3 ++-
 internal.c       |    3 ++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/chipset_enable.c b/chipset_enable.c
index 687682a..bdc3b17 100644
--- a/chipset_enable.c
+++ b/chipset_enable.c
@@ -1289,6 +1289,10 @@ int chipset_flash_enable(void)
    		msg_pinfo("OK.\n");
    	else if (ret == ERROR_NONFATAL)
    		msg_pinfo("PROBLEMS, continuing anyway\n");
+		else if (ret == ERROR_FATAL) {
+			msg_perr("FATAL ERROR!\n");
+			return ret;
+		}
    }

s = flashbuses_to_text(buses_supported);
diff --git a/flash.h b/flash.h
index 3d47a16..7386ecb 100644
--- a/flash.h
+++ b/flash.h
@@ -224,8 +224,9 @@ int write_buf_to_file(unsigned char *buf, unsigned long size, const char *filena
 #define OK 0
 #define NT 1    /* Not tested */

-/* Something happened that shouldn't happen, but we can go on */
+/* Something happened that shouldn't happen, but we can go on, or not */
 #define ERROR_NONFATAL 0x100
+#define ERROR_FATAL -0xEE

/* cli_output.c */
 /* Let gcc and clang check for correct printf-style format strings. */
diff --git a/internal.c b/internal.c
index 846cf1e..46836b7 100644
--- a/internal.c
+++ b/internal.c
@@ -268,7 +268,8 @@ int internal_init(void)
    if (ret == -2) {
    	msg_perr("WARNING: No chipset found. Flash detection "
    		 "will most likely fail.\n");
-	}
+	} else if (ret == ERROR_FATAL)
+		return ret;

#if defined(__i386__) || defined(__x86_64__)
    /* Probe unconditionally for IT87* LPC->SPI translation and for

[PATCH 1/1] Introduce ERROR_FATAL in chipset enables[now really updated]

[Tadas Slotkus]

Currently we are continuing even if chipset enable has failed.
Some chipset enables do exits, so to kill those exits I have
prepared one sneaky check :) Errors are negative there, so I
have chosen this constant.

Signed-off-by: Tadas Slotkus 
---
 chipset_enable.c |    4 ++++
 flash.h          |    3 ++-
 internal.c       |    3 ++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/chipset_enable.c b/chipset_enable.c
index 687682a..519c55a 100644
--- a/chipset_enable.c
+++ b/chipset_enable.c
@@ -1289,6 +1289,10 @@ int chipset_flash_enable(void)
    		msg_pinfo("OK.\n");
    	else if (ret == ERROR_NONFATAL)
    		msg_pinfo("PROBLEMS, continuing anyway\n");
+		if (ret == ERROR_FATAL) {
+			msg_perr("FATAL ERROR!\n");
+			return ret;
+		}
    }

s = flashbuses_to_text(buses_supported);
diff --git a/flash.h b/flash.h
index 3d47a16..7386ecb 100644
--- a/flash.h
+++ b/flash.h
@@ -224,8 +224,9 @@ int write_buf_to_file(unsigned char *buf, unsigned long size, const char *filena
 #define OK 0
 #define NT 1    /* Not tested */

-/* Something happened that shouldn't happen, but we can go on */
+/* Something happened that shouldn't happen, but we can go on, or not */
 #define ERROR_NONFATAL 0x100
+#define ERROR_FATAL -0xEE

/* cli_output.c */
 /* Let gcc and clang check for correct printf-style format strings. */
diff --git a/internal.c b/internal.c
index 846cf1e..46836b7 100644
--- a/internal.c
+++ b/internal.c
@@ -268,7 +268,8 @@ int internal_init(void)
    if (ret == -2) {
    	msg_perr("WARNING: No chipset found. Flash detection "
    		 "will most likely fail.\n");
-	}
+	} else if (ret == ERROR_FATAL)
+		return ret;

#if defined(__i386__) || defined(__x86_64__)
    /* Probe unconditionally for IT87* LPC->SPI translation and for

[PATCH 1/1] Introduce ERROR_FATAL in chipset enables[now really updated]

[Uwe Hermann]

On Mon, Aug 22, 2011 at 09:48:10AM +0300, Tadas Slotkus wrote:
> Currently we are continuing even if chipset enable has failed.
> Some chipset enables do exits, so to kill those exits I have
> prepared one sneaky check :) Errors are negative there, so I
> have chosen this constant.
>
> Signed-off-by: Tadas Slotkus

Thanks, r1426 with some small changes. Not sure if we'll keep -0xee as
error code, this is up for discussion and unification as soon as
libflashrom gets useful and consistent return codes, I guess.

Uwe.

[PATCH 1/1] Introduce ERROR_FATAL in chipset enables[now really updated]

[Carl-Daniel Hailfinger]

Am 03.09.2011 19:15 schrieb Uwe Hermann:
> On Mon, Aug 22, 2011 at 09:48:10AM +0300, Tadas Slotkus wrote:
> > Currently we are continuing even if chipset enable has failed.
> > Some chipset enables do exits, so to kill those exits I have
> > prepared one sneaky check :) Errors are negative there, so I
> > have chosen this constant.
> >
> > Signed-off-by: Tadas Slotkus
>
> Thanks, r1426 with some small changes. Not sure if we'll keep -0xee as
> error code, this is up for discussion and unification as soon as
> libflashrom gets useful and consistent return codes, I guess.

Ouch, -0xee is the worst error code I ever saw.
> Modified: trunk/chipset_enable.c
>
> --- trunk/chipset_enable.c	Sat Sep  3 13:22:27 2011	(r1425)
> +++ trunk/chipset_enable.c	Sat Sep  3 19:15:00 2011	(r1426)
> @@ -1325,6 +1325,10 @@
>    		msg_pinfo("OK.\n");
>    	else if (ret == ERROR_NONFATAL)
>    		msg_pinfo("PROBLEMS, continuing anyway\n");
> if (ret == ERROR_FATAL) {

Was not using else if here intentional?
> 	msg_perr("FATAL ERROR!\n");
> 	return ret;
> }
>
> }
>
> s = flashbuses_to_text(buses_supported);

Regards,
Carl-Daniel

[PATCH 1/1] Introduce ERROR_FATAL in chipset enables[now really updated]

[Tadas Slotkus]

> Am 03.09.2011 19:15 schrieb Uwe Hermann:
> > On Mon, Aug 22, 2011 at 09:48:10AM +0300, Tadas Slotkus wrote:
> > > Currently we are continuing even if chipset enable has failed.
> > > Some chipset enables do exits, so to kill those exits I have
> > > prepared one sneaky check :) Errors are negative there, so I
> > > have chosen this constant.
> > >
> > > Signed-off-by: Tadas Slotkus
> >
> > Thanks, r1426 with some small changes. Not sure if we'll keep -0xee as
> > error code, this is up for discussion and unification as soon as
> > libflashrom gets useful and consistent return codes, I guess.
>
> Ouch, -0xee is the worst error code I ever saw.

It may be. But aren't people familiar with negative numbers? :)
> > Modified: trunk/chipset_enable.c
> >
> > --- trunk/chipset_enable.c	Sat Sep  3 13:22:27 2011	(r1425)
> > +++ trunk/chipset_enable.c	Sat Sep  3 19:15:00 2011	(r1426)
> > @@ -1325,6 +1325,10 @@
> >    		msg_pinfo("OK.\n");
> >    	else if (ret == ERROR_NONFATAL)
> >    		msg_pinfo("PROBLEMS, continuing anyway\n");
> > if (ret == ERROR_FATAL) {
>
> Was not using else if here intentional?

I thought that both messages "FAILED!\n" and "FATAL ERROR!\n" should be
printed in this case.

It could be changed to positive, then that check should be left as "else
if".

Thanks for reviews,
Tadas