Netdev List
 help / color / mirror / Atom feed
* Re: inconsistent lock state
From: David Miller @ 2010-03-15 21:09 UTC (permalink / raw)
  To: eric.dumazet
  Cc: oleg, mingo, sergey.senozhatsky, romieu, a.p.zijlstra, netdev,
	linux-kernel
In-Reply-To: <1268686865.2824.4.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 15 Mar 2010 22:01:05 +0100

> Also, this driver schedules a reset in case a fifo error is reported in
> rtl8169_rx_interrupt()
> 
>                 if (status & RxFOVF) {
>                         rtl8169_schedule_work(dev, rtl8169_reset_task);
>                         dev->stats.rx_fifo_errors++;
>                 }
> 
> 
> This seems very strange too : In case of a RX spike, we reset card ?

It's possible that this has been found to hang the card.

If so, it should be documented because otherwise yes we
should not be doing this.

^ permalink raw reply

* Re: inconsistent lock state
From: Eric Dumazet @ 2010-03-15 21:01 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Ingo Molnar, Sergey Senozhatsky, Francois Romieu, Peter Zijlstra,
	netdev, linux-kernel, David Miller
In-Reply-To: <20100308125122.GA11242@redhat.com>

Le lundi 08 mars 2010 à 13:51 +0100, Oleg Nesterov a écrit :
> Sergey Senozhatsky wrote:
> >
> > Hello,
> >
> > Hardly reproducible.
> > /*
> > * 2.6.33. x86. ASUS f3jc
> > */
> >
> > [329645.010697] =================================
> > [329645.010699] [ INFO: inconsistent lock state ]
> > [329645.010703] 2.6.33-33-0-dbg #31
> > [329645.010705] ---------------------------------
> > [329645.010708] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> >
> > ...
> >
> > [329645.011083]  [<c128858d>] netif_receive_skb+0x340/0x360
> > [329645.011093]  [<fd1ab6f4>] rtl8169_rx_interrupt+0x2bf/0x37e [r8169]
> > [329645.011100]  [<fd1aba02>] rtl8169_reset_task+0x38/0xcd [r8169]
> > [329645.011105]  [<c1044f71>] worker_thread+0x1ac/0x27c
> 
> I don't understand this code, but at first glance drivers/net/r8169.c
> is obviously buggy.
> 
> The work->func, rtl8169_reset_task(), calls rtl8169_rx_interrupt() ->
> netif_receive_skb(), and the last one may only be called from softirq.
> 

Yes, this is wrong. In this context (process context, not softirq), we
should use netif_rx() or just drop frames if we are in reset phase.

Also, this driver schedules a reset in case a fifo error is reported in
rtl8169_rx_interrupt()

                if (status & RxFOVF) {
                        rtl8169_schedule_work(dev, rtl8169_reset_task);
                        dev->stats.rx_fifo_errors++;
                }


This seems very strange too : In case of a RX spike, we reset card ?

^ permalink raw reply

* [PATCH 0/4] Some typo fixing
From: Thomas Weber @ 2010-03-15 20:55 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Karsten Keil, Lin Ming, Takashi Iwai, Benjamin Herrenschmidt,
	Jaroslav Kysela, Pavel Machek, David Brownell,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA, KOSAKI Motohiro, Bjorn Helgaas,
	Rusty Russell, John W. Linville, Steve Conklin, Ralph Campbell,
	Anton Vorontsov, cbe-oss-dev-mnsaURCQ41sdnm+yROfE0A,
	Liam Girdwood, Anthony Liguori, Jiri Kosina, Randy Dunlap,
	Tejun Heo

I have fixed some typos.

Thomas Weber (4):
  Fix typo: [Ss]ytem => [Ss]ystem
  Fix typo: udpate => update
  Fix typo: paramters => parameters
  Fix typo: orginal => original

 Documentation/cgroups/cgroups.txt               |    2 +-
 Documentation/kbuild/kconfig.txt                |    2 +-
 Documentation/sysfs-rules.txt                   |    2 +-
 Documentation/trace/events.txt                  |    8 ++++----
 drivers/acpi/osl.c                              |    4 ++--
 drivers/ata/ata_piix.c                          |    2 +-
 drivers/firewire/ohci.c                         |    2 +-
 drivers/gpu/drm/drm_bufs.c                      |    2 +-
 drivers/infiniband/hw/ipath/ipath_iba6110.c     |    2 +-
 drivers/infiniband/hw/ipath/ipath_iba6120.c     |    4 ++--
 drivers/infiniband/hw/ipath/ipath_iba7220.c     |    2 +-
 drivers/isdn/hisax/hfc4s8s_l1.c                 |    2 +-
 drivers/macintosh/windfarm_pm81.c               |    2 +-
 drivers/media/dvb/dvb-usb/friio-fe.c            |    2 +-
 drivers/net/smsc911x.c                          |    4 ++--
 drivers/pci/hotplug/cpqphp_core.c               |    2 +-
 drivers/pci/pci.c                               |    2 +-
 drivers/ps3/ps3-sys-manager.c                   |    2 +-
 drivers/regulator/core.c                        |    2 +-
 drivers/s390/char/sclp_cpi_sys.c                |    2 +-
 drivers/scsi/bfa/include/defs/bfa_defs_cee.h    |    2 +-
 drivers/scsi/bfa/include/defs/bfa_defs_status.h |    4 ++--
 drivers/spi/spi_mpc8xxx.c                       |    2 +-
 drivers/staging/iio/Documentation/overview.txt  |    2 +-
 drivers/staging/rt2860/rtmp.h                   |    2 +-
 drivers/staging/rtl8187se/r8180_core.c          |    4 ++--
 drivers/staging/rtl8187se/r8180_dm.c            |    2 +-
 drivers/staging/rtl8187se/r8185b_init.c         |    2 +-
 drivers/virtio/virtio_pci.c                     |    2 +-
 fs/jfs/jfs_dmap.c                               |    2 +-
 kernel/cgroup.c                                 |    2 +-
 mm/page_alloc.c                                 |    2 +-
 net/wimax/op-rfkill.c                           |    2 +-
 sound/pci/emu10k1/emu10k1_main.c                |    2 +-
 34 files changed, 42 insertions(+), 42 deletions(-)

^ permalink raw reply

* Re: [PATCH v2.6.33 2/2] jme: Adding lock to protect vlgrp structure.
From: Guo-Fu Tseng @ 2010-03-15 19:13 UTC (permalink / raw)
  To: Laurent Chavey; +Cc: David Miller, Ethan Hsiao, linux-netdev
In-Reply-To: <97949e3e1003151122q68ce19f4ja7f53ad380d3033f@mail.gmail.com>

Hi Laurent:

The vlan_rx_register is called through ioctl.
And the packet feeding is called in the tasklet.
I see no lock in register_vlan_dev(), register_vlan_device(), and vlan_ioctl_handler()
which is related to the vlan_hwaccel_receive_skb(), vlan_hwaccel_rx().

It prevents the vlgrp pointer be modified while trying to feed the packet.
Ex:
> if (jme->vlgrp) {
True.
vlan_rx_register called, vlgrp set to NULL.
>         jme->jme_vlan_rx(skb, jme->vlgrp,
>                          le16_to_cpu(rxdesc->descwb.vlan));
passing vlgrp with the value "NULL" to jme_vlan_rx.
Or it might even be modified during the vlan_hwaccel_{receive_skb|rx} call.

Please correct me if I'm wrong.

On Mon, 15 Mar 2010 11:22:48 -0700, Laurent Chavey wrote
> what does the spinlock protect ?
> 

--
Guo-Fu Tseng


^ permalink raw reply

* Re: [PATCH] net/netfilter/ipvs/ip_vs_ftp.c: Remove use of NIPQUAD
From: David Miller @ 2010-03-15 19:01 UTC (permalink / raw)
  To: kaber; +Cc: joe, wensong, horms, ja, netdev, lvs-devel, linux-kernel
In-Reply-To: <4B9E5E43.9060206@trash.net>

From: Patrick McHardy <kaber@trash.net>
Date: Mon, 15 Mar 2010 17:20:19 +0100

> Joe Perches wrote:
>> NIPQUAD has very few uses left.
>> 
>> Remove this use and make the code have the identical form of the only
>> other use of "%u,%u,%u,%u,%u,%u" in net/ipv4/netfilter/nf_nat_ftp.c
> 
> Are we trying to remove NIPQUAD?

Yes.


^ permalink raw reply

* Re: [Regression] r8169: enable 64-bit DMA by default for PCI Express devices (v2)
From: David Miller @ 2010-03-15 18:57 UTC (permalink / raw)
  To: joern; +Cc: hancockrwd, torvalds, linux-kernel, netdev
In-Reply-To: <20100315151041.GA15667@Dublin.logfs.org>

From: Jörn Engel <joern@dublin.logfs.org>
Date: Mon, 15 Mar 2010 16:10:41 +0100

> Forgot to Cc: lkml.

Actually, what you really need to CC: is netdev, added.
> On Mon, 15 March 2010 16:08:06 +0100, J�rn Engel wrote:
>> 
>> Hello Robert!
>> 
>> Looks like 353176888386d9025062a12dcec08d49af10cf2c broke one of my
>> machines.  Reverting that patch fixes things again.  Machine is
>> running an x86_64 kernel.  Lspci gives me this:
>> 
>> 01:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8101E/RTL8102E PCI Express Fast Ethernet controller (rev 02)
>>         Subsystem: ASRock Incorporation Device 8136
>>         Flags: bus master, fast devsel, latency 0, IRQ 26
>>         I/O ports at e800 [size=256]
>>         Memory at fdeff000 (64-bit, prefetchable) [size=4K]
>>         Memory at fdee0000 (64-bit, prefetchable) [size=64K]
>>         [virtual] Expansion ROM at fde00000 [disabled] [size=128K]
>>         Capabilities: [40] Power Management version 3
>>         Capabilities: [50] MSI: Enable+ Count=1/1 Maskable- 64bit+
>>         Capabilities: [70] Express Endpoint, MSI 01
>>         Capabilities: [ac] MSI-X: Enable- Count=2 Masked-
>>         Capabilities: [cc] Vital Product Data
>>         Capabilities: [100] Advanced Error Reporting
>>         Capabilities: [140] Virtual Channel <?>
>>         Capabilities: [160] Device Serial Number 08-00-00-00-ff-ff-00-00
>>         Kernel driver in use: r8169
>> 
>> If you have any test patches to try, please send them my way. :)
>> 
>> J�rn

^ permalink raw reply

* Re: [PATCH] NET: tg3: fix brace, switch, and comma spacing coding style in tg3.c This is a patch to the tg3.c file that aligns switch and case statements, else statements following braces, and inserts a space after commas listed in the coding style g
From: Matt Carlson @ 2010-03-15 18:39 UTC (permalink / raw)
  To: Joe Perches
  Cc: David Miller, thisdyingdream@gmail.com, sjharms@gmail.com,
	Matthew Carlson, Michael Chan, Benjamin Li,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	sharms@ubuntu.com
In-Reply-To: <1268511742.30289.74.camel@Joe-Laptop.home>

I have a patch awaiting net-next that does these kinds of things.  I
can roll (most of) Steven's changes into that patch and get it all
done in one shot if you like.

