mbox series

[V2,0/2] Add tracepoint support and remote name mapping to smp2p

Message ID 20240611123351.3813190-1-quic_sudeepgo@quicinc.com
Headers show
Series Add tracepoint support and remote name mapping to smp2p | expand

Message

Sudeepgoud Patil June 11, 2024, 12:33 p.m. UTC 
This commit introduces tracepoint support to smp2p module, enabling logging of communication
events between local and remote processors. The tracepoints capture essential details
such as remote processor ID, subsystem names, negotiation specifics, supported features,
bit changes, and subsystem restart (SSR) activity.
These tracepoints enhance debugging capabilities for inter-subsystem issues.

Addressing feedback from v1 to map remote PID (Process ID) along with the remote name
in tracepoints added new patch in V2 1/2, adding support to include the remote name
in the smp2p irq devname which fetches remote name from respective smp2p dtsi node,
which also makes the wakeup source distinguishable in irq wakeup prints.

Changes in v2:
- Added support to include the remote name in the smp2p IRQ devname, allowing for remote PID-name mapping
- Mapped the remote PID (Process ID) along with the remote name in tracepoints, as suggested by Chris
- Modified to capture all `out->features` instead of just the `ssr_ack`, following Chris's recommendation
- Expanded the commit description to provide additional context
- Link to v1: https://lore.kernel.org/all/20240429075528.1723133-1-quic_sudeepgo@quicinc.com

Sudeepgoud Patil (2):
  soc: qcom: smp2p: Add remote name into smp2p irq devname
  soc: qcom: smp2p: Introduce tracepoint support

 drivers/soc/qcom/Makefile      |   1 +
 drivers/soc/qcom/smp2p.c       |  26 +++++++-
 drivers/soc/qcom/trace-smp2p.h | 116 +++++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+), 1 deletion(-)
 create mode 100644 drivers/soc/qcom/trace-smp2p.h

--

Comments

Bjorn Andersson June 11, 2024, 4:06 p.m. UTC | #1 
On Tue, Jun 11, 2024 at 06:03:50PM +0530, Sudeepgoud Patil wrote:
> Add smp2p irq devname which fetches remote name from respective
> smp2p dtsi node, which makes the wakeup source distinguishable
> in irq wakeup prints.
> 
> Signed-off-by: Sudeepgoud Patil <quic_sudeepgo@quicinc.com>
> ---
>  drivers/soc/qcom/smp2p.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
> index a21241cbeec7..a77fee048b38 100644
> --- a/drivers/soc/qcom/smp2p.c
> +++ b/drivers/soc/qcom/smp2p.c
> @@ -122,6 +122,7 @@ struct smp2p_entry {
>   * @ssr_ack_enabled: SMP2P_FEATURE_SSR_ACK feature is supported and was enabled
>   * @ssr_ack: current cached state of the local ack bit
>   * @negotiation_done: whether negotiating finished
> + * @irq_devname: poniter to the smp2p irq devname
>   * @local_pid:	processor id of the inbound edge
>   * @remote_pid:	processor id of the outbound edge
>   * @ipc_regmap:	regmap for the outbound ipc
> @@ -146,6 +147,7 @@ struct qcom_smp2p {
>  	bool ssr_ack;
>  	bool negotiation_done;
>  
> +	char *irq_devname;
>  	unsigned local_pid;
>  	unsigned remote_pid;
>  
> @@ -614,10 +616,16 @@ static int qcom_smp2p_probe(struct platform_device *pdev)
>  	/* Kick the outgoing edge after allocating entries */
>  	qcom_smp2p_kick(smp2p);
>  
> +	smp2p->irq_devname = kasprintf(GFP_KERNEL, "%s", pdev->dev.of_node->name);

That's a lot of extra instructions for copying a string, which doesn't
need to be copied because of_node->name is const char and the argument
to devm_request_threaded_irq() is const char.

So, kstrdup_const() is what you're looking for.

You can then go devm_kstrdup_const() and avoid the kfree() (then
kfree_const()) below.


That said, looking at /proc/interrupts, I think it would make sense to
make this devm_kasprintf(..., "smp2p-%s", name);

Regards,
Bjorn

> +	if (!smp2p->irq_devname) {
> +		ret = -ENOMEM;
> +		goto unwind_interfaces;
> +	}
> +
>  	ret = devm_request_threaded_irq(&pdev->dev, irq,
>  					NULL, qcom_smp2p_intr,
>  					IRQF_ONESHOT,
> -					"smp2p", (void *)smp2p);
> +					smp2p->irq_devname, (void *)smp2p);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to request interrupt\n");
>  		goto unwind_interfaces;
> @@ -650,6 +658,8 @@ static int qcom_smp2p_probe(struct platform_device *pdev)
>  	list_for_each_entry(entry, &smp2p->outbound, node)
>  		qcom_smem_state_unregister(entry->state);
>  
> +	kfree(smp2p->irq_devname);
> +
>  	smp2p->out->valid_entries = 0;
>  
>  release_mbox:
> @@ -677,6 +687,8 @@ static void qcom_smp2p_remove(struct platform_device *pdev)
>  
>  	mbox_free_channel(smp2p->mbox_chan);
>  
> +	kfree(smp2p->irq_devname);
> +
>  	smp2p->out->valid_entries = 0;
>  }
>  
> -- 
>