Messages in this thread | | | Subject | Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook | From | Larry Finger <> | Date | Tue, 11 Jun 2019 17:20:12 -0500 |
| |
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
| |