[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2404a7a5-f4ac-4e62-b4b7-df0494d71150@oss.qualcomm.com>
Date: Mon, 17 Nov 2025 14:42:23 +0530
From: Kumari Pallavi <kumari.pallavi@....qualcomm.com>
To: Bjorn Andersson <andersson@...nel.org>
Cc: kpallavi@....qualcomm.com, srini@...nel.org, amahesh@....qualcomm.com,
arnd@...db.de, gregkh@...uxfoundation.org, quic_bkumar@...cinc.com,
ekansh.gupta@....qualcomm.com, linux-kernel@...r.kernel.org,
quic_chennak@...cinc.com, dri-devel@...ts.freedesktop.org,
linux-arm-msm@...r.kernel.org, jingyi.wang@....qualcomm.com,
aiqun.yu@....qualcomm.com, ktadakam@....qualcomm.com
Subject: Re: [PATCH v3 4/4] misc: fastrpc: Update dma_bits for CDSP support on
Kaanapali SoC
On 11/14/2025 9:30 PM, Bjorn Andersson wrote:
> On Fri, Nov 14, 2025 at 02:11:42PM +0530, Kumari Pallavi wrote:
>> DSP currently supports 32-bit IOVA (32-bit PA + 4-bit SID) for
>> both Q6 and user DMA (uDMA) access. This is being upgraded to
>> 34-bit PA + 4-bit SID due to a hardware revision in CDSP for
>> Kaanapali SoC, which expands the DMA addressable range.
>> Update DMA bits configuration in the driver to support CDSP on
>> Kaanapali SoC. Set the default `dma_bits` to 32-bit and update
>> it to 34-bit based on CDSP and OF matching on the fastrpc node.
>>
>> Signed-off-by: Kumari Pallavi <kumari.pallavi@....qualcomm.com>
>> ---
>> drivers/misc/fastrpc.c | 15 +++++++++++++--
>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index bcf3c7f8d3e9..2eb8d37cd9b4 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -267,6 +267,8 @@ struct fastrpc_session_ctx {
>>
>> struct fastrpc_soc_data {
>> u32 sid_pos;
>> + u32 cdsp_dma_bits;
>> + u32 dsp_default_dma_bits;
>> };
>>
>> struct fastrpc_channel_ctx {
>> @@ -2186,6 +2188,7 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
>> int i, sessions = 0;
>> unsigned long flags;
>> int rc;
>> + u32 dma_bits;
>>
>> cctx = dev_get_drvdata(dev->parent);
>> if (!cctx)
>> @@ -2199,12 +2202,16 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
>> spin_unlock_irqrestore(&cctx->lock, flags);
>> return -ENOSPC;
>> }
>> + dma_bits = cctx->soc_data->dsp_default_dma_bits;
>> sess = &cctx->session[cctx->sesscount++];
>> sess->used = false;
>> sess->valid = true;
>> sess->dev = dev;
>> dev_set_drvdata(dev, sess);
>>
>> + if (cctx->domain_id == CDSP_DOMAIN_ID)
>> + dma_bits = cctx->soc_data->cdsp_dma_bits;
>> +
>> if (of_property_read_u32(dev->of_node, "reg", &sess->sid))
>> dev_info(dev, "FastRPC Session ID not specified in DT\n");
>>
>> @@ -2219,9 +2226,9 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
>> }
>> }
>> spin_unlock_irqrestore(&cctx->lock, flags);
>> - rc = dma_set_mask(dev, DMA_BIT_MASK(32));
>> + rc = dma_set_mask(dev, DMA_BIT_MASK(dma_bits));
>> if (rc) {
>> - dev_err(dev, "32-bit DMA enable failed\n");
>> + dev_err(dev, "%u-bit DMA enable failed\n", dma_bits);
>> return rc;
>> }
>>
>> @@ -2308,10 +2315,14 @@ static int fastrpc_get_domain_id(const char *domain)
>>
>> static const struct fastrpc_soc_data kaanapali_soc_data = {
>> .sid_pos = 56,
>> + .cdsp_dma_bits = 34,
>> + .dsp_default_dma_bits = 32,
>> };
>>
>> static const struct fastrpc_soc_data default_soc_data = {
>> .sid_pos = 32,
>> + .cdsp_dma_bits = 32,
>> + .dsp_default_dma_bits = 32,
>
> So, "dsp_default_dma_bits" specified "what is the dma_mask for the
> non-CDSP fastrpc instances"? I don't find "dsp_default" to naturally
> mean "not the cdsp".
>
>
> Wouldn't it be better to introduce two different compatibles, one being
> the "qcom,kaanapali-fastrpc" and one being the
> "qcom,kaanapali-cdsp-fastrpc" and then use that to select things here?
>
Thank you for the suggestion. In this case, sid_pos is common across all
DSP domains on kaanapali Soc. Splitting into two compatibles would lead
to duplication of these shared property in the DT schema and driver logic.
The only difference here is the DMA address width for CDSP (34-bit)
versus other DSPs (32-bit).
To address the concern about naming, I can provide:
dma_bits_cdsp → clearly indicates this applies to the CDSP domain.
dma_bits_non_cdsp (or dma_bits_other_dsp) → for ADSP and other DSP domains.
Please let me know if this aligns with your suggestion ?
>
> PS. You store "dma_bits" just for the sake of turning it into a
> dma_mask, just store the DMA_BIT_MASK() directly here instead.
>
The current approach of assigning a value to cdsp_dma_mask allows for
adaptable logging behavior, making it easier to trace.
> Regards,
> Bjorn
>
>> };
>>
>> static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>> --
>> 2.34.1
>>
>>
Thanks,
Pallavi
Powered by blists - more mailing lists