Re: [PATCH V2 1/2] soc: qcom: smp2p: Add remote name into smp2p irq devname

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 6/11/2024 9:06 AM, Bjorn Andersson wrote:
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@xxxxxxxxxxx>
---
  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);


Is it ok to rely on the "of_node->name"? I think device tree tends to always have the node name as "smp2p-%s" already, so ("smp2p-%s", name) would result in "smp2p-smp2p-adsp".

Also Sudeepgoud, I think this will update the irqname in /proc/interrupts for the ipcc irqchip entry. It would also be helpful if we could differentiate the instances of smp2p irqchips as well. That way we can see what processors the 'ready' and 'fatal' interrupts apply to in /proc/interrupts.

Can you refer to my internal patch that adds .irq_print_chip() and incorporate those changes here?

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;
  }
--





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux