| Message ID | 20240328160632.848414-1-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Manivannan Sadhasivam |
| Headers | show |
| Series | [1/1] misc: pci_endpoint_test: Refactor dma_set_mask_and_coherent() logic | expand |
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 >
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
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) {
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(-)