linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] r8169: set coherent DMA mask as well as streaming DMA mask
@ 2016-10-14 11:39 Ard Biesheuvel
  2016-10-14 13:31 ` David Miller
  0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2016-10-14 11:39 UTC (permalink / raw)
  To: romieu, nic_swsd, netdev, linux-kernel, davem; +Cc: Ard Biesheuvel

PCI devices that are 64-bit DMA capable should set the coherent
DMA mask as well as the streaming DMA mask. On some architectures,
these are managed separately, and so the coherent DMA mask will be
left at its default value of 32 if it is not set explicitly. This
results in errors such as

     r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
     hwdev DMA mask = 0x00000000ffffffff, dev_addr = 0x00000080fbfff000
     swiotlb: coherent allocation failed for device 0000:02:00.0 size=4096
     CPU: 0 PID: 1062 Comm: systemd-udevd Not tainted 4.8.0+ #35
     Hardware name: AMD Seattle/Seattle, BIOS 10:53:24 Oct 13 2016

on systems without memory that is 32-bit addressable by PCI devices.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/net/ethernet/realtek/r8169.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index e55638c7505a..04957a36b11f 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -8273,7 +8273,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if ((sizeof(dma_addr_t) > 4) &&
 	    (use_dac == 1 || (use_dac == -1 && pci_is_pcie(pdev) &&
 			      tp->mac_version >= RTL_GIGA_MAC_VER_18)) &&
-	    !pci_set_dma_mask(pdev, DMA_BIT_MASK(64))) {
+	    !pci_set_dma_mask(pdev, DMA_BIT_MASK(64)) &&
+	    !pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64))) {
 
 		/* CPlusCmd Dual Access Cycle is only needed for non-PCIe */
 		if (!pci_is_pcie(pdev))
@@ -8281,6 +8282,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		dev->features |= NETIF_F_HIGHDMA;
 	} else {
 		rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
+		if (!rc)
+			rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
 		if (rc < 0) {
 			netif_err(tp, probe, dev, "DMA configuration failed\n");
 			goto err_out_unmap_4;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] r8169: set coherent DMA mask as well as streaming DMA mask
  2016-10-14 11:39 [PATCH] r8169: set coherent DMA mask as well as streaming DMA mask Ard Biesheuvel
@ 2016-10-14 13:31 ` David Miller
  2016-10-14 13:32   ` Ard Biesheuvel
  0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2016-10-14 13:31 UTC (permalink / raw)
  To: ard.biesheuvel; +Cc: romieu, nic_swsd, netdev, linux-kernel

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Fri, 14 Oct 2016 12:39:30 +0100

> PCI devices that are 64-bit DMA capable should set the coherent
> DMA mask as well as the streaming DMA mask. On some architectures,
> these are managed separately, and so the coherent DMA mask will be
> left at its default value of 32 if it is not set explicitly. This
> results in errors such as
> 
>      r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
>      hwdev DMA mask = 0x00000000ffffffff, dev_addr = 0x00000080fbfff000
>      swiotlb: coherent allocation failed for device 0000:02:00.0 size=4096
>      CPU: 0 PID: 1062 Comm: systemd-udevd Not tainted 4.8.0+ #35
>      Hardware name: AMD Seattle/Seattle, BIOS 10:53:24 Oct 13 2016
> 
> on systems without memory that is 32-bit addressable by PCI devices.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
 ...
> @@ -8281,6 +8282,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		dev->features |= NETIF_F_HIGHDMA;
>  	} else {
>  		rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
> +		if (!rc)
> +			rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));

As you state 32-bit is the default, therefore this part of your patch is unnecessary.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] r8169: set coherent DMA mask as well as streaming DMA mask
  2016-10-14 13:31 ` David Miller
@ 2016-10-14 13:32   ` Ard Biesheuvel
  2016-10-14 13:34     ` David Miller
  0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2016-10-14 13:32 UTC (permalink / raw)
  To: David Miller
  Cc: Francois Romieu, Realtek linux nic maintainers,
	<netdev@vger.kernel.org>,
	linux-kernel

