On Mon, Mar 25, 2024 at 03:23:55PM +0100, Niklas Cassel wrote:
> There are two things that made me read this code multiple times:
>
> 1) There is a parenthesis around the first conditional, but not
> around the second conditional. This is inconsistent, and makes
> you assume that the return value should be treated differently.
>
> 2) There is no need to explicitly write != 0 in a conditional.
>
> Remove the superfluous parenthesis and != 0.
>
> No functional change intended.
>
> Signed-off-by: Niklas Cassel <cassel@xxxxxxxxxx>
> ---
> drivers/misc/pci_endpoint_test.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index bf64d3aff7d8..1005dfdf664e 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -854,8 +854,8 @@ 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) {
> + if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(48)) &&
> + dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32))) {
Actaully above orginal code is wrong. If
dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(48)) failure,
dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)) must be failure.
Needn't retry 32bit.
https://lore.kernel.org/linux-pci/925da248-f582-6dd5-57ab-0fadc4e84f9e@xxxxxxxxxx/
I am also strange where 48 come from. It should be EP side access windows
capiblity. Idealy, it should read from BAR0 or use device id to decide
dma mask. If EP side only support 32bit dma space.
dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(48) still return
success because host side may support more than 48bit DMA cap.
endpoint_test will set > 4G address to EP side. EP actaully can't access
it.
Most dwc ep controller it should be 64.
//64bit mask never return failure.
dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
if (drvdata.flags & 32_BIT_ONLY)
if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32))
err_code..
Or
if (dma_set_mask_and_coherent(&pdev->dev, drvdata.dmamask))
err_code;
Frank
> dev_err(dev, "Cannot set DMA mask\n");
> return -EINVAL;
> }
> --
> 2.44.0
>
