linux-kernel - Re: [PATCH v6 01/12] dmaengine: qcom: bam_dma: Add bam_sw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <3d0ef611-2b21-d22e-8dbc-f092c36ceb1d@quicinc.com>
Date: Fri, 17 Jan 2025 10:02:37 +0530
From: Md Sadre Alam <quic_mdalam@...cinc.com>
To: Stephan Gerhold <stephan.gerhold@...aro.org>
CC: <vkoul@...nel.org>, <corbet@....net>, <thara.gopinath@...il.com>,
        <herbert@...dor.apana.org.au>, <davem@...emloft.net>,
        <martin.petersen@...cle.com>, <enghua.yu@...el.com>,
        <u.kleine-koenig@...libre.com>, <dmaengine@...r.kernel.org>,
        <linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-crypto@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <quic_utiwari@...cinc.com>, <quic_srichara@...cinc.com>,
        <quic_varada@...cinc.com>
Subject: Re: [PATCH v6 01/12] dmaengine: qcom: bam_dma: Add bam_sw_version
 register read



On 1/16/2025 10:36 PM, Stephan Gerhold wrote:
> On Wed, Jan 15, 2025 at 03:59:53PM +0530, Md Sadre Alam wrote:
>> Add bam_sw_version register read. This will help to
>> differentiate b/w some new BAM features across multiple
>> BAM IP, feature like LOCK/UNLOCK of BAM pipe.
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@...cinc.com>
>> ---
>>
>> change in [v6]
>>
>> * No change
>>
>> change in [v5]
>>
>> * No change
>>
>> change in [v4]
>>
>> * Added BAM_SW_VERSION register read
>>
>> change in [v3]
>>
>> * This patch was not included in [v3]
>>
>> change in [v2]
>>
>> * This patch was not included in [v2]
>>
>> change in [v1]
>>
>> * This patch was not included in [v1]
>>
>>   drivers/dma/qcom/bam_dma.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
>> index c14557efd577..daeacd5cb8e9 100644
>> --- a/drivers/dma/qcom/bam_dma.c
>> +++ b/drivers/dma/qcom/bam_dma.c
>> @@ -83,6 +83,7 @@ struct bam_async_desc {
>>   enum bam_reg {
>>   	BAM_CTRL,
>>   	BAM_REVISION,
>> +	BAM_SW_VERSION,
>>   	BAM_NUM_PIPES,
>>   	BAM_DESC_CNT_TRSHLD,
>>   	BAM_IRQ_SRCS,
>> @@ -117,6 +118,7 @@ struct reg_offset_data {
>>   static const struct reg_offset_data bam_v1_3_reg_info[] = {
>>   	[BAM_CTRL]		= { 0x0F80, 0x00, 0x00, 0x00 },
>>   	[BAM_REVISION]		= { 0x0F84, 0x00, 0x00, 0x00 },
>> +	[BAM_SW_VERSION]	= { 0x0F88, 0x00, 0x00, 0x00 },
>>   	[BAM_NUM_PIPES]		= { 0x0FBC, 0x00, 0x00, 0x00 },
>>   	[BAM_DESC_CNT_TRSHLD]	= { 0x0F88, 0x00, 0x00, 0x00 },
>>   	[BAM_IRQ_SRCS]		= { 0x0F8C, 0x00, 0x00, 0x00 },
>> @@ -146,6 +148,7 @@ static const struct reg_offset_data bam_v1_3_reg_info[] = {
>>   static const struct reg_offset_data bam_v1_4_reg_info[] = {
>>   	[BAM_CTRL]		= { 0x0000, 0x00, 0x00, 0x00 },
>>   	[BAM_REVISION]		= { 0x0004, 0x00, 0x00, 0x00 },
>> +	[BAM_SW_VERSION]	= { 0x0008, 0x00, 0x00, 0x00 },
>>   	[BAM_NUM_PIPES]		= { 0x003C, 0x00, 0x00, 0x00 },
>>   	[BAM_DESC_CNT_TRSHLD]	= { 0x0008, 0x00, 0x00, 0x00 },
>>   	[BAM_IRQ_SRCS]		= { 0x000C, 0x00, 0x00, 0x00 },
>> @@ -175,6 +178,7 @@ static const struct reg_offset_data bam_v1_4_reg_info[] = {
>>   static const struct reg_offset_data bam_v1_7_reg_info[] = {
>>   	[BAM_CTRL]		= { 0x00000, 0x00, 0x00, 0x00 },
>>   	[BAM_REVISION]		= { 0x01000, 0x00, 0x00, 0x00 },
>> +	[BAM_SW_VERSION]	= { 0x01004, 0x00, 0x00, 0x00 },
>>   	[BAM_NUM_PIPES]		= { 0x01008, 0x00, 0x00, 0x00 },
>>   	[BAM_DESC_CNT_TRSHLD]	= { 0x00008, 0x00, 0x00, 0x00 },
>>   	[BAM_IRQ_SRCS]		= { 0x03010, 0x00, 0x00, 0x00 },
>> @@ -393,6 +397,7 @@ struct bam_device {
>>   	bool controlled_remotely;
>>   	bool powered_remotely;
>>   	u32 active_channels;
>> +	u32 bam_sw_version;
>>   
>>   	const struct reg_offset_data *layout;
>>   
>> @@ -1306,6 +1311,9 @@ static int bam_dma_probe(struct platform_device *pdev)
>>   		return ret;
>>   	}
>>   
>> +	bdev->bam_sw_version = readl_relaxed(bam_addr(bdev, 0, BAM_SW_VERSION));
>> +	dev_info(bdev->dev, "BAM software version:0x%08x\n", bdev->bam_sw_version);
> 
> This will cause crashes for the same reason as your other patch. During
> probe(), we can't read from BAM registers if we don't have a clock
> assigned. There is no guarantee that the BAM is powered up.
> 
> To make this work properly on all platforms, you would need to defer
> reading this register until the first channel is requested by the
> consumer driver. Or limit this functionality to the if (bdev->bamclk)
> case for now.
Sure thanks for suggestion. Will fix in next revision.
> 
> We should also prioritize fixing the existing regression before adding
> new functionality.
Sure, I am working on it will post quickly.

Thanks,
Alam.