On 14 October 2016 at 14:31, David Miller <davem@davemloft.net> wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Date: Fri, 14 Oct 2016 12:39:30 +0100
>
>> PCI devices that are 64-bit DMA capable should set the coherent
>> DMA mask as well as the streaming DMA mask. On some architectures,
>> these are managed separately, and so the coherent DMA mask will be
>> left at its default value of 32 if it is not set explicitly. This
>> results in errors such as
>>
>>      r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
>>      hwdev DMA mask = 0x00000000ffffffff, dev_addr = 0x00000080fbfff000
>>      swiotlb: coherent allocation failed for device 0000:02:00.0 size=4096
>>      CPU: 0 PID: 1062 Comm: systemd-udevd Not tainted 4.8.0+ #35
>>      Hardware name: AMD Seattle/Seattle, BIOS 10:53:24 Oct 13 2016
>>
>> on systems without memory that is 32-bit addressable by PCI devices.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>  ...
>> @@ -8281,6 +8282,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>               dev->features |= NETIF_F_HIGHDMA;
>>       } else {
>>               rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>> +             if (!rc)
>> +                     rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
>
> As you state 32-bit is the default, therefore this part of your patch is unnecessary.

Perhaps, but the original code did not assume that either. Should I
remove the other call in a subsequent patch as well?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] r8169: set coherent DMA mask as well as streaming DMA mask
  2016-10-14 13:32   ` Ard Biesheuvel
@ 2016-10-14 13:34     ` David Miller
  2016-10-14 13:36       ` Ard Biesheuvel
  0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2016-10-14 13:34 UTC (permalink / raw)
  To: ard.biesheuvel; +Cc: romieu, nic_swsd, netdev, linux-kernel

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Fri, 14 Oct 2016 14:32:24 +0100

> On 14 October 2016 at 14:31, David Miller <davem@davemloft.net> wrote:
>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Date: Fri, 14 Oct 2016 12:39:30 +0100
>>
>>> PCI devices that are 64-bit DMA capable should set the coherent
>>> DMA mask as well as the streaming DMA mask. On some architectures,
>>> these are managed separately, and so the coherent DMA mask will be
>>> left at its default value of 32 if it is not set explicitly. This
>>> results in errors such as
>>>
>>>      r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
>>>      hwdev DMA mask = 0x00000000ffffffff, dev_addr = 0x00000080fbfff000
>>>      swiotlb: coherent allocation failed for device 0000:02:00.0 size=4096
>>>      CPU: 0 PID: 1062 Comm: systemd-udevd Not tainted 4.8.0+ #35
>>>      Hardware name: AMD Seattle/Seattle, BIOS 10:53:24 Oct 13 2016
>>>
>>> on systems without memory that is 32-bit addressable by PCI devices.
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>  ...
>>> @@ -8281,6 +8282,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>               dev->features |= NETIF_F_HIGHDMA;
>>>       } else {
>>>               rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>>> +             if (!rc)
>>> +                     rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
>>
>> As you state 32-bit is the default, therefore this part of your patch is unnecessary.
> 
> Perhaps, but the original code did not assume that either. Should I
> remove the other call in a subsequent patch as well?

I simply want you to respin this with the above hunk removed.

Your code changes and your commit message must be consistent.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] r8169: set coherent DMA mask as well as streaming DMA mask
  2016-10-14 13:34     ` David Miller
@ 2016-10-14 13:36       ` Ard Biesheuvel
  0 siblings, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2016-10-14 13:36 UTC (permalink / raw)
  To: David Miller
  Cc: Francois Romieu, Realtek linux nic maintainers,
	<netdev@vger.kernel.org>,
	linux-kernel

On 14 October 2016 at 14:34, David Miller <davem@davemloft.net> wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Date: Fri, 14 Oct 2016 14:32:24 +0100
>
>> On 14 October 2016 at 14:31, David Miller <davem@davemloft.net> wrote:
>>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Date: Fri, 14 Oct 2016 12:39:30 +0100
>>>
>>>> PCI devices that are 64-bit DMA capable should set the coherent
>>>> DMA mask as well as the streaming DMA mask. On some architectures,
>>>> these are managed separately, and so the coherent DMA mask will be
>>>> left at its default value of 32 if it is not set explicitly. This
>>>> results in errors such as
>>>>
>>>>      r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
>>>>      hwdev DMA mask = 0x00000000ffffffff, dev_addr = 0x00000080fbfff000
>>>>      swiotlb: coherent allocation failed for device 0000:02:00.0 size=4096
>>>>      CPU: 0 PID: 1062 Comm: systemd-udevd Not tainted 4.8.0+ #35
>>>>      Hardware name: AMD Seattle/Seattle, BIOS 10:53:24 Oct 13 2016
>>>>
>>>> on systems without memory that is 32-bit addressable by PCI devices.
>>>>
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>  ...
>>>> @@ -8281,6 +8282,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>>               dev->features |= NETIF_F_HIGHDMA;
>>>>       } else {
>>>>               rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>>>> +             if (!rc)
>>>> +                     rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
>>>
>>> As you state 32-bit is the default, therefore this part of your patch is unnecessary.
>>
>> Perhaps, but the original code did not assume that either. Should I
>> remove the other call in a subsequent patch as well?
>
> I simply want you to respin this with the above hunk removed.
>
> Your code changes and your commit message must be consistent.

OK, fair enough

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-10-14 13:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-14 11:39 [PATCH] r8169: set coherent DMA mask as well as streaming DMA mask Ard Biesheuvel
2016-10-14 13:31 ` David Miller
2016-10-14 13:32   ` Ard Biesheuvel
2016-10-14 13:34     ` David Miller
2016-10-14 13:36       ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).