* [PATCH linux dev-5.15] fsi: Add trace events in initialization path
@ 2022-01-25 16:11 Eddie James
2022-01-25 16:38 ` Paul Menzel
2022-01-31 5:59 ` Joel Stanley
0 siblings, 2 replies; 6+ messages in thread
From: Eddie James @ 2022-01-25 16:11 UTC (permalink / raw)
To: openbmc; +Cc: Eddie James
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(-)
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>
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH linux dev-5.15] fsi: Add trace events in initialization path
2022-01-25 16:11 [PATCH linux dev-5.15] fsi: Add trace events in initialization path Eddie James
@ 2022-01-25 16:38 ` Paul Menzel
2022-01-25 19:31 ` Eddie James
2022-01-31 5:59 ` Joel Stanley
1 sibling, 1 reply; 6+ messages in thread
From: Paul Menzel @ 2022-01-25 16:38 UTC (permalink / raw)
To: Eddie James; +Cc: openbmc
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH linux dev-5.15] fsi: Add trace events in initialization path
2022-01-25 16:38 ` Paul Menzel
@ 2022-01-25 19:31 ` Eddie James
0 siblings, 0 replies; 6+ messages in thread
From: Eddie James @ 2022-01-25 19:31 UTC (permalink / raw)
To: Paul Menzel; +Cc: openbmc
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH linux dev-5.15] fsi: Add trace events in initialization path
2022-01-25 16:11 [PATCH linux dev-5.15] fsi: Add trace events in initialization path Eddie James
2022-01-25 16:38 ` Paul Menzel
@ 2022-01-31 5:59 ` Joel Stanley
2022-01-31 15:08 ` Eddie James
1 sibling, 1 reply; 6+ messages in thread
From: Joel Stanley @ 2022-01-31 5:59 UTC (permalink / raw)
To: Eddie James; +Cc: OpenBMC Maillist
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH linux dev-5.15] fsi: Add trace events in initialization path
2022-01-31 5:59 ` Joel Stanley
@ 2022-01-31 15:08 ` Eddie James
2022-02-07 9:57 ` Joel Stanley
0 siblings, 1 reply; 6+ messages in thread
From: Eddie James @ 2022-01-31 15:08 UTC (permalink / raw)
To: Joel Stanley; +Cc: OpenBMC Maillist
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
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH linux dev-5.15] fsi: Add trace events in initialization path
2022-01-31 15:08 ` Eddie James
@ 2022-02-07 9:57 ` Joel Stanley
0 siblings, 0 replies; 6+ messages in thread
From: Joel Stanley @ 2022-02-07 9:57 UTC (permalink / raw)
To: Eddie James; +Cc: OpenBMC Maillist
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
> >>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-07 9:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-25 16:11 [PATCH linux dev-5.15] fsi: Add trace events in initialization path Eddie James
2022-01-25 16:38 ` Paul Menzel
2022-01-25 19:31 ` Eddie James
2022-01-31 5:59 ` Joel Stanley
2022-01-31 15:08 ` Eddie James
2022-02-07 9:57 ` Joel Stanley
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.