Re: [PATCH 14/14] aspeed: Add I2C new register DMA slave mode support

2022-06-28 Thread Peter Delevoryas 


> On Jun 28, 2022, at 12:02 AM, Cédric Le Goater  wrote:
> 
> On 6/28/22 00:27, Peter Delevoryas wrote:
>> Signed-off-by: Peter Delevoryas 
> 
> 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 & 0x10ff;
>> @@ -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] &= 0xfffc;
>> +}
>> +} 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));
>> +}
>> +

Re: [PATCH 14/14] aspeed: Add I2C new register DMA slave mode support

2022-06-28 Thread Cédric Le Goater 

On 6/28/22 00:27, Peter Delevoryas wrote:

Signed-off-by: Peter Delevoryas 


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 & 0x10ff;
@@ -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] &= 0xfffc;
+}
+} 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

[PATCH 14/14] aspeed: Add I2C new register DMA slave mode support

2022-06-27 Thread Peter Delevoryas 
Signed-off-by: Peter Delevoryas 
---
 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 & 0x10ff;
@@ -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] &= 0xfffc;
+}
+} 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,