* [PATCH] net: tulip: turn compile-time warning into dev_warn()
@ 2015-11-19 10:42 Arnd Bergmann
2015-11-19 12:26 ` Will Deacon
2015-11-20 16:03 ` David Miller
0 siblings, 2 replies; 8+ messages in thread
From: Arnd Bergmann @ 2015-11-19 10:42 UTC (permalink / raw)
To: netdev, David Miller; +Cc: will.deacon, ard.biesheuvel, Grant Grundler
The tulip driver causes annoying build-time warnings for allmodconfig
builds for all recent architectures:
dec/tulip/winbond-840.c:910:2: warning: #warning Processor architecture undefined
dec/tulip/tulip_core.c:101:2: warning: #warning Processor architecture undefined!
This is the last remaining warning for arm64, and I'd like to get rid of
it. We don't really know the cache line size, architecturally it would
be at least 16 bytes, but all implementations I found have 64 or 128
bytes. Configuring tulip for 32-byte lines as we do on ARM32 seems to
be the safe but slow default, and nobody who cares about performance these
days would use a tulip chip anyway, so we can just use that.
To save the next person the job of trying to find out what this is for
and picking a default for their architecture just to kill off the warning,
I'm now removing the preprocessor #warning and turning it into a pr_warn
or dev_warn that prints the equivalent information when the driver gets
loaded.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
index ed41559bae77..b553409e04ad 100644
--- a/drivers/net/ethernet/dec/tulip/tulip_core.c
+++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
@@ -98,8 +98,7 @@ static int csr0 = 0x01A00000 | 0x4800;
#elif defined(__mips__)
static int csr0 = 0x00200000 | 0x4000;
#else
-#warning Processor architecture undefined!
-static int csr0 = 0x00A00000 | 0x4800;
+static int csr0;
#endif
/* Operational parameters that usually are not changed. */
@@ -1982,6 +1981,12 @@ static int __init tulip_init (void)
pr_info("%s", version);
#endif
+ if (!csr0) {
+ pr_warn("tulip: unknown CPU architecture, using default csr0\n");
+ /* default to 8 longword cache line alignment */
+ csr0 = 0x00A00000 | 0x4800;
+ }
+
/* copy module parms into globals */
tulip_rx_copybreak = rx_copybreak;
tulip_max_interrupt_work = max_interrupt_work;
diff --git a/drivers/net/ethernet/dec/tulip/winbond-840.c b/drivers/net/ethernet/dec/tulip/winbond-840.c
index 9beb3d34d4ba..3c0e4d5c5fef 100644
--- a/drivers/net/ethernet/dec/tulip/winbond-840.c
+++ b/drivers/net/ethernet/dec/tulip/winbond-840.c
@@ -907,7 +907,7 @@ static void init_registers(struct net_device *dev)
#elif defined(CONFIG_SPARC) || defined (CONFIG_PARISC) || defined(CONFIG_ARM)
i |= 0x4800;
#else
-#warning Processor architecture undefined
+ dev_warn(&dev->dev, "unknown CPU architecture, using default csr0 setting\n");
i |= 0x4800;
#endif
iowrite32(i, ioaddr + PCIBusCfg);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] net: tulip: turn compile-time warning into dev_warn()
2015-11-19 10:42 [PATCH] net: tulip: turn compile-time warning into dev_warn() Arnd Bergmann
@ 2015-11-19 12:26 ` Will Deacon
2015-11-19 20:29 ` Florian Fainelli
2015-11-19 21:37 ` Grant Grundler
2015-11-20 16:03 ` David Miller
1 sibling, 2 replies; 8+ messages in thread
From: Will Deacon @ 2015-11-19 12:26 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: netdev, David Miller, ard.biesheuvel, Grant Grundler
On Thu, Nov 19, 2015 at 11:42:26AM +0100, Arnd Bergmann wrote:
> The tulip driver causes annoying build-time warnings for allmodconfig
> builds for all recent architectures:
>
> dec/tulip/winbond-840.c:910:2: warning: #warning Processor architecture undefined
> dec/tulip/tulip_core.c:101:2: warning: #warning Processor architecture undefined!
>
> This is the last remaining warning for arm64, and I'd like to get rid of
> it. We don't really know the cache line size, architecturally it would
> be at least 16 bytes, but all implementations I found have 64 or 128
> bytes. Configuring tulip for 32-byte lines as we do on ARM32 seems to
> be the safe but slow default, and nobody who cares about performance these
> days would use a tulip chip anyway, so we can just use that.
>
> To save the next person the job of trying to find out what this is for
> and picking a default for their architecture just to kill off the warning,
> I'm now removing the preprocessor #warning and turning it into a pr_warn
> or dev_warn that prints the equivalent information when the driver gets
> loaded.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
> index ed41559bae77..b553409e04ad 100644
> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
> @@ -98,8 +98,7 @@ static int csr0 = 0x01A00000 | 0x4800;
> #elif defined(__mips__)
> static int csr0 = 0x00200000 | 0x4000;
> #else
> -#warning Processor architecture undefined!
> -static int csr0 = 0x00A00000 | 0x4800;
> +static int csr0;
> #endif
>
> /* Operational parameters that usually are not changed. */
> @@ -1982,6 +1981,12 @@ static int __init tulip_init (void)
> pr_info("%s", version);
> #endif
>
> + if (!csr0) {
> + pr_warn("tulip: unknown CPU architecture, using default csr0\n");
> + /* default to 8 longword cache line alignment */
> + csr0 = 0x00A00000 | 0x4800;
Maybe print "defaulting to 8 longword cache line alignment" instead of
"default csr0"?
> diff --git a/drivers/net/ethernet/dec/tulip/winbond-840.c b/drivers/net/ethernet/dec/tulip/winbond-840.c
> index 9beb3d34d4ba..3c0e4d5c5fef 100644
> --- a/drivers/net/ethernet/dec/tulip/winbond-840.c
> +++ b/drivers/net/ethernet/dec/tulip/winbond-840.c
> @@ -907,7 +907,7 @@ static void init_registers(struct net_device *dev)
> #elif defined(CONFIG_SPARC) || defined (CONFIG_PARISC) || defined(CONFIG_ARM)
> i |= 0x4800;
> #else
> -#warning Processor architecture undefined
> + dev_warn(&dev->dev, "unknown CPU architecture, using default csr0 setting\n");
> i |= 0x4800;
Then we could print the default csr0 value here.
But, to be honest, this patch fixes a #warning on arm64 for a driver that
I never expect to be used. So whatever you do to silence it:
Acked-by: Will Deacon <will.deacon@arm.com>
/me waits for on-soc tulip integration.
Will
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: tulip: turn compile-time warning into dev_warn()
2015-11-19 12:26 ` Will Deacon
@ 2015-11-19 20:29 ` Florian Fainelli
2015-11-19 21:57 ` Grant Grundler
2015-11-19 21:37 ` Grant Grundler
1 sibling, 1 reply; 8+ messages in thread
From: Florian Fainelli @ 2015-11-19 20:29 UTC (permalink / raw)
To: Will Deacon, Arnd Bergmann
Cc: netdev, David Miller, ard.biesheuvel, Grant Grundler
On 19/11/15 04:26, Will Deacon wrote:
> On Thu, Nov 19, 2015 at 11:42:26AM +0100, Arnd Bergmann wrote:
>> The tulip driver causes annoying build-time warnings for allmodconfig
>> builds for all recent architectures:
>>
>> dec/tulip/winbond-840.c:910:2: warning: #warning Processor architecture undefined
>> dec/tulip/tulip_core.c:101:2: warning: #warning Processor architecture undefined!
>>
>> This is the last remaining warning for arm64, and I'd like to get rid of
>> it. We don't really know the cache line size, architecturally it would
>> be at least 16 bytes, but all implementations I found have 64 or 128
>> bytes. Configuring tulip for 32-byte lines as we do on ARM32 seems to
>> be the safe but slow default, and nobody who cares about performance these
>> days would use a tulip chip anyway, so we can just use that.
>>
>> To save the next person the job of trying to find out what this is for
>> and picking a default for their architecture just to kill off the warning,
>> I'm now removing the preprocessor #warning and turning it into a pr_warn
>> or dev_warn that prints the equivalent information when the driver gets
>> loaded.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>
>> diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
>> index ed41559bae77..b553409e04ad 100644
>> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
>> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
>> @@ -98,8 +98,7 @@ static int csr0 = 0x01A00000 | 0x4800;
>> #elif defined(__mips__)
>> static int csr0 = 0x00200000 | 0x4000;
>> #else
>> -#warning Processor architecture undefined!
>> -static int csr0 = 0x00A00000 | 0x4800;
>> +static int csr0;
>> #endif
>>
>> /* Operational parameters that usually are not changed. */
>> @@ -1982,6 +1981,12 @@ static int __init tulip_init (void)
>> pr_info("%s", version);
>> #endif
>>
>> + if (!csr0) {
>> + pr_warn("tulip: unknown CPU architecture, using default csr0\n");
>> + /* default to 8 longword cache line alignment */
>> + csr0 = 0x00A00000 | 0x4800;
>
> Maybe print "defaulting to 8 longword cache line alignment" instead of
> "default csr0"?
>
>> diff --git a/drivers/net/ethernet/dec/tulip/winbond-840.c b/drivers/net/ethernet/dec/tulip/winbond-840.c
>> index 9beb3d34d4ba..3c0e4d5c5fef 100644
>> --- a/drivers/net/ethernet/dec/tulip/winbond-840.c
>> +++ b/drivers/net/ethernet/dec/tulip/winbond-840.c
>> @@ -907,7 +907,7 @@ static void init_registers(struct net_device *dev)
>> #elif defined(CONFIG_SPARC) || defined (CONFIG_PARISC) || defined(CONFIG_ARM)
>> i |= 0x4800;
>> #else
>> -#warning Processor architecture undefined
>> + dev_warn(&dev->dev, "unknown CPU architecture, using default csr0 setting\n");
>> i |= 0x4800;
>
> Then we could print the default csr0 value here.
>
> But, to be honest, this patch fixes a #warning on arm64 for a driver that
> I never expect to be used. So whatever you do to silence it:
>
> Acked-by: Will Deacon <will.deacon@arm.com>
>
> /me waits for on-soc tulip integration.
FWIW, this already happened, the ADMtek/Infineon ADM8668 actually
integrated a Tulip chip. I have not submitted these patches below from
the OpenWrt tree because the chip is barely used nowadays, and it was
only mostly popular with the Linksys WRTU54G.
The patches could be made less intrusive if we did convert the pci_dma*
calls into regular DMA-API calls, which they are nowadays, oh well!
https://dev.openwrt.org/browser/trunk/target/linux/adm8668/patches-3.18/004-tulip_pci_split.patch
https://dev.openwrt.org/browser/trunk/target/linux/adm8668/patches-3.18/005-tulip_platform.patch
--
Florian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: tulip: turn compile-time warning into dev_warn()
2015-11-19 12:26 ` Will Deacon
2015-11-19 20:29 ` Florian Fainelli
@ 2015-11-19 21:37 ` Grant Grundler
1 sibling, 0 replies; 8+ messages in thread
From: Grant Grundler @ 2015-11-19 21:37 UTC (permalink / raw)
To: Will Deacon
Cc: Arnd Bergmann, open list:TULIP NETWORK DRI...,
David Miller, ard.biesheuvel, Grant Grundler
On Thu, Nov 19, 2015 at 4:26 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Thu, Nov 19, 2015 at 11:42:26AM +0100, Arnd Bergmann wrote:
>> The tulip driver causes annoying build-time warnings for allmodconfig
>> builds for all recent architectures:
>>
>> dec/tulip/winbond-840.c:910:2: warning: #warning Processor architecture undefined
>> dec/tulip/tulip_core.c:101:2: warning: #warning Processor architecture undefined!
>>
>> This is the last remaining warning for arm64, and I'd like to get rid of
>> it. We don't really know the cache line size, architecturally it would
>> be at least 16 bytes, but all implementations I found have 64 or 128
>> bytes. Configuring tulip for 32-byte lines as we do on ARM32 seems to
>> be the safe but slow default, and nobody who cares about performance these
>> days would use a tulip chip anyway, so we can just use that.
>>
>> To save the next person the job of trying to find out what this is for
>> and picking a default for their architecture just to kill off the warning,
>> I'm now removing the preprocessor #warning and turning it into a pr_warn
>> or dev_warn that prints the equivalent information when the driver gets
>> loaded.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>
>> diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
>> index ed41559bae77..b553409e04ad 100644
>> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
>> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
>> @@ -98,8 +98,7 @@ static int csr0 = 0x01A00000 | 0x4800;
>> #elif defined(__mips__)
>> static int csr0 = 0x00200000 | 0x4000;
>> #else
>> -#warning Processor architecture undefined!
>> -static int csr0 = 0x00A00000 | 0x4800;
>> +static int csr0;
>> #endif
>>
>> /* Operational parameters that usually are not changed. */
>> @@ -1982,6 +1981,12 @@ static int __init tulip_init (void)
>> pr_info("%s", version);
>> #endif
>>
>> + if (!csr0) {
>> + pr_warn("tulip: unknown CPU architecture, using default csr0\n");
>> + /* default to 8 longword cache line alignment */
>> + csr0 = 0x00A00000 | 0x4800;
>
> Maybe print "defaulting to 8 longword cache line alignment" instead of
> "default csr0"?
This is a good idea.
>> diff --git a/drivers/net/ethernet/dec/tulip/winbond-840.c b/drivers/net/ethernet/dec/tulip/winbond-840.c
>> index 9beb3d34d4ba..3c0e4d5c5fef 100644
>> --- a/drivers/net/ethernet/dec/tulip/winbond-840.c
>> +++ b/drivers/net/ethernet/dec/tulip/winbond-840.c
>> @@ -907,7 +907,7 @@ static void init_registers(struct net_device *dev)
>> #elif defined(CONFIG_SPARC) || defined (CONFIG_PARISC) || defined(CONFIG_ARM)
>> i |= 0x4800;
>> #else
>> -#warning Processor architecture undefined
>> + dev_warn(&dev->dev, "unknown CPU architecture, using default csr0 setting\n");
>> i |= 0x4800;
>
> Then we could print the default csr0 value here.
>
> But, to be honest, this patch fixes a #warning on arm64 for a driver that
> I never expect to be used. So whatever you do to silence it:
>
> Acked-by: Will Deacon <will.deacon@arm.com>
yeah - same here:
Acked-by: Grant Grundler <grundler@parisc-linux.org>
cheers,
grant
>
> /me waits for on-soc tulip integration.
>
> Will
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: tulip: turn compile-time warning into dev_warn()
2015-11-19 20:29 ` Florian Fainelli
@ 2015-11-19 21:57 ` Grant Grundler
2015-11-19 23:50 ` Francois Romieu
0 siblings, 1 reply; 8+ messages in thread
From: Grant Grundler @ 2015-11-19 21:57 UTC (permalink / raw)
To: Florian Fainelli
Cc: Will Deacon, Arnd Bergmann, open list:TULIP NETWORK DRI...,
David Miller, ard.biesheuvel, Grant Grundler
On Thu, Nov 19, 2015 at 12:29 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 19/11/15 04:26, Will Deacon wrote:
...
>> /me waits for on-soc tulip integration.
>
> FWIW, this already happened, the ADMtek/Infineon ADM8668 actually
> integrated a Tulip chip. I have not submitted these patches below from
> the OpenWrt tree because the chip is barely used nowadays, and it was
> only mostly popular with the Linksys WRTU54G.
>
> The patches could be made less intrusive if we did convert the pci_dma*
> calls into regular DMA-API calls, which they are nowadays, oh well!
I agree. IIRC, there was no DMA-API when this driver was written.
James Bottomley added DMA API later and there was no need to convert
since Tulip devices were _only_ PCI at the time.
> https://dev.openwrt.org/browser/trunk/target/linux/adm8668/patches-3.18/004-tulip_pci_split.patch
In general this would be a reasonable patch to submit here with some caveats:
1) convert to DMA API (first patch)
2) add CONFIG_PCI code (second patch) to handle the remaining
discovery and PCI Config space bits.
Some additional minor refactoring of the code could convert this into
a "multi-bus driver" if there is any system that could incorporate
both a platform device and a PCI device.
I expect the conversion to DMA API to be straight forward as the next
patch shows:
> https://dev.openwrt.org/browser/trunk/target/linux/adm8668/patches-3.18/005-tulip_platform.patch
Split this patch into two parts: convert to DMA-API (first patch) and
platform device support (third patch). Should be a "no brainer" to
accept.
Lastly, net/ethernet/dec/tulip driver is up for adoption. I've just
been extremely lazy about updating the MAINTAINERS entry but will
submit that shortly (apologies to Arndt for the bounced email - I know
it's a bit disconcerting to see that.)
I'm happy to continue review tulip code changes anyway.
cheers,
grant
> --
> Florian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: tulip: turn compile-time warning into dev_warn()
2015-11-19 21:57 ` Grant Grundler
@ 2015-11-19 23:50 ` Francois Romieu
2015-11-20 1:41 ` Grant Grundler
0 siblings, 1 reply; 8+ messages in thread
From: Francois Romieu @ 2015-11-19 23:50 UTC (permalink / raw)
To: Grant Grundler
Cc: Florian Fainelli, Will Deacon, Arnd Bergmann,
open list:TULIP NETWORK DRI...,
David Miller, ard.biesheuvel, Grant Grundler
Grant Grundler <grantgrundler@gmail.com> :
[...]
> Some additional minor refactoring of the code could convert this into
> a "multi-bus driver" if there is any system that could incorporate
> both a platform device and a PCI device.
>
> I expect the conversion to DMA API to be straight forward as the next
> patch shows:
You may change your mind once error checking is factored in.
Uncompiled stuff below includes a remaining WTF I am no too sure about
but it should be closer to what is needed.
diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
index ed41559..0e18b5d9 100644
--- a/drivers/net/ethernet/dec/tulip/tulip_core.c
+++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
@@ -263,7 +263,6 @@ static netdev_tx_t tulip_start_xmit(struct sk_buff *skb,
struct net_device *dev);
static int tulip_open(struct net_device *dev);
static int tulip_close(struct net_device *dev);
-static void tulip_up(struct net_device *dev);
static void tulip_down(struct net_device *dev);
static struct net_device_stats *tulip_get_stats(struct net_device *dev);
static int private_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
@@ -291,7 +290,7 @@ static void tulip_set_power_state (struct tulip_private *tp,
}
-static void tulip_up(struct net_device *dev)
+static int tulip_up(struct net_device *dev)
{
struct tulip_private *tp = netdev_priv(dev);
void __iomem *ioaddr = tp->base_addr;
@@ -299,10 +298,6 @@ static void tulip_up(struct net_device *dev)
u32 reg;
int i;
-#ifdef CONFIG_TULIP_NAPI
- napi_enable(&tp->napi);
-#endif
-
/* Wake the chip from sleep/snooze mode. */
tulip_set_power_state (tp, 0, 0);
@@ -353,6 +348,7 @@ static void tulip_up(struct net_device *dev)
/* This is set_rx_mode(), but without starting the transmitter. */
u16 *eaddrs = (u16 *)dev->dev_addr;
u16 *setup_frm = &tp->setup_frame[15*6];
+ struct device *d = &tp->pdev->dev;
dma_addr_t mapping;
/* 21140 bug: you must add the broadcast address. */
@@ -362,9 +358,12 @@ static void tulip_up(struct net_device *dev)
*setup_frm++ = eaddrs[1]; *setup_frm++ = eaddrs[1];
*setup_frm++ = eaddrs[2]; *setup_frm++ = eaddrs[2];
- mapping = pci_map_single(tp->pdev, tp->setup_frame,
- sizeof(tp->setup_frame),
- PCI_DMA_TODEVICE);
+ mapping = dma_map_single(d, tp->setup_frame,
+ sizeof(tp->setup_frame), DMA_TO_DEVICE);
+ if (unlikely(dma_mapping_error(d, mapping))) {
+ tulip_set_power_state(tp, 0, 1);
+ return -EIO;
+ }
tp->tx_buffers[tp->cur_tx].skb = NULL;
tp->tx_buffers[tp->cur_tx].mapping = mapping;
@@ -376,6 +375,10 @@ static void tulip_up(struct net_device *dev)
tp->cur_tx++;
}
+#ifdef CONFIG_TULIP_NAPI
+ napi_enable(&tp->napi);
+#endif
+
tp->saved_if_port = dev->if_port;
if (dev->if_port == 0)
dev->if_port = tp->default_port;
@@ -516,24 +519,30 @@ static int
tulip_open(struct net_device *dev)
{
struct tulip_private *tp = netdev_priv(dev);
- int retval;
+ int irq = tp->pdev->irq;
+ int rc;
- tulip_init_ring (dev);
+ rc = tulip_init_ring(dev);
+ if (rc < 0)
+ goto out;
- retval = request_irq(tp->pdev->irq, tulip_interrupt, IRQF_SHARED,
- dev->name, dev);
- if (retval)
- goto free_ring;
+ rc = request_irq(irq, tulip_interrupt, IRQF_SHARED, dev->name, dev);
+ if (rc < 0)
+ goto free_ring_0;
- tulip_up (dev);
+ rc = tulip_up(dev);
+ if (rc < 0)
+ goto free_irq_1;
netif_start_queue (dev);
+out:
+ return rc;
- return 0;
-
-free_ring:
- tulip_free_ring (dev);
- return retval;
+free_irq_1:
+ free_irq(irq, dev);
+free_ring_0:
+ tulip_free_ring(dev);
+ return rc;
}
@@ -614,9 +623,11 @@ out_unlock:
/* Initialize the Rx and Tx rings, along with various 'dev' bits. */
-static void tulip_init_ring(struct net_device *dev)
+static int tulip_init_ring(struct net_device *dev)
{
struct tulip_private *tp = netdev_priv(dev);
+ struct device *d = &tp->pdev->dev;
+ int rc;
int i;
tp->susp_rx = 0;
@@ -642,10 +653,17 @@ static void tulip_init_ring(struct net_device *dev)
use skb_reserve() to align the IP header! */
struct sk_buff *skb = netdev_alloc_skb(dev, PKT_BUF_SZ);
tp->rx_buffers[i].skb = skb;
- if (skb == NULL)
- break;
- mapping = pci_map_single(tp->pdev, skb->data,
- PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
+ if (!skb) {
+ rc = -ENOMEM;
+ goto err_out;
+ }
+ mapping = dma_map_single(d, skb->data, PKT_BUF_SZ,
+ DMA_FROM_DEVICE);
+ if (unlikely(dma_mapping_error(d, mapping))) {
+ rc = -EIO;
+ dev_kfree_skb(skb);
+ goto err_out;
+ }
tp->rx_buffers[i].mapping = mapping;
tp->rx_ring[i].status = cpu_to_le32(DescOwned); /* Owned by Tulip chip */
tp->rx_ring[i].buffer1 = cpu_to_le32(mapping);
@@ -661,12 +679,24 @@ static void tulip_init_ring(struct net_device *dev)
tp->tx_ring[i].buffer2 = cpu_to_le32(tp->tx_ring_dma + sizeof(struct tulip_tx_desc) * (i + 1));
}
tp->tx_ring[i-1].buffer2 = cpu_to_le32(tp->tx_ring_dma);
+
+ return 0;
+
+err_out:
+ while (--i) {
+ struct ring_info *info = tp->rx_buffers + i;
+
+ dma_unmap_single(d, info->mapping, PKT_BUF_SZ, DMA_FROM_DEVICE);
+ dev_kfree_skb(info->skb);
+ }
+ return rc;
}
static netdev_tx_t
tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct tulip_private *tp = netdev_priv(dev);
+ struct device *d = &tp->pdev->dev;
int entry;
u32 flag;
dma_addr_t mapping;
@@ -678,8 +708,14 @@ tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
entry = tp->cur_tx % TX_RING_SIZE;
tp->tx_buffers[entry].skb = skb;
- mapping = pci_map_single(tp->pdev, skb->data,
- skb->len, PCI_DMA_TODEVICE);
+
+ mapping = dma_map_single(d, skb->data, skb->len, DMA_TO_DEVICE);
+ if (unlikely(dma_mapping_error(d, mapping))) {
+ dev->stats.tx_dropped++;
+ dev_kfree_skb_any(skb);
+ goto out_unlock;
+ }
+
tp->tx_buffers[entry].mapping = mapping;
tp->tx_ring[entry].buffer1 = cpu_to_le32(mapping);
@@ -707,6 +743,7 @@ tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
/* Trigger an immediate transmit demand. */
iowrite32(0, tp->base_addr + CSR1);
+out_unlock:
spin_unlock_irqrestore(&tp->lock, flags);
return NETDEV_TX_OK;
@@ -714,6 +751,7 @@ tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
static void tulip_clean_tx_ring(struct tulip_private *tp)
{
+ struct device *d = &tp->pdev->dev;
unsigned int dirty_tx;
for (dirty_tx = tp->dirty_tx ; tp->cur_tx - dirty_tx > 0;
@@ -730,16 +768,15 @@ static void tulip_clean_tx_ring(struct tulip_private *tp)
if (tp->tx_buffers[entry].skb == NULL) {
/* test because dummy frames not mapped */
if (tp->tx_buffers[entry].mapping)
- pci_unmap_single(tp->pdev,
+ dma_unmap_single(d,
tp->tx_buffers[entry].mapping,
sizeof(tp->setup_frame),
- PCI_DMA_TODEVICE);
+ DMA_TO_DEVICE);
continue;
}
- pci_unmap_single(tp->pdev, tp->tx_buffers[entry].mapping,
- tp->tx_buffers[entry].skb->len,
- PCI_DMA_TODEVICE);
+ dma_unmap_single(d, tp->tx_buffers[entry].mapping,
+ tp->tx_buffers[entry].skb->len, DMA_TO_DEVICE);
/* Free the original skb. */
dev_kfree_skb_irq(tp->tx_buffers[entry].skb);
@@ -796,6 +833,7 @@ static void tulip_down (struct net_device *dev)
static void tulip_free_ring (struct net_device *dev)
{
struct tulip_private *tp = netdev_priv(dev);
+ struct device *d = &tp->pdev->dev;
int i;
/* Free all the skbuffs in the Rx queue. */
@@ -811,8 +849,7 @@ static void tulip_free_ring (struct net_device *dev)
/* An invalid address. */
tp->rx_ring[i].buffer1 = cpu_to_le32(0xBADF00D0);
if (skb) {
- pci_unmap_single(tp->pdev, mapping, PKT_BUF_SZ,
- PCI_DMA_FROMDEVICE);
+ dma_unmap_single(d, mapping, PKT_BUF_SZ, DMA_FROM_DEVICE);
dev_kfree_skb (skb);
}
}
@@ -821,8 +858,8 @@ static void tulip_free_ring (struct net_device *dev)
struct sk_buff *skb = tp->tx_buffers[i].skb;
if (skb != NULL) {
- pci_unmap_single(tp->pdev, tp->tx_buffers[i].mapping,
- skb->len, PCI_DMA_TODEVICE);
+ dma_unmap_single(d, tp->tx_buffers[i].mapping, skb->len,
+ DMA_TO_DEVICE);
dev_kfree_skb (skb);
}
tp->tx_buffers[i].skb = NULL;
@@ -1143,7 +1180,9 @@ static void set_rx_mode(struct net_device *dev)
if (tp->cur_tx - tp->dirty_tx > TX_RING_SIZE - 2) {
/* Same setup recently queued, we need not add it. */
} else {
+ struct device *d = &tp->pdev->dev;
unsigned int entry;
+ dma_addr_t mapping;
int dummy = -1;
/* Now add this frame to the Tx list. */
@@ -1164,16 +1203,18 @@ static void set_rx_mode(struct net_device *dev)
}
tp->tx_buffers[entry].skb = NULL;
- tp->tx_buffers[entry].mapping =
- pci_map_single(tp->pdev, tp->setup_frame,
- sizeof(tp->setup_frame),
- PCI_DMA_TODEVICE);
+ mapping = dma_map_single(d, tp->setup_frame,
+ sizeof(tp->setup_frame),
+ DMA_TO_DEVICE);
+ if (unlikely(dma_mapping_error(d, mapping))) {
+ // WTF ?
+ }
+ tp->tx_buffers[entry].mapping = mapping;
/* Put the setup frame on the Tx list. */
if (entry == TX_RING_SIZE-1)
tx_flags |= DESC_RING_WRAP; /* Wrap ring. */
tp->tx_ring[entry].length = cpu_to_le32(tx_flags);
- tp->tx_ring[entry].buffer1 =
- cpu_to_le32(tp->tx_buffers[entry].mapping);
+ tp->tx_ring[entry].buffer1 = cpu_to_le32(mapping);
tp->tx_ring[entry].status = cpu_to_le32(DescOwned);
if (dummy >= 0)
tp->tx_ring[dummy].status = cpu_to_le32(DescOwned);
@@ -1445,10 +1486,10 @@ static int tulip_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
tp = netdev_priv(dev);
tp->dev = dev;
- tp->rx_ring = pci_alloc_consistent(pdev,
- sizeof(struct tulip_rx_desc) * RX_RING_SIZE +
- sizeof(struct tulip_tx_desc) * TX_RING_SIZE,
- &tp->rx_ring_dma);
+ tp->rx_ring = dma_alloc_coherent(&pdev->dev,
+ sizeof(struct tulip_rx_desc) * RX_RING_SIZE +
+ sizeof(struct tulip_tx_desc) * TX_RING_SIZE,
+ &tp->rx_ring_dma, GFP_KERNEL);
if (!tp->rx_ring)
goto err_out_mtable;
tp->tx_ring = (struct tulip_tx_desc *)(tp->rx_ring + RX_RING_SIZE);
@@ -1783,10 +1824,10 @@ static int tulip_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
return 0;
err_out_free_ring:
- pci_free_consistent (pdev,
- sizeof (struct tulip_rx_desc) * RX_RING_SIZE +
- sizeof (struct tulip_tx_desc) * TX_RING_SIZE,
- tp->rx_ring, tp->rx_ring_dma);
+ dma_free_coherent(&pdev->dev,
+ sizeof(struct tulip_rx_desc) * RX_RING_SIZE +
+ sizeof(struct tulip_tx_desc) * TX_RING_SIZE,
+ tp->rx_ring, tp->rx_ring_dma);
err_out_mtable:
kfree (tp->mtable);
@@ -1874,8 +1915,8 @@ static int tulip_resume(struct pci_dev *pdev)
struct net_device *dev = pci_get_drvdata(pdev);
struct tulip_private *tp = netdev_priv(dev);
void __iomem *ioaddr = tp->base_addr;
- int retval;
unsigned int tmp;
+ int retval;
if (!dev)
return -EINVAL;
@@ -1913,9 +1954,9 @@ static int tulip_resume(struct pci_dev *pdev)
netif_device_attach(dev);
if (netif_running(dev))
- tulip_up(dev);
+ retval = tulip_up(dev);
- return 0;
+ return retval;
}
#endif /* CONFIG_PM */
@@ -1924,17 +1965,13 @@ static int tulip_resume(struct pci_dev *pdev)
static void tulip_remove_one(struct pci_dev *pdev)
{
struct net_device *dev = pci_get_drvdata (pdev);
- struct tulip_private *tp;
-
- if (!dev)
- return;
+ struct tulip_private *tp = netdev_priv(dev);
- tp = netdev_priv(dev);
unregister_netdev(dev);
- pci_free_consistent (pdev,
- sizeof (struct tulip_rx_desc) * RX_RING_SIZE +
- sizeof (struct tulip_tx_desc) * TX_RING_SIZE,
- tp->rx_ring, tp->rx_ring_dma);
+ dma_free_coherent(&pdev->dev,
+ sizeof(struct tulip_rx_desc) * RX_RING_SIZE +
+ sizeof(struct tulip_tx_desc) * TX_RING_SIZE,
+ tp->rx_ring, tp->rx_ring_dma);
kfree (tp->mtable);
pci_iounmap(pdev, tp->base_addr);
free_netdev (dev);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] net: tulip: turn compile-time warning into dev_warn()
2015-11-19 23:50 ` Francois Romieu
@ 2015-11-20 1:41 ` Grant Grundler
0 siblings, 0 replies; 8+ messages in thread
From: Grant Grundler @ 2015-11-20 1:41 UTC (permalink / raw)
To: Francois Romieu
Cc: Florian Fainelli, Will Deacon, Arnd Bergmann,
open list:TULIP NETWORK DRI...,
David Miller, ard.biesheuvel, Grant Grundler
On Thu, Nov 19, 2015 at 3:50 PM, Francois Romieu <romieu@fr.zoreil.com> wrote:
> Grant Grundler <grantgrundler@gmail.com> :
> [...]
>> Some additional minor refactoring of the code could convert this into
>> a "multi-bus driver" if there is any system that could incorporate
>> both a platform device and a PCI device.
>>
>> I expect the conversion to DMA API to be straight forward as the next
>> patch shows:
>
> You may change your mind once error checking is factored in.
In the absence of your patch, I definitely wouldn't.
The reason is the linux 2.4 PCI-DMA mapping API would panic on failure
and never return an error:
http://lists.parisc-linux.org/pipermail/parisc-linux/2000-November/009847.html
The simplistic "conversion" would just panic the system on DMA API
errors and still behave the same as before.
To be clear, I have never advocated for ignoring DMA API errors (just
accepted the linux-2.4 design choice to ignore them since that's what
davem wanted at the time).
> Uncompiled stuff below includes a remaining WTF I am no too sure about
> but it should be closer to what is needed.
Yup - agreed. Over all this LGTM and below I've suggested three ways
to deal with DMA API error handling in set_rx_mode().
cheers,
grant
>
> diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
> index ed41559..0e18b5d9 100644
> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
> @@ -263,7 +263,6 @@ static netdev_tx_t tulip_start_xmit(struct sk_buff *skb,
> struct net_device *dev);
> static int tulip_open(struct net_device *dev);
> static int tulip_close(struct net_device *dev);
> -static void tulip_up(struct net_device *dev);
> static void tulip_down(struct net_device *dev);
> static struct net_device_stats *tulip_get_stats(struct net_device *dev);
> static int private_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
> @@ -291,7 +290,7 @@ static void tulip_set_power_state (struct tulip_private *tp,
> }
>
>
> -static void tulip_up(struct net_device *dev)
> +static int tulip_up(struct net_device *dev)
> {
> struct tulip_private *tp = netdev_priv(dev);
> void __iomem *ioaddr = tp->base_addr;
> @@ -299,10 +298,6 @@ static void tulip_up(struct net_device *dev)
> u32 reg;
> int i;
>
> -#ifdef CONFIG_TULIP_NAPI
> - napi_enable(&tp->napi);
> -#endif
> -
> /* Wake the chip from sleep/snooze mode. */
> tulip_set_power_state (tp, 0, 0);
>
> @@ -353,6 +348,7 @@ static void tulip_up(struct net_device *dev)
> /* This is set_rx_mode(), but without starting the transmitter. */
> u16 *eaddrs = (u16 *)dev->dev_addr;
> u16 *setup_frm = &tp->setup_frame[15*6];
> + struct device *d = &tp->pdev->dev;
> dma_addr_t mapping;
>
> /* 21140 bug: you must add the broadcast address. */
> @@ -362,9 +358,12 @@ static void tulip_up(struct net_device *dev)
> *setup_frm++ = eaddrs[1]; *setup_frm++ = eaddrs[1];
> *setup_frm++ = eaddrs[2]; *setup_frm++ = eaddrs[2];
>
> - mapping = pci_map_single(tp->pdev, tp->setup_frame,
> - sizeof(tp->setup_frame),
> - PCI_DMA_TODEVICE);
> + mapping = dma_map_single(d, tp->setup_frame,
> + sizeof(tp->setup_frame), DMA_TO_DEVICE);
> + if (unlikely(dma_mapping_error(d, mapping))) {
> + tulip_set_power_state(tp, 0, 1);
> + return -EIO;
> + }
> tp->tx_buffers[tp->cur_tx].skb = NULL;
> tp->tx_buffers[tp->cur_tx].mapping = mapping;
>
> @@ -376,6 +375,10 @@ static void tulip_up(struct net_device *dev)
> tp->cur_tx++;
> }
>
> +#ifdef CONFIG_TULIP_NAPI
> + napi_enable(&tp->napi);
> +#endif
> +
> tp->saved_if_port = dev->if_port;
> if (dev->if_port == 0)
> dev->if_port = tp->default_port;
> @@ -516,24 +519,30 @@ static int
> tulip_open(struct net_device *dev)
> {
> struct tulip_private *tp = netdev_priv(dev);
> - int retval;
> + int irq = tp->pdev->irq;
> + int rc;
>
> - tulip_init_ring (dev);
> + rc = tulip_init_ring(dev);
> + if (rc < 0)
> + goto out;
>
> - retval = request_irq(tp->pdev->irq, tulip_interrupt, IRQF_SHARED,
> - dev->name, dev);
> - if (retval)
> - goto free_ring;
> + rc = request_irq(irq, tulip_interrupt, IRQF_SHARED, dev->name, dev);
> + if (rc < 0)
> + goto free_ring_0;
>
> - tulip_up (dev);
> + rc = tulip_up(dev);
> + if (rc < 0)
> + goto free_irq_1;
>
> netif_start_queue (dev);
> +out:
> + return rc;
>
> - return 0;
> -
> -free_ring:
> - tulip_free_ring (dev);
> - return retval;
> +free_irq_1:
> + free_irq(irq, dev);
> +free_ring_0:
> + tulip_free_ring(dev);
> + return rc;
> }
>
>
> @@ -614,9 +623,11 @@ out_unlock:
>
>
> /* Initialize the Rx and Tx rings, along with various 'dev' bits. */
> -static void tulip_init_ring(struct net_device *dev)
> +static int tulip_init_ring(struct net_device *dev)
> {
> struct tulip_private *tp = netdev_priv(dev);
> + struct device *d = &tp->pdev->dev;
> + int rc;
> int i;
>
> tp->susp_rx = 0;
> @@ -642,10 +653,17 @@ static void tulip_init_ring(struct net_device *dev)
> use skb_reserve() to align the IP header! */
> struct sk_buff *skb = netdev_alloc_skb(dev, PKT_BUF_SZ);
> tp->rx_buffers[i].skb = skb;
> - if (skb == NULL)
> - break;
> - mapping = pci_map_single(tp->pdev, skb->data,
> - PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
> + if (!skb) {
> + rc = -ENOMEM;
> + goto err_out;
> + }
> + mapping = dma_map_single(d, skb->data, PKT_BUF_SZ,
> + DMA_FROM_DEVICE);
> + if (unlikely(dma_mapping_error(d, mapping))) {
> + rc = -EIO;
> + dev_kfree_skb(skb);
> + goto err_out;
> + }
> tp->rx_buffers[i].mapping = mapping;
> tp->rx_ring[i].status = cpu_to_le32(DescOwned); /* Owned by Tulip chip */
> tp->rx_ring[i].buffer1 = cpu_to_le32(mapping);
> @@ -661,12 +679,24 @@ static void tulip_init_ring(struct net_device *dev)
> tp->tx_ring[i].buffer2 = cpu_to_le32(tp->tx_ring_dma + sizeof(struct tulip_tx_desc) * (i + 1));
> }
> tp->tx_ring[i-1].buffer2 = cpu_to_le32(tp->tx_ring_dma);
> +
> + return 0;
> +
> +err_out:
> + while (--i) {
> + struct ring_info *info = tp->rx_buffers + i;
> +
> + dma_unmap_single(d, info->mapping, PKT_BUF_SZ, DMA_FROM_DEVICE);
> + dev_kfree_skb(info->skb);
> + }
> + return rc;
> }
>
> static netdev_tx_t
> tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
> {
> struct tulip_private *tp = netdev_priv(dev);
> + struct device *d = &tp->pdev->dev;
> int entry;
> u32 flag;
> dma_addr_t mapping;
> @@ -678,8 +708,14 @@ tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
> entry = tp->cur_tx % TX_RING_SIZE;
>
> tp->tx_buffers[entry].skb = skb;
> - mapping = pci_map_single(tp->pdev, skb->data,
> - skb->len, PCI_DMA_TODEVICE);
> +
> + mapping = dma_map_single(d, skb->data, skb->len, DMA_TO_DEVICE);
> + if (unlikely(dma_mapping_error(d, mapping))) {
> + dev->stats.tx_dropped++;
> + dev_kfree_skb_any(skb);
> + goto out_unlock;
> + }
> +
> tp->tx_buffers[entry].mapping = mapping;
> tp->tx_ring[entry].buffer1 = cpu_to_le32(mapping);
>
> @@ -707,6 +743,7 @@ tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
> /* Trigger an immediate transmit demand. */
> iowrite32(0, tp->base_addr + CSR1);
>
> +out_unlock:
> spin_unlock_irqrestore(&tp->lock, flags);
>
> return NETDEV_TX_OK;
> @@ -714,6 +751,7 @@ tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
>
> static void tulip_clean_tx_ring(struct tulip_private *tp)
> {
> + struct device *d = &tp->pdev->dev;
> unsigned int dirty_tx;
>
> for (dirty_tx = tp->dirty_tx ; tp->cur_tx - dirty_tx > 0;
> @@ -730,16 +768,15 @@ static void tulip_clean_tx_ring(struct tulip_private *tp)
> if (tp->tx_buffers[entry].skb == NULL) {
> /* test because dummy frames not mapped */
> if (tp->tx_buffers[entry].mapping)
> - pci_unmap_single(tp->pdev,
> + dma_unmap_single(d,
> tp->tx_buffers[entry].mapping,
> sizeof(tp->setup_frame),
> - PCI_DMA_TODEVICE);
> + DMA_TO_DEVICE);
> continue;
> }
>
> - pci_unmap_single(tp->pdev, tp->tx_buffers[entry].mapping,
> - tp->tx_buffers[entry].skb->len,
> - PCI_DMA_TODEVICE);
> + dma_unmap_single(d, tp->tx_buffers[entry].mapping,
> + tp->tx_buffers[entry].skb->len, DMA_TO_DEVICE);
>
> /* Free the original skb. */
> dev_kfree_skb_irq(tp->tx_buffers[entry].skb);
> @@ -796,6 +833,7 @@ static void tulip_down (struct net_device *dev)
> static void tulip_free_ring (struct net_device *dev)
> {
> struct tulip_private *tp = netdev_priv(dev);
> + struct device *d = &tp->pdev->dev;
> int i;
>
> /* Free all the skbuffs in the Rx queue. */
> @@ -811,8 +849,7 @@ static void tulip_free_ring (struct net_device *dev)
> /* An invalid address. */
> tp->rx_ring[i].buffer1 = cpu_to_le32(0xBADF00D0);
> if (skb) {
> - pci_unmap_single(tp->pdev, mapping, PKT_BUF_SZ,
> - PCI_DMA_FROMDEVICE);
> + dma_unmap_single(d, mapping, PKT_BUF_SZ, DMA_FROM_DEVICE);
> dev_kfree_skb (skb);
> }
> }
> @@ -821,8 +858,8 @@ static void tulip_free_ring (struct net_device *dev)
> struct sk_buff *skb = tp->tx_buffers[i].skb;
>
> if (skb != NULL) {
> - pci_unmap_single(tp->pdev, tp->tx_buffers[i].mapping,
> - skb->len, PCI_DMA_TODEVICE);
> + dma_unmap_single(d, tp->tx_buffers[i].mapping, skb->len,
> + DMA_TO_DEVICE);
> dev_kfree_skb (skb);
> }
> tp->tx_buffers[i].skb = NULL;
> @@ -1143,7 +1180,9 @@ static void set_rx_mode(struct net_device *dev)
> if (tp->cur_tx - tp->dirty_tx > TX_RING_SIZE - 2) {
> /* Same setup recently queued, we need not add it. */
> } else {
> + struct device *d = &tp->pdev->dev;
> unsigned int entry;
> + dma_addr_t mapping;
> int dummy = -1;
>
> /* Now add this frame to the Tx list. */
> @@ -1164,16 +1203,18 @@ static void set_rx_mode(struct net_device *dev)
> }
>
> tp->tx_buffers[entry].skb = NULL;
> - tp->tx_buffers[entry].mapping =
> - pci_map_single(tp->pdev, tp->setup_frame,
> - sizeof(tp->setup_frame),
> - PCI_DMA_TODEVICE);
> + mapping = dma_map_single(d, tp->setup_frame,
> + sizeof(tp->setup_frame),
> + DMA_TO_DEVICE);
> + if (unlikely(dma_mapping_error(d, mapping))) {
> + // WTF ?
I see three choices (maybe there are more):
1) change ndo_set_rx_mode to return an int (error code)
2) WARN_ON() and return.
3) panic (which is linux-2.4 original behavior)
> + }
> + tp->tx_buffers[entry].mapping = mapping;
> /* Put the setup frame on the Tx list. */
> if (entry == TX_RING_SIZE-1)
> tx_flags |= DESC_RING_WRAP; /* Wrap ring. */
> tp->tx_ring[entry].length = cpu_to_le32(tx_flags);
> - tp->tx_ring[entry].buffer1 =
> - cpu_to_le32(tp->tx_buffers[entry].mapping);
> + tp->tx_ring[entry].buffer1 = cpu_to_le32(mapping);
> tp->tx_ring[entry].status = cpu_to_le32(DescOwned);
> if (dummy >= 0)
> tp->tx_ring[dummy].status = cpu_to_le32(DescOwned);
> @@ -1445,10 +1486,10 @@ static int tulip_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> tp = netdev_priv(dev);
> tp->dev = dev;
>
> - tp->rx_ring = pci_alloc_consistent(pdev,
> - sizeof(struct tulip_rx_desc) * RX_RING_SIZE +
> - sizeof(struct tulip_tx_desc) * TX_RING_SIZE,
> - &tp->rx_ring_dma);
> + tp->rx_ring = dma_alloc_coherent(&pdev->dev,
> + sizeof(struct tulip_rx_desc) * RX_RING_SIZE +
> + sizeof(struct tulip_tx_desc) * TX_RING_SIZE,
> + &tp->rx_ring_dma, GFP_KERNEL);
> if (!tp->rx_ring)
> goto err_out_mtable;
> tp->tx_ring = (struct tulip_tx_desc *)(tp->rx_ring + RX_RING_SIZE);
> @@ -1783,10 +1824,10 @@ static int tulip_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> return 0;
>
> err_out_free_ring:
> - pci_free_consistent (pdev,
> - sizeof (struct tulip_rx_desc) * RX_RING_SIZE +
> - sizeof (struct tulip_tx_desc) * TX_RING_SIZE,
> - tp->rx_ring, tp->rx_ring_dma);
> + dma_free_coherent(&pdev->dev,
> + sizeof(struct tulip_rx_desc) * RX_RING_SIZE +
> + sizeof(struct tulip_tx_desc) * TX_RING_SIZE,
> + tp->rx_ring, tp->rx_ring_dma);
>
> err_out_mtable:
> kfree (tp->mtable);
> @@ -1874,8 +1915,8 @@ static int tulip_resume(struct pci_dev *pdev)
> struct net_device *dev = pci_get_drvdata(pdev);
> struct tulip_private *tp = netdev_priv(dev);
> void __iomem *ioaddr = tp->base_addr;
> - int retval;
> unsigned int tmp;
> + int retval;
>
> if (!dev)
> return -EINVAL;
> @@ -1913,9 +1954,9 @@ static int tulip_resume(struct pci_dev *pdev)
> netif_device_attach(dev);
>
> if (netif_running(dev))
> - tulip_up(dev);
> + retval = tulip_up(dev);
>
> - return 0;
> + return retval;
> }
>
> #endif /* CONFIG_PM */
> @@ -1924,17 +1965,13 @@ static int tulip_resume(struct pci_dev *pdev)
> static void tulip_remove_one(struct pci_dev *pdev)
> {
> struct net_device *dev = pci_get_drvdata (pdev);
> - struct tulip_private *tp;
> -
> - if (!dev)
> - return;
> + struct tulip_private *tp = netdev_priv(dev);
>
> - tp = netdev_priv(dev);
> unregister_netdev(dev);
> - pci_free_consistent (pdev,
> - sizeof (struct tulip_rx_desc) * RX_RING_SIZE +
> - sizeof (struct tulip_tx_desc) * TX_RING_SIZE,
> - tp->rx_ring, tp->rx_ring_dma);
> + dma_free_coherent(&pdev->dev,
> + sizeof(struct tulip_rx_desc) * RX_RING_SIZE +
> + sizeof(struct tulip_tx_desc) * TX_RING_SIZE,
> + tp->rx_ring, tp->rx_ring_dma);
> kfree (tp->mtable);
> pci_iounmap(pdev, tp->base_addr);
> free_netdev (dev);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: tulip: turn compile-time warning into dev_warn()
2015-11-19 10:42 [PATCH] net: tulip: turn compile-time warning into dev_warn() Arnd Bergmann
2015-11-19 12:26 ` Will Deacon
@ 2015-11-20 16:03 ` David Miller
1 sibling, 0 replies; 8+ messages in thread
From: David Miller @ 2015-11-20 16:03 UTC (permalink / raw)
To: arnd; +Cc: netdev, will.deacon, ard.biesheuvel, grundler
From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 19 Nov 2015 11:42:26 +0100
> The tulip driver causes annoying build-time warnings for allmodconfig
> builds for all recent architectures:
>
> dec/tulip/winbond-840.c:910:2: warning: #warning Processor architecture undefined
> dec/tulip/tulip_core.c:101:2: warning: #warning Processor architecture undefined!
>
> This is the last remaining warning for arm64, and I'd like to get rid of
> it. We don't really know the cache line size, architecturally it would
> be at least 16 bytes, but all implementations I found have 64 or 128
> bytes. Configuring tulip for 32-byte lines as we do on ARM32 seems to
> be the safe but slow default, and nobody who cares about performance these
> days would use a tulip chip anyway, so we can just use that.
>
> To save the next person the job of trying to find out what this is for
> and picking a default for their architecture just to kill off the warning,
> I'm now removing the preprocessor #warning and turning it into a pr_warn
> or dev_warn that prints the equivalent information when the driver gets
> loaded.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Seems reasonable, applied, thanks Arnd!
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-11-20 16:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-19 10:42 [PATCH] net: tulip: turn compile-time warning into dev_warn() Arnd Bergmann
2015-11-19 12:26 ` Will Deacon
2015-11-19 20:29 ` Florian Fainelli
2015-11-19 21:57 ` Grant Grundler
2015-11-19 23:50 ` Francois Romieu
2015-11-20 1:41 ` Grant Grundler
2015-11-19 21:37 ` Grant Grundler
2015-11-20 16:03 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.