lkml.org 
[lkml]   [2019]   [Jun]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From
Date
On 6/11/19 1:05 AM, Christoph Hellwig wrote:
> On Mon, Jun 10, 2019 at 11:09:47AM -0500, Larry Finger wrote:
>
> What might be confusing in your output is that dev->dma_mask is a pointer,
> and we are setting it in dma_set_mask. That is before we only check
> if the pointer is set, and later we override it. Of course this doesn't
> actually explain the failure. But what is even more strange to me
> is that you get a return value from dma_supported() that isn't 0 or 1,
> as that function is supposed to return a boolean, and I really can't see
> how mask >= __phys_to_dma(dev, min_mask), would return anything but 0
> or 1. Does the output change if you use the correct printk specifiers?
>
> i.e. with a debug patch like this:
>
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 2c2772e9702a..9e5b30b12b10 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -378,6 +378,7 @@ EXPORT_SYMBOL(dma_direct_map_resource);
> int dma_direct_supported(struct device *dev, u64 mask)
> {
> u64 min_mask;
> + bool ret;
>
> if (IS_ENABLED(CONFIG_ZONE_DMA))
> min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> @@ -391,7 +392,12 @@ int dma_direct_supported(struct device *dev, u64 mask)
> * use __phys_to_dma() here so that the SME encryption mask isn't
> * part of the check.
> */
> - return mask >= __phys_to_dma(dev, min_mask);
> + ret = (mask >= __phys_to_dma(dev, min_mask));
> + if (!ret)
> + dev_info(dev,
> + "%s: failed (mask = 0x%llx, min_mask = 0x%llx/0x%llx, dma bits = %d\n",
> + __func__, mask, min_mask, __phys_to_dma(dev, min_mask), ARCH_ZONE_DMA_BITS);
> + return ret;
> }
>
> size_t dma_direct_max_mapping_size(struct device *dev)
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index f7afdadb6770..6c57ccdee2ae 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -317,8 +317,14 @@ void arch_dma_set_mask(struct device *dev, u64 mask);
>
> int dma_set_mask(struct device *dev, u64 mask)
> {
> - if (!dev->dma_mask || !dma_supported(dev, mask))
> + if (!dev->dma_mask) {
> + dev_info(dev, "no DMA mask set!\n");
> return -EIO;
> + }
> + if (!dma_supported(dev, mask)) {
> + printk("DMA not supported\n");
> + return -EIO;
> + }
>
> arch_dma_set_mask(dev, mask);
> dma_check_mask(dev, mask);
>

After I got the correct formatting, the output with this patch only gives the
following in dmesg:

b43-pci-bridge 0001:11:00.0: dma_direct_supported: failed (mask = 0x3fffffff,
min_mask = 0x5ffff000/0x5ffff000, dma bits = 0x1f
DMA not supported
b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit
DMA mask

Your first patch did not work as the configuration does not have
CONFIG_ZONE_DMA. As a result, the initial value of min_mask always starts at 32
bits and is taken down to 31 with the maximum pfn minimization. When I forced
the initial value of min_mask to 30 bits, the device worked.

It is obvious that the case of a mask smaller than min_mask should be handled by
the IOMMU. In my system, CONFIG_IOMMU_SUPPORT is selected. All other CONFIG
variables containing IOMMU are not selected. When dma_direct_supported() fails,
should the system not try for an IOMMU solution? Is the driver asking for the
wrong type of memory? It is doing a dma_and_set_mask_coherent() call.

Larry

\
 
 \ /
  Last update: 2019-06-12 00:21    [W:0.448 / U:0.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site