* Re: e1000 driver and samba
From: James Chapman @ 2007-09-15 17:44 UTC (permalink / raw)
To: Kok, Auke, L F; +Cc: netdev
In-Reply-To: <46EAF644.1040006@intel.com>
Kok, Auke wrote:
> L F wrote:
>> On 9/14/07, Kok, Auke <auke-jan.h.kok@intel.com> wrote:
>>> this slowness might have been masking the issue
>> That is possible. However, it worked for upwards of twelve months
>> without an error.
>>
>>> I have not yet seen other reports of this issue, and it would be
>>> interesting to
>>> see if the stack or driver is seeing errors. Please post `ethtool -S
>>> eth0` after
>>> the samba connection resets or fails.
>> If you look for it on the Realtek cards, there had been sporadic
>> issues up to late 2005. The solution posted universally was 'change
>> card'.
>>
>> I include the content of ethtool -S as requested:
>> beehive:~# ethtool -S eth4
>> NIC statistics:
>> rx_packets: 43538709
>> tx_packets: 68726231
>> rx_bytes: 34124849453
>> tx_bytes: 74817483835
>> rx_broadcast: 20891
>> tx_broadcast: 8941
>> rx_multicast: 459
>> tx_multicast: 0
>> rx_errors: 0
>> tx_errors: 0
>> tx_dropped: 0
>> multicast: 459
>> collisions: 0
>> rx_length_errors: 0
>> rx_over_errors: 0
>> rx_crc_errors: 0
>> rx_frame_errors: 0
>> rx_no_buffer_count: 0
>> rx_missed_errors: 0
>> tx_aborted_errors: 0
>> tx_carrier_errors: 0
>> tx_fifo_errors: 0
>> tx_heartbeat_errors: 0
>> tx_window_errors: 0
>> tx_abort_late_coll: 0
>> tx_deferred_ok: 486
>> tx_single_coll_ok: 0
>> tx_multi_coll_ok: 0
>> tx_timeout_count: 0
>> tx_restart_queue: 0
>> rx_long_length_errors: 0
>> rx_short_length_errors: 0
>> rx_align_errors: 0
>> tx_tcp_seg_good: 0
>> tx_tcp_seg_failed: 0
>> rx_flow_control_xon: 488
>> rx_flow_control_xoff: 488
>> tx_flow_control_xon: 0
>> tx_flow_control_xoff: 0
>> rx_long_byte_count: 34124849453
Are these long frames expected in your network? What is the MTU of the
transmitting clients? Perhaps this might explain why reads work (because
data is coming from the Linux box so the packets have smaller MTU) while
writes cause delays or packet loss because the clients are sending long
frames which are getting fragmented?
>> rx_csum_offload_good: 43449333
>> rx_csum_offload_errors: 0
>> rx_header_split: 0
>> alloc_rx_buff_failed: 0
>> tx_smbus: 0
>> rx_smbus: 0
>> dropped_smbus: 0
>>
>> I am no expert, but I do not see anything that obviously points to an
>> issue there.
>> Now, something I did not mention before, though it was clearly evident
>> from context, is that the errors ONLY occur on samba WRITE. I can read
>> hundreds of GBs of data without error.
>
> can you describe your setup a bit more in detail? you're writing from a
> linux client to a windows smb server? or even to a linux server? which
> end sees the connection drop? the samba server? the samba linux client?
>
> Auke
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development
^ permalink raw reply
* Re: Distributed storage. Move away from char device ioctls.
From: Andreas Dilger @ 2007-09-15 17:51 UTC (permalink / raw)
To: Robin Humble
Cc: Jeff Garzik, Evgeniy Polyakov, netdev, linux-kernel,
linux-fsdevel
In-Reply-To: <20070915162056.GA31576@lemming.cita.utoronto.ca>
On Sep 15, 2007 12:20 -0400, Robin Humble wrote:
> On Sat, Sep 15, 2007 at 10:35:16AM -0400, Jeff Garzik wrote:
> >Lustre is tilted far too much towards high-priced storage,
>
> many (most?) Lustre deployments are with SATA and md raid5 and GigE -
> can't get much cheaper than that.
I have to agree - while Lustre CAN scale up to huge servers and fat pipes,
it can definitely also scale down (which is a LOT easier to do :-). I can
run a client + MDS + 5 OSTs in a single UML instance using loop devices
for testing w/o problems.
> interestingly, one of the ways to provide dual-attached storage behind
> a failover pair of lustre servers (apart from buying SAS) would be via
> a networked-raid-1 device like Evgeniy's, so I don't see distributed
> block devices and distributed filesystems as being mutually exclusive.
That is definitely true, and there are a number of users who run in
this mode. We're also working to make Lustre handle the replication
internally (RAID5/6+ at the OST level) so you wouldn't need any kind of
block-level redundancy at all. I suspect some sites may still use RAID5/6
back-ends anyways to avoid performance loss from taking out a whole OST
due to a single disk failure, but that would definitely not be required.
> >and needs improvement before it could be considered for mainline.
It's definitely true, and we are always working at improving it. It
used to be in the past that one of the reasons we DIDN'T want to go
into mainline was because this would restrict our ability to make
network protocol changes. Because our install base is large enough
and many of the large sites with mutliple supercomputers mounting
multiple global filesystems we aren't at liberty to change the network
protocol at will anymore. That said, we also have network protocol
versioning that is akin to the ext3 COMPAT/INCOMPAT feature flags, so
we are able to add/change features without breaking old clients
> from what I understand (hopefully I am mistaken) they consider a merge
> task to be too daunting as the number of kernel subsystems that any
> scalable distributed filesystem touches is necessarily large.
That's partly true - Lustre has its own RDMA RPC mechanism, but it does
not need kernel patches anymore (we removed the zero-copy callback and
do this at the protocol level because there was too much resistance to it).
We are now also able to run a client filesystem that doesn't require any
kernel patches, since we've given up on trying to get the intents and
raw operations into the VFS, and have worked out other ways to improve
the performance to compensate. Likewise with parallel directory operations.
It's a bit sad, in a way, because these are features that other filesystems
(especially network fs) could have benefitted from also.
> roadmaps indicate that parts of lustre are likely to move to userspace
> (partly to ease solaris and ZFS ports) so perhaps those performance
> critical parts that remain kernel space will be easier to merge.
This is also true - when that is done the only parts that will remain
in the kernel are the network drivers. With some network stacks there
is even direct userspace acceleration. We'll use RDMA and direct IO to
avoid doing any user<->kernel data copies.
Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.
^ permalink raw reply
* Re: [PATCH] IPV4 : convert rt_check_expire() from softirq processing to workqueue
From: David Miller @ 2007-09-15 17:58 UTC (permalink / raw)
To: dada1; +Cc: netdev
In-Reply-To: <20070912153444.bb564a6f.dada1@cosmosbay.com>
From: Eric Dumazet <dada1@cosmosbay.com>
Date: Wed, 12 Sep 2007 15:34:44 +0200
> On loaded/big hosts, rt_check_expire() if of litle use, because it
> generally breaks out of its main loop because of a jiffies change.
>
> It can take a long time (read : timer invocations) to actually
> scan the whole hash table, freeing unused entries.
>
> Converting it to use a workqueue instead of softirq is a nice
> move because we can allow rt_check_expire() to do the scan
> it is supposed to do, without hogging the CPU.
>
> This has an impact on the average number of entries in cache,
> reducing ram usage. Cache is more responsive to parameter
> changes (/proc/sys/net/ipv4/route/gc_timeout and
> /proc/sys/net/ipv4/route/gc_interval)
>
> Note: Maybe the default value of gc_interval (60 seconds)
> is too high, since this means we actually need 5 (300/60)
> invocations to scan the whole table.
>
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
Applied, thanks Eric. I'm rebasing my net-2.6.24 tree over the
weekend so it may take a little time for this to show up.
It just occurred to me that we need to fix something in these two
workqueue conversions. If it runs for more than a jiffy this can
unfairly penalize other tasks that want to run on that cpu.
Therefore, we need to add a need_resched() or similar check in the
loop.
^ permalink raw reply
* Re: Please pull 'adm8211' branch of wireless-2.6
From: David Miller @ 2007-09-15 18:25 UTC (permalink / raw)
To: linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: jeff-o2qLIJkoznsdnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20070915170018.GC18930-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Date: Sat, 15 Sep 2007 13:00:18 -0400
> Come to think of it, this driver will depend on some of the mac80211
> patches Dave M. has queued for net-2.6.24. Perhaps it would be better
> if Dave were to merge it with his tree?
>
> Jeff, if you have no objection then please sign-off/ack/whatever so
> davem can see it. Dave, in that case please pull this to net-2.6.24.
There are a lot of things to sort out :-)
What I'm going to try and accomplish right now is:
1) Rebase my tree, I just finished that.
2) Integrate linville's upstream-davem stuff into net-2.6.24
3) Integrate Jeff's upstream branch into net-2.6.24
4) Integrate linville's adm8211 driver into net-2.6.24
We'll see how well that goes.
^ permalink raw reply
* Re: e1000 driver and samba
From: Kok, Auke @ 2007-09-15 19:07 UTC (permalink / raw)
To: James Chapman, L F; +Cc: Kok, Auke, netdev
In-Reply-To: <46EC1A00.2000304@katalix.com>
James Chapman wrote:
> Kok, Auke wrote:
>> L F wrote:
>>> On 9/14/07, Kok, Auke <auke-jan.h.kok@intel.com> wrote:
>>>> this slowness might have been masking the issue
>>> That is possible. However, it worked for upwards of twelve months
>>> without an error.
>>>
>>>> I have not yet seen other reports of this issue, and it would be
>>>> interesting to
>>>> see if the stack or driver is seeing errors. Please post `ethtool -S
>>>> eth0` after
>>>> the samba connection resets or fails.
>>> If you look for it on the Realtek cards, there had been sporadic
>>> issues up to late 2005. The solution posted universally was 'change
>>> card'.
>>>
>>> I include the content of ethtool -S as requested:
>>> beehive:~# ethtool -S eth4
>>> NIC statistics:
>>> rx_packets: 43538709
>>> tx_packets: 68726231
>>> rx_bytes: 34124849453
>>> tx_bytes: 74817483835
>>> rx_broadcast: 20891
>>> tx_broadcast: 8941
>>> rx_multicast: 459
>>> tx_multicast: 0
>>> rx_errors: 0
>>> tx_errors: 0
>>> tx_dropped: 0
>>> multicast: 459
>>> collisions: 0
>>> rx_length_errors: 0
>>> rx_over_errors: 0
>>> rx_crc_errors: 0
>>> rx_frame_errors: 0
>>> rx_no_buffer_count: 0
>>> rx_missed_errors: 0
>>> tx_aborted_errors: 0
>>> tx_carrier_errors: 0
>>> tx_fifo_errors: 0
>>> tx_heartbeat_errors: 0
>>> tx_window_errors: 0
>>> tx_abort_late_coll: 0
>>> tx_deferred_ok: 486
this one I wonder about, and might cause delays, I'll have to look up what it
exactly could implicate though.
>>> tx_single_coll_ok: 0
>>> tx_multi_coll_ok: 0
>>> tx_timeout_count: 0
>>> tx_restart_queue: 0
>>> rx_long_length_errors: 0
>>> rx_short_length_errors: 0
>>> rx_align_errors: 0
>>> tx_tcp_seg_good: 0
>>> tx_tcp_seg_failed: 0
>>> rx_flow_control_xon: 488
>>> rx_flow_control_xoff: 488
>>> tx_flow_control_xon: 0
>>> tx_flow_control_xoff: 0
>>> rx_long_byte_count: 34124849453
>
> Are these long frames expected in your network? What is the MTU of the
> transmitting clients? Perhaps this might explain why reads work (because
> data is coming from the Linux box so the packets have smaller MTU) while
> writes cause delays or packet loss because the clients are sending long
> frames which are getting fragmented?
those are not "long frames" but the number of bytes the hardware counted in its
"long" data type based byte counter.
Auke
^ permalink raw reply
* Re: ne driver crashes when unloaded in 2.6.22.6
From: Chris Rankin @ 2007-09-15 20:27 UTC (permalink / raw)
To: Dan Williams; +Cc: netdev
In-Reply-To: <1189656163.30762.3.camel@xo-3E-67-34.localdomain>
--- Dan Williams <dcbw@redhat.com> wrote:
> On Wed, 2007-09-12 at 19:23 +0100, Chris Rankin wrote:
> > Hmm, apparently not. The light on the card goes out though, so could this just be a lack of
> driver
> > support?
>
> Likely, yes.
I've been trawling the Internet for 8390 specifications and have discovered that there is a
"Carrier Sense Loss" flag on the "Transmit Status Register". However, there doesn't seem to be an
explicit "media status" test. Would this more likely be part of the NE2000's functionality? I
can't find any signs of MII support, but then the NE2000 is so heavily cloned that
"NE2000-compatible" seems to have become more of a generic description these days.
Does anyone have any ideas, please? Does NetworkManager even need full carrier-detection support?
Cheers,
Chris
___________________________________________________________
Want ideas for reducing your carbon footprint? Visit Yahoo! For Good http://uk.promotions.yahoo.com/forgood/environment.html
^ permalink raw reply
* Re: Please pull 'adm8211' branch of wireless-2.6
From: Jeff Garzik @ 2007-09-15 21:32 UTC (permalink / raw)
To: John W. Linville
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, David Miller
In-Reply-To: <20070915132220.GE6060-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Review summary: many minor issues, only one major one: irq handler loop
John W. Linville wrote:
> +static unsigned int tx_ring_size __read_mostly = 16;
> +static unsigned int rx_ring_size __read_mostly = 16;
> +
> +module_param(tx_ring_size, uint, 0);
> +module_param(rx_ring_size, uint, 0);
should be in ethtool (or another per-interface runtime config tool), not
a module parameter.
> +static void adm8211_set_rx_mode(struct ieee80211_hw *dev,
> + unsigned short flags, int mc_count)
> +{
> + struct adm8211_priv *priv = dev->priv;
> + unsigned int bit_nr;
> + u32 mc_filter[2];
> + struct dev_mc_list *mclist;
> + void *tmp;
> +
> + if (flags & IFF_PROMISC) {
> + priv->nar |= ADM8211_NAR_PR;
> + priv->nar &= ~ADM8211_NAR_MM;
> + mc_filter[1] = mc_filter[0] = ~0;
> + } else if ((flags & IFF_ALLMULTI) || (mc_count > -1)) {
mc_count > -1 ?
what kind of bogus placeholder is that?
> + priv->nar &= ~ADM8211_NAR_PR;
> + priv->nar |= ADM8211_NAR_MM;
> + mc_filter[1] = mc_filter[0] = ~0;
> + } else {
> + priv->nar &= ~(ADM8211_NAR_MM | ADM8211_NAR_PR);
> + mc_filter[1] = mc_filter[0] = 0;
> + mclist = NULL;
> + while ((mclist = ieee80211_get_mc_list_item(dev, mclist, &tmp))) {
> + bit_nr = ether_crc(ETH_ALEN, mclist->dmi_addr) >> 26;
> +
> + bit_nr &= 0x3F;
> + mc_filter[bit_nr >> 5] |= 1 << (bit_nr & 31);
> + }
> + }
> +
> + ADM8211_IDLE_RX();
> +
> + ADM8211_CSR_WRITE(MAR0, mc_filter[0]);
> + ADM8211_CSR_WRITE(MAR1, mc_filter[1]);
> + ADM8211_CSR_READ(NAR);
> +
> + if (flags & IFF_PROMISC)
> + dev->flags |= IEEE80211_HW_RX_INCLUDES_FCS;
> + else
> + dev->flags &= ~IEEE80211_HW_RX_INCLUDES_FCS;
why does promisc dictate inclusion of FCS?
> +static void adm8211_interrupt_tci(struct ieee80211_hw *dev)
> +{
> + struct adm8211_priv *priv = dev->priv;
> + unsigned int dirty_tx;
> +
> + spin_lock(&priv->lock);
> +
> + for (dirty_tx = priv->dirty_tx; priv->cur_tx - dirty_tx; dirty_tx++) {
> + unsigned int entry = dirty_tx % priv->tx_ring_size;
> + u32 status = le32_to_cpu(priv->tx_ring[entry].status);
> + struct adm8211_tx_ring_info *info;
> + struct sk_buff *skb;
> +
> + if (status & TDES0_CONTROL_OWN ||
> + !(status & TDES0_CONTROL_DONE))
> + break;
> +
> + info = &priv->tx_buffers[entry];
> + skb = info->skb;
> +
> + /* TODO: check TDES0_STATUS_TUF and TDES0_STATUS_TRO */
> +
> + pci_unmap_single(priv->pdev, info->mapping,
> + info->skb->len, PCI_DMA_TODEVICE);
> +
> + if (info->tx_control.flags & IEEE80211_TXCTL_REQ_TX_STATUS) {
> + struct ieee80211_tx_status tx_status = {{0}};
> + struct ieee80211_hdr *hdr;
> + size_t hdrlen = info->hdrlen;
> +
> + skb_pull(skb, sizeof(struct adm8211_tx_hdr));
> + hdr = (struct ieee80211_hdr *)skb_push(skb, hdrlen);
> + memcpy(hdr, skb->cb, hdrlen);
> + memcpy(&tx_status.control, &info->tx_control,
> + sizeof(tx_status.control));
> + if (!(status & TDES0_STATUS_ES))
> + tx_status.flags |= IEEE80211_TX_STATUS_ACK;
> + ieee80211_tx_status_irqsafe(dev, skb, &tx_status);
> + } else
> + dev_kfree_skb_irq(skb);
> + info->skb = NULL;
> + }
> +
> + if (priv->cur_tx - dirty_tx < priv->tx_ring_size - 2)
> + ieee80211_wake_queue(dev, 0);
not this driver's fault, but, ieee80211_wake_queue() warrants revisiting
now that MQ stuff is in
when queue==0 is hardcoded, you can just use netif_wake/stop_queue() AFAICS
> +static void adm8211_interrupt_rci(struct ieee80211_hw *dev)
> +{
> + struct adm8211_priv *priv = dev->priv;
> + unsigned int entry = priv->cur_rx % priv->rx_ring_size;
> + u32 status;
> + unsigned int pktlen;
> + struct sk_buff *skb, *newskb;
> + unsigned int limit = priv->rx_ring_size;
> + static const u8 rate_tbl[] = {10, 20, 55, 110, 220};
> + u8 rssi, rate;
> +
> + while (!(priv->rx_ring[entry].status & cpu_to_le32(RDES0_STATUS_OWN))) {
> + if (!limit--)
> + break;
> +
> + status = le32_to_cpu(priv->rx_ring[entry].status);
> + rate = (status & RDES0_STATUS_RXDR) >> 12;
> + rssi = le32_to_cpu(priv->rx_ring[entry].length) &
> + RDES1_STATUS_RSSI;
> +
> + pktlen = status & RDES0_STATUS_FL;
> + if (pktlen > RX_PKT_SIZE) {
> + if (net_ratelimit())
> + printk(KERN_DEBUG "%s: frame too long (%d)\n",
> + wiphy_name(dev->wiphy), pktlen);
> + pktlen = RX_PKT_SIZE;
> + }
> +
> + if (!priv->soft_rx_crc && status & RDES0_STATUS_ES) {
> + skb = NULL; /* old buffer will be reused */
> + /* TODO: update RX error stats */
> + /* TODO: check RDES0_STATUS_CRC*E */
> + } else if (pktlen < RX_COPY_BREAK) {
> + skb = dev_alloc_skb(pktlen);
> + if (skb) {
> + pci_dma_sync_single_for_cpu(
> + priv->pdev,
> + priv->rx_buffers[entry].mapping,
> + pktlen, PCI_DMA_FROMDEVICE);
> + memcpy(skb_put(skb, pktlen),
> + skb_tail_pointer(priv->rx_buffers[entry].skb),
> + pktlen);
> + pci_dma_sync_single_for_device(
> + priv->pdev,
> + priv->rx_buffers[entry].mapping,
> + RX_PKT_SIZE, PCI_DMA_FROMDEVICE);
> + }
> + } else {
> + newskb = dev_alloc_skb(RX_PKT_SIZE);
> + if (newskb) {
> + skb = priv->rx_buffers[entry].skb;
> + skb_put(skb, pktlen);
> + pci_unmap_single(
> + priv->pdev,
> + priv->rx_buffers[entry].mapping,
> + RX_PKT_SIZE, PCI_DMA_FROMDEVICE);
> + priv->rx_buffers[entry].skb = newskb;
> + priv->rx_buffers[entry].mapping =
> + pci_map_single(priv->pdev,
> + skb_tail_pointer(newskb),
> + RX_PKT_SIZE,
> + PCI_DMA_FROMDEVICE);
> + } else {
> + skb = NULL;
> + /* TODO: update rx dropped stats */
> + }
> +
> + priv->rx_ring[entry].buffer1 =
> + cpu_to_le32(priv->rx_buffers[entry].mapping);
> + }
> +
> + priv->rx_ring[entry].status = cpu_to_le32(RDES0_STATUS_OWN |
> + RDES0_STATUS_SQL);
> + priv->rx_ring[entry].length =
> + cpu_to_le32(RX_PKT_SIZE |
> + (entry == priv->rx_ring_size - 1 ?
> + RDES1_CONTROL_RER : 0));
> +
> + if (skb) {
> + struct ieee80211_rx_status rx_status = {0};
> +
> + if (priv->revid < ADM8211_REV_CA)
> + rx_status.ssi = rssi;
> + else
> + rx_status.ssi = 100 - rssi;
> +
> + if (rate <= 4)
> + rx_status.rate = rate_tbl[rate];
> +
> + rx_status.channel = priv->channel;
> + rx_status.freq = adm8211_channels[priv->channel - 1].freq;
> + rx_status.phymode = MODE_IEEE80211B;
> +
> + ieee80211_rx_irqsafe(dev, skb, &rx_status);
> + }
> +
> + entry = (++priv->cur_rx) % priv->rx_ring_size;
the '%' operator is expensive. mask combined with power-of-2 is better
> +static irqreturn_t adm8211_interrupt(int irq, void *dev_id)
> +{
> +#define ADM8211_INT(x) \
> +do { \
> + if (unlikely(stsr & ADM8211_STSR_ ## x)) \
> + printk(KERN_DEBUG "%s: " #x "\n", wiphy_name(dev->wiphy)); \
> +} while (0)
> +
> + struct ieee80211_hw *dev = dev_id;
> + struct adm8211_priv *priv = dev->priv;
> + unsigned int count = 0;
> + u32 stsr;
> +
> + do {
> + stsr = ADM8211_CSR_READ(STSR);
> + ADM8211_CSR_WRITE(STSR, stsr);
> + if (stsr == 0xffffffff)
> + return IRQ_HANDLED;
> +
> + if (!(stsr & (ADM8211_STSR_NISS | ADM8211_STSR_AISS)))
> + break;
> +
> + if (stsr & ADM8211_STSR_RCI)
> + adm8211_interrupt_rci(dev);
> + if (stsr & ADM8211_STSR_TCI)
> + adm8211_interrupt_tci(dev);
> +
> + /*ADM8211_INT(LinkOn);*/
> + /*ADM8211_INT(LinkOff);*/
> +
> + ADM8211_INT(PCF);
> + ADM8211_INT(BCNTC);
> + ADM8211_INT(GPINT);
> + ADM8211_INT(ATIMTC);
> + ADM8211_INT(TSFTF);
> + ADM8211_INT(TSCZ);
> + ADM8211_INT(SQL);
> + ADM8211_INT(WEPTD);
> + ADM8211_INT(ATIME);
> + /*ADM8211_INT(TBTT);*/
> + ADM8211_INT(TEIS);
> + ADM8211_INT(FBE);
> + ADM8211_INT(REIS);
> + ADM8211_INT(GPTT);
> + ADM8211_INT(RPS);
> + ADM8211_INT(RDU);
> + ADM8211_INT(TUF);
> + /*ADM8211_INT(TRT);*/
> + /*ADM8211_INT(TLT);*/
> + /*ADM8211_INT(TDU);*/
> + ADM8211_INT(TPS);
lame. WAY too many branches, even if marked with unlikely()
this should be surrounded by a single "if stsr &
BITS_WE_SHOULD_NEVER_SEE" test
> + } while (count++ < 20);
NAK -- talk about back to the past.
It's bogus to loop in the interrupt handler, then loop again in both RX
and TX paths. You are getting close to reinventing the wheel here.
Use NAPI rather than doing such a loop.
> +#define WRITE_SYN(name,v_mask,v_shift,a_mask,a_shift,bits,prewrite,postwrite)\
> +static void adm8211_rf_write_syn_ ## name (struct ieee80211_hw *dev, \
> + u16 addr, u32 value) { \
> + struct adm8211_priv *priv = dev->priv; \
> + unsigned int i; \
> + u32 reg, bitbuf; \
> + \
> + value &= v_mask; \
> + addr &= a_mask; \
> + bitbuf = (value << v_shift) | (addr << a_shift); \
> + \
> + ADM8211_CSR_WRITE(SYNRF, ADM8211_SYNRF_IF_SELECT_1); \
> + ADM8211_CSR_READ(SYNRF); \
> + ADM8211_CSR_WRITE(SYNRF, ADM8211_SYNRF_IF_SELECT_0); \
> + ADM8211_CSR_READ(SYNRF); \
> + \
> + if (prewrite) { \
> + ADM8211_CSR_WRITE(SYNRF, ADM8211_SYNRF_WRITE_SYNDATA_0); \
> + ADM8211_CSR_READ(SYNRF); \
> + } \
> + \
> + for (i = 0; i <= bits; i++) { \
> + if (bitbuf & (1 << (bits - i))) \
> + reg = ADM8211_SYNRF_WRITE_SYNDATA_1; \
> + else \
> + reg = ADM8211_SYNRF_WRITE_SYNDATA_0; \
> + \
> + ADM8211_CSR_WRITE(SYNRF, reg); \
> + ADM8211_CSR_READ(SYNRF); \
> + \
> + ADM8211_CSR_WRITE(SYNRF, reg | ADM8211_SYNRF_WRITE_CLOCK_1); \
> + ADM8211_CSR_READ(SYNRF); \
> + ADM8211_CSR_WRITE(SYNRF, reg | ADM8211_SYNRF_WRITE_CLOCK_0); \
> + ADM8211_CSR_READ(SYNRF); \
> + } \
> + \
> + if (postwrite == 1) { \
> + ADM8211_CSR_WRITE(SYNRF, reg | ADM8211_SYNRF_IF_SELECT_0); \
> + ADM8211_CSR_READ(SYNRF); \
> + } \
> + if (postwrite == 2) { \
> + ADM8211_CSR_WRITE(SYNRF, reg | ADM8211_SYNRF_IF_SELECT_1); \
> + ADM8211_CSR_READ(SYNRF); \
> + } \
> + \
> + ADM8211_CSR_WRITE(SYNRF, 0); \
> + ADM8211_CSR_READ(SYNRF); \
> +}
> +
> +WRITE_SYN(max2820, 0x00FFF, 0, 0x0F, 12, 15, 1, 1)
> +WRITE_SYN(al2210l, 0xFFFFF, 4, 0x0F, 0, 23, 1, 1)
> +WRITE_SYN(rfmd2958, 0x3FFFF, 0, 0x1F, 18, 23, 0, 1)
> +WRITE_SYN(rfmd2948, 0x0FFFF, 4, 0x0F, 0, 21, 0, 2)
code bloat from hell. just pass these arguments (or a pointer to an
entry in a table that contains this info)
> +static void adm8211_hw_init(struct ieee80211_hw *dev)
> +{
> + struct adm8211_priv *priv = dev->priv;
> + u32 reg;
> + u8 cline;
> +
> + reg = le32_to_cpu(ADM8211_CSR_READ(PAR));
> + reg |= ADM8211_PAR_MRLE | ADM8211_PAR_MRME;
> + reg &= ~(ADM8211_PAR_BAR | ADM8211_PAR_CAL);
what do BAR and CAL bits do?
> + if (!pci_set_mwi(priv->pdev)) {
> + reg |= 0x1 << 24;
> + pci_read_config_byte(priv->pdev, PCI_CACHE_LINE_SIZE, &cline);
> +
> + switch (cline) {
> + case 0x8: reg |= (0x1 << 14);
> + break;
> + case 0x16: reg |= (0x2 << 14);
> + break;
> + case 0x32: reg |= (0x3 << 14);
> + break;
> + default: reg |= (0x0 << 14);
> + break;
> + }
> + }
> +
> + ADM8211_CSR_WRITE(PAR, reg);
this should be set regardless of MWI usage AFAICS
> + reg = ADM8211_CSR_READ(CSR_TEST1);
> + reg &= ~(0xF << 28);
> + reg |= (1 << 28) | (1 << 31);
> + ADM8211_CSR_WRITE(CSR_TEST1, reg);
> +
> + /* lose link after 4 lost beacons */
> + reg = (0x04 << 21) | ADM8211_WCSR_TSFTWE | ADM8211_WCSR_LSOE;
> + ADM8211_CSR_WRITE(WCSR, reg);
> +
> + /* Disable APM, enable receive FIFO threshold, and set drain receive
> + * threshold to store-and-forward */
> + reg = ADM8211_CSR_READ(CMDR);
> + reg &= ~(ADM8211_CMDR_APM | ADM8211_CMDR_DRT);
> + reg |= ADM8211_CMDR_RTE | ADM8211_CMDR_DRT_SF;
> + ADM8211_CSR_WRITE(CMDR, reg);
> +
> + adm8211_set_rate(dev);
> +
> + /* 4-bit values:
> + * PWR1UP = 8 * 2 ms
> + * PWR0PAPE = 8 us or 5 us
> + * PWR1PAPE = 1 us or 3 us
> + * PWR0TRSW = 5 us
> + * PWR1TRSW = 12 us
> + * PWR0PE2 = 13 us
> + * PWR1PE2 = 1 us
> + * PWR0TXPE = 8 or 6 */
> + if (priv->revid < ADM8211_REV_CA)
> + ADM8211_CSR_WRITE(TOFS2, 0x8815cd18);
> + else
> + ADM8211_CSR_WRITE(TOFS2, 0x8535cd16);
> +
> + /* Enable store and forward for transmit */
> + priv->nar = ADM8211_NAR_SF | ADM8211_NAR_PB;
> + ADM8211_CSR_WRITE(NAR, priv->nar);
> +
> + /* Reset RF */
> + ADM8211_CSR_WRITE(SYNRF, ADM8211_SYNRF_RADIO);
> + ADM8211_CSR_READ(SYNRF);
> + msleep(10);
> + ADM8211_CSR_WRITE(SYNRF, 0);
> + ADM8211_CSR_READ(SYNRF);
> + msleep(5);
> +
> + /* Set CFP Max Duration to 0x10 TU */
> + reg = ADM8211_CSR_READ(CFPP);
> + reg &= ~(0xffff << 8);
> + reg |= 0x0010 << 8;
> + ADM8211_CSR_WRITE(CFPP, reg);
> +
> + /* USCNT = 0x16 (number of system clocks, 22 MHz, in 1us
> + * TUCNT = 0x3ff - Tu counter 1024 us */
> + ADM8211_CSR_WRITE(TOFS0, (0x16 << 24) | 0x3ff);
> +
> + /* SLOT=20 us, SIFS=110 cycles of 22 MHz (5 us),
> + * DIFS=50 us, EIFS=100 us */
> + if (priv->revid < ADM8211_REV_CA)
> + ADM8211_CSR_WRITE(IFST, (20 << 23) | (110 << 15) |
> + (50 << 9) | 100);
> + else
> + ADM8211_CSR_WRITE(IFST, (20 << 23) | (24 << 15) |
> + (50 << 9) | 100);
> +
> + /* PCNT = 1 (MAC idle time awake/sleep, unit S)
> + * RMRD = 2346 * 8 + 1 us (max RX duration) */
> + ADM8211_CSR_WRITE(RMD, (1 << 16) | 18769);
> +
> + /* MART=65535 us, MIRT=256 us, TSFTOFST=0 us */
> + ADM8211_CSR_WRITE(RSPT, 0xffffff00);
> +
> + /* Initialize BBP (and SYN) */
> + adm8211_hw_init_bbp(dev);
> +
> + /* make sure interrupts are off */
> + ADM8211_CSR_WRITE(IER, 0);
> +
> + /* ACK interrupts */
> + ADM8211_CSR_WRITE(STSR, ADM8211_CSR_READ(STSR));
> +
> + /* Setup WEP (turns it off for now) */
> + reg = ADM8211_CSR_READ(MACTEST);
> + reg &= ~(7 << 20);
> + ADM8211_CSR_WRITE(MACTEST, reg);
> +
> + reg = ADM8211_CSR_READ(WEPCTL);
> + reg &= ~ADM8211_WEPCTL_WEPENABLE;
> + reg |= ADM8211_WEPCTL_WEPRXBYP;
> + ADM8211_CSR_WRITE(WEPCTL, reg);
> +
> + /* Clear the missed-packet counter. */
> + ADM8211_CSR_READ(LPC);
> +
> + if (!priv->mac_addr)
> + return;
> +
> + /* set mac address */
> + ADM8211_CSR_WRITE(PAR0, *(u32 *)priv->mac_addr);
> + ADM8211_CSR_WRITE(PAR1, *(u16 *)&priv->mac_addr[4]);
> +}
> +static int adm8211_init_rings(struct ieee80211_hw *dev)
> +{
> + struct adm8211_priv *priv = dev->priv;
> + struct adm8211_desc *desc = NULL;
> + struct adm8211_rx_ring_info *rx_info;
> + struct adm8211_tx_ring_info *tx_info;
> + unsigned int i;
> +
> + for (i = 0; i < priv->rx_ring_size; i++) {
> + desc = &priv->rx_ring[i];
> + desc->status = 0;
> + desc->length = cpu_to_le32(RX_PKT_SIZE);
> + priv->rx_buffers[i].skb = NULL;
> + }
> + /* Mark the end of RX ring; hw returns to base address after this
> + * descriptor */
> + desc->length |= cpu_to_le32(RDES1_CONTROL_RER);
> +
> + for (i = 0; i < priv->rx_ring_size; i++) {
> + desc = &priv->rx_ring[i];
> + rx_info = &priv->rx_buffers[i];
> +
> + rx_info->skb = dev_alloc_skb(RX_PKT_SIZE);
> + if (rx_info->skb == NULL)
> + break;
it seems highly bogus to set RX_PKT_SIZE for all RX descriptors, then
bail if allocation starts failing. the state of the ring becomes out of
sync with reality.
> +/* Transmit skb w/adm8211_tx_hdr (802.11 header created by hardware) */
> +static void adm8211_tx_raw(struct ieee80211_hw *dev, struct sk_buff *skb,
> + u16 plcp_signal,
> + struct ieee80211_tx_control *control,
> + size_t hdrlen)
> +{
> + struct adm8211_priv *priv = dev->priv;
> + unsigned long flags;
> + dma_addr_t mapping;
> + unsigned int entry;
> + u32 flag;
> +
> + mapping = pci_map_single(priv->pdev, skb->data, skb->len,
> + PCI_DMA_TODEVICE);
> +
> + spin_lock_irqsave(&priv->lock, flags);
> +
> + if (priv->cur_tx - priv->dirty_tx == priv->tx_ring_size / 2)
> + flag = TDES1_CONTROL_IC | TDES1_CONTROL_LS | TDES1_CONTROL_FS;
> + else
> + flag = TDES1_CONTROL_LS | TDES1_CONTROL_FS;
> +
> + if (priv->cur_tx - priv->dirty_tx == priv->tx_ring_size - 2)
extra parens would be nice for readability
> + ieee80211_stop_queue(dev, 0);
same as above: when queue==0 is hardcoded, things (in the stack) could
be simplified. not a comment on this driver, but a comment on the stack.
> + entry = priv->cur_tx % priv->tx_ring_size;
'%' is expensive
> + priv->tx_buffers[entry].skb = skb;
> + priv->tx_buffers[entry].mapping = mapping;
> + memcpy(&priv->tx_buffers[entry].tx_control, control, sizeof(*control));
> + priv->tx_buffers[entry].hdrlen = hdrlen;
> + priv->tx_ring[entry].buffer1 = cpu_to_le32(mapping);
> +
> + if (entry == priv->tx_ring_size - 1)
> + flag |= TDES1_CONTROL_TER;
> + priv->tx_ring[entry].length = cpu_to_le32(flag | skb->len);
> +
> + /* Set TX rate (SIGNAL field in PLCP PPDU format) */
> + flag = TDES0_CONTROL_OWN | (plcp_signal << 20) | 8 /* ? */;
> + priv->tx_ring[entry].status = cpu_to_le32(flag);
> +
> + priv->cur_tx++;
> +
> + spin_unlock_irqrestore(&priv->lock, flags);
> +
> + /* Trigger transmit poll */
> + ADM8211_CSR_WRITE(TDR, 0);
> +}
> +
> +/* Put adm8211_tx_hdr on skb and transmit */
> +static int adm8211_tx(struct ieee80211_hw *dev, struct sk_buff *skb,
> + struct ieee80211_tx_control *control)
> +{
> + struct adm8211_tx_hdr *txhdr;
> + u16 fc;
> + size_t payload_len, hdrlen;
> + int plcp, dur, len, plcp_signal, short_preamble;
> + struct ieee80211_hdr *hdr;
> +
> + if (control->tx_rate < 0) {
> + short_preamble = 1;
> + plcp_signal = -control->tx_rate;
> + } else {
> + short_preamble = 0;
> + plcp_signal = control->tx_rate;
> + }
> +
> + hdr = (struct ieee80211_hdr *)skb->data;
> + fc = le16_to_cpu(hdr->frame_control) & ~IEEE80211_FCTL_PROTECTED;
> + hdrlen = ieee80211_get_hdrlen(fc);
> + memcpy(skb->cb, skb->data, hdrlen);
to confirm: I thought skb->cb was owned by the skb creator? In this
the driver is definitely _not_ the skb creator
maybe this is just a wireless thing I need to learn.
> + hdr = (struct ieee80211_hdr *)skb->cb;
> + skb_pull(skb, hdrlen);
> + payload_len = skb->len;
> +
> + txhdr = (struct adm8211_tx_hdr *) skb_push(skb, sizeof(*txhdr));
> + memset(txhdr, 0, sizeof(*txhdr));
> + memcpy(txhdr->da, ieee80211_get_DA(hdr), ETH_ALEN);
> + txhdr->signal = plcp_signal;
> + txhdr->frame_body_size = cpu_to_le16(payload_len);
> + txhdr->frame_control = hdr->frame_control;
> +
> + len = hdrlen + payload_len + FCS_LEN;
> + if (fc & IEEE80211_FCTL_PROTECTED)
> + len += 8;
> +
> + txhdr->frag = cpu_to_le16(0x0FFF);
> + adm8211_calc_durations(&dur, &plcp, payload_len,
> + len, plcp_signal, short_preamble);
> + txhdr->plcp_frag_head_len = cpu_to_le16(plcp);
> + txhdr->plcp_frag_tail_len = cpu_to_le16(plcp);
> + txhdr->dur_frag_head = cpu_to_le16(dur);
> + txhdr->dur_frag_tail = cpu_to_le16(dur);
> +
> + txhdr->header_control = cpu_to_le16(ADM8211_TXHDRCTL_ENABLE_EXTEND_HEADER);
> +
> + if (short_preamble)
> + txhdr->header_control |= cpu_to_le16(ADM8211_TXHDRCTL_SHORT_PREAMBLE);
> +
> + if (control->flags & IEEE80211_TXCTL_USE_RTS_CTS)
> + txhdr->header_control |= cpu_to_le16(ADM8211_TXHDRCTL_ENABLE_RTS);
> +
> + if (fc & IEEE80211_FCTL_PROTECTED)
> + txhdr->header_control |= cpu_to_le16(ADM8211_TXHDRCTL_ENABLE_WEP_ENGINE);
> +
> + txhdr->retry_limit = control->retry_limit;
> +
> + adm8211_tx_raw(dev, skb, plcp_signal, control, hdrlen);
> +
> + return NETDEV_TX_OK;
> +}
> +
> +static int adm8211_alloc_rings(struct ieee80211_hw *dev)
> +{
> + struct adm8211_priv *priv = dev->priv;
> + unsigned int ring_size;
> +
> + priv->rx_buffers = kmalloc(sizeof(*priv->rx_buffers) * priv->rx_ring_size +
> + sizeof(*priv->tx_buffers) * priv->tx_ring_size, GFP_KERNEL);
> + if (!priv->rx_buffers)
> + return -ENOMEM;
> +
> + priv->tx_buffers = (void *)priv->rx_buffers +
> + sizeof(*priv->rx_buffers) * priv->rx_ring_size;
> +
> + /* Allocate TX/RX descriptors */
> + ring_size = sizeof(struct adm8211_desc) * priv->rx_ring_size +
> + sizeof(struct adm8211_desc) * priv->tx_ring_size;
> + priv->rx_ring = pci_alloc_consistent(priv->pdev, ring_size,
> + &priv->rx_ring_dma);
> +
> + if (!priv->rx_ring) {
> + kfree(priv->rx_buffers);
> + priv->rx_buffers = NULL;
> + priv->tx_buffers = NULL;
> + return -ENOMEM;
> + }
> +
> + priv->tx_ring = (struct adm8211_desc *)(priv->rx_ring +
> + priv->rx_ring_size);
> + priv->tx_ring_dma = priv->rx_ring_dma +
> + sizeof(struct adm8211_desc) * priv->rx_ring_size;
> +
why not alloc ring contents (skbs) here too?
> +static int __devinit adm8211_probe(struct pci_dev *pdev,
> + const struct pci_device_id *id)
> +{
> + struct ieee80211_hw *dev;
> + struct adm8211_priv *priv;
> + unsigned long mem_addr, mem_len;
> + unsigned int io_addr, io_len;
> + int err;
> + u32 reg;
> + u8 perm_addr[ETH_ALEN];
> +
> +#ifndef MODULE
> + static unsigned int cardidx;
> + if (!cardidx++)
> + printk(version);
> +#endif
this is silly leftover logic from ancient days.
unconditionally printk() the version here, and remove from module init
> + err = pci_enable_device(pdev);
> + if (err) {
> + printk(KERN_ERR "%s (adm8211): Cannot enable new PCI device\n",
> + pci_name(pdev));
no need for printk, PCI layer already does this
> + return err;
> + }
> +
> + io_addr = pci_resource_start(pdev, 0);
> + io_len = pci_resource_len(pdev, 0);
> + mem_addr = pci_resource_start(pdev, 1);
> + mem_len = pci_resource_len(pdev, 1);
> + if (io_len < 256 || mem_len < 1024) {
> + printk(KERN_ERR "%s (adm8211): Too short PCI resources\n",
> + pci_name(pdev));
> + goto err_disable_pdev;
> + }
> +
> +
> + /* check signature */
> + pci_read_config_dword(pdev, 0x80 /* CR32 */, ®);
> + if (reg != ADM8211_SIG1 && reg != ADM8211_SIG2) {
> + printk(KERN_ERR "%s (adm8211): Invalid signature (0x%x)\n",
> + pci_name(pdev), reg);
> + goto err_disable_pdev;
> + }
> +
> + err = pci_request_regions(pdev, "adm8211");
> + if (err) {
> + printk(KERN_ERR "%s (adm8211): Cannot obtain PCI resources\n",
> + pci_name(pdev));
ditto
> + return err; /* someone else grabbed it? don't disable it */
> + }
> +
> + if (pci_set_dma_mask(pdev, DMA_32BIT_MASK) ||
> + pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK)) {
> + printk(KERN_ERR "%s (adm8211): No suitable DMA available\n",
> + pci_name(pdev));
> + goto err_free_reg;
> + }
> +
> + pci_set_master(pdev);
> +
> + dev = ieee80211_alloc_hw(sizeof(*priv), &adm8211_ops);
> + if (!dev) {
> + printk(KERN_ERR "%s (adm8211): ieee80211 alloc failed\n",
> + pci_name(pdev));
> + err = -ENOMEM;
> + goto err_free_reg;
> + }
> + priv = dev->priv;
> + priv->pdev = pdev;
> +
> + spin_lock_init(&priv->lock);
> +
> + SET_IEEE80211_DEV(dev, &pdev->dev);
> +
> + pci_set_drvdata(pdev, dev);
> +
> + priv->map = pci_iomap(pdev, 1, mem_len);
> + if (!priv->map)
> + priv->map = pci_iomap(pdev, 0, io_len);
is this paranoia?
just code 100% MMIO only, and ditch the iomap per-register-access overhead
> + pci_read_config_byte(pdev, PCI_CLASS_REVISION, &priv->revid);
this is in struct pci_dev now
> +static int __init adm8211_init(void)
> +{
> +#ifdef MODULE
> + printk(version);
> +#endif
see above
> + return pci_register_driver(&adm8211_driver);
> +}
> +
> +
> +static void __exit adm8211_exit(void)
> +{
> + pci_unregister_driver(&adm8211_driver);
> --- /dev/null
> +++ b/drivers/net/wireless/adm8211.h
> @@ -0,0 +1,659 @@
> +#ifndef ADM8211_H
> +#define ADM8211_H
> +
> +/* ADM8211 Registers */
> +
> +/* CR32 (SIG) signature */
> +#define ADM8211_SIG1 0x82011317 /* ADM8211A */
> +#define ADM8211_SIG2 0x82111317 /* ADM8211B/ADM8211C */
> +
> +#define ADM8211_CSR_READ(r) ioread32(&priv->map->r)
> +#define ADM8211_CSR_WRITE(r, val) iowrite32((val), &priv->map->r)
see above, re MMIO-only
> +/* CSR (Host Control and Status Registers) */
> +struct adm8211_csr {
> + __le32 PAR; /* 0x00 CSR0 */
> + __le32 FRCTL; /* 0x04 CSR0A */
> + __le32 TDR; /* 0x08 CSR1 */
> + __le32 WTDP; /* 0x0C CSR1A */
> + __le32 RDR; /* 0x10 CSR2 */
> + __le32 WRDP; /* 0x14 CSR2A */
> + __le32 RDB; /* 0x18 CSR3 */
> + __le32 TDBH; /* 0x1C CSR3A */
> + __le32 TDBD; /* 0x20 CSR4 */
> + __le32 TDBP; /* 0x24 CSR4A */
> + __le32 STSR; /* 0x28 CSR5 */
> + __le32 TDBB; /* 0x2C CSR5A */
> + __le32 NAR; /* 0x30 CSR6 */
> + __le32 CSR6A; /* reserved */
> + __le32 IER; /* 0x38 CSR7 */
> + __le32 TKIPSCEP; /* 0x3C CSR7A */
> + __le32 LPC; /* 0x40 CSR8 */
> + __le32 CSR_TEST1; /* 0x44 CSR8A */
> + __le32 SPR; /* 0x48 CSR9 */
> + __le32 CSR_TEST0; /* 0x4C CSR9A */
> + __le32 WCSR; /* 0x50 CSR10 */
> + __le32 WPDR; /* 0x54 CSR10A */
> + __le32 GPTMR; /* 0x58 CSR11 */
> + __le32 GPIO; /* 0x5C CSR11A */
> + __le32 BBPCTL; /* 0x60 CSR12 */
> + __le32 SYNCTL; /* 0x64 CSR12A */
> + __le32 PLCPHD; /* 0x68 CSR13 */
> + __le32 MMIWA; /* 0x6C CSR13A */
> + __le32 MMIRD0; /* 0x70 CSR14 */
> + __le32 MMIRD1; /* 0x74 CSR14A */
> + __le32 TXBR; /* 0x78 CSR15 */
> + __le32 SYNDATA; /* 0x7C CSR15A */
> + __le32 ALCS; /* 0x80 CSR16 */
> + __le32 TOFS2; /* 0x84 CSR17 */
> + __le32 CMDR; /* 0x88 CSR18 */
> + __le32 PCIC; /* 0x8C CSR19 */
> + __le32 PMCSR; /* 0x90 CSR20 */
> + __le32 PAR0; /* 0x94 CSR21 */
> + __le32 PAR1; /* 0x98 CSR22 */
> + __le32 MAR0; /* 0x9C CSR23 */
> + __le32 MAR1; /* 0xA0 CSR24 */
> + __le32 ATIMDA0; /* 0xA4 CSR25 */
> + __le32 ABDA1; /* 0xA8 CSR26 */
> + __le32 BSSID0; /* 0xAC CSR27 */
> + __le32 TXLMT; /* 0xB0 CSR28 */
> + __le32 MIBCNT; /* 0xB4 CSR29 */
> + __le32 BCNT; /* 0xB8 CSR30 */
> + __le32 TSFTH; /* 0xBC CSR31 */
> + __le32 TSC; /* 0xC0 CSR32 */
> + __le32 SYNRF; /* 0xC4 CSR33 */
> + __le32 BPLI; /* 0xC8 CSR34 */
> + __le32 CAP0; /* 0xCC CSR35 */
> + __le32 CAP1; /* 0xD0 CSR36 */
> + __le32 RMD; /* 0xD4 CSR37 */
> + __le32 CFPP; /* 0xD8 CSR38 */
> + __le32 TOFS0; /* 0xDC CSR39 */
> + __le32 TOFS1; /* 0xE0 CSR40 */
> + __le32 IFST; /* 0xE4 CSR41 */
> + __le32 RSPT; /* 0xE8 CSR42 */
> + __le32 TSFTL; /* 0xEC CSR43 */
> + __le32 WEPCTL; /* 0xF0 CSR44 */
> + __le32 WESK; /* 0xF4 CSR45 */
> + __le32 WEPCNT; /* 0xF8 CSR46 */
> + __le32 MACTEST; /* 0xFC CSR47 */
> + __le32 FER; /* 0x100 */
> + __le32 FEMR; /* 0x104 */
> + __le32 FPSR; /* 0x108 */
> + __le32 FFER; /* 0x10C */
> +} __attribute__ ((packed));
attributed(packed) is unneccesary here, and its use results in
sub-optimal code
> +/* CSR0 - PAR (PCI Address Register) */
> +#define ADM8211_PAR_MWIE (1 << 24)
> +#define ADM8211_PAR_MRLE (1 << 23)
> +#define ADM8211_PAR_MRME (1 << 21)
> +#define ADM8211_PAR_RAP ((1 << 18) | (1 << 17))
> +#define ADM8211_PAR_CAL ((1 << 15) | (1 << 14))
> +#define ADM8211_PAR_PBL 0x00003f00
> +#define ADM8211_PAR_BLE (1 << 7)
> +#define ADM8211_PAR_DSL 0x0000007c
> +#define ADM8211_PAR_BAR (1 << 1)
> +#define ADM8211_PAR_SWR (1 << 0)
> +
> +/* CSR1 - FRCTL (Frame Control Register) */
> +#define ADM8211_FRCTL_PWRMGT (1 << 31)
> +#define ADM8211_FRCTL_MAXPSP (1 << 27)
> +#define ADM8211_FRCTL_DRVPRSP (1 << 26)
> +#define ADM8211_FRCTL_DRVBCON (1 << 25)
> +#define ADM8211_FRCTL_AID 0x0000ffff
> +#define ADM8211_FRCTL_AID_ON 0x0000c000
> +
> +/* CSR5 - STSR (Status Register) */
> +#define ADM8211_STSR_PCF (1 << 31)
> +#define ADM8211_STSR_BCNTC (1 << 30)
> +#define ADM8211_STSR_GPINT (1 << 29)
> +#define ADM8211_STSR_LinkOff (1 << 28)
> +#define ADM8211_STSR_ATIMTC (1 << 27)
> +#define ADM8211_STSR_TSFTF (1 << 26)
> +#define ADM8211_STSR_TSCZ (1 << 25)
> +#define ADM8211_STSR_LinkOn (1 << 24)
> +#define ADM8211_STSR_SQL (1 << 23)
> +#define ADM8211_STSR_WEPTD (1 << 22)
> +#define ADM8211_STSR_ATIME (1 << 21)
> +#define ADM8211_STSR_TBTT (1 << 20)
> +#define ADM8211_STSR_NISS (1 << 16)
> +#define ADM8211_STSR_AISS (1 << 15)
> +#define ADM8211_STSR_TEIS (1 << 14)
> +#define ADM8211_STSR_FBE (1 << 13)
> +#define ADM8211_STSR_REIS (1 << 12)
> +#define ADM8211_STSR_GPTT (1 << 11)
> +#define ADM8211_STSR_RPS (1 << 8)
> +#define ADM8211_STSR_RDU (1 << 7)
> +#define ADM8211_STSR_RCI (1 << 6)
> +#define ADM8211_STSR_TUF (1 << 5)
> +#define ADM8211_STSR_TRT (1 << 4)
> +#define ADM8211_STSR_TLT (1 << 3)
> +#define ADM8211_STSR_TDU (1 << 2)
> +#define ADM8211_STSR_TPS (1 << 1)
> +#define ADM8211_STSR_TCI (1 << 0)
> +
> +/* CSR6 - NAR (Network Access Register) */
> +#define ADM8211_NAR_TXCF (1 << 31)
> +#define ADM8211_NAR_HF (1 << 30)
> +#define ADM8211_NAR_UTR (1 << 29)
> +#define ADM8211_NAR_SQ (1 << 28)
> +#define ADM8211_NAR_CFP (1 << 27)
> +#define ADM8211_NAR_SF (1 << 21)
> +#define ADM8211_NAR_TR ((1 << 15) | (1 << 14))
> +#define ADM8211_NAR_ST (1 << 13)
> +#define ADM8211_NAR_OM ((1 << 11) | (1 << 10))
> +#define ADM8211_NAR_MM (1 << 7)
> +#define ADM8211_NAR_PR (1 << 6)
> +#define ADM8211_NAR_EA (1 << 5)
> +#define ADM8211_NAR_PB (1 << 3)
> +#define ADM8211_NAR_STPDMA (1 << 2)
> +#define ADM8211_NAR_SR (1 << 1)
> +#define ADM8211_NAR_CTX (1 << 0)
enums are preferred. they do not disappear at the cpp stage, and confer
type information.
> +#define ADM8211_IDLE_RX() \
> +do { \
> + if (priv->nar & ADM8211_NAR_SR) { \
> + ADM8211_CSR_WRITE(NAR, priv->nar & ~ADM8211_NAR_SR); \
> + ADM8211_CSR_READ(NAR); \
> + mdelay(20); \
> + } \
> +} while (0)
should be msleep() AFAICS
> +struct adm8211_eeprom {
> + __le16 signature; /* 0x00 */
> + u8 major_version; /* 0x02 */
> + u8 minor_version; /* 0x03 */
> + u8 reserved_1[4]; /* 0x04 */
> + u8 hwaddr[6]; /* 0x08 */
> + u8 reserved_2[8]; /* 0x1E */
> + __le16 cr49; /* 0x16 */
> + u8 cr03; /* 0x18 */
> + u8 cr28; /* 0x19 */
> + u8 cr29; /* 0x1A */
> + u8 country_code; /* 0x1B */
> +
> +/* specific bbp types */
> +#define ADM8211_BBP_RFMD3000 0x00
> +#define ADM8211_BBP_RFMD3002 0x01
> +#define ADM8211_BBP_ADM8011 0x04
> + u8 specific_bbptype; /* 0x1C */
> + u8 specific_rftype; /* 0x1D */
> + u8 reserved_3[2]; /* 0x1E */
> + __le16 device_id; /* 0x20 */
> + __le16 vendor_id; /* 0x22 */
> + __le16 subsystem_id; /* 0x24 */
> + __le16 subsystem_vendor_id; /* 0x26 */
> + u8 maxlat; /* 0x28 */
> + u8 mingnt; /* 0x29 */
> + __le16 cis_pointer_low; /* 0x2A */
> + __le16 cis_pointer_high; /* 0x2C */
> + __le16 csr18; /* 0x2E */
> + u8 reserved_4[16]; /* 0x30 */
> + u8 d1_pwrdara; /* 0x40 */
> + u8 d0_pwrdara; /* 0x41 */
> + u8 d3_pwrdara; /* 0x42 */
> + u8 d2_pwrdara; /* 0x43 */
> + u8 antenna_power[14]; /* 0x44 */
> + __le16 cis_wordcnt; /* 0x52 */
> + u8 tx_power[14]; /* 0x54 */
> + u8 lpf_cutoff[14]; /* 0x62 */
> + u8 lnags_threshold[14]; /* 0x70 */
> + __le16 checksum; /* 0x7E */
> + u8 cis_data[0]; /* 0x80, 384 bytes */
> +} __attribute__ ((packed));
same attributed(packed) comment as above
> +struct adm8211_priv {
> + struct pci_dev *pdev;
> + spinlock_t lock;
> + struct adm8211_csr __iomem *map;
> + struct adm8211_desc *rx_ring;
> + struct adm8211_desc *tx_ring;
> + dma_addr_t rx_ring_dma;
> + dma_addr_t tx_ring_dma;
> + struct adm8211_rx_ring_info *rx_buffers;
> + struct adm8211_tx_ring_info *tx_buffers;
> + unsigned int rx_ring_size, tx_ring_size;
> + unsigned int cur_tx, dirty_tx, cur_rx;
> +
> + struct ieee80211_low_level_stats stats;
> + struct ieee80211_hw_mode modes[1];
> + struct ieee80211_channel channels[ARRAY_SIZE(adm8211_channels)];
> + struct ieee80211_rate rates[ARRAY_SIZE(adm8211_rates)];
> + int mode;
> +
> + int channel;
> + u8 bssid[ETH_ALEN];
> + u8 ssid[32];
> + size_t ssid_len;
> + u8 *mac_addr;
> +
> + u8 soft_rx_crc;
> + u8 retry_limit;
> +
> + u8 ant_power;
> + u8 tx_power;
> + u8 lpf_cutoff;
> + u8 lnags_threshold;
> + struct adm8211_eeprom *eeprom;
> + size_t eeprom_len;
> +
> + u8 revid;
> +
> + u32 nar;
use of <tab> to separate type and name greatly enhances readability.
look at many other net drivers, to see the positive effects
^ permalink raw reply
* READ ME Major net-2.6.24 rebase
From: David Miller @ 2007-09-15 22:39 UTC (permalink / raw)
To: netdev; +Cc: jeff, linville, akpm
I've fully rebased the tree at:
kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6.24.git
Also, I've fully integrated both John Linville's wireless
infrastructure and driver updates, as well as Jeff Garzik's
upstream netdevice changes.
This should resolve all of the merging conflicts that were
occuring between these trees, and from now until the
2.6.24 merge window opens Jeff and I will keep our
trees close together so that there is no divergence.
I've sanity-strapped this thing by doing a make allmodconfig
on sparc64, and I'm running this tree on my SunBlade1500
workstation with no problems so far.
With this much churn there is guarenteed to be a small issue
here or there, and we'll fix those up.
Andrew, this means you can stop pulling in Jeff's tree for the
time being, everything is in my tree above.
Enjoy.
^ permalink raw reply
* Re: READ ME Major net-2.6.24 rebase
From: Jeff Garzik @ 2007-09-15 22:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linville, akpm
In-Reply-To: <20070915.153936.41632998.davem@davemloft.net>
David Miller wrote:
> Andrew, this means you can stop pulling in Jeff's tree for the
> time being, everything is in my tree above.
ACK
^ permalink raw reply
* Re: Please pull 'fixes-jgarzik' branch of wireless-2.6
From: Jeff Garzik @ 2007-09-15 23:29 UTC (permalink / raw)
To: John W. Linville; +Cc: netdev
In-Reply-To: <20070915131452.GA6060@tuxdriver.com>
John W. Linville wrote:
> Jeff,
>
> Two more fixes for 2.6.23, including one for kernel.org bug 8937...
>
> Thanks,
>
> John
>
> ---
>
> The following changes since commit 0d4cbb5e7f60b2f1a4d8b7f6ea4cc264262c7a01:
> Linus Torvalds (1):
> Linux 2.6.23-rc6
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git fixes-jgarzik
>
> Larry Finger (1):
> bcm43xx: Fix cancellation of work queue crashes
>
> Masakazu Mokuno (1):
> As struct iw_point is bi-directional payload, we should copy back the content
>
> drivers/net/wireless/bcm43xx/bcm43xx_main.c | 28 ++++++++++++++++++-------
> drivers/net/wireless/bcm43xx/bcm43xx_main.h | 2 +-
> drivers/net/wireless/bcm43xx/bcm43xx_sysfs.c | 2 +-
> fs/compat_ioctl.c | 22 ++++++++++++++++---
> 4 files changed, 40 insertions(+), 14 deletions(-)
pulled
^ permalink raw reply
* Re: [PATCH] mv643xx_eth: Fix tx_bytes stats calculation
From: Jeff Garzik @ 2007-09-15 23:32 UTC (permalink / raw)
To: Dale Farnsworth; +Cc: netdev
In-Reply-To: <20070914182316.GA10642@xyzzy.farnsworth.org>
Dale Farnsworth wrote:
> Reported by Corey Minyard <cminyard@mvista.com>
>
> ---
> drivers/net/mv643xx_eth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
applied
^ permalink raw reply
* Re: [PATCH] ucc_geth: fix compilation
From: Jeff Garzik @ 2007-09-15 23:32 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: linuxppc-dev, netdev
In-Reply-To: <20070913152333.GA2381@localhost.localdomain>
Anton Vorontsov wrote:
> Currently qe_bd_t is used in the macro call -- dma_unmap_single,
> which is a no-op on PPC32, thus error is hidden today. Starting
> with 2.6.24, macro will be replaced by the empty static function,
> and erroneous use of qe_bd_t will trigger compilation error.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
>
> Reposting this to include netdev in Cc.
>
> drivers/net/ucc_geth.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
applied
^ permalink raw reply
* Re: [PATCH 3/3]: sk98lin: neuter device to only SysKonnect boards
From: Jeff Garzik @ 2007-09-15 23:36 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Linux Netdev
In-Reply-To: <20070912133043.1a15aabe@oldman>
applied 1-3 as a single commit, so that git-bisect will not break
Jeff
^ permalink raw reply
* Re: [PATCH 3/3] netxen: ethtool fixes
From: Jeff Garzik @ 2007-09-15 23:36 UTC (permalink / raw)
To: Dhananjay Phadke; +Cc: netdev, rob
In-Reply-To: <Pine.LNX.4.64.0709031029110.2882@stellar.unminc.com>
Dhananjay Phadke wrote:
> Resubmitting the patch.
>
> This patch improves ethtool support for printing correct ring statistics,
> segmentation offload status, etc.
>
> Signed-off by: Dhananjay Phadke <dhananjay@netxen.com>
>
is this for 2.6.23 or 2.6.24?
^ permalink raw reply
* Re: Please pull 'adm8211' branch of wireless-2.6
From: Michael Wu @ 2007-09-16 0:16 UTC (permalink / raw)
To: Jeff Garzik
Cc: John W. Linville, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, David Miller
In-Reply-To: <46EC4F78.4050700-o2qLIJkoznsdnm+yROfE0A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 14235 bytes --]
On Saturday 15 September 2007 17:32, Jeff Garzik wrote:
> Review summary: many minor issues, only one major one: irq handler loop
>
CCing me would help.
> John W. Linville wrote:
> > +static unsigned int tx_ring_size __read_mostly = 16;
> > +static unsigned int rx_ring_size __read_mostly = 16;
> > +
> > +module_param(tx_ring_size, uint, 0);
> > +module_param(rx_ring_size, uint, 0);
>
> should be in ethtool (or another per-interface runtime config tool), not
> a module parameter.
>
ethtool is not accessible to mac80211 drivers. I'll just make it a constant
because tuning the ring size doesn't help much.
> > + } else if ((flags & IFF_ALLMULTI) || (mc_count > -1)) {
>
> mc_count > -1 ?
>
> what kind of bogus placeholder is that?
>
Hm, right, I disabled the multicast filter config for some reason by doing
s/32/-1/ and I forgot to reenable it. Will fix.
> > + if (flags & IFF_PROMISC)
> > + dev->flags |= IEEE80211_HW_RX_INCLUDES_FCS;
> > + else
> > + dev->flags &= ~IEEE80211_HW_RX_INCLUDES_FCS;
>
> why does promisc dictate inclusion of FCS?
>
Because that's the way the hardware works.
> not this driver's fault, but, ieee80211_wake_queue() warrants revisiting
> now that MQ stuff is in
>
> when queue==0 is hardcoded, you can just use netif_wake/stop_queue() AFAICS
>
mac80211 have no access to any struct net_device.
> the '%' operator is expensive. mask combined with power-of-2 is better
>
Sure.
> > +static irqreturn_t adm8211_interrupt(int irq, void *dev_id)
> > +{
> > +#define ADM8211_INT(x) \
> > +do { \
> > + if (unlikely(stsr & ADM8211_STSR_ ## x)) \
> > + printk(KERN_DEBUG "%s: " #x "\n", wiphy_name(dev->wiphy)); \
> > +} while (0)
> > +
> > + struct ieee80211_hw *dev = dev_id;
> > + struct adm8211_priv *priv = dev->priv;
> > + unsigned int count = 0;
> > + u32 stsr;
> > +
> > + do {
> > + stsr = ADM8211_CSR_READ(STSR);
> > + ADM8211_CSR_WRITE(STSR, stsr);
> > + if (stsr == 0xffffffff)
> > + return IRQ_HANDLED;
> > +
> > + if (!(stsr & (ADM8211_STSR_NISS | ADM8211_STSR_AISS)))
> > + break;
> > +
> > + if (stsr & ADM8211_STSR_RCI)
> > + adm8211_interrupt_rci(dev);
> > + if (stsr & ADM8211_STSR_TCI)
> > + adm8211_interrupt_tci(dev);
> > +
> > + /*ADM8211_INT(LinkOn);*/
> > + /*ADM8211_INT(LinkOff);*/
> > +
> > + ADM8211_INT(PCF);
> > + ADM8211_INT(BCNTC);
> > + ADM8211_INT(GPINT);
> > + ADM8211_INT(ATIMTC);
> > + ADM8211_INT(TSFTF);
> > + ADM8211_INT(TSCZ);
> > + ADM8211_INT(SQL);
> > + ADM8211_INT(WEPTD);
> > + ADM8211_INT(ATIME);
> > + /*ADM8211_INT(TBTT);*/
> > + ADM8211_INT(TEIS);
> > + ADM8211_INT(FBE);
> > + ADM8211_INT(REIS);
> > + ADM8211_INT(GPTT);
> > + ADM8211_INT(RPS);
> > + ADM8211_INT(RDU);
> > + ADM8211_INT(TUF);
> > + /*ADM8211_INT(TRT);*/
> > + /*ADM8211_INT(TLT);*/
> > + /*ADM8211_INT(TDU);*/
> > + ADM8211_INT(TPS);
>
> lame. WAY too many branches, even if marked with unlikely()
>
> this should be surrounded by a single "if stsr &
> BITS_WE_SHOULD_NEVER_SEE" test
>
I'm just gonna delete these. This was only mildly interesting when the driver
was younger.
> > + } while (count++ < 20);
>
> NAK -- talk about back to the past.
>
> It's bogus to loop in the interrupt handler, then loop again in both RX
> and TX paths. You are getting close to reinventing the wheel here.
>
> Use NAPI rather than doing such a loop.
>
This is old interrupt handler code from Jouni's original driver. I never
bothered to replace it with the simpler designs used in newer drivers I've
worked on.
Also - mac80211 drivers have no access to NAPI.
> > +#define
> > WRITE_SYN(name,v_mask,v_shift,a_mask,a_shift,bits,prewrite,postwrite)\
> > +static void adm8211_rf_write_syn_ ## name (struct ieee80211_hw *dev,
> > \ + u16 addr, u32 value) { \
> > + struct adm8211_priv *priv = dev->priv; \
> > + unsigned int i; \
> > + u32 reg, bitbuf; \
> > + \
> > + value &= v_mask; \
> > + addr &= a_mask; \
> > + bitbuf = (value << v_shift) | (addr << a_shift); \
> > + \
> > + ADM8211_CSR_WRITE(SYNRF, ADM8211_SYNRF_IF_SELECT_1); \
> > + ADM8211_CSR_READ(SYNRF); \
> > + ADM8211_CSR_WRITE(SYNRF, ADM8211_SYNRF_IF_SELECT_0); \
> > + ADM8211_CSR_READ(SYNRF); \
> > + \
> > + if (prewrite) { \
> > + ADM8211_CSR_WRITE(SYNRF, ADM8211_SYNRF_WRITE_SYNDATA_0); \
> > + ADM8211_CSR_READ(SYNRF); \
> > + } \
> > + \
> > + for (i = 0; i <= bits; i++) { \
> > + if (bitbuf & (1 << (bits - i))) \
> > + reg = ADM8211_SYNRF_WRITE_SYNDATA_1; \
> > + else \
> > + reg = ADM8211_SYNRF_WRITE_SYNDATA_0; \
> > + \
> > + ADM8211_CSR_WRITE(SYNRF, reg); \
> > + ADM8211_CSR_READ(SYNRF); \
> > + \
> > + ADM8211_CSR_WRITE(SYNRF, reg | ADM8211_SYNRF_WRITE_CLOCK_1); \
> > + ADM8211_CSR_READ(SYNRF); \
> > + ADM8211_CSR_WRITE(SYNRF, reg | ADM8211_SYNRF_WRITE_CLOCK_0); \
> > + ADM8211_CSR_READ(SYNRF); \
> > + } \
> > + \
> > + if (postwrite == 1) { \
> > + ADM8211_CSR_WRITE(SYNRF, reg | ADM8211_SYNRF_IF_SELECT_0); \
> > + ADM8211_CSR_READ(SYNRF); \
> > + } \
> > + if (postwrite == 2) { \
> > + ADM8211_CSR_WRITE(SYNRF, reg | ADM8211_SYNRF_IF_SELECT_1); \
> > + ADM8211_CSR_READ(SYNRF); \
> > + } \
> > + \
> > + ADM8211_CSR_WRITE(SYNRF, 0); \
> > + ADM8211_CSR_READ(SYNRF); \
> > +}
> > +
> > +WRITE_SYN(max2820, 0x00FFF, 0, 0x0F, 12, 15, 1, 1)
> > +WRITE_SYN(al2210l, 0xFFFFF, 4, 0x0F, 0, 23, 1, 1)
> > +WRITE_SYN(rfmd2958, 0x3FFFF, 0, 0x1F, 18, 23, 0, 1)
> > +WRITE_SYN(rfmd2948, 0x0FFFF, 4, 0x0F, 0, 21, 0, 2)
>
> code bloat from hell. just pass these arguments (or a pointer to an
> entry in a table that contains this info)
>
Yeah sure. I wrote that code a long time ago,, would never do something like
that now. :p
> > +static void adm8211_hw_init(struct ieee80211_hw *dev)
> > +{
> > + struct adm8211_priv *priv = dev->priv;
> > + u32 reg;
> > + u8 cline;
> > +
> > + reg = le32_to_cpu(ADM8211_CSR_READ(PAR));
> > + reg |= ADM8211_PAR_MRLE | ADM8211_PAR_MRME;
> > + reg &= ~(ADM8211_PAR_BAR | ADM8211_PAR_CAL);
>
> what do BAR and CAL bits do?
>
Need to dig up the specs to find out.
> > + if (!pci_set_mwi(priv->pdev)) {
> > + reg |= 0x1 << 24;
> > + pci_read_config_byte(priv->pdev, PCI_CACHE_LINE_SIZE, &cline);
> > +
> > + switch (cline) {
> > + case 0x8: reg |= (0x1 << 14);
> > + break;
> > + case 0x16: reg |= (0x2 << 14);
> > + break;
> > + case 0x32: reg |= (0x3 << 14);
> > + break;
> > + default: reg |= (0x0 << 14);
> > + break;
> > + }
> > + }
> > +
> > + ADM8211_CSR_WRITE(PAR, reg);
>
> this should be set regardless of MWI usage AFAICS
>
Sure.
> > + rx_info->skb = dev_alloc_skb(RX_PKT_SIZE);
> > + if (rx_info->skb == NULL)
> > + break;
>
> it seems highly bogus to set RX_PKT_SIZE for all RX descriptors, then
> bail if allocation starts failing. the state of the ring becomes out of
> sync with reality.
>
Yeah, probably. Much of the tx/rx ring code is somewhat old and questionable.
(but tested and working!)
> > + if (priv->cur_tx - priv->dirty_tx == priv->tx_ring_size - 2)
>
> extra parens would be nice for readability
>
Ok.
> > + priv->tx_buffers[entry].skb = skb;
> > + priv->tx_buffers[entry].mapping = mapping;
> > + memcpy(&priv->tx_buffers[entry].tx_control, control, sizeof(*control));
> > + priv->tx_buffers[entry].hdrlen = hdrlen;
> > + priv->tx_ring[entry].buffer1 = cpu_to_le32(mapping);
> > +
> > + if (entry == priv->tx_ring_size - 1)
> > + flag |= TDES1_CONTROL_TER;
> > + priv->tx_ring[entry].length = cpu_to_le32(flag | skb->len);
> > +
> > + /* Set TX rate (SIGNAL field in PLCP PPDU format) */
> > + flag = TDES0_CONTROL_OWN | (plcp_signal << 20) | 8 /* ? */;
> > + priv->tx_ring[entry].status = cpu_to_le32(flag);
> > +
> > + priv->cur_tx++;
> > +
> > + spin_unlock_irqrestore(&priv->lock, flags);
> > +
> > + /* Trigger transmit poll */
> > + ADM8211_CSR_WRITE(TDR, 0);
> > +}
> > +
> > +/* Put adm8211_tx_hdr on skb and transmit */
> > +static int adm8211_tx(struct ieee80211_hw *dev, struct sk_buff *skb,
> > + struct ieee80211_tx_control *control)
> > +{
> > + struct adm8211_tx_hdr *txhdr;
> > + u16 fc;
> > + size_t payload_len, hdrlen;
> > + int plcp, dur, len, plcp_signal, short_preamble;
> > + struct ieee80211_hdr *hdr;
> > +
> > + if (control->tx_rate < 0) {
> > + short_preamble = 1;
> > + plcp_signal = -control->tx_rate;
> > + } else {
> > + short_preamble = 0;
> > + plcp_signal = control->tx_rate;
> > + }
> > +
> > + hdr = (struct ieee80211_hdr *)skb->data;
> > + fc = le16_to_cpu(hdr->frame_control) & ~IEEE80211_FCTL_PROTECTED;
> > + hdrlen = ieee80211_get_hdrlen(fc);
> > + memcpy(skb->cb, skb->data, hdrlen);
>
> to confirm: I thought skb->cb was owned by the skb creator? In this
> the driver is definitely _not_ the skb creator
>
Once the skb is passed to the driver, the driver owns cb. cb is owned by the
layer which has the skb.
> > +static int adm8211_alloc_rings(struct ieee80211_hw *dev)
> > +{
> > + struct adm8211_priv *priv = dev->priv;
> > + unsigned int ring_size;
> > +
> > + priv->rx_buffers = kmalloc(sizeof(*priv->rx_buffers) *
> > priv->rx_ring_size + + sizeof(*priv->tx_buffers) *
> > priv->tx_ring_size, GFP_KERNEL); + if (!priv->rx_buffers)
> > + return -ENOMEM;
> > +
> > + priv->tx_buffers = (void *)priv->rx_buffers +
> > + sizeof(*priv->rx_buffers) * priv->rx_ring_size;
> > +
> > + /* Allocate TX/RX descriptors */
> > + ring_size = sizeof(struct adm8211_desc) * priv->rx_ring_size +
> > + sizeof(struct adm8211_desc) * priv->tx_ring_size;
> > + priv->rx_ring = pci_alloc_consistent(priv->pdev, ring_size,
> > + &priv->rx_ring_dma);
> > +
> > + if (!priv->rx_ring) {
> > + kfree(priv->rx_buffers);
> > + priv->rx_buffers = NULL;
> > + priv->tx_buffers = NULL;
> > + return -ENOMEM;
> > + }
> > +
> > + priv->tx_ring = (struct adm8211_desc *)(priv->rx_ring +
> > + priv->rx_ring_size);
> > + priv->tx_ring_dma = priv->rx_ring_dma +
> > + sizeof(struct adm8211_desc) * priv->rx_ring_size;
> > +
>
> why not alloc ring contents (skbs) here too?
>
I really don't like this function. It allocates some ring structures at pci
initialization when it can really be deferred to open time. So, instead of
allocating the ring contents here, the ring should be allocated at the same
place where the ring contents are allocated. Doing that reduces the number of
things that can fail at pci init time.
> > +static int __devinit adm8211_probe(struct pci_dev *pdev,
> > + const struct pci_device_id *id)
> > +{
> > + struct ieee80211_hw *dev;
> > + struct adm8211_priv *priv;
> > + unsigned long mem_addr, mem_len;
> > + unsigned int io_addr, io_len;
> > + int err;
> > + u32 reg;
> > + u8 perm_addr[ETH_ALEN];
> > +
> > +#ifndef MODULE
> > + static unsigned int cardidx;
> > + if (!cardidx++)
> > + printk(version);
> > +#endif
>
> this is silly leftover logic from ancient days.
>
> unconditionally printk() the version here, and remove from module init
>
Sure.
> > + err = pci_enable_device(pdev);
> > + if (err) {
> > + printk(KERN_ERR "%s (adm8211): Cannot enable new PCI device\n",
> > + pci_name(pdev));
>
> no need for printk, PCI layer already does this
>
Ok.
> > + err = pci_request_regions(pdev, "adm8211");
> > + if (err) {
> > + printk(KERN_ERR "%s (adm8211): Cannot obtain PCI resources\n",
> > + pci_name(pdev));
>
> ditto
>
Ok.
> > + return err; /* someone else grabbed it? don't disable it */
> > + }
> > +
> > + if (pci_set_dma_mask(pdev, DMA_32BIT_MASK) ||
> > + pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK)) {
> > + printk(KERN_ERR "%s (adm8211): No suitable DMA available\n",
> > + pci_name(pdev));
> > + goto err_free_reg;
> > + }
> > +
> > + pci_set_master(pdev);
> > +
> > + dev = ieee80211_alloc_hw(sizeof(*priv), &adm8211_ops);
> > + if (!dev) {
> > + printk(KERN_ERR "%s (adm8211): ieee80211 alloc failed\n",
> > + pci_name(pdev));
> > + err = -ENOMEM;
> > + goto err_free_reg;
> > + }
> > + priv = dev->priv;
> > + priv->pdev = pdev;
> > +
> > + spin_lock_init(&priv->lock);
> > +
> > + SET_IEEE80211_DEV(dev, &pdev->dev);
> > +
> > + pci_set_drvdata(pdev, dev);
> > +
> > + priv->map = pci_iomap(pdev, 1, mem_len);
> > + if (!priv->map)
> > + priv->map = pci_iomap(pdev, 0, io_len);
>
> is this paranoia?
>
> just code 100% MMIO only, and ditch the iomap per-register-access overhead
>
Will do.
> > + pci_read_config_byte(pdev, PCI_CLASS_REVISION, &priv->revid);
>
> this is in struct pci_dev now
>
Convenient.
> > +/* CSR (Host Control and Status Registers) */
> > +struct adm8211_csr {
>[snip]
> > +} __attribute__ ((packed));
>
> attributed(packed) is unneccesary here, and its use results in
> sub-optimal code
>
How? Doesn't this just turn into a bunch of offsets?
> enums are preferred. they do not disappear at the cpp stage, and confer
> type information.
>
I'd rather not.
> > +#define ADM8211_IDLE_RX() \
> > +do { \
> > + if (priv->nar & ADM8211_NAR_SR) { \
> > + ADM8211_CSR_WRITE(NAR, priv->nar & ~ADM8211_NAR_SR); \
> > + ADM8211_CSR_READ(NAR); \
> > + mdelay(20); \
> > + } \
> > +} while (0)
>
> should be msleep() AFAICS
>
Nope, this can be called in atomic context. Admittedly, 20 ms delay is
ridiculously long.. I'll find a better way to handle this.
> use of <tab> to separate type and name greatly enhances readability.
> look at many other net drivers, to see the positive effects
>
Sure, will do.
Thanks,
-Michael Wu
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Please pull 'adm8211' branch of wireless-2.6
From: Michael Wu @ 2007-09-16 0:37 UTC (permalink / raw)
To: John W. Linville; +Cc: jeff, netdev, linux-wireless
In-Reply-To: <20070915132220.GE6060@tuxdriver.com>
[-- Attachment #1: Type: text/plain, Size: 500 bytes --]
On Saturday 15 September 2007 09:22, John W. Linville wrote:
> It is reverse-engineered and still has more magic
> initialization numbers than I'd like, but overall I think it would
> be better to have this upstream than not.
>
Not entirely accurate, as ADMtek released specs (publically downloadable, no
NDA), released their Linux driver source (under GPL), sent hardware samples,
and offered an engineer's help a few months after the first release of the
adm8211 driver.
-Michael Wu
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Please pull 'adm8211' branch of wireless-2.6
From: Jeff Garzik @ 2007-09-16 0:56 UTC (permalink / raw)
To: Michael Wu, David Miller; +Cc: John W. Linville, netdev, linux-wireless
In-Reply-To: <200709152017.02646.flamingice@sourmilk.net>
Michael Wu wrote:
> On Saturday 15 September 2007 17:32, Jeff Garzik wrote:
>> Review summary: many minor issues, only one major one: irq handler loop
>>
> CCing me would help.
Sorry. I just hit 'reply to all'... apparently you were not CC'd on
the submission.
>>> + if (flags & IFF_PROMISC)
>>> + dev->flags |= IEEE80211_HW_RX_INCLUDES_FCS;
>>> + else
>>> + dev->flags &= ~IEEE80211_HW_RX_INCLUDES_FCS;
>> why does promisc dictate inclusion of FCS?
>>
> Because that's the way the hardware works.
Why not always include it, regardless of promisc?
>>> + } while (count++ < 20);
>> NAK -- talk about back to the past.
>>
>> It's bogus to loop in the interrupt handler, then loop again in both RX
>> and TX paths. You are getting close to reinventing the wheel here.
>>
>> Use NAPI rather than doing such a loop.
>>
> This is old interrupt handler code from Jouni's original driver. I never
> bothered to replace it with the simpler designs used in newer drivers I've
> worked on.
>
> Also - mac80211 drivers have no access to NAPI.
Not true as of net-2.6.24.
>>> +/* CSR (Host Control and Status Registers) */
>>> +struct adm8211_csr {
>> [snip]
>>> +} __attribute__ ((packed));
>> attributed(packed) is unneccesary here, and its use results in
>> sub-optimal code
>>
> How? Doesn't this just turn into a bunch of offsets?
It depends on how its used in the code.
>> enums are preferred. they do not disappear at the cpp stage, and confer
>> type information.
>>
> I'd rather not.
Elaboration?
Remember this code is not just for you, but for all people working with
the code. It makes debugging (and sometimes tracebacks) much more
readable, since it hasn't been eaten by cpp.
Thanks,
Jeff
^ permalink raw reply
* Re: r8169: slow samba performance
From: David Madsen @ 2007-09-16 1:11 UTC (permalink / raw)
To: Francois Romieu; +Cc: netdev
In-Reply-To: <20070909153312.GA29586@electric-eye.fr.zoreil.com>
> Do you see a difference in the system load too, say a few lines of 'vmstat 1' ?
This is running on a dual core machine which explains the 50/50
sys/idle in vmstat.
with 8168 hack (patch #0002):
writes:
isis tmp # dd if=/dev/zero of=test.fil bs=1M count=1000
1000+0 records in
1000+0 records out
1048576000 bytes (1.0 GB) copied, 13.5288 s, 77.5 MB/s
procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
r b swpd free buff cache si so bi bo in cs us sy id wa
1 0 200 714568 2944 227964 0 0 4 74592 44243 12190 1 45 52 2
1 0 200 637656 3028 305016 0 0 0 79324 45579 12795 1 48 27 24
1 0 200 556688 3112 383816 0 0 4 82880 48222 13901 1 49 50 1
1 0 200 475936 3196 462280 0 0 8 78736 47925 13942 1 50 49 1
0 0 200 394992 3284 540676 0 0 12 74592 47657 13949 1 48 50 1
reads:
isis tmp # dd if=test.fil of=/dev/null bs=1M count=1000
1000+0 records in
1000+0 records out
1048576000 bytes (1.0 GB) copied, 34.7543 s, 30.2 MB/s
procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
r b swpd free buff cache si so bi bo in cs us sy id wa
0 0 200 18652 3952 913524 0 0 19996 0 18650 2000 1 13 83 4
0 0 200 19476 3940 912772 0 0 24576 0 15846 1725 0 12 88 0
0 0 200 22540 3940 910156 0 0 14336 0 9470 1024 0 8 91 0
0 0 200 25180 3924 907660 0 0 14336 0 10084 1076 0 8 92 0
1 0 200 17764 3920 915248 0 0 24576 0 16046 1669 1 13 87 0
0 0 200 18732 3936 914528 0 0 24592 0 17136 1963 0 13 86 1
0 0 200 29152 3924 904816 0 0 40996 0 25609 2776 0 19 80 1
0 0 200 35040 3880 899548 0 0 36872 0 24288 2589 1 18 81 1
0 0 200 15524 3904 919540 0 0 36888 0 24506 2664 0 19 80 0
0 0 200 23964 3904 911872 0 0 43048 64 27498 2934 0 22 76 2
0 0 200 15960 3908 920564 0 0 59444 0 38224 4096 0 29 68 3
0 0 200 14936 3908 921916 0 0 26652 0 18401 1957 1 15 82 3
1 0 200 30392 3916 906864 0 0 10248 0 7225 863 0 6 94 1
0 0 200 14836 3896 922768 0 0 32796 0 20830 2313 1 16 80 4
0 0 200 35152 3896 902788 0 0 30748 0 20679 2340 0 16 79 5
with ndelay(10) loop:
writes:
isis tmp # dd if=/dev/zero of=test.fil bs=1M count=1000
1000+0 records in
1000+0 records out
1048576000 bytes (1.0 GB) copied, 13.7967 s, 76.0 MB/s
1 0 0 694080 6448 248252 0 0 0 78736 44235 12588 1 50 50 0
1 0 0 613448 6524 326400 0 0 0 78736 44215 12477 1 50 50 0
1 0 0 535612 6628 403796 0 0 8 91668 45789 10741 0 52 23 24
0 0 0 454132 6704 482848 0 0 0 78736 47082 10795 1 51 49 0
1 0 0 373804 6784 560780 0 0 4 75008 46826 10418 1 49 51 0
1 0 0 292216 6860 639976 0 0 0 82880 47279 10544 1 51 49 0
reads:
isis tmp # dd if=test.fil of=/dev/null bs=1M count=1000
1000+0 records in
1000+0 records out
1048576000 bytes (1.0 GB) copied, 21.894 s, 47.9 MB/s
procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
r b swpd free buff cache si so bi bo in cs us sy id wa
1 0 0 14968 3736 920920 0 0 44968 0 21886 3068 0 33 60 7
0 0 0 13120 3672 922908 0 0 32768 0 17486 2326 0 26 74 0
1 0 0 24796 3640 912136 0 0 51212 4 29417 3637 1 44 56 0
1 0 0 52104 3600 886072 0 0 49688 0 29329 3602 0 42 57 0
1 0 0 44548 3564 894164 0 0 43808 0 28066 3418 0 41 57 2
0 0 0 16676 3608 922400 0 0 47148 0 25292 3313 0 36 59 4
0 0 0 37392 3604 902580 0 0 51248 0 27554 3512 1 40 59 1
1 0 0 23988 3648 916468 0 0 49196 0 26621 3393 0 39 61 1
1 0 0 16232 3696 924692 0 0 51248 0 28792 3536 1 43 56 1
0 1 0 18548 3732 921472 0 0 39416 0 21470 2736 1 31 66 2
2 0 0 13620 3760 928296 0 0 40520 0 22081 2818 0 33 65 1
0 0 0 18828 3732 923900 0 0 53252 0 28577 3611 0 43 57 0
1 0 160 13308 3736 929712 0 0 43012 0 22924 2920 1 33 66 0
1 0 176 13316 2668 931348 0 0 40964 0 23122 2899 0 34 66 0
0 0 176 13764 1900 932416 0 0 53260 0 28571 3601 0 42 57 0
0 1 176 14076 1744 931300 0 0 51672 0 28600 3845 1 42 58 0
1 0 176 16380 1620 931164 0 0 52828 0 27832 3518 1 41 58 1
Load is definately higher with the ndelay(10) loop but throughput on
reads is quite a bit better as well.
> Can you add the patch below on top of #0002 and see if there is some
> benefit from it ?
writes:
isis tmp # dd if=/dev/zero of=test.fil bs=1M count=1000
1000+0 records in
1000+0 records out
1048576000 bytes (1.0 GB) copied, 13.6414 s, 76.9 MB/s
procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
r b swpd free buff cache si so bi bo in cs us sy id wa
1 1 200 496436 5676 440184 0 0 342 16003 8770 2051 4 26 55 14
2 0 200 431808 5692 504456 0 0 4 67180 36908 11169 1 43 26 31
1 0 200 351524 5692 582756 0 0 0 76508 45336 11191 1 49 50 0
2 0 200 270928 5768 661040 0 0 0 78736 46283 11709 1 50 50 0
1 0 200 190804 5844 738648 0 0 0 78736 45110 10442 0 49 51 0
reads:
isis tmp # dd if=test.fil of=/dev/null bs=1M count=1000
1000+0 records in
1000+0 records out
1048576000 bytes (1.0 GB) copied, 31.3864 s, 33.4 MB/s
procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
r b swpd free buff cache si so bi bo in cs us sy id wa
1 0 200 14612 2604 929304 0 0 24576 0 16047 1715 1 15 85 0
0 0 200 16752 2604 927504 0 0 36864 4 24174 2618 1 23 77 0
0 0 200 14308 2592 930324 0 0 45060 1272 28799 3281 0 27 69 4
1 0 200 13800 2592 930896 0 0 34816 0 22206 2596 1 20 77 3
0 0 200 13940 2592 930736 0 0 12288 0 7745 943 0 8 92 0
1 0 200 17096 2580 927552 0 0 12288 0 7963 957 0 7 93 0
>
> I'd welcome if you could try the patch below on top of #0002 too:
>
writes:
after finishing writes: eth0: wait_max = 9
isis tmp # dd if=/dev/zero of=test.fil bs=1M count=1000
1000+0 records in
1000+0 records out
1048576000 bytes (1.0 GB) copied, 13.8243 s, 75.9 MB/s
procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
r b swpd free buff cache si so bi bo in cs us sy id wa
1 0 0 180056 3000 752876 0 0 0 74592 37500 4810 1 52 47 0
2 0 0 103060 3072 827492 0 0 0 77780 36297 3803 1 49 50 0
1 0 0 32320 3152 901552 0 0 0 75964 38510 5022 1 54 40 5
1 0 0 40728 3024 891948 0 0 0 74592 35930 3861 1 51 49 0
1 0 0 18328 3092 913288 0 0 0 74592 36354 4391 1 53 47 0
reads:
isis tmp # dd if=test.fil of=/dev/null bs=1M count=1000
1000+0 records in
1000+0 records out
1048576000 bytes (1.0 GB) copied, 23.5534 s, 44.5 MB/s
0 0 200 15696 2884 921820 0 0 40960 0 26874 2792 0 38 62 0
1 0 200 21572 2876 915680 0 0 44728 60 30070 3142 1 43 56 0
1 0 200 143944 2876 796260 0 0 43336 0 29704 3064 1 44 51 6
1 0 200 99132 2876 841268 0 0 45056 0 30130 3062 0 42 58 0
1 0 200 62292 2876 877832 0 0 36864 0 23757 2468 1 34 66 0
1 0 200 19332 2876 921140 0 0 43008 0 29924 3073 0 42 57 0
I've never seen wait_max go higher than 9 as of yet.
Let me know if I can provide any more useful information.
--David Madsen
^ permalink raw reply
* Re: Please pull 'adm8211' branch of wireless-2.6
From: John W. Linville @ 2007-09-16 2:36 UTC (permalink / raw)
To: David Miller
Cc: jeff-o2qLIJkoznsdnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
flamingice-R9e9/4HEdknk1uMJSBkQmQ
In-Reply-To: <20070915.112549.79067357.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Sat, Sep 15, 2007 at 11:25:49AM -0700, David Miller wrote:
> From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> Date: Sat, 15 Sep 2007 13:00:18 -0400
>
> > Come to think of it, this driver will depend on some of the mac80211
> > patches Dave M. has queued for net-2.6.24. Perhaps it would be better
> > if Dave were to merge it with his tree?
> >
> > Jeff, if you have no objection then please sign-off/ack/whatever so
> > davem can see it. Dave, in that case please pull this to net-2.6.24.
>
> There are a lot of things to sort out :-)
>
> What I'm going to try and accomplish right now is:
>
> 1) Rebase my tree, I just finished that.
> 2) Integrate linville's upstream-davem stuff into net-2.6.24
> 3) Integrate Jeff's upstream branch into net-2.6.24
Forgot about this part...would you be doing this otherwise? If not,
I can rearrange this patch (or its successor) so that it is based
off your tree instead.
> 4) Integrate linville's adm8211 driver into net-2.6.24
>
> We'll see how well that goes.
It sounds like Michael will be respinning in response to Jeff's
comments...?
John
--
John W. Linville
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org
^ permalink raw reply
* [PATCH 1/3] Blackfin EMAC driver: add function to change the MAC address
From: Bryan Wu @ 2007-09-16 2:57 UTC (permalink / raw)
To: jeff, akpm, linux-kernel, netdev, uclinux-dist-devel
>From 157dfddae50708a716c2a42a314eccb9621d8793 Mon Sep 17 00:00:00 2001
From: Alex Landau <lirsb@yahoo.com>
Date: Sun, 5 Aug 2007 15:58:03 +0800
Subject: [PATCH] Blackfin Ethernet MAC driver: add function to change the MAC address
Alex Landau writes in the forums:
Previously, changing the MAC address (e.g. via ifconfig) resulted
in a generic function to be called that only changed a variable in
memory. This patch also updated the Blackfin MAC address registers
to filter the correct new MAC.
Signed-off-by: Alex Landau <lirsb@yahoo.com>
Signed-off-by: Mike Frysinger <michael.frysinger@analog.com>
Signed-off-by: Bryan Wu <bryan.wu@analog.com>
---
drivers/net/bfin_mac.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index 2bb97d4..8d61ca9 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -468,7 +468,7 @@ void setup_system_regs(struct net_device *dev)
bfin_write_DMA1_Y_MODIFY(0);
}
-void setup_mac_addr(u8 * mac_addr)
+static void setup_mac_addr(u8 *mac_addr)
{
u32 addr_low = le32_to_cpu(*(__le32 *) & mac_addr[0]);
u16 addr_hi = le16_to_cpu(*(__le16 *) & mac_addr[4]);
@@ -478,6 +478,16 @@ void setup_mac_addr(u8 * mac_addr)
bfin_write_EMAC_ADDRHI(addr_hi);
}
+static int bf537mac_set_mac_address(struct net_device *dev, void *p)
+{
+ struct sockaddr *addr = p;
+ if (netif_running(dev))
+ return -EBUSY;
+ memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
+ setup_mac_addr(dev->dev_addr);
+ return 0;
+}
+
static void adjust_tx_list(void)
{
int timeout_cnt = MAX_TIMEOUT_CNT;
@@ -895,6 +905,7 @@ static int __init bf537mac_probe(struct net_device *dev)
dev->open = bf537mac_open;
dev->stop = bf537mac_close;
dev->hard_start_xmit = bf537mac_hard_start_xmit;
+ dev->set_mac_address = bf537mac_set_mac_address;
dev->tx_timeout = bf537mac_timeout;
dev->get_stats = bf537mac_query_statistics;
dev->set_multicast_list = bf537mac_set_multicast_list;
--
1.5.2
^ permalink raw reply related
* [PATCH 2/3] Blackfin EMAC driver: add power management interface and change the bf537mac_reset to bf537mac_disable
From: Bryan Wu @ 2007-09-16 2:57 UTC (permalink / raw)
To: jeff, akpm, linux-kernel, netdev, uclinux-dist-devel
Signed-off-by: Bryan Wu <bryan.wu@analog.com>
---
drivers/net/bfin_mac.c | 23 +++++++++++++++++++----
1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index 8d61ca9..3015385 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -677,7 +677,7 @@ static void bf537mac_poll(struct net_device *dev)
}
#endif /* CONFIG_NET_POLL_CONTROLLER */
-static void bf537mac_reset(void)
+static void bf537mac_disable(void)
{
unsigned int opmode;
@@ -735,7 +735,7 @@ static void bf537mac_timeout(struct net_device *dev)
{
pr_debug("%s: %s\n", dev->name, __FUNCTION__);
- bf537mac_reset();
+ bf537mac_disable();
/* reset tx queue */
tx_list_tail = tx_list_head->next;
@@ -829,7 +829,7 @@ static int bf537mac_open(struct net_device *dev)
bf537mac_setphy(dev);
setup_system_regs(dev);
- bf537mac_reset();
+ bf537mac_disable();
bf537mac_enable(dev);
pr_debug("hardware init finished\n");
@@ -989,15 +989,30 @@ static int bfin_mac_remove(struct platform_device *pdev)
return 0;
}
-static int bfin_mac_suspend(struct platform_device *pdev, pm_message_t state)
+#ifdef CONFIG_PM
+static int bfin_mac_suspend(struct platform_device *pdev, pm_message_t mesg)
{
+ struct net_device *net_dev = platform_get_drvdata(pdev);
+
+ if (netif_running(net_dev))
+ bf537mac_close(net_dev);
+
return 0;
}
static int bfin_mac_resume(struct platform_device *pdev)
{
+ struct net_device *net_dev = platform_get_drvdata(pdev);
+
+ if (netif_running(net_dev))
+ bf537mac_open(net_dev);
+
return 0;
}
+#else
+#define bfin_mac_suspend NULL
+#define bfin_mac_resume NULL
+#endif /* CONFIG_PM */
static struct platform_driver bfin_mac_driver = {
.probe = bfin_mac_probe,
--
1.5.2
^ permalink raw reply related
* [PATCH 3/3] Blackfin EMAC driver: Add phy abstraction layer supporting in bfin_emac driver
From: Bryan Wu @ 2007-09-16 2:57 UTC (permalink / raw)
To: jeff-o2qLIJkoznsdnm+yROfE0A,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b
- add MDIO functions and register mdio bus
- add phy abstraction layer (PAL) functions and use PAL API
- test on STAMP537 board
Signed-off-by: Bryan Wu <bryan.wu-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
---
drivers/net/bfin_mac.c | 311 +++++++++++++++++++++++++++++-------------------
drivers/net/bfin_mac.h | 53 ++-------
2 files changed, 195 insertions(+), 169 deletions(-)
diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index 3015385..c9808a1 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -47,6 +47,7 @@
#include <linux/spinlock.h>
#include <linux/ethtool.h>
#include <linux/mii.h>
+#include <linux/phy.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
@@ -99,6 +100,9 @@ static struct net_dma_desc_tx *current_tx_ptr;
static struct net_dma_desc_tx *tx_desc;
static struct net_dma_desc_rx *rx_desc;
+static void bf537mac_disable(void);
+static void bf537mac_enable(void);
+
static void desc_list_free(void)
{
struct net_dma_desc_rx *r;
@@ -287,8 +291,11 @@ static int setup_pin_mux(int action)
return 0;
}
+/*
+ * MII operations
+ */
/* Wait until the previous MDC/MDIO transaction has completed */
-static void poll_mdc_done(void)
+static void mdio_poll(void)
{
int timeout_cnt = MAX_TIMEOUT_CNT;
@@ -304,154 +311,201 @@ static void poll_mdc_done(void)
}
/* Read an off-chip register in a PHY through the MDC/MDIO port */
-static u16 read_phy_reg(u16 PHYAddr, u16 RegAddr)
+static int mdiobus_read(struct mii_bus *bus, int phy_addr, int regnum)
{
- poll_mdc_done();
+ mdio_poll();
+
/* read mode */
- bfin_write_EMAC_STAADD(SET_PHYAD(PHYAddr) |
- SET_REGAD(RegAddr) |
+ bfin_write_EMAC_STAADD(SET_PHYAD((u16) phy_addr) |
+ SET_REGAD((u16) regnum) |
STABUSY);
- poll_mdc_done();
- return (u16) bfin_read_EMAC_STADAT();
+ mdio_poll();
+
+ return (int) bfin_read_EMAC_STADAT();
}
/* Write an off-chip register in a PHY through the MDC/MDIO port */
-static void raw_write_phy_reg(u16 PHYAddr, u16 RegAddr, u32 Data)
+static int mdiobus_write(struct mii_bus *bus, int phy_addr, int regnum,
+ u16 value)
{
- bfin_write_EMAC_STADAT(Data);
+ mdio_poll();
+
+ bfin_write_EMAC_STADAT((u32) value);
/* write mode */
- bfin_write_EMAC_STAADD(SET_PHYAD(PHYAddr) |
- SET_REGAD(RegAddr) |
+ bfin_write_EMAC_STAADD(SET_PHYAD((u16) phy_addr) |
+ SET_REGAD((u16) regnum) |
STAOP |
STABUSY);
- poll_mdc_done();
+ mdio_poll();
+
+ return 0;
}
-static void write_phy_reg(u16 PHYAddr, u16 RegAddr, u32 Data)
+static int mdiobus_reset(struct mii_bus *bus)
{
- poll_mdc_done();
- raw_write_phy_reg(PHYAddr, RegAddr, Data);
+ return 0;
}
-/* set up the phy */
-static void bf537mac_setphy(struct net_device *dev)
+static void bf537_adjust_link(struct net_device *dev)
{
- u16 phydat;
struct bf537mac_local *lp = netdev_priv(dev);
+ struct phy_device *phydev = lp->phydev;
+ unsigned long flags;
+ int new_state = 0;
+
+ spin_lock_irqsave(&lp->lock, flags);
+ if (phydev->link) {
+ /* Now we make sure that we can be in full duplex mode.
+ * If not, we operate in half-duplex mode. */
+ if (phydev->duplex != lp->old_duplex) {
+ u32 opmode = bfin_read_EMAC_OPMODE();
+ new_state = 1;
+
+ if (phydev->duplex)
+ opmode |= FDMODE;
+ else
+ opmode &= ~(FDMODE);
+
+ bfin_write_EMAC_OPMODE(opmode);
+ lp->old_duplex = phydev->duplex;
+ }
- /* Program PHY registers */
- pr_debug("start setting up phy\n");
-
- /* issue a reset */
- raw_write_phy_reg(lp->PhyAddr, PHYREG_MODECTL, 0x8000);
-
- /* wait half a second */
- msleep(500);
-
- phydat = read_phy_reg(lp->PhyAddr, PHYREG_MODECTL);
-
- /* advertise flow control supported */
- phydat = read_phy_reg(lp->PhyAddr, PHYREG_ANAR);
- phydat |= (1 << 10);
- write_phy_reg(lp->PhyAddr, PHYREG_ANAR, phydat);
+ if (phydev->speed != lp->old_speed) {
+#if defined(CONFIG_BFIN_MAC_RMII)
+ u32 opmode = bfin_read_EMAC_OPMODE();
+ bf537mac_disable();
+ switch (phydev->speed) {
+ case 10:
+ opmode |= RMII_10;
+ break;
+ case 100:
+ opmode &= ~(RMII_10);
+ break;
+ default:
+ printk(KERN_WARNING
+ "%s: Ack! Speed (%d) is not 10/100!\n",
+ DRV_NAME, phydev->speed);
+ break;
+ }
+ bfin_write_EMAC_OPMODE(opmode);
+ bf537mac_enable();
+#endif
- phydat = 0;
- if (lp->Negotiate)
- phydat |= 0x1000; /* enable auto negotiation */
- else {
- if (lp->FullDuplex)
- phydat |= (1 << 8); /* full duplex */
- else
- phydat &= (~(1 << 8)); /* half duplex */
+ new_state = 1;
+ lp->old_speed = phydev->speed;
+ }
- if (!lp->Port10)
- phydat |= (1 << 13); /* 100 Mbps */
- else
- phydat &= (~(1 << 13)); /* 10 Mbps */
+ if (!lp->old_link) {
+ new_state = 1;
+ lp->old_link = 1;
+ netif_schedule(dev);
+ }
+ } else if (lp->old_link) {
+ new_state = 1;
+ lp->old_link = 0;
+ lp->old_speed = 0;
+ lp->old_duplex = -1;
}
- if (lp->Loopback)
- phydat |= (1 << 14); /* enable TX->RX loopback */
-
- write_phy_reg(lp->PhyAddr, PHYREG_MODECTL, phydat);
- msleep(500);
-
- phydat = read_phy_reg(lp->PhyAddr, PHYREG_MODECTL);
- /* check for SMSC PHY */
- if ((read_phy_reg(lp->PhyAddr, PHYREG_PHYID1) == 0x7) &&
- ((read_phy_reg(lp->PhyAddr, PHYREG_PHYID2) & 0xfff0) == 0xC0A0)) {
- /*
- * we have SMSC PHY so reqest interrupt
- * on link down condition
- */
-
- /* enable interrupts */
- write_phy_reg(lp->PhyAddr, 30, 0x0ff);
+ if (new_state) {
+ u32 opmode = bfin_read_EMAC_OPMODE();
+ phy_print_status(phydev);
+ pr_debug("EMAC_OPMODE = 0x%08x\n", opmode);
}
+
+ spin_unlock_irqrestore(&lp->lock, flags);
}
-/**************************************************************************/
-void setup_system_regs(struct net_device *dev)
+static int mii_probe(struct net_device *dev)
{
- int phyaddr;
- unsigned short sysctl, phydat;
- u32 opmode;
struct bf537mac_local *lp = netdev_priv(dev);
- int count = 0;
-
- phyaddr = lp->PhyAddr;
+ struct phy_device *phydev = NULL;
+ unsigned short sysctl;
+ int i;
- /* Enable PHY output */
+ /* Enable PHY output early */
if (!(bfin_read_VR_CTL() & PHYCLKOE))
bfin_write_VR_CTL(bfin_read_VR_CTL() | PHYCLKOE);
/* MDC = 2.5 MHz */
- sysctl = SET_MDCDIV(24);
- /* Odd word alignment for Receive Frame DMA word */
- /* Configure checksum support and rcve frame word alignment */
-#if defined(BFIN_MAC_CSUM_OFFLOAD)
- sysctl |= RXDWA | RXCKS;
-#else
- sysctl |= RXDWA;
-#endif
+ sysctl = bfin_read_EMAC_SYSCTL();
+ sysctl |= SET_MDCDIV(24);
bfin_write_EMAC_SYSCTL(sysctl);
- /* auto negotiation on */
- /* full duplex */
- /* 100 Mbps */
- phydat = PHY_ANEG_EN | PHY_DUPLEX | PHY_SPD_SET;
- write_phy_reg(phyaddr, PHYREG_MODECTL, phydat);
- /* test if full duplex supported */
- do {
- msleep(100);
- phydat = read_phy_reg(phyaddr, PHYREG_MODESTAT);
- if (count > 30) {
- printk(KERN_NOTICE DRV_NAME ": Link is down\n");
- printk(KERN_NOTICE DRV_NAME
- "please check your network connection\n");
- break;
- }
- count++;
- } while (!(phydat & 0x0004));
+ /* search for connect PHY device */
+ for (i = 0; i < PHY_MAX_ADDR; i++) {
+ struct phy_device *const tmp_phydev = lp->mii_bus.phy_map[i];
- phydat = read_phy_reg(phyaddr, PHYREG_ANLPAR);
+ if (!tmp_phydev)
+ continue; /* no PHY here... */
- if ((phydat & 0x0100) || (phydat & 0x0040)) {
- opmode = FDMODE;
- } else {
- opmode = 0;
- printk(KERN_INFO DRV_NAME
- ": Network is set to half duplex\n");
+ phydev = tmp_phydev;
+ break; /* found it */
+ }
+
+ /* now we are supposed to have a proper phydev, to attach to... */
+ if (!phydev) {
+ printk(KERN_INFO "%s: Don't found any phy device at all\n",
+ dev->name);
+ return -ENODEV;
}
#if defined(CONFIG_BFIN_MAC_RMII)
- opmode |= RMII; /* For Now only 100MBit are supported */
+ phydev = phy_connect(dev, phydev->dev.bus_id, &bf537_adjust_link, 0,
+ PHY_INTERFACE_MODE_RMII);
+#else
+ phydev = phy_connect(dev, phydev->dev.bus_id, &bf537_adjust_link, 0,
+ PHY_INTERFACE_MODE_MII);
#endif
- bfin_write_EMAC_OPMODE(opmode);
+ if (IS_ERR(phydev)) {
+ printk(KERN_ERR "%s: Could not attach to PHY\n", dev->name);
+ return PTR_ERR(phydev);
+ }
+
+ /* mask with MAC supported features */
+ phydev->supported &= (SUPPORTED_10baseT_Half
+ | SUPPORTED_10baseT_Full
+ | SUPPORTED_100baseT_Half
+ | SUPPORTED_100baseT_Full
+ | SUPPORTED_Autoneg
+ | SUPPORTED_Pause | SUPPORTED_Asym_Pause
+ | SUPPORTED_MII
+ | SUPPORTED_TP);
+
+ phydev->advertising = phydev->supported;
+
+ lp->old_link = 0;
+ lp->old_speed = 0;
+ lp->old_duplex = -1;
+ lp->phydev = phydev;
+
+ printk(KERN_INFO "%s: attached PHY driver [%s] "
+ "(mii_bus:phy_addr=%s, irq=%d)\n",
+ DRV_NAME, phydev->drv->name, phydev->dev.bus_id, phydev->irq);
+
+ return 0;
+}
+
+/**************************************************************************/
+void setup_system_regs(struct net_device *dev)
+{
+ unsigned short sysctl;
+
+ /*
+ * Odd word alignment for Receive Frame DMA word
+ * Configure checksum support and rcve frame word alignment
+ */
+ sysctl = bfin_read_EMAC_SYSCTL();
+#if defined(BFIN_MAC_CSUM_OFFLOAD)
+ sysctl |= RXDWA | RXCKS;
+#else
+ sysctl |= RXDWA;
+#endif
+ bfin_write_EMAC_SYSCTL(sysctl);
bfin_write_EMAC_MMC_CTL(RSTC | CROLL);
@@ -691,18 +745,18 @@ static void bf537mac_disable(void)
/*
* Enable Interrupts, Receive, and Transmit
*/
-static int bf537mac_enable(struct net_device *dev)
+static void bf537mac_enable(void)
{
u32 opmode;
- pr_debug("%s: %s\n", dev->name, __FUNCTION__);
+ pr_debug("%s: %s\n", DRV_NAME, __FUNCTION__);
/* Set RX DMA */
bfin_write_DMA1_NEXT_DESC_PTR(&(rx_list_head->desc_a));
bfin_write_DMA1_CONFIG(rx_list_head->desc_a.config);
/* Wait MII done */
- poll_mdc_done();
+ mdio_poll();
/* We enable only RX here */
/* ASTP : Enable Automatic Pad Stripping
@@ -726,8 +780,6 @@ static int bf537mac_enable(struct net_device *dev)
#endif
/* Turn on the EMAC rx */
bfin_write_EMAC_OPMODE(opmode);
-
- return 0;
}
/* Our watchdog timed out. Called by the networking layer */
@@ -740,7 +792,7 @@ static void bf537mac_timeout(struct net_device *dev)
/* reset tx queue */
tx_list_tail = tx_list_head->next;
- bf537mac_enable(dev);
+ bf537mac_enable();
/* We can accept TX packets again */
dev->trans_start = jiffies;
@@ -808,6 +860,7 @@ static void bf537mac_shutdown(struct net_device *dev)
*/
static int bf537mac_open(struct net_device *dev)
{
+ struct bf537mac_local *lp = netdev_priv(dev);
int retval;
pr_debug("%s: %s\n", dev->name, __FUNCTION__);
@@ -827,10 +880,10 @@ static int bf537mac_open(struct net_device *dev)
if (retval)
return retval;
- bf537mac_setphy(dev);
+ phy_start(lp->phydev);
setup_system_regs(dev);
bf537mac_disable();
- bf537mac_enable(dev);
+ bf537mac_enable();
pr_debug("hardware init finished\n");
netif_start_queue(dev);
@@ -847,11 +900,14 @@ static int bf537mac_open(struct net_device *dev)
*/
static int bf537mac_close(struct net_device *dev)
{
+ struct bf537mac_local *lp = netdev_priv(dev);
pr_debug("%s: %s\n", dev->name, __FUNCTION__);
netif_stop_queue(dev);
netif_carrier_off(dev);
+ phy_stop(lp->phydev);
+
/* clear everything */
bf537mac_shutdown(dev);
@@ -865,6 +921,7 @@ static int __init bf537mac_probe(struct net_device *dev)
{
struct bf537mac_local *lp = netdev_priv(dev);
int retval;
+ int i;
/* Grab the MAC address in the MAC */
*(__le32 *) (&(dev->dev_addr[0])) = cpu_to_le32(bfin_read_EMAC_ADDRLO());
@@ -881,7 +938,6 @@ static int __init bf537mac_probe(struct net_device *dev)
/* set the GPIO pins to Ethernet mode */
retval = setup_pin_mux(1);
-
if (retval)
return retval;
@@ -899,6 +955,23 @@ static int __init bf537mac_probe(struct net_device *dev)
setup_mac_addr(dev->dev_addr);
+ /* MDIO bus initial */
+ lp->mii_bus.priv = dev;
+ lp->mii_bus.read = mdiobus_read;
+ lp->mii_bus.write = mdiobus_write;
+ lp->mii_bus.reset = mdiobus_reset;
+ lp->mii_bus.name = "bfin_mac_mdio";
+ lp->mii_bus.id = 0;
+ lp->mii_bus.irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
+ for (i = 0; i < PHY_MAX_ADDR; ++i)
+ lp->mii_bus.irq[i] = PHY_POLL;
+
+ mdiobus_register(&lp->mii_bus);
+
+ retval = mii_probe(dev);
+ if (retval)
+ return retval;
+
/* Fill in the fields of the device structure with ethernet values. */
ether_setup(dev);
@@ -913,13 +986,6 @@ static int __init bf537mac_probe(struct net_device *dev)
dev->poll_controller = bf537mac_poll;
#endif
- /* fill in some of the fields */
- lp->version = 1;
- lp->PhyAddr = 0x01;
- lp->CLKIN = 25;
- lp->FullDuplex = 0;
- lp->Negotiate = 1;
- lp->FlowControl = 0;
spin_lock_init(&lp->lock);
/* now, enable interrupts */
@@ -932,9 +998,6 @@ static int __init bf537mac_probe(struct net_device *dev)
return -EBUSY;
}
- /* Enable PHY output early */
- if (!(bfin_read_VR_CTL() & PHYCLKOE))
- bfin_write_VR_CTL(bfin_read_VR_CTL() | PHYCLKOE);
retval = register_netdev(dev);
if (retval == 0) {
diff --git a/drivers/net/bfin_mac.h b/drivers/net/bfin_mac.h
index af87189..3a107ad 100644
--- a/drivers/net/bfin_mac.h
+++ b/drivers/net/bfin_mac.h
@@ -31,32 +31,6 @@
* 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
*/
-/*
- * PHY REGISTER NAMES
- */
-#define PHYREG_MODECTL 0x0000
-#define PHYREG_MODESTAT 0x0001
-#define PHYREG_PHYID1 0x0002
-#define PHYREG_PHYID2 0x0003
-#define PHYREG_ANAR 0x0004
-#define PHYREG_ANLPAR 0x0005
-#define PHYREG_ANER 0x0006
-#define PHYREG_NSR 0x0010
-#define PHYREG_LBREMR 0x0011
-#define PHYREG_REC 0x0012
-#define PHYREG_10CFG 0x0013
-#define PHYREG_PHY1_1 0x0014
-#define PHYREG_PHY1_2 0x0015
-#define PHYREG_PHY2 0x0016
-#define PHYREG_TW_1 0x0017
-#define PHYREG_TW_2 0x0018
-#define PHYREG_TEST 0x0019
-
-#define PHY_RESET 0x8000
-#define PHY_ANEG_EN 0x1000
-#define PHY_DUPLEX 0x0100
-#define PHY_SPD_SET 0x2000
-
#define BFIN_MAC_CSUM_OFFLOAD
struct dma_descriptor {
@@ -106,27 +80,16 @@ struct bf537mac_local {
*/
struct net_device_stats stats;
- int version;
-
- int FlowEnabled; /* record if data flow is active */
- int EtherIntIVG; /* IVG for the ethernet interrupt */
- int RXIVG; /* IVG for the RX completion */
- int TXIVG; /* IVG for the TX completion */
- int PhyAddr; /* PHY address */
- int OpMode; /* set these bits n the OPMODE regs */
- int Port10; /* set port speed to 10 Mbit/s */
- int GenChksums; /* IP checksums to be calculated */
- int NoRcveLnth; /* dont insert recv length at start of buffer */
- int StripPads; /* remove trailing pad bytes */
- int FullDuplex; /* set full duplex mode */
- int Negotiate; /* enable auto negotiation */
- int Loopback; /* loopback at the PHY */
- int Cache; /* Buffers may be cached */
- int FlowControl; /* flow control active */
- int CLKIN; /* clock in value in MHZ */
- unsigned short IntMask; /* interrupt mask */
unsigned char Mac[6]; /* MAC address of the board */
spinlock_t lock;
+
+ /* MII and PHY stuffs */
+ int old_link; /* used by bf537_adjust_link */
+ int old_speed;
+ int old_duplex;
+
+ struct phy_device *phydev;
+ struct mii_bus mii_bus;
};
extern void get_bf537_ether_addr(char *addr);
--
1.5.2
^ permalink raw reply related
* Re: Please pull 'adm8211' branch of wireless-2.6
From: Jeff Garzik @ 2007-09-16 3:26 UTC (permalink / raw)
To: John W. Linville
Cc: David Miller, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
flamingice-R9e9/4HEdknk1uMJSBkQmQ
In-Reply-To: <20070916023600.GA3967-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
John W. Linville wrote:
> On Sat, Sep 15, 2007 at 11:25:49AM -0700, David Miller wrote:
>> 4) Integrate linville's adm8211 driver into net-2.6.24
>>
>> We'll see how well that goes.
>
> It sounds like Michael will be respinning in response to Jeff's
> comments...?
It's already pulled, so I assume it will be in the form of a follow-up
patch.
Jeff
^ permalink raw reply
* Re: e1000 driver and samba
From: L F @ 2007-09-16 4:06 UTC (permalink / raw)
To: Kok, Auke; +Cc: James Chapman, netdev
In-Reply-To: <46EC2D5A.7080504@intel.com>
> >>> tx_deferred_ok: 486
>
> this one I wonder about, and might cause delays, I'll have to look up what it
> exactly could implicate though.
Please do and let me know. samba 3.0.26 helped, but the issue is still there.
> those are not "long frames" but the number of bytes the hardware counted in its
> "long" data type based byte counter.
Thank you for confirming that. It looks like it comes out to a little
over 32GB received... that sounds right.
> Auke
LF
^ permalink raw reply
* Re: e1000 driver and samba
From: L F @ 2007-09-16 4:07 UTC (permalink / raw)
To: James Chapman; +Cc: netdev
In-Reply-To: <46EC1A00.2000304@katalix.com>
On 9/15/07, James Chapman <jchapman@katalix.com> wrote:
> Are these long frames expected in your network? What is the MTU of the
> transmitting clients? Perhaps this might explain why reads work (because
> data is coming from the Linux box so the packets have smaller MTU) while
> writes cause delays or packet loss because the clients are sending long
> frames which are getting fragmented?
I doublechecked in any case. All machines on the network have the
default XP MTU of 1500. So does the linux machine. I therefore foresee
no problem there.
> James Chapman
LF
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox