On Jun 2 08:49, Cédric Le Goater wrote: > On 6/1/22 23:08, Klaus Jensen wrote: > > From: Klaus Jensen <k.jen...@samsung.com> > > > > Build a single string instead of having several parameters on the trace > > event. > > > > Suggested-by: Cédric Le Goater <c...@kaod.org> > > Signed-off-by: Klaus Jensen <k.jen...@samsung.com> > > --- > > hw/i2c/aspeed_i2c.c | 55 +++++++++++++++++++++++++++++++++++---------- > > hw/i2c/trace-events | 2 +- > > 2 files changed, 44 insertions(+), 13 deletions(-) > > > > diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c > > index 5fce516517a5..576425898b09 100644 > > --- a/hw/i2c/aspeed_i2c.c > > +++ b/hw/i2c/aspeed_i2c.c > > @@ -21,6 +21,7 @@ > > #include "qemu/osdep.h" > > #include "hw/sysbus.h" > > #include "migration/vmstate.h" > > +#include "qemu/cutils.h" > > #include "qemu/log.h" > > #include "qemu/module.h" > > #include "qemu/error-report.h" > > @@ -31,6 +32,9 @@ > > #include "hw/registerfields.h" > > #include "trace.h" > > +#define ASPEED_I2C_TRACE_INTR_TEMPLATE \ > > + "pktdone|nak|ack|done|normal|abnormal|" > > + > > static inline void aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus) > > { > > AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); > > @@ -38,23 +42,50 @@ static inline void > > aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus) > > uint32_t intr_ctrl_reg = aspeed_i2c_bus_intr_ctrl_offset(bus); > > bool raise_irq; > > - trace_aspeed_i2c_bus_raise_interrupt(bus->regs[reg_intr_sts], > > - aspeed_i2c_bus_pkt_mode_en(bus) && > > - ARRAY_FIELD_EX32(bus->regs, I2CM_INTR_STS, PKT_CMD_DONE) ? > > - "pktdone|" > > : "", > > - SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, TX_NAK) ? "nak|" > > : "", > > - SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, TX_ACK) ? "ack|" > > : "", > > - SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, RX_DONE) ? "done|" > > - : "", > > - SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, NORMAL_STOP) ? > > - "normal|" > > : "", > > - SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, ABNORMAL) ? > > "abnormal" > > - : ""); > > + if > > (trace_event_get_state_backends(TRACE_ASPEED_I2C_BUS_RAISE_INTERRUPT)) { > > + static const size_t BUF_SIZE = > > strlen(ASPEED_I2C_TRACE_INTR_TEMPLATE); > > + g_autofree char *buf = g_malloc0(BUF_SIZE); > > + > > + /* > > + * Remember to update ASPEED_I2C_TRACE_INTR_TEMPLATE if you add a > > new > > + * status string. > > + */ > > + > > + if (aspeed_i2c_bus_pkt_mode_en(bus) && > > + ARRAY_FIELD_EX32(bus->regs, I2CM_INTR_STS, PKT_CMD_DONE)) { > > + pstrcat(buf, BUF_SIZE, "pktdone|"); > > + } > > + > > + if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, TX_NAK)) { > > + pstrcat(buf, BUF_SIZE, "nak|"); > > + } > > + > > + if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, TX_ACK)) { > > + pstrcat(buf, BUF_SIZE, "ack|"); > > + } > > + > > + if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, RX_DONE)) { > > + pstrcat(buf, BUF_SIZE, "done|"); > > + } > > + > > + if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, NORMAL_STOP)) > > { > > + pstrcat(buf, BUF_SIZE, "normal|"); > > + } > > + > > + if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, ABNORMAL)) { > > + pstrcat(buf, BUF_SIZE, "abnormal|"); > > + } > > + > > + trace_aspeed_i2c_bus_raise_interrupt(bus->regs[reg_intr_sts], buf); > > + } > > + > > How about : > > if (trace_event_get_state_backends(TRACE_ASPEED_I2C_BUS_RAISE_INTERRUPT)) > { > g_autofree char *buf = g_strdup_printf("%s%s%s%s%s%s", > aspeed_i2c_bus_pkt_mode_en(bus) && > ARRAY_FIELD_EX32(bus->regs, I2CM_INTR_STS, PKT_CMD_DONE) ? > "pktdone|" : "", > SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, TX_NAK)? > "nak|" : "", > SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, TX_ACK), > "ack|" : "", > SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, RX_DONE) ? > "done|" : "", > SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, NORMAL_STOP)? > "normal|" : "", > SHARED_ARRAY_FIELD_EX32(bus->regs, reg_intr_sts, ABNORMAL) ? > "abnormal" : ""); > > trace_aspeed_i2c_bus_raise_interrupt(bus->regs[reg_intr_sts], > buf); > } > >
Uhm, yeah - that's way better :)
signature.asc
Description: PGP signature