Received-SPF: pass (google.com: domain of linux-kernel+bounces-210211-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Tue, 11 Jun 2024 09:06:35 -0700
From: Bjorn Andersson <quic_bjorande@quicinc.com>
To: Sudeepgoud Patil <quic_sudeepgo@quicinc.com>
CC: <andersson@kernel.org>, <quic_clew@quicinc.com>,
        <mathieu.poirier@linaro.org>, <linux-kernel@vger.kernel.org>,
        <quic_deesin@quicinc.com>, <linux-arm-msm@vger.kernel.org>,
        <linux-remoteproc@vger.kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>
Subject: Re: [PATCH V2 1/2] soc: qcom: smp2p: Add remote name into smp2p irq
 devname
Message-ID: <Zmh2CzGpJrmzs+6K@hu-bjorande-lv.qualcomm.com>
References: <20240611123351.3813190-1-quic_sudeepgo@quicinc.com>
 <20240611123351.3813190-2-quic_sudeepgo@quicinc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240611123351.3813190-2-quic_sudeepgo@quicinc.com>

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