From: Eddie James <eajames@linux.ibm.com>
To: Joel Stanley <joel@jms.id.au>
Cc: OpenBMC Maillist <openbmc@lists.ozlabs.org>
Subject: Re: [PATCH linux dev-5.15] fsi: Add trace events in initialization path
Date: Mon, 31 Jan 2022 09:08:45 -0600 [thread overview]
Message-ID: <bfb4ed15-3082-8495-a4e1-d027263c77ab@linux.ibm.com> (raw)
In-Reply-To: <CACPK8XevA9_DNpGkfDe8=2zy7VPZcTa1ehzoJRHLVtrWapf42Q@mail.gmail.com>
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
>>
next prev parent reply other threads:[~2022-01-31 15:09 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2022-02-07 9:57 ` Joel Stanley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bfb4ed15-3082-8495-a4e1-d027263c77ab@linux.ibm.com \
--to=eajames@linux.ibm.com \
--cc=joel@jms.id.au \
--cc=openbmc@lists.ozlabs.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.