* [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register
@ 2024-12-20 9:42 Md Sadre Alam
2024-12-24 10:41 ` Vinod Koul
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Md Sadre Alam @ 2024-12-20 9:42 UTC (permalink / raw)
To: vkoul, robin.murphy, u.kleine-koenig, martin.petersen, fenghua.yu,
av2082000, linux-arm-msm, dmaengine, linux-kernel
Cc: quic_mmanikan, quic_srichara, quic_varada, quic_mdalam
Avoid writing unavailable register in BAM-Lite mode.
BAM_DESC_CNT_TRSHLD register is unavailable in BAM-Lite
mode. Its only available in BAM-NDP mode. So only write
this register for clients who is using BAM-NDP.
Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---
Change in [v4]
* Added in_range() macro
Change in [v3]
* Removed BAM_LITE macro
* Updated commit message
* Adjusted if condition check
* Renamed BAM-NDP macro to BAM_NDP_REVISION_START and
BAM_NDP_REVISION_END
Change in [v2]
* Replace 0xff with REVISION_MASK in the statement
bdev->bam_revision = val & REVISION_MASK
Change in [v1]
* Added initial patch
drivers/dma/qcom/bam_dma.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index bbc3276992bb..c14557efd577 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -59,6 +59,9 @@ struct bam_desc_hw {
#define DESC_FLAG_NWD BIT(12)
#define DESC_FLAG_CMD BIT(11)
+#define BAM_NDP_REVISION_START 0x20
+#define BAM_NDP_REVISION_END 0x27
+
struct bam_async_desc {
struct virt_dma_desc vd;
@@ -398,6 +401,7 @@ struct bam_device {
/* dma start transaction tasklet */
struct tasklet_struct task;
+ u32 bam_revision;
};
/**
@@ -441,8 +445,10 @@ static void bam_reset(struct bam_device *bdev)
writel_relaxed(val, bam_addr(bdev, 0, BAM_CTRL));
/* set descriptor threshold, start with 4 bytes */
- writel_relaxed(DEFAULT_CNT_THRSHLD,
- bam_addr(bdev, 0, BAM_DESC_CNT_TRSHLD));
+ if (in_range(bdev->bam_revision, BAM_NDP_REVISION_START,
+ BAM_NDP_REVISION_END))
+ writel_relaxed(DEFAULT_CNT_THRSHLD,
+ bam_addr(bdev, 0, BAM_DESC_CNT_TRSHLD));
/* Enable default set of h/w workarounds, ie all except BAM_FULL_PIPE */
writel_relaxed(BAM_CNFG_BITS_DEFAULT, bam_addr(bdev, 0, BAM_CNFG_BITS));
@@ -1000,9 +1006,10 @@ static void bam_apply_new_config(struct bam_chan *bchan,
maxburst = bchan->slave.src_maxburst;
else
maxburst = bchan->slave.dst_maxburst;
-
- writel_relaxed(maxburst,
- bam_addr(bdev, 0, BAM_DESC_CNT_TRSHLD));
+ if (in_range(bdev->bam_revision, BAM_NDP_REVISION_START,
+ BAM_NDP_REVISION_END))
+ writel_relaxed(maxburst,
+ bam_addr(bdev, 0, BAM_DESC_CNT_TRSHLD));
}
bchan->reconfigure = 0;
@@ -1192,10 +1199,11 @@ static int bam_init(struct bam_device *bdev)
u32 val;
/* read revision and configuration information */
- if (!bdev->num_ees) {
- val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
+ val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
+ if (!bdev->num_ees)
bdev->num_ees = (val >> NUM_EES_SHIFT) & NUM_EES_MASK;
- }
+
+ bdev->bam_revision = val & REVISION_MASK;
/* check that configured EE is within range */
if (bdev->ee >= bdev->num_ees)
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register
2024-12-20 9:42 [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register Md Sadre Alam
@ 2024-12-24 10:41 ` Vinod Koul
2025-01-07 21:30 ` Georgi Djakov
2025-01-10 10:29 ` Stephan Gerhold
2 siblings, 0 replies; 12+ messages in thread
From: Vinod Koul @ 2024-12-24 10:41 UTC (permalink / raw)
To: robin.murphy, u.kleine-koenig, martin.petersen, fenghua.yu,
av2082000, linux-arm-msm, dmaengine, linux-kernel, Md Sadre Alam
Cc: quic_mmanikan, quic_srichara, quic_varada
On Fri, 20 Dec 2024 15:12:03 +0530, Md Sadre Alam wrote:
> Avoid writing unavailable register in BAM-Lite mode.
> BAM_DESC_CNT_TRSHLD register is unavailable in BAM-Lite
> mode. Its only available in BAM-NDP mode. So only write
> this register for clients who is using BAM-NDP.
>
>
Applied, thanks!
[1/1] dmaengine: qcom: bam_dma: Avoid writing unavailable register
commit: 57a7138d0627309d469719f1845d2778c251f358
Best regards,
--
~Vinod
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register
2024-12-20 9:42 [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register Md Sadre Alam
2024-12-24 10:41 ` Vinod Koul
@ 2025-01-07 21:30 ` Georgi Djakov
2025-01-08 11:10 ` Vinod Koul
` (2 more replies)
2025-01-10 10:29 ` Stephan Gerhold
2 siblings, 3 replies; 12+ messages in thread
From: Georgi Djakov @ 2025-01-07 21:30 UTC (permalink / raw)
To: Md Sadre Alam, vkoul, linux-arm-msm, dmaengine
Cc: quic_mmanikan, quic_srichara, quic_varada, robin.murphy,
u.kleine-koenig, martin.petersen, fenghua.yu, av2082000,
linux-kernel
On 20.12.24 11:42, Md Sadre Alam wrote:
> Avoid writing unavailable register in BAM-Lite mode.
> BAM_DESC_CNT_TRSHLD register is unavailable in BAM-Lite
> mode. Its only available in BAM-NDP mode. So only write
> this register for clients who is using BAM-NDP.
>
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> ---
My Dragonboard db845c fails to boot on recent linux-next releases and
git bisect points to this patch. It boots again when it's reverted.
[..]
>
> bchan->reconfigure = 0;
> @@ -1192,10 +1199,11 @@ static int bam_init(struct bam_device *bdev)
> u32 val;
>
> /* read revision and configuration information */
> - if (!bdev->num_ees) {
> - val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
> + val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
> + if (!bdev->num_ees)
> bdev->num_ees = (val >> NUM_EES_SHIFT) & NUM_EES_MASK;
> - }
> +
> + bdev->bam_revision = val & REVISION_MASK;
The problem seems to occur when we try to read the revision for the
slimbus bam instance at 0x17184000 (which has "qcom,num-ees = <2>;").
Thanks,
Georgi
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register
2025-01-07 21:30 ` Georgi Djakov
@ 2025-01-08 11:10 ` Vinod Koul
2025-01-08 11:14 ` Md Sadre Alam
2025-01-10 5:23 ` Md Sadre Alam
2025-01-08 11:14 ` Md Sadre Alam
2025-01-10 5:21 ` Md Sadre Alam
2 siblings, 2 replies; 12+ messages in thread
From: Vinod Koul @ 2025-01-08 11:10 UTC (permalink / raw)
To: Georgi Djakov
Cc: Md Sadre Alam, linux-arm-msm, dmaengine, quic_mmanikan,
quic_srichara, quic_varada, robin.murphy, u.kleine-koenig,
martin.petersen, fenghua.yu, av2082000, linux-kernel
On 07-01-25, 23:30, Georgi Djakov wrote:
> On 20.12.24 11:42, Md Sadre Alam wrote:
> > Avoid writing unavailable register in BAM-Lite mode.
> > BAM_DESC_CNT_TRSHLD register is unavailable in BAM-Lite
> > mode. Its only available in BAM-NDP mode. So only write
> > this register for clients who is using BAM-NDP.
> >
> > Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> > ---
>
> My Dragonboard db845c fails to boot on recent linux-next releases and
> git bisect points to this patch. It boots again when it's reverted.
Should we revert?
>
> [..]
>
> > bchan->reconfigure = 0;
> > @@ -1192,10 +1199,11 @@ static int bam_init(struct bam_device *bdev)
> > u32 val;
> > /* read revision and configuration information */
> > - if (!bdev->num_ees) {
> > - val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
> > + val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
> > + if (!bdev->num_ees)
> > bdev->num_ees = (val >> NUM_EES_SHIFT) & NUM_EES_MASK;
> > - }
> > +
> > + bdev->bam_revision = val & REVISION_MASK;
>
> The problem seems to occur when we try to read the revision for the
> slimbus bam instance at 0x17184000 (which has "qcom,num-ees = <2>;").
>
> Thanks,
> Georgi
--
~Vinod
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register
2025-01-07 21:30 ` Georgi Djakov
2025-01-08 11:10 ` Vinod Koul
@ 2025-01-08 11:14 ` Md Sadre Alam
2025-01-10 5:21 ` Md Sadre Alam
2 siblings, 0 replies; 12+ messages in thread
From: Md Sadre Alam @ 2025-01-08 11:14 UTC (permalink / raw)
To: Georgi Djakov, vkoul, linux-arm-msm, dmaengine
Cc: quic_mmanikan, quic_srichara, quic_varada, robin.murphy,
u.kleine-koenig, martin.petersen, fenghua.yu, av2082000,
linux-kernel
On 1/8/2025 3:00 AM, Georgi Djakov wrote:
> On 20.12.24 11:42, Md Sadre Alam wrote:
>> Avoid writing unavailable register in BAM-Lite mode.
>> BAM_DESC_CNT_TRSHLD register is unavailable in BAM-Lite
>> mode. Its only available in BAM-NDP mode. So only write
>> this register for clients who is using BAM-NDP.
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> ---
>
> My Dragonboard db845c fails to boot on recent linux-next releases and
> git bisect points to this patch. It boots again when it's reverted.
>
> [..]
>
>> bchan->reconfigure = 0;
>> @@ -1192,10 +1199,11 @@ static int bam_init(struct bam_device *bdev)
>> u32 val;
>> /* read revision and configuration information */
>> - if (!bdev->num_ees) {
>> - val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
>> + val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
>> + if (!bdev->num_ees)
>> bdev->num_ees = (val >> NUM_EES_SHIFT) & NUM_EES_MASK;
>> - }
>> +
>> + bdev->bam_revision = val & REVISION_MASK;
>
> The problem seems to occur when we try to read the revision for the
> slimbus bam instance at 0x17184000 (which has "qcom,num-ees = <2>;").
I am checking this will fix and post the patch asap.
Thanks,
Alam.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register
2025-01-08 11:10 ` Vinod Koul
@ 2025-01-08 11:14 ` Md Sadre Alam
2025-01-10 5:23 ` Md Sadre Alam
1 sibling, 0 replies; 12+ messages in thread
From: Md Sadre Alam @ 2025-01-08 11:14 UTC (permalink / raw)
To: Vinod Koul, Georgi Djakov
Cc: linux-arm-msm, dmaengine, quic_mmanikan, quic_srichara,
quic_varada, robin.murphy, u.kleine-koenig, martin.petersen,
fenghua.yu, av2082000, linux-kernel
On 1/8/2025 4:40 PM, Vinod Koul wrote:
> On 07-01-25, 23:30, Georgi Djakov wrote:
>> On 20.12.24 11:42, Md Sadre Alam wrote:
>>> Avoid writing unavailable register in BAM-Lite mode.
>>> BAM_DESC_CNT_TRSHLD register is unavailable in BAM-Lite
>>> mode. Its only available in BAM-NDP mode. So only write
>>> this register for clients who is using BAM-NDP.
>>>
>>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>> ---
>>
>> My Dragonboard db845c fails to boot on recent linux-next releases and
>> git bisect points to this patch. It boots again when it's reverted.
>
> Should we revert?
I am checking this will fix and post patch quickly.
>
>>
>> [..]
>>
>>> bchan->reconfigure = 0;
>>> @@ -1192,10 +1199,11 @@ static int bam_init(struct bam_device *bdev)
>>> u32 val;
>>> /* read revision and configuration information */
>>> - if (!bdev->num_ees) {
>>> - val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
>>> + val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
>>> + if (!bdev->num_ees)
>>> bdev->num_ees = (val >> NUM_EES_SHIFT) & NUM_EES_MASK;
>>> - }
>>> +
>>> + bdev->bam_revision = val & REVISION_MASK;
>>
>> The problem seems to occur when we try to read the revision for the
>> slimbus bam instance at 0x17184000 (which has "qcom,num-ees = <2>;").
>>
>> Thanks,
>> Georgi
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register
2025-01-07 21:30 ` Georgi Djakov
2025-01-08 11:10 ` Vinod Koul
2025-01-08 11:14 ` Md Sadre Alam
@ 2025-01-10 5:21 ` Md Sadre Alam
2 siblings, 0 replies; 12+ messages in thread
From: Md Sadre Alam @ 2025-01-10 5:21 UTC (permalink / raw)
To: Georgi Djakov, vkoul, linux-arm-msm, dmaengine
Cc: quic_mmanikan, quic_srichara, quic_varada, robin.murphy,
u.kleine-koenig, martin.petersen, fenghua.yu, av2082000,
linux-kernel
On 1/8/2025 3:00 AM, Georgi Djakov wrote:
> On 20.12.24 11:42, Md Sadre Alam wrote:
>> Avoid writing unavailable register in BAM-Lite mode.
>> BAM_DESC_CNT_TRSHLD register is unavailable in BAM-Lite
>> mode. Its only available in BAM-NDP mode. So only write
>> this register for clients who is using BAM-NDP.
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> ---
>
> My Dragonboard db845c fails to boot on recent linux-next releases and
> git bisect points to this patch. It boots again when it's reverted.
>
> [..]
>
>> bchan->reconfigure = 0;
>> @@ -1192,10 +1199,11 @@ static int bam_init(struct bam_device *bdev)
>> u32 val;
>> /* read revision and configuration information */
>> - if (!bdev->num_ees) {
>> - val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
>> + val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
>> + if (!bdev->num_ees)
>> bdev->num_ees = (val >> NUM_EES_SHIFT) & NUM_EES_MASK;
>> - }
>> +
>> + bdev->bam_revision = val & REVISION_MASK;
>
> The problem seems to occur when we try to read the revision for the
> slimbus bam instance at 0x17184000 (which has "qcom,num-ees = <2>;").
I have posted the fix. Patch available at [1]
[1]
https://patchwork.kernel.org/project/linux-arm-msm/patch/20250110051409.4099727-1-quic_mdalam@quicinc.com/
Please check once.
Thanks,
Alam.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register
2025-01-08 11:10 ` Vinod Koul
2025-01-08 11:14 ` Md Sadre Alam
@ 2025-01-10 5:23 ` Md Sadre Alam
1 sibling, 0 replies; 12+ messages in thread
From: Md Sadre Alam @ 2025-01-10 5:23 UTC (permalink / raw)
To: Vinod Koul, Georgi Djakov
Cc: linux-arm-msm, dmaengine, quic_mmanikan, quic_srichara,
quic_varada, robin.murphy, u.kleine-koenig, martin.petersen,
fenghua.yu, av2082000, linux-kernel
On 1/8/2025 4:40 PM, Vinod Koul wrote:
> On 07-01-25, 23:30, Georgi Djakov wrote:
>> On 20.12.24 11:42, Md Sadre Alam wrote:
>>> Avoid writing unavailable register in BAM-Lite mode.
>>> BAM_DESC_CNT_TRSHLD register is unavailable in BAM-Lite
>>> mode. Its only available in BAM-NDP mode. So only write
>>> this register for clients who is using BAM-NDP.
>>>
>>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>> ---
>>
>> My Dragonboard db845c fails to boot on recent linux-next releases and
>> git bisect points to this patch. It boots again when it's reverted.
>
> Should we revert?
I have posted the fix. Patch available at [1]
[1]
https://patchwork.kernel.org/project/linux-arm-msm/patch/20250110051409.4099727-1-quic_mdalam@quicinc.com/
>
>>
>> [..]
>>
>>> bchan->reconfigure = 0;
>>> @@ -1192,10 +1199,11 @@ static int bam_init(struct bam_device *bdev)
>>> u32 val;
>>> /* read revision and configuration information */
>>> - if (!bdev->num_ees) {
>>> - val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
>>> + val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
>>> + if (!bdev->num_ees)
>>> bdev->num_ees = (val >> NUM_EES_SHIFT) & NUM_EES_MASK;
>>> - }
>>> +
>>> + bdev->bam_revision = val & REVISION_MASK;
>>
>> The problem seems to occur when we try to read the revision for the
>> slimbus bam instance at 0x17184000 (which has "qcom,num-ees = <2>;").
>>
>> Thanks,
>> Georgi
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register
2024-12-20 9:42 [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register Md Sadre Alam
2024-12-24 10:41 ` Vinod Koul
2025-01-07 21:30 ` Georgi Djakov
@ 2025-01-10 10:29 ` Stephan Gerhold
2025-01-10 11:59 ` Md Sadre Alam
2 siblings, 1 reply; 12+ messages in thread
From: Stephan Gerhold @ 2025-01-10 10:29 UTC (permalink / raw)
To: Md Sadre Alam
Cc: vkoul, robin.murphy, u.kleine-koenig, martin.petersen, fenghua.yu,
av2082000, linux-arm-msm, dmaengine, linux-kernel, quic_mmanikan,
quic_srichara, quic_varada
On Fri, Dec 20, 2024 at 03:12:03PM +0530, Md Sadre Alam wrote:
> Avoid writing unavailable register in BAM-Lite mode.
> BAM_DESC_CNT_TRSHLD register is unavailable in BAM-Lite
> mode. Its only available in BAM-NDP mode. So only write
> this register for clients who is using BAM-NDP.
>
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
What are we actually fixing here? Which platform is affected? Is there a
crash, reset, or incorrect behavior?
We have had this code for years without reported issues, with both
BAM-NDP and BAM-Lite instances. The register documentation on APQ8016E
documents the BAM_DESC_CNT_TRSHLD register even for the BAM-Lite
instance. There is a comment that it doesn't apply to BAM-Lite, but I
would expect the written value just ends up being ignored in that case.
Also, there is not just BAM-NDP and BAM-Lite, but also plain "BAM". What
about that one? Should we write to BAM_DESC_CNT_TRSHLD?
> ---
> Change in [v4]
>
> * Added in_range() macro
>
> Change in [v3]
>
> * Removed BAM_LITE macro
>
> * Updated commit message
>
> * Adjusted if condition check
>
> * Renamed BAM-NDP macro to BAM_NDP_REVISION_START and
> BAM_NDP_REVISION_END
>
> Change in [v2]
>
> * Replace 0xff with REVISION_MASK in the statement
> bdev->bam_revision = val & REVISION_MASK
>
> Change in [v1]
>
> * Added initial patch
>
> drivers/dma/qcom/bam_dma.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index bbc3276992bb..c14557efd577 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
> @@ -59,6 +59,9 @@ struct bam_desc_hw {
> #define DESC_FLAG_NWD BIT(12)
> #define DESC_FLAG_CMD BIT(11)
>
> +#define BAM_NDP_REVISION_START 0x20
> +#define BAM_NDP_REVISION_END 0x27
> +
Are you sure this covers all SoCs we support upstream? If one of the
older or newer supported SoCs uses a value outside of this range, it
will now be missing the register write.
Thanks,
Stephan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register
2025-01-10 10:29 ` Stephan Gerhold
@ 2025-01-10 11:59 ` Md Sadre Alam
2025-01-10 12:07 ` Stephan Gerhold
0 siblings, 1 reply; 12+ messages in thread
From: Md Sadre Alam @ 2025-01-10 11:59 UTC (permalink / raw)
To: Stephan Gerhold
Cc: vkoul, robin.murphy, u.kleine-koenig, martin.petersen, fenghua.yu,
av2082000, linux-arm-msm, dmaengine, linux-kernel, quic_mmanikan,
quic_srichara, quic_varada
On 1/10/2025 3:59 PM, Stephan Gerhold wrote:
> On Fri, Dec 20, 2024 at 03:12:03PM +0530, Md Sadre Alam wrote:
>> Avoid writing unavailable register in BAM-Lite mode.
>> BAM_DESC_CNT_TRSHLD register is unavailable in BAM-Lite
>> mode. Its only available in BAM-NDP mode. So only write
>> this register for clients who is using BAM-NDP.
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>
> What are we actually fixing here? Which platform is affected? Is there a
> crash, reset, or incorrect behavior?
On SDX75, QPIC use BAM-Lite and as per HW description this
BAM_DESC_CNT_TRSHLD register is not available, and writing to this
SDX75 was hanging.
>
> We have had this code for years without reported issues, with both
> BAM-NDP and BAM-Lite instances. The register documentation on APQ8016E
> documents the BAM_DESC_CNT_TRSHLD register even for the BAM-Lite
> instance. There is a comment that it doesn't apply to BAM-Lite, but I
> would expect the written value just ends up being ignored in that case.
With older xPU it was being ignored but with new xPU its hanging. HW
team suggested don't write this register for BAM-Lite mode since its not
available.
>
> Also, there is not just BAM-NDP and BAM-Lite, but also plain "BAM". What
> about that one? Should we write to BAM_DESC_CNT_TRSHLD?
Apart from BAM-Lite this register available in all the BAM
>
>> ---
>> Change in [v4]
>>
>> * Added in_range() macro
>>
>> Change in [v3]
>>
>> * Removed BAM_LITE macro
>>
>> * Updated commit message
>>
>> * Adjusted if condition check
>>
>> * Renamed BAM-NDP macro to BAM_NDP_REVISION_START and
>> BAM_NDP_REVISION_END
>>
>> Change in [v2]
>>
>> * Replace 0xff with REVISION_MASK in the statement
>> bdev->bam_revision = val & REVISION_MASK
>>
>> Change in [v1]
>>
>> * Added initial patch
>>
>> drivers/dma/qcom/bam_dma.c | 24 ++++++++++++++++--------
>> 1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
>> index bbc3276992bb..c14557efd577 100644
>> --- a/drivers/dma/qcom/bam_dma.c
>> +++ b/drivers/dma/qcom/bam_dma.c
>> @@ -59,6 +59,9 @@ struct bam_desc_hw {
>> #define DESC_FLAG_NWD BIT(12)
>> #define DESC_FLAG_CMD BIT(11)
>>
>> +#define BAM_NDP_REVISION_START 0x20
>> +#define BAM_NDP_REVISION_END 0x27
>> +
>
> Are you sure this covers all SoCs we support upstream? If one of the
> older or newer supported SoCs uses a value outside of this range, it
> will now be missing the register write.
I got this data from HW team. Will confirm once again if any SOCs we are
missing.
Thanks,
Alam.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register
2025-01-10 11:59 ` Md Sadre Alam
@ 2025-01-10 12:07 ` Stephan Gerhold
2025-01-10 12:23 ` Md Sadre Alam
0 siblings, 1 reply; 12+ messages in thread
From: Stephan Gerhold @ 2025-01-10 12:07 UTC (permalink / raw)
To: Md Sadre Alam
Cc: vkoul, robin.murphy, u.kleine-koenig, martin.petersen, fenghua.yu,
av2082000, linux-arm-msm, dmaengine, linux-kernel, quic_mmanikan,
quic_srichara, quic_varada
On Fri, Jan 10, 2025 at 05:29:29PM +0530, Md Sadre Alam wrote:
> On 1/10/2025 3:59 PM, Stephan Gerhold wrote:
> > On Fri, Dec 20, 2024 at 03:12:03PM +0530, Md Sadre Alam wrote:
> > > Avoid writing unavailable register in BAM-Lite mode.
> > > BAM_DESC_CNT_TRSHLD register is unavailable in BAM-Lite
> > > mode. Its only available in BAM-NDP mode. So only write
> > > this register for clients who is using BAM-NDP.
> > >
> > > Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> >
> > What are we actually fixing here? Which platform is affected? Is there a
> > crash, reset, or incorrect behavior?
> On SDX75, QPIC use BAM-Lite and as per HW description this
> BAM_DESC_CNT_TRSHLD register is not available, and writing to this
> SDX75 was hanging.
> >
> > We have had this code for years without reported issues, with both
> > BAM-NDP and BAM-Lite instances. The register documentation on APQ8016E
> > documents the BAM_DESC_CNT_TRSHLD register even for the BAM-Lite
> > instance. There is a comment that it doesn't apply to BAM-Lite, but I
> > would expect the written value just ends up being ignored in that case.
> With older xPU it was being ignored but with new xPU its hanging. HW
> team suggested don't write this register for BAM-Lite mode since its not
> available.
> >
OK, thanks for the explanation.
> > Also, there is not just BAM-NDP and BAM-Lite, but also plain "BAM". What
> > about that one? Should we write to BAM_DESC_CNT_TRSHLD?
> Apart from BAM-Lite this register available in all the BAM
Please check again if we need to check for additional revision numbers
for the non-NDP BAM types then. Or alternatively, change the check to
write the register on if (!BAM-Lite) instead of if (BAM-NDP). That might
be easier.
Thanks,
Stephan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register
2025-01-10 12:07 ` Stephan Gerhold
@ 2025-01-10 12:23 ` Md Sadre Alam
0 siblings, 0 replies; 12+ messages in thread
From: Md Sadre Alam @ 2025-01-10 12:23 UTC (permalink / raw)
To: Stephan Gerhold
Cc: vkoul, robin.murphy, u.kleine-koenig, martin.petersen, fenghua.yu,
av2082000, linux-arm-msm, dmaengine, linux-kernel, quic_mmanikan,
quic_srichara, quic_varada
On 1/10/2025 5:37 PM, Stephan Gerhold wrote:
> On Fri, Jan 10, 2025 at 05:29:29PM +0530, Md Sadre Alam wrote:
>> On 1/10/2025 3:59 PM, Stephan Gerhold wrote:
>>> On Fri, Dec 20, 2024 at 03:12:03PM +0530, Md Sadre Alam wrote:
>>>> Avoid writing unavailable register in BAM-Lite mode.
>>>> BAM_DESC_CNT_TRSHLD register is unavailable in BAM-Lite
>>>> mode. Its only available in BAM-NDP mode. So only write
>>>> this register for clients who is using BAM-NDP.
>>>>
>>>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>>
>>> What are we actually fixing here? Which platform is affected? Is there a
>>> crash, reset, or incorrect behavior?
>> On SDX75, QPIC use BAM-Lite and as per HW description this
>> BAM_DESC_CNT_TRSHLD register is not available, and writing to this
>> SDX75 was hanging.
>>>
>>> We have had this code for years without reported issues, with both
>>> BAM-NDP and BAM-Lite instances. The register documentation on APQ8016E
>>> documents the BAM_DESC_CNT_TRSHLD register even for the BAM-Lite
>>> instance. There is a comment that it doesn't apply to BAM-Lite, but I
>>> would expect the written value just ends up being ignored in that case.
>> With older xPU it was being ignored but with new xPU its hanging. HW
>> team suggested don't write this register for BAM-Lite mode since its not
>> available.
>>>
>
> OK, thanks for the explanation.
>
>>> Also, there is not just BAM-NDP and BAM-Lite, but also plain "BAM". What
>>> about that one? Should we write to BAM_DESC_CNT_TRSHLD?
>> Apart from BAM-Lite this register available in all the BAM
>
> Please check again if we need to check for additional revision numbers
> for the non-NDP BAM types then. Or alternatively, change the check to
> write the register on if (!BAM-Lite) instead of if (BAM-NDP). That might
> be easier.
Ok Thanks. will check once again this and post in next revision.
>
> Thanks,
> Stephan
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-01-10 12:23 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-20 9:42 [PATCH v4] dmaengine: qcom: bam_dma: Avoid writing unavailable register Md Sadre Alam
2024-12-24 10:41 ` Vinod Koul
2025-01-07 21:30 ` Georgi Djakov
2025-01-08 11:10 ` Vinod Koul
2025-01-08 11:14 ` Md Sadre Alam
2025-01-10 5:23 ` Md Sadre Alam
2025-01-08 11:14 ` Md Sadre Alam
2025-01-10 5:21 ` Md Sadre Alam
2025-01-10 10:29 ` Stephan Gerhold
2025-01-10 11:59 ` Md Sadre Alam
2025-01-10 12:07 ` Stephan Gerhold
2025-01-10 12:23 ` Md Sadre Alam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox