[PATCH v2] serprog: Fix FWH/LPC by implementing serprog_map

[Urja Rannikko]

also make serprog_map report if mapping doesnt fit
in the 24-bit address space of serprog.

Signed-off-by: Urja Rannikko <[e-mail]>
---
The previous version didnt compile on 64-bit 
because size_t and -Werror ... thus v2.

---
 flashrom.c   |  2 +-
 programmer.h |  1 +
 serprog.c    | 12 ++++++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/flashrom.c b/flashrom.c
index a389cb2..d51a44c 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -212,7 +212,7 @@ const struct programmer_entry programmer_table[] = {
 					/* FIXME */
 		.devs.note		= "All programmer devices speaking the serprog protocol\n",
 		.init			= serprog_init,
-		.map_flash_region	= fallback_map,
+		.map_flash_region	= serprog_map,
 		.unmap_flash_region	= fallback_unmap,
 		.delay			= serprog_delay,
 	},
diff --git a/programmer.h b/programmer.h
index 913522b..3bf292d 100644
--- a/programmer.h
+++ b/programmer.h
@@ -706,6 +706,7 @@ int register_master(const struct registered_master *mst);
 #if CONFIG_SERPROG == 1
 int serprog_init(void);
 void serprog_delay(unsigned int usecs);
+void *serprog_map(const char *descr, uintptr_t phys_addr, size_t len);
 #endif
 
 /* serial.c */
diff --git a/serprog.c b/serprog.c
index 3de0182..8e4286a 100644
--- a/serprog.c
+++ b/serprog.c
@@ -943,3 +943,15 @@ static int serprog_spi_read(struct flashctx *flash, uint8_t *buf,
 	}
 	return 0;
 }
+
+void *serprog_map(const char *descr, uintptr_t phys_addr, size_t len)
+{
+	if ((phys_addr & 0xFF000000) == 0xFF000000) {
+		/* This is normal, no need to report anything. */
+		return (void*)phys_addr;
+	} else {
+		msg_pwarn(MSGHEADER "incompatible mapping %s, 0x%zx bytes at 0x%0*" PRIxPTR ", returning NULL\n",
+			descr, len, PRIxPTR_WIDTH, phys_addr);
+		return NULL;
+	}
+}
-- 
2.3.1

[PATCH v2] serprog: Fix FWH/LPC by implementing serprog_map

[Stefan Tauner]

On Sun, 8 Mar 2015 07:54:11 +0000
Urja Rannikko <[e-mail]> wrote:
> also make serprog_map report if mapping doesnt fit
> in the 24-bit address space of serprog.

Hi,

what exactly happens without this fake mapping? I think I don't
completely understand how non-spi serprog handles chip addresses...

BTW the documentation on non-spi stuff especially the opbuf handling in
Documentation/serprog-protocol.txt has some room for improvements ;)

-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner

[Urja Rannikko]

On Mon, Jun 29, 2015 at 2:38 AM, Stefan Tauner
<[e-mail]> wrote:
> On Sun, 8 Mar 2015 07:54:11 +0000
> Urja Rannikko <[e-mail]> wrote:
> > also make serprog_map report if mapping doesnt fit
> > in the 24-bit address space of serprog.
>
> Hi,
>
> what exactly happens without this fake mapping? I think I don't
> completely understand how non-spi serprog handles chip addresses...

For parallel it was essentially that address bits past the chip size
wouldnt be connected in hardware since chips dont have(/look at) more
address lines than their size. So the chip would always be addressed and the
address space would be as many mirrors of the chip as would fit (in 24bit/16M).

For LPC/FWH they have a real 32/28-bit address space, within
which the chip will natively be mapped to the highest addresses (at 4G
- chip size)
 - below which they can have lock regs etc.
The serprog (libfrser) LPC/FWH maps the 24-bit address
space to the highest 16M of the 4G space since thats where the chips are.
If you used the null mapping and tried to access eg addr 0 of 512k LPC
chip, it'd
try to access 0xFF000000 instead of 4G - 512k = 0xFFF80000.
The chip would just ignore the accesses since that isnt the address it
listens to.
Also, the lock regs would get the same null mapping and nope.

Essentially this is a bug in the original serprog because the
mapping just wasnt thought about at the time (worked without).

I think it is also a bug of fallback_map in returning NULL always
- that does the lock regs and chip at same address so it just cant
work for all things,
but I'm not sure if some programmer relies on that by now,
and serprog has an use for a map function: reporting about the attempted
mappings if they're outside the 24bit space it has.
> BTW the documentation on non-spi stuff especially the opbuf handling in
> Documentation/serprog-protocol.txt has some room for improvements ;)

I'll look at this later.

-- 
Urja Rannikko