[PATCH net v7 0/2] net/ps3_gelic_net: DMA related fixes

[PATCH net v7 0/2] net/ps3_gelic_net: DMA related fixes

[Geoff Levand]

v7: Remove all cleanups, sync to spider net.
v6: Reworked and cleaned up patches.
v5: Some additional patch cleanups.
v4: More patch cleanups.
v3: Cleaned up patches as requested.

Geoff Levand (2):
  net/ps3_gelic_net: Fix RX sk_buff length
  net/ps3_gelic_net: Use dma_mapping_error

 drivers/net/ethernet/toshiba/ps3_gelic_net.c | 16 +++++++++-------
 drivers/net/ethernet/toshiba/ps3_gelic_net.h |  5 +++--
 2 files changed, 12 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH net v7 1/2] net/ps3_gelic_net: Fix RX sk_buff length

[Geoff Levand]

The Gelic Ethernet device needs to have the RX sk_buffs aligned to
GELIC_NET_RXBUF_ALIGN, and also the length of the RX sk_buffs must
be a multiple of GELIC_NET_RXBUF_ALIGN.

The current Gelic Ethernet driver was not allocating sk_buffs large
enough to allow for this alignment.

Also, correct the maximum and minimum MTU sizes, and add a new
preprocessor macro for the maximum frame size, GELIC_NET_MAX_FRAME.

Fixes various randomly occurring runtime network errors.

Fixes: 02c1889166b4 ("ps3: gigabit ethernet driver for PS3, take3")
Signed-off-by: Geoff Levand <geoff@infradead.org>
---
 drivers/net/ethernet/toshiba/ps3_gelic_net.c | 10 ++++++----
 drivers/net/ethernet/toshiba/ps3_gelic_net.h |  5 +++--
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
index cf8de8a7a8a1..b0ebe0e603b4 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
@@ -375,11 +375,13 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
 	if (gelic_descr_get_status(descr) !=  GELIC_DESCR_DMA_NOT_IN_USE)
 		dev_info(ctodev(card), "%s: ERROR status\n", __func__);
 	/* we need to round up the buffer size to a multiple of 128 */
-	bufsize = ALIGN(GELIC_NET_MAX_MTU, GELIC_NET_RXBUF_ALIGN);
+	bufsize = (GELIC_NET_MAX_FRAME + GELIC_NET_RXBUF_ALIGN - 1) &
+		(~(GELIC_NET_RXBUF_ALIGN - 1));
 
 	/* and we need to have it 128 byte aligned, therefore we allocate a
 	 * bit more */
-	descr->skb = dev_alloc_skb(bufsize + GELIC_NET_RXBUF_ALIGN - 1);
+	descr->skb = netdev_alloc_skb(*card->netdev, bufsize +
+		GELIC_NET_RXBUF_ALIGN - 1);
 	if (!descr->skb) {
 		descr->buf_addr = 0; /* tell DMAC don't touch memory */
 		return -ENOMEM;
@@ -397,7 +399,7 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
 	/* io-mmu-map the skb */
 	descr->buf_addr = cpu_to_be32(dma_map_single(ctodev(card),
 						     descr->skb->data,
-						     GELIC_NET_MAX_MTU,
+						     GELIC_NET_MAX_FRAME,
 						     DMA_FROM_DEVICE));
 	if (!descr->buf_addr) {
 		dev_kfree_skb_any(descr->skb);
@@ -915,7 +917,7 @@ static void gelic_net_pass_skb_up(struct gelic_descr *descr,
 	data_error = be32_to_cpu(descr->data_error);
 	/* unmap skb buffer */
 	dma_unmap_single(ctodev(card), be32_to_cpu(descr->buf_addr),
-			 GELIC_NET_MAX_MTU,
+			 GELIC_NET_MAX_FRAME,
 			 DMA_FROM_DEVICE);
 
 	skb_put(skb, be32_to_cpu(descr->valid_size)?
diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.h b/drivers/net/ethernet/toshiba/ps3_gelic_net.h
index 68f324ed4eaf..0d98defb011e 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.h
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.h
@@ -19,8 +19,9 @@
 #define GELIC_NET_RX_DESCRIPTORS        128 /* num of descriptors */
 #define GELIC_NET_TX_DESCRIPTORS        128 /* num of descriptors */
 
-#define GELIC_NET_MAX_MTU               VLAN_ETH_FRAME_LEN
-#define GELIC_NET_MIN_MTU               VLAN_ETH_ZLEN
+#define GELIC_NET_MAX_FRAME             2312
+#define GELIC_NET_MAX_MTU               2294
+#define GELIC_NET_MIN_MTU               64
 #define GELIC_NET_RXBUF_ALIGN           128
 #define GELIC_CARD_RX_CSUM_DEFAULT      1 /* hw chksum */
 #define GELIC_NET_WATCHDOG_TIMEOUT      5*HZ
-- 
2.34.1

[PATCH net v7 2/2] net/ps3_gelic_net: Use dma_mapping_error

[Geoff Levand]

The current Gelic Etherenet driver was checking the return value of its
dma_map_single call, and not using the dma_mapping_error() routine.

Fixes runtime problems like these:

  DMA-API: ps3_gelic_driver sb_05: device driver failed to check map error
  WARNING: CPU: 0 PID: 0 at kernel/dma/debug.c:1027 .check_unmap+0x888/0x8dc

Fixes: 02c1889166b4 ("ps3: gigabit ethernet driver for PS3, take3")
Signed-off-by: Geoff Levand <geoff@infradead.org>
---
 drivers/net/ethernet/toshiba/ps3_gelic_net.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
index b0ebe0e603b4..40261947e0ea 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
@@ -323,7 +323,7 @@ static int gelic_card_init_chain(struct gelic_card *card,
 				       GELIC_DESCR_SIZE,
 				       DMA_BIDIRECTIONAL);
 
-		if (!descr->bus_addr)
+		if (dma_mapping_error(ctodev(card), descr->bus_addr))
 			goto iommu_error;
 
 		descr->next = descr + 1;
@@ -401,7 +401,7 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
 						     descr->skb->data,
 						     GELIC_NET_MAX_FRAME,
 						     DMA_FROM_DEVICE));
-	if (!descr->buf_addr) {
+	if (dma_mapping_error(ctodev(card), descr->buf_addr)) {
 		dev_kfree_skb_any(descr->skb);
 		descr->skb = NULL;
 		dev_info(ctodev(card),
@@ -781,7 +781,7 @@ static int gelic_descr_prepare_tx(struct gelic_card *card,
 
 	buf = dma_map_single(ctodev(card), skb->data, skb->len, DMA_TO_DEVICE);
 
-	if (!buf) {
+	if (dma_mapping_error(ctodev(card), buf)) {
 		dev_err(ctodev(card),
 			"dma map 2 failed (%p, %i). Dropping packet\n",
 			skb->data, skb->len);
-- 
2.34.1

Re: [PATCH net v7 1/2] net/ps3_gelic_net: Fix RX sk_buff length

[Alexander H Duyck]

On Sun, 2023-03-05 at 02:08 +0000, Geoff Levand wrote:
> The Gelic Ethernet device needs to have the RX sk_buffs aligned to
> GELIC_NET_RXBUF_ALIGN, and also the length of the RX sk_buffs must
> be a multiple of GELIC_NET_RXBUF_ALIGN.
> 
> The current Gelic Ethernet driver was not allocating sk_buffs large
> enough to allow for this alignment.
> 
> Also, correct the maximum and minimum MTU sizes, and add a new
> preprocessor macro for the maximum frame size, GELIC_NET_MAX_FRAME.
> 
> Fixes various randomly occurring runtime network errors.
> 
> Fixes: 02c1889166b4 ("ps3: gigabit ethernet driver for PS3, take3")
> Signed-off-by: Geoff Levand <geoff@infradead.org>
> ---
>  drivers/net/ethernet/toshiba/ps3_gelic_net.c | 10 ++++++----
>  drivers/net/ethernet/toshiba/ps3_gelic_net.h |  5 +++--
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index cf8de8a7a8a1..b0ebe0e603b4 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -375,11 +375,13 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
>  	if (gelic_descr_get_status(descr) !=  GELIC_DESCR_DMA_NOT_IN_USE)
>  		dev_info(ctodev(card), "%s: ERROR status\n", __func__);
>  	/* we need to round up the buffer size to a multiple of 128 */
> -	bufsize = ALIGN(GELIC_NET_MAX_MTU, GELIC_NET_RXBUF_ALIGN);
> +	bufsize = (GELIC_NET_MAX_FRAME + GELIC_NET_RXBUF_ALIGN - 1) &
> +		(~(GELIC_NET_RXBUF_ALIGN - 1));

Why did you stop using ALIGN? What you coded looks exactly like what
the code for ALIGN does. From what I can tell you just need to replace
GELIC_NET_MAX_MTU with GELIC_NET_MAX_FRAME.

>  
>  	/* and we need to have it 128 byte aligned, therefore we allocate a
>  	 * bit more */
> -	descr->skb = dev_alloc_skb(bufsize + GELIC_NET_RXBUF_ALIGN - 1);
> +	descr->skb = netdev_alloc_skb(*card->netdev, bufsize +
> +		GELIC_NET_RXBUF_ALIGN - 1);

This wrapping doesn't look right to me. Also why add the align value
again here? I would think that it being added above should have taken
care of what you needed. Are you adding any data beyond the end of what
is DMAed into the frame?

>  	if (!descr->skb) {
>  		descr->buf_addr = 0; /* tell DMAC don't touch memory */
>  		return -ENOMEM;
> @@ -397,7 +399,7 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
>  	/* io-mmu-map the skb */
>  	descr->buf_addr = cpu_to_be32(dma_map_single(ctodev(card),
>  						     descr->skb->data,
> -						     GELIC_NET_MAX_MTU,
> +						     GELIC_NET_MAX_FRAME,
>  						     DMA_FROM_DEVICE));

Rather than using the define GELIC_NET_MAX_FRAME, why not just use
bufsize since that is what you actually have allocated for receive?

>  	if (!descr->buf_addr) {
>  		dev_kfree_skb_any(descr->skb);
> @@ -915,7 +917,7 @@ static void gelic_net_pass_skb_up(struct gelic_descr *descr,
>  	data_error = be32_to_cpu(descr->data_error);
>  	/* unmap skb buffer */
>  	dma_unmap_single(ctodev(card), be32_to_cpu(descr->buf_addr),
> -			 GELIC_NET_MAX_MTU,
> +			 GELIC_NET_MAX_FRAME,
>  			 DMA_FROM_DEVICE);

I suppose my suggestion above would cause a problem here since you
would need to also define buffer size here. However since it looks like
you are adding a define below anyway maybe you should just look at
adding a new RX_BUFSZ define and just drop bufsize completely.

>  
>  	skb_put(skb, be32_to_cpu(descr->valid_size)?
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.h b/drivers/net/ethernet/toshiba/ps3_gelic_net.h
> index 68f324ed4eaf..0d98defb011e 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.h
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.h
> @@ -19,8 +19,9 @@
>  #define GELIC_NET_RX_DESCRIPTORS        128 /* num of descriptors */
>  #define GELIC_NET_TX_DESCRIPTORS        128 /* num of descriptors */
>  
> -#define GELIC_NET_MAX_MTU               VLAN_ETH_FRAME_LEN
> -#define GELIC_NET_MIN_MTU               VLAN_ETH_ZLEN
> +#define GELIC_NET_MAX_FRAME             2312
> +#define GELIC_NET_MAX_MTU               2294
> +#define GELIC_NET_MIN_MTU               64
>  #define GELIC_NET_RXBUF_ALIGN           128
>  #define GELIC_CARD_RX_CSUM_DEFAULT      1 /* hw chksum */
>  #define GELIC_NET_WATCHDOG_TIMEOUT      5*HZ

Since you are adding defines why not just add:
#define GELIC_NET_RX_BUFSZ \
	ALIGN(GELIC_NET_MAX_FRAME, GELIC_NET_RXBUF_ALIGN)

Re: [PATCH net v7 2/2] net/ps3_gelic_net: Use dma_mapping_error

[Alexander H Duyck]

On Sun, 2023-03-05 at 02:08 +0000, Geoff Levand wrote:
> The current Gelic Etherenet driver was checking the return value of its
> dma_map_single call, and not using the dma_mapping_error() routine.
> 
> Fixes runtime problems like these:
> 
>   DMA-API: ps3_gelic_driver sb_05: device driver failed to check map error
>   WARNING: CPU: 0 PID: 0 at kernel/dma/debug.c:1027 .check_unmap+0x888/0x8dc
> 
> Fixes: 02c1889166b4 ("ps3: gigabit ethernet driver for PS3, take3")
> Signed-off-by: Geoff Levand <geoff@infradead.org>
> ---
>  drivers/net/ethernet/toshiba/ps3_gelic_net.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index b0ebe0e603b4..40261947e0ea 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -323,7 +323,7 @@ static int gelic_card_init_chain(struct gelic_card *card,
>  				       GELIC_DESCR_SIZE,
>  				       DMA_BIDIRECTIONAL);
>  
> -		if (!descr->bus_addr)
> +		if (dma_mapping_error(ctodev(card), descr->bus_addr))
>  			goto iommu_error;
>  
>  		descr->next = descr + 1;

The bus_addr value is __be32 and the dma_mapping_error should be CPU
ordered. I think there was a byteswap using cpu_to_be32 missing here.
In addition you will probably need to have an intermediate variable to
store it in to test the DMA address before you byte swap it and store
it in the descriptor.

> @@ -401,7 +401,7 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
>  						     descr->skb->data,
>  						     GELIC_NET_MAX_FRAME,
>  						     DMA_FROM_DEVICE));
> -	if (!descr->buf_addr) {
> +	if (dma_mapping_error(ctodev(card), descr->buf_addr)) {
>  		dev_kfree_skb_any(descr->skb);
>  		descr->skb = NULL;
>  		dev_info(ctodev(card),

This is happening AFTER the DMA is passed through a cpu_to_be32 right?
The test should be on the raw value, not the byteswapped value.

> @@ -781,7 +781,7 @@ static int gelic_descr_prepare_tx(struct gelic_card *card,
>  
>  	buf = dma_map_single(ctodev(card), skb->data, skb->len, DMA_TO_DEVICE);
>  
> -	if (!buf) {
> +	if (dma_mapping_error(ctodev(card), buf)) {
>  		dev_err(ctodev(card),
>  			"dma map 2 failed (%p, %i). Dropping packet\n",
>  			skb->data, skb->len);

This one is correct from what I can tell. I would recommend using it as
a template and applying it to the two above so that you can sort out
the byte ordering issues and perform the test and the CPU ordered DMA
variable.

Re: [PATCH net v7 1/2] net/ps3_gelic_net: Fix RX sk_buff length

[Geoff Levand]

Hi,

On 3/6/23 07:54, Alexander H Duyck wrote:
> On Sun, 2023-03-05 at 02:08 +0000, Geoff Levand wrote:
>> The Gelic Ethernet device needs to have the RX sk_buffs aligned to
>> GELIC_NET_RXBUF_ALIGN, and also the length of the RX sk_buffs must
>> be a multiple of GELIC_NET_RXBUF_ALIGN.
>>
>> The current Gelic Ethernet driver was not allocating sk_buffs large
>> enough to allow for this alignment.
>>
>> Also, correct the maximum and minimum MTU sizes, and add a new
>> preprocessor macro for the maximum frame size, GELIC_NET_MAX_FRAME.
>>
>> Fixes various randomly occurring runtime network errors.
>>
>> Fixes: 02c1889166b4 ("ps3: gigabit ethernet driver for PS3, take3")
>> Signed-off-by: Geoff Levand <geoff@infradead.org>
>> ---
>>  drivers/net/ethernet/toshiba/ps3_gelic_net.c | 10 ++++++----
>>  drivers/net/ethernet/toshiba/ps3_gelic_net.h |  5 +++--
>>  2 files changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
>> index cf8de8a7a8a1..b0ebe0e603b4 100644
>> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
>> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
>> @@ -375,11 +375,13 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
>>  	if (gelic_descr_get_status(descr) !=  GELIC_DESCR_DMA_NOT_IN_USE)
>>  		dev_info(ctodev(card), "%s: ERROR status\n", __func__);
>>  	/* we need to round up the buffer size to a multiple of 128 */
>> -	bufsize = ALIGN(GELIC_NET_MAX_MTU, GELIC_NET_RXBUF_ALIGN);
>> +	bufsize = (GELIC_NET_MAX_FRAME + GELIC_NET_RXBUF_ALIGN - 1) &
>> +		(~(GELIC_NET_RXBUF_ALIGN - 1));
> 
> Why did you stop using ALIGN? What you coded looks exactly like what
> the code for ALIGN does. From what I can tell you just need to replace
> GELIC_NET_MAX_MTU with GELIC_NET_MAX_FRAME.

OK, I'll use ALIGN for the next patch set.
   
>>  	/* and we need to have it 128 byte aligned, therefore we allocate a
>>  	 * bit more */
>> -	descr->skb = dev_alloc_skb(bufsize + GELIC_NET_RXBUF_ALIGN - 1);
>> +	descr->skb = netdev_alloc_skb(*card->netdev, bufsize +
>> +		GELIC_NET_RXBUF_ALIGN - 1);
> 
> This wrapping doesn't look right to me. Also why add the align value
> again here? I would think that it being added above should have taken
> care of what you needed. Are you adding any data beyond the end of what
> is DMAed into the frame?

This is just a straight copy of what is done in the spider net driver.
As I mentioned in the comment of this patch, the DMA buffer must be a
multiple of GELIC_NET_RXBUF_ALIGN, and that is what that extra
'GELIC_NET_RXBUF_ALIGN - 1' is for.
 
>>  	if (!descr->skb) {
>>  		descr->buf_addr = 0; /* tell DMAC don't touch memory */
>>  		return -ENOMEM;
>> @@ -397,7 +399,7 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
>>  	/* io-mmu-map the skb */
>>  	descr->buf_addr = cpu_to_be32(dma_map_single(ctodev(card),
>>  						     descr->skb->data,
>> -						     GELIC_NET_MAX_MTU,
>> +						     GELIC_NET_MAX_FRAME,
>>  						     DMA_FROM_DEVICE));
> 
> Rather than using the define GELIC_NET_MAX_FRAME, why not just use
> bufsize since that is what you actually have allocated for receive?

Again, this is a straight copy of what is done in the spider net driver.
It does seem like using bufsize would be better, so I'll change that.

>>  	if (!descr->buf_addr) {
>>  		dev_kfree_skb_any(descr->skb);
>> @@ -915,7 +917,7 @@ static void gelic_net_pass_skb_up(struct gelic_descr *descr,
>>  	data_error = be32_to_cpu(descr->data_error);
>>  	/* unmap skb buffer */
>>  	dma_unmap_single(ctodev(card), be32_to_cpu(descr->buf_addr),
>> -			 GELIC_NET_MAX_MTU,
>> +			 GELIC_NET_MAX_FRAME,
>>  			 DMA_FROM_DEVICE);
> 
> I suppose my suggestion above would cause a problem here since you
> would need to also define buffer size here. However since it looks like
> you are adding a define below anyway maybe you should just look at
> adding a new RX_BUFSZ define and just drop bufsize completely.

I think making bufsize a static const would be better, since it
would avoid using the pre-processor.

>>  	skb_put(skb, be32_to_cpu(descr->valid_size)?
>> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.h b/drivers/net/ethernet/toshiba/ps3_gelic_net.h
>> index 68f324ed4eaf..0d98defb011e 100644
>> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.h
>> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.h
>> @@ -19,8 +19,9 @@
>>  #define GELIC_NET_RX_DESCRIPTORS        128 /* num of descriptors */
>>  #define GELIC_NET_TX_DESCRIPTORS        128 /* num of descriptors */
>>  
>> -#define GELIC_NET_MAX_MTU               VLAN_ETH_FRAME_LEN
>> -#define GELIC_NET_MIN_MTU               VLAN_ETH_ZLEN
>> +#define GELIC_NET_MAX_FRAME             2312
>> +#define GELIC_NET_MAX_MTU               2294
>> +#define GELIC_NET_MIN_MTU               64
>>  #define GELIC_NET_RXBUF_ALIGN           128
>>  #define GELIC_CARD_RX_CSUM_DEFAULT      1 /* hw chksum */
>>  #define GELIC_NET_WATCHDOG_TIMEOUT      5*HZ
> 
> Since you are adding defines why not just add:
> #define GELIC_NET_RX_BUFSZ \
> 	ALIGN(GELIC_NET_MAX_FRAME, GELIC_NET_RXBUF_ALIGN)

Thanks for the review.

-Geoff

Re: [PATCH net v7 2/2] net/ps3_gelic_net: Use dma_mapping_error

[Geoff Levand]

Hi,

On 3/6/23 08:01, Alexander H Duyck wrote:
> On Sun, 2023-03-05 at 02:08 +0000, Geoff Levand wrote:
>> The current Gelic Etherenet driver was checking the return value of its
>> dma_map_single call, and not using the dma_mapping_error() routine.
>>
>> Fixes runtime problems like these:
>>
>>   DMA-API: ps3_gelic_driver sb_05: device driver failed to check map error
>>   WARNING: CPU: 0 PID: 0 at kernel/dma/debug.c:1027 .check_unmap+0x888/0x8dc
>>
>> Fixes: 02c1889166b4 ("ps3: gigabit ethernet driver for PS3, take3")
>> Signed-off-by: Geoff Levand <geoff@infradead.org>
>> ---
>>  drivers/net/ethernet/toshiba/ps3_gelic_net.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
>> index b0ebe0e603b4..40261947e0ea 100644
>> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
>> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
>> @@ -323,7 +323,7 @@ static int gelic_card_init_chain(struct gelic_card *card,
>>  				       GELIC_DESCR_SIZE,
>>  				       DMA_BIDIRECTIONAL);
>>  
>> -		if (!descr->bus_addr)
>> +		if (dma_mapping_error(ctodev(card), descr->bus_addr))
>>  			goto iommu_error;
>>  
>>  		descr->next = descr + 1;
> 
> The bus_addr value is __be32 and the dma_mapping_error should be CPU
> ordered. I think there was a byteswap using cpu_to_be32 missing here.
> In addition you will probably need to have an intermediate variable to
> store it in to test the DMA address before you byte swap it and store
> it in the descriptor.

I added a local variable 'cpu_addr' as you recommend.

>> @@ -401,7 +401,7 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
>>  						     descr->skb->data,
>>  						     GELIC_NET_MAX_FRAME,
>>  						     DMA_FROM_DEVICE));
>> -	if (!descr->buf_addr) {
>> +	if (dma_mapping_error(ctodev(card), descr->buf_addr)) {
>>  		dev_kfree_skb_any(descr->skb);
>>  		descr->skb = NULL;
>>  		dev_info(ctodev(card),
> 
> This is happening AFTER the DMA is passed through a cpu_to_be32 right?
> The test should be on the raw value, not the byteswapped value.

Did the same here.

>> @@ -781,7 +781,7 @@ static int gelic_descr_prepare_tx(struct gelic_card *card,
>>  
>>  	buf = dma_map_single(ctodev(card), skb->data, skb->len, DMA_TO_DEVICE);
>>  
>> -	if (!buf) {
>> +	if (dma_mapping_error(ctodev(card), buf)) {
>>  		dev_err(ctodev(card),
>>  			"dma map 2 failed (%p, %i). Dropping packet\n",
>>  			skb->data, skb->len);
> 
> This one is correct from what I can tell. I would recommend using it as
> a template and applying it to the two above so that you can sort out
> the byte ordering issues and perform the test and the CPU ordered DMA
> variable.

Thanks for the review.

-Geoff