bcma: get SoC device struct & copy its DMA params to the subdevices
Rafał Miłecki
maintainerPATCH 1/1 bcma: get SoC device struct & copy its DMA params to the subdevices
From: Rafał Miłecki <[email protected]>
For bus devices to be fully usable it's required to set their DMA parameters.
For years it has been missing and remained unnoticed because of mips_dma_alloc_coherent() silently handling the empty coherent_dma_mask. Kernel 4.19 came with a lot of DMA changes and caused a regression on the bcm47xx. Starting with the commit f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for simple noncoherent platforms") DMA coherent allocations just fail. Example:
[ 1.114914] bgmac_bcma bcma0:2: Allocation of TX ring 0x200 failed [ 1.121215] bgmac_bcma bcma0:2: Unable to alloc memory for DMA [ 1.127626] bgmac_bcma: probe of bcma0:2 failed with error -12 [ 1.133838] bgmac_bcma: Broadcom 47xx GBit MAC driver loaded
This change fixes above regression in addition to the MIPS bcm47xx commit 321c46b91550 ("MIPS: BCM47XX: Setup struct device for the SoC").
It also fixes another *old* GPIO regression caused by a parent pointing to the NULL:
[ 0.157054] missing gpiochip .dev parent pointer [ 0.157287] bcma: bus0: Error registering GPIO driver: -22
introduced by the commit 74f4e0cc6108 ("bcma: switch GPIO portions to use GPIOLIB_IRQCHIP").
---
While this patch is a regression fix, it depends on a change present in the wireless-drivers-next.git:
bcma: keep a direct pointer to the struct device
That's why I suggest pushing it into the wireless-drivers-next.git and I can take care of picking it for the [email protected] later.
Another option would be cherry-picking commit 5a1c18b761dd ("bcma: keep a direct pointer to the struct device") to the wireless-drivers.git but I don't think it's a common practice.
--- drivers/bcma/host_soc.c | 2 ++ drivers/bcma/main.c | 6 +++++- include/linux/bcma/bcma_soc.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-)
drivers/bcma/host_soc.c +2 -0
@@ -191,6 +191,8 @@ int __init bcma_host_soc_init(struct bcma_soc *soc)struct bcma_bus *bus = &soc->bus;int err; + bus->dev = soc->dev; +/* Scan bus and initialize it */ err =bcma_bus_early_register (bus);if (err)
drivers/bcma/main.c +5 -1
@@ -236,12 +236,16 @@ EXPORT_SYMBOL(bcma_core_irq);void bcma_prepare_core (struct bcma_bus *bus,struct bcma_device *core) { +struct device *dev = &core->dev; + core->dev.release = bcma_release_core_dev; core->dev.bus = &bcma_bus_type;dev_set_name (&core->dev,"bcma%d:%d" , bus->num, core->core_index); core->dev.parent = bus->dev; -if (bus->dev) +if (bus->dev) {bcma_of_fill_device (bus->dev, core); +dma_coerce_mask_and_coherent (dev, bus->dev->coherent_dma_mask); + }switch (bus->hosttype) {case BCMA_HOSTTYPE_PCI :
include/linux/bcma/bcma_soc.h +1 -0
@@ -6,6 +6,7 @@struct bcma_soc {struct bcma_bus bus; +struct device *dev; };int __init bcma_host_soc_register (struct bcma_soc *soc);
Signature · 1 line
2.20.1
Quoted 71 lines · Rafał Miłecki
From: Rafał Miłecki <[email protected]>
For bus devices to be fully usable it's required to set their DMA parameters.
For years it has been missing and remained unnoticed because of
mips_dma_alloc_coherent()silently handling the emptycoherent_dma_mask. Kernel 4.19 came with a lot of DMA changes and caused a regression on the bcm47xx. Starting with the commitf8c55dc6e828("MIPS: use generic dma noncoherent ops for simple noncoherent platforms") DMA coherent allocations just fail. Example:[ 1.114914] bgmac_bcma bcma0:2: Allocation of TX ring 0x200 failed [ 1.121215] bgmac_bcma bcma0:2: Unable to alloc memory for DMA [ 1.127626] bgmac_bcma: probe of bcma0:2 failed with error -12 [ 1.133838] bgmac_bcma: Broadcom 47xx GBit MAC driver loadedThis change fixes above regression in addition to the MIPS bcm47xx commit
321c46b91550("MIPS: BCM47XX: Setup struct device for the SoC").It also fixes another *old* GPIO regression caused by a parent pointing to the NULL:
[ 0.157054] missing gpiochip .dev parent pointer [ 0.157287] bcma: bus0: Error registering GPIO driver: -22introduced by the commit
74f4e0cc6108("bcma: switch GPIO portions to useGPIOLIB_IRQCHIP").Fixes f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for simple noncoherent platforms")Fixes 74f4e0cc6108 ("bcma: switch GPIO portions to use GPIOLIB_IRQCHIP")Cc Christoph Hellwig <[email protected]>Cc Linus Walleij <[email protected]>Signed-off-by Rafał Miłecki <[email protected]>---
While this patch is a regression fix, it depends on a change present in the wireless-drivers-next.git:
bcma: keep a direct pointer to the struct deviceThat's why I suggest pushing it into the wireless-drivers-next.git and I can take care of picking it for the [email protected] later.
Another option would be cherry-picking commit
5a1c18b761dd("bcma: keep a direct pointer to the struct device") to the wireless-drivers.git but I don't think it's a common practice.--- drivers/bcma/host_soc.c | 2 ++ drivers/bcma/main.c | 6 +++++- include/linux/bcma/bcma_soc.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-)drivers/bcma/host_soc.c +2
-0@@ -191,6 +191,8 @@ int __init bcma_host_soc_init(struct bcma_soc *soc)struct bcma_bus *bus = &soc->bus;int err; + bus->dev = soc->dev; +/* Scan bus and initialize it */ err =bcma_bus_early_register (bus);if (err)drivers/bcma/main.c +1
-0@@ -236,12 +236,16 @@ EXPORT_SYMBOL(bcma_core_irq);void bcma_prepare_core (struct bcma_bus *bus,struct bcma_device *core) { +struct device *dev = &core->dev;
drivers/bcma/main.c EXPORT_SYMBOL(bcma_core_irq); · line 248
Quoted 10 lines
drivers/bcma/main.c +4
-1+ core->dev.release = bcma_release_core_dev; core->dev.bus = &bcma_bus_type;dev_set_name (&core->dev,"bcma%d:%d" , bus->num, core->core_index); core->dev.parent = bus->dev; -if (bus->dev) +if (bus->dev) {bcma_of_fill_device (bus->dev, core); +dma_coerce_mask_and_coherent (dev, bus->dev->coherent_dma_mask); + }
I don't think this does the right thing for bcma devices behind PCI/PCIe, as those might need a IOMMU which relies on having a device it has properly enumerated to be passed into the DMA API. In other
words: I think you need to change the layering so that the DMA API is always called on the underlying PCI/PCIe/platform device, not of the child bus.
Rafał Miłecki
maintainerQuoted 81 lines · Christoph Hellwig
Quoted 80 lines · Rafał Miłecki
From: Rafał Miłecki <[email protected]>
For bus devices to be fully usable it's required to set their DMA parameters.
For years it has been missing and remained unnoticed because of
mips_dma_alloc_coherent()silently handling the emptycoherent_dma_mask. Kernel 4.19 came with a lot of DMA changes and caused a regression on the bcm47xx. Starting with the commitf8c55dc6e828("MIPS: use generic dma noncoherent ops for simple noncoherent platforms") DMA coherent allocations just fail. Example:[ 1.114914] bgmac_bcma bcma0:2: Allocation of TX ring 0x200 failed [ 1.121215] bgmac_bcma bcma0:2: Unable to alloc memory for DMA [ 1.127626] bgmac_bcma: probe of bcma0:2 failed with error -12 [ 1.133838] bgmac_bcma: Broadcom 47xx GBit MAC driver loadedThis change fixes above regression in addition to the MIPS bcm47xx commit
321c46b91550("MIPS: BCM47XX: Setup struct device for the SoC").It also fixes another *old* GPIO regression caused by a parent pointing to the NULL:
[ 0.157054] missing gpiochip .dev parent pointer [ 0.157287] bcma: bus0: Error registering GPIO driver: -22introduced by the commit
74f4e0cc6108("bcma: switch GPIO portions to useGPIOLIB_IRQCHIP").Fixes f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for simple noncoherent platforms")Fixes 74f4e0cc6108 ("bcma: switch GPIO portions to use GPIOLIB_IRQCHIP")Cc Christoph Hellwig <[email protected]>Cc Linus Walleij <[email protected]>Signed-off-by Rafał Miłecki <[email protected]>---
While this patch is a regression fix, it depends on a change present in the wireless-drivers-next.git:
bcma: keep a direct pointer to the struct deviceThat's why I suggest pushing it into the wireless-drivers-next.git and I can take care of picking it for the [email protected] later.
Another option would be cherry-picking commit
5a1c18b761dd("bcma: keep a direct pointer to the struct device") to the wireless-drivers.git but I don't think it's a common practice.--- drivers/bcma/host_soc.c | 2 ++ drivers/bcma/main.c | 6 +++++- include/linux/bcma/bcma_soc.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-)drivers/bcma/host_soc.c +2
-0@@ -191,6 +191,8 @@ int __init bcma_host_soc_init(struct bcma_soc *soc)struct bcma_bus *bus = &soc->bus;int err; + bus->dev = soc->dev; +/* Scan bus and initialize it */ err =bcma_bus_early_register (bus);if (err)drivers/bcma/main.c +4
-1@@ -236,12 +236,16 @@ EXPORT_SYMBOL(bcma_core_irq);void bcma_prepare_core (struct bcma_bus *bus,struct bcma_device *core) { +struct device *dev = &core->dev; + core->dev.release = bcma_release_core_dev; core->dev.bus = &bcma_bus_type;dev_set_name (&core->dev,"bcma%d:%d" , bus->num, core->core_index); core->dev.parent = bus->dev; -if (bus->dev) +if (bus->dev) {bcma_of_fill_device (bus->dev, core); +dma_coerce_mask_and_coherent (dev, bus->dev->coherent_dma_mask);
Quoted 8 lines
drivers/bcma/main.c EXPORT_SYMBOL(bcma_core_irq); · line 248
Quoted 1 line
drivers/bcma/main.c +1
-0+ }I don't think this does the right thing for bcma devices behind PCI/PCIe, as those might need a IOMMU which relies on having a device it has properly enumerated to be passed into the DMA API. In other
words: I think you need to change the layering so that the DMA API is always called on the underlying PCI/PCIe/platform device, not of the child bus.
This is already implemented (we have dma_dev pointer), but it may be indeed more correct to apply this change for the SoC code only. I'll send V2.
Signature · 1 line
Rafał
Quoted 32 lines · RafaB MiBecki
From: Rafał Miłecki <[email protected]>
For bus devices to be fully usable it's required to set their DMA parameters.
For years it has been missing and remained unnoticed because of
mips_dma_alloc_coherent()silently handling the emptycoherent_dma_mask. Kernel 4.19 came with a lot of DMA changes and caused a regression on the bcm47xx. Starting with the commitf8c55dc6e828("MIPS: use generic dma noncoherent ops for simple noncoherent platforms") DMA coherent allocations just fail. Example:[ 1.114914] bgmac_bcma bcma0:2: Allocation of TX ring 0x200 failed [ 1.121215] bgmac_bcma bcma0:2: Unable to alloc memory for DMA [ 1.127626] bgmac_bcma: probe of bcma0:2 failed with error -12 [ 1.133838] bgmac_bcma: Broadcom 47xx GBit MAC driver loadedThis change fixes above regression in addition to the MIPS bcm47xx commit
321c46b91550("MIPS: BCM47XX: Setup struct device for the SoC").It also fixes another *old* GPIO regression caused by a parent pointing to the NULL:
[ 0.157054] missing gpiochip .dev parent pointer [ 0.157287] bcma: bus0: Error registering GPIO driver: -22introduced by the commit
74f4e0cc6108("bcma: switch GPIO portions to useGPIOLIB_IRQCHIP").Fixes f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for simple noncoherent platforms")Fixes 74f4e0cc6108 ("bcma: switch GPIO portions to use GPIOLIB_IRQCHIP")Cc Christoph Hellwig <[email protected]>Cc Linus Walleij <[email protected]>Signed-off-by Rafał Miłecki <[email protected]>
Quoted 4 lines
---
While this patch is a regression fix, it depends on a change present in the wireless-drivers-next.git:
bcma: keep a direct pointer to the struct device
Rafal, there was supposed to be a v2, but I could not find one, this is the regression that prevented bgmac_bcma from loading while testing upstream, did you have a v2 ready already to be submitted somehow?
Thanks!
Signature · 1 line
Florian
Please Cc me on the next version. Copying over dma parameters sounds like the wrong thing to do.