b43 / b43 legacy dma mask cleanups

b43 / b43 legacy dma mask cleanups

[Christoph Hellwig]

Hi all,

below are a few cleanups for the DMA mask handling.  I came up with these
untested patches after looking through the code to debug the 32-bit pmac
30-bit dma issue involving b43legacy.

[PATCH 1/4] b43legacy: remove b43legacy_dma_set_mask

[Christoph Hellwig]

These days drivers are not required to fallback to smaller DMA masks,
but can just set the largest mask they support, removing the need for
this trial and error logic.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/net/wireless/broadcom/b43legacy/dma.c | 39 +------------------
 1 file changed, 1 insertion(+), 38 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43legacy/dma.c b/drivers/net/wireless/broadcom/b43legacy/dma.c
index 2ce1537d983c..0c2de20622e3 100644
--- a/drivers/net/wireless/broadcom/b43legacy/dma.c
+++ b/drivers/net/wireless/broadcom/b43legacy/dma.c
@@ -797,43 +797,6 @@ void b43legacy_dma_free(struct b43legacy_wldev *dev)
 	dma->tx_ring0 = NULL;
 }
 
-static int b43legacy_dma_set_mask(struct b43legacy_wldev *dev, u64 mask)
-{
-	u64 orig_mask = mask;
-	bool fallback = false;
-	int err;
-
-	/* Try to set the DMA mask. If it fails, try falling back to a
-	 * lower mask, as we can always also support a lower one. */
-	while (1) {
-		err = dma_set_mask_and_coherent(dev->dev->dma_dev, mask);
-		if (!err)
-			break;
-		if (mask == DMA_BIT_MASK(64)) {
-			mask = DMA_BIT_MASK(32);
-			fallback = true;
-			continue;
-		}
-		if (mask == DMA_BIT_MASK(32)) {
-			mask = DMA_BIT_MASK(30);
-			fallback = true;
-			continue;
-		}
-		b43legacyerr(dev->wl, "The machine/kernel does not support "
-		       "the required %u-bit DMA mask\n",
-		       (unsigned int)dma_mask_to_engine_type(orig_mask));
-		return -EOPNOTSUPP;
-	}
-	if (fallback) {
-		b43legacyinfo(dev->wl, "DMA mask fallback from %u-bit to %u-"
-			"bit\n",
-			(unsigned int)dma_mask_to_engine_type(orig_mask),
-			(unsigned int)dma_mask_to_engine_type(mask));
-	}
-
-	return 0;
-}
-
 int b43legacy_dma_init(struct b43legacy_wldev *dev)
 {
 	struct b43legacy_dma *dma = &dev->dma;
@@ -844,7 +807,7 @@ int b43legacy_dma_init(struct b43legacy_wldev *dev)
 
 	dmamask = supported_dma_mask(dev);
 	type = dma_mask_to_engine_type(dmamask);
-	err = b43legacy_dma_set_mask(dev, dmamask);
+	err = dma_set_mask_and_coherent(dev->dev->dma_dev, dmamask);
 	if (err) {
 #ifdef CONFIG_B43LEGACY_PIO
 		b43legacywarn(dev->wl, "DMA for this device not supported. "
-- 
2.20.1

Re: [PATCH 1/4] b43legacy: remove b43legacy_dma_set_mask

[Larry Finger]

On 6/25/19 5:29 AM, Christoph Hellwig wrote:
> These days drivers are not required to fallback to smaller DMA masks,
> but can just set the largest mask they support, removing the need for
> this trial and error logic.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/net/wireless/broadcom/b43legacy/dma.c | 39 +------------------
>   1 file changed, 1 insertion(+), 38 deletions(-)

The patches work for PPC32 for both b43legacy and b43.

Tested-by: Larry Finger <Larry.Finger@lwfinger.net>

Thanks,

Larry

> 
> diff --git a/drivers/net/wireless/broadcom/b43legacy/dma.c b/drivers/net/wireless/broadcom/b43legacy/dma.c
> index 2ce1537d983c..0c2de20622e3 100644
> --- a/drivers/net/wireless/broadcom/b43legacy/dma.c
> +++ b/drivers/net/wireless/broadcom/b43legacy/dma.c
> @@ -797,43 +797,6 @@ void b43legacy_dma_free(struct b43legacy_wldev *dev)
>   	dma->tx_ring0 = NULL;
>   }
>   
> -static int b43legacy_dma_set_mask(struct b43legacy_wldev *dev, u64 mask)
> -{
> -	u64 orig_mask = mask;
> -	bool fallback = false;
> -	int err;
> -
> -	/* Try to set the DMA mask. If it fails, try falling back to a
> -	 * lower mask, as we can always also support a lower one. */
> -	while (1) {
> -		err = dma_set_mask_and_coherent(dev->dev->dma_dev, mask);
> -		if (!err)
> -			break;
> -		if (mask == DMA_BIT_MASK(64)) {
> -			mask = DMA_BIT_MASK(32);
> -			fallback = true;
> -			continue;
> -		}
> -		if (mask == DMA_BIT_MASK(32)) {
> -			mask = DMA_BIT_MASK(30);
> -			fallback = true;
> -			continue;
> -		}
> -		b43legacyerr(dev->wl, "The machine/kernel does not support "
> -		       "the required %u-bit DMA mask\n",
> -		       (unsigned int)dma_mask_to_engine_type(orig_mask));
> -		return -EOPNOTSUPP;
> -	}
> -	if (fallback) {
> -		b43legacyinfo(dev->wl, "DMA mask fallback from %u-bit to %u-"
> -			"bit\n",
> -			(unsigned int)dma_mask_to_engine_type(orig_mask),
> -			(unsigned int)dma_mask_to_engine_type(mask));
> -	}
> -
> -	return 0;
> -}
> -
>   int b43legacy_dma_init(struct b43legacy_wldev *dev)
>   {
>   	struct b43legacy_dma *dma = &dev->dma;
> @@ -844,7 +807,7 @@ int b43legacy_dma_init(struct b43legacy_wldev *dev)
>   
>   	dmamask = supported_dma_mask(dev);
>   	type = dma_mask_to_engine_type(dmamask);
> -	err = b43legacy_dma_set_mask(dev, dmamask);
> +	err = dma_set_mask_and_coherent(dev->dev->dma_dev, dmamask);
>   	if (err) {
>   #ifdef CONFIG_B43LEGACY_PIO
>   		b43legacywarn(dev->wl, "DMA for this device not supported. "
> 

Re: [PATCH 1/4] b43legacy: remove b43legacy_dma_set_mask

[Kalle Valo]

Christoph Hellwig <hch@lst.de> wrote:

> These days drivers are not required to fallback to smaller DMA masks,
> but can just set the largest mask they support, removing the need for
> this trial and error logic.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Tested-by: Larry Finger <Larry.Finger@lwfinger.net>

4 patches applied to wireless-drivers-next.git, thanks.

258989000849 b43legacy: remove b43legacy_dma_set_mask
80372782e4cb b43legacy: simplify engine type / DMA mask selection
c897523febae b43: remove b43_dma_set_mask
288aa4ee7acf b43: simplify engine type / DMA mask selection

-- 
https://patchwork.kernel.org/patch/11015245/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH 2/4] b43legacy: simplify engine type / DMA mask selection

[Christoph Hellwig]

Return the engine type from the function looking at the registers, and
just derive the DMA mask from that in the one place we care.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/net/wireless/broadcom/b43legacy/dma.c | 20 +++----------------
 1 file changed, 3 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43legacy/dma.c b/drivers/net/wireless/broadcom/b43legacy/dma.c
index 0c2de20622e3..e66d05ae2466 100644
--- a/drivers/net/wireless/broadcom/b43legacy/dma.c
+++ b/drivers/net/wireless/broadcom/b43legacy/dma.c
@@ -616,7 +616,7 @@ static void free_all_descbuffers(struct b43legacy_dmaring *ring)
 	}
 }
 
-static u64 supported_dma_mask(struct b43legacy_wldev *dev)
+static enum b43legacy_dmatype b43legacy_engine_type(struct b43legacy_wldev *dev)
 {
 	u32 tmp;
 	u16 mmio_base;
@@ -628,18 +628,7 @@ static u64 supported_dma_mask(struct b43legacy_wldev *dev)
 	tmp = b43legacy_read32(dev, mmio_base +
 			       B43legacy_DMA32_TXCTL);
 	if (tmp & B43legacy_DMA32_TXADDREXT_MASK)
-		return DMA_BIT_MASK(32);
-
-	return DMA_BIT_MASK(30);
-}
-
-static enum b43legacy_dmatype dma_mask_to_engine_type(u64 dmamask)
-{
-	if (dmamask == DMA_BIT_MASK(30))
-		return B43legacy_DMA_30BIT;
-	if (dmamask == DMA_BIT_MASK(32))
 		return B43legacy_DMA_32BIT;
-	B43legacy_WARN_ON(1);
 	return B43legacy_DMA_30BIT;
 }
 
@@ -801,13 +790,10 @@ int b43legacy_dma_init(struct b43legacy_wldev *dev)
 {
 	struct b43legacy_dma *dma = &dev->dma;
 	struct b43legacy_dmaring *ring;
+	enum b43legacy_dmatype type = b43legacy_engine_type(dev);
 	int err;
-	u64 dmamask;
-	enum b43legacy_dmatype type;
 
-	dmamask = supported_dma_mask(dev);
-	type = dma_mask_to_engine_type(dmamask);
-	err = dma_set_mask_and_coherent(dev->dev->dma_dev, dmamask);
+	err = dma_set_mask_and_coherent(dev->dev->dma_dev, DMA_BIT_MASK(type));
 	if (err) {
 #ifdef CONFIG_B43LEGACY_PIO
 		b43legacywarn(dev->wl, "DMA for this device not supported. "
-- 
2.20.1

[PATCH 3/4] b43: remove b43_dma_set_mask

[Christoph Hellwig]

These days drivers are not required to fallback to smaller DMA masks,
but can just set the largest mask they support, removing the need for
this trial and error logic.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/net/wireless/broadcom/b43/dma.c | 43 +++----------------------
 1 file changed, 5 insertions(+), 38 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/dma.c b/drivers/net/wireless/broadcom/b43/dma.c
index b34e51933257..1d5ace4d3372 100644
--- a/drivers/net/wireless/broadcom/b43/dma.c
+++ b/drivers/net/wireless/broadcom/b43/dma.c
@@ -1056,42 +1056,6 @@ void b43_dma_free(struct b43_wldev *dev)
 	destroy_ring(dma, tx_ring_mcast);
 }
 
-static int b43_dma_set_mask(struct b43_wldev *dev, u64 mask)
-{
-	u64 orig_mask = mask;
-	bool fallback = false;
-	int err;
-
-	/* Try to set the DMA mask. If it fails, try falling back to a
-	 * lower mask, as we can always also support a lower one. */
-	while (1) {
-		err = dma_set_mask_and_coherent(dev->dev->dma_dev, mask);
-		if (!err)
-			break;
-		if (mask == DMA_BIT_MASK(64)) {
-			mask = DMA_BIT_MASK(32);
-			fallback = true;
-			continue;
-		}
-		if (mask == DMA_BIT_MASK(32)) {
-			mask = DMA_BIT_MASK(30);
-			fallback = true;
-			continue;
-		}
-		b43err(dev->wl, "The machine/kernel does not support "
-		       "the required %u-bit DMA mask\n",
-		       (unsigned int)dma_mask_to_engine_type(orig_mask));
-		return -EOPNOTSUPP;
-	}
-	if (fallback) {
-		b43info(dev->wl, "DMA mask fallback from %u-bit to %u-bit\n",
-			(unsigned int)dma_mask_to_engine_type(orig_mask),
-			(unsigned int)dma_mask_to_engine_type(mask));
-	}
-
-	return 0;
-}
-
 /* Some hardware with 64-bit DMA seems to be bugged and looks for translation
  * bit in low address word instead of high one.
  */
@@ -1120,9 +1084,12 @@ int b43_dma_init(struct b43_wldev *dev)
 
 	dmamask = supported_dma_mask(dev);
 	type = dma_mask_to_engine_type(dmamask);
-	err = b43_dma_set_mask(dev, dmamask);
-	if (err)
+	err = dma_set_mask_and_coherent(dev->dev->dma_dev, dmamask);
+	if (err) {
+		b43err(dev->wl, "The machine/kernel does not support "
+		       "the required %u-bit DMA mask\n", type);
 		return err;
+	}
 
 	switch (dev->dev->bus_type) {
 #ifdef CONFIG_B43_BCMA
-- 
2.20.1

[PATCH 4/4] b43: simplify engine type / DMA mask selection

[Christoph Hellwig]

Return the engine type from the function looking at the registers, and
just derive the DMA mask from that in the one place we care.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/net/wireless/broadcom/b43/dma.c | 28 ++++++-------------------
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/dma.c b/drivers/net/wireless/broadcom/b43/dma.c
index 1d5ace4d3372..e8958edb9094 100644
--- a/drivers/net/wireless/broadcom/b43/dma.c
+++ b/drivers/net/wireless/broadcom/b43/dma.c
@@ -810,7 +810,7 @@ static void free_all_descbuffers(struct b43_dmaring *ring)
 	}
 }
 
-static u64 supported_dma_mask(struct b43_wldev *dev)
+static enum b43_dmatype b43_engine_type(struct b43_wldev *dev)
 {
 	u32 tmp;
 	u16 mmio_base;
@@ -820,14 +820,14 @@ static u64 supported_dma_mask(struct b43_wldev *dev)
 	case B43_BUS_BCMA:
 		tmp = bcma_aread32(dev->dev->bdev, BCMA_IOST);
 		if (tmp & BCMA_IOST_DMA64)
-			return DMA_BIT_MASK(64);
+			return B43_DMA_64BIT;
 		break;
 #endif
 #ifdef CONFIG_B43_SSB
 	case B43_BUS_SSB:
 		tmp = ssb_read32(dev->dev->sdev, SSB_TMSHIGH);
 		if (tmp & SSB_TMSHIGH_DMA64)
-			return DMA_BIT_MASK(64);
+			return B43_DMA_64BIT;
 		break;
 #endif
 	}
@@ -836,20 +836,7 @@ static u64 supported_dma_mask(struct b43_wldev *dev)
 	b43_write32(dev, mmio_base + B43_DMA32_TXCTL, B43_DMA32_TXADDREXT_MASK);
 	tmp = b43_read32(dev, mmio_base + B43_DMA32_TXCTL);
 	if (tmp & B43_DMA32_TXADDREXT_MASK)
-		return DMA_BIT_MASK(32);
-
-	return DMA_BIT_MASK(30);
-}
-
-static enum b43_dmatype dma_mask_to_engine_type(u64 dmamask)
-{
-	if (dmamask == DMA_BIT_MASK(30))
-		return B43_DMA_30BIT;
-	if (dmamask == DMA_BIT_MASK(32))
 		return B43_DMA_32BIT;
-	if (dmamask == DMA_BIT_MASK(64))
-		return B43_DMA_64BIT;
-	B43_WARN_ON(1);
 	return B43_DMA_30BIT;
 }
 
@@ -1078,13 +1065,10 @@ static bool b43_dma_translation_in_low_word(struct b43_wldev *dev,
 int b43_dma_init(struct b43_wldev *dev)
 {
 	struct b43_dma *dma = &dev->dma;
+	enum b43_dmatype type = b43_engine_type(dev);
 	int err;
-	u64 dmamask;
-	enum b43_dmatype type;
 
-	dmamask = supported_dma_mask(dev);
-	type = dma_mask_to_engine_type(dmamask);
-	err = dma_set_mask_and_coherent(dev->dev->dma_dev, dmamask);
+	err = dma_set_mask_and_coherent(dev->dev->dma_dev, DMA_BIT_MASK(type));
 	if (err) {
 		b43err(dev->wl, "The machine/kernel does not support "
 		       "the required %u-bit DMA mask\n", type);
@@ -1793,7 +1777,7 @@ void b43_dma_direct_fifo_rx(struct b43_wldev *dev,
 	enum b43_dmatype type;
 	u16 mmio_base;
 
-	type = dma_mask_to_engine_type(supported_dma_mask(dev));
+	type = b43_engine_type(dev);
 
 	mmio_base = b43_dmacontroller_base(type, engine_index);
 	direct_fifo_rx(dev, type, mmio_base, enable);
-- 
2.20.1