On Mon, Jul 23, 2018 at 11:16:11PM +0100, Robin Murphy wrote:
> Now that we can track upstream DMA constraints properly with
> bus_dma_mask instead of trying (and failing) to maintain it in
> coherent_dma_mask, it doesn't make much sense for the firmware code to
> be touching the latter at all. It's merely papering over bugs wherein a
> driver has failed to call dma_set_coherent_mask() *and* the bus code has
> not initialised any default value.

Nit: I do not think the driver had a chance to probe and therefore call
dma_set_coherent_mask() before iort_dma_setup() is executed, right ?

Anyway, the patch makes perfect sense:

Acked-by: Lorenzo Pieralisi <lorenzo.pieral...@arm.com>

> We don't really want to encourage more drivers coercing dma_mask so
> we'll continue to fix that up if necessary, but add a warning to help
> flush out any such buggy bus code that remains.
> 
> CC: Lorenzo Pieralisi <lorenzo.pieral...@arm.com>
> CC: Hanjun Guo <hanjun....@linaro.org>
> CC: Sudeep Holla <sudeep.ho...@arm.com>
> Signed-off-by: Robin Murphy <robin.mur...@arm.com>
> ---
>  drivers/acpi/arm64/iort.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index bc51cff5505e..08f26db2da7e 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -978,20 +978,20 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, 
> u64 *dma_size)
>       int ret, msb;
>  
>       /*
> -      * Set default coherent_dma_mask to 32 bit.  Drivers are expected to
> -      * setup the correct supported mask.
> +      * If @dev is expected to be DMA-capable then the bus code that created
> +      * it should have initialised its dma_mask pointer by this point. For
> +      * now, we'll continue the legacy behaviour of coercing it to the
> +      * coherent mask if not, but we'll no longer do so quietly.
>        */
> -     if (!dev->coherent_dma_mask)
> -             dev->coherent_dma_mask = DMA_BIT_MASK(32);
> -
> -     /*
> -      * Set it to coherent_dma_mask by default if the architecture
> -      * code has not set it.
> -      */
> -     if (!dev->dma_mask)
> +     if (!dev->dma_mask) {
> +             dev_warn(dev, "DMA mask not set\n");
>               dev->dma_mask = &dev->coherent_dma_mask;
> +     }
>  
> -     size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
> +     if (dev->coherent_dma_mask)
> +             size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
> +     else
> +             size = 1ULL << 32;
>  
>       if (dev_is_pci(dev)) {
>               ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
> -- 
> 2.17.1.dirty
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to