* [Qemu-devel] [PATCH v5 1/9] ssi: change ssi_slave_init to be a realize ops
2016-06-28 18:24 [Qemu-devel] [PATCH v5 0/9] ast2400: SMC controllers Cédric Le Goater
@ 2016-06-28 18:24 ` Cédric Le Goater
2016-07-01 15:16 ` Peter Maydell
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 2/9] m25p80: do not put iovec on the stack Cédric Le Goater
` (8 subsequent siblings)
9 siblings, 1 reply; 33+ messages in thread
From: Cédric Le Goater @ 2016-06-28 18:24 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
Cédric Le Goater
This enables qemu to handle late inits and report errors. All the SSI
slave routine names were changed accordingly. Code was modified to
handle errors when possible (m25p80 and ssi-sd)
Tested with the m25p80 slave object.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/arm/spitz.c | 12 ++++--------
hw/arm/tosa.c | 5 ++---
hw/arm/z2.c | 6 ++----
hw/block/m25p80.c | 12 +++++-------
hw/display/ads7846.c | 5 ++---
hw/display/ssd0323.c | 5 ++---
hw/misc/max111x.c | 12 ++++++------
hw/sd/ssi-sd.c | 9 +++++----
hw/ssi/ssi.c | 6 +++---
include/hw/ssi/ssi.h | 2 +-
10 files changed, 32 insertions(+), 42 deletions(-)
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index ba40f8302bc6..41cc2eeeb1ba 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -598,15 +598,13 @@ static uint32_t spitz_lcdtg_transfer(SSISlave *dev, uint32_t value)
return 0;
}
-static int spitz_lcdtg_init(SSISlave *dev)
+static void spitz_lcdtg_realize(SSISlave *dev, Error **errp)
{
SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, dev);
spitz_lcdtg = s;
s->bl_power = 0;
s->bl_intensity = 0x20;
-
- return 0;
}
/* SSP devices */
@@ -666,7 +664,7 @@ static void spitz_adc_temp_on(void *opaque, int line, int level)
max111x_set_input(max1111, MAX1111_BATT_TEMP, 0);
}
-static int corgi_ssp_init(SSISlave *d)
+static void corgi_ssp_realize(SSISlave *d, Error **errp)
{
DeviceState *dev = DEVICE(d);
CorgiSSPState *s = FROM_SSI_SLAVE(CorgiSSPState, d);
@@ -675,8 +673,6 @@ static int corgi_ssp_init(SSISlave *d)
s->bus[0] = ssi_create_bus(dev, "ssi0");
s->bus[1] = ssi_create_bus(dev, "ssi1");
s->bus[2] = ssi_create_bus(dev, "ssi2");
-
- return 0;
}
static void spitz_ssp_attach(PXA2xxState *cpu)
@@ -1121,7 +1117,7 @@ static void corgi_ssp_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
- k->init = corgi_ssp_init;
+ k->realize = corgi_ssp_realize;
k->transfer = corgi_ssp_transfer;
dc->vmsd = &vmstate_corgi_ssp_regs;
}
@@ -1150,7 +1146,7 @@ static void spitz_lcdtg_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
- k->init = spitz_lcdtg_init;
+ k->realize = spitz_lcdtg_realize;
k->transfer = spitz_lcdtg_transfer;
dc->vmsd = &vmstate_spitz_lcdtg_regs;
}
diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
index 4e9494f94c20..2db66508b5b6 100644
--- a/hw/arm/tosa.c
+++ b/hw/arm/tosa.c
@@ -127,10 +127,9 @@ static uint32_t tosa_ssp_tansfer(SSISlave *dev, uint32_t value)
return 0;
}
-static int tosa_ssp_init(SSISlave *dev)
+static void tosa_ssp_realize(SSISlave *dev, Error **errp)
{
/* Nothing to do. */
- return 0;
}
#define TYPE_TOSA_DAC "tosa_dac"
@@ -283,7 +282,7 @@ static void tosa_ssp_class_init(ObjectClass *klass, void *data)
{
SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
- k->init = tosa_ssp_init;
+ k->realize = tosa_ssp_realize;
k->transfer = tosa_ssp_tansfer;
}
diff --git a/hw/arm/z2.c b/hw/arm/z2.c
index aea895a500ef..68a92f3184d7 100644
--- a/hw/arm/z2.c
+++ b/hw/arm/z2.c
@@ -151,14 +151,12 @@ static void z2_lcd_cs(void *opaque, int line, int level)
z2_lcd->selected = !level;
}
-static int zipit_lcd_init(SSISlave *dev)
+static void zipit_lcd_realize(SSISlave *dev, Error **errp)
{
ZipitLCD *z = FROM_SSI_SLAVE(ZipitLCD, dev);
z->selected = 0;
z->enabled = 0;
z->pos = 0;
-
- return 0;
}
static VMStateDescription vmstate_zipit_lcd_state = {
@@ -181,7 +179,7 @@ static void zipit_lcd_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
- k->init = zipit_lcd_init;
+ k->realize = zipit_lcd_realize;
k->transfer = zipit_lcd_transfer;
dc->vmsd = &vmstate_zipit_lcd_state;
}
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 326b688e8328..3cdcfce07ba9 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -28,6 +28,7 @@
#include "hw/ssi/ssi.h"
#include "qemu/bitops.h"
#include "qemu/log.h"
+#include "qapi/error.h"
#ifndef M25P80_ERR_DEBUG
#define M25P80_ERR_DEBUG 0
@@ -1132,7 +1133,7 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
return r;
}
-static int m25p80_init(SSISlave *ss)
+static void m25p80_realize(SSISlave *ss, Error **errp)
{
DriveInfo *dinfo;
Flash *s = M25P80(ss);
@@ -1153,18 +1154,15 @@ static int m25p80_init(SSISlave *ss)
s->storage = blk_blockalign(s->blk, s->size);
- /* FIXME: Move to late init */
if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
- fprintf(stderr, "Failed to initialize SPI flash!\n");
- return 1;
+ error_setg(errp, "failed to read the initial flash content");
+ return;
}
} else {
DB_PRINT_L(0, "No BDRV - binding to RAM\n");
s->storage = blk_blockalign(NULL, s->size);
memset(s->storage, 0xFF, s->size);
}
-
- return 0;
}
static void m25p80_reset(DeviceState *d)
@@ -1224,7 +1222,7 @@ static void m25p80_class_init(ObjectClass *klass, void *data)
SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
M25P80Class *mc = M25P80_CLASS(klass);
- k->init = m25p80_init;
+ k->realize = m25p80_realize;
k->transfer = m25p80_transfer8;
k->set_cs = m25p80_cs;
k->cs_polarity = SSI_CS_LOW;
diff --git a/hw/display/ads7846.c b/hw/display/ads7846.c
index 05aa2d1e6b6e..166edade7dc0 100644
--- a/hw/display/ads7846.c
+++ b/hw/display/ads7846.c
@@ -133,7 +133,7 @@ static const VMStateDescription vmstate_ads7846 = {
}
};
-static int ads7846_init(SSISlave *d)
+static void ads7846_realize(SSISlave *d, Error **errp)
{
DeviceState *dev = DEVICE(d);
ADS7846State *s = FROM_SSI_SLAVE(ADS7846State, d);
@@ -152,14 +152,13 @@ static int ads7846_init(SSISlave *d)
ads7846_int_update(s);
vmstate_register(NULL, -1, &vmstate_ads7846, s);
- return 0;
}
static void ads7846_class_init(ObjectClass *klass, void *data)
{
SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
- k->init = ads7846_init;
+ k->realize = ads7846_realize;
k->transfer = ads7846_transfer;
}
diff --git a/hw/display/ssd0323.c b/hw/display/ssd0323.c
index 14c1bf339ccf..6d1faf44afd7 100644
--- a/hw/display/ssd0323.c
+++ b/hw/display/ssd0323.c
@@ -361,7 +361,7 @@ static const GraphicHwOps ssd0323_ops = {
.gfx_update = ssd0323_update_display,
};
-static int ssd0323_init(SSISlave *d)
+static void ssd0323_realize(SSISlave *d, Error **errp)
{
DeviceState *dev = DEVICE(d);
ssd0323_state *s = FROM_SSI_SLAVE(ssd0323_state, d);
@@ -375,14 +375,13 @@ static int ssd0323_init(SSISlave *d)
register_savevm(dev, "ssd0323_oled", -1, 1,
ssd0323_save, ssd0323_load, s);
- return 0;
}
static void ssd0323_class_init(ObjectClass *klass, void *data)
{
SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
- k->init = ssd0323_init;
+ k->realize = ssd0323_realize;
k->transfer = ssd0323_transfer;
k->cs_polarity = SSI_CS_HIGH;
}
diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
index 9014f0f705d6..2a277bdb86dd 100644
--- a/hw/misc/max111x.c
+++ b/hw/misc/max111x.c
@@ -147,14 +147,14 @@ static int max111x_init(SSISlave *d, int inputs)
return 0;
}
-static int max1110_init(SSISlave *dev)
+static void max1110_realize(SSISlave *dev, Error **errp)
{
- return max111x_init(dev, 8);
+ max111x_init(dev, 8);
}
-static int max1111_init(SSISlave *dev)
+static void max1111_realize(SSISlave *dev, Error **errp)
{
- return max111x_init(dev, 4);
+ max111x_init(dev, 4);
}
void max111x_set_input(DeviceState *dev, int line, uint8_t value)
@@ -183,7 +183,7 @@ static void max1110_class_init(ObjectClass *klass, void *data)
{
SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
- k->init = max1110_init;
+ k->realize = max1110_realize;
}
static const TypeInfo max1110_info = {
@@ -196,7 +196,7 @@ static void max1111_class_init(ObjectClass *klass, void *data)
{
SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
- k->init = max1111_init;
+ k->realize = max1111_realize;
}
static const TypeInfo max1111_info = {
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 075e4ed5df85..3ff0886dd55d 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -15,6 +15,7 @@
#include "sysemu/blockdev.h"
#include "hw/ssi/ssi.h"
#include "hw/sd/sd.h"
+#include "qapi/error.h"
//#define DEBUG_SSI_SD 1
@@ -249,7 +250,7 @@ static int ssi_sd_load(QEMUFile *f, void *opaque, int version_id)
return 0;
}
-static int ssi_sd_init(SSISlave *d)
+static void ssi_sd_realize(SSISlave *d, Error **errp)
{
DeviceState *dev = DEVICE(d);
ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, d);
@@ -260,17 +261,17 @@ static int ssi_sd_init(SSISlave *d)
dinfo = drive_get_next(IF_SD);
s->sd = sd_init(dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, true);
if (s->sd == NULL) {
- return -1;
+ error_setg(errp, "Device initialization failed.");
+ return;
}
register_savevm(dev, "ssi_sd", -1, 1, ssi_sd_save, ssi_sd_load, s);
- return 0;
}
static void ssi_sd_class_init(ObjectClass *klass, void *data)
{
SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
- k->init = ssi_sd_init;
+ k->realize = ssi_sd_realize;
k->transfer = ssi_sd_transfer;
k->cs_polarity = SSI_CS_LOW;
}
diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c
index 9791c0d94714..7eaaf565fd67 100644
--- a/hw/ssi/ssi.c
+++ b/hw/ssi/ssi.c
@@ -54,7 +54,7 @@ static uint32_t ssi_transfer_raw_default(SSISlave *dev, uint32_t val)
return 0;
}
-static int ssi_slave_init(DeviceState *dev)
+static void ssi_slave_realize(DeviceState *dev, Error **errp)
{
SSISlave *s = SSI_SLAVE(dev);
SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(s);
@@ -64,7 +64,7 @@ static int ssi_slave_init(DeviceState *dev)
qdev_init_gpio_in_named(dev, ssi_cs_default, SSI_GPIO_CS, 1);
}
- return ssc->init(s);
+ ssc->realize(s, errp);
}
static void ssi_slave_class_init(ObjectClass *klass, void *data)
@@ -72,7 +72,7 @@ static void ssi_slave_class_init(ObjectClass *klass, void *data)
SSISlaveClass *ssc = SSI_SLAVE_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
- dc->init = ssi_slave_init;
+ dc->realize = ssi_slave_realize;
dc->bus_type = TYPE_SSI_BUS;
if (!ssc->transfer_raw) {
ssc->transfer_raw = ssi_transfer_raw_default;
diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
index 4a0a53903c66..6a0c3c3cdb28 100644
--- a/include/hw/ssi/ssi.h
+++ b/include/hw/ssi/ssi.h
@@ -37,7 +37,7 @@ enum SSICSMode {
struct SSISlaveClass {
DeviceClass parent_class;
- int (*init)(SSISlave *dev);
+ void (*realize)(SSISlave *dev, Error **errp);
/* if you have standard or no CS behaviour, just override transfer.
* This is called when the device cs is active (true by default).
--
2.1.4
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH v5 2/9] m25p80: do not put iovec on the stack
2016-06-28 18:24 [Qemu-devel] [PATCH v5 0/9] ast2400: SMC controllers Cédric Le Goater
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 1/9] ssi: change ssi_slave_init to be a realize ops Cédric Le Goater
@ 2016-06-28 18:24 ` Cédric Le Goater
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 3/9] m25p80: avoid out of bounds accesses Cédric Le Goater
` (7 subsequent siblings)
9 siblings, 0 replies; 33+ messages in thread
From: Cédric Le Goater @ 2016-06-28 18:24 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
Paolo Bonzini, Cédric Le Goater
From: Paolo Bonzini <pbonzini@redhat.com>
When doing a read-modify-write cycle, QEMU uses the iovec after returning
from blk_aio_pwritev. m25p80 puts the iovec on the stack of blk_aio_pwritev's
caller, which causes trouble in this case. This has been a problem
since commit 243e6f6 ("m25p80: Switch to byte-based block access",
2016-05-12) started doing writes at a smaller granularity than 512 bytes.
In principle however it could have broken before when using -drive
if=mtd,cache=none on a disk with 4K native sectors.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/block/m25p80.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 3cdcfce07ba9..8e4c11573521 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -447,6 +447,11 @@ static inline Manufacturer get_man(Flash *s)
static void blk_sync_complete(void *opaque, int ret)
{
+ QEMUIOVector *iov = opaque;
+
+ qemu_iovec_destroy(iov);
+ g_free(iov);
+
/* do nothing. Masters do not directly interact with the backing store,
* only the working copy so no mutexing required.
*/
@@ -454,31 +459,31 @@ static void blk_sync_complete(void *opaque, int ret)
static void flash_sync_page(Flash *s, int page)
{
- QEMUIOVector iov;
+ QEMUIOVector *iov = g_new(QEMUIOVector, 1);
if (!s->blk || blk_is_read_only(s->blk)) {
return;
}
- qemu_iovec_init(&iov, 1);
- qemu_iovec_add(&iov, s->storage + page * s->pi->page_size,
+ qemu_iovec_init(iov, 1);
+ qemu_iovec_add(iov, s->storage + page * s->pi->page_size,
s->pi->page_size);
- blk_aio_pwritev(s->blk, page * s->pi->page_size, &iov, 0,
- blk_sync_complete, NULL);
+ blk_aio_pwritev(s->blk, page * s->pi->page_size, iov, 0,
+ blk_sync_complete, iov);
}
static inline void flash_sync_area(Flash *s, int64_t off, int64_t len)
{
- QEMUIOVector iov;
+ QEMUIOVector *iov = g_new(QEMUIOVector, 1);
if (!s->blk || blk_is_read_only(s->blk)) {
return;
}
assert(!(len % BDRV_SECTOR_SIZE));
- qemu_iovec_init(&iov, 1);
- qemu_iovec_add(&iov, s->storage + off, len);
- blk_aio_pwritev(s->blk, off, &iov, 0, blk_sync_complete, NULL);
+ qemu_iovec_init(iov, 1);
+ qemu_iovec_add(iov, s->storage + off, len);
+ blk_aio_pwritev(s->blk, off, iov, 0, blk_sync_complete, iov);
}
static void flash_erase(Flash *s, int offset, FlashCMD cmd)
--
2.1.4
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH v5 3/9] m25p80: avoid out of bounds accesses
2016-06-28 18:24 [Qemu-devel] [PATCH v5 0/9] ast2400: SMC controllers Cédric Le Goater
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 1/9] ssi: change ssi_slave_init to be a realize ops Cédric Le Goater
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 2/9] m25p80: do not put iovec on the stack Cédric Le Goater
@ 2016-06-28 18:24 ` Cédric Le Goater
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 4/9] m25p80: change cur_addr to 32 bit integer Cédric Le Goater
` (6 subsequent siblings)
9 siblings, 0 replies; 33+ messages in thread
From: Cédric Le Goater @ 2016-06-28 18:24 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
Paolo Bonzini, Cédric Le Goater
From: Paolo Bonzini <pbonzini@redhat.com>
s->cur_addr can be made to point outside s->storage, either by
writing a value >= 128 to s->ear (because s->ear * MAX_3BYTES_SIZE
is a signed integer and sign-extends into the 64-bit cur_addr),
or just by writing an address beyond the size of the flash being
emulated. Avoid the sign extension to make the code cleaner, and
on top of that mask s->cur_addr to s->size.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/block/m25p80.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 8e4c11573521..4836b4eb9666 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -587,18 +587,16 @@ static inline int get_addr_length(Flash *s)
static void complete_collecting_data(Flash *s)
{
- int i;
-
- s->cur_addr = 0;
+ int i, n;
- for (i = 0; i < get_addr_length(s); ++i) {
+ n = get_addr_length(s);
+ s->cur_addr = (n == 3 ? s->ear : 0);
+ for (i = 0; i < n; ++i) {
s->cur_addr <<= 8;
s->cur_addr |= s->data[i];
}
- if (get_addr_length(s) == 3) {
- s->cur_addr += s->ear * MAX_3BYTES_SIZE;
- }
+ s->cur_addr &= s->size - 1;
s->state = STATE_IDLE;
@@ -1100,14 +1098,14 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
DB_PRINT_L(1, "page program cur_addr=%#" PRIx64 " data=%" PRIx8 "\n",
s->cur_addr, (uint8_t)tx);
flash_write8(s, s->cur_addr, (uint8_t)tx);
- s->cur_addr++;
+ s->cur_addr = (s->cur_addr + 1) & (s->size - 1);
break;
case STATE_READ:
r = s->storage[s->cur_addr];
DB_PRINT_L(1, "READ 0x%" PRIx64 "=%" PRIx8 "\n", s->cur_addr,
(uint8_t)r);
- s->cur_addr = (s->cur_addr + 1) % s->size;
+ s->cur_addr = (s->cur_addr + 1) & (s->size - 1);
break;
case STATE_COLLECTING_DATA:
--
2.1.4
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH v5 4/9] m25p80: change cur_addr to 32 bit integer
2016-06-28 18:24 [Qemu-devel] [PATCH v5 0/9] ast2400: SMC controllers Cédric Le Goater
` (2 preceding siblings ...)
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 3/9] m25p80: avoid out of bounds accesses Cédric Le Goater
@ 2016-06-28 18:24 ` Cédric Le Goater
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 5/9] m25p80: qdev-ify drive property Cédric Le Goater
` (5 subsequent siblings)
9 siblings, 0 replies; 33+ messages in thread
From: Cédric Le Goater @ 2016-06-28 18:24 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
Paolo Bonzini, Cédric Le Goater
From: Paolo Bonzini <pbonzini@redhat.com>
The maximum amount of storage that can be addressed by the m25p80 command
set is 4 GiB. However, cur_addr is currently a 64-bit integer. To avoid
further problems related to sign extension of signed 32-bit integer
expressions, change cur_addr to a 32 bit integer. Preserve migration
format by adding a dummy 4-byte field in place of the (big-endian)
high four bytes in the formerly 64-bit cur_addr field.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/block/m25p80.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 4836b4eb9666..80e60c23e009 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -390,7 +390,7 @@ typedef struct Flash {
uint32_t pos;
uint8_t needed_bytes;
uint8_t cmd_in_progress;
- uint64_t cur_addr;
+ uint32_t cur_addr;
uint32_t nonvolatile_cfg;
/* Configuration register for Macronix */
uint32_t volatile_cfg;
@@ -536,9 +536,9 @@ static inline void flash_sync_dirty(Flash *s, int64_t newpage)
}
static inline
-void flash_write8(Flash *s, uint64_t addr, uint8_t data)
+void flash_write8(Flash *s, uint32_t addr, uint8_t data)
{
- int64_t page = addr / s->pi->page_size;
+ uint32_t page = addr / s->pi->page_size;
uint8_t prev = s->storage[s->cur_addr];
if (!s->write_enable) {
@@ -546,7 +546,7 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t data)
}
if ((prev ^ data) & data) {
- DB_PRINT_L(1, "programming zero to one! addr=%" PRIx64 " %" PRIx8
+ DB_PRINT_L(1, "programming zero to one! addr=%" PRIx32 " %" PRIx8
" -> %" PRIx8 "\n", addr, prev, data);
}
@@ -1095,7 +1095,7 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
switch (s->state) {
case STATE_PAGE_PROGRAM:
- DB_PRINT_L(1, "page program cur_addr=%#" PRIx64 " data=%" PRIx8 "\n",
+ DB_PRINT_L(1, "page program cur_addr=%#" PRIx32 " data=%" PRIx8 "\n",
s->cur_addr, (uint8_t)tx);
flash_write8(s, s->cur_addr, (uint8_t)tx);
s->cur_addr = (s->cur_addr + 1) & (s->size - 1);
@@ -1103,7 +1103,7 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
case STATE_READ:
r = s->storage[s->cur_addr];
- DB_PRINT_L(1, "READ 0x%" PRIx64 "=%" PRIx8 "\n", s->cur_addr,
+ DB_PRINT_L(1, "READ 0x%" PRIx32 "=%" PRIx8 "\n", s->cur_addr,
(uint8_t)r);
s->cur_addr = (s->cur_addr + 1) & (s->size - 1);
break;
@@ -1202,7 +1202,8 @@ static const VMStateDescription vmstate_m25p80 = {
VMSTATE_UINT32(pos, Flash),
VMSTATE_UINT8(needed_bytes, Flash),
VMSTATE_UINT8(cmd_in_progress, Flash),
- VMSTATE_UINT64(cur_addr, Flash),
+ VMSTATE_UNUSED(4),
+ VMSTATE_UINT32(cur_addr, Flash),
VMSTATE_BOOL(write_enable, Flash),
VMSTATE_BOOL_V(reset_enable, Flash, 2),
VMSTATE_UINT8_V(ear, Flash, 2),
--
2.1.4
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH v5 5/9] m25p80: qdev-ify drive property
2016-06-28 18:24 [Qemu-devel] [PATCH v5 0/9] ast2400: SMC controllers Cédric Le Goater
` (3 preceding siblings ...)
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 4/9] m25p80: change cur_addr to 32 bit integer Cédric Le Goater
@ 2016-06-28 18:24 ` Cédric Le Goater
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 6/9] ast2400: add SMC controllers (FMC and SPI) Cédric Le Goater
` (4 subsequent siblings)
9 siblings, 0 replies; 33+ messages in thread
From: Cédric Le Goater @ 2016-06-28 18:24 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
Paolo Bonzini, Cédric Le Goater
From: Paolo Bonzini <pbonzini@redhat.com>
This allows specifying the property via -drive if=none and creating
the flash device with -device.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
[clg: added an extra fix for sabrelite_init()
keeping the test on flash_dev did not seem necessary. ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/arm/sabrelite.c | 18 ++++++++++++------
hw/arm/xilinx_zynq.c | 8 +++++++-
hw/arm/xlnx-ep108.c | 9 ++++++++-
hw/block/m25p80.c | 10 ++--------
hw/microblaze/petalogix_ml605_mmu.c | 9 ++++++++-
5 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c
index 776c51e39869..4e7ac8cc4f4b 100644
--- a/hw/arm/sabrelite.c
+++ b/hw/arm/sabrelite.c
@@ -86,13 +86,19 @@ static void sabrelite_init(MachineState *machine)
spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(spi_dev), "spi");
if (spi_bus) {
DeviceState *flash_dev;
-
- flash_dev = ssi_create_slave(spi_bus, "sst25vf016b");
- if (flash_dev) {
- qemu_irq cs_line = qdev_get_gpio_in_named(flash_dev,
- SSI_GPIO_CS, 0);
- sysbus_connect_irq(SYS_BUS_DEVICE(spi_dev), 1, cs_line);
+ qemu_irq cs_line;
+ DriveInfo *dinfo = drive_get_next(IF_MTD);
+
+ flash_dev = ssi_create_slave_no_init(spi_bus, "sst25vf016b");
+ if (dinfo) {
+ qdev_prop_set_drive(flash_dev, "drive",
+ blk_by_legacy_dinfo(dinfo),
+ &error_fatal);
}
+ qdev_init_nofail(flash_dev);
+
+ cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
+ sysbus_connect_irq(SYS_BUS_DEVICE(spi_dev), 1, cs_line);
}
}
}
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index aefebcfa6d8f..b0cabe2e4a67 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -138,7 +138,13 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq,
spi = (SSIBus *)qdev_get_child_bus(dev, bus_name);
for (j = 0; j < num_ss; ++j) {
- flash_dev = ssi_create_slave(spi, "n25q128");
+ DriveInfo *dinfo = drive_get_next(IF_MTD);
+ flash_dev = ssi_create_slave_no_init(spi, "n25q128");
+ if (dinfo) {
+ qdev_prop_set_drive(flash_dev, "drive",
+ blk_by_legacy_dinfo(dinfo), &error_fatal);
+ }
+ qdev_init_nofail(flash_dev);
cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
sysbus_connect_irq(busdev, i * num_ss + j + 1, cs_line);
diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
index 34b464171266..4ec590a25db5 100644
--- a/hw/arm/xlnx-ep108.c
+++ b/hw/arm/xlnx-ep108.c
@@ -88,12 +88,19 @@ static void xlnx_ep108_init(MachineState *machine)
SSIBus *spi_bus;
DeviceState *flash_dev;
qemu_irq cs_line;
+ DriveInfo *dinfo = drive_get_next(IF_MTD);
gchar *bus_name = g_strdup_printf("spi%d", i);
spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc), bus_name);
g_free(bus_name);
- flash_dev = ssi_create_slave(spi_bus, "sst25wf080");
+ flash_dev = ssi_create_slave_no_init(spi_bus, "sst25wf080");
+ if (dinfo) {
+ qdev_prop_set_drive(flash_dev, "drive", blk_by_legacy_dinfo(dinfo),
+ &error_fatal);
+ }
+ qdev_init_nofail(flash_dev);
+
cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[i]), 1, cs_line);
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 80e60c23e009..d9b27939dde6 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -1138,7 +1138,6 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
static void m25p80_realize(SSISlave *ss, Error **errp)
{
- DriveInfo *dinfo;
Flash *s = M25P80(ss);
M25P80Class *mc = M25P80_GET_CLASS(s);
@@ -1147,14 +1146,8 @@ static void m25p80_realize(SSISlave *ss, Error **errp)
s->size = s->pi->sector_size * s->pi->n_sectors;
s->dirty_page = -1;
- /* FIXME use a qdev drive property instead of drive_get_next() */
- dinfo = drive_get_next(IF_MTD);
-
- if (dinfo) {
+ if (s->blk) {
DB_PRINT_L(0, "Binding to IF_MTD drive\n");
- s->blk = blk_by_legacy_dinfo(dinfo);
- blk_attach_dev_nofail(s->blk, s);
-
s->storage = blk_blockalign(s->blk, s->size);
if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
@@ -1187,6 +1180,7 @@ static Property m25p80_properties[] = {
DEFINE_PROP_UINT8("spansion-cr2nv", Flash, spansion_cr2nv, 0x8),
DEFINE_PROP_UINT8("spansion-cr3nv", Flash, spansion_cr3nv, 0x2),
DEFINE_PROP_UINT8("spansion-cr4nv", Flash, spansion_cr4nv, 0x10),
+ DEFINE_PROP_DRIVE("drive", Flash, blk),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index 07527b677b37..4968bdbb2821 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -191,9 +191,16 @@ petalogix_ml605_init(MachineState *machine)
spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
for (i = 0; i < NUM_SPI_FLASHES; i++) {
+ DriveInfo *dinfo = drive_get_next(IF_MTD);
qemu_irq cs_line;
- dev = ssi_create_slave(spi, "n25q128");
+ dev = ssi_create_slave_no_init(spi, "n25q128");
+ if (dinfo) {
+ qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
+ &error_fatal);
+ }
+ qdev_init_nofail(dev);
+
cs_line = qdev_get_gpio_in_named(dev, SSI_GPIO_CS, 0);
sysbus_connect_irq(busdev, i+1, cs_line);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH v5 6/9] ast2400: add SMC controllers (FMC and SPI)
2016-06-28 18:24 [Qemu-devel] [PATCH v5 0/9] ast2400: SMC controllers Cédric Le Goater
` (4 preceding siblings ...)
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 5/9] m25p80: qdev-ify drive property Cédric Le Goater
@ 2016-06-28 18:24 ` Cédric Le Goater
2016-06-29 7:58 ` Cédric Le Goater
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 7/9] ast2400: add SPI flash slaves Cédric Le Goater
` (3 subsequent siblings)
9 siblings, 1 reply; 33+ messages in thread
From: Cédric Le Goater @ 2016-06-28 18:24 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
Cédric Le Goater
The Aspeed AST2400 soc includes a static memory controller for the BMC
which supports NOR, NAND and SPI flash memory modules. This controller
has two modes : the SMC for the legacy interface which supports only
one module and the FMC for the new interface which supports up to five
modules. The AST2400 also includes a SPI only controller used for the
host firmware, commonly called BIOS on Intel. It can be used in three
mode : a SPI master, SPI slave and SPI pass-through
Below is the initial framework for the SMC controller (FMC mode only)
and the SPI controller: the sysbus object, MMIO for registers
configuration and controls. Each controller has a SPI bus and a
configurable number of CS lines for SPI flash slaves.
The differences between the controllers are small, so they are
abstracted using indirections on the register numbers.
Only SPI flash modules are supported.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
Changes since v3:
- Fixed typos on CTRL
- Fixed multiple error handling when setting properties in
ast2400_realize()
- Added error messages when max_slaves is exceeded
- Added definitions for R_INTR_CTRL register bits
- Added definitions for R_DMA_* registers
- Constantified a couple of routine arguments
- Sorted out what was need for migration (registers only apriori)
Changes since v2:
- Switched to a realize ops to be able to handle errors.
hw/arm/ast2400.c | 34 ++++-
hw/ssi/Makefile.objs | 1 +
hw/ssi/aspeed_smc.c | 326 ++++++++++++++++++++++++++++++++++++++++++++
include/hw/arm/ast2400.h | 3 +
include/hw/ssi/aspeed_smc.h | 79 +++++++++++
5 files changed, 442 insertions(+), 1 deletion(-)
create mode 100644 hw/ssi/aspeed_smc.c
create mode 100644 include/hw/ssi/aspeed_smc.h
diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
index b14a82fcdef1..b16ba2d0c516 100644
--- a/hw/arm/ast2400.c
+++ b/hw/arm/ast2400.c
@@ -23,6 +23,9 @@
#define AST2400_UART_5_BASE 0x00184000
#define AST2400_IOMEM_SIZE 0x00200000
#define AST2400_IOMEM_BASE 0x1E600000
+#define AST2400_SMC_BASE AST2400_IOMEM_BASE /* Legacy SMC */
+#define AST2400_FMC_BASE 0X1E620000
+#define AST2400_SPI_BASE 0X1E630000
#define AST2400_VIC_BASE 0x1E6C0000
#define AST2400_SCU_BASE 0x1E6E2000
#define AST2400_TIMER_BASE 0x1E782000
@@ -85,13 +88,21 @@ static void ast2400_init(Object *obj)
"hw-strap1", &error_abort);
object_property_add_alias(obj, "hw-strap2", OBJECT(&s->scu),
"hw-strap2", &error_abort);
+
+ object_initialize(&s->smc, sizeof(s->smc), "aspeed.smc.fmc");
+ object_property_add_child(obj, "smc", OBJECT(&s->smc), NULL);
+ qdev_set_parent_bus(DEVICE(&s->smc), sysbus_get_default());
+
+ object_initialize(&s->spi, sizeof(s->spi), "aspeed.smc.spi");
+ object_property_add_child(obj, "spi", OBJECT(&s->spi), NULL);
+ qdev_set_parent_bus(DEVICE(&s->spi), sysbus_get_default());
}
static void ast2400_realize(DeviceState *dev, Error **errp)
{
int i;
AST2400State *s = AST2400(dev);
- Error *err = NULL;
+ Error *err = NULL, *local_err = NULL;
/* IO space */
memory_region_init_io(&s->iomem, NULL, &ast2400_io_ops, NULL,
@@ -147,6 +158,27 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, AST2400_I2C_BASE);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
qdev_get_gpio_in(DEVICE(&s->vic), 12));
+
+ /* SMC */
+ object_property_set_int(OBJECT(&s->smc), 1, "num-cs", &err);
+ object_property_set_bool(OBJECT(&s->smc), true, "realized", &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->smc), 0, AST2400_FMC_BASE);
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->smc), 0,
+ qdev_get_gpio_in(DEVICE(&s->vic), 19));
+
+ /* SPI */
+ object_property_set_int(OBJECT(&s->spi), 1, "num-cs", &err);
+ object_property_set_bool(OBJECT(&s->spi), true, "realized", &local_err);
+ error_propagate(&err, local_err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 0, AST2400_SPI_BASE);
}
static void ast2400_class_init(ObjectClass *oc, void *data)
diff --git a/hw/ssi/Makefile.objs b/hw/ssi/Makefile.objs
index fcbb79ef0185..c79a8dcd86a9 100644
--- a/hw/ssi/Makefile.objs
+++ b/hw/ssi/Makefile.objs
@@ -2,6 +2,7 @@ common-obj-$(CONFIG_PL022) += pl022.o
common-obj-$(CONFIG_SSI) += ssi.o
common-obj-$(CONFIG_XILINX_SPI) += xilinx_spi.o
common-obj-$(CONFIG_XILINX_SPIPS) += xilinx_spips.o
+common-obj-$(CONFIG_ASPEED_SOC) += aspeed_smc.o
obj-$(CONFIG_OMAP) += omap_spi.o
obj-$(CONFIG_IMX) += imx_spi.o
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
new file mode 100644
index 000000000000..537635e18d64
--- /dev/null
+++ b/hw/ssi/aspeed_smc.c
@@ -0,0 +1,326 @@
+/*
+ * ASPEED AST2400 SMC Controller (SPI Flash Only)
+ *
+ * Copyright (C) 2016 IBM Corp.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "sysemu/sysemu.h"
+#include "qemu/log.h"
+#include "include/qemu/error-report.h"
+#include "exec/address-spaces.h"
+
+#include "hw/ssi/aspeed_smc.h"
+
+/* CE Type Setting Register */
+#define R_CONF (0x00 / 4)
+#define CONF_LEGACY_DISABLE (1 << 31)
+#define CONF_ENABLE_W4 20
+#define CONF_ENABLE_W3 19
+#define CONF_ENABLE_W2 18
+#define CONF_ENABLE_W1 17
+#define CONF_ENABLE_W0 16
+#define CONF_FLASH_TYPE4 9
+#define CONF_FLASH_TYPE3 7
+#define CONF_FLASH_TYPE2 5
+#define CONF_FLASH_TYPE1 3
+#define CONF_FLASH_TYPE0 1
+
+/* CE Control Register */
+#define R_CE_CTRL (0x04 / 4)
+#define CTRL_EXTENDED4 4 /* 32 bit addressing for SPI */
+#define CTRL_EXTENDED3 3 /* 32 bit addressing for SPI */
+#define CTRL_EXTENDED2 2 /* 32 bit addressing for SPI */
+#define CTRL_EXTENDED1 1 /* 32 bit addressing for SPI */
+#define CTRL_EXTENDED0 0 /* 32 bit addressing for SPI */
+
+/* Interrupt Control and Status Register */
+#define R_INTR_CTRL (0x08 / 4)
+#define INTR_CTRL_DMA_STATUS (1 << 11)
+#define INTR_CTRL_CMD_ABORT_STATUS (1 << 10)
+#define INTR_CTRL_WRITE_PROTECT_STATUS (1 << 9)
+#define INTR_CTRL_DMA_EN (1 << 3)
+#define INTR_CTRL_CMD_ABORT_EN (1 << 2)
+#define INTR_CTRL_WRITE_PROTECT_EN (1 << 1)
+
+/* CEx Control Register */
+#define R_CTRL0 (0x10 / 4)
+#define CTRL_CMD_SHIFT 16
+#define CTRL_CMD_MASK 0xff
+#define CTRL_CE_STOP_ACTIVE (1 << 2)
+#define CTRL_CMD_MODE_MASK 0x3
+#define CTRL_READMODE 0x0
+#define CTRL_FREADMODE 0x1
+#define CTRL_WRITEMODE 0x2
+#define CTRL_USERMODE 0x3
+#define R_CTRL1 (0x14 / 4)
+#define R_CTRL2 (0x18 / 4)
+#define R_CTRL3 (0x1C / 4)
+#define R_CTRL4 (0x20 / 4)
+
+/* CEx Segment Address Register */
+#define R_SEG_ADDR0 (0x30 / 4)
+#define SEG_SIZE_SHIFT 24 /* 8MB units */
+#define SEG_SIZE_MASK 0x7f
+#define SEG_START_SHIFT 16 /* address bit [A29-A23] */
+#define SEG_START_MASK 0x7f
+#define R_SEG_ADDR1 (0x34 / 4)
+#define R_SEG_ADDR2 (0x38 / 4)
+#define R_SEG_ADDR3 (0x3C / 4)
+#define R_SEG_ADDR4 (0x40 / 4)
+
+/* Misc Control Register #1 */
+#define R_MISC_CTRL1 (0x50 / 4)
+
+/* Misc Control Register #2 */
+#define R_MISC_CTRL2 (0x54 / 4)
+
+/* DMA Control/Status Register */
+#define R_DMA_CTRL (0x80 / 4)
+#define DMA_CTRL_DELAY_MASK 0xf
+#define DMA_CTRL_DELAY_SHIFT 8
+#define DMA_CTRL_FREQ_MASK 0xf
+#define DMA_CTRL_FREQ_SHIFT 4
+#define DMA_CTRL_MODE (1 << 3)
+#define DMA_CTRL_CKSUM (1 << 2)
+#define DMA_CTRL_DIR (1 << 1)
+#define DMA_CTRL_EN (1 << 0)
+
+/* DMA Flash Side Address */
+#define R_DMA_FLASH_ADDR (0x84 / 4)
+
+/* DMA DRAM Side Address */
+#define R_DMA_DRAM_ADDR (0x88 / 4)
+
+/* DMA Length Register */
+#define R_DMA_LEN (0x8C / 4)
+
+/* Checksum Calculation Result */
+#define R_DMA_CHECKSUM (0x90 / 4)
+
+/* Misc Control Register #2 */
+#define R_TIMINGS (0x94 / 4)
+
+/* SPI controller registers and bits */
+#define R_SPI_CONF (0x00 / 4)
+#define SPI_CONF_ENABLE_W0 0
+#define R_SPI_CTRL0 (0x4 / 4)
+#define R_SPI_MISC_CTRL (0x10 / 4)
+#define R_SPI_TIMINGS (0x14 / 4)
+
+static const AspeedSMCController controllers[] = {
+ { "aspeed.smc.smc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
+ CONF_ENABLE_W0, 5 },
+ { "aspeed.smc.fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
+ CONF_ENABLE_W0, 5 },
+ { "aspeed.smc.spi", R_SPI_CONF, 0xff, R_SPI_CTRL0, R_SPI_TIMINGS,
+ SPI_CONF_ENABLE_W0, 1 },
+};
+
+static bool aspeed_smc_is_ce_stop_active(const AspeedSMCState *s, int cs)
+{
+ return s->regs[s->r_ctrl0 + cs] & CTRL_CE_STOP_ACTIVE;
+}
+
+static void aspeed_smc_update_cs(const AspeedSMCState *s)
+{
+ int i;
+
+ for (i = 0; i < s->num_cs; ++i) {
+ qemu_set_irq(s->cs_lines[i], aspeed_smc_is_ce_stop_active(s, i));
+ }
+}
+
+static void aspeed_smc_reset(DeviceState *d)
+{
+ AspeedSMCState *s = ASPEED_SMC(d);
+ int i;
+
+ memset(s->regs, 0, sizeof s->regs);
+
+ /* Unselect all slaves */
+ for (i = 0; i < s->num_cs; ++i) {
+ s->regs[s->r_ctrl0 + i] |= CTRL_CE_STOP_ACTIVE;
+ }
+
+ aspeed_smc_update_cs(s);
+}
+
+static bool aspeed_smc_is_implemented(AspeedSMCState *s, hwaddr addr)
+{
+ return (addr == s->r_conf || addr == s->r_timings || addr == s->r_ce_ctrl ||
+ (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs));
+}
+
+static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
+{
+ AspeedSMCState *s = ASPEED_SMC(opaque);
+
+ addr >>= 2;
+
+ if (addr >= ARRAY_SIZE(s->regs)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Out-of-bounds read at 0x%" HWADDR_PRIx "\n",
+ __func__, addr);
+ return 0;
+ }
+
+ if (!aspeed_smc_is_implemented(s, addr)) {
+ qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
+ __func__, addr);
+ return 0;
+ }
+
+ return s->regs[addr];
+}
+
+static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned int size)
+{
+ AspeedSMCState *s = ASPEED_SMC(opaque);
+ uint32_t value = data;
+
+ addr >>= 2;
+
+ if (addr >= ARRAY_SIZE(s->regs)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Out-of-bounds write at 0x%" HWADDR_PRIx "\n",
+ __func__, addr);
+ return;
+ }
+
+ if (!aspeed_smc_is_implemented(s, addr)) {
+ qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
+ __func__, addr);
+ return;
+ }
+
+ /*
+ * Not much to do apart from storing the value and set the cs
+ * lines if the register is a controlling one.
+ */
+ s->regs[addr] = value;
+ if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs) {
+ aspeed_smc_update_cs(s);
+ }
+}
+
+static const MemoryRegionOps aspeed_smc_ops = {
+ .read = aspeed_smc_read,
+ .write = aspeed_smc_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid.unaligned = true,
+};
+
+static void aspeed_smc_realize(DeviceState *dev, Error **errp)
+{
+ SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+ AspeedSMCState *s = ASPEED_SMC(dev);
+ AspeedSMCClass *mc = ASPEED_SMC_GET_CLASS(s);
+ int i;
+
+ s->ctrl = mc->ctrl;
+
+ /* keep a copy under AspeedSMCState to speed up accesses */
+ s->r_conf = s->ctrl->r_conf;
+ s->r_ce_ctrl = s->ctrl->r_ce_ctrl;
+ s->r_ctrl0 = s->ctrl->r_ctrl0;
+ s->r_timings = s->ctrl->r_timings;
+ s->conf_enable_w0 = s->ctrl->conf_enable_w0;
+
+ /* Enforce some real HW limits */
+ if (s->num_cs > s->ctrl->max_slaves) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: num_cs cannot exceed: %d\n",
+ __func__, s->ctrl->max_slaves);
+ s->num_cs = s->ctrl->max_slaves;
+ }
+
+ s->spi = ssi_create_bus(dev, "spi");
+
+ /* Setup cs_lines for slaves */
+ sysbus_init_irq(sbd, &s->irq);
+ s->cs_lines = g_new0(qemu_irq, s->num_cs);
+ ssi_auto_connect_slaves(dev, s->cs_lines, s->spi);
+
+ for (i = 0; i < s->num_cs; ++i) {
+ sysbus_init_irq(sbd, &s->cs_lines[i]);
+ }
+
+ aspeed_smc_reset(dev);
+
+ memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s,
+ s->ctrl->name, ASPEED_SMC_R_MAX * 4);
+ sysbus_init_mmio(sbd, &s->mmio);
+}
+
+static const VMStateDescription vmstate_aspeed_smc = {
+ .name = "aspeed.smc",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32_ARRAY(regs, AspeedSMCState, ASPEED_SMC_R_MAX),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static Property aspeed_smc_properties[] = {
+ DEFINE_PROP_UINT32("num-cs", AspeedSMCState, num_cs, 1),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void aspeed_smc_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ AspeedSMCClass *mc = ASPEED_SMC_CLASS(klass);
+
+ dc->realize = aspeed_smc_realize;
+ dc->reset = aspeed_smc_reset;
+ dc->props = aspeed_smc_properties;
+ dc->vmsd = &vmstate_aspeed_smc;
+ mc->ctrl = data;
+}
+
+static const TypeInfo aspeed_smc_info = {
+ .name = TYPE_ASPEED_SMC,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(AspeedSMCState),
+ .class_size = sizeof(AspeedSMCClass),
+ .abstract = true,
+};
+
+static void aspeed_smc_register_types(void)
+{
+ int i;
+
+ type_register_static(&aspeed_smc_info);
+ for (i = 0; i < ARRAY_SIZE(controllers); ++i) {
+ TypeInfo ti = {
+ .name = controllers[i].name,
+ .parent = TYPE_ASPEED_SMC,
+ .class_init = aspeed_smc_class_init,
+ .class_data = (void *)&controllers[i],
+ };
+ type_register(&ti);
+ }
+}
+
+type_init(aspeed_smc_register_types)
diff --git a/include/hw/arm/ast2400.h b/include/hw/arm/ast2400.h
index f1a64fd3893d..7833bc716cd8 100644
--- a/include/hw/arm/ast2400.h
+++ b/include/hw/arm/ast2400.h
@@ -17,6 +17,7 @@
#include "hw/misc/aspeed_scu.h"
#include "hw/timer/aspeed_timer.h"
#include "hw/i2c/aspeed_i2c.h"
+#include "hw/ssi/aspeed_smc.h"
typedef struct AST2400State {
/*< private >*/
@@ -29,6 +30,8 @@ typedef struct AST2400State {
AspeedTimerCtrlState timerctrl;
AspeedI2CState i2c;
AspeedSCUState scu;
+ AspeedSMCState smc;
+ AspeedSMCState spi;
} AST2400State;
#define TYPE_AST2400 "ast2400"
diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
new file mode 100644
index 000000000000..c4a4960cd880
--- /dev/null
+++ b/include/hw/ssi/aspeed_smc.h
@@ -0,0 +1,79 @@
+/*
+ * ASPEED AST2400 SMC Controller (SPI Flash Only)
+ *
+ * Copyright (C) 2016 IBM Corp.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef ASPEED_SMC_H
+#define ASPEED_SMC_H
+
+#include "hw/ssi/ssi.h"
+
+typedef struct AspeedSMCController {
+ const char *name;
+ uint8_t r_conf;
+ uint8_t r_ce_ctrl;
+ uint8_t r_ctrl0;
+ uint8_t r_timings;
+ uint8_t conf_enable_w0;
+ uint8_t max_slaves;
+} AspeedSMCController;
+
+#define TYPE_ASPEED_SMC "aspeed.smc"
+#define ASPEED_SMC(obj) OBJECT_CHECK(AspeedSMCState, (obj), TYPE_ASPEED_SMC)
+#define ASPEED_SMC_CLASS(klass) \
+ OBJECT_CLASS_CHECK(AspeedSMCClass, (klass), TYPE_ASPEED_SMC)
+#define ASPEED_SMC_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(AspeedSMCClass, (obj), TYPE_ASPEED_SMC)
+
+typedef struct AspeedSMCClass {
+ SysBusDevice parent_obj;
+ const AspeedSMCController *ctrl;
+} AspeedSMCClass;
+
+#define ASPEED_SMC_R_MAX (0x100 / 4)
+
+typedef struct AspeedSMCState {
+ SysBusDevice parent_obj;
+
+ const AspeedSMCController *ctrl;
+
+ MemoryRegion mmio;
+
+ qemu_irq irq;
+ int irqline;
+
+ uint32_t num_cs;
+ qemu_irq *cs_lines;
+
+ SSIBus *spi;
+
+ uint32_t regs[ASPEED_SMC_R_MAX];
+
+ /* depends on the controller type */
+ uint8_t r_conf;
+ uint8_t r_ce_ctrl;
+ uint8_t r_ctrl0;
+ uint8_t r_timings;
+ uint8_t conf_enable_w0;
+} AspeedSMCState;
+
+#endif /* ASPEED_SMC_H */
--
2.1.4
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 6/9] ast2400: add SMC controllers (FMC and SPI)
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 6/9] ast2400: add SMC controllers (FMC and SPI) Cédric Le Goater
@ 2016-06-29 7:58 ` Cédric Le Goater
2016-07-02 17:00 ` mar.krzeminski
0 siblings, 1 reply; 33+ messages in thread
From: Cédric Le Goater @ 2016-06-29 7:58 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery
On 06/28/2016 08:24 PM, Cédric Le Goater wrote:
> The Aspeed AST2400 soc includes a static memory controller for the BMC
> which supports NOR, NAND and SPI flash memory modules. This controller
> has two modes : the SMC for the legacy interface which supports only
> one module and the FMC for the new interface which supports up to five
> modules. The AST2400 also includes a SPI only controller used for the
> host firmware, commonly called BIOS on Intel. It can be used in three
> mode : a SPI master, SPI slave and SPI pass-through
>
> Below is the initial framework for the SMC controller (FMC mode only)
> and the SPI controller: the sysbus object, MMIO for registers
> configuration and controls. Each controller has a SPI bus and a
> configurable number of CS lines for SPI flash slaves.
>
> The differences between the controllers are small, so they are
> abstracted using indirections on the register numbers.
>
> Only SPI flash modules are supported.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
> Changes since v3:
>
> - Fixed typos on CTRL
> - Fixed multiple error handling when setting properties in
> ast2400_realize()
Peter,
I missed one ... See below.
> - Added error messages when max_slaves is exceeded
> - Added definitions for R_INTR_CTRL register bits
> - Added definitions for R_DMA_* registers
> - Constantified a couple of routine arguments
> - Sorted out what was need for migration (registers only apriori)
>
> Changes since v2:
>
> - Switched to a realize ops to be able to handle errors.
>
> hw/arm/ast2400.c | 34 ++++-
> hw/ssi/Makefile.objs | 1 +
> hw/ssi/aspeed_smc.c | 326 ++++++++++++++++++++++++++++++++++++++++++++
> include/hw/arm/ast2400.h | 3 +
> include/hw/ssi/aspeed_smc.h | 79 +++++++++++
> 5 files changed, 442 insertions(+), 1 deletion(-)
> create mode 100644 hw/ssi/aspeed_smc.c
> create mode 100644 include/hw/ssi/aspeed_smc.h
>
> diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
> index b14a82fcdef1..b16ba2d0c516 100644
> --- a/hw/arm/ast2400.c
> +++ b/hw/arm/ast2400.c
> @@ -23,6 +23,9 @@
> #define AST2400_UART_5_BASE 0x00184000
> #define AST2400_IOMEM_SIZE 0x00200000
> #define AST2400_IOMEM_BASE 0x1E600000
> +#define AST2400_SMC_BASE AST2400_IOMEM_BASE /* Legacy SMC */
> +#define AST2400_FMC_BASE 0X1E620000
> +#define AST2400_SPI_BASE 0X1E630000
> #define AST2400_VIC_BASE 0x1E6C0000
> #define AST2400_SCU_BASE 0x1E6E2000
> #define AST2400_TIMER_BASE 0x1E782000
> @@ -85,13 +88,21 @@ static void ast2400_init(Object *obj)
> "hw-strap1", &error_abort);
> object_property_add_alias(obj, "hw-strap2", OBJECT(&s->scu),
> "hw-strap2", &error_abort);
> +
> + object_initialize(&s->smc, sizeof(s->smc), "aspeed.smc.fmc");
> + object_property_add_child(obj, "smc", OBJECT(&s->smc), NULL);
> + qdev_set_parent_bus(DEVICE(&s->smc), sysbus_get_default());
> +
> + object_initialize(&s->spi, sizeof(s->spi), "aspeed.smc.spi");
> + object_property_add_child(obj, "spi", OBJECT(&s->spi), NULL);
> + qdev_set_parent_bus(DEVICE(&s->spi), sysbus_get_default());
> }
>
> static void ast2400_realize(DeviceState *dev, Error **errp)
> {
> int i;
> AST2400State *s = AST2400(dev);
> - Error *err = NULL;
> + Error *err = NULL, *local_err = NULL;
>
> /* IO space */
> memory_region_init_io(&s->iomem, NULL, &ast2400_io_ops, NULL,
> @@ -147,6 +158,27 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, AST2400_I2C_BASE);
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
> qdev_get_gpio_in(DEVICE(&s->vic), 12));
> +
> + /* SMC */
> + object_property_set_int(OBJECT(&s->smc), 1, "num-cs", &err);
> + object_property_set_bool(OBJECT(&s->smc), true, "realized", &err);
It should be a '&local_err' above and it is missing a :
error_propagate(&err, local_err);
Please tell me if you want a resend.
Thanks,
C.
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->smc), 0, AST2400_FMC_BASE);
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->smc), 0,
> + qdev_get_gpio_in(DEVICE(&s->vic), 19));
> +
> + /* SPI */
> + object_property_set_int(OBJECT(&s->spi), 1, "num-cs", &err);
> + object_property_set_bool(OBJECT(&s->spi), true, "realized", &local_err);
> + error_propagate(&err, local_err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 0, AST2400_SPI_BASE);
> }
>
> static void ast2400_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/ssi/Makefile.objs b/hw/ssi/Makefile.objs
> index fcbb79ef0185..c79a8dcd86a9 100644
> --- a/hw/ssi/Makefile.objs
> +++ b/hw/ssi/Makefile.objs
> @@ -2,6 +2,7 @@ common-obj-$(CONFIG_PL022) += pl022.o
> common-obj-$(CONFIG_SSI) += ssi.o
> common-obj-$(CONFIG_XILINX_SPI) += xilinx_spi.o
> common-obj-$(CONFIG_XILINX_SPIPS) += xilinx_spips.o
> +common-obj-$(CONFIG_ASPEED_SOC) += aspeed_smc.o
>
> obj-$(CONFIG_OMAP) += omap_spi.o
> obj-$(CONFIG_IMX) += imx_spi.o
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> new file mode 100644
> index 000000000000..537635e18d64
> --- /dev/null
> +++ b/hw/ssi/aspeed_smc.c
> @@ -0,0 +1,326 @@
> +/*
> + * ASPEED AST2400 SMC Controller (SPI Flash Only)
> + *
> + * Copyright (C) 2016 IBM Corp.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/sysbus.h"
> +#include "sysemu/sysemu.h"
> +#include "qemu/log.h"
> +#include "include/qemu/error-report.h"
> +#include "exec/address-spaces.h"
> +
> +#include "hw/ssi/aspeed_smc.h"
> +
> +/* CE Type Setting Register */
> +#define R_CONF (0x00 / 4)
> +#define CONF_LEGACY_DISABLE (1 << 31)
> +#define CONF_ENABLE_W4 20
> +#define CONF_ENABLE_W3 19
> +#define CONF_ENABLE_W2 18
> +#define CONF_ENABLE_W1 17
> +#define CONF_ENABLE_W0 16
> +#define CONF_FLASH_TYPE4 9
> +#define CONF_FLASH_TYPE3 7
> +#define CONF_FLASH_TYPE2 5
> +#define CONF_FLASH_TYPE1 3
> +#define CONF_FLASH_TYPE0 1
> +
> +/* CE Control Register */
> +#define R_CE_CTRL (0x04 / 4)
> +#define CTRL_EXTENDED4 4 /* 32 bit addressing for SPI */
> +#define CTRL_EXTENDED3 3 /* 32 bit addressing for SPI */
> +#define CTRL_EXTENDED2 2 /* 32 bit addressing for SPI */
> +#define CTRL_EXTENDED1 1 /* 32 bit addressing for SPI */
> +#define CTRL_EXTENDED0 0 /* 32 bit addressing for SPI */
> +
> +/* Interrupt Control and Status Register */
> +#define R_INTR_CTRL (0x08 / 4)
> +#define INTR_CTRL_DMA_STATUS (1 << 11)
> +#define INTR_CTRL_CMD_ABORT_STATUS (1 << 10)
> +#define INTR_CTRL_WRITE_PROTECT_STATUS (1 << 9)
> +#define INTR_CTRL_DMA_EN (1 << 3)
> +#define INTR_CTRL_CMD_ABORT_EN (1 << 2)
> +#define INTR_CTRL_WRITE_PROTECT_EN (1 << 1)
> +
> +/* CEx Control Register */
> +#define R_CTRL0 (0x10 / 4)
> +#define CTRL_CMD_SHIFT 16
> +#define CTRL_CMD_MASK 0xff
> +#define CTRL_CE_STOP_ACTIVE (1 << 2)
> +#define CTRL_CMD_MODE_MASK 0x3
> +#define CTRL_READMODE 0x0
> +#define CTRL_FREADMODE 0x1
> +#define CTRL_WRITEMODE 0x2
> +#define CTRL_USERMODE 0x3
> +#define R_CTRL1 (0x14 / 4)
> +#define R_CTRL2 (0x18 / 4)
> +#define R_CTRL3 (0x1C / 4)
> +#define R_CTRL4 (0x20 / 4)
> +
> +/* CEx Segment Address Register */
> +#define R_SEG_ADDR0 (0x30 / 4)
> +#define SEG_SIZE_SHIFT 24 /* 8MB units */
> +#define SEG_SIZE_MASK 0x7f
> +#define SEG_START_SHIFT 16 /* address bit [A29-A23] */
> +#define SEG_START_MASK 0x7f
> +#define R_SEG_ADDR1 (0x34 / 4)
> +#define R_SEG_ADDR2 (0x38 / 4)
> +#define R_SEG_ADDR3 (0x3C / 4)
> +#define R_SEG_ADDR4 (0x40 / 4)
> +
> +/* Misc Control Register #1 */
> +#define R_MISC_CTRL1 (0x50 / 4)
> +
> +/* Misc Control Register #2 */
> +#define R_MISC_CTRL2 (0x54 / 4)
> +
> +/* DMA Control/Status Register */
> +#define R_DMA_CTRL (0x80 / 4)
> +#define DMA_CTRL_DELAY_MASK 0xf
> +#define DMA_CTRL_DELAY_SHIFT 8
> +#define DMA_CTRL_FREQ_MASK 0xf
> +#define DMA_CTRL_FREQ_SHIFT 4
> +#define DMA_CTRL_MODE (1 << 3)
> +#define DMA_CTRL_CKSUM (1 << 2)
> +#define DMA_CTRL_DIR (1 << 1)
> +#define DMA_CTRL_EN (1 << 0)
> +
> +/* DMA Flash Side Address */
> +#define R_DMA_FLASH_ADDR (0x84 / 4)
> +
> +/* DMA DRAM Side Address */
> +#define R_DMA_DRAM_ADDR (0x88 / 4)
> +
> +/* DMA Length Register */
> +#define R_DMA_LEN (0x8C / 4)
> +
> +/* Checksum Calculation Result */
> +#define R_DMA_CHECKSUM (0x90 / 4)
> +
> +/* Misc Control Register #2 */
> +#define R_TIMINGS (0x94 / 4)
> +
> +/* SPI controller registers and bits */
> +#define R_SPI_CONF (0x00 / 4)
> +#define SPI_CONF_ENABLE_W0 0
> +#define R_SPI_CTRL0 (0x4 / 4)
> +#define R_SPI_MISC_CTRL (0x10 / 4)
> +#define R_SPI_TIMINGS (0x14 / 4)
> +
> +static const AspeedSMCController controllers[] = {
> + { "aspeed.smc.smc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
> + CONF_ENABLE_W0, 5 },
> + { "aspeed.smc.fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
> + CONF_ENABLE_W0, 5 },
> + { "aspeed.smc.spi", R_SPI_CONF, 0xff, R_SPI_CTRL0, R_SPI_TIMINGS,
> + SPI_CONF_ENABLE_W0, 1 },
> +};
> +
> +static bool aspeed_smc_is_ce_stop_active(const AspeedSMCState *s, int cs)
> +{
> + return s->regs[s->r_ctrl0 + cs] & CTRL_CE_STOP_ACTIVE;
> +}
> +
> +static void aspeed_smc_update_cs(const AspeedSMCState *s)
> +{
> + int i;
> +
> + for (i = 0; i < s->num_cs; ++i) {
> + qemu_set_irq(s->cs_lines[i], aspeed_smc_is_ce_stop_active(s, i));
> + }
> +}
> +
> +static void aspeed_smc_reset(DeviceState *d)
> +{
> + AspeedSMCState *s = ASPEED_SMC(d);
> + int i;
> +
> + memset(s->regs, 0, sizeof s->regs);
> +
> + /* Unselect all slaves */
> + for (i = 0; i < s->num_cs; ++i) {
> + s->regs[s->r_ctrl0 + i] |= CTRL_CE_STOP_ACTIVE;
> + }
> +
> + aspeed_smc_update_cs(s);
> +}
> +
> +static bool aspeed_smc_is_implemented(AspeedSMCState *s, hwaddr addr)
> +{
> + return (addr == s->r_conf || addr == s->r_timings || addr == s->r_ce_ctrl ||
> + (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs));
> +}
> +
> +static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> + AspeedSMCState *s = ASPEED_SMC(opaque);
> +
> + addr >>= 2;
> +
> + if (addr >= ARRAY_SIZE(s->regs)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Out-of-bounds read at 0x%" HWADDR_PRIx "\n",
> + __func__, addr);
> + return 0;
> + }
> +
> + if (!aspeed_smc_is_implemented(s, addr)) {
> + qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
> + __func__, addr);
> + return 0;
> + }
> +
> + return s->regs[addr];
> +}
> +
> +static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
> + unsigned int size)
> +{
> + AspeedSMCState *s = ASPEED_SMC(opaque);
> + uint32_t value = data;
> +
> + addr >>= 2;
> +
> + if (addr >= ARRAY_SIZE(s->regs)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Out-of-bounds write at 0x%" HWADDR_PRIx "\n",
> + __func__, addr);
> + return;
> + }
> +
> + if (!aspeed_smc_is_implemented(s, addr)) {
> + qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
> + __func__, addr);
> + return;
> + }
> +
> + /*
> + * Not much to do apart from storing the value and set the cs
> + * lines if the register is a controlling one.
> + */
> + s->regs[addr] = value;
> + if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs) {
> + aspeed_smc_update_cs(s);
> + }
> +}
> +
> +static const MemoryRegionOps aspeed_smc_ops = {
> + .read = aspeed_smc_read,
> + .write = aspeed_smc_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> + .valid.unaligned = true,
> +};
> +
> +static void aspeed_smc_realize(DeviceState *dev, Error **errp)
> +{
> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> + AspeedSMCState *s = ASPEED_SMC(dev);
> + AspeedSMCClass *mc = ASPEED_SMC_GET_CLASS(s);
> + int i;
> +
> + s->ctrl = mc->ctrl;
> +
> + /* keep a copy under AspeedSMCState to speed up accesses */
> + s->r_conf = s->ctrl->r_conf;
> + s->r_ce_ctrl = s->ctrl->r_ce_ctrl;
> + s->r_ctrl0 = s->ctrl->r_ctrl0;
> + s->r_timings = s->ctrl->r_timings;
> + s->conf_enable_w0 = s->ctrl->conf_enable_w0;
> +
> + /* Enforce some real HW limits */
> + if (s->num_cs > s->ctrl->max_slaves) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: num_cs cannot exceed: %d\n",
> + __func__, s->ctrl->max_slaves);
> + s->num_cs = s->ctrl->max_slaves;
> + }
> +
> + s->spi = ssi_create_bus(dev, "spi");
> +
> + /* Setup cs_lines for slaves */
> + sysbus_init_irq(sbd, &s->irq);
> + s->cs_lines = g_new0(qemu_irq, s->num_cs);
> + ssi_auto_connect_slaves(dev, s->cs_lines, s->spi);
> +
> + for (i = 0; i < s->num_cs; ++i) {
> + sysbus_init_irq(sbd, &s->cs_lines[i]);
> + }
> +
> + aspeed_smc_reset(dev);
> +
> + memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s,
> + s->ctrl->name, ASPEED_SMC_R_MAX * 4);
> + sysbus_init_mmio(sbd, &s->mmio);
> +}
> +
> +static const VMStateDescription vmstate_aspeed_smc = {
> + .name = "aspeed.smc",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32_ARRAY(regs, AspeedSMCState, ASPEED_SMC_R_MAX),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static Property aspeed_smc_properties[] = {
> + DEFINE_PROP_UINT32("num-cs", AspeedSMCState, num_cs, 1),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void aspeed_smc_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + AspeedSMCClass *mc = ASPEED_SMC_CLASS(klass);
> +
> + dc->realize = aspeed_smc_realize;
> + dc->reset = aspeed_smc_reset;
> + dc->props = aspeed_smc_properties;
> + dc->vmsd = &vmstate_aspeed_smc;
> + mc->ctrl = data;
> +}
> +
> +static const TypeInfo aspeed_smc_info = {
> + .name = TYPE_ASPEED_SMC,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(AspeedSMCState),
> + .class_size = sizeof(AspeedSMCClass),
> + .abstract = true,
> +};
> +
> +static void aspeed_smc_register_types(void)
> +{
> + int i;
> +
> + type_register_static(&aspeed_smc_info);
> + for (i = 0; i < ARRAY_SIZE(controllers); ++i) {
> + TypeInfo ti = {
> + .name = controllers[i].name,
> + .parent = TYPE_ASPEED_SMC,
> + .class_init = aspeed_smc_class_init,
> + .class_data = (void *)&controllers[i],
> + };
> + type_register(&ti);
> + }
> +}
> +
> +type_init(aspeed_smc_register_types)
> diff --git a/include/hw/arm/ast2400.h b/include/hw/arm/ast2400.h
> index f1a64fd3893d..7833bc716cd8 100644
> --- a/include/hw/arm/ast2400.h
> +++ b/include/hw/arm/ast2400.h
> @@ -17,6 +17,7 @@
> #include "hw/misc/aspeed_scu.h"
> #include "hw/timer/aspeed_timer.h"
> #include "hw/i2c/aspeed_i2c.h"
> +#include "hw/ssi/aspeed_smc.h"
>
> typedef struct AST2400State {
> /*< private >*/
> @@ -29,6 +30,8 @@ typedef struct AST2400State {
> AspeedTimerCtrlState timerctrl;
> AspeedI2CState i2c;
> AspeedSCUState scu;
> + AspeedSMCState smc;
> + AspeedSMCState spi;
> } AST2400State;
>
> #define TYPE_AST2400 "ast2400"
> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
> new file mode 100644
> index 000000000000..c4a4960cd880
> --- /dev/null
> +++ b/include/hw/ssi/aspeed_smc.h
> @@ -0,0 +1,79 @@
> +/*
> + * ASPEED AST2400 SMC Controller (SPI Flash Only)
> + *
> + * Copyright (C) 2016 IBM Corp.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#ifndef ASPEED_SMC_H
> +#define ASPEED_SMC_H
> +
> +#include "hw/ssi/ssi.h"
> +
> +typedef struct AspeedSMCController {
> + const char *name;
> + uint8_t r_conf;
> + uint8_t r_ce_ctrl;
> + uint8_t r_ctrl0;
> + uint8_t r_timings;
> + uint8_t conf_enable_w0;
> + uint8_t max_slaves;
> +} AspeedSMCController;
> +
> +#define TYPE_ASPEED_SMC "aspeed.smc"
> +#define ASPEED_SMC(obj) OBJECT_CHECK(AspeedSMCState, (obj), TYPE_ASPEED_SMC)
> +#define ASPEED_SMC_CLASS(klass) \
> + OBJECT_CLASS_CHECK(AspeedSMCClass, (klass), TYPE_ASPEED_SMC)
> +#define ASPEED_SMC_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(AspeedSMCClass, (obj), TYPE_ASPEED_SMC)
> +
> +typedef struct AspeedSMCClass {
> + SysBusDevice parent_obj;
> + const AspeedSMCController *ctrl;
> +} AspeedSMCClass;
> +
> +#define ASPEED_SMC_R_MAX (0x100 / 4)
> +
> +typedef struct AspeedSMCState {
> + SysBusDevice parent_obj;
> +
> + const AspeedSMCController *ctrl;
> +
> + MemoryRegion mmio;
> +
> + qemu_irq irq;
> + int irqline;
> +
> + uint32_t num_cs;
> + qemu_irq *cs_lines;
> +
> + SSIBus *spi;
> +
> + uint32_t regs[ASPEED_SMC_R_MAX];
> +
> + /* depends on the controller type */
> + uint8_t r_conf;
> + uint8_t r_ce_ctrl;
> + uint8_t r_ctrl0;
> + uint8_t r_timings;
> + uint8_t conf_enable_w0;
> +} AspeedSMCState;
> +
> +#endif /* ASPEED_SMC_H */
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 6/9] ast2400: add SMC controllers (FMC and SPI)
2016-06-29 7:58 ` Cédric Le Goater
@ 2016-07-02 17:00 ` mar.krzeminski
2016-07-04 6:58 ` Cédric Le Goater
0 siblings, 1 reply; 33+ messages in thread
From: mar.krzeminski @ 2016-07-02 17:00 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Peter Crosthwaite
Cc: kwolf, Andrew Jeffery, qemu-arm, qemu-devel, armbru
W dniu 29.06.2016 o 09:58, Cédric Le Goater pisze:
> On 06/28/2016 08:24 PM, Cédric Le Goater wrote:
>> The Aspeed AST2400 soc includes a static memory controller for the BMC
>> which supports NOR, NAND and SPI flash memory modules. This controller
>> has two modes : the SMC for the legacy interface which supports only
>> one module and the FMC for the new interface which supports up to five
>> modules. The AST2400 also includes a SPI only controller used for the
>> host firmware, commonly called BIOS on Intel. It can be used in three
>> mode : a SPI master, SPI slave and SPI pass-through
>>
>> Below is the initial framework for the SMC controller (FMC mode only)
>> and the SPI controller: the sysbus object, MMIO for registers
>> configuration and controls. Each controller has a SPI bus and a
>> configurable number of CS lines for SPI flash slaves.
>>
>> The differences between the controllers are small, so they are
>> abstracted using indirections on the register numbers.
>>
>> Only SPI flash modules are supported.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>> Changes since v3:
>>
>> - Fixed typos on CTRL
>> - Fixed multiple error handling when setting properties in
>> ast2400_realize()
> Peter,
>
> I missed one ... See below.
>
>> - Added error messages when max_slaves is exceeded
>> - Added definitions for R_INTR_CTRL register bits
>> - Added definitions for R_DMA_* registers
>> - Constantified a couple of routine arguments
>> - Sorted out what was need for migration (registers only apriori)
>>
>> Changes since v2:
>>
>> - Switched to a realize ops to be able to handle errors.
>>
>> hw/arm/ast2400.c | 34 ++++-
>> hw/ssi/Makefile.objs | 1 +
>> hw/ssi/aspeed_smc.c | 326 ++++++++++++++++++++++++++++++++++++++++++++
>> include/hw/arm/ast2400.h | 3 +
>> include/hw/ssi/aspeed_smc.h | 79 +++++++++++
>> 5 files changed, 442 insertions(+), 1 deletion(-)
>> create mode 100644 hw/ssi/aspeed_smc.c
>> create mode 100644 include/hw/ssi/aspeed_smc.h
>>
>> diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
>> index b14a82fcdef1..b16ba2d0c516 100644
>> --- a/hw/arm/ast2400.c
>> +++ b/hw/arm/ast2400.c
>> @@ -23,6 +23,9 @@
>> #define AST2400_UART_5_BASE 0x00184000
>> #define AST2400_IOMEM_SIZE 0x00200000
>> #define AST2400_IOMEM_BASE 0x1E600000
>> +#define AST2400_SMC_BASE AST2400_IOMEM_BASE /* Legacy SMC */
>> +#define AST2400_FMC_BASE 0X1E620000
>> +#define AST2400_SPI_BASE 0X1E630000
>> #define AST2400_VIC_BASE 0x1E6C0000
>> #define AST2400_SCU_BASE 0x1E6E2000
>> #define AST2400_TIMER_BASE 0x1E782000
>> @@ -85,13 +88,21 @@ static void ast2400_init(Object *obj)
>> "hw-strap1", &error_abort);
>> object_property_add_alias(obj, "hw-strap2", OBJECT(&s->scu),
>> "hw-strap2", &error_abort);
>> +
>> + object_initialize(&s->smc, sizeof(s->smc), "aspeed.smc.fmc");
>> + object_property_add_child(obj, "smc", OBJECT(&s->smc), NULL);
>> + qdev_set_parent_bus(DEVICE(&s->smc), sysbus_get_default());
>> +
>> + object_initialize(&s->spi, sizeof(s->spi), "aspeed.smc.spi");
>> + object_property_add_child(obj, "spi", OBJECT(&s->spi), NULL);
>> + qdev_set_parent_bus(DEVICE(&s->spi), sysbus_get_default());
>> }
>>
>> static void ast2400_realize(DeviceState *dev, Error **errp)
>> {
>> int i;
>> AST2400State *s = AST2400(dev);
>> - Error *err = NULL;
>> + Error *err = NULL, *local_err = NULL;
>>
>> /* IO space */
>> memory_region_init_io(&s->iomem, NULL, &ast2400_io_ops, NULL,
>> @@ -147,6 +158,27 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
>> sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, AST2400_I2C_BASE);
>> sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
>> qdev_get_gpio_in(DEVICE(&s->vic), 12));
>> +
>> + /* SMC */
>> + object_property_set_int(OBJECT(&s->smc), 1, "num-cs", &err);
>> + object_property_set_bool(OBJECT(&s->smc), true, "realized", &err);
>
> It should be a '&local_err' above and it is missing a :
>
> error_propagate(&err, local_err);
>
> Please tell me if you want a resend.
>
> Thanks,
>
> C.
>
>> + if (err) {
>> + error_propagate(errp, err);
>> + return;
>> + }
>> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->smc), 0, AST2400_FMC_BASE);
>> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->smc), 0,
>> + qdev_get_gpio_in(DEVICE(&s->vic), 19));
>> +
>> + /* SPI */
>> + object_property_set_int(OBJECT(&s->spi), 1, "num-cs", &err);
>> + object_property_set_bool(OBJECT(&s->spi), true, "realized", &local_err);
>> + error_propagate(&err, local_err);
>> + if (err) {
>> + error_propagate(errp, err);
>> + return;
>> + }
>> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 0, AST2400_SPI_BASE);
>> }
>>
>> static void ast2400_class_init(ObjectClass *oc, void *data)
>> diff --git a/hw/ssi/Makefile.objs b/hw/ssi/Makefile.objs
>> index fcbb79ef0185..c79a8dcd86a9 100644
>> --- a/hw/ssi/Makefile.objs
>> +++ b/hw/ssi/Makefile.objs
>> @@ -2,6 +2,7 @@ common-obj-$(CONFIG_PL022) += pl022.o
>> common-obj-$(CONFIG_SSI) += ssi.o
>> common-obj-$(CONFIG_XILINX_SPI) += xilinx_spi.o
>> common-obj-$(CONFIG_XILINX_SPIPS) += xilinx_spips.o
>> +common-obj-$(CONFIG_ASPEED_SOC) += aspeed_smc.o
>>
>> obj-$(CONFIG_OMAP) += omap_spi.o
>> obj-$(CONFIG_IMX) += imx_spi.o
>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
>> new file mode 100644
>> index 000000000000..537635e18d64
>> --- /dev/null
>> +++ b/hw/ssi/aspeed_smc.c
>> @@ -0,0 +1,326 @@
>> +/*
>> + * ASPEED AST2400 SMC Controller (SPI Flash Only)
>> + *
>> + * Copyright (C) 2016 IBM Corp.
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to deal
>> + * in the Software without restriction, including without limitation the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/sysbus.h"
>> +#include "sysemu/sysemu.h"
>> +#include "qemu/log.h"
>> +#include "include/qemu/error-report.h"
>> +#include "exec/address-spaces.h"
>> +
>> +#include "hw/ssi/aspeed_smc.h"
>> +
>> +/* CE Type Setting Register */
>> +#define R_CONF (0x00 / 4)
>> +#define CONF_LEGACY_DISABLE (1 << 31)
>> +#define CONF_ENABLE_W4 20
>> +#define CONF_ENABLE_W3 19
>> +#define CONF_ENABLE_W2 18
>> +#define CONF_ENABLE_W1 17
>> +#define CONF_ENABLE_W0 16
>> +#define CONF_FLASH_TYPE4 9
>> +#define CONF_FLASH_TYPE3 7
>> +#define CONF_FLASH_TYPE2 5
>> +#define CONF_FLASH_TYPE1 3
>> +#define CONF_FLASH_TYPE0 1
>> +
>> +/* CE Control Register */
>> +#define R_CE_CTRL (0x04 / 4)
>> +#define CTRL_EXTENDED4 4 /* 32 bit addressing for SPI */
>> +#define CTRL_EXTENDED3 3 /* 32 bit addressing for SPI */
>> +#define CTRL_EXTENDED2 2 /* 32 bit addressing for SPI */
>> +#define CTRL_EXTENDED1 1 /* 32 bit addressing for SPI */
>> +#define CTRL_EXTENDED0 0 /* 32 bit addressing for SPI */
Hi
Above comments does not say anything.
Regards,
Marcin
>> +
>> +/* Interrupt Control and Status Register */
>> +#define R_INTR_CTRL (0x08 / 4)
>> +#define INTR_CTRL_DMA_STATUS (1 << 11)
>> +#define INTR_CTRL_CMD_ABORT_STATUS (1 << 10)
>> +#define INTR_CTRL_WRITE_PROTECT_STATUS (1 << 9)
>> +#define INTR_CTRL_DMA_EN (1 << 3)
>> +#define INTR_CTRL_CMD_ABORT_EN (1 << 2)
>> +#define INTR_CTRL_WRITE_PROTECT_EN (1 << 1)
>> +
>> +/* CEx Control Register */
>> +#define R_CTRL0 (0x10 / 4)
>> +#define CTRL_CMD_SHIFT 16
>> +#define CTRL_CMD_MASK 0xff
>> +#define CTRL_CE_STOP_ACTIVE (1 << 2)
>> +#define CTRL_CMD_MODE_MASK 0x3
>> +#define CTRL_READMODE 0x0
>> +#define CTRL_FREADMODE 0x1
>> +#define CTRL_WRITEMODE 0x2
>> +#define CTRL_USERMODE 0x3
>> +#define R_CTRL1 (0x14 / 4)
>> +#define R_CTRL2 (0x18 / 4)
>> +#define R_CTRL3 (0x1C / 4)
>> +#define R_CTRL4 (0x20 / 4)
>> +
>> +/* CEx Segment Address Register */
>> +#define R_SEG_ADDR0 (0x30 / 4)
>> +#define SEG_SIZE_SHIFT 24 /* 8MB units */
>> +#define SEG_SIZE_MASK 0x7f
>> +#define SEG_START_SHIFT 16 /* address bit [A29-A23] */
>> +#define SEG_START_MASK 0x7f
>> +#define R_SEG_ADDR1 (0x34 / 4)
>> +#define R_SEG_ADDR2 (0x38 / 4)
>> +#define R_SEG_ADDR3 (0x3C / 4)
>> +#define R_SEG_ADDR4 (0x40 / 4)
>> +
>> +/* Misc Control Register #1 */
>> +#define R_MISC_CTRL1 (0x50 / 4)
>> +
>> +/* Misc Control Register #2 */
>> +#define R_MISC_CTRL2 (0x54 / 4)
>> +
>> +/* DMA Control/Status Register */
>> +#define R_DMA_CTRL (0x80 / 4)
>> +#define DMA_CTRL_DELAY_MASK 0xf
>> +#define DMA_CTRL_DELAY_SHIFT 8
>> +#define DMA_CTRL_FREQ_MASK 0xf
>> +#define DMA_CTRL_FREQ_SHIFT 4
>> +#define DMA_CTRL_MODE (1 << 3)
>> +#define DMA_CTRL_CKSUM (1 << 2)
>> +#define DMA_CTRL_DIR (1 << 1)
>> +#define DMA_CTRL_EN (1 << 0)
>> +
>> +/* DMA Flash Side Address */
>> +#define R_DMA_FLASH_ADDR (0x84 / 4)
>> +
>> +/* DMA DRAM Side Address */
>> +#define R_DMA_DRAM_ADDR (0x88 / 4)
>> +
>> +/* DMA Length Register */
>> +#define R_DMA_LEN (0x8C / 4)
>> +
>> +/* Checksum Calculation Result */
>> +#define R_DMA_CHECKSUM (0x90 / 4)
>> +
>> +/* Misc Control Register #2 */
>> +#define R_TIMINGS (0x94 / 4)
>> +
>> +/* SPI controller registers and bits */
>> +#define R_SPI_CONF (0x00 / 4)
>> +#define SPI_CONF_ENABLE_W0 0
>> +#define R_SPI_CTRL0 (0x4 / 4)
>> +#define R_SPI_MISC_CTRL (0x10 / 4)
>> +#define R_SPI_TIMINGS (0x14 / 4)
>> +
>> +static const AspeedSMCController controllers[] = {
>> + { "aspeed.smc.smc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
>> + CONF_ENABLE_W0, 5 },
>> + { "aspeed.smc.fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
>> + CONF_ENABLE_W0, 5 },
>> + { "aspeed.smc.spi", R_SPI_CONF, 0xff, R_SPI_CTRL0, R_SPI_TIMINGS,
>> + SPI_CONF_ENABLE_W0, 1 },
>> +};
>> +
>> +static bool aspeed_smc_is_ce_stop_active(const AspeedSMCState *s, int cs)
>> +{
>> + return s->regs[s->r_ctrl0 + cs] & CTRL_CE_STOP_ACTIVE;
>> +}
>> +
>> +static void aspeed_smc_update_cs(const AspeedSMCState *s)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < s->num_cs; ++i) {
>> + qemu_set_irq(s->cs_lines[i], aspeed_smc_is_ce_stop_active(s, i));
>> + }
>> +}
>> +
>> +static void aspeed_smc_reset(DeviceState *d)
>> +{
>> + AspeedSMCState *s = ASPEED_SMC(d);
>> + int i;
>> +
>> + memset(s->regs, 0, sizeof s->regs);
>> +
>> + /* Unselect all slaves */
>> + for (i = 0; i < s->num_cs; ++i) {
>> + s->regs[s->r_ctrl0 + i] |= CTRL_CE_STOP_ACTIVE;
>> + }
>> +
>> + aspeed_smc_update_cs(s);
>> +}
>> +
>> +static bool aspeed_smc_is_implemented(AspeedSMCState *s, hwaddr addr)
>> +{
>> + return (addr == s->r_conf || addr == s->r_timings || addr == s->r_ce_ctrl ||
>> + (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs));
>> +}
>> +
>> +static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
>> +{
>> + AspeedSMCState *s = ASPEED_SMC(opaque);
>> +
>> + addr >>= 2;
>> +
>> + if (addr >= ARRAY_SIZE(s->regs)) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Out-of-bounds read at 0x%" HWADDR_PRIx "\n",
>> + __func__, addr);
>> + return 0;
>> + }
>> +
>> + if (!aspeed_smc_is_implemented(s, addr)) {
>> + qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
>> + __func__, addr);
>> + return 0;
>> + }
>> +
>> + return s->regs[addr];
>> +}
>> +
>> +static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
>> + unsigned int size)
>> +{
>> + AspeedSMCState *s = ASPEED_SMC(opaque);
>> + uint32_t value = data;
>> +
>> + addr >>= 2;
>> +
>> + if (addr >= ARRAY_SIZE(s->regs)) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Out-of-bounds write at 0x%" HWADDR_PRIx "\n",
>> + __func__, addr);
>> + return;
>> + }
>> +
>> + if (!aspeed_smc_is_implemented(s, addr)) {
>> + qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
>> + __func__, addr);
>> + return;
>> + }
>> +
>> + /*
>> + * Not much to do apart from storing the value and set the cs
>> + * lines if the register is a controlling one.
>> + */
>> + s->regs[addr] = value;
>> + if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs) {
>> + aspeed_smc_update_cs(s);
>> + }
>> +}
>> +
>> +static const MemoryRegionOps aspeed_smc_ops = {
>> + .read = aspeed_smc_read,
>> + .write = aspeed_smc_write,
>> + .endianness = DEVICE_LITTLE_ENDIAN,
>> + .valid.unaligned = true,
>> +};
>> +
>> +static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>> +{
>> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> + AspeedSMCState *s = ASPEED_SMC(dev);
>> + AspeedSMCClass *mc = ASPEED_SMC_GET_CLASS(s);
>> + int i;
>> +
>> + s->ctrl = mc->ctrl;
>> +
>> + /* keep a copy under AspeedSMCState to speed up accesses */
>> + s->r_conf = s->ctrl->r_conf;
>> + s->r_ce_ctrl = s->ctrl->r_ce_ctrl;
>> + s->r_ctrl0 = s->ctrl->r_ctrl0;
>> + s->r_timings = s->ctrl->r_timings;
>> + s->conf_enable_w0 = s->ctrl->conf_enable_w0;
>> +
>> + /* Enforce some real HW limits */
>> + if (s->num_cs > s->ctrl->max_slaves) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: num_cs cannot exceed: %d\n",
>> + __func__, s->ctrl->max_slaves);
>> + s->num_cs = s->ctrl->max_slaves;
>> + }
>> +
>> + s->spi = ssi_create_bus(dev, "spi");
>> +
>> + /* Setup cs_lines for slaves */
>> + sysbus_init_irq(sbd, &s->irq);
>> + s->cs_lines = g_new0(qemu_irq, s->num_cs);
>> + ssi_auto_connect_slaves(dev, s->cs_lines, s->spi);
>> +
>> + for (i = 0; i < s->num_cs; ++i) {
>> + sysbus_init_irq(sbd, &s->cs_lines[i]);
>> + }
>> +
>> + aspeed_smc_reset(dev);
>> +
>> + memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s,
>> + s->ctrl->name, ASPEED_SMC_R_MAX * 4);
>> + sysbus_init_mmio(sbd, &s->mmio);
>> +}
>> +
>> +static const VMStateDescription vmstate_aspeed_smc = {
>> + .name = "aspeed.smc",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_UINT32_ARRAY(regs, AspeedSMCState, ASPEED_SMC_R_MAX),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> +static Property aspeed_smc_properties[] = {
>> + DEFINE_PROP_UINT32("num-cs", AspeedSMCState, num_cs, 1),
>> + DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void aspeed_smc_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> + AspeedSMCClass *mc = ASPEED_SMC_CLASS(klass);
>> +
>> + dc->realize = aspeed_smc_realize;
>> + dc->reset = aspeed_smc_reset;
>> + dc->props = aspeed_smc_properties;
>> + dc->vmsd = &vmstate_aspeed_smc;
>> + mc->ctrl = data;
>> +}
>> +
>> +static const TypeInfo aspeed_smc_info = {
>> + .name = TYPE_ASPEED_SMC,
>> + .parent = TYPE_SYS_BUS_DEVICE,
>> + .instance_size = sizeof(AspeedSMCState),
>> + .class_size = sizeof(AspeedSMCClass),
>> + .abstract = true,
>> +};
>> +
>> +static void aspeed_smc_register_types(void)
>> +{
>> + int i;
>> +
>> + type_register_static(&aspeed_smc_info);
>> + for (i = 0; i < ARRAY_SIZE(controllers); ++i) {
>> + TypeInfo ti = {
>> + .name = controllers[i].name,
>> + .parent = TYPE_ASPEED_SMC,
>> + .class_init = aspeed_smc_class_init,
>> + .class_data = (void *)&controllers[i],
>> + };
>> + type_register(&ti);
>> + }
>> +}
>> +
>> +type_init(aspeed_smc_register_types)
>> diff --git a/include/hw/arm/ast2400.h b/include/hw/arm/ast2400.h
>> index f1a64fd3893d..7833bc716cd8 100644
>> --- a/include/hw/arm/ast2400.h
>> +++ b/include/hw/arm/ast2400.h
>> @@ -17,6 +17,7 @@
>> #include "hw/misc/aspeed_scu.h"
>> #include "hw/timer/aspeed_timer.h"
>> #include "hw/i2c/aspeed_i2c.h"
>> +#include "hw/ssi/aspeed_smc.h"
>>
>> typedef struct AST2400State {
>> /*< private >*/
>> @@ -29,6 +30,8 @@ typedef struct AST2400State {
>> AspeedTimerCtrlState timerctrl;
>> AspeedI2CState i2c;
>> AspeedSCUState scu;
>> + AspeedSMCState smc;
>> + AspeedSMCState spi;
>> } AST2400State;
>>
>> #define TYPE_AST2400 "ast2400"
>> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
>> new file mode 100644
>> index 000000000000..c4a4960cd880
>> --- /dev/null
>> +++ b/include/hw/ssi/aspeed_smc.h
>> @@ -0,0 +1,79 @@
>> +/*
>> + * ASPEED AST2400 SMC Controller (SPI Flash Only)
>> + *
>> + * Copyright (C) 2016 IBM Corp.
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to deal
>> + * in the Software without restriction, including without limitation the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#ifndef ASPEED_SMC_H
>> +#define ASPEED_SMC_H
>> +
>> +#include "hw/ssi/ssi.h"
>> +
>> +typedef struct AspeedSMCController {
>> + const char *name;
>> + uint8_t r_conf;
>> + uint8_t r_ce_ctrl;
>> + uint8_t r_ctrl0;
>> + uint8_t r_timings;
>> + uint8_t conf_enable_w0;
>> + uint8_t max_slaves;
>> +} AspeedSMCController;
>> +
>> +#define TYPE_ASPEED_SMC "aspeed.smc"
>> +#define ASPEED_SMC(obj) OBJECT_CHECK(AspeedSMCState, (obj), TYPE_ASPEED_SMC)
>> +#define ASPEED_SMC_CLASS(klass) \
>> + OBJECT_CLASS_CHECK(AspeedSMCClass, (klass), TYPE_ASPEED_SMC)
>> +#define ASPEED_SMC_GET_CLASS(obj) \
>> + OBJECT_GET_CLASS(AspeedSMCClass, (obj), TYPE_ASPEED_SMC)
>> +
>> +typedef struct AspeedSMCClass {
>> + SysBusDevice parent_obj;
>> + const AspeedSMCController *ctrl;
>> +} AspeedSMCClass;
>> +
>> +#define ASPEED_SMC_R_MAX (0x100 / 4)
>> +
>> +typedef struct AspeedSMCState {
>> + SysBusDevice parent_obj;
>> +
>> + const AspeedSMCController *ctrl;
>> +
>> + MemoryRegion mmio;
>> +
>> + qemu_irq irq;
>> + int irqline;
>> +
>> + uint32_t num_cs;
>> + qemu_irq *cs_lines;
>> +
>> + SSIBus *spi;
>> +
>> + uint32_t regs[ASPEED_SMC_R_MAX];
>> +
>> + /* depends on the controller type */
>> + uint8_t r_conf;
>> + uint8_t r_ce_ctrl;
>> + uint8_t r_ctrl0;
>> + uint8_t r_timings;
>> + uint8_t conf_enable_w0;
>> +} AspeedSMCState;
>> +
>> +#endif /* ASPEED_SMC_H */
>>
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 6/9] ast2400: add SMC controllers (FMC and SPI)
2016-07-02 17:00 ` mar.krzeminski
@ 2016-07-04 6:58 ` Cédric Le Goater
2016-07-04 7:12 ` Marcin Krzemiński
0 siblings, 1 reply; 33+ messages in thread
From: Cédric Le Goater @ 2016-07-04 6:58 UTC (permalink / raw)
To: mar.krzeminski, Peter Maydell, Peter Crosthwaite
Cc: kwolf, Andrew Jeffery, qemu-arm, qemu-devel, armbru
Hello Marcin,
On 07/02/2016 07:00 PM, mar.krzeminski wrote:
>>
>> +
>> +/* CE Control Register */
>> +#define R_CE_CTRL (0x04 / 4)
>> +#define CTRL_EXTENDED4 4 /* 32 bit addressing for SPI */
>> +#define CTRL_EXTENDED3 3 /* 32 bit addressing for SPI */
>> +#define CTRL_EXTENDED2 2 /* 32 bit addressing for SPI */
>> +#define CTRL_EXTENDED1 1 /* 32 bit addressing for SPI */
>> +#define CTRL_EXTENDED0 0 /* 32 bit addressing for SPI */
> Hi
>
> Above comments does not say anything.
>
I suppose you mean that the names are confusing ?
The default setting for CE0 and CE1 is done with the hardware strapping
SCU70 register.
Thanks,
C.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v5 6/9] ast2400: add SMC controllers (FMC and SPI)
2016-07-04 6:58 ` Cédric Le Goater
@ 2016-07-04 7:12 ` Marcin Krzemiński
0 siblings, 0 replies; 33+ messages in thread
From: Marcin Krzemiński @ 2016-07-04 7:12 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Maydell, Peter Crosthwaite, kwolf, Andrew Jeffery, qemu-arm,
qemu-devel@nongnu.org Developers, armbru
2016-07-04 8:58 GMT+02:00 Cédric Le Goater <clg@kaod.org>:
> Hello Marcin,
>
> On 07/02/2016 07:00 PM, mar.krzeminski wrote:
> >>
> >> +
> >> +/* CE Control Register */
> >> +#define R_CE_CTRL (0x04 / 4)
> >> +#define CTRL_EXTENDED4 4 /* 32 bit addressing for SPI */
> >> +#define CTRL_EXTENDED3 3 /* 32 bit addressing for SPI */
> >> +#define CTRL_EXTENDED2 2 /* 32 bit addressing for SPI */
> >> +#define CTRL_EXTENDED1 1 /* 32 bit addressing for SPI */
> >> +#define CTRL_EXTENDED0 0 /* 32 bit addressing for SPI */
> > Hi
> >
> > Above comments does not say anything.
> >
>
> I suppose you mean that the names are confusing ?
>
> The default setting for CE0 and CE1 is done with the hardware strapping
> SCU70 register.
>
> Yes, is hard to say what does it mean and what it does (at least for me).
This is a minor.
Thanks,
Marcin
Thanks,
>
> C.
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v5 7/9] ast2400: add SPI flash slaves
2016-06-28 18:24 [Qemu-devel] [PATCH v5 0/9] ast2400: SMC controllers Cédric Le Goater
` (5 preceding siblings ...)
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 6/9] ast2400: add SMC controllers (FMC and SPI) Cédric Le Goater
@ 2016-06-28 18:24 ` Cédric Le Goater
2016-07-01 16:24 ` Peter Maydell
2016-07-02 17:08 ` mar.krzeminski
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 8/9] ast2400: create " Cédric Le Goater
` (2 subsequent siblings)
9 siblings, 2 replies; 33+ messages in thread
From: Cédric Le Goater @ 2016-06-28 18:24 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
Cédric Le Goater
Each controller on the ast2400 has a memory range on which it maps its
flash module slaves. Each slave is assigned a memory segment for its
mapping that can be changed at bootime with the Segment Address
Register. This is not supported in the current implementation so we
are using the defaults provided by the specs.
Each SPI flash slave can then be accessed in two modes: Command and
User. When in User mode, accesses to the memory segment of the slaves
are translated in SPI transfers. When in Command mode, the HW
generates the SPI commands automatically and the memory segment is
accessed as if doing a MMIO. Other SPI controllers call that mode
linear addressing mode.
For this purpose, we are adding below each crontoller an array of
structs gathering for each SPI flash module, a segment rank, a
MemoryRegion to handle the memory accesses and the associated SPI
slave device, which should be a m25p80.
Only the User mode is supported for now but we are preparing ground
for the Command mode. The framework is sufficient to support Linux.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
Changes since v3:
- Fixed error logging
- Constantified a couple of routine arguments
- Sorted out what was need for migration (nothing apriori, but flash
module object will need some care)
- Reworked the structures handling the IOs on the flash modules :
. Replaced the SysBusDevice object with a simple struct
. Added a global memory region for the flash memory address space
. Added sub regions for each segment
. Moved mapping in SOC initialization
- Rephrased commit log
hw/arm/ast2400.c | 5 ++
hw/ssi/aspeed_smc.c | 150 +++++++++++++++++++++++++++++++++++++++++++-
include/hw/ssi/aspeed_smc.h | 21 +++++++
3 files changed, 173 insertions(+), 3 deletions(-)
diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
index b16ba2d0c516..9c30b45f87a9 100644
--- a/hw/arm/ast2400.c
+++ b/hw/arm/ast2400.c
@@ -31,6 +31,9 @@
#define AST2400_TIMER_BASE 0x1E782000
#define AST2400_I2C_BASE 0x1E78A000
+#define AST2400_FMC_FLASH_BASE 0x20000000
+#define AST2400_SPI_FLASH_BASE 0x30000000
+
#define AST2400_A0_SILICON_REV 0x02000303
static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
@@ -167,6 +170,7 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
return;
}
sysbus_mmio_map(SYS_BUS_DEVICE(&s->smc), 0, AST2400_FMC_BASE);
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->smc), 1, AST2400_FMC_FLASH_BASE);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->smc), 0,
qdev_get_gpio_in(DEVICE(&s->vic), 19));
@@ -179,6 +183,7 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
return;
}
sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 0, AST2400_SPI_BASE);
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 1, AST2400_SPI_FLASH_BASE);
}
static void ast2400_class_init(ObjectClass *oc, void *data)
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 537635e18d64..cb0b23750bcf 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -127,13 +127,129 @@
#define R_SPI_MISC_CTRL (0x10 / 4)
#define R_SPI_TIMINGS (0x14 / 4)
+/*
+ * Default segments mapping addresses and size for each slave per
+ * controller. These can be changed when board is initialized with the
+ * Segment Address Registers but they don't seem do be used on the
+ * field.
+ */
+static const AspeedSegments aspeed_segments_legacy[] = {
+ { 0x10000000, 32 * 1024 * 1024 },
+};
+
+static const AspeedSegments aspeed_segments_fmc[] = {
+ { 0x20000000, 64 * 1024 * 1024 },
+ { 0x24000000, 32 * 1024 * 1024 },
+ { 0x26000000, 32 * 1024 * 1024 },
+ { 0x28000000, 32 * 1024 * 1024 },
+ { 0x2A000000, 32 * 1024 * 1024 }
+};
+
+static const AspeedSegments aspeed_segments_spi[] = {
+ { 0x30000000, 64 * 1024 * 1024 },
+};
+
static const AspeedSMCController controllers[] = {
{ "aspeed.smc.smc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
- CONF_ENABLE_W0, 5 },
+ CONF_ENABLE_W0, 5, aspeed_segments_legacy, 0x6000000 },
{ "aspeed.smc.fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
- CONF_ENABLE_W0, 5 },
+ CONF_ENABLE_W0, 5, aspeed_segments_fmc, 0x10000000 },
{ "aspeed.smc.spi", R_SPI_CONF, 0xff, R_SPI_CTRL0, R_SPI_TIMINGS,
- SPI_CONF_ENABLE_W0, 1 },
+ SPI_CONF_ENABLE_W0, 1, aspeed_segments_spi, 0x10000000 },
+};
+
+static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr,
+ unsigned size)
+{
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: To 0x%" HWADDR_PRIx " of size %u"
+ PRIx64 "\n", __func__, addr, size);
+ return 0;
+}
+
+static void aspeed_smc_flash_default_write(void *opaque, hwaddr addr,
+ uint64_t data, unsigned size)
+{
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: To 0x%" HWADDR_PRIx " of size %u: 0x%"
+ PRIx64 "\n", __func__, addr, size, data);
+}
+
+static const MemoryRegionOps aspeed_smc_flash_default_ops = {
+ .read = aspeed_smc_flash_default_read,
+ .write = aspeed_smc_flash_default_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid = {
+ .min_access_size = 1,
+ .max_access_size = 4,
+ },
+};
+
+static inline int aspeed_smc_flash_mode(const AspeedSMCState *s, int cs)
+{
+ return s->regs[s->r_ctrl0 + cs] & CTRL_CMD_MODE_MASK;
+}
+
+static inline bool aspeed_smc_is_usermode(const AspeedSMCState *s, int cs)
+{
+ return aspeed_smc_flash_mode(s, cs) == CTRL_USERMODE;
+}
+
+static inline bool aspeed_smc_is_writable(const AspeedSMCState *s, int cs)
+{
+ return s->regs[s->r_conf] & (1 << (s->conf_enable_w0 + cs));
+}
+
+static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size)
+{
+ AspeedSMCFlash *fl = opaque;
+ const AspeedSMCState *s = fl->controller;
+ uint64_t ret = 0;
+ int i;
+
+ if (aspeed_smc_is_usermode(s, fl->id)) {
+ for (i = 0; i < size; i++) {
+ ret |= ssi_transfer(s->spi, 0x0) << (8 * i);
+ }
+ } else {
+ qemu_log_mask(LOG_UNIMP, "%s: usermode not implemented\n",
+ __func__);
+ ret = -1;
+ }
+
+ return ret;
+}
+
+static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned size)
+{
+ AspeedSMCFlash *fl = opaque;
+ const AspeedSMCState *s = fl->controller;
+ int i;
+
+ if (!aspeed_smc_is_writable(s, fl->id)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: flash is not writable at 0x%"
+ HWADDR_PRIx "\n", __func__, addr);
+ return;
+ }
+
+ if (!aspeed_smc_is_usermode(s, fl->id)) {
+ qemu_log_mask(LOG_UNIMP, "%s: usermode not implemented\n",
+ __func__);
+ return;
+ }
+
+ for (i = 0; i < size; i++) {
+ ssi_transfer(s->spi, (data >> (8 * i)) & 0xff);
+ }
+}
+
+static const MemoryRegionOps aspeed_smc_flash_ops = {
+ .read = aspeed_smc_flash_read,
+ .write = aspeed_smc_flash_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid = {
+ .min_access_size = 1,
+ .max_access_size = 4,
+ },
};
static bool aspeed_smc_is_ce_stop_active(const AspeedSMCState *s, int cs)
@@ -237,6 +353,8 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
AspeedSMCState *s = ASPEED_SMC(dev);
AspeedSMCClass *mc = ASPEED_SMC_GET_CLASS(s);
int i;
+ char name[32];
+ hwaddr offset = 0;
s->ctrl = mc->ctrl;
@@ -270,6 +388,32 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s,
s->ctrl->name, ASPEED_SMC_R_MAX * 4);
sysbus_init_mmio(sbd, &s->mmio);
+
+ /*
+ * Memory region where flash modules are remapped
+ */
+ snprintf(name, sizeof(name), "%s.flash", s->ctrl->name);
+
+ memory_region_init_io(&s->mmio_flash, OBJECT(s),
+ &aspeed_smc_flash_default_ops, s, name,
+ s->ctrl->mapping_window_size);
+ sysbus_init_mmio(sbd, &s->mmio_flash);
+
+ s->flashes = g_malloc0(sizeof(AspeedSMCFlash) * s->num_cs);
+
+ for (i = 0; i < s->num_cs; ++i) {
+ AspeedSMCFlash *fl = &s->flashes[i];
+
+ snprintf(name, sizeof(name), "%s.%d", s->ctrl->name, i);
+
+ fl->id = i;
+ fl->controller = s;
+ fl->size = s->ctrl->segments[i].size;
+ memory_region_init_io(&fl->mmio, OBJECT(s), &aspeed_smc_flash_ops,
+ fl, name, fl->size);
+ memory_region_add_subregion(&s->mmio_flash, offset, &fl->mmio);
+ offset += fl->size;
+ }
}
static const VMStateDescription vmstate_aspeed_smc = {
diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index c4a4960cd880..def3b4507e75 100644
--- a/include/hw/ssi/aspeed_smc.h
+++ b/include/hw/ssi/aspeed_smc.h
@@ -27,6 +27,12 @@
#include "hw/ssi/ssi.h"
+typedef struct AspeedSegments {
+ hwaddr addr;
+ uint32_t size;
+} AspeedSegments;
+
+struct AspeedSMCState;
typedef struct AspeedSMCController {
const char *name;
uint8_t r_conf;
@@ -35,8 +41,20 @@ typedef struct AspeedSMCController {
uint8_t r_timings;
uint8_t conf_enable_w0;
uint8_t max_slaves;
+ const AspeedSegments *segments;
+ uint32_t mapping_window_size;
} AspeedSMCController;
+typedef struct AspeedSMCFlash {
+ const struct AspeedSMCState *controller;
+
+ uint8_t id;
+ uint32_t size;
+
+ MemoryRegion mmio;
+ DeviceState *flash;
+} AspeedSMCFlash;
+
#define TYPE_ASPEED_SMC "aspeed.smc"
#define ASPEED_SMC(obj) OBJECT_CHECK(AspeedSMCState, (obj), TYPE_ASPEED_SMC)
#define ASPEED_SMC_CLASS(klass) \
@@ -57,6 +75,7 @@ typedef struct AspeedSMCState {
const AspeedSMCController *ctrl;
MemoryRegion mmio;
+ MemoryRegion mmio_flash;
qemu_irq irq;
int irqline;
@@ -74,6 +93,8 @@ typedef struct AspeedSMCState {
uint8_t r_ctrl0;
uint8_t r_timings;
uint8_t conf_enable_w0;
+
+ AspeedSMCFlash *flashes;
} AspeedSMCState;
#endif /* ASPEED_SMC_H */
--
2.1.4
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 7/9] ast2400: add SPI flash slaves
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 7/9] ast2400: add SPI flash slaves Cédric Le Goater
@ 2016-07-01 16:24 ` Peter Maydell
2016-07-01 16:44 ` Cédric Le Goater
2016-07-02 17:08 ` mar.krzeminski
1 sibling, 1 reply; 33+ messages in thread
From: Peter Maydell @ 2016-07-01 16:24 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 28 June 2016 at 19:24, Cédric Le Goater <clg@kaod.org> wrote:
> Each controller on the ast2400 has a memory range on which it maps its
> flash module slaves. Each slave is assigned a memory segment for its
> mapping that can be changed at bootime with the Segment Address
> Register. This is not supported in the current implementation so we
> are using the defaults provided by the specs.
>
> Each SPI flash slave can then be accessed in two modes: Command and
> User. When in User mode, accesses to the memory segment of the slaves
> are translated in SPI transfers. When in Command mode, the HW
> generates the SPI commands automatically and the memory segment is
> accessed as if doing a MMIO. Other SPI controllers call that mode
> linear addressing mode.
>
> For this purpose, we are adding below each crontoller an array of
> structs gathering for each SPI flash module, a segment rank, a
> MemoryRegion to handle the memory accesses and the associated SPI
> slave device, which should be a m25p80.
>
> Only the User mode is supported for now but we are preparing ground
> for the Command mode. The framework is sufficient to support Linux.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> +/*
> + * Default segments mapping addresses and size for each slave per
> + * controller. These can be changed when board is initialized with the
> + * Segment Address Registers but they don't seem do be used on the
> + * field.
> + */
> +static const AspeedSegments aspeed_segments_legacy[] = {
> + { 0x10000000, 32 * 1024 * 1024 },
> +};
> +
> +static const AspeedSegments aspeed_segments_fmc[] = {
> + { 0x20000000, 64 * 1024 * 1024 },
> + { 0x24000000, 32 * 1024 * 1024 },
> + { 0x26000000, 32 * 1024 * 1024 },
> + { 0x28000000, 32 * 1024 * 1024 },
> + { 0x2A000000, 32 * 1024 * 1024 }
> +};
> +
> +static const AspeedSegments aspeed_segments_spi[] = {
> + { 0x30000000, 64 * 1024 * 1024 },
> +};
If I understand this correctly, and the Segment Address Registers
are part of the SMC controller, then eventually if we want to
implement making these writable then the correct approach is
probably to have a container memory region which the SMC controller
provides to the board (and which the board then maps into the
system memory space), and then the controller is responsible for
mapping and unmapping the individual memory regions inside that
container. This is basically how we do PCI controllers, which also
allow guests to write to PCI BARs to map and unmap bits of memory.
This will be fine for now, though.
> static bool aspeed_smc_is_ce_stop_active(const AspeedSMCState *s, int cs)
> @@ -237,6 +353,8 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
> AspeedSMCState *s = ASPEED_SMC(dev);
> AspeedSMCClass *mc = ASPEED_SMC_GET_CLASS(s);
> int i;
> + char name[32];
> + hwaddr offset = 0;
>
> s->ctrl = mc->ctrl;
>
> @@ -270,6 +388,32 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
> memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s,
> s->ctrl->name, ASPEED_SMC_R_MAX * 4);
> sysbus_init_mmio(sbd, &s->mmio);
> +
> + /*
> + * Memory region where flash modules are remapped
> + */
> + snprintf(name, sizeof(name), "%s.flash", s->ctrl->name);
> +
> + memory_region_init_io(&s->mmio_flash, OBJECT(s),
> + &aspeed_smc_flash_default_ops, s, name,
> + s->ctrl->mapping_window_size);
> + sysbus_init_mmio(sbd, &s->mmio_flash);
> +
> + s->flashes = g_malloc0(sizeof(AspeedSMCFlash) * s->num_cs);
This should be g_new0(AspeedSMCFlash, s->num_cs); -- multiplying
in a g_malloc0() is usually a sign you should use g_new0 instead.
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
so I'll just fix that when I put the series in target-arm.next.
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 7/9] ast2400: add SPI flash slaves
2016-07-01 16:24 ` Peter Maydell
@ 2016-07-01 16:44 ` Cédric Le Goater
2016-07-01 17:00 ` Peter Maydell
0 siblings, 1 reply; 33+ messages in thread
From: Cédric Le Goater @ 2016-07-01 16:44 UTC (permalink / raw)
To: Peter Maydell
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 07/01/2016 06:24 PM, Peter Maydell wrote:
> On 28 June 2016 at 19:24, Cédric Le Goater <clg@kaod.org> wrote:
>> Each controller on the ast2400 has a memory range on which it maps its
>> flash module slaves. Each slave is assigned a memory segment for its
>> mapping that can be changed at bootime with the Segment Address
>> Register. This is not supported in the current implementation so we
>> are using the defaults provided by the specs.
>>
>> Each SPI flash slave can then be accessed in two modes: Command and
>> User. When in User mode, accesses to the memory segment of the slaves
>> are translated in SPI transfers. When in Command mode, the HW
>> generates the SPI commands automatically and the memory segment is
>> accessed as if doing a MMIO. Other SPI controllers call that mode
>> linear addressing mode.
>>
>> For this purpose, we are adding below each crontoller an array of
>> structs gathering for each SPI flash module, a segment rank, a
>> MemoryRegion to handle the memory accesses and the associated SPI
>> slave device, which should be a m25p80.
>>
>> Only the User mode is supported for now but we are preparing ground
>> for the Command mode. The framework is sufficient to support Linux.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>
>> +/*
>> + * Default segments mapping addresses and size for each slave per
>> + * controller. These can be changed when board is initialized with the
>> + * Segment Address Registers but they don't seem do be used on the
>> + * field.
>> + */
>> +static const AspeedSegments aspeed_segments_legacy[] = {
>> + { 0x10000000, 32 * 1024 * 1024 },
>> +};
>> +
>> +static const AspeedSegments aspeed_segments_fmc[] = {
>> + { 0x20000000, 64 * 1024 * 1024 },
>> + { 0x24000000, 32 * 1024 * 1024 },
>> + { 0x26000000, 32 * 1024 * 1024 },
>> + { 0x28000000, 32 * 1024 * 1024 },
>> + { 0x2A000000, 32 * 1024 * 1024 }
>> +};
>> +
>> +static const AspeedSegments aspeed_segments_spi[] = {
>> + { 0x30000000, 64 * 1024 * 1024 },
>> +};
>
> If I understand this correctly, and the Segment Address Registers
> are part of the SMC controller, then eventually if we want to
> implement making these writable then the correct approach is
> probably to have a container memory region which the SMC controller
> provides to the board (and which the board then maps into the
> system memory space), and then the controller is responsible for
> mapping and unmapping the individual memory regions inside that
> container. This is basically how we do PCI controllers, which also
> allow guests to write to PCI BARs to map and unmap bits of memory.
OK. I will take a look at the approach. The default setting seems to
satisfy the different implementations I know of.
> This will be fine for now, though.
>
>> static bool aspeed_smc_is_ce_stop_active(const AspeedSMCState *s, int cs)
>> @@ -237,6 +353,8 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>> AspeedSMCState *s = ASPEED_SMC(dev);
>> AspeedSMCClass *mc = ASPEED_SMC_GET_CLASS(s);
>> int i;
>> + char name[32];
>> + hwaddr offset = 0;
>>
>> s->ctrl = mc->ctrl;
>>
>> @@ -270,6 +388,32 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>> memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s,
>> s->ctrl->name, ASPEED_SMC_R_MAX * 4);
>> sysbus_init_mmio(sbd, &s->mmio);
>> +
>> + /*
>> + * Memory region where flash modules are remapped
>> + */
>> + snprintf(name, sizeof(name), "%s.flash", s->ctrl->name);
>> +
>> + memory_region_init_io(&s->mmio_flash, OBJECT(s),
>> + &aspeed_smc_flash_default_ops, s, name,
>> + s->ctrl->mapping_window_size);
>> + sysbus_init_mmio(sbd, &s->mmio_flash);
>> +
>> + s->flashes = g_malloc0(sizeof(AspeedSMCFlash) * s->num_cs);
>
> This should be g_new0(AspeedSMCFlash, s->num_cs); -- multiplying
> in a g_malloc0() is usually a sign you should use g_new0 instead.
ah yes. I have changed that back and forth and kept the wrong one ...
> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> so I'll just fix that when I put the series in target-arm.next.
I have some extra patches to use a rom device and boot from flash0.
That is for next week.
Thanks,
C.
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 7/9] ast2400: add SPI flash slaves
2016-07-01 16:44 ` Cédric Le Goater
@ 2016-07-01 17:00 ` Peter Maydell
0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2016-07-01 17:00 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 1 July 2016 at 17:44, Cédric Le Goater <clg@kaod.org> wrote:
> I have some extra patches to use a rom device and boot from flash0.
> That is for next week.
We're in softfreeze now, so really I should stop
taking non-bugfix patches, though for a new board
with missing stuff that prevents boot there's a little
flexibility.
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v5 7/9] ast2400: add SPI flash slaves
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 7/9] ast2400: add SPI flash slaves Cédric Le Goater
2016-07-01 16:24 ` Peter Maydell
@ 2016-07-02 17:08 ` mar.krzeminski
2016-07-02 17:51 ` mar.krzeminski
1 sibling, 1 reply; 33+ messages in thread
From: mar.krzeminski @ 2016-07-02 17:08 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Peter Crosthwaite
Cc: kwolf, Andrew Jeffery, qemu-devel, armbru, qemu-arm
W dniu 28.06.2016 o 20:24, Cédric Le Goater pisze:
> Each controller on the ast2400 has a memory range on which it maps its
> flash module slaves. Each slave is assigned a memory segment for its
> mapping that can be changed at bootime with the Segment Address
> Register. This is not supported in the current implementation so we
> are using the defaults provided by the specs.
>
> Each SPI flash slave can then be accessed in two modes: Command and
> User. When in User mode, accesses to the memory segment of the slaves
> are translated in SPI transfers. When in Command mode, the HW
> generates the SPI commands automatically and the memory segment is
> accessed as if doing a MMIO. Other SPI controllers call that mode
> linear addressing mode.
>
> For this purpose, we are adding below each crontoller an array of
> structs gathering for each SPI flash module, a segment rank, a
> MemoryRegion to handle the memory accesses and the associated SPI
> slave device, which should be a m25p80.
>
> Only the User mode is supported for now but we are preparing ground
> for the Command mode. The framework is sufficient to support Linux.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
> Changes since v3:
>
> - Fixed error logging
> - Constantified a couple of routine arguments
> - Sorted out what was need for migration (nothing apriori, but flash
> module object will need some care)
> - Reworked the structures handling the IOs on the flash modules :
> . Replaced the SysBusDevice object with a simple struct
> . Added a global memory region for the flash memory address space
> . Added sub regions for each segment
> . Moved mapping in SOC initialization
> - Rephrased commit log
>
> hw/arm/ast2400.c | 5 ++
> hw/ssi/aspeed_smc.c | 150 +++++++++++++++++++++++++++++++++++++++++++-
> include/hw/ssi/aspeed_smc.h | 21 +++++++
> 3 files changed, 173 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
> index b16ba2d0c516..9c30b45f87a9 100644
> --- a/hw/arm/ast2400.c
> +++ b/hw/arm/ast2400.c
> @@ -31,6 +31,9 @@
> #define AST2400_TIMER_BASE 0x1E782000
> #define AST2400_I2C_BASE 0x1E78A000
>
> +#define AST2400_FMC_FLASH_BASE 0x20000000
> +#define AST2400_SPI_FLASH_BASE 0x30000000
> +
> #define AST2400_A0_SILICON_REV 0x02000303
>
> static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
> @@ -167,6 +170,7 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
> return;
> }
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->smc), 0, AST2400_FMC_BASE);
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->smc), 1, AST2400_FMC_FLASH_BASE);
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->smc), 0,
> qdev_get_gpio_in(DEVICE(&s->vic), 19));
>
> @@ -179,6 +183,7 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
> return;
> }
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 0, AST2400_SPI_BASE);
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 1, AST2400_SPI_FLASH_BASE);
> }
>
> static void ast2400_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 537635e18d64..cb0b23750bcf 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -127,13 +127,129 @@
> #define R_SPI_MISC_CTRL (0x10 / 4)
> #define R_SPI_TIMINGS (0x14 / 4)
>
> +/*
> + * Default segments mapping addresses and size for each slave per
> + * controller. These can be changed when board is initialized with the
> + * Segment Address Registers but they don't seem do be used on the
> + * field.
> + */
> +static const AspeedSegments aspeed_segments_legacy[] = {
> + { 0x10000000, 32 * 1024 * 1024 },
> +};
> +
> +static const AspeedSegments aspeed_segments_fmc[] = {
> + { 0x20000000, 64 * 1024 * 1024 },
> + { 0x24000000, 32 * 1024 * 1024 },
> + { 0x26000000, 32 * 1024 * 1024 },
> + { 0x28000000, 32 * 1024 * 1024 },
> + { 0x2A000000, 32 * 1024 * 1024 }
> +};
> +
> +static const AspeedSegments aspeed_segments_spi[] = {
> + { 0x30000000, 64 * 1024 * 1024 },
> +};
> +
> static const AspeedSMCController controllers[] = {
> { "aspeed.smc.smc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
> - CONF_ENABLE_W0, 5 },
> + CONF_ENABLE_W0, 5, aspeed_segments_legacy, 0x6000000 },
> { "aspeed.smc.fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
> - CONF_ENABLE_W0, 5 },
> + CONF_ENABLE_W0, 5, aspeed_segments_fmc, 0x10000000 },
> { "aspeed.smc.spi", R_SPI_CONF, 0xff, R_SPI_CTRL0, R_SPI_TIMINGS,
> - SPI_CONF_ENABLE_W0, 1 },
> + SPI_CONF_ENABLE_W0, 1, aspeed_segments_spi, 0x10000000 },
> +};
> +
> +static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr,
> + unsigned size)
> +{
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: To 0x%" HWADDR_PRIx " of size %u"
> + PRIx64 "\n", __func__, addr, size);
> + return 0;
> +}
> +
> +static void aspeed_smc_flash_default_write(void *opaque, hwaddr addr,
> + uint64_t data, unsigned size)
> +{
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: To 0x%" HWADDR_PRIx " of size %u: 0x%"
> + PRIx64 "\n", __func__, addr, size, data);
> +}
> +
> +static const MemoryRegionOps aspeed_smc_flash_default_ops = {
> + .read = aspeed_smc_flash_default_read,
> + .write = aspeed_smc_flash_default_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> + .valid = {
> + .min_access_size = 1,
> + .max_access_size = 4,
> + },
> +};
> +
> +static inline int aspeed_smc_flash_mode(const AspeedSMCState *s, int cs)
> +{
> + return s->regs[s->r_ctrl0 + cs] & CTRL_CMD_MODE_MASK;
> +}
> +
> +static inline bool aspeed_smc_is_usermode(const AspeedSMCState *s, int cs)
> +{
> + return aspeed_smc_flash_mode(s, cs) == CTRL_USERMODE;
> +}
> +
> +static inline bool aspeed_smc_is_writable(const AspeedSMCState *s, int cs)
> +{
> + return s->regs[s->r_conf] & (1 << (s->conf_enable_w0 + cs));
> +}
> +
> +static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + AspeedSMCFlash *fl = opaque;
> + const AspeedSMCState *s = fl->controller;
> + uint64_t ret = 0;
> + int i;
> +
> + if (aspeed_smc_is_usermode(s, fl->id)) {
> + for (i = 0; i < size; i++) {
> + ret |= ssi_transfer(s->spi, 0x0) << (8 * i);
> + }
> + } else {
> + qemu_log_mask(LOG_UNIMP, "%s: usermode not implemented\n",
> + __func__);
> + ret = -1;
> + }
> +
> + return ret;
> +}
> +
> +static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data,
> + unsigned size)
> +{
> + AspeedSMCFlash *fl = opaque;
> + const AspeedSMCState *s = fl->controller;
> + int i;
> +
> + if (!aspeed_smc_is_writable(s, fl->id)) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: flash is not writable at 0x%"
> + HWADDR_PRIx "\n", __func__, addr);
> + return;
> + }
> +
> + if (!aspeed_smc_is_usermode(s, fl->id)) {
> + qemu_log_mask(LOG_UNIMP, "%s: usermode not implemented\n",
> + __func__);
> + return;
> + }
> +
> + for (i = 0; i < size; i++) {
> + ssi_transfer(s->spi, (data >> (8 * i)) & 0xff);
Hi,
I think the error with m25p80 tests is here.
The flash device I know (eg. n25q*) takes address in BE, MSB first.
Since we do not care about MSB/LSB stuff, we should fallow BE/LE.
If your machine is BE, you will start to send address backwards.
Regard,
Marcin
> + }
> +}
> +
> +static const MemoryRegionOps aspeed_smc_flash_ops = {
> + .read = aspeed_smc_flash_read,
> + .write = aspeed_smc_flash_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> + .valid = {
> + .min_access_size = 1,
> + .max_access_size = 4,
> + },
> };
>
> static bool aspeed_smc_is_ce_stop_active(const AspeedSMCState *s, int cs)
> @@ -237,6 +353,8 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
> AspeedSMCState *s = ASPEED_SMC(dev);
> AspeedSMCClass *mc = ASPEED_SMC_GET_CLASS(s);
> int i;
> + char name[32];
> + hwaddr offset = 0;
>
> s->ctrl = mc->ctrl;
>
> @@ -270,6 +388,32 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
> memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s,
> s->ctrl->name, ASPEED_SMC_R_MAX * 4);
> sysbus_init_mmio(sbd, &s->mmio);
> +
> + /*
> + * Memory region where flash modules are remapped
> + */
> + snprintf(name, sizeof(name), "%s.flash", s->ctrl->name);
> +
> + memory_region_init_io(&s->mmio_flash, OBJECT(s),
> + &aspeed_smc_flash_default_ops, s, name,
> + s->ctrl->mapping_window_size);
> + sysbus_init_mmio(sbd, &s->mmio_flash);
> +
> + s->flashes = g_malloc0(sizeof(AspeedSMCFlash) * s->num_cs);
> +
> + for (i = 0; i < s->num_cs; ++i) {
> + AspeedSMCFlash *fl = &s->flashes[i];
> +
> + snprintf(name, sizeof(name), "%s.%d", s->ctrl->name, i);
> +
> + fl->id = i;
> + fl->controller = s;
> + fl->size = s->ctrl->segments[i].size;
> + memory_region_init_io(&fl->mmio, OBJECT(s), &aspeed_smc_flash_ops,
> + fl, name, fl->size);
> + memory_region_add_subregion(&s->mmio_flash, offset, &fl->mmio);
> + offset += fl->size;
> + }
> }
>
> static const VMStateDescription vmstate_aspeed_smc = {
> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
> index c4a4960cd880..def3b4507e75 100644
> --- a/include/hw/ssi/aspeed_smc.h
> +++ b/include/hw/ssi/aspeed_smc.h
> @@ -27,6 +27,12 @@
>
> #include "hw/ssi/ssi.h"
>
> +typedef struct AspeedSegments {
> + hwaddr addr;
> + uint32_t size;
> +} AspeedSegments;
> +
> +struct AspeedSMCState;
> typedef struct AspeedSMCController {
> const char *name;
> uint8_t r_conf;
> @@ -35,8 +41,20 @@ typedef struct AspeedSMCController {
> uint8_t r_timings;
> uint8_t conf_enable_w0;
> uint8_t max_slaves;
> + const AspeedSegments *segments;
> + uint32_t mapping_window_size;
> } AspeedSMCController;
>
> +typedef struct AspeedSMCFlash {
> + const struct AspeedSMCState *controller;
> +
> + uint8_t id;
> + uint32_t size;
> +
> + MemoryRegion mmio;
> + DeviceState *flash;
> +} AspeedSMCFlash;
> +
> #define TYPE_ASPEED_SMC "aspeed.smc"
> #define ASPEED_SMC(obj) OBJECT_CHECK(AspeedSMCState, (obj), TYPE_ASPEED_SMC)
> #define ASPEED_SMC_CLASS(klass) \
> @@ -57,6 +75,7 @@ typedef struct AspeedSMCState {
> const AspeedSMCController *ctrl;
>
> MemoryRegion mmio;
> + MemoryRegion mmio_flash;
>
> qemu_irq irq;
> int irqline;
> @@ -74,6 +93,8 @@ typedef struct AspeedSMCState {
> uint8_t r_ctrl0;
> uint8_t r_timings;
> uint8_t conf_enable_w0;
> +
> + AspeedSMCFlash *flashes;
> } AspeedSMCState;
>
> #endif /* ASPEED_SMC_H */
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 7/9] ast2400: add SPI flash slaves
2016-07-02 17:08 ` mar.krzeminski
@ 2016-07-02 17:51 ` mar.krzeminski
0 siblings, 0 replies; 33+ messages in thread
From: mar.krzeminski @ 2016-07-02 17:51 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Peter Crosthwaite
Cc: kwolf, Andrew Jeffery, qemu-devel, armbru, qemu-arm
W dniu 02.07.2016 o 19:08, mar.krzeminski pisze:
>
>
> W dniu 28.06.2016 o 20:24, Cédric Le Goater pisze:
>> Each controller on the ast2400 has a memory range on which it maps its
>> flash module slaves. Each slave is assigned a memory segment for its
>> mapping that can be changed at bootime with the Segment Address
>> Register. This is not supported in the current implementation so we
>> are using the defaults provided by the specs.
>>
>> Each SPI flash slave can then be accessed in two modes: Command and
>> User. When in User mode, accesses to the memory segment of the slaves
>> are translated in SPI transfers. When in Command mode, the HW
>> generates the SPI commands automatically and the memory segment is
>> accessed as if doing a MMIO. Other SPI controllers call that mode
>> linear addressing mode.
>>
>> For this purpose, we are adding below each crontoller an array of
>> structs gathering for each SPI flash module, a segment rank, a
>> MemoryRegion to handle the memory accesses and the associated SPI
>> slave device, which should be a m25p80.
>>
>> Only the User mode is supported for now but we are preparing ground
>> for the Command mode. The framework is sufficient to support Linux.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>> Changes since v3:
>>
>> - Fixed error logging
>> - Constantified a couple of routine arguments
>> - Sorted out what was need for migration (nothing apriori, but flash
>> module object will need some care)
>> - Reworked the structures handling the IOs on the flash modules :
>> . Replaced the SysBusDevice object with a simple struct
>> . Added a global memory region for the flash memory address space
>> . Added sub regions for each segment
>> . Moved mapping in SOC initialization
>> - Rephrased commit log
>>
>> hw/arm/ast2400.c | 5 ++
>> hw/ssi/aspeed_smc.c | 150
>> +++++++++++++++++++++++++++++++++++++++++++-
>> include/hw/ssi/aspeed_smc.h | 21 +++++++
>> 3 files changed, 173 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
>> index b16ba2d0c516..9c30b45f87a9 100644
>> --- a/hw/arm/ast2400.c
>> +++ b/hw/arm/ast2400.c
>> @@ -31,6 +31,9 @@
>> #define AST2400_TIMER_BASE 0x1E782000
>> #define AST2400_I2C_BASE 0x1E78A000
>> +#define AST2400_FMC_FLASH_BASE 0x20000000
>> +#define AST2400_SPI_FLASH_BASE 0x30000000
>> +
>> #define AST2400_A0_SILICON_REV 0x02000303
>> static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
>> @@ -167,6 +170,7 @@ static void ast2400_realize(DeviceState *dev,
>> Error **errp)
>> return;
>> }
>> sysbus_mmio_map(SYS_BUS_DEVICE(&s->smc), 0, AST2400_FMC_BASE);
>> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->smc), 1,
>> AST2400_FMC_FLASH_BASE);
>> sysbus_connect_irq(SYS_BUS_DEVICE(&s->smc), 0,
>> qdev_get_gpio_in(DEVICE(&s->vic), 19));
>> @@ -179,6 +183,7 @@ static void ast2400_realize(DeviceState *dev,
>> Error **errp)
>> return;
>> }
>> sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 0, AST2400_SPI_BASE);
>> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 1,
>> AST2400_SPI_FLASH_BASE);
>> }
>> static void ast2400_class_init(ObjectClass *oc, void *data)
>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
>> index 537635e18d64..cb0b23750bcf 100644
>> --- a/hw/ssi/aspeed_smc.c
>> +++ b/hw/ssi/aspeed_smc.c
>> @@ -127,13 +127,129 @@
>> #define R_SPI_MISC_CTRL (0x10 / 4)
>> #define R_SPI_TIMINGS (0x14 / 4)
>> +/*
>> + * Default segments mapping addresses and size for each slave per
>> + * controller. These can be changed when board is initialized with the
>> + * Segment Address Registers but they don't seem do be used on the
>> + * field.
>> + */
>> +static const AspeedSegments aspeed_segments_legacy[] = {
>> + { 0x10000000, 32 * 1024 * 1024 },
>> +};
>> +
>> +static const AspeedSegments aspeed_segments_fmc[] = {
>> + { 0x20000000, 64 * 1024 * 1024 },
>> + { 0x24000000, 32 * 1024 * 1024 },
>> + { 0x26000000, 32 * 1024 * 1024 },
>> + { 0x28000000, 32 * 1024 * 1024 },
>> + { 0x2A000000, 32 * 1024 * 1024 }
>> +};
>> +
>> +static const AspeedSegments aspeed_segments_spi[] = {
>> + { 0x30000000, 64 * 1024 * 1024 },
>> +};
>> +
>> static const AspeedSMCController controllers[] = {
>> { "aspeed.smc.smc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
>> - CONF_ENABLE_W0, 5 },
>> + CONF_ENABLE_W0, 5, aspeed_segments_legacy, 0x6000000 },
>> { "aspeed.smc.fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
>> - CONF_ENABLE_W0, 5 },
>> + CONF_ENABLE_W0, 5, aspeed_segments_fmc, 0x10000000 },
>> { "aspeed.smc.spi", R_SPI_CONF, 0xff, R_SPI_CTRL0, R_SPI_TIMINGS,
>> - SPI_CONF_ENABLE_W0, 1 },
>> + SPI_CONF_ENABLE_W0, 1, aspeed_segments_spi, 0x10000000 },
>> +};
>> +
>> +static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr
>> addr,
>> + unsigned size)
>> +{
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: To 0x%" HWADDR_PRIx " of
>> size %u"
>> + PRIx64 "\n", __func__, addr, size);
>> + return 0;
>> +}
>> +
>> +static void aspeed_smc_flash_default_write(void *opaque, hwaddr addr,
>> + uint64_t data, unsigned
>> size)
>> +{
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: To 0x%" HWADDR_PRIx " of size
>> %u: 0x%"
>> + PRIx64 "\n", __func__, addr, size, data);
>> +}
>> +
>> +static const MemoryRegionOps aspeed_smc_flash_default_ops = {
>> + .read = aspeed_smc_flash_default_read,
>> + .write = aspeed_smc_flash_default_write,
>> + .endianness = DEVICE_LITTLE_ENDIAN,
>> + .valid = {
>> + .min_access_size = 1,
>> + .max_access_size = 4,
>> + },
>> +};
>> +
>> +static inline int aspeed_smc_flash_mode(const AspeedSMCState *s, int
>> cs)
>> +{
>> + return s->regs[s->r_ctrl0 + cs] & CTRL_CMD_MODE_MASK;
>> +}
>> +
>> +static inline bool aspeed_smc_is_usermode(const AspeedSMCState *s,
>> int cs)
>> +{
>> + return aspeed_smc_flash_mode(s, cs) == CTRL_USERMODE;
>> +}
>> +
>> +static inline bool aspeed_smc_is_writable(const AspeedSMCState *s,
>> int cs)
>> +{
>> + return s->regs[s->r_conf] & (1 << (s->conf_enable_w0 + cs));
>> +}
>> +
>> +static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr,
>> unsigned size)
>> +{
>> + AspeedSMCFlash *fl = opaque;
>> + const AspeedSMCState *s = fl->controller;
>> + uint64_t ret = 0;
>> + int i;
>> +
>> + if (aspeed_smc_is_usermode(s, fl->id)) {
>> + for (i = 0; i < size; i++) {
>> + ret |= ssi_transfer(s->spi, 0x0) << (8 * i);
>> + }
>> + } else {
>> + qemu_log_mask(LOG_UNIMP, "%s: usermode not implemented\n",
>> + __func__);
>> + ret = -1;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static void aspeed_smc_flash_write(void *opaque, hwaddr addr,
>> uint64_t data,
>> + unsigned size)
>> +{
>> + AspeedSMCFlash *fl = opaque;
>> + const AspeedSMCState *s = fl->controller;
>> + int i;
>> +
>> + if (!aspeed_smc_is_writable(s, fl->id)) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: flash is not writable at
>> 0x%"
>> + HWADDR_PRIx "\n", __func__, addr);
>> + return;
>> + }
>> +
>> + if (!aspeed_smc_is_usermode(s, fl->id)) {
>> + qemu_log_mask(LOG_UNIMP, "%s: usermode not implemented\n",
>> + __func__);
>> + return;
>> + }
>> +
>> + for (i = 0; i < size; i++) {
>> + ssi_transfer(s->spi, (data >> (8 * i)) & 0xff);
> Hi,
>
> I think the error with m25p80 tests is here.
> The flash device I know (eg. n25q*) takes address in BE, MSB first.
> Since we do not care about MSB/LSB stuff, we should fallow BE/LE.
> If your machine is BE, you will start to send address backwards.
>
> Regard,
> Marcin
Sorry, I have just noticed DEVICE_LITTLE_ENDIAN functionality...
Regards,
Marcin
>
>> + }
>> +}
>> +
>> +static const MemoryRegionOps aspeed_smc_flash_ops = {
>> + .read = aspeed_smc_flash_read,
>> + .write = aspeed_smc_flash_write,
>> + .endianness = DEVICE_LITTLE_ENDIAN,
>> + .valid = {
>> + .min_access_size = 1,
>> + .max_access_size = 4,
>> + },
>> };
>> static bool aspeed_smc_is_ce_stop_active(const AspeedSMCState *s,
>> int cs)
>> @@ -237,6 +353,8 @@ static void aspeed_smc_realize(DeviceState *dev,
>> Error **errp)
>> AspeedSMCState *s = ASPEED_SMC(dev);
>> AspeedSMCClass *mc = ASPEED_SMC_GET_CLASS(s);
>> int i;
>> + char name[32];
>> + hwaddr offset = 0;
>> s->ctrl = mc->ctrl;
>> @@ -270,6 +388,32 @@ static void aspeed_smc_realize(DeviceState
>> *dev, Error **errp)
>> memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s,
>> s->ctrl->name, ASPEED_SMC_R_MAX * 4);
>> sysbus_init_mmio(sbd, &s->mmio);
>> +
>> + /*
>> + * Memory region where flash modules are remapped
>> + */
>> + snprintf(name, sizeof(name), "%s.flash", s->ctrl->name);
>> +
>> + memory_region_init_io(&s->mmio_flash, OBJECT(s),
>> + &aspeed_smc_flash_default_ops, s, name,
>> + s->ctrl->mapping_window_size);
>> + sysbus_init_mmio(sbd, &s->mmio_flash);
>> +
>> + s->flashes = g_malloc0(sizeof(AspeedSMCFlash) * s->num_cs);
>> +
>> + for (i = 0; i < s->num_cs; ++i) {
>> + AspeedSMCFlash *fl = &s->flashes[i];
>> +
>> + snprintf(name, sizeof(name), "%s.%d", s->ctrl->name, i);
>> +
>> + fl->id = i;
>> + fl->controller = s;
>> + fl->size = s->ctrl->segments[i].size;
>> + memory_region_init_io(&fl->mmio, OBJECT(s),
>> &aspeed_smc_flash_ops,
>> + fl, name, fl->size);
>> + memory_region_add_subregion(&s->mmio_flash, offset, &fl->mmio);
>> + offset += fl->size;
>> + }
>> }
>> static const VMStateDescription vmstate_aspeed_smc = {
>> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
>> index c4a4960cd880..def3b4507e75 100644
>> --- a/include/hw/ssi/aspeed_smc.h
>> +++ b/include/hw/ssi/aspeed_smc.h
>> @@ -27,6 +27,12 @@
>> #include "hw/ssi/ssi.h"
>> +typedef struct AspeedSegments {
>> + hwaddr addr;
>> + uint32_t size;
>> +} AspeedSegments;
>> +
>> +struct AspeedSMCState;
>> typedef struct AspeedSMCController {
>> const char *name;
>> uint8_t r_conf;
>> @@ -35,8 +41,20 @@ typedef struct AspeedSMCController {
>> uint8_t r_timings;
>> uint8_t conf_enable_w0;
>> uint8_t max_slaves;
>> + const AspeedSegments *segments;
>> + uint32_t mapping_window_size;
>> } AspeedSMCController;
>> +typedef struct AspeedSMCFlash {
>> + const struct AspeedSMCState *controller;
>> +
>> + uint8_t id;
>> + uint32_t size;
>> +
>> + MemoryRegion mmio;
>> + DeviceState *flash;
>> +} AspeedSMCFlash;
>> +
>> #define TYPE_ASPEED_SMC "aspeed.smc"
>> #define ASPEED_SMC(obj) OBJECT_CHECK(AspeedSMCState, (obj),
>> TYPE_ASPEED_SMC)
>> #define ASPEED_SMC_CLASS(klass) \
>> @@ -57,6 +75,7 @@ typedef struct AspeedSMCState {
>> const AspeedSMCController *ctrl;
>> MemoryRegion mmio;
>> + MemoryRegion mmio_flash;
>> qemu_irq irq;
>> int irqline;
>> @@ -74,6 +93,8 @@ typedef struct AspeedSMCState {
>> uint8_t r_ctrl0;
>> uint8_t r_timings;
>> uint8_t conf_enable_w0;
>> +
>> + AspeedSMCFlash *flashes;
>> } AspeedSMCState;
>> #endif /* ASPEED_SMC_H */
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v5 8/9] ast2400: create SPI flash slaves
2016-06-28 18:24 [Qemu-devel] [PATCH v5 0/9] ast2400: SMC controllers Cédric Le Goater
` (6 preceding siblings ...)
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 7/9] ast2400: add SPI flash slaves Cédric Le Goater
@ 2016-06-28 18:24 ` Cédric Le Goater
2016-07-01 15:19 ` Peter Maydell
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test Cédric Le Goater
2016-07-01 16:25 ` [Qemu-devel] [PATCH v5 0/9] ast2400: SMC controllers Peter Maydell
9 siblings, 1 reply; 33+ messages in thread
From: Cédric Le Goater @ 2016-06-28 18:24 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
Cédric Le Goater
A set of SPI flash slaves is attached under the flash controllers of
the palmetto platform. "n25q256a" flash modules are used for the BMC
and "mx25l25635e" for the host. These types are common in the
OpenPower ecosystem.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
Changes since v3 :
- moved memory mapping to platform initialization
Changes since v2 :
- moved the initialization of the flash modules under the palmetto
platform
hw/arm/palmetto-bmc.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/hw/arm/palmetto-bmc.c b/hw/arm/palmetto-bmc.c
index b8eed21348d8..54e29a865d88 100644
--- a/hw/arm/palmetto-bmc.c
+++ b/hw/arm/palmetto-bmc.c
@@ -18,6 +18,8 @@
#include "hw/arm/ast2400.h"
#include "hw/boards.h"
#include "qemu/log.h"
+#include "sysemu/block-backend.h"
+#include "sysemu/blockdev.h"
static struct arm_boot_info palmetto_bmc_binfo = {
.loader_start = AST2400_SDRAM_BASE,
@@ -30,6 +32,32 @@ typedef struct PalmettoBMCState {
MemoryRegion ram;
} PalmettoBMCState;
+static void palmetto_bmc_init_flashes(AspeedSMCState *s, const char *flashtype,
+ Error **errp)
+{
+ int i ;
+
+ for (i = 0; i < s->num_cs; ++i) {
+ AspeedSMCFlash *fl = &s->flashes[i];
+ DriveInfo *dinfo = drive_get_next(IF_MTD);
+ qemu_irq cs_line;
+
+ /*
+ * FIXME: check that we are not using a flash module exceeding
+ * the controller segment size
+ */
+ fl->flash = ssi_create_slave_no_init(s->spi, flashtype);
+ if (dinfo) {
+ qdev_prop_set_drive(fl->flash, "drive", blk_by_legacy_dinfo(dinfo),
+ errp);
+ }
+ qdev_init_nofail(fl->flash);
+
+ cs_line = qdev_get_gpio_in_named(fl->flash, SSI_GPIO_CS, 0);
+ sysbus_connect_irq(SYS_BUS_DEVICE(s), i + 1, cs_line);
+ }
+}
+
static void palmetto_bmc_init(MachineState *machine)
{
PalmettoBMCState *bmc;
@@ -49,6 +77,9 @@ static void palmetto_bmc_init(MachineState *machine)
object_property_set_bool(OBJECT(&bmc->soc), true, "realized",
&error_abort);
+ palmetto_bmc_init_flashes(&bmc->soc.smc, "n25q256a", &error_abort);
+ palmetto_bmc_init_flashes(&bmc->soc.spi, "mx25l25635e", &error_abort);
+
palmetto_bmc_binfo.kernel_filename = machine->kernel_filename;
palmetto_bmc_binfo.initrd_filename = machine->initrd_filename;
palmetto_bmc_binfo.kernel_cmdline = machine->kernel_cmdline;
--
2.1.4
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 8/9] ast2400: create SPI flash slaves
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 8/9] ast2400: create " Cédric Le Goater
@ 2016-07-01 15:19 ` Peter Maydell
0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2016-07-01 15:19 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 28 June 2016 at 19:24, Cédric Le Goater <clg@kaod.org> wrote:
> A set of SPI flash slaves is attached under the flash controllers of
> the palmetto platform. "n25q256a" flash modules are used for the BMC
> and "mx25l25635e" for the host. These types are common in the
> OpenPower ecosystem.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
> Changes since v3 :
>
> - moved memory mapping to platform initialization
>
> Changes since v2 :
>
> - moved the initialization of the flash modules under the palmetto
> platform
>
> hw/arm/palmetto-bmc.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test
2016-06-28 18:24 [Qemu-devel] [PATCH v5 0/9] ast2400: SMC controllers Cédric Le Goater
` (7 preceding siblings ...)
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 8/9] ast2400: create " Cédric Le Goater
@ 2016-06-28 18:24 ` Cédric Le Goater
2016-07-01 17:18 ` Peter Maydell
2016-07-02 18:05 ` mar.krzeminski
2016-07-01 16:25 ` [Qemu-devel] [PATCH v5 0/9] ast2400: SMC controllers Peter Maydell
9 siblings, 2 replies; 33+ messages in thread
From: Cédric Le Goater @ 2016-06-28 18:24 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
Cédric Le Goater
This test uses the palmetto platform and the AST2400 SPI controller to
test the m25p80 flash module device model. The flash model is defined
by the platform (n25q256a) and it would be nice to find way to control
it, using a property probably.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
Changes since v4:
- fixed Makefile targets
- replaced -M with -m in qtest command line
Changes since v2:
- changed mkstemp() path prefix
Changes since v1:
- fixed guest args to use -drive and not -mtdblock
tests/Makefile.include | 2 +
tests/m25p80-test.c | 242 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 244 insertions(+)
create mode 100644 tests/m25p80-test.c
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 6c09962f7581..149f92585c1b 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -252,6 +252,7 @@ gcov-files-sparc-y += hw/timer/m48t59.c
gcov-files-sparc64-y += hw/timer/m48t59.c
check-qtest-arm-y = tests/tmp105-test$(EXESUF)
check-qtest-arm-y += tests/ds1338-test$(EXESUF)
+check-qtest-arm-y += tests/m25p80-test$(EXESUF)
gcov-files-arm-y += hw/misc/tmp105.c
check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
@@ -568,6 +569,7 @@ tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
tests/ds1338-test$(EXESUF): tests/ds1338-test.o $(libqos-imx-obj-y)
+tests/m25p80-test$(EXESUF): tests/m25p80-test.o
tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
tests/q35-test$(EXESUF): tests/q35-test.o $(libqos-pc-obj-y)
tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
diff --git a/tests/m25p80-test.c b/tests/m25p80-test.c
new file mode 100644
index 000000000000..305c52319a33
--- /dev/null
+++ b/tests/m25p80-test.c
@@ -0,0 +1,242 @@
+/*
+ * QTest testcase for the M25P80 Flash (Using the AST2400 SPI Controller)
+ *
+ * Copyright (C) 2016 IBM Corp.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bswap.h"
+#include "libqtest.h"
+
+/*
+ * AST2400 SPI Controller registers
+ */
+#define R_CONF 0x00
+#define CONF_ENABLE_W0 (1 << 16)
+#define R_CE_CTRL 0x04
+#define CRTL_EXTENDED0 0 /* 32 bit addressing for SPI */
+#define R_CTRL0 0x10
+#define CTRL_CE_STOP_ACTIVE (1 << 2)
+#define CTRL_USERMODE 0x3
+
+#define AST2400_FMC_BASE 0x1E620000
+#define AST2400_FLASH_BASE 0x20000000
+
+/*
+ * Flash commands
+ */
+enum {
+ JEDEC_READ = 0x9f,
+ BULK_ERASE = 0xc7,
+ READ = 0x03,
+ PP = 0x02,
+ WREN = 0x6,
+ EN_4BYTE_ADDR = 0xB7,
+ ERASE_SECTOR = 0xd8,
+};
+
+#define FLASH_JEDEC 0x20ba19 /* n25q256a */
+#define FLASH_SIZE (32 * 1024 * 1024)
+
+#define PAGE_SIZE 256
+
+static void spi_conf(uint32_t value)
+{
+ uint32_t conf = readl(AST2400_FMC_BASE + R_CONF);
+
+ conf |= value;
+ writel(AST2400_FMC_BASE + R_CONF, conf);
+}
+
+static void spi_ctrl_start_user(void)
+{
+ uint32_t ctrl = readl(AST2400_FMC_BASE + R_CTRL0);
+
+ ctrl |= CTRL_USERMODE | CTRL_CE_STOP_ACTIVE;
+ writel(AST2400_FMC_BASE + R_CTRL0, ctrl);
+
+ ctrl &= ~CTRL_CE_STOP_ACTIVE;
+ writel(AST2400_FMC_BASE + R_CTRL0, ctrl);
+}
+
+static void spi_ctrl_stop_user(void)
+{
+ uint32_t ctrl = readl(AST2400_FMC_BASE + R_CTRL0);
+
+ ctrl |= CTRL_USERMODE | CTRL_CE_STOP_ACTIVE;
+ writel(AST2400_FMC_BASE + R_CTRL0, ctrl);
+}
+
+static void test_read_jedec(void)
+{
+ uint32_t jedec = 0x0;
+
+ spi_conf(CONF_ENABLE_W0);
+
+ spi_ctrl_start_user();
+ writeb(AST2400_FLASH_BASE, JEDEC_READ);
+ jedec |= readb(AST2400_FLASH_BASE) << 16;
+ jedec |= readb(AST2400_FLASH_BASE) << 8;
+ jedec |= readb(AST2400_FLASH_BASE);
+ spi_ctrl_stop_user();
+
+ g_assert_cmphex(jedec, ==, FLASH_JEDEC);
+}
+
+static void read_page(uint32_t addr, uint32_t *page)
+{
+ int i;
+
+ spi_ctrl_start_user();
+
+ writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
+ writeb(AST2400_FLASH_BASE, READ);
+ writel(AST2400_FLASH_BASE, cpu_to_be32(addr));
+
+ /* Continuous read are supported */
+ for (i = 0; i < PAGE_SIZE / 4; i++) {
+ page[i] = be32_to_cpu(readl(AST2400_FLASH_BASE));
+ }
+ spi_ctrl_stop_user();
+}
+
+static void test_erase_sector(void)
+{
+ uint32_t some_page_addr = 0x600 * PAGE_SIZE;
+ uint32_t page[PAGE_SIZE / 4];
+ int i;
+
+ spi_conf(CONF_ENABLE_W0);
+
+ spi_ctrl_start_user();
+ writeb(AST2400_FLASH_BASE, WREN);
+ writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
+ writeb(AST2400_FLASH_BASE, ERASE_SECTOR);
+ writel(AST2400_FLASH_BASE, cpu_to_be32(some_page_addr));
+ spi_ctrl_stop_user();
+
+ /* Previous page should be full of zeroes as backend is not
+ * initialized */
+ read_page(some_page_addr - PAGE_SIZE, page);
+ for (i = 0; i < PAGE_SIZE / 4; i++) {
+ g_assert_cmphex(page[i], ==, 0x0);
+ }
+
+ /* But this one was erased */
+ read_page(some_page_addr, page);
+ for (i = 0; i < PAGE_SIZE / 4; i++) {
+ g_assert_cmphex(page[i], ==, 0xffffffff);
+ }
+}
+
+static void test_erase_all(void)
+{
+ uint32_t some_page_addr = 0x15000 * PAGE_SIZE;
+ uint32_t page[PAGE_SIZE / 4];
+ int i;
+
+ spi_conf(CONF_ENABLE_W0);
+
+ /* Check some random page. Should be full of zeroes as backend is
+ * not initialized */
+ read_page(some_page_addr, page);
+ for (i = 0; i < PAGE_SIZE / 4; i++) {
+ g_assert_cmphex(page[i], ==, 0x0);
+ }
+
+ spi_ctrl_start_user();
+ writeb(AST2400_FLASH_BASE, WREN);
+ writeb(AST2400_FLASH_BASE, BULK_ERASE);
+ spi_ctrl_stop_user();
+
+ /* Recheck that some random page */
+ read_page(some_page_addr, page);
+ for (i = 0; i < PAGE_SIZE / 4; i++) {
+ g_assert_cmphex(page[i], ==, 0xffffffff);
+ }
+}
+
+static void test_write_page(void)
+{
+ uint32_t my_page_addr = 0x14000 * PAGE_SIZE; /* beyond 16MB */
+ uint32_t some_page_addr = 0x15000 * PAGE_SIZE;
+ uint32_t page[PAGE_SIZE / 4];
+ int i;
+
+ spi_conf(CONF_ENABLE_W0);
+
+ spi_ctrl_start_user();
+ writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
+ writeb(AST2400_FLASH_BASE, PP);
+ writel(AST2400_FLASH_BASE, cpu_to_be32(my_page_addr));
+
+ /* Fill the page with its own addresses */
+ for (i = 0; i < PAGE_SIZE / 4; i++) {
+ writel(AST2400_FLASH_BASE, cpu_to_be32(my_page_addr + i * 4));
+ }
+ spi_ctrl_stop_user();
+
+ /* Check what was written */
+ read_page(my_page_addr, page);
+ for (i = 0; i < PAGE_SIZE / 4; i++) {
+ g_assert_cmphex(page[i], ==, my_page_addr + i * 4);
+ }
+
+ /* Check some other page. It should be full of 0xff */
+ read_page(some_page_addr, page);
+ for (i = 0; i < PAGE_SIZE / 4; i++) {
+ g_assert_cmphex(page[i], ==, 0xffffffff);
+ }
+}
+
+static char tmp_path[] = "/tmp/qtest.m25p80.XXXXXX";
+
+int main(int argc, char **argv)
+{
+ int ret;
+ int fd;
+ char *args;
+
+ g_test_init(&argc, &argv, NULL);
+
+ fd = mkstemp(tmp_path);
+ g_assert(fd >= 0);
+ ret = ftruncate(fd, FLASH_SIZE);
+ g_assert(ret == 0);
+ close(fd);
+
+ args = g_strdup_printf("-m 256 -machine palmetto-bmc "
+ "-drive file=%s,format=raw,if=mtd",
+ tmp_path);
+ qtest_start(args);
+
+ qtest_add_func("/m25p80/read_jedec", test_read_jedec);
+ qtest_add_func("/m25p80/erase_sector", test_erase_sector);
+ qtest_add_func("/m25p80/erase_all", test_erase_all);
+ qtest_add_func("/m25p80/write_page", test_write_page);
+
+ ret = g_test_run();
+
+ qtest_quit(global_qtest);
+ unlink(tmp_path);
+ g_free(args);
+ return ret;
+}
--
2.1.4
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test Cédric Le Goater
@ 2016-07-01 17:18 ` Peter Maydell
2016-07-01 17:22 ` Peter Maydell
2016-07-01 17:30 ` Cédric Le Goater
2016-07-02 18:05 ` mar.krzeminski
1 sibling, 2 replies; 33+ messages in thread
From: Peter Maydell @ 2016-07-01 17:18 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 28 June 2016 at 19:24, Cédric Le Goater <clg@kaod.org> wrote:
> This test uses the palmetto platform and the AST2400 SPI controller to
> test the m25p80 flash module device model. The flash model is defined
> by the platform (n25q256a) and it would be nice to find way to control
> it, using a property probably.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>
This test fails on ppc64be:
TEST: tests/m25p80-test... (pid=65123)
/arm/m25p80/read_jedec: OK
/arm/m25p80/erase_sector: OK
/arm/m25p80/erase_all: **
ERROR:/home/pm215/qemu/tests/m25p80-test.c:162:test_erase_all:
assertion failed (page[i] == 0x0):
(0xffffffff == 0x00000000)
FAIL
GTester: last random seed: R02S54b2016fda21b092e18d7a23a2db86ba
(pid=65128)
/arm/m25p80/write_page: **
ERROR:/home/pm215/qemu/tests/m25p80-test.c:200:test_write_page:
assertion failed (page[i] == my_page_addr + i * 4): (0x00000000 ==
0x01400000)
FAIL
GTester: last random seed: R02S8708910d6b72f700bc41e9340a516239
(pid=65133)
FAIL: tests/m25p80-test
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test
2016-07-01 17:18 ` Peter Maydell
@ 2016-07-01 17:22 ` Peter Maydell
2016-07-01 17:30 ` Cédric Le Goater
1 sibling, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2016-07-01 17:22 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 1 July 2016 at 18:18, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 28 June 2016 at 19:24, Cédric Le Goater <clg@kaod.org> wrote:
>> This test uses the palmetto platform and the AST2400 SPI controller to
>> test the m25p80 flash module device model. The flash model is defined
>> by the platform (n25q256a) and it would be nice to find way to control
>> it, using a property probably.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>
>
> This test fails on ppc64be:
>
> TEST: tests/m25p80-test... (pid=65123)
> /arm/m25p80/read_jedec: OK
> /arm/m25p80/erase_sector: OK
> /arm/m25p80/erase_all: **
> ERROR:/home/pm215/qemu/tests/m25p80-test.c:162:test_erase_all:
> assertion failed (page[i] == 0x0):
> (0xffffffff == 0x00000000)
> FAIL
> GTester: last random seed: R02S54b2016fda21b092e18d7a23a2db86ba
> (pid=65128)
> /arm/m25p80/write_page: **
> ERROR:/home/pm215/qemu/tests/m25p80-test.c:200:test_write_page:
> assertion failed (page[i] == my_page_addr + i * 4): (0x00000000 ==
> 0x01400000)
> FAIL
> GTester: last random seed: R02S8708910d6b72f700bc41e9340a516239
> (pid=65133)
> FAIL: tests/m25p80-test
I'm going to take the easy approach of just dropping this patch;
please fix and resend it.
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test
2016-07-01 17:18 ` Peter Maydell
2016-07-01 17:22 ` Peter Maydell
@ 2016-07-01 17:30 ` Cédric Le Goater
2016-07-01 21:46 ` Greg Kurz
1 sibling, 1 reply; 33+ messages in thread
From: Cédric Le Goater @ 2016-07-01 17:30 UTC (permalink / raw)
To: Peter Maydell
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery, Greg Kurz
On 07/01/2016 07:18 PM, Peter Maydell wrote:
> On 28 June 2016 at 19:24, Cédric Le Goater <clg@kaod.org> wrote:
>> This test uses the palmetto platform and the AST2400 SPI controller to
>> test the m25p80 flash module device model. The flash model is defined
>> by the platform (n25q256a) and it would be nice to find way to control
>> it, using a property probably.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>
>
> This test fails on ppc64be:
>
> TEST: tests/m25p80-test... (pid=65123)
> /arm/m25p80/read_jedec: OK
> /arm/m25p80/erase_sector: OK
> /arm/m25p80/erase_all: **
> ERROR:/home/pm215/qemu/tests/m25p80-test.c:162:test_erase_all:
> assertion failed (page[i] == 0x0):
> (0xffffffff == 0x00000000)
> FAIL
> GTester: last random seed: R02S54b2016fda21b092e18d7a23a2db86ba
> (pid=65128)
> /arm/m25p80/write_page: **
> ERROR:/home/pm215/qemu/tests/m25p80-test.c:200:test_write_page:
> assertion failed (page[i] == my_page_addr + i * 4): (0x00000000 ==
> 0x01400000)
> FAIL
> GTester: last random seed: R02S8708910d6b72f700bc41e9340a516239
> (pid=65133)
> FAIL: tests/m25p80-test
yes ... I am not sure how to fix this :/
I started with a patch using qtest_big_endian() and I found that
this one was fixing the problem :
https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07876.html
but it feels wrong. The interesting part is that the guest fully
boots on a ppc64be. We need an endian shaman for this. Greg ?
Thanks,
C.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test
2016-07-01 17:30 ` Cédric Le Goater
@ 2016-07-01 21:46 ` Greg Kurz
0 siblings, 0 replies; 33+ messages in thread
From: Greg Kurz @ 2016-07-01 21:46 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Maydell, Peter Crosthwaite, QEMU Developers, qemu-arm,
Kevin Wolf, Markus Armbruster, Andrew Jeffery
On Fri, 1 Jul 2016 19:30:30 +0200
Cédric Le Goater <clg@kaod.org> wrote:
> On 07/01/2016 07:18 PM, Peter Maydell wrote:
> > On 28 June 2016 at 19:24, Cédric Le Goater <clg@kaod.org> wrote:
> >> This test uses the palmetto platform and the AST2400 SPI controller to
> >> test the m25p80 flash module device model. The flash model is defined
> >> by the platform (n25q256a) and it would be nice to find way to control
> >> it, using a property probably.
> >>
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> >> ---
> >>
> >
> > This test fails on ppc64be:
> >
> > TEST: tests/m25p80-test... (pid=65123)
> > /arm/m25p80/read_jedec: OK
> > /arm/m25p80/erase_sector: OK
> > /arm/m25p80/erase_all: **
> > ERROR:/home/pm215/qemu/tests/m25p80-test.c:162:test_erase_all:
> > assertion failed (page[i] == 0x0):
> > (0xffffffff == 0x00000000)
> > FAIL
> > GTester: last random seed: R02S54b2016fda21b092e18d7a23a2db86ba
> > (pid=65128)
> > /arm/m25p80/write_page: **
> > ERROR:/home/pm215/qemu/tests/m25p80-test.c:200:test_write_page:
> > assertion failed (page[i] == my_page_addr + i * 4): (0x00000000 ==
> > 0x01400000)
> > FAIL
> > GTester: last random seed: R02S8708910d6b72f700bc41e9340a516239
> > (pid=65133)
> > FAIL: tests/m25p80-test
>
> yes ... I am not sure how to fix this :/
>
> I started with a patch using qtest_big_endian() and I found that
> this one was fixing the problem :
>
> https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07876.html
>
> but it feels wrong. The interesting part is that the guest fully
> boots on a ppc64be. We need an endian shaman for this. Greg ?
>
Heh ! Looking at the Cc list I guess you have chances to find the
shaman you're looking for :)
Anyway, doing bswap32() systematically like in the patch mentioned above
looks weird indeed.
> Thanks,
>
> C.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test Cédric Le Goater
2016-07-01 17:18 ` Peter Maydell
@ 2016-07-02 18:05 ` mar.krzeminski
2016-07-04 9:15 ` Cédric Le Goater
1 sibling, 1 reply; 33+ messages in thread
From: mar.krzeminski @ 2016-07-02 18:05 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Peter Crosthwaite
Cc: kwolf, Andrew Jeffery, qemu-devel, armbru, qemu-arm
W dniu 28.06.2016 o 20:24, Cédric Le Goater pisze:
> This test uses the palmetto platform and the AST2400 SPI controller to
> test the m25p80 flash module device model. The flash model is defined
> by the platform (n25q256a) and it would be nice to find way to control
> it, using a property probably.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>
> Changes since v4:
>
> - fixed Makefile targets
> - replaced -M with -m in qtest command line
>
> Changes since v2:
>
> - changed mkstemp() path prefix
>
> Changes since v1:
>
> - fixed guest args to use -drive and not -mtdblock
>
> tests/Makefile.include | 2 +
> tests/m25p80-test.c | 242 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 244 insertions(+)
> create mode 100644 tests/m25p80-test.c
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 6c09962f7581..149f92585c1b 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -252,6 +252,7 @@ gcov-files-sparc-y += hw/timer/m48t59.c
> gcov-files-sparc64-y += hw/timer/m48t59.c
> check-qtest-arm-y = tests/tmp105-test$(EXESUF)
> check-qtest-arm-y += tests/ds1338-test$(EXESUF)
> +check-qtest-arm-y += tests/m25p80-test$(EXESUF)
> gcov-files-arm-y += hw/misc/tmp105.c
> check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
> gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
> @@ -568,6 +569,7 @@ tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
> tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
> tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
> tests/ds1338-test$(EXESUF): tests/ds1338-test.o $(libqos-imx-obj-y)
> +tests/m25p80-test$(EXESUF): tests/m25p80-test.o
> tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
> tests/q35-test$(EXESUF): tests/q35-test.o $(libqos-pc-obj-y)
> tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
> diff --git a/tests/m25p80-test.c b/tests/m25p80-test.c
> new file mode 100644
> index 000000000000..305c52319a33
> --- /dev/null
> +++ b/tests/m25p80-test.c
> @@ -0,0 +1,242 @@
> +/*
> + * QTest testcase for the M25P80 Flash (Using the AST2400 SPI Controller)
> + *
> + * Copyright (C) 2016 IBM Corp.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/bswap.h"
> +#include "libqtest.h"
> +
> +/*
> + * AST2400 SPI Controller registers
> + */
> +#define R_CONF 0x00
> +#define CONF_ENABLE_W0 (1 << 16)
> +#define R_CE_CTRL 0x04
> +#define CRTL_EXTENDED0 0 /* 32 bit addressing for SPI */
> +#define R_CTRL0 0x10
> +#define CTRL_CE_STOP_ACTIVE (1 << 2)
> +#define CTRL_USERMODE 0x3
> +
> +#define AST2400_FMC_BASE 0x1E620000
> +#define AST2400_FLASH_BASE 0x20000000
> +
> +/*
> + * Flash commands
> + */
> +enum {
> + JEDEC_READ = 0x9f,
> + BULK_ERASE = 0xc7,
> + READ = 0x03,
> + PP = 0x02,
> + WREN = 0x6,
> + EN_4BYTE_ADDR = 0xB7,
> + ERASE_SECTOR = 0xd8,
> +};
> +
> +#define FLASH_JEDEC 0x20ba19 /* n25q256a */
> +#define FLASH_SIZE (32 * 1024 * 1024)
> +
> +#define PAGE_SIZE 256
> +
> +static void spi_conf(uint32_t value)
> +{
> + uint32_t conf = readl(AST2400_FMC_BASE + R_CONF);
> +
> + conf |= value;
> + writel(AST2400_FMC_BASE + R_CONF, conf);
> +}
> +
> +static void spi_ctrl_start_user(void)
> +{
> + uint32_t ctrl = readl(AST2400_FMC_BASE + R_CTRL0);
> +
> + ctrl |= CTRL_USERMODE | CTRL_CE_STOP_ACTIVE;
> + writel(AST2400_FMC_BASE + R_CTRL0, ctrl);
> +
> + ctrl &= ~CTRL_CE_STOP_ACTIVE;
> + writel(AST2400_FMC_BASE + R_CTRL0, ctrl);
> +}
> +
> +static void spi_ctrl_stop_user(void)
> +{
> + uint32_t ctrl = readl(AST2400_FMC_BASE + R_CTRL0);
> +
> + ctrl |= CTRL_USERMODE | CTRL_CE_STOP_ACTIVE;
> + writel(AST2400_FMC_BASE + R_CTRL0, ctrl);
> +}
> +
> +static void test_read_jedec(void)
> +{
> + uint32_t jedec = 0x0;
> +
> + spi_conf(CONF_ENABLE_W0);
> +
> + spi_ctrl_start_user();
> + writeb(AST2400_FLASH_BASE, JEDEC_READ);
> + jedec |= readb(AST2400_FLASH_BASE) << 16;
> + jedec |= readb(AST2400_FLASH_BASE) << 8;
> + jedec |= readb(AST2400_FLASH_BASE);
> + spi_ctrl_stop_user();
> +
> + g_assert_cmphex(jedec, ==, FLASH_JEDEC);
> +}
> +
> +static void read_page(uint32_t addr, uint32_t *page)
> +{
> + int i;
> +
> + spi_ctrl_start_user();
> +
> + writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
> + writeb(AST2400_FLASH_BASE, READ);
> + writel(AST2400_FLASH_BASE, cpu_to_be32(addr));
> +
> + /* Continuous read are supported */
> + for (i = 0; i < PAGE_SIZE / 4; i++) {
> + page[i] = be32_to_cpu(readl(AST2400_FLASH_BASE));
> + }
> + spi_ctrl_stop_user();
> +}
> +
> +static void test_erase_sector(void)
> +{
> + uint32_t some_page_addr = 0x600 * PAGE_SIZE;
> + uint32_t page[PAGE_SIZE / 4];
> + int i;
> +
> + spi_conf(CONF_ENABLE_W0);
> +
> + spi_ctrl_start_user();
> + writeb(AST2400_FLASH_BASE, WREN);
> + writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
> + writeb(AST2400_FLASH_BASE, ERASE_SECTOR);
> + writel(AST2400_FLASH_BASE, cpu_to_be32(some_page_addr));
Hi,
I think you should not make any byte swapping because
qtest for all write* instructions (see qtest.c:377).
Regards,
Marcin
> + spi_ctrl_stop_user();
> +
> + /* Previous page should be full of zeroes as backend is not
> + * initialized */
> + read_page(some_page_addr - PAGE_SIZE, page);
> + for (i = 0; i < PAGE_SIZE / 4; i++) {
> + g_assert_cmphex(page[i], ==, 0x0);
> + }
> +
> + /* But this one was erased */
> + read_page(some_page_addr, page);
> + for (i = 0; i < PAGE_SIZE / 4; i++) {
> + g_assert_cmphex(page[i], ==, 0xffffffff);
> + }
> +}
> +
> +static void test_erase_all(void)
> +{
> + uint32_t some_page_addr = 0x15000 * PAGE_SIZE;
> + uint32_t page[PAGE_SIZE / 4];
> + int i;
> +
> + spi_conf(CONF_ENABLE_W0);
> +
> + /* Check some random page. Should be full of zeroes as backend is
> + * not initialized */
> + read_page(some_page_addr, page);
> + for (i = 0; i < PAGE_SIZE / 4; i++) {
> + g_assert_cmphex(page[i], ==, 0x0);
> + }
> +
> + spi_ctrl_start_user();
> + writeb(AST2400_FLASH_BASE, WREN);
> + writeb(AST2400_FLASH_BASE, BULK_ERASE);
> + spi_ctrl_stop_user();
> +
> + /* Recheck that some random page */
> + read_page(some_page_addr, page);
> + for (i = 0; i < PAGE_SIZE / 4; i++) {
> + g_assert_cmphex(page[i], ==, 0xffffffff);
> + }
> +}
> +
> +static void test_write_page(void)
> +{
> + uint32_t my_page_addr = 0x14000 * PAGE_SIZE; /* beyond 16MB */
> + uint32_t some_page_addr = 0x15000 * PAGE_SIZE;
> + uint32_t page[PAGE_SIZE / 4];
> + int i;
> +
> + spi_conf(CONF_ENABLE_W0);
> +
> + spi_ctrl_start_user();
> + writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
> + writeb(AST2400_FLASH_BASE, PP);
> + writel(AST2400_FLASH_BASE, cpu_to_be32(my_page_addr));
> +
> + /* Fill the page with its own addresses */
> + for (i = 0; i < PAGE_SIZE / 4; i++) {
> + writel(AST2400_FLASH_BASE, cpu_to_be32(my_page_addr + i * 4));
> + }
> + spi_ctrl_stop_user();
> +
> + /* Check what was written */
> + read_page(my_page_addr, page);
> + for (i = 0; i < PAGE_SIZE / 4; i++) {
> + g_assert_cmphex(page[i], ==, my_page_addr + i * 4);
> + }
> +
> + /* Check some other page. It should be full of 0xff */
> + read_page(some_page_addr, page);
> + for (i = 0; i < PAGE_SIZE / 4; i++) {
> + g_assert_cmphex(page[i], ==, 0xffffffff);
> + }
> +}
> +
> +static char tmp_path[] = "/tmp/qtest.m25p80.XXXXXX";
> +
> +int main(int argc, char **argv)
> +{
> + int ret;
> + int fd;
> + char *args;
> +
> + g_test_init(&argc, &argv, NULL);
> +
> + fd = mkstemp(tmp_path);
> + g_assert(fd >= 0);
> + ret = ftruncate(fd, FLASH_SIZE);
> + g_assert(ret == 0);
> + close(fd);
> +
> + args = g_strdup_printf("-m 256 -machine palmetto-bmc "
> + "-drive file=%s,format=raw,if=mtd",
> + tmp_path);
> + qtest_start(args);
> +
> + qtest_add_func("/m25p80/read_jedec", test_read_jedec);
> + qtest_add_func("/m25p80/erase_sector", test_erase_sector);
> + qtest_add_func("/m25p80/erase_all", test_erase_all);
> + qtest_add_func("/m25p80/write_page", test_write_page);
> +
> + ret = g_test_run();
> +
> + qtest_quit(global_qtest);
> + unlink(tmp_path);
> + g_free(args);
> + return ret;
> +}
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test
2016-07-02 18:05 ` mar.krzeminski
@ 2016-07-04 9:15 ` Cédric Le Goater
2016-07-04 9:50 ` Cédric Le Goater
2016-07-04 11:14 ` Peter Maydell
0 siblings, 2 replies; 33+ messages in thread
From: Cédric Le Goater @ 2016-07-04 9:15 UTC (permalink / raw)
To: mar.krzeminski, Peter Maydell, Peter Crosthwaite
Cc: kwolf, Andrew Jeffery, qemu-devel, armbru, qemu-arm
On 07/02/2016 08:05 PM, mar.krzeminski wrote:
>
>
> W dniu 28.06.2016 o 20:24, Cédric Le Goater pisze:
>> This test uses the palmetto platform and the AST2400 SPI controller to
>> test the m25p80 flash module device model. The flash model is defined
>> by the platform (n25q256a) and it would be nice to find way to control
>> it, using a property probably.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>
>> Changes since v4:
>>
>> - fixed Makefile targets
>> - replaced -M with -m in qtest command line
>> Changes since v2:
>>
>> - changed mkstemp() path prefix
>> Changes since v1:
>>
>> - fixed guest args to use -drive and not -mtdblock
>>
>> tests/Makefile.include | 2 +
>> tests/m25p80-test.c | 242 +++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 244 insertions(+)
>> create mode 100644 tests/m25p80-test.c
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index 6c09962f7581..149f92585c1b 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -252,6 +252,7 @@ gcov-files-sparc-y += hw/timer/m48t59.c
>> gcov-files-sparc64-y += hw/timer/m48t59.c
>> check-qtest-arm-y = tests/tmp105-test$(EXESUF)
>> check-qtest-arm-y += tests/ds1338-test$(EXESUF)
>> +check-qtest-arm-y += tests/m25p80-test$(EXESUF)
>> gcov-files-arm-y += hw/misc/tmp105.c
>> check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
>> gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
>> @@ -568,6 +569,7 @@ tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
>> tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
>> tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
>> tests/ds1338-test$(EXESUF): tests/ds1338-test.o $(libqos-imx-obj-y)
>> +tests/m25p80-test$(EXESUF): tests/m25p80-test.o
>> tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
>> tests/q35-test$(EXESUF): tests/q35-test.o $(libqos-pc-obj-y)
>> tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
>> diff --git a/tests/m25p80-test.c b/tests/m25p80-test.c
>> new file mode 100644
>> index 000000000000..305c52319a33
>> --- /dev/null
>> +++ b/tests/m25p80-test.c
>> @@ -0,0 +1,242 @@
>> +/*
>> + * QTest testcase for the M25P80 Flash (Using the AST2400 SPI Controller)
>> + *
>> + * Copyright (C) 2016 IBM Corp.
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to deal
>> + * in the Software without restriction, including without limitation the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/bswap.h"
>> +#include "libqtest.h"
>> +
>> +/*
>> + * AST2400 SPI Controller registers
>> + */
>> +#define R_CONF 0x00
>> +#define CONF_ENABLE_W0 (1 << 16)
>> +#define R_CE_CTRL 0x04
>> +#define CRTL_EXTENDED0 0 /* 32 bit addressing for SPI */
>> +#define R_CTRL0 0x10
>> +#define CTRL_CE_STOP_ACTIVE (1 << 2)
>> +#define CTRL_USERMODE 0x3
>> +
>> +#define AST2400_FMC_BASE 0x1E620000
>> +#define AST2400_FLASH_BASE 0x20000000
>> +
>> +/*
>> + * Flash commands
>> + */
>> +enum {
>> + JEDEC_READ = 0x9f,
>> + BULK_ERASE = 0xc7,
>> + READ = 0x03,
>> + PP = 0x02,
>> + WREN = 0x6,
>> + EN_4BYTE_ADDR = 0xB7,
>> + ERASE_SECTOR = 0xd8,
>> +};
>> +
>> +#define FLASH_JEDEC 0x20ba19 /* n25q256a */
>> +#define FLASH_SIZE (32 * 1024 * 1024)
>> +
>> +#define PAGE_SIZE 256
>> +
>> +static void spi_conf(uint32_t value)
>> +{
>> + uint32_t conf = readl(AST2400_FMC_BASE + R_CONF);
>> +
>> + conf |= value;
>> + writel(AST2400_FMC_BASE + R_CONF, conf);
>> +}
>> +
>> +static void spi_ctrl_start_user(void)
>> +{
>> + uint32_t ctrl = readl(AST2400_FMC_BASE + R_CTRL0);
>> +
>> + ctrl |= CTRL_USERMODE | CTRL_CE_STOP_ACTIVE;
>> + writel(AST2400_FMC_BASE + R_CTRL0, ctrl);
>> +
>> + ctrl &= ~CTRL_CE_STOP_ACTIVE;
>> + writel(AST2400_FMC_BASE + R_CTRL0, ctrl);
>> +}
>> +
>> +static void spi_ctrl_stop_user(void)
>> +{
>> + uint32_t ctrl = readl(AST2400_FMC_BASE + R_CTRL0);
>> +
>> + ctrl |= CTRL_USERMODE | CTRL_CE_STOP_ACTIVE;
>> + writel(AST2400_FMC_BASE + R_CTRL0, ctrl);
>> +}
>> +
>> +static void test_read_jedec(void)
>> +{
>> + uint32_t jedec = 0x0;
>> +
>> + spi_conf(CONF_ENABLE_W0);
>> +
>> + spi_ctrl_start_user();
>> + writeb(AST2400_FLASH_BASE, JEDEC_READ);
>> + jedec |= readb(AST2400_FLASH_BASE) << 16;
>> + jedec |= readb(AST2400_FLASH_BASE) << 8;
>> + jedec |= readb(AST2400_FLASH_BASE);
>> + spi_ctrl_stop_user();
>> +
>> + g_assert_cmphex(jedec, ==, FLASH_JEDEC);
>> +}
>> +
>> +static void read_page(uint32_t addr, uint32_t *page)
>> +{
>> + int i;
>> +
>> + spi_ctrl_start_user();
>> +
>> + writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
>> + writeb(AST2400_FLASH_BASE, READ);
>> + writel(AST2400_FLASH_BASE, cpu_to_be32(addr));
>> +
>> + /* Continuous read are supported */
>> + for (i = 0; i < PAGE_SIZE / 4; i++) {
>> + page[i] = be32_to_cpu(readl(AST2400_FLASH_BASE));
>> + }
>> + spi_ctrl_stop_user();
>> +}
>> +
>> +static void test_erase_sector(void)
>> +{
>> + uint32_t some_page_addr = 0x600 * PAGE_SIZE;
>> + uint32_t page[PAGE_SIZE / 4];
>> + int i;
>> +
>> + spi_conf(CONF_ENABLE_W0);
>> +
>> + spi_ctrl_start_user();
>> + writeb(AST2400_FLASH_BASE, WREN);
>> + writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
>> + writeb(AST2400_FLASH_BASE, ERASE_SECTOR);
>> + writel(AST2400_FLASH_BASE, cpu_to_be32(some_page_addr));
> Hi,
>
> I think you should not make any byte swapping because
> qtest for all write* instructions (see qtest.c:377).
yes. on a BE, we should not byte swap.
When on a LE host, the cpu_to_be32() call in the test byte-swaps
the address, tswap32() in qtest.c does nothing as the host endian
and guest endian are in sync and so the address reaches the
controller region in the expected order.
When on a BE host, the cpu_to_be32() does nothing but tswap32() in
qtest.c reverses the order and so the address reaches the controller
region in the wrong order (LE).
I see two possible fixes :
1 - replace the read* routines by out* (no tswap there)
2 - add an extra byteswap on BE hosts using qtest_big_endian() as
in virtio-blk-test.c. see virtio_blk_fix_request()
Thanks,
C.
> Regards,
> Marcin
>
>> + spi_ctrl_stop_user();
>> +
>> + /* Previous page should be full of zeroes as backend is not
>> + * initialized */
>> + read_page(some_page_addr - PAGE_SIZE, page);
>> + for (i = 0; i < PAGE_SIZE / 4; i++) {
>> + g_assert_cmphex(page[i], ==, 0x0);
>> + }
>> +
>> + /* But this one was erased */
>> + read_page(some_page_addr, page);
>> + for (i = 0; i < PAGE_SIZE / 4; i++) {
>> + g_assert_cmphex(page[i], ==, 0xffffffff);
>> + }
>> +}
>> +
>> +static void test_erase_all(void)
>> +{
>> + uint32_t some_page_addr = 0x15000 * PAGE_SIZE;
>> + uint32_t page[PAGE_SIZE / 4];
>> + int i;
>> +
>> + spi_conf(CONF_ENABLE_W0);
>> +
>> + /* Check some random page. Should be full of zeroes as backend is
>> + * not initialized */
>> + read_page(some_page_addr, page);
>> + for (i = 0; i < PAGE_SIZE / 4; i++) {
>> + g_assert_cmphex(page[i], ==, 0x0);
>> + }
>> +
>> + spi_ctrl_start_user();
>> + writeb(AST2400_FLASH_BASE, WREN);
>> + writeb(AST2400_FLASH_BASE, BULK_ERASE);
>> + spi_ctrl_stop_user();
>> +
>> + /* Recheck that some random page */
>> + read_page(some_page_addr, page);
>> + for (i = 0; i < PAGE_SIZE / 4; i++) {
>> + g_assert_cmphex(page[i], ==, 0xffffffff);
>> + }
>> +}
>> +
>> +static void test_write_page(void)
>> +{
>> + uint32_t my_page_addr = 0x14000 * PAGE_SIZE; /* beyond 16MB */
>> + uint32_t some_page_addr = 0x15000 * PAGE_SIZE;
>> + uint32_t page[PAGE_SIZE / 4];
>> + int i;
>> +
>> + spi_conf(CONF_ENABLE_W0);
>> +
>> + spi_ctrl_start_user();
>> + writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
>> + writeb(AST2400_FLASH_BASE, PP);
>> + writel(AST2400_FLASH_BASE, cpu_to_be32(my_page_addr));
>> +
>> + /* Fill the page with its own addresses */
>> + for (i = 0; i < PAGE_SIZE / 4; i++) {
>> + writel(AST2400_FLASH_BASE, cpu_to_be32(my_page_addr + i * 4));
>> + }
>> + spi_ctrl_stop_user();
>> +
>> + /* Check what was written */
>> + read_page(my_page_addr, page);
>> + for (i = 0; i < PAGE_SIZE / 4; i++) {
>> + g_assert_cmphex(page[i], ==, my_page_addr + i * 4);
>> + }
>> +
>> + /* Check some other page. It should be full of 0xff */
>> + read_page(some_page_addr, page);
>> + for (i = 0; i < PAGE_SIZE / 4; i++) {
>> + g_assert_cmphex(page[i], ==, 0xffffffff);
>> + }
>> +}
>> +
>> +static char tmp_path[] = "/tmp/qtest.m25p80.XXXXXX";
>> +
>> +int main(int argc, char **argv)
>> +{
>> + int ret;
>> + int fd;
>> + char *args;
>> +
>> + g_test_init(&argc, &argv, NULL);
>> +
>> + fd = mkstemp(tmp_path);
>> + g_assert(fd >= 0);
>> + ret = ftruncate(fd, FLASH_SIZE);
>> + g_assert(ret == 0);
>> + close(fd);
>> +
>> + args = g_strdup_printf("-m 256 -machine palmetto-bmc "
>> + "-drive file=%s,format=raw,if=mtd",
>> + tmp_path);
>> + qtest_start(args);
>> +
>> + qtest_add_func("/m25p80/read_jedec", test_read_jedec);
>> + qtest_add_func("/m25p80/erase_sector", test_erase_sector);
>> + qtest_add_func("/m25p80/erase_all", test_erase_all);
>> + qtest_add_func("/m25p80/write_page", test_write_page);
>> +
>> + ret = g_test_run();
>> +
>> + qtest_quit(global_qtest);
>> + unlink(tmp_path);
>> + g_free(args);
>> + return ret;
>> +}
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test
2016-07-04 9:15 ` Cédric Le Goater
@ 2016-07-04 9:50 ` Cédric Le Goater
2016-07-04 11:14 ` Peter Maydell
1 sibling, 0 replies; 33+ messages in thread
From: Cédric Le Goater @ 2016-07-04 9:50 UTC (permalink / raw)
To: mar.krzeminski, Peter Maydell, Peter Crosthwaite
Cc: kwolf, Andrew Jeffery, qemu-devel, armbru, qemu-arm
>>> +static void test_erase_sector(void)
>>> +{
>>> + uint32_t some_page_addr = 0x600 * PAGE_SIZE;
>>> + uint32_t page[PAGE_SIZE / 4];
>>> + int i;
>>> +
>>> + spi_conf(CONF_ENABLE_W0);
>>> +
>>> + spi_ctrl_start_user();
>>> + writeb(AST2400_FLASH_BASE, WREN);
>>> + writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
>>> + writeb(AST2400_FLASH_BASE, ERASE_SECTOR);
>>> + writel(AST2400_FLASH_BASE, cpu_to_be32(some_page_addr));
>> Hi,
>>
>> I think you should not make any byte swapping because
>> qtest for all write* instructions (see qtest.c:377).
>
> yes. on a BE, we should not byte swap.
>
> When on a LE host, the cpu_to_be32() call in the test byte-swaps
> the address, tswap32() in qtest.c does nothing as the host endian
> and guest endian are in sync and so the address reaches the
> controller region in the expected order.
>
> When on a BE host, the cpu_to_be32() does nothing but tswap32() in
> qtest.c reverses the order and so the address reaches the controller
> region in the wrong order (LE).
>
> I see two possible fixes :
>
> 1 - replace the read* routines by out* (no tswap there)
> 2 - add an extra byteswap on BE hosts using qtest_big_endian() as
> in virtio-blk-test.c. see virtio_blk_fix_request()
or :
3 - use memwrite/read
which is I think the appropriate fix.
Thanks,
C.
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test
2016-07-04 9:15 ` Cédric Le Goater
2016-07-04 9:50 ` Cédric Le Goater
@ 2016-07-04 11:14 ` Peter Maydell
2016-07-04 12:01 ` Cédric Le Goater
1 sibling, 1 reply; 33+ messages in thread
From: Peter Maydell @ 2016-07-04 11:14 UTC (permalink / raw)
To: Cédric Le Goater
Cc: mar.krzeminski, Peter Crosthwaite, Kevin Wolf, Andrew Jeffery,
QEMU Developers, Markus Armbruster, qemu-arm
On 4 July 2016 at 10:15, Cédric Le Goater <clg@kaod.org> wrote:
> On 07/02/2016 08:05 PM, mar.krzeminski wrote:
>>
>>
>> W dniu 28.06.2016 o 20:24, Cédric Le Goater pisze:
>>> +static void test_erase_sector(void)
>>> +{
>>> + uint32_t some_page_addr = 0x600 * PAGE_SIZE;
>>> + uint32_t page[PAGE_SIZE / 4];
>>> + int i;
>>> +
>>> + spi_conf(CONF_ENABLE_W0);
>>> +
>>> + spi_ctrl_start_user();
>>> + writeb(AST2400_FLASH_BASE, WREN);
>>> + writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
>>> + writeb(AST2400_FLASH_BASE, ERASE_SECTOR);
>>> + writel(AST2400_FLASH_BASE, cpu_to_be32(some_page_addr));
>> Hi,
>>
>> I think you should not make any byte swapping because
>> qtest for all write* instructions (see qtest.c:377).
>
> yes. on a BE, we should not byte swap.
>
> When on a LE host, the cpu_to_be32() call in the test byte-swaps
> the address, tswap32() in qtest.c does nothing as the host endian
> and guest endian are in sync and so the address reaches the
> controller region in the expected order.
>
> When on a BE host, the cpu_to_be32() does nothing but tswap32() in
> qtest.c reverses the order and so the address reaches the controller
> region in the wrong order (LE).
>
> I see two possible fixes :
>
> 1 - replace the read* routines by out* (no tswap there)
This would be wrong: "out*" is a request for an I/O access,
which is meaningless for ARM. (On x86 it's an actual outb/outw/outl
instruction.) In any case it wouldn't change the number of
byteswaps that happen:
* "outl" qtest command calls cpu_outl(), which calls
stl_p() on the value, to do "store in target endianness
to buffer"; stl_p() will do a byteswap if the target and
host endianness differ. The buffer is then passed to
address_space_write(), which is a wrapper for
address_space_rw(), which treats its buffer as a pile
of bytes to be written.
* "writel" qtest command does a tswap32s() on the data, which
converts it to target endianness in a local variable.
The address of the variable is then passed to
cpu_physical_memory_write(), which is just a thin wrapper
around address_space_rw(), which as above treats its
buffer as a pile of bytes.
So the two things end up the same, it's just that for writel
we do the conversion from a host-ordered value to a
target-ordered value in a direct tswapl(), but for outl
this happens implicitly in cpu_outl()'s stl_p().
> 2 - add an extra byteswap on BE hosts using qtest_big_endian() as
> in virtio-blk-test.c. see virtio_blk_fix_request()
What is the test actually supposed to be doing?
writel() says "write this 32 bit value as if the (guest) CPU
wrote it to memory with a 32-bit write instruction".
Your problem here is entirely on the test code side, I think.
It should be issuing the same commands down the qtest protocol
for any host endianness, but as it stands on a little endian
host it will issue "writel 0x20000000 0x600" whereas on a big
endian host it will issue "writel 0x20000000 0x60000".
(virtio is special because it has some weird ideas about
endianness due to being a virtual-only device with a spec
written by all-the-world-is-little-endian x86 people, so
it's a bad example to copy.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test
2016-07-04 11:14 ` Peter Maydell
@ 2016-07-04 12:01 ` Cédric Le Goater
2016-07-04 12:11 ` Peter Maydell
0 siblings, 1 reply; 33+ messages in thread
From: Cédric Le Goater @ 2016-07-04 12:01 UTC (permalink / raw)
To: Peter Maydell
Cc: mar.krzeminski, Peter Crosthwaite, Kevin Wolf, Andrew Jeffery,
QEMU Developers, Markus Armbruster, qemu-arm
On 07/04/2016 01:14 PM, Peter Maydell wrote:
> On 4 July 2016 at 10:15, Cédric Le Goater <clg@kaod.org> wrote:
>> On 07/02/2016 08:05 PM, mar.krzeminski wrote:
>>>
>>>
>>> W dniu 28.06.2016 o 20:24, Cédric Le Goater pisze:
>
>>>> +static void test_erase_sector(void)
>>>> +{
>>>> + uint32_t some_page_addr = 0x600 * PAGE_SIZE;
>>>> + uint32_t page[PAGE_SIZE / 4];
>>>> + int i;
>>>> +
>>>> + spi_conf(CONF_ENABLE_W0);
>>>> +
>>>> + spi_ctrl_start_user();
>>>> + writeb(AST2400_FLASH_BASE, WREN);
>>>> + writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
>>>> + writeb(AST2400_FLASH_BASE, ERASE_SECTOR);
>>>> + writel(AST2400_FLASH_BASE, cpu_to_be32(some_page_addr));
>>> Hi,
>>>
>>> I think you should not make any byte swapping because
>>> qtest for all write* instructions (see qtest.c:377).
>>
>> yes. on a BE, we should not byte swap.
>>
>> When on a LE host, the cpu_to_be32() call in the test byte-swaps
>> the address, tswap32() in qtest.c does nothing as the host endian
>> and guest endian are in sync and so the address reaches the
>> controller region in the expected order.
>>
>> When on a BE host, the cpu_to_be32() does nothing but tswap32() in
>> qtest.c reverses the order and so the address reaches the controller
>> region in the wrong order (LE).
>>
>> I see two possible fixes :
>>
>> 1 - replace the read* routines by out* (no tswap there)
>
> This would be wrong: "out*" is a request for an I/O access,
yes. that felt really wrong.
> which is meaningless for ARM. (On x86 it's an actual outb/outw/outl
> instruction.) In any case it wouldn't change the number of
> byteswaps that happen:
>
> * "outl" qtest command calls cpu_outl(), which calls
> stl_p() on the value, to do "store in target endianness
> to buffer"; stl_p() will do a byteswap if the target and
> host endianness differ. The buffer is then passed to
> address_space_write(), which is a wrapper for
> address_space_rw(), which treats its buffer as a pile
> of bytes to be written.
> * "writel" qtest command does a tswap32s() on the data, which
> converts it to target endianness in a local variable.
> The address of the variable is then passed to
> cpu_physical_memory_write(), which is just a thin wrapper
> around address_space_rw(), which as above treats its
> buffer as a pile of bytes.
>
> So the two things end up the same, it's just that for writel
> we do the conversion from a host-ordered value to a
> target-ordered value in a direct tswapl(), but for outl
> this happens implicitly in cpu_outl()'s stl_p().
ok. thanks for the summary/explanation.
>> 2 - add an extra byteswap on BE hosts using qtest_big_endian() as
>> in virtio-blk-test.c. see virtio_blk_fix_request()
>
> What is the test actually supposed to be doing?
> writel() says "write this 32 bit value as if the (guest) CPU
> wrote it to memory with a 32-bit write instruction".
The test (and the guest) is writing and reading data on the memory
region used by the SPI controller. This 'data' is then passed on
to the SPI flash module objects which expects BE order when there
are flash storage addresses are in the flow.
So I think the test needs to use mem{write,read} and not write*.
The result looks correct in the code. I will send a patchset
right after starting with this patch.
> Your problem here is entirely on the test code side, I think.
As the guest boots correctly, I would say yes also.
> It should be issuing the same commands down the qtest protocol
> for any host endianness, but as it stands on a little endian
> host it will issue "writel 0x20000000 0x600" whereas on a big
> endian host it will issue "writel 0x20000000 0x60000".
>
> (virtio is special because it has some weird ideas about
> endianness due to being a virtual-only device with a spec
> written by all-the-world-is-little-endian x86 people, so
> it's a bad example to copy.)
yes, we had a taste of that when virtio support was added for
the ppc64be guests :)
Thanks,
C.
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test
2016-07-04 12:01 ` Cédric Le Goater
@ 2016-07-04 12:11 ` Peter Maydell
2016-07-04 12:32 ` Cédric Le Goater
0 siblings, 1 reply; 33+ messages in thread
From: Peter Maydell @ 2016-07-04 12:11 UTC (permalink / raw)
To: Cédric Le Goater
Cc: mar.krzeminski, Peter Crosthwaite, Kevin Wolf, Andrew Jeffery,
QEMU Developers, Markus Armbruster, qemu-arm
On 4 July 2016 at 13:01, Cédric Le Goater <clg@kaod.org> wrote:
> On 07/04/2016 01:14 PM, Peter Maydell wrote:
>> What is the test actually supposed to be doing?
>> writel() says "write this 32 bit value as if the (guest) CPU
>> wrote it to memory with a 32-bit write instruction".
>
> The test (and the guest) is writing and reading data on the memory
> region used by the SPI controller. This 'data' is then passed on
> to the SPI flash module objects which expects BE order when there
> are flash storage addresses are in the flow.
>
> So I think the test needs to use mem{write,read} and not write*.
> The result looks correct in the code. I will send a patchset
> right after starting with this patch.
If you were writing this test in a bit of native ARM code
running in the guest, how would you write it?
memread/memwrite are only the correct answer if you'd write
the native arm code as something like
memcpy(SPI_REGISTER, buf, len), which is not generally the
way you talk to devices.
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test
2016-07-04 12:11 ` Peter Maydell
@ 2016-07-04 12:32 ` Cédric Le Goater
0 siblings, 0 replies; 33+ messages in thread
From: Cédric Le Goater @ 2016-07-04 12:32 UTC (permalink / raw)
To: Peter Maydell
Cc: mar.krzeminski, Peter Crosthwaite, Kevin Wolf, Andrew Jeffery,
QEMU Developers, Markus Armbruster, qemu-arm
On 07/04/2016 02:11 PM, Peter Maydell wrote:
> On 4 July 2016 at 13:01, Cédric Le Goater <clg@kaod.org> wrote:
>> On 07/04/2016 01:14 PM, Peter Maydell wrote:
>>> What is the test actually supposed to be doing?
>>> writel() says "write this 32 bit value as if the (guest) CPU
>>> wrote it to memory with a 32-bit write instruction".
>>
>> The test (and the guest) is writing and reading data on the memory
>> region used by the SPI controller. This 'data' is then passed on
>> to the SPI flash module objects which expects BE order when there
>> are flash storage addresses are in the flow.
>>
>> So I think the test needs to use mem{write,read} and not write*.
>> The result looks correct in the code. I will send a patchset
>> right after starting with this patch.
>
> If you were writing this test in a bit of native ARM code
> running in the guest, how would you write it?
> memread/memwrite are only the correct answer if you'd write
> the native arm code as something like
> memcpy(SPI_REGISTER, buf, len), which is not generally the
> way you talk to devices.
The SMC/SPI controller are driven by mmios and the flash module
in a very similar way. The modules are mapped at some address,
0x2000000 for the BMC, 0x3000000 for the host firmware. Writing
to the mapped address triggers SPI transfers, you need to put the
slave in user mode for that, by setting the appropriate registers
in the controller. When not in that mode, you are in command mode,
and in this case you access directly the contents of the flash.
Thanks,
C.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v5 0/9] ast2400: SMC controllers
2016-06-28 18:24 [Qemu-devel] [PATCH v5 0/9] ast2400: SMC controllers Cédric Le Goater
` (8 preceding siblings ...)
2016-06-28 18:24 ` [Qemu-devel] [PATCH v5 9/9] tests: add a m25p80 test Cédric Le Goater
@ 2016-07-01 16:25 ` Peter Maydell
9 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2016-07-01 16:25 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 28 June 2016 at 19:24, Cédric Le Goater <clg@kaod.org> wrote:
> Hello,
>
> The following adds support for the AST2400 SMC controllers. The device
> model does not implement all the HW features, linear addressing mode
> is underway, but it should be complete enough for the OpenBMC distro.
>
> Flash images can be grabbed here for testing :
>
> https://openpower.xyz/job/openbmc-build/distro=ubuntu,target=palmetto/lastSuccessfulBuild/artifact/images/palmetto/flash-palmetto
> https://openpower.xyz/job/openpower-op-build/distro=ubuntu,target=palmetto/lastSuccessfulBuild/artifact/images/palmetto.pnor
>
> Based on commit dc154b1db4b9.
Applied to target-arm.next, with the g_new0() nit and
the missing error_propagate() fixed.
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread