| Message ID | 20220125161107.77962-1-eajames@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [linux,dev-5.15] fsi: Add trace events in initialization path | expand |
Dear Eddie, Am 25.01.22 um 17:11 schrieb Eddie James: > Add definitions for trace events to show the scanning flow in order > to debug recent scanning problems. Maybe give an example how to trace one of the new trace events. Kind regards, Paul > Signed-off-by: Eddie James <eajames@linux.ibm.com> > --- > drivers/fsi/fsi-core.c | 13 ++- > drivers/fsi/fsi-master-aspeed.c | 2 + > include/trace/events/fsi.h | 109 +++++++++++++++++++++++ > include/trace/events/fsi_master_aspeed.h | 12 +++ > 4 files changed, 133 insertions(+), 3 deletions(-) > > diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c > index 59ddc9fd5bca..78710087aa05 100644 > --- a/drivers/fsi/fsi-core.c > +++ b/drivers/fsi/fsi-core.c > @@ -24,9 +24,6 @@ > > #include "fsi-master.h" > > -#define CREATE_TRACE_POINTS > -#include <trace/events/fsi.h> > - > #define FSI_SLAVE_CONF_NEXT_MASK GENMASK(31, 31) > #define FSI_SLAVE_CONF_SLOTS_MASK GENMASK(23, 16) > #define FSI_SLAVE_CONF_SLOTS_SHIFT 16 > @@ -95,6 +92,9 @@ struct fsi_slave { > u8 t_echo_delay; > }; > > +#define CREATE_TRACE_POINTS > +#include <trace/events/fsi.h> > + > #define to_fsi_master(d) container_of(d, struct fsi_master, dev) > #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev) > > @@ -524,6 +524,8 @@ static int fsi_slave_scan(struct fsi_slave *slave) > dev->addr = engine_addr; > dev->size = slots * engine_page_size; > > + trace_fsi_dev_init(dev); > + > dev_dbg(&slave->dev, > "engine[%i]: type %x, version %x, addr %x size %x\n", > dev->unit, dev->engine_type, version, > @@ -953,6 +955,7 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type, > if (id >= 0) { > *out_index = fsi_adjust_index(cid); > *out_dev = fsi_base_dev + id; > + trace_fsi_minor(cid, type, true, cid); > return 0; > } > /* Other failure */ > @@ -966,6 +969,7 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type, > return id; > *out_index = fsi_adjust_index(id); > *out_dev = fsi_base_dev + id; > + trace_fsi_minor(cid, type, false, id); > return 0; > } > > @@ -1006,6 +1010,7 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) > > crc = crc4(0, cfam_id, 32); > if (crc) { > + trace_fsi_slave_invalid_cfam(master, link, cfam_id); > dev_warn(&master->dev, "slave %02x:%02x invalid cfam id CRC!\n", > link, id); > return -EIO; > @@ -1080,6 +1085,8 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) > if (rc) > goto err_free; > > + trace_fsi_slave_init(slave); > + > /* Create chardev for userspace access */ > cdev_init(&slave->cdev, &cfam_fops); > rc = cdev_device_add(&slave->cdev, &slave->dev); > diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c > index 8606e55c1721..04fec1aab23c 100644 > --- a/drivers/fsi/fsi-master-aspeed.c > +++ b/drivers/fsi/fsi-master-aspeed.c > @@ -449,11 +449,13 @@ static ssize_t cfam_reset_store(struct device *dev, struct device_attribute *att > { > struct fsi_master_aspeed *aspeed = dev_get_drvdata(dev); > > + trace_fsi_master_aspeed_cfam_reset(true); > mutex_lock(&aspeed->lock); > gpiod_set_value(aspeed->cfam_reset_gpio, 1); > usleep_range(900, 1000); > gpiod_set_value(aspeed->cfam_reset_gpio, 0); > mutex_unlock(&aspeed->lock); > + trace_fsi_master_aspeed_cfam_reset(false); > > return count; > } > diff --git a/include/trace/events/fsi.h b/include/trace/events/fsi.h > index 9832cb8e0eb0..251bc57a8b7f 100644 > --- a/include/trace/events/fsi.h > +++ b/include/trace/events/fsi.h > @@ -122,6 +122,115 @@ TRACE_EVENT(fsi_master_break, > ) > ); > > +TRACE_EVENT(fsi_slave_init, > + TP_PROTO(const struct fsi_slave *slave), > + TP_ARGS(slave), > + TP_STRUCT__entry( > + __field(int, master_idx) > + __field(int, master_n_links) > + __field(int, idx) > + __field(int, link) > + __field(int, chip_id) > + __field(__u32, cfam_id) > + __field(__u32, size) > + ), > + TP_fast_assign( > + __entry->master_idx = slave->master->idx; > + __entry->master_n_links = slave->master->n_links; > + __entry->idx = slave->cdev_idx; > + __entry->link = slave->link; > + __entry->chip_id = slave->chip_id; > + __entry->cfam_id = slave->cfam_id; > + __entry->size = slave->size; > + ), > + TP_printk("fsi%d: idx:%d link:%d/%d cid:%d cfam:%08x %08x", > + __entry->master_idx, > + __entry->idx, > + __entry->link, > + __entry->master_n_links, > + __entry->chip_id, > + __entry->cfam_id, > + __entry->size > + ) > +); > + > +TRACE_EVENT(fsi_slave_invalid_cfam, > + TP_PROTO(const struct fsi_master *master, int link, uint32_t cfam_id), > + TP_ARGS(master, link, cfam_id), > + TP_STRUCT__entry( > + __field(int, master_idx) > + __field(int, master_n_links) > + __field(int, link) > + __field(__u32, cfam_id) > + ), > + TP_fast_assign( > + __entry->master_idx = master->idx; > + __entry->master_n_links = master->n_links; > + __entry->link = link; > + __entry->cfam_id = cfam_id; > + ), > + TP_printk("fsi%d: cfam:%08x link:%d/%d", > + __entry->master_idx, > + __entry->cfam_id, > + __entry->link, > + __entry->master_n_links > + ) > +); > + > +TRACE_EVENT(fsi_minor, > + TP_PROTO(int cid, enum fsi_dev_type type, bool legacy, int result), > + TP_ARGS(cid, type, legacy, result), > + TP_STRUCT__entry( > + __field(int, cid) > + __field(int, type) > + __field(bool, legacy) > + __field(int, result) > + ), > + TP_fast_assign( > + __entry->cid = cid; > + __entry->type = type; > + __entry->legacy = legacy; > + __entry->result = result; > + ), > + TP_printk("%d: cid:%d type:%d%s", > + __entry->result, > + __entry->cid, > + __entry->type, > + __entry->legacy ? " legacy" : "" > + ) > +); > + > +TRACE_EVENT(fsi_dev_init, > + TP_PROTO(const struct fsi_device *dev), > + TP_ARGS(dev), > + TP_STRUCT__entry( > + __field(int, master_idx) > + __field(int, link) > + __field(int, type) > + __field(int, unit) > + __field(int, version) > + __field(__u32, addr) > + __field(__u32, size) > + ), > + TP_fast_assign( > + __entry->master_idx = dev->slave->master->idx; > + __entry->link = dev->slave->link; > + __entry->type = dev->engine_type; > + __entry->unit = dev->unit; > + __entry->version = dev->version; > + __entry->addr = dev->addr; > + __entry->size = dev->size; > + ), > + TP_printk("fsi%d: slv%d: t:%02x u:%02x v:%02x %08x@%08x", > + __entry->master_idx, > + __entry->link, > + __entry->type, > + __entry->unit, > + __entry->version, > + __entry->size, > + __entry->addr > + ) > +); > > #endif /* _TRACE_FSI_H */ > > diff --git a/include/trace/events/fsi_master_aspeed.h b/include/trace/events/fsi_master_aspeed.h > index a355ceacc33f..0fff873775f1 100644 > --- a/include/trace/events/fsi_master_aspeed.h > +++ b/include/trace/events/fsi_master_aspeed.h > @@ -72,6 +72,18 @@ TRACE_EVENT(fsi_master_aspeed_opb_error, > ) > ); > > +TRACE_EVENT(fsi_master_aspeed_cfam_reset, > + TP_PROTO(bool start), > + TP_ARGS(start), > + TP_STRUCT__entry( > + __field(bool, start) > + ), > + TP_fast_assign( > + __entry->start = start; > + ), > + TP_printk("%s", __entry->start ? "start" : "end") > +); > + > #endif > > #include <trace/define_trace.h>
On 1/25/22 10:38, Paul Menzel wrote: > Dear Eddie, > > > Am 25.01.22 um 17:11 schrieb Eddie James: >> Add definitions for trace events to show the scanning flow in order >> to debug recent scanning problems. > > Maybe give an example how to trace one of the new trace events. Hi, sure. To enable: echo fsi_slave_init >> /sys/kernel/debug/tracing/set_event To look at the traces: cat /sys/kernel/debug/tracing/trace From one of our systems: openpower-proc--588 [000] .n... 36.544026: fsi_slave_init: fsi0: idx:1 link:0/1 cid:0 cfam:c0020da6 00800000 openpower-proc--588 [001] .n... 36.777409: fsi_slave_init: fsi1: idx:2 link:1/8 cid:1 cfam:c0020da6 00800000 openpower-proc--588 [000] .n... 36.931405: fsi_slave_init: fsi1: idx:3 link:2/8 cid:2 cfam:c0020da6 00800000 openpower-proc--588 [000] .n... 37.202587: fsi_slave_init: fsi1: idx:4 link:3/8 cid:3 cfam:c0020da6 00800000 openpower-proc--588 [000] .n... 37.874995: fsi_slave_init: fsi1: idx:2 link:1/8 cid:1 cfam:c0020da6 00800000 openpower-proc--588 [000] .n... 38.062801: fsi_slave_init: fsi1: idx:3 link:2/8 cid:2 cfam:c0020da6 00800000 openpower-proc--588 [000] .n... 38.335173: fsi_slave_init: fsi1: idx:4 link:3/8 cid:3 cfam:c0020da6 00800000 openpower-proc--679 [000] .n... 39.607437: fsi_slave_init: fsi0: idx:1 link:0/1 cid:0 cfam:c0020da6 00800000 openpower-proc--679 [000] .n... 39.908873: fsi_slave_init: fsi1: idx:2 link:1/8 cid:1 cfam:c0020da6 00800000 openpower-proc--679 [000] .n... 40.275172: fsi_slave_init: fsi1: idx:3 link:2/8 cid:2 cfam:c0020da6 00800000 openpower-proc--679 [000] .n... 40.772409: fsi_slave_init: fsi1: idx:4 link:3/8 cid:3 cfam:c0020da6 00800000 openpower-proc--679 [000] .n... 41.474989: fsi_slave_init: fsi1: idx:2 link:1/8 cid:1 cfam:c0020da6 00800000 openpower-proc--679 [000] .n... 41.749825: fsi_slave_init: fsi1: idx:3 link:2/8 cid:2 cfam:c0020da6 00800000 openpower-proc--679 [001] .n... 42.111040: fsi_slave_init: fsi1: idx:4 link:3/8 cid:3 cfam:c0020da6 00800000 Thanks, Eddie > > > Kind regards, > > Paul > > >> Signed-off-by: Eddie James <eajames@linux.ibm.com> >> --- >> drivers/fsi/fsi-core.c | 13 ++- >> drivers/fsi/fsi-master-aspeed.c | 2 + >> include/trace/events/fsi.h | 109 +++++++++++++++++++++++ >> include/trace/events/fsi_master_aspeed.h | 12 +++ >> 4 files changed, 133 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c >> index 59ddc9fd5bca..78710087aa05 100644 >> --- a/drivers/fsi/fsi-core.c >> +++ b/drivers/fsi/fsi-core.c >> @@ -24,9 +24,6 @@ >> #include "fsi-master.h" >> -#define CREATE_TRACE_POINTS >> -#include <trace/events/fsi.h> >> - >> #define FSI_SLAVE_CONF_NEXT_MASK GENMASK(31, 31) >> #define FSI_SLAVE_CONF_SLOTS_MASK GENMASK(23, 16) >> #define FSI_SLAVE_CONF_SLOTS_SHIFT 16 >> @@ -95,6 +92,9 @@ struct fsi_slave { >> u8 t_echo_delay; >> }; >> +#define CREATE_TRACE_POINTS >> +#include <trace/events/fsi.h> >> + >> #define to_fsi_master(d) container_of(d, struct fsi_master, dev) >> #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev) >> @@ -524,6 +524,8 @@ static int fsi_slave_scan(struct fsi_slave *slave) >> dev->addr = engine_addr; >> dev->size = slots * engine_page_size; >> + trace_fsi_dev_init(dev); >> + >> dev_dbg(&slave->dev, >> "engine[%i]: type %x, version %x, addr %x size %x\n", >> dev->unit, dev->engine_type, version, >> @@ -953,6 +955,7 @@ static int __fsi_get_new_minor(struct fsi_slave >> *slave, enum fsi_dev_type type, >> if (id >= 0) { >> *out_index = fsi_adjust_index(cid); >> *out_dev = fsi_base_dev + id; >> + trace_fsi_minor(cid, type, true, cid); >> return 0; >> } >> /* Other failure */ >> @@ -966,6 +969,7 @@ static int __fsi_get_new_minor(struct fsi_slave >> *slave, enum fsi_dev_type type, >> return id; >> *out_index = fsi_adjust_index(id); >> *out_dev = fsi_base_dev + id; >> + trace_fsi_minor(cid, type, false, id); >> return 0; >> } >> @@ -1006,6 +1010,7 @@ static int fsi_slave_init(struct fsi_master >> *master, int link, uint8_t id) >> crc = crc4(0, cfam_id, 32); >> if (crc) { >> + trace_fsi_slave_invalid_cfam(master, link, cfam_id); >> dev_warn(&master->dev, "slave %02x:%02x invalid cfam id >> CRC!\n", >> link, id); >> return -EIO; >> @@ -1080,6 +1085,8 @@ static int fsi_slave_init(struct fsi_master >> *master, int link, uint8_t id) >> if (rc) >> goto err_free; >> + trace_fsi_slave_init(slave); >> + >> /* Create chardev for userspace access */ >> cdev_init(&slave->cdev, &cfam_fops); >> rc = cdev_device_add(&slave->cdev, &slave->dev); >> diff --git a/drivers/fsi/fsi-master-aspeed.c >> b/drivers/fsi/fsi-master-aspeed.c >> index 8606e55c1721..04fec1aab23c 100644 >> --- a/drivers/fsi/fsi-master-aspeed.c >> +++ b/drivers/fsi/fsi-master-aspeed.c >> @@ -449,11 +449,13 @@ static ssize_t cfam_reset_store(struct device >> *dev, struct device_attribute *att >> { >> struct fsi_master_aspeed *aspeed = dev_get_drvdata(dev); >> + trace_fsi_master_aspeed_cfam_reset(true); >> mutex_lock(&aspeed->lock); >> gpiod_set_value(aspeed->cfam_reset_gpio, 1); >> usleep_range(900, 1000); >> gpiod_set_value(aspeed->cfam_reset_gpio, 0); >> mutex_unlock(&aspeed->lock); >> + trace_fsi_master_aspeed_cfam_reset(false); >> return count; >> } >> diff --git a/include/trace/events/fsi.h b/include/trace/events/fsi.h >> index 9832cb8e0eb0..251bc57a8b7f 100644 >> --- a/include/trace/events/fsi.h >> +++ b/include/trace/events/fsi.h >> @@ -122,6 +122,115 @@ TRACE_EVENT(fsi_master_break, >> ) >> ); >> +TRACE_EVENT(fsi_slave_init, >> + TP_PROTO(const struct fsi_slave *slave), >> + TP_ARGS(slave), >> + TP_STRUCT__entry( >> + __field(int, master_idx) >> + __field(int, master_n_links) >> + __field(int, idx) >> + __field(int, link) >> + __field(int, chip_id) >> + __field(__u32, cfam_id) >> + __field(__u32, size) >> + ), >> + TP_fast_assign( >> + __entry->master_idx = slave->master->idx; >> + __entry->master_n_links = slave->master->n_links; >> + __entry->idx = slave->cdev_idx; >> + __entry->link = slave->link; >> + __entry->chip_id = slave->chip_id; >> + __entry->cfam_id = slave->cfam_id; >> + __entry->size = slave->size; >> + ), >> + TP_printk("fsi%d: idx:%d link:%d/%d cid:%d cfam:%08x %08x", >> + __entry->master_idx, >> + __entry->idx, >> + __entry->link, >> + __entry->master_n_links, >> + __entry->chip_id, >> + __entry->cfam_id, >> + __entry->size >> + ) >> +); >> + >> +TRACE_EVENT(fsi_slave_invalid_cfam, >> + TP_PROTO(const struct fsi_master *master, int link, uint32_t >> cfam_id), >> + TP_ARGS(master, link, cfam_id), >> + TP_STRUCT__entry( >> + __field(int, master_idx) >> + __field(int, master_n_links) >> + __field(int, link) >> + __field(__u32, cfam_id) >> + ), >> + TP_fast_assign( >> + __entry->master_idx = master->idx; >> + __entry->master_n_links = master->n_links; >> + __entry->link = link; >> + __entry->cfam_id = cfam_id; >> + ), >> + TP_printk("fsi%d: cfam:%08x link:%d/%d", >> + __entry->master_idx, >> + __entry->cfam_id, >> + __entry->link, >> + __entry->master_n_links >> + ) >> +); >> + >> +TRACE_EVENT(fsi_minor, >> + TP_PROTO(int cid, enum fsi_dev_type type, bool legacy, int result), >> + TP_ARGS(cid, type, legacy, result), >> + TP_STRUCT__entry( >> + __field(int, cid) >> + __field(int, type) >> + __field(bool, legacy) >> + __field(int, result) >> + ), >> + TP_fast_assign( >> + __entry->cid = cid; >> + __entry->type = type; >> + __entry->legacy = legacy; >> + __entry->result = result; >> + ), >> + TP_printk("%d: cid:%d type:%d%s", >> + __entry->result, >> + __entry->cid, >> + __entry->type, >> + __entry->legacy ? " legacy" : "" >> + ) >> +); >> + >> +TRACE_EVENT(fsi_dev_init, >> + TP_PROTO(const struct fsi_device *dev), >> + TP_ARGS(dev), >> + TP_STRUCT__entry( >> + __field(int, master_idx) >> + __field(int, link) >> + __field(int, type) >> + __field(int, unit) >> + __field(int, version) >> + __field(__u32, addr) >> + __field(__u32, size) >> + ), >> + TP_fast_assign( >> + __entry->master_idx = dev->slave->master->idx; >> + __entry->link = dev->slave->link; >> + __entry->type = dev->engine_type; >> + __entry->unit = dev->unit; >> + __entry->version = dev->version; >> + __entry->addr = dev->addr; >> + __entry->size = dev->size; >> + ), >> + TP_printk("fsi%d: slv%d: t:%02x u:%02x v:%02x %08x@%08x", >> + __entry->master_idx, >> + __entry->link, >> + __entry->type, >> + __entry->unit, >> + __entry->version, >> + __entry->size, >> + __entry->addr >> + ) >> +); >> #endif /* _TRACE_FSI_H */ >> diff --git a/include/trace/events/fsi_master_aspeed.h >> b/include/trace/events/fsi_master_aspeed.h >> index a355ceacc33f..0fff873775f1 100644 >> --- a/include/trace/events/fsi_master_aspeed.h >> +++ b/include/trace/events/fsi_master_aspeed.h >> @@ -72,6 +72,18 @@ TRACE_EVENT(fsi_master_aspeed_opb_error, >> ) >> ); >> +TRACE_EVENT(fsi_master_aspeed_cfam_reset, >> + TP_PROTO(bool start), >> + TP_ARGS(start), >> + TP_STRUCT__entry( >> + __field(bool, start) >> + ), >> + TP_fast_assign( >> + __entry->start = start; >> + ), >> + TP_printk("%s", __entry->start ? "start" : "end") >> +); >> + >> #endif >> #include <trace/define_trace.h>
On Tue, 25 Jan 2022 at 16:11, Eddie James <eajames@linux.ibm.com> wrote: > > Add definitions for trace events to show the scanning flow in order > to debug recent scanning problems. Do you intend this to be merged upstream? Some of the traces look like they might be useful in a general sense, but others (trace_fsi_minor) look like they might be just debugging? > > Signed-off-by: Eddie James <eajames@linux.ibm.com> > --- > drivers/fsi/fsi-core.c | 13 ++- > drivers/fsi/fsi-master-aspeed.c | 2 + > include/trace/events/fsi.h | 109 +++++++++++++++++++++++ > include/trace/events/fsi_master_aspeed.h | 12 +++ > 4 files changed, 133 insertions(+), 3 deletions(-) > > diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c > index 59ddc9fd5bca..78710087aa05 100644 > --- a/drivers/fsi/fsi-core.c > +++ b/drivers/fsi/fsi-core.c > @@ -24,9 +24,6 @@ > > #include "fsi-master.h" > > -#define CREATE_TRACE_POINTS > -#include <trace/events/fsi.h> > - > #define FSI_SLAVE_CONF_NEXT_MASK GENMASK(31, 31) > #define FSI_SLAVE_CONF_SLOTS_MASK GENMASK(23, 16) > #define FSI_SLAVE_CONF_SLOTS_SHIFT 16 > @@ -95,6 +92,9 @@ struct fsi_slave { > u8 t_echo_delay; > }; > > +#define CREATE_TRACE_POINTS > +#include <trace/events/fsi.h> Did this move for a reason? > + > #define to_fsi_master(d) container_of(d, struct fsi_master, dev) > #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev) > > @@ -524,6 +524,8 @@ static int fsi_slave_scan(struct fsi_slave *slave) > dev->addr = engine_addr; > dev->size = slots * engine_page_size; > > + trace_fsi_dev_init(dev); > + > dev_dbg(&slave->dev, > "engine[%i]: type %x, version %x, addr %x size %x\n", > dev->unit, dev->engine_type, version, > @@ -953,6 +955,7 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type, > if (id >= 0) { > *out_index = fsi_adjust_index(cid); > *out_dev = fsi_base_dev + id; > + trace_fsi_minor(cid, type, true, cid); > return 0; > } > /* Other failure */ > @@ -966,6 +969,7 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type, > return id; > *out_index = fsi_adjust_index(id); > *out_dev = fsi_base_dev + id; > + trace_fsi_minor(cid, type, false, id); > return 0; > } > > @@ -1006,6 +1010,7 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) > > crc = crc4(0, cfam_id, 32); > if (crc) { > + trace_fsi_slave_invalid_cfam(master, link, cfam_id); > dev_warn(&master->dev, "slave %02x:%02x invalid cfam id CRC!\n", > link, id); > return -EIO; > @@ -1080,6 +1085,8 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) > if (rc) > goto err_free; > > + trace_fsi_slave_init(slave); > + > /* Create chardev for userspace access */ > cdev_init(&slave->cdev, &cfam_fops); > rc = cdev_device_add(&slave->cdev, &slave->dev); > diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c > index 8606e55c1721..04fec1aab23c 100644 > --- a/drivers/fsi/fsi-master-aspeed.c > +++ b/drivers/fsi/fsi-master-aspeed.c > @@ -449,11 +449,13 @@ static ssize_t cfam_reset_store(struct device *dev, struct device_attribute *att > { > struct fsi_master_aspeed *aspeed = dev_get_drvdata(dev); > > + trace_fsi_master_aspeed_cfam_reset(true); > mutex_lock(&aspeed->lock); > gpiod_set_value(aspeed->cfam_reset_gpio, 1); > usleep_range(900, 1000); > gpiod_set_value(aspeed->cfam_reset_gpio, 0); > mutex_unlock(&aspeed->lock); > + trace_fsi_master_aspeed_cfam_reset(false); > > return count; > } > diff --git a/include/trace/events/fsi.h b/include/trace/events/fsi.h > index 9832cb8e0eb0..251bc57a8b7f 100644 > --- a/include/trace/events/fsi.h > +++ b/include/trace/events/fsi.h > @@ -122,6 +122,115 @@ TRACE_EVENT(fsi_master_break, > ) > ); > > +TRACE_EVENT(fsi_slave_init, > + TP_PROTO(const struct fsi_slave *slave), > + TP_ARGS(slave), > + TP_STRUCT__entry( > + __field(int, master_idx) > + __field(int, master_n_links) > + __field(int, idx) > + __field(int, link) > + __field(int, chip_id) > + __field(__u32, cfam_id) > + __field(__u32, size) > + ), > + TP_fast_assign( > + __entry->master_idx = slave->master->idx; > + __entry->master_n_links = slave->master->n_links; > + __entry->idx = slave->cdev_idx; > + __entry->link = slave->link; > + __entry->chip_id = slave->chip_id; > + __entry->cfam_id = slave->cfam_id; > + __entry->size = slave->size; > + ), > + TP_printk("fsi%d: idx:%d link:%d/%d cid:%d cfam:%08x %08x", > + __entry->master_idx, > + __entry->idx, > + __entry->link, > + __entry->master_n_links, > + __entry->chip_id, > + __entry->cfam_id, > + __entry->size > + ) > +); > + > +TRACE_EVENT(fsi_slave_invalid_cfam, > + TP_PROTO(const struct fsi_master *master, int link, uint32_t cfam_id), > + TP_ARGS(master, link, cfam_id), > + TP_STRUCT__entry( > + __field(int, master_idx) > + __field(int, master_n_links) > + __field(int, link) > + __field(__u32, cfam_id) > + ), > + TP_fast_assign( > + __entry->master_idx = master->idx; > + __entry->master_n_links = master->n_links; > + __entry->link = link; > + __entry->cfam_id = cfam_id; > + ), > + TP_printk("fsi%d: cfam:%08x link:%d/%d", > + __entry->master_idx, > + __entry->cfam_id, > + __entry->link, > + __entry->master_n_links > + ) > +); > + > +TRACE_EVENT(fsi_minor, > + TP_PROTO(int cid, enum fsi_dev_type type, bool legacy, int result), > + TP_ARGS(cid, type, legacy, result), > + TP_STRUCT__entry( > + __field(int, cid) > + __field(int, type) > + __field(bool, legacy) > + __field(int, result) > + ), > + TP_fast_assign( > + __entry->cid = cid; > + __entry->type = type; > + __entry->legacy = legacy; > + __entry->result = result; > + ), > + TP_printk("%d: cid:%d type:%d%s", > + __entry->result, > + __entry->cid, > + __entry->type, > + __entry->legacy ? " legacy" : "" > + ) > +); > + > +TRACE_EVENT(fsi_dev_init, > + TP_PROTO(const struct fsi_device *dev), > + TP_ARGS(dev), > + TP_STRUCT__entry( > + __field(int, master_idx) > + __field(int, link) > + __field(int, type) > + __field(int, unit) > + __field(int, version) > + __field(__u32, addr) > + __field(__u32, size) > + ), > + TP_fast_assign( > + __entry->master_idx = dev->slave->master->idx; > + __entry->link = dev->slave->link; > + __entry->type = dev->engine_type; > + __entry->unit = dev->unit; > + __entry->version = dev->version; > + __entry->addr = dev->addr; > + __entry->size = dev->size; > + ), > + TP_printk("fsi%d: slv%d: t:%02x u:%02x v:%02x %08x@%08x", > + __entry->master_idx, > + __entry->link, > + __entry->type, > + __entry->unit, > + __entry->version, > + __entry->size, > + __entry->addr > + ) > +); > > #endif /* _TRACE_FSI_H */ > > diff --git a/include/trace/events/fsi_master_aspeed.h b/include/trace/events/fsi_master_aspeed.h > index a355ceacc33f..0fff873775f1 100644 > --- a/include/trace/events/fsi_master_aspeed.h > +++ b/include/trace/events/fsi_master_aspeed.h > @@ -72,6 +72,18 @@ TRACE_EVENT(fsi_master_aspeed_opb_error, > ) > ); > > +TRACE_EVENT(fsi_master_aspeed_cfam_reset, > + TP_PROTO(bool start), > + TP_ARGS(start), > + TP_STRUCT__entry( > + __field(bool, start) > + ), > + TP_fast_assign( > + __entry->start = start; > + ), > + TP_printk("%s", __entry->start ? "start" : "end") > +); > + > #endif > > #include <trace/define_trace.h> > -- > 2.27.0 >
On 1/30/22 23:59, Joel Stanley wrote: > On Tue, 25 Jan 2022 at 16:11, Eddie James <eajames@linux.ibm.com> wrote: >> Add definitions for trace events to show the scanning flow in order >> to debug recent scanning problems. > Do you intend this to be merged upstream? Was planning to send it up, yes. > > Some of the traces look like they might be useful in a general sense, > but others (trace_fsi_minor) look like they might be just debugging? Yea its purely for debugging. I could drop that one. > >> Signed-off-by: Eddie James <eajames@linux.ibm.com> >> --- >> drivers/fsi/fsi-core.c | 13 ++- >> drivers/fsi/fsi-master-aspeed.c | 2 + >> include/trace/events/fsi.h | 109 +++++++++++++++++++++++ >> include/trace/events/fsi_master_aspeed.h | 12 +++ >> 4 files changed, 133 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c >> index 59ddc9fd5bca..78710087aa05 100644 >> --- a/drivers/fsi/fsi-core.c >> +++ b/drivers/fsi/fsi-core.c >> @@ -24,9 +24,6 @@ >> >> #include "fsi-master.h" >> >> -#define CREATE_TRACE_POINTS >> -#include <trace/events/fsi.h> >> - >> #define FSI_SLAVE_CONF_NEXT_MASK GENMASK(31, 31) >> #define FSI_SLAVE_CONF_SLOTS_MASK GENMASK(23, 16) >> #define FSI_SLAVE_CONF_SLOTS_SHIFT 16 >> @@ -95,6 +92,9 @@ struct fsi_slave { >> u8 t_echo_delay; >> }; >> >> +#define CREATE_TRACE_POINTS >> +#include <trace/events/fsi.h> > Did this move for a reason? Yes, I wanted to use the fsi_slave structure in the trace event, so I had to include the traces after the structure definition. Thanks, Eddie > >> + >> #define to_fsi_master(d) container_of(d, struct fsi_master, dev) >> #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev) >> >> @@ -524,6 +524,8 @@ static int fsi_slave_scan(struct fsi_slave *slave) >> dev->addr = engine_addr; >> dev->size = slots * engine_page_size; >> >> + trace_fsi_dev_init(dev); >> + >> dev_dbg(&slave->dev, >> "engine[%i]: type %x, version %x, addr %x size %x\n", >> dev->unit, dev->engine_type, version, >> @@ -953,6 +955,7 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type, >> if (id >= 0) { >> *out_index = fsi_adjust_index(cid); >> *out_dev = fsi_base_dev + id; >> + trace_fsi_minor(cid, type, true, cid); >> return 0; >> } >> /* Other failure */ >> @@ -966,6 +969,7 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type, >> return id; >> *out_index = fsi_adjust_index(id); >> *out_dev = fsi_base_dev + id; >> + trace_fsi_minor(cid, type, false, id); >> return 0; >> } >> >> @@ -1006,6 +1010,7 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) >> >> crc = crc4(0, cfam_id, 32); >> if (crc) { >> + trace_fsi_slave_invalid_cfam(master, link, cfam_id); >> dev_warn(&master->dev, "slave %02x:%02x invalid cfam id CRC!\n", >> link, id); >> return -EIO; >> @@ -1080,6 +1085,8 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) >> if (rc) >> goto err_free; >> >> + trace_fsi_slave_init(slave); >> + >> /* Create chardev for userspace access */ >> cdev_init(&slave->cdev, &cfam_fops); >> rc = cdev_device_add(&slave->cdev, &slave->dev); >> diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c >> index 8606e55c1721..04fec1aab23c 100644 >> --- a/drivers/fsi/fsi-master-aspeed.c >> +++ b/drivers/fsi/fsi-master-aspeed.c >> @@ -449,11 +449,13 @@ static ssize_t cfam_reset_store(struct device *dev, struct device_attribute *att >> { >> struct fsi_master_aspeed *aspeed = dev_get_drvdata(dev); >> >> + trace_fsi_master_aspeed_cfam_reset(true); >> mutex_lock(&aspeed->lock); >> gpiod_set_value(aspeed->cfam_reset_gpio, 1); >> usleep_range(900, 1000); >> gpiod_set_value(aspeed->cfam_reset_gpio, 0); >> mutex_unlock(&aspeed->lock); >> + trace_fsi_master_aspeed_cfam_reset(false); >> >> return count; >> } >> diff --git a/include/trace/events/fsi.h b/include/trace/events/fsi.h >> index 9832cb8e0eb0..251bc57a8b7f 100644 >> --- a/include/trace/events/fsi.h >> +++ b/include/trace/events/fsi.h >> @@ -122,6 +122,115 @@ TRACE_EVENT(fsi_master_break, >> ) >> ); >> >> +TRACE_EVENT(fsi_slave_init, >> + TP_PROTO(const struct fsi_slave *slave), >> + TP_ARGS(slave), >> + TP_STRUCT__entry( >> + __field(int, master_idx) >> + __field(int, master_n_links) >> + __field(int, idx) >> + __field(int, link) >> + __field(int, chip_id) >> + __field(__u32, cfam_id) >> + __field(__u32, size) >> + ), >> + TP_fast_assign( >> + __entry->master_idx = slave->master->idx; >> + __entry->master_n_links = slave->master->n_links; >> + __entry->idx = slave->cdev_idx; >> + __entry->link = slave->link; >> + __entry->chip_id = slave->chip_id; >> + __entry->cfam_id = slave->cfam_id; >> + __entry->size = slave->size; >> + ), >> + TP_printk("fsi%d: idx:%d link:%d/%d cid:%d cfam:%08x %08x", >> + __entry->master_idx, >> + __entry->idx, >> + __entry->link, >> + __entry->master_n_links, >> + __entry->chip_id, >> + __entry->cfam_id, >> + __entry->size >> + ) >> +); >> + >> +TRACE_EVENT(fsi_slave_invalid_cfam, >> + TP_PROTO(const struct fsi_master *master, int link, uint32_t cfam_id), >> + TP_ARGS(master, link, cfam_id), >> + TP_STRUCT__entry( >> + __field(int, master_idx) >> + __field(int, master_n_links) >> + __field(int, link) >> + __field(__u32, cfam_id) >> + ), >> + TP_fast_assign( >> + __entry->master_idx = master->idx; >> + __entry->master_n_links = master->n_links; >> + __entry->link = link; >> + __entry->cfam_id = cfam_id; >> + ), >> + TP_printk("fsi%d: cfam:%08x link:%d/%d", >> + __entry->master_idx, >> + __entry->cfam_id, >> + __entry->link, >> + __entry->master_n_links >> + ) >> +); >> + >> +TRACE_EVENT(fsi_minor, >> + TP_PROTO(int cid, enum fsi_dev_type type, bool legacy, int result), >> + TP_ARGS(cid, type, legacy, result), >> + TP_STRUCT__entry( >> + __field(int, cid) >> + __field(int, type) >> + __field(bool, legacy) >> + __field(int, result) >> + ), >> + TP_fast_assign( >> + __entry->cid = cid; >> + __entry->type = type; >> + __entry->legacy = legacy; >> + __entry->result = result; >> + ), >> + TP_printk("%d: cid:%d type:%d%s", >> + __entry->result, >> + __entry->cid, >> + __entry->type, >> + __entry->legacy ? " legacy" : "" >> + ) >> +); >> + >> +TRACE_EVENT(fsi_dev_init, >> + TP_PROTO(const struct fsi_device *dev), >> + TP_ARGS(dev), >> + TP_STRUCT__entry( >> + __field(int, master_idx) >> + __field(int, link) >> + __field(int, type) >> + __field(int, unit) >> + __field(int, version) >> + __field(__u32, addr) >> + __field(__u32, size) >> + ), >> + TP_fast_assign( >> + __entry->master_idx = dev->slave->master->idx; >> + __entry->link = dev->slave->link; >> + __entry->type = dev->engine_type; >> + __entry->unit = dev->unit; >> + __entry->version = dev->version; >> + __entry->addr = dev->addr; >> + __entry->size = dev->size; >> + ), >> + TP_printk("fsi%d: slv%d: t:%02x u:%02x v:%02x %08x@%08x", >> + __entry->master_idx, >> + __entry->link, >> + __entry->type, >> + __entry->unit, >> + __entry->version, >> + __entry->size, >> + __entry->addr >> + ) >> +); >> >> #endif /* _TRACE_FSI_H */ >> >> diff --git a/include/trace/events/fsi_master_aspeed.h b/include/trace/events/fsi_master_aspeed.h >> index a355ceacc33f..0fff873775f1 100644 >> --- a/include/trace/events/fsi_master_aspeed.h >> +++ b/include/trace/events/fsi_master_aspeed.h >> @@ -72,6 +72,18 @@ TRACE_EVENT(fsi_master_aspeed_opb_error, >> ) >> ); >> >> +TRACE_EVENT(fsi_master_aspeed_cfam_reset, >> + TP_PROTO(bool start), >> + TP_ARGS(start), >> + TP_STRUCT__entry( >> + __field(bool, start) >> + ), >> + TP_fast_assign( >> + __entry->start = start; >> + ), >> + TP_printk("%s", __entry->start ? "start" : "end") >> +); >> + >> #endif >> >> #include <trace/define_trace.h> >> -- >> 2.27.0 >>
On Mon, 31 Jan 2022 at 15:08, Eddie James <eajames@linux.ibm.com> wrote: > > > On 1/30/22 23:59, Joel Stanley wrote: > > On Tue, 25 Jan 2022 at 16:11, Eddie James <eajames@linux.ibm.com> wrote: > >> Add definitions for trace events to show the scanning flow in order > >> to debug recent scanning problems. > > Do you intend this to be merged upstream? > > > Was planning to send it up, yes. > > > > > > Some of the traces look like they might be useful in a general sense, > > but others (trace_fsi_minor) look like they might be just debugging? > > > Yea its purely for debugging. I could drop that one. Can you send a v2 on the upstream list so we can get this merged? Cheers, Joel > > > > > >> Signed-off-by: Eddie James <eajames@linux.ibm.com> > >> --- > >> drivers/fsi/fsi-core.c | 13 ++- > >> drivers/fsi/fsi-master-aspeed.c | 2 + > >> include/trace/events/fsi.h | 109 +++++++++++++++++++++++ > >> include/trace/events/fsi_master_aspeed.h | 12 +++ > >> 4 files changed, 133 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c > >> index 59ddc9fd5bca..78710087aa05 100644 > >> --- a/drivers/fsi/fsi-core.c > >> +++ b/drivers/fsi/fsi-core.c > >> @@ -24,9 +24,6 @@ > >> > >> #include "fsi-master.h" > >> > >> -#define CREATE_TRACE_POINTS > >> -#include <trace/events/fsi.h> > >> - > >> #define FSI_SLAVE_CONF_NEXT_MASK GENMASK(31, 31) > >> #define FSI_SLAVE_CONF_SLOTS_MASK GENMASK(23, 16) > >> #define FSI_SLAVE_CONF_SLOTS_SHIFT 16 > >> @@ -95,6 +92,9 @@ struct fsi_slave { > >> u8 t_echo_delay; > >> }; > >> > >> +#define CREATE_TRACE_POINTS > >> +#include <trace/events/fsi.h> > > Did this move for a reason? > > > Yes, I wanted to use the fsi_slave structure in the trace event, so I > had to include the traces after the structure definition. > > > Thanks, > > Eddie > > > > > >> + > >> #define to_fsi_master(d) container_of(d, struct fsi_master, dev) > >> #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev) > >> > >> @@ -524,6 +524,8 @@ static int fsi_slave_scan(struct fsi_slave *slave) > >> dev->addr = engine_addr; > >> dev->size = slots * engine_page_size; > >> > >> + trace_fsi_dev_init(dev); > >> + > >> dev_dbg(&slave->dev, > >> "engine[%i]: type %x, version %x, addr %x size %x\n", > >> dev->unit, dev->engine_type, version, > >> @@ -953,6 +955,7 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type, > >> if (id >= 0) { > >> *out_index = fsi_adjust_index(cid); > >> *out_dev = fsi_base_dev + id; > >> + trace_fsi_minor(cid, type, true, cid); > >> return 0; > >> } > >> /* Other failure */ > >> @@ -966,6 +969,7 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type, > >> return id; > >> *out_index = fsi_adjust_index(id); > >> *out_dev = fsi_base_dev + id; > >> + trace_fsi_minor(cid, type, false, id); > >> return 0; > >> } > >> > >> @@ -1006,6 +1010,7 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) > >> > >> crc = crc4(0, cfam_id, 32); > >> if (crc) { > >> + trace_fsi_slave_invalid_cfam(master, link, cfam_id); > >> dev_warn(&master->dev, "slave %02x:%02x invalid cfam id CRC!\n", > >> link, id); > >> return -EIO; > >> @@ -1080,6 +1085,8 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) > >> if (rc) > >> goto err_free; > >> > >> + trace_fsi_slave_init(slave); > >> + > >> /* Create chardev for userspace access */ > >> cdev_init(&slave->cdev, &cfam_fops); > >> rc = cdev_device_add(&slave->cdev, &slave->dev); > >> diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c > >> index 8606e55c1721..04fec1aab23c 100644 > >> --- a/drivers/fsi/fsi-master-aspeed.c > >> +++ b/drivers/fsi/fsi-master-aspeed.c > >> @@ -449,11 +449,13 @@ static ssize_t cfam_reset_store(struct device *dev, struct device_attribute *att > >> { > >> struct fsi_master_aspeed *aspeed = dev_get_drvdata(dev); > >> > >> + trace_fsi_master_aspeed_cfam_reset(true); > >> mutex_lock(&aspeed->lock); > >> gpiod_set_value(aspeed->cfam_reset_gpio, 1); > >> usleep_range(900, 1000); > >> gpiod_set_value(aspeed->cfam_reset_gpio, 0); > >> mutex_unlock(&aspeed->lock); > >> + trace_fsi_master_aspeed_cfam_reset(false); > >> > >> return count; > >> } > >> diff --git a/include/trace/events/fsi.h b/include/trace/events/fsi.h > >> index 9832cb8e0eb0..251bc57a8b7f 100644 > >> --- a/include/trace/events/fsi.h > >> +++ b/include/trace/events/fsi.h > >> @@ -122,6 +122,115 @@ TRACE_EVENT(fsi_master_break, > >> ) > >> ); > >> > >> +TRACE_EVENT(fsi_slave_init, > >> + TP_PROTO(const struct fsi_slave *slave), > >> + TP_ARGS(slave), > >> + TP_STRUCT__entry( > >> + __field(int, master_idx) > >> + __field(int, master_n_links) > >> + __field(int, idx) > >> + __field(int, link) > >> + __field(int, chip_id) > >> + __field(__u32, cfam_id) > >> + __field(__u32, size) > >> + ), > >> + TP_fast_assign( > >> + __entry->master_idx = slave->master->idx; > >> + __entry->master_n_links = slave->master->n_links; > >> + __entry->idx = slave->cdev_idx; > >> + __entry->link = slave->link; > >> + __entry->chip_id = slave->chip_id; > >> + __entry->cfam_id = slave->cfam_id; > >> + __entry->size = slave->size; > >> + ), > >> + TP_printk("fsi%d: idx:%d link:%d/%d cid:%d cfam:%08x %08x", > >> + __entry->master_idx, > >> + __entry->idx, > >> + __entry->link, > >> + __entry->master_n_links, > >> + __entry->chip_id, > >> + __entry->cfam_id, > >> + __entry->size > >> + ) > >> +); > >> + > >> +TRACE_EVENT(fsi_slave_invalid_cfam, > >> + TP_PROTO(const struct fsi_master *master, int link, uint32_t cfam_id), > >> + TP_ARGS(master, link, cfam_id), > >> + TP_STRUCT__entry( > >> + __field(int, master_idx) > >> + __field(int, master_n_links) > >> + __field(int, link) > >> + __field(__u32, cfam_id) > >> + ), > >> + TP_fast_assign( > >> + __entry->master_idx = master->idx; > >> + __entry->master_n_links = master->n_links; > >> + __entry->link = link; > >> + __entry->cfam_id = cfam_id; > >> + ), > >> + TP_printk("fsi%d: cfam:%08x link:%d/%d", > >> + __entry->master_idx, > >> + __entry->cfam_id, > >> + __entry->link, > >> + __entry->master_n_links > >> + ) > >> +); > >> + > >> +TRACE_EVENT(fsi_minor, > >> + TP_PROTO(int cid, enum fsi_dev_type type, bool legacy, int result), > >> + TP_ARGS(cid, type, legacy, result), > >> + TP_STRUCT__entry( > >> + __field(int, cid) > >> + __field(int, type) > >> + __field(bool, legacy) > >> + __field(int, result) > >> + ), > >> + TP_fast_assign( > >> + __entry->cid = cid; > >> + __entry->type = type; > >> + __entry->legacy = legacy; > >> + __entry->result = result; > >> + ), > >> + TP_printk("%d: cid:%d type:%d%s", > >> + __entry->result, > >> + __entry->cid, > >> + __entry->type, > >> + __entry->legacy ? " legacy" : "" > >> + ) > >> +); > >> + > >> +TRACE_EVENT(fsi_dev_init, > >> + TP_PROTO(const struct fsi_device *dev), > >> + TP_ARGS(dev), > >> + TP_STRUCT__entry( > >> + __field(int, master_idx) > >> + __field(int, link) > >> + __field(int, type) > >> + __field(int, unit) > >> + __field(int, version) > >> + __field(__u32, addr) > >> + __field(__u32, size) > >> + ), > >> + TP_fast_assign( > >> + __entry->master_idx = dev->slave->master->idx; > >> + __entry->link = dev->slave->link; > >> + __entry->type = dev->engine_type; > >> + __entry->unit = dev->unit; > >> + __entry->version = dev->version; > >> + __entry->addr = dev->addr; > >> + __entry->size = dev->size; > >> + ), > >> + TP_printk("fsi%d: slv%d: t:%02x u:%02x v:%02x %08x@%08x", > >> + __entry->master_idx, > >> + __entry->link, > >> + __entry->type, > >> + __entry->unit, > >> + __entry->version, > >> + __entry->size, > >> + __entry->addr > >> + ) > >> +); > >> > >> #endif /* _TRACE_FSI_H */ > >> > >> diff --git a/include/trace/events/fsi_master_aspeed.h b/include/trace/events/fsi_master_aspeed.h > >> index a355ceacc33f..0fff873775f1 100644 > >> --- a/include/trace/events/fsi_master_aspeed.h > >> +++ b/include/trace/events/fsi_master_aspeed.h > >> @@ -72,6 +72,18 @@ TRACE_EVENT(fsi_master_aspeed_opb_error, > >> ) > >> ); > >> > >> +TRACE_EVENT(fsi_master_aspeed_cfam_reset, > >> + TP_PROTO(bool start), > >> + TP_ARGS(start), > >> + TP_STRUCT__entry( > >> + __field(bool, start) > >> + ), > >> + TP_fast_assign( > >> + __entry->start = start; > >> + ), > >> + TP_printk("%s", __entry->start ? "start" : "end") > >> +); > >> + > >> #endif > >> > >> #include <trace/define_trace.h> > >> -- > >> 2.27.0 > >>
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c index 59ddc9fd5bca..78710087aa05 100644 --- a/drivers/fsi/fsi-core.c +++ b/drivers/fsi/fsi-core.c @@ -24,9 +24,6 @@ #include "fsi-master.h" -#define CREATE_TRACE_POINTS -#include <trace/events/fsi.h> - #define FSI_SLAVE_CONF_NEXT_MASK GENMASK(31, 31) #define FSI_SLAVE_CONF_SLOTS_MASK GENMASK(23, 16) #define FSI_SLAVE_CONF_SLOTS_SHIFT 16 @@ -95,6 +92,9 @@ struct fsi_slave { u8 t_echo_delay; }; +#define CREATE_TRACE_POINTS +#include <trace/events/fsi.h> + #define to_fsi_master(d) container_of(d, struct fsi_master, dev) #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev) @@ -524,6 +524,8 @@ static int fsi_slave_scan(struct fsi_slave *slave) dev->addr = engine_addr; dev->size = slots * engine_page_size; + trace_fsi_dev_init(dev); + dev_dbg(&slave->dev, "engine[%i]: type %x, version %x, addr %x size %x\n", dev->unit, dev->engine_type, version, @@ -953,6 +955,7 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type, if (id >= 0) { *out_index = fsi_adjust_index(cid); *out_dev = fsi_base_dev + id; + trace_fsi_minor(cid, type, true, cid); return 0; } /* Other failure */ @@ -966,6 +969,7 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type, return id; *out_index = fsi_adjust_index(id); *out_dev = fsi_base_dev + id; + trace_fsi_minor(cid, type, false, id); return 0; } @@ -1006,6 +1010,7 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) crc = crc4(0, cfam_id, 32); if (crc) { + trace_fsi_slave_invalid_cfam(master, link, cfam_id); dev_warn(&master->dev, "slave %02x:%02x invalid cfam id CRC!\n", link, id); return -EIO; @@ -1080,6 +1085,8 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) if (rc) goto err_free; + trace_fsi_slave_init(slave); + /* Create chardev for userspace access */ cdev_init(&slave->cdev, &cfam_fops); rc = cdev_device_add(&slave->cdev, &slave->dev); diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c index 8606e55c1721..04fec1aab23c 100644 --- a/drivers/fsi/fsi-master-aspeed.c +++ b/drivers/fsi/fsi-master-aspeed.c @@ -449,11 +449,13 @@ static ssize_t cfam_reset_store(struct device *dev, struct device_attribute *att { struct fsi_master_aspeed *aspeed = dev_get_drvdata(dev); + trace_fsi_master_aspeed_cfam_reset(true); mutex_lock(&aspeed->lock); gpiod_set_value(aspeed->cfam_reset_gpio, 1); usleep_range(900, 1000); gpiod_set_value(aspeed->cfam_reset_gpio, 0); mutex_unlock(&aspeed->lock); + trace_fsi_master_aspeed_cfam_reset(false); return count; } diff --git a/include/trace/events/fsi.h b/include/trace/events/fsi.h index 9832cb8e0eb0..251bc57a8b7f 100644 --- a/include/trace/events/fsi.h +++ b/include/trace/events/fsi.h @@ -122,6 +122,115 @@ TRACE_EVENT(fsi_master_break, ) ); +TRACE_EVENT(fsi_slave_init, + TP_PROTO(const struct fsi_slave *slave), + TP_ARGS(slave), + TP_STRUCT__entry( + __field(int, master_idx) + __field(int, master_n_links) + __field(int, idx) + __field(int, link) + __field(int, chip_id) + __field(__u32, cfam_id) + __field(__u32, size) + ), + TP_fast_assign( + __entry->master_idx = slave->master->idx; + __entry->master_n_links = slave->master->n_links; + __entry->idx = slave->cdev_idx; + __entry->link = slave->link; + __entry->chip_id = slave->chip_id; + __entry->cfam_id = slave->cfam_id; + __entry->size = slave->size; + ), + TP_printk("fsi%d: idx:%d link:%d/%d cid:%d cfam:%08x %08x", + __entry->master_idx, + __entry->idx, + __entry->link, + __entry->master_n_links, + __entry->chip_id, + __entry->cfam_id, + __entry->size + ) +); + +TRACE_EVENT(fsi_slave_invalid_cfam, + TP_PROTO(const struct fsi_master *master, int link, uint32_t cfam_id), + TP_ARGS(master, link, cfam_id), + TP_STRUCT__entry( + __field(int, master_idx) + __field(int, master_n_links) + __field(int, link) + __field(__u32, cfam_id) + ), + TP_fast_assign( + __entry->master_idx = master->idx; + __entry->master_n_links = master->n_links; + __entry->link = link; + __entry->cfam_id = cfam_id; + ), + TP_printk("fsi%d: cfam:%08x link:%d/%d", + __entry->master_idx, + __entry->cfam_id, + __entry->link, + __entry->master_n_links + ) +); + +TRACE_EVENT(fsi_minor, + TP_PROTO(int cid, enum fsi_dev_type type, bool legacy, int result), + TP_ARGS(cid, type, legacy, result), + TP_STRUCT__entry( + __field(int, cid) + __field(int, type) + __field(bool, legacy) + __field(int, result) + ), + TP_fast_assign( + __entry->cid = cid; + __entry->type = type; + __entry->legacy = legacy; + __entry->result = result; + ), + TP_printk("%d: cid:%d type:%d%s", + __entry->result, + __entry->cid, + __entry->type, + __entry->legacy ? " legacy" : "" + ) +); + +TRACE_EVENT(fsi_dev_init, + TP_PROTO(const struct fsi_device *dev), + TP_ARGS(dev), + TP_STRUCT__entry( + __field(int, master_idx) + __field(int, link) + __field(int, type) + __field(int, unit) + __field(int, version) + __field(__u32, addr) + __field(__u32, size) + ), + TP_fast_assign( + __entry->master_idx = dev->slave->master->idx; + __entry->link = dev->slave->link; + __entry->type = dev->engine_type; + __entry->unit = dev->unit; + __entry->version = dev->version; + __entry->addr = dev->addr; + __entry->size = dev->size; + ), + TP_printk("fsi%d: slv%d: t:%02x u:%02x v:%02x %08x@%08x", + __entry->master_idx, + __entry->link, + __entry->type, + __entry->unit, + __entry->version, + __entry->size, + __entry->addr + ) +); #endif /* _TRACE_FSI_H */ diff --git a/include/trace/events/fsi_master_aspeed.h b/include/trace/events/fsi_master_aspeed.h index a355ceacc33f..0fff873775f1 100644 --- a/include/trace/events/fsi_master_aspeed.h +++ b/include/trace/events/fsi_master_aspeed.h @@ -72,6 +72,18 @@ TRACE_EVENT(fsi_master_aspeed_opb_error, ) ); +TRACE_EVENT(fsi_master_aspeed_cfam_reset, + TP_PROTO(bool start), + TP_ARGS(start), + TP_STRUCT__entry( + __field(bool, start) + ), + TP_fast_assign( + __entry->start = start; + ), + TP_printk("%s", __entry->start ? "start" : "end") +); + #endif #include <trace/define_trace.h>
Add definitions for trace events to show the scanning flow in order to debug recent scanning problems. Signed-off-by: Eddie James <eajames@linux.ibm.com> --- drivers/fsi/fsi-core.c | 13 ++- drivers/fsi/fsi-master-aspeed.c | 2 + include/trace/events/fsi.h | 109 +++++++++++++++++++++++ include/trace/events/fsi_master_aspeed.h | 12 +++ 4 files changed, 133 insertions(+), 3 deletions(-)