[PATCH 1/1] misc: pci_endpoint_test: Refactor dma_set_mask_and_coherent() logic

[PATCH 1/1] misc: pci_endpoint_test: Refactor dma_set_mask_and_coherent() logic

[Frank Li]

dma_set_mask_and_coherent() will never return failure when mask >= 32bit.
So needn't  fall back to set dma_set_mask_and_coherent(32).

Even if dma_set_mask_and_coherent(48) failure,
dma_set_mask_and_coherent(32) will be failure according to the same reason.

The function dma_set_mask_and_coherent() defines the device DMA access
address width. If it's capable of accessing 48 bits, it inherently supports
32-bit space as well.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Ref: https://lore.kernel.org/linux-pci/20240328154827.809286-1-Frank.Li@nxp.com/T/#u
    
    for document change patch. DMA document sample code is miss leading.

 drivers/misc/pci_endpoint_test.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index c38a6083f0a73..56ac6969a8f59 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -824,11 +824,7 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 	init_completion(&test->irq_raised);
 	mutex_init(&test->mutex);
 
-	if ((dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(48)) != 0) &&
-	    dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)) != 0) {
-		dev_err(dev, "Cannot set DMA mask\n");
-		return -EINVAL;
-	}
+	dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(48));
 
 	err = pci_enable_device(pdev);
 	if (err) {
-- 
2.34.1

Re: [PATCH 1/1] misc: pci_endpoint_test: Refactor dma_set_mask_and_coherent() logic

[Manivannan Sadhasivam]

On Thu, Mar 28, 2024 at 12:06:32PM -0400, Frank Li wrote:
> dma_set_mask_and_coherent() will never return failure when mask >= 32bit.
> So needn't  fall back to set dma_set_mask_and_coherent(32).

dma_set_mask_and_coherent() should never fail when the mask is >= 32bit, unless
the architecture has no DMA support. So no need check for the error and also no
need to set dma_set_mask_and_coherent(32) as a fallback.

> 
> Even if dma_set_mask_and_coherent(48) failure,
> dma_set_mask_and_coherent(32) will be failure according to the same reason.
> 

Even if dma_set_mask_and_coherent(48) fails due to the lack of DMA support
(theoretically), then dma_set_mask_and_coherent(32) will also fail for the same
reason. So the fallback doesn't make sense.

> The function dma_set_mask_and_coherent() defines the device DMA access
> address width. If it's capable of accessing 48 bits, it inherently supports
> 32-bit space as well.
> 

Due to the above reasons, let's simplify the code by setting the streaming and
coherent DMA mask to 48 bits.

> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
> 
> Notes:
>     Ref: https://lore.kernel.org/linux-pci/20240328154827.809286-1-Frank.Li@nxp.com/T/#u
>     
>     for document change patch. DMA document sample code is miss leading.
> 
>  drivers/misc/pci_endpoint_test.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index c38a6083f0a73..56ac6969a8f59 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -824,11 +824,7 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
>  	init_completion(&test->irq_raised);
>  	mutex_init(&test->mutex);
>  
> -	if ((dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(48)) != 0) &&
> -	    dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)) != 0) {
> -		dev_err(dev, "Cannot set DMA mask\n");
> -		return -EINVAL;
> -	}
> +	dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(48));
>  
>  	err = pci_enable_device(pdev);
>  	if (err) {
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 1/1] misc: pci_endpoint_test: Refactor dma_set_mask_and_coherent() logic

[Niklas Cassel]

On Thu, Mar 28, 2024 at 12:06:32PM -0400, Frank Li wrote:
> dma_set_mask_and_coherent() will never return failure when mask >= 32bit.
> So needn't  fall back to set dma_set_mask_and_coherent(32).
> 
> Even if dma_set_mask_and_coherent(48) failure,
> dma_set_mask_and_coherent(32) will be failure according to the same reason.
> 
> The function dma_set_mask_and_coherent() defines the device DMA access
> address width. If it's capable of accessing 48 bits, it inherently supports
> 32-bit space as well.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---

Kishon,

do you remember why you set the DMA mask to 48-bits in the first place?
It seems a bit random.


Kind regards,
Niklas