* [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support
@ 2022-06-27 19:54 Peter Delevoryas
2022-06-27 19:54 ` [PATCH 01/14] hw/i2c: support multiple masters Peter Delevoryas
` (12 more replies)
0 siblings, 13 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 19:54 UTC (permalink / raw)
Cc: pdel, zhdaniel, clg, qemu-devel, qemu-arm, komlodi, titusr, andrew, joel
Hey everyone,
I'm sending a big patch series for this, but only the last commit is really
intended to be accepted right now. I'm just including the rest of them
because it depends on them for testing.
Klaus's changes include the multi-master stuff in hw/i2c/core.c, and the old
register mode slave mode support.
My series of patches includes a bunch of changes to eliminate most (if not
all) of the I2C errors reported by the fb OpenBIC firmware for fby35
CraterLake Bridge Interconnect (BIC) (shortname: oby35-cl) upon startup.
In particular, I needed to add the IC_DEVICE_ID to the isl voltage regulator
implementation, which required a fix to the pmbus implementation when
switching pages. We weren't resetting the buffer state when switching
pages there.
I also added a placeholder implementation of the PECI controller, that does
almost nothing, but doesn't produce errors.
I added the fby35-cpld, which oby35-cl queries using master-mode TX and RX
commands.
And finally, I added an "intel-me" device (Intel Management Engine) that
attempts to respond to the first IPMB message sent by the BIC. I used this
to test the final patch, which I actually want to merge, the I2C new
register DMA slave mode support.
All the patches except the last one can be ignored for now if you want,
again, I just included them for testing purposes.
The final patch is pretty rough, but I wanted to start the review instead of
waiting too long. I expect the interrupt handling part will be
the main blocker.
Thanks,
Peter
Klaus Jensen (3):
hw/i2c: support multiple masters
hw/i2c: add asynchronous send
hw/i2c/aspeed: add slave device in old register mode
Peter Delevoryas (11):
aspeed: i2c: Fix DMA len write-enable bit handling
aspeed: i2c: Fix R_I2CD_FUN_CTRL reference
aspeed: i2c: Fix MASTER_EN missing error message
aspeed: Add PECI controller
hw/misc: Add fby35-cpld
pmbus: Reset out buf after switching pages
pmbus: Add read-only IC_DEVICE_ID support
aspeed: Add oby35-cl machine
hw/misc: Add intel-me
aspeed: Add intel-me on i2c6 instead of BMC
aspeed: Add I2C new register DMA slave mode support
hw/arm/aspeed.c | 42 ++++++
hw/arm/aspeed_ast10x0.c | 11 ++
hw/arm/pxa2xx.c | 2 +
hw/display/sii9022.c | 2 +
hw/display/ssd0303.c | 2 +
hw/i2c/aspeed_i2c.c | 234 +++++++++++++++++++++++++++----
hw/i2c/core.c | 70 ++++++++-
hw/i2c/pmbus_device.c | 6 +
hw/i2c/smbus_slave.c | 4 +
hw/i2c/trace-events | 2 +
hw/misc/aspeed_peci.c | 225 +++++++++++++++++++++++++++++
hw/misc/fby35_cpld.c | 137 ++++++++++++++++++
hw/misc/intel_me.c | 176 +++++++++++++++++++++++
hw/misc/meson.build | 5 +-
hw/nvram/eeprom_at24c.c | 2 +
hw/sensor/isl_pmbus_vr.c | 30 ++++
hw/sensor/lsm303dlhc_mag.c | 2 +
include/hw/arm/aspeed_soc.h | 3 +
include/hw/i2c/aspeed_i2c.h | 11 ++
include/hw/i2c/i2c.h | 30 ++++
include/hw/i2c/pmbus_device.h | 1 +
include/hw/misc/aspeed_peci.h | 34 +++++
include/hw/sensor/isl_pmbus_vr.h | 1 +
23 files changed, 1002 insertions(+), 30 deletions(-)
create mode 100644 hw/misc/aspeed_peci.c
create mode 100644 hw/misc/fby35_cpld.c
create mode 100644 hw/misc/intel_me.c
create mode 100644 include/hw/misc/aspeed_peci.h
--
2.30.2
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 01/14] hw/i2c: support multiple masters
2022-06-27 19:54 [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
@ 2022-06-27 19:54 ` Peter Delevoryas
2022-06-27 19:54 ` [PATCH 02/14] hw/i2c: add asynchronous send Peter Delevoryas
` (11 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 19:54 UTC (permalink / raw)
Cc: pdel, zhdaniel, clg, qemu-devel, qemu-arm, komlodi, titusr,
andrew, joel, Klaus Jensen
From: Klaus Jensen <k.jensen@samsung.com>
Allow slaves to master the bus by registering a bottom halve. If the bus
is busy, the bottom half is queued up. When a slave has succesfully
mastered the bus, the bottom half is scheduled.
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
[ clg : - fixed typos in commit log ]
Message-Id: <20220601210831.67259-4-its@irrelevant.dk>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
hw/i2c/core.c | 34 +++++++++++++++++++++++++++++++++-
include/hw/i2c/i2c.h | 14 ++++++++++++++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index d0cb2d32fa..145dce6078 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -13,6 +13,7 @@
#include "migration/vmstate.h"
#include "qapi/error.h"
#include "qemu/module.h"
+#include "qemu/main-loop.h"
#include "trace.h"
#define I2C_BROADCAST 0x00
@@ -62,6 +63,7 @@ I2CBus *i2c_init_bus(DeviceState *parent, const char *name)
bus = I2C_BUS(qbus_new(TYPE_I2C_BUS, parent, name));
QLIST_INIT(&bus->current_devs);
+ QSIMPLEQ_INIT(&bus->pending_masters);
vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY, &vmstate_i2c_bus, bus);
return bus;
}
@@ -74,7 +76,7 @@ void i2c_slave_set_address(I2CSlave *dev, uint8_t address)
/* Return nonzero if bus is busy. */
int i2c_bus_busy(I2CBus *bus)
{
- return !QLIST_EMPTY(&bus->current_devs);
+ return !QLIST_EMPTY(&bus->current_devs) || bus->bh;
}
bool i2c_scan_bus(I2CBus *bus, uint8_t address, bool broadcast,
@@ -180,6 +182,26 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address, bool is_recv)
: I2C_START_SEND);
}
+void i2c_bus_master(I2CBus *bus, QEMUBH *bh)
+{
+ if (i2c_bus_busy(bus)) {
+ I2CPendingMaster *node = g_new(struct I2CPendingMaster, 1);
+ node->bh = bh;
+
+ QSIMPLEQ_INSERT_TAIL(&bus->pending_masters, node, entry);
+
+ return;
+ }
+
+ bus->bh = bh;
+ qemu_bh_schedule(bus->bh);
+}
+
+void i2c_bus_release(I2CBus *bus)
+{
+ bus->bh = NULL;
+}
+
int i2c_start_recv(I2CBus *bus, uint8_t address)
{
return i2c_do_start_transfer(bus, address, I2C_START_RECV);
@@ -206,6 +228,16 @@ void i2c_end_transfer(I2CBus *bus)
g_free(node);
}
bus->broadcast = false;
+
+ if (!QSIMPLEQ_EMPTY(&bus->pending_masters)) {
+ I2CPendingMaster *node = QSIMPLEQ_FIRST(&bus->pending_masters);
+ bus->bh = node->bh;
+
+ QSIMPLEQ_REMOVE_HEAD(&bus->pending_masters, entry);
+ g_free(node);
+
+ qemu_bh_schedule(bus->bh);
+ }
}
int i2c_send(I2CBus *bus, uint8_t data)
diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index 5ca3b708c0..be8bb8b78a 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -69,13 +69,25 @@ struct I2CNode {
QLIST_ENTRY(I2CNode) next;
};
+typedef struct I2CPendingMaster I2CPendingMaster;
+
+struct I2CPendingMaster {
+ QEMUBH *bh;
+ QSIMPLEQ_ENTRY(I2CPendingMaster) entry;
+};
+
typedef QLIST_HEAD(I2CNodeList, I2CNode) I2CNodeList;
+typedef QSIMPLEQ_HEAD(I2CPendingMasters, I2CPendingMaster) I2CPendingMasters;
struct I2CBus {
BusState qbus;
I2CNodeList current_devs;
+ I2CPendingMasters pending_masters;
uint8_t saved_address;
bool broadcast;
+
+ /* Set from slave currently mastering the bus. */
+ QEMUBH *bh;
};
I2CBus *i2c_init_bus(DeviceState *parent, const char *name);
@@ -117,6 +129,8 @@ int i2c_start_send(I2CBus *bus, uint8_t address);
void i2c_end_transfer(I2CBus *bus);
void i2c_nack(I2CBus *bus);
+void i2c_bus_master(I2CBus *bus, QEMUBH *bh);
+void i2c_bus_release(I2CBus *bus);
int i2c_send(I2CBus *bus, uint8_t data);
uint8_t i2c_recv(I2CBus *bus);
bool i2c_scan_bus(I2CBus *bus, uint8_t address, bool broadcast,
--
2.30.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 02/14] hw/i2c: add asynchronous send
2022-06-27 19:54 [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
2022-06-27 19:54 ` [PATCH 01/14] hw/i2c: support multiple masters Peter Delevoryas
@ 2022-06-27 19:54 ` Peter Delevoryas
2022-06-27 19:54 ` [PATCH 03/14] hw/i2c/aspeed: add slave device in old register mode Peter Delevoryas
` (10 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 19:54 UTC (permalink / raw)
Cc: pdel, zhdaniel, clg, qemu-devel, qemu-arm, komlodi, titusr,
andrew, joel, Klaus Jensen
From: Klaus Jensen <k.jensen@samsung.com>
Add an asynchronous version of i2c_send() that requires the slave to
explicitly acknowledge on the bus with i2c_ack().
The current master must use the new i2c_start_send_async() to indicate
that it wants to do an asynchronous transfer. This allows the i2c core
to check if the target slave supports this or not. This approach relies
on adding a new enum i2c_event member, which is why a bunch of other
devices needs changes in their event handling switches.
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Message-Id: <20220601210831.67259-5-its@irrelevant.dk>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
hw/arm/pxa2xx.c | 2 ++
hw/display/sii9022.c | 2 ++
hw/display/ssd0303.c | 2 ++
hw/i2c/core.c | 36 +++++++++++++++++++++++++++++++++++-
hw/i2c/smbus_slave.c | 4 ++++
hw/i2c/trace-events | 2 ++
hw/nvram/eeprom_at24c.c | 2 ++
hw/sensor/lsm303dlhc_mag.c | 2 ++
include/hw/i2c/i2c.h | 16 ++++++++++++++++
9 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index f4f687df68..93dda83d7a 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -1305,6 +1305,8 @@ static int pxa2xx_i2c_event(I2CSlave *i2c, enum i2c_event event)
case I2C_NACK:
s->status |= 1 << 1; /* set ACKNAK */
break;
+ default:
+ return -1;
}
pxa2xx_i2c_update(s);
diff --git a/hw/display/sii9022.c b/hw/display/sii9022.c
index b591a58789..664fd4046d 100644
--- a/hw/display/sii9022.c
+++ b/hw/display/sii9022.c
@@ -76,6 +76,8 @@ static int sii9022_event(I2CSlave *i2c, enum i2c_event event)
break;
case I2C_NACK:
break;
+ default:
+ return -1;
}
return 0;
diff --git a/hw/display/ssd0303.c b/hw/display/ssd0303.c
index aeae22da9c..d67b0ad7b5 100644
--- a/hw/display/ssd0303.c
+++ b/hw/display/ssd0303.c
@@ -196,6 +196,8 @@ static int ssd0303_event(I2CSlave *i2c, enum i2c_event event)
case I2C_NACK:
/* Nothing to do. */
break;
+ default:
+ return -1;
}
return 0;
diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 145dce6078..d4ba8146bf 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -161,7 +161,8 @@ static int i2c_do_start_transfer(I2CBus *bus, uint8_t address,
start condition. */
if (sc->event) {
- trace_i2c_event("start", s->address);
+ trace_i2c_event(event == I2C_START_SEND ? "start" : "start_async",
+ s->address);
rv = sc->event(s, event);
if (rv && !bus->broadcast) {
if (bus_scanned) {
@@ -212,6 +213,11 @@ int i2c_start_send(I2CBus *bus, uint8_t address)
return i2c_do_start_transfer(bus, address, I2C_START_SEND);
}
+int i2c_start_send_async(I2CBus *bus, uint8_t address)
+{
+ return i2c_do_start_transfer(bus, address, I2C_START_SEND_ASYNC);
+}
+
void i2c_end_transfer(I2CBus *bus)
{
I2CSlaveClass *sc;
@@ -261,6 +267,23 @@ int i2c_send(I2CBus *bus, uint8_t data)
return ret ? -1 : 0;
}
+int i2c_send_async(I2CBus *bus, uint8_t data)
+{
+ I2CNode *node = QLIST_FIRST(&bus->current_devs);
+ I2CSlave *slave = node->elt;
+ I2CSlaveClass *sc = I2C_SLAVE_GET_CLASS(slave);
+
+ if (!sc->send_async) {
+ return -1;
+ }
+
+ trace_i2c_send_async(slave->address, data);
+
+ sc->send_async(slave, data);
+
+ return 0;
+}
+
uint8_t i2c_recv(I2CBus *bus)
{
uint8_t data = 0xff;
@@ -297,6 +320,17 @@ void i2c_nack(I2CBus *bus)
}
}
+void i2c_ack(I2CBus *bus)
+{
+ if (!bus->bh) {
+ return;
+ }
+
+ trace_i2c_ack();
+
+ qemu_bh_schedule(bus->bh);
+}
+
static int i2c_slave_post_load(void *opaque, int version_id)
{
I2CSlave *dev = opaque;
diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c
index 5d10e27664..feb3ec6333 100644
--- a/hw/i2c/smbus_slave.c
+++ b/hw/i2c/smbus_slave.c
@@ -143,6 +143,10 @@ static int smbus_i2c_event(I2CSlave *s, enum i2c_event event)
dev->mode = SMBUS_CONFUSED;
break;
}
+ break;
+
+ default:
+ return -1;
}
return 0;
diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
index 209275ed2d..af181d43ee 100644
--- a/hw/i2c/trace-events
+++ b/hw/i2c/trace-events
@@ -4,7 +4,9 @@
i2c_event(const char *event, uint8_t address) "%s(addr:0x%02x)"
i2c_send(uint8_t address, uint8_t data) "send(addr:0x%02x) data:0x%02x"
+i2c_send_async(uint8_t address, uint8_t data) "send_async(addr:0x%02x) data:0x%02x"
i2c_recv(uint8_t address, uint8_t data) "recv(addr:0x%02x) data:0x%02x"
+i2c_ack(void) ""
# aspeed_i2c.c
diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c
index 01a3093600..d695f6ae89 100644
--- a/hw/nvram/eeprom_at24c.c
+++ b/hw/nvram/eeprom_at24c.c
@@ -75,6 +75,8 @@ int at24c_eeprom_event(I2CSlave *s, enum i2c_event event)
break;
case I2C_NACK:
break;
+ default:
+ return -1;
}
return 0;
}
diff --git a/hw/sensor/lsm303dlhc_mag.c b/hw/sensor/lsm303dlhc_mag.c
index 4c98ddbf20..bb8d48b2fd 100644
--- a/hw/sensor/lsm303dlhc_mag.c
+++ b/hw/sensor/lsm303dlhc_mag.c
@@ -427,6 +427,8 @@ static int lsm303dlhc_mag_event(I2CSlave *i2c, enum i2c_event event)
break;
case I2C_NACK:
break;
+ default:
+ return -1;
}
s->len = 0;
diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index be8bb8b78a..9b9581d230 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -12,6 +12,7 @@
enum i2c_event {
I2C_START_RECV,
I2C_START_SEND,
+ I2C_START_SEND_ASYNC,
I2C_FINISH,
I2C_NACK /* Masker NACKed a receive byte. */
};
@@ -28,6 +29,9 @@ struct I2CSlaveClass {
/* Master to slave. Returns non-zero for a NAK, 0 for success. */
int (*send)(I2CSlave *s, uint8_t data);
+ /* Master to slave (asynchronous). Receiving slave must call i2c_ack(). */
+ void (*send_async)(I2CSlave *s, uint8_t data);
+
/*
* Slave to master. This cannot fail, the device should always
* return something here.
@@ -127,11 +131,23 @@ int i2c_start_recv(I2CBus *bus, uint8_t address);
*/
int i2c_start_send(I2CBus *bus, uint8_t address);
+/**
+ * i2c_start_send_async: start an asynchronous 'send' transfer on an I2C bus.
+ *
+ * @bus: #I2CBus to be used
+ * @address: address of the slave
+ *
+ * Return: 0 on success, -1 on error
+ */
+int i2c_start_send_async(I2CBus *bus, uint8_t address);
+
void i2c_end_transfer(I2CBus *bus);
void i2c_nack(I2CBus *bus);
+void i2c_ack(I2CBus *bus);
void i2c_bus_master(I2CBus *bus, QEMUBH *bh);
void i2c_bus_release(I2CBus *bus);
int i2c_send(I2CBus *bus, uint8_t data);
+int i2c_send_async(I2CBus *bus, uint8_t data);
uint8_t i2c_recv(I2CBus *bus);
bool i2c_scan_bus(I2CBus *bus, uint8_t address, bool broadcast,
I2CNodeList *current_devs);
--
2.30.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 03/14] hw/i2c/aspeed: add slave device in old register mode
2022-06-27 19:54 [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
2022-06-27 19:54 ` [PATCH 01/14] hw/i2c: support multiple masters Peter Delevoryas
2022-06-27 19:54 ` [PATCH 02/14] hw/i2c: add asynchronous send Peter Delevoryas
@ 2022-06-27 19:54 ` Peter Delevoryas
2022-06-27 19:54 ` [PATCH 04/14] aspeed: i2c: Fix DMA len write-enable bit handling Peter Delevoryas
` (9 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 19:54 UTC (permalink / raw)
Cc: pdel, zhdaniel, clg, qemu-devel, qemu-arm, komlodi, titusr,
andrew, joel, Klaus Jensen
From: Klaus Jensen <k.jensen@samsung.com>
Add slave mode functionality for the Aspeed I2C controller in old
register mode. This is implemented by realizing an I2C slave device
owned by the I2C controller and attached to its own bus.
The I2C slave device only implements asynchronous sends on the bus, so
slaves not supporting that will not be able to communicate with it.
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
[ clg: checkpatch fixes ]
Message-Id: <20220601210831.67259-6-its@irrelevant.dk>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
hw/i2c/aspeed_i2c.c | 89 +++++++++++++++++++++++++++++++++----
include/hw/i2c/aspeed_i2c.h | 8 ++++
2 files changed, 88 insertions(+), 9 deletions(-)
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 37ae1f2e04..2cfd05cb6c 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -696,9 +696,7 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
switch (offset) {
case A_I2CD_FUN_CTRL:
if (SHARED_FIELD_EX32(value, SLAVE_EN)) {
- qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
- __func__);
- break;
+ i2c_slave_set_address(bus->slave, bus->regs[R_I2CD_DEV_ADDR]);
}
bus->regs[R_I2CD_FUN_CTRL] = value & 0x0071C3FF;
break;
@@ -719,12 +717,15 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
bus->controller->intr_status &= ~(1 << bus->id);
qemu_irq_lower(aic->bus_get_irq(bus));
}
- if (handle_rx && (SHARED_ARRAY_FIELD_EX32(bus->regs, R_I2CD_CMD,
- M_RX_CMD) ||
- SHARED_ARRAY_FIELD_EX32(bus->regs, R_I2CD_CMD,
- M_S_RX_CMD_LAST))) {
- aspeed_i2c_handle_rx_cmd(bus);
- aspeed_i2c_bus_raise_interrupt(bus);
+ if (handle_rx) {
+ if (SHARED_ARRAY_FIELD_EX32(bus->regs, R_I2CD_CMD, M_RX_CMD) ||
+ SHARED_ARRAY_FIELD_EX32(bus->regs, R_I2CD_CMD,
+ M_S_RX_CMD_LAST)) {
+ aspeed_i2c_handle_rx_cmd(bus);
+ aspeed_i2c_bus_raise_interrupt(bus);
+ } else if (aspeed_i2c_get_state(bus) == I2CD_STXD) {
+ i2c_ack(bus->bus);
+ }
}
break;
case A_I2CD_DEV_ADDR:
@@ -1036,6 +1037,73 @@ static const TypeInfo aspeed_i2c_info = {
.abstract = true,
};
+static int aspeed_i2c_bus_slave_event(I2CSlave *slave, enum i2c_event event)
+{
+ BusState *qbus = qdev_get_parent_bus(DEVICE(slave));
+ AspeedI2CBus *bus = ASPEED_I2C_BUS(qbus->parent);
+ uint32_t reg_intr_sts = aspeed_i2c_bus_intr_sts_offset(bus);
+ uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus);
+ uint32_t value;
+
+ switch (event) {
+ case I2C_START_SEND_ASYNC:
+ value = SHARED_ARRAY_FIELD_EX32(bus->regs, reg_byte_buf, TX_BUF);
+ SHARED_ARRAY_FIELD_DP32(bus->regs, reg_byte_buf, RX_BUF, value << 1);
+
+ ARRAY_FIELD_DP32(bus->regs, I2CD_INTR_STS, SLAVE_ADDR_RX_MATCH, 1);
+ SHARED_ARRAY_FIELD_DP32(bus->regs, reg_intr_sts, RX_DONE, 1);
+
+ aspeed_i2c_set_state(bus, I2CD_STXD);
+
+ break;
+
+ case I2C_FINISH:
+ SHARED_ARRAY_FIELD_DP32(bus->regs, reg_intr_sts, NORMAL_STOP, 1);
+
+ aspeed_i2c_set_state(bus, I2CD_IDLE);
+
+ break;
+
+ default:
+ return -1;
+ }
+
+ aspeed_i2c_bus_raise_interrupt(bus);
+
+ return 0;
+}
+
+static void aspeed_i2c_bus_slave_send_async(I2CSlave *slave, uint8_t data)
+{
+ BusState *qbus = qdev_get_parent_bus(DEVICE(slave));
+ AspeedI2CBus *bus = ASPEED_I2C_BUS(qbus->parent);
+ uint32_t reg_intr_sts = aspeed_i2c_bus_intr_sts_offset(bus);
+ uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus);
+
+ SHARED_ARRAY_FIELD_DP32(bus->regs, reg_byte_buf, RX_BUF, data);
+ SHARED_ARRAY_FIELD_DP32(bus->regs, reg_intr_sts, RX_DONE, 1);
+
+ aspeed_i2c_bus_raise_interrupt(bus);
+}
+
+static void aspeed_i2c_bus_slave_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
+
+ dc->desc = "Aspeed I2C Bus Slave";
+
+ sc->event = aspeed_i2c_bus_slave_event;
+ sc->send_async = aspeed_i2c_bus_slave_send_async;
+}
+
+static const TypeInfo aspeed_i2c_bus_slave_info = {
+ .name = TYPE_ASPEED_I2C_BUS_SLAVE,
+ .parent = TYPE_I2C_SLAVE,
+ .instance_size = sizeof(AspeedI2CBusSlave),
+ .class_init = aspeed_i2c_bus_slave_class_init,
+};
+
static void aspeed_i2c_bus_reset(DeviceState *dev)
{
AspeedI2CBus *s = ASPEED_I2C_BUS(dev);
@@ -1060,6 +1128,8 @@ static void aspeed_i2c_bus_realize(DeviceState *dev, Error **errp)
sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
s->bus = i2c_init_bus(dev, name);
+ s->slave = i2c_slave_create_simple(s->bus, TYPE_ASPEED_I2C_BUS_SLAVE,
+ 0xff);
memory_region_init_io(&s->mr, OBJECT(s), &aspeed_i2c_bus_ops,
s, name, aic->reg_size);
@@ -1219,6 +1289,7 @@ static const TypeInfo aspeed_1030_i2c_info = {
static void aspeed_i2c_register_types(void)
{
type_register_static(&aspeed_i2c_bus_info);
+ type_register_static(&aspeed_i2c_bus_slave_info);
type_register_static(&aspeed_i2c_info);
type_register_static(&aspeed_2400_i2c_info);
type_register_static(&aspeed_2500_i2c_info);
diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
index 1398befc10..ba148b2f6d 100644
--- a/include/hw/i2c/aspeed_i2c.h
+++ b/include/hw/i2c/aspeed_i2c.h
@@ -223,6 +223,9 @@ struct AspeedI2CBus {
struct AspeedI2CState *controller;
+ /* slave mode */
+ I2CSlave *slave;
+
MemoryRegion mr;
I2CBus *bus;
@@ -249,6 +252,11 @@ struct AspeedI2CState {
AddressSpace dram_as;
};
+#define TYPE_ASPEED_I2C_BUS_SLAVE "aspeed.i2c.slave"
+OBJECT_DECLARE_SIMPLE_TYPE(AspeedI2CBusSlave, ASPEED_I2C_BUS_SLAVE)
+struct AspeedI2CBusSlave {
+ I2CSlave i2c;
+};
struct AspeedI2CClass {
SysBusDeviceClass parent_class;
--
2.30.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 04/14] aspeed: i2c: Fix DMA len write-enable bit handling
2022-06-27 19:54 [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
` (2 preceding siblings ...)
2022-06-27 19:54 ` [PATCH 03/14] hw/i2c/aspeed: add slave device in old register mode Peter Delevoryas
@ 2022-06-27 19:54 ` Peter Delevoryas
2022-06-28 7:01 ` Cédric Le Goater
2022-06-27 19:54 ` [PATCH 05/14] aspeed: i2c: Fix R_I2CD_FUN_CTRL reference Peter Delevoryas
` (8 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 19:54 UTC (permalink / raw)
Cc: pdel, zhdaniel, clg, qemu-devel, qemu-arm, komlodi, titusr, andrew, joel
I noticed i2c rx transfers were getting shortened to "1" on Zephyr. It
seems to be because the Zephyr i2c driver sets the RX DMA len with the
RX field write-enable bit set (bit 31) to avoid a read-modify-write. [1]
/* 0x1C : I2CM Master DMA Transfer Length Register */
I think we should be checking the write-enable bits on the incoming
value, not checking the register array. I'm not sure we're even writing
the write-enable bits to the register array, actually.
[1] https://github.com/AspeedTech-BMC/zephyr/blob/db3dbcc9c52e67a47180890ac938ed380b33f91c/drivers/i2c/i2c_aspeed.c#L145-L148
Fixes: ba2cccd64e90f34 ("aspeed: i2c: Add new mode support")
Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
hw/i2c/aspeed_i2c.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 2cfd05cb6c..6c8222717f 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -644,18 +644,18 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
RX_BUF_LEN) + 1;
break;
case A_I2CM_DMA_LEN:
- w1t = ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, RX_BUF_LEN_W1T) ||
- ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, TX_BUF_LEN_W1T);
+ w1t = FIELD_EX32(value, I2CM_DMA_LEN, RX_BUF_LEN_W1T) ||
+ FIELD_EX32(value, I2CM_DMA_LEN, TX_BUF_LEN_W1T);
/* If none of the w1t bits are set, just write to the reg as normal. */
if (!w1t) {
bus->regs[R_I2CM_DMA_LEN] = value;
break;
}
- if (ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, RX_BUF_LEN_W1T)) {
+ if (FIELD_EX32(value, I2CM_DMA_LEN, RX_BUF_LEN_W1T)) {
ARRAY_FIELD_DP32(bus->regs, I2CM_DMA_LEN, RX_BUF_LEN,
FIELD_EX32(value, I2CM_DMA_LEN, RX_BUF_LEN));
}
- if (ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, TX_BUF_LEN_W1T)) {
+ if (FIELD_EX32(value, I2CM_DMA_LEN, TX_BUF_LEN_W1T)) {
ARRAY_FIELD_DP32(bus->regs, I2CM_DMA_LEN, TX_BUF_LEN,
FIELD_EX32(value, I2CM_DMA_LEN, TX_BUF_LEN));
}
--
2.30.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 05/14] aspeed: i2c: Fix R_I2CD_FUN_CTRL reference
2022-06-27 19:54 [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
` (3 preceding siblings ...)
2022-06-27 19:54 ` [PATCH 04/14] aspeed: i2c: Fix DMA len write-enable bit handling Peter Delevoryas
@ 2022-06-27 19:54 ` Peter Delevoryas
2022-06-28 7:01 ` Cédric Le Goater
2022-06-27 19:54 ` [PATCH 07/14] aspeed: Add PECI controller Peter Delevoryas
` (7 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 19:54 UTC (permalink / raw)
Cc: pdel, zhdaniel, clg, qemu-devel, qemu-arm, komlodi, titusr, andrew, joel
Very minor, doesn't effect functionality, but this is supposed to be
R_I2CC_FUN_CTRL (new-mode, not old-mode).
Fixes: ba2cccd64e9 ("aspeed: i2c: Add new mode support")
Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
hw/i2c/aspeed_i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 6c8222717f..6abd9b033e 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -552,7 +552,7 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
__func__);
break;
}
- bus->regs[R_I2CD_FUN_CTRL] = value & 0x007dc3ff;
+ bus->regs[R_I2CC_FUN_CTRL] = value & 0x007dc3ff;
break;
case A_I2CC_AC_TIMING:
bus->regs[R_I2CC_AC_TIMING] = value & 0x1ffff0ff;
--
2.30.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 07/14] aspeed: Add PECI controller
2022-06-27 19:54 [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
` (4 preceding siblings ...)
2022-06-27 19:54 ` [PATCH 05/14] aspeed: i2c: Fix R_I2CD_FUN_CTRL reference Peter Delevoryas
@ 2022-06-27 19:54 ` Peter Delevoryas
2022-06-28 6:47 ` Cédric Le Goater
2022-06-27 19:55 ` [PATCH 08/14] hw/misc: Add fby35-cpld Peter Delevoryas
` (6 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 19:54 UTC (permalink / raw)
Cc: pdel, zhdaniel, clg, qemu-devel, qemu-arm, komlodi, titusr, andrew, joel
Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
hw/arm/aspeed_ast10x0.c | 11 ++
hw/misc/aspeed_peci.c | 225 ++++++++++++++++++++++++++++++++++
hw/misc/meson.build | 3 +-
include/hw/arm/aspeed_soc.h | 3 +
include/hw/misc/aspeed_peci.h | 34 +++++
5 files changed, 275 insertions(+), 1 deletion(-)
create mode 100644 hw/misc/aspeed_peci.c
create mode 100644 include/hw/misc/aspeed_peci.h
diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 5df480a21f..780841ea84 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -47,6 +47,7 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
[ASPEED_DEV_UART13] = 0x7E790700,
[ASPEED_DEV_WDT] = 0x7E785000,
[ASPEED_DEV_LPC] = 0x7E789000,
+ [ASPEED_DEV_PECI] = 0x7E78B000,
[ASPEED_DEV_I2C] = 0x7E7B0000,
};
@@ -75,6 +76,7 @@ static const int aspeed_soc_ast1030_irqmap[] = {
[ASPEED_DEV_TIMER8] = 23,
[ASPEED_DEV_WDT] = 24,
[ASPEED_DEV_LPC] = 35,
+ [ASPEED_DEV_PECI] = 38,
[ASPEED_DEV_FMC] = 39,
[ASPEED_DEV_PWM] = 44,
[ASPEED_DEV_ADC] = 46,
@@ -133,6 +135,8 @@ static void aspeed_soc_ast1030_init(Object *obj)
object_initialize_child(obj, "lpc", &s->lpc, TYPE_ASPEED_LPC);
+ object_initialize_child(obj, "peci", &s->peci, TYPE_ASPEED_PECI);
+
object_initialize_child(obj, "sbc", &s->sbc, TYPE_ASPEED_SBC);
for (i = 0; i < sc->wdts_num; i++) {
@@ -238,6 +242,13 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
/* UART */
aspeed_soc_uart_init(s);
+ /* PECI */
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->peci), errp)) {
+ return;
+ }
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->peci), 0, sc->memmap[ASPEED_DEV_PECI]);
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->peci), 0, aspeed_soc_get_irq(s, ASPEED_DEV_PECI));
+
/* Timer */
object_property_set_link(OBJECT(&s->timerctrl), "scu", OBJECT(&s->scu),
&error_abort);
diff --git a/hw/misc/aspeed_peci.c b/hw/misc/aspeed_peci.c
new file mode 100644
index 0000000000..670e532fc0
--- /dev/null
+++ b/hw/misc/aspeed_peci.c
@@ -0,0 +1,225 @@
+/*
+ * Aspeed PECI Controller
+ *
+ * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
+ *
+ * This code is licensed under the GPL version 2 or later. See the COPYING
+ * file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/irq.h"
+#include "hw/misc/aspeed_peci.h"
+
+#define U(x) (x##U)
+#define GENMASK(h, l) \
+ (((~U(0)) - (U(1) << (l)) + 1) & \
+ (~U(0) >> (32 - 1 - (h))))
+
+/* ASPEED PECI Registers */
+/* Control Register */
+#define ASPEED_PECI_CTRL (0x00 / 4)
+#define ASPEED_PECI_CTRL_SAMPLING_MASK GENMASK(19, 16)
+#define ASPEED_PECI_CTRL_RD_MODE_MASK GENMASK(13, 12)
+#define ASPEED_PECI_CTRL_RD_MODE_DBG BIT(13)
+#define ASPEED_PECI_CTRL_RD_MODE_COUNT BIT(12)
+#define ASPEED_PECI_CTRL_CLK_SRC_HCLK BIT(11)
+#define ASPEED_PECI_CTRL_CLK_DIV_MASK GENMASK(10, 8)
+#define ASPEED_PECI_CTRL_INVERT_OUT BIT(7)
+#define ASPEED_PECI_CTRL_INVERT_IN BIT(6)
+#define ASPEED_PECI_CTRL_BUS_CONTENTION_EN BIT(5)
+#define ASPEED_PECI_CTRL_PECI_EN BIT(4)
+#define ASPEED_PECI_CTRL_PECI_CLK_EN BIT(0)
+
+/* Timing Negotiation Register */
+#define ASPEED_PECI_TIMING_NEGOTIATION (0x04 / 4)
+#define ASPEED_PECI_T_NEGO_MSG_MASK GENMASK(15, 8)
+#define ASPEED_PECI_T_NEGO_ADDR_MASK GENMASK(7, 0)
+
+/* Command Register */
+#define ASPEED_PECI_CMD (0x08 / 4)
+#define ASPEED_PECI_CMD_PIN_MONITORING BIT(31)
+#define ASPEED_PECI_CMD_STS_MASK GENMASK(27, 24)
+#define ASPEED_PECI_CMD_STS_ADDR_T_NEGO 0x3
+#define ASPEED_PECI_CMD_IDLE_MASK \
+ (ASPEED_PECI_CMD_STS_MASK | ASPEED_PECI_CMD_PIN_MONITORING)
+#define ASPEED_PECI_CMD_FIRE BIT(0)
+
+/* Read/Write Length Register */
+#define ASPEED_PECI_RW_LENGTH (0x0c / 4)
+#define ASPEED_PECI_AW_FCS_EN BIT(31)
+#define ASPEED_PECI_RD_LEN_MASK GENMASK(23, 16)
+#define ASPEED_PECI_WR_LEN_MASK GENMASK(15, 8)
+#define ASPEED_PECI_TARGET_ADDR_MASK GENMASK(7, 0)
+
+/* Expected FCS Data Register */
+#define ASPEED_PECI_EXPECTED_FCS (0x10 / 4)
+#define ASPEED_PECI_EXPECTED_RD_FCS_MASK GENMASK(23, 16)
+#define ASPEED_PECI_EXPECTED_AW_FCS_AUTO_MASK GENMASK(15, 8)
+#define ASPEED_PECI_EXPECTED_WR_FCS_MASK GENMASK(7, 0)
+
+/* Captured FCS Data Register */
+#define ASPEED_PECI_CAPTURED_FCS (0x14 / 4)
+#define ASPEED_PECI_CAPTURED_RD_FCS_MASK GENMASK(23, 16)
+#define ASPEED_PECI_CAPTURED_WR_FCS_MASK GENMASK(7, 0)
+
+/* Interrupt Register */
+#define ASPEED_PECI_INT_CTRL (0x18 / 4)
+#define ASPEED_PECI_TIMING_NEGO_SEL_MASK GENMASK(31, 30)
+#define ASPEED_PECI_1ST_BIT_OF_ADDR_NEGO 0
+#define ASPEED_PECI_2ND_BIT_OF_ADDR_NEGO 1
+#define ASPEED_PECI_MESSAGE_NEGO 2
+#define ASPEED_PECI_INT_MASK GENMASK(4, 0)
+#define ASPEED_PECI_INT_BUS_TIMEOUT BIT(4)
+#define ASPEED_PECI_INT_BUS_CONTENTION BIT(3)
+#define ASPEED_PECI_INT_WR_FCS_BAD BIT(2)
+#define ASPEED_PECI_INT_WR_FCS_ABORT BIT(1)
+#define ASPEED_PECI_INT_CMD_DONE BIT(0)
+
+/* Interrupt Status Register */
+#define ASPEED_PECI_INT_STS (0x1c / 4)
+#define ASPEED_PECI_INT_TIMING_RESULT_MASK GENMASK(29, 16)
+ /* bits[4..0]: Same bit fields in the 'Interrupt Register' */
+
+/* Rx/Tx Data Buffer Registers */
+#define ASPEED_PECI_WR_DATA0 (0x20 / 4)
+#define ASPEED_PECI_WR_DATA1 (0x24 / 4)
+#define ASPEED_PECI_WR_DATA2 (0x28 / 4)
+#define ASPEED_PECI_WR_DATA3 (0x2c / 4)
+#define ASPEED_PECI_RD_DATA0 (0x30 / 4)
+#define ASPEED_PECI_RD_DATA1 (0x34 / 4)
+#define ASPEED_PECI_RD_DATA2 (0x38 / 4)
+#define ASPEED_PECI_RD_DATA3 (0x3c / 4)
+#define ASPEED_PECI_WR_DATA4 (0x40 / 4)
+#define ASPEED_PECI_WR_DATA5 (0x44 / 4)
+#define ASPEED_PECI_WR_DATA6 (0x48 / 4)
+#define ASPEED_PECI_WR_DATA7 (0x4c / 4)
+#define ASPEED_PECI_RD_DATA4 (0x50 / 4)
+#define ASPEED_PECI_RD_DATA5 (0x54 / 4)
+#define ASPEED_PECI_RD_DATA6 (0x58 / 4)
+#define ASPEED_PECI_RD_DATA7 (0x5c / 4)
+#define ASPEED_PECI_DATA_BUF_SIZE_MAX 32
+
+/** PECI read/write supported responses */
+#define PECI_CC_RSP_SUCCESS (0x40U)
+#define PECI_CC_RSP_TIMEOUT (0x80U)
+#define PECI_CC_OUT_OF_RESOURCES_TIMEOUT (0x81U)
+#define PECI_CC_RESOURCES_LOWPWR_TIMEOUT (0x82U)
+#define PECI_CC_ILLEGAL_REQUEST (0x90U)
+
+static void aspeed_peci_instance_init(Object *obj)
+{
+}
+
+static uint64_t aspeed_peci_read(void *opaque, hwaddr addr, unsigned size)
+{
+ AspeedPECIState *s = ASPEED_PECI(opaque);
+
+ if (addr >= ASPEED_PECI_NR_REGS << 2) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx "\n",
+ __func__, addr);
+ return 0;
+ }
+ addr >>= 2;
+
+ uint32_t reg = s->regs[addr];
+ //printf("%s: 0x%08lx 0x%08x %u\n", __func__, addr << 2, reg, size);
+
+ return reg;
+}
+
+static void aspeed_peci_write(void *opaque, hwaddr addr, uint64_t data, unsigned size)
+{
+ AspeedPECIState *s = ASPEED_PECI(opaque);
+
+ //printf("%s: 0x%08lx 0x%08x %u\n", __func__, addr, reg, size);
+
+ if (addr >= ASPEED_PECI_NR_REGS << 2) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n",
+ __func__, addr);
+ return;
+ }
+ addr >>= 2;
+
+ switch (addr) {
+ case ASPEED_PECI_INT_STS:
+ s->regs[addr] &= ~data;
+ break;
+ default:
+ s->regs[addr] = data;
+ break;
+ }
+
+ switch (addr) {
+ case ASPEED_PECI_CMD:
+ if (!(s->regs[ASPEED_PECI_CMD] & ASPEED_PECI_CMD_FIRE)) {
+ break;
+ }
+ s->regs[ASPEED_PECI_RD_DATA0] = PECI_CC_RSP_SUCCESS;
+ s->regs[ASPEED_PECI_WR_DATA0] = PECI_CC_RSP_SUCCESS;
+
+ s->regs[ASPEED_PECI_INT_STS] |= ASPEED_PECI_INT_CMD_DONE;
+ qemu_irq_raise(s->irq);
+ break;
+ case ASPEED_PECI_INT_STS:
+ if (s->regs[ASPEED_PECI_INT_STS]) {
+ break;
+ }
+ qemu_irq_lower(s->irq);
+ break;
+ default:
+ qemu_log_mask(LOG_UNIMP, "%s: register 0x%03" HWADDR_PRIx " writes unimplemented\n",
+ __func__, addr);
+ break;
+ }
+}
+
+static const MemoryRegionOps aspeed_peci_ops = {
+ .read = aspeed_peci_read,
+ .write = aspeed_peci_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void aspeed_peci_realize(DeviceState *dev, Error **errp)
+{
+ AspeedPECIState *s = ASPEED_PECI(dev);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+ memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_peci_ops, s, TYPE_ASPEED_PECI, 0x1000);
+ sysbus_init_mmio(sbd, &s->mmio);
+ sysbus_init_irq(sbd, &s->irq);
+}
+
+static void aspeed_peci_reset(DeviceState *dev)
+{
+ AspeedPECIState *s = ASPEED_PECI(dev);
+
+ memset(s->regs, 0, sizeof(s->regs));
+}
+
+static void aspeed_peci_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->realize = aspeed_peci_realize;
+ dc->reset = aspeed_peci_reset;
+ dc->desc = "Aspeed PECI Controller";
+}
+
+static const TypeInfo aspeed_peci_info = {
+ .name = TYPE_ASPEED_PECI,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_init = aspeed_peci_instance_init,
+ .instance_size = sizeof(AspeedPECIState),
+ .class_init = aspeed_peci_class_init,
+ .class_size = sizeof(AspeedPECIClass),
+ .abstract = false,
+};
+
+static void aspeed_peci_register_types(void)
+{
+ type_register_static(&aspeed_peci_info);
+}
+
+type_init(aspeed_peci_register_types);
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 132b7b7344..95268eddc0 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -116,7 +116,8 @@ softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
'aspeed_scu.c',
'aspeed_sbc.c',
'aspeed_sdmc.c',
- 'aspeed_xdma.c'))
+ 'aspeed_xdma.c',
+ 'aspeed_peci.c'))
softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-sysreg.c'))
softmmu_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_rng.c'))
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 02a5a9ffcb..fd2aa1880a 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -34,6 +34,7 @@
#include "hw/usb/hcd-ehci.h"
#include "qom/object.h"
#include "hw/misc/aspeed_lpc.h"
+#include "hw/misc/aspeed_peci.h"
#define ASPEED_SPIS_NUM 2
#define ASPEED_EHCIS_NUM 2
@@ -73,6 +74,7 @@ struct AspeedSoCState {
AspeedSDHCIState sdhci;
AspeedSDHCIState emmc;
AspeedLPCState lpc;
+ AspeedPECIState peci;
uint32_t uart_default;
Clock *sysclk;
};
@@ -161,6 +163,7 @@ enum {
ASPEED_DEV_DPMCU,
ASPEED_DEV_DP,
ASPEED_DEV_I3C,
+ ASPEED_DEV_PECI,
};
qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev);
diff --git a/include/hw/misc/aspeed_peci.h b/include/hw/misc/aspeed_peci.h
new file mode 100644
index 0000000000..81c7d31700
--- /dev/null
+++ b/include/hw/misc/aspeed_peci.h
@@ -0,0 +1,34 @@
+/*
+ * Aspeed PECI Controller
+ *
+ * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
+ *
+ * This code is licensed under the GPL version 2 or later. See the COPYING
+ * file in the top-level directory.
+ */
+
+#ifndef ASPEED_PECI_H
+#define ASPEED_PECI_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_ASPEED_PECI "aspeed.peci"
+OBJECT_DECLARE_TYPE(AspeedPECIState, AspeedPECIClass, ASPEED_PECI);
+
+#define ASPEED_PECI_NR_REGS ((0xFC + 4) >> 2)
+
+struct AspeedPECIState {
+ /* <private> */
+ SysBusDevice parent;
+
+ MemoryRegion mmio;
+ qemu_irq irq;
+
+ uint32_t regs[ASPEED_PECI_NR_REGS];
+};
+
+struct AspeedPECIClass {
+ SysBusDeviceClass parent_class;
+};
+
+#endif
--
2.30.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 08/14] hw/misc: Add fby35-cpld
2022-06-27 19:54 [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
` (5 preceding siblings ...)
2022-06-27 19:54 ` [PATCH 07/14] aspeed: Add PECI controller Peter Delevoryas
@ 2022-06-27 19:55 ` Peter Delevoryas
2022-06-28 6:50 ` Cédric Le Goater
2022-06-27 19:55 ` [PATCH 09/14] pmbus: Reset out buf after switching pages Peter Delevoryas
` (5 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 19:55 UTC (permalink / raw)
Cc: pdel, zhdaniel, clg, qemu-devel, qemu-arm, komlodi, titusr, andrew, joel
Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
hw/misc/fby35_cpld.c | 137 +++++++++++++++++++++++++++++++++++++++++++
hw/misc/meson.build | 3 +-
2 files changed, 139 insertions(+), 1 deletion(-)
create mode 100644 hw/misc/fby35_cpld.c
diff --git a/hw/misc/fby35_cpld.c b/hw/misc/fby35_cpld.c
new file mode 100644
index 0000000000..a783a0a2c8
--- /dev/null
+++ b/hw/misc/fby35_cpld.c
@@ -0,0 +1,137 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
+ *
+ * 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/i2c/i2c.h"
+#include "hw/registerfields.h"
+
+#define BOARD_ID_CLASS1 0b0000
+#define BOARD_ID_CLASS2 0b0001
+
+#define TYPE_FBY35_CPLD "fby35-cpld"
+OBJECT_DECLARE_SIMPLE_TYPE(Fby35CpldState, FBY35_CPLD);
+
+REG8(CLASS_TYPE, 0x5);
+ FIELD(CLASS_TYPE, RESERVED, 0, 2);
+ FIELD(CLASS_TYPE, 1OU_EXPANSION_NOT_PRESENT, 2, 1);
+ FIELD(CLASS_TYPE, 2OU_EXPANSION_NOT_PRESENT, 3, 1);
+ FIELD(CLASS_TYPE, BOARD_ID, 4, 4);
+REG8(BOARD_REVISION, 0x8);
+ FIELD(BOARD_REVISION, VALUE, 0, 4);
+ FIELD(BOARD_REVISION, RESERVED, 4, 4);
+
+struct Fby35CpldState {
+ I2CSlave parent_obj;
+
+ uint8_t target_reg;
+ uint32_t regs[10];
+};
+
+static void fby35_cpld_realize(DeviceState *dev, Error **errp)
+{
+ Fby35CpldState *s = FBY35_CPLD(dev);
+
+ memset(s->regs, 0, sizeof(s->regs));
+ s->target_reg = 0;
+
+ ARRAY_FIELD_DP32(s->regs, CLASS_TYPE, BOARD_ID, 0b0000);
+ ARRAY_FIELD_DP32(s->regs, CLASS_TYPE, 1OU_EXPANSION_NOT_PRESENT, 1);
+ ARRAY_FIELD_DP32(s->regs, CLASS_TYPE, 2OU_EXPANSION_NOT_PRESENT, 1);
+ ARRAY_FIELD_DP32(s->regs, BOARD_REVISION, VALUE, 0x1);
+}
+
+static int fby35_cpld_i2c_event(I2CSlave *i2c, enum i2c_event event)
+{
+ Fby35CpldState *s = FBY35_CPLD(i2c);
+
+ switch (event) {
+ case I2C_START_RECV:
+ break;
+ case I2C_START_SEND:
+ s->target_reg = 0;
+ break;
+ case I2C_START_SEND_ASYNC:
+ case I2C_FINISH:
+ case I2C_NACK:
+ break;
+ }
+
+ return 0;
+}
+
+static uint8_t fby35_cpld_i2c_recv(I2CSlave *i2c)
+{
+ Fby35CpldState *s = FBY35_CPLD(i2c);
+
+ switch (s->target_reg) {
+ case R_CLASS_TYPE:
+ case R_BOARD_REVISION:
+ return s->regs[s->target_reg];
+ default:
+ printf("%s: unexpected register read 0x%02x\n", __func__, s->target_reg);
+ return 0xff;
+ }
+}
+
+static int fby35_cpld_i2c_send(I2CSlave *i2c, uint8_t data)
+{
+ Fby35CpldState *s = FBY35_CPLD(i2c);
+
+ if (s->target_reg == 0) {
+ s->target_reg = data;
+ return 0;
+ }
+
+ switch (s->target_reg) {
+ case R_CLASS_TYPE:
+ case R_BOARD_REVISION:
+ s->regs[s->target_reg] = data;
+ break;
+ default:
+ printf("%s: unexpected register write 0x%02x 0x%02x\n", __func__, s->target_reg, data);
+ break;
+ }
+
+ return 0;
+}
+
+static void fby35_cpld_class_init(ObjectClass *oc, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(oc);
+ I2CSlaveClass *i2c = I2C_SLAVE_CLASS(oc);
+
+ dc->realize = fby35_cpld_realize;
+ i2c->event = fby35_cpld_i2c_event;
+ i2c->recv = fby35_cpld_i2c_recv;
+ i2c->send = fby35_cpld_i2c_send;
+}
+
+static const TypeInfo types[] = {
+ {
+ .name = TYPE_FBY35_CPLD,
+ .parent = TYPE_I2C_SLAVE,
+ .instance_size = sizeof(Fby35CpldState),
+ .class_init = fby35_cpld_class_init,
+ },
+};
+
+DEFINE_TYPES(types);
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 95268eddc0..1edad44b6b 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -117,7 +117,8 @@ softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
'aspeed_sbc.c',
'aspeed_sdmc.c',
'aspeed_xdma.c',
- 'aspeed_peci.c'))
+ 'aspeed_peci.c',
+ 'fby35_cpld.c'))
softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-sysreg.c'))
softmmu_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_rng.c'))
--
2.30.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 09/14] pmbus: Reset out buf after switching pages
2022-06-27 19:54 [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
` (6 preceding siblings ...)
2022-06-27 19:55 ` [PATCH 08/14] hw/misc: Add fby35-cpld Peter Delevoryas
@ 2022-06-27 19:55 ` Peter Delevoryas
2022-06-28 6:51 ` Cédric Le Goater
2022-06-27 19:55 ` [PATCH 10/14] pmbus: Add read-only IC_DEVICE_ID support Peter Delevoryas
` (4 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 19:55 UTC (permalink / raw)
Cc: pdel, zhdaniel, clg, qemu-devel, qemu-arm, komlodi, titusr, andrew, joel
Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
hw/i2c/pmbus_device.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index 62885fa6a1..efddc36fd9 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -1088,6 +1088,7 @@ static int pmbus_write_data(SMBusDevice *smd, uint8_t *buf, uint8_t len)
if (pmdev->code == PMBUS_PAGE) {
pmdev->page = pmbus_receive8(pmdev);
+ pmdev->out_buf_len = 0;
return 0;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 10/14] pmbus: Add read-only IC_DEVICE_ID support
2022-06-27 19:54 [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
` (7 preceding siblings ...)
2022-06-27 19:55 ` [PATCH 09/14] pmbus: Reset out buf after switching pages Peter Delevoryas
@ 2022-06-27 19:55 ` Peter Delevoryas
2022-06-27 19:55 ` [PATCH 11/14] aspeed: Add oby35-cl machine Peter Delevoryas
` (3 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 19:55 UTC (permalink / raw)
Cc: pdel, zhdaniel, clg, qemu-devel, qemu-arm, komlodi, titusr, andrew, joel
Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
hw/i2c/pmbus_device.c | 5 +++++
hw/sensor/isl_pmbus_vr.c | 30 ++++++++++++++++++++++++++++++
include/hw/i2c/pmbus_device.h | 1 +
include/hw/sensor/isl_pmbus_vr.h | 1 +
4 files changed, 37 insertions(+)
diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index efddc36fd9..82131fff85 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -984,6 +984,11 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd)
}
break;
+ case PMBUS_IC_DEVICE_ID:
+ pmbus_send(pmdev, pmdev->pages[index].ic_device_id,
+ sizeof(pmdev->pages[index].ic_device_id));
+ break;
+
case PMBUS_CLEAR_FAULTS: /* Send Byte */
case PMBUS_PAGE_PLUS_WRITE: /* Block Write-only */
case PMBUS_STORE_DEFAULT_ALL: /* Send Byte */
diff --git a/hw/sensor/isl_pmbus_vr.c b/hw/sensor/isl_pmbus_vr.c
index e11e028884..b0d2f49e9d 100644
--- a/hw/sensor/isl_pmbus_vr.c
+++ b/hw/sensor/isl_pmbus_vr.c
@@ -218,6 +218,27 @@ static void isl_pmbus_vr_class_init(ObjectClass *klass, void *data,
k->device_num_pages = pages;
}
+static void isl69259_init(Object *obj)
+{
+ static const uint8_t ic_device_id[] = {0x04, 0x00, 0x81, 0xD2, 0x49};
+ PMBusDevice *pmdev = PMBUS_DEVICE(obj);
+ int i;
+
+ raa22xx_init(obj);
+ for (i = 0; i < pmdev->num_pages; i++) {
+ memcpy(pmdev->pages[i].ic_device_id, ic_device_id, sizeof(ic_device_id));
+ }
+}
+
+static void isl69259_class_init(ObjectClass *klass, void *data)
+{
+ ResettableClass *rc = RESETTABLE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ dc->desc = "Renesas ISL69259 Digital Multiphase Voltage Regulator";
+ rc->phases.exit = isl_pmbus_vr_exit_reset;
+ isl_pmbus_vr_class_init(klass, data, 2);
+}
+
static void isl69260_class_init(ObjectClass *klass, void *data)
{
ResettableClass *rc = RESETTABLE_CLASS(klass);
@@ -245,6 +266,14 @@ static void raa229004_class_init(ObjectClass *klass, void *data)
isl_pmbus_vr_class_init(klass, data, 2);
}
+static const TypeInfo isl69259_info = {
+ .name = TYPE_ISL69259,
+ .parent = TYPE_PMBUS_DEVICE,
+ .instance_size = sizeof(ISLState),
+ .instance_init = isl69259_init,
+ .class_init = isl69259_class_init,
+};
+
static const TypeInfo isl69260_info = {
.name = TYPE_ISL69260,
.parent = TYPE_PMBUS_DEVICE,
@@ -271,6 +300,7 @@ static const TypeInfo raa228000_info = {
static void isl_pmbus_vr_register_types(void)
{
+ type_register_static(&isl69259_info);
type_register_static(&isl69260_info);
type_register_static(&raa228000_info);
type_register_static(&raa229004_info);
diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h
index 0f4d6b3fad..aed7809841 100644
--- a/include/hw/i2c/pmbus_device.h
+++ b/include/hw/i2c/pmbus_device.h
@@ -407,6 +407,7 @@ typedef struct PMBusPage {
uint16_t mfr_max_temp_1; /* R/W word */
uint16_t mfr_max_temp_2; /* R/W word */
uint16_t mfr_max_temp_3; /* R/W word */
+ uint8_t ic_device_id[16]; /* Read-Only block-read */
} PMBusPage;
/* State */
diff --git a/include/hw/sensor/isl_pmbus_vr.h b/include/hw/sensor/isl_pmbus_vr.h
index 3e47ff7e48..d501b3bc82 100644
--- a/include/hw/sensor/isl_pmbus_vr.h
+++ b/include/hw/sensor/isl_pmbus_vr.h
@@ -12,6 +12,7 @@
#include "hw/i2c/pmbus_device.h"
#include "qom/object.h"
+#define TYPE_ISL69259 "isl69259"
#define TYPE_ISL69260 "isl69260"
#define TYPE_RAA228000 "raa228000"
#define TYPE_RAA229004 "raa229004"
--
2.30.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 11/14] aspeed: Add oby35-cl machine
2022-06-27 19:54 [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
` (8 preceding siblings ...)
2022-06-27 19:55 ` [PATCH 10/14] pmbus: Add read-only IC_DEVICE_ID support Peter Delevoryas
@ 2022-06-27 19:55 ` Peter Delevoryas
2022-06-27 20:04 ` [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
` (2 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 19:55 UTC (permalink / raw)
Cc: pdel, zhdaniel, clg, qemu-devel, qemu-arm, komlodi, titusr, andrew, joel
Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
hw/arm/aspeed.c | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index a06f7c1b62..2b9c1600c6 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1429,6 +1429,42 @@ static void aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
amc->macs_mask = 0;
}
+static void oby35_cl_i2c_init(AspeedMachineState *bmc)
+{
+ AspeedSoCState *soc = &bmc->soc;
+ I2CBus *i2c[14];
+ I2CBus *ssd[8];
+ int i;
+
+ for (i = 0; i < 14; i++) {
+ i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
+ }
+ get_pca9548_channels(i2c[1], 0x71, ssd);
+
+ i2c_slave_create_simple(i2c[0], "fby35-cpld", 0x21);
+ i2c_slave_create_simple(i2c[1], "tmp105", 0x48);
+ i2c_slave_create_simple(i2c[1], "tmp105", 0x49);
+ i2c_slave_create_simple(i2c[1], "tmp105", 0x4a);
+ i2c_slave_create_simple(i2c[1], "adm1272", 0x40);
+ i2c_slave_create_simple(i2c[1], "tmp421", 0x4c);
+ i2c_slave_create_simple(i2c[4], "isl69259", 0x76);
+ i2c_slave_create_simple(i2c[4], "isl69259", 0x62);
+ i2c_slave_create_simple(i2c[4], "isl69259", 0x60);
+
+ for (int i = 0; i < 8; i++) {
+ i2c_slave_create_simple(ssd[i], "tmp105", 0x6a);
+ }
+}
+
+static void aspeed_machine_oby35_cl_class_init(ObjectClass *oc, void *data)
+{
+ MachineClass *mc = MACHINE_CLASS(oc);
+ AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+ mc->desc = "Meta Platforms fby35 CraterLake BIC (Cortex-M4)";
+ amc->i2c_init = oby35_cl_i2c_init;
+}
+
static const TypeInfo aspeed_machine_types[] = {
{
.name = MACHINE_TYPE_NAME("palmetto-bmc"),
@@ -1494,6 +1530,10 @@ static const TypeInfo aspeed_machine_types[] = {
.name = MACHINE_TYPE_NAME("ast1030-evb"),
.parent = TYPE_ASPEED_MACHINE,
.class_init = aspeed_minibmc_machine_ast1030_evb_class_init,
+ }, {
+ .name = MACHINE_TYPE_NAME("oby35-cl"),
+ .parent = MACHINE_TYPE_NAME("ast1030-evb"),
+ .class_init = aspeed_machine_oby35_cl_class_init,
}, {
.name = TYPE_ASPEED_MACHINE,
.parent = TYPE_MACHINE,
--
2.30.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support
2022-06-27 19:54 [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
` (9 preceding siblings ...)
2022-06-27 19:55 ` [PATCH 11/14] aspeed: Add oby35-cl machine Peter Delevoryas
@ 2022-06-27 20:04 ` Peter Delevoryas
2022-06-27 22:27 ` [PATCH 12/14] hw/misc: Add intel-me Peter Delevoryas
2022-06-28 7:05 ` [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Cédric Le Goater
12 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 20:04 UTC (permalink / raw)
Cc: Peter Delevoryas, Dan Zhang, Cédric Le Goater,
Cameron Esfahani via, qemu-arm, komlodi, titusr, Andrew Jeffery,
Joel Stanley
> On Jun 27, 2022, at 12:54 PM, Peter Delevoryas <pdel@fb.com> wrote:
>
> Hey everyone,
>
> I'm sending a big patch series for this, but only the last commit is really
> intended to be accepted right now. I'm just including the rest of them
> because it depends on them for testing.
>
> Klaus's changes include the multi-master stuff in hw/i2c/core.c, and the old
> register mode slave mode support.
>
> My series of patches includes a bunch of changes to eliminate most (if not
> all) of the I2C errors reported by the fb OpenBIC firmware for fby35
> CraterLake Bridge Interconnect (BIC) (shortname: oby35-cl) upon startup.
>
> In particular, I needed to add the IC_DEVICE_ID to the isl voltage regulator
> implementation, which required a fix to the pmbus implementation when
> switching pages. We weren't resetting the buffer state when switching
> pages there.
>
> I also added a placeholder implementation of the PECI controller, that does
> almost nothing, but doesn't produce errors.
>
> I added the fby35-cpld, which oby35-cl queries using master-mode TX and RX
> commands.
>
> And finally, I added an "intel-me" device (Intel Management Engine) that
> attempts to respond to the first IPMB message sent by the BIC. I used this
> to test the final patch, which I actually want to merge, the I2C new
> register DMA slave mode support.
>
> All the patches except the last one can be ignored for now if you want,
> again, I just included them for testing purposes.
>
> The final patch is pretty rough, but I wanted to start the review instead of
> waiting too long. I expect the interrupt handling part will be
> the main blocker.
>
Arg, I forgot to send a link to the reference image for oby35-cl:
wget https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.17.01/Y35BCL.elf
qemu-system-arm -machine oby35-cl -kernel Y35BCL.elf -nographic
When it boots, it should look like this:
[00:00:00.006,000] <inf> usb_dc_aspeed: select ep[0x81] as IN endpoint
[00:00:00.006,000] <inf> usb_dc_aspeed: select ep[0x82] as IN endpoint
[00:00:00.006,000] <wrn> usb_dc_aspeed: pre-selected ep[0x1] as IN endpoint
[00:00:00.006,000] <wrn> usb_dc_aspeed: pre-selected ep[0x2] as IN endpoint
[00:00:00.006,000] <inf> usb_dc_aspeed: select ep[0x3] as OUT endpoint
*** Booting Zephyr OS build v00.01.05 ***
Hello, welcome to yv35 craterlake 2022.25.1
BIC class type(class-1), 1ou present status(0), 2ou present status(0), board revision(0x1)
[init_drive_type] sensor 0x14 post sensor read failed!
[init_drive_type] sensor 0x30 post sensor read failed!
[init_drive_type] sensor 0x39 post sensor read failed!
ipmi_init
[set_DC_status] gpio number(15) status(0)
[set_post_status] gpio number(1) status(1)
uart:~$ [00:00:00.680,000] <inf> kcs_aspeed: KCS3: addr=0xca2, idr=0x2c, odr=0x38, str=0x44
[00:00:00.686,000] <err> spi_nor_multi_dev: [1216][spi1_cs0]SFDP magic 00000000 invalid
[00:00:00.686,000] <err> spi_nor_multi_dev: [1456]SFDP read failed: -22
[00:00:00.680,000] <inf> kcs_aspeed: KCS3: addr=0xca2, idr=0x2c, odr=0x38, str=0x44
[00:00:00.686,000] <err> spi_nor_multi_dev: [1216][spi1_cs0]SFDP magic 00000000 invalid
[00:00:00.686,000] <err> spi_nor_multi_dev: [1456]SFDP read failed: -22
uart:~$ BIC Ready
If the i2c patches aren’t included, then the BIC will be
unable to get the initial ME IPMB response and print
an error message:
uart:~$ BIC Ready
Failed to get ME self test result, ret: 0x8
> Thanks,
> Peter
>
> Klaus Jensen (3):
> hw/i2c: support multiple masters
> hw/i2c: add asynchronous send
> hw/i2c/aspeed: add slave device in old register mode
>
> Peter Delevoryas (11):
> aspeed: i2c: Fix DMA len write-enable bit handling
> aspeed: i2c: Fix R_I2CD_FUN_CTRL reference
> aspeed: i2c: Fix MASTER_EN missing error message
> aspeed: Add PECI controller
> hw/misc: Add fby35-cpld
> pmbus: Reset out buf after switching pages
> pmbus: Add read-only IC_DEVICE_ID support
> aspeed: Add oby35-cl machine
> hw/misc: Add intel-me
> aspeed: Add intel-me on i2c6 instead of BMC
> aspeed: Add I2C new register DMA slave mode support
>
> hw/arm/aspeed.c | 42 ++++++
> hw/arm/aspeed_ast10x0.c | 11 ++
> hw/arm/pxa2xx.c | 2 +
> hw/display/sii9022.c | 2 +
> hw/display/ssd0303.c | 2 +
> hw/i2c/aspeed_i2c.c | 234 +++++++++++++++++++++++++++----
> hw/i2c/core.c | 70 ++++++++-
> hw/i2c/pmbus_device.c | 6 +
> hw/i2c/smbus_slave.c | 4 +
> hw/i2c/trace-events | 2 +
> hw/misc/aspeed_peci.c | 225 +++++++++++++++++++++++++++++
> hw/misc/fby35_cpld.c | 137 ++++++++++++++++++
> hw/misc/intel_me.c | 176 +++++++++++++++++++++++
> hw/misc/meson.build | 5 +-
> hw/nvram/eeprom_at24c.c | 2 +
> hw/sensor/isl_pmbus_vr.c | 30 ++++
> hw/sensor/lsm303dlhc_mag.c | 2 +
> include/hw/arm/aspeed_soc.h | 3 +
> include/hw/i2c/aspeed_i2c.h | 11 ++
> include/hw/i2c/i2c.h | 30 ++++
> include/hw/i2c/pmbus_device.h | 1 +
> include/hw/misc/aspeed_peci.h | 34 +++++
> include/hw/sensor/isl_pmbus_vr.h | 1 +
> 23 files changed, 1002 insertions(+), 30 deletions(-)
> create mode 100644 hw/misc/aspeed_peci.c
> create mode 100644 hw/misc/fby35_cpld.c
> create mode 100644 hw/misc/intel_me.c
> create mode 100644 include/hw/misc/aspeed_peci.h
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 12/14] hw/misc: Add intel-me
2022-06-27 19:54 [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
` (10 preceding siblings ...)
2022-06-27 20:04 ` [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
@ 2022-06-27 22:27 ` Peter Delevoryas
2022-06-27 22:27 ` [PATCH 13/14] aspeed: Add intel-me on i2c6 instead of BMC Peter Delevoryas
` (2 more replies)
2022-06-28 7:05 ` [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Cédric Le Goater
12 siblings, 3 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 22:27 UTC (permalink / raw)
Cc: qemu-devel, qemu-arm, Peter Delevoryas
Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
hw/arm/aspeed.c | 1 +
hw/misc/intel_me.c | 176 ++++++++++++++++++++++++++++++++++++++++++++
hw/misc/meson.build | 3 +-
3 files changed, 179 insertions(+), 1 deletion(-)
create mode 100644 hw/misc/intel_me.c
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 2b9c1600c6..88e9a47dc2 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1447,6 +1447,7 @@ static void oby35_cl_i2c_init(AspeedMachineState *bmc)
i2c_slave_create_simple(i2c[1], "tmp105", 0x4a);
i2c_slave_create_simple(i2c[1], "adm1272", 0x40);
i2c_slave_create_simple(i2c[1], "tmp421", 0x4c);
+ i2c_slave_create_simple(i2c[2], "intel-me", 0x16);
i2c_slave_create_simple(i2c[4], "isl69259", 0x76);
i2c_slave_create_simple(i2c[4], "isl69259", 0x62);
i2c_slave_create_simple(i2c[4], "isl69259", 0x60);
diff --git a/hw/misc/intel_me.c b/hw/misc/intel_me.c
new file mode 100644
index 0000000000..fdc9180c26
--- /dev/null
+++ b/hw/misc/intel_me.c
@@ -0,0 +1,176 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
+ *
+ * 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/main-loop.h"
+#include "hw/i2c/i2c.h"
+
+#define TYPE_INTEL_ME "intel-me"
+OBJECT_DECLARE_SIMPLE_TYPE(IntelMEState, INTEL_ME);
+
+#define printf(...)
+
+struct IntelMEState {
+ I2CSlave parent_obj;
+
+ I2CBus *bus;
+ QEMUBH *bh;
+ int rx_len;
+ int tx_len;
+ int tx_pos;
+ uint8_t rx_buf[512];
+ uint8_t tx_buf[512];
+};
+
+static void intel_me_bh(void *opaque)
+{
+ IntelMEState *s = opaque;
+
+ assert(s->bus->bh == s->bh);
+
+ if (s->tx_pos == 0) {
+ if (i2c_start_send_async(s->bus, s->tx_buf[s->tx_pos++]) != 0) {
+ goto done;
+ }
+ return;
+ }
+
+ if (s->tx_pos < s->tx_len) {
+ if (i2c_send_async(s->bus, s->tx_buf[s->tx_pos++]) != 0) {
+ goto done;
+ }
+ return;
+ }
+
+done:
+ i2c_end_transfer(s->bus);
+ i2c_bus_release(s->bus);
+ s->tx_len = 0;
+ s->tx_pos = 0;
+ memset(s->tx_buf, 0, sizeof(s->tx_buf));
+}
+
+static void intel_me_realize(DeviceState *dev, Error **errp)
+{
+ IntelMEState *s = INTEL_ME(dev);
+
+ s->bus = I2C_BUS(qdev_get_parent_bus(dev));
+ s->bh = qemu_bh_new(intel_me_bh, s);
+ s->rx_len = 0;
+ s->tx_len = 0;
+ s->tx_pos = 0;
+ memset(s->rx_buf, 0, sizeof(s->rx_buf));
+ memset(s->tx_buf, 0, sizeof(s->tx_buf));
+}
+
+static uint8_t checksum(const uint8_t *ptr, int len)
+{
+ int sum = 0;
+
+ for (int i = 0; i < len; i++) {
+ sum += ptr[i];
+ }
+
+ return 256 - sum;
+}
+
+static int intel_me_i2c_event(I2CSlave *i2c, enum i2c_event event)
+{
+ IntelMEState *s = INTEL_ME(i2c);
+
+ switch (event) {
+ case I2C_START_RECV:
+ break;
+ case I2C_START_SEND:
+ s->rx_len = 0;
+ memset(s->rx_buf, 0, sizeof(s->rx_buf));
+ break;
+ case I2C_START_SEND_ASYNC:
+ break;
+ case I2C_FINISH:
+ printf("IntelME rx: [");
+ for (int i = 0; i < s->rx_len; i++) {
+ if (i) {
+ printf(", ");
+ }
+ printf("0x%02x", s->rx_buf[i]);
+ }
+ printf("]\n");
+
+ s->tx_len = 10;
+ s->tx_pos = 0;
+ s->tx_buf[0] = s->rx_buf[2];
+ s->tx_buf[1] = ((s->rx_buf[0] >> 2) + 1) << 2;
+ s->tx_buf[2] = 256 - s->tx_buf[0] - s->tx_buf[1];
+ s->tx_buf[3] = i2c->address; // rsSA response Slave Address
+ s->tx_buf[4] = (s->rx_buf[3] >> 2) << 2; // sequence number
+ s->tx_buf[5] = s->rx_buf[4]; // Same command code
+ s->tx_buf[6] = 0x00; // OK
+ s->tx_buf[7] = 0x55; // NO_ERROR
+ s->tx_buf[8] = 0x00;
+ s->tx_buf[9] = checksum(s->tx_buf, s->tx_len - 1);
+ s->tx_buf[0] >>= 1;
+ i2c_bus_master(s->bus, s->bh);
+ break;
+ case I2C_NACK:
+ break;
+ }
+
+ return 0;
+}
+
+static uint8_t intel_me_i2c_recv(I2CSlave *i2c)
+{
+ return 0xff;
+}
+
+static int intel_me_i2c_send(I2CSlave *i2c, uint8_t data)
+{
+ IntelMEState *s = INTEL_ME(i2c);
+
+ assert(s->rx_len < sizeof(s->rx_buf));
+ s->rx_buf[s->rx_len++] = data;
+
+ return 0;
+}
+
+static void intel_me_class_init(ObjectClass *oc, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(oc);
+ I2CSlaveClass *i2c = I2C_SLAVE_CLASS(oc);
+
+ dc->realize = intel_me_realize;
+ i2c->event = intel_me_i2c_event;
+ i2c->recv = intel_me_i2c_recv;
+ i2c->send = intel_me_i2c_send;
+}
+
+static const TypeInfo types[] = {
+ {
+ .name = TYPE_INTEL_ME,
+ .parent = TYPE_I2C_SLAVE,
+ .instance_size = sizeof(IntelMEState),
+ .class_init = intel_me_class_init,
+ },
+};
+
+DEFINE_TYPES(types);
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 1edad44b6b..a2c75894a3 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -118,7 +118,8 @@ softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
'aspeed_sdmc.c',
'aspeed_xdma.c',
'aspeed_peci.c',
- 'fby35_cpld.c'))
+ 'fby35_cpld.c',
+ 'intel_me.c'))
softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-sysreg.c'))
softmmu_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_rng.c'))
--
2.30.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 13/14] aspeed: Add intel-me on i2c6 instead of BMC
2022-06-27 22:27 ` [PATCH 12/14] hw/misc: Add intel-me Peter Delevoryas
@ 2022-06-27 22:27 ` Peter Delevoryas
2022-06-27 22:27 ` [PATCH 14/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
2022-06-28 6:58 ` [PATCH 12/14] hw/misc: Add intel-me Cédric Le Goater
2 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 22:27 UTC (permalink / raw)
Cc: qemu-devel, qemu-arm, Peter Delevoryas
Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
hw/arm/aspeed.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 88e9a47dc2..375d87e6c7 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1448,6 +1448,7 @@ static void oby35_cl_i2c_init(AspeedMachineState *bmc)
i2c_slave_create_simple(i2c[1], "adm1272", 0x40);
i2c_slave_create_simple(i2c[1], "tmp421", 0x4c);
i2c_slave_create_simple(i2c[2], "intel-me", 0x16);
+ i2c_slave_create_simple(i2c[6], "intel-me", 0x10);
i2c_slave_create_simple(i2c[4], "isl69259", 0x76);
i2c_slave_create_simple(i2c[4], "isl69259", 0x62);
i2c_slave_create_simple(i2c[4], "isl69259", 0x60);
--
2.30.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 14/14] aspeed: Add I2C new register DMA slave mode support
2022-06-27 22:27 ` [PATCH 12/14] hw/misc: Add intel-me Peter Delevoryas
2022-06-27 22:27 ` [PATCH 13/14] aspeed: Add intel-me on i2c6 instead of BMC Peter Delevoryas
@ 2022-06-27 22:27 ` Peter Delevoryas
2022-06-28 7:02 ` Cédric Le Goater
2022-06-28 6:58 ` [PATCH 12/14] hw/misc: Add intel-me Cédric Le Goater
2 siblings, 1 reply; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-27 22:27 UTC (permalink / raw)
Cc: qemu-devel, qemu-arm, Peter Delevoryas
Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
hw/i2c/aspeed_i2c.c | 133 ++++++++++++++++++++++++++++++++----
include/hw/i2c/aspeed_i2c.h | 3 +
2 files changed, 124 insertions(+), 12 deletions(-)
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 8a8514586f..fc8b6b62cf 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -78,6 +78,18 @@ static inline void aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus)
}
}
+static inline void aspeed_i2c_bus_raise_slave_interrupt(AspeedI2CBus *bus)
+{
+ AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
+
+ if (!bus->regs[R_I2CS_INTR_STS]) {
+ return;
+ }
+
+ bus->controller->intr_status |= 1 << bus->id;
+ qemu_irq_raise(aic->bus_get_irq(bus));
+}
+
static uint64_t aspeed_i2c_bus_old_read(AspeedI2CBus *bus, hwaddr offset,
unsigned size)
{
@@ -140,8 +152,17 @@ static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset,
case A_I2CM_DMA_LEN_STS:
case A_I2CC_DMA_ADDR:
case A_I2CC_DMA_LEN:
+
+ case A_I2CS_DEV_ADDR:
+ case A_I2CS_DMA_RX_ADDR:
+ case A_I2CS_DMA_LEN:
+ case A_I2CS_CMD:
+ case A_I2CS_INTR_CTRL:
+ case A_I2CS_DMA_LEN_STS:
/* Value is already set, don't do anything. */
break;
+ case A_I2CS_INTR_STS:
+ break;
case A_I2CM_CMD:
value = SHARED_FIELD_DP32(value, BUS_BUSY_STS, i2c_bus_busy(bus->bus));
break;
@@ -547,12 +568,7 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
switch (offset) {
case A_I2CC_FUN_CTRL:
- if (SHARED_FIELD_EX32(value, SLAVE_EN)) {
- qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
- __func__);
- break;
- }
- bus->regs[R_I2CC_FUN_CTRL] = value & 0x007dc3ff;
+ bus->regs[R_I2CC_FUN_CTRL] = value;
break;
case A_I2CC_AC_TIMING:
bus->regs[R_I2CC_AC_TIMING] = value & 0x1ffff0ff;
@@ -580,6 +596,7 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
bus->controller->intr_status &= ~(1 << bus->id);
qemu_irq_lower(aic->bus_get_irq(bus));
}
+ aspeed_i2c_bus_raise_slave_interrupt(bus);
break;
}
bus->regs[R_I2CM_INTR_STS] &= ~(value & 0xf007f07f);
@@ -668,15 +685,53 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
case A_I2CC_DMA_LEN:
/* RO */
break;
- case A_I2CS_DMA_LEN_STS:
- case A_I2CS_DMA_TX_ADDR:
- case A_I2CS_DMA_RX_ADDR:
case A_I2CS_DEV_ADDR:
+ bus->regs[R_I2CS_DEV_ADDR] = value;
+ break;
+ case A_I2CS_DMA_RX_ADDR:
+ bus->regs[R_I2CS_DMA_RX_ADDR] = value;
+ break;
+ case A_I2CS_DMA_LEN:
+ assert(FIELD_EX32(value, I2CS_DMA_LEN, TX_BUF_LEN) == 0);
+ if (FIELD_EX32(value, I2CS_DMA_LEN, RX_BUF_LEN_W1T)) {
+ ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN, RX_BUF_LEN,
+ FIELD_EX32(value, I2CS_DMA_LEN, RX_BUF_LEN));
+ } else {
+ bus->regs[R_I2CS_DMA_LEN] = value;
+ }
+ break;
+ case A_I2CS_CMD:
+ if (FIELD_EX32(value, I2CS_CMD, W1_CTRL)) {
+ bus->regs[R_I2CS_CMD] |= value;
+ } else {
+ bus->regs[R_I2CS_CMD] = value;
+ }
+ i2c_slave_set_address(bus->slave, bus->regs[R_I2CS_DEV_ADDR]);
+ break;
case A_I2CS_INTR_CTRL:
+ bus->regs[R_I2CS_INTR_CTRL] = value;
+ break;
+
case A_I2CS_INTR_STS:
- case A_I2CS_CMD:
- case A_I2CS_DMA_LEN:
- qemu_log_mask(LOG_UNIMP, "%s: Slave mode is not implemented\n",
+ if (ARRAY_FIELD_EX32(bus->regs, I2CS_INTR_CTRL, PKT_CMD_DONE)) {
+ if (ARRAY_FIELD_EX32(bus->regs, I2CS_INTR_STS, PKT_CMD_DONE) &&
+ FIELD_EX32(value, I2CS_INTR_STS, PKT_CMD_DONE)) {
+ bus->regs[R_I2CS_INTR_STS] &= 0xfffc0000;
+ }
+ } else {
+ bus->regs[R_I2CS_INTR_STS] &= ~value;
+ }
+ if (!bus->regs[R_I2CS_INTR_STS]) {
+ bus->controller->intr_status &= ~(1 << bus->id);
+ qemu_irq_lower(aic->bus_get_irq(bus));
+ }
+ aspeed_i2c_bus_raise_interrupt(bus);
+ break;
+ case A_I2CS_DMA_LEN_STS:
+ bus->regs[R_I2CS_DMA_LEN_STS] = 0;
+ break;
+ case A_I2CS_DMA_TX_ADDR:
+ qemu_log_mask(LOG_UNIMP, "%s: Slave mode DMA TX is not implemented\n",
__func__);
break;
default:
@@ -1037,6 +1092,39 @@ static const TypeInfo aspeed_i2c_info = {
.abstract = true,
};
+static int aspeed_i2c_bus_new_slave_event(AspeedI2CBus *bus,
+ enum i2c_event event)
+{
+ switch (event) {
+ case I2C_START_SEND_ASYNC:
+ if (!SHARED_ARRAY_FIELD_EX32(bus->regs, R_I2CS_CMD, RX_DMA_EN)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Slave mode RX DMA is not enabled\n", __func__);
+ return -1;
+ }
+ ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN, 0);
+ bus->regs[R_I2CC_DMA_ADDR] =
+ ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_RX_ADDR, ADDR);
+ bus->regs[R_I2CC_DMA_LEN] =
+ ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_LEN, RX_BUF_LEN) + 1;
+ i2c_ack(bus->bus);
+ break;
+ case I2C_FINISH:
+ ARRAY_FIELD_DP32(bus->regs, I2CS_INTR_STS, PKT_CMD_DONE, 1);
+ ARRAY_FIELD_DP32(bus->regs, I2CS_INTR_STS, SLAVE_ADDR_RX_MATCH, 1);
+ SHARED_ARRAY_FIELD_DP32(bus->regs, R_I2CS_INTR_STS, NORMAL_STOP, 1);
+ SHARED_ARRAY_FIELD_DP32(bus->regs, R_I2CS_INTR_STS, RX_DONE, 1);
+ aspeed_i2c_bus_raise_slave_interrupt(bus);
+ break;
+ default:
+ qemu_log_mask(LOG_UNIMP, "%s: i2c event %d unimplemented\n",
+ __func__, event);
+ return -1;
+ }
+
+ return 0;
+}
+
static int aspeed_i2c_bus_slave_event(I2CSlave *slave, enum i2c_event event)
{
BusState *qbus = qdev_get_parent_bus(DEVICE(slave));
@@ -1045,6 +1133,10 @@ static int aspeed_i2c_bus_slave_event(I2CSlave *slave, enum i2c_event event)
uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus);
uint32_t value;
+ if (aspeed_i2c_is_new_mode(bus->controller)) {
+ return aspeed_i2c_bus_new_slave_event(bus, event);
+ }
+
switch (event) {
case I2C_START_SEND_ASYNC:
value = SHARED_ARRAY_FIELD_EX32(bus->regs, reg_byte_buf, TX_BUF);
@@ -1073,6 +1165,19 @@ static int aspeed_i2c_bus_slave_event(I2CSlave *slave, enum i2c_event event)
return 0;
}
+static void aspeed_i2c_bus_new_slave_send_async(AspeedI2CBus *bus, uint8_t data)
+{
+ assert(address_space_write(&bus->controller->dram_as,
+ bus->regs[R_I2CC_DMA_ADDR],
+ MEMTXATTRS_UNSPECIFIED, &data, 1) == MEMTX_OK);
+
+ bus->regs[R_I2CC_DMA_ADDR]++;
+ bus->regs[R_I2CC_DMA_LEN]--;
+ ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN,
+ ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN) + 1);
+ i2c_ack(bus->bus);
+}
+
static void aspeed_i2c_bus_slave_send_async(I2CSlave *slave, uint8_t data)
{
BusState *qbus = qdev_get_parent_bus(DEVICE(slave));
@@ -1080,6 +1185,10 @@ static void aspeed_i2c_bus_slave_send_async(I2CSlave *slave, uint8_t data)
uint32_t reg_intr_sts = aspeed_i2c_bus_intr_sts_offset(bus);
uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus);
+ if (aspeed_i2c_is_new_mode(bus->controller)) {
+ return aspeed_i2c_bus_new_slave_send_async(bus, data);
+ }
+
SHARED_ARRAY_FIELD_DP32(bus->regs, reg_byte_buf, RX_BUF, data);
SHARED_ARRAY_FIELD_DP32(bus->regs, reg_intr_sts, RX_DONE, 1);
diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
index ba148b2f6d..300a89b343 100644
--- a/include/hw/i2c/aspeed_i2c.h
+++ b/include/hw/i2c/aspeed_i2c.h
@@ -174,6 +174,8 @@ REG32(I2CM_DMA_LEN, 0x1c)
FIELD(I2CM_DMA_LEN, TX_BUF_LEN_W1T, 15, 1)
FIELD(I2CM_DMA_LEN, TX_BUF_LEN, 0, 11)
REG32(I2CS_INTR_CTRL, 0x20)
+ FIELD(I2CS_INTR_CTRL, PKT_CMD_FAIL, 17, 1)
+ FIELD(I2CS_INTR_CTRL, PKT_CMD_DONE, 16, 1)
REG32(I2CS_INTR_STS, 0x24)
/* 31:29 shared with I2CD_INTR_STS[31:29] */
FIELD(I2CS_INTR_STS, SLAVE_PARKING_STS, 24, 2)
@@ -184,6 +186,7 @@ REG32(I2CS_INTR_STS, 0x24)
FIELD(I2CS_INTR_STS, PKT_CMD_FAIL, 17, 1)
FIELD(I2CS_INTR_STS, PKT_CMD_DONE, 16, 1)
/* 14:0 shared with I2CD_INTR_STS[14:0] */
+ FIELD(I2CS_INTR_STS, SLAVE_ADDR_RX_MATCH, 7, 1)
REG32(I2CS_CMD, 0x28)
FIELD(I2CS_CMD, W1_CTRL, 31, 1)
FIELD(I2CS_CMD, PKT_MODE_ACTIVE_ADDR, 17, 2)
--
2.30.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 07/14] aspeed: Add PECI controller
2022-06-27 19:54 ` [PATCH 07/14] aspeed: Add PECI controller Peter Delevoryas
@ 2022-06-28 6:47 ` Cédric Le Goater
2022-06-28 6:58 ` Peter Delevoryas
0 siblings, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2022-06-28 6:47 UTC (permalink / raw)
To: Peter Delevoryas
Cc: zhdaniel, qemu-devel, qemu-arm, komlodi, titusr, andrew, joel
On 6/27/22 21:54, Peter Delevoryas wrote:
Could we have some short intro ? :)
> Signed-off-by: Peter Delevoryas <pdel@fb.com>
> ---
> hw/arm/aspeed_ast10x0.c | 11 ++
> hw/misc/aspeed_peci.c | 225 ++++++++++++++++++++++++++++++++++
> hw/misc/meson.build | 3 +-
> include/hw/arm/aspeed_soc.h | 3 +
> include/hw/misc/aspeed_peci.h | 34 +++++
> 5 files changed, 275 insertions(+), 1 deletion(-)
> create mode 100644 hw/misc/aspeed_peci.c
> create mode 100644 include/hw/misc/aspeed_peci.h
>
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index 5df480a21f..780841ea84 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -47,6 +47,7 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
> [ASPEED_DEV_UART13] = 0x7E790700,
> [ASPEED_DEV_WDT] = 0x7E785000,
> [ASPEED_DEV_LPC] = 0x7E789000,
> + [ASPEED_DEV_PECI] = 0x7E78B000,
> [ASPEED_DEV_I2C] = 0x7E7B0000,
> };
>
> @@ -75,6 +76,7 @@ static const int aspeed_soc_ast1030_irqmap[] = {
> [ASPEED_DEV_TIMER8] = 23,
> [ASPEED_DEV_WDT] = 24,
> [ASPEED_DEV_LPC] = 35,
> + [ASPEED_DEV_PECI] = 38,
> [ASPEED_DEV_FMC] = 39,
> [ASPEED_DEV_PWM] = 44,
> [ASPEED_DEV_ADC] = 46,
> @@ -133,6 +135,8 @@ static void aspeed_soc_ast1030_init(Object *obj)
>
> object_initialize_child(obj, "lpc", &s->lpc, TYPE_ASPEED_LPC);
>
> + object_initialize_child(obj, "peci", &s->peci, TYPE_ASPEED_PECI);
> +
> object_initialize_child(obj, "sbc", &s->sbc, TYPE_ASPEED_SBC);
>
> for (i = 0; i < sc->wdts_num; i++) {
> @@ -238,6 +242,13 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
> /* UART */
> aspeed_soc_uart_init(s);
>
> + /* PECI */
> + if (!sysbus_realize(SYS_BUS_DEVICE(&s->peci), errp)) {
> + return;
> + }
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->peci), 0, sc->memmap[ASPEED_DEV_PECI]);
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->peci), 0, aspeed_soc_get_irq(s, ASPEED_DEV_PECI));
> +
> /* Timer */
> object_property_set_link(OBJECT(&s->timerctrl), "scu", OBJECT(&s->scu),
> &error_abort);
> diff --git a/hw/misc/aspeed_peci.c b/hw/misc/aspeed_peci.c
> new file mode 100644
> index 0000000000..670e532fc0
> --- /dev/null
> +++ b/hw/misc/aspeed_peci.c
> @@ -0,0 +1,225 @@
> +/*
> + * Aspeed PECI Controller
> + *
> + * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
> + *
> + * This code is licensed under the GPL version 2 or later. See the COPYING
> + * file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "hw/irq.h"
> +#include "hw/misc/aspeed_peci.h"
> +
> +#define U(x) (x##U)
> +#define GENMASK(h, l) \
> + (((~U(0)) - (U(1) << (l)) + 1) & \
> + (~U(0) >> (32 - 1 - (h))))
I beleive QEMU has similar macros to generate masks.
> +/* ASPEED PECI Registers */
> +/* Control Register */
> +#define ASPEED_PECI_CTRL (0x00 / 4)
> +#define ASPEED_PECI_CTRL_SAMPLING_MASK GENMASK(19, 16)
> +#define ASPEED_PECI_CTRL_RD_MODE_MASK GENMASK(13, 12)
> +#define ASPEED_PECI_CTRL_RD_MODE_DBG BIT(13)
> +#define ASPEED_PECI_CTRL_RD_MODE_COUNT BIT(12)
> +#define ASPEED_PECI_CTRL_CLK_SRC_HCLK BIT(11)
> +#define ASPEED_PECI_CTRL_CLK_DIV_MASK GENMASK(10, 8)
> +#define ASPEED_PECI_CTRL_INVERT_OUT BIT(7)
> +#define ASPEED_PECI_CTRL_INVERT_IN BIT(6)
> +#define ASPEED_PECI_CTRL_BUS_CONTENTION_EN BIT(5)
> +#define ASPEED_PECI_CTRL_PECI_EN BIT(4)
> +#define ASPEED_PECI_CTRL_PECI_CLK_EN BIT(0)
> +
> +/* Timing Negotiation Register */
> +#define ASPEED_PECI_TIMING_NEGOTIATION (0x04 / 4)
> +#define ASPEED_PECI_T_NEGO_MSG_MASK GENMASK(15, 8)
> +#define ASPEED_PECI_T_NEGO_ADDR_MASK GENMASK(7, 0)
> +
> +/* Command Register */
> +#define ASPEED_PECI_CMD (0x08 / 4)
> +#define ASPEED_PECI_CMD_PIN_MONITORING BIT(31)
> +#define ASPEED_PECI_CMD_STS_MASK GENMASK(27, 24)
> +#define ASPEED_PECI_CMD_STS_ADDR_T_NEGO 0x3
> +#define ASPEED_PECI_CMD_IDLE_MASK \
> + (ASPEED_PECI_CMD_STS_MASK | ASPEED_PECI_CMD_PIN_MONITORING)
> +#define ASPEED_PECI_CMD_FIRE BIT(0)
> +
> +/* Read/Write Length Register */
> +#define ASPEED_PECI_RW_LENGTH (0x0c / 4)
> +#define ASPEED_PECI_AW_FCS_EN BIT(31)
> +#define ASPEED_PECI_RD_LEN_MASK GENMASK(23, 16)
> +#define ASPEED_PECI_WR_LEN_MASK GENMASK(15, 8)
> +#define ASPEED_PECI_TARGET_ADDR_MASK GENMASK(7, 0)
> +
> +/* Expected FCS Data Register */
> +#define ASPEED_PECI_EXPECTED_FCS (0x10 / 4)
> +#define ASPEED_PECI_EXPECTED_RD_FCS_MASK GENMASK(23, 16)
> +#define ASPEED_PECI_EXPECTED_AW_FCS_AUTO_MASK GENMASK(15, 8)
> +#define ASPEED_PECI_EXPECTED_WR_FCS_MASK GENMASK(7, 0)
> +
> +/* Captured FCS Data Register */
> +#define ASPEED_PECI_CAPTURED_FCS (0x14 / 4)
> +#define ASPEED_PECI_CAPTURED_RD_FCS_MASK GENMASK(23, 16)
> +#define ASPEED_PECI_CAPTURED_WR_FCS_MASK GENMASK(7, 0)
> +
> +/* Interrupt Register */
> +#define ASPEED_PECI_INT_CTRL (0x18 / 4)
> +#define ASPEED_PECI_TIMING_NEGO_SEL_MASK GENMASK(31, 30)
> +#define ASPEED_PECI_1ST_BIT_OF_ADDR_NEGO 0
> +#define ASPEED_PECI_2ND_BIT_OF_ADDR_NEGO 1
> +#define ASPEED_PECI_MESSAGE_NEGO 2
> +#define ASPEED_PECI_INT_MASK GENMASK(4, 0)
> +#define ASPEED_PECI_INT_BUS_TIMEOUT BIT(4)
> +#define ASPEED_PECI_INT_BUS_CONTENTION BIT(3)
> +#define ASPEED_PECI_INT_WR_FCS_BAD BIT(2)
> +#define ASPEED_PECI_INT_WR_FCS_ABORT BIT(1)
> +#define ASPEED_PECI_INT_CMD_DONE BIT(0)
> +
> +/* Interrupt Status Register */
> +#define ASPEED_PECI_INT_STS (0x1c / 4)
> +#define ASPEED_PECI_INT_TIMING_RESULT_MASK GENMASK(29, 16)
> + /* bits[4..0]: Same bit fields in the 'Interrupt Register' */
> +
> +/* Rx/Tx Data Buffer Registers */
> +#define ASPEED_PECI_WR_DATA0 (0x20 / 4)
> +#define ASPEED_PECI_WR_DATA1 (0x24 / 4)
> +#define ASPEED_PECI_WR_DATA2 (0x28 / 4)
> +#define ASPEED_PECI_WR_DATA3 (0x2c / 4)
> +#define ASPEED_PECI_RD_DATA0 (0x30 / 4)
> +#define ASPEED_PECI_RD_DATA1 (0x34 / 4)
> +#define ASPEED_PECI_RD_DATA2 (0x38 / 4)
> +#define ASPEED_PECI_RD_DATA3 (0x3c / 4)
> +#define ASPEED_PECI_WR_DATA4 (0x40 / 4)
> +#define ASPEED_PECI_WR_DATA5 (0x44 / 4)
> +#define ASPEED_PECI_WR_DATA6 (0x48 / 4)
> +#define ASPEED_PECI_WR_DATA7 (0x4c / 4)
> +#define ASPEED_PECI_RD_DATA4 (0x50 / 4)
> +#define ASPEED_PECI_RD_DATA5 (0x54 / 4)
> +#define ASPEED_PECI_RD_DATA6 (0x58 / 4)
> +#define ASPEED_PECI_RD_DATA7 (0x5c / 4)
> +#define ASPEED_PECI_DATA_BUF_SIZE_MAX 32
> +
> +/** PECI read/write supported responses */
> +#define PECI_CC_RSP_SUCCESS (0x40U)
> +#define PECI_CC_RSP_TIMEOUT (0x80U)
> +#define PECI_CC_OUT_OF_RESOURCES_TIMEOUT (0x81U)
> +#define PECI_CC_RESOURCES_LOWPWR_TIMEOUT (0x82U)
> +#define PECI_CC_ILLEGAL_REQUEST (0x90U)
> +
> +static void aspeed_peci_instance_init(Object *obj)
> +{
> +}
May be drop this routine if it is empty.
> +static uint64_t aspeed_peci_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + AspeedPECIState *s = ASPEED_PECI(opaque);
> +
> + if (addr >= ASPEED_PECI_NR_REGS << 2) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx "\n",
> + __func__, addr);
> + return 0;
> + }
> + addr >>= 2;
> +
> + uint32_t reg = s->regs[addr];
> + //printf("%s: 0x%08lx 0x%08x %u\n", __func__, addr << 2, reg, size);
Convert to trace event ? or please remove.
> +
> + return reg;
> +}
> +
> +static void aspeed_peci_write(void *opaque, hwaddr addr, uint64_t data, unsigned size)
> +{
> + AspeedPECIState *s = ASPEED_PECI(opaque);
> +
> + //printf("%s: 0x%08lx 0x%08x %u\n", __func__, addr, reg, size);
> +
> + if (addr >= ASPEED_PECI_NR_REGS << 2) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n",
> + __func__, addr);
> + return;
> + }
> + addr >>= 2;
> +
> + switch (addr) {
> + case ASPEED_PECI_INT_STS:
> + s->regs[addr] &= ~data;
> + break;
> + default:
> + s->regs[addr] = data;
> + break;
> + }
> +
> + switch (addr) {
> + case ASPEED_PECI_CMD:
> + if (!(s->regs[ASPEED_PECI_CMD] & ASPEED_PECI_CMD_FIRE)) {
> + break;
> + }
> + s->regs[ASPEED_PECI_RD_DATA0] = PECI_CC_RSP_SUCCESS;
> + s->regs[ASPEED_PECI_WR_DATA0] = PECI_CC_RSP_SUCCESS;
> +
> + s->regs[ASPEED_PECI_INT_STS] |= ASPEED_PECI_INT_CMD_DONE;
> + qemu_irq_raise(s->irq);
> + break;
> + case ASPEED_PECI_INT_STS:
> + if (s->regs[ASPEED_PECI_INT_STS]) {
> + break;
> + }
> + qemu_irq_lower(s->irq);
> + break;
> + default:
> + qemu_log_mask(LOG_UNIMP, "%s: register 0x%03" HWADDR_PRIx " writes unimplemented\n",
> + __func__, addr);
> + break;
> + }
> +}
> +
> +static const MemoryRegionOps aspeed_peci_ops = {
> + .read = aspeed_peci_read,
> + .write = aspeed_peci_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static void aspeed_peci_realize(DeviceState *dev, Error **errp)
> +{
> + AspeedPECIState *s = ASPEED_PECI(dev);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> + memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_peci_ops, s, TYPE_ASPEED_PECI, 0x1000);
> + sysbus_init_mmio(sbd, &s->mmio);
> + sysbus_init_irq(sbd, &s->irq);
> +}
> +
> +static void aspeed_peci_reset(DeviceState *dev)
> +{
> + AspeedPECIState *s = ASPEED_PECI(dev);
> +
> + memset(s->regs, 0, sizeof(s->regs));
> +}
> +
> +static void aspeed_peci_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->realize = aspeed_peci_realize;
> + dc->reset = aspeed_peci_reset;
> + dc->desc = "Aspeed PECI Controller";
> +}
> +
> +static const TypeInfo aspeed_peci_info = {
> + .name = TYPE_ASPEED_PECI,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_init = aspeed_peci_instance_init,
> + .instance_size = sizeof(AspeedPECIState),
> + .class_init = aspeed_peci_class_init,
> + .class_size = sizeof(AspeedPECIClass),
> + .abstract = false,
> +};
> +
> +static void aspeed_peci_register_types(void)
> +{
> + type_register_static(&aspeed_peci_info);
> +}
> +
> +type_init(aspeed_peci_register_types);
> diff --git a/hw/misc/meson.build b/hw/misc/meson.build
> index 132b7b7344..95268eddc0 100644
> --- a/hw/misc/meson.build
> +++ b/hw/misc/meson.build
> @@ -116,7 +116,8 @@ softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
> 'aspeed_scu.c',
> 'aspeed_sbc.c',
> 'aspeed_sdmc.c',
> - 'aspeed_xdma.c'))
> + 'aspeed_xdma.c',
> + 'aspeed_peci.c'))
>
> softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-sysreg.c'))
> softmmu_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_rng.c'))
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 02a5a9ffcb..fd2aa1880a 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -34,6 +34,7 @@
> #include "hw/usb/hcd-ehci.h"
> #include "qom/object.h"
> #include "hw/misc/aspeed_lpc.h"
> +#include "hw/misc/aspeed_peci.h"
>
> #define ASPEED_SPIS_NUM 2
> #define ASPEED_EHCIS_NUM 2
> @@ -73,6 +74,7 @@ struct AspeedSoCState {
> AspeedSDHCIState sdhci;
> AspeedSDHCIState emmc;
> AspeedLPCState lpc;
> + AspeedPECIState peci;
> uint32_t uart_default;
> Clock *sysclk;
> };
> @@ -161,6 +163,7 @@ enum {
> ASPEED_DEV_DPMCU,
> ASPEED_DEV_DP,
> ASPEED_DEV_I3C,
> + ASPEED_DEV_PECI,
> };
>
> qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev);
> diff --git a/include/hw/misc/aspeed_peci.h b/include/hw/misc/aspeed_peci.h
> new file mode 100644
> index 0000000000..81c7d31700
> --- /dev/null
> +++ b/include/hw/misc/aspeed_peci.h
> @@ -0,0 +1,34 @@
> +/*
> + * Aspeed PECI Controller
> + *
> + * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
> + *
> + * This code is licensed under the GPL version 2 or later. See the COPYING
> + * file in the top-level directory.
> + */
> +
> +#ifndef ASPEED_PECI_H
> +#define ASPEED_PECI_H
> +
> +#include "hw/sysbus.h"
> +
> +#define TYPE_ASPEED_PECI "aspeed.peci"
> +OBJECT_DECLARE_TYPE(AspeedPECIState, AspeedPECIClass, ASPEED_PECI);
> +
> +#define ASPEED_PECI_NR_REGS ((0xFC + 4) >> 2)
> +
> +struct AspeedPECIState {
> + /* <private> */
> + SysBusDevice parent;
> +
> + MemoryRegion mmio;
> + qemu_irq irq;
> +
> + uint32_t regs[ASPEED_PECI_NR_REGS];
> +};
> +
> +struct AspeedPECIClass {
> + SysBusDeviceClass parent_class;
> +};
> +
> +#endif
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 08/14] hw/misc: Add fby35-cpld
2022-06-27 19:55 ` [PATCH 08/14] hw/misc: Add fby35-cpld Peter Delevoryas
@ 2022-06-28 6:50 ` Cédric Le Goater
2022-06-28 7:00 ` Peter Delevoryas
0 siblings, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2022-06-28 6:50 UTC (permalink / raw)
To: Peter Delevoryas
Cc: zhdaniel, qemu-devel, qemu-arm, komlodi, titusr, andrew, joel
On 6/27/22 21:55, Peter Delevoryas wrote:
> Signed-off-by: Peter Delevoryas <pdel@fb.com>
some intro ?
> ---
> hw/misc/fby35_cpld.c | 137 +++++++++++++++++++++++++++++++++++++++++++
> hw/misc/meson.build | 3 +-
> 2 files changed, 139 insertions(+), 1 deletion(-)
> create mode 100644 hw/misc/fby35_cpld.c
>
> diff --git a/hw/misc/fby35_cpld.c b/hw/misc/fby35_cpld.c
> new file mode 100644
> index 0000000000..a783a0a2c8
> --- /dev/null
> +++ b/hw/misc/fby35_cpld.c
> @@ -0,0 +1,137 @@
> +/*
> + * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
> + *
> + * 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.
> + */
I prefer the short version of the license.
> +#include "qemu/osdep.h"
> +#include "hw/i2c/i2c.h"
> +#include "hw/registerfields.h"
> +
> +#define BOARD_ID_CLASS1 0b0000
> +#define BOARD_ID_CLASS2 0b0001
> +
> +#define TYPE_FBY35_CPLD "fby35-cpld"
> +OBJECT_DECLARE_SIMPLE_TYPE(Fby35CpldState, FBY35_CPLD);
> +
> +REG8(CLASS_TYPE, 0x5);
> + FIELD(CLASS_TYPE, RESERVED, 0, 2);
> + FIELD(CLASS_TYPE, 1OU_EXPANSION_NOT_PRESENT, 2, 1);
> + FIELD(CLASS_TYPE, 2OU_EXPANSION_NOT_PRESENT, 3, 1);
> + FIELD(CLASS_TYPE, BOARD_ID, 4, 4);
> +REG8(BOARD_REVISION, 0x8);
> + FIELD(BOARD_REVISION, VALUE, 0, 4);
> + FIELD(BOARD_REVISION, RESERVED, 4, 4);
> +
> +struct Fby35CpldState {
> + I2CSlave parent_obj;
> +
> + uint8_t target_reg;
> + uint32_t regs[10];
> +};
> +
> +static void fby35_cpld_realize(DeviceState *dev, Error **errp)
> +{
> + Fby35CpldState *s = FBY35_CPLD(dev);
> +
> + memset(s->regs, 0, sizeof(s->regs));
> + s->target_reg = 0;
> +
> + ARRAY_FIELD_DP32(s->regs, CLASS_TYPE, BOARD_ID, 0b0000);
> + ARRAY_FIELD_DP32(s->regs, CLASS_TYPE, 1OU_EXPANSION_NOT_PRESENT, 1);
> + ARRAY_FIELD_DP32(s->regs, CLASS_TYPE, 2OU_EXPANSION_NOT_PRESENT, 1);
> + ARRAY_FIELD_DP32(s->regs, BOARD_REVISION, VALUE, 0x1);
> +}
> +
> +static int fby35_cpld_i2c_event(I2CSlave *i2c, enum i2c_event event)
> +{
> + Fby35CpldState *s = FBY35_CPLD(i2c);
> +
> + switch (event) {
> + case I2C_START_RECV:
> + break;
> + case I2C_START_SEND:
> + s->target_reg = 0;
> + break;
> + case I2C_START_SEND_ASYNC:
> + case I2C_FINISH:
> + case I2C_NACK:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static uint8_t fby35_cpld_i2c_recv(I2CSlave *i2c)
> +{
> + Fby35CpldState *s = FBY35_CPLD(i2c);
> +
> + switch (s->target_reg) {
> + case R_CLASS_TYPE:
> + case R_BOARD_REVISION:
> + return s->regs[s->target_reg];
> + default:
> + printf("%s: unexpected register read 0x%02x\n", __func__, s->target_reg);
please use the qemu logging system
> + return 0xff;
> + }
> +}
> +
> +static int fby35_cpld_i2c_send(I2CSlave *i2c, uint8_t data)
> +{
> + Fby35CpldState *s = FBY35_CPLD(i2c);
> +
> + if (s->target_reg == 0) {
> + s->target_reg = data;
> + return 0;
> + }
> +
> + switch (s->target_reg) {
> + case R_CLASS_TYPE:
> + case R_BOARD_REVISION:
> + s->regs[s->target_reg] = data;
> + break;
> + default:
> + printf("%s: unexpected register write 0x%02x 0x%02x\n", __func__, s->target_reg, data);
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static void fby35_cpld_class_init(ObjectClass *oc, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(oc);
> + I2CSlaveClass *i2c = I2C_SLAVE_CLASS(oc);
> +
> + dc->realize = fby35_cpld_realize;
> + i2c->event = fby35_cpld_i2c_event;
> + i2c->recv = fby35_cpld_i2c_recv;
> + i2c->send = fby35_cpld_i2c_send;
> +}
> +
> +static const TypeInfo types[] = {
> + {
> + .name = TYPE_FBY35_CPLD,
> + .parent = TYPE_I2C_SLAVE,
> + .instance_size = sizeof(Fby35CpldState),
> + .class_init = fby35_cpld_class_init,
> + },
> +};
> +
> +DEFINE_TYPES(types);
> diff --git a/hw/misc/meson.build b/hw/misc/meson.build
> index 95268eddc0..1edad44b6b 100644
> --- a/hw/misc/meson.build
> +++ b/hw/misc/meson.build
> @@ -117,7 +117,8 @@ softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
> 'aspeed_sbc.c',
> 'aspeed_sdmc.c',
> 'aspeed_xdma.c',
> - 'aspeed_peci.c'))
> + 'aspeed_peci.c',
> + 'fby35_cpld.c'))
>
> softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-sysreg.c'))
> softmmu_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_rng.c'))
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 09/14] pmbus: Reset out buf after switching pages
2022-06-27 19:55 ` [PATCH 09/14] pmbus: Reset out buf after switching pages Peter Delevoryas
@ 2022-06-28 6:51 ` Cédric Le Goater
2022-06-28 7:04 ` Peter Delevoryas
0 siblings, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2022-06-28 6:51 UTC (permalink / raw)
To: Peter Delevoryas
Cc: zhdaniel, qemu-devel, qemu-arm, komlodi, titusr, andrew, joel
On 6/27/22 21:55, Peter Delevoryas wrote:
> Signed-off-by: Peter Delevoryas <pdel@fb.com>
is that a bug ?
> ---
> hw/i2c/pmbus_device.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
> index 62885fa6a1..efddc36fd9 100644
> --- a/hw/i2c/pmbus_device.c
> +++ b/hw/i2c/pmbus_device.c
> @@ -1088,6 +1088,7 @@ static int pmbus_write_data(SMBusDevice *smd, uint8_t *buf, uint8_t len)
>
> if (pmdev->code == PMBUS_PAGE) {
> pmdev->page = pmbus_receive8(pmdev);
> + pmdev->out_buf_len = 0;
> return 0;
> }
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 12/14] hw/misc: Add intel-me
2022-06-27 22:27 ` [PATCH 12/14] hw/misc: Add intel-me Peter Delevoryas
2022-06-27 22:27 ` [PATCH 13/14] aspeed: Add intel-me on i2c6 instead of BMC Peter Delevoryas
2022-06-27 22:27 ` [PATCH 14/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
@ 2022-06-28 6:58 ` Cédric Le Goater
2022-06-28 7:17 ` Peter Delevoryas
2 siblings, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2022-06-28 6:58 UTC (permalink / raw)
To: Peter Delevoryas; +Cc: qemu-devel, qemu-arm
On 6/28/22 00:27, Peter Delevoryas wrote:
> Signed-off-by: Peter Delevoryas <pdel@fb.com>
Intro ?
I would rather have 2 patches, one for the slave model and one adding
a device to the machine.
Please replace the printf with trace events.
Thanks,
C.
> ---
> hw/arm/aspeed.c | 1 +
> hw/misc/intel_me.c | 176 ++++++++++++++++++++++++++++++++++++++++++++
> hw/misc/meson.build | 3 +-
> 3 files changed, 179 insertions(+), 1 deletion(-)
> create mode 100644 hw/misc/intel_me.c
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 2b9c1600c6..88e9a47dc2 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -1447,6 +1447,7 @@ static void oby35_cl_i2c_init(AspeedMachineState *bmc)
> i2c_slave_create_simple(i2c[1], "tmp105", 0x4a);
> i2c_slave_create_simple(i2c[1], "adm1272", 0x40);
> i2c_slave_create_simple(i2c[1], "tmp421", 0x4c);
> + i2c_slave_create_simple(i2c[2], "intel-me", 0x16);
> i2c_slave_create_simple(i2c[4], "isl69259", 0x76);
> i2c_slave_create_simple(i2c[4], "isl69259", 0x62);
> i2c_slave_create_simple(i2c[4], "isl69259", 0x60);
> diff --git a/hw/misc/intel_me.c b/hw/misc/intel_me.c
> new file mode 100644
> index 0000000000..fdc9180c26
> --- /dev/null
> +++ b/hw/misc/intel_me.c
> @@ -0,0 +1,176 @@
> +/*
> + * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
> + *
> + * 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/main-loop.h"
> +#include "hw/i2c/i2c.h"
> +
> +#define TYPE_INTEL_ME "intel-me"
> +OBJECT_DECLARE_SIMPLE_TYPE(IntelMEState, INTEL_ME);
> +
> +#define printf(...)
> +
> +struct IntelMEState {
> + I2CSlave parent_obj;
> +
> + I2CBus *bus;
> + QEMUBH *bh;
> + int rx_len;
> + int tx_len;
> + int tx_pos;
> + uint8_t rx_buf[512];
> + uint8_t tx_buf[512];
> +};
> +
> +static void intel_me_bh(void *opaque)
> +{
> + IntelMEState *s = opaque;
> +
> + assert(s->bus->bh == s->bh);
> +
> + if (s->tx_pos == 0) {
> + if (i2c_start_send_async(s->bus, s->tx_buf[s->tx_pos++]) != 0) {
> + goto done;
> + }
> + return;
> + }
> +
> + if (s->tx_pos < s->tx_len) {
> + if (i2c_send_async(s->bus, s->tx_buf[s->tx_pos++]) != 0) {
> + goto done;
> + }
> + return;
> + }
> +
> +done:
> + i2c_end_transfer(s->bus);
> + i2c_bus_release(s->bus);
> + s->tx_len = 0;
> + s->tx_pos = 0;
> + memset(s->tx_buf, 0, sizeof(s->tx_buf));
> +}
> +
> +static void intel_me_realize(DeviceState *dev, Error **errp)
> +{
> + IntelMEState *s = INTEL_ME(dev);
> +
> + s->bus = I2C_BUS(qdev_get_parent_bus(dev));
> + s->bh = qemu_bh_new(intel_me_bh, s);
> + s->rx_len = 0;
> + s->tx_len = 0;
> + s->tx_pos = 0;
> + memset(s->rx_buf, 0, sizeof(s->rx_buf));
> + memset(s->tx_buf, 0, sizeof(s->tx_buf));
> +}
> +
> +static uint8_t checksum(const uint8_t *ptr, int len)
> +{
> + int sum = 0;
> +
> + for (int i = 0; i < len; i++) {
> + sum += ptr[i];
> + }
> +
> + return 256 - sum;
> +}
> +
> +static int intel_me_i2c_event(I2CSlave *i2c, enum i2c_event event)
> +{
> + IntelMEState *s = INTEL_ME(i2c);
> +
> + switch (event) {
> + case I2C_START_RECV:
> + break;
> + case I2C_START_SEND:
> + s->rx_len = 0;
> + memset(s->rx_buf, 0, sizeof(s->rx_buf));
> + break;
> + case I2C_START_SEND_ASYNC:
> + break;
> + case I2C_FINISH:
> + printf("IntelME rx: [");
> + for (int i = 0; i < s->rx_len; i++) {
> + if (i) {
> + printf(", ");
> + }
> + printf("0x%02x", s->rx_buf[i]);
> + }
> + printf("]\n");
> +
> + s->tx_len = 10;
> + s->tx_pos = 0;
> + s->tx_buf[0] = s->rx_buf[2];
> + s->tx_buf[1] = ((s->rx_buf[0] >> 2) + 1) << 2;
> + s->tx_buf[2] = 256 - s->tx_buf[0] - s->tx_buf[1];
> + s->tx_buf[3] = i2c->address; // rsSA response Slave Address
> + s->tx_buf[4] = (s->rx_buf[3] >> 2) << 2; // sequence number
> + s->tx_buf[5] = s->rx_buf[4]; // Same command code
> + s->tx_buf[6] = 0x00; // OK
> + s->tx_buf[7] = 0x55; // NO_ERROR
> + s->tx_buf[8] = 0x00;
> + s->tx_buf[9] = checksum(s->tx_buf, s->tx_len - 1);
> + s->tx_buf[0] >>= 1;
> + i2c_bus_master(s->bus, s->bh);
> + break;
> + case I2C_NACK:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static uint8_t intel_me_i2c_recv(I2CSlave *i2c)
> +{
> + return 0xff;
> +}
> +
> +static int intel_me_i2c_send(I2CSlave *i2c, uint8_t data)
> +{
> + IntelMEState *s = INTEL_ME(i2c);
> +
> + assert(s->rx_len < sizeof(s->rx_buf));
> + s->rx_buf[s->rx_len++] = data;
> +
> + return 0;
> +}
> +
> +static void intel_me_class_init(ObjectClass *oc, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(oc);
> + I2CSlaveClass *i2c = I2C_SLAVE_CLASS(oc);
> +
> + dc->realize = intel_me_realize;
> + i2c->event = intel_me_i2c_event;
> + i2c->recv = intel_me_i2c_recv;
> + i2c->send = intel_me_i2c_send;
> +}
> +
> +static const TypeInfo types[] = {
> + {
> + .name = TYPE_INTEL_ME,
> + .parent = TYPE_I2C_SLAVE,
> + .instance_size = sizeof(IntelMEState),
> + .class_init = intel_me_class_init,
> + },
> +};
> +
> +DEFINE_TYPES(types);
> diff --git a/hw/misc/meson.build b/hw/misc/meson.build
> index 1edad44b6b..a2c75894a3 100644
> --- a/hw/misc/meson.build
> +++ b/hw/misc/meson.build
> @@ -118,7 +118,8 @@ softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
> 'aspeed_sdmc.c',
> 'aspeed_xdma.c',
> 'aspeed_peci.c',
> - 'fby35_cpld.c'))
> + 'fby35_cpld.c',
> + 'intel_me.c'))
>
> softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-sysreg.c'))
> softmmu_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_rng.c'))
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/14] aspeed: Add PECI controller
2022-06-28 6:47 ` Cédric Le Goater
@ 2022-06-28 6:58 ` Peter Delevoryas
0 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-28 6:58 UTC (permalink / raw)
Cc: Peter Delevoryas, Dan Zhang, Cameron Esfahani via, qemu-arm,
komlodi, titusr, Andrew Jeffery, Joel Stanley,
Cédric Le Goater
> On Jun 27, 2022, at 11:47 PM, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 6/27/22 21:54, Peter Delevoryas wrote:
>
> Could we have some short intro ? :)
Yes, definitely, I’ll resubmit with some more details.
>
>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>> ---
>> hw/arm/aspeed_ast10x0.c | 11 ++
>> hw/misc/aspeed_peci.c | 225 ++++++++++++++++++++++++++++++++++
>> hw/misc/meson.build | 3 +-
>> include/hw/arm/aspeed_soc.h | 3 +
>> include/hw/misc/aspeed_peci.h | 34 +++++
>> 5 files changed, 275 insertions(+), 1 deletion(-)
>> create mode 100644 hw/misc/aspeed_peci.c
>> create mode 100644 include/hw/misc/aspeed_peci.h
>> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
>> index 5df480a21f..780841ea84 100644
>> --- a/hw/arm/aspeed_ast10x0.c
>> +++ b/hw/arm/aspeed_ast10x0.c
>> @@ -47,6 +47,7 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
>> [ASPEED_DEV_UART13] = 0x7E790700,
>> [ASPEED_DEV_WDT] = 0x7E785000,
>> [ASPEED_DEV_LPC] = 0x7E789000,
>> + [ASPEED_DEV_PECI] = 0x7E78B000,
>> [ASPEED_DEV_I2C] = 0x7E7B0000,
>> };
>> @@ -75,6 +76,7 @@ static const int aspeed_soc_ast1030_irqmap[] = {
>> [ASPEED_DEV_TIMER8] = 23,
>> [ASPEED_DEV_WDT] = 24,
>> [ASPEED_DEV_LPC] = 35,
>> + [ASPEED_DEV_PECI] = 38,
>> [ASPEED_DEV_FMC] = 39,
>> [ASPEED_DEV_PWM] = 44,
>> [ASPEED_DEV_ADC] = 46,
>> @@ -133,6 +135,8 @@ static void aspeed_soc_ast1030_init(Object *obj)
>> object_initialize_child(obj, "lpc", &s->lpc, TYPE_ASPEED_LPC);
>> + object_initialize_child(obj, "peci", &s->peci, TYPE_ASPEED_PECI);
>> +
>> object_initialize_child(obj, "sbc", &s->sbc, TYPE_ASPEED_SBC);
>> for (i = 0; i < sc->wdts_num; i++) {
>> @@ -238,6 +242,13 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>> /* UART */
>> aspeed_soc_uart_init(s);
>> + /* PECI */
>> + if (!sysbus_realize(SYS_BUS_DEVICE(&s->peci), errp)) {
>> + return;
>> + }
>> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->peci), 0, sc->memmap[ASPEED_DEV_PECI]);
>> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->peci), 0, aspeed_soc_get_irq(s, ASPEED_DEV_PECI));
>> +
>> /* Timer */
>> object_property_set_link(OBJECT(&s->timerctrl), "scu", OBJECT(&s->scu),
>> &error_abort);
>> diff --git a/hw/misc/aspeed_peci.c b/hw/misc/aspeed_peci.c
>> new file mode 100644
>> index 0000000000..670e532fc0
>> --- /dev/null
>> +++ b/hw/misc/aspeed_peci.c
>> @@ -0,0 +1,225 @@
>> +/*
>> + * Aspeed PECI Controller
>> + *
>> + * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
>> + *
>> + * This code is licensed under the GPL version 2 or later. See the COPYING
>> + * file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/log.h"
>> +#include "hw/irq.h"
>> +#include "hw/misc/aspeed_peci.h"
>> +
>> +#define U(x) (x##U)
>> +#define GENMASK(h, l) \
>> + (((~U(0)) - (U(1) << (l)) + 1) & \
>> + (~U(0) >> (32 - 1 - (h))))
>
> I beleive QEMU has similar macros to generate masks.
Ah yes good catch, I can probably fix that pretty easily.
>
>
>> +/* ASPEED PECI Registers */
>> +/* Control Register */
>> +#define ASPEED_PECI_CTRL (0x00 / 4)
>> +#define ASPEED_PECI_CTRL_SAMPLING_MASK GENMASK(19, 16)
>> +#define ASPEED_PECI_CTRL_RD_MODE_MASK GENMASK(13, 12)
>> +#define ASPEED_PECI_CTRL_RD_MODE_DBG BIT(13)
>> +#define ASPEED_PECI_CTRL_RD_MODE_COUNT BIT(12)
>> +#define ASPEED_PECI_CTRL_CLK_SRC_HCLK BIT(11)
>> +#define ASPEED_PECI_CTRL_CLK_DIV_MASK GENMASK(10, 8)
>> +#define ASPEED_PECI_CTRL_INVERT_OUT BIT(7)
>> +#define ASPEED_PECI_CTRL_INVERT_IN BIT(6)
>> +#define ASPEED_PECI_CTRL_BUS_CONTENTION_EN BIT(5)
>> +#define ASPEED_PECI_CTRL_PECI_EN BIT(4)
>> +#define ASPEED_PECI_CTRL_PECI_CLK_EN BIT(0)
>> +
>> +/* Timing Negotiation Register */
>> +#define ASPEED_PECI_TIMING_NEGOTIATION (0x04 / 4)
>> +#define ASPEED_PECI_T_NEGO_MSG_MASK GENMASK(15, 8)
>> +#define ASPEED_PECI_T_NEGO_ADDR_MASK GENMASK(7, 0)
>> +
>> +/* Command Register */
>> +#define ASPEED_PECI_CMD (0x08 / 4)
>> +#define ASPEED_PECI_CMD_PIN_MONITORING BIT(31)
>> +#define ASPEED_PECI_CMD_STS_MASK GENMASK(27, 24)
>> +#define ASPEED_PECI_CMD_STS_ADDR_T_NEGO 0x3
>> +#define ASPEED_PECI_CMD_IDLE_MASK \
>> + (ASPEED_PECI_CMD_STS_MASK | ASPEED_PECI_CMD_PIN_MONITORING)
>> +#define ASPEED_PECI_CMD_FIRE BIT(0)
>> +
>> +/* Read/Write Length Register */
>> +#define ASPEED_PECI_RW_LENGTH (0x0c / 4)
>> +#define ASPEED_PECI_AW_FCS_EN BIT(31)
>> +#define ASPEED_PECI_RD_LEN_MASK GENMASK(23, 16)
>> +#define ASPEED_PECI_WR_LEN_MASK GENMASK(15, 8)
>> +#define ASPEED_PECI_TARGET_ADDR_MASK GENMASK(7, 0)
>> +
>> +/* Expected FCS Data Register */
>> +#define ASPEED_PECI_EXPECTED_FCS (0x10 / 4)
>> +#define ASPEED_PECI_EXPECTED_RD_FCS_MASK GENMASK(23, 16)
>> +#define ASPEED_PECI_EXPECTED_AW_FCS_AUTO_MASK GENMASK(15, 8)
>> +#define ASPEED_PECI_EXPECTED_WR_FCS_MASK GENMASK(7, 0)
>> +
>> +/* Captured FCS Data Register */
>> +#define ASPEED_PECI_CAPTURED_FCS (0x14 / 4)
>> +#define ASPEED_PECI_CAPTURED_RD_FCS_MASK GENMASK(23, 16)
>> +#define ASPEED_PECI_CAPTURED_WR_FCS_MASK GENMASK(7, 0)
>> +
>> +/* Interrupt Register */
>> +#define ASPEED_PECI_INT_CTRL (0x18 / 4)
>> +#define ASPEED_PECI_TIMING_NEGO_SEL_MASK GENMASK(31, 30)
>> +#define ASPEED_PECI_1ST_BIT_OF_ADDR_NEGO 0
>> +#define ASPEED_PECI_2ND_BIT_OF_ADDR_NEGO 1
>> +#define ASPEED_PECI_MESSAGE_NEGO 2
>> +#define ASPEED_PECI_INT_MASK GENMASK(4, 0)
>> +#define ASPEED_PECI_INT_BUS_TIMEOUT BIT(4)
>> +#define ASPEED_PECI_INT_BUS_CONTENTION BIT(3)
>> +#define ASPEED_PECI_INT_WR_FCS_BAD BIT(2)
>> +#define ASPEED_PECI_INT_WR_FCS_ABORT BIT(1)
>> +#define ASPEED_PECI_INT_CMD_DONE BIT(0)
>> +
>> +/* Interrupt Status Register */
>> +#define ASPEED_PECI_INT_STS (0x1c / 4)
>> +#define ASPEED_PECI_INT_TIMING_RESULT_MASK GENMASK(29, 16)
>> + /* bits[4..0]: Same bit fields in the 'Interrupt Register' */
>> +
>> +/* Rx/Tx Data Buffer Registers */
>> +#define ASPEED_PECI_WR_DATA0 (0x20 / 4)
>> +#define ASPEED_PECI_WR_DATA1 (0x24 / 4)
>> +#define ASPEED_PECI_WR_DATA2 (0x28 / 4)
>> +#define ASPEED_PECI_WR_DATA3 (0x2c / 4)
>> +#define ASPEED_PECI_RD_DATA0 (0x30 / 4)
>> +#define ASPEED_PECI_RD_DATA1 (0x34 / 4)
>> +#define ASPEED_PECI_RD_DATA2 (0x38 / 4)
>> +#define ASPEED_PECI_RD_DATA3 (0x3c / 4)
>> +#define ASPEED_PECI_WR_DATA4 (0x40 / 4)
>> +#define ASPEED_PECI_WR_DATA5 (0x44 / 4)
>> +#define ASPEED_PECI_WR_DATA6 (0x48 / 4)
>> +#define ASPEED_PECI_WR_DATA7 (0x4c / 4)
>> +#define ASPEED_PECI_RD_DATA4 (0x50 / 4)
>> +#define ASPEED_PECI_RD_DATA5 (0x54 / 4)
>> +#define ASPEED_PECI_RD_DATA6 (0x58 / 4)
>> +#define ASPEED_PECI_RD_DATA7 (0x5c / 4)
>> +#define ASPEED_PECI_DATA_BUF_SIZE_MAX 32
>> +
>> +/** PECI read/write supported responses */
>> +#define PECI_CC_RSP_SUCCESS (0x40U)
>> +#define PECI_CC_RSP_TIMEOUT (0x80U)
>> +#define PECI_CC_OUT_OF_RESOURCES_TIMEOUT (0x81U)
>> +#define PECI_CC_RESOURCES_LOWPWR_TIMEOUT (0x82U)
>> +#define PECI_CC_ILLEGAL_REQUEST (0x90U)
>> +
>> +static void aspeed_peci_instance_init(Object *obj)
>> +{
>> +}
>
> May be drop this routine if it is empty.
Agreed, will do.
>
>> +static uint64_t aspeed_peci_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> + AspeedPECIState *s = ASPEED_PECI(opaque);
>> +
>> + if (addr >= ASPEED_PECI_NR_REGS << 2) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx "\n",
>> + __func__, addr);
>> + return 0;
>> + }
>> + addr >>= 2;
>> +
>> + uint32_t reg = s->regs[addr];
>> + //printf("%s: 0x%08lx 0x%08x %u\n", __func__, addr << 2, reg, size);
>
> Convert to trace event ? or please remove.
Arg yes, I’ll fix this.
>
>> +
>> + return reg;
>> +}
>> +
>> +static void aspeed_peci_write(void *opaque, hwaddr addr, uint64_t data, unsigned size)
>> +{
>> + AspeedPECIState *s = ASPEED_PECI(opaque);
>> +
>> + //printf("%s: 0x%08lx 0x%08x %u\n", __func__, addr, reg, size);
>> +
>> + if (addr >= ASPEED_PECI_NR_REGS << 2) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n",
>> + __func__, addr);
>> + return;
>> + }
>> + addr >>= 2;
>> +
>> + switch (addr) {
>> + case ASPEED_PECI_INT_STS:
>> + s->regs[addr] &= ~data;
>> + break;
>> + default:
>> + s->regs[addr] = data;
>> + break;
>> + }
>> +
>> + switch (addr) {
>> + case ASPEED_PECI_CMD:
>> + if (!(s->regs[ASPEED_PECI_CMD] & ASPEED_PECI_CMD_FIRE)) {
>> + break;
>> + }
>> + s->regs[ASPEED_PECI_RD_DATA0] = PECI_CC_RSP_SUCCESS;
>> + s->regs[ASPEED_PECI_WR_DATA0] = PECI_CC_RSP_SUCCESS;
>> +
>> + s->regs[ASPEED_PECI_INT_STS] |= ASPEED_PECI_INT_CMD_DONE;
>> + qemu_irq_raise(s->irq);
>> + break;
>> + case ASPEED_PECI_INT_STS:
>> + if (s->regs[ASPEED_PECI_INT_STS]) {
>> + break;
>> + }
>> + qemu_irq_lower(s->irq);
>> + break;
>> + default:
>> + qemu_log_mask(LOG_UNIMP, "%s: register 0x%03" HWADDR_PRIx " writes unimplemented\n",
>> + __func__, addr);
>> + break;
>> + }
>> +}
>> +
>> +static const MemoryRegionOps aspeed_peci_ops = {
>> + .read = aspeed_peci_read,
>> + .write = aspeed_peci_write,
>> + .endianness = DEVICE_LITTLE_ENDIAN,
>> +};
>> +
>> +static void aspeed_peci_realize(DeviceState *dev, Error **errp)
>> +{
>> + AspeedPECIState *s = ASPEED_PECI(dev);
>> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> +
>> + memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_peci_ops, s, TYPE_ASPEED_PECI, 0x1000);
>> + sysbus_init_mmio(sbd, &s->mmio);
>> + sysbus_init_irq(sbd, &s->irq);
>> +}
>> +
>> +static void aspeed_peci_reset(DeviceState *dev)
>> +{
>> + AspeedPECIState *s = ASPEED_PECI(dev);
>> +
>> + memset(s->regs, 0, sizeof(s->regs));
>> +}
>> +
>> +static void aspeed_peci_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> + dc->realize = aspeed_peci_realize;
>> + dc->reset = aspeed_peci_reset;
>> + dc->desc = "Aspeed PECI Controller";
>> +}
>> +
>> +static const TypeInfo aspeed_peci_info = {
>> + .name = TYPE_ASPEED_PECI,
>> + .parent = TYPE_SYS_BUS_DEVICE,
>> + .instance_init = aspeed_peci_instance_init,
>> + .instance_size = sizeof(AspeedPECIState),
>> + .class_init = aspeed_peci_class_init,
>> + .class_size = sizeof(AspeedPECIClass),
>> + .abstract = false,
>> +};
>> +
>> +static void aspeed_peci_register_types(void)
>> +{
>> + type_register_static(&aspeed_peci_info);
>> +}
>> +
>> +type_init(aspeed_peci_register_types);
>> diff --git a/hw/misc/meson.build b/hw/misc/meson.build
>> index 132b7b7344..95268eddc0 100644
>> --- a/hw/misc/meson.build
>> +++ b/hw/misc/meson.build
>> @@ -116,7 +116,8 @@ softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
>> 'aspeed_scu.c',
>> 'aspeed_sbc.c',
>> 'aspeed_sdmc.c',
>> - 'aspeed_xdma.c'))
>> + 'aspeed_xdma.c',
>> + 'aspeed_peci.c'))
>> softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-sysreg.c'))
>> softmmu_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_rng.c'))
>> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
>> index 02a5a9ffcb..fd2aa1880a 100644
>> --- a/include/hw/arm/aspeed_soc.h
>> +++ b/include/hw/arm/aspeed_soc.h
>> @@ -34,6 +34,7 @@
>> #include "hw/usb/hcd-ehci.h"
>> #include "qom/object.h"
>> #include "hw/misc/aspeed_lpc.h"
>> +#include "hw/misc/aspeed_peci.h"
>> #define ASPEED_SPIS_NUM 2
>> #define ASPEED_EHCIS_NUM 2
>> @@ -73,6 +74,7 @@ struct AspeedSoCState {
>> AspeedSDHCIState sdhci;
>> AspeedSDHCIState emmc;
>> AspeedLPCState lpc;
>> + AspeedPECIState peci;
>> uint32_t uart_default;
>> Clock *sysclk;
>> };
>> @@ -161,6 +163,7 @@ enum {
>> ASPEED_DEV_DPMCU,
>> ASPEED_DEV_DP,
>> ASPEED_DEV_I3C,
>> + ASPEED_DEV_PECI,
>> };
>> qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev);
>> diff --git a/include/hw/misc/aspeed_peci.h b/include/hw/misc/aspeed_peci.h
>> new file mode 100644
>> index 0000000000..81c7d31700
>> --- /dev/null
>> +++ b/include/hw/misc/aspeed_peci.h
>> @@ -0,0 +1,34 @@
>> +/*
>> + * Aspeed PECI Controller
>> + *
>> + * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
>> + *
>> + * This code is licensed under the GPL version 2 or later. See the COPYING
>> + * file in the top-level directory.
>> + */
>> +
>> +#ifndef ASPEED_PECI_H
>> +#define ASPEED_PECI_H
>> +
>> +#include "hw/sysbus.h"
>> +
>> +#define TYPE_ASPEED_PECI "aspeed.peci"
>> +OBJECT_DECLARE_TYPE(AspeedPECIState, AspeedPECIClass, ASPEED_PECI);
>> +
>> +#define ASPEED_PECI_NR_REGS ((0xFC + 4) >> 2)
>> +
>> +struct AspeedPECIState {
>> + /* <private> */
>> + SysBusDevice parent;
>> +
>> + MemoryRegion mmio;
>> + qemu_irq irq;
>> +
>> + uint32_t regs[ASPEED_PECI_NR_REGS];
>> +};
>> +
>> +struct AspeedPECIClass {
>> + SysBusDeviceClass parent_class;
>> +};
>> +
>> +#endif
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 08/14] hw/misc: Add fby35-cpld
2022-06-28 6:50 ` Cédric Le Goater
@ 2022-06-28 7:00 ` Peter Delevoryas
0 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-28 7:00 UTC (permalink / raw)
Cc: Peter Delevoryas, Dan Zhang, Cameron Esfahani via, qemu-arm,
komlodi, titusr, Andrew Jeffery, Joel Stanley,
Cédric Le Goater
> On Jun 27, 2022, at 11:50 PM, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 6/27/22 21:55, Peter Delevoryas wrote:
>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>
>
> some intro ?
Will do.
>
>> ---
>> hw/misc/fby35_cpld.c | 137 +++++++++++++++++++++++++++++++++++++++++++
>> hw/misc/meson.build | 3 +-
>> 2 files changed, 139 insertions(+), 1 deletion(-)
>> create mode 100644 hw/misc/fby35_cpld.c
>> diff --git a/hw/misc/fby35_cpld.c b/hw/misc/fby35_cpld.c
>> new file mode 100644
>> index 0000000000..a783a0a2c8
>> --- /dev/null
>> +++ b/hw/misc/fby35_cpld.c
>> @@ -0,0 +1,137 @@
>> +/*
>> + * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
>> + *
>> + * 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.
>> + */
>
> I prefer the short version of the license.
Oh, yes, I can see if I can reduce this. To be honest I was copying this
from our legal wiki, and I’m not that familiar with licensing or what
shortening it implies, but I can look into that.
>
>> +#include "qemu/osdep.h"
>> +#include "hw/i2c/i2c.h"
>> +#include "hw/registerfields.h"
>> +
>> +#define BOARD_ID_CLASS1 0b0000
>> +#define BOARD_ID_CLASS2 0b0001
>> +
>> +#define TYPE_FBY35_CPLD "fby35-cpld"
>> +OBJECT_DECLARE_SIMPLE_TYPE(Fby35CpldState, FBY35_CPLD);
>> +
>> +REG8(CLASS_TYPE, 0x5);
>> + FIELD(CLASS_TYPE, RESERVED, 0, 2);
>> + FIELD(CLASS_TYPE, 1OU_EXPANSION_NOT_PRESENT, 2, 1);
>> + FIELD(CLASS_TYPE, 2OU_EXPANSION_NOT_PRESENT, 3, 1);
>> + FIELD(CLASS_TYPE, BOARD_ID, 4, 4);
>> +REG8(BOARD_REVISION, 0x8);
>> + FIELD(BOARD_REVISION, VALUE, 0, 4);
>> + FIELD(BOARD_REVISION, RESERVED, 4, 4);
>> +
>> +struct Fby35CpldState {
>> + I2CSlave parent_obj;
>> +
>> + uint8_t target_reg;
>> + uint32_t regs[10];
>> +};
>> +
>> +static void fby35_cpld_realize(DeviceState *dev, Error **errp)
>> +{
>> + Fby35CpldState *s = FBY35_CPLD(dev);
>> +
>> + memset(s->regs, 0, sizeof(s->regs));
>> + s->target_reg = 0;
>> +
>> + ARRAY_FIELD_DP32(s->regs, CLASS_TYPE, BOARD_ID, 0b0000);
>> + ARRAY_FIELD_DP32(s->regs, CLASS_TYPE, 1OU_EXPANSION_NOT_PRESENT, 1);
>> + ARRAY_FIELD_DP32(s->regs, CLASS_TYPE, 2OU_EXPANSION_NOT_PRESENT, 1);
>> + ARRAY_FIELD_DP32(s->regs, BOARD_REVISION, VALUE, 0x1);
>> +}
>> +
>> +static int fby35_cpld_i2c_event(I2CSlave *i2c, enum i2c_event event)
>> +{
>> + Fby35CpldState *s = FBY35_CPLD(i2c);
>> +
>> + switch (event) {
>> + case I2C_START_RECV:
>> + break;
>> + case I2C_START_SEND:
>> + s->target_reg = 0;
>> + break;
>> + case I2C_START_SEND_ASYNC:
>> + case I2C_FINISH:
>> + case I2C_NACK:
>> + break;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static uint8_t fby35_cpld_i2c_recv(I2CSlave *i2c)
>> +{
>> + Fby35CpldState *s = FBY35_CPLD(i2c);
>> +
>> + switch (s->target_reg) {
>> + case R_CLASS_TYPE:
>> + case R_BOARD_REVISION:
>> + return s->regs[s->target_reg];
>> + default:
>> + printf("%s: unexpected register read 0x%02x\n", __func__, s->target_reg);
>
> please use the qemu logging system
Yes, sorry about that, I’ll fix this. In the future I’ll probably
try to just use qemu_log from the start, to avoid these.
>
>> + return 0xff;
>> + }
>> +}
>> +
>> +static int fby35_cpld_i2c_send(I2CSlave *i2c, uint8_t data)
>> +{
>> + Fby35CpldState *s = FBY35_CPLD(i2c);
>> +
>> + if (s->target_reg == 0) {
>> + s->target_reg = data;
>> + return 0;
>> + }
>> +
>> + switch (s->target_reg) {
>> + case R_CLASS_TYPE:
>> + case R_BOARD_REVISION:
>> + s->regs[s->target_reg] = data;
>> + break;
>> + default:
>> + printf("%s: unexpected register write 0x%02x 0x%02x\n", __func__, s->target_reg, data);
>> + break;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void fby35_cpld_class_init(ObjectClass *oc, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(oc);
>> + I2CSlaveClass *i2c = I2C_SLAVE_CLASS(oc);
>> +
>> + dc->realize = fby35_cpld_realize;
>> + i2c->event = fby35_cpld_i2c_event;
>> + i2c->recv = fby35_cpld_i2c_recv;
>> + i2c->send = fby35_cpld_i2c_send;
>> +}
>> +
>> +static const TypeInfo types[] = {
>> + {
>> + .name = TYPE_FBY35_CPLD,
>> + .parent = TYPE_I2C_SLAVE,
>> + .instance_size = sizeof(Fby35CpldState),
>> + .class_init = fby35_cpld_class_init,
>> + },
>> +};
>> +
>> +DEFINE_TYPES(types);
>> diff --git a/hw/misc/meson.build b/hw/misc/meson.build
>> index 95268eddc0..1edad44b6b 100644
>> --- a/hw/misc/meson.build
>> +++ b/hw/misc/meson.build
>> @@ -117,7 +117,8 @@ softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
>> 'aspeed_sbc.c',
>> 'aspeed_sdmc.c',
>> 'aspeed_xdma.c',
>> - 'aspeed_peci.c'))
>> + 'aspeed_peci.c',
>> + 'fby35_cpld.c'))
>> softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-sysreg.c'))
>> softmmu_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_rng.c'))
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 04/14] aspeed: i2c: Fix DMA len write-enable bit handling
2022-06-27 19:54 ` [PATCH 04/14] aspeed: i2c: Fix DMA len write-enable bit handling Peter Delevoryas
@ 2022-06-28 7:01 ` Cédric Le Goater
2022-06-28 7:05 ` Peter Delevoryas
0 siblings, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2022-06-28 7:01 UTC (permalink / raw)
To: Peter Delevoryas
Cc: zhdaniel, qemu-devel, qemu-arm, komlodi, titusr, andrew, joel
On 6/27/22 21:54, Peter Delevoryas wrote:
> I noticed i2c rx transfers were getting shortened to "1" on Zephyr. It
> seems to be because the Zephyr i2c driver sets the RX DMA len with the
> RX field write-enable bit set (bit 31) to avoid a read-modify-write. [1]
>
> /* 0x1C : I2CM Master DMA Transfer Length Register */
>
> I think we should be checking the write-enable bits on the incoming
> value, not checking the register array. I'm not sure we're even writing
> the write-enable bits to the register array, actually.
>
> [1] https://github.com/AspeedTech-BMC/zephyr/blob/db3dbcc9c52e67a47180890ac938ed380b33f91c/drivers/i2c/i2c_aspeed.c#L145-L148
>
> Fixes: ba2cccd64e90f34 ("aspeed: i2c: Add new mode support")
> Signed-off-by: Peter Delevoryas <pdel@fb.com>
> ---
Nice. Can you move this patch at beginning of the series ?
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> hw/i2c/aspeed_i2c.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 2cfd05cb6c..6c8222717f 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -644,18 +644,18 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
> RX_BUF_LEN) + 1;
> break;
> case A_I2CM_DMA_LEN:
> - w1t = ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, RX_BUF_LEN_W1T) ||
> - ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, TX_BUF_LEN_W1T);
> + w1t = FIELD_EX32(value, I2CM_DMA_LEN, RX_BUF_LEN_W1T) ||
> + FIELD_EX32(value, I2CM_DMA_LEN, TX_BUF_LEN_W1T);
> /* If none of the w1t bits are set, just write to the reg as normal. */
> if (!w1t) {
> bus->regs[R_I2CM_DMA_LEN] = value;
> break;
> }
> - if (ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, RX_BUF_LEN_W1T)) {
> + if (FIELD_EX32(value, I2CM_DMA_LEN, RX_BUF_LEN_W1T)) {
> ARRAY_FIELD_DP32(bus->regs, I2CM_DMA_LEN, RX_BUF_LEN,
> FIELD_EX32(value, I2CM_DMA_LEN, RX_BUF_LEN));
> }
> - if (ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, TX_BUF_LEN_W1T)) {
> + if (FIELD_EX32(value, I2CM_DMA_LEN, TX_BUF_LEN_W1T)) {
> ARRAY_FIELD_DP32(bus->regs, I2CM_DMA_LEN, TX_BUF_LEN,
> FIELD_EX32(value, I2CM_DMA_LEN, TX_BUF_LEN));
> }
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 05/14] aspeed: i2c: Fix R_I2CD_FUN_CTRL reference
2022-06-27 19:54 ` [PATCH 05/14] aspeed: i2c: Fix R_I2CD_FUN_CTRL reference Peter Delevoryas
@ 2022-06-28 7:01 ` Cédric Le Goater
2022-06-28 7:05 ` Peter Delevoryas
0 siblings, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2022-06-28 7:01 UTC (permalink / raw)
To: Peter Delevoryas
Cc: zhdaniel, qemu-devel, qemu-arm, komlodi, titusr, andrew, joel
On 6/27/22 21:54, Peter Delevoryas wrote:
> Very minor, doesn't effect functionality, but this is supposed to be
> R_I2CC_FUN_CTRL (new-mode, not old-mode).
>
> Fixes: ba2cccd64e9 ("aspeed: i2c: Add new mode support")
> Signed-off-by: Peter Delevoryas <pdel@fb.com>
> ---
Can you move this patch at beginning of the series ?
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> hw/i2c/aspeed_i2c.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 6c8222717f..6abd9b033e 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -552,7 +552,7 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
> __func__);
> break;
> }
> - bus->regs[R_I2CD_FUN_CTRL] = value & 0x007dc3ff;
> + bus->regs[R_I2CC_FUN_CTRL] = value & 0x007dc3ff;
> break;
> case A_I2CC_AC_TIMING:
> bus->regs[R_I2CC_AC_TIMING] = value & 0x1ffff0ff;
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 14/14] aspeed: Add I2C new register DMA slave mode support
2022-06-27 22:27 ` [PATCH 14/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
@ 2022-06-28 7:02 ` Cédric Le Goater
2022-06-28 7:06 ` Peter Delevoryas
0 siblings, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2022-06-28 7:02 UTC (permalink / raw)
To: Peter Delevoryas; +Cc: qemu-devel, qemu-arm
On 6/28/22 00:27, Peter Delevoryas wrote:
> Signed-off-by: Peter Delevoryas <pdel@fb.com>
Some Intro would be welcome. Please move the patch after Klaus patches.
Thanks,
C.
> ---
> hw/i2c/aspeed_i2c.c | 133 ++++++++++++++++++++++++++++++++----
> include/hw/i2c/aspeed_i2c.h | 3 +
> 2 files changed, 124 insertions(+), 12 deletions(-)
>
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 8a8514586f..fc8b6b62cf 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -78,6 +78,18 @@ static inline void aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus)
> }
> }
>
> +static inline void aspeed_i2c_bus_raise_slave_interrupt(AspeedI2CBus *bus)
> +{
> + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
> +
> + if (!bus->regs[R_I2CS_INTR_STS]) {
> + return;
> + }
> +
> + bus->controller->intr_status |= 1 << bus->id;
> + qemu_irq_raise(aic->bus_get_irq(bus));
> +}
> +
> static uint64_t aspeed_i2c_bus_old_read(AspeedI2CBus *bus, hwaddr offset,
> unsigned size)
> {
> @@ -140,8 +152,17 @@ static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset,
> case A_I2CM_DMA_LEN_STS:
> case A_I2CC_DMA_ADDR:
> case A_I2CC_DMA_LEN:
> +
> + case A_I2CS_DEV_ADDR:
> + case A_I2CS_DMA_RX_ADDR:
> + case A_I2CS_DMA_LEN:
> + case A_I2CS_CMD:
> + case A_I2CS_INTR_CTRL:
> + case A_I2CS_DMA_LEN_STS:
> /* Value is already set, don't do anything. */
> break;
> + case A_I2CS_INTR_STS:
> + break;
> case A_I2CM_CMD:
> value = SHARED_FIELD_DP32(value, BUS_BUSY_STS, i2c_bus_busy(bus->bus));
> break;
> @@ -547,12 +568,7 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>
> switch (offset) {
> case A_I2CC_FUN_CTRL:
> - if (SHARED_FIELD_EX32(value, SLAVE_EN)) {
> - qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
> - __func__);
> - break;
> - }
> - bus->regs[R_I2CC_FUN_CTRL] = value & 0x007dc3ff;
> + bus->regs[R_I2CC_FUN_CTRL] = value;
> break;
> case A_I2CC_AC_TIMING:
> bus->regs[R_I2CC_AC_TIMING] = value & 0x1ffff0ff;
> @@ -580,6 +596,7 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
> bus->controller->intr_status &= ~(1 << bus->id);
> qemu_irq_lower(aic->bus_get_irq(bus));
> }
> + aspeed_i2c_bus_raise_slave_interrupt(bus);
> break;
> }
> bus->regs[R_I2CM_INTR_STS] &= ~(value & 0xf007f07f);
> @@ -668,15 +685,53 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
> case A_I2CC_DMA_LEN:
> /* RO */
> break;
> - case A_I2CS_DMA_LEN_STS:
> - case A_I2CS_DMA_TX_ADDR:
> - case A_I2CS_DMA_RX_ADDR:
> case A_I2CS_DEV_ADDR:
> + bus->regs[R_I2CS_DEV_ADDR] = value;
> + break;
> + case A_I2CS_DMA_RX_ADDR:
> + bus->regs[R_I2CS_DMA_RX_ADDR] = value;
> + break;
> + case A_I2CS_DMA_LEN:
> + assert(FIELD_EX32(value, I2CS_DMA_LEN, TX_BUF_LEN) == 0);
> + if (FIELD_EX32(value, I2CS_DMA_LEN, RX_BUF_LEN_W1T)) {
> + ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN, RX_BUF_LEN,
> + FIELD_EX32(value, I2CS_DMA_LEN, RX_BUF_LEN));
> + } else {
> + bus->regs[R_I2CS_DMA_LEN] = value;
> + }
> + break;
> + case A_I2CS_CMD:
> + if (FIELD_EX32(value, I2CS_CMD, W1_CTRL)) {
> + bus->regs[R_I2CS_CMD] |= value;
> + } else {
> + bus->regs[R_I2CS_CMD] = value;
> + }
> + i2c_slave_set_address(bus->slave, bus->regs[R_I2CS_DEV_ADDR]);
> + break;
> case A_I2CS_INTR_CTRL:
> + bus->regs[R_I2CS_INTR_CTRL] = value;
> + break;
> +
> case A_I2CS_INTR_STS:
> - case A_I2CS_CMD:
> - case A_I2CS_DMA_LEN:
> - qemu_log_mask(LOG_UNIMP, "%s: Slave mode is not implemented\n",
> + if (ARRAY_FIELD_EX32(bus->regs, I2CS_INTR_CTRL, PKT_CMD_DONE)) {
> + if (ARRAY_FIELD_EX32(bus->regs, I2CS_INTR_STS, PKT_CMD_DONE) &&
> + FIELD_EX32(value, I2CS_INTR_STS, PKT_CMD_DONE)) {
> + bus->regs[R_I2CS_INTR_STS] &= 0xfffc0000;
> + }
> + } else {
> + bus->regs[R_I2CS_INTR_STS] &= ~value;
> + }
> + if (!bus->regs[R_I2CS_INTR_STS]) {
> + bus->controller->intr_status &= ~(1 << bus->id);
> + qemu_irq_lower(aic->bus_get_irq(bus));
> + }
> + aspeed_i2c_bus_raise_interrupt(bus);
> + break;
> + case A_I2CS_DMA_LEN_STS:
> + bus->regs[R_I2CS_DMA_LEN_STS] = 0;
> + break;
> + case A_I2CS_DMA_TX_ADDR:
> + qemu_log_mask(LOG_UNIMP, "%s: Slave mode DMA TX is not implemented\n",
> __func__);
> break;
> default:
> @@ -1037,6 +1092,39 @@ static const TypeInfo aspeed_i2c_info = {
> .abstract = true,
> };
>
> +static int aspeed_i2c_bus_new_slave_event(AspeedI2CBus *bus,
> + enum i2c_event event)
> +{
> + switch (event) {
> + case I2C_START_SEND_ASYNC:
> + if (!SHARED_ARRAY_FIELD_EX32(bus->regs, R_I2CS_CMD, RX_DMA_EN)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Slave mode RX DMA is not enabled\n", __func__);
> + return -1;
> + }
> + ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN, 0);
> + bus->regs[R_I2CC_DMA_ADDR] =
> + ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_RX_ADDR, ADDR);
> + bus->regs[R_I2CC_DMA_LEN] =
> + ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_LEN, RX_BUF_LEN) + 1;
> + i2c_ack(bus->bus);
> + break;
> + case I2C_FINISH:
> + ARRAY_FIELD_DP32(bus->regs, I2CS_INTR_STS, PKT_CMD_DONE, 1);
> + ARRAY_FIELD_DP32(bus->regs, I2CS_INTR_STS, SLAVE_ADDR_RX_MATCH, 1);
> + SHARED_ARRAY_FIELD_DP32(bus->regs, R_I2CS_INTR_STS, NORMAL_STOP, 1);
> + SHARED_ARRAY_FIELD_DP32(bus->regs, R_I2CS_INTR_STS, RX_DONE, 1);
> + aspeed_i2c_bus_raise_slave_interrupt(bus);
> + break;
> + default:
> + qemu_log_mask(LOG_UNIMP, "%s: i2c event %d unimplemented\n",
> + __func__, event);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> static int aspeed_i2c_bus_slave_event(I2CSlave *slave, enum i2c_event event)
> {
> BusState *qbus = qdev_get_parent_bus(DEVICE(slave));
> @@ -1045,6 +1133,10 @@ static int aspeed_i2c_bus_slave_event(I2CSlave *slave, enum i2c_event event)
> uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus);
> uint32_t value;
>
> + if (aspeed_i2c_is_new_mode(bus->controller)) {
> + return aspeed_i2c_bus_new_slave_event(bus, event);
> + }
> +
> switch (event) {
> case I2C_START_SEND_ASYNC:
> value = SHARED_ARRAY_FIELD_EX32(bus->regs, reg_byte_buf, TX_BUF);
> @@ -1073,6 +1165,19 @@ static int aspeed_i2c_bus_slave_event(I2CSlave *slave, enum i2c_event event)
> return 0;
> }
>
> +static void aspeed_i2c_bus_new_slave_send_async(AspeedI2CBus *bus, uint8_t data)
> +{
> + assert(address_space_write(&bus->controller->dram_as,
> + bus->regs[R_I2CC_DMA_ADDR],
> + MEMTXATTRS_UNSPECIFIED, &data, 1) == MEMTX_OK);
> +
> + bus->regs[R_I2CC_DMA_ADDR]++;
> + bus->regs[R_I2CC_DMA_LEN]--;
> + ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN,
> + ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN) + 1);
> + i2c_ack(bus->bus);
> +}
> +
> static void aspeed_i2c_bus_slave_send_async(I2CSlave *slave, uint8_t data)
> {
> BusState *qbus = qdev_get_parent_bus(DEVICE(slave));
> @@ -1080,6 +1185,10 @@ static void aspeed_i2c_bus_slave_send_async(I2CSlave *slave, uint8_t data)
> uint32_t reg_intr_sts = aspeed_i2c_bus_intr_sts_offset(bus);
> uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus);
>
> + if (aspeed_i2c_is_new_mode(bus->controller)) {
> + return aspeed_i2c_bus_new_slave_send_async(bus, data);
> + }
> +
> SHARED_ARRAY_FIELD_DP32(bus->regs, reg_byte_buf, RX_BUF, data);
> SHARED_ARRAY_FIELD_DP32(bus->regs, reg_intr_sts, RX_DONE, 1);
>
> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
> index ba148b2f6d..300a89b343 100644
> --- a/include/hw/i2c/aspeed_i2c.h
> +++ b/include/hw/i2c/aspeed_i2c.h
> @@ -174,6 +174,8 @@ REG32(I2CM_DMA_LEN, 0x1c)
> FIELD(I2CM_DMA_LEN, TX_BUF_LEN_W1T, 15, 1)
> FIELD(I2CM_DMA_LEN, TX_BUF_LEN, 0, 11)
> REG32(I2CS_INTR_CTRL, 0x20)
> + FIELD(I2CS_INTR_CTRL, PKT_CMD_FAIL, 17, 1)
> + FIELD(I2CS_INTR_CTRL, PKT_CMD_DONE, 16, 1)
> REG32(I2CS_INTR_STS, 0x24)
> /* 31:29 shared with I2CD_INTR_STS[31:29] */
> FIELD(I2CS_INTR_STS, SLAVE_PARKING_STS, 24, 2)
> @@ -184,6 +186,7 @@ REG32(I2CS_INTR_STS, 0x24)
> FIELD(I2CS_INTR_STS, PKT_CMD_FAIL, 17, 1)
> FIELD(I2CS_INTR_STS, PKT_CMD_DONE, 16, 1)
> /* 14:0 shared with I2CD_INTR_STS[14:0] */
> + FIELD(I2CS_INTR_STS, SLAVE_ADDR_RX_MATCH, 7, 1)
> REG32(I2CS_CMD, 0x28)
> FIELD(I2CS_CMD, W1_CTRL, 31, 1)
> FIELD(I2CS_CMD, PKT_MODE_ACTIVE_ADDR, 17, 2)
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 09/14] pmbus: Reset out buf after switching pages
2022-06-28 6:51 ` Cédric Le Goater
@ 2022-06-28 7:04 ` Peter Delevoryas
0 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-28 7:04 UTC (permalink / raw)
Cc: Peter Delevoryas, Dan Zhang, Cameron Esfahani via, qemu-arm,
komlodi, titusr, Andrew Jeffery, Joel Stanley,
Cédric Le Goater
> On Jun 27, 2022, at 11:51 PM, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 6/27/22 21:55, Peter Delevoryas wrote:
>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>
> is that a bug ?
I believe so yes, although I don’t really have any experience with
real pmbus devices. But, I would assume that when you’re switching
pages, you wouldn’t retain any remaining data from the transfer
buffer of the previous page, because that would return data
from the previous page.
Here’s the tag I should have included:
Fixes: 3746d5c15e70570b ("hw/i2c: add support for PMBus”)
>
>
>> ---
>> hw/i2c/pmbus_device.c | 1 +
>> 1 file changed, 1 insertion(+)
>> diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
>> index 62885fa6a1..efddc36fd9 100644
>> --- a/hw/i2c/pmbus_device.c
>> +++ b/hw/i2c/pmbus_device.c
>> @@ -1088,6 +1088,7 @@ static int pmbus_write_data(SMBusDevice *smd, uint8_t *buf, uint8_t len)
>> if (pmdev->code == PMBUS_PAGE) {
>> pmdev->page = pmbus_receive8(pmdev);
>> + pmdev->out_buf_len = 0;
>> return 0;
>> }
>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support
2022-06-27 19:54 [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
` (11 preceding siblings ...)
2022-06-27 22:27 ` [PATCH 12/14] hw/misc: Add intel-me Peter Delevoryas
@ 2022-06-28 7:05 ` Cédric Le Goater
2022-06-28 7:16 ` Peter Delevoryas
12 siblings, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2022-06-28 7:05 UTC (permalink / raw)
To: Peter Delevoryas
Cc: zhdaniel, qemu-devel, qemu-arm, komlodi, titusr, andrew, joel
On 6/27/22 21:54, Peter Delevoryas wrote:
> Hey everyone,
>
> I'm sending a big patch series for this, but only the last commit is really
> intended to be accepted right now. I'm just including the rest of them
> because it depends on them for testing.
>
> Klaus's changes include the multi-master stuff in hw/i2c/core.c, and the old
> register mode slave mode support.
>
> My series of patches includes a bunch of changes to eliminate most (if not
> all) of the I2C errors reported by the fb OpenBIC firmware for fby35
> CraterLake Bridge Interconnect (BIC) (shortname: oby35-cl) upon startup.
>
> In particular, I needed to add the IC_DEVICE_ID to the isl voltage regulator
> implementation, which required a fix to the pmbus implementation when
> switching pages. We weren't resetting the buffer state when switching
> pages there.
>
> I also added a placeholder implementation of the PECI controller, that does
> almost nothing, but doesn't produce errors.
>
> I added the fby35-cpld, which oby35-cl queries using master-mode TX and RX
> commands.
>
> And finally, I added an "intel-me" device (Intel Management Engine) that
> attempts to respond to the first IPMB message sent by the BIC. I used this
> to test the final patch, which I actually want to merge, the I2C new
> register DMA slave mode support.
>
> All the patches except the last one can be ignored for now if you want,
> again, I just included them for testing purposes.
>
> The final patch is pretty rough, but I wanted to start the review instead of
> waiting too long. I expect the interrupt handling part will be
> the main blocker.
>
> Thanks,
> Peter
>
> Klaus Jensen (3):
> hw/i2c: support multiple masters
> hw/i2c: add asynchronous send
> hw/i2c/aspeed: add slave device in old register mode
>
> Peter Delevoryas (11):
> aspeed: i2c: Fix DMA len write-enable bit handling
> aspeed: i2c: Fix R_I2CD_FUN_CTRL reference
> aspeed: i2c: Fix MASTER_EN missing error message
I didn't receive this patch ?
C.
> aspeed: Add PECI controller
> hw/misc: Add fby35-cpld
> pmbus: Reset out buf after switching pages
> pmbus: Add read-only IC_DEVICE_ID support
> aspeed: Add oby35-cl machine
> hw/misc: Add intel-me
> aspeed: Add intel-me on i2c6 instead of BMC
> aspeed: Add I2C new register DMA slave mode support
>
> hw/arm/aspeed.c | 42 ++++++
> hw/arm/aspeed_ast10x0.c | 11 ++
> hw/arm/pxa2xx.c | 2 +
> hw/display/sii9022.c | 2 +
> hw/display/ssd0303.c | 2 +
> hw/i2c/aspeed_i2c.c | 234 +++++++++++++++++++++++++++----
> hw/i2c/core.c | 70 ++++++++-
> hw/i2c/pmbus_device.c | 6 +
> hw/i2c/smbus_slave.c | 4 +
> hw/i2c/trace-events | 2 +
> hw/misc/aspeed_peci.c | 225 +++++++++++++++++++++++++++++
> hw/misc/fby35_cpld.c | 137 ++++++++++++++++++
> hw/misc/intel_me.c | 176 +++++++++++++++++++++++
> hw/misc/meson.build | 5 +-
> hw/nvram/eeprom_at24c.c | 2 +
> hw/sensor/isl_pmbus_vr.c | 30 ++++
> hw/sensor/lsm303dlhc_mag.c | 2 +
> include/hw/arm/aspeed_soc.h | 3 +
> include/hw/i2c/aspeed_i2c.h | 11 ++
> include/hw/i2c/i2c.h | 30 ++++
> include/hw/i2c/pmbus_device.h | 1 +
> include/hw/misc/aspeed_peci.h | 34 +++++
> include/hw/sensor/isl_pmbus_vr.h | 1 +
> 23 files changed, 1002 insertions(+), 30 deletions(-)
> create mode 100644 hw/misc/aspeed_peci.c
> create mode 100644 hw/misc/fby35_cpld.c
> create mode 100644 hw/misc/intel_me.c
> create mode 100644 include/hw/misc/aspeed_peci.h
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 04/14] aspeed: i2c: Fix DMA len write-enable bit handling
2022-06-28 7:01 ` Cédric Le Goater
@ 2022-06-28 7:05 ` Peter Delevoryas
0 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-28 7:05 UTC (permalink / raw)
Cc: Peter Delevoryas, Dan Zhang, Cameron Esfahani via, qemu-arm,
komlodi, titusr, Andrew Jeffery, Joel Stanley,
Cédric Le Goater
> On Jun 28, 2022, at 12:01 AM, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 6/27/22 21:54, Peter Delevoryas wrote:
>> I noticed i2c rx transfers were getting shortened to "1" on Zephyr. It
>> seems to be because the Zephyr i2c driver sets the RX DMA len with the
>> RX field write-enable bit set (bit 31) to avoid a read-modify-write. [1]
>> /* 0x1C : I2CM Master DMA Transfer Length Register */
>> I think we should be checking the write-enable bits on the incoming
>> value, not checking the register array. I'm not sure we're even writing
>> the write-enable bits to the register array, actually.
>> [1] https://github.com/AspeedTech-BMC/zephyr/blob/db3dbcc9c52e67a47180890ac938ed380b33f91c/drivers/i2c/i2c_aspeed.c#L145-L148
>> Fixes: ba2cccd64e90f34 ("aspeed: i2c: Add new mode support")
>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>> ---
>
> Nice. Can you move this patch at beginning of the series ?
Yeah sure! I’ll do that on v2.
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>
> Thanks,
>
> C.
>
>
>> hw/i2c/aspeed_i2c.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
>> index 2cfd05cb6c..6c8222717f 100644
>> --- a/hw/i2c/aspeed_i2c.c
>> +++ b/hw/i2c/aspeed_i2c.c
>> @@ -644,18 +644,18 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>> RX_BUF_LEN) + 1;
>> break;
>> case A_I2CM_DMA_LEN:
>> - w1t = ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, RX_BUF_LEN_W1T) ||
>> - ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, TX_BUF_LEN_W1T);
>> + w1t = FIELD_EX32(value, I2CM_DMA_LEN, RX_BUF_LEN_W1T) ||
>> + FIELD_EX32(value, I2CM_DMA_LEN, TX_BUF_LEN_W1T);
>> /* If none of the w1t bits are set, just write to the reg as normal. */
>> if (!w1t) {
>> bus->regs[R_I2CM_DMA_LEN] = value;
>> break;
>> }
>> - if (ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, RX_BUF_LEN_W1T)) {
>> + if (FIELD_EX32(value, I2CM_DMA_LEN, RX_BUF_LEN_W1T)) {
>> ARRAY_FIELD_DP32(bus->regs, I2CM_DMA_LEN, RX_BUF_LEN,
>> FIELD_EX32(value, I2CM_DMA_LEN, RX_BUF_LEN));
>> }
>> - if (ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, TX_BUF_LEN_W1T)) {
>> + if (FIELD_EX32(value, I2CM_DMA_LEN, TX_BUF_LEN_W1T)) {
>> ARRAY_FIELD_DP32(bus->regs, I2CM_DMA_LEN, TX_BUF_LEN,
>> FIELD_EX32(value, I2CM_DMA_LEN, TX_BUF_LEN));
>> }
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 05/14] aspeed: i2c: Fix R_I2CD_FUN_CTRL reference
2022-06-28 7:01 ` Cédric Le Goater
@ 2022-06-28 7:05 ` Peter Delevoryas
0 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-28 7:05 UTC (permalink / raw)
Cc: Peter Delevoryas, Dan Zhang, Cameron Esfahani via, qemu-arm,
komlodi, titusr, Andrew Jeffery, Joel Stanley,
Cédric Le Goater
> On Jun 28, 2022, at 12:01 AM, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 6/27/22 21:54, Peter Delevoryas wrote:
>> Very minor, doesn't effect functionality, but this is supposed to be
>> R_I2CC_FUN_CTRL (new-mode, not old-mode).
>> Fixes: ba2cccd64e9 ("aspeed: i2c: Add new mode support")
>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>> ---
>
>
> Can you move this patch at beginning of the series ?
Yep, will-do.
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>
> Thanks,
>
> C.
>
>
>
>> hw/i2c/aspeed_i2c.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
>> index 6c8222717f..6abd9b033e 100644
>> --- a/hw/i2c/aspeed_i2c.c
>> +++ b/hw/i2c/aspeed_i2c.c
>> @@ -552,7 +552,7 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>> __func__);
>> break;
>> }
>> - bus->regs[R_I2CD_FUN_CTRL] = value & 0x007dc3ff;
>> + bus->regs[R_I2CC_FUN_CTRL] = value & 0x007dc3ff;
>> break;
>> case A_I2CC_AC_TIMING:
>> bus->regs[R_I2CC_AC_TIMING] = value & 0x1ffff0ff;
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 14/14] aspeed: Add I2C new register DMA slave mode support
2022-06-28 7:02 ` Cédric Le Goater
@ 2022-06-28 7:06 ` Peter Delevoryas
0 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-28 7:06 UTC (permalink / raw)
Cc: Peter Delevoryas, Cameron Esfahani via, qemu-arm, Cédric Le Goater
> On Jun 28, 2022, at 12:02 AM, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 6/28/22 00:27, Peter Delevoryas wrote:
>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>
> Some Intro would be welcome. Please move the patch after Klaus patches.
Yes, I’ll describe the state machine flow. That’s my bad. And I’ll
move it to the beginning with the other i2c patches.
>
> Thanks,
>
> C.
>
>
>
>> ---
>> hw/i2c/aspeed_i2c.c | 133 ++++++++++++++++++++++++++++++++----
>> include/hw/i2c/aspeed_i2c.h | 3 +
>> 2 files changed, 124 insertions(+), 12 deletions(-)
>> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
>> index 8a8514586f..fc8b6b62cf 100644
>> --- a/hw/i2c/aspeed_i2c.c
>> +++ b/hw/i2c/aspeed_i2c.c
>> @@ -78,6 +78,18 @@ static inline void aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus)
>> }
>> }
>> +static inline void aspeed_i2c_bus_raise_slave_interrupt(AspeedI2CBus *bus)
>> +{
>> + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
>> +
>> + if (!bus->regs[R_I2CS_INTR_STS]) {
>> + return;
>> + }
>> +
>> + bus->controller->intr_status |= 1 << bus->id;
>> + qemu_irq_raise(aic->bus_get_irq(bus));
>> +}
>> +
>> static uint64_t aspeed_i2c_bus_old_read(AspeedI2CBus *bus, hwaddr offset,
>> unsigned size)
>> {
>> @@ -140,8 +152,17 @@ static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset,
>> case A_I2CM_DMA_LEN_STS:
>> case A_I2CC_DMA_ADDR:
>> case A_I2CC_DMA_LEN:
>> +
>> + case A_I2CS_DEV_ADDR:
>> + case A_I2CS_DMA_RX_ADDR:
>> + case A_I2CS_DMA_LEN:
>> + case A_I2CS_CMD:
>> + case A_I2CS_INTR_CTRL:
>> + case A_I2CS_DMA_LEN_STS:
>> /* Value is already set, don't do anything. */
>> break;
>> + case A_I2CS_INTR_STS:
>> + break;
>> case A_I2CM_CMD:
>> value = SHARED_FIELD_DP32(value, BUS_BUSY_STS, i2c_bus_busy(bus->bus));
>> break;
>> @@ -547,12 +568,7 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>> switch (offset) {
>> case A_I2CC_FUN_CTRL:
>> - if (SHARED_FIELD_EX32(value, SLAVE_EN)) {
>> - qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
>> - __func__);
>> - break;
>> - }
>> - bus->regs[R_I2CC_FUN_CTRL] = value & 0x007dc3ff;
>> + bus->regs[R_I2CC_FUN_CTRL] = value;
>> break;
>> case A_I2CC_AC_TIMING:
>> bus->regs[R_I2CC_AC_TIMING] = value & 0x1ffff0ff;
>> @@ -580,6 +596,7 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>> bus->controller->intr_status &= ~(1 << bus->id);
>> qemu_irq_lower(aic->bus_get_irq(bus));
>> }
>> + aspeed_i2c_bus_raise_slave_interrupt(bus);
>> break;
>> }
>> bus->regs[R_I2CM_INTR_STS] &= ~(value & 0xf007f07f);
>> @@ -668,15 +685,53 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>> case A_I2CC_DMA_LEN:
>> /* RO */
>> break;
>> - case A_I2CS_DMA_LEN_STS:
>> - case A_I2CS_DMA_TX_ADDR:
>> - case A_I2CS_DMA_RX_ADDR:
>> case A_I2CS_DEV_ADDR:
>> + bus->regs[R_I2CS_DEV_ADDR] = value;
>> + break;
>> + case A_I2CS_DMA_RX_ADDR:
>> + bus->regs[R_I2CS_DMA_RX_ADDR] = value;
>> + break;
>> + case A_I2CS_DMA_LEN:
>> + assert(FIELD_EX32(value, I2CS_DMA_LEN, TX_BUF_LEN) == 0);
>> + if (FIELD_EX32(value, I2CS_DMA_LEN, RX_BUF_LEN_W1T)) {
>> + ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN, RX_BUF_LEN,
>> + FIELD_EX32(value, I2CS_DMA_LEN, RX_BUF_LEN));
>> + } else {
>> + bus->regs[R_I2CS_DMA_LEN] = value;
>> + }
>> + break;
>> + case A_I2CS_CMD:
>> + if (FIELD_EX32(value, I2CS_CMD, W1_CTRL)) {
>> + bus->regs[R_I2CS_CMD] |= value;
>> + } else {
>> + bus->regs[R_I2CS_CMD] = value;
>> + }
>> + i2c_slave_set_address(bus->slave, bus->regs[R_I2CS_DEV_ADDR]);
>> + break;
>> case A_I2CS_INTR_CTRL:
>> + bus->regs[R_I2CS_INTR_CTRL] = value;
>> + break;
>> +
>> case A_I2CS_INTR_STS:
>> - case A_I2CS_CMD:
>> - case A_I2CS_DMA_LEN:
>> - qemu_log_mask(LOG_UNIMP, "%s: Slave mode is not implemented\n",
>> + if (ARRAY_FIELD_EX32(bus->regs, I2CS_INTR_CTRL, PKT_CMD_DONE)) {
>> + if (ARRAY_FIELD_EX32(bus->regs, I2CS_INTR_STS, PKT_CMD_DONE) &&
>> + FIELD_EX32(value, I2CS_INTR_STS, PKT_CMD_DONE)) {
>> + bus->regs[R_I2CS_INTR_STS] &= 0xfffc0000;
>> + }
>> + } else {
>> + bus->regs[R_I2CS_INTR_STS] &= ~value;
>> + }
>> + if (!bus->regs[R_I2CS_INTR_STS]) {
>> + bus->controller->intr_status &= ~(1 << bus->id);
>> + qemu_irq_lower(aic->bus_get_irq(bus));
>> + }
>> + aspeed_i2c_bus_raise_interrupt(bus);
>> + break;
>> + case A_I2CS_DMA_LEN_STS:
>> + bus->regs[R_I2CS_DMA_LEN_STS] = 0;
>> + break;
>> + case A_I2CS_DMA_TX_ADDR:
>> + qemu_log_mask(LOG_UNIMP, "%s: Slave mode DMA TX is not implemented\n",
>> __func__);
>> break;
>> default:
>> @@ -1037,6 +1092,39 @@ static const TypeInfo aspeed_i2c_info = {
>> .abstract = true,
>> };
>> +static int aspeed_i2c_bus_new_slave_event(AspeedI2CBus *bus,
>> + enum i2c_event event)
>> +{
>> + switch (event) {
>> + case I2C_START_SEND_ASYNC:
>> + if (!SHARED_ARRAY_FIELD_EX32(bus->regs, R_I2CS_CMD, RX_DMA_EN)) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Slave mode RX DMA is not enabled\n", __func__);
>> + return -1;
>> + }
>> + ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN, 0);
>> + bus->regs[R_I2CC_DMA_ADDR] =
>> + ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_RX_ADDR, ADDR);
>> + bus->regs[R_I2CC_DMA_LEN] =
>> + ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_LEN, RX_BUF_LEN) + 1;
>> + i2c_ack(bus->bus);
>> + break;
>> + case I2C_FINISH:
>> + ARRAY_FIELD_DP32(bus->regs, I2CS_INTR_STS, PKT_CMD_DONE, 1);
>> + ARRAY_FIELD_DP32(bus->regs, I2CS_INTR_STS, SLAVE_ADDR_RX_MATCH, 1);
>> + SHARED_ARRAY_FIELD_DP32(bus->regs, R_I2CS_INTR_STS, NORMAL_STOP, 1);
>> + SHARED_ARRAY_FIELD_DP32(bus->regs, R_I2CS_INTR_STS, RX_DONE, 1);
>> + aspeed_i2c_bus_raise_slave_interrupt(bus);
>> + break;
>> + default:
>> + qemu_log_mask(LOG_UNIMP, "%s: i2c event %d unimplemented\n",
>> + __func__, event);
>> + return -1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int aspeed_i2c_bus_slave_event(I2CSlave *slave, enum i2c_event event)
>> {
>> BusState *qbus = qdev_get_parent_bus(DEVICE(slave));
>> @@ -1045,6 +1133,10 @@ static int aspeed_i2c_bus_slave_event(I2CSlave *slave, enum i2c_event event)
>> uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus);
>> uint32_t value;
>> + if (aspeed_i2c_is_new_mode(bus->controller)) {
>> + return aspeed_i2c_bus_new_slave_event(bus, event);
>> + }
>> +
>> switch (event) {
>> case I2C_START_SEND_ASYNC:
>> value = SHARED_ARRAY_FIELD_EX32(bus->regs, reg_byte_buf, TX_BUF);
>> @@ -1073,6 +1165,19 @@ static int aspeed_i2c_bus_slave_event(I2CSlave *slave, enum i2c_event event)
>> return 0;
>> }
>> +static void aspeed_i2c_bus_new_slave_send_async(AspeedI2CBus *bus, uint8_t data)
>> +{
>> + assert(address_space_write(&bus->controller->dram_as,
>> + bus->regs[R_I2CC_DMA_ADDR],
>> + MEMTXATTRS_UNSPECIFIED, &data, 1) == MEMTX_OK);
>> +
>> + bus->regs[R_I2CC_DMA_ADDR]++;
>> + bus->regs[R_I2CC_DMA_LEN]--;
>> + ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN,
>> + ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN) + 1);
>> + i2c_ack(bus->bus);
>> +}
>> +
>> static void aspeed_i2c_bus_slave_send_async(I2CSlave *slave, uint8_t data)
>> {
>> BusState *qbus = qdev_get_parent_bus(DEVICE(slave));
>> @@ -1080,6 +1185,10 @@ static void aspeed_i2c_bus_slave_send_async(I2CSlave *slave, uint8_t data)
>> uint32_t reg_intr_sts = aspeed_i2c_bus_intr_sts_offset(bus);
>> uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus);
>> + if (aspeed_i2c_is_new_mode(bus->controller)) {
>> + return aspeed_i2c_bus_new_slave_send_async(bus, data);
>> + }
>> +
>> SHARED_ARRAY_FIELD_DP32(bus->regs, reg_byte_buf, RX_BUF, data);
>> SHARED_ARRAY_FIELD_DP32(bus->regs, reg_intr_sts, RX_DONE, 1);
>> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
>> index ba148b2f6d..300a89b343 100644
>> --- a/include/hw/i2c/aspeed_i2c.h
>> +++ b/include/hw/i2c/aspeed_i2c.h
>> @@ -174,6 +174,8 @@ REG32(I2CM_DMA_LEN, 0x1c)
>> FIELD(I2CM_DMA_LEN, TX_BUF_LEN_W1T, 15, 1)
>> FIELD(I2CM_DMA_LEN, TX_BUF_LEN, 0, 11)
>> REG32(I2CS_INTR_CTRL, 0x20)
>> + FIELD(I2CS_INTR_CTRL, PKT_CMD_FAIL, 17, 1)
>> + FIELD(I2CS_INTR_CTRL, PKT_CMD_DONE, 16, 1)
>> REG32(I2CS_INTR_STS, 0x24)
>> /* 31:29 shared with I2CD_INTR_STS[31:29] */
>> FIELD(I2CS_INTR_STS, SLAVE_PARKING_STS, 24, 2)
>> @@ -184,6 +186,7 @@ REG32(I2CS_INTR_STS, 0x24)
>> FIELD(I2CS_INTR_STS, PKT_CMD_FAIL, 17, 1)
>> FIELD(I2CS_INTR_STS, PKT_CMD_DONE, 16, 1)
>> /* 14:0 shared with I2CD_INTR_STS[14:0] */
>> + FIELD(I2CS_INTR_STS, SLAVE_ADDR_RX_MATCH, 7, 1)
>> REG32(I2CS_CMD, 0x28)
>> FIELD(I2CS_CMD, W1_CTRL, 31, 1)
>> FIELD(I2CS_CMD, PKT_MODE_ACTIVE_ADDR, 17, 2)
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support
2022-06-28 7:05 ` [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Cédric Le Goater
@ 2022-06-28 7:16 ` Peter Delevoryas
0 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-28 7:16 UTC (permalink / raw)
Cc: Peter Delevoryas, Dan Zhang, Cameron Esfahani via, qemu-arm,
komlodi, titusr, Andrew Jeffery, Joel Stanley,
Cédric Le Goater
> On Jun 28, 2022, at 12:05 AM, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 6/27/22 21:54, Peter Delevoryas wrote:
>> Hey everyone,
>> I'm sending a big patch series for this, but only the last commit is really
>> intended to be accepted right now. I'm just including the rest of them
>> because it depends on them for testing.
>> Klaus's changes include the multi-master stuff in hw/i2c/core.c, and the old
>> register mode slave mode support.
>> My series of patches includes a bunch of changes to eliminate most (if not
>> all) of the I2C errors reported by the fb OpenBIC firmware for fby35
>> CraterLake Bridge Interconnect (BIC) (shortname: oby35-cl) upon startup.
>> In particular, I needed to add the IC_DEVICE_ID to the isl voltage regulator
>> implementation, which required a fix to the pmbus implementation when
>> switching pages. We weren't resetting the buffer state when switching
>> pages there.
>> I also added a placeholder implementation of the PECI controller, that does
>> almost nothing, but doesn't produce errors.
>> I added the fby35-cpld, which oby35-cl queries using master-mode TX and RX
>> commands.
>> And finally, I added an "intel-me" device (Intel Management Engine) that
>> attempts to respond to the first IPMB message sent by the BIC. I used this
>> to test the final patch, which I actually want to merge, the I2C new
>> register DMA slave mode support.
>> All the patches except the last one can be ignored for now if you want,
>> again, I just included them for testing purposes.
>> The final patch is pretty rough, but I wanted to start the review instead of
>> waiting too long. I expect the interrupt handling part will be
>> the main blocker.
>> Thanks,
>> Peter
>> Klaus Jensen (3):
>> hw/i2c: support multiple masters
>> hw/i2c: add asynchronous send
>> hw/i2c/aspeed: add slave device in old register mode
>> Peter Delevoryas (11):
>> aspeed: i2c: Fix DMA len write-enable bit handling
>> aspeed: i2c: Fix R_I2CD_FUN_CTRL reference
>> aspeed: i2c: Fix MASTER_EN missing error message
>
>
> I didn't receive this patch ?
Yes, sorry about that, the FB email filter is very delicate,
the behavior is seemingly random to me. I’ve been trying to
debug it. I re-sent 12-14 manually, but resending patch 6
didn’t work for some reason. I’ve been attempting to
switch to a personal email, me@pjd.dev, but I can’t seem
to get git-send-email working with it. It’s really ridiculous.
Probably have to start bringing a personal laptop to the office.
I’ve attempted to resend this patch, number 6, one more time,
just now. I’ll include it again in v2, and hopefully
I’ll have my email working at that point.
>
> C.
>
>
>> aspeed: Add PECI controller
>> hw/misc: Add fby35-cpld
>> pmbus: Reset out buf after switching pages
>> pmbus: Add read-only IC_DEVICE_ID support
>> aspeed: Add oby35-cl machine
>> hw/misc: Add intel-me
>> aspeed: Add intel-me on i2c6 instead of BMC
>> aspeed: Add I2C new register DMA slave mode support
>> hw/arm/aspeed.c | 42 ++++++
>> hw/arm/aspeed_ast10x0.c | 11 ++
>> hw/arm/pxa2xx.c | 2 +
>> hw/display/sii9022.c | 2 +
>> hw/display/ssd0303.c | 2 +
>> hw/i2c/aspeed_i2c.c | 234 +++++++++++++++++++++++++++----
>> hw/i2c/core.c | 70 ++++++++-
>> hw/i2c/pmbus_device.c | 6 +
>> hw/i2c/smbus_slave.c | 4 +
>> hw/i2c/trace-events | 2 +
>> hw/misc/aspeed_peci.c | 225 +++++++++++++++++++++++++++++
>> hw/misc/fby35_cpld.c | 137 ++++++++++++++++++
>> hw/misc/intel_me.c | 176 +++++++++++++++++++++++
>> hw/misc/meson.build | 5 +-
>> hw/nvram/eeprom_at24c.c | 2 +
>> hw/sensor/isl_pmbus_vr.c | 30 ++++
>> hw/sensor/lsm303dlhc_mag.c | 2 +
>> include/hw/arm/aspeed_soc.h | 3 +
>> include/hw/i2c/aspeed_i2c.h | 11 ++
>> include/hw/i2c/i2c.h | 30 ++++
>> include/hw/i2c/pmbus_device.h | 1 +
>> include/hw/misc/aspeed_peci.h | 34 +++++
>> include/hw/sensor/isl_pmbus_vr.h | 1 +
>> 23 files changed, 1002 insertions(+), 30 deletions(-)
>> create mode 100644 hw/misc/aspeed_peci.c
>> create mode 100644 hw/misc/fby35_cpld.c
>> create mode 100644 hw/misc/intel_me.c
>> create mode 100644 include/hw/misc/aspeed_peci.h
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 12/14] hw/misc: Add intel-me
2022-06-28 6:58 ` [PATCH 12/14] hw/misc: Add intel-me Cédric Le Goater
@ 2022-06-28 7:17 ` Peter Delevoryas
0 siblings, 0 replies; 31+ messages in thread
From: Peter Delevoryas @ 2022-06-28 7:17 UTC (permalink / raw)
Cc: Peter Delevoryas, Cameron Esfahani via, qemu-arm, Cédric Le Goater
> On Jun 27, 2022, at 11:58 PM, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 6/28/22 00:27, Peter Delevoryas wrote:
>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>
> Intro ?
Yep, will do
>
> I would rather have 2 patches, one for the slave model and one adding
> a device to the machine.
Got it, I’ll split it.
>
> Please replace the printf with trace events.
Yeah sorry about that
>
> Thanks,
>
> C.
>
>> ---
>> hw/arm/aspeed.c | 1 +
>> hw/misc/intel_me.c | 176 ++++++++++++++++++++++++++++++++++++++++++++
>> hw/misc/meson.build | 3 +-
>> 3 files changed, 179 insertions(+), 1 deletion(-)
>> create mode 100644 hw/misc/intel_me.c
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index 2b9c1600c6..88e9a47dc2 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -1447,6 +1447,7 @@ static void oby35_cl_i2c_init(AspeedMachineState *bmc)
>> i2c_slave_create_simple(i2c[1], "tmp105", 0x4a);
>> i2c_slave_create_simple(i2c[1], "adm1272", 0x40);
>> i2c_slave_create_simple(i2c[1], "tmp421", 0x4c);
>> + i2c_slave_create_simple(i2c[2], "intel-me", 0x16);
>> i2c_slave_create_simple(i2c[4], "isl69259", 0x76);
>> i2c_slave_create_simple(i2c[4], "isl69259", 0x62);
>> i2c_slave_create_simple(i2c[4], "isl69259", 0x60);
>> diff --git a/hw/misc/intel_me.c b/hw/misc/intel_me.c
>> new file mode 100644
>> index 0000000000..fdc9180c26
>> --- /dev/null
>> +++ b/hw/misc/intel_me.c
>> @@ -0,0 +1,176 @@
>> +/*
>> + * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
>> + *
>> + * 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/main-loop.h"
>> +#include "hw/i2c/i2c.h"
>> +
>> +#define TYPE_INTEL_ME "intel-me"
>> +OBJECT_DECLARE_SIMPLE_TYPE(IntelMEState, INTEL_ME);
>> +
>> +#define printf(...)
>> +
>> +struct IntelMEState {
>> + I2CSlave parent_obj;
>> +
>> + I2CBus *bus;
>> + QEMUBH *bh;
>> + int rx_len;
>> + int tx_len;
>> + int tx_pos;
>> + uint8_t rx_buf[512];
>> + uint8_t tx_buf[512];
>> +};
>> +
>> +static void intel_me_bh(void *opaque)
>> +{
>> + IntelMEState *s = opaque;
>> +
>> + assert(s->bus->bh == s->bh);
>> +
>> + if (s->tx_pos == 0) {
>> + if (i2c_start_send_async(s->bus, s->tx_buf[s->tx_pos++]) != 0) {
>> + goto done;
>> + }
>> + return;
>> + }
>> +
>> + if (s->tx_pos < s->tx_len) {
>> + if (i2c_send_async(s->bus, s->tx_buf[s->tx_pos++]) != 0) {
>> + goto done;
>> + }
>> + return;
>> + }
>> +
>> +done:
>> + i2c_end_transfer(s->bus);
>> + i2c_bus_release(s->bus);
>> + s->tx_len = 0;
>> + s->tx_pos = 0;
>> + memset(s->tx_buf, 0, sizeof(s->tx_buf));
>> +}
>> +
>> +static void intel_me_realize(DeviceState *dev, Error **errp)
>> +{
>> + IntelMEState *s = INTEL_ME(dev);
>> +
>> + s->bus = I2C_BUS(qdev_get_parent_bus(dev));
>> + s->bh = qemu_bh_new(intel_me_bh, s);
>> + s->rx_len = 0;
>> + s->tx_len = 0;
>> + s->tx_pos = 0;
>> + memset(s->rx_buf, 0, sizeof(s->rx_buf));
>> + memset(s->tx_buf, 0, sizeof(s->tx_buf));
>> +}
>> +
>> +static uint8_t checksum(const uint8_t *ptr, int len)
>> +{
>> + int sum = 0;
>> +
>> + for (int i = 0; i < len; i++) {
>> + sum += ptr[i];
>> + }
>> +
>> + return 256 - sum;
>> +}
>> +
>> +static int intel_me_i2c_event(I2CSlave *i2c, enum i2c_event event)
>> +{
>> + IntelMEState *s = INTEL_ME(i2c);
>> +
>> + switch (event) {
>> + case I2C_START_RECV:
>> + break;
>> + case I2C_START_SEND:
>> + s->rx_len = 0;
>> + memset(s->rx_buf, 0, sizeof(s->rx_buf));
>> + break;
>> + case I2C_START_SEND_ASYNC:
>> + break;
>> + case I2C_FINISH:
>> + printf("IntelME rx: [");
>> + for (int i = 0; i < s->rx_len; i++) {
>> + if (i) {
>> + printf(", ");
>> + }
>> + printf("0x%02x", s->rx_buf[i]);
>> + }
>> + printf("]\n");
>> +
>> + s->tx_len = 10;
>> + s->tx_pos = 0;
>> + s->tx_buf[0] = s->rx_buf[2];
>> + s->tx_buf[1] = ((s->rx_buf[0] >> 2) + 1) << 2;
>> + s->tx_buf[2] = 256 - s->tx_buf[0] - s->tx_buf[1];
>> + s->tx_buf[3] = i2c->address; // rsSA response Slave Address
>> + s->tx_buf[4] = (s->rx_buf[3] >> 2) << 2; // sequence number
>> + s->tx_buf[5] = s->rx_buf[4]; // Same command code
>> + s->tx_buf[6] = 0x00; // OK
>> + s->tx_buf[7] = 0x55; // NO_ERROR
>> + s->tx_buf[8] = 0x00;
>> + s->tx_buf[9] = checksum(s->tx_buf, s->tx_len - 1);
>> + s->tx_buf[0] >>= 1;
>> + i2c_bus_master(s->bus, s->bh);
>> + break;
>> + case I2C_NACK:
>> + break;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static uint8_t intel_me_i2c_recv(I2CSlave *i2c)
>> +{
>> + return 0xff;
>> +}
>> +
>> +static int intel_me_i2c_send(I2CSlave *i2c, uint8_t data)
>> +{
>> + IntelMEState *s = INTEL_ME(i2c);
>> +
>> + assert(s->rx_len < sizeof(s->rx_buf));
>> + s->rx_buf[s->rx_len++] = data;
>> +
>> + return 0;
>> +}
>> +
>> +static void intel_me_class_init(ObjectClass *oc, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(oc);
>> + I2CSlaveClass *i2c = I2C_SLAVE_CLASS(oc);
>> +
>> + dc->realize = intel_me_realize;
>> + i2c->event = intel_me_i2c_event;
>> + i2c->recv = intel_me_i2c_recv;
>> + i2c->send = intel_me_i2c_send;
>> +}
>> +
>> +static const TypeInfo types[] = {
>> + {
>> + .name = TYPE_INTEL_ME,
>> + .parent = TYPE_I2C_SLAVE,
>> + .instance_size = sizeof(IntelMEState),
>> + .class_init = intel_me_class_init,
>> + },
>> +};
>> +
>> +DEFINE_TYPES(types);
>> diff --git a/hw/misc/meson.build b/hw/misc/meson.build
>> index 1edad44b6b..a2c75894a3 100644
>> --- a/hw/misc/meson.build
>> +++ b/hw/misc/meson.build
>> @@ -118,7 +118,8 @@ softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
>> 'aspeed_sdmc.c',
>> 'aspeed_xdma.c',
>> 'aspeed_peci.c',
>> - 'fby35_cpld.c'))
>> + 'fby35_cpld.c',
>> + 'intel_me.c'))
>> softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-sysreg.c'))
>> softmmu_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_rng.c'))
>
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2022-06-28 9:23 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-27 19:54 [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
2022-06-27 19:54 ` [PATCH 01/14] hw/i2c: support multiple masters Peter Delevoryas
2022-06-27 19:54 ` [PATCH 02/14] hw/i2c: add asynchronous send Peter Delevoryas
2022-06-27 19:54 ` [PATCH 03/14] hw/i2c/aspeed: add slave device in old register mode Peter Delevoryas
2022-06-27 19:54 ` [PATCH 04/14] aspeed: i2c: Fix DMA len write-enable bit handling Peter Delevoryas
2022-06-28 7:01 ` Cédric Le Goater
2022-06-28 7:05 ` Peter Delevoryas
2022-06-27 19:54 ` [PATCH 05/14] aspeed: i2c: Fix R_I2CD_FUN_CTRL reference Peter Delevoryas
2022-06-28 7:01 ` Cédric Le Goater
2022-06-28 7:05 ` Peter Delevoryas
2022-06-27 19:54 ` [PATCH 07/14] aspeed: Add PECI controller Peter Delevoryas
2022-06-28 6:47 ` Cédric Le Goater
2022-06-28 6:58 ` Peter Delevoryas
2022-06-27 19:55 ` [PATCH 08/14] hw/misc: Add fby35-cpld Peter Delevoryas
2022-06-28 6:50 ` Cédric Le Goater
2022-06-28 7:00 ` Peter Delevoryas
2022-06-27 19:55 ` [PATCH 09/14] pmbus: Reset out buf after switching pages Peter Delevoryas
2022-06-28 6:51 ` Cédric Le Goater
2022-06-28 7:04 ` Peter Delevoryas
2022-06-27 19:55 ` [PATCH 10/14] pmbus: Add read-only IC_DEVICE_ID support Peter Delevoryas
2022-06-27 19:55 ` [PATCH 11/14] aspeed: Add oby35-cl machine Peter Delevoryas
2022-06-27 20:04 ` [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
2022-06-27 22:27 ` [PATCH 12/14] hw/misc: Add intel-me Peter Delevoryas
2022-06-27 22:27 ` [PATCH 13/14] aspeed: Add intel-me on i2c6 instead of BMC Peter Delevoryas
2022-06-27 22:27 ` [PATCH 14/14] aspeed: Add I2C new register DMA slave mode support Peter Delevoryas
2022-06-28 7:02 ` Cédric Le Goater
2022-06-28 7:06 ` Peter Delevoryas
2022-06-28 6:58 ` [PATCH 12/14] hw/misc: Add intel-me Cédric Le Goater
2022-06-28 7:17 ` Peter Delevoryas
2022-06-28 7:05 ` [PATCH 00/14] aspeed: Add I2C new register DMA slave mode support Cédric Le Goater
2022-06-28 7:16 ` Peter Delevoryas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.