Hi Christoph,
On Fri, May 25, 2018 at 11:20:52AM +0200, Christoph Hellwig wrote:
> swiotlb_dma_supported will always return true for the a mask
> large enough to be covered by wired up physical address, so this
> function is pointless.
Shouldn't this be "large enough to cover all wired up physical
addresses"? Or maybe even more correctly "large enough to cover all DMA
addresses that might be used"?
Also, there's a spurious "the" on the first line.
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> arch/mips/loongson64/common/dma-swiotlb.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/arch/mips/loongson64/common/dma-swiotlb.c b/arch/mips/loongson64/common/dma-swiotlb.c
> index 6a739f8ae110..a5e50f2ec301 100644
> --- a/arch/mips/loongson64/common/dma-swiotlb.c
> +++ b/arch/mips/loongson64/common/dma-swiotlb.c
> @@ -56,13 +56,6 @@ static void loongson_dma_sync_sg_for_device(struct device *dev,
> mb();
> }
>
> -static int loongson_dma_supported(struct device *dev, u64 mask)
> -{
> - if (mask > DMA_BIT_MASK(loongson_sysconf.dma_mask_bits))
> - return 0;
The comparison here will only evaluate true (indicating that DMA is not
supported) if the mask is larger than that supported by the system,
right? Therefore is this not essentially a check that the mask is
appropriately small, whilst swiotlb_dma_supported() checks that the mask
is appropriately large?
I'm struggling to understand how it follows that this function is
pointless given the behaviour of swiotlb_dma_supported(), which is what
the commit message suggests.
Thanks,
Paul
> - return swiotlb_dma_supported(dev, mask);
> -}
> -
> dma_addr_t __phys_to_dma(struct device *dev, phys_addr_t paddr)
> {
> long nid;
> @@ -99,7 +92,7 @@ static const struct dma_map_ops loongson_dma_map_ops = {
> .sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
> .sync_sg_for_device = loongson_dma_sync_sg_for_device,
> .mapping_error = swiotlb_dma_mapping_error,
> - .dma_supported = loongson_dma_supported,
> + .dma_supported = swiotlb_dma_supported,
> };
>
> void __init plat_swiotlb_setup(void)
> --
> 2.17.0
>
>