On Sat, Mar 13, 2010 at 12:22:22PM -0800, Joe Perches wrote:
> On Sat, 2010-03-13 at 11:55 -0800, David Miller wrote:
> > From: Steven Harms <thisdyingdream@gmail.com>
> > Date: Sat, 13 Mar 2010 14:52:26 -0500
> > 
> > > Would you like me to submit the patch adding only the spaces after
> > > commas and leave the rest of the defines as is, or discard the changes
> > > all together?
> > 
> > I would like you to toss them altogether.
> 
> I think that Steven got off on the wrong foot
> a bit by adding the parentheses to the defines.
> 
> I think the other changes he suggested are
> tree-wide style consistency only and are ok.
> 
> Perhaps you might reconsider later, after
> net-next reopens.
> 
> 
> 


^ permalink raw reply

* Re: [PATCH v2.6.33 2/2] jme: Adding lock to protect vlgrp structure.
From: Laurent Chavey @ 2010-03-15 18:22 UTC (permalink / raw)
  To: cooldavid; +Cc: David Miller, linux-netdev, Ethan Hsiao
In-Reply-To: <1268630132-10410-2-git-send-email-cooldavid@cooldavid.org>

what does the spinlock protect ?


On Sun, Mar 14, 2010 at 10:15 PM,  <cooldavid@cooldavid.org> wrote:
> From: Guo-Fu Tseng <cooldavid@cooldavid.org>
>
> Adding a lock to prevent modifying the vlgrp structure while receiving
> VLAN packet.
>
> Signed-off-by: Guo-Fu Tseng <cooldavid@cooldavid.org>
> ---
>  drivers/net/jme.c |    6 ++++++
>  drivers/net/jme.h |    1 +
>  2 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/jme.c b/drivers/net/jme.c
> index 3da390a..f10d9db 100644
> --- a/drivers/net/jme.c
> +++ b/drivers/net/jme.c
> @@ -942,11 +942,14 @@ jme_alloc_and_feed_skb(struct jme_adapter *jme, int idx)
>                        skb->ip_summed = CHECKSUM_NONE;
>
>                if (rxdesc->descwb.flags & cpu_to_le16(RXWBFLAG_TAGON)) {
> +                       spin_lock(&jme->vlgrp_lock);
>                        if (jme->vlgrp) {
>                                jme->jme_vlan_rx(skb, jme->vlgrp,
>                                        le16_to_cpu(rxdesc->descwb.vlan));
> +                               spin_unlock(&jme->vlgrp_lock);
>                                NET_STAT(jme).rx_bytes += 4;
>                        } else {
> +                               spin_unlock(&jme->vlgrp_lock);
>                                dev_kfree_skb(skb);
>                        }
>                } else {
> @@ -2092,7 +2095,9 @@ jme_vlan_rx_register(struct net_device *netdev, struct vlan_group *grp)
>  {
>        struct jme_adapter *jme = netdev_priv(netdev);
>
> +       spin_lock_bh(&jme->vlgrp_lock);
>        jme->vlgrp = grp;
> +       spin_unlock_bh(&jme->vlgrp_lock);
>  }
>
>  static void
> @@ -2759,6 +2764,7 @@ jme_init_one(struct pci_dev *pdev,
>        spin_lock_init(&jme->phy_lock);
>        spin_lock_init(&jme->macaddr_lock);
>        spin_lock_init(&jme->rxmcs_lock);
> +       spin_lock_init(&jme->vlgrp_lock);
>
>        atomic_set(&jme->link_changing, 1);
>        atomic_set(&jme->rx_cleaning, 1);
> diff --git a/drivers/net/jme.h b/drivers/net/jme.h
> index 251abed..fbde5c5 100644
> --- a/drivers/net/jme.h
> +++ b/drivers/net/jme.h
> @@ -420,6 +420,7 @@ struct jme_adapter {
>        spinlock_t              phy_lock;
>        spinlock_t              macaddr_lock;
>        spinlock_t              rxmcs_lock;
> +       spinlock_t              vlgrp_lock;
>        struct tasklet_struct   rxempty_task;
>        struct tasklet_struct   rxclean_task;
>        struct tasklet_struct   txclean_task;
> --
> 1.6.4.4
>
> --
> 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
>

^ permalink raw reply

* [PATCH] tipc: Increase frequency of load distribution over broadcast link
From: Neil Horman @ 2010-03-15 18:02 UTC (permalink / raw)
  To: netdev; +Cc: davem, nhorman, allan.stephens

Forward port commit 29eb572941501c40ac6e62dbc5043bf9ee76ee56
from git://tipc.cslab.ericsson.net/pub/git/people/allan/tipc.git

Origional commit message:
Increase frequency of load distribution over broadcast link

This patch enhances the behavior of TIPC's broadcast link so that it
alternates between redundant bearers (if available) after every
message sent, rather than after every 10 messages.  This change helps
to speed up delivery of retransmitted messages by ensuring that
they are not sent repeatedly over a bearer that is no longer working,
but not yet recognized as failed.

Tested by myself in the latest net-2.6 tree using the tipc sanity test suite

Origionally-signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


bcast.c |   35 ++++++++++++++---------------------
1 file changed, 14 insertions(+), 21 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index a3bfd40..90a0519 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -558,10 +558,7 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 			      struct tipc_bearer *unused1,
 			      struct tipc_media_addr *unused2)
 {
-	static int send_count = 0;
-
 	int bp_index;
-	int swap_time;
 
 	/* Prepare buffer for broadcasting (if first time trying to send it) */
 
@@ -575,11 +572,6 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 		msg_set_mc_netid(msg, tipc_net_id);
 	}
 
-	/* Determine if bearer pairs should be swapped following this attempt */
-
-	if ((swap_time = (++send_count >= 10)))
-		send_count = 0;
-
 	/* Send buffer over bearers until all targets reached */
 
 	bcbearer->remains = tipc_cltr_bcast_nodes;
@@ -595,21 +587,22 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 		if (bcbearer->remains_new.count == bcbearer->remains.count)
 			continue;	/* bearer pair doesn't add anything */
 
-		if (!p->publ.blocked &&
-		    !p->media->send_msg(buf, &p->publ, &p->media->bcast_addr)) {
-			if (swap_time && s && !s->publ.blocked)
-				goto swap;
-			else
-				goto update;
+		if (p->publ.blocked ||
+		    p->media->send_msg(buf, &p->publ, &p->media->bcast_addr)) {
+			/* unable to send on primary bearer */
+			if (!s || s->publ.blocked ||
+			    s->media->send_msg(buf, &s->publ,
+					       &s->media->bcast_addr)) {
+				/* unable to send on either bearer */
+				continue;
+			}
+		}
+
+		if (s) {
+			bcbearer->bpairs[bp_index].primary = s;
+			bcbearer->bpairs[bp_index].secondary = p;
 		}
 
-		if (!s || s->publ.blocked ||
-		    s->media->send_msg(buf, &s->publ, &s->media->bcast_addr))
-			continue;	/* unable to send using bearer pair */
-swap:
-		bcbearer->bpairs[bp_index].primary = s;
-		bcbearer->bpairs[bp_index].secondary = p;
-update:
 		if (bcbearer->remains_new.count == 0)
 			return 0;
 

^ permalink raw reply related

* [PATCH] tipc: Allow retransmission of cloned buffers
From: Neil Horman @ 2010-03-15 17:58 UTC (permalink / raw)
  To: netdev; +Cc: davem, allan.stephens, nhorman

Forward port commit
fc477e160af086f6e30c3d4fdf5f5c000d29beb5
from git://tipc.cslab.ericsson.net/pub/git/people/allan/tipc.git

Origional commit message:

Allow retransmission of cloned buffers

This patch fixes an issue with TIPC's message retransmission logic
that prevented retransmission of clone sk_buffs.  Originally intended
as a means of avoiding wasted work in retransmitting messages that
were still on the driver's outbound queue, it also prevented TIPC
from retransmitting messages through other means -- such as the
secondary bearer of the broadcast link, or another interface in a
set of bonded interfaces.  This fix removes existing checks for
cloned sk_buffs that prevented such retransmission.

Origionally-Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

diff --git a/net/tipc/link.c b/net/tipc/link.c
index 1a7e466..49f2be8 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1553,7 +1553,7 @@ u32 tipc_link_push_packet(struct link *l_ptr)
 
 	/* Continue retransmission now, if there is anything: */
 
-	if (r_q_size && buf && !skb_cloned(buf)) {
+	if (r_q_size && buf) {
 		msg_set_ack(buf_msg(buf), mod(l_ptr->next_in_no - 1));
 		msg_set_bcast_ack(buf_msg(buf), l_ptr->owner->bclink.last_in);
 		if (tipc_bearer_send(l_ptr->b_ptr, buf, &l_ptr->media_addr)) {
@@ -1722,15 +1722,16 @@ void tipc_link_retransmit(struct link *l_ptr, struct sk_buff *buf,
 	dbg("Retransmitting %u in link %x\n", retransmits, l_ptr);
 
 	if (tipc_bearer_congested(l_ptr->b_ptr, l_ptr)) {
-		if (!skb_cloned(buf)) {
+		if (l_ptr->retransm_queue_size == 0) {
 			msg_dbg(msg, ">NO_RETR->BCONG>");
 			dbg_print_link(l_ptr, "   ");
 			l_ptr->retransm_queue_head = msg_seqno(msg);
 			l_ptr->retransm_queue_size = retransmits;
-			return;
 		} else {
-			/* Don't retransmit if driver already has the buffer */
+			err("Unexpected retransmit on link %s (qsize=%d)\n",
+			    l_ptr->name, l_ptr->retransm_queue_size);
 		}
+		return;
 	} else {
 		/* Detect repeated retransmit failures on uncongested bearer */
 
@@ -1745,7 +1746,7 @@ void tipc_link_retransmit(struct link *l_ptr, struct sk_buff *buf,
 		}
 	}
 
-	while (retransmits && (buf != l_ptr->next_out) && buf && !skb_cloned(buf)) {
+	while (retransmits && (buf != l_ptr->next_out) && buf) {
 		msg = buf_msg(buf);
 		msg_set_ack(msg, mod(l_ptr->next_in_no - 1));
 		msg_set_bcast_ack(msg, l_ptr->owner->bclink.last_in);

^ permalink raw reply related

* Fw: [Bug 15541] New: POLLIN bit is set when there is OOB data only
From: Stephen Hemminger @ 2010-03-15 17:26 UTC (permalink / raw)
  To: David Miller; +Cc: netdev



Begin forwarded message:

Date: Mon, 15 Mar 2010 16:19:44 GMT
From: bugzilla-daemon@bugzilla.kernel.org
To: shemminger@linux-foundation.org
Subject: [Bug 15541] New: POLLIN bit is set when there is OOB data only


http://bugzilla.kernel.org/show_bug.cgi?id=15541

           Summary: POLLIN bit is set when there is OOB data only
           Product: Networking
           Version: 2.5
    Kernel Version: 2.6.28, 2.6.32, 2.6.33
          Platform: All
        OS/Version: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: IPV4
        AssignedTo: shemminger@linux-foundation.org
        ReportedBy: Alexandra.Kossovsky@oktetlabs.ru
                CC: davem@davemloft.net
        Regression: Yes


Starting from 2.6.28, Linux kernel has incorrect behaviour when poll() is
called on TCP socket with out-of-band byte received.

When TCP socket has OOB byte available (and no normal data), old Linux,
Solaris, FreeBSD and other systems return POLLPRI bit without POLLRD.  Starting
from 2.6.28, Linux returns POLLRD | POLLPRI | POLLRDNORM.

The broken commt is
http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.28.y.git;a=commitdiff;h=c7004482e8dcb7c3c72666395cfa98a216a4fb70
.
Following 1-line patch fixes the problem:
--- a/net/ipv4/tcp.c 2010-03-15 19:13:45.000000000 +0300
+++ b/net/ipv4/tcp.c      2010-03-15 19:13:21.000000000 +0300
@@ -428,7 +428,7 @@
                if (tp->urg_seq == tp->copied_seq &&
                    !sock_flag(sk, SOCK_URGINLINE) &&
                    tp->urg_data)
-                       target--;
+                       target++;

                /* Potential race condition. If read of tp below will
                 * escape above sk->sk_state, we can be illegally awaken

-- 
Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


-- 

^ permalink raw reply

* RE: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
From: John Linn @ 2010-03-15 17:23 UTC (permalink / raw)
  To: Stephen Neuendorffer, netdev, linuxppc-dev, grant.likely, jwboyer
  Cc: michal.simek, John Tyner, john.williams
In-Reply-To: <977C41F842E66D4CB2E41332313B615009A27BCA@XSJ-EXCHVS1.xlnx.xilinx.com>


> -----Original Message-----
> From: Stephen Neuendorffer
> Sent: Monday, March 15, 2010 11:03 AM
> To: John Linn; netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
grant.likely@secretlab.ca;
> jwboyer@linux.vnet.ibm.com
> Cc: michal.simek@petalogix.com; John Tyner;
john.williams@petalogix.com
> Subject: RE: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
> 
> 
> 
> > -----Original Message-----
> > From:
linuxppc-dev-bounces+stephen=neuendorffer.name@lists.ozlabs.org
[mailto:linuxppc-dev-
> > bounces+stephen=neuendorffer.name@lists.ozlabs.org] On Behalf Of
John Linn
> > Sent: Friday, March 12, 2010 5:06 PM
> > To: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
grant.likely@secretlab.ca;
> > jwboyer@linux.vnet.ibm.com
> > Cc: michal.simek@petalogix.com; John Tyner; John Linn;
john.williams@petalogix.com
> > Subject: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
> >
> > This patch adds support for using the LL TEMAC Ethernet driver on
> > non-Virtex 5 platforms by adding support for accessing the Soft DMA
> > registers as if they were memory mapped instead of solely through
the
> > DCR's (available on the Virtex 5).
> >
> > The patch also updates the driver so that it runs on the MicroBlaze.
> > The changes were tested on the PowerPC 440, PowerPC 405, and the
> > MicroBlaze platforms.
> >
> > Signed-off-by: John Tyner <jtyner@cs.ucr.edu>
> > Signed-off-by: John Linn <john.linn@xilinx.com>
> > ---
> >
> > V2 - Incorporated comments from Grant and added more logic to allow
the driver
> > to work on MicroBlaze.
> >
> >  drivers/net/Kconfig         |    1 -
> >  drivers/net/ll_temac.h      |   17 +++++-
> >  drivers/net/ll_temac_main.c |  124
++++++++++++++++++++++++++++++++++---------
> >  3 files changed, 113 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> > index 9b6efe1..5402105 100644
> > --- a/drivers/net/Kconfig
> > +++ b/drivers/net/Kconfig
> > @@ -2443,7 +2443,6 @@ config MV643XX_ETH
> >  config XILINX_LL_TEMAC
> >  	tristate "Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC)
driver"
> >  	select PHYLIB
> > -	depends on PPC_DCR_NATIVE
> >  	help
> >  	  This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
> >  	  core used in Xilinx Spartan and Virtex FPGAs
> > diff --git a/drivers/net/ll_temac.h b/drivers/net/ll_temac.h
> > index 1af66a1..915aa34 100644
> > --- a/drivers/net/ll_temac.h
> > +++ b/drivers/net/ll_temac.h
> > @@ -5,8 +5,11 @@
> >  #include <linux/netdevice.h>
> >  #include <linux/of.h>
> >  #include <linux/spinlock.h>
> > +
> > +#ifdef CONFIG_PPC_DCR
> >  #include <asm/dcr.h>
> >  #include <asm/dcr-regs.h>
> > +#endif
> >
> >  /* packet size info */
> >  #define XTE_HDR_SIZE			14      /* size of
Ethernet header */
> > @@ -290,8 +293,12 @@ This option defaults to enabled (set) */
> >
> >  #define TX_CONTROL_CALC_CSUM_MASK   1
> >
> > +/* Align the IP data in the packet on word boundaries as MicroBlaze
> > + * needs it.
> > + */
> > +
> >  #define XTE_ALIGN       32
> > -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN)
> > +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN)
> 
> Is '34' really XTE_ALIGN + 2?  (I really have no idea.... it just
looks like a suspicious change.)
> 
> Steve

Valid point that it is XTE_ALIGN + 2. As the comment says, it aligns the
IP data in the packet.

-- John

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.



^ permalink raw reply

* Re: [PATCH] net/netfilter/ipvs/ip_vs_ftp.c: Remove use of NIPQUAD
From: Patrick McHardy @ 2010-03-15 17:04 UTC (permalink / raw)
  To: Joe Perches
  Cc: Wensong Zhang, Simon Horman, Julian Anastasov, David S. Miller,
	netdev, lvs-devel, LKML
In-Reply-To: <1268672314.4027.46.camel@Joe-Laptop.home>

Joe Perches wrote:
> On Mon, 2010-03-15 at 17:20 +0100, Patrick McHardy wrote:
>> Joe Perches wrote:
>>> NIPQUAD has very few uses left.
>>> Remove this use and make the code have the identical form of the only
>>> other use of "%u,%u,%u,%u,%u,%u" in net/ipv4/netfilter/nf_nat_ftp.c
>> Are we trying to remove NIPQUAD? 
> 
> That is a goal for me yes.
> Elimination of unnecessary stuff from kernel.h is useful.
> 
>> In my opinion the current code is
>> preferrable to open-coding NIPQUAD.
> 
> $ grep -r --include=*.[ch] NIPQUAD *
> include/linux/kernel.h:#define NIPQUAD(addr) \
> include/linux/kernel.h:#define NIPQUAD_FMT "%u.%u.%u.%u"
> net/netfilter/ipvs/ip_vs_ftp.c:		sprintf(buf, "%u,%u,%u,%u,%u,%u", NIPQUAD(from.ip),
> 
> As of right now NIPQUAD_FMT is unused and
> could be removed from kernel.h
> 
> There's 1 use of NIPQUAD.
> 
> Maybe instead of removal the #define could be
> moved to netfilter.h or netfilter_ipv4.h
> 
> Another is to consolidate the two uses of
> "%u,%u,%u,%u,%u,%u"
> 
> $ grep -r --include=*.[ch] "%u,%u,%u,%u,%u,u%" *
> fs/cachefiles/bind.c:	_enter("{%u,%u,%u,%u,%u,%u},%s",
> net/ipv4/netfilter/nf_nat_ftp.c:		return snprintf(buffer, buflen, "%u,%u,%u,%u,%u,%u",
> net/netfilter/ipvs/ip_vs_ftp.c:		sprintf(buf, "%u,%u,%u,%u,%u,%u", NIPQUAD(from.ip),
> 
> Maybe these 2 open coded sprintf/snprintf could be
> consolidated and moved to an appropriate include
> like netfilter_ipv4 or another.
> 
> What do you suggest?

Well, removing the last user sounds acceptable :) Applied, thanks.

^ permalink raw reply

* RE: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
From: Stephen Neuendorffer @ 2010-03-15 17:03 UTC (permalink / raw)
  To: John Linn, netdev, linuxppc-dev, grant.likely, jwboyer
  Cc: michal.simek, John Tyner, john.williams
In-Reply-To: <bda26c8d-00b2-411d-b4b7-ad6954b9049e@SG2EHSMHS005.ehs.local>



> -----Original Message-----
> From: linuxppc-dev-bounces+stephen=neuendorffer.name@lists.ozlabs.org
[mailto:linuxppc-dev-
> bounces+stephen=neuendorffer.name@lists.ozlabs.org] On Behalf Of John
Linn
> Sent: Friday, March 12, 2010 5:06 PM
> To: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
grant.likely@secretlab.ca;
> jwboyer@linux.vnet.ibm.com
> Cc: michal.simek@petalogix.com; John Tyner; John Linn;
john.williams@petalogix.com
> Subject: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
> 
> This patch adds support for using the LL TEMAC Ethernet driver on
> non-Virtex 5 platforms by adding support for accessing the Soft DMA
> registers as if they were memory mapped instead of solely through the
> DCR's (available on the Virtex 5).
> 
> The patch also updates the driver so that it runs on the MicroBlaze.
> The changes were tested on the PowerPC 440, PowerPC 405, and the
> MicroBlaze platforms.
> 
> Signed-off-by: John Tyner <jtyner@cs.ucr.edu>
> Signed-off-by: John Linn <john.linn@xilinx.com>
> ---
> 
> V2 - Incorporated comments from Grant and added more logic to allow
the driver
> to work on MicroBlaze.
> 
>  drivers/net/Kconfig         |    1 -
>  drivers/net/ll_temac.h      |   17 +++++-
>  drivers/net/ll_temac_main.c |  124
++++++++++++++++++++++++++++++++++---------
>  3 files changed, 113 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 9b6efe1..5402105 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -2443,7 +2443,6 @@ config MV643XX_ETH
>  config XILINX_LL_TEMAC
>  	tristate "Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC)
driver"
>  	select PHYLIB
> -	depends on PPC_DCR_NATIVE
>  	help
>  	  This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
>  	  core used in Xilinx Spartan and Virtex FPGAs
> diff --git a/drivers/net/ll_temac.h b/drivers/net/ll_temac.h
> index 1af66a1..915aa34 100644
> --- a/drivers/net/ll_temac.h
> +++ b/drivers/net/ll_temac.h
> @@ -5,8 +5,11 @@
>  #include <linux/netdevice.h>
>  #include <linux/of.h>
>  #include <linux/spinlock.h>
> +
> +#ifdef CONFIG_PPC_DCR
>  #include <asm/dcr.h>
>  #include <asm/dcr-regs.h>
> +#endif
> 
>  /* packet size info */
>  #define XTE_HDR_SIZE			14      /* size of Ethernet
header */
> @@ -290,8 +293,12 @@ This option defaults to enabled (set) */
> 
>  #define TX_CONTROL_CALC_CSUM_MASK   1
> 
> +/* Align the IP data in the packet on word boundaries as MicroBlaze
> + * needs it.
> + */
> +
>  #define XTE_ALIGN       32
> -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN)
> +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN)

Is '34' really XTE_ALIGN + 2?  (I really have no idea.... it just looks
like a suspicious change.)

Steve

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.



^ permalink raw reply

* Re: [PATCH] net/netfilter/ipvs/ip_vs_ftp.c: Remove use of NIPQUAD
From: Joe Perches @ 2010-03-15 16:58 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Wensong Zhang, Simon Horman, Julian Anastasov, David S. Miller,
	netdev, lvs-devel, LKML
In-Reply-To: <4B9E5E43.9060206@trash.net>

On Mon, 2010-03-15 at 17:20 +0100, Patrick McHardy wrote:
> Joe Perches wrote:
> > NIPQUAD has very few uses left.
> > Remove this use and make the code have the identical form of the only
> > other use of "%u,%u,%u,%u,%u,%u" in net/ipv4/netfilter/nf_nat_ftp.c
> 
> Are we trying to remove NIPQUAD? 

That is a goal for me yes.
Elimination of unnecessary stuff from kernel.h is useful.

> In my opinion the current code is
> preferrable to open-coding NIPQUAD.

$ grep -r --include=*.[ch] NIPQUAD *
include/linux/kernel.h:#define NIPQUAD(addr) \
include/linux/kernel.h:#define NIPQUAD_FMT "%u.%u.%u.%u"
net/netfilter/ipvs/ip_vs_ftp.c:		sprintf(buf, "%u,%u,%u,%u,%u,%u", NIPQUAD(from.ip),

As of right now NIPQUAD_FMT is unused and
could be removed from kernel.h

There's 1 use of NIPQUAD.

Maybe instead of removal the #define could be
moved to netfilter.h or netfilter_ipv4.h

Another is to consolidate the two uses of
"%u,%u,%u,%u,%u,%u"

$ grep -r --include=*.[ch] "%u,%u,%u,%u,%u,u%" *
fs/cachefiles/bind.c:	_enter("{%u,%u,%u,%u,%u,%u},%s",
net/ipv4/netfilter/nf_nat_ftp.c:		return snprintf(buffer, buflen, "%u,%u,%u,%u,%u,%u",
net/netfilter/ipvs/ip_vs_ftp.c:		sprintf(buf, "%u,%u,%u,%u,%u,%u", NIPQUAD(from.ip),

Maybe these 2 open coded sprintf/snprintf could be
consolidated and moved to an appropriate include
like netfilter_ipv4 or another.

What do you suggest?


^ permalink raw reply

* Re: [RFC v2 00/10] snet: Security for NETwork syscalls
From: Samir Bellabes @ 2010-03-15 16:43 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: linux-kernel, netdev, netfilter-devel, hadi, kaber, zbr, nhorman,
	root, linux-security-module
In-Reply-To: <201003071445.FJB39029.QLSHtOFOJFOVMF@I-love.SAKURA.ne.jp>

Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:

> Hello.
>
> Samir Bellabes wrote:
>> Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:
>> 
>> > Regarding [RFC v2 09/10] snet: introduce snet_ticket
>> > +enum snet_verdict snet_ticket_check(struct snet_info *info)
>> > +{
>> > +       struct snet_ticket *st = NULL;
>> > +       unsigned int h = 0, verdict = SNET_VERDICT_NONE;
>> > +       struct list_head *l = NULL;
>> > +       struct snet_task_security *tsec = NULL;
>> > +
>> > +       if (snet_ticket_mode == SNET_TICKET_OFF)
>> > +               goto out;
>> > +
>> > +       tsec = (struct snet_task_security*) current_security();
>> > +
>> > +       h = jhash_2words(info->syscall, info->protocol, 0) % HSIZE;
>> > +       l = &tsec->hash[h];
>> > +
>> > +       read_lock_bh(&tsec->lock);
>> >
>> > Credentials are allocated for copy-on-write basis.
>> > Sharing "tsec" among multiple "struct task_struct" is what you intended?
>> 
>> No, there is no shared "tsec".
>> snet_ticket_check() is called from the process context. So "tsec" is
>> a pointer to the "void *security" pointer from its own "struct
>> task_struct".
>>
> Until 2.6.28:
>
>   "void *security" is directory attached to "struct task_struct".
>   copy_process() calls security_task_alloc().
>
>   Therefore, task1->security != task2->security is guaranteed as long as you do
>
>     task->security = kmalloc();
>
>   at security_task_alloc().
>
> Since 2.6.29:
>
>   "void *security" is attached to "struct cred", and "struct cred *" is
>   attached to "struct task_struct". copy_process() calls copy_creds() and
>   prepare_creds() calls security_prepare_creds(). But copy_creds() does not
>   call prepare_creds() for clone(CLONE_THREAD) case.
>
>   Therefore, task1->cred->security != task2->cred->security is not guaranteed
>   even if you do
>
>     cred->security = kmalloc();
>
>   at security_prepare_creds().
>
>> every task_struct have a "tsec" allocated to its "void *security"
>> pointer. 
>
> You meant to have assigned "void *security" dedicated to "struct task_struct".
> But "void *security" is no longer directly attached to "struct task_struct".
> I couldn't find code that checks whether "current->cred" is used by only
> current thread or not. "current->cred" being used by only current thread is
> a requirement for having a "tsec" allocated to every "struct task_struct".
>
> Your code will share "tsec" among multiple threads if a process created
> threads using clone(CLONE_THREAD | CLONE_SIGHAND | CLONE_VM). Each thread has
> its own "struct task_struct" but they share "cred->security".
> Sharing "tsec" among multiple threads is what you intended?

Hello Tetsuo,

ok, I get your point now.
It seems that this is the common behaviour of credentials, so yes, "tsec"
will be shared between this kind of threads, and it makes sense.

Maybe, I need to protect read-write access to this "tsec" structure.

thank you for this comment Tetsuo,

sam

^ permalink raw reply

* TBF parameters: overhead and linklayer ?
From: Julien Vehent @ 2010-03-15 16:42 UTC (permalink / raw)
  To: Netdev

Hello,

TC_TBF man page seem outdated (even in latest version of iproute2) and
doesn't explain what those two parameters do:

# tc qdisc add tbf help
Usage: ... tbf limit BYTES burst BYTES[/BYTES] rate KBPS [ mtu
BYTES[/BYTES] ]
        [ peakrate KBPS ] [ latency TIME ] [ overhead BYTES ] [ linklayer
TYPE ]


 - overhead ?
 - linklayer ?

Or did I miss something ?


Thanks,
Julien

^ permalink raw reply

* Re: [RFC PATCH net-next 0/7 v2]IPv6:netfilter: defragment
From: Patrick McHardy @ 2010-03-15 16:27 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki
  Cc: Shan Wei, YOSHIFUJI Hideaki, David Miller, Alexey Dobriyan,
	Yasuyuki KOZAKAI, netdev@vger.kernel.org, netfilter-devel
In-Reply-To: <4B9B9766.3090200@linux-ipv6.org>

YOSHIFUJI Hideaki wrote:
> (2010/03/11 18:16), Shan Wei wrote:
>>> On the other hand, I'd even say we should NOT send
>>> icmp here (at least by default) because standard routers
>>> never send such packet.
>>
>> Yes,for routers, the patch-set does not send icmp message to
>> source host. It only does on destination host with IPv6 connection
>> track enable.
> 
> Please make it optional (via parameter) at least.

The ICMP messages are only sent if the packet is destined for the
local host, similar to what IPv6 defrag would do if conntrack wouldn't
be used. So this patch increases consistency, why should we make this
optional?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] net/netfilter/ipvs/ip_vs_ftp.c: Remove use of NIPQUAD
From: Patrick McHardy @ 2010-03-15 16:20 UTC (permalink / raw)
  To: Joe Perches
  Cc: Wensong Zhang, Simon Horman, Julian Anastasov, David S. Miller,
	netdev, lvs-devel, LKML
In-Reply-To: <1268094699.1617.5.camel@Joe-Laptop.home>

Joe Perches wrote:
> NIPQUAD has very few uses left.
> 
> Remove this use and make the code have the identical form of the only
> other use of "%u,%u,%u,%u,%u,%u" in net/ipv4/netfilter/nf_nat_ftp.c

Are we trying to remove NIPQUAD? In my opinion the current code is
preferrable to open-coding NIPQUAD.

^ permalink raw reply

* Re: [PATCH] vsprintf.c: remove stack variable ksym from
From: Paulo Marques @ 2010-03-15 15:01 UTC (permalink / raw)
  To: Joe Perches
  Cc: Linus Torvalds, Andrew Morton, Nick Andrew,
	Linux Kernel Mailing List, Greg Kroah-Hartman, netdev,
	Bjorn Helgaas
In-Reply-To: <1268510079.30289.69.camel@Joe-Laptop.home>

Joe Perches wrote:
> On Sat, 2010-03-13 at 09:44 -0800, Joe Perches wrote:
>> On Sat, 2010-03-13 at 07:35 -0800, Linus Torvalds wrote:
>>> On Fri, 12 Mar 2010, Andrew Morton wrote:
>>>> nice.
>>> But the kallsyms_lookup()/sprint_symbol() functions don't take a 
>>> length parameter, so we have to do the worst-case thing (which itself has 
>>> tons of unnecessary padding).
>> Perhaps a new snprint_symbol function with the
>> other kallsyms... functions changed as necessary.
> 
> Perhaps something like this:

Just one minor nit:

[...]
>  
> -	*result = '\0';
> +	if (size)
> +		*result = '\0';

This test seems to be here to handle the "size == 0" case, but

>[...]
> +const char *kallsyms_lookup_n(unsigned long addr,
> +			      unsigned long *symbolsize,
> +			      unsigned long *offset,
> +			      char **modname, char *namebuf, size_t size)
> +{
> +	if (size)
> +		namebuf[size - 1] = 0;
> +	namebuf[0] = 0;

here we seem to write the namebuf[0] even if "size == 0". So maybe both
assignments in this function should be inside the "if (size)" test.

Other than that, the patch looks good:

Reviewed-by: Paulo Marques <pmarques@grupopie.com>

-- 
Paulo Marques - www.grupopie.com

"As far as we know, our computer has never had an undetected error."
Weisert

^ permalink raw reply

* Re: [PATCH 0/3] gigaset fixes for 2.6.34-rc1
From: Karsten Keil @ 2010-03-15 15:00 UTC (permalink / raw)
  To: Tilman Schmidt
  Cc: David Miller, Hansjoerg Lipp, Alan Cox, isdn4linux, i4ldeveloper,
	netdev, linux-kernel
In-Reply-To: <cover.1268580322.git.tilman@imap.cc>

On Sonntag, 14. März 2010 23:58:05 Tilman Schmidt wrote:
> Karsten, David,
> 
> following are three bugfix patches to the Gigaset driver which should
> still go into 2.6.34 if possible, and also into the 2.6.33 stable branch.

Ack for all

> 
> Thanks,
> Tilman
> 
> Tilman Schmidt (3):
>   gigaset: avoid registering CAPI driver more than once
>   gigaset: correct clearing of at_state strings on RING
>   gigaset: prune use of tty_buffer_request_room
> 
>  drivers/isdn/gigaset/capi.c      |   44
>  +++++++++++++++++++++---------------- drivers/isdn/gigaset/common.c    |  
>   6 +++-
>  drivers/isdn/gigaset/ev-layer.c  |   12 +++-------
>  drivers/isdn/gigaset/gigaset.h   |    6 +++-
>  drivers/isdn/gigaset/i4l.c       |   28 +++++++++++++++++-------
>  drivers/isdn/gigaset/interface.c |    1 -
>  6 files changed, 57 insertions(+), 40 deletions(-)
> 

^ permalink raw reply

* RE: [PATCH 2/2] davinci: introduce EMAC PHY clock usage
From: Nori, Sekhar @ 2010-03-15 14:59 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: netdev@vger.kernel.org,
	davinci-linux-open-source@linux.davincidsp.com,
	Govindarajan, Sriramakrishnan
In-Reply-To: <87wrxh5foj.fsf@deeprootsystems.com>

On Sat, Mar 13, 2010 at 04:08:20, Kevin Hilman wrote:
> Sriramakrishnan <srk@ti.com> writes:
>
> > From: Sekhar Nori <nsekhar@ti.com>
> >
> > The patch "TI DaVinci EMAC: Add EMAC PHY clock handling" adds
> > support for enabling and disabling the EMAC PHY clock.
> >
> > The PHY clock on all DaVinci boards is derived from a fixed
> > on board clock. This patch adds the PHY clock definition to
> > the clock tree for all the DaVinci boards using EMAC. Also,
> > the existing input to EMAC module is differentiated from the
> > PHY clock using the clock name "emac_clk".
> >
> > Without this patch ethernet fails to initialize since it cannot
> > get the PHY clock and EMAC clock.
> >
> > Tested on EVM boards for DM365, DM6467, DM644x, DA830 and DA850.
> >
> > Signed-off-by: Sekhar Nori <nsekhar@ti.com>

[...]

> >
> > +#define EMAC_PHY_CLK_RATE  50000000
> > +
> > +static struct clk emac_phy = {
> > +   .name   = "emac_phy",
> > +   .rate   = EMAC_PHY_CLK_RATE,
> > +};
> > +
> > +static struct clk_lookup emac_phy_clks[] = {
> > +   CLK("davinci_emac.1", "phy_clk", &emac_phy),
>

[...]

>
> > +   CLK(NULL, NULL, NULL),
> > +};
> > +
>
> I'm not crazy about the clock definitions in the board files.  I
> assume you put it here instead of <soc>.c is because each clock
> has a board specific rate.

That and the fact that none of the DaVinci devices have an
integrated PHY on the SoC. The clock comes from an onboard
oscillator too. So, it seemed odd to include phy clock structure
in the SoC specific file.

>
> Instead, what I'd rather see is the clock defined once for each
> <soc>.c with a custom set_rate hook.  The default rate could
> be a per-SoC default (25MHz looks common) and any board files
> that don't use the default can use clk_set_rate() to change it.
>
> This approach should simplfy things and minimize changes to board
> files.

Right, that surely will lead to a much simpler patch so I will
go ahead and change. Also, only DA850 offers a choice between
MII (25MHz) and RMII (50MHz) phy. So, the set_rate needs to be
implemented only for this SoC.

Thanks,
Sekhar


^ permalink raw reply

* Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
From: Michal Simek @ 2010-03-15 14:57 UTC (permalink / raw)
  To: John Linn
  Cc: netdev, linuxppc-dev, grant.likely, jwboyer, john.williams,
	John Tyner
In-Reply-To: <89d94bf2-02cf-492c-89cb-31f0ac1501ba@SG2EHSMHS010.ehs.local>

John Linn wrote:
>> -----Original Message-----
>> From: Michal Simek [mailto:michal.simek@petalogix.com]
>> Sent: Monday, March 15, 2010 8:40 AM
>> To: John Linn
>> Cc: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
> grant.likely@secretlab.ca;
>> jwboyer@linux.vnet.ibm.com; john.williams@petalogix.com; John Tyner
>> Subject: Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
>>
>> John Linn wrote:
>>>> -----Original Message-----
>>>> From: Michal Simek [mailto:michal.simek@petalogix.com]
>>>> Sent: Monday, March 15, 2010 2:40 AM
>>>> To: John Linn
>>>> Cc: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
>>> grant.likely@secretlab.ca;
>>>> jwboyer@linux.vnet.ibm.com; john.williams@petalogix.com; John Tyner
>>>> Subject: Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC
> driver
>>>> John Linn wrote:
>>>>> This patch adds support for using the LL TEMAC Ethernet driver on
>>>>> non-Virtex 5 platforms by adding support for accessing the Soft
> DMA
>>>>> registers as if they were memory mapped instead of solely through
>>> the
>>>>> DCR's (available on the Virtex 5).
>>>>>
>>>>> The patch also updates the driver so that it runs on the
> MicroBlaze.
>>>>> The changes were tested on the PowerPC 440, PowerPC 405, and the
>>>>> MicroBlaze platforms.
>>>> Which git-tree have you tested on? (Of course microblaze)
>>> It was tested on the Xilinx tree for MicroBlaze which is based on
> the
>>> mainline and the Petalogix tree as DMA was needed. I tried to build
>>> against the mainline head but got errors with the DMA routines. I
> guess
>>> it's possible that it was a configuration issue there as I didn't
> dig
>>> real deep.
>> New dma api is in for-linus branch.
>> I tested it on that version and I am seeing some weird things. :-(
>> Access to bad area. I will try your tree.
>>
>> The second thing which I see is in ll_temac_recv function.
>> On the following line is read a packet length which could be 0-16k.
>> 		length = cur_p->app4 & 0x3FFF;
>>
>> But allocated skb has max size XTE_MAX_JUMBO_FRAME_SIZE + XTE_ALIGN.
>>
>> What happen if driver get packet greater than 9kB?
>> I got it (I don't know how) but skb_put has one checking mechanism
> which
>> will cal skb_over_panic which caused panic.
> 
> That's the whole reason for that panic to me and when I got it in the
> past and looked it up it made sense to me.  I personally don't see a
> good reason to check for it in the driver since the kernel catches it,
> but maybe others do.

yes, kernel is checking it but caused panic which ends in fault in 
kernel and caused kernel crash. :-( This could be a good reason to check 
it in driver. I don't want to reset the board when ll_temac gets longer 
packet, do you?

I have seen the second type of fault which caused kernel access to bad 
area (+ kernel panic). Have you done any iperf tests or flood ping?

Thanks,
Michal

> 
> Thanks,
> John
> 
>> That's why I think that will be good always to check that length is
> less
>> than XTE_MAX_JUMBO_FRAME_SIZE + XTE_ALIGN.
>>
>> What do you think?
>>
>> Thanks,
>> Michal
>>
>>> I tested the PowerPC against the head of mainline.
>>>
>>> Thanks,
>>> John
>>>
>>>> Michal
>>>>
>>>>> Signed-off-by: John Tyner <jtyner@cs.ucr.edu>
>>>>> Signed-off-by: John Linn <john.linn@xilinx.com>
>>>>> ---
>>>>>
>>>>> V2 - Incorporated comments from Grant and added more logic to
> allow
>>> the driver
>>>>> to work on MicroBlaze.
>>>>>
>>>>>  drivers/net/Kconfig         |    1 -
>>>>>  drivers/net/ll_temac.h      |   17 +++++-
>>>>>  drivers/net/ll_temac_main.c |  124
>>> ++++++++++++++++++++++++++++++++++---------
>>>>>  3 files changed, 113 insertions(+), 29 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>>>>> index 9b6efe1..5402105 100644
>>>>> --- a/drivers/net/Kconfig
>>>>> +++ b/drivers/net/Kconfig
>>>>> @@ -2443,7 +2443,6 @@ config MV643XX_ETH
>>>>>  config XILINX_LL_TEMAC
>>>>>  	tristate "Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC)
>>> driver"
>>>>>  	select PHYLIB
>>>>> -	depends on PPC_DCR_NATIVE
>>>>>  	help
>>>>>  	  This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
>>>>>  	  core used in Xilinx Spartan and Virtex FPGAs
>>>>> diff --git a/drivers/net/ll_temac.h b/drivers/net/ll_temac.h
>>>>> index 1af66a1..915aa34 100644
>>>>> --- a/drivers/net/ll_temac.h
>>>>> +++ b/drivers/net/ll_temac.h
>>>>> @@ -5,8 +5,11 @@
>>>>>  #include <linux/netdevice.h>
>>>>>  #include <linux/of.h>
>>>>>  #include <linux/spinlock.h>
>>>>> +
>>>>> +#ifdef CONFIG_PPC_DCR
>>>>>  #include <asm/dcr.h>
>>>>>  #include <asm/dcr-regs.h>
>>>>> +#endif
>>>>>
>>>>>  /* packet size info */
>>>>>  #define XTE_HDR_SIZE			14      /* size of
>>> Ethernet header */
>>>>> @@ -290,8 +293,12 @@ This option defaults to enabled (set) */
>>>>>
>>>>>  #define TX_CONTROL_CALC_CSUM_MASK   1
>>>>>
>>>>> +/* Align the IP data in the packet on word boundaries as
> MicroBlaze
>>>>> + * needs it.
>>>>> + */
>>>>> +
>>>>>  #define XTE_ALIGN       32
>>>>> -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN)
>>>>> +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN)
>>>>>
>>>>>  #define MULTICAST_CAM_TABLE_NUM 4
>>>>>
>>>>> @@ -335,9 +342,15 @@ struct temac_local {
>>>>>  	struct mii_bus *mii_bus;	/* MII bus reference */
>>>>>  	int mdio_irqs[PHY_MAX_ADDR];	/* IRQs table for MDIO bus */
>>>>>
>>>>> -	/* IO registers and IRQs */
>>>>> +	/* IO registers, dma functions and IRQs */
>>>>>  	void __iomem *regs;
>>>>> +	void __iomem *sdma_regs;
>>>>> +#ifdef CONFIG_PPC_DCR
>>>>>  	dcr_host_t sdma_dcrs;
>>>>> +#endif
>>>>> +	u32 (*dma_in)(struct temac_local *, int);
>>>>> +	void (*dma_out)(struct temac_local *, int, u32);
>>>>> +
>>>>>  	int tx_irq;
>>>>>  	int rx_irq;
>>>>>  	int emac_num;
>>>>> diff --git a/drivers/net/ll_temac_main.c
>>> b/drivers/net/ll_temac_main.c
>>>>> index a18e348..9aedf9b 100644
>>>>> --- a/drivers/net/ll_temac_main.c
>>>>> +++ b/drivers/net/ll_temac_main.c
>>>>> @@ -20,9 +20,6 @@
>>>>>   *   or rx, so this should be okay.
>>>>>   *
>>>>>   * TODO:
>>>>> - * - Fix driver to work on more than just Virtex5.  Right now the
>>> driver
>>>>> - *   assumes that the locallink DMA registers are accessed via
> DCR
>>>>> - *   instructions.
>>>>>   * - Factor out locallink DMA code into separate driver
>>>>>   * - Fix multicast assignment.
>>>>>   * - Fix support for hardware checksumming.
>>>>> @@ -115,17 +112,86 @@ void temac_indirect_out32(struct temac_local
>>> *lp, int reg, u32 value)
>>>>>  	temac_iow(lp, XTE_CTL0_OFFSET, CNTLREG_WRITE_ENABLE_MASK | reg);
>>>>>  }
>>>>>
>>>>> +/**
>>>>> + * temac_dma_in32 - Memory mapped DMA read, this function expects
> a
>>>>> + * register input that is based on DCR word addresses which
>>>>> + * are then converted to memory mapped byte addresses
>>>>> + */
>>>>>  static u32 temac_dma_in32(struct temac_local *lp, int reg)
>>>>>  {
>>>>> -	return dcr_read(lp->sdma_dcrs, reg);
>>>>> +	return in_be32((u32 *)(lp->sdma_regs + (reg << 2)));
>>>>>  }
>>>>>
>>>>> +/**
>>>>> + * temac_dma_out32 - Memory mapped DMA read, this function
> expects
>>> a
>>>>> + * register input that is based on DCR word addresses which
>>>>> + * are then converted to memory mapped byte addresses
>>>>> + */
>>>>>  static void temac_dma_out32(struct temac_local *lp, int reg, u32
>>> value)
>>>>>  {
>>>>> +	out_be32((u32 *)(lp->sdma_regs + (reg << 2)), value);
>>>>> +}
>>>>> +
>>>>> +/* DMA register access functions can be DCR based or memory
> mapped.
>>>>> + * The PowerPC 440 is DCR based, the PowerPC 405 and MicroBlaze
> are
>>> both
>>>>> + * memory mapped.
>>>>> + */
>>>>> +#ifdef CONFIG_PPC_DCR
>>>>> +
>>>>> +/**
>>>>> + * temac_dma_dcr_in32 - DCR based DMA read
>>>>> + */
>>>>> +static u32 temac_dma_dcr_in(struct temac_local *lp, int reg)
>>>>> +{
>>>>> +	return dcr_read(lp->sdma_dcrs, reg);
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * temac_dma_dcr_out32 - DCR based DMA write
>>>>> + */
>>>>> +static void temac_dma_dcr_out(struct temac_local *lp, int reg,
> u32
>>> value)
>>>>> +{
>>>>>  	dcr_write(lp->sdma_dcrs, reg, value);
>>>>>  }
>>>>>
>>>>>  /**
>>>>> + * temac_dcr_setup - If the DMA is DCR based, then setup the
>>> address and
>>>>> + * I/O  functions
>>>>> + */
>>>>> +static int temac_dcr_setup(struct temac_local *lp, struct
> of_device
>>> *op,
>>>>> +				struct device_node *np)
>>>>> +{
>>>>> +	unsigned int dcrs;
>>>>> +
>>>>> +	/* setup the dcr address mapping if it's in the device tree */
>>>>> +
>>>>> +	dcrs = dcr_resource_start(np, 0);
>>>>> +	if (dcrs != 0) {
>>>>> +		lp->sdma_dcrs = dcr_map(np, dcrs, dcr_resource_len(np,
>>> 0));
>>>>> +		lp->dma_in = temac_dma_dcr_in;
>>>>> +		lp->dma_out = temac_dma_dcr_out;
>>>>> +		dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
>>>>> +		return 0;
>>>>> +	}
>>>>> +	/* no DCR in the device tree, indicate a failure */
>>>>> +	return -1;
>>>>> +}
>>>>> +
>>>>> +#else
>>>>> +
>>>>> +/*
>>>>> + * temac_dcr_setup - This is a stub for when DCR is not
> supported,
>>>>> + * such as with MicroBlaze
>>>>> + */
>>>>> +static int temac_dcr_setup(struct temac_local *lp, struct
> of_device
>>> *op,
>>>>> +				struct device_node *np)
>>>>> +{
>>>>> +	return -1;
>>>>> +}
>>>>> +
>>>>> +#endif
>>>>> +
>>>>> +/**
>>>>>   * temac_dma_bd_init - Setup buffer descriptor rings
>>>>>   */
>>>>>  static int temac_dma_bd_init(struct net_device *ndev)
>>>>> @@ -172,23 +238,23 @@ static int temac_dma_bd_init(struct
> net_device
>>> *ndev)
>>>>>  		lp->rx_bd_v[i].app0 = STS_CTRL_APP0_IRQONEND;
>>>>>  	}
>>>>>
>>>>> -	temac_dma_out32(lp, TX_CHNL_CTRL, 0x10220400 |
>>>>> +	lp->dma_out(lp, TX_CHNL_CTRL, 0x10220400 |
>>>>>  					  CHNL_CTRL_IRQ_EN |
>>>>>  					  CHNL_CTRL_IRQ_DLY_EN |
>>>>>  					  CHNL_CTRL_IRQ_COAL_EN);
>>>>>  	/* 0x10220483 */
>>>>>  	/* 0x00100483 */
>>>>> -	temac_dma_out32(lp, RX_CHNL_CTRL, 0xff010000 |
>>>>> +	lp->dma_out(lp, RX_CHNL_CTRL, 0xff010000 |
>>>>>  					  CHNL_CTRL_IRQ_EN |
>>>>>  					  CHNL_CTRL_IRQ_DLY_EN |
>>>>>  					  CHNL_CTRL_IRQ_COAL_EN |
>>>>>  					  CHNL_CTRL_IRQ_IOE);
>>>>>  	/* 0xff010283 */
>>>>>
>>>>> -	temac_dma_out32(lp, RX_CURDESC_PTR,  lp->rx_bd_p);
>>>>> -	temac_dma_out32(lp, RX_TAILDESC_PTR,
>>>>> +	lp->dma_out(lp, RX_CURDESC_PTR,  lp->rx_bd_p);
>>>>> +	lp->dma_out(lp, RX_TAILDESC_PTR,
>>>>>  		       lp->rx_bd_p + (sizeof(*lp->rx_bd_v) * (RX_BD_NUM
>>> - 1)));
>>>>> -	temac_dma_out32(lp, TX_CURDESC_PTR, lp->tx_bd_p);
>>>>> +	lp->dma_out(lp, TX_CURDESC_PTR, lp->tx_bd_p);
>>>>>
>>>>>  	return 0;
>>>>>  }
>>>>> @@ -426,9 +492,9 @@ static void temac_device_reset(struct
> net_device
>>> *ndev)
>>>>>  	temac_indirect_out32(lp, XTE_RXC1_OFFSET, val &
>>> ~XTE_RXC1_RXEN_MASK);
>>>>>  	/* Reset Local Link (DMA) */
>>>>> -	temac_dma_out32(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
>>>>> +	lp->dma_out(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
>>>>>  	timeout = 1000;
>>>>> -	while (temac_dma_in32(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
>>>>> +	while (lp->dma_in(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
>>>>>  		udelay(1);
>>>>>  		if (--timeout == 0) {
>>>>>  			dev_err(&ndev->dev,
>>>>> @@ -436,7 +502,7 @@ static void temac_device_reset(struct
> net_device
>>> *ndev)
>>>>>  			break;
>>>>>  		}
>>>>>  	}
>>>>> -	temac_dma_out32(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
>>>>> +	lp->dma_out(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
>>>>>
>>>>>  	temac_dma_bd_init(ndev);
>>>>>
>>>>> @@ -597,7 +663,7 @@ static int temac_start_xmit(struct sk_buff
> *skb,
>>> struct net_device *ndev)
>>>>>  		lp->tx_bd_tail = 0;
>>>>>
>>>>>  	/* Kick off the transfer */
>>>>> -	temac_dma_out32(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
>>>>> +	lp->dma_out(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
>>>>>
>>>>>  	return NETDEV_TX_OK;
>>>>>  }
>>>>> @@ -663,7 +729,7 @@ static void ll_temac_recv(struct net_device
>>> *ndev)
>>>>>  		cur_p = &lp->rx_bd_v[lp->rx_bd_ci];
>>>>>  		bdstat = cur_p->app0;
>>>>>  	}
>>>>> -	temac_dma_out32(lp, RX_TAILDESC_PTR, tail_p);
>>>>> +	lp->dma_out(lp, RX_TAILDESC_PTR, tail_p);
>>>>>
>>>>>  	spin_unlock_irqrestore(&lp->rx_lock, flags);
>>>>>  }
>>>>> @@ -674,8 +740,8 @@ static irqreturn_t ll_temac_tx_irq(int irq,
> void
>>> *_ndev)
>>>>>  	struct temac_local *lp = netdev_priv(ndev);
>>>>>  	unsigned int status;
>>>>>
>>>>> -	status = temac_dma_in32(lp, TX_IRQ_REG);
>>>>> -	temac_dma_out32(lp, TX_IRQ_REG, status);
>>>>> +	status = lp->dma_in(lp, TX_IRQ_REG);
>>>>> +	lp->dma_out(lp, TX_IRQ_REG, status);
>>>>>
>>>>>  	if (status & (IRQ_COAL | IRQ_DLY))
>>>>>  		temac_start_xmit_done(lp->ndev);
>>>>> @@ -692,8 +758,8 @@ static irqreturn_t ll_temac_rx_irq(int irq,
> void
>>> *_ndev)
>>>>>  	unsigned int status;
>>>>>
>>>>>  	/* Read and clear the status registers */
>>>>> -	status = temac_dma_in32(lp, RX_IRQ_REG);
>>>>> -	temac_dma_out32(lp, RX_IRQ_REG, status);
>>>>> +	status = lp->dma_in(lp, RX_IRQ_REG);
>>>>> +	lp->dma_out(lp, RX_IRQ_REG, status);
>>>>>
>>>>>  	if (status & (IRQ_COAL | IRQ_DLY))
>>>>>  		ll_temac_recv(lp->ndev);
>>>>> @@ -794,7 +860,7 @@ static ssize_t temac_show_llink_regs(struct
>>> device *dev,
>>>>>  	int i, len = 0;
>>>>>
>>>>>  	for (i = 0; i < 0x11; i++)
>>>>> -		len += sprintf(buf + len, "%.8x%s", temac_dma_in32(lp,
>>> i),
>>>>> +		len += sprintf(buf + len, "%.8x%s", lp->dma_in(lp, i),
>>>>>  			       (i % 8) == 7 ? "\n" : " ");
>>>>>  	len += sprintf(buf + len, "\n");
>>>>>
>>>>> @@ -820,7 +886,6 @@ temac_of_probe(struct of_device *op, const
>>> struct of_device_id *match)
>>>>>  	struct net_device *ndev;
>>>>>  	const void *addr;
>>>>>  	int size, rc = 0;
>>>>> -	unsigned int dcrs;
>>>>>
>>>>>  	/* Init network device structure */
>>>>>  	ndev = alloc_etherdev(sizeof(*lp));
>>>>> @@ -870,13 +935,20 @@ temac_of_probe(struct of_device *op, const
>>> struct of_device_id *match)
>>>>>  		goto nodev;
>>>>>  	}
>>>>>
>>>>> -	dcrs = dcr_resource_start(np, 0);
>>>>> -	if (dcrs == 0) {
>>>>> -		dev_err(&op->dev, "could not get DMA register
>>> address\n");
>>>>> -		goto nodev;
>>>>> +	/* Setup the DMA register accesses, could be DCR or memory
>>> mapped */
>>>>> +	if (temac_dcr_setup(lp, op, np)) {
>>>>> +
>>>>> +		/* no DCR in the device tree, try non-DCR */
>>>>> +		lp->sdma_regs = of_iomap(np, 0);
>>>>> +		if (lp->sdma_regs) {
>>>>> +			lp->dma_in = temac_dma_in32;
>>>>> +			lp->dma_out = temac_dma_out32;
>>>>> +			dev_dbg(&op->dev, "MEM base: %p\n",
>>> lp->sdma_regs);
>>>>> +		} else {
>>>>> +			dev_err(&op->dev, "unable to map DMA
>>> registers\n");
>>>>> +			goto nodev;
>>>>> +		}
>>>>>  	}
>>>>> -	lp->sdma_dcrs = dcr_map(np, dcrs, dcr_resource_len(np, 0));
>>>>> -	dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
>>>>>
>>>>>  	lp->rx_irq = irq_of_parse_and_map(np, 0);
>>>>>  	lp->tx_irq = irq_of_parse_and_map(np, 1);
>>>> --
>>>> Michal Simek, Ing. (M.Eng)
>>>> PetaLogix - Linux Solutions for a Reconfigurable World
>>>> w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f:
>>> +61-7-30090663
>>>
>>>
>>> This email and any attachments are intended for the sole use of the
> named recipient(s) and
>> contain(s) confidential information that may be proprietary,
> privileged or copyrighted under
>> applicable law. If you are not the intended recipient, do not read,
> copy, or forward this email
>> message or any attachments. Delete this email message and any
> attachments immediately.
>>>
>>
>> --
>> Michal Simek, Ing. (M.Eng)
>> PetaLogix - Linux Solutions for a Reconfigurable World
>> w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f:
> +61-7-30090663
> 
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 
> 


-- 
Michal Simek, Ing. (M.Eng)
PetaLogix - Linux Solutions for a Reconfigurable World
w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f: +61-7-30090663

^ permalink raw reply

* RE: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
From: John Linn @ 2010-03-15 14:51 UTC (permalink / raw)
  To: michal.simek
  Cc: netdev, linuxppc-dev, grant.likely, jwboyer, john.williams,
	John Tyner
In-Reply-To: <4B9E46B5.1060508@petalogix.com>

> -----Original Message-----
> From: Michal Simek [mailto:michal.simek@petalogix.com]
> Sent: Monday, March 15, 2010 8:40 AM
> To: John Linn
> Cc: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
grant.likely@secretlab.ca;
> jwboyer@linux.vnet.ibm.com; john.williams@petalogix.com; John Tyner
> Subject: Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
> 
> John Linn wrote:
> >> -----Original Message-----
> >> From: Michal Simek [mailto:michal.simek@petalogix.com]
> >> Sent: Monday, March 15, 2010 2:40 AM
> >> To: John Linn
> >> Cc: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
> > grant.likely@secretlab.ca;
> >> jwboyer@linux.vnet.ibm.com; john.williams@petalogix.com; John Tyner
> >> Subject: Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC
driver
> >>
> >> John Linn wrote:
> >>> This patch adds support for using the LL TEMAC Ethernet driver on
> >>> non-Virtex 5 platforms by adding support for accessing the Soft
DMA
> >>> registers as if they were memory mapped instead of solely through
> > the
> >>> DCR's (available on the Virtex 5).
> >>>
> >>> The patch also updates the driver so that it runs on the
MicroBlaze.
> >>> The changes were tested on the PowerPC 440, PowerPC 405, and the
> >>> MicroBlaze platforms.
> >> Which git-tree have you tested on? (Of course microblaze)
> >
> > It was tested on the Xilinx tree for MicroBlaze which is based on
the
> > mainline and the Petalogix tree as DMA was needed. I tried to build
> > against the mainline head but got errors with the DMA routines. I
guess
> > it's possible that it was a configuration issue there as I didn't
dig
> > real deep.
> 
> New dma api is in for-linus branch.
> I tested it on that version and I am seeing some weird things. :-(
> Access to bad area. I will try your tree.
> 
> The second thing which I see is in ll_temac_recv function.
> On the following line is read a packet length which could be 0-16k.
> 		length = cur_p->app4 & 0x3FFF;
> 
> But allocated skb has max size XTE_MAX_JUMBO_FRAME_SIZE + XTE_ALIGN.
> 
> What happen if driver get packet greater than 9kB?
> I got it (I don't know how) but skb_put has one checking mechanism
which
> will cal skb_over_panic which caused panic.

That's the whole reason for that panic to me and when I got it in the
past and looked it up it made sense to me.  I personally don't see a
good reason to check for it in the driver since the kernel catches it,
but maybe others do.

Thanks,
John

> That's why I think that will be good always to check that length is
less
> than XTE_MAX_JUMBO_FRAME_SIZE + XTE_ALIGN.
> 
> What do you think?
> 
> Thanks,
> Michal
> 
> >
> > I tested the PowerPC against the head of mainline.
> >
> > Thanks,
> > John
> >
> >> Michal
> >>
> >>> Signed-off-by: John Tyner <jtyner@cs.ucr.edu>
> >>> Signed-off-by: John Linn <john.linn@xilinx.com>
> >>> ---
> >>>
> >>> V2 - Incorporated comments from Grant and added more logic to
allow
> > the driver
> >>> to work on MicroBlaze.
> >>>
> >>>  drivers/net/Kconfig         |    1 -
> >>>  drivers/net/ll_temac.h      |   17 +++++-
> >>>  drivers/net/ll_temac_main.c |  124
> > ++++++++++++++++++++++++++++++++++---------
> >>>  3 files changed, 113 insertions(+), 29 deletions(-)
> >>>
> >>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> >>> index 9b6efe1..5402105 100644
> >>> --- a/drivers/net/Kconfig
> >>> +++ b/drivers/net/Kconfig
> >>> @@ -2443,7 +2443,6 @@ config MV643XX_ETH
> >>>  config XILINX_LL_TEMAC
> >>>  	tristate "Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC)
> > driver"
> >>>  	select PHYLIB
> >>> -	depends on PPC_DCR_NATIVE
> >>>  	help
> >>>  	  This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
> >>>  	  core used in Xilinx Spartan and Virtex FPGAs
> >>> diff --git a/drivers/net/ll_temac.h b/drivers/net/ll_temac.h
> >>> index 1af66a1..915aa34 100644
> >>> --- a/drivers/net/ll_temac.h
> >>> +++ b/drivers/net/ll_temac.h
> >>> @@ -5,8 +5,11 @@
> >>>  #include <linux/netdevice.h>
> >>>  #include <linux/of.h>
> >>>  #include <linux/spinlock.h>
> >>> +
> >>> +#ifdef CONFIG_PPC_DCR
> >>>  #include <asm/dcr.h>
> >>>  #include <asm/dcr-regs.h>
> >>> +#endif
> >>>
> >>>  /* packet size info */
> >>>  #define XTE_HDR_SIZE			14      /* size of
> > Ethernet header */
> >>> @@ -290,8 +293,12 @@ This option defaults to enabled (set) */
> >>>
> >>>  #define TX_CONTROL_CALC_CSUM_MASK   1
> >>>
> >>> +/* Align the IP data in the packet on word boundaries as
MicroBlaze
> >>> + * needs it.
> >>> + */
> >>> +
> >>>  #define XTE_ALIGN       32
> >>> -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN)
> >>> +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN)
> >>>
> >>>  #define MULTICAST_CAM_TABLE_NUM 4
> >>>
> >>> @@ -335,9 +342,15 @@ struct temac_local {
> >>>  	struct mii_bus *mii_bus;	/* MII bus reference */
> >>>  	int mdio_irqs[PHY_MAX_ADDR];	/* IRQs table for MDIO bus */
> >>>
> >>> -	/* IO registers and IRQs */
> >>> +	/* IO registers, dma functions and IRQs */
> >>>  	void __iomem *regs;
> >>> +	void __iomem *sdma_regs;
> >>> +#ifdef CONFIG_PPC_DCR
> >>>  	dcr_host_t sdma_dcrs;
> >>> +#endif
> >>> +	u32 (*dma_in)(struct temac_local *, int);
> >>> +	void (*dma_out)(struct temac_local *, int, u32);
> >>> +
> >>>  	int tx_irq;
> >>>  	int rx_irq;
> >>>  	int emac_num;
> >>> diff --git a/drivers/net/ll_temac_main.c
> > b/drivers/net/ll_temac_main.c
> >>> index a18e348..9aedf9b 100644
> >>> --- a/drivers/net/ll_temac_main.c
> >>> +++ b/drivers/net/ll_temac_main.c
> >>> @@ -20,9 +20,6 @@
> >>>   *   or rx, so this should be okay.
> >>>   *
> >>>   * TODO:
> >>> - * - Fix driver to work on more than just Virtex5.  Right now the
> > driver
> >>> - *   assumes that the locallink DMA registers are accessed via
DCR
> >>> - *   instructions.
> >>>   * - Factor out locallink DMA code into separate driver
> >>>   * - Fix multicast assignment.
> >>>   * - Fix support for hardware checksumming.
> >>> @@ -115,17 +112,86 @@ void temac_indirect_out32(struct temac_local
> > *lp, int reg, u32 value)
> >>>  	temac_iow(lp, XTE_CTL0_OFFSET, CNTLREG_WRITE_ENABLE_MASK | reg);
> >>>  }
> >>>
> >>> +/**
> >>> + * temac_dma_in32 - Memory mapped DMA read, this function expects
a
> >>> + * register input that is based on DCR word addresses which
> >>> + * are then converted to memory mapped byte addresses
> >>> + */
> >>>  static u32 temac_dma_in32(struct temac_local *lp, int reg)
> >>>  {
> >>> -	return dcr_read(lp->sdma_dcrs, reg);
> >>> +	return in_be32((u32 *)(lp->sdma_regs + (reg << 2)));
> >>>  }
> >>>
> >>> +/**
> >>> + * temac_dma_out32 - Memory mapped DMA read, this function
expects
> > a
> >>> + * register input that is based on DCR word addresses which
> >>> + * are then converted to memory mapped byte addresses
> >>> + */
> >>>  static void temac_dma_out32(struct temac_local *lp, int reg, u32
> > value)
> >>>  {
> >>> +	out_be32((u32 *)(lp->sdma_regs + (reg << 2)), value);
> >>> +}
> >>> +
> >>> +/* DMA register access functions can be DCR based or memory
mapped.
> >>> + * The PowerPC 440 is DCR based, the PowerPC 405 and MicroBlaze
are
> > both
> >>> + * memory mapped.
> >>> + */
> >>> +#ifdef CONFIG_PPC_DCR
> >>> +
> >>> +/**
> >>> + * temac_dma_dcr_in32 - DCR based DMA read
> >>> + */
> >>> +static u32 temac_dma_dcr_in(struct temac_local *lp, int reg)
> >>> +{
> >>> +	return dcr_read(lp->sdma_dcrs, reg);
> >>> +}
> >>> +
> >>> +/**
> >>> + * temac_dma_dcr_out32 - DCR based DMA write
> >>> + */
> >>> +static void temac_dma_dcr_out(struct temac_local *lp, int reg,
u32
> > value)
> >>> +{
> >>>  	dcr_write(lp->sdma_dcrs, reg, value);
> >>>  }
> >>>
> >>>  /**
> >>> + * temac_dcr_setup - If the DMA is DCR based, then setup the
> > address and
> >>> + * I/O  functions
> >>> + */
> >>> +static int temac_dcr_setup(struct temac_local *lp, struct
of_device
> > *op,
> >>> +				struct device_node *np)
> >>> +{
> >>> +	unsigned int dcrs;
> >>> +
> >>> +	/* setup the dcr address mapping if it's in the device tree */
> >>> +
> >>> +	dcrs = dcr_resource_start(np, 0);
> >>> +	if (dcrs != 0) {
> >>> +		lp->sdma_dcrs = dcr_map(np, dcrs, dcr_resource_len(np,
> > 0));
> >>> +		lp->dma_in = temac_dma_dcr_in;
> >>> +		lp->dma_out = temac_dma_dcr_out;
> >>> +		dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
> >>> +		return 0;
> >>> +	}
> >>> +	/* no DCR in the device tree, indicate a failure */
> >>> +	return -1;
> >>> +}
> >>> +
> >>> +#else
> >>> +
> >>> +/*
> >>> + * temac_dcr_setup - This is a stub for when DCR is not
supported,
> >>> + * such as with MicroBlaze
> >>> + */
> >>> +static int temac_dcr_setup(struct temac_local *lp, struct
of_device
> > *op,
> >>> +				struct device_node *np)
> >>> +{
> >>> +	return -1;
> >>> +}
> >>> +
> >>> +#endif
> >>> +
> >>> +/**
> >>>   * temac_dma_bd_init - Setup buffer descriptor rings
> >>>   */
> >>>  static int temac_dma_bd_init(struct net_device *ndev)
> >>> @@ -172,23 +238,23 @@ static int temac_dma_bd_init(struct
net_device
> > *ndev)
> >>>  		lp->rx_bd_v[i].app0 = STS_CTRL_APP0_IRQONEND;
> >>>  	}
> >>>
> >>> -	temac_dma_out32(lp, TX_CHNL_CTRL, 0x10220400 |
> >>> +	lp->dma_out(lp, TX_CHNL_CTRL, 0x10220400 |
> >>>  					  CHNL_CTRL_IRQ_EN |
> >>>  					  CHNL_CTRL_IRQ_DLY_EN |
> >>>  					  CHNL_CTRL_IRQ_COAL_EN);
> >>>  	/* 0x10220483 */
> >>>  	/* 0x00100483 */
> >>> -	temac_dma_out32(lp, RX_CHNL_CTRL, 0xff010000 |
> >>> +	lp->dma_out(lp, RX_CHNL_CTRL, 0xff010000 |
> >>>  					  CHNL_CTRL_IRQ_EN |
> >>>  					  CHNL_CTRL_IRQ_DLY_EN |
> >>>  					  CHNL_CTRL_IRQ_COAL_EN |
> >>>  					  CHNL_CTRL_IRQ_IOE);
> >>>  	/* 0xff010283 */
> >>>
> >>> -	temac_dma_out32(lp, RX_CURDESC_PTR,  lp->rx_bd_p);
> >>> -	temac_dma_out32(lp, RX_TAILDESC_PTR,
> >>> +	lp->dma_out(lp, RX_CURDESC_PTR,  lp->rx_bd_p);
> >>> +	lp->dma_out(lp, RX_TAILDESC_PTR,
> >>>  		       lp->rx_bd_p + (sizeof(*lp->rx_bd_v) * (RX_BD_NUM
> > - 1)));
> >>> -	temac_dma_out32(lp, TX_CURDESC_PTR, lp->tx_bd_p);
> >>> +	lp->dma_out(lp, TX_CURDESC_PTR, lp->tx_bd_p);
> >>>
> >>>  	return 0;
> >>>  }
> >>> @@ -426,9 +492,9 @@ static void temac_device_reset(struct
net_device
> > *ndev)
> >>>  	temac_indirect_out32(lp, XTE_RXC1_OFFSET, val &
> > ~XTE_RXC1_RXEN_MASK);
> >>>  	/* Reset Local Link (DMA) */
> >>> -	temac_dma_out32(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
> >>> +	lp->dma_out(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
> >>>  	timeout = 1000;
> >>> -	while (temac_dma_in32(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
> >>> +	while (lp->dma_in(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
> >>>  		udelay(1);
> >>>  		if (--timeout == 0) {
> >>>  			dev_err(&ndev->dev,
> >>> @@ -436,7 +502,7 @@ static void temac_device_reset(struct
net_device
> > *ndev)
> >>>  			break;
> >>>  		}
> >>>  	}
> >>> -	temac_dma_out32(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
> >>> +	lp->dma_out(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
> >>>
> >>>  	temac_dma_bd_init(ndev);
> >>>
> >>> @@ -597,7 +663,7 @@ static int temac_start_xmit(struct sk_buff
*skb,
> > struct net_device *ndev)
> >>>  		lp->tx_bd_tail = 0;
> >>>
> >>>  	/* Kick off the transfer */
> >>> -	temac_dma_out32(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
> >>> +	lp->dma_out(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
> >>>
> >>>  	return NETDEV_TX_OK;
> >>>  }
> >>> @@ -663,7 +729,7 @@ static void ll_temac_recv(struct net_device
> > *ndev)
> >>>  		cur_p = &lp->rx_bd_v[lp->rx_bd_ci];
> >>>  		bdstat = cur_p->app0;
> >>>  	}
> >>> -	temac_dma_out32(lp, RX_TAILDESC_PTR, tail_p);
> >>> +	lp->dma_out(lp, RX_TAILDESC_PTR, tail_p);
> >>>
> >>>  	spin_unlock_irqrestore(&lp->rx_lock, flags);
> >>>  }
> >>> @@ -674,8 +740,8 @@ static irqreturn_t ll_temac_tx_irq(int irq,
void
> > *_ndev)
> >>>  	struct temac_local *lp = netdev_priv(ndev);
> >>>  	unsigned int status;
> >>>
> >>> -	status = temac_dma_in32(lp, TX_IRQ_REG);
> >>> -	temac_dma_out32(lp, TX_IRQ_REG, status);
> >>> +	status = lp->dma_in(lp, TX_IRQ_REG);
> >>> +	lp->dma_out(lp, TX_IRQ_REG, status);
> >>>
> >>>  	if (status & (IRQ_COAL | IRQ_DLY))
> >>>  		temac_start_xmit_done(lp->ndev);
> >>> @@ -692,8 +758,8 @@ static irqreturn_t ll_temac_rx_irq(int irq,
void
> > *_ndev)
> >>>  	unsigned int status;
> >>>
> >>>  	/* Read and clear the status registers */
> >>> -	status = temac_dma_in32(lp, RX_IRQ_REG);
> >>> -	temac_dma_out32(lp, RX_IRQ_REG, status);
> >>> +	status = lp->dma_in(lp, RX_IRQ_REG);
> >>> +	lp->dma_out(lp, RX_IRQ_REG, status);
> >>>
> >>>  	if (status & (IRQ_COAL | IRQ_DLY))
> >>>  		ll_temac_recv(lp->ndev);
> >>> @@ -794,7 +860,7 @@ static ssize_t temac_show_llink_regs(struct
> > device *dev,
> >>>  	int i, len = 0;
> >>>
> >>>  	for (i = 0; i < 0x11; i++)
> >>> -		len += sprintf(buf + len, "%.8x%s", temac_dma_in32(lp,
> > i),
> >>> +		len += sprintf(buf + len, "%.8x%s", lp->dma_in(lp, i),
> >>>  			       (i % 8) == 7 ? "\n" : " ");
> >>>  	len += sprintf(buf + len, "\n");
> >>>
> >>> @@ -820,7 +886,6 @@ temac_of_probe(struct of_device *op, const
> > struct of_device_id *match)
> >>>  	struct net_device *ndev;
> >>>  	const void *addr;
> >>>  	int size, rc = 0;
> >>> -	unsigned int dcrs;
> >>>
> >>>  	/* Init network device structure */
> >>>  	ndev = alloc_etherdev(sizeof(*lp));
> >>> @@ -870,13 +935,20 @@ temac_of_probe(struct of_device *op, const
> > struct of_device_id *match)
> >>>  		goto nodev;
> >>>  	}
> >>>
> >>> -	dcrs = dcr_resource_start(np, 0);
> >>> -	if (dcrs == 0) {
> >>> -		dev_err(&op->dev, "could not get DMA register
> > address\n");
> >>> -		goto nodev;
> >>> +	/* Setup the DMA register accesses, could be DCR or memory
> > mapped */
> >>> +	if (temac_dcr_setup(lp, op, np)) {
> >>> +
> >>> +		/* no DCR in the device tree, try non-DCR */
> >>> +		lp->sdma_regs = of_iomap(np, 0);
> >>> +		if (lp->sdma_regs) {
> >>> +			lp->dma_in = temac_dma_in32;
> >>> +			lp->dma_out = temac_dma_out32;
> >>> +			dev_dbg(&op->dev, "MEM base: %p\n",
> > lp->sdma_regs);
> >>> +		} else {
> >>> +			dev_err(&op->dev, "unable to map DMA
> > registers\n");
> >>> +			goto nodev;
> >>> +		}
> >>>  	}
> >>> -	lp->sdma_dcrs = dcr_map(np, dcrs, dcr_resource_len(np, 0));
> >>> -	dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
> >>>
> >>>  	lp->rx_irq = irq_of_parse_and_map(np, 0);
> >>>  	lp->tx_irq = irq_of_parse_and_map(np, 1);
> >>
> >> --
> >> Michal Simek, Ing. (M.Eng)
> >> PetaLogix - Linux Solutions for a Reconfigurable World
> >> w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f:
> > +61-7-30090663
> >
> >
> > This email and any attachments are intended for the sole use of the
named recipient(s) and
> contain(s) confidential information that may be proprietary,
privileged or copyrighted under
> applicable law. If you are not the intended recipient, do not read,
copy, or forward this email
> message or any attachments. Delete this email message and any
attachments immediately.
> >
> >
> 
> 
> --
> Michal Simek, Ing. (M.Eng)
> PetaLogix - Linux Solutions for a Reconfigurable World
> w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f:
+61-7-30090663


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.



^ permalink raw reply

* Re: [3/3] gigaset: prune use of tty_buffer_request_room
From: Karsten Keil @ 2010-03-15 14:48 UTC (permalink / raw)
  To: Tilman Schmidt
  Cc: David Miller, netdev, linux-kernel, stable, isdn4linux,
	Hansjoerg Lipp, i4ldeveloper, Alan Cox

Calling tty_buffer_request_room() before tty_insert_flip_string()
is unnecessary, costs CPU and for big buffers can mess up the
multi-page allocation avoidance.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
Acked-by: Karsten Keil <keil@b1-systems.de>

---
Note to -stable: applies correctly to 2.6.33 with fuzz 2.

 drivers/isdn/gigaset/interface.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/isdn/gigaset/interface.c b/drivers/isdn/gigaset/interface.c
index a1bcbc2..f0dc6c9 100644
--- a/drivers/isdn/gigaset/interface.c
+++ b/drivers/isdn/gigaset/interface.c
@@ -628,7 +628,6 @@ void gigaset_if_receive(struct cardstate *cs,
 	if (tty == NULL)
 		gig_dbg(DEBUG_IF, "receive on closed device");
 	else {
-		tty_buffer_request_room(tty, len);
 		tty_insert_flip_string(tty, buffer, len);
 		tty_flip_buffer_push(tty);
 	}

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox