Re: [PATCH] net: tulip: turn compile-time warning into dev_warn()

2015-11-20 Thread David Miller
From: Arnd Bergmann 
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 

Seems reasonable, applied, thanks Arnd!
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] net: tulip: turn compile-time warning into dev_warn()

2015-11-19 Thread Grant Grundler
On Thu, Nov 19, 2015 at 3:50 PM, Francois Romieu  wrote:
> Grant Grundler  :
> [...]
>> 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(>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 = >setup_frame[15*6];
> +   struct device *d = >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(>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:

Re: [PATCH] net: tulip: turn compile-time warning into dev_warn()

2015-11-19 Thread Francois Romieu
Grant Grundler  :
[...]
> 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(>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 = >setup_frame[15*6];
+   struct device *d = >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(>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 = >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 = 

Re: [PATCH] net: tulip: turn compile-time warning into dev_warn()

2015-11-19 Thread Florian Fainelli
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 
>>
>> 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 = 0x01A0 | 0x4800;
>>  #elif defined(__mips__)
>>  static int csr0 = 0x0020 | 0x4000;
>>  #else
>> -#warning Processor architecture undefined!
>> -static int csr0 = 0x00A0 | 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 = 0x00A0 | 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, "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 
> 
> /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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] net: tulip: turn compile-time warning into dev_warn()

2015-11-19 Thread Grant Grundler
On Thu, Nov 19, 2015 at 4:26 AM, 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 
>>
>> 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 = 0x01A0 | 0x4800;
>>  #elif defined(__mips__)
>>  static int csr0 = 0x0020 | 0x4000;
>>  #else
>> -#warning Processor architecture undefined!
>> -static int csr0 = 0x00A0 | 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 = 0x00A0 | 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, "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 

yeah - same here:
Acked-by: Grant Grundler 

cheers,
grant

>
> /me waits for on-soc tulip integration.
>
> Will
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] net: tulip: turn compile-time warning into dev_warn()

2015-11-19 Thread Grant Grundler
On Thu, Nov 19, 2015 at 12:29 PM, Florian Fainelli  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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] net: tulip: turn compile-time warning into dev_warn()

2015-11-19 Thread Will Deacon
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 
> 
> 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 = 0x01A0 | 0x4800;
>  #elif defined(__mips__)
>  static int csr0 = 0x0020 | 0x4000;
>  #else
> -#warning Processor architecture undefined!
> -static int csr0 = 0x00A0 | 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 = 0x00A0 | 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, "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 

/me waits for on-soc tulip integration.

Will
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] net: tulip: turn compile-time warning into dev_warn()

2015-11-19 Thread Arnd Bergmann
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 

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 = 0x01A0 | 0x4800;
 #elif defined(__mips__)
 static int csr0 = 0x0020 | 0x4000;
 #else
-#warning Processor architecture undefined!
-static int csr0 = 0x00A0 | 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 = 0x00A0 | 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, "unknown CPU architecture, using default csr0 
setting\n");
i |= 0x4800;
 #endif
iowrite32(i, ioaddr + PCIBusCfg);

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html