[PATCH 0/4] firmware: arm_scmi: Add xfer inflight debug and trace

[PATCH 0/4] firmware: arm_scmi: Add xfer inflight debug and trace

[Philip Radford]

Hi all,

This series adds a new counter to the Arm SCMI firmware driver to track
the number of in-flight message transfers during debug and trace. This
will be useful for examining behaviour under a large load with regards
to concurrent messages being sent and received. As the counter only gives
a live value, printing the value in trace allows logging of the in-flight
xfers.

The series is divided into four small patches:
Patch [1/4]
	- Adds a fuction to decrement debug counters, similar to the existing
	  counter to increment debug counters
Patch [2/4]
	- Adds a new xfers_inflight debug counter to track active transfers
Patch [3/4]
	- Enhances two tracepoints (scmi_xfer_begin and scmi_xfer_end) to
	  include the number of in-flight transfers, setting default values
	  of 0
Patch [4/4]
	- Adds a function (scmi_inflight_count) that retrieves the current
	  in-flight count for use in tracing

Based on v6.16-rc2

Regards,
Phil

Philip Radford (4):
  firmware: arm_scmi: Add debug decrement counter
  firmware: arm_scmi: Add xfer_inflight counter
  include: trace: Add inflight_xfer counter tracepoint
  firmware: arm_scmi: Add new inflight tracing functionality

 drivers/firmware/arm_scmi/common.h   |  8 ++++++++
 drivers/firmware/arm_scmi/driver.c   | 28 ++++++++++++++++++++++++++--
 drivers/firmware/arm_scmi/raw_mode.c |  6 ++++--
 include/trace/events/scmi.h          | 24 ++++++++++++++----------
 4 files changed, 52 insertions(+), 14 deletions(-)

--
2.25.1

[PATCH 1/4] firmware: arm_scmi: Add debug decrement counter

[Philip Radford]

Create scmi_dec_count function to decrease any of the Arm SCMI
debug counters

Signed-off-by: Philip Radford <philip.radford@arm.com>
---
 drivers/firmware/arm_scmi/common.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index dab758c5fdea..c6495c4a0e8a 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -314,6 +314,12 @@ static inline void scmi_inc_count(atomic_t *arr, int stat)
 		atomic_inc(&arr[stat]);
 }
 
