* Re: libpcap and tc filters
From: Eric Dumazet @ 2011-07-04 10:20 UTC (permalink / raw)
To: Adam Katz; +Cc: netdev
In-Reply-To: <CAA0qwj6fWJFVCDW9M9jZhvQnSTy=Pnyp0AhWFYT8FOiroq7C=Q@mail.gmail.com>
Le lundi 04 juillet 2011 à 10:38 +0300, Adam Katz a écrit :
> Hi Everyone
>
> I'm sorry for littering the mailing list with this question, but no
> other place could help me..
>
> I'm attempting to use tc to shape traffic sent using libpcap, I'm
> doing this for a research project. I have a classful scheduler with
> several classes, to this scheduler I attach a few filters based on
> destination tcp ports.
>
> My problem is this: When sending packets using a normal userland
> socket, the filters work and I see the appropriate traffic entering
> the right class. BUT when sending packets with libpcap, all packets
> end up in the scheduler's default band as if the filters simply refuse
> to work.
>
> Can anyone suggest a solution?
> --
You already asked this yesterday.
No need to repost.
4th July is not the day many people will answer you...
What gives :
tc -s -d filter show dev eth0
^ permalink raw reply
* Re: [PATCH 1/2] netpoll: Remove unused EXPORT_SYMBOLs of netpoll_poll and netpoll_poll_dev
From: Joe Perches @ 2011-07-04 7:46 UTC (permalink / raw)
To: Harry Wei; +Cc: amwang, linux-kernel, davem, netdev
In-Reply-To: <20110704065320.GA2588@gmail.com>
On Mon, 2011-07-04 at 14:53 +0800, Harry Wei wrote:
> On Thu, Jun 30, 2011 at 06:08:57PM -0700, Joe Perches wrote:
> > Unused symbols waste space.
> > Commit 0e34e93177fb
> > "(netpoll: add generic support for bridge and bonding devices)"
> > added the symbol more than a year ago with the promise of "future use".
> > Because it is so far unused, remove it for now.
> > It can be easily readded if or when it actually needs to be used.
> > cc: WANG Cong <amwang@redhat.com>
> > Signed-off-by: Joe Perches <joe@perches.com>
> > ---
> > include/linux/netpoll.h | 2 --
> > net/core/netpoll.c | 6 ++----
> > 2 files changed, 2 insertions(+), 6 deletions(-)
> > diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
[]
> > @@ -40,8 +40,6 @@ struct netpoll_info {
> > struct netpoll *netpoll;
> > };
> >
> > -void netpoll_poll_dev(struct net_device *dev);
> > -void netpoll_poll(struct netpoll *np);
> > void netpoll_send_udp(struct netpoll *np, const char *msg, int len);
> > void netpoll_print_options(struct netpoll *np);
> > int netpoll_parse_options(struct netpoll *np, char *opt);
> > diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> > index 18d9cbd..4ce595e 100644
> > --- a/net/core/netpoll.c
> > +++ b/net/core/netpoll.c
> > @@ -177,7 +177,7 @@ static void service_arp_queue(struct netpoll_info *npi)
> > }
> > }
> >
> > -void netpoll_poll_dev(struct net_device *dev)
> > +static void netpoll_poll_dev(struct net_device *dev)
> You have removed the function declaration in include/linux/netpoll.h.
Because it's unused by anything in the tree
but netpoll.c
> And you changed the function like
> static void netpoll_poll_dev(...)
> {
> ...
> }
> But i don't know its usage.
It's now a local function to netpoll.c
> The 'static' before this function just says the active area of 'netpoll_poll_dev'
> is in the file net/core/netpoll.c. What is the purpose? Could you please tell me?
It prods the device to perform network work.
/* Process pending work on NIC */
ops->ndo_poll_controller(dev);
poll_napi(dev);
^ permalink raw reply
* bnx2: FTQ dump on heavy workload(bnx2-2.0.23b + kernel 2.6.32.36)
From: MaoXiaoyun @ 2011-07-04 7:40 UTC (permalink / raw)
To: netdev
In-Reply-To: <BLU157-w63DCDB99F9F311C7BF326EDA5C0@phx.gbl>
Hi:
I met bnx2 FTQ dump over and over again during my testing on Xen live migration which generate
heavy network workload.
I have two physcial machine, both have xen 4.0.1 installed, and kernel 2.6.32.36, bnx2 2.0.23b.
I start 15 Virtual Machines totoally, and doing migration between the host over and over again,
about 16hours, the network will not work, and sometimes, it can reset successfully, sometimes, it
cause kernel crash.
I've tried debug some, add code in the driver. below is the code when FTQ happened.
It looks like the NIC is stop transmit the packets, and cause timeout.
BTW, cpu max_cstate=1 in my grub.
Thanks.
--------------
static void
bnx2_tx_timeout(struct net_device *dev)
{
struct bnx2 *bp = netdev_priv(dev);
struct bnx2_napi *bnapi = &bp->bnx2_napi[0];
struct bnx2_tx_ring_info *txr = &bnapi->tx_ring;
struct bnx2_rx_ring_info *rxr = &bnapi->rx_ring;
int i ;
bnx2_dump_ftq(bp);
bnx2_dump_state(bp);
if (stop_on_tx_timeout) {
printk(KERN_WARNING PFX
"%s: prevent chip reset during tx timeout\n",
bp->dev->name);
smp_rmb();
printk("last status idx %d \n", bnapi->last_status_idx);
printk("hw_tx_cons %d, txr->hw_tx_conds %d txr->tx_prod %d txr->tx_cons %d\n",
bnx2_get_hw_tx_cons(bnapi), txr->hw_tx_cons, txr->tx_prod, txr->tx_cons);
printk("hw_rx_cons %d, txr->hw_rx_conds %d\n", bnx2_get_hw_rx_cons(bnapi), rxr->rx_cons);
printk("sblk->status_attn_bits %d\n",bnapi->status_blk.msi->status_attn_bits);
printk("sblk->status_attn_bits_ack %d\n",bnapi->status_blk.msi->status_attn_bits_ack);
printk("bnx2_tx_avail %d \n",(bnx2_tx_avail(bp, txr)));
printk("sblk->status_tx_quick_consumer_index0 %d\n",bnapi->status_blk.msi->status_tx_quick_consumer_index0);
printk("sblk->status_tx_quick_consumer_index1 %d\n",bnapi->status_blk.msi->status_tx_quick_consumer_index1);
printk("sblk->status_tx_quick_consumer_index2 %d\n",bnapi->status_blk.msi->status_tx_quick_consumer_index2);
printk("sblk->status_tx_quick_consumer_index3 %d\n",bnapi->status_blk.msi->status_tx_quick_consumer_index3);
printk("sblk->status_rx_quick_consumer_index0 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index0);
printk("sblk->status_rx_quick_consumer_index1 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index1);
printk("sblk->status_rx_quick_consumer_index2 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index2);
printk("sblk->status_rx_quick_consumer_index3 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index3);
printk("sblk->status_rx_quick_consumer_index4 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index4);
printk("sblk->status_rx_quick_consumer_index5 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index5);
printk("sblk->status_rx_quick_consumer_index6 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index6);
printk("sblk->status_rx_quick_consumer_index7 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index7);
printk("sblk->status_rx_quick_consumer_index8 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index8);
printk("sblk->status_rx_quick_consumer_index9 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index9);
printk("sblk->status_rx_quick_consumer_index10 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index10);
printk("sblk->status_rx_quick_consumer_index11 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index11);
printk("sblk->status_rx_quick_consumer_index12 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index12);
printk("sblk->status_rx_quick_consumer_index13 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index13);
printk("sblk->status_rx_quick_consumer_index14 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index14);
printk("sblk->status_rx_quick_consumer_index15 %d\n",bnapi->status_blk.msi->status_rx_quick_consumer_index15);
printk("sblk->status_completion_producer_index %d\n",bnapi->status_blk.msi->status_completion_producer_index);
printk("sblk->status_cmd_consumer_index %d\n",bnapi->status_blk.msi->status_cmd_consumer_index);
printk("sblk->status_idx %d\n",bnapi->status_blk.msi->status_idx);
printk("sblk->status_unused %d\n",bnapi->status_blk.msi->status_unused);
printk("sblk->status_blk_num %d\n",bnapi->status_blk.msi->status_blk_num);
is_timedout = 1;
for (i = 0; i < bp->irq_nvecs; i++) {
bnapi = &bp->bnx2_napi[i];
bnx2_tx_int(bp, bnapi, 0);
}
return;
}
-----------------
-------------FTQ log in /var/log/message
------------[ cut here ]------------
WARNING: at net/sched/sch_generic.c:261 dev_watchdog+0x105/0x16a()
Hardware name: Tecal RH2285
Modules linked in: iptable_filter ip_tables nfs fscache nfs_acl auth_rpcgss bridge stp llc autofs4 ipmi_devintf ipmi_si ipmi_msghandler lockd sunrpc ipv6 xenfs dm_multipath fuse xen_netback xen_blkback blktap blkback_pagemap loop nbd video output sbs sbshc parport_pc lp parport snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss snd_mixer_oss bnx2 serio_raw snd_pcm snd_timer snd soundcore snd_page_alloc i2c_i801 iTCO_wdt iTCO_vendor_support i2c_core pata_acpi ata_generic pcspkr ata_piix shpchp mptsas mptscsih mptbase [last unloaded: freq_table]
Pid: 0, comm: swapper Not tainted 2.6.32.36xen #1
Call Trace:
<IRQ> [<ffffffff813ba154>] ? dev_watchdog+0x105/0x16a
[<ffffffff81056666>] warn_slowpath_common+0x7c/0x94
[<ffffffff81056738>] warn_slowpath_fmt+0xa4/0xa6
[<ffffffff81080bfa>] ? clockevents_program_event+0x78/0x81
[<ffffffff81081fce>] ? tick_program_event+0x2a/0x2c
[<ffffffff813b951d>] ? __netif_tx_lock+0x1b/0x24
[<ffffffff813b95a8>] ? netif_tx_lock+0x46/0x6e
[<ffffffff813a3ed1>] ? netdev_drivername+0x48/0x4f
[<ffffffff813ba154>] dev_watchdog+0x105/0x16a
[<ffffffff81063d98>] run_timer_softirq+0x156/0x1f8
[<ffffffff813ba04f>] ? dev_watchdog+0x0/0x16a
[<ffffffff8105d6f0>] __do_softirq+0xd7/0x19e
[<ffffffff81013eac>] call_softirq+0x1c/0x30
[<ffffffff8101564b>] do_softirq+0x46/0x87
[<ffffffff8105d575>] irq_exit+0x3b/0x7a
[<ffffffff8128dcfe>] xen_evtchn_do_upcall+0x38/0x46
[<ffffffff81013efe>] xen_do_hypervisor_callback+0x1e/0x30
<EOI> [<ffffffff8103f642>] ? pick_next_task_idle+0x18/0x22
[<ffffffff810093aa>] ? hypercall_page+0x3aa/0x1000
[<ffffffff810093aa>] ? hypercall_page+0x3aa/0x1000
[<ffffffff8100f1bb>] ? xen_safe_halt+0x10/0x1a
[<ffffffff81019e14>] ? default_idle+0x39/0x56
[<ffffffff81011cd0>] ? cpu_idle+0x5d/0x8c
[<ffffffff8143375d>] ? cpu_bringup_and_idle+0x13/0x15
---[ end trace 867bb8f6cd959b03 ]---
bnx2: <--- start FTQ dump on peth0 --->
bnx2: peth0: BNX2_RV2P_PFTQ_CTL 10000
bnx2: peth0: BNX2_RV2P_TFTQ_CTL 20000
bnx2: peth0: BNX2_RV2P_MFTQ_CTL 4000
bnx2: peth0: BNX2_TBDR_FTQ_CTL 1004002
bnx2: peth0: BNX2_TDMA_FTQ_CTL 4010002
bnx2: peth0: BNX2_TXP_FTQ_CTL 2410002
bnx2: peth0: BNX2_TPAT_FTQ_CTL 10002
bnx2: peth0: BNX2_RXP_CFTQ_CTL 8000
bnx2: peth0: BNX2_RXP_FTQ_CTL 100000
bnx2: peth0: BNX2_COM_COMXQ_FTQ_CTL 10000
bnx2: peth0: BNX2_COM_COMTQ_FTQ_CTL 20000
bnx2: peth0: BNX2_COM_COMQ_FTQ_CTL 10000
bnx2: peth0: BNX2_CP_CPQ_FTQ_CTL 4000
bnx2: peth0: TXP mode b84c state 80005000 evt_mask 500 pc 8000d60 pc 8000d60 instr 8f860000
bnx2: peth0: TPAT mode b84c state 80009000 evt_mask 500 pc 8000a5c pc 8000a5c instr 10400016
bnx2: peth0: RXP mode b84c state 80001000 evt_mask 500 pc 8004c14 pc 8004c14 instr 10e00088
bnx2: peth0: COM mode b8cc state 80000000 evt_mask 500 pc 8000b28 pc 8000a9c instr 8c530000
bnx2: peth0: CP mode b8cc state 80000000 evt_mask 500 pc 8000c50 pc 8000c58 instr 8ca50020
bnx2: <--- end FTQ dump on peth0 --->
bnx2: peth0 DEBUG: intr_sem[0]
bnx2: peth0 DEBUG: intr_sem[0] PCI_CMD[20100406]
bnx2: peth0 DEBUG: PCI_PM[19002008] PCI_MISC_CFG[92000088]
bnx2: peth0 DEBUG: EMAC_TX_STATUS[00000008] EMAC_RX_STATUS[00000000]
bnx2: peth0 RPM_MGMT_PKT_CTRL[40000088]
bnx2: peth0 DEBUG: MCP_STATE_P0[0007e10e] MCP_STATE_P1[0003e00e]
bnx2: peth0 DEBUG: HC_STATS_INTERRUPT_STATUS[01ff0000]
bnx2: peth0 DEBUG: PBA[00000000]
BNX2_PCICFG_INT_ACK_CMD[00013ce1]
bnx2: peth0: prevent chip reset during tx timeout
last status idx 2426
hw_tx_cons 32474, txr->hw_tx_conds 32474 txr->tx_prod 32641 txr->tx_cons 32474
hw_rx_cons 19665, txr->hw_rx_conds 19665
sblk->status_attn_bits 1
sblk->status_attn_bits_ack 1
bnx2_tx_avail 88
sblk->status_tx_quick_consumer_index0 32474
sblk->status_tx_quick_consumer_index1 0
sblk->status_tx_quick_consumer_index2 0
sblk->status_tx_quick_consumer_index3 0
sblk->status_rx_quick_consumer_index0 19665
sblk->status_rx_quick_consumer_index1 0
sblk->status_rx_quick_consumer_index2 0
sblk->status_rx_quick_consumer_index3 0
sblk->status_rx_quick_consumer_index4 0
sblk->status_rx_quick_consumer_index5 0
sblk->status_rx_quick_consumer_index6 0
sblk->status_rx_quick_consumer_index7 0
sblk->status_rx_quick_consumer_index8 0
sblk->status_rx_quick_consumer_index9 0
sblk->status_rx_quick_consumer_index10 0
sblk->status_rx_quick_consumer_index11 0
sblk->status_rx_quick_consumer_index12 0
sblk->status_rx_quick_consumer_index13 0
sblk->status_rx_quick_consumer_index14 0
sblk->status_rx_quick_consumer_index15 0
sblk->status_completion_producer_index 0
sblk->status_cmd_consumer_index 0
sblk->status_idx 2426
sblk->status_unused 0
sblk->status_blk_num 0
hw_cons 32474 sw_cons 32474 ffff8801d27f85c0 bnapi
return hw_cons 32474 sw_cons 32474 ffff8801d27f85c0 bnapi
hw_cons 3628 sw_cons 3625 ffff8801d27f8bc0 bnapi
return hw_cons 3628 sw_cons 3625 ffff8801d27f8bc0 bnapi
hw_cons 62094 sw_cons 62090 ffff8801d27f91c0 bnapi
return hw_cons 62094 sw_cons 62090 ffff8801d27f91c0 bnapi
hw_cons 3184 sw_cons 3173 ffff8801d27f97c0 bnapi
return hw_cons 3184 sw_cons 3173 ffff8801d27f97c0 bnapi
hw_cons 0 sw_cons 0 ffff8801d27f9dc0 bnapi
return hw_cons 0 sw_cons 0 ffff8801d27f9dc0 bnapi
^ permalink raw reply
* libpcap and tc filters
From: Adam Katz @ 2011-07-04 7:38 UTC (permalink / raw)
To: netdev
Hi Everyone
I'm sorry for littering the mailing list with this question, but no
other place could help me..
I'm attempting to use tc to shape traffic sent using libpcap, I'm
doing this for a research project. I have a classful scheduler with
several classes, to this scheduler I attach a few filters based on
destination tcp ports.
My problem is this: When sending packets using a normal userland
socket, the filters work and I see the appropriate traffic entering
the right class. BUT when sending packets with libpcap, all packets
end up in the scheduler's default band as if the filters simply refuse
to work.
Can anyone suggest a solution?
^ permalink raw reply
* [PATCH] gianfar v2: code cleanups
From: Sebastian Pöhn @ 2011-07-04 7:35 UTC (permalink / raw)
To: Linux Netdev; +Cc: Joe Perches, Sebastian Pöhn
This patch:
# improves readability of some gianfar nfc code
Signed-off-by: Sebastian Poehn <sebastian.poehn@belden.com>
---
drivers/net/gianfar.h | 1 +
drivers/net/gianfar_ethtool.c | 52
++++++++++++++++++++++-------------------
2 files changed, 29 insertions(+), 24 deletions(-)
diff --git a/drivers/net/gianfar.h b/drivers/net/gianfar.h
index 76f14d0..27499c6 100644
--- a/drivers/net/gianfar.h
+++ b/drivers/net/gianfar.h
@@ -409,6 +409,7 @@ extern const char gfar_driver_version[];
#define RQFCR_HASHTBL_2 0x00060000
#define RQFCR_HASHTBL_3 0x00080000
#define RQFCR_HASH 0x00010000
+#define RQFCR_QUEUE 0x0000FC00
#define RQFCR_CLE 0x00000200
#define RQFCR_RJE 0x00000100
#define RQFCR_AND 0x00000080
diff --git a/drivers/net/gianfar_ethtool.c
b/drivers/net/gianfar_ethtool.c
index 2ecdc9a..203369c 100644
--- a/drivers/net/gianfar_ethtool.c
+++ b/drivers/net/gianfar_ethtool.c
@@ -40,6 +40,7 @@
#include <linux/mii.h>
#include <linux/phy.h>
#include <linux/sort.h>
+#include <linux/if_vlan.h>
#include "gianfar.h"
@@ -883,7 +884,7 @@ static void gfar_set_attribute(u32 value, u32 mask,
u32 flag,
struct filer_table *tab)
{
switch (flag) {
- /* 3bit */
+ /* 3bit */
case RQFCR_PID_PRI:
if (!(value | mask))
return;
@@ -1051,17 +1052,17 @@ static int gfar_convert_to_filer(struct
ethtool_rx_flow_spec *rule,
vlan_mask = RQFPR_VLN;
/* Separate the fields */
- id = rule->h_ext.vlan_tci & 0xFFF;
- id_mask = rule->m_ext.vlan_tci & 0xFFF;
- cfi = (rule->h_ext.vlan_tci >> 12) & 1;
- cfi_mask = (rule->m_ext.vlan_tci >> 12) & 1;
- prio = (rule->h_ext.vlan_tci >> 13) & 0x7;
- prio_mask = (rule->m_ext.vlan_tci >> 13) & 0x7;
-
- if (cfi == 1 && cfi_mask == 1) {
+ id = rule->h_ext.vlan_tci & VLAN_VID_MASK;
+ id_mask = rule->m_ext.vlan_tci & VLAN_VID_MASK;
+ cfi = rule->h_ext.vlan_tci & VLAN_CFI_MASK;
+ cfi_mask = rule->m_ext.vlan_tci & VLAN_CFI_MASK;
+ prio = (rule->h_ext.vlan_tci & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
+ prio_mask = (rule->m_ext.vlan_tci & VLAN_PRIO_MASK) >>
VLAN_PRIO_SHIFT;
+
+ if (cfi == VLAN_TAG_PRESENT && cfi_mask == VLAN_TAG_PRESENT) {
vlan |= RQFPR_CFI;
vlan_mask |= RQFPR_CFI;
- } else if (cfi == 0 && cfi_mask == 1) {
+ } else if (cfi != VLAN_TAG_PRESENT && cfi_mask == VLAN_TAG_PRESENT) {
vlan_mask |= RQFPR_CFI;
}
}
@@ -1262,21 +1263,21 @@ static void gfar_cluster_filer(struct
filer_table *tab)
}
}
-/* Swaps the 0xFF80 masked bits of a1<>a2 and b1<>b2 */
-static void gfar_swap_ff80_bits(struct gfar_filer_entry *a1,
+/* Swaps the masked bits of a1<>a2 and b1<>b2 */
+static void gfar_swap_bits(struct gfar_filer_entry *a1,
struct gfar_filer_entry *a2, struct gfar_filer_entry *b1,
- struct gfar_filer_entry *b2)
+ struct gfar_filer_entry *b2, u32 mask)
{
u32 temp[4];
- temp[0] = a1->ctrl & 0xFF80;
- temp[1] = a2->ctrl & 0xFF80;
- temp[2] = b1->ctrl & 0xFF80;
- temp[3] = b2->ctrl & 0xFF80;
+ temp[0] = a1->ctrl & mask;
+ temp[1] = a2->ctrl & mask;
+ temp[2] = b1->ctrl & mask;
+ temp[3] = b2->ctrl & mask;
- a1->ctrl &= ~0xFF80;
- a2->ctrl &= ~0xFF80;
- b1->ctrl &= ~0xFF80;
- b2->ctrl &= ~0xFF80;
+ a1->ctrl &= ~mask;
+ a2->ctrl &= ~mask;
+ b1->ctrl &= ~mask;
+ b2->ctrl &= ~mask;
a1->ctrl |= temp[1];
a2->ctrl |= temp[0];
@@ -1305,7 +1306,7 @@ static u32 gfar_generate_mask_table(struct
gfar_mask_entry *mask_table,
mask_table[and_index - 1].end = i - 1;
and_index++;
}
- /* cluster starts will be separated because they should
+ /* cluster starts and ends will be separated because they should
* hold their position */
if (tab->fe[i].ctrl & RQFCR_CLE)
block_index++;
@@ -1356,10 +1357,13 @@ static void gfar_sort_mask_table(struct
gfar_mask_entry *mask_table,
new_first = mask_table[start].start + 1;
new_last = mask_table[i - 1].end;
- gfar_swap_ff80_bits(&temp_table->fe[new_first],
+ gfar_swap_bits(&temp_table->fe[new_first],
&temp_table->fe[old_first],
&temp_table->fe[new_last],
- &temp_table->fe[old_last]);
+ &temp_table->fe[old_last],
+ RQFCR_QUEUE | RQFCR_CLE |
+ RQFCR_RJE | RQFCR_AND
+ );
start = i;
size = 0;
^ permalink raw reply related
* Re: [PATCH] net/fec: gasket needs to be enabled for some i.mx
From: Sascha Hauer @ 2011-07-04 6:55 UTC (permalink / raw)
To: Shawn Guo
Cc: netdev, linux-arm-kernel, u.kleine-koenig, LW, Troy Kisky,
David S. Miller
In-Reply-To: <1309515082-7386-1-git-send-email-shawn.guo@linaro.org>
David,
On Fri, Jul 01, 2011 at 06:11:22PM +0800, Shawn Guo wrote:
> On the recent i.mx (mx25/50/53), there is a gasket inside fec
> controller which needs to be enabled no matter phy works in MII
> or RMII mode.
>
> The current code enables the gasket only when phy interface is RMII.
> It's broken when the driver works with a MII phy. The patch uses
> platform_device_id to distinguish the SoCs that have the gasket and
> enables it on these SoCs for both MII and RMII mode.
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> arch/arm/mach-imx/clock-imx25.c | 2 +-
> arch/arm/mach-imx/clock-imx27.c | 2 +-
> arch/arm/mach-imx/clock-imx35.c | 2 +-
> arch/arm/mach-mx5/clock-mx51-mx53.c | 4 +-
> arch/arm/plat-mxc/devices/platform-fec.c | 17 ++++++++-------
> arch/arm/plat-mxc/include/mach/devices-common.h | 1 +
> drivers/net/fec.c | 26 ++++++++++++++++++++--
I either need your Ack for this or we need to split this up into two
patches. Pushing this through the net tree is probably not a good idea.
Sascha
> 7 files changed, 38 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/mach-imx/clock-imx25.c b/arch/arm/mach-imx/clock-imx25.c
> index a65838f..5527fff 100644
> --- a/arch/arm/mach-imx/clock-imx25.c
> +++ b/arch/arm/mach-imx/clock-imx25.c
> @@ -294,7 +294,7 @@ static struct clk_lookup lookups[] = {
> _REGISTER_CLOCK("imx-i2c.0", NULL, i2c_clk)
> _REGISTER_CLOCK("imx-i2c.1", NULL, i2c_clk)
> _REGISTER_CLOCK("imx-i2c.2", NULL, i2c_clk)
> - _REGISTER_CLOCK("fec.0", NULL, fec_clk)
> + _REGISTER_CLOCK("imx25-fec.0", NULL, fec_clk)
> _REGISTER_CLOCK("imxdi_rtc.0", NULL, dryice_clk)
> _REGISTER_CLOCK("imx-fb.0", NULL, lcdc_clk)
> _REGISTER_CLOCK("imx2-wdt.0", NULL, wdt_clk)
> diff --git a/arch/arm/mach-imx/clock-imx27.c b/arch/arm/mach-imx/clock-imx27.c
> index 583f251..d197df2 100644
> --- a/arch/arm/mach-imx/clock-imx27.c
> +++ b/arch/arm/mach-imx/clock-imx27.c
> @@ -662,7 +662,7 @@ static struct clk_lookup lookups[] = {
> _REGISTER_CLOCK(NULL, "brom", brom_clk)
> _REGISTER_CLOCK(NULL, "emma", emma_clk)
> _REGISTER_CLOCK(NULL, "slcdc", slcdc_clk)
> - _REGISTER_CLOCK("fec.0", NULL, fec_clk)
> + _REGISTER_CLOCK("imx27-fec.0", NULL, fec_clk)
> _REGISTER_CLOCK(NULL, "emi", emi_clk)
> _REGISTER_CLOCK(NULL, "sahara2", sahara2_clk)
> _REGISTER_CLOCK(NULL, "ata", ata_clk)
> diff --git a/arch/arm/mach-imx/clock-imx35.c b/arch/arm/mach-imx/clock-imx35.c
> index 5a4cc1e..fb4007b 100644
> --- a/arch/arm/mach-imx/clock-imx35.c
> +++ b/arch/arm/mach-imx/clock-imx35.c
> @@ -461,7 +461,7 @@ static struct clk_lookup lookups[] = {
> _REGISTER_CLOCK("sdhci-esdhc-imx.0", NULL, esdhc1_clk)
> _REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_clk)
> _REGISTER_CLOCK("sdhci-esdhc-imx.2", NULL, esdhc3_clk)
> - _REGISTER_CLOCK("fec.0", NULL, fec_clk)
> + _REGISTER_CLOCK("imx35-fec.0", NULL, fec_clk)
> _REGISTER_CLOCK(NULL, "gpio", gpio1_clk)
> _REGISTER_CLOCK(NULL, "gpio", gpio2_clk)
> _REGISTER_CLOCK(NULL, "gpio", gpio3_clk)
> diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
> index 699b0d2..04c5a01 100644
> --- a/arch/arm/mach-mx5/clock-mx51-mx53.c
> +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
> @@ -1426,7 +1426,7 @@ static struct clk_lookup mx51_lookups[] = {
> _REGISTER_CLOCK("imx-uart.1", NULL, uart2_clk)
> _REGISTER_CLOCK("imx-uart.2", NULL, uart3_clk)
> _REGISTER_CLOCK(NULL, "gpt", gpt_clk)
> - _REGISTER_CLOCK("fec.0", NULL, fec_clk)
> + _REGISTER_CLOCK("imx51-fec.0", NULL, fec_clk)
> _REGISTER_CLOCK("mxc_pwm.0", "pwm", pwm1_clk)
> _REGISTER_CLOCK("mxc_pwm.1", "pwm", pwm2_clk)
> _REGISTER_CLOCK("imx-i2c.0", NULL, i2c1_clk)
> @@ -1475,7 +1475,7 @@ static struct clk_lookup mx53_lookups[] = {
> _REGISTER_CLOCK("imx-uart.3", NULL, uart4_clk)
> _REGISTER_CLOCK("imx-uart.4", NULL, uart5_clk)
> _REGISTER_CLOCK(NULL, "gpt", gpt_clk)
> - _REGISTER_CLOCK("fec.0", NULL, fec_clk)
> + _REGISTER_CLOCK("imx53-fec.0", NULL, fec_clk)
> _REGISTER_CLOCK(NULL, "iim_clk", iim_clk)
> _REGISTER_CLOCK("imx-i2c.0", NULL, i2c1_clk)
> _REGISTER_CLOCK("imx-i2c.1", NULL, i2c2_clk)
> diff --git a/arch/arm/plat-mxc/devices/platform-fec.c b/arch/arm/plat-mxc/devices/platform-fec.c
> index ccc789e..3790c59 100644
> --- a/arch/arm/plat-mxc/devices/platform-fec.c
> +++ b/arch/arm/plat-mxc/devices/platform-fec.c
> @@ -10,40 +10,41 @@
> #include <mach/hardware.h>
> #include <mach/devices-common.h>
>
> -#define imx_fec_data_entry_single(soc) \
> +#define imx_fec_data_entry_single(soc, _devid) \
> { \
> + .devid = _devid, \
> .iobase = soc ## _FEC_BASE_ADDR, \
> .irq = soc ## _INT_FEC, \
> }
>
> #ifdef CONFIG_SOC_IMX25
> const struct imx_fec_data imx25_fec_data __initconst =
> - imx_fec_data_entry_single(MX25);
> + imx_fec_data_entry_single(MX25, "imx25-fec");
> #endif /* ifdef CONFIG_SOC_IMX25 */
>
> #ifdef CONFIG_SOC_IMX27
> const struct imx_fec_data imx27_fec_data __initconst =
> - imx_fec_data_entry_single(MX27);
> + imx_fec_data_entry_single(MX27, "imx27-fec");
> #endif /* ifdef CONFIG_SOC_IMX27 */
>
> #ifdef CONFIG_SOC_IMX35
> const struct imx_fec_data imx35_fec_data __initconst =
> - imx_fec_data_entry_single(MX35);
> + imx_fec_data_entry_single(MX35, "imx35-fec");
> #endif
>
> #ifdef CONFIG_SOC_IMX50
> const struct imx_fec_data imx50_fec_data __initconst =
> - imx_fec_data_entry_single(MX50);
> + imx_fec_data_entry_single(MX50, "imx50-fec");
> #endif
>
> #ifdef CONFIG_SOC_IMX51
> const struct imx_fec_data imx51_fec_data __initconst =
> - imx_fec_data_entry_single(MX51);
> + imx_fec_data_entry_single(MX51, "imx51-fec");
> #endif
>
> #ifdef CONFIG_SOC_IMX53
> const struct imx_fec_data imx53_fec_data __initconst =
> - imx_fec_data_entry_single(MX53);
> + imx_fec_data_entry_single(MX53, "imx53-fec");
> #endif
>
> struct platform_device *__init imx_add_fec(
> @@ -62,7 +63,7 @@ struct platform_device *__init imx_add_fec(
> },
> };
>
> - return imx_add_platform_device_dmamask("fec", 0,
> + return imx_add_platform_device_dmamask(data->devid, 0,
> res, ARRAY_SIZE(res),
> pdata, sizeof(*pdata), DMA_BIT_MASK(32));
> }
> diff --git a/arch/arm/plat-mxc/include/mach/devices-common.h b/arch/arm/plat-mxc/include/mach/devices-common.h
> index bf93820..6ac2450 100644
> --- a/arch/arm/plat-mxc/include/mach/devices-common.h
> +++ b/arch/arm/plat-mxc/include/mach/devices-common.h
> @@ -30,6 +30,7 @@ static inline struct platform_device *imx_add_platform_device(
>
> #include <linux/fec.h>
> struct imx_fec_data {
> + const char *devid;
> resource_size_t iobase;
> resource_size_t irq;
> };
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index 885d8ba..9161a82 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -66,14 +66,31 @@
> #define FEC_QUIRK_ENET_MAC (1 << 0)
> /* Controller needs driver to swap frame */
> #define FEC_QUIRK_SWAP_FRAME (1 << 1)
> +/* Controller uses gasket */
> +#define FEC_QUIRK_USE_GASKET (1 << 2)
>
> static struct platform_device_id fec_devtype[] = {
> {
> - .name = DRIVER_NAME,
> + .name = "imx25-fec",
> + .driver_data = FEC_QUIRK_USE_GASKET,
> + }, {
> + .name = "imx27-fec",
> .driver_data = 0,
> }, {
> .name = "imx28-fec",
> .driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME,
> + }, {
> + .name = "imx35-fec",
> + .driver_data = 0,
> + }, {
> + .name = "imx50-fec",
> + .driver_data = FEC_QUIRK_USE_GASKET,
> + }, {
> + .name = "imx51-fec",
> + .driver_data = 0,
> + }, {
> + .name = "imx53-fec",
> + .driver_data = FEC_QUIRK_USE_GASKET,
> },
> { }
> };
> @@ -425,7 +442,7 @@ fec_restart(struct net_device *ndev, int duplex)
>
> } else {
> #ifdef FEC_MIIGSK_ENR
> - if (fep->phy_interface == PHY_INTERFACE_MODE_RMII) {
> + if (id_entry->driver_data & FEC_QUIRK_USE_GASKET) {
> /* disable the gasket and wait */
> writel(0, fep->hwp + FEC_MIIGSK_ENR);
> while (readl(fep->hwp + FEC_MIIGSK_ENR) & 4)
> @@ -434,8 +451,11 @@ fec_restart(struct net_device *ndev, int duplex)
> /*
> * configure the gasket:
> * RMII, 50 MHz, no loopback, no echo
> + * MII, 25 MHz, no loopback, no echo
> */
> - writel(1, fep->hwp + FEC_MIIGSK_CFGR);
> + writel((fep->phy_interface == PHY_INTERFACE_MODE_RMII) ?
> + 1 : 0, fep->hwp + FEC_MIIGSK_CFGR);
> +
>
> /* re-enable the gasket */
> writel(2, fep->hwp + FEC_MIIGSK_ENR);
> --
> 1.7.4.1
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 1/2] netpoll: Remove unused EXPORT_SYMBOLs of netpoll_poll and netpoll_poll_dev
From: Harry Wei @ 2011-07-04 6:53 UTC (permalink / raw)
To: Joe Perches; +Cc: amwang, linux-kernel, davem, netdev
In-Reply-To: <282ce7d68b97c4dbfb759c1c0752e6cbddfeeafc.1309482314.git.joe@perches.com>
On Thu, Jun 30, 2011 at 06:08:57PM -0700, Joe Perches wrote:
> Unused symbols waste space.
>
> Commit 0e34e93177fb
> "(netpoll: add generic support for bridge and bonding devices)"
> added the symbol more than a year ago with the promise of "future use".
>
> Because it is so far unused, remove it for now.
> It can be easily readded if or when it actually needs to be used.
>
> cc: WANG Cong <amwang@redhat.com>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> include/linux/netpoll.h | 2 --
> net/core/netpoll.c | 6 ++----
> 2 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
> index 79358bb..5dfa091 100644
> --- a/include/linux/netpoll.h
> +++ b/include/linux/netpoll.h
> @@ -40,8 +40,6 @@ struct netpoll_info {
> struct netpoll *netpoll;
> };
>
> -void netpoll_poll_dev(struct net_device *dev);
> -void netpoll_poll(struct netpoll *np);
> void netpoll_send_udp(struct netpoll *np, const char *msg, int len);
> void netpoll_print_options(struct netpoll *np);
> int netpoll_parse_options(struct netpoll *np, char *opt);
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index 18d9cbd..4ce595e 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -177,7 +177,7 @@ static void service_arp_queue(struct netpoll_info *npi)
> }
> }
>
> -void netpoll_poll_dev(struct net_device *dev)
> +static void netpoll_poll_dev(struct net_device *dev)
Hi Joe,
You have removed the function declaration in include/linux/netpoll.h.
And you changed the function like
static void netpoll_poll_dev(...)
{
...
}
But i don't know its usage.
The 'static' before this function just says the active area of 'netpoll_poll_dev'
is in the file net/core/netpoll.c. What is the purpose? Could you please tell me?
Thanks in advance.
> {
> const struct net_device_ops *ops;
>
> @@ -208,13 +208,11 @@ void netpoll_poll_dev(struct net_device *dev)
>
> zap_completion_queue();
> }
> -EXPORT_SYMBOL(netpoll_poll_dev);
>
> -void netpoll_poll(struct netpoll *np)
> +static void netpoll_poll(struct netpoll *np)
> {
> netpoll_poll_dev(np->dev);
> }
> -EXPORT_SYMBOL(netpoll_poll);
>
> static void refill_skbs(void)
> {
> --
> 1.7.6.rc1
^ permalink raw reply
* Re: possible bridge regression in "bridge: implement [add/del]_slave ops"?
From: Alexander Stein @ 2011-07-04 6:46 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, bridge, netdev
In-Reply-To: <20110630100819.0e4b536f@nehalam.ftrdhcpuser.net>
On Thursday 30 June 2011 19:08:19 Stephen Hemminger wrote:
> On Thu, 30 Jun 2011 10:33:23 +0200
>
> Alexander Stein <alexander.stein@systec-electronic.com> wrote:
> > * echo $(pgrep rstpd) > /var/run/rstpd.pid
> > * brctl addbr br1
> > * echo 1 > /sys/class/net/br1/bridge/stp_state
>
> This bogus. You are running both kernel and spanning
> tree daemon at the same time!
>
> Doing the echo of 1 to stp_state forces kernel spanning
> tree. You want 2 which is what is supposed to be use for user
> mode spanning tree.
Well, at first sight you're right. But looking at the code at
br_stp_set_enabled() in br_stp_if.c it doesn't matter which value you echo
into stp_state, it just have to be non-zero
The detection of usermode oder kernelmode STP is done using the usermode
helper (see br_stp_start). I tried echoing 1, 2 or even 5 into stp_state, with
a working /sbin/bridge-stp I get 2 from stp_state each time.
Maybe it is valuable to change this odd behavior.
Regards,
Alexander
^ permalink raw reply
* Re: [PATCH] net/fec: add device tree probe support
From: Grant Likely @ 2011-07-04 6:38 UTC (permalink / raw)
To: Shawn Guo
Cc: Shawn Guo, patches, netdev, devicetree-discuss, Jason Liu,
David S. Miller, linux-arm-kernel
In-Reply-To: <20110704062521.GD10245@S2100-06.ap.freescale.net>
On Mon, Jul 04, 2011 at 02:25:22PM +0800, Shawn Guo wrote:
> On Sun, Jul 03, 2011 at 11:59:56PM -0600, Grant Likely wrote:
> > On Mon, Jul 04, 2011 at 01:50:22PM +0800, Shawn Guo wrote:
> > > On Sun, Jul 03, 2011 at 03:23:12PM -0600, Grant Likely wrote:
> > > > On Sun, Jul 03, 2011 at 04:06:41PM +0800, Shawn Guo wrote:
> > > > > It adds device tree probe support for fec driver.
> > > > >
> > > > > Signed-off-by: Jason Liu <jason.hui@linaro.org>
> > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > Cc: David S. Miller <davem@davemloft.net>
> > > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > >
> > > > Minor comments below. After addressing them you can add my:
> > > >
> > > > Acked-by: Grant Likely <grant.likely@secretlab.ca>
> > > >
> > > > I don't see any reason not to merge this one in v3.1
> > > >
> > > > g.
> > > >
> > > > > ---
> > > > > Documentation/devicetree/bindings/net/fsl-fec.txt | 24 ++++
> > > > > drivers/net/fec.c | 120 ++++++++++++++++++++-
> > > > > 2 files changed, 139 insertions(+), 5 deletions(-)
> > > > > create mode 100644 Documentation/devicetree/bindings/net/fsl-fec.txt
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
> > > > > new file mode 100644
> > > > > index 0000000..1dad888
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
> > > > > @@ -0,0 +1,24 @@
> > > > > +* Freescale Fast Ethernet Controller (FEC)
> > > > > +
> > > > > +Required properties:
> > > > > +- compatible : Should be "fsl,<soc>-fec"
> > > > > +- reg : Address and length of the register set for the device
> > > > > +- interrupts : Should contain fec interrupt
> > > > > +- phy-mode : String, operation mode of the PHY interface.
> > > > > + Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> > > > > + "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi".
> > > >
> > > > We don't have a common binding for this yet. Should be
> > > > "fsl,phy-mode".
> > > >
> > > There is nothing really fsl specific.
> >
> > That's not really the point. It might be that phy-mode as you're
> > defining it here will not be sufficient for other ethernet devices.
>
> I'm not defining the phy mode. Instead, it's just a direct mapping
> of 'enum phy_interface_t' found in include/linux/phy.h, which is
> something common and generic, I think?
>
> > You should avoid creating 'generic' property definitions, unless
> > you're doing the leg work of figuring out which devices would use this
> > property and creating multiple users.
> >
>
> Any net device driver currently referring to 'enum phy_interface_t'
> is going to need this property when migrating to dt, I guess.
>
> > So, you may be right that this isn't fsl specific, but it is still
> > good practise to use that "fsl," prefix.
> >
> >
> > > How does thn following patch
> > > look to you?
> > >
> > > ---8<---------
> > > diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
> > > index 86f334a..cc117db 100644
> > > --- a/drivers/of/of_net.c
> > > +++ b/drivers/of/of_net.c
> > > @@ -8,6 +8,49 @@
> > > #include <linux/etherdevice.h>
> > > #include <linux/kernel.h>
> > > #include <linux/of_net.h>
> > > +#include <linux/phy.h>
> > > +
> > > +/**
> > > + * It maps 'enum phy_interface_t' found in include/linux/phy.h
> > > + * into the device tree binding of 'phy-mode', so that Ethernet
> > > + * device driver can get phy interface from device tree.
> > > + */
> > > +static const char *phy_modes[] = {
> > > + [PHY_INTERFACE_MODE_MII] = "mii",
> > > + [PHY_INTERFACE_MODE_GMII] = "gmii",
> > > + [PHY_INTERFACE_MODE_SGMII] = "sgmii",
> > > + [PHY_INTERFACE_MODE_TBI] = "tbi",
> > > + [PHY_INTERFACE_MODE_RMII] = "rmii",
> > > + [PHY_INTERFACE_MODE_RGMII] = "rgmii",
> > > + [PHY_INTERFACE_MODE_RGMII_ID] = "rgmii-id",
> > > + [PHY_INTERFACE_MODE_RGMII_RXID] = "rgmii-rxid",
> > > + [PHY_INTERFACE_MODE_RGMII_TXID] = "rgmii-txid",
> > > + [PHY_INTERFACE_MODE_RTBI] = "rtbi",
> > > +};
> > > +
> > > +/**
> > > + * of_get_phy_mode - Get phy mode for given device_node
> > > + * @np: Pointer to the given device_node
> > > + *
> > > + * The function gets phy interface string from property 'phy-mode',
> > > + * and return its index in phy_modes table, or errno in error case.
> > > + */
> > > +const int of_get_phy_mode(struct device_node *np)
> > > +{
> > > + const char *pm;
> > > + int err, i;
> > > +
> > > + err = of_property_read_string(np, "phy-mode", &pm);
> > > + if (err < 0)
> > > + return err;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(phy_modes); i++)
> > > + if (!strcasecmp(pm, phy_modes[i]))
> > > + return i;
> > > +
> > > + return -ENODEV;
> > > +}
> > > +EXPORT_SYMBOL_GPL(of_get_phy_mode);
> >
> > The code looks fine, but I'm just not keen on this until I see
> > multiple users. What are the other DT-aware network drivers doing
> > right now?
> >
> You can find a bunch of examples using 'phy-mode' in this way with
> the grep below.
>
> $ grep phy-mode arch/powerpc/boot/dts/*
>
> The code I can find is drivers/net/ibm_newemac/core.c, function
> emac_init_config. And I think it should use 'enum phy_interface_t'
> than defining its own PHY_MODE_*.
Fair enough. Go ahead and create the common definition. Can you
craft a patch to convert the ibm_newemac driver to use your common
function too?
g.
^ permalink raw reply
* Re: [PATCH] net/fec: add device tree probe support
From: Shawn Guo @ 2011-07-04 6:25 UTC (permalink / raw)
To: Grant Likely
Cc: patches, netdev, devicetree-discuss, Jason Liu, Shawn Guo,
David S. Miller, linux-arm-kernel
In-Reply-To: <20110704055956.GD15152@ponder.secretlab.ca>
On Sun, Jul 03, 2011 at 11:59:56PM -0600, Grant Likely wrote:
> On Mon, Jul 04, 2011 at 01:50:22PM +0800, Shawn Guo wrote:
> > On Sun, Jul 03, 2011 at 03:23:12PM -0600, Grant Likely wrote:
> > > On Sun, Jul 03, 2011 at 04:06:41PM +0800, Shawn Guo wrote:
> > > > It adds device tree probe support for fec driver.
> > > >
> > > > Signed-off-by: Jason Liu <jason.hui@linaro.org>
> > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > Cc: David S. Miller <davem@davemloft.net>
> > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > >
> > > Minor comments below. After addressing them you can add my:
> > >
> > > Acked-by: Grant Likely <grant.likely@secretlab.ca>
> > >
> > > I don't see any reason not to merge this one in v3.1
> > >
> > > g.
> > >
> > > > ---
> > > > Documentation/devicetree/bindings/net/fsl-fec.txt | 24 ++++
> > > > drivers/net/fec.c | 120 ++++++++++++++++++++-
> > > > 2 files changed, 139 insertions(+), 5 deletions(-)
> > > > create mode 100644 Documentation/devicetree/bindings/net/fsl-fec.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
> > > > new file mode 100644
> > > > index 0000000..1dad888
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
> > > > @@ -0,0 +1,24 @@
> > > > +* Freescale Fast Ethernet Controller (FEC)
> > > > +
> > > > +Required properties:
> > > > +- compatible : Should be "fsl,<soc>-fec"
> > > > +- reg : Address and length of the register set for the device
> > > > +- interrupts : Should contain fec interrupt
> > > > +- phy-mode : String, operation mode of the PHY interface.
> > > > + Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> > > > + "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi".
> > >
> > > We don't have a common binding for this yet. Should be
> > > "fsl,phy-mode".
> > >
> > There is nothing really fsl specific.
>
> That's not really the point. It might be that phy-mode as you're
> defining it here will not be sufficient for other ethernet devices.
I'm not defining the phy mode. Instead, it's just a direct mapping
of 'enum phy_interface_t' found in include/linux/phy.h, which is
something common and generic, I think?
> You should avoid creating 'generic' property definitions, unless
> you're doing the leg work of figuring out which devices would use this
> property and creating multiple users.
>
Any net device driver currently referring to 'enum phy_interface_t'
is going to need this property when migrating to dt, I guess.
> So, you may be right that this isn't fsl specific, but it is still
> good practise to use that "fsl," prefix.
>
>
> > How does thn following patch
> > look to you?
> >
> > ---8<---------
> > diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
> > index 86f334a..cc117db 100644
> > --- a/drivers/of/of_net.c
> > +++ b/drivers/of/of_net.c
> > @@ -8,6 +8,49 @@
> > #include <linux/etherdevice.h>
> > #include <linux/kernel.h>
> > #include <linux/of_net.h>
> > +#include <linux/phy.h>
> > +
> > +/**
> > + * It maps 'enum phy_interface_t' found in include/linux/phy.h
> > + * into the device tree binding of 'phy-mode', so that Ethernet
> > + * device driver can get phy interface from device tree.
> > + */
> > +static const char *phy_modes[] = {
> > + [PHY_INTERFACE_MODE_MII] = "mii",
> > + [PHY_INTERFACE_MODE_GMII] = "gmii",
> > + [PHY_INTERFACE_MODE_SGMII] = "sgmii",
> > + [PHY_INTERFACE_MODE_TBI] = "tbi",
> > + [PHY_INTERFACE_MODE_RMII] = "rmii",
> > + [PHY_INTERFACE_MODE_RGMII] = "rgmii",
> > + [PHY_INTERFACE_MODE_RGMII_ID] = "rgmii-id",
> > + [PHY_INTERFACE_MODE_RGMII_RXID] = "rgmii-rxid",
> > + [PHY_INTERFACE_MODE_RGMII_TXID] = "rgmii-txid",
> > + [PHY_INTERFACE_MODE_RTBI] = "rtbi",
> > +};
> > +
> > +/**
> > + * of_get_phy_mode - Get phy mode for given device_node
> > + * @np: Pointer to the given device_node
> > + *
> > + * The function gets phy interface string from property 'phy-mode',
> > + * and return its index in phy_modes table, or errno in error case.
> > + */
> > +const int of_get_phy_mode(struct device_node *np)
> > +{
> > + const char *pm;
> > + int err, i;
> > +
> > + err = of_property_read_string(np, "phy-mode", &pm);
> > + if (err < 0)
> > + return err;
> > +
> > + for (i = 0; i < ARRAY_SIZE(phy_modes); i++)
> > + if (!strcasecmp(pm, phy_modes[i]))
> > + return i;
> > +
> > + return -ENODEV;
> > +}
> > +EXPORT_SYMBOL_GPL(of_get_phy_mode);
>
> The code looks fine, but I'm just not keen on this until I see
> multiple users. What are the other DT-aware network drivers doing
> right now?
>
You can find a bunch of examples using 'phy-mode' in this way with
the grep below.
$ grep phy-mode arch/powerpc/boot/dts/*
The code I can find is drivers/net/ibm_newemac/core.c, function
emac_init_config. And I think it should use 'enum phy_interface_t'
than defining its own PHY_MODE_*.
--
Regards,
Shawn
^ permalink raw reply
* Re: [PATCH] net/fec: add device tree probe support
From: Grant Likely @ 2011-07-04 5:59 UTC (permalink / raw)
To: Shawn Guo
Cc: Shawn Guo, patches, netdev, devicetree-discuss, Jason Liu,
David S. Miller, linux-arm-kernel
In-Reply-To: <20110704055021.GC10245@S2100-06.ap.freescale.net>
On Mon, Jul 04, 2011 at 01:50:22PM +0800, Shawn Guo wrote:
> On Sun, Jul 03, 2011 at 03:23:12PM -0600, Grant Likely wrote:
> > On Sun, Jul 03, 2011 at 04:06:41PM +0800, Shawn Guo wrote:
> > > It adds device tree probe support for fec driver.
> > >
> > > Signed-off-by: Jason Liu <jason.hui@linaro.org>
> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > Cc: David S. Miller <davem@davemloft.net>
> > > Cc: Grant Likely <grant.likely@secretlab.ca>
> >
> > Minor comments below. After addressing them you can add my:
> >
> > Acked-by: Grant Likely <grant.likely@secretlab.ca>
> >
> > I don't see any reason not to merge this one in v3.1
> >
> > g.
> >
> > > ---
> > > Documentation/devicetree/bindings/net/fsl-fec.txt | 24 ++++
> > > drivers/net/fec.c | 120 ++++++++++++++++++++-
> > > 2 files changed, 139 insertions(+), 5 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/net/fsl-fec.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
> > > new file mode 100644
> > > index 0000000..1dad888
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
> > > @@ -0,0 +1,24 @@
> > > +* Freescale Fast Ethernet Controller (FEC)
> > > +
> > > +Required properties:
> > > +- compatible : Should be "fsl,<soc>-fec"
> > > +- reg : Address and length of the register set for the device
> > > +- interrupts : Should contain fec interrupt
> > > +- phy-mode : String, operation mode of the PHY interface.
> > > + Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> > > + "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi".
> >
> > We don't have a common binding for this yet. Should be
> > "fsl,phy-mode".
> >
> There is nothing really fsl specific.
That's not really the point. It might be that phy-mode as you're
defining it here will not be sufficient for other ethernet devices.
You should avoid creating 'generic' property definitions, unless
you're doing the leg work of figuring out which devices would use this
property and creating multiple users.
So, you may be right that this isn't fsl specific, but it is still
good practise to use that "fsl," prefix.
> How does thn following patch
> look to you?
>
> ---8<---------
> diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
> index 86f334a..cc117db 100644
> --- a/drivers/of/of_net.c
> +++ b/drivers/of/of_net.c
> @@ -8,6 +8,49 @@
> #include <linux/etherdevice.h>
> #include <linux/kernel.h>
> #include <linux/of_net.h>
> +#include <linux/phy.h>
> +
> +/**
> + * It maps 'enum phy_interface_t' found in include/linux/phy.h
> + * into the device tree binding of 'phy-mode', so that Ethernet
> + * device driver can get phy interface from device tree.
> + */
> +static const char *phy_modes[] = {
> + [PHY_INTERFACE_MODE_MII] = "mii",
> + [PHY_INTERFACE_MODE_GMII] = "gmii",
> + [PHY_INTERFACE_MODE_SGMII] = "sgmii",
> + [PHY_INTERFACE_MODE_TBI] = "tbi",
> + [PHY_INTERFACE_MODE_RMII] = "rmii",
> + [PHY_INTERFACE_MODE_RGMII] = "rgmii",
> + [PHY_INTERFACE_MODE_RGMII_ID] = "rgmii-id",
> + [PHY_INTERFACE_MODE_RGMII_RXID] = "rgmii-rxid",
> + [PHY_INTERFACE_MODE_RGMII_TXID] = "rgmii-txid",
> + [PHY_INTERFACE_MODE_RTBI] = "rtbi",
> +};
> +
> +/**
> + * of_get_phy_mode - Get phy mode for given device_node
> + * @np: Pointer to the given device_node
> + *
> + * The function gets phy interface string from property 'phy-mode',
> + * and return its index in phy_modes table, or errno in error case.
> + */
> +const int of_get_phy_mode(struct device_node *np)
> +{
> + const char *pm;
> + int err, i;
> +
> + err = of_property_read_string(np, "phy-mode", &pm);
> + if (err < 0)
> + return err;
> +
> + for (i = 0; i < ARRAY_SIZE(phy_modes); i++)
> + if (!strcasecmp(pm, phy_modes[i]))
> + return i;
> +
> + return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(of_get_phy_mode);
The code looks fine, but I'm just not keen on this until I see
multiple users. What are the other DT-aware network drivers doing
right now?
>
> /**
> * Search the device tree for the best MAC address to use. 'mac-address' is
> ------------
>
> [...]
> > > @@ -734,8 +748,25 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> > > */
> > > iap = macaddr;
> > >
> > > +#ifdef CONFIG_OF
> > > + /*
> > > + * 2) from device tree data
> > > + */
> > > + if (!is_valid_ether_addr(iap)) {
> > > + struct device_node *np = fep->pdev->dev.of_node;
> > > + if (np) {
> > > + const char *mac;
> > > + int err;
> > > + err = of_property_read_string(np,
> > > + "local-mac-address", &mac);
> > > + if (err)
> > > + iap = (unsigned char *) mac;
> >
> > There is already a function for doing this. of_get_mac_address().
> > And mac address is *not* a string, it is a byte array.
> >
> Yes, I mistakenly converted of_find_property to of_property_read_string.
> Anyway, I'm now using of_get_mac_address as you suggested.
:)
g.
^ permalink raw reply
* Re: [PATCH] net/fec: add device tree probe support
From: Shawn Guo @ 2011-07-04 5:50 UTC (permalink / raw)
To: Grant Likely
Cc: Shawn Guo, patches, netdev, devicetree-discuss, Jason Liu,
David S. Miller, linux-arm-kernel
In-Reply-To: <20110703212312.GD13742@ponder.secretlab.ca>
On Sun, Jul 03, 2011 at 03:23:12PM -0600, Grant Likely wrote:
> On Sun, Jul 03, 2011 at 04:06:41PM +0800, Shawn Guo wrote:
> > It adds device tree probe support for fec driver.
> >
> > Signed-off-by: Jason Liu <jason.hui@linaro.org>
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > Cc: David S. Miller <davem@davemloft.net>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
>
> Minor comments below. After addressing them you can add my:
>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
>
> I don't see any reason not to merge this one in v3.1
>
> g.
>
> > ---
> > Documentation/devicetree/bindings/net/fsl-fec.txt | 24 ++++
> > drivers/net/fec.c | 120 ++++++++++++++++++++-
> > 2 files changed, 139 insertions(+), 5 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/net/fsl-fec.txt
> >
> > diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
> > new file mode 100644
> > index 0000000..1dad888
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
> > @@ -0,0 +1,24 @@
> > +* Freescale Fast Ethernet Controller (FEC)
> > +
> > +Required properties:
> > +- compatible : Should be "fsl,<soc>-fec"
> > +- reg : Address and length of the register set for the device
> > +- interrupts : Should contain fec interrupt
> > +- phy-mode : String, operation mode of the PHY interface.
> > + Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> > + "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi".
>
> We don't have a common binding for this yet. Should be
> "fsl,phy-mode".
>
There is nothing really fsl specific. How does the following patch
look to you?
---8<---------
diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index 86f334a..cc117db 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -8,6 +8,49 @@
#include <linux/etherdevice.h>
#include <linux/kernel.h>
#include <linux/of_net.h>
+#include <linux/phy.h>
+
+/**
+ * It maps 'enum phy_interface_t' found in include/linux/phy.h
+ * into the device tree binding of 'phy-mode', so that Ethernet
+ * device driver can get phy interface from device tree.
+ */
+static const char *phy_modes[] = {
+ [PHY_INTERFACE_MODE_MII] = "mii",
+ [PHY_INTERFACE_MODE_GMII] = "gmii",
+ [PHY_INTERFACE_MODE_SGMII] = "sgmii",
+ [PHY_INTERFACE_MODE_TBI] = "tbi",
+ [PHY_INTERFACE_MODE_RMII] = "rmii",
+ [PHY_INTERFACE_MODE_RGMII] = "rgmii",
+ [PHY_INTERFACE_MODE_RGMII_ID] = "rgmii-id",
+ [PHY_INTERFACE_MODE_RGMII_RXID] = "rgmii-rxid",
+ [PHY_INTERFACE_MODE_RGMII_TXID] = "rgmii-txid",
+ [PHY_INTERFACE_MODE_RTBI] = "rtbi",
+};
+
+/**
+ * of_get_phy_mode - Get phy mode for given device_node
+ * @np: Pointer to the given device_node
+ *
+ * The function gets phy interface string from property 'phy-mode',
+ * and return its index in phy_modes table, or errno in error case.
+ */
+const int of_get_phy_mode(struct device_node *np)
+{
+ const char *pm;
+ int err, i;
+
+ err = of_property_read_string(np, "phy-mode", &pm);
+ if (err < 0)
+ return err;
+
+ for (i = 0; i < ARRAY_SIZE(phy_modes); i++)
+ if (!strcasecmp(pm, phy_modes[i]))
+ return i;
+
+ return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(of_get_phy_mode);
/**
* Search the device tree for the best MAC address to use. 'mac-address' is
------------
[...]
> > @@ -734,8 +748,25 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> > */
> > iap = macaddr;
> >
> > +#ifdef CONFIG_OF
> > + /*
> > + * 2) from device tree data
> > + */
> > + if (!is_valid_ether_addr(iap)) {
> > + struct device_node *np = fep->pdev->dev.of_node;
> > + if (np) {
> > + const char *mac;
> > + int err;
> > + err = of_property_read_string(np,
> > + "local-mac-address", &mac);
> > + if (err)
> > + iap = (unsigned char *) mac;
>
> There is already a function for doing this. of_get_mac_address().
> And mac address is *not* a string, it is a byte array.
>
Yes, I mistakenly converted of_find_property to of_property_read_string.
Anyway, I'm now using of_get_mac_address as you suggested.
--
Regards,
Shawn
^ permalink raw reply related
* Re: [PATCH] natsemi: silence dma-debug warnings
From: David Miller @ 2011-07-04 5:34 UTC (permalink / raw)
To: fujita.tomonori; +Cc: thockin, netdev, jim.cromie
In-Reply-To: <20110704142153O.fujita.tomonori@lab.ntt.co.jp>
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Date: Mon, 4 Jul 2011 14:30:19 +0900
> This silences dma-debug warnings:
>
> https://lkml.org/lkml/2011/6/30/341
...
> Reported-by: Jim Cromie <jim.cromie@gmail.com>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Tested-by: Jim Cromie <jim.cromie@gmail.com>
Applied, thanks!
^ permalink raw reply
* [PATCH] natsemi: silence dma-debug warnings
From: FUJITA Tomonori @ 2011-07-04 5:30 UTC (permalink / raw)
To: thockin; +Cc: netdev, jim.cromie
This silences dma-debug warnings:
https://lkml.org/lkml/2011/6/30/341
------------[ cut here ]------------
WARNING: at /home/jimc/projects/lx/linux-2.6/lib/dma-debug.c:820
check_unmap+0x1fe/0x56a()
natsemi 0000:00:06.0: DMA-API: device driver frees DMA memory with
different size [device address=0x0000000006ef0040] [map size=1538
bytes] [unmap size=1522 bytes]
Modules linked in: pc8736x_gpio pc87360 hwmon_vid scx200_gpio nsc_gpio
scx200_hrt scx200_acb i2c_core arc4 rtl8180 mac80211 eeprom_93cx6
cfg80211 pcspkr rfkill scx200 ide_gd_mod ide_pci_generic ohci_hcd
usbcore sc1200 ide_core
Pid: 870, comm: collector Not tainted 3.0.0-rc5-sk-00080-gca56a95 #1
Call Trace:
[<c011a556>] warn_slowpath_common+0x4a/0x5f
[<c02565cb>] ? check_unmap+0x1fe/0x56a
[<c011a5cf>] warn_slowpath_fmt+0x26/0x2a
[<c02565cb>] check_unmap+0x1fe/0x56a
[<c0256aaa>] debug_dma_unmap_page+0x53/0x5b
[<c029d6cd>] pci_unmap_single+0x4d/0x57
[<c029ea0a>] natsemi_poll+0x343/0x5ca
[<c0116f41>] ? try_to_wake_up+0xea/0xfc
[<c0122416>] ? spin_unlock_irq.clone.28+0x18/0x23
[<c02d4667>] net_rx_action+0x3f/0xe5
[<c011e35e>] __do_softirq+0x5b/0xd1
[<c011e303>] ? local_bh_enable+0xa/0xa
<IRQ> [<c011e54b>] ? irq_exit+0x34/0x75
[<c01034b9>] ? do_IRQ+0x66/0x79
[<c034e869>] ? common_interrupt+0x29/0x30
[<c0115ed0>] ? finish_task_switch.clone.118+0x31/0x72
[<c034cb92>] ? schedule+0x3b2/0x3f1
[<c012f4b0>] ? hrtimer_start_range_ns+0x10/0x12
[<c012f4ce>] ? hrtimer_start_expires+0x1c/0x24
[<c034d5aa>] ? schedule_hrtimeout_range_clock+0x8e/0xb4
[<c012ed27>] ? update_rmtp+0x68/0x68
[<c034d5da>] ? schedule_hrtimeout_range+0xa/0xc
[<c017a913>] ? poll_schedule_timeout+0x27/0x3e
[<c017b051>] ? do_select+0x488/0x4cd
[<c0115ee2>] ? finish_task_switch.clone.118+0x43/0x72
[<c01157ad>] ? need_resched+0x14/0x1e
[<c017a99e>] ? poll_freewait+0x74/0x74
[<c01157ad>] ? need_resched+0x14/0x1e
[<c034cbc1>] ? schedule+0x3e1/0x3f1
[<c011e55e>] ? irq_exit+0x47/0x75
[<c01157ad>] ? need_resched+0x14/0x1e
[<c034cf8a>] ? preempt_schedule_irq+0x44/0x4a
[<c034dd1e>] ? need_resched+0x17/0x19
[<c024bc12>] ? put_dec_full+0x7b/0xaa
[<c0240060>] ? blkdev_ioctl+0x434/0x618
[<c024bc70>] ? put_dec+0x2f/0x6d
[<c024c6a5>] ? number.clone.1+0x10b/0x1d0
[<c034cf8a>] ? preempt_schedule_irq+0x44/0x4a
[<c034dd1e>] ? need_resched+0x17/0x19
[<c024d046>] ? vsnprintf+0x225/0x264
[<c024cea0>] ? vsnprintf+0x7f/0x264
[<c018346f>] ? seq_printf+0x22/0x40
[<c01a2fcc>] ? do_task_stat+0x582/0x5a3
[<c017a913>] ? poll_schedule_timeout+0x27/0x3e
[<c017b1b5>] ? core_sys_select+0x11f/0x1a3
[<c017a913>] ? poll_schedule_timeout+0x27/0x3e
[<c01a34a1>] ? proc_tgid_stat+0xd/0xf
[<c012357c>] ? recalc_sigpending+0x32/0x35
[<c0123b9c>] ? __set_task_blocked+0x64/0x6a
[<c011dfb0>] ? timespec_add_safe+0x24/0x48
[<c0123449>] ? spin_unlock_irq.clone.16+0x18/0x23
[<c017b3a1>] ? sys_pselect6+0xe5/0x13e
[<c034dd65>] ? syscall_call+0x7/0xb
[<c0340000>] ? rpc_clntdir_depopulate+0x26/0x30
---[ end trace 180dcac41a50938b ]---
Reported-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Tested-by: Jim Cromie <jim.cromie@gmail.com>
---
drivers/net/natsemi.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/natsemi.c b/drivers/net/natsemi.c
index b78be08..8f8b65a 100644
--- a/drivers/net/natsemi.c
+++ b/drivers/net/natsemi.c
@@ -2360,7 +2360,8 @@ static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do)
PCI_DMA_FROMDEVICE);
} else {
pci_unmap_single(np->pci_dev, np->rx_dma[entry],
- buflen, PCI_DMA_FROMDEVICE);
+ buflen + NATSEMI_PADDING,
+ PCI_DMA_FROMDEVICE);
skb_put(skb = np->rx_skbuff[entry], pkt_len);
np->rx_skbuff[entry] = NULL;
}
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH] net: sh_eth: remove __flush_purge_region
From: David Miller @ 2011-07-04 3:10 UTC (permalink / raw)
To: yoshihiro.shimoda.uh; +Cc: netdev, linux-sh
In-Reply-To: <4E0D8ABD.9030102@renesas.com>
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Date: Fri, 01 Jul 2011 17:52:13 +0900
> It is a function of SuperH architecture. There is no good to use
> the function on a driver generally. So, the driver uses
> dma_map_single() instead of __flush_purge_region.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Applied.
^ permalink raw reply
* Re: [PATCH] af_econet: Use current logging styles and neatening
From: David Miller @ 2011-07-04 3:05 UTC (permalink / raw)
To: joe; +Cc: netdev, linux-kernel, philb
In-Reply-To: <8e47a6e010a39bd35d3544311dd451abda8b7fb3.1309717580.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Sun, 3 Jul 2011 11:28:33 -0700
> Use pr_fmt() without KBUILD_MODNAME to allow AUN and econet prefixes.
> Convert printks with KERN_DEBUG to pr_debug.
> Hoist assigns from if.
> 80 column wrapping.
> Move open braces to end of line.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 2/2] netpoll: Remove wrapper function netpoll_poll
From: David Miller @ 2011-07-04 3:02 UTC (permalink / raw)
To: joe; +Cc: linux-kernel, amwang, netdev
In-Reply-To: <6d0de0ce0430d29d1808ecba1ce306ccca52247e.1309482314.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Thu, 30 Jun 2011 18:08:58 -0700
> Too trivial to live.
>
> cc: WANG Cong <amwang@redhat.com>
> Signed-off-by: Joe Perches <joe@perches.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/2] netpoll: Remove unused EXPORT_SYMBOLs of netpoll_poll and netpoll_poll_dev
From: David Miller @ 2011-07-04 3:02 UTC (permalink / raw)
To: joe; +Cc: linux-kernel, amwang, netdev
In-Reply-To: <282ce7d68b97c4dbfb759c1c0752e6cbddfeeafc.1309482314.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Thu, 30 Jun 2011 18:08:57 -0700
> Unused symbols waste space.
>
> Commit 0e34e93177fb
> "(netpoll: add generic support for bridge and bonding devices)"
> added the symbol more than a year ago with the promise of "future use".
>
> Because it is so far unused, remove it for now.
> It can be easily readded if or when it actually needs to be used.
>
> cc: WANG Cong <amwang@redhat.com>
> Signed-off-by: Joe Perches <joe@perches.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: 8139too: Initial necessary vlan_features to support vlan
From: David Miller @ 2011-07-04 2:54 UTC (permalink / raw)
To: romieu; +Cc: shanwei, netdev, adobriyan, tj
In-Reply-To: <20110702095138.GA4736@electric-eye.fr.zoreil.com>
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Sat, 2 Jul 2011 11:51:38 +0200
> Shan Wei <shanwei@cn.fujitsu.com> :
>> Offload setting of vlan device requires
>> vlan_features to be initialed.
> [...]
>> @@ -993,6 +993,7 @@ static int __devinit rtl8139_init_one (struct pci_dev *pdev,
>> * features
>> */
>> dev->features |= NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_HIGHDMA;
>> + dev->vlan_features = dev->features;
>
> /* note: the hardware is not capable of sg/csum/highdma, however
> * through the use of skb_copy_and_csum_dev we enable these
> * features
> */
>
> The commit message is a bit misleading wrt hardware capabilities but the
> patch makes sense.
>
> Acked-by: Francois Romieu <romieu@fr.zoreil.com>
Applied.
^ permalink raw reply
* Re: [rfc] Remove member .name from struct netpoll
From: Cong Wang @ 2011-07-04 2:07 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, LKML, Matt Mackall
In-Reply-To: <1309540071.7277.63.camel@Joe-Laptop>
于 2011年07月02日 01:07, Joe Perches 写道:
> struct netpoll has member .name
>
> struct netpoll {
> struct net_device *dev;
> char dev_name[IFNAMSIZ];
> const char *name;
> [...]
> };
>
> that is set only by netconsole.c
>
> static struct netconsole_target *alloc_param_target(char *target_config)
> {
> [...]
> nt->np.name = "netconsole";
>
> and used only by net/core/netpoll.c to emit "netconsole: " on
> logging messages.
>
Probably this is due to that in history netdump was another user of it.
>
> Are there out of tree users or plans to use "struct netpoll"
> by other modules?
>
AFAIK, netoops will use it too, see
https://lkml.org/lkml/2010/11/2/299
Cc'ing Matt in case I miss something...
Thanks.
^ permalink raw reply
* Re: [PATCH] net/fec: add device tree probe support
From: Grant Likely @ 2011-07-03 21:23 UTC (permalink / raw)
To: Shawn Guo
Cc: netdev, linux-arm-kernel, devicetree-discuss, patches, Jason Liu,
David S. Miller
In-Reply-To: <1309680401-22904-1-git-send-email-shawn.guo@linaro.org>
On Sun, Jul 03, 2011 at 04:06:41PM +0800, Shawn Guo wrote:
> It adds device tree probe support for fec driver.
>
> Signed-off-by: Jason Liu <jason.hui@linaro.org>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Grant Likely <grant.likely@secretlab.ca>
Minor comments below. After addressing them you can add my:
Acked-by: Grant Likely <grant.likely@secretlab.ca>
I don't see any reason not to merge this one in v3.1
g.
> ---
> Documentation/devicetree/bindings/net/fsl-fec.txt | 24 ++++
> drivers/net/fec.c | 120 ++++++++++++++++++++-
> 2 files changed, 139 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/fsl-fec.txt
>
> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
> new file mode 100644
> index 0000000..1dad888
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
> @@ -0,0 +1,24 @@
> +* Freescale Fast Ethernet Controller (FEC)
> +
> +Required properties:
> +- compatible : Should be "fsl,<soc>-fec"
> +- reg : Address and length of the register set for the device
> +- interrupts : Should contain fec interrupt
> +- phy-mode : String, operation mode of the PHY interface.
> + Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> + "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi".
We don't have a common binding for this yet. Should be
"fsl,phy-mode".
> +- gpios : Should specify the gpio for phy reset
> +
> +Optional properties:
> +- local-mac-address : 6 bytes, mac address
> +
> +Example:
> +
> +fec@83fec000 {
> + compatible = "fsl,imx51-fec", "fsl,imx27-fec";
> + reg = <0x83fec000 0x4000>;
> + interrupts = <87>;
> + phy-mode = "mii";
> + gpios = <&gpio1 14 0>; /* phy-reset, GPIO2_14 */
> + local-mac-address = [00 04 9F 01 1B B9];
> +};
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index 7ae3f28..3ee05ff 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -44,6 +44,9 @@
> #include <linux/platform_device.h>
> #include <linux/phy.h>
> #include <linux/fec.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
>
> #include <asm/cacheflush.h>
>
> @@ -78,6 +81,17 @@ static struct platform_device_id fec_devtype[] = {
> { }
> };
>
> +enum fec_type {
> + IMX27_FEC,
> + IMX28_FEC,
> +};
> +
> +static const struct of_device_id fec_dt_ids[] = {
> + { .compatible = "fsl,imx27-fec", .data = &fec_devtype[IMX27_FEC], },
> + { .compatible = "fsl,imx28-fec", .data = &fec_devtype[IMX28_FEC], },
Personally, I'd drop the fec_devtype array entirely and have discrete
structures for each, but that isn't a big enough objection to withhold
my acked-by.
> + { /* sentinel */ },
Drop the trailing comma.
> +};
> +
> static unsigned char macaddr[ETH_ALEN];
> module_param_array(macaddr, byte, NULL, 0);
> MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
> @@ -734,8 +748,25 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> */
> iap = macaddr;
>
> +#ifdef CONFIG_OF
> + /*
> + * 2) from device tree data
> + */
> + if (!is_valid_ether_addr(iap)) {
> + struct device_node *np = fep->pdev->dev.of_node;
> + if (np) {
> + const char *mac;
> + int err;
> + err = of_property_read_string(np,
> + "local-mac-address", &mac);
> + if (err)
> + iap = (unsigned char *) mac;
There is already a function for doing this. of_get_mac_address().
And mac address is *not* a string, it is a byte array.
> + }
> + }
> +#endif
> +
> /*
> - * 2) from flash or fuse (via platform data)
> + * 3) from flash or fuse (via platform data)
> */
> if (!is_valid_ether_addr(iap)) {
> #ifdef CONFIG_M5272
> @@ -748,7 +779,7 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> }
>
> /*
> - * 3) FEC mac registers set by bootloader
> + * 4) FEC mac registers set by bootloader
> */
> if (!is_valid_ether_addr(iap)) {
> *((unsigned long *) &tmpaddr[0]) =
> @@ -1358,6 +1389,72 @@ static int fec_enet_init(struct net_device *ndev)
> return 0;
> }
>
> +#ifdef CONFIG_OF
> +static int __devinit fec_get_phy_mode_dt(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + int err, i;
> + const char *pm, *phy_modes[] = {
> + [PHY_INTERFACE_MODE_MII] = "mii",
> + [PHY_INTERFACE_MODE_GMII] = "gmii",
> + [PHY_INTERFACE_MODE_SGMII] = "sgmii",
> + [PHY_INTERFACE_MODE_TBI] = "tbi",
> + [PHY_INTERFACE_MODE_RMII] = "rmii",
> + [PHY_INTERFACE_MODE_RGMII] = "rgmii",
> + [PHY_INTERFACE_MODE_RGMII_ID] = "rgmii-id",
> + [PHY_INTERFACE_MODE_RGMII_RXID] = "rgmii-rxid",
> + [PHY_INTERFACE_MODE_RGMII_TXID] = "rgmii-txid",
> + [PHY_INTERFACE_MODE_RTBI] = "rtbi",
> + };
The phymodes table should be defined outside of the function.
> +
> + if (np) {
> + err = of_property_read_string(np, "phy-mode", &pm);
> + if (err)
> + return err;
> + for (i = 0; i < ARRAY_SIZE(phy_modes); i++)
> + if (!strcasecmp(pm, phy_modes[i]))
> + return i;
> + }
> +
> + return -ENODEV;
> +}
> +
> +static int __devinit fec_reset_phy(struct platform_device *pdev)
> +{
> + int err, phy_reset;
> + struct device_node *np = pdev->dev.of_node;
> +
> + if (!np)
> + return -ENODEV;
> +
> + phy_reset = of_get_gpio(np, 0);
> + err = gpio_request_one(phy_reset, GPIOF_OUT_INIT_LOW, "phy-reset");
> + if (err) {
> + pr_warn("FEC: failed to get gpio phy-reset: %d\n", err);
> + return err;
> + }
> +
> + msleep(1);
> + gpio_set_value(phy_reset, 1);
> +
> + return 0;
> +}
> +#else /* CONFIG_OF */
> +static inline int fec_get_phy_mode_dt(struct platform_device *pdev)
> +{
> + return -ENODEV;
> +}
> +
> +static inline int fec_reset_phy(struct platform_device *pdev)
> +{
> + /*
> + * In case of platform probe, the reset has been done
> + * by machine code.
> + */
> + return 0;
> +}
> +#endif /* CONFIG_OF */
> +
> static int __devinit
> fec_probe(struct platform_device *pdev)
> {
> @@ -1366,6 +1463,11 @@ fec_probe(struct platform_device *pdev)
> struct net_device *ndev;
> int i, irq, ret = 0;
> struct resource *r;
> + const struct of_device_id *of_id;
> +
> + of_id = of_match_device(fec_dt_ids, &pdev->dev);
> + if (of_id)
> + pdev->id_entry = (struct platform_device_id *) of_id->data;
of_id->data is a void*. You don't need the cast.
>
> r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!r)
> @@ -1397,9 +1499,16 @@ fec_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, ndev);
>
> - pdata = pdev->dev.platform_data;
> - if (pdata)
> - fep->phy_interface = pdata->phy;
> + fep->phy_interface = fec_get_phy_mode_dt(pdev);
> + if (fep->phy_interface == -ENODEV) {
> + pdata = pdev->dev.platform_data;
> + if (pdata)
> + fep->phy_interface = pdata->phy;
> + else
> + fep->phy_interface = PHY_INTERFACE_MODE_MII;
> + }
> +
> + fec_reset_phy(pdev);
>
> /* This device has up to three irqs on some platforms */
> for (i = 0; i < 3; i++) {
> @@ -1534,6 +1643,7 @@ static struct platform_driver fec_driver = {
> #ifdef CONFIG_PM
> .pm = &fec_pm_ops,
> #endif
> + .of_match_table = fec_dt_ids,
> },
> .id_table = fec_devtype,
> .probe = fec_probe,
> --
> 1.7.4.1
>
^ permalink raw reply
* Re: [PATCH next] netfilter: nfqueue: batch verdict support
From: Florian Westphal @ 2011-07-03 19:23 UTC (permalink / raw)
To: Eric Dumazet
Cc: Florian Westphal, Patrick McHardy, netfilter-devel,
Pablo Neira Ayuso, netdev, Eric Leblond
In-Reply-To: <1309688655.2523.16.camel@edumazet-laptop>
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Introduces a new nfnetlink type that applies a given
> > verdict to all queued packets with an id <= the id in the verdict
> > message.
> >
> > If a mark is provided it is applied to all matched packets.
> >
> > This reduces the number of verdicts that have to be sent.
> > Applications that make use of this feature need to maintain
> > a timeout to send a batchverdict periodically to avoid starvation.
> >
> > Signed-off-by: Florian Westphal <fw@strlen.de>
>
> The real question hidden here is : "Should packet ids be monotonic" in
> current implementation and all future ones ?
>
> Before we accept this patch, we should make sure packets id are
> monotonic, and I am afraid its not the case right now.
You're right, good catch.
I was fooled by atomic_inc being monotonically increasing, but since
packet building is not protected by the queue spin lock
reordering can indeed happen.
> [PATCH] netfilter: nfqueue: assert monotonic packet ids
>
> Packet identifier is currently setup in nfqnl_build_packet_message(),
> using one atomic_inc_return().
>
> Problem is that since several cpus might concurrently call
> nfqnl_enqueue_packet() for the same queue, we can deliver packets to
> consumer in non monotonic way (packet N+1 being delivered after packet
> N)
I would actually consider your patch a bug fix...
Thanks a lot for spending time on this!
^ permalink raw reply
* Re: [PATCH] bridge: mask forwarding of IEEE 802 local multicast groups
From: Nick Carter @ 2011-07-03 18:30 UTC (permalink / raw)
To: David Lamparter
Cc: netdev, Michał Mirosław, Stephen Hemminger, davem
In-Reply-To: <20110701223756.GG2613105@jupiter.n2.diac24.net>
On 1 July 2011 23:37, David Lamparter <equinox@diac24.net> wrote:
> On Fri, Jul 01, 2011 at 10:21:44PM +0100, Nick Carter wrote:
>> Introduce sysfs ../bridge/group_fwd_mask attribute so users can
>> configure which group mac addresses are forwarded.
>>
>> These diffs do not change the default behaviour of bridge.ko. By
>> changing the group_fwd_mask value users can select any combination of
>> the 01-80-C2-00-00-00 - 01-80-C2-00-00-0F addresses to be forwarded.
>>
>> Signed-off-by: Nick Carter <ncarter100@gmail.com>
>
> Hm. Makes it very easy to shoot yourself in the foot, but...
>
>> + br->group_fwd_mask = 0;
>
> ... but the default is safe, so we're following the unix philosophy of
> handing out the gun to shoot yourself :)
>
> Reviewed-by: David Lamparter <equinox@diac24.net>
>
>
> PS:
>> @@ -166,6 +166,9 @@ struct sk_buff *br_handle_frame(struct sk_buff *skb)
>> if (p->br->stp_enabled == BR_NO_STP && dest[5] == 0)
>> goto forward;
>
> Not sure if we still need the STP one when we have the mask, it should
> probably go away for consistency. It would change existing behaviour
> though... either way it can be a separate patch.
Yes I agree. Probably best to wait until the user space tools are
updated to support the new mask, so users can easily configure a
workaround to the change in behaviour. I'm happy to update brctl to
support the mask, I pretty much have the diffs already.
Nick
>
>> + if (p->br->group_fwd_mask & (1 << dest[5]))
>> + goto forward;
>> +
>> if (NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
>> NULL, br_handle_local_finish))
>> return NULL; /* frame consumed by filter */
>
^ permalink raw reply
* [PATCH] af_econet: Use current logging styles and neatening
From: Joe Perches @ 2011-07-03 18:28 UTC (permalink / raw)
To: netdev, linux-kernel; +Cc: Phil Blundell, David S. Miller
Use pr_fmt() without KBUILD_MODNAME to allow AUN and econet prefixes.
Convert printks with KERN_DEBUG to pr_debug.
Hoist assigns from if.
80 column wrapping.
Move open braces to end of line.
Signed-off-by: Joe Perches <joe@perches.com>
---
net/econet/af_econet.c | 149 ++++++++++++++++++++++++------------------------
1 files changed, 74 insertions(+), 75 deletions(-)
diff --git a/net/econet/af_econet.c b/net/econet/af_econet.c
index 5afdc8f..1c1f26c 100644
--- a/net/econet/af_econet.c
+++ b/net/econet/af_econet.c
@@ -9,6 +9,8 @@
*
*/
+#define pr_fmt(fmt) fmt
+
#include <linux/module.h>
#include <linux/types.h>
@@ -44,7 +46,7 @@
#include <linux/bitops.h>
#include <linux/mutex.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
#include <asm/system.h>
static const struct proto_ops econet_ops;
@@ -63,9 +65,7 @@ static DEFINE_SPINLOCK(aun_queue_lock);
static struct socket *udpsock;
#define AUN_PORT 0x8000
-
-struct aunhdr
-{
+struct aunhdr {
unsigned char code; /* AUN magic protocol byte */
unsigned char port;
unsigned char cb;
@@ -82,8 +82,7 @@ static struct timer_list ab_cleanup_timer;
#endif /* CONFIG_ECONET_AUNUDP */
/* Per-packet information */
-struct ec_cb
-{
+struct ec_cb {
struct sockaddr_ec sec;
unsigned long cookie; /* Supplied by user. */
#ifdef CONFIG_ECONET_AUNUDP
@@ -137,7 +136,7 @@ static int econet_recvmsg(struct kiocb *iocb, struct socket *sock,
* but then it will block.
*/
- skb=skb_recv_datagram(sk,flags,flags&MSG_DONTWAIT,&err);
+ skb = skb_recv_datagram(sk, flags, flags & MSG_DONTWAIT, &err);
/*
* An error occurred so return it. Because skb_recv_datagram()
@@ -145,7 +144,7 @@ static int econet_recvmsg(struct kiocb *iocb, struct socket *sock,
* retries.
*/
- if(skb==NULL)
+ if (skb == NULL)
goto out;
/*
@@ -154,10 +153,9 @@ static int econet_recvmsg(struct kiocb *iocb, struct socket *sock,
*/
copied = skb->len;
- if (copied > len)
- {
- copied=len;
- msg->msg_flags|=MSG_TRUNC;
+ if (copied > len) {
+ copied = len;
+ msg->msg_flags |= MSG_TRUNC;
}
/* We can't use skb_copy_datagram here */
@@ -186,7 +184,8 @@ out:
* Bind an Econet socket.
*/
-static int econet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
+static int econet_bind(struct socket *sock, struct sockaddr *uaddr,
+ int addr_len)
{
struct sockaddr_ec *sec = (struct sockaddr_ec *)uaddr;
struct sock *sk;
@@ -226,9 +225,8 @@ static void tx_result(struct sock *sk, unsigned long cookie, int result)
struct ec_cb *eb;
struct sockaddr_ec *sec;
- if (skb == NULL)
- {
- printk(KERN_DEBUG "ec: memory squeeze, transmit result dropped.\n");
+ if (skb == NULL) {
+ pr_debug("econet: memory squeeze, transmit result dropped\n");
return;
}
@@ -265,7 +263,7 @@ static void ec_tx_done(struct sk_buff *skb, int result)
static int econet_sendmsg(struct kiocb *iocb, struct socket *sock,
struct msghdr *msg, size_t len)
{
- struct sockaddr_ec *saddr=(struct sockaddr_ec *)msg->msg_name;
+ struct sockaddr_ec *saddr = (struct sockaddr_ec *)msg->msg_name;
struct net_device *dev;
struct ec_addr addr;
int err;
@@ -298,14 +296,14 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock,
mutex_lock(&econet_mutex);
- if (saddr == NULL || msg->msg_namelen < sizeof(struct sockaddr_ec)) {
- mutex_unlock(&econet_mutex);
- return -EINVAL;
- }
- addr.station = saddr->addr.station;
- addr.net = saddr->addr.net;
- port = saddr->port;
- cb = saddr->cb;
+ if (saddr == NULL || msg->msg_namelen < sizeof(struct sockaddr_ec)) {
+ mutex_unlock(&econet_mutex);
+ return -EINVAL;
+ }
+ addr.station = saddr->addr.station;
+ addr.net = saddr->addr.net;
+ port = saddr->port;
+ cb = saddr->cb;
/* Look for a device with the right network number. */
dev = net2dev_map[addr.net];
@@ -333,9 +331,9 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock,
dev_hold(dev);
- skb = sock_alloc_send_skb(sk, len+LL_ALLOCATED_SPACE(dev),
+ skb = sock_alloc_send_skb(sk, len + LL_ALLOCATED_SPACE(dev),
msg->msg_flags & MSG_DONTWAIT, &err);
- if (skb==NULL)
+ if (skb == NULL)
goto out_unlock;
skb_reserve(skb, LL_RESERVED_SPACE(dev));
@@ -355,7 +353,7 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock,
struct ec_framehdr *fh;
/* Poke in our control byte and
port number. Hack, hack. */
- fh = (struct ec_framehdr *)(skb->data);
+ fh = (struct ec_framehdr *)skb->data;
fh->cb = cb;
fh->port = port;
if (sock->type != SOCK_DGRAM) {
@@ -365,7 +363,7 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock,
}
/* Copy the data. Returns -EFAULT on error */
- err = memcpy_fromiovec(skb_put(skb,len), msg->msg_iov, len);
+ err = memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len);
skb->protocol = proto;
skb->dev = dev;
skb->priority = sk->sk_priority;
@@ -385,9 +383,9 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock,
mutex_unlock(&econet_mutex);
return len;
- out_free:
+out_free:
kfree_skb(skb);
- out_unlock:
+out_unlock:
if (dev)
dev_put(dev);
#else
@@ -458,15 +456,14 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock,
goto error_free_buf;
/* Get a skbuff (no data, just holds our cb information) */
- if ((skb = sock_alloc_send_skb(sk, 0,
- msg->msg_flags & MSG_DONTWAIT,
- &err)) == NULL)
+ skb = sock_alloc_send_skb(sk, 0, msg->msg_flags & MSG_DONTWAIT, &err);
+ if (skb == NULL)
goto error_free_buf;
eb = (struct ec_cb *)&skb->cb;
eb->cookie = saddr->cookie;
- eb->timeout = (5*HZ);
+ eb->timeout = 5 * HZ;
eb->start = jiffies;
ah.handle = aun_seq;
eb->seq = (aun_seq++);
@@ -480,9 +477,10 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock,
udpmsg.msg_iovlen = 2;
udpmsg.msg_control = NULL;
udpmsg.msg_controllen = 0;
- udpmsg.msg_flags=0;
+ udpmsg.msg_flags = 0;
- oldfs = get_fs(); set_fs(KERNEL_DS); /* More privs :-) */
+ oldfs = get_fs();
+ set_fs(KERNEL_DS); /* More privs :-) */
err = sock_sendmsg(udpsock, &udpmsg, size);
set_fs(oldfs);
@@ -530,7 +528,7 @@ static int econet_getname(struct socket *sock, struct sockaddr *uaddr,
static void econet_destroy_timer(unsigned long data)
{
- struct sock *sk=(struct sock *)data;
+ struct sock *sk = (struct sock *)data;
if (!sk_has_allocations(sk)) {
sk_free(sk);
@@ -539,7 +537,7 @@ static void econet_destroy_timer(unsigned long data)
sk->sk_timer.expires = jiffies + 10 * HZ;
add_timer(&sk->sk_timer);
- printk(KERN_DEBUG "econet socket destroy delayed\n");
+ pr_debug("econet: socket destroy delayed\n");
}
/*
@@ -651,7 +649,8 @@ static int ec_dev_ioctl(struct socket *sock, unsigned int cmd, void __user *arg)
if (copy_from_user(&ifr, arg, sizeof(struct ifreq)))
return -EFAULT;
- if ((dev = dev_get_by_name(&init_net, ifr.ifr_name)) == NULL)
+ dev = dev_get_by_name(&init_net, ifr.ifr_name);
+ if (dev == NULL)
return -ENODEV;
sec = (struct sockaddr_ec *)&ifr.ifr_addr;
@@ -715,7 +714,8 @@ static int ec_dev_ioctl(struct socket *sock, unsigned int cmd, void __user *arg)
* Handle generic ioctls
*/
-static int econet_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
+static int econet_ioctl(struct socket *sock, unsigned int cmd,
+ unsigned long arg)
{
struct sock *sk = sock->sk;
void __user *argp = (void __user *)arg;
@@ -833,7 +833,7 @@ static void aun_send_response(__u32 addr, unsigned long seq, int code, int cb)
udpmsg.msg_namelen = sizeof(sin);
udpmsg.msg_control = NULL;
udpmsg.msg_controllen = 0;
- udpmsg.msg_flags=0;
+ udpmsg.msg_flags = 0;
kernel_sendmsg(udpsock, &udpmsg, &iov, 1, sizeof(ah));
}
@@ -856,26 +856,25 @@ static void aun_incoming(struct sk_buff *skb, struct aunhdr *ah, size_t len)
if (dst)
edev = dst->dev->ec_ptr;
- if (! edev)
+ if (!edev)
goto bad;
- if ((sk = ec_listening_socket(ah->port, stn, edev->net)) == NULL)
+ sk = ec_listening_socket(ah->port, stn, edev->net);
+ if (sk == NULL)
goto bad; /* Nobody wants it */
newskb = alloc_skb((len - sizeof(struct aunhdr) + 15) & ~15,
GFP_ATOMIC);
- if (newskb == NULL)
- {
- printk(KERN_DEBUG "AUN: memory squeeze, dropping packet.\n");
+ if (newskb == NULL) {
+ pr_debug("AUN: memory squeeze, dropping packet\n");
/* Send nack and hope sender tries again */
goto bad;
}
- memcpy(skb_put(newskb, len - sizeof(struct aunhdr)), (void *)(ah+1),
+ memcpy(skb_put(newskb, len - sizeof(struct aunhdr)), (void *)(ah + 1),
len - sizeof(struct aunhdr));
- if (ec_queue_packet(sk, newskb, stn, edev->net, ah->cb, ah->port))
- {
+ if (ec_queue_packet(sk, newskb, stn, edev->net, ah->cb, ah->port)) {
/* Socket is bankrupt. */
kfree_skb(newskb);
goto bad;
@@ -911,7 +910,7 @@ static void aun_tx_ack(unsigned long seq, int result)
goto foundit;
}
spin_unlock_irqrestore(&aun_queue_lock, flags);
- printk(KERN_DEBUG "AUN: unknown sequence %ld\n", seq);
+ pr_debug("AUN: unknown sequence %ld\n", seq);
return;
foundit:
@@ -936,18 +935,17 @@ static void aun_data_available(struct sock *sk, int slen)
while ((skb = skb_recv_datagram(sk, 0, 1, &err)) == NULL) {
if (err == -EAGAIN) {
- printk(KERN_ERR "AUN: no data available?!");
+ pr_err("AUN: no data available?!\n");
return;
}
- printk(KERN_DEBUG "AUN: recvfrom() error %d\n", -err);
+ pr_debug("AUN: recvfrom() error %d\n", -err);
}
data = skb_transport_header(skb) + sizeof(struct udphdr);
ah = (struct aunhdr *)data;
len = skb->len - sizeof(struct udphdr);
- switch (ah->code)
- {
+ switch (ah->code) {
case 2:
aun_incoming(skb, ah, len);
break;
@@ -958,7 +956,7 @@ static void aun_data_available(struct sock *sk, int slen)
aun_tx_ack(ah->handle, ECTYPE_TRANSMIT_NOT_LISTENING);
break;
default:
- printk(KERN_DEBUG "unknown AUN packet (type %d)\n", data[0]);
+ pr_debug("AUN: unknown packet type: %d\n", data[0]);
}
skb_free_datagram(sk, skb);
@@ -988,7 +986,7 @@ static void ab_cleanup(unsigned long h)
}
spin_unlock_irqrestore(&aun_queue_lock, flags);
- mod_timer(&ab_cleanup_timer, jiffies + (HZ*2));
+ mod_timer(&ab_cleanup_timer, jiffies + (HZ * 2));
}
static int __init aun_udp_initialise(void)
@@ -998,7 +996,7 @@ static int __init aun_udp_initialise(void)
skb_queue_head_init(&aun_queue);
setup_timer(&ab_cleanup_timer, ab_cleanup, 0);
- ab_cleanup_timer.expires = jiffies + (HZ*2);
+ ab_cleanup_timer.expires = jiffies + (HZ * 2);
add_timer(&ab_cleanup_timer);
memset(&sin, 0, sizeof(sin));
@@ -1006,9 +1004,9 @@ static int __init aun_udp_initialise(void)
/* We can count ourselves lucky Acorn machines are too dim to
speak IPv6. :-) */
- if ((error = sock_create_kern(PF_INET, SOCK_DGRAM, 0, &udpsock)) < 0)
- {
- printk("AUN: socket error %d\n", -error);
+ error = sock_create_kern(PF_INET, SOCK_DGRAM, 0, &udpsock);
+ if (error < 0) {
+ pr_err("AUN: socket error %d\n", -error);
return error;
}
@@ -1017,10 +1015,9 @@ static int __init aun_udp_initialise(void)
from interrupts */
error = udpsock->ops->bind(udpsock, (struct sockaddr *)&sin,
- sizeof(sin));
- if (error < 0)
- {
- printk("AUN: bind error %d\n", -error);
+ sizeof(sin));
+ if (error < 0) {
+ pr_err("AUN: bind error %d\n", -error);
goto release;
}
@@ -1041,7 +1038,8 @@ release:
* Receive an Econet frame from a device.
*/
-static int econet_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+static int econet_rcv(struct sk_buff *skb, struct net_device *dev,
+ struct packet_type *pt, struct net_device *orig_dev)
{
struct ec_framehdr *hdr;
struct sock *sk = NULL;
@@ -1056,13 +1054,14 @@ static int econet_rcv(struct sk_buff *skb, struct net_device *dev, struct packet
if (!edev)
goto drop;
- if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL)
+ skb = skb_share_check(skb, GFP_ATOMIC);
+ if (skb == NULL)
return NET_RX_DROP;
if (!pskb_may_pull(skb, sizeof(struct ec_framehdr)))
goto drop;
- hdr = (struct ec_framehdr *) skb->data;
+ hdr = (struct ec_framehdr *)skb->data;
/* First check for encapsulated IP */
if (hdr->port == EC_PORT_IP) {
@@ -1090,8 +1089,8 @@ drop:
}
static struct packet_type econet_packet_type __read_mostly = {
- .type = cpu_to_be16(ETH_P_ECONET),
- .func = econet_rcv,
+ .type = cpu_to_be16(ETH_P_ECONET),
+ .func = econet_rcv,
};
static void econet_hw_initialise(void)
@@ -1101,9 +1100,10 @@ static void econet_hw_initialise(void)
#endif
-static int econet_notifier(struct notifier_block *this, unsigned long msg, void *data)
+static int econet_notifier(struct notifier_block *this, unsigned long msg,
+ void *data)
{
- struct net_device *dev = (struct net_device *)data;
+ struct net_device *dev = data;
struct ec_device *edev;
if (!net_eq(dev_net(dev), &init_net))
@@ -1113,8 +1113,7 @@ static int econet_notifier(struct notifier_block *this, unsigned long msg, void
case NETDEV_UNREGISTER:
/* A device has gone down - kill any data we hold for it. */
edev = dev->ec_ptr;
- if (edev)
- {
+ if (edev) {
if (net2dev_map[0] == dev)
net2dev_map[0] = NULL;
net2dev_map[edev->net] = NULL;
@@ -1128,7 +1127,7 @@ static int econet_notifier(struct notifier_block *this, unsigned long msg, void
}
static struct notifier_block econet_netdev_notifier = {
- .notifier_call =econet_notifier,
+ .notifier_call = econet_notifier,
};
static void __exit econet_proto_exit(void)
--
1.7.6.rc1
^ permalink raw reply related
* Re: [net-next PATCH 1/2] dcbnl: Aggregated CEE GET operation
From: John Fastabend @ 2011-07-03 18:09 UTC (permalink / raw)
To: Shmulik Ravid; +Cc: davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <1309624423.15528.43.camel@lb-tlvb-shmulik.il.broadcom.com>
On 7/2/2011 9:33 AM, Shmulik Ravid wrote:
> The following couple of patches add dcbnl an unsolicited notification of
> the the DCB configuration for the CEE flavor of the DCBX protocol. This
> is useful when the user-mode DCB client is not responsible for
> conducting and resolving the DCBX negotiation (either because the DCBX
> stack is embedded in the HW or the negotiation is handled by another
> agent in he host), but still needs to get the negotiated parameters.
> This functionality already exists for the IEEE flavor of the DCBX
> protocol and these patches add it to the older CEE flavor.
>
> The first patch extends the CEE attribute GET operation to include not
> only the peer information, but also all the pertinent local
> configuration (negotiated parameters). The second patch adds and export
> a CEE specific notification routine.
>
> Signed-off-by: Shmulik Ravid <shmulikr@broadcom.com>
> ---
> include/linux/dcbnl.h | 20 ++++++-
> net/dcb/dcbnl.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 168 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/dcbnl.h b/include/linux/dcbnl.h
> index 66a6723..c875244 100644
> --- a/include/linux/dcbnl.h
> +++ b/include/linux/dcbnl.h
> @@ -333,15 +333,26 @@ enum ieee_attrs_app {
> #define DCB_ATTR_IEEE_APP_MAX (__DCB_ATTR_IEEE_APP_MAX - 1)
>
> /**
> - * enum cee_attrs - CEE DCBX get attributes
> + * enum cee_attrs - CEE DCBX get attributes. An aggregated collection of the
> + * cee std negotiated parameters
> *
> * @DCB_ATTR_CEE_UNSPEC: unspecified
> + * @DCB_ATTR_CEE_FEAT: DCBX features flags (DCB_CMD_GFEATCFG)
> + * @DCB_ATTR_CEE_RX_PG: RX PG configuration (DCB_CMD_PGRX_GCFG)
> + * @DCB_ATTR_CEE_TX_PG: TX PG configuration (DCB_CMD_PGTX_GCFG)
> + * @DCB_ATTR_CEE_PFC: PFC configuration (DCB_CMD_PFC_GCFG)
> + * @DCB_ATTR_CEE_APP_TABLE: APP configuration (multi DCB_CMD_GAPP)
> * @DCB_ATTR_CEE_PEER_PG: peer PG configuration - get only
> * @DCB_ATTR_CEE_PEER_PFC: peer PFC configuration - get only
> * @DCB_ATTR_CEE_PEER_APP: peer APP tlv - get only
> */
> enum cee_attrs {
> DCB_ATTR_CEE_UNSPEC,
> + DCB_ATTR_CEE_TX_PG,
> + DCB_ATTR_CEE_RX_PG,
> + DCB_ATTR_CEE_PFC,
> + DCB_ATTR_CEE_APP_TABLE,
> + DCB_ATTR_CEE_FEAT,
> DCB_ATTR_CEE_PEER_PG,
> DCB_ATTR_CEE_PEER_PFC,
> DCB_ATTR_CEE_PEER_APP_TABLE,
PEER_PG, PEER_PFC, and PEER_APP_TABLE are already in kernel and user
space. So we shouldn't change their enum'd value. Put the new values
below them.
> @@ -357,6 +368,13 @@ enum peer_app_attr {
> };
> #define DCB_ATTR_CEE_PEER_APP_MAX (__DCB_ATTR_CEE_PEER_APP_MAX - 1)
>
> +enum cee_attrs_app {
> + DCB_ATTR_CEE_APP_UNSPEC,
> + DCB_ATTR_CEE_APP,
> + __DCB_ATTR_CEE_APP_MAX
> +};
> +#define DCB_ATTR_CEE_APP_MAX (__DCB_ATTR_CEE_APP_MAX - 1)
> +
> /**
> * enum dcbnl_pfc_attrs - DCB Priority Flow Control user priority nested attrs
> *
> diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
> index fc56e85..5b75ed7 100644
> --- a/net/dcb/dcbnl.c
> +++ b/net/dcb/dcbnl.c
> @@ -1642,6 +1642,58 @@ err:
> return ret;
> }
>
> +static int dcbnl_cee_pg_fill(struct sk_buff *skb, struct net_device *dev,
> + int dir)
Why the 'dir'? Could you just pack both tx and rx together? It looks like all
cases pack TX then pack RX. Did I miss something?
> +{
> + u8 pgid, up_map, prio, tc_pct;
> + const struct dcbnl_rtnl_ops *ops = dev->dcbnl_ops;
> + int i = dir ? DCB_ATTR_CEE_TX_PG : DCB_ATTR_CEE_RX_PG;
> + struct nlattr *pg = nla_nest_start(skb, i);
I believe its preferred to put an empty line here. At least I prefer it.
> + if (!pg)
> + goto nla_put_failure;
> +
> + for (i = DCB_PG_ATTR_TC_0; i <= DCB_PG_ATTR_TC_7; i++) {
> + struct nlattr *tc_nest = nla_nest_start(skb, i);
same here.
> + if (!tc_nest)
> + goto nla_put_failure;
> +
> + pgid = DCB_ATTR_VALUE_UNDEFINED;
> + prio = DCB_ATTR_VALUE_UNDEFINED;
> + tc_pct = DCB_ATTR_VALUE_UNDEFINED;
> + up_map = DCB_ATTR_VALUE_UNDEFINED;
> +
> + if (!dir)
> + ops->getpgtccfgrx(dev, i - DCB_PG_ATTR_TC_0,
> + &prio, &pgid, &tc_pct, &up_map);
> + else
> + ops->getpgtccfgtx(dev, i - DCB_PG_ATTR_TC_0,
> + &prio, &pgid, &tc_pct, &up_map);
> +
Thanks for doing this. We should be able to get firmware CEE DCBX agents
working correctly with this.
//John
^ 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