Skip to contentSkip to conversation

dmaengine: ppc4xx: fix format specifiers for size_t arguments

PATCH Superseded Rosen Penev 3 participants 3 messages Sep 10 – Sep 11 dmaengine

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Rosen Penev

author
#0 lore raw

PATCH 1/1 dmaengine: ppc4xx: fix format specifiers for size_t arguments

Several dev_err, dev_dbg, pr_debug, and printk calls use %d or %u format specifiers for size_t arguments (len, pool_size). Fix them to use %zu to avoid warnings on 64-bit builds.

Fixes 12458ea06efd7 ("ppc440spe-adma: adds updated ppc440spe adma driver")
Assisted-by opencode:big-pickle
Signed-off-by Rosen Penev <[email protected]>
---
 drivers/dma/ppc4xx/adma.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/dma/ppc4xx/adma.c +6 -6
index 279a431ccae3..bcc54d584e2b 100644
@@ -799,7 +799,7 @@ static int ppc440spe_chan_xor_slot_count(size_t len, int src_cnt,
 	if (likely(len <= PPC440SPE_ADMA_XOR_MAX_BYTE_COUNT))
 		return slot_cnt;
 
-	printk(KERN_ERR "%s: len %d > max %d !!\n",
+	printk(KERN_ERR "%s: len %zu > max %d !!\n",
 		__func__, len, PPC440SPE_ADMA_XOR_MAX_BYTE_COUNT);
 	BUG();
 	return slot_cnt;

@@ -1989,7 +1989,7 @@ static struct dma_async_tx_descriptor *ppc440spe_adma_prep_dma_memcpy(
 	spin_lock_bh(&ppc440spe_chan->lock);
 
 	dev_dbg(ppc440spe_chan->device->common.dev,
-		"ppc440spe adma%d: %s len: %u int_en %d\n",
+		"ppc440spe adma%d: %s len: %zu int_en %d\n",
 		ppc440spe_chan->device->id, __func__, len,
 		flags & DMA_PREP_INTERRUPT ? 1 : 0);
 	slot_cnt = slots_per_op = 1;

@@ -2030,7 +2030,7 @@ static struct dma_async_tx_descriptor *ppc440spe_adma_prep_dma_xor(
 	BUG_ON(len > PPC440SPE_ADMA_XOR_MAX_BYTE_COUNT);
 
 	dev_dbg(ppc440spe_chan->device->common.dev,
-		"ppc440spe adma%d: %s src_cnt: %d len: %u int_en: %d\n",
+		"ppc440spe adma%d: %s src_cnt: %d len: %zu int_en: %d\n",
 		ppc440spe_chan->device->id, __func__, src_cnt, len,
 		flags & DMA_PREP_INTERRUPT ? 1 : 0);
 

@@ -2279,7 +2279,7 @@ static struct ppc440spe_adma_desc_slot *ppc440spe_dma01_prep_pq(
 	unsigned long op = 0;
 	unsigned char mult = 1;
 
-	pr_debug("%s: dst_cnt %d, src_cnt %d, len %d\n",
+	pr_debug("%s: dst_cnt %d, src_cnt %d, len %zu\n",
 		 __func__, dst_cnt, src_cnt, len);
 	/*  select operations WXOR/RXOR depending on the
 	 * source addresses of operators and the number

@@ -2553,7 +2553,7 @@ static struct dma_async_tx_descriptor *ppc440spe_adma_prep_dma_pq(
 	BUG_ON(!dst_cnt);
 
 	dev_dbg(ppc440spe_chan->device->common.dev,
-		"ppc440spe adma%d: %s src_cnt: %d len: %u int_en: %d\n",
+		"ppc440spe adma%d: %s src_cnt: %d len: %zu int_en: %d\n",
 		ppc440spe_chan->device->id, __func__, src_cnt, len,
 		flags & DMA_PREP_INTERRUPT ? 1 : 0);
 

@@ -4077,7 +4077,7 @@ static int ppc440spe_adma_probe(struct platform_device *ofdev)
 					adev->pool_size, &adev->dma_desc_pool,
 					GFP_KERNEL);
 	if (adev->dma_desc_pool_virt == NULL) {
-		dev_err(&ofdev->dev, "failed to allocate %d bytes of coherent "
+		dev_err(&ofdev->dev, "failed to allocate %zu bytes of coherent "
 			"memory for hardware descriptors\n",
 			adev->pool_size);
 		initcode = PPC_ADMA_INIT_COHERENT;
Signature · 1 line
2.55.0
AI review
#1 lore raw
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing check for dma_map_page() failure in device probe.
- [High] Calling dma_free_coherent() with an uninitialized (NULL) device pointer in the ioremap error path.
- [High] Missing free_irq() for adev->err_irq in the error path of ppc440spe_adma_setup_irqs. --

commit 6506d0727c09a20b6e812b6d3009a7ffe42bfacf
Author: Rosen Penev <[email protected]>

dmaengine: ppc4xx: fix format specifiers for size_t arguments

Several dev_err, dev_dbg, pr_debug, and printk calls use %d or %u format specifiers for size_t arguments. Fix them to use %zu to avoid warnings on 64-bit builds.

Quoted 4 lines
drivers/dma/ppc4xx/adma.c +0 -0
index 279a431ccae38..bcc54d584e2bf 100644

[ ... ]

drivers/dma/ppc4xx/adma.c static int ppc440spe_adma_probe(struct platform_device *ofdev) · line 4083

Quoted 9 lines
drivers/dma/ppc4xx/adma.c +1 -1
@@ -4077,7 +4077,7 @@ static int ppc440spe_adma_probe(struct platform_device *ofdev)
 					adev->pool_size, &adev->dma_desc_pool,
 					GFP_KERNEL);
 	if (adev->dma_desc_pool_virt == NULL) {
-		dev_err(&ofdev->dev, "failed to allocate %d bytes of coherent "
+		dev_err(&ofdev->dev, "failed to allocate %zu bytes of coherent "
 			"memory for hardware descriptors\n",
 			adev->pool_size);
 		initcode = PPC_ADMA_INIT_COHERENT;

[Severity: High]
This isn't a bug introduced by this patch, but while looking at ppc440spe_adma_probe(), I noticed we map the destination pages without checking for mapping errors:

    chan->pdest = dma_map_page(&ofdev->dev, chan->pdest_page, 0,
                               PAGE_SIZE, DMA_BIDIRECTIONAL);
    chan->qdest = dma_map_page(&ofdev->dev, chan->qdest_page, 0,
                               PAGE_SIZE, DMA_BIDIRECTIONAL);

If the mapping fails, it returns an error address. Does this mean the hardware might be configured with an invalid DMA destination, potentially causing memory corruption or IOMMU faults?

Should we be validating the return values with dma_mapping_error()?

[Severity: High]
This is another pre-existing issue in ppc440spe_adma_probe(), but I'm curious about the error handling if ioremap() fails:

    regs = ioremap(res.start, resource_size(&res));
    if (!regs) {
        dev_err(&ofdev->dev, "failed to ioremap regs!\n");
        ret = -ENOMEM;
        goto err_regs_alloc;
    }

This jumps to err_regs_alloc before adev->dev is initialized, where we do:

err_regs_alloc:

    dma_free_coherent(adev->dev, adev->pool_size,
                      adev->dma_desc_pool_virt,
                      adev->dma_desc_pool);

Since adev->dev is still NULL at this point, could this cause a crash in dma_free_coherent() when trying to resolve the DMA operations?

[Severity: High]
A third pre-existing issue I noticed is in the error path of ppc440spe_adma_setup_irqs(), which is called from this probe function.

If we fail to find the device tree node after successfully registering adev->err_irq:

    np = of_find_compatible_node(NULL, NULL, "ibm,i2o-440spe");
    if (!np) {
        pr_err("%s: can't find I2O device tree node\n", __func__);
        ret = -ENODEV;
        goto err_req2;
    }

We jump to err_req2, which frees adev->irq but misses freeing adev->err_irq:

err_req2:

    free_irq(adev->irq, chan);
err_req1:
    irq_dispose_mapping(adev->irq);
err_irq_map:
    if (adev->err_irq > 0) {
        if (atomic_dec_and_test(&ppc440spe_adma_err_irq_ref))
            irq_dispose_mapping(adev->err_irq);
    }

Since the probe function later frees the chan structure on failure, could this leave a dangling interrupt handler pointing to the freed chan pointer?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#2 lore raw
In reply to Rosen Penev

On Thu, Sep 10, 2026 at 02:14:40PM -0700, Rosen Penev wrote:

dmaengine: ppc4xx: change to %zu format specifiers for size_t arguments.

Frank

Quoted 72 lines

Several dev_err, dev_dbg, pr_debug, and printk calls use %d or %u format specifiers for size_t arguments (len, pool_size). Fix them to use %zu to avoid warnings on 64-bit builds.

Fixes 12458ea06efd7 ("ppc440spe-adma: adds updated ppc440spe adma driver")
Assisted-by opencode:big-pickle
Signed-off-by Rosen Penev <[email protected]>
---
 drivers/dma/ppc4xx/adma.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/dma/ppc4xx/adma.c +6 -7
index 279a431ccae3..bcc54d584e2b 100644
@@ -799,7 +799,7 @@ static int ppc440spe_chan_xor_slot_count(size_t len, int src_cnt,
 	if (likely(len <= PPC440SPE_ADMA_XOR_MAX_BYTE_COUNT))
 		return slot_cnt;
 
-	printk(KERN_ERR "%s: len %d > max %d !!\n",
+	printk(KERN_ERR "%s: len %zu > max %d !!\n",
 		__func__, len, PPC440SPE_ADMA_XOR_MAX_BYTE_COUNT);
 	BUG();
 	return slot_cnt;

@@ -1989,7 +1989,7 @@ static struct dma_async_tx_descriptor *ppc440spe_adma_prep_dma_memcpy(
 	spin_lock_bh(&ppc440spe_chan->lock);
 
 	dev_dbg(ppc440spe_chan->device->common.dev,
-		"ppc440spe adma%d: %s len: %u int_en %d\n",
+		"ppc440spe adma%d: %s len: %zu int_en %d\n",
 		ppc440spe_chan->device->id, __func__, len,
 		flags & DMA_PREP_INTERRUPT ? 1 : 0);
 	slot_cnt = slots_per_op = 1;

@@ -2030,7 +2030,7 @@ static struct dma_async_tx_descriptor *ppc440spe_adma_prep_dma_xor(
 	BUG_ON(len > PPC440SPE_ADMA_XOR_MAX_BYTE_COUNT);
 
 	dev_dbg(ppc440spe_chan->device->common.dev,
-		"ppc440spe adma%d: %s src_cnt: %d len: %u int_en: %d\n",
+		"ppc440spe adma%d: %s src_cnt: %d len: %zu int_en: %d\n",
 		ppc440spe_chan->device->id, __func__, src_cnt, len,
 		flags & DMA_PREP_INTERRUPT ? 1 : 0);
 

@@ -2279,7 +2279,7 @@ static struct ppc440spe_adma_desc_slot *ppc440spe_dma01_prep_pq(
 	unsigned long op = 0;
 	unsigned char mult = 1;
 
-	pr_debug("%s: dst_cnt %d, src_cnt %d, len %d\n",
+	pr_debug("%s: dst_cnt %d, src_cnt %d, len %zu\n",
 		 __func__, dst_cnt, src_cnt, len);
 	/*  select operations WXOR/RXOR depending on the
 	 * source addresses of operators and the number

@@ -2553,7 +2553,7 @@ static struct dma_async_tx_descriptor *ppc440spe_adma_prep_dma_pq(
 	BUG_ON(!dst_cnt);
 
 	dev_dbg(ppc440spe_chan->device->common.dev,
-		"ppc440spe adma%d: %s src_cnt: %d len: %u int_en: %d\n",
+		"ppc440spe adma%d: %s src_cnt: %d len: %zu int_en: %d\n",
 		ppc440spe_chan->device->id, __func__, src_cnt, len,
 		flags & DMA_PREP_INTERRUPT ? 1 : 0);
 

@@ -4077,7 +4077,7 @@ static int ppc440spe_adma_probe(struct platform_device *ofdev)
 					adev->pool_size, &adev->dma_desc_pool,
 					GFP_KERNEL);
 	if (adev->dma_desc_pool_virt == NULL) {
-		dev_err(&ofdev->dev, "failed to allocate %d bytes of coherent "
+		dev_err(&ofdev->dev, "failed to allocate %zu bytes of coherent "
 			"memory for hardware descriptors\n",
 			adev->pool_size);
 		initcode = PPC_ADMA_INIT_COHERENT;
--

2.55.0