+static inline void scmi_dec_count(atomic_t *arr, int stat)
+{
+	if (IS_ENABLED(CONFIG_ARM_SCMI_DEBUG_COUNTERS))
+		atomic_dec(&arr[stat]);
+}
+
 enum scmi_bad_msg {
 	MSG_UNEXPECTED = -1,
 	MSG_INVALID = -2,
-- 
2.25.1

Re: [PATCH 1/4] firmware: arm_scmi: Add debug decrement counter

[Dan Carpenter]

On Thu, Jun 19, 2025 at 12:20:01PM +0000, Philip Radford wrote:
> Create scmi_dec_count function to decrease any of the Arm SCMI
> debug counters
> 

Please mention that "There is already a scmi_inc_count() function but no
decrement function."  Otherwise everyone will have to look it up to
verify that it exists.  Please put a period on the end of your sentences.

regards,
dan carpenter

Re: [PATCH 1/4] firmware: arm_scmi: Add debug decrement counter

[Cristian Marussi]

On Thu, Jun 19, 2025 at 12:20:01PM +0000, Philip Radford wrote:
> Create scmi_dec_count function to decrease any of the Arm SCMI
> debug counters
> 

I agree on Dan's comment, other than that, LGTM.

Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>

Thanks,
Cristian

[PATCH 2/4] firmware: arm_scmi: Add xfer_inflight counter

[Philip Radford]

Added an xfer_inflight counter to the debug_counters struct
to record the current number of inflight xfers

Signed-off-by: Philip Radford <philip.radford@arm.com>
---
 drivers/firmware/arm_scmi/common.h |  1 +
 drivers/firmware/arm_scmi/driver.c | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index c6495c4a0e8a..ad9232c982ce 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -305,6 +305,7 @@ enum debug_counters {
 	ERR_MSG_INVALID,
 	ERR_MSG_NOMEM,
 	ERR_PROTOCOL,
+	XFERS_INFLIGHT,
 	SCMI_DEBUG_COUNTERS_LAST
 };
 
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 395fe9289035..5a4dac27afdf 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -190,6 +190,7 @@ struct scmi_info {
 };
 
 #define handle_to_scmi_info(h)	container_of(h, struct scmi_info, handle)
+#define tx_minfo_to_scmi_info(h) container_of(h, struct scmi_info, tx_minfo)
 #define bus_nb_to_scmi_info(nb)	container_of(nb, struct scmi_info, bus_nb)
 #define req_nb_to_scmi_info(nb)	container_of(nb, struct scmi_info, dev_req_nb)
 
@@ -603,9 +604,14 @@ static inline void
 scmi_xfer_inflight_register_unlocked(struct scmi_xfer *xfer,
 				     struct scmi_xfers_info *minfo)
 {
+	/* In this context minfo will be tx_minfo due to the xfer pending */
+	struct scmi_info *info = tx_minfo_to_scmi_info(minfo);
+
 	/* Set in-flight */
 	set_bit(xfer->hdr.seq, minfo->xfer_alloc_table);
 	hash_add(minfo->pending_xfers, &xfer->node, xfer->hdr.seq);
+	scmi_inc_count(info->dbg->counters, XFERS_INFLIGHT);
+
 	xfer->pending = true;
 }
 
@@ -807,9 +813,13 @@ __scmi_xfer_put(struct scmi_xfers_info *minfo, struct scmi_xfer *xfer)
 	spin_lock_irqsave(&minfo->xfer_lock, flags);
 	if (refcount_dec_and_test(&xfer->users)) {
 		if (xfer->pending) {
+			struct scmi_info *info = tx_minfo_to_scmi_info(minfo);
+
 			scmi_xfer_token_clear(minfo, xfer);
 			hash_del(&xfer->node);
 			xfer->pending = false;
+
+			scmi_dec_count(info->dbg->counters, XFERS_INFLIGHT);
 		}
 		hlist_add_head(&xfer->node, &minfo->free_xfers);
 	}
@@ -2912,6 +2922,7 @@ static const char * const dbg_counter_strs[] = {
 	"err_msg_invalid",
 	"err_msg_nomem",
 	"err_protocol",
+	"xfers_inflight",
 };
 
 static ssize_t reset_all_on_write(struct file *filp, const char __user *buf,
-- 
2.25.1

Re: [PATCH 2/4] firmware: arm_scmi: Add xfer_inflight counter

[Cristian Marussi]

On Thu, Jun 19, 2025 at 12:20:02PM +0000, Philip Radford wrote:
> Added an xfer_inflight counter to the debug_counters struct
> to record the current number of inflight xfers
> 
Hi,

nitpick around the commite message.

Add a counter to the debug_counters struct to record the current number
of outstanding inflight xfers.

...note also the full-stop :P

With this:

Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>

Thanks,
Cristian

[PATCH 3/4] include: trace: Add inflight_xfer counter tracepoint

[Philip Radford]

Add tracepoint for inflight_xfers and patch all related callsites.

Signed-off-by: Philip Radford <philip.radford@arm.com>
---
 drivers/firmware/arm_scmi/driver.c   |  4 ++--
 drivers/firmware/arm_scmi/raw_mode.c |  5 +++--
 include/trace/events/scmi.h          | 24 ++++++++++++++----------
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 5a4dac27afdf..c6657582c9ab 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -1443,7 +1443,7 @@ static int do_xfer(const struct scmi_protocol_handle *ph,
 
 	trace_scmi_xfer_begin(xfer->transfer_id, xfer->hdr.id,
 			      xfer->hdr.protocol_id, xfer->hdr.seq,
-			      xfer->hdr.poll_completion);
+			      xfer->hdr.poll_completion, 0);
 
 	/* Clear any stale status */
 	xfer->hdr.status = SCMI_SUCCESS;
@@ -1479,7 +1479,7 @@ static int do_xfer(const struct scmi_protocol_handle *ph,
 		info->desc->ops->mark_txdone(cinfo, ret, xfer);
 
 	trace_scmi_xfer_end(xfer->transfer_id, xfer->hdr.id,
-			    xfer->hdr.protocol_id, xfer->hdr.seq, ret);
+			    xfer->hdr.protocol_id, xfer->hdr.seq, ret, 0);
 
 	return ret;
 }
diff --git a/drivers/firmware/arm_scmi/raw_mode.c b/drivers/firmware/arm_scmi/raw_mode.c
index 3d543b1d8947..2ccb6b3f0e64 100644
--- a/drivers/firmware/arm_scmi/raw_mode.c
+++ b/drivers/firmware/arm_scmi/raw_mode.c
@@ -475,7 +475,8 @@ static void scmi_xfer_raw_worker(struct work_struct *work)
 			raw->desc->ops->mark_txdone(rw->cinfo, ret, xfer);
 
 		trace_scmi_xfer_end(xfer->transfer_id, xfer->hdr.id,
-				    xfer->hdr.protocol_id, xfer->hdr.seq, ret);
+				    xfer->hdr.protocol_id, xfer->hdr.seq,
+				    ret, 0);
 
 		/* Wait also for an async delayed response if needed */
 		if (!ret && xfer->async_done) {
@@ -642,7 +643,7 @@ static int scmi_do_xfer_raw_start(struct scmi_raw_mode_info *raw,
 
 	trace_scmi_xfer_begin(xfer->transfer_id, xfer->hdr.id,
 			      xfer->hdr.protocol_id, xfer->hdr.seq,
-			      xfer->hdr.poll_completion);
+			      xfer->hdr.poll_completion, 0);
 
 	ret = raw->desc->ops->send_message(rw->cinfo, xfer);
 	if (ret) {
diff --git a/include/trace/events/scmi.h b/include/trace/events/scmi.h
index 127300481123..703b7bb68e44 100644
--- a/include/trace/events/scmi.h
+++ b/include/trace/events/scmi.h
@@ -36,8 +36,8 @@ TRACE_EVENT(scmi_fc_call,
 
 TRACE_EVENT(scmi_xfer_begin,
 	TP_PROTO(int transfer_id, u8 msg_id, u8 protocol_id, u16 seq,
-		 bool poll),
-	TP_ARGS(transfer_id, msg_id, protocol_id, seq, poll),
+		 bool poll, int inflight),
+	TP_ARGS(transfer_id, msg_id, protocol_id, seq, poll, inflight),
 
 	TP_STRUCT__entry(
 		__field(int, transfer_id)
@@ -45,6 +45,7 @@ TRACE_EVENT(scmi_xfer_begin,
 		__field(u8, protocol_id)
 		__field(u16, seq)
 		__field(bool, poll)
+		__field(int, inflight)
 	),
 
 	TP_fast_assign(
@@ -53,11 +54,12 @@ TRACE_EVENT(scmi_xfer_begin,
 		__entry->protocol_id = protocol_id;
 		__entry->seq = seq;
 		__entry->poll = poll;
+		__entry->inflight = inflight;
 	),
 
-	TP_printk("pt=%02X msg_id=%02X seq=%04X transfer_id=%X poll=%u",
-		__entry->protocol_id, __entry->msg_id, __entry->seq,
-		__entry->transfer_id, __entry->poll)
+	TP_printk("pt=%02X msg_id=%02X seq=%04X transfer_id=%X poll=%u inflight=%d",
+		  __entry->protocol_id, __entry->msg_id, __entry->seq,
+		  __entry->transfer_id, __entry->poll, __entry->inflight)
 );
 
 TRACE_EVENT(scmi_xfer_response_wait,
@@ -90,8 +92,8 @@ TRACE_EVENT(scmi_xfer_response_wait,
 
 TRACE_EVENT(scmi_xfer_end,
 	TP_PROTO(int transfer_id, u8 msg_id, u8 protocol_id, u16 seq,
-		 int status),
-	TP_ARGS(transfer_id, msg_id, protocol_id, seq, status),
+		 int status, int inflight),
+	TP_ARGS(transfer_id, msg_id, protocol_id, seq, status, inflight),
 
 	TP_STRUCT__entry(
 		__field(int, transfer_id)
@@ -99,6 +101,7 @@ TRACE_EVENT(scmi_xfer_end,
 		__field(u8, protocol_id)
 		__field(u16, seq)
 		__field(int, status)
+		__field(int, inflight)
 	),
 
 	TP_fast_assign(
@@ -107,11 +110,12 @@ TRACE_EVENT(scmi_xfer_end,
 		__entry->protocol_id = protocol_id;
 		__entry->seq = seq;
 		__entry->status = status;
+		__entry->inflight = inflight;
 	),
 
-	TP_printk("pt=%02X msg_id=%02X seq=%04X transfer_id=%X s=%d",
-		__entry->protocol_id, __entry->msg_id, __entry->seq,
-		__entry->transfer_id, __entry->status)
+	TP_printk("pt=%02X msg_id=%02X seq=%04X transfer_id=%X s=%d inflight=%d",
+		  __entry->protocol_id, __entry->msg_id, __entry->seq,
+		  __entry->transfer_id, __entry->status, __entry->inflight)
 );
 
 TRACE_EVENT(scmi_rx_done,
-- 
2.25.1

[PATCH 4/4] firmware: arm_scmi: Add new inflight tracing functionality

[Philip Radford]

Adds scmi_inflight_count function to fetch the current xfer
inflight count to use in trace

Signed-off-by: Philip Radford <philip.radford@arm.com>
---
 drivers/firmware/arm_scmi/common.h   |  1 +
 drivers/firmware/arm_scmi/driver.c   | 17 +++++++++++++++--
 drivers/firmware/arm_scmi/raw_mode.c |  5 +++--
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index ad9232c982ce..07b9e629276d 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -505,4 +505,5 @@ static struct platform_driver __drv = {					       \
 void scmi_notification_instance_data_set(const struct scmi_handle *handle,
 					 void *priv);
 void *scmi_notification_instance_data_get(const struct scmi_handle *handle);
+int scmi_inflight_count(const struct scmi_handle *handle);
 #endif /* _SCMI_COMMON_H */
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index c6657582c9ab..d128d497f96e 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -1443,7 +1443,8 @@ static int do_xfer(const struct scmi_protocol_handle *ph,
 
 	trace_scmi_xfer_begin(xfer->transfer_id, xfer->hdr.id,
 			      xfer->hdr.protocol_id, xfer->hdr.seq,
-			      xfer->hdr.poll_completion, 0);
+			      xfer->hdr.poll_completion,
+				  scmi_inflight_count(&info->handle));
 
 	/* Clear any stale status */
 	xfer->hdr.status = SCMI_SUCCESS;
@@ -1479,7 +1480,8 @@ static int do_xfer(const struct scmi_protocol_handle *ph,
 		info->desc->ops->mark_txdone(cinfo, ret, xfer);
 
 	trace_scmi_xfer_end(xfer->transfer_id, xfer->hdr.id,
-			    xfer->hdr.protocol_id, xfer->hdr.seq, ret, 0);
+			    xfer->hdr.protocol_id, xfer->hdr.seq, ret,
+				scmi_inflight_count(&info->handle));
 
 	return ret;
 }
@@ -3416,6 +3418,17 @@ static struct dentry *scmi_debugfs_init(void)
 	return d;
 }
 
+int scmi_inflight_count(const struct scmi_handle *handle)
+{
+	if (IS_ENABLED(CONFIG_ARM_SCMI_DEBUG_COUNTERS)) {
+		struct scmi_info *info = handle_to_scmi_info(handle);
+
+		return atomic_read(&info->dbg->counters[XFERS_INFLIGHT]);
+	} else {
+		return 0;
+	}
+}
+
 static int __init scmi_driver_init(void)
 {
 	scmi_quirks_initialize();
diff --git a/drivers/firmware/arm_scmi/raw_mode.c b/drivers/firmware/arm_scmi/raw_mode.c
index 2ccb6b3f0e64..eebe1a5297ef 100644
--- a/drivers/firmware/arm_scmi/raw_mode.c
+++ b/drivers/firmware/arm_scmi/raw_mode.c
@@ -476,7 +476,7 @@ static void scmi_xfer_raw_worker(struct work_struct *work)
 
 		trace_scmi_xfer_end(xfer->transfer_id, xfer->hdr.id,
 				    xfer->hdr.protocol_id, xfer->hdr.seq,
-				    ret, 0);
+				    ret, scmi_inflight_count(raw->handle));
 
 		/* Wait also for an async delayed response if needed */
 		if (!ret && xfer->async_done) {
@@ -643,7 +643,8 @@ static int scmi_do_xfer_raw_start(struct scmi_raw_mode_info *raw,
 
 	trace_scmi_xfer_begin(xfer->transfer_id, xfer->hdr.id,
 			      xfer->hdr.protocol_id, xfer->hdr.seq,
-			      xfer->hdr.poll_completion, 0);
+			      xfer->hdr.poll_completion,
+				  scmi_inflight_count(raw->handle));
 
 	ret = raw->desc->ops->send_message(rw->cinfo, xfer);
 	if (ret) {
-- 
2.25.1

Re: [PATCH 4/4] firmware: arm_scmi: Add new inflight tracing functionality

[Dan Carpenter]

On Thu, Jun 19, 2025 at 12:20:04PM +0000, Philip Radford wrote:
> Adds scmi_inflight_count function to fetch the current xfer
> inflight count to use in trace
> 
> Signed-off-by: Philip Radford <philip.radford@arm.com>
> ---
>  drivers/firmware/arm_scmi/common.h   |  1 +
>  drivers/firmware/arm_scmi/driver.c   | 17 +++++++++++++++--
>  drivers/firmware/arm_scmi/raw_mode.c |  5 +++--
>  3 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> index ad9232c982ce..07b9e629276d 100644
> --- a/drivers/firmware/arm_scmi/common.h
> +++ b/drivers/firmware/arm_scmi/common.h
> @@ -505,4 +505,5 @@ static struct platform_driver __drv = {					       \
>  void scmi_notification_instance_data_set(const struct scmi_handle *handle,
>  					 void *priv);
>  void *scmi_notification_instance_data_get(const struct scmi_handle *handle);
> +int scmi_inflight_count(const struct scmi_handle *handle);
>  #endif /* _SCMI_COMMON_H */
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index c6657582c9ab..d128d497f96e 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -1443,7 +1443,8 @@ static int do_xfer(const struct scmi_protocol_handle *ph,
>  
>  	trace_scmi_xfer_begin(xfer->transfer_id, xfer->hdr.id,
>  			      xfer->hdr.protocol_id, xfer->hdr.seq,
> -			      xfer->hdr.poll_completion, 0);
> +			      xfer->hdr.poll_completion,
> +				  scmi_inflight_count(&info->handle));

White space is messed up.  It might be better to fold this into patch 3?

regards,
dan carpenter

Re: [PATCH 4/4] firmware: arm_scmi: Add new inflight tracing functionality

[Cristian Marussi]

On Thu, Jun 19, 2025 at 12:20:04PM +0000, Philip Radford wrote:
> Adds scmi_inflight_count function to fetch the current xfer
> inflight count to use in trace
> 

Hi,

I agree with Dan's comments on patches 3 and 4...once those comments are
addressed, for me:

Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>

Thanks,
Cristian

Re: [PATCH 0/4] firmware: arm_scmi: Add xfer inflight debug and trace

[Peng Fan]

On Thu, Jun 19, 2025 at 12:20:00PM +0000, Philip Radford wrote:
>Hi all,
>
>This series adds a new counter to the Arm SCMI firmware driver to track
>the number of in-flight message transfers during debug and trace. This
>will be useful for examining behaviour under a large load with regards
>to concurrent messages being sent and received. As the counter only gives
>a live value, printing the value in trace allows logging of the in-flight
>xfers.

Just a general question, is this counter count in flight messages
for a scmi instance or it is per transport? I ask because 
one scmi instance could have multiple mailboxes. If counting based
on scmi instance, it may not be that accurate.

Thanks,
Peng

>
>The series is divided into four small patches:
>Patch [1/4]
>	- Adds a fuction to decrement debug counters, similar to the existing
>	  counter to increment debug counters
>Patch [2/4]
>	- Adds a new xfers_inflight debug counter to track active transfers
>Patch [3/4]
>	- Enhances two tracepoints (scmi_xfer_begin and scmi_xfer_end) to
>	  include the number of in-flight transfers, setting default values
>	  of 0
>Patch [4/4]
>	- Adds a function (scmi_inflight_count) that retrieves the current
>	  in-flight count for use in tracing
>
>Based on v6.16-rc2
>
>Regards,
>Phil
>
>Philip Radford (4):
>  firmware: arm_scmi: Add debug decrement counter
>  firmware: arm_scmi: Add xfer_inflight counter
>  include: trace: Add inflight_xfer counter tracepoint
>  firmware: arm_scmi: Add new inflight tracing functionality
>
> drivers/firmware/arm_scmi/common.h   |  8 ++++++++
> drivers/firmware/arm_scmi/driver.c   | 28 ++++++++++++++++++++++++++--
> drivers/firmware/arm_scmi/raw_mode.c |  6 ++++--
> include/trace/events/scmi.h          | 24 ++++++++++++++----------
> 4 files changed, 52 insertions(+), 14 deletions(-)
>
>--
>2.25.1
>

RE: [PATCH 0/4] firmware: arm_scmi: Add xfer inflight debug and trace

[Philip Radford]

> -----Original Message-----
> From: Peng Fan <peng.fan@oss.nxp.com>
> Sent: Friday, June 20, 2025 9:47 AM
> To: Philip Radford <Philip.Radford@arm.com>

Hi,
Thanks for the review.

> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; arm-
> scmi@vger.kernel.org; Sudeep Holla <Sudeep.Holla@arm.com>; Cristian
> Marussi <Cristian.Marussi@arm.com>; Luke Parkin <Luke.Parkin@arm.com>
> Subject: Re: [PATCH 0/4] firmware: arm_scmi: Add xfer inflight debug and
> trace
> 
> On Thu, Jun 19, 2025 at 12:20:00PM +0000, Philip Radford wrote:
> >Hi all,
> >
> >This series adds a new counter to the Arm SCMI firmware driver to track
> >the number of in-flight message transfers during debug and trace. This
> >will be useful for examining behaviour under a large load with regards
> >to concurrent messages being sent and received. As the counter only gives
> >a live value, printing the value in trace allows logging of the in-flight
> >xfers.
> 
> Just a general question, is this counter count in flight messages
> for a scmi instance or it is per transport? I ask because
> one scmi instance could have multiple mailboxes. If counting based
> on scmi instance, it may not be that accurate.
> 
> Thanks,
> Peng
> 

Yes, you are correct that the counter is per instance, as are the other counters.
This would mean that if you have multiple channels you would see the total
number of inflight xfers across all channels, limited to that instance.
If the inflight counter is non-zero and something is wrong, it would not be
apparent which channel had an issue, however this is the same for the other
counters.

I feel like this series is a helpful addition to the current counters, but what you
have pointed out is something that could be reworked in a future series?

Regards,
Phil

Re: [PATCH 0/4] firmware: arm_scmi: Add xfer inflight debug and trace

[Peng Fan]

On Fri, Jun 20, 2025 at 09:27:52AM +0000, Philip Radford wrote:
>
>
>> -----Original Message-----
>> From: Peng Fan <peng.fan@oss.nxp.com>
>> Sent: Friday, June 20, 2025 9:47 AM
>> To: Philip Radford <Philip.Radford@arm.com>
>
>Hi,
>Thanks for the review.
>
>> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; arm-
>> scmi@vger.kernel.org; Sudeep Holla <Sudeep.Holla@arm.com>; Cristian
>> Marussi <Cristian.Marussi@arm.com>; Luke Parkin <Luke.Parkin@arm.com>
>> Subject: Re: [PATCH 0/4] firmware: arm_scmi: Add xfer inflight debug and
>> trace
>> 
>> On Thu, Jun 19, 2025 at 12:20:00PM +0000, Philip Radford wrote:
>> >Hi all,
>> >
>> >This series adds a new counter to the Arm SCMI firmware driver to track
>> >the number of in-flight message transfers during debug and trace. This
>> >will be useful for examining behaviour under a large load with regards
>> >to concurrent messages being sent and received. As the counter only gives
>> >a live value, printing the value in trace allows logging of the in-flight
>> >xfers.
>> 
>> Just a general question, is this counter count in flight messages
>> for a scmi instance or it is per transport? I ask because
>> one scmi instance could have multiple mailboxes. If counting based
>> on scmi instance, it may not be that accurate.
>> 
>> Thanks,
>> Peng
>> 
>
>Yes, you are correct that the counter is per instance, as are the other counters.
>This would mean that if you have multiple channels you would see the total
>number of inflight xfers across all channels, limited to that instance.
>If the inflight counter is non-zero and something is wrong, it would not be
>apparent which channel had an issue, however this is the same for the other
>counters.
>
>I feel like this series is a helpful addition to the current counters, but what you
>have pointed out is something that could be reworked in a future series?

Sure. It is ok from my view, you could extend the driver in future.

Regards,
Peng

>
>Regards,
>Phil

Re: [PATCH 0/4] firmware: arm_scmi: Add xfer inflight debug and trace

[Cristian Marussi]

On Fri, Jun 20, 2025 at 10:27:52AM +0100, Philip Radford wrote:
> 
> 
> > -----Original Message-----
> > From: Peng Fan <peng.fan@oss.nxp.com>
> > Sent: Friday, June 20, 2025 9:47 AM
> > To: Philip Radford <Philip.Radford@arm.com>
> 
> Hi,
> Thanks for the review.
> 

Hi,

> > Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; arm-
> > scmi@vger.kernel.org; Sudeep Holla <Sudeep.Holla@arm.com>; Cristian
> > Marussi <Cristian.Marussi@arm.com>; Luke Parkin <Luke.Parkin@arm.com>
> > Subject: Re: [PATCH 0/4] firmware: arm_scmi: Add xfer inflight debug and
> > trace
> > 
> > On Thu, Jun 19, 2025 at 12:20:00PM +0000, Philip Radford wrote:
> > >Hi all,
> > >
> > >This series adds a new counter to the Arm SCMI firmware driver to track
> > >the number of in-flight message transfers during debug and trace. This
> > >will be useful for examining behaviour under a large load with regards
> > >to concurrent messages being sent and received. As the counter only gives
> > >a live value, printing the value in trace allows logging of the in-flight
> > >xfers.
> > 
> > Just a general question, is this counter count in flight messages
> > for a scmi instance or it is per transport? I ask because
> > one scmi instance could have multiple mailboxes. If counting based
> > on scmi instance, it may not be that accurate.
> > 

... so that is a good point ...
...thanks Peng for pointing out this first of all...

So, in general all of these counters are per-instance, we don't have any
finer per-channel granularity....we could in the future split them out
to be per-channel counters, but I wonder if it would be worth the
effort: because, as I see it, errors reported by these counters are more
of a alarm-bell than a triage tool, in the sense that I would expect
that seeing a lot of errors of some kind on an instance should just act
as a warning that something is NOT right somewhere, so that you can
investigate further by enabling the already existent and more comprehensive
SCMI trace events to fully inveestigate the problem...since SCMI full event
traces DO also include the used-channel beside a lot of other info about
the xfer transactions.

Moreover, in the specific case of tracking inflight xfers, note that
the counter added in this series tracks the pool of xfers allocated in
tx_minfo(A2P) free-lists (i.e. commands...P2A msgs hardly can be lost),
BUT this structure is per-instance (NOT per-channel), so even if you had
say a few more dedicated per-protocol channels defined on a system,
all the A2P transactions will pick their xfers from the same per-instance
pool... (..because the max_inflights is meant to cap the maximum number
of outstanding transactions that the server has to cope with...)

Thanks,
Cristian

Re: [PATCH 0/4] firmware: arm_scmi: Add xfer inflight debug and trace

[Peng Fan]

Hi Cristian,

On Thu, Jun 26, 2025 at 12:37:49PM +0100, Cristian Marussi wrote:
>> > Just a general question, is this counter count in flight messages
>> > for a scmi instance or it is per transport? I ask because
>> > one scmi instance could have multiple mailboxes. If counting based
>> > on scmi instance, it may not be that accurate.
>> > 
>
>... so that is a good point ...
>...thanks Peng for pointing out this first of all...
>
>So, in general all of these counters are per-instance, we don't have any
>finer per-channel granularity....we could in the future split them out
>to be per-channel counters, but I wonder if it would be worth the
>effort: because, as I see it, errors reported by these counters are more
>of a alarm-bell than a triage tool, in the sense that I would expect
>that seeing a lot of errors of some kind on an instance should just act
>as a warning that something is NOT right somewhere, so that you can
>investigate further by enabling the already existent and more comprehensive
>SCMI trace events to fully inveestigate the problem...since SCMI full event
>traces DO also include the used-channel beside a lot of other info about
>the xfer transactions.
>
>Moreover, in the specific case of tracking inflight xfers, note that
>the counter added in this series tracks the pool of xfers allocated in
>tx_minfo(A2P) free-lists (i.e. commands...P2A msgs hardly can be lost),
>BUT this structure is per-instance (NOT per-channel), so even if you had
>say a few more dedicated per-protocol channels defined on a system,
>all the A2P transactions will pick their xfers from the same per-instance
>pool... (..because the max_inflights is meant to cap the maximum number
>of outstanding transactions that the server has to cope with...)

Thanks for explaining this. It is clear that there is no need finer
granularity. Just leave as it is.

Thanks,
Peng

>
>Thanks,
>Cristian

Re: [PATCH 0/4] firmware: arm_scmi: Add xfer inflight debug and trace

[Cristian Marussi]

On Thu, Jun 19, 2025 at 12:20:00PM +0000, Philip Radford wrote:
> Hi all,

Hi,

as mentioned offline, please rebase V2 on sudeep/for-next/scmi/updates
and add proper version diffs summary all-over.

Thanks,
Cristian

Re: [PATCH 4/4] firmware: arm_scmi: Add new inflight tracing functionality

[Cristian Marussi]

On Thu, Jun 19, 2025 at 04:13:57PM +0100, Philip Radford wrote:
> > -----Original Message-----
> > From: Dan Carpenter <dan.carpenter@linaro.org>
> > Sent: Thursday, June 19, 2025 3:38 PM
> > To: Philip Radford <Philip.Radford@arm.com>
> 
> Hi, 
> 
> > Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; arm-
> > scmi@vger.kernel.org; Sudeep Holla <Sudeep.Holla@arm.com>; Cristian
> > Marussi <Cristian.Marussi@arm.com>; Luke Parkin <Luke.Parkin@arm.com>
> > Subject: Re: [PATCH 4/4] firmware: arm_scmi: Add new inflight tracing
> > functionality

Hi Phil,

please reply to all the recipient of the original mail-thread when answering a
review, so we remain on the list. (mutt -> g .. I think :P) 

(I added the CCs to this)

Thanks,
Cristian