Change in coreboot[master]: drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37043 )

Change subject: drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support
......................................................................

drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support

Change-Id: I81a610a6d119745f2fc637629b8ba7ade76503bc
Signed-off-by: Arthur Heymans 
---
M src/drivers/spi/flashconsole.c
1 file changed, 27 insertions(+), 45 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/37043/1

diff --git a/src/drivers/spi/flashconsole.c b/src/drivers/spi/flashconsole.c
index 8874812..d98bb4a 100644
--- a/src/drivers/spi/flashconsole.c
+++ b/src/drivers/spi/flashconsole.c
@@ -11,7 +11,6 @@
  * GNU General Public License for more details.
  */

-#include <arch/early_variables.h>
 #include <commonlib/region.h>
 #include <boot_device.h>
 #include <fmap.h>
@@ -23,26 +22,25 @@
 #define LINE_BUFFER_SIZE 128
 #define READ_BUFFER_SIZE 0x100

-static const struct region_device *g_rdev_ptr CAR_GLOBAL;
-static struct region_device g_rdev CAR_GLOBAL;
-static uint8_t g_line_buffer[LINE_BUFFER_SIZE] CAR_GLOBAL;
-static size_t g_offset CAR_GLOBAL;
-static size_t g_line_offset CAR_GLOBAL;
+static const struct region_device *g_rdev_ptr;
+static struct region_device g_rdev;
+static uint8_t g_line_buffer[LINE_BUFFER_SIZE];
+static size_t g_offset;
+static size_t g_line_offset;

void flashconsole_init(void)
 {
-	struct region_device *rdev = car_get_var_ptr(&g_rdev);
    uint8_t buffer[READ_BUFFER_SIZE];
    size_t size;
    size_t offset = 0;
    size_t len = READ_BUFFER_SIZE;
    size_t i;

-	if (fmap_locate_area_as_rdev_rw("CONSOLE", rdev)) {
+	if (fmap_locate_area_as_rdev_rw("CONSOLE", &g_rdev)) {
    	printk(BIOS_INFO, "Can't find 'CONSOLE' area in FMAP\n");
    	return;
    }
-	size = region_device_sz(rdev);
+	size = region_device_sz(&g_rdev);

/*
     * We need to check the region until we find a 0xff indicating
@@ -58,7 +56,7 @@
    	// Fill the buffer on first iteration
    	if (i == 0) {
    		len = min(READ_BUFFER_SIZE, size - offset);
-			if (rdev_readat(rdev, buffer, offset, len) != len)
+			if (rdev_readat(&g_rdev, buffer, offset, len) != len)
    			return;
    	}
    	if (buffer[i] == 0xff) {
@@ -77,65 +75,49 @@
    	return;
    }

-	car_set_var(g_offset, offset);
-	/* Set g_rdev_ptr last so tx_byte doesn't get executed early */
-	car_set_var(g_rdev_ptr, rdev);
+	g_offset = offset;
+	g_rdev_ptr = &g_rdev;
 }

void flashconsole_tx_byte(unsigned char c)
 {
-	const struct region_device *rdev = car_get_var(g_rdev_ptr);
-	uint8_t *line_buffer;
-	size_t offset;
-	size_t len;
-	size_t region_size;
+	size_t region_size = region_device_sz(g_rdev_ptr);

-	if (!rdev)
-		return;
+	g_line_buffer[g_line_offset++] = c;

-	line_buffer = car_get_var_ptr(g_line_buffer);
-	offset = car_get_var(g_offset);
-	len = car_get_var(g_line_offset);
-	region_size = region_device_sz(rdev);
-
-	line_buffer[len++] = c;
-	car_set_var(g_line_offset, len);
-
-	if (len >= LINE_BUFFER_SIZE ||
-		offset + len >= region_size || c == '\n') {
+	if (g_line_offset >= LINE_BUFFER_SIZE ||
+	    g_offset + g_line_offset >= region_size || c == '\n') {
    	flashconsole_tx_flush();
    }
 }

void flashconsole_tx_flush(void)
 {
-	const struct region_device *rdev = car_get_var(g_rdev_ptr);
-	uint8_t *line_buffer = car_get_var_ptr(g_line_buffer);
-	size_t offset = car_get_var(g_offset);
-	size_t len = car_get_var(g_line_offset);
+	size_t offset = g_offset;
+	size_t len = g_line_offset;
    size_t region_size;
-
-	if (!rdev)
-		return;
+	static int busy;

/* Prevent any recursive loops in case the spi flash driver
     * calls printk (in case of transaction timeout or
     * any other error while writing) */
-	car_set_var(g_rdev_ptr, NULL);
+	if (busy)
+		return;

-	region_size = region_device_sz(rdev);
+	busy = 1;
+	region_size = region_device_sz(g_rdev_ptr);
    if (offset + len >= region_size)
    	len = region_size - offset;

-	if (rdev_writeat(rdev, line_buffer, offset, len) != len)
-		rdev = NULL;
+	if (rdev_writeat(&g_rdev, g_line_buffer, offset, len) != len)
+		return;

// If the region is full, stop future write attempts
    if (offset + len >= region_size)
-		rdev = NULL;
+	        return;

-	car_set_var(g_offset, offset + len);
-	car_set_var(g_line_offset, 0);
+	g_offset = offset + len;
+	g_line_offset = 0;

-	car_set_var(g_rdev_ptr, rdev);
+	busy = 0;
 }

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37043 )

Change subject: drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37043/1/src/drivers/spi/flashconsol... 
File src/drivers/spi/flashconsole.c:

https://review.coreboot.org/c/coreboot/+/37043/1/src/drivers/spi/flashconsol... 
PS1, Line 117: 	        return;
code indent should use tabs where possible

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37043 )

Change subject: drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37043/2/src/drivers/spi/flashconsol... 
File src/drivers/spi/flashconsole.c:

https://review.coreboot.org/c/coreboot/+/37043/2/src/drivers/spi/flashconsol... 
PS2, Line 117: 	        return;
code indent should use tabs where possible

[Arthur Heymans (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37043

to look at the new patch set (#3).

Change subject: drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support
......................................................................

drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support

Change-Id: I81a610a6d119745f2fc637629b8ba7ade76503bc
Signed-off-by: Arthur Heymans 
---
M src/drivers/spi/flashconsole.c
1 file changed, 27 insertions(+), 45 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/37043/3

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37043 )

Change subject: drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support
......................................................................

Patch Set 7: Code-Review+1

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37043 )

Change subject: drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support
......................................................................

Patch Set 7: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37043 )

Change subject: drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37043/7/src/drivers/spi/flashconsol... 
File src/drivers/spi/flashconsole.c:

https://review.coreboot.org/c/coreboot/+/37043/7/src/drivers/spi/flashconsol... 
PS7, Line 93: 	if (!rdev)
            : 		return;
g_rdev_ptr may be null (before flashconsole_init), so leave a test like this in?

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/37043 )

Change subject: drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support
......................................................................

Removed Code-Review+2 by Kyösti Mälkki

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37043 )

Change subject: drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support
......................................................................

Patch Set 8:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37043/7/src/drivers/spi/flashconsol... 
File src/drivers/spi/flashconsole.c:

https://review.coreboot.org/c/coreboot/+/37043/7/src/drivers/spi/flashconsol... 
PS7, Line 93: 	if (!rdev)
            : 		return;
> g_rdev_ptr may be null (before flashconsole_init), so leave a test like this in?

See CB:37375

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has uploaded a new patch set (#9) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/37043 )

Change subject: drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support
......................................................................

drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support

Change-Id: I81a610a6d119745f2fc637629b8ba7ade76503bc
Signed-off-by: Arthur Heymans 
---
M src/drivers/spi/flashconsole.c
1 file changed, 31 insertions(+), 43 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/37043/9

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37043 )

Change subject: drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support
......................................................................

Patch Set 9: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/37043/7/src/drivers/spi/flashconsol... 
File src/drivers/spi/flashconsole.c:

https://review.coreboot.org/c/coreboot/+/37043/7/src/drivers/spi/flashconsol... 
PS7, Line 93: 	if (!rdev)
            : 		return;
> See CB:37375

Done

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37043 )

Change subject: drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support
......................................................................

Patch Set 9: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37043 )

Change subject: drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support
......................................................................

drivers/spi/flashconsole.c: Drop CAR_GLOBAL_MIGRATION support

Change-Id: I81a610a6d119745f2fc637629b8ba7ade76503bc
Signed-off-by: Arthur Heymans 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37043
Tested-by: build bot (Jenkins) 
Reviewed-by: Kyösti Mälkki 
Reviewed-by: Angel Pons 
---
M src/drivers/spi/flashconsole.c
1 file changed, 31 insertions(+), 43 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Kyösti Mälkki: Looks good to me, but someone else must approve
  Angel Pons: Looks good to me, approved

diff --git a/src/drivers/spi/flashconsole.c b/src/drivers/spi/flashconsole.c
index 8874812..c56bf52 100644
--- a/src/drivers/spi/flashconsole.c
+++ b/src/drivers/spi/flashconsole.c
@@ -11,7 +11,6 @@
  * GNU General Public License for more details.
  */

-#include <arch/early_variables.h>
 #include <commonlib/region.h>
 #include <boot_device.h>
 #include <fmap.h>
@@ -23,26 +22,25 @@
 #define LINE_BUFFER_SIZE 128
 #define READ_BUFFER_SIZE 0x100

-static const struct region_device *g_rdev_ptr CAR_GLOBAL;
-static struct region_device g_rdev CAR_GLOBAL;
-static uint8_t g_line_buffer[LINE_BUFFER_SIZE] CAR_GLOBAL;
-static size_t g_offset CAR_GLOBAL;
-static size_t g_line_offset CAR_GLOBAL;
+static const struct region_device *g_rdev_ptr;
+static struct region_device g_rdev;
+static uint8_t g_line_buffer[LINE_BUFFER_SIZE];
+static size_t g_offset;
+static size_t g_line_offset;

void flashconsole_init(void)
 {
-	struct region_device *rdev = car_get_var_ptr(&g_rdev);
    uint8_t buffer[READ_BUFFER_SIZE];
    size_t size;
    size_t offset = 0;
    size_t len = READ_BUFFER_SIZE;
    size_t i;

-	if (fmap_locate_area_as_rdev_rw("CONSOLE", rdev)) {
+	if (fmap_locate_area_as_rdev_rw("CONSOLE", &g_rdev)) {
    	printk(BIOS_INFO, "Can't find 'CONSOLE' area in FMAP\n");
    	return;
    }
-	size = region_device_sz(rdev);
+	size = region_device_sz(&g_rdev);

/*
     * We need to check the region until we find a 0xff indicating
@@ -58,7 +56,7 @@
    	// Fill the buffer on first iteration
    	if (i == 0) {
    		len = min(READ_BUFFER_SIZE, size - offset);
-			if (rdev_readat(rdev, buffer, offset, len) != len)
+			if (rdev_readat(&g_rdev, buffer, offset, len) != len)
    			return;
    	}
    	if (buffer[i] == 0xff) {
@@ -77,65 +75,55 @@
    	return;
    }

-	car_set_var(g_offset, offset);
-	/* Set g_rdev_ptr last so tx_byte doesn't get executed early */
-	car_set_var(g_rdev_ptr, rdev);
+	g_offset = offset;
+	g_rdev_ptr = &g_rdev;
 }

void flashconsole_tx_byte(unsigned char c)
 {
-	const struct region_device *rdev = car_get_var(g_rdev_ptr);
-	uint8_t *line_buffer;
-	size_t offset;
-	size_t len;
-	size_t region_size;
-
-	if (!rdev)
+	if (!g_rdev_ptr)
    	return;

-	line_buffer = car_get_var_ptr(g_line_buffer);
-	offset = car_get_var(g_offset);
-	len = car_get_var(g_line_offset);
-	region_size = region_device_sz(rdev);
+	size_t region_size = region_device_sz(g_rdev_ptr);

-	line_buffer[len++] = c;
-	car_set_var(g_line_offset, len);
+	g_line_buffer[g_line_offset++] = c;

-	if (len >= LINE_BUFFER_SIZE ||
-		offset + len >= region_size || c == '\n') {
+	if (g_line_offset >= LINE_BUFFER_SIZE ||
+	    g_offset + g_line_offset >= region_size || c == '\n') {
    	flashconsole_tx_flush();
    }
 }

void flashconsole_tx_flush(void)
 {
-	const struct region_device *rdev = car_get_var(g_rdev_ptr);
-	uint8_t *line_buffer = car_get_var_ptr(g_line_buffer);
-	size_t offset = car_get_var(g_offset);
-	size_t len = car_get_var(g_line_offset);
+	size_t offset = g_offset;
+	size_t len = g_line_offset;
    size_t region_size;
-
-	if (!rdev)
-		return;
+	static int busy;

/* Prevent any recursive loops in case the spi flash driver
     * calls printk (in case of transaction timeout or
     * any other error while writing) */
-	car_set_var(g_rdev_ptr, NULL);
+	if (busy)
+		return;

-	region_size = region_device_sz(rdev);
+	if (!g_rdev_ptr)
+		return;
+
+	busy = 1;
+	region_size = region_device_sz(g_rdev_ptr);
    if (offset + len >= region_size)
    	len = region_size - offset;

-	if (rdev_writeat(rdev, line_buffer, offset, len) != len)
-		rdev = NULL;
+	if (rdev_writeat(&g_rdev, g_line_buffer, offset, len) != len)
+		return;

// If the region is full, stop future write attempts
    if (offset + len >= region_size)
-		rdev = NULL;
+		return;

-	car_set_var(g_offset, offset + len);
-	car_set_var(g_line_offset, 0);
+	g_offset = offset + len;
+	g_line_offset = 0;

-	car_set_var(g_rdev_ptr, rdev);
+	busy = 0;
 }