[PATCH] net: bgmac: Fix errant feature flag check

[Jon Mason]

During the conversion to the feature flags, a check against
ci->id != BCMA_CHIP_ID_BCM47162
became
bgmac->feature_flags & BGMAC_FEAT_CLKCTLS
instead of
!(bgmac->feature_flags & BGMAC_FEAT_CLKCTLS)

Reported-by: Rafał Miłecki <zaj...@gmail.com>
Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c 
b/drivers/net/ethernet/broadcom/bgmac.c
index c4751ec..87d00ea 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1024,7 +1024,7 @@ static void bgmac_enable(struct bgmac *bgmac)
 
mode = (bgmac_read(bgmac, BGMAC_DEV_STATUS) & BGMAC_DS_MM_MASK) >>
BGMAC_DS_MM_SHIFT;
-   if (bgmac->feature_flags & BGMAC_FEAT_CLKCTLST || mode != 0)
+   if (!(bgmac->feature_flags & BGMAC_FEAT_CLKCTLST) || mode != 0)
bgmac_set(bgmac, BCMA_CLKCTLST, BCMA_CLKCTLST_FORCEHT);
if (bgmac->feature_flags & BGMAC_FEAT_CLKCTLST && mode == 2)
bgmac_cco_ctl_maskset(bgmac, 1, ~0,
-- 
2.7.4

Re: [PATCH v2 4/6] net: ethernet: bgmac: convert to feature flags

[Jon Mason]

On Tue, Sep 20, 2016 at 1:19 AM, Rafał Miłecki <zaj...@gmail.com> wrote:
> On 17 August 2016 at 13:34, Rafał Miłecki <zaj...@gmail.com> wrote:
>> On 8 July 2016 at 01:08, Jon Mason <jon.ma...@broadcom.com> wrote:
>>> mode = (bgmac_read(bgmac, BGMAC_DEV_STATUS) & BGMAC_DS_MM_MASK) >>
>>> BGMAC_DS_MM_SHIFT;
>>> -   if (ci->id != BCMA_CHIP_ID_BCM47162 || mode != 0)
>>> +   if (bgmac->feature_flags & BGMAC_FEAT_CLKCTLST || mode != 0)
>>> bgmac_set(bgmac, BCMA_CLKCTLST, BCMA_CLKCTLST_FORCEHT);
>>> -   if (ci->id == BCMA_CHIP_ID_BCM47162 && mode == 2)
>>> +   if (bgmac->feature_flags & BGMAC_FEAT_CLKCTLST && mode == 2)
>>> bcma_chipco_chipctl_maskset(>core->bus->drv_cc, 1, 
>>> ~0,
>>> BGMAC_CHIPCTL_1_RXC_DLL_BYPASS);
>>
>> Jon, it looks to me you translated two following conditions:
>> ci->id != BCMA_CHIP_ID_BCM47162
>> and
>> ci->id == BCMA_CHIP_ID_BCM47162
>> into the same flag check:
>> bgmac->feature_flags & BGMAC_FEAT_CLKCTLST
>>
>> I don't think it's intentional, is it? Do you have a moment to fix this?
>
> Ping

Sorry, just seeing this now.  I'll double check the original code and
verify it (or fix it).

Thanks,
Jon

>
> --
> Rafał

Re: [PATCH RFC 4/4] xfs: Transmit flow steering

[Chris Mason]

On 08/30/2016 08:00 PM, Tom Herbert wrote:

XFS maintains a per device flow table that is indexed by the skbuff
hash. The XFS table is only consulted when there is no queue saved in
a transmit socket for an skbuff.

Each entry in the flow table contains a queue index and a queue
pointer. The queue pointer is set when a queue is chosen using a
flow table entry. This pointer is set to the head pointer in the
transmit queue (which is maintained by BQL).

The new function get_xfs_index that looks up flows in the XPS table.
The entry returned gives the last queue a matching flow used. The
returned queue is compared against the normal XPS queue. If they
are different, then we only switch if the tail pointer in the TX
queue has advanced past the pointer saved in the entry. In this
way OOO should be avoided when XPS wants to use a different queue.



I'd love for Dave Chinner to get some networking bug reports, but maybe 
we shouldn't call it XFS?


At least CONFIG_XFS should be something else.  It doesn't conflict now 
because we have CONFIG_XFS_FS, but even CONFIG_XFS_NET sounds like it's 
related to the filesystem instead of transmit flows.


[ Sorry, four patches in and all I do is complain about the name ]

-chris


Signed-off-by: Tom Herbert 
---
 net/Kconfig|  6 
 net/core/dev.c | 93 --
 2 files changed, 84 insertions(+), 15 deletions(-)

diff --git a/net/Kconfig b/net/Kconfig
index 7b6cd34..5e3eddf 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -255,6 +255,12 @@ config XPS
depends on SMP
default y

+config XFS
+   bool
+   depends on XPS
+   depends on BQL
+   default y
+
 config HWBM
bool


...


-static u16 __netdev_pick_tx(struct net_device *dev, struct sk_buff *skb)
+/* Must be called with RCU read_lock */
+static int get_xfs_index(struct net_device *dev, struct sk_buff *skb)
 {
-   struct sock *sk = skb->sk;
-   int queue_index = sk_tx_queue_get(sk);
+#ifdef CONFIG_XFS
+   struct xps_dev_flow_table *flow_table;
+   struct xps_dev_flow ent;
+   int queue_index;
+   struct netdev_queue *txq;
+   u32 hash;

Re: [PATCH 1/1] net: s2io: simplify logical constraint

[Jon Mason]

On Sun, Jul 31, 2016 at 5:53 AM, Heinrich Schuchardt <xypron.g...@gmx.de> wrote:
> (!A || (A && B)) is equivalent to (!A || B)

Seems logical to me.

Acked-by: Jon Mason <jdma...@kudzu.us>

> Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de>
> ---
>  drivers/net/ethernet/neterion/s2io.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/neterion/s2io.c 
> b/drivers/net/ethernet/neterion/s2io.c
> index 2874dff..eaa37c0 100644
> --- a/drivers/net/ethernet/neterion/s2io.c
> +++ b/drivers/net/ethernet/neterion/s2io.c
> @@ -7412,7 +7412,7 @@ static int rx_osm_handler(struct ring_info *ring_data, 
> struct RxD_t * rxdp)
>
> if ((rxdp->Control_1 & TCP_OR_UDP_FRAME) &&
> ((!ring_data->lro) ||
> -(ring_data->lro && (!(rxdp->Control_1 & RXD_FRAME_IP_FRAG &&
> +(!(rxdp->Control_1 & RXD_FRAME_IP_FRAG))) &&
> (dev->features & NETIF_F_RXCSUM)) {
> l3_csum = RXD_GET_L3_CKSUM(rxdp->Control_1);
> l4_csum = RXD_GET_L4_CKSUM(rxdp->Control_1);
> --
> 2.8.1
>

Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc

[Mason]

On 12/07/2016 16:38, Mason wrote:

> With Eric's patch applied, I get this warning at boot:
> 
> [4.668309] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow 
> control rx/tx
> [4.688609] Sending DHCP requests ., OK
> [4.711935] IP-Config: Got DHCP answer from 172.27.200.1, my address is 
> 172.27.64.49
> [4.719956] IP-Config: Complete:
> [4.723221]  device=eth0, hwaddr=00:16:e8:02:08:42, 
> ipaddr=172.27.64.49, mask=255.255.192.0, gw=172.27.64.1
> [4.733376]  host=toto5, domain=france.foo.com sac.foo.com 
> asic.foo.com soft.sde, nis-domain=france.foo.com
> [4.745279]  bootserver=172.27.64.1, rootserver=172.27.64.1, 
> rootpath=/export/roots/titi/6_2_0_8756,v3 nameserver0=172.27.0.17
> [4.759725] [ cut here ]
> [4.764426] WARNING: CPU: 0 PID: 877 at net/core/sock.c:1468 
> sk_destruct+0x74/0x78
> [4.772056] Modules linked in:
> [4.775133] CPU: 0 PID: 877 Comm: kworker/0:1H Not tainted 
> 4.7.0-rc6-00010-gd07031bdc433-dirty #6
> [4.784050] Hardware name: Sigma Tango DT
> [4.788084] Workqueue: rpciod rpc_async_schedule
> [4.792725] Backtrace: 
> [4.795196] [] (dump_backtrace) from [] 
> (show_stack+0x18/0x1c)
> [4.802802]  r7:6013 r6:c080ea84 r5: r4:c080ea84
> [4.808513] [] (show_stack) from [] 
> (dump_stack+0x80/0x94)
> [4.815781] [] (dump_stack) from [] (__warn+0xec/0x104)
> [4.822776]  r7:0009 r6:c05e4d04 r5: r4:
> [4.828482] [] (__warn) from [] 
> (warn_slowpath_null+0x28/0x30)
> [4.836089]  r9: r8:df5edb58 r7:df711364 r6:df006c80 r5:df5edb58 
> r4:df5eda40
> [4.843898] [] (warn_slowpath_null) from [] 
> (sk_destruct+0x74/0x78)
> [4.851945] [] (sk_destruct) from [] 
> (__sk_free+0x50/0xbc)
> [4.859203]  r5:df5edb58 r4:df5eda40
> [4.862802] [] (__sk_free) from [] (sk_free+0x3c/0x40)
> [4.869710]  r5:df5edb58 r4:df5eda40
> [4.873310] [] (sk_free) from [] 
> (sk_common_release+0xe8/0xf4)
> [4.880924] [] (sk_common_release) from [] 
> (udp_lib_close+0x10/0x14)
> [4.889054]  r5:df006c80 r4:df5eda40
> [4.892657] [] (udp_lib_close) from [] 
> (inet_release+0x4c/0x78)
> [4.900360] [] (inet_release) from [] 
> (sock_release+0x28/0xa8)
> [4.907967]  r5: r4:df006c80
> [4.911575] [] (sock_release) from [] 
> (xs_reset_transport+0xac/0xbc)
> [4.919706]  r5:df5eda40 r4:df711000
> [4.923306] [] (xs_reset_transport) from [] 
> (xs_destroy+0x24/0x54)
> [4.931262]  r9: r8:c049c614 r7:df7c3218 r6:df711000 r5: 
> r4:df711000
> [4.939070] [] (xs_destroy) from [] 
> (xprt_destroy+0x88/0x8c)
> [4.946502]  r5:df711218 r4:df711000
> [4.950102] [] (xprt_destroy) from [] 
> (xprt_put+0x40/0x44)
> [4.957358]  r5:df7c3200 r4:df613d00
> [4.960959] [] (xprt_put) from [] 
> (rpc_task_release_client+0x7c/0x80)
> [4.969181] [] (rpc_task_release_client) from [] 
> (rpc_release_resources_task+0x34/0x38)
> [4.978971]  r7:c049c298 r6:0001 r5: r4:df613d00
> [4.984674] [] (rpc_release_resources_task) from [] 
> (__rpc_execute+0xb0/0x2a8)
> [4.993678]  r5: r4:df613d00
> [4.997277] [] (__rpc_execute) from [] 
> (rpc_async_schedule+0x14/0x18)
> [5.005496]  r10:df683500 r9: r8:dfbe4700 r7: r6:dfbdcc80 
> r5:df683500
> [5.013383]  r4:df613d24
> [5.015935] [] (rpc_async_schedule) from [] 
> (process_one_work+0x128/0x3fc)
> [5.024595] [] (process_one_work) from [] 
> (worker_thread+0x58/0x574)
> [5.032726]  r10:df683500 r9:df6d4000 r8:dfbdcc98 r7:c0802100 r6:0008 
> r5:df683518
> [5.040614]  r4:dfbdcc80
> [5.043162] [] (worker_thread) from [] 
> (kthread+0xe4/0xfc)
> [5.050419]  r10: r9: r8: r7:c0132914 r6:df683500 
> r5:df6cc600
> [5.058309]  r4:
> [5.060858] [] (kthread) from [] 
> (ret_from_fork+0x14/0x3c)
> [5.068116]  r7: r6: r5:c013838c r4:df6cc600
> [5.073846] ---[ end trace 05b24e2dedd2f2a0 ]---
> [5.082009] VFS: Mounted root (nfs filesystem) readonly on device 0:11.
> [5.090328] Freeing unused kernel memory: 1024K (c070 - c080)
> [5.452552] random: udevd urandom read with 68 bits of entropy available

AFAICT, I get the same call stack for every boot.

Is this an unexpected call sequence?

Are there other tests I can run?

Regards.

Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc

[Mason]

On 12/07/2016 16:25, Eric Dumazet wrote:

> Could you try this debug patch ?

Note: I've been unable to trigger the warning again. Dunno what has changed...

With your patch applied, I get a warning at boot:

[4.668309] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow 
control rx/tx
[4.688609] Sending DHCP requests ., OK
[4.711935] IP-Config: Got DHCP answer from 172.27.200.1, my address is 
172.27.64.49
[4.719956] IP-Config: Complete:
[4.723221]  device=eth0, hwaddr=00:16:e8:02:08:42, ipaddr=172.27.64.49, 
mask=255.255.192.0, gw=172.27.64.1
[4.733376]  host=toto5, domain=france.foo.com sac.foo.com asic.foo.com 
soft.sde, nis-domain=france.foo.com
[4.745279]  bootserver=172.27.64.1, rootserver=172.27.64.1, 
rootpath=/export/roots/titi/6_2_0_8756,v3 nameserver0=172.27.0.17
[4.759725] [ cut here ]
[4.764426] WARNING: CPU: 0 PID: 877 at net/core/sock.c:1468 
sk_destruct+0x74/0x78
[4.772056] Modules linked in:
[4.775133] CPU: 0 PID: 877 Comm: kworker/0:1H Not tainted 
4.7.0-rc6-00010-gd07031bdc433-dirty #6
[4.784050] Hardware name: Sigma Tango DT
[4.788084] Workqueue: rpciod rpc_async_schedule
[4.792725] Backtrace: 
[4.795196] [] (dump_backtrace) from [] 
(show_stack+0x18/0x1c)
[4.802802]  r7:6013 r6:c080ea84 r5: r4:c080ea84
[4.808513] [] (show_stack) from [] 
(dump_stack+0x80/0x94)
[4.815781] [] (dump_stack) from [] (__warn+0xec/0x104)
[4.822776]  r7:0009 r6:c05e4d04 r5: r4:
[4.828482] [] (__warn) from [] 
(warn_slowpath_null+0x28/0x30)
[4.836089]  r9: r8:df5edb58 r7:df711364 r6:df006c80 r5:df5edb58 
r4:df5eda40
[4.843898] [] (warn_slowpath_null) from [] 
(sk_destruct+0x74/0x78)
[4.851945] [] (sk_destruct) from [] 
(__sk_free+0x50/0xbc)
[4.859203]  r5:df5edb58 r4:df5eda40
[4.862802] [] (__sk_free) from [] (sk_free+0x3c/0x40)
[4.869710]  r5:df5edb58 r4:df5eda40
[4.873310] [] (sk_free) from [] 
(sk_common_release+0xe8/0xf4)
[4.880924] [] (sk_common_release) from [] 
(udp_lib_close+0x10/0x14)
[4.889054]  r5:df006c80 r4:df5eda40
[4.892657] [] (udp_lib_close) from [] 
(inet_release+0x4c/0x78)
[4.900360] [] (inet_release) from [] 
(sock_release+0x28/0xa8)
[4.907967]  r5: r4:df006c80
[4.911575] [] (sock_release) from [] 
(xs_reset_transport+0xac/0xbc)
[4.919706]  r5:df5eda40 r4:df711000
[4.923306] [] (xs_reset_transport) from [] 
(xs_destroy+0x24/0x54)
[4.931262]  r9: r8:c049c614 r7:df7c3218 r6:df711000 r5: 
r4:df711000
[4.939070] [] (xs_destroy) from [] 
(xprt_destroy+0x88/0x8c)
[4.946502]  r5:df711218 r4:df711000
[4.950102] [] (xprt_destroy) from [] 
(xprt_put+0x40/0x44)
[4.957358]  r5:df7c3200 r4:df613d00
[4.960959] [] (xprt_put) from [] 
(rpc_task_release_client+0x7c/0x80)
[4.969181] [] (rpc_task_release_client) from [] 
(rpc_release_resources_task+0x34/0x38)
[4.978971]  r7:c049c298 r6:0001 r5: r4:df613d00
[4.984674] [] (rpc_release_resources_task) from [] 
(__rpc_execute+0xb0/0x2a8)
[4.993678]  r5: r4:df613d00
[4.997277] [] (__rpc_execute) from [] 
(rpc_async_schedule+0x14/0x18)
[5.005496]  r10:df683500 r9: r8:dfbe4700 r7: r6:dfbdcc80 
r5:df683500
[5.013383]  r4:df613d24
[5.015935] [] (rpc_async_schedule) from [] 
(process_one_work+0x128/0x3fc)
[5.024595] [] (process_one_work) from [] 
(worker_thread+0x58/0x574)
[5.032726]  r10:df683500 r9:df6d4000 r8:dfbdcc98 r7:c0802100 r6:0008 
r5:df683518
[5.040614]  r4:dfbdcc80
[5.043162] [] (worker_thread) from [] 
(kthread+0xe4/0xfc)
[5.050419]  r10: r9: r8: r7:c0132914 r6:df683500 
r5:df6cc600
[5.058309]  r4:
[5.060858] [] (kthread) from [] 
(ret_from_fork+0x14/0x3c)
[5.068116]  r7: r6: r5:c013838c r4:df6cc600
[5.073846] ---[ end trace 05b24e2dedd2f2a0 ]---
[5.082009] VFS: Mounted root (nfs filesystem) readonly on device 0:11.
[5.090328] Freeing unused kernel memory: 1024K (c070 - c080)
[5.452552] random: udevd urandom read with 68 bits of entropy available

Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc

[Mason]

On 12/07/2016 11:53, Mason wrote:

> However, the 310 seconds time span still seems to be relevant.
> 
> Steps to reproduce: I booted the system, logged in as root,
> mounted an NFS file system, then left the system idling at
> the prompt.
> 
> (I don't remember seeing this warning in v4.1 and v4.4)
> 
> What's going wrong here? Is it related to NFS?
> 
> Here is the defconfig I'm using
> http://pastebin.ubuntu.com/19160299/
> 
> 
> [  317.940133] [ cut here ]
> [  317.944815] WARNING: CPU: 1 PID: 0 at net/ipv4/af_inet.c:155 
> inet_sock_destruct+0x1c4/0x1dc
> [  317.953223] Modules linked in:
> [  317.956305] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 
> 4.7.0-rc6-00010-gd07031bdc433-dirty #2
> [  317.964784] Hardware name: Sigma Tango DT
> [  317.968809] Backtrace: 
> [  317.971279] [] (dump_backtrace) from [] 
> (show_stack+0x18/0x1c)
> [  317.978884]  r7:6113 r6:c080ea84 r5: r4:c080ea84
> [  317.984590] [] (show_stack) from [] 
> (dump_stack+0x80/0x94)
> [  317.991856] [] (dump_stack) from [] (__warn+0xec/0x104)
> [  317.998849]  r7:0009 r6:c05e3fc8 r5: r4:
> [  318.004549] [] (__warn) from [] 
> (warn_slowpath_null+0x28/0x30)
> [  318.012154]  r9:dfbea4e0 r8:000a r7:df45fe30 r6:dec19594 r5:df68f144 
> r4:df68f040
> [  318.019954] [] (warn_slowpath_null) from [] 
> (inet_sock_destruct+0x1c4/0x1dc)
> [  318.028788] [] (inet_sock_destruct) from [] 
> (__sk_destruct+0x28/0xe0)
> [  318.037005]  r7:df45fe30 r6:dec19594 r5:df68f040 r4:df68f1ec
> [  318.042710] [] (__sk_destruct) from [] 
> (rcu_process_callbacks+0x488/0x59c)
> [  318.051363]  r5: r4:
> [  318.054962] [] (rcu_process_callbacks) from [] 
> (__do_softirq+0x138/0x264)
> [  318.063527]  r10:c08020a0 r9:4001 r8:0101 r7:df45e000 r6:c08020a4 
> r5:0009
> [  318.071408]  r4:
> [  318.073953] [] (__do_softirq) from [] 
> (irq_exit+0xc8/0x104)
> [  318.081296]  r10:df45ff58 r9:df402400 r8:0001 r7: r6:0013 
> r5:
> [  318.089176]  r4:c0735428
> [  318.091723] [] (irq_exit) from [] 
> (__handle_domain_irq+0x88/0xf4)
> [  318.099595] [] (__handle_domain_irq) from [] 
> (gic_handle_irq+0x50/0x94)
> [  318.107986]  r10: r9:e0803100 r8:e0802100 r7:df45ff58 r6:e080210c 
> r5:c080277c
> [  318.115865]  r4:c080eca0 r3:df45ff58
> [  318.119461] [] (gic_handle_irq) from [] 
> (__irq_svc+0x54/0x90)
> [  318.126980] Exception stack(0xdf45ff58 to 0xdf45ffa0)
> [  318.132053] ff40:   
> 0001 
> [  318.140273] ff60: ab80 c0117c80 df45e000 c08024f8 c0802494 c081e2d6 
> c05b9550 413fc090
> [  318.148492] ff80:  df45ffb4 df45ffb8 df45ffa8 c01086b0 c01086b4 
> 6013 
> [  318.156709]  r9:413fc090 r8:c05b9550 r7:df45ff8c r6: r5:6013 
> r4:c01086b4
> [  318.164512] [] (arch_cpu_idle) from [] 
> (default_idle_call+0x28/0x34)
> [  318.172646] [] (default_idle_call) from [] 
> (cpu_startup_entry+0x128/0x17c)
> [  318.181305] [] (cpu_startup_entry) from [] 
> (secondary_start_kernel+0x158/0x164)
> [  318.190395]  r7:c081e7c8 r4:c080b4f0
> [  318.193993] [] (secondary_start_kernel) from [<8010158c>] 
> (0x8010158c)
> [  318.201423]  r5:0051 r4:9f44006a
> [  318.205024] ---[ end trace 6e04001434b19cb9 ]---
> 
> 
> Just to be sure, I performed the same steps a second time:
> 
> [  316.238527] [ cut here ]
> [  316.243210] WARNING: CPU: 1 PID: 0 at net/ipv4/af_inet.c:155 
> inet_sock_destruct+0x1c4/0x1dc
> [  316.251619] Modules linked in:
> [  316.254702] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 
> 4.7.0-rc6-00010-gd07031bdc433-dirty #2
> [  316.263182] Hardware name: Sigma Tango DT
> [  316.267206] Backtrace: 
> [  316.269675] [] (dump_backtrace) from [] 
> (show_stack+0x18/0x1c)
> [  316.277280]  r7:6113 r6:c080ea84 r5: r4:c080ea84
> [  316.282986] [] (show_stack) from [] 
> (dump_stack+0x80/0x94)
> [  316.290254] [] (dump_stack) from [] (__warn+0xec/0x104)
> [  316.297247]  r7:0009 r6:c05e3fc8 r5: r4:
> [  316.302947] [] (__warn) from [] 
> (warn_slowpath_null+0x28/0x30)
> [  316.310552]  r9:dfbea4e0 r8:000a r7:df45fe30 r6:dec15694 r5:df6063c4 
> r4:df6062c0
> [  316.318354] [] (warn_slowpath_null) from [] 
> (inet_sock_destruct+0x1c4/0x1dc)
> [  316.327190] [] (inet_sock_destruct) from [] 
> (__sk_destruct+0x28/0xe0)
> [  316.335406]  r7:df45fe30 r6:dec15694 r5:df6062c0 r4:df60646c
> [  316.341112] [] (__sk_destruct) from [] 
> (rcu_process_callbacks+0x488/0x59c)
> [  316.349765]  r5: r4:
> [  316.353363] [] (rcu_process_callbacks) fr

Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc

[Mason]

On 05/07/2016 17:28, Florian Fainelli wrote:

> Le 05/07/2016 07:50, Mason wrote:
> 
>> On 05/07/2016 15:33, Mason wrote:
>> 
>>> I was testing suspend/resume sequences where the suspend operation
>>> fails and returns without having suspended the platform.

Please forget I ever mentioned suspend, that was a red herring.
The warning is displayed even if I never suspend.
(Dropping linux-pm from this discussion.)

>> I rebooted, tried the same operation, and hit the same warning
>> still 310 seconds later:

However, the 310 seconds time span still seems to be relevant.

Steps to reproduce: I booted the system, logged in as root,
mounted an NFS file system, then left the system idling at
the prompt.

(I don't remember seeing this warning in v4.1 and v4.4)

What's going wrong here? Is it related to NFS?

Here is the defconfig I'm using
http://pastebin.ubuntu.com/19160299/

Regards.


[  317.940133] [ cut here ]
[  317.944815] WARNING: CPU: 1 PID: 0 at net/ipv4/af_inet.c:155 
inet_sock_destruct+0x1c4/0x1dc
[  317.953223] Modules linked in:
[  317.956305] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 
4.7.0-rc6-00010-gd07031bdc433-dirty #2
[  317.964784] Hardware name: Sigma Tango DT
[  317.968809] Backtrace: 
[  317.971279] [] (dump_backtrace) from [] 
(show_stack+0x18/0x1c)
[  317.978884]  r7:6113 r6:c080ea84 r5: r4:c080ea84
[  317.984590] [] (show_stack) from [] 
(dump_stack+0x80/0x94)
[  317.991856] [] (dump_stack) from [] (__warn+0xec/0x104)
[  317.998849]  r7:0009 r6:c05e3fc8 r5: r4:
[  318.004549] [] (__warn) from [] 
(warn_slowpath_null+0x28/0x30)
[  318.012154]  r9:dfbea4e0 r8:000a r7:df45fe30 r6:dec19594 r5:df68f144 
r4:df68f040
[  318.019954] [] (warn_slowpath_null) from [] 
(inet_sock_destruct+0x1c4/0x1dc)
[  318.028788] [] (inet_sock_destruct) from [] 
(__sk_destruct+0x28/0xe0)
[  318.037005]  r7:df45fe30 r6:dec19594 r5:df68f040 r4:df68f1ec
[  318.042710] [] (__sk_destruct) from [] 
(rcu_process_callbacks+0x488/0x59c)
[  318.051363]  r5: r4:
[  318.054962] [] (rcu_process_callbacks) from [] 
(__do_softirq+0x138/0x264)
[  318.063527]  r10:c08020a0 r9:4001 r8:0101 r7:df45e000 r6:c08020a4 
r5:0009
[  318.071408]  r4:
[  318.073953] [] (__do_softirq) from [] 
(irq_exit+0xc8/0x104)
[  318.081296]  r10:df45ff58 r9:df402400 r8:0001 r7: r6:0013 
r5:
[  318.089176]  r4:c0735428
[  318.091723] [] (irq_exit) from [] 
(__handle_domain_irq+0x88/0xf4)
[  318.099595] [] (__handle_domain_irq) from [] 
(gic_handle_irq+0x50/0x94)
[  318.107986]  r10: r9:e0803100 r8:e0802100 r7:df45ff58 r6:e080210c 
r5:c080277c
[  318.115865]  r4:c080eca0 r3:df45ff58
[  318.119461] [] (gic_handle_irq) from [] 
(__irq_svc+0x54/0x90)
[  318.126980] Exception stack(0xdf45ff58 to 0xdf45ffa0)
[  318.132053] ff40:   
0001 
[  318.140273] ff60: ab80 c0117c80 df45e000 c08024f8 c0802494 c081e2d6 
c05b9550 413fc090
[  318.148492] ff80:  df45ffb4 df45ffb8 df45ffa8 c01086b0 c01086b4 
6013 
[  318.156709]  r9:413fc090 r8:c05b9550 r7:df45ff8c r6: r5:6013 
r4:c01086b4
[  318.164512] [] (arch_cpu_idle) from [] 
(default_idle_call+0x28/0x34)
[  318.172646] [] (default_idle_call) from [] 
(cpu_startup_entry+0x128/0x17c)
[  318.181305] [] (cpu_startup_entry) from [] 
(secondary_start_kernel+0x158/0x164)
[  318.190395]  r7:c081e7c8 r4:c080b4f0
[  318.193993] [] (secondary_start_kernel) from [<8010158c>] 
(0x8010158c)
[  318.201423]  r5:0051 r4:9f44006a
[  318.205024] ---[ end trace 6e04001434b19cb9 ]---


Just to be sure, I performed the same steps a second time:

[  316.238527] [ cut here ]
[  316.243210] WARNING: CPU: 1 PID: 0 at net/ipv4/af_inet.c:155 
inet_sock_destruct+0x1c4/0x1dc
[  316.251619] Modules linked in:
[  316.254702] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 
4.7.0-rc6-00010-gd07031bdc433-dirty #2
[  316.263182] Hardware name: Sigma Tango DT
[  316.267206] Backtrace: 
[  316.269675] [] (dump_backtrace) from [] 
(show_stack+0x18/0x1c)
[  316.277280]  r7:6113 r6:c080ea84 r5: r4:c080ea84
[  316.282986] [] (show_stack) from [] 
(dump_stack+0x80/0x94)
[  316.290254] [] (dump_stack) from [] (__warn+0xec/0x104)
[  316.297247]  r7:0009 r6:c05e3fc8 r5: r4:
[  316.302947] [] (__warn) from [] 
(warn_slowpath_null+0x28/0x30)
[  316.310552]  r9:dfbea4e0 r8:000a r7:df45fe30 r6:dec15694 r5:df6063c4 
r4:df6062c0
[  316.318354] [] (warn_slowpath_null) from [] 
(inet_sock_destruct+0x1c4/0x1dc)
[  316.327190] [] (inet_sock_destruct) from [] 
(__sk_destruct+0x28/0xe0)
[  316.335406]  r7:df45fe30 r6:dec15694 r5:df6062c0 r4:df60646c
[  316.341112] [] (__sk_destruct) from [] 
(rcu_process_callbacks+0x488/0x59c)
[  316.349765]  r5: r4:
[  316.353363] [] (rcu_process_callbacks) from [] 
(__do_softirq+0x

Re: [PATCH v2 0/6] net: ethernet: bgmac: Add platform device support

[Jon Mason]

On Thu, Jul 7, 2016 at 7:08 PM, Jon Mason <jon.ma...@broadcom.com> wrote:
> David Miller, Please consider including patches 1-5 in net-next
>
> Florian Fainelli, Please consider including patches 6 & 7 in
>   devicetree/next

Oops.  I didn't send out the 7th patch in this series.  Sending out
shortly as 7/7.

Thanks,
Jon

> Changes in v2:
> * Made device tree binding changes suggested by Sergei Shtylyov,
>   Ray Jui, Rob Herring, Florian Fainelli, and Arnd Bergmann
> * Removed devm_* error paths in the bgmac_platform.c suggested by
>   Florian Fainelli
> * Added Arnd Bergmann's Acked-by to the first 5 (there were changes
>   outlined in the bullets above, but I believe them to be minor enough
>   for him to not revoke his acks)
>
>
> This patch series adds support for other, non-bcma iProc SoC's to the
> bgmac driver.  This series only adds NSP support, but we are interested
> in adding support for the Cygnus and NS2 families (with more possible
> down the road).
>
> To support non-bcma enabled SoCs, we need to add the standard device
> tree "platform device" support.  Unfortunately, this driver is very
> tighly coupled with the bcma bus and much unwinding is needed.  I tried
> to break this up into a number of patches to make it more obvious what
> was being done to add platform device support.  I was able to verify
> that the bcma code still works using a 53012K board (NS SoC), and that
> the platform code works using a 58625K board (NSP SoC).
>
> Thanks,
> Jon
>
>
> Jon Mason (6):
>   net: ethernet: bgmac: change bgmac_* prints to dev_* prints
>   net: ethernet: bgmac: add dma_dev pointer
>   net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file
>   net: ethernet: bgmac: convert to feature flags
>   net: ethernet: bgmac: Add platform device support
>   dt-bindings: net: bgmac: add bindings documentation for bgmac
>
>  .../devicetree/bindings/net/brcm,amac.txt  |  24 +
>  .../devicetree/bindings/net/brcm,bgmac-nsp.txt |  24 +
>  drivers/net/ethernet/broadcom/Kconfig  |  23 +-
>  drivers/net/ethernet/broadcom/Makefile |   2 +
>  drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c| 266 +
>  drivers/net/ethernet/broadcom/bgmac-bcma.c | 315 ++
>  drivers/net/ethernet/broadcom/bgmac-platform.c | 189 ++
>  drivers/net/ethernet/broadcom/bgmac.c  | 658 
> +
>  drivers/net/ethernet/broadcom/bgmac.h  | 112 +++-
>  9 files changed, 1097 insertions(+), 516 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/brcm,amac.txt
>  create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
>  create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
>  create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma.c
>  create mode 100644 drivers/net/ethernet/broadcom/bgmac-platform.c
>
> --
> 1.9.1
>

[PATCH 7/7] ARM: dts: NSP: Add bgmac entries

[Jon Mason]

Add device tree entries for the ethernet devices present on the
Broadcom Northstar Plus SoCs

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 arch/arm/boot/dts/bcm-nsp.dtsi   | 18 ++
 arch/arm/boot/dts/bcm958625k.dts |  8 
 2 files changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
index def9e78..f6d5abe 100644
--- a/arch/arm/boot/dts/bcm-nsp.dtsi
+++ b/arch/arm/boot/dts/bcm-nsp.dtsi
@@ -192,6 +192,24 @@
status = "disabled";
};
 
+   amac0: ethernet@22000 {
+   compatible = "brcm,nsp-amac";
+   reg = <0x022000 0x1000>,
+ <0x11 0x1000>;
+   reg-names = "amac_base", "idm_base";
+   interrupts = ;
+   status = "disabled";
+   };
+
+   amac1: ethernet@23000 {
+   compatible = "brcm,nsp-amac";
+   reg = <0x023000 0x1000>,
+ <0x111000 0x1000>;
+   reg-names = "amac_base", "idm_base";
+   interrupts = ;
+   status = "disabled";
+   };
+
nand: nand@26000 {
compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1";
reg = <0x026000 0x600>,
diff --git a/arch/arm/boot/dts/bcm958625k.dts b/arch/arm/boot/dts/bcm958625k.dts
index e298450..f41a13b 100644
--- a/arch/arm/boot/dts/bcm958625k.dts
+++ b/arch/arm/boot/dts/bcm958625k.dts
@@ -56,6 +56,14 @@
status = "okay";
 };
 
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
  {
status = "okay";
 };
-- 
1.9.1

[PATCH v2 4/6] net: ethernet: bgmac: convert to feature flags

[Jon Mason]

The bgmac driver is using the bcma provides device ID and revision, as
well as the SoC ID and package, to determine which features are
necessary to enable, reset, etc in the driver.   In anticipation of
removing the bcma requirement for this driver, these must be changed to
not reference that struct.  In place of that, each "feature" has been
given a flag, and the flags are enabled for their respective device and
SoC.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
Acked-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/net/ethernet/broadcom/bgmac.c | 167 --
 drivers/net/ethernet/broadcom/bgmac.h |  21 -
 2 files changed, 140 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c 
b/drivers/net/ethernet/broadcom/bgmac.c
index 6c6bb18..b85e39a 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -109,7 +109,7 @@ static void bgmac_dma_tx_enable(struct bgmac *bgmac,
u32 ctl;
 
ctl = bgmac_read(bgmac, ring->mmio_base + BGMAC_DMA_TX_CTL);
-   if (bgmac->core->id.rev >= 4) {
+   if (bgmac->feature_flags & BGMAC_FEAT_TX_MASK_SETUP) {
ctl &= ~BGMAC_DMA_TX_BL_MASK;
ctl |= BGMAC_DMA_TX_BL_128 << BGMAC_DMA_TX_BL_SHIFT;
 
@@ -330,7 +330,7 @@ static void bgmac_dma_rx_enable(struct bgmac *bgmac,
u32 ctl;
 
ctl = bgmac_read(bgmac, ring->mmio_base + BGMAC_DMA_RX_CTL);
-   if (bgmac->core->id.rev >= 4) {
+   if (bgmac->feature_flags & BGMAC_FEAT_RX_MASK_SETUP) {
ctl &= ~BGMAC_DMA_RX_BL_MASK;
ctl |= BGMAC_DMA_RX_BL_128 << BGMAC_DMA_RX_BL_SHIFT;
 
@@ -768,14 +768,20 @@ static void bgmac_cmdcfg_maskset(struct bgmac *bgmac, u32 
mask, u32 set,
 {
u32 cmdcfg = bgmac_read(bgmac, BGMAC_CMDCFG);
u32 new_val = (cmdcfg & mask) | set;
+   u32 cmdcfg_sr;
 
-   bgmac_set(bgmac, BGMAC_CMDCFG, BGMAC_CMDCFG_SR(bgmac->core->id.rev));
+   if (bgmac->feature_flags & BGMAC_FEAT_CMDCFG_SR_REV4)
+   cmdcfg_sr = BGMAC_CMDCFG_SR_REV4;
+   else
+   cmdcfg_sr = BGMAC_CMDCFG_SR_REV0;
+
+   bgmac_set(bgmac, BGMAC_CMDCFG, cmdcfg_sr);
udelay(2);
 
if (new_val != cmdcfg || force)
bgmac_write(bgmac, BGMAC_CMDCFG, new_val);
 
-   bgmac_mask(bgmac, BGMAC_CMDCFG, ~BGMAC_CMDCFG_SR(bgmac->core->id.rev));
+   bgmac_mask(bgmac, BGMAC_CMDCFG, ~cmdcfg_sr);
udelay(2);
 }
 
@@ -804,7 +810,7 @@ static void bgmac_chip_stats_update(struct bgmac *bgmac)
 {
int i;
 
-   if (bgmac->core->id.id != BCMA_CORE_4706_MAC_GBIT) {
+   if (!(bgmac->feature_flags & BGMAC_FEAT_NO_CLR_MIB)) {
for (i = 0; i < BGMAC_NUM_MIB_TX_REGS; i++)
bgmac->mib_tx_regs[i] =
bgmac_read(bgmac,
@@ -823,7 +829,7 @@ static void bgmac_clear_mib(struct bgmac *bgmac)
 {
int i;
 
-   if (bgmac->core->id.id == BCMA_CORE_4706_MAC_GBIT)
+   if (bgmac->feature_flags & BGMAC_FEAT_NO_CLR_MIB)
return;
 
bgmac_set(bgmac, BGMAC_DEV_CTL, BGMAC_DC_MROR);
@@ -866,9 +872,8 @@ static void bgmac_mac_speed(struct bgmac *bgmac)
 static void bgmac_miiconfig(struct bgmac *bgmac)
 {
struct bcma_device *core = bgmac->core;
-   u8 imode;
 
-   if (bgmac_is_bcm4707_family(bgmac)) {
+   if (bgmac->feature_flags & BGMAC_FEAT_FORCE_SPEED_2500) {
bcma_awrite32(core, BCMA_IOCTL,
  bcma_aread32(core, BCMA_IOCTL) | 0x40 |
  BGMAC_BCMA_IOCTL_SW_CLKEN);
@@ -876,6 +881,8 @@ static void bgmac_miiconfig(struct bgmac *bgmac)
bgmac->mac_duplex = DUPLEX_FULL;
bgmac_mac_speed(bgmac);
} else {
+   u8 imode;
+
imode = (bgmac_read(bgmac, BGMAC_DEV_STATUS) &
BGMAC_DS_MM_MASK) >> BGMAC_DS_MM_SHIFT;
if (imode == 0 || imode == 1) {
@@ -890,9 +897,7 @@ static void bgmac_miiconfig(struct bgmac *bgmac)
 static void bgmac_chip_reset(struct bgmac *bgmac)
 {
struct bcma_device *core = bgmac->core;
-   struct bcma_bus *bus = core->bus;
-   struct bcma_chipinfo *ci = >chipinfo;
-   u32 flags;
+   u32 cmdcfg_sr;
u32 iost;
int i;
 
@@ -915,15 +920,12 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
}
 
iost = bcma_aread32(core, BCMA_IOST);
-   if ((ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg == BCMA_PKG_ID_BCM47186) 
||
-   (ci->id == BCMA_CHIP_ID_BCM4749 && ci->pkg == 10) ||
-   (ci->id == BCMA_CHIP_ID_BCM53572 && ci->pkg == 
BCMA_PKG_ID_BCM47188))
+   if (bgmac->feature_flags & BGMAC_FEAT_IOST_ATTACHED)
   

[PATCH v2 5/6] net: ethernet: bgmac: Add platform device support

[Jon Mason]

The bcma portion of the driver has been split off into a bcma specific
driver.  This has been mirrored for the platform driver.  The last
references to the bcma core struct have been changed into a generic
function call.  These function calls are wrappers to either the original
bcma code or new platform functions that access the same areas via MMIO.
This necessitated adding function pointers for both platform and bcma to
hide which backend is being used from the generic bgmac code.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
Acked-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/net/ethernet/broadcom/Kconfig   |  23 +-
 drivers/net/ethernet/broadcom/Makefile  |   4 +-
 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c |   2 +
 drivers/net/ethernet/broadcom/bgmac-bcma.c  | 315 +++
 drivers/net/ethernet/broadcom/bgmac-platform.c  | 189 ++
 drivers/net/ethernet/broadcom/bgmac.c   | 329 
 drivers/net/ethernet/broadcom/bgmac.h   |  73 +-
 7 files changed, 650 insertions(+), 285 deletions(-)
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma.c
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-platform.c

diff --git a/drivers/net/ethernet/broadcom/Kconfig 
b/drivers/net/ethernet/broadcom/Kconfig
index d74a92e..bd8c80c 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -140,10 +140,18 @@ config BNX2X_SRIOV
  allows for virtual function acceleration in virtual environments.
 
 config BGMAC
-   tristate "BCMA bus GBit core support"
+   tristate
+   help
+ This enables the integrated ethernet controller support for many
+ Broadcom (mostly iProc) SoCs. An appropriate bus interface driver
+ needs to be enabled to select this.
+
+config BGMAC_BCMA
+   tristate "Broadcom iProc GBit BCMA support"
depends on BCMA && BCMA_HOST_SOC
depends on HAS_DMA
depends on BCM47XX || ARCH_BCM_5301X || COMPILE_TEST
+   select BGMAC
select PHYLIB
select FIXED_PHY
---help---
@@ -152,6 +160,19 @@ config BGMAC
  In case of using this driver on BCM4706 it's also requires to enable
  BCMA_DRIVER_GMAC_CMN to make it work.
 
+config BGMAC_PLATFORM
+   tristate "Broadcom iProc GBit platform support"
+   depends on HAS_DMA
+   depends on ARCH_BCM_IPROC || COMPILE_TEST
+   depends on OF
+   select BGMAC
+   select PHYLIB
+   select FIXED_PHY
+   default ARCH_BCM_IPROC
+   ---help---
+ Say Y here if you want to use the Broadcom iProc Gigabit Ethernet
+ controller through the generic platform interface
+
 config SYSTEMPORT
tristate "Broadcom SYSTEMPORT internal MAC support"
depends on OF
diff --git a/drivers/net/ethernet/broadcom/Makefile 
b/drivers/net/ethernet/broadcom/Makefile
index f559794..79f2372 100644
--- a/drivers/net/ethernet/broadcom/Makefile
+++ b/drivers/net/ethernet/broadcom/Makefile
@@ -10,6 +10,8 @@ obj-$(CONFIG_CNIC) += cnic.o
 obj-$(CONFIG_BNX2X) += bnx2x/
 obj-$(CONFIG_SB1250_MAC) += sb1250-mac.o
 obj-$(CONFIG_TIGON3) += tg3.o
-obj-$(CONFIG_BGMAC) += bgmac.o bgmac-bcma-mdio.o
+obj-$(CONFIG_BGMAC) += bgmac.o
+obj-$(CONFIG_BGMAC_BCMA) += bgmac-bcma.o bgmac-bcma-mdio.o
+obj-$(CONFIG_BGMAC_PLATFORM) += bgmac-platform.o
 obj-$(CONFIG_SYSTEMPORT) += bcmsysport.o
 obj-$(CONFIG_BNXT) += bnxt/
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c 
b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
index 1e65349..7c19c8e 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
@@ -245,6 +245,7 @@ err:
kfree(bcma_mdio);
return ERR_PTR(err);
 }
+EXPORT_SYMBOL_GPL(bcma_mdio_mii_register);
 
 void bcma_mdio_mii_unregister(struct mii_bus *mii_bus)
 {
@@ -259,6 +260,7 @@ void bcma_mdio_mii_unregister(struct mii_bus *mii_bus)
mdiobus_free(mii_bus);
kfree(bcma_mdio);
 }
+EXPORT_SYMBOL_GPL(bcma_mdio_mii_unregister);
 
 MODULE_AUTHOR("Rafał Miłecki");
 MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c 
b/drivers/net/ethernet/broadcom/bgmac-bcma.c
new file mode 100644
index 000..9a9745c4
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -0,0 +1,315 @@
+/*
+ * Driver for (BCM4706)? GBit MAC core on BCMA bus.
+ *
+ * Copyright (C) 2012 Rafał Miłecki <zaj...@gmail.com>
+ *
+ * Licensed under the GNU/GPL. See COPYING for details.
+ */
+
+#define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include "bgmac.h"
+
+static inline bool bgmac_is_bcm4707_family(struct bcma_device *core)
+{
+   switch (core->bus->chipinfo.id) {
+   case BCMA_CHIP_ID_BCM4707:
+   case BCMA_CHIP_ID_BCM47094

[PATCH v2 0/6] net: ethernet: bgmac: Add platform device support

[Jon Mason]

David Miller, Please consider including patches 1-5 in net-next

Florian Fainelli, Please consider including patches 6 & 7 in
  devicetree/next

Changes in v2:
* Made device tree binding changes suggested by Sergei Shtylyov,
  Ray Jui, Rob Herring, Florian Fainelli, and Arnd Bergmann
* Removed devm_* error paths in the bgmac_platform.c suggested by
  Florian Fainelli
* Added Arnd Bergmann's Acked-by to the first 5 (there were changes
  outlined in the bullets above, but I believe them to be minor enough
  for him to not revoke his acks)


This patch series adds support for other, non-bcma iProc SoC's to the
bgmac driver.  This series only adds NSP support, but we are interested
in adding support for the Cygnus and NS2 families (with more possible
down the road).

To support non-bcma enabled SoCs, we need to add the standard device
tree "platform device" support.  Unfortunately, this driver is very
tighly coupled with the bcma bus and much unwinding is needed.  I tried
to break this up into a number of patches to make it more obvious what
was being done to add platform device support.  I was able to verify
that the bcma code still works using a 53012K board (NS SoC), and that
the platform code works using a 58625K board (NSP SoC).

Thanks,
Jon


Jon Mason (6):
  net: ethernet: bgmac: change bgmac_* prints to dev_* prints
  net: ethernet: bgmac: add dma_dev pointer
  net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file
  net: ethernet: bgmac: convert to feature flags
  net: ethernet: bgmac: Add platform device support
  dt-bindings: net: bgmac: add bindings documentation for bgmac

 .../devicetree/bindings/net/brcm,amac.txt  |  24 +
 .../devicetree/bindings/net/brcm,bgmac-nsp.txt |  24 +
 drivers/net/ethernet/broadcom/Kconfig  |  23 +-
 drivers/net/ethernet/broadcom/Makefile |   2 +
 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c| 266 +
 drivers/net/ethernet/broadcom/bgmac-bcma.c | 315 ++
 drivers/net/ethernet/broadcom/bgmac-platform.c | 189 ++
 drivers/net/ethernet/broadcom/bgmac.c  | 658 +
 drivers/net/ethernet/broadcom/bgmac.h  | 112 +++-
 9 files changed, 1097 insertions(+), 516 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/brcm,amac.txt
 create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma.c
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-platform.c

-- 
1.9.1

[PATCH v2 6/6] dt-bindings: net: bgmac: add bindings documentation for bgmac

[Jon Mason]

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 .../devicetree/bindings/net/brcm,amac.txt  | 24 ++
 .../devicetree/bindings/net/brcm,bgmac-nsp.txt | 24 ++
 2 files changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/brcm,amac.txt
 create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt

diff --git a/Documentation/devicetree/bindings/net/brcm,amac.txt 
b/Documentation/devicetree/bindings/net/brcm,amac.txt
new file mode 100644
index 000..ba5ecc1
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/brcm,amac.txt
@@ -0,0 +1,24 @@
+Broadcom AMAC Ethernet Controller Device Tree Bindings
+-
+
+Required properties:
+ - compatible: "brcm,amac" or "brcm,nsp-amac"
+ - reg:Address and length of the GMAC registers,
+   Address and length of the GMAC IDM registers
+ - reg-names:  Names of the registers.  Must have both "amac_base" and
+   "idm_base"
+ - interrupts: Interrupt number
+
+Optional properties:
+- mac-address: See ethernet.txt file in the same directory
+
+Examples:
+
+amac0: ethernet@18022000 {
+   compatible = "brcm,nsp-amac";
+   reg = <0x18022000 0x1000>,
+ <0x1811 0x1000>;
+   reg-names = "amac_base", "idm_base";
+   interrupts = ;
+   status = "disabled";
+};
diff --git a/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt 
b/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
new file mode 100644
index 000..022946c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
@@ -0,0 +1,24 @@
+Broadcom GMAC Ethernet Controller Device Tree Bindings
+-
+
+Required properties:
+ - compatible: "brcm,bgmac-nsp"
+ - reg:Address and length of the GMAC registers,
+   Address and length of the GMAC IDM registers
+ - reg-names:  Names of the registers.  Must have both "gmac_base" and
+   "idm_base"
+ - interrupts: Interrupt number
+
+Optional properties:
+- mac-address: See ethernet.txt file in the same directory
+
+Examples:
+
+gmac0: ethernet@18022000 {
+   compatible = "brcm,bgmac-nsp";
+   reg = <0x18022000 0x1000>,
+ <0x1811 0x1000>;
+   reg-names = "gmac_base", "idm_base";
+   interrupts = ;
+   status = "disabled";
+};
-- 
1.9.1

[PATCH v2 1/6] net: ethernet: bgmac: change bgmac_* prints to dev_* prints

[Jon Mason]

The bgmac_* print wrappers call dev_* prints with the dev pointer from
the bcma core.  In anticipation of removing the bcma requirement for
this driver, these must be changed to not reference that struct.  So,
simply change all of the bgmac_* prints to their dev_* counterparts.  In
some cases netdev_* prints are more appropriate, so change those as
well.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
Acked-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/net/ethernet/broadcom/bgmac.c | 103 +-
 drivers/net/ethernet/broadcom/bgmac.h |  14 +
 2 files changed, 55 insertions(+), 62 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c 
b/drivers/net/ethernet/broadcom/bgmac.c
index e6e74ca..37b3b68 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -50,7 +50,7 @@ static bool bgmac_wait_value(struct bcma_device *core, u16 
reg, u32 mask,
return true;
udelay(10);
}
-   pr_err("Timeout waiting for reg 0x%X\n", reg);
+   dev_err(>dev, "Timeout waiting for reg 0x%X\n", reg);
return false;
 }
 
@@ -84,8 +84,8 @@ static void bgmac_dma_tx_reset(struct bgmac *bgmac, struct 
bgmac_dma_ring *ring)
udelay(10);
}
if (i)
-   bgmac_err(bgmac, "Timeout suspending DMA TX ring 0x%X 
(BGMAC_DMA_TX_STAT: 0x%08X)\n",
- ring->mmio_base, val);
+   dev_err(bgmac->dev, "Timeout suspending DMA TX ring 0x%X 
(BGMAC_DMA_TX_STAT: 0x%08X)\n",
+   ring->mmio_base, val);
 
/* Remove SUSPEND bit */
bgmac_write(bgmac, ring->mmio_base + BGMAC_DMA_TX_CTL, 0);
@@ -93,13 +93,13 @@ static void bgmac_dma_tx_reset(struct bgmac *bgmac, struct 
bgmac_dma_ring *ring)
  ring->mmio_base + BGMAC_DMA_TX_STATUS,
  BGMAC_DMA_TX_STAT, BGMAC_DMA_TX_STAT_DISABLED,
  1)) {
-   bgmac_warn(bgmac, "DMA TX ring 0x%X wasn't disabled on time, 
waiting additional 300us\n",
-  ring->mmio_base);
+   dev_warn(bgmac->dev, "DMA TX ring 0x%X wasn't disabled on time, 
waiting additional 300us\n",
+ring->mmio_base);
udelay(300);
val = bgmac_read(bgmac, ring->mmio_base + BGMAC_DMA_TX_STATUS);
if ((val & BGMAC_DMA_TX_STAT) != BGMAC_DMA_TX_STAT_DISABLED)
-   bgmac_err(bgmac, "Reset of DMA TX ring 0x%X failed\n",
- ring->mmio_base);
+   dev_err(bgmac->dev, "Reset of DMA TX ring 0x%X 
failed\n",
+   ring->mmio_base);
}
 }
 
@@ -161,7 +161,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
int i;
 
if (skb->len > BGMAC_DESC_CTL1_LEN) {
-   bgmac_err(bgmac, "Too long skb (%d)\n", skb->len);
+   netdev_err(bgmac->net_dev, "Too long skb (%d)\n", skb->len);
goto err_drop;
}
 
@@ -174,7 +174,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 * even when ring->end overflows
 */
if (ring->end - ring->start + nr_frags + 1 >= BGMAC_TX_RING_SLOTS) {
-   bgmac_err(bgmac, "TX ring is full, queue should be stopped!\n");
+   netdev_err(bgmac->net_dev, "TX ring is full, queue should be 
stopped!\n");
netif_stop_queue(net_dev);
return NETDEV_TX_BUSY;
}
@@ -241,8 +241,8 @@ err_dma:
}
 
 err_dma_head:
-   bgmac_err(bgmac, "Mapping error of skb on ring 0x%X\n",
- ring->mmio_base);
+   netdev_err(bgmac->net_dev, "Mapping error of skb on ring 0x%X\n",
+  ring->mmio_base);
 
 err_drop:
dev_kfree_skb(skb);
@@ -320,8 +320,8 @@ static void bgmac_dma_rx_reset(struct bgmac *bgmac, struct 
bgmac_dma_ring *ring)
  ring->mmio_base + BGMAC_DMA_RX_STATUS,
  BGMAC_DMA_RX_STAT, BGMAC_DMA_RX_STAT_DISABLED,
  1))
-   bgmac_err(bgmac, "Reset of ring 0x%X RX failed\n",
- ring->mmio_base);
+   dev_err(bgmac->dev, "Reset of ring 0x%X RX failed\n",
+   ring->mmio_base);
 }
 
 static void bgmac_dma_rx_enable(struct bgmac *bgmac,
@@ -370,7 +370,7 @@ static int bgmac_dma_rx_skb_for_slot(struct bgmac *bgmac,
dma_addr = dma_map_single(dma_dev, buf + BGMAC_RX_BUF_OFFSET,
  BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
if (dma_mapping_error(dma_dev, dma_addr)) {
-  

[PATCH v2 2/6] net: ethernet: bgmac: add dma_dev pointer

[Jon Mason]

The dma buffer allocation, etc references a dma_dev device pointer from
the bcma core.  In anticipation of removing the bcma requirement for
this driver, these must be changed to not reference that struct.  Add a
dma_dev device pointer to the bgmac stuct and reference that instead.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
Acked-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/net/ethernet/broadcom/bgmac.c | 17 +
 drivers/net/ethernet/broadcom/bgmac.h |  1 +
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c 
b/drivers/net/ethernet/broadcom/bgmac.c
index 37b3b68..3614bd8 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -152,7 +152,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
struct bgmac_dma_ring *ring,
struct sk_buff *skb)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct net_device *net_dev = bgmac->net_dev;
int index = ring->end % BGMAC_TX_RING_SLOTS;
struct bgmac_slot_info *slot = >slots[index];
@@ -254,7 +254,7 @@ err_drop:
 /* Free transmitted packets */
 static void bgmac_dma_tx_free(struct bgmac *bgmac, struct bgmac_dma_ring *ring)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
int empty_slot;
bool freed = false;
unsigned bytes_compl = 0, pkts_compl = 0;
@@ -351,7 +351,7 @@ static void bgmac_dma_rx_enable(struct bgmac *bgmac,
 static int bgmac_dma_rx_skb_for_slot(struct bgmac *bgmac,
 struct bgmac_slot_info *slot)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
dma_addr_t dma_addr;
struct bgmac_rx_header *rx;
void *buf;
@@ -440,7 +440,7 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct 
bgmac_dma_ring *ring,
end_slot /= sizeof(struct bgmac_dma_desc);
 
while (ring->start != end_slot) {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct bgmac_slot_info *slot = >slots[ring->start];
struct bgmac_rx_header *rx = slot->buf + BGMAC_RX_BUF_OFFSET;
struct sk_buff *skb;
@@ -543,7 +543,7 @@ static bool bgmac_dma_unaligned(struct bgmac *bgmac,
 static void bgmac_dma_tx_ring_free(struct bgmac *bgmac,
   struct bgmac_dma_ring *ring)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct bgmac_dma_desc *dma_desc = ring->cpu_base;
struct bgmac_slot_info *slot;
int i;
@@ -569,7 +569,7 @@ static void bgmac_dma_tx_ring_free(struct bgmac *bgmac,
 static void bgmac_dma_rx_ring_free(struct bgmac *bgmac,
   struct bgmac_dma_ring *ring)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct bgmac_slot_info *slot;
int i;
 
@@ -590,7 +590,7 @@ static void bgmac_dma_ring_desc_free(struct bgmac *bgmac,
 struct bgmac_dma_ring *ring,
 int num_slots)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
int size;
 
if (!ring->cpu_base)
@@ -628,7 +628,7 @@ static void bgmac_dma_free(struct bgmac *bgmac)
 
 static int bgmac_dma_alloc(struct bgmac *bgmac)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct bgmac_dma_ring *ring;
static const u16 ring_base[] = { BGMAC_DMA_BASE0, BGMAC_DMA_BASE1,
 BGMAC_DMA_BASE2, BGMAC_DMA_BASE3, };
@@ -1701,6 +1701,7 @@ static int bgmac_probe(struct bcma_device *core)
net_dev->ethtool_ops = _ethtool_ops;
bgmac = netdev_priv(net_dev);
bgmac->dev = >dev;
+   bgmac->dma_dev = core->dma_dev;
bgmac->net_dev = net_dev;
bgmac->core = core;
bcma_set_drvdata(core, bgmac);
diff --git a/drivers/net/ethernet/broadcom/bgmac.h 
b/drivers/net/ethernet/broadcom/bgmac.h
index abb9dd8..fd20018 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -429,6 +429,7 @@ struct bgmac {
struct bcma_device *cmn; /* Reference to CMN core for BCM4706 */
 
struct device *dev;
+   struct device *dma_dev;
struct net_device *net_dev;
struct napi_struct napi;
struct mii_bus *mii_bus;
-- 
1.9.1

[PATCH v2 3/6] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file

[Jon Mason]

Move the BCMA MDIO phy into a separate file, as it is very tightly
coupled with the BCMA bus.  This will help with the upcoming BCMA
removal from the bgmac driver.  Optimally, this should be moved into
phy drivers, but it is too tightly coupled with the bgmac driver to
effectively move it without more changes to the driver.

Note: the phy_reset was intentionally removed, as the mdio phy subsystem
automatically resets the phy if a reset function pointer is present.  In
addition to the moving of the driver, this reset function is added.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
Acked-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/net/ethernet/broadcom/Makefile  |   2 +-
 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c | 264 
 drivers/net/ethernet/broadcom/bgmac.c   | 246 +++---
 drivers/net/ethernet/broadcom/bgmac.h   |   3 +
 4 files changed, 298 insertions(+), 217 deletions(-)
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c

diff --git a/drivers/net/ethernet/broadcom/Makefile 
b/drivers/net/ethernet/broadcom/Makefile
index 00584d7..f559794 100644
--- a/drivers/net/ethernet/broadcom/Makefile
+++ b/drivers/net/ethernet/broadcom/Makefile
@@ -10,6 +10,6 @@ obj-$(CONFIG_CNIC) += cnic.o
 obj-$(CONFIG_BNX2X) += bnx2x/
 obj-$(CONFIG_SB1250_MAC) += sb1250-mac.o
 obj-$(CONFIG_TIGON3) += tg3.o
-obj-$(CONFIG_BGMAC) += bgmac.o
+obj-$(CONFIG_BGMAC) += bgmac.o bgmac-bcma-mdio.o
 obj-$(CONFIG_SYSTEMPORT) += bcmsysport.o
 obj-$(CONFIG_BNXT) += bnxt/
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c 
b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
new file mode 100644
index 000..1e65349
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
@@ -0,0 +1,264 @@
+/*
+ * Driver for (BCM4706)? GBit MAC core on BCMA bus.
+ *
+ * Copyright (C) 2012 Rafał Miłecki <zaj...@gmail.com>
+ *
+ * Licensed under the GNU/GPL. See COPYING for details.
+ */
+
+#define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include "bgmac.h"
+
+struct bcma_mdio {
+   struct bcma_device *core;
+   u8 phyaddr;
+};
+
+static bool bcma_mdio_wait_value(struct bcma_device *core, u16 reg, u32 mask,
+u32 value, int timeout)
+{
+   u32 val;
+   int i;
+
+   for (i = 0; i < timeout / 10; i++) {
+   val = bcma_read32(core, reg);
+   if ((val & mask) == value)
+   return true;
+   udelay(10);
+   }
+   dev_err(>dev, "Timeout waiting for reg 0x%X\n", reg);
+   return false;
+}
+
+/**
+ * PHY ops
+ **/
+
+static u16 bcma_mdio_phy_read(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg)
+{
+   struct bcma_device *core;
+   u16 phy_access_addr;
+   u16 phy_ctl_addr;
+   u32 tmp;
+
+   BUILD_BUG_ON(BGMAC_PA_DATA_MASK != BCMA_GMAC_CMN_PA_DATA_MASK);
+   BUILD_BUG_ON(BGMAC_PA_ADDR_MASK != BCMA_GMAC_CMN_PA_ADDR_MASK);
+   BUILD_BUG_ON(BGMAC_PA_ADDR_SHIFT != BCMA_GMAC_CMN_PA_ADDR_SHIFT);
+   BUILD_BUG_ON(BGMAC_PA_REG_MASK != BCMA_GMAC_CMN_PA_REG_MASK);
+   BUILD_BUG_ON(BGMAC_PA_REG_SHIFT != BCMA_GMAC_CMN_PA_REG_SHIFT);
+   BUILD_BUG_ON(BGMAC_PA_WRITE != BCMA_GMAC_CMN_PA_WRITE);
+   BUILD_BUG_ON(BGMAC_PA_START != BCMA_GMAC_CMN_PA_START);
+   BUILD_BUG_ON(BGMAC_PC_EPA_MASK != BCMA_GMAC_CMN_PC_EPA_MASK);
+   BUILD_BUG_ON(BGMAC_PC_MCT_MASK != BCMA_GMAC_CMN_PC_MCT_MASK);
+   BUILD_BUG_ON(BGMAC_PC_MCT_SHIFT != BCMA_GMAC_CMN_PC_MCT_SHIFT);
+   BUILD_BUG_ON(BGMAC_PC_MTE != BCMA_GMAC_CMN_PC_MTE);
+
+   if (bcma_mdio->core->id.id == BCMA_CORE_4706_MAC_GBIT) {
+   core = bcma_mdio->core->bus->drv_gmac_cmn.core;
+   phy_access_addr = BCMA_GMAC_CMN_PHY_ACCESS;
+   phy_ctl_addr = BCMA_GMAC_CMN_PHY_CTL;
+   } else {
+   core = bcma_mdio->core;
+   phy_access_addr = BGMAC_PHY_ACCESS;
+   phy_ctl_addr = BGMAC_PHY_CNTL;
+   }
+
+   tmp = bcma_read32(core, phy_ctl_addr);
+   tmp &= ~BGMAC_PC_EPA_MASK;
+   tmp |= phyaddr;
+   bcma_write32(core, phy_ctl_addr, tmp);
+
+   tmp = BGMAC_PA_START;
+   tmp |= phyaddr << BGMAC_PA_ADDR_SHIFT;
+   tmp |= reg << BGMAC_PA_REG_SHIFT;
+   bcma_write32(core, phy_access_addr, tmp);
+
+   if (!bcma_mdio_wait_value(core, phy_access_addr, BGMAC_PA_START, 0,
+ 1000)) {
+   dev_err(>dev, "Reading PHY %d register 0x%X failed\n",
+   phyaddr, reg);
+   return 0x;
+   }
+
+   return bcma_read32(core, phy_access_addr) & BGMAC_PA_DATA_MASK;
+}
+
+/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphywr */
+static int bcma_mdio_phy_write

Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

[Jon Mason]

On Wed, Jul 6, 2016 at 3:34 AM, Arnd Bergmann <a...@arndb.de> wrote:
> On Tuesday, July 5, 2016 7:18:45 PM CEST Jon Mason wrote:
>> >
>> > Ok, then I'd suggest making the compatible string here
>> >
>> > compatible = "brcm,nsp-amac", "brcm,amac";
>>
>> It is called GMAC in the NS and NSP documentation, but AMAC is fine
>> with me (as it is called this in the NS2 documentation).  I'll make
>> the necessary change and repush.
>
> Ok, then we can use
>
> compatible = "brcm,nsp-gmac", "brcm,amac";
>
> to be consistent with that documentation and have the generic name as the
> fallback.

After looking at the docs again, I see the blocks being referred to as
"AXI MAC" (while the registers are being called GMAC).  To keep it
consistent, I'll just change everything to be "amac".

Thanks,
Jon

>
> Arnd

Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

[Jon Mason]

On Tue, Jul 5, 2016 at 9:37 AM, Arnd Bergmann <a...@arndb.de> wrote:
> On Monday, July 4, 2016 9:34:35 AM CEST Ray Jui wrote:
>> On 7/1/2016 8:42 AM, Arnd Bergmann wrote:
>> > On Friday, July 1, 2016 11:17:25 AM CEST Jon Mason wrote:
>> >> On Fri, Jul 1, 2016 at 5:46 AM, Arnd Bergmann <a...@arndb.de> wrote:
>> >>> On Thursday, June 30, 2016 6:59:13 PM CEST Jon Mason wrote:
>> >>>> +
>> >>>> +Required properties:
>> >>>> + - compatible: "brcm,bgmac-nsp"
>> >>>> + - reg:Address and length of the GMAC registers,
>> >>>> +   Address and length of the GMAC IDM registers
>> >>>> + - reg-names:  Names of the registers.  Must have both "gmac_base" and
>> >>>> +   "idm_base"
>> >>>> + - interrupts: Interrupt number
>> >>>> +
>> >>>
>> >>>
>> >>> "brcm,bgmac-nsp" sounds a bit too general. As I understand, this is a 
>> >>> family
>> >>> of SoCs that might not all have the exact same implementation of this
>> >>> ethernet device, as we can see from the long lookup table in 
>> >>> bgmac_probe().
>> >>
>> >> The Broadcom iProc family of SoCs contains:
>> >> Northstar
>> >> Northstar Plus
>> >> Cygnus
>> >> Northstar 2
>> >> a few SoCs that are under development
>> >> and a number of ethernet switches (which might never be officially 
>> >> supported)
>> >>
>> >> Each one of these SoCs could have a different revision of the gmac IP
>> >> block, but they should be uniform within each SoC (though there might
>> >> be a A0/B0 change necessary).  The Northstar Plus product family has a
>> >> number of different implementations, but the SoC is unchanged.  So, I
>> >> think this might be too specific, when we really need a general compat
>> >> string.
>> >
>> > Ok, thanks for the clarification, that sounds good enough.
>> >
>> >> Broadcom has a history of sharing IP blocks amongst the different
>> >> divisions.  So, this driver might be used on other SoC families (as it
>> >> apparently has been done in the past, based on the code you
>> >> reference).  I do not know of any way to know what legacy, non-iProc
>> >> chips have used this IP block.  I can make this "brcm,iproc-bgmac",
>> >> and add "brcm,iproc-nsp-bgmac" as an alternative compatible string in
>> >> this file (which I believe you are suggesting), but there might be
>> >> non-iProc SoCs that use this driver.  Is this acceptable?
>> >
>> > If it is also used outside of iProc, then I see no need for the
>> > extra compatible string, although it would not do any harm either.
>> >
>> > Ideally we should name it whatever the name for this IP block is
>> > inside of the company, with "nsp" as the designation for the variant
>> > in Northstar Plus. A lot of Broadcom IP blocks themselves seem to have
>> > some four-digit or five-digit number, maybe this one does too?
>> >
>> >   Arnd
>> >
>>
>> Note this IP block has an official IP controller name of "amac" from the
>> ASIC team.
>
> Ok, then I'd suggest making the compatible string here
>
> compatible = "brcm,nsp-amac", "brcm,amac";

It is called GMAC in the NS and NSP documentation, but AMAC is fine
with me (as it is called this in the NS2 documentation).  I'll make
the necessary change and repush.

Thanks for all of the input.

> or even better if you have a version number associated with it, make that
>
> compatible = "brcm,nsp-amac", "brcm,amac-1.234", "brcm,amac";
>
> replacing 1.234 with the actual version of course.
>
> Arnd
>

Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc

[Mason]

On 05/07/2016 23:22, Florian Fainelli wrote:
> On 07/05/2016 01:26 PM, Mason wrote:
>> On 05/07/2016 18:20, Florian Fainelli wrote:
>>> On 07/05/2016 08:56 AM, Mason wrote:
>>>> On 05/07/2016 17:28, Florian Fainelli wrote:
>>>>
>>>>> nb8800.c does not currently show suspend/resume hooks implemented, are
>>>>> you positive that when you suspend, you properly tear down all HW, stop
>>>>> transmit queues, etc. and do the opposite upon resumption?
>>>>
>>>> I am currently testing the error path for my suspend routine.
>>>> Firmware is, in fact, denying the suspend request, and immediately
>>>> returns control to Linux, without having powered anything down.
>>>>
>>>> I expected not having to save any context in that situation.
>>>> Am I mistaken?
>>>
>>> It depends what power state you are going to and resuming from, and how
>>> much of this is platform dependent, on the platforms I work with S2
>>> preserves register states for our On/Off domain, while S3 only keeps an
>>> always-on power island and shuts off the On/Off domain, you therefore
>>> need to have your drivers in the On/Off domain suspend any activity and
>>> preserve important register states, or re-initialize them from scratch
>>> whichever is the most convenient.
>>
>> Thanks for bringing these details to my attention, they will
>> definitely prove useful when I test an actual suspend/resume
>> sequence. However, I must stress that the platform did NOT
>> power down in my test case, because the firmware currently
>> denies all suspend requests.
>>
>> Therefore, loss of context cannot possibly explain the
>> warning I am seeing.
> 
> No, but if you go all the way down to trying to suspend and the last
> step is the firmware failing, anything you have suspended needs to be
> unwinded, for your ethernet driver that means that you went through a
> successful suspend then resume cycle even if it failed down later when
> the platform attempted to suspend.

So it is the driver's responsibility to "shut down" on resume?
(I had the vague impression that the suspend framework would
"disable" the device through the appropriate callback.)

>>> See drivers/net/ethernet/broadcom/genet/bcmgenet.c which is a driver
>>> that takes care of that for instance, look for bcmgenet_{suspend,resume}
>>
>> Thanks. I will look into it.
>>
>> If I understand correctly, something is missing in the
>> network interface code? (My system is using an NFS root
>> filesystem, so network is an important subsystem.)
> 
> The typical things are detaching the network device and stopping
> transmit queues, but without knowing what changes you have done to
> nb8800.c, hard to tell what else is needed.

I'm using the driver unaltered. So I guess I need to figure out
the exact steps required for suspending a network device.
(I'll look at bcmgenet.c tomorrow.)

>>>>> Is your system clocksource also correctly saved/restored, or if you go
>>>>> through a firmware in-between could it be changing the counter values
>>>>> and make Linux think that more time as elapsed than it really happened?
>>>>
>>>> Thanks for pointing this out, I was not aware I was supposed to save
>>>> and restore the tick counter on suspend/resume. (This is not an issue
>>>> in this specific situation, as the platform is NOT suspended.)
>>>
>>> You don't have to save and restore the clocksource counter, although if
>>> you want proper time accounting to be done across suspend states, you
>>> would want to use a clocksource which is persistent across these suspend
>>> states.
>>
>> The clocksource is a 27 MHz 32-bit tick counter. In other words,
>> the counter wraps around every 159 seconds. If Linux suspends
>> for several hours, how can it determine how much time went by?
> 
> Well, that's unfortunate, then you are pretty much either doomed to
> accepting to lose time in between and rely on e.g: NTP to resync your
> time upon resumption, or, if you had smarter hardware you could have a
> prescaler or something that makes this counter wrap far ahead (like
> years or days after).

Maybe the hardware devs thought of that problem, because they
"widened" the counter to 64 bits on newer platforms.

Regards.

Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc

[Mason]

On 05/07/2016 18:20, Florian Fainelli wrote:
> On 07/05/2016 08:56 AM, Mason wrote:
>> On 05/07/2016 17:28, Florian Fainelli wrote:
>>
>>> nb8800.c does not currently show suspend/resume hooks implemented, are
>>> you positive that when you suspend, you properly tear down all HW, stop
>>> transmit queues, etc. and do the opposite upon resumption?
>>
>> I am currently testing the error path for my suspend routine.
>> Firmware is, in fact, denying the suspend request, and immediately
>> returns control to Linux, without having powered anything down.
>>
>> I expected not having to save any context in that situation.
>> Am I mistaken?
> 
> It depends what power state you are going to and resuming from, and how
> much of this is platform dependent, on the platforms I work with S2
> preserves register states for our On/Off domain, while S3 only keeps an
> always-on power island and shuts off the On/Off domain, you therefore
> need to have your drivers in the On/Off domain suspend any activity and
> preserve important register states, or re-initialize them from scratch
> whichever is the most convenient.

Thanks for bringing these details to my attention, they will
definitely prove useful when I test an actual suspend/resume
sequence. However, I must stress that the platform did NOT
power down in my test case, because the firmware currently
denies all suspend requests.

Therefore, loss of context cannot possibly explain the
warning I am seeing.

>> You mention "stop transmit queues". Can you say more about this?
> 
> See drivers/net/ethernet/broadcom/genet/bcmgenet.c which is a driver
> that takes care of that for instance, look for bcmgenet_{suspend,resume}

Thanks. I will look into it.

If I understand correctly, something is missing in the
network interface code? (My system is using an NFS root
filesystem, so network is an important subsystem.)

>>> Is your system clocksource also correctly saved/restored, or if you go
>>> through a firmware in-between could it be changing the counter values
>>> and make Linux think that more time as elapsed than it really happened?
>>
>> Thanks for pointing this out, I was not aware I was supposed to save
>> and restore the tick counter on suspend/resume. (This is not an issue
>> in this specific situation, as the platform is NOT suspended.)
> 
> You don't have to save and restore the clocksource counter, although if
> you want proper time accounting to be done across suspend states, you
> would want to use a clocksource which is persistent across these suspend
> states.

The clocksource is a 27 MHz 32-bit tick counter. In other words,
the counter wraps around every 159 seconds. If Linux suspends
for several hours, how can it determine how much time went by?

Regards.

Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc

[Mason]

On 05/07/2016 17:28, Florian Fainelli wrote:

> nb8800.c does not currently show suspend/resume hooks implemented, are
> you positive that when you suspend, you properly tear down all HW, stop
> transmit queues, etc. and do the opposite upon resumption?

I am currently testing the error path for my suspend routine.
Firmware is, in fact, denying the suspend request, and immediately
returns control to Linux, without having powered anything down.

I expected not having to save any context in that situation.
Am I mistaken?

You mention "stop transmit queues". Can you say more about this?

> Is your system clocksource also correctly saved/restored, or if you go
> through a firmware in-between could it be changing the counter values
> and make Linux think that more time as elapsed than it really happened?

Thanks for pointing this out, I was not aware I was supposed to save
and restore the tick counter on suspend/resume. (This is not an issue
in this specific situation, as the platform is NOT suspended.)

However, your remark has brought some more confusion to my mind.
Linux is expecting time to stand still when it suspends?
What if the tick counter is in an always-on power domain, and other
processors depend on the counter? I can't just overwrite the reg
when Linux resumes...

Regards.

Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc

[Mason]

On 05/07/2016 15:33, Mason wrote:

> I was testing suspend/resume sequences where the suspend operation
> fails and returns without having suspended the platform.
> 
> # echo mem > /sys/power/state
> [   90.322264] PM: Syncing filesystems ... done.
> [   90.328758] Freezing user space processes ... (elapsed 0.001 seconds) done.
> [   90.337092] Double checking all user space processes after OOM killer 
> disable... (elapsed 0.000 seconds)
> [   90.346765] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) 
> done.
> [   90.355357] Suspending console(s) (use no_console_suspend to debug)
> [   90.364590] PM: suspend of devices complete after 2.068 msecs
> [   90.365554] PM: late suspend of devices complete after 0.954 msecs
> [   90.366223] PM: noirq suspend of devices complete after 0.662 msecs
> [   90.366227] Disabling non-boot CPUs ...
> [   90.379004] CPU1: shutdown
> [   90.412661] Enabling non-boot CPUs ...
> [   90.450385] CPU1 is up
> [   90.450979] PM: noirq resume of devices complete after 0.584 msecs
> [   90.451672] PM: early resume of devices complete after 0.667 msecs
> [   90.453149] nb8800 26000.ethernet eth0: Link is Down
> [   90.453264] PM: resume of devices complete after 1.583 msecs
> [   90.508180] Restarting tasks ... done.
> -sh: echo: write error: Input/output error
> [   93.860411] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow 
> control rx/tx
> 
> (The error message is expected, as my suspend routine returns -EIO
> on failure.)
> 
> I left the system to idle at the prompt; then 5 minutes later,
> the system printed the following trace.
> 
> [  400.718491] [ cut here ]
> [  400.723175] WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 
> inet_sock_destruct+0x1c4/0x1dc
> [  400.731582] Modules linked in:
> [  400.734689] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
> 4.7.0-rc6-00010-gd07031bdc433 #1
> [  400.742646] Hardware name: Sigma Tango DT
> [  400.746671] Backtrace: 
> [  400.749141] [] (dump_backtrace) from [] 
> (show_stack+0x18/0x1c)
> [  400.756747]  r7:6113 r6:c080ea84 r5: r4:c080ea84
> [  400.762454] [] (show_stack) from [] 
> (dump_stack+0x80/0x94)
> [  400.769722] [] (dump_stack) from [] (__warn+0xec/0x104)
> [  400.776717]  r7:0009 r6:c05e3fbc r5: r4:
> [  400.782417] [] (__warn) from [] 
> (warn_slowpath_null+0x28/0x30)
> [  400.790022]  r9:dfbdd4e0 r8:000a r7:c0801de8 r6:df6f9514 r5:df5df144 
> r4:df5df040
> [  400.797825] [] (warn_slowpath_null) from [] 
> (inet_sock_destruct+0x1c4/0x1dc)
> [  400.806661] [] (inet_sock_destruct) from [] 
> (__sk_destruct+0x28/0xe0)
> [  400.814878]  r7:c0801de8 r6:df6f9514 r5:df5df040 r4:df5df1ec
> [  400.820584] [] (__sk_destruct) from [] 
> (rcu_process_callbacks+0x488/0x59c)
> [  400.829237]  r5: r4:
> [  400.832836] [] (rcu_process_callbacks) from [] 
> (__do_softirq+0x138/0x264)
> [  400.841402]  r10:c08020a0 r9:4001 r8:0101 r7:c080 r6:c08020a4 
> r5:0009
> [  400.849285]  r4:
> [  400.851829] [] (__do_softirq) from [] 
> (irq_exit+0xc8/0x104)
> [  400.859172]  r10:c0801f10 r9:df402400 r8:0001 r7: r6:0013 
> r5:
> [  400.867053]  r4:c0735428
> [  400.869601] [] (irq_exit) from [] 
> (__handle_domain_irq+0x88/0xf4)
> [  400.877473] [] (__handle_domain_irq) from [] 
> (gic_handle_irq+0x50/0x94)
> [  400.885865]  r10:dfffcdc0 r9:e0803100 r8:e0802100 r7:c0801f10 r6:e080210c 
> r5:c080277c
> [  400.893747]  r4:c080eca0 r3:c0801f10
> [  400.897342] [] (gic_handle_irq) from [] 
> (__irq_svc+0x54/0x90)
> [  400.904861] Exception stack(0xc0801f10 to 0xc0801f58)
> [  400.909936] 1f00:   
> 826a c0117c80
> [  400.918156] 1f20: c080 c08024f8 c0802494 c081e2d6 c05b954c c07268c0 
> dfffcdc0 c0801f6c
> [  400.926376] 1f40: c0801f70 c0801f60 c01086b0 c01086b4 6013 
> [  400.933020]  r9:c07268c0 r8:c05b954c r7:c0801f44 r6: r5:6013 
> r4:c01086b4
> [  400.940826] [] (arch_cpu_idle) from [] 
> (default_idle_call+0x28/0x34)
> [  400.948960] [] (default_idle_call) from [] 
> (cpu_startup_entry+0x128/0x17c)
> [  400.957620] [] (cpu_startup_entry) from [] 
> (rest_init+0x8c/0x90)
> [  400.965400]  r7: r4:0002
> [  400.969005] [] (rest_init) from [] 
> (start_kernel+0x310/0x31c)
> [  400.976522]  r5:c081e4c0 r4:0001
> [  400.980121] [] (start_kernel) from [<8000807c>] (0x8000807c)
> [  400.986716] ---[ end trace f8deb50d1b3d3c7a ]---


NB: The warning shows up 310 seconds after the suspend attempt.

I rebooted, tried the same operation, and hit the same warning
still 310 seconds later:

# echo mem 

WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc

[Mason]

Hello,

I was testing suspend/resume sequences where the suspend operation
fails and returns without having suspended the platform.

# echo mem > /sys/power/state
[   90.322264] PM: Syncing filesystems ... done.
[   90.328758] Freezing user space processes ... (elapsed 0.001 seconds) done.
[   90.337092] Double checking all user space processes after OOM killer 
disable... (elapsed 0.000 seconds)
[   90.346765] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) 
done.
[   90.355357] Suspending console(s) (use no_console_suspend to debug)
[   90.364590] PM: suspend of devices complete after 2.068 msecs
[   90.365554] PM: late suspend of devices complete after 0.954 msecs
[   90.366223] PM: noirq suspend of devices complete after 0.662 msecs
[   90.366227] Disabling non-boot CPUs ...
[   90.379004] CPU1: shutdown
[   90.412661] Enabling non-boot CPUs ...
[   90.450385] CPU1 is up
[   90.450979] PM: noirq resume of devices complete after 0.584 msecs
[   90.451672] PM: early resume of devices complete after 0.667 msecs
[   90.453149] nb8800 26000.ethernet eth0: Link is Down
[   90.453264] PM: resume of devices complete after 1.583 msecs
[   90.508180] Restarting tasks ... done.
-sh: echo: write error: Input/output error
[   93.860411] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow 
control rx/tx

(The error message is expected, as my suspend routine returns -EIO
on failure.)

I left the system to idle at the prompt; then 5 minutes later,
the system printed the following trace.

[  400.718491] [ cut here ]
[  400.723175] WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 
inet_sock_destruct+0x1c4/0x1dc
[  400.731582] Modules linked in:
[  400.734689] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
4.7.0-rc6-00010-gd07031bdc433 #1
[  400.742646] Hardware name: Sigma Tango DT
[  400.746671] Backtrace: 
[  400.749141] [] (dump_backtrace) from [] 
(show_stack+0x18/0x1c)
[  400.756747]  r7:6113 r6:c080ea84 r5: r4:c080ea84
[  400.762454] [] (show_stack) from [] 
(dump_stack+0x80/0x94)
[  400.769722] [] (dump_stack) from [] (__warn+0xec/0x104)
[  400.776717]  r7:0009 r6:c05e3fbc r5: r4:
[  400.782417] [] (__warn) from [] 
(warn_slowpath_null+0x28/0x30)
[  400.790022]  r9:dfbdd4e0 r8:000a r7:c0801de8 r6:df6f9514 r5:df5df144 
r4:df5df040
[  400.797825] [] (warn_slowpath_null) from [] 
(inet_sock_destruct+0x1c4/0x1dc)
[  400.806661] [] (inet_sock_destruct) from [] 
(__sk_destruct+0x28/0xe0)
[  400.814878]  r7:c0801de8 r6:df6f9514 r5:df5df040 r4:df5df1ec
[  400.820584] [] (__sk_destruct) from [] 
(rcu_process_callbacks+0x488/0x59c)
[  400.829237]  r5: r4:
[  400.832836] [] (rcu_process_callbacks) from [] 
(__do_softirq+0x138/0x264)
[  400.841402]  r10:c08020a0 r9:4001 r8:0101 r7:c080 r6:c08020a4 
r5:0009
[  400.849285]  r4:
[  400.851829] [] (__do_softirq) from [] 
(irq_exit+0xc8/0x104)
[  400.859172]  r10:c0801f10 r9:df402400 r8:0001 r7: r6:0013 
r5:
[  400.867053]  r4:c0735428
[  400.869601] [] (irq_exit) from [] 
(__handle_domain_irq+0x88/0xf4)
[  400.877473] [] (__handle_domain_irq) from [] 
(gic_handle_irq+0x50/0x94)
[  400.885865]  r10:dfffcdc0 r9:e0803100 r8:e0802100 r7:c0801f10 r6:e080210c 
r5:c080277c
[  400.893747]  r4:c080eca0 r3:c0801f10
[  400.897342] [] (gic_handle_irq) from [] 
(__irq_svc+0x54/0x90)
[  400.904861] Exception stack(0xc0801f10 to 0xc0801f58)
[  400.909936] 1f00:   
826a c0117c80
[  400.918156] 1f20: c080 c08024f8 c0802494 c081e2d6 c05b954c c07268c0 
dfffcdc0 c0801f6c
[  400.926376] 1f40: c0801f70 c0801f60 c01086b0 c01086b4 6013 
[  400.933020]  r9:c07268c0 r8:c05b954c r7:c0801f44 r6: r5:6013 
r4:c01086b4
[  400.940826] [] (arch_cpu_idle) from [] 
(default_idle_call+0x28/0x34)
[  400.948960] [] (default_idle_call) from [] 
(cpu_startup_entry+0x128/0x17c)
[  400.957620] [] (cpu_startup_entry) from [] 
(rest_init+0x8c/0x90)
[  400.965400]  r7: r4:0002
[  400.969005] [] (rest_init) from [] 
(start_kernel+0x310/0x31c)
[  400.976522]  r5:c081e4c0 r4:0001
[  400.980121] [] (start_kernel) from [<8000807c>] (0x8000807c)
[  400.986716] ---[ end trace f8deb50d1b3d3c7a ]---


Did I implement something incorrectly?

Regards.

Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

[Jon Mason]

On Fri, Jul 1, 2016 at 5:46 AM, Arnd Bergmann <a...@arndb.de> wrote:
> On Thursday, June 30, 2016 6:59:13 PM CEST Jon Mason wrote:
>> +
>> +Required properties:
>> + - compatible: "brcm,bgmac-nsp"
>> + - reg:Address and length of the GMAC registers,
>> +   Address and length of the GMAC IDM registers
>> + - reg-names:  Names of the registers.  Must have both "gmac_base" and
>> +   "idm_base"
>> + - interrupts: Interrupt number
>> +
>
>
> "brcm,bgmac-nsp" sounds a bit too general. As I understand, this is a family
> of SoCs that might not all have the exact same implementation of this
> ethernet device, as we can see from the long lookup table in bgmac_probe().

The Broadcom iProc family of SoCs contains:
Northstar
Northstar Plus
Cygnus
Northstar 2
a few SoCs that are under development
and a number of ethernet switches (which might never be officially supported)

Each one of these SoCs could have a different revision of the gmac IP
block, but they should be uniform within each SoC (though there might
be a A0/B0 change necessary).  The Northstar Plus product family has a
number of different implementations, but the SoC is unchanged.  So, I
think this might be too specific, when we really need a general compat
string.

Broadcom has a history of sharing IP blocks amongst the different
divisions.  So, this driver might be used on other SoC families (as it
apparently has been done in the past, based on the code you
reference).  I do not know of any way to know what legacy, non-iProc
chips have used this IP block.  I can make this "brcm,iproc-bgmac",
and add "brcm,iproc-nsp-bgmac" as an alternative compatible string in
this file (which I believe you are suggesting), but there might be
non-iProc SoCs that use this driver.  Is this acceptable?

Thanks,
Jon

> Please document the specific product numbers here that are publically
> known already. Having the driver match just on "brcm,bgmac-nsp" as a fallback
> is fine, so you can document that one as required for all users.
>
> Arnd

Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

[Jon Mason]

On Thu, Jun 30, 2016 at 10:56 PM, Rob Herring <r...@kernel.org> wrote:
> On Thu, Jun 30, 2016 at 06:59:13PM -0400, Jon Mason wrote:
>> Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
>> ---
>>  .../devicetree/bindings/net/brcm,bgmac-nsp.txt | 24 
>> ++
>>  1 file changed, 24 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt 
>> b/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
>> new file mode 100644
>> index 000..022946c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
>> @@ -0,0 +1,24 @@
>> +Broadcom GMAC Ethernet Controller Device Tree Bindings
>> +-
>> +
>> +Required properties:
>> + - compatible:   "brcm,bgmac-nsp"
>
> Usually we do - order.

Thanks, I'll make the necessary changes and push out a v2.

>
>> + - reg:  Address and length of the GMAC registers,
>> + Address and length of the GMAC IDM registers
>> + - reg-names:Names of the registers.  Must have both "gmac_base" and
>> + "idm_base"
>> + - interrupts:   Interrupt number
>> +
>> +Optional properties:
>> +- mac-address:   See ethernet.txt file in the same directory
>> +
>> +Examples:
>> +
>> +gmac0: ethernet@18022000 {
>> + compatible = "brcm,bgmac-nsp";
>> + reg = <0x18022000 0x1000>,
>> +   <0x1811 0x1000>;
>> + reg-names = "gmac_base", "idm_base";
>> + interrupts = ;
>> + status = "disabled";
>> +};
>> --
>> 1.9.1
>>

[PATCH 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file

[Jon Mason]

Move the BCMA MDIO phy into a separate file, as it is very tightly
coupled with the BCMA bus.  This will help with the upcoming BCMA
removal from the bgmac driver.  Optimally, this should be moved into
phy drivers, but it is too tightly coupled with the bgmac driver to
effectively move it without more changes to the driver.

Note: the phy_reset was intentionally removed, as the mdio phy subsystem
automatically resets the phy if a reset function pointer is present.  In
addition to the moving of the driver, this reset function is added.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 drivers/net/ethernet/broadcom/Makefile  |   2 +-
 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c | 264 
 drivers/net/ethernet/broadcom/bgmac.c   | 246 +++---
 drivers/net/ethernet/broadcom/bgmac.h   |   3 +
 4 files changed, 298 insertions(+), 217 deletions(-)
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c

diff --git a/drivers/net/ethernet/broadcom/Makefile 
b/drivers/net/ethernet/broadcom/Makefile
index 00584d7..f559794 100644
--- a/drivers/net/ethernet/broadcom/Makefile
+++ b/drivers/net/ethernet/broadcom/Makefile
@@ -10,6 +10,6 @@ obj-$(CONFIG_CNIC) += cnic.o
 obj-$(CONFIG_BNX2X) += bnx2x/
 obj-$(CONFIG_SB1250_MAC) += sb1250-mac.o
 obj-$(CONFIG_TIGON3) += tg3.o
-obj-$(CONFIG_BGMAC) += bgmac.o
+obj-$(CONFIG_BGMAC) += bgmac.o bgmac-bcma-mdio.o
 obj-$(CONFIG_SYSTEMPORT) += bcmsysport.o
 obj-$(CONFIG_BNXT) += bnxt/
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c 
b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
new file mode 100644
index 000..1e65349
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
@@ -0,0 +1,264 @@
+/*
+ * Driver for (BCM4706)? GBit MAC core on BCMA bus.
+ *
+ * Copyright (C) 2012 Rafał Miłecki <zaj...@gmail.com>
+ *
+ * Licensed under the GNU/GPL. See COPYING for details.
+ */
+
+#define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include "bgmac.h"
+
+struct bcma_mdio {
+   struct bcma_device *core;
+   u8 phyaddr;
+};
+
+static bool bcma_mdio_wait_value(struct bcma_device *core, u16 reg, u32 mask,
+u32 value, int timeout)
+{
+   u32 val;
+   int i;
+
+   for (i = 0; i < timeout / 10; i++) {
+   val = bcma_read32(core, reg);
+   if ((val & mask) == value)
+   return true;
+   udelay(10);
+   }
+   dev_err(>dev, "Timeout waiting for reg 0x%X\n", reg);
+   return false;
+}
+
+/**
+ * PHY ops
+ **/
+
+static u16 bcma_mdio_phy_read(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg)
+{
+   struct bcma_device *core;
+   u16 phy_access_addr;
+   u16 phy_ctl_addr;
+   u32 tmp;
+
+   BUILD_BUG_ON(BGMAC_PA_DATA_MASK != BCMA_GMAC_CMN_PA_DATA_MASK);
+   BUILD_BUG_ON(BGMAC_PA_ADDR_MASK != BCMA_GMAC_CMN_PA_ADDR_MASK);
+   BUILD_BUG_ON(BGMAC_PA_ADDR_SHIFT != BCMA_GMAC_CMN_PA_ADDR_SHIFT);
+   BUILD_BUG_ON(BGMAC_PA_REG_MASK != BCMA_GMAC_CMN_PA_REG_MASK);
+   BUILD_BUG_ON(BGMAC_PA_REG_SHIFT != BCMA_GMAC_CMN_PA_REG_SHIFT);
+   BUILD_BUG_ON(BGMAC_PA_WRITE != BCMA_GMAC_CMN_PA_WRITE);
+   BUILD_BUG_ON(BGMAC_PA_START != BCMA_GMAC_CMN_PA_START);
+   BUILD_BUG_ON(BGMAC_PC_EPA_MASK != BCMA_GMAC_CMN_PC_EPA_MASK);
+   BUILD_BUG_ON(BGMAC_PC_MCT_MASK != BCMA_GMAC_CMN_PC_MCT_MASK);
+   BUILD_BUG_ON(BGMAC_PC_MCT_SHIFT != BCMA_GMAC_CMN_PC_MCT_SHIFT);
+   BUILD_BUG_ON(BGMAC_PC_MTE != BCMA_GMAC_CMN_PC_MTE);
+
+   if (bcma_mdio->core->id.id == BCMA_CORE_4706_MAC_GBIT) {
+   core = bcma_mdio->core->bus->drv_gmac_cmn.core;
+   phy_access_addr = BCMA_GMAC_CMN_PHY_ACCESS;
+   phy_ctl_addr = BCMA_GMAC_CMN_PHY_CTL;
+   } else {
+   core = bcma_mdio->core;
+   phy_access_addr = BGMAC_PHY_ACCESS;
+   phy_ctl_addr = BGMAC_PHY_CNTL;
+   }
+
+   tmp = bcma_read32(core, phy_ctl_addr);
+   tmp &= ~BGMAC_PC_EPA_MASK;
+   tmp |= phyaddr;
+   bcma_write32(core, phy_ctl_addr, tmp);
+
+   tmp = BGMAC_PA_START;
+   tmp |= phyaddr << BGMAC_PA_ADDR_SHIFT;
+   tmp |= reg << BGMAC_PA_REG_SHIFT;
+   bcma_write32(core, phy_access_addr, tmp);
+
+   if (!bcma_mdio_wait_value(core, phy_access_addr, BGMAC_PA_START, 0,
+ 1000)) {
+   dev_err(>dev, "Reading PHY %d register 0x%X failed\n",
+   phyaddr, reg);
+   return 0x;
+   }
+
+   return bcma_read32(core, phy_access_addr) & BGMAC_PA_DATA_MASK;
+}
+
+/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphywr */
+static int bcma_mdio_phy_write(struct bcma_mdio *bcma_mdio, u8 phya

[PATCH 5/7] net: ethernet: bgmac: Add platform device support

[Jon Mason]

The bcma portion of the driver has been split off into a bcma specific
driver.  This has been mirrored for the platform driver.  The last
references to the bcma core struct have been changed into a generic
function call.  These function calls are wrappers to either the original
bcma code or new platform functions that access the same areas via MMIO.
This necessitated adding function pointers for both platform and bcma to
hide which backend is being used from the generic bgmac code.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 drivers/net/ethernet/broadcom/Kconfig   |  23 +-
 drivers/net/ethernet/broadcom/Makefile  |   4 +-
 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c |   2 +
 drivers/net/ethernet/broadcom/bgmac-bcma.c  | 315 +++
 drivers/net/ethernet/broadcom/bgmac-platform.c  | 210 +++
 drivers/net/ethernet/broadcom/bgmac.c   | 329 
 drivers/net/ethernet/broadcom/bgmac.h   |  73 +-
 7 files changed, 671 insertions(+), 285 deletions(-)
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma.c
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-platform.c

diff --git a/drivers/net/ethernet/broadcom/Kconfig 
b/drivers/net/ethernet/broadcom/Kconfig
index d74a92e..bd8c80c 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -140,10 +140,18 @@ config BNX2X_SRIOV
  allows for virtual function acceleration in virtual environments.
 
 config BGMAC
-   tristate "BCMA bus GBit core support"
+   tristate
+   help
+ This enables the integrated ethernet controller support for many
+ Broadcom (mostly iProc) SoCs. An appropriate bus interface driver
+ needs to be enabled to select this.
+
+config BGMAC_BCMA
+   tristate "Broadcom iProc GBit BCMA support"
depends on BCMA && BCMA_HOST_SOC
depends on HAS_DMA
depends on BCM47XX || ARCH_BCM_5301X || COMPILE_TEST
+   select BGMAC
select PHYLIB
select FIXED_PHY
---help---
@@ -152,6 +160,19 @@ config BGMAC
  In case of using this driver on BCM4706 it's also requires to enable
  BCMA_DRIVER_GMAC_CMN to make it work.
 
+config BGMAC_PLATFORM
+   tristate "Broadcom iProc GBit platform support"
+   depends on HAS_DMA
+   depends on ARCH_BCM_IPROC || COMPILE_TEST
+   depends on OF
+   select BGMAC
+   select PHYLIB
+   select FIXED_PHY
+   default ARCH_BCM_IPROC
+   ---help---
+ Say Y here if you want to use the Broadcom iProc Gigabit Ethernet
+ controller through the generic platform interface
+
 config SYSTEMPORT
tristate "Broadcom SYSTEMPORT internal MAC support"
depends on OF
diff --git a/drivers/net/ethernet/broadcom/Makefile 
b/drivers/net/ethernet/broadcom/Makefile
index f559794..79f2372 100644
--- a/drivers/net/ethernet/broadcom/Makefile
+++ b/drivers/net/ethernet/broadcom/Makefile
@@ -10,6 +10,8 @@ obj-$(CONFIG_CNIC) += cnic.o
 obj-$(CONFIG_BNX2X) += bnx2x/
 obj-$(CONFIG_SB1250_MAC) += sb1250-mac.o
 obj-$(CONFIG_TIGON3) += tg3.o
-obj-$(CONFIG_BGMAC) += bgmac.o bgmac-bcma-mdio.o
+obj-$(CONFIG_BGMAC) += bgmac.o
+obj-$(CONFIG_BGMAC_BCMA) += bgmac-bcma.o bgmac-bcma-mdio.o
+obj-$(CONFIG_BGMAC_PLATFORM) += bgmac-platform.o
 obj-$(CONFIG_SYSTEMPORT) += bcmsysport.o
 obj-$(CONFIG_BNXT) += bnxt/
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c 
b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
index 1e65349..7c19c8e 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
@@ -245,6 +245,7 @@ err:
kfree(bcma_mdio);
return ERR_PTR(err);
 }
+EXPORT_SYMBOL_GPL(bcma_mdio_mii_register);
 
 void bcma_mdio_mii_unregister(struct mii_bus *mii_bus)
 {
@@ -259,6 +260,7 @@ void bcma_mdio_mii_unregister(struct mii_bus *mii_bus)
mdiobus_free(mii_bus);
kfree(bcma_mdio);
 }
+EXPORT_SYMBOL_GPL(bcma_mdio_mii_unregister);
 
 MODULE_AUTHOR("Rafał Miłecki");
 MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c 
b/drivers/net/ethernet/broadcom/bgmac-bcma.c
new file mode 100644
index 000..9a9745c4
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -0,0 +1,315 @@
+/*
+ * Driver for (BCM4706)? GBit MAC core on BCMA bus.
+ *
+ * Copyright (C) 2012 Rafał Miłecki <zaj...@gmail.com>
+ *
+ * Licensed under the GNU/GPL. See COPYING for details.
+ */
+
+#define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include "bgmac.h"
+
+static inline bool bgmac_is_bcm4707_family(struct bcma_device *core)
+{
+   switch (core->bus->chipinfo.id) {
+   case BCMA_CHIP_ID_BCM4707:
+   case BCMA_CHIP_ID_BCM47094:
+   

[PATCH 4/7] net: ethernet: bgmac: convert to feature flags

[Jon Mason]

The bgmac driver is using the bcma provides device ID and revision, as
well as the SoC ID and package, to determine which features are
necessary to enable, reset, etc in the driver.   In anticipation of
removing the bcma requirement for this driver, these must be changed to
not reference that struct.  In place of that, each "feature" has been
given a flag, and the flags are enabled for their respective device and
SoC.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac.c | 167 --
 drivers/net/ethernet/broadcom/bgmac.h |  21 -
 2 files changed, 140 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c 
b/drivers/net/ethernet/broadcom/bgmac.c
index 6c6bb18..b85e39a 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -109,7 +109,7 @@ static void bgmac_dma_tx_enable(struct bgmac *bgmac,
u32 ctl;
 
ctl = bgmac_read(bgmac, ring->mmio_base + BGMAC_DMA_TX_CTL);
-   if (bgmac->core->id.rev >= 4) {
+   if (bgmac->feature_flags & BGMAC_FEAT_TX_MASK_SETUP) {
ctl &= ~BGMAC_DMA_TX_BL_MASK;
ctl |= BGMAC_DMA_TX_BL_128 << BGMAC_DMA_TX_BL_SHIFT;
 
@@ -330,7 +330,7 @@ static void bgmac_dma_rx_enable(struct bgmac *bgmac,
u32 ctl;
 
ctl = bgmac_read(bgmac, ring->mmio_base + BGMAC_DMA_RX_CTL);
-   if (bgmac->core->id.rev >= 4) {
+   if (bgmac->feature_flags & BGMAC_FEAT_RX_MASK_SETUP) {
ctl &= ~BGMAC_DMA_RX_BL_MASK;
ctl |= BGMAC_DMA_RX_BL_128 << BGMAC_DMA_RX_BL_SHIFT;
 
@@ -768,14 +768,20 @@ static void bgmac_cmdcfg_maskset(struct bgmac *bgmac, u32 
mask, u32 set,
 {
u32 cmdcfg = bgmac_read(bgmac, BGMAC_CMDCFG);
u32 new_val = (cmdcfg & mask) | set;
+   u32 cmdcfg_sr;
 
-   bgmac_set(bgmac, BGMAC_CMDCFG, BGMAC_CMDCFG_SR(bgmac->core->id.rev));
+   if (bgmac->feature_flags & BGMAC_FEAT_CMDCFG_SR_REV4)
+   cmdcfg_sr = BGMAC_CMDCFG_SR_REV4;
+   else
+   cmdcfg_sr = BGMAC_CMDCFG_SR_REV0;
+
+   bgmac_set(bgmac, BGMAC_CMDCFG, cmdcfg_sr);
udelay(2);
 
if (new_val != cmdcfg || force)
bgmac_write(bgmac, BGMAC_CMDCFG, new_val);
 
-   bgmac_mask(bgmac, BGMAC_CMDCFG, ~BGMAC_CMDCFG_SR(bgmac->core->id.rev));
+   bgmac_mask(bgmac, BGMAC_CMDCFG, ~cmdcfg_sr);
udelay(2);
 }
 
@@ -804,7 +810,7 @@ static void bgmac_chip_stats_update(struct bgmac *bgmac)
 {
int i;
 
-   if (bgmac->core->id.id != BCMA_CORE_4706_MAC_GBIT) {
+   if (!(bgmac->feature_flags & BGMAC_FEAT_NO_CLR_MIB)) {
for (i = 0; i < BGMAC_NUM_MIB_TX_REGS; i++)
bgmac->mib_tx_regs[i] =
bgmac_read(bgmac,
@@ -823,7 +829,7 @@ static void bgmac_clear_mib(struct bgmac *bgmac)
 {
int i;
 
-   if (bgmac->core->id.id == BCMA_CORE_4706_MAC_GBIT)
+   if (bgmac->feature_flags & BGMAC_FEAT_NO_CLR_MIB)
return;
 
bgmac_set(bgmac, BGMAC_DEV_CTL, BGMAC_DC_MROR);
@@ -866,9 +872,8 @@ static void bgmac_mac_speed(struct bgmac *bgmac)
 static void bgmac_miiconfig(struct bgmac *bgmac)
 {
struct bcma_device *core = bgmac->core;
-   u8 imode;
 
-   if (bgmac_is_bcm4707_family(bgmac)) {
+   if (bgmac->feature_flags & BGMAC_FEAT_FORCE_SPEED_2500) {
bcma_awrite32(core, BCMA_IOCTL,
  bcma_aread32(core, BCMA_IOCTL) | 0x40 |
  BGMAC_BCMA_IOCTL_SW_CLKEN);
@@ -876,6 +881,8 @@ static void bgmac_miiconfig(struct bgmac *bgmac)
bgmac->mac_duplex = DUPLEX_FULL;
bgmac_mac_speed(bgmac);
} else {
+   u8 imode;
+
imode = (bgmac_read(bgmac, BGMAC_DEV_STATUS) &
BGMAC_DS_MM_MASK) >> BGMAC_DS_MM_SHIFT;
if (imode == 0 || imode == 1) {
@@ -890,9 +897,7 @@ static void bgmac_miiconfig(struct bgmac *bgmac)
 static void bgmac_chip_reset(struct bgmac *bgmac)
 {
struct bcma_device *core = bgmac->core;
-   struct bcma_bus *bus = core->bus;
-   struct bcma_chipinfo *ci = >chipinfo;
-   u32 flags;
+   u32 cmdcfg_sr;
u32 iost;
int i;
 
@@ -915,15 +920,12 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
}
 
iost = bcma_aread32(core, BCMA_IOST);
-   if ((ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg == BCMA_PKG_ID_BCM47186) 
||
-   (ci->id == BCMA_CHIP_ID_BCM4749 && ci->pkg == 10) ||
-   (ci->id == BCMA_CHIP_ID_BCM53572 && ci->pkg == 
BCMA_PKG_ID_BCM47188))
+   if (bgmac->feature_flags & BGMAC_FEAT_IOST_ATTACHED)
iost &= ~BGMAC_BCMA_IOST_ATTACHED;
 
  

[PATCH 1/7] net: ethernet: bgmac: change bgmac_* prints to dev_* prints

[Jon Mason]

The bgmac_* print wrappers call dev_* prints with the dev pointer from
the bcma core.  In anticipation of removing the bcma requirement for
this driver, these must be changed to not reference that struct.  So,
simply change all of the bgmac_* prints to their dev_* counterparts.  In
some cases netdev_* prints are more appropriate, so change those as
well.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac.c | 103 +-
 drivers/net/ethernet/broadcom/bgmac.h |  14 +
 2 files changed, 55 insertions(+), 62 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c 
b/drivers/net/ethernet/broadcom/bgmac.c
index e6e74ca..37b3b68 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -50,7 +50,7 @@ static bool bgmac_wait_value(struct bcma_device *core, u16 
reg, u32 mask,
return true;
udelay(10);
}
-   pr_err("Timeout waiting for reg 0x%X\n", reg);
+   dev_err(>dev, "Timeout waiting for reg 0x%X\n", reg);
return false;
 }
 
@@ -84,8 +84,8 @@ static void bgmac_dma_tx_reset(struct bgmac *bgmac, struct 
bgmac_dma_ring *ring)
udelay(10);
}
if (i)
-   bgmac_err(bgmac, "Timeout suspending DMA TX ring 0x%X 
(BGMAC_DMA_TX_STAT: 0x%08X)\n",
- ring->mmio_base, val);
+   dev_err(bgmac->dev, "Timeout suspending DMA TX ring 0x%X 
(BGMAC_DMA_TX_STAT: 0x%08X)\n",
+   ring->mmio_base, val);
 
/* Remove SUSPEND bit */
bgmac_write(bgmac, ring->mmio_base + BGMAC_DMA_TX_CTL, 0);
@@ -93,13 +93,13 @@ static void bgmac_dma_tx_reset(struct bgmac *bgmac, struct 
bgmac_dma_ring *ring)
  ring->mmio_base + BGMAC_DMA_TX_STATUS,
  BGMAC_DMA_TX_STAT, BGMAC_DMA_TX_STAT_DISABLED,
  1)) {
-   bgmac_warn(bgmac, "DMA TX ring 0x%X wasn't disabled on time, 
waiting additional 300us\n",
-  ring->mmio_base);
+   dev_warn(bgmac->dev, "DMA TX ring 0x%X wasn't disabled on time, 
waiting additional 300us\n",
+ring->mmio_base);
udelay(300);
val = bgmac_read(bgmac, ring->mmio_base + BGMAC_DMA_TX_STATUS);
if ((val & BGMAC_DMA_TX_STAT) != BGMAC_DMA_TX_STAT_DISABLED)
-   bgmac_err(bgmac, "Reset of DMA TX ring 0x%X failed\n",
- ring->mmio_base);
+   dev_err(bgmac->dev, "Reset of DMA TX ring 0x%X 
failed\n",
+   ring->mmio_base);
}
 }
 
@@ -161,7 +161,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
int i;
 
if (skb->len > BGMAC_DESC_CTL1_LEN) {
-   bgmac_err(bgmac, "Too long skb (%d)\n", skb->len);
+   netdev_err(bgmac->net_dev, "Too long skb (%d)\n", skb->len);
goto err_drop;
}
 
@@ -174,7 +174,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 * even when ring->end overflows
 */
if (ring->end - ring->start + nr_frags + 1 >= BGMAC_TX_RING_SLOTS) {
-   bgmac_err(bgmac, "TX ring is full, queue should be stopped!\n");
+   netdev_err(bgmac->net_dev, "TX ring is full, queue should be 
stopped!\n");
netif_stop_queue(net_dev);
return NETDEV_TX_BUSY;
}
@@ -241,8 +241,8 @@ err_dma:
}
 
 err_dma_head:
-   bgmac_err(bgmac, "Mapping error of skb on ring 0x%X\n",
- ring->mmio_base);
+   netdev_err(bgmac->net_dev, "Mapping error of skb on ring 0x%X\n",
+  ring->mmio_base);
 
 err_drop:
dev_kfree_skb(skb);
@@ -320,8 +320,8 @@ static void bgmac_dma_rx_reset(struct bgmac *bgmac, struct 
bgmac_dma_ring *ring)
  ring->mmio_base + BGMAC_DMA_RX_STATUS,
  BGMAC_DMA_RX_STAT, BGMAC_DMA_RX_STAT_DISABLED,
  1))
-   bgmac_err(bgmac, "Reset of ring 0x%X RX failed\n",
- ring->mmio_base);
+   dev_err(bgmac->dev, "Reset of ring 0x%X RX failed\n",
+   ring->mmio_base);
 }
 
 static void bgmac_dma_rx_enable(struct bgmac *bgmac,
@@ -370,7 +370,7 @@ static int bgmac_dma_rx_skb_for_slot(struct bgmac *bgmac,
dma_addr = dma_map_single(dma_dev, buf + BGMAC_RX_BUF_OFFSET,
  BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
if (dma_mapping_error(dma_dev, dma_addr)) {
-   bgmac_err(bgmac, "DMA mapping error

[PATCH 7/7] ARM: dts: NSP: Add bgmac entries

[Jon Mason]

Add device tree entries for the ethernet devices present on the
Broadcom Northstar Plus SoCs

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 arch/arm/boot/dts/bcm-nsp.dtsi   | 18 ++
 arch/arm/boot/dts/bcm958625k.dts |  8 
 2 files changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
index def9e78..8f4343b 100644
--- a/arch/arm/boot/dts/bcm-nsp.dtsi
+++ b/arch/arm/boot/dts/bcm-nsp.dtsi
@@ -192,6 +192,24 @@
status = "disabled";
};
 
+   gmac0: ethernet@22000 {
+   compatible = "brcm,bgmac-nsp";
+   reg = <0x022000 0x1000>,
+ <0x11 0x1000>;
+   reg-names = "gmac_base", "idm_base";
+   interrupts = ;
+   status = "disabled";
+   };
+
+   gmac1: ethernet@23000 {
+   compatible = "brcm,bgmac-nsp";
+   reg = <0x023000 0x1000>,
+ <0x111000 0x1000>;
+   reg-names = "gmac_base", "idm_base";
+   interrupts = ;
+   status = "disabled";
+   };
+
nand: nand@26000 {
compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1";
reg = <0x026000 0x600>,
diff --git a/arch/arm/boot/dts/bcm958625k.dts b/arch/arm/boot/dts/bcm958625k.dts
index e298450..d16ab53 100644
--- a/arch/arm/boot/dts/bcm958625k.dts
+++ b/arch/arm/boot/dts/bcm958625k.dts
@@ -56,6 +56,14 @@
status = "okay";
 };
 
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
  {
status = "okay";
 };
-- 
1.9.1

[PATCH 0/7] net: ethernet: bgmac: Add platform device support

[Jon Mason]

Well, no compilained too loudly at the RFC version of this patch series
(see https://lkml.org/lkml/2016/6/28/863).  So, I'm officially sending
this out for inclusion.  All comments from the RFC were addressed in
this version.

This patch series adds support for other, non-bcma iProc SoC's to the
bgmac driver.  This series only adds NSP support, but we are interested
in adding support for the Cygnus and NS2 families (with more possible
down the road).

To support non-bcma enabled SoCs, we need to add the standard device
tree "platform device" support.  Unfortunately, this driver is very
tighly coupled with the bcma bus and much unwinding is needed.  I tried
to break this up into a number of patches to make it more obvious what
was being done to add platform device support.  I was able to verify
that the bcma code still works using a 53012K board (NS SoC), and that
the platform code works using a 58625K board (NSP SoC).

Thanks,
Jon

Jon Mason (7):
  net: ethernet: bgmac: change bgmac_* prints to dev_* prints
  net: ethernet: bgmac: add dma_dev pointer
  net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file
  net: ethernet: bgmac: convert to feature flags
  net: ethernet: bgmac: Add platform device support
  dt-bindings: net: bgmac: add bindings documentation for bgmac
  ARM: dts: NSP: Add bgmac entries

 .../devicetree/bindings/net/brcm,bgmac-nsp.txt |  24 +
 arch/arm/boot/dts/bcm-nsp.dtsi |  18 +
 arch/arm/boot/dts/bcm958625k.dts   |   8 +
 drivers/net/ethernet/broadcom/Kconfig  |  23 +-
 drivers/net/ethernet/broadcom/Makefile |   2 +
 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c| 266 +
 drivers/net/ethernet/broadcom/bgmac-bcma.c | 315 ++
 drivers/net/ethernet/broadcom/bgmac-platform.c | 210 +++
 drivers/net/ethernet/broadcom/bgmac.c  | 658 +
 drivers/net/ethernet/broadcom/bgmac.h  | 112 +++-
 10 files changed, 1120 insertions(+), 516 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma.c
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-platform.c

-- 
1.9.1

[PATCH 2/7] net: ethernet: bgmac: add dma_dev pointer

[Jon Mason]

The dma buffer allocation, etc references a dma_dev device pointer from
the bcma core.  In anticipation of removing the bcma requirement for
this driver, these must be changed to not reference that struct.  Add a
dma_dev device pointer to the bgmac stuct and reference that instead.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac.c | 17 +
 drivers/net/ethernet/broadcom/bgmac.h |  1 +
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c 
b/drivers/net/ethernet/broadcom/bgmac.c
index 37b3b68..3614bd8 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -152,7 +152,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
struct bgmac_dma_ring *ring,
struct sk_buff *skb)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct net_device *net_dev = bgmac->net_dev;
int index = ring->end % BGMAC_TX_RING_SLOTS;
struct bgmac_slot_info *slot = >slots[index];
@@ -254,7 +254,7 @@ err_drop:
 /* Free transmitted packets */
 static void bgmac_dma_tx_free(struct bgmac *bgmac, struct bgmac_dma_ring *ring)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
int empty_slot;
bool freed = false;
unsigned bytes_compl = 0, pkts_compl = 0;
@@ -351,7 +351,7 @@ static void bgmac_dma_rx_enable(struct bgmac *bgmac,
 static int bgmac_dma_rx_skb_for_slot(struct bgmac *bgmac,
 struct bgmac_slot_info *slot)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
dma_addr_t dma_addr;
struct bgmac_rx_header *rx;
void *buf;
@@ -440,7 +440,7 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct 
bgmac_dma_ring *ring,
end_slot /= sizeof(struct bgmac_dma_desc);
 
while (ring->start != end_slot) {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct bgmac_slot_info *slot = >slots[ring->start];
struct bgmac_rx_header *rx = slot->buf + BGMAC_RX_BUF_OFFSET;
struct sk_buff *skb;
@@ -543,7 +543,7 @@ static bool bgmac_dma_unaligned(struct bgmac *bgmac,
 static void bgmac_dma_tx_ring_free(struct bgmac *bgmac,
   struct bgmac_dma_ring *ring)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct bgmac_dma_desc *dma_desc = ring->cpu_base;
struct bgmac_slot_info *slot;
int i;
@@ -569,7 +569,7 @@ static void bgmac_dma_tx_ring_free(struct bgmac *bgmac,
 static void bgmac_dma_rx_ring_free(struct bgmac *bgmac,
   struct bgmac_dma_ring *ring)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct bgmac_slot_info *slot;
int i;
 
@@ -590,7 +590,7 @@ static void bgmac_dma_ring_desc_free(struct bgmac *bgmac,
 struct bgmac_dma_ring *ring,
 int num_slots)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
int size;
 
if (!ring->cpu_base)
@@ -628,7 +628,7 @@ static void bgmac_dma_free(struct bgmac *bgmac)
 
 static int bgmac_dma_alloc(struct bgmac *bgmac)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct bgmac_dma_ring *ring;
static const u16 ring_base[] = { BGMAC_DMA_BASE0, BGMAC_DMA_BASE1,
 BGMAC_DMA_BASE2, BGMAC_DMA_BASE3, };
@@ -1701,6 +1701,7 @@ static int bgmac_probe(struct bcma_device *core)
net_dev->ethtool_ops = _ethtool_ops;
bgmac = netdev_priv(net_dev);
bgmac->dev = >dev;
+   bgmac->dma_dev = core->dma_dev;
bgmac->net_dev = net_dev;
bgmac->core = core;
bcma_set_drvdata(core, bgmac);
diff --git a/drivers/net/ethernet/broadcom/bgmac.h 
b/drivers/net/ethernet/broadcom/bgmac.h
index abb9dd8..fd20018 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -429,6 +429,7 @@ struct bgmac {
struct bcma_device *cmn; /* Reference to CMN core for BCM4706 */
 
struct device *dev;
+   struct device *dma_dev;
struct net_device *net_dev;
struct napi_struct napi;
struct mii_bus *mii_bus;
-- 
1.9.1

[PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

[Jon Mason]

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 .../devicetree/bindings/net/brcm,bgmac-nsp.txt | 24 ++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt

diff --git a/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt 
b/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
new file mode 100644
index 000..022946c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
@@ -0,0 +1,24 @@
+Broadcom GMAC Ethernet Controller Device Tree Bindings
+-
+
+Required properties:
+ - compatible: "brcm,bgmac-nsp"
+ - reg:Address and length of the GMAC registers,
+   Address and length of the GMAC IDM registers
+ - reg-names:  Names of the registers.  Must have both "gmac_base" and
+   "idm_base"
+ - interrupts: Interrupt number
+
+Optional properties:
+- mac-address: See ethernet.txt file in the same directory
+
+Examples:
+
+gmac0: ethernet@18022000 {
+   compatible = "brcm,bgmac-nsp";
+   reg = <0x18022000 0x1000>,
+ <0x1811 0x1000>;
+   reg-names = "gmac_base", "idm_base";
+   interrupts = ;
+   status = "disabled";
+};
-- 
1.9.1

Re: [RFC 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

[Jon Mason]

On Thu, Jun 30, 2016 at 2:06 PM, Ray Jui <ray@broadcom.com> wrote:
> Hi Jon,
>
> On 6/28/2016 12:34 PM, Jon Mason wrote:
>>
>> Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
>> ---
>>  .../devicetree/bindings/net/brcm,bgmac-enet.txt | 21
>> +
>>  1 file changed, 21 insertions(+)
>>  create mode 100644
>> Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
>> b/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
>> new file mode 100644
>> index 000..efd36d5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
>> @@ -0,0 +1,21 @@
>> +Broadcom GMAC Ethernet Controller Device Tree Bindings
>> +-
>> +
>> +Required properties:
>> + - compatible: "brcm,bgmac-enet"
>> + - reg:Address and length of the GMAC registers,
>> +   Address and length of the GMAC IDM registers
>
>
> As we know there will be additional optional register banks required for
> some of the other SoCs that the current driver has not yet supported. In my
> opinion, we should consider to make "reg-names" a mandatory property now and
> map the register blocks based on names.
>
> I think this will help to make our life easier in the future when new
> optional SoC specific register blocks are added, such that we can map the
> register blocks based on names instead of indices, which will change and be
> different among different SoCs and will require much more complex logic in
> the driver to deal with.

I don't have any objection to this.  I'll tweak the patches to do it by name.

>
>> + - interrupts: Interrupt number
>> +
>> +Optional properties:
>> +- mac-address: mac address to be assigned to the device
>> +
>> +Examples:
>> +
>> +gmac0: enet@18022000 {
>> +   compatible = "brcm,bgmac-enet";
>> +   reg = <0x18022000 0x1000>,
>> + <0x1811 0x1000>;
>> +   interrupts = ;
>> +   status = "disabled";
>> +};
>>
>
> Btw, I think Rob Herring should be included in the review for device tree
> binding document changes.

Thanks, I'll add him and the other DT maintainers when I send this out
as a "PATCH" shortly.

Thanks,
Jon

>
> Thanks,
>
> Ray

Re: [RFC 5/7] net: ethernet: bgmac: Add platform device support

[Jon Mason]

On Thu, Jun 30, 2016 at 1:58 PM, Ray Jui <ray@broadcom.com> wrote:
> Hi Jon,
>
>
> On 6/28/2016 12:34 PM, Jon Mason wrote:
>>
>> The bcma portion of the driver has been split off into a bcma specific
>> driver.  This has been mirrored for the platform driver.  The last
>> references to the bcma core struct have been changed into a generic
>> function call.  These function calls are wrappers to either the original
>> bcma code or new platform functions that access the same areas via MMIO.
>> This necessitated adding function pointers for both platform and bcma to
>> hide which backend is being used from the generic bgmac code.
>>
>> Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
>> ---
>>  drivers/net/ethernet/broadcom/Kconfig  |  23 +-
>>  drivers/net/ethernet/broadcom/Makefile |   4 +-
>>  drivers/net/ethernet/broadcom/bgmac-bcma.c | 315
>> 
>>  drivers/net/ethernet/broadcom/bgmac-platform.c | 208 
>>  drivers/net/ethernet/broadcom/bgmac.c  | 327
>> -
>>  drivers/net/ethernet/broadcom/bgmac.h  |  73 +-
>>  6 files changed, 666 insertions(+), 284 deletions(-)
>>  create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma.c
>>  create mode 100644 drivers/net/ethernet/broadcom/bgmac-platform.c
>>
>> diff --git a/drivers/net/ethernet/broadcom/Kconfig
>> b/drivers/net/ethernet/broadcom/Kconfig
>> index d74a92e..bd8c80c 100644
>> --- a/drivers/net/ethernet/broadcom/Kconfig
>> +++ b/drivers/net/ethernet/broadcom/Kconfig
>> @@ -140,10 +140,18 @@ config BNX2X_SRIOV
>>   allows for virtual function acceleration in virtual
>> environments.
>>
>>  config BGMAC
>> -   tristate "BCMA bus GBit core support"
>> +   tristate
>> +   help
>> + This enables the integrated ethernet controller support for many
>> + Broadcom (mostly iProc) SoCs. An appropriate bus interface
>> driver
>> + needs to be enabled to select this.
>> +
>> +config BGMAC_BCMA
>> +   tristate "Broadcom iProc GBit BCMA support"
>> depends on BCMA && BCMA_HOST_SOC
>> depends on HAS_DMA
>> depends on BCM47XX || ARCH_BCM_5301X || COMPILE_TEST
>> +   select BGMAC
>> select PHYLIB
>> select FIXED_PHY
>> ---help---
>> @@ -152,6 +160,19 @@ config BGMAC
>>   In case of using this driver on BCM4706 it's also requires to
>> enable
>>   BCMA_DRIVER_GMAC_CMN to make it work.
>>
>> +config BGMAC_PLATFORM
>> +   tristate "Broadcom iProc GBit platform support"
>> +   depends on HAS_DMA
>> +   depends on ARCH_BCM_IPROC || COMPILE_TEST
>> +   depends on OF
>> +   select BGMAC
>> +   select PHYLIB
>> +   select FIXED_PHY
>> +   default ARCH_BCM_IPROC
>> +   ---help---
>> + Say Y here if you want to use the Broadcom iProc Gigabit
>> Ethernet
>> + controller through the generic platform interface
>> +
>>  config SYSTEMPORT
>> tristate "Broadcom SYSTEMPORT internal MAC support"
>> depends on OF
>> diff --git a/drivers/net/ethernet/broadcom/Makefile
>> b/drivers/net/ethernet/broadcom/Makefile
>> index f559794..79f2372 100644
>> --- a/drivers/net/ethernet/broadcom/Makefile
>> +++ b/drivers/net/ethernet/broadcom/Makefile
>> @@ -10,6 +10,8 @@ obj-$(CONFIG_CNIC) += cnic.o
>>  obj-$(CONFIG_BNX2X) += bnx2x/
>>  obj-$(CONFIG_SB1250_MAC) += sb1250-mac.o
>>  obj-$(CONFIG_TIGON3) += tg3.o
>> -obj-$(CONFIG_BGMAC) += bgmac.o bgmac-bcma-mdio.o
>> +obj-$(CONFIG_BGMAC) += bgmac.o
>> +obj-$(CONFIG_BGMAC_BCMA) += bgmac-bcma.o bgmac-bcma-mdio.o
>> +obj-$(CONFIG_BGMAC_PLATFORM) += bgmac-platform.o
>>  obj-$(CONFIG_SYSTEMPORT) += bcmsysport.o
>>  obj-$(CONFIG_BNXT) += bnxt/
>> diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c
>> b/drivers/net/ethernet/broadcom/bgmac-bcma.c
>> new file mode 100644
>> index 000..9a9745c4
>> --- /dev/null
>> +++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
>> @@ -0,0 +1,315 @@
>> +/*
>> + * Driver for (BCM4706)? GBit MAC core on BCMA bus.
>> + *
>> + * Copyright (C) 2012 Rafał Miłecki <zaj...@gmail.com>
>> + *
>> + * Licensed under the GNU/GPL. See COPYING for details.
>> + */
>> +
>> +#define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
>> +
>> +#include 
&g

Re: [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file

[Jon Mason]

On Wed, Jun 29, 2016 at 4:15 PM, Andrew Lunn <and...@lunn.ch> wrote:
> On Wed, Jun 29, 2016 at 04:08:20PM -0400, Jon Mason wrote:
>> On Wed, Jun 29, 2016 at 2:46 PM, Andrew Lunn <and...@lunn.ch> wrote:
>> > On Wed, Jun 29, 2016 at 11:35:28AM -0700, Florian Fainelli wrote:
>> >> On 06/29/2016 07:13 AM, Andrew Lunn wrote:
>> >> > Hi Jon
>> >> >
>> >> > I know you are just refactoring code, but at some point it would be
>> >> > good to take a closer look at this MDIO bus driver.
>> >
>> >> And, to re-iterate all of your points are valid, but this is premature
>> >
>> > We agree then :-)
>> >
>> >Andrew
>>
>> I also agree with all of your points, but hope this is not something
>> that would prevent this patch series from being acceptable.
>
> No, it is acceptable as is.
>
> However, it would be nice the clean up the mess, especially if you are
> planning on using this code with new platforms, not just legacy stuff
> which is bit rotting.

I don't believe we'll be using the BCMA infrastructure going forward.
We'll be doing our own MDIO Phys, adding them to the device tree, and
referencing them through the DT/OF method.  This is the current plan
for NSP, NS2, Cygnus, and other upcoming platforms.  If someone can
find an elegant way of splitting off the Phy code from the BCMA core,
then this can be better than it is now.  But, it will take someone
with docs, time, and hardware to give this code the audit and cleanup
it really needs.  BTW, my last comment isn't intended as a slight
against the authors of this code.  It is unfortunate that they were
put in a position of having to reverse engineer drivers for this HW.

Thanks,
Jon

>
>   Andrew

Re: [RFC 1/7] net: ethernet: bgmac: change bgmac_* prints to dev_* prints

[Jon Mason]

On Tue, Jun 28, 2016 at 3:43 PM, Joe Perches <j...@perches.com> wrote:
> On Tue, 2016-06-28 at 15:34 -0400, Jon Mason wrote:
>> The bgmac_* print wrappers call dev_* prints with the dev pointer from
>> the bcma core.  In anticipation of removing the bcma requirement for
>> this driver, these must be changed to not reference that struct.  So,
>> simply change all of the bgmac_* prints to their dev_* counterparts.  In
>> some cases netdev_* prints are more appropriate, so change those as
>> well.
>
> Thanks, but...
>
>> diff --git a/drivers/net/ethernet/broadcom/bgmac.c 
>> b/drivers/net/ethernet/broadcom/bgmac.c
> []
>> @@ -1515,7 +1516,7 @@ static void bgmac_get_drvinfo(struct net_device 
>> *net_dev,
>> struct ethtool_drvinfo *info)
>>  {
>>   strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
>> - strlcpy(info->bus_info, "BCMA", sizeof(info->bus_info));
>> + strlcpy(info->bus_info, "AXI", sizeof(info->bus_info));
>>  }
>
> Unrelated change?

This is a needed change, but unrelated to this patch.  I'll move it to
the 5/7 patch, where it more logically fits.

Thanks,
Jon

>

Re: [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file

[Jon Mason]

On Wed, Jun 29, 2016 at 2:46 PM, Andrew Lunn  wrote:
> On Wed, Jun 29, 2016 at 11:35:28AM -0700, Florian Fainelli wrote:
>> On 06/29/2016 07:13 AM, Andrew Lunn wrote:
>> > Hi Jon
>> >
>> > I know you are just refactoring code, but at some point it would be
>> > good to take a closer look at this MDIO bus driver.
>
>> And, to re-iterate all of your points are valid, but this is premature
>
> We agree then :-)
>
>Andrew

I also agree with all of your points, but hope this is not something
that would prevent this patch series from being acceptable.  It could
be quite an effort to try and properly document the code and what it
is doing, get HW to verify the assumptions from the documentation (as
the MIPS HW was done by a different group, no hardware is available,
etc), relocate the code to the drivers/net/phy and add a standard
interface.  From your original comment, I believe that you agree that
this additional effort is not necessary at this point in time.

Thanks,
Jon

[RFC 1/7] net: ethernet: bgmac: change bgmac_* prints to dev_* prints

[Jon Mason]

The bgmac_* print wrappers call dev_* prints with the dev pointer from
the bcma core.  In anticipation of removing the bcma requirement for
this driver, these must be changed to not reference that struct.  So,
simply change all of the bgmac_* prints to their dev_* counterparts.  In
some cases netdev_* prints are more appropriate, so change those as
well.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac.c | 105 +-
 drivers/net/ethernet/broadcom/bgmac.h |  14 +
 2 files changed, 56 insertions(+), 63 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c 
b/drivers/net/ethernet/broadcom/bgmac.c
index e6e74ca..2ab0887 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -50,7 +50,7 @@ static bool bgmac_wait_value(struct bcma_device *core, u16 
reg, u32 mask,
return true;
udelay(10);
}
-   pr_err("Timeout waiting for reg 0x%X\n", reg);
+   dev_err(>dev, "Timeout waiting for reg 0x%X\n", reg);
return false;
 }
 
@@ -84,8 +84,8 @@ static void bgmac_dma_tx_reset(struct bgmac *bgmac, struct 
bgmac_dma_ring *ring)
udelay(10);
}
if (i)
-   bgmac_err(bgmac, "Timeout suspending DMA TX ring 0x%X 
(BGMAC_DMA_TX_STAT: 0x%08X)\n",
- ring->mmio_base, val);
+   dev_err(bgmac->dev, "Timeout suspending DMA TX ring 0x%X 
(BGMAC_DMA_TX_STAT: 0x%08X)\n",
+   ring->mmio_base, val);
 
/* Remove SUSPEND bit */
bgmac_write(bgmac, ring->mmio_base + BGMAC_DMA_TX_CTL, 0);
@@ -93,13 +93,13 @@ static void bgmac_dma_tx_reset(struct bgmac *bgmac, struct 
bgmac_dma_ring *ring)
  ring->mmio_base + BGMAC_DMA_TX_STATUS,
  BGMAC_DMA_TX_STAT, BGMAC_DMA_TX_STAT_DISABLED,
  1)) {
-   bgmac_warn(bgmac, "DMA TX ring 0x%X wasn't disabled on time, 
waiting additional 300us\n",
-  ring->mmio_base);
+   dev_warn(bgmac->dev, "DMA TX ring 0x%X wasn't disabled on time, 
waiting additional 300us\n",
+ring->mmio_base);
udelay(300);
val = bgmac_read(bgmac, ring->mmio_base + BGMAC_DMA_TX_STATUS);
if ((val & BGMAC_DMA_TX_STAT) != BGMAC_DMA_TX_STAT_DISABLED)
-   bgmac_err(bgmac, "Reset of DMA TX ring 0x%X failed\n",
- ring->mmio_base);
+   dev_err(bgmac->dev, "Reset of DMA TX ring 0x%X 
failed\n",
+   ring->mmio_base);
}
 }
 
@@ -161,7 +161,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
int i;
 
if (skb->len > BGMAC_DESC_CTL1_LEN) {
-   bgmac_err(bgmac, "Too long skb (%d)\n", skb->len);
+   netdev_err(bgmac->net_dev, "Too long skb (%d)\n", skb->len);
goto err_drop;
}
 
@@ -174,7 +174,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 * even when ring->end overflows
 */
if (ring->end - ring->start + nr_frags + 1 >= BGMAC_TX_RING_SLOTS) {
-   bgmac_err(bgmac, "TX ring is full, queue should be stopped!\n");
+   netdev_err(bgmac->net_dev, "TX ring is full, queue should be 
stopped!\n");
netif_stop_queue(net_dev);
return NETDEV_TX_BUSY;
}
@@ -241,8 +241,8 @@ err_dma:
}
 
 err_dma_head:
-   bgmac_err(bgmac, "Mapping error of skb on ring 0x%X\n",
- ring->mmio_base);
+   netdev_err(bgmac->net_dev, "Mapping error of skb on ring 0x%X\n",
+  ring->mmio_base);
 
 err_drop:
dev_kfree_skb(skb);
@@ -320,8 +320,8 @@ static void bgmac_dma_rx_reset(struct bgmac *bgmac, struct 
bgmac_dma_ring *ring)
  ring->mmio_base + BGMAC_DMA_RX_STATUS,
  BGMAC_DMA_RX_STAT, BGMAC_DMA_RX_STAT_DISABLED,
  1))
-   bgmac_err(bgmac, "Reset of ring 0x%X RX failed\n",
- ring->mmio_base);
+   dev_err(bgmac->dev, "Reset of ring 0x%X RX failed\n",
+   ring->mmio_base);
 }
 
 static void bgmac_dma_rx_enable(struct bgmac *bgmac,
@@ -370,7 +370,7 @@ static int bgmac_dma_rx_skb_for_slot(struct bgmac *bgmac,
dma_addr = dma_map_single(dma_dev, buf + BGMAC_RX_BUF_OFFSET,
  BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
if (dma_mapping_error(dma_dev, dma_addr)) {
-   bgmac_err(bgmac, "DMA mapping error

[RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file

[Jon Mason]

Move the BCMA MDIO phy into a separate file, as it is very tightly
coupled with the BCMA bus.  This will help with the upcoming BCMA
removal from the bgmac driver.  Optimally, this should be moved into
phy drivers, but it is too tightly coupled with the bgmac driver to
effectively move it without more changes to the driver.

Note: the phy_reset was intentionally removed, as the mdio phy subsystem
automatically resets the phy if a reset function pointer is present.  In
addition to the moving of the driver, this reset function is added.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 drivers/net/ethernet/broadcom/Makefile  |   2 +-
 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c | 264 
 drivers/net/ethernet/broadcom/bgmac.c   | 246 +++---
 drivers/net/ethernet/broadcom/bgmac.h   |   3 +
 4 files changed, 298 insertions(+), 217 deletions(-)
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c

diff --git a/drivers/net/ethernet/broadcom/Makefile 
b/drivers/net/ethernet/broadcom/Makefile
index 00584d7..f559794 100644
--- a/drivers/net/ethernet/broadcom/Makefile
+++ b/drivers/net/ethernet/broadcom/Makefile
@@ -10,6 +10,6 @@ obj-$(CONFIG_CNIC) += cnic.o
 obj-$(CONFIG_BNX2X) += bnx2x/
 obj-$(CONFIG_SB1250_MAC) += sb1250-mac.o
 obj-$(CONFIG_TIGON3) += tg3.o
-obj-$(CONFIG_BGMAC) += bgmac.o
+obj-$(CONFIG_BGMAC) += bgmac.o bgmac-bcma-mdio.o
 obj-$(CONFIG_SYSTEMPORT) += bcmsysport.o
 obj-$(CONFIG_BNXT) += bnxt/
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c 
b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
new file mode 100644
index 000..1e65349
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
@@ -0,0 +1,264 @@
+/*
+ * Driver for (BCM4706)? GBit MAC core on BCMA bus.
+ *
+ * Copyright (C) 2012 Rafał Miłecki <zaj...@gmail.com>
+ *
+ * Licensed under the GNU/GPL. See COPYING for details.
+ */
+
+#define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include "bgmac.h"
+
+struct bcma_mdio {
+   struct bcma_device *core;
+   u8 phyaddr;
+};
+
+static bool bcma_mdio_wait_value(struct bcma_device *core, u16 reg, u32 mask,
+u32 value, int timeout)
+{
+   u32 val;
+   int i;
+
+   for (i = 0; i < timeout / 10; i++) {
+   val = bcma_read32(core, reg);
+   if ((val & mask) == value)
+   return true;
+   udelay(10);
+   }
+   dev_err(>dev, "Timeout waiting for reg 0x%X\n", reg);
+   return false;
+}
+
+/**
+ * PHY ops
+ **/
+
+static u16 bcma_mdio_phy_read(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg)
+{
+   struct bcma_device *core;
+   u16 phy_access_addr;
+   u16 phy_ctl_addr;
+   u32 tmp;
+
+   BUILD_BUG_ON(BGMAC_PA_DATA_MASK != BCMA_GMAC_CMN_PA_DATA_MASK);
+   BUILD_BUG_ON(BGMAC_PA_ADDR_MASK != BCMA_GMAC_CMN_PA_ADDR_MASK);
+   BUILD_BUG_ON(BGMAC_PA_ADDR_SHIFT != BCMA_GMAC_CMN_PA_ADDR_SHIFT);
+   BUILD_BUG_ON(BGMAC_PA_REG_MASK != BCMA_GMAC_CMN_PA_REG_MASK);
+   BUILD_BUG_ON(BGMAC_PA_REG_SHIFT != BCMA_GMAC_CMN_PA_REG_SHIFT);
+   BUILD_BUG_ON(BGMAC_PA_WRITE != BCMA_GMAC_CMN_PA_WRITE);
+   BUILD_BUG_ON(BGMAC_PA_START != BCMA_GMAC_CMN_PA_START);
+   BUILD_BUG_ON(BGMAC_PC_EPA_MASK != BCMA_GMAC_CMN_PC_EPA_MASK);
+   BUILD_BUG_ON(BGMAC_PC_MCT_MASK != BCMA_GMAC_CMN_PC_MCT_MASK);
+   BUILD_BUG_ON(BGMAC_PC_MCT_SHIFT != BCMA_GMAC_CMN_PC_MCT_SHIFT);
+   BUILD_BUG_ON(BGMAC_PC_MTE != BCMA_GMAC_CMN_PC_MTE);
+
+   if (bcma_mdio->core->id.id == BCMA_CORE_4706_MAC_GBIT) {
+   core = bcma_mdio->core->bus->drv_gmac_cmn.core;
+   phy_access_addr = BCMA_GMAC_CMN_PHY_ACCESS;
+   phy_ctl_addr = BCMA_GMAC_CMN_PHY_CTL;
+   } else {
+   core = bcma_mdio->core;
+   phy_access_addr = BGMAC_PHY_ACCESS;
+   phy_ctl_addr = BGMAC_PHY_CNTL;
+   }
+
+   tmp = bcma_read32(core, phy_ctl_addr);
+   tmp &= ~BGMAC_PC_EPA_MASK;
+   tmp |= phyaddr;
+   bcma_write32(core, phy_ctl_addr, tmp);
+
+   tmp = BGMAC_PA_START;
+   tmp |= phyaddr << BGMAC_PA_ADDR_SHIFT;
+   tmp |= reg << BGMAC_PA_REG_SHIFT;
+   bcma_write32(core, phy_access_addr, tmp);
+
+   if (!bcma_mdio_wait_value(core, phy_access_addr, BGMAC_PA_START, 0,
+ 1000)) {
+   dev_err(>dev, "Reading PHY %d register 0x%X failed\n",
+   phyaddr, reg);
+   return 0x;
+   }
+
+   return bcma_read32(core, phy_access_addr) & BGMAC_PA_DATA_MASK;
+}
+
+/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphywr */
+static int bcma_mdio_phy_write(struct bcma_mdio *bcma_mdio, u8 phya

[RFC 5/7] net: ethernet: bgmac: Add platform device support

[Jon Mason]

The bcma portion of the driver has been split off into a bcma specific
driver.  This has been mirrored for the platform driver.  The last
references to the bcma core struct have been changed into a generic
function call.  These function calls are wrappers to either the original
bcma code or new platform functions that access the same areas via MMIO.
This necessitated adding function pointers for both platform and bcma to
hide which backend is being used from the generic bgmac code.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 drivers/net/ethernet/broadcom/Kconfig  |  23 +-
 drivers/net/ethernet/broadcom/Makefile |   4 +-
 drivers/net/ethernet/broadcom/bgmac-bcma.c | 315 
 drivers/net/ethernet/broadcom/bgmac-platform.c | 208 
 drivers/net/ethernet/broadcom/bgmac.c  | 327 -
 drivers/net/ethernet/broadcom/bgmac.h  |  73 +-
 6 files changed, 666 insertions(+), 284 deletions(-)
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma.c
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-platform.c

diff --git a/drivers/net/ethernet/broadcom/Kconfig 
b/drivers/net/ethernet/broadcom/Kconfig
index d74a92e..bd8c80c 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -140,10 +140,18 @@ config BNX2X_SRIOV
  allows for virtual function acceleration in virtual environments.
 
 config BGMAC
-   tristate "BCMA bus GBit core support"
+   tristate
+   help
+ This enables the integrated ethernet controller support for many
+ Broadcom (mostly iProc) SoCs. An appropriate bus interface driver
+ needs to be enabled to select this.
+
+config BGMAC_BCMA
+   tristate "Broadcom iProc GBit BCMA support"
depends on BCMA && BCMA_HOST_SOC
depends on HAS_DMA
depends on BCM47XX || ARCH_BCM_5301X || COMPILE_TEST
+   select BGMAC
select PHYLIB
select FIXED_PHY
---help---
@@ -152,6 +160,19 @@ config BGMAC
  In case of using this driver on BCM4706 it's also requires to enable
  BCMA_DRIVER_GMAC_CMN to make it work.
 
+config BGMAC_PLATFORM
+   tristate "Broadcom iProc GBit platform support"
+   depends on HAS_DMA
+   depends on ARCH_BCM_IPROC || COMPILE_TEST
+   depends on OF
+   select BGMAC
+   select PHYLIB
+   select FIXED_PHY
+   default ARCH_BCM_IPROC
+   ---help---
+ Say Y here if you want to use the Broadcom iProc Gigabit Ethernet
+ controller through the generic platform interface
+
 config SYSTEMPORT
tristate "Broadcom SYSTEMPORT internal MAC support"
depends on OF
diff --git a/drivers/net/ethernet/broadcom/Makefile 
b/drivers/net/ethernet/broadcom/Makefile
index f559794..79f2372 100644
--- a/drivers/net/ethernet/broadcom/Makefile
+++ b/drivers/net/ethernet/broadcom/Makefile
@@ -10,6 +10,8 @@ obj-$(CONFIG_CNIC) += cnic.o
 obj-$(CONFIG_BNX2X) += bnx2x/
 obj-$(CONFIG_SB1250_MAC) += sb1250-mac.o
 obj-$(CONFIG_TIGON3) += tg3.o
-obj-$(CONFIG_BGMAC) += bgmac.o bgmac-bcma-mdio.o
+obj-$(CONFIG_BGMAC) += bgmac.o
+obj-$(CONFIG_BGMAC_BCMA) += bgmac-bcma.o bgmac-bcma-mdio.o
+obj-$(CONFIG_BGMAC_PLATFORM) += bgmac-platform.o
 obj-$(CONFIG_SYSTEMPORT) += bcmsysport.o
 obj-$(CONFIG_BNXT) += bnxt/
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c 
b/drivers/net/ethernet/broadcom/bgmac-bcma.c
new file mode 100644
index 000..9a9745c4
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -0,0 +1,315 @@
+/*
+ * Driver for (BCM4706)? GBit MAC core on BCMA bus.
+ *
+ * Copyright (C) 2012 Rafał Miłecki <zaj...@gmail.com>
+ *
+ * Licensed under the GNU/GPL. See COPYING for details.
+ */
+
+#define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include "bgmac.h"
+
+static inline bool bgmac_is_bcm4707_family(struct bcma_device *core)
+{
+   switch (core->bus->chipinfo.id) {
+   case BCMA_CHIP_ID_BCM4707:
+   case BCMA_CHIP_ID_BCM47094:
+   case BCMA_CHIP_ID_BCM53018:
+   return true;
+   default:
+   return false;
+   }
+}
+
+/**
+ * BCMA bus ops
+ **/
+
+static u32 bcma_bgmac_read(struct bgmac *bgmac, u16 offset)
+{
+   return bcma_read32(bgmac->bcma.core, offset);
+}
+
+static void bcma_bgmac_write(struct bgmac *bgmac, u16 offset, u32 value)
+{
+   bcma_write32(bgmac->bcma.core, offset, value);
+}
+
+static u32 bcma_bgmac_idm_read(struct bgmac *bgmac, u16 offset)
+{
+   return bcma_aread32(bgmac->bcma.core, offset);
+}
+
+static void bcma_bgmac_idm_write(struct bgmac *bgmac, u16 offset, u32 value)
+{
+   return bcma_awrite32(bgmac->bcma.core, offset, value);
+}
+
+static bool bcma_bgmac_clk_enable

[RFC 4/7] net: ethernet: bgmac: convert to feature flags

[Jon Mason]

The bgmac driver is using the bcma provides device ID and revision, as
well as the SoC ID and package, to determine which features are
necessary to enable, reset, etc in the driver.   In anticipation of
removing the bcma requirement for this driver, these must be changed to
not reference that struct.  In place of that, each "feature" has been
given a flag, and the flags are enabled for their respective device and
SoC.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac.c | 167 --
 drivers/net/ethernet/broadcom/bgmac.h |  21 -
 2 files changed, 140 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c 
b/drivers/net/ethernet/broadcom/bgmac.c
index 88b9513..4346994 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -109,7 +109,7 @@ static void bgmac_dma_tx_enable(struct bgmac *bgmac,
u32 ctl;
 
ctl = bgmac_read(bgmac, ring->mmio_base + BGMAC_DMA_TX_CTL);
-   if (bgmac->core->id.rev >= 4) {
+   if (bgmac->feature_flags & BGMAC_FEAT_TX_MASK_SETUP) {
ctl &= ~BGMAC_DMA_TX_BL_MASK;
ctl |= BGMAC_DMA_TX_BL_128 << BGMAC_DMA_TX_BL_SHIFT;
 
@@ -330,7 +330,7 @@ static void bgmac_dma_rx_enable(struct bgmac *bgmac,
u32 ctl;
 
ctl = bgmac_read(bgmac, ring->mmio_base + BGMAC_DMA_RX_CTL);
-   if (bgmac->core->id.rev >= 4) {
+   if (bgmac->feature_flags & BGMAC_FEAT_RX_MASK_SETUP) {
ctl &= ~BGMAC_DMA_RX_BL_MASK;
ctl |= BGMAC_DMA_RX_BL_128 << BGMAC_DMA_RX_BL_SHIFT;
 
@@ -768,14 +768,20 @@ static void bgmac_cmdcfg_maskset(struct bgmac *bgmac, u32 
mask, u32 set,
 {
u32 cmdcfg = bgmac_read(bgmac, BGMAC_CMDCFG);
u32 new_val = (cmdcfg & mask) | set;
+   u32 cmdcfg_sr;
 
-   bgmac_set(bgmac, BGMAC_CMDCFG, BGMAC_CMDCFG_SR(bgmac->core->id.rev));
+   if (bgmac->feature_flags & BGMAC_FEAT_CMDCFG_SR_REV4)
+   cmdcfg_sr = BGMAC_CMDCFG_SR_REV4;
+   else
+   cmdcfg_sr = BGMAC_CMDCFG_SR_REV0;
+
+   bgmac_set(bgmac, BGMAC_CMDCFG, cmdcfg_sr);
udelay(2);
 
if (new_val != cmdcfg || force)
bgmac_write(bgmac, BGMAC_CMDCFG, new_val);
 
-   bgmac_mask(bgmac, BGMAC_CMDCFG, ~BGMAC_CMDCFG_SR(bgmac->core->id.rev));
+   bgmac_mask(bgmac, BGMAC_CMDCFG, ~cmdcfg_sr);
udelay(2);
 }
 
@@ -804,7 +810,7 @@ static void bgmac_chip_stats_update(struct bgmac *bgmac)
 {
int i;
 
-   if (bgmac->core->id.id != BCMA_CORE_4706_MAC_GBIT) {
+   if (!(bgmac->feature_flags & BGMAC_FEAT_NO_CLR_MIB)) {
for (i = 0; i < BGMAC_NUM_MIB_TX_REGS; i++)
bgmac->mib_tx_regs[i] =
bgmac_read(bgmac,
@@ -823,7 +829,7 @@ static void bgmac_clear_mib(struct bgmac *bgmac)
 {
int i;
 
-   if (bgmac->core->id.id == BCMA_CORE_4706_MAC_GBIT)
+   if (bgmac->feature_flags & BGMAC_FEAT_NO_CLR_MIB)
return;
 
bgmac_set(bgmac, BGMAC_DEV_CTL, BGMAC_DC_MROR);
@@ -866,9 +872,8 @@ static void bgmac_mac_speed(struct bgmac *bgmac)
 static void bgmac_miiconfig(struct bgmac *bgmac)
 {
struct bcma_device *core = bgmac->core;
-   u8 imode;
 
-   if (bgmac_is_bcm4707_family(bgmac)) {
+   if (bgmac->feature_flags & BGMAC_FEAT_FORCE_SPEED_2500) {
bcma_awrite32(core, BCMA_IOCTL,
  bcma_aread32(core, BCMA_IOCTL) | 0x40 |
  BGMAC_BCMA_IOCTL_SW_CLKEN);
@@ -876,6 +881,8 @@ static void bgmac_miiconfig(struct bgmac *bgmac)
bgmac->mac_duplex = DUPLEX_FULL;
bgmac_mac_speed(bgmac);
} else {
+   u8 imode;
+
imode = (bgmac_read(bgmac, BGMAC_DEV_STATUS) &
BGMAC_DS_MM_MASK) >> BGMAC_DS_MM_SHIFT;
if (imode == 0 || imode == 1) {
@@ -890,9 +897,7 @@ static void bgmac_miiconfig(struct bgmac *bgmac)
 static void bgmac_chip_reset(struct bgmac *bgmac)
 {
struct bcma_device *core = bgmac->core;
-   struct bcma_bus *bus = core->bus;
-   struct bcma_chipinfo *ci = >chipinfo;
-   u32 flags;
+   u32 cmdcfg_sr;
u32 iost;
int i;
 
@@ -915,15 +920,12 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
}
 
iost = bcma_aread32(core, BCMA_IOST);
-   if ((ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg == BCMA_PKG_ID_BCM47186) 
||
-   (ci->id == BCMA_CHIP_ID_BCM4749 && ci->pkg == 10) ||
-   (ci->id == BCMA_CHIP_ID_BCM53572 && ci->pkg == 
BCMA_PKG_ID_BCM47188))
+   if (bgmac->feature_flags & BGMAC_FEAT_IOST_ATTACHED)
iost &= ~BGMAC_BCMA_IOST_ATTACHED;
 
  

[RFC 0/7] net: ethernet: bgmac: Add platform device support

[Jon Mason]

I'm sending out this RFC to see if this is the direction the maintainers
would like to go to add support for other, non-bcma iProc SoC's to the
bgmac driver.  Specifically, we are interested in adding support for the
NSP, Cygnus, and NS2 families (with more possible down the road).

To support non-bcma enabled SoCs, we need to add the standard device
tree "platform device" support.  Unfortunately, this driver is very
tighly coupled with the bcma bus and much unwinding is needed.  I tried
to break this up into a number of patches to make it more obvious what
was being done to add platform device support.  I was able to verify
that the bcma code still works using a 53012K board (NS SoC), and that
the platform code works using a 58625K board (NSP SoC).

It is worth noting that the phy logic present in the driver needs to be
moved to drivers/phy.  However, I was not able to fully decouple that
code from the bgmac driver.  I was able to move it into a separate C
file, with only 2 function calls needed to create and destroy the mii
bus.  Someone with more knowledge of this and HW to test it needs to do
it properly.  This would natually dovetail into creating an interface
which the NSP bgmac can use for the external MDIO Phy to properly
connect (instead of using the fixed phy).

Thanks,
Jon


Jon Mason (7):
  net: ethernet: bgmac: change bgmac_* prints to dev_* prints
  net: ethernet: bgmac: add dma_dev pointer
  net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file
  net: ethernet: bgmac: convert to feature flags
  net: ethernet: bgmac: Add platform device support
  dt-bindings: net: bgmac: add bindings documentation for bgmac
  ARM: dts: NSP: Add bgmac entries

 .../devicetree/bindings/net/brcm,bgmac-enet.txt|  21 +
 arch/arm/boot/dts/bcm-nsp.dtsi |  16 +
 arch/arm/boot/dts/bcm958625k.dts   |   8 +
 drivers/net/ethernet/broadcom/Kconfig  |  23 +-
 drivers/net/ethernet/broadcom/Makefile |   2 +
 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c| 264 +
 drivers/net/ethernet/broadcom/bgmac-bcma.c | 315 ++
 drivers/net/ethernet/broadcom/bgmac-platform.c | 208 +++
 drivers/net/ethernet/broadcom/bgmac.c  | 658 +
 drivers/net/ethernet/broadcom/bgmac.h  | 112 +++-
 10 files changed,  insertions(+), 516 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma.c
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-platform.c

-- 
1.9.1

[RFC 7/7] ARM: dts: NSP: Add bgmac entries

[Jon Mason]

Add device tree entries for the ethernet devices present on the
Broadcom Northstar Plus SoCs

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 arch/arm/boot/dts/bcm-nsp.dtsi   | 16 
 arch/arm/boot/dts/bcm958625k.dts |  8 
 2 files changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
index def9e78..9b4e45e 100644
--- a/arch/arm/boot/dts/bcm-nsp.dtsi
+++ b/arch/arm/boot/dts/bcm-nsp.dtsi
@@ -192,6 +192,22 @@
status = "disabled";
};
 
+   gmac0: enet@22000 {
+   compatible = "brcm,bgmac-enet";
+   reg = <0x022000 0x1000>,
+ <0x11 0x1000>;
+   interrupts = ;
+   status = "disabled";
+   };
+
+   gmac1: enet@23000 {
+   compatible = "brcm,bgmac-enet";
+   reg = <0x023000 0x1000>,
+ <0x111000 0x1000>;
+   interrupts = ;
+   status = "disabled";
+   };
+
nand: nand@26000 {
compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1";
reg = <0x026000 0x600>,
diff --git a/arch/arm/boot/dts/bcm958625k.dts b/arch/arm/boot/dts/bcm958625k.dts
index e298450..d16ab53 100644
--- a/arch/arm/boot/dts/bcm958625k.dts
+++ b/arch/arm/boot/dts/bcm958625k.dts
@@ -56,6 +56,14 @@
status = "okay";
 };
 
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
  {
status = "okay";
 };
-- 
1.9.1

[RFC 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

[Jon Mason]

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 .../devicetree/bindings/net/brcm,bgmac-enet.txt | 21 +
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt

diff --git a/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt 
b/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
new file mode 100644
index 000..efd36d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
@@ -0,0 +1,21 @@
+Broadcom GMAC Ethernet Controller Device Tree Bindings
+-
+
+Required properties:
+ - compatible: "brcm,bgmac-enet"
+ - reg:Address and length of the GMAC registers,
+   Address and length of the GMAC IDM registers
+ - interrupts: Interrupt number
+
+Optional properties:
+- mac-address: mac address to be assigned to the device
+
+Examples:
+
+gmac0: enet@18022000 {
+   compatible = "brcm,bgmac-enet";
+   reg = <0x18022000 0x1000>,
+ <0x1811 0x1000>;
+   interrupts = ;
+   status = "disabled";
+};
-- 
1.9.1

[RFC 2/7] net: ethernet: bgmac: add dma_dev pointer

[Jon Mason]

The dma buffer allocation, etc references a dma_dev device pointer from
the bcma core.  In anticipation of removing the bcma requirement for
this driver, these must be changed to not reference that struct.  Add a
dma_dev device pointer to the bgmac stuct and reference that instead.

Signed-off-by: Jon Mason <jon.ma...@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac.c | 17 +
 drivers/net/ethernet/broadcom/bgmac.h |  1 +
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c 
b/drivers/net/ethernet/broadcom/bgmac.c
index 2ab0887..b5c35bc 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -152,7 +152,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
struct bgmac_dma_ring *ring,
struct sk_buff *skb)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct net_device *net_dev = bgmac->net_dev;
int index = ring->end % BGMAC_TX_RING_SLOTS;
struct bgmac_slot_info *slot = >slots[index];
@@ -254,7 +254,7 @@ err_drop:
 /* Free transmitted packets */
 static void bgmac_dma_tx_free(struct bgmac *bgmac, struct bgmac_dma_ring *ring)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
int empty_slot;
bool freed = false;
unsigned bytes_compl = 0, pkts_compl = 0;
@@ -351,7 +351,7 @@ static void bgmac_dma_rx_enable(struct bgmac *bgmac,
 static int bgmac_dma_rx_skb_for_slot(struct bgmac *bgmac,
 struct bgmac_slot_info *slot)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
dma_addr_t dma_addr;
struct bgmac_rx_header *rx;
void *buf;
@@ -440,7 +440,7 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct 
bgmac_dma_ring *ring,
end_slot /= sizeof(struct bgmac_dma_desc);
 
while (ring->start != end_slot) {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct bgmac_slot_info *slot = >slots[ring->start];
struct bgmac_rx_header *rx = slot->buf + BGMAC_RX_BUF_OFFSET;
struct sk_buff *skb;
@@ -543,7 +543,7 @@ static bool bgmac_dma_unaligned(struct bgmac *bgmac,
 static void bgmac_dma_tx_ring_free(struct bgmac *bgmac,
   struct bgmac_dma_ring *ring)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct bgmac_dma_desc *dma_desc = ring->cpu_base;
struct bgmac_slot_info *slot;
int i;
@@ -569,7 +569,7 @@ static void bgmac_dma_tx_ring_free(struct bgmac *bgmac,
 static void bgmac_dma_rx_ring_free(struct bgmac *bgmac,
   struct bgmac_dma_ring *ring)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct bgmac_slot_info *slot;
int i;
 
@@ -590,7 +590,7 @@ static void bgmac_dma_ring_desc_free(struct bgmac *bgmac,
 struct bgmac_dma_ring *ring,
 int num_slots)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
int size;
 
if (!ring->cpu_base)
@@ -628,7 +628,7 @@ static void bgmac_dma_free(struct bgmac *bgmac)
 
 static int bgmac_dma_alloc(struct bgmac *bgmac)
 {
-   struct device *dma_dev = bgmac->core->dma_dev;
+   struct device *dma_dev = bgmac->dma_dev;
struct bgmac_dma_ring *ring;
static const u16 ring_base[] = { BGMAC_DMA_BASE0, BGMAC_DMA_BASE1,
 BGMAC_DMA_BASE2, BGMAC_DMA_BASE3, };
@@ -1701,6 +1701,7 @@ static int bgmac_probe(struct bcma_device *core)
net_dev->ethtool_ops = _ethtool_ops;
bgmac = netdev_priv(net_dev);
bgmac->dev = >dev;
+   bgmac->dma_dev = core->dma_dev;
bgmac->net_dev = net_dev;
bgmac->core = core;
bcma_set_drvdata(core, bgmac);
diff --git a/drivers/net/ethernet/broadcom/bgmac.h 
b/drivers/net/ethernet/broadcom/bgmac.h
index abb9dd8..fd20018 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -429,6 +429,7 @@ struct bgmac {
struct bcma_device *cmn; /* Reference to CMN core for BCM4706 */
 
struct device *dev;
+   struct device *dma_dev;
struct net_device *net_dev;
struct napi_struct napi;
struct mii_bus *mii_bus;
-- 
1.9.1

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

[Mason]

On 22/03/2016 20:42, Uwe Kleine-König wrote:

> Preconditions:
>  - Some of the devices a given driver handles have a reset line and
>others don't.
>  - A non-empty subset (maybe all) of the devices that have a reset line
>require that this reset line is used.
> 
> Then the way to handle this in the driver should be done as follows:
> 
>   unless reset_handling_not_necessary():
> gpio = gpiod_get_optional("reset")
> if IS_ERR(gpio):
>   return PTR_ERR(gpio)
> 
> Checking for -ENOSYS or GPIOLIB=n is not allowed because the device
> you're currently handling might need the GPIO, so you must not continue
> without the ability to control the line.
> 
> So the options you have (as you have a phy that doesn't need the reset
> handling):
> 
>  - enable GPIOLIB (either in your .config or introduce a Kconfig
>dependency)
>  - improve reset_handling_not_necessary() to return true for your case
> 
> There is nothing else.

Here are some numbers for GPIOLIB, on an ARM build:

   textdata bss dec hex filename
   1830   0   01830 726 devres.o
627   0   0 627 273 gpiolib-legacy.o
  11018  40   4   110622b36 gpiolib.o
   1598   0   01598 63e gpiolib-of.o

  15073  40   4   151173b0d built-in.o

So ~15 kilobytes.


By the way, since the "reset-by-GPIO" solution is used only for
the Atheros 8030, would it be possible to make the
devm_gpiod_get_optional conditional on ATH8030_PHY_ID?

I'm thinking of something along these lines, for illustration:

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 2d020a3ec0b5..576e7873e049 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -198,12 +198,16 @@ static int at803x_probe(struct phy_device *phydev)
if (!priv)
return -ENOMEM;
 
+   if (phydev->drv->phy_id != ATH8030_PHY_ID)
+   goto no_gpio;
+
gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(gpiod_reset))
return PTR_ERR(gpiod_reset);
 
priv->gpiod_reset = gpiod_reset;
 
+no_gpio:
phydev->priv = priv;
 
return 0;


Regards.

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

[Mason]

On 18/03/2016 20:12, Uwe Kleine-König wrote:

> On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote:
> 
>> What would you think of making at803x_link_change_notify() print a
>> message every time it should do a reset but does not has a way to do it?
> 
> Then this question is obsolete because the device doesn't probe.

I don't understand this statement.

What does it mean for a question to be obsolete?

Regards.

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

[Mason]

On 18/03/2016 21:11, Uwe Kleine-König wrote:

> Hello,
> 
> On Fri, Mar 18, 2016 at 08:31:20PM +0100, Mason wrote:
>
>> On 18/03/2016 20:12, Uwe Kleine-König wrote:
>>
>>> On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote:
>>>
>>>> What would you think of making at803x_link_change_notify() print a
>>>> message every time it should do a reset but does not has a way to do it?
>>>
>>> Then this question is obsolete because the device doesn't probe.
>>
>> I don't understand this statement.
>>
>> What does it mean for a question to be obsolete?
> 
> If the driver doesn't probe because it cannot control the reset line,
> you don't need to think about how it should behave in
> at803x_link_change_notify without control of the reset line, because
> this code isn't reached then.

If I understand correctly, it is possible to soft-reset the PHY
by writing to a specific register. The GPIO pin is useful only to
force a hardware-reset when the PHY is wedged by some random event.

(Or am I completely off the mark?)

Regards.

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

[Mason]

[ CCing a few devs who might be interested ]

On 16/03/2016 18:25, Sebastian Frias wrote:

> Commit 687908c2b649 ("net: phy: at803x: simplify using
> devm_gpiod_get_optional and its 4th argument") introduced a dependency
> on GPIOLIB that was not there before.
> 
> This commit removes such dependency by checking the return code and
> comparing it against ENOSYS which is returned when GPIOLIB is not
> selected.
> 
> Fixes: 687908c2b649 ("net: phy: at803x: simplify using
> devm_gpiod_get_optional and its 4th argument")
> 
> Signed-off-by: Sebastian Frias 
> ---
>  drivers/net/phy/at803x.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 2174ec9..88b7ff3 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -252,7 +252,9 @@ static int at803x_probe(struct phy_device *phydev)
>   return -ENOMEM;
> 
>   gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> - if (IS_ERR(gpiod_reset))
> + if (PTR_ERR(gpiod_reset) == -ENOSYS)
> + gpiod_reset = NULL;
> + else if (IS_ERR(gpiod_reset))
>   return PTR_ERR(gpiod_reset);
> 
>   priv->gpiod_reset = gpiod_reset;
> 

Re: [PATCH v5] net: ethernet: nb8800: support fixed-link DT node

[Mason]

On 16/02/2016 21:04, David Miller wrote:

> This doesn't apply, please respin against my tree.

I fixed several formatting issues with Sebastian's patch,
and submitted v6.

Regards.

[PATCH v6] net: ethernet: nb8800: support fixed-link DT node

[Mason]

From: Sebastian Frias <s...@laposte.net>

Under some circumstances, e.g. when connecting to a switch, the ethernet
port will not be connected to a PHY. In that case a "fixed-link" DT node
can be used to replace it.

https://stackoverflow.com/questions/31046172/device-tree-for-phy-less-connection-to-a-dsa-switch

This patch adds support for the "fixed-link" node to the nb8800 driver.

Signed-off-by: Sebastian Frias <s...@laposte.net>
Acked-by: Mans Rullgard <m...@mansr.com>
Cc: Mason <slash@free.fr>
---
There were spurious spaces in the previous patch.
---
 drivers/net/ethernet/aurora/nb8800.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/aurora/nb8800.c 
b/drivers/net/ethernet/aurora/nb8800.c
index f71ab2647a3b..08a23e6b60e9 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -1460,7 +1460,19 @@ static int nb8800_probe(struct platform_device *pdev)
goto err_disable_clk;
}
 
-   priv->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
+   if (of_phy_is_fixed_link(pdev->dev.of_node)) {
+   ret = of_phy_register_fixed_link(pdev->dev.of_node);
+   if (ret < 0) {
+   dev_err(>dev, "bad fixed-link spec\n");
+   goto err_free_bus;
+   }
+   priv->phy_node = of_node_get(pdev->dev.of_node);
+   }
+
+   if (!priv->phy_node)
+   priv->phy_node = of_parse_phandle(pdev->dev.of_node,
+ "phy-handle", 0);
+
if (!priv->phy_node) {
dev_err(>dev, "no PHY specified\n");
ret = -ENODEV;
-- 
2.7.0

Re: [PATCH v3] net: ethernet: support "fixed-link" DT node on nb8800 driver

[Mason]

On 08/02/2016 14:37, Måns Rullgård wrote:

> Sebastian Frias wrote:
> 
>> By the way, I know some people like the command line, email, etc. but
>> there ought to be other tools better suited for patch review...
> 
> Some kernel subsystems use http://patchwork.ozlabs.org/ to track status
> of various patches.

There's also a kernel bugzilla, but it may be for actual bugs.
https://bugzilla.kernel.org/

Re: [PATCH 2/4] net: phy: at803x: Allow specifying the RGMII RX clock delay via phy mode

[Mason]

On 27/12/2015 04:28, Florian Fainelli wrote:

> Le 25/12/2015 16:27, Martin Blumenstingl wrote:
>
>> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
>> index f566b6e..0b262a2 100644
>> --- a/drivers/net/phy/at803x.c
>> +++ b/drivers/net/phy/at803x.c
>> @@ -36,8 +36,10 @@
>>  #define AT803X_INSR 0x0013
>>  #define AT803X_DEBUG_ADDR   0x1D
>>  #define AT803X_DEBUG_DATA   0x1E
>> -#define AT803X_DEBUG_SYSTEM_MODE_CTRL   0x05
>> -#define AT803X_DEBUG_RGMII_TX_CLK_DLY   BIT(8)
>> +#define AT803X_DEBUG_REG_0  0x00
> 
> Seems like the previous register name might have been clearer that the
> new name you suggest here, did that come from a different GPL tarball or
> documentation?

http://www.redeszone.net/app/uploads/2014/04/AR8035.pdf

According to the 8035 data sheet, the debug register at offset 0
is just "Debug register 0". In fact, the only non-reserved bit is
"rgmii rx clock delay enable/disable"

So the SYSTEM_MODE_CTRL name is misleading. Unless the register
has different semantics on the other PHYs?

Regards.

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

Re: Small improvements for the at803x PHY driver

[Mason]

[ CCing people who might be interested in this patch series ]

On 26/12/2015 01:26, Martin Blumenstingl wrote:

> while trying to debug a problem on a board with an AR8030 PHY (which turned
> out to be an incorrectly configured MDC clock) I made a few changes to the
> at803x driver.
> Due to lack of other hardware I could only test these changes with an
> AR8030 chip.

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

Re: [PATCH v8] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

[Mason]

On 25/11/2015 17:16, Måns Rullgård wrote:

> Alexander Duyck writes:
> 
>> On Wed, Nov 25, 2015 at 5:04 AM, Måns Rullgård wrote:
>>
>>> Mason writes:
>>>
>>>> On 25/11/2015 13:45, Måns Rullgård wrote:
>>>>
>>>>> Mason wrote:
>>>>>
>>>>>> On 19/11/2015 14:02, Mans Rullgard wrote:
>>>>>>
>>>>>>> +  if (dma_mapping_error(>dev, dma_addr)) {
>>>>>>> +  skb_free_frag(data);
>>>>>>> +  return -ENOMEM;
>>>>>>> +  }
>>>>>>
>>>>>> I'm back-porting this driver to 4.1
>>>>>>
>>>>>> skb_free_frag() was introduced in 4.2 by 181edb2bfa22b IIUC.
>>>>>>
>>>>>> +static inline void skb_free_frag(void *addr)
>>>>>> +{
>>>>>> +   __free_page_frag(addr);
>>>>>> +}
>>>>>>
>>>>>> Should I just copy the definition of __free_page_frag() ?
>>>>>
>>>>> Looks like it ought to work.  Try and find out.  Not that you'll ever
>>>>> hit that error condition unless you fake it.
>>>>
>>>> Turns out __free_pages_ok() is static and I'd rather not touch
>>>> mm/page_alloc.c in my back-port.
>>>>
>>>> Since you say the error condition is rare, I think I'll go with
>>>> the code that 181edb2bfa22b replaced (put_page, IIUC).
>>>>
>>>> #include 
>>>> #if LINUX_VERSION_CODE < KERNEL_VERSION(4,2,0)
>>>> #define skb_free_frag(data) put_page(virt_to_head_page(data))
>>>> #else
>>>> #error DELETE ME NOW (see commit 181edb2bfa22b)
>>>> #endif
>>>
>>> You can simply put_page(page) instead since we already have the
>>> virt_to_head_page() a few lines up.
>>
>> What you could do is use __free_pages instead of __free_pages_ok.
>> Generally you will want to use __free_pages instead of put_page just
>> to avoid a bunch of unnecessary tests and function pointer accesses.
> 
> Note that this is on a should-never-happen error path, so there's no
> need to be counting cycles.

Per Mans' advice, I have locally committed this patch:

diff --git a/drivers/net/ethernet/aurora/nb8800.c 
b/drivers/net/ethernet/aurora/nb8800.c
index ecc4a334c507..9f929be96b74 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -39,6 +39,13 @@
 
 #include "nb8800.h"
 
+#include 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(4,2,0)
+#define skb_free_frag(data) put_page(page)
+#else
+#error DELETE ME NOW (see commit 181edb2bfa22)
+#endif
+
 static void nb8800_tx_done(struct net_device *dev);
 static int nb8800_dma_stop(struct net_device *dev);
 

Regards.

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

Re: [PATCH v8] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

[Mason]

On 19/11/2015 14:02, Mans Rullgard wrote:

> + if (dma_mapping_error(>dev, dma_addr)) {
> + skb_free_frag(data);
> + return -ENOMEM;
> + }

I'm back-porting this driver to 4.1

skb_free_frag() was introduced in 4.2 by 181edb2bfa22b IIUC.

+static inline void skb_free_frag(void *addr)
+{
+   __free_page_frag(addr);
+}

Should I just copy the definition of __free_page_frag() ?

/*
 * Frees a page fragment allocated out of either a compound or order 0 page.
 */
void __free_page_frag(void *addr)
{
struct page *page = virt_to_head_page(addr);

if (unlikely(put_page_testzero(page)))
__free_pages_ok(page, compound_order(page));
}

Regards.

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

Re: [PATCH v8] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

[Mason]

[ Using different address for Alexander ]

On 25/11/2015 13:36, Mason wrote:

> On 19/11/2015 14:02, Mans Rullgard wrote:
> 
>> +if (dma_mapping_error(>dev, dma_addr)) {
>> +skb_free_frag(data);
>> +return -ENOMEM;
>> +}
> 
> I'm back-porting this driver to 4.1
> 
> skb_free_frag() was introduced in 4.2 by 181edb2bfa22b IIUC.
> 
> +static inline void skb_free_frag(void *addr)
> +{
> +   __free_page_frag(addr);
> +}
> 
> Should I just copy the definition of __free_page_frag() ?
> 
> /*
>  * Frees a page fragment allocated out of either a compound or order 0 page.
>  */
> void __free_page_frag(void *addr)
> {
>   struct page *page = virt_to_head_page(addr);
> 
>   if (unlikely(put_page_testzero(page)))
>   __free_pages_ok(page, compound_order(page));
> }

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

Re: [PATCH v8] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

[Mason]

On 25/11/2015 13:45, Måns Rullgård wrote:

> Mason wrote:
> 
>> On 19/11/2015 14:02, Mans Rullgard wrote:
>>
>>> +   if (dma_mapping_error(>dev, dma_addr)) {
>>> +   skb_free_frag(data);
>>> +   return -ENOMEM;
>>> +   }
>>
>> I'm back-porting this driver to 4.1
>>
>> skb_free_frag() was introduced in 4.2 by 181edb2bfa22b IIUC.
>>
>> +static inline void skb_free_frag(void *addr)
>> +{
>> +   __free_page_frag(addr);
>> +}
>>
>> Should I just copy the definition of __free_page_frag() ?
> 
> Looks like it ought to work.  Try and find out.  Not that you'll ever
> hit that error condition unless you fake it.

Turns out __free_pages_ok() is static and I'd rather not touch
mm/page_alloc.c in my back-port.

Since you say the error condition is rare, I think I'll go with
the code that 181edb2bfa22b replaced (put_page, IIUC).

#include 
#if LINUX_VERSION_CODE < KERNEL_VERSION(4,2,0)
#define skb_free_frag(data) put_page(virt_to_head_page(data))
#else
#error DELETE ME NOW (see commit 181edb2bfa22b)
#endif

Alexander, does that look correct?

Regards.

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

Re: [PATCH] net: phy: vitesse: add support for VSC8601

[Mason]

On 12/11/2015 19:41, Mans Rullgard wrote:

> + .phy_id = PHY_ID_VSC8601,
> + .name   = "Vitesse VSC8601",
> + .phy_id_mask= 0x0000,
> + .features   = PHY_GBIT_FEATURES,
> + .flags  = PHY_HAS_INTERRUPT,
> + .config_init= _config_init,
> + .config_aneg= _config_aneg,
> + .read_status= _read_status,
> + .ack_interrupt  = _ack_interrupt,
> + .config_intr= _config_intr,

I expected Documentation/CodingStyle to forbid taking the address
of functions.

Regards.

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

Re: [PATCH] net: phy: at803x: support interrupt on 8030 and 8035

[Mason]

On 12/11/2015 20:14, Florian Fainelli wrote:
> On 12/11/15 11:09, Måns Rullgård wrote:
>> On 12 November 2015 19:06:23 GMT+00:00, Mason wrote:
>>> On 12/11/2015 18:40, Mans Rullgard wrote:
>>>> Commit 77a993942 "phy/at8031: enable at8031 to work on interrupt mode"
>>>> added interrupt support for the 8031 PHY but left out the other two
>>>> chips supported by this driver.
>>>>
>>>> This patch sets the .ack_interrupt and .config_intr functions for the
>>>> 8030 and 8035 drivers as well.
>>>>
>>>> Signed-off-by: Mans Rullgard <m...@mansr.com>
>>>> ---
>>>> I have only tested this with an 8035.  I can't find a datasheet for
>>>> the 8030, but since 8031, 8032, and 8035 all have the same register
>>>> layout, there's a good chance 8030 does as well.
>>>> ---
>>>>  drivers/net/phy/at803x.c | 4 
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
>>>> index fabf11d..2d020a3 100644
>>>> --- a/drivers/net/phy/at803x.c
>>>> +++ b/drivers/net/phy/at803x.c
>>>> @@ -308,6 +308,8 @@ static struct phy_driver at803x_driver[] = {
>>>>.flags  = PHY_HAS_INTERRUPT,
>>>>.config_aneg= genphy_config_aneg,
>>>>.read_status= genphy_read_status,
>>>> +  .ack_interrupt  = at803x_ack_interrupt,
>>>> +  .config_intr= at803x_config_intr,
>>>>.driver = {
>>>>.owner = THIS_MODULE,
>>>>},
>>>> @@ -327,6 +329,8 @@ static struct phy_driver at803x_driver[] = {
>>>>.flags  = PHY_HAS_INTERRUPT,
>>>>.config_aneg= genphy_config_aneg,
>>>>.read_status= genphy_read_status,
>>>> +  .ack_interrupt  = at803x_ack_interrupt,
>>>> +  .config_intr= at803x_config_intr,
>>>>.driver = {
>>>>.owner = THIS_MODULE,
>>>>},
>>>
>>> Shouldn't we take the opportunity to clean up the duplicated register
>>> definitions? (I'll send an informal patch to spur discussion.)
>>>
>>> Regards.
>>
>> That can be done independently. Feel free to send a patch.
> 
> Agreed, that deserve a separate patch.

Isn't there a problem when at803x_set_wol() sets the AT803X_WOL_ENABLE
bit, but a DISABLE/ENABLE cycle through at803x_config_intr() will
discard that bit?

Regards.

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

Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

[Mason]

On 10/11/2015 20:25, Måns Rullgård wrote:

> Mason writes:
> 
>> On 10/11/2015 17:14, Mans Rullgard wrote:
>>
>>> This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
>>> It is an almost complete rewrite of a driver originally found in
>>> a Sigma Designs 2.6.22 tree.
>>>
>>> Signed-off-by: Mans Rullgard <m...@mansr.com>
>>> ---
>>> Changes:
>>> - Refactored mdio access functions
>>> - Refactored register access helpers
>>> - Improved error handling in rx buffer allocation
>>> - Optimised some fifo parameters
>>> - Overhauled tx dma. Multiple packets are now chained in a single dma
>>>   operation if xmit_more is set, improving performance.
>>> - Improved rx irq handling. It's not possible to disable interrupts
>>>   entirely for napi poll, but they can be slowed down a little.
>>> - Use readx_poll_timeout in various places
>>> - Improved error detection
>>> - Improved statistics
>>> - Report hardware statistics counters through ethtool
>>> - Improved tangox-specific setup
>>> - Support for flow control using pause frames
>>> - Explanatory comments added
>>> - Various minor stylistic changes
>>> ---
>>>  drivers/net/ethernet/Kconfig |1 +
>>>  drivers/net/ethernet/Makefile|1 +
>>>  drivers/net/ethernet/aurora/Kconfig  |   20 +
>>>  drivers/net/ethernet/aurora/Makefile |1 +
>>>  drivers/net/ethernet/aurora/nb8800.c | 1530 
>>> ++
>>>  drivers/net/ethernet/aurora/nb8800.h |  314 +++
>>>  6 files changed, 1867 insertions(+)
>>
>> The code has grown much since the previous patch, despite some
>> refactoring. Is this mostly due to ethtool_ops support?
>>
>>  drivers/net/ethernet/aurora/nb8800.c | 1146 
>> ++
>>  drivers/net/ethernet/aurora/nb8800.h |  230 +++
> 
> Some of the increase is from new features, some from improvements, and
> then there are a bunch of new comments.

Sweet.

With this version, my kernel boots faster than before
(I had been using a 5 month-old version.)

Before:

[0.613623] tangox-enet 26000.ethernet: SMP86xx internal Ethernet at 0x26000
[0.623638] libphy: tangox-mii: probed
[0.686527] tangox-enet 26000.ethernet: PHY: found Atheros 8035 ethernet at 
0x4
[0.697169] tangox-enet 26000.ethernet eth0: MAC address 00:16:e8:02:08:42
...
[1.306360] Sending DHCP requests ..
[4.699969] tangox-enet 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow 
control rx/tx
[8.899671] ., OK
[8.926343] IP-Config: Got DHCP answer from 172.27.200.1, my address is 
172.27.64.49
...
[8.987327] Freeing unused kernel memory: 168K (c039e000 - c03c8000)

After:

[0.623526] libphy: nb8800-mii: probed
[0.628092] nb8800 26000.ethernet eth0: MAC address 00:16:e8:02:08:42
...
[4.732948] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow 
control rx/tx
[4.752655] Sending DHCP requests ., OK
[4.782644] IP-Config: Got DHCP answer from 172.27.200.1, my address is 
172.27.64.49
...
[4.849298] Freeing unused kernel memory: 164K (c039f000 - c03c8000)


The DHCP request is sent later, but the kernel doesn't twiddle its thumbs
for 4 seconds after the link comes up. Does this come from not probing the
PHY anymore?

BTW, you're not using the PHY IRQ, right? I think I remember you saying
it didn't work reliably?

Regards.

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

Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

[Mason]

[ CCing a few knowledgeable people ]

Despite the subject, this is about an Atheros 8035 PHY :-)

On 12/11/2015 15:04, Måns Rullgård wrote:

> Mason wrote:
> 
>> BTW, you're not using the PHY IRQ, right? I think I remember you saying
>> it didn't work reliably?
> 
> It doesn't seem to be wired up on any of my boards, or there's some
> magic required to activate it that I'm unaware of.

Weird. The board schematics for the 1172 show Tango ETH0_MDINT# pin
properly connected to AR8035 INT pin (pin 20).



http://www.redeszone.net/app/uploads/2014/04/AR8035.pdf

INT pin 20
I/O, D, PD
Interrupt Signal to System; default OD-gate, needs an external
10Kohm pull-up, active low; can be configured to I/O by register,
active high.


4.1.17 Interrupt Enable
Offset: 0x12
Mode: Read/Write
Hardware Reset: 0


Strange... it looks like AT803X_INER and AT803X_INTR_ENABLE refer to
the same "Interrupt Enable" register?

In fact, AT803X_INER_INIT == 0xec00 makes sense for register 0x12:
link success/fail, speed/duplex changed, autoneg error

Looks like at803x_config_intr() is used for 8031, but not for 8035...

Relevant commit:
77a9939426f7a "phy/at8031: enable at8031 to work on interrupt mode"

If I add .config_intr and .ack_interrupt to the 8035 struct, then I get
(also added some traces)

[0.883517] *** at803x_config_intr: ENABLE
[1.576108] *** at803x_config_intr: DISABLE
[1.580467] *** at803x_config_intr: ENABLE
[1.584959] *** at803x_config_intr: DISABLE
[1.589297] *** at803x_config_intr: ENABLE
[4.321722] *** at803x_config_intr: DISABLE
[4.326054] *** at803x_config_intr: ENABLE
[4.330489] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow 
control rx/tx
[4.338335] *** at803x_config_intr: ENABLE

(Are all the ENABLE/DISABLE events expected?)

And if I unplug/replug the Ethernet cable,

[   71.903051] *** at803x_config_intr: DISABLE
[   71.907410] *** at803x_config_intr: ENABLE
[   71.912232] nb8800 26000.ethernet eth0: Link is Down
[   71.917309] *** at803x_config_intr: ENABLE
[   78.008972] *** at803x_config_intr: DISABLE
[   78.013375] *** at803x_config_intr: ENABLE
[   78.017797] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow 
control rx/tx
[   78.025702] *** at803x_config_intr: ENABLE

(Are all the ENABLE/DISABLE events expected there too?)

# cat /proc/interrupts 
CPU0   CPU1   
 18:107  0  irq0   1 Level serial
 54:  5  0  irq0  37 Edge  phy_interrupt
 55:   4953  0  irq0  38 Level eth0
211:   1147254   GIC  29 Edge  twd


Questions:

Can't at803x_ack_interrupt() just return phy_read(phydev, AT803X_INSR);
Can at803x_config_intr() be used with the 8035
What about AT803X_INER/AT803X_INTR_ENABLE and AT803X_INSR/AT803X_INTR_STATUS

Regards.

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

Re: [PATCH] net: phy: at803x: support interrupt on 8030 and 8035

[Mason]

On 12/11/2015 18:40, Mans Rullgard wrote:
> Commit 77a993942 "phy/at8031: enable at8031 to work on interrupt mode"
> added interrupt support for the 8031 PHY but left out the other two
> chips supported by this driver.
> 
> This patch sets the .ack_interrupt and .config_intr functions for the
> 8030 and 8035 drivers as well.
> 
> Signed-off-by: Mans Rullgard 
> ---
> I have only tested this with an 8035.  I can't find a datasheet for
> the 8030, but since 8031, 8032, and 8035 all have the same register
> layout, there's a good chance 8030 does as well.
> ---
>  drivers/net/phy/at803x.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index fabf11d..2d020a3 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -308,6 +308,8 @@ static struct phy_driver at803x_driver[] = {
>   .flags  = PHY_HAS_INTERRUPT,
>   .config_aneg= genphy_config_aneg,
>   .read_status= genphy_read_status,
> + .ack_interrupt  = at803x_ack_interrupt,
> + .config_intr= at803x_config_intr,
>   .driver = {
>   .owner = THIS_MODULE,
>   },
> @@ -327,6 +329,8 @@ static struct phy_driver at803x_driver[] = {
>   .flags  = PHY_HAS_INTERRUPT,
>   .config_aneg= genphy_config_aneg,
>   .read_status= genphy_read_status,
> + .ack_interrupt  = at803x_ack_interrupt,
> + .config_intr= at803x_config_intr,
>   .driver = {
>   .owner = THIS_MODULE,
>   },

Shouldn't we take the opportunity to clean up the duplicated register
definitions? (I'll send an informal patch to spur discussion.)

Regards.

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

Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

[Mason]

On 10/11/2015 17:14, Mans Rullgard wrote:

> This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
> It is an almost complete rewrite of a driver originally found in
> a Sigma Designs 2.6.22 tree.
> 
> Signed-off-by: Mans Rullgard 
> ---
> Changes:
> - Refactored mdio access functions
> - Refactored register access helpers
> - Improved error handling in rx buffer allocation
> - Optimised some fifo parameters
> - Overhauled tx dma. Multiple packets are now chained in a single dma
>   operation if xmit_more is set, improving performance.
> - Improved rx irq handling. It's not possible to disable interrupts
>   entirely for napi poll, but they can be slowed down a little.
> - Use readx_poll_timeout in various places
> - Improved error detection
> - Improved statistics
> - Report hardware statistics counters through ethtool
> - Improved tangox-specific setup
> - Support for flow control using pause frames
> - Explanatory comments added
> - Various minor stylistic changes
> ---
>  drivers/net/ethernet/Kconfig |1 +
>  drivers/net/ethernet/Makefile|1 +
>  drivers/net/ethernet/aurora/Kconfig  |   20 +
>  drivers/net/ethernet/aurora/Makefile |1 +
>  drivers/net/ethernet/aurora/nb8800.c | 1530 
> ++
>  drivers/net/ethernet/aurora/nb8800.h |  314 +++
>  6 files changed, 1867 insertions(+)

The code has grown much since the previous patch, despite some
refactoring. Is this mostly due to ethtool_ops support?

 drivers/net/ethernet/aurora/nb8800.c | 1146 ++
 drivers/net/ethernet/aurora/nb8800.h |  230 +++

Regards.

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

Re: [RFC] net: use atomic allocation for order-3 page allocation

[Chris Mason]

On 06/11/2015 04:48 PM, Eric Dumazet wrote:
 On Thu, 2015-06-11 at 13:24 -0700, Shaohua Li wrote:
 We saw excessive memory compaction triggered by skb_page_frag_refill.
 This causes performance issues. Commit 5640f7685831e0 introduces the
 order-3 allocation to improve performance. But memory compaction has
 high overhead. The benefit of order-3 allocation can't compensate the
 overhead of memory compaction.

 This patch makes the order-3 page allocation atomic. If there is no
 memory pressure and memory isn't fragmented, the alloction will still
 success, so we don't sacrifice the order-3 benefit here. If the atomic
 allocation fails, compaction will not be triggered and we will fallback
 to order-0 immediately.

 The mellanox driver does similar thing, if this is accepted, we must fix
 the driver too.

 Cc: Eric Dumazet eduma...@google.com
 Signed-off-by: Shaohua Li s...@fb.com
 ---
  net/core/sock.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/net/core/sock.c b/net/core/sock.c
 index 292f422..e9855a4 100644
 --- a/net/core/sock.c
 +++ b/net/core/sock.c
 @@ -1883,7 +1883,7 @@ bool skb_page_frag_refill(unsigned int sz, struct 
 page_frag *pfrag, gfp_t gfp)
  
  pfrag-offset = 0;
  if (SKB_FRAG_PAGE_ORDER) {
 -pfrag-page = alloc_pages(gfp | __GFP_COMP |
 +pfrag-page = alloc_pages((gfp  ~__GFP_WAIT) | __GFP_COMP |
__GFP_NOWARN | __GFP_NORETRY,
SKB_FRAG_PAGE_ORDER);
  if (likely(pfrag-page)) {
 
 This is not a specific networking issue, but mm one.
 
 You really need to start a discussion with mm experts.
 
 Your changelog does not exactly explains what _is_ the problem.
 
 If the problem lies in mm layer, it might be time to fix it, instead of
 work around the bug by never triggering it from this particular point,
 which is a safe point where a process is willing to wait a bit.
 
 Memory compaction is either working as intending, or not.
 
 If we enabled it but never run it because it hurts, what is the point
 enabling it ?

networking is asking for 32KB, and the MM layer is doing what it can to
provide it.  Are the gains from getting 32KB contig bigger than the cost
of moving pages around if the MM has to actually go into compaction?
Should we start disk IO to give back 32KB contig?

I think we want to tell the MM to compact in the background and give
networking 32KB if it happens to have it available.  If not, fall back
to smaller allocations without doing anything expensive.

-chris

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

Re: [RFC] net: use atomic allocation for order-3 page allocation

[Chris Mason]

On 06/11/2015 05:22 PM, Eric Dumazet wrote:
 On Thu, 2015-06-11 at 17:16 -0400, Chris Mason wrote:
 On 06/11/2015 04:48 PM, Eric Dumazet wrote:


 networking is asking for 32KB, and the MM layer is doing what it can to
 provide it.  Are the gains from getting 32KB contig bigger than the cost
 of moving pages around if the MM has to actually go into compaction?
 Should we start disk IO to give back 32KB contig?

 I think we want to tell the MM to compact in the background and give
 networking 32KB if it happens to have it available.  If not, fall back
 to smaller allocations without doing anything expensive.
 
 Exactly my point. (And I mentioned this about 4 months ago)

Sorry, reading this again I wasn't very clear.  I agree with Shaohua's
patch because it is telling the allocator that we don't want to wait for
reclaim or compaction to find contiguous pages.

But, is there any fallback to a single page allocation somewhere else?
If this is the only way to get memory, we might want to add a single
alloc_page path that won't trigger compaction but is at least able to
wait for kswapd to make progress.

-chris




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

[PATCH] add NAPI support to sb1250-mac.c (take 2)

[Mark Mason]

Patch to add NAPI support to sb1250-mac.c (rev 2).  This patch differs from
the last in that the NAPI support isn't marked as experimental, nor is it
configurable (ie. once applied - NAPI is enabled all the time).  This was
based on feedback from Ralf and others.

Signed-off-by: Mark Mason [EMAIL PROTECTED]

---
 drivers/net/sb1250-mac.c |  278 ++
 1 files changed, 182 insertions(+), 96 deletions(-)

diff --git a/drivers/net/sb1250-mac.c b/drivers/net/sb1250-mac.c
index 103c317..f8df1ec 100644
--- a/drivers/net/sb1250-mac.c
+++ b/drivers/net/sb1250-mac.c
@@ -95,19 +95,28 @@ MODULE_PARM_DESC(full_duplex, 1- 
__MODULE_STRING(MAX_UNITS));
 #endif
 
 #ifdef CONFIG_SBMAC_COALESCE
-static int int_pktcnt = 0;
-module_param(int_pktcnt, int, S_IRUGO);
-MODULE_PARM_DESC(int_pktcnt, Packet count);
+static int int_pktcnt_tx = 255;
+module_param(int_pktcnt_tx, int, S_IRUGO);
+MODULE_PARM_DESC(int_pktcnt_tx, TX packet count);
 
-static int int_timeout = 0;
-module_param(int_timeout, int, S_IRUGO);
-MODULE_PARM_DESC(int_timeout, Timeout value);
+static int int_timeout_tx = 255;
+module_param(int_timeout_tx, int, S_IRUGO);
+MODULE_PARM_DESC(int_timeout_tx, TX timeout value);
+
+static int int_pktcnt_rx = 64;
+module_param(int_pktcnt_rx, int, S_IRUGO);
+MODULE_PARM_DESC(int_pktcnt_rx, RX packet count);
+
+static int int_timeout_rx = 64;
+module_param(int_timeout_rx, int, S_IRUGO);
+MODULE_PARM_DESC(int_timeout_rx, RX timeout value);
 #endif
 
 #include asm/sibyte/sb1250.h
 #if defined(CONFIG_SIBYTE_BCM1x55) || defined(CONFIG_SIBYTE_BCM1x80)
 #include asm/sibyte/bcm1480_regs.h
 #include asm/sibyte/bcm1480_int.h
+#define R_MAC_DMA_OODPKTLOST_RXR_MAC_DMA_OODPKTLOST
 #elif defined(CONFIG_SIBYTE_SB1250) || defined(CONFIG_SIBYTE_BCM112X)
 #include asm/sibyte/sb1250_regs.h
 #include asm/sibyte/sb1250_int.h
@@ -155,8 +164,8 @@ typedef enum { sbmac_state_uninit, sbmac_state_off, 
sbmac_state_on,
 
 #define NUMCACHEBLKS(x) (((x)+SMP_CACHE_BYTES-1)/SMP_CACHE_BYTES)
 
-#define SBMAC_MAX_TXDESCR  32
-#define SBMAC_MAX_RXDESCR  32
+#define SBMAC_MAX_TXDESCR  256
+#define SBMAC_MAX_RXDESCR  256
 
 #define ETHER_ALIGN2
 #define ETHER_ADDR_LEN 6
@@ -185,10 +194,10 @@ typedef struct sbmacdma_s {
 * associated with it.
 */
 
-   struct sbmac_softc *sbdma_eth;  /* back pointer to associated 
MAC */
-   int  sbdma_channel; /* channel number */
+   struct sbmac_softc *sbdma_eth;  /* back pointer to associated MAC */
+   int  sbdma_channel; /* channel number */
int  sbdma_txdir;   /* direction (1=transmit) */
-   int  sbdma_maxdescr;/* total # of descriptors in 
ring */
+   int  sbdma_maxdescr;/* total # of descriptors in ring */
 #ifdef CONFIG_SBMAC_COALESCE
int  sbdma_int_pktcnt;  /* # descriptors rx/tx before 
interrupt*/
int  sbdma_int_timeout; /* # usec rx/tx interrupt */
@@ -197,13 +206,16 @@ typedef struct sbmacdma_s {
volatile void __iomem *sbdma_config0;   /* DMA config register 0 */
volatile void __iomem *sbdma_config1;   /* DMA config register 1 */
volatile void __iomem *sbdma_dscrbase;  /* Descriptor base address */
-   volatile void __iomem *sbdma_dscrcnt; /* Descriptor count register 
*/
+   volatile void __iomem *sbdma_dscrcnt;   /* Descriptor count register */
volatile void __iomem *sbdma_curdscr;   /* current descriptor address */
+   volatile void __iomem *sbdma_oodpktlost;/* pkt drop (rx only) */
+
 
/*
 * This stuff is for maintenance of the ring
 */
 
+   sbdmadscr_t *sbdma_dscrtable_unaligned;
sbdmadscr_t *sbdma_dscrtable;   /* base of descriptor table */
sbdmadscr_t *sbdma_dscrtable_end; /* end of descriptor table */
 
@@ -286,8 +298,8 @@ static int sbdma_add_rcvbuffer(sbmacdma_t *d,struct sk_buff 
*m);
 static int sbdma_add_txbuffer(sbmacdma_t *d,struct sk_buff *m);
 static void sbdma_emptyring(sbmacdma_t *d);
 static void sbdma_fillring(sbmacdma_t *d);
-static void sbdma_rx_process(struct sbmac_softc *sc,sbmacdma_t *d);
-static void sbdma_tx_process(struct sbmac_softc *sc,sbmacdma_t *d);
+static int sbdma_rx_process(struct sbmac_softc *sc,sbmacdma_t *d, int 
work_to_do, int poll);
+static void sbdma_tx_process(struct sbmac_softc *sc,sbmacdma_t *d, int poll);
 static int sbmac_initctx(struct sbmac_softc *s);
 static void sbmac_channel_start(struct sbmac_softc *s);
 static void sbmac_channel_stop(struct sbmac_softc *s);
@@ -308,6 +320,8 @@ static struct net_device_stats *sbmac_get_stats(struct 
net_device *dev);
 static void sbmac_set_rx_mode(struct net_device *dev);
 static int sbmac_mii_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
 static int sbmac_close(struct net_device *dev);
+static int sbmac_poll(struct net_device *poll_dev, int *budget);
+
 static

RE: [PATCH] NAPI support for Sibyte MAC

[Mark E Mason]

Hello, 

 -Original Message-
 From: Sergei Shtylyov [mailto:[EMAIL PROTECTED] 
 Sent: Saturday, March 24, 2007 12:49 PM
 To: Mark E Mason
 Cc: [EMAIL PROTECTED]; netdev@vger.kernel.org
 Subject: Re: [PATCH] NAPI support for Sibyte MAC

[snip]
  @@ -2075,12 +2143,52 @@
   */
   
  if (isr  (M_MAC_INT_CHANNEL  S_MAC_TX_CH0)) {
  -   sbdma_tx_process(sc,(sc-sbm_txdma));
  +   sbdma_tx_process(sc,(sc-sbm_txdma), 0);
  +#ifdef CONFIG_NETPOLL_TRAP
  +  if (netpoll_trap()) {
  +  if 
 (test_and_clear_bit(__LINK_STATE_XOFF, dev-state)) 
  +  __netif_schedule(dev);
  +  }
  +#endif
  }
 
 This just doesn't make sense. That option is enabled to 
 *prevent* calls to 
 __netif_schedule() -- you can't override it that way. (Well, 
 how it works 
 currently, doesn't make much sense either since it totally 
 breaks the TX queue 
 control -- I was going to post a patch).

I'll admit - this was just copied over from the non-NAPI version we
started with ;-).

Are you saying that the #ifdef test should simply be an #ifndef here?

[I'm working on a revised patch based on feedback from Ralf and others
which removes the Kconfig option for NAPI - it'll just be on, all the
time.]

Thanks,
Mark

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

[PATCH] NAPI support for Sibyte MAC

[mason]

[ This is a re-post, but the patch still applies and works fine against
the linux-mips.org tip. We'd really like to get this in. -Mark]

  This patch completes the NAPI functionality for SB1250 MAC, including making
  NAPI a kernel option that can be turned on or off and adds the sbmac_poll
  routine.

Signed off by: Mark Mason ([EMAIL PROTECTED])
Signed off by: Dan Krejsa ([EMAIL PROTECTED])
Signed off by: Steve Yang ([EMAIL PROTECTED])

Index: linux-2.6.14-cgl/drivers/net/Kconfig
===
--- linux-2.6.14-cgl.orig/drivers/net/Kconfig   2006-09-20 14:58:54.0 
-0700
+++ linux-2.6.14-cgl/drivers/net/Kconfig2006-09-20 17:04:31.0 
-0700
@@ -2031,6 +2031,23 @@
tristate SB1250 Ethernet support
depends on SIBYTE_SB1xxx_SOC
 
+config SBMAC_NAPI
+   bool SBMAC: Use Rx Polling (NAPI) (EXPERIMENTAL)
+   depends on NET_SB1250_MAC  EXPERIMENTAL
+   help
+ NAPI is a new driver API designed to reduce CPU and interrupt load
+ when the driver is receiving lots of packets from the card. It is
+ still somewhat experimental and thus not yet enabled by default.
+
+ If your estimated Rx load is 10kpps or more, or if the card will be
+ deployed on potentially unfriendly networks (e.g. in a firewall),
+ then say Y here.
+
+ See file:Documentation/networking/NAPI_HOWTO.txt for more
+ information.
+
+ If in doubt, say y.
+
 config R8169_VLAN
bool VLAN support
depends on R8169  VLAN_8021Q
@@ -2826,3 +2843,5 @@
def_bool NETPOLL
 
 endmenu
+
+
Index: linux-2.6.14-cgl/drivers/net/sb1250-mac.c
===
--- linux-2.6.14-cgl.orig/drivers/net/sb1250-mac.c  2006-09-20 
14:59:00.0 -0700
+++ linux-2.6.14-cgl/drivers/net/sb1250-mac.c   2006-09-20 20:16:27.0 
-0700
@@ -95,19 +95,28 @@
 #endif
 
 #ifdef CONFIG_SBMAC_COALESCE
-static int int_pktcnt = 0;
-module_param(int_pktcnt, int, S_IRUGO);
-MODULE_PARM_DESC(int_pktcnt, Packet count);
-
-static int int_timeout = 0;
-module_param(int_timeout, int, S_IRUGO);
-MODULE_PARM_DESC(int_timeout, Timeout value);
+static int int_pktcnt_tx = 255;
+module_param(int_pktcnt_tx, int, S_IRUGO);
+MODULE_PARM_DESC(int_pktcnt_tx, TX packet count);
+
+static int int_timeout_tx = 255;
+module_param(int_timeout_tx, int, S_IRUGO);
+MODULE_PARM_DESC(int_timeout_tx, TX timeout value);
+
+static int int_pktcnt_rx = 64;
+module_param(int_pktcnt_rx, int, S_IRUGO);
+MODULE_PARM_DESC(int_pktcnt_rx, RX packet count);
+
+static int int_timeout_rx = 64;
+module_param(int_timeout_rx, int, S_IRUGO);
+MODULE_PARM_DESC(int_timeout_rx, RX timeout value);
 #endif
 
 #include asm/sibyte/sb1250.h
 #if defined(CONFIG_SIBYTE_BCM1x55) || defined(CONFIG_SIBYTE_BCM1x80)
 #include asm/sibyte/bcm1480_regs.h
 #include asm/sibyte/bcm1480_int.h
+#define R_MAC_DMA_OODPKTLOST_RXR_MAC_DMA_OODPKTLOST
 #elif defined(CONFIG_SIBYTE_SB1250) || defined(CONFIG_SIBYTE_BCM112X)
 #include asm/sibyte/sb1250_regs.h
 #include asm/sibyte/sb1250_int.h
@@ -155,8 +164,8 @@
 
 #define NUMCACHEBLKS(x) (((x)+SMP_CACHE_BYTES-1)/SMP_CACHE_BYTES)
 
-#define SBMAC_MAX_TXDESCR  32
-#define SBMAC_MAX_RXDESCR  32
+#define SBMAC_MAX_TXDESCR  256
+#define SBMAC_MAX_RXDESCR  256
 
 #define ETHER_ALIGN2
 #define ETHER_ADDR_LEN 6
@@ -185,10 +194,10 @@
 * associated with it.
 */
 
-   struct sbmac_softc *sbdma_eth;  /* back pointer to associated 
MAC */
-   int  sbdma_channel; /* channel number */
+   struct sbmac_softc *sbdma_eth;  /* back pointer to associated MAC */
+   int  sbdma_channel; /* channel number */
int  sbdma_txdir;   /* direction (1=transmit) */
-   int  sbdma_maxdescr;/* total # of descriptors in 
ring */
+   int  sbdma_maxdescr;/* total # of descriptors in ring */
 #ifdef CONFIG_SBMAC_COALESCE
int  sbdma_int_pktcnt;  /* # descriptors rx/tx before 
interrupt*/
int  sbdma_int_timeout; /* # usec rx/tx interrupt */
@@ -197,13 +206,16 @@
volatile void __iomem *sbdma_config0;   /* DMA config register 0 */
volatile void __iomem *sbdma_config1;   /* DMA config register 1 */
volatile void __iomem *sbdma_dscrbase;  /* Descriptor base address */
-   volatile void __iomem *sbdma_dscrcnt; /* Descriptor count register 
*/
+   volatile void __iomem *sbdma_dscrcnt;   /* Descriptor count register */
volatile void __iomem *sbdma_curdscr;   /* current descriptor address */
+   volatile void __iomem *sbdma_oodpktlost;/* pkt drop (rx only) */
+
 
/*
 * This stuff is for maintenance of the ring
 */
 
+   sbdmadscr_t *sbdma_dscrtable_unaligned;
sbdmadscr_t *sbdma_dscrtable;   /* base

[PATCH] bridge: add ETH_HLEN to packet_length

[Jon Mason]

In br_dev_queue_push_xmit, why is the check to drop mtu oversized
packets not checking for enough room for the impending ETH_HLEN size
skb_push?  In some code currently under development, we are seeing
skb_under_panic being called from the skb_push(skb, ETH_HLEN) in that
code.  It seems to me it would be better to drop those skbs than panic.
Attached is a patch to do this.

Thanks,
Jon

Signed-off-by: Jon Mason [EMAIL PROTECTED]

diff -r b1d36669f98d net/bridge/br_forward.c
--- a/net/bridge/br_forward.c   Mon Sep  4 03:00:04 2006 +
+++ b/net/bridge/br_forward.c   Thu Sep 14 13:18:04 2006 -0500
@@ -29,7 +29,8 @@ static inline int should_deliver(const s
 
 static inline unsigned packet_length(const struct sk_buff *skb)
 {
-   return skb-len - (skb-protocol == htons(ETH_P_8021Q) ? VLAN_HLEN : 0);
+   return skb-len - ETH_HLEN - 
+   (skb-protocol == htons(ETH_P_8021Q) ? VLAN_HLEN : 0);
 }
 
 int br_dev_queue_push_xmit(struct sk_buff *skb)
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bridge: add ETH_HLEN to packet_length

[Jon Mason]

On Thu, Sep 14, 2006 at 11:53:48PM +0300, Mika Penttil? wrote:
 Jon Mason wrote:
 In br_dev_queue_push_xmit, why is the check to drop mtu oversized
 packets not checking for enough room for the impending ETH_HLEN size
 skb_push?  In some code currently under development, we are seeing
 skb_under_panic being called from the skb_push(skb, ETH_HLEN) in that
 code.  It seems to me it would be better to drop those skbs than panic.
 Attached is a patch to do this.
 
 Thanks,
 Jon
 
 Signed-off-by: Jon Mason [EMAIL PROTECTED]
 
 diff -r b1d36669f98d net/bridge/br_forward.c
 --- a/net/bridge/br_forward.cMon Sep  4 03:00:04 2006 +
 +++ b/net/bridge/br_forward.cThu Sep 14 13:18:04 2006 -0500
 @@ -29,7 +29,8 @@ static inline int should_deliver(const s
  
  static inline unsigned packet_length(const struct sk_buff *skb)
  {
 -return skb-len - (skb-protocol == htons(ETH_P_8021Q) ? VLAN_HLEN : 
 0);
 +return skb-len - ETH_HLEN - 
 +(skb-protocol == htons(ETH_P_8021Q) ? VLAN_HLEN : 0);
  }
 
   
 packet_length() is a wrong place to do that, mtu has nothing to do with 
 skb headroom.

Oops, you are 100% correct.  So what would be the best way to handle
this, as a panic is pretty nasty?  Wouldn't it be better to check the
packet to see if there is sufficient headroom, BUG, and then drop it
(or allocate a new packet with sufficient headroom and proceed with
the skb_push)?  

Thanks,
Jon

 
 --Mika
 
 
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: PATCH SB1250 NAPI support

[Mark E Mason]

Hello,

We've done some further work on the bcm1250/bcm1480 MAC driver since
then, primarily in the area of performance improvements, but we've not
quite ready to submit the patch for those changes yet.

In the meantime, Tom's patch fixes all of the really bad problems, and
we'd really appreciate it if it could be merged in.

Thx,
Mark Mason
Broadcom

 -Original Message-
 From: Martin Michlmayr [mailto:[EMAIL PROTECTED] 
 Sent: Thursday, June 29, 2006 1:22 AM
 To: [EMAIL PROTECTED]; netdev@vger.kernel.org
 Cc: Tom Rix; Mark E Mason
 Subject: Re: PATCH SB1250 NAPI support
 
 * Tom Rix [EMAIL PROTECTED] [2006-06-04 11:45]:
  A while ago I submitted this patch for NAPI support for 
 SB1250 / SB1480.
  I have tested it again on the recent (6/3/06) linux-mips kernel.
 
 Are there any comments regarding this patch (apart from the fact that
 Tom forgot to include a Signed-off-by line).  Can it go in when it's
 resent with a Signed-off-by line or does the patch have any other
 problems?
 -- 
 Martin Michlmayr
 [EMAIL PROTECTED]
 
 

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

Re: [RFT] pcnet32 NAPI changes

[Jon Mason]

On Mon, Jun 19, 2006 at 04:49:33PM -0400, Lennart Sorensen wrote:
 On Mon, Jun 19, 2006 at 03:41:40PM -0500, Jon Mason wrote:
  I believe it is preferred to be a compile option for non-gigabit
  drivers, given that it will be eating a lot of cycles for infrequent
  packets (especially for the 10Mb).  I believe there was a thread about
  this last year when e100 was having NAPI problems.
 
 How does NAPI eat cycles?  It goes back to interrupt mode when the queue
 is empty, and only on RX interrupt does it turn on polling again.

The amount of polls per received packet is very low, thus removing the
benefit of NAPI.  A compile time option would allow those users who know
better to DTRT.

 It is certainly possible that there are bugs in a NAPI conversion, which
 I guess could be a reason to have the option to stick with the old
 method, although then again not having the option ensures the bugs get
 found sooner.
 
  A general nit.  There are ALOT of magic numbers in the code, most
  existing prior to this patch.  The driver would benefit from a little
  clean-up.
  
  Also nothing to do with this patch, but I noticed it when the code was
  moved.  A comment about why the following is necessary might be nice:
  lp-rx_ring[i].buf_length = le16_to_cpu(2 - PKT_BUF_SZ);
 
 I suspect many drivers are in need of some cleanup.

Yup, but the everyone else is doing it argument never worked with my
parents. All it takes is one brave soul to determine the reasoning
behind the magic numbers and convert them into #define's.  Shouldn't be
more than one day's work.

 
 Len Sorensen
 -
 To unsubscribe from this list: send the line unsubscribe netdev in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFT] pcnet32 NAPI changes

[Jon Mason]

On Tue, Jun 20, 2006 at 10:48:07AM -0400, Lennart Sorensen wrote:
 On Tue, Jun 20, 2006 at 08:53:55AM -0500, Jon Mason wrote:
  The amount of polls per received packet is very low, thus removing the
  benefit of NAPI.  A compile time option would allow those users who know
  better to DTRT.
 
 Well I know on the slow poke system I run on, with the napi polling, the
 system can process packets, and get work done, and not fall over and die
 from handling interrupts.  Without it, even 70Mbit of data on a single
 port will flood the system with packet overruns to the point the
 watchdog times out and the system reboots.  So I don't know if polling
 is slightly more inefficient with little traffic, it is certainly a lot
 more efficient and safer when there is suddenly a lot more traffic.
 Maybe it should be a module option, so that you can pick what you want.
 Heck it could be a per port option even. :)

The point of my comment was CPU utilization.

It appears that a bug is trying to be fixed by adding NAPI. This
sounds a bit hackish to me, and could hide the root cause of the
problem. So I'm not sure that is the best idea, but I will defer to
the maintainer.

 
  Yup, but the everyone else is doing it argument never worked with my
  parents. All it takes is one brave soul to determine the reasoning
  behind the magic numbers and convert them into #define's.  Shouldn't be
  more than one day's work.
 
 Is this a magic number in your opinion?
 
 lp-a.write_csr(ioaddr, 0, 0x0002);  /* Set STRT bit */
 
 I guess one could do
 #define CSR0_RST 0x0001
 #define CSR0_STRT 0x0002
 #define CSR0_STOP 0x0004
 etc...
 
 and then
 lp-a.write_csr(ioaddr, 0, CSR0_STRT); /* Set STRT bit */
 
 Does that help?  I am not sure.  I think the comment behind it is
 plenty.

But your example is just one instance.  Here is one without a comment:

lp-a.write_csr(ioaddr, 4, 0x0915);

What is it doing?  Is it still needed?  Can it be done anywhere else?  
Who knows, because it is magic.  The 4 can be defined as CSR0_STOP, per
your example above, but what does value 0x0915 do?

My point was that there are certain parts of the code which are
non-intuative and should be commented and there are others which a
good descrptive value would be nice.

 
 Len Sorensen
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFT] pcnet32 NAPI changes

[Jon Mason]

On Fri, Jun 16, 2006 at 12:11:54PM -0700, Don Fry wrote:
 This patch is a collection of changes to pcnet32 which does the
 following: 
 
 - Fix section mismatch warning.
 - fix set_ringparam to correctly handle memory allocation failures
 - fix off-by-one in get_ringparam.
 - cleanup at end of loopback_test when not up.
 - Add NAPI to driver, fixing set_ringparam and loopback_test to work
   correctly with poll.
 - for multicast, do not reset the chip unless cannot enter suspend mode
   to avoid race with poll.
 
 The set_ringparam code is larger than I would prefer, but it will not
 leave null pointers around for the code to stumble over when memory
 allocation fails.  If anyone has a better idea, please let me know.
 
 Some complexity could be avoided by allocating memory for the maximum
 number of tx and rx buffers at probe time.  Requiring 14k for the tx
 ring and arrays, and another 14k for rx; instead of about 10k total for
 the default sizes.
 
 It is NAPI only, unlike Len Sorensen's version which allows for compile
 time selection.  Some drivers are NAPI only, others have compile
 options.  Which is preferred?

I believe it is preferred to be a compile option for non-gigabit
drivers, given that it will be eating a lot of cycles for infrequent
packets (especially for the 10Mb).  I believe there was a thread about
this last year when e100 was having NAPI problems.

A general nit.  There are ALOT of magic numbers in the code, most
existing prior to this patch.  The driver would benefit from a little
clean-up.

Also nothing to do with this patch, but I noticed it when the code was
moved.  A comment about why the following is necessary might be nice:
lp-rx_ring[i].buf_length = le16_to_cpu(2 - PKT_BUF_SZ);

Thanks,
Jon

 
 I have tested these changes with a 79C971, 973, 976, and 978 on a ppc64
 machine, and 970A, 972, 973, 975, and 976 on an x86 machine.
 
 I have not tested these changes with VMware or Xen.
 
 
 
 --- linux-2.6.17-rc6/drivers/net/orig.pcnet32.c   2006-06-15 
 11:49:39.0 -0700
 +++ linux-2.6.17-rc6/drivers/net/pcnet32.c2006-06-16 11:30:45.0 
 -0700
 @@ -22,8 +22,8 @@
   */
  
  #define DRV_NAME pcnet32
 -#define DRV_VERSION  1.32
 -#define DRV_RELDATE  18.Mar.2006
 +#define DRV_VERSION  1.33-NAPI
 +#define DRV_RELDATE  16.Jun.2006
  #define PFX  DRV_NAME : 
  
  static const char *const version =
 @@ -277,13 +277,12 @@ struct pcnet32_private {
   u32 phymask;
  };
  
 -static void pcnet32_probe_vlbus(void);
  static int pcnet32_probe_pci(struct pci_dev *, const struct pci_device_id *);
  static int pcnet32_probe1(unsigned long, int, struct pci_dev *);
  static int pcnet32_open(struct net_device *);
  static int pcnet32_init_ring(struct net_device *);
  static int pcnet32_start_xmit(struct sk_buff *, struct net_device *);
 -static int pcnet32_rx(struct net_device *);
 +static int pcnet32_poll(struct net_device *dev, int *budget);
  static void pcnet32_tx_timeout(struct net_device *dev);
  static irqreturn_t pcnet32_interrupt(int, void *, struct pt_regs *);
  static int pcnet32_close(struct net_device *);
 @@ -425,6 +424,215 @@ static struct pcnet32_access pcnet32_dwi
   .reset = pcnet32_dwio_reset
  };
  
 +static void pcnet32_netif_stop(struct net_device *dev)
 +{
 + dev-trans_start = jiffies;
 + netif_poll_disable(dev);
 + netif_tx_disable(dev);
 +}
 +
 +static void pcnet32_netif_start(struct net_device *dev)
 +{
 + netif_wake_queue(dev);
 + netif_poll_enable(dev);
 +}
 +
 +/*
 + * Allocate space for the new sized tx ring.
 + * Free old resources
 + * Save new resources.
 + * Any failure keeps old resources.
 + * Must be called with lp-lock held.
 + */
 +static void pcnet32_realloc_tx_ring(struct net_device *dev,
 + struct pcnet32_private *lp,
 + unsigned int size)
 +{
 + dma_addr_t new_ring_dma_addr;
 + dma_addr_t *new_dma_addr_list;
 + struct pcnet32_tx_head *new_tx_ring;
 + struct sk_buff **new_skb_list;
 +
 + pcnet32_purge_tx_ring(dev);
 +
 + new_tx_ring = pci_alloc_consistent(lp-pci_dev,
 +sizeof(struct pcnet32_tx_head) *
 +(1  size),
 +new_ring_dma_addr);
 + if (new_tx_ring == NULL) {
 + if (pcnet32_debug  NETIF_MSG_DRV)
 + printk(\n KERN_ERR PFX
 +%s: Consistent memory allocation failed.\n,
 +dev-name);
 + return;
 + }
 + memset(new_tx_ring, 0, sizeof(struct pcnet32_tx_head) * (1  size));
 +
 + new_dma_addr_list = kcalloc(sizeof(dma_addr_t), (1  size), 
 GFP_ATOMIC);
 + if (!new_dma_addr_list) {
 + if (pcnet32_debug  NETIF_MSG_DRV)
 + printk(\n KERN_ERR PFX
 +  

Re: dl2k: PCI parity error

[Jon Mason]

On Sat, May 13, 2006 at 09:57:15AM -0700, Shane wrote:
 Hello list,
 
 Not sure if this is the best place to report this.  I seem
 to be having a conflict of sorts and believe it may be a
 dl2k issue.  The card is a Dlink 550t gigabit pci-64 card. 
 When connected to the pci-64 slot on the mb which shares an
 IRQ with the onboard SCSI controller, the SCSI subsystem
 starts reporting PCI parity errors.  If I move the NIC to a
 pci-32 slot, the errors go away.

Do you have any other 64bit slots?

 scsi0: PCI error Interrupt at seqaddr = 0x9
 scsi0: Data Parity Error Detected during address or write data phase
 scsi1: PCI error Interrupt at seqaddr = 0x9
 scsi1: Data Parity Error Detected during address or write data phase
 scsi1: PCI error Interrupt at seqaddr = 0x9
 scsi1: Data Parity Error Detected during address or write data phase
 scsi0: PCI error Interrupt at seqaddr = 0x8
 scsi0: Data Parity Error Detected during address or write data phase
 scsi0: WARNING WARNING WARNING WARNING
 scsi0: Too many PCI parity errors observed as a target.
 scsi0: Some device on this bus is generating b ad parity.
 scsi0: This is an error *observed by*, not *generated by*, this controller.
 scsi0: PCI parity error checking has been disabled.
 
 This is using a 2.6.15.4 kernel but I can certainly update
 if that will help.  Is this likely a dl2k issue?  The
 driver does seem a bit dated.

The driver is mostly unmaintained.  I did a fix a month or so ago which
fixed a DMA free issue, but that doesn't sound like the problem you have
here.

There is another error I am seeing that I haven't had time to track
down, where the adapter appears to be DMA'ing somewhere its not supposed
to.

I would be suprised if either of these caused a SCSI data parity error.

Thanks,
Jon
 
 Tia,
 Shane
 
 -- 
 http://www.cm.nu/~shane/
 -
 To unsubscribe from this list: send the line unsubscribe netdev in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pcnet32.c: modify RX ring size through module parameter

[Jon Mason]

Why is this necessary?  There is already an ethtool function to set
the rx ring size (pcnet32_set_ringparam).  Since module parameters
are being phased out in favor of the ethtool functions, why not use
the existing ethtool infrastructure for this?

Thanks,
Jon

On Mon, May 15, 2006 at 11:32:14AM +0800, Wen Hsin Chang wrote:
 This patch is created from pcnet32.c v1.32. it will allow users to 
 specify RX ring size upon module
 insertion via module parameter 'rx_log_size'. This is needed in some 
 cases where too small the rx ring
 size will cause RX errors upon remote installation via pcnet32 NIC card.
 
 Signed-off-by: Wen Hsin Chang [EMAIL PROTECTED]
 
 
 --- a/drivers/net/pcnet32.c2006-03-30 09:49:10.0 +0800
 +++ b/drivers/net/pcnet32.c2006-05-15 11:14:45.0 +0800
 @@ -93,6 +93,9 @@ static struct net_device *pcnet32_dev;
 static int max_interrupt_work = 2;
 static int rx_copybreak = 200;
 
 +/* Module parameter to specify RX ring size at module insertion */
 +static int rx_log_size = 0;
 +
 #define PCNET32_PORT_AUI  0x00
 #define PCNET32_PORT_10BT 0x01
 #define PCNET32_PORT_GPSI 0x02
 @@ -1264,7 +1267,10 @@ pcnet32_probe1(unsigned long ioaddr, int
 lp-name = chipname;
 lp-shared_irq = shared;
 lp-tx_ring_size = TX_RING_SIZE;/* default tx ring size */
 -lp-rx_ring_size = RX_RING_SIZE;/* default rx ring size */
 +if (!rx_log_size)
 +lp-rx_ring_size = RX_RING_SIZE;/* default rx ring size */
 +else
 +lp-rx_ring_size = (1  (rx_log_size));
 lp-tx_mod_mask = lp-tx_ring_size - 1;
 lp-rx_mod_mask = lp-rx_ring_size - 1;
 lp-tx_len_bits = (PCNET32_LOG_TX_BUFFERS  12);
 @@ -2707,6 +2713,11 @@ module_param(tx_start_pt, int, 0);
 MODULE_PARM_DESC(tx_start_pt, DRV_NAME  transmit start point (0-3));
 module_param(pcnet32vlb, int, 0);
 MODULE_PARM_DESC(pcnet32vlb, DRV_NAME  Vesa local bus (VLB) support 
 (0/1));
 +
 +/* Module parameter to specify RX ring size at module insertion */
 +module_param(rx_log_size, int, 0);
 +MODULE_PARM_DESC(rx_log_size, DRV_NAME  RX Ring Buffer Size (log_2 
 #BUF) );
 +
 module_param_array(options, int, NULL, 0);
 MODULE_PARM_DESC(options, DRV_NAME  initial option setting(s) (0-15));
 module_param_array(full_duplex, int, NULL, 0);
 @@ -2731,6 +2742,12 @@ static int __init pcnet32_init_module(vo
 
 if ((tx_start_pt = 0)  (tx_start_pt = 3))
 tx_start = tx_start_pt;
 +   
 +/* validating rx_log_size */
 +if ((rx_log_size = 0) ||
 +(rx_log_size  PCNET32_LOG_MAX_RX_BUFFERS))
 +rx_log_size = 0;
 +   
 
 /* find the PCI devices */
 if (!pci_module_init(pcnet32_driver))
 
 -
 To unsubscribe from this list: send the line unsubscribe netdev in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -mm] dl2k gcc 4.1 warning fix

[Jon Mason]

On Tue, May 09, 2006 at 07:55:57PM -0700, Daniel Walker wrote:
 Fixes the following warning,

Please CC netdev on networking patches.

All the changed lines are over 80 chars.  Please fix.

Thanks,
Jon

 drivers/net/dl2k.c: In function 'rio_free_tx':
 drivers/net/dl2k.c:768: warning: integer constant is too large for 'long' type
 drivers/net/dl2k.c: In function 'receive_packet':
 drivers/net/dl2k.c:896: warning: integer constant is too large for 'long' type
 drivers/net/dl2k.c:904: warning: integer constant is too large for 'long' type
 drivers/net/dl2k.c:916: warning: integer constant is too large for 'long' type
 drivers/net/dl2k.c: In function 'rio_close':
 drivers/net/dl2k.c:1803: warning: integer constant is too large for 'long' 
 type
 drivers/net/dl2k.c:1813: warning: integer constant is too large for 'long' 
 type
 
 Signed-Off-By: Daniel Walker [EMAIL PROTECTED]
 
 Index: linux-2.6.16/drivers/net/dl2k.c
 ===
 --- linux-2.6.16.orig/drivers/net/dl2k.c
 +++ linux-2.6.16/drivers/net/dl2k.c
 @@ -765,7 +765,7 @@ rio_free_tx (struct net_device *dev, int
   break;
   skb = np-tx_skbuff[entry];
   pci_unmap_single (np-pdev,
 -   np-tx_ring[entry].fraginfo  0x,
 +   np-tx_ring[entry].fraginfo  
 0xULL,
 skb-len, PCI_DMA_TODEVICE);
   if (irq)
   dev_kfree_skb_irq (skb);
 @@ -893,7 +893,7 @@ receive_packet (struct net_device *dev)
   /* Small skbuffs for short packets */
   if (pkt_len  copy_thresh) {
   pci_unmap_single (np-pdev,
 -   desc-fraginfo  
 0x,
 +   desc-fraginfo  
 0xULL,
 np-rx_buf_sz,
 PCI_DMA_FROMDEVICE);
   skb_put (skb = np-rx_skbuff[entry], pkt_len);
 @@ -901,7 +901,7 @@ receive_packet (struct net_device *dev)
   } else if ((skb = dev_alloc_skb (pkt_len + 2)) != NULL) 
 {
   pci_dma_sync_single_for_cpu(np-pdev,
   desc-fraginfo  
 - 0x,
 + 
 0xULL,
   np-rx_buf_sz,
   PCI_DMA_FROMDEVICE);
   skb-dev = dev;
 @@ -913,7 +913,7 @@ receive_packet (struct net_device *dev)
   skb_put (skb, pkt_len);
   pci_dma_sync_single_for_device(np-pdev,
  desc-fraginfo 
 -  0x,
 +  
 0xULL,
  np-rx_buf_sz,
  
 PCI_DMA_FROMDEVICE);
   }
 @@ -1800,7 +1800,7 @@ rio_close (struct net_device *dev)
   skb = np-rx_skbuff[i];
   if (skb) {
   pci_unmap_single(np-pdev, 
 -  np-rx_ring[i].fraginfo  
 0x,
 +  np-rx_ring[i].fraginfo  
 0xULL,
skb-len, PCI_DMA_FROMDEVICE);
   dev_kfree_skb (skb);
   np-rx_skbuff[i] = NULL;
 @@ -1810,7 +1810,7 @@ rio_close (struct net_device *dev)
   skb = np-tx_skbuff[i];
   if (skb) {
   pci_unmap_single(np-pdev, 
 -  np-tx_ring[i].fraginfo  
 0x,
 +  np-tx_ring[i].fraginfo  
 0xULL,
skb-len, PCI_DMA_TODEVICE);
   dev_kfree_skb (skb);
   np-tx_skbuff[i] = NULL;
 -
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dl2k: use explicit DMA_48BIT_MASK

[Jon Mason]

On Wed, May 10, 2006 at 08:57:18PM +0200, Francois Romieu wrote:
 Typo will be harder with this one.

While I agree that a #define is much better than the magic number, I
think this is bastardizing the intended use of DMA_*BIT_MASK.
DMA_*BIT_MASK is intended to be used in the DMA_API's checking of
DMA controller's addressable memory, where as this is masking off the
lower 48bits of a descriptor for its DMA address.  

I think a better solution (which I should've done when I pushed the
original patch) would be a driver specific #define.  

Thanks,
Jon

 
 Signed-off-by: Francois Romieu [EMAIL PROTECTED]
 
 ---
 
  drivers/net/dl2k.c  |   13 ++---
  include/linux/dma-mapping.h |1 +
  2 files changed, 7 insertions(+), 7 deletions(-)
 
 5019a27a2a4e259f29a7bd03e905764eedfa034c
 diff --git a/drivers/net/dl2k.c b/drivers/net/dl2k.c
 index ca73f07..18d67cf 100644
 --- a/drivers/net/dl2k.c
 +++ b/drivers/net/dl2k.c
 @@ -765,8 +765,7 @@ rio_free_tx (struct net_device *dev, int
   break;
   skb = np-tx_skbuff[entry];
   pci_unmap_single (np-pdev,
 -   np-tx_ring[entry].fraginfo  
 - 0xULL,
 +   np-tx_ring[entry].fraginfo  DMA_48BIT_MASK,
 skb-len, PCI_DMA_TODEVICE);
   if (irq)
   dev_kfree_skb_irq (skb);
 @@ -895,7 +894,7 @@ receive_packet (struct net_device *dev)
   if (pkt_len  copy_thresh) {
   pci_unmap_single (np-pdev,
 desc-fraginfo  
 - 0xULL,
 + DMA_48BIT_MASK,
 np-rx_buf_sz,
 PCI_DMA_FROMDEVICE);
   skb_put (skb = np-rx_skbuff[entry], pkt_len);
 @@ -903,7 +902,7 @@ receive_packet (struct net_device *dev)
   } else if ((skb = dev_alloc_skb (pkt_len + 2)) != NULL) 
 {
   pci_dma_sync_single_for_cpu(np-pdev,
   desc-fraginfo  
 -   0xULL,
 +   DMA_48BIT_MASK,
   np-rx_buf_sz,
   PCI_DMA_FROMDEVICE);
   skb-dev = dev;
 @@ -915,7 +914,7 @@ receive_packet (struct net_device *dev)
   skb_put (skb, pkt_len);
   pci_dma_sync_single_for_device(np-pdev,
 desc-fraginfo  
 -   0xULL,
 +   DMA_48BIT_MASK,
  np-rx_buf_sz,
  
 PCI_DMA_FROMDEVICE);
   }
 @@ -1803,7 +1802,7 @@ rio_close (struct net_device *dev)
   if (skb) {
   pci_unmap_single(np-pdev, 
np-rx_ring[i].fraginfo  
 - 0xULL,
 + DMA_48BIT_MASK,
skb-len, PCI_DMA_FROMDEVICE);
   dev_kfree_skb (skb);
   np-rx_skbuff[i] = NULL;
 @@ -1814,7 +1813,7 @@ rio_close (struct net_device *dev)
   if (skb) {
   pci_unmap_single(np-pdev, 
np-tx_ring[i].fraginfo  
 - 0xULL,
 + DMA_48BIT_MASK,
skb-len, PCI_DMA_TODEVICE);
   dev_kfree_skb (skb);
   np-tx_skbuff[i] = NULL;
 diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
 index ff61817..635690c 100644
 --- a/include/linux/dma-mapping.h
 +++ b/include/linux/dma-mapping.h
 @@ -14,6 +14,7 @@ enum dma_data_direction {
  };
  
  #define DMA_64BIT_MASK   0xULL
 +#define DMA_48BIT_MASK   0xULL
  #define DMA_40BIT_MASK   0x00ffULL
  #define DMA_39BIT_MASK   0x007fULL
  #define DMA_32BIT_MASK   0xULL
 -- 
 1.3.1
 
 -
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  

[PATCH] dl2k: DMA freeing error

[Jon Mason]

This patch fixes an error in the dl2k driver's DMA mapping/unmapping.
The adapter uses the upper 16bits of the DMA address for the buffer
size.  However, this is not masked off when referencing the DMA 
address, and can lead to errors by trying to free a DMA address out of
range.

Thanks,
Jon

Signed-off-by: Jon Mason [EMAIL PROTECTED]

diff -r c698c94ff82e drivers/net/dl2k.c
--- a/drivers/net/dl2k.cThu Mar  9 15:03:49 2006
+++ b/drivers/net/dl2k.cFri Mar 10 08:48:50 2006
@@ -50,8 +50,8 @@
 
 */
 #define DRV_NAME   D-Link DL2000-based linux driver
-#define DRV_VERSIONv1.17a
-#define DRV_RELDATE2002/10/04
+#define DRV_VERSIONv1.17b
+#define DRV_RELDATE2006/03/10
 #include dl2k.h
 
 static char version[] __devinitdata =
@@ -765,7 +765,7 @@
break;
skb = np-tx_skbuff[entry];
pci_unmap_single (np-pdev,
- np-tx_ring[entry].fraginfo,
+ np-tx_ring[entry].fraginfo  0x,
  skb-len, PCI_DMA_TODEVICE);
if (irq)
dev_kfree_skb_irq (skb);
@@ -892,14 +892,16 @@
 
/* Small skbuffs for short packets */
if (pkt_len  copy_thresh) {
-   pci_unmap_single (np-pdev, desc-fraginfo,
+   pci_unmap_single (np-pdev,
+ desc-fraginfo0x,
  np-rx_buf_sz,
  PCI_DMA_FROMDEVICE);
skb_put (skb = np-rx_skbuff[entry], pkt_len);
np-rx_skbuff[entry] = NULL;
} else if ((skb = dev_alloc_skb (pkt_len + 2)) != NULL) 
{
pci_dma_sync_single_for_cpu(np-pdev,
-   desc-fraginfo,
+   desc-fraginfo  
+   0x,
np-rx_buf_sz,
PCI_DMA_FROMDEVICE);
skb-dev = dev;
@@ -910,7 +912,8 @@
  pkt_len, 0);
skb_put (skb, pkt_len);
pci_dma_sync_single_for_device(np-pdev,
-  desc-fraginfo,
+  desc-fraginfo 
+0x,
   np-rx_buf_sz,
   
PCI_DMA_FROMDEVICE);
}
@@ -1796,8 +1799,9 @@
np-rx_ring[i].fraginfo = 0;
skb = np-rx_skbuff[i];
if (skb) {
-   pci_unmap_single (np-pdev, np-rx_ring[i].fraginfo,
- skb-len, PCI_DMA_FROMDEVICE);
+   pci_unmap_single(np-pdev, 
+np-rx_ring[i].fraginfo0x,
+skb-len, PCI_DMA_FROMDEVICE);
dev_kfree_skb (skb);
np-rx_skbuff[i] = NULL;
}
@@ -1805,8 +1809,9 @@
for (i = 0; i  TX_RING_SIZE; i++) {
skb = np-tx_skbuff[i];
if (skb) {
-   pci_unmap_single (np-pdev, np-tx_ring[i].fraginfo,
- skb-len, PCI_DMA_TODEVICE);
+   pci_unmap_single(np-pdev, 
+np-tx_ring[i].fraginfo0x,
+skb-len, PCI_DMA_TODEVICE);
dev_kfree_skb (skb);
np-tx_skbuff[i] = NULL;
}
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: help with kernel crasing in skb_over_panic

[Jon Mason]

On Thu, Mar 09, 2006 at 10:06:29AM -0600, Kumar Gala wrote:
 I was hoping someone might have some ideas on what might have happened 
 with the following oops/BUG().  This is on an embedded PPC running 
 2.6.16-rc5.  From the description I got from my coworker who say this, he 
 was doing NFS on the system and at the time the oops occured his desktop 
 linux box X froze up as well.  So some odd network activity.
 
 I just looking for some direction at things to look at or a better 
 understanding of what might have gone wrong.

The driver wrote beyond the end of the skb.  This was caused by the skb
being to small for your payload.  If this is reproduceable, verify that
your packet's sizes are = skb size.  

Thanks,
Jon

 
 Thanks
 
 - kumar
 
 
 skb_over_panic: text:c015d25c len:65580 put:65532 head:c7279000 data:c727904e 
 t0
 
 kernel BUG in skb_over_panic at net/core/skbuff.c:94!
 
 Oops: Exception in kernel mode, sig: 5 [#1]
 
 PREEMPT
 
 Modules linked in:
 
 NIP: C01B6E98 LR: C01B6E98 CTR: 
 REGS: c900 TRAP: 0700   Not tainted  (2.6.16-rc5.p0.8.0)
 MSR: 00029032 EE,ME,IR,DR  CR: 24002422  XER: 
 TASK = cfb0d110[1079] 'tar' THREAD: c90dc000
 
 GPR00: C01B6E98 C90DDE80 CFB0D110 0075 23F2  C032 C02BD36C
 GPR08: C02C   C032  100A6704 0FFAD60C 100A
 GPR16: 100A 100A 0007 100A 100A 100A 007F C02C
 GPR24: 0001 FFFC CFBC5000 003F CF85E2B8  CF91DDE0 CFBC5260
 
 NIP [C01B6E98] skb_over_panic+0x50/0x64
 LR [C01B6E98] skb_over_panic+0x50/0x64
 
 Call Trace:
 
 [C90DDE80] [C01B6E98] skb_over_panic+0x50/0x64 (unreliable)
 [C90DDE90] [C015D270] gfar_clean_rx_ring+0x524/0x61c
 [C90DDED0] [C015D3AC] gfar_receive+0x44/0xa8
 [C90DDEE0] [C003CE70] handle_IRQ_event+0x64/0xdc
 [C90DDF00] [C003D004] __do_IRQ+0x11c/0x1fc
 [C90DDF20] [C000534C] do_IRQ+0x48/0x8c
 [C90DDF40] [C000DB74] ret_from_except+0x0/0x14
 
 --- Exception: 501 at 0x10005f98
 
 LR = 0x10005f98
 
 Instruction dump:
 
 80eb0084 2f80 80ab0060 386343f8 810b0088 812b008c 814b0090 409e000c
 3d60c027 380b58bc 90010008 4be63435 0fe0 80010014 38210010 7c0803a6
 
  0Kernel panic - not syncing: Fatal exception in interrupt
 
  
 
 I was doing a lot of NFS  flash I/O at the time.
 
  
 
 
 -
 To unsubscribe from this list: send the line unsubscribe netdev in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] trivial: fix spelling errors in Kconfigs

[Jon Mason]

Resubmission of patch originally sent on November 17, modified to apply
cleanly to 2.6.16

This patch corrects a few spelling and grammar errors found in 
drivers/net

Signed-off-by: Jon Mason [EMAIL PROTECTED]

diff -r 2fa13972604f drivers/net/Kconfig
--- a/drivers/net/Kconfig   Wed Feb  8 17:58:27 2006
+++ b/drivers/net/Kconfig   Sun Feb 12 11:19:21 2006
@@ -66,7 +66,7 @@
  'Trunking' by Sun, 802.3ad by the IEEE, and 'Bonding' in Linux.
 
  The driver supports multiple bonding modes to allow for both high
- perfomance and high availability operation.
+ performance and high availability operation.
 
  Refer to file:Documentation/networking/bonding.txt for more
  information.
@@ -698,8 +698,8 @@
depends on NET_VENDOR_3COM  (PCI || EISA)
select MII
---help---
- This option enables driver support for a large number of 10mbps and
- 10/100mbps EISA, PCI and PCMCIA 3Com network cards:
+ This option enables driver support for a large number of 10Mbps and
+ 10/100Mbps EISA, PCI and PCMCIA 3Com network cards:
 
  Vortex(Fast EtherLink 3c590/3c592/3c595/3c597) EISA and PCI
  Boomerang (EtherLink XL 3c900 or 3c905)PCI
@@ -1021,7 +1021,7 @@
depends on NET_ISA
---help---
  If you have a network (Ethernet) card of this type, say Y. This
- driver supports intel i82595{FX,TX} based boards. Note however
+ driver supports Intel i82595{FX,TX} based boards. Note however
  that the EtherExpress PRO/100 Ethernet card has its own separate
  driver.  Please read the Ethernet-HOWTO, available from
  http://www.tldp.org/docs.html#howto.
@@ -1207,7 +1207,7 @@
help
  Additional receive skb headroom. Note, that driver
  will always reserve at least 2 bytes to make IP header
- aligned, so usualy there is no need to add any additional
+ aligned, so usually there is no need to add any additional
  headroom.
  
  If unsure, set to 0.
@@ -1933,7 +1933,7 @@
  will be called myri_sbus.  This is recommended.
 
 config NS83820
-   tristate National Semiconduct DP83820 support
+   tristate National Semiconductor DP83820 support
depends on PCI
help
  This is a driver for the National Semiconductor DP83820 series
@@ -2514,7 +2514,7 @@
  Say Y here if you want to be able to filter the packets passing over
  PPP interfaces.  This allows you to control which packets count as
  activity (i.e. which packets will reset the idle timer or bring up
- a demand-dialled link) and which packets are to be dropped entirely.
+ a demand-dialed link) and which packets are to be dropped entirely.
  You need to say Y here if you wish to use the pass-filter and
  active-filter options to pppd.
 
@@ -2702,8 +2702,8 @@
  file:Documentation/networking/shaper.txt for more information.
 
  An alternative to this traffic shaper is the experimental
- Class-Based Queueing (CBQ) scheduling support which you get if you
- say Y to QoS and/or fair queueing above.
+ Class-Based Queuing (CBQ) scheduling support which you get if you
+ say Y to QoS and/or fair queuing above.
 
  To compile this driver as a module, choose M here: the module
  will be called shaper.  If unsure, say N.
diff -r 2fa13972604f drivers/net/arcnet/Kconfig
--- a/drivers/net/arcnet/KconfigWed Feb  8 17:58:27 2006
+++ b/drivers/net/arcnet/KconfigSun Feb 12 11:19:21 2006
@@ -68,10 +68,10 @@
   packet is stuffed with an extra 4 byte cookie which doesn't
   actually appear on the network. After transmit the driver will send
   back a packet with protocol byte 0 containing the status of the
-  transmition:
+  transmission:
  0=no hardware acknowledge
  1=excessive nak
- 2=transmition accepted by the reciever hardware
+ 2=transmission accepted by the receiver hardware
 
   Received packets are also stuffed with the extra 4 bytes but it will
   be random data.
diff -r 2fa13972604f drivers/net/irda/Kconfig
--- a/drivers/net/irda/Kconfig  Wed Feb  8 17:58:27 2006
+++ b/drivers/net/irda/Kconfig  Sun Feb 12 11:19:21 2006
@@ -283,7 +283,7 @@
  Say Y here if you want to build support for the USB IrDA FIR Dongle
  device driver.  To compile it as a module, choose M here: the module
  will be called irda-usb.  IrDA-USB support the various IrDA USB
- dongles available and most of their pecularities.  Those dongles
+ dongles available and most of their peculiarities.  Those dongles
  plug in the USB port of your computer, are plug and play, and
  support SIR and FIR (4Mbps) speeds.  On the other hand, those
  dongles

[PATCH] pcnet32: Add PCI_DEVICE macro to pcnet32_pci_tbl

[Jon Mason]

This patch adds the PCI_DEVICE macro to the pcnet32 driver.

This has been tested on my opteron with my trident adapter.

Thanks,
Jon

Signed-off-by: Jon Mason [EMAIL PROTECTED]

diff -r 4a7597b41d25 drivers/net/pcnet32.c
--- a/drivers/net/pcnet32.c Wed Jan 11 19:14:08 2006
+++ b/drivers/net/pcnet32.c Thu Jan 12 15:22:57 2006
@@ -58,13 +58,13 @@
  * PCI device identifiers for new style Linux PCI Device Drivers
  */
 static struct pci_device_id pcnet32_pci_tbl[] = {
-{ PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_LANCE_HOME, PCI_ANY_ID, PCI_ANY_ID, 
0, 0, 0 },
-{ PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_LANCE, PCI_ANY_ID, PCI_ANY_ID, 0, 
0, 0 },
+{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_LANCE_HOME), 0, 0, 0},
+{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_LANCE), 0, 0, 0},
 /*
  * Adapters that were sold with IBM's RS/6000 or pSeries hardware have
  * the incorrect vendor id.
  */
-{ PCI_VENDOR_ID_TRIDENT, PCI_DEVICE_ID_AMD_LANCE, PCI_ANY_ID, PCI_ANY_ID,
+{ PCI_DEVICE(PCI_VENDOR_ID_TRIDENT, PCI_DEVICE_ID_AMD_LANCE), 
PCI_CLASS_NETWORK_ETHERNET  8, 0x00, 0 },
 { 0, }
 };
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] [PATCH 0/3] ioat: DMA engine support

[Jon Mason]

On Wed, Nov 23, 2005 at 05:45:36PM -0500, Jeff Garzik wrote:
 Andrew Grover wrote:
 As presented in our talk at this year's OLS, the Bensley platform, which 
 will be out in early 2006, will have an asyncronous DMA engine. It can be 
 used to offload copies from the CPU, such as the kernel copies of received 
 packets into the user buffer.
 
 More than a one-paragraph description would be nice...  URLs to OLS and 
 IDF presentations, other info?
 
   Jeff
 
FYI,
OLS paper can be found at
http://www.linuxsymposium.org/2005/linuxsymposium_procv1.pdf
Starting at page 281.

Other info can be found at
http://www.intel.com/technology/ioacceleration/index.htm
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] fix spelling errors in netdev Kconfigs

[Jon Mason]

I ran aspell on the Kconfigs in drivers/net/ and found the following
spelling errors.

Signed-off-by: Jon Mason [EMAIL PROTECTED]

diff -r 971dbac4c501 drivers/net/Kconfig
--- a/drivers/net/Kconfig   Wed Nov 16 16:25:28 2005
+++ b/drivers/net/Kconfig   Thu Nov 17 08:24:31 2005
@@ -55,7 +55,7 @@
  'Trunking' by Sun, 802.3ad by the IEEE, and 'Bonding' in Linux.
 
  The driver supports multiple bonding modes to allow for both high
- perfomance and high availability operation.
+ performance and high availability operation.
 
  Refer to file:Documentation/networking/bonding.txt for more
  information.
@@ -687,8 +687,8 @@
depends on NET_VENDOR_3COM  (PCI || EISA)
select MII
---help---
- This option enables driver support for a large number of 10mbps and
- 10/100mbps EISA, PCI and PCMCIA 3Com network cards:
+ This option enables driver support for a large number of 10Mbps and
+ 10/100Mbps EISA, PCI and PCMCIA 3Com network cards:
 
  Vortex(Fast EtherLink 3c590/3c592/3c595/3c597) EISA and PCI
  Boomerang (EtherLink XL 3c900 or 3c905)PCI
@@ -1010,7 +1010,7 @@
depends on NET_ISA
---help---
  If you have a network (Ethernet) card of this type, say Y. This
- driver supports intel i82595{FX,TX} based boards. Note however
+ driver supports Intel i82595{FX,TX} based boards. Note however
  that the EtherExpress PRO/100 Ethernet card has its own separate
  driver.  Please read the Ethernet-HOWTO, available from
  http://www.tldp.org/docs.html#howto.
@@ -1196,7 +1196,7 @@
help
  Additional receive skb headroom. Note, that driver
  will always reserve at least 2 bytes to make IP header
- aligned, so usualy there is no need to add any additional
+ aligned, so usually there is no need to add any additional
  headroom.
  
  If unsure, set to 0.
@@ -1911,7 +1911,7 @@
  will be called myri_sbus.  This is recommended.
 
 config NS83820
-   tristate National Semiconduct DP83820 support
+   tristate National Semiconductor DP83820 support
depends on PCI
help
  This is a driver for the National Semiconductor DP83820 series
@@ -2459,7 +2459,7 @@
  Say Y here if you want to be able to filter the packets passing over
  PPP interfaces.  This allows you to control which packets count as
  activity (i.e. which packets will reset the idle timer or bring up
- a demand-dialled link) and which packets are to be dropped entirely.
+ a demand-dialed link) and which packets are to be dropped entirely.
  You need to say Y here if you wish to use the pass-filter and
  active-filter options to pppd.
 
@@ -2647,8 +2647,8 @@
  file:Documentation/networking/shaper.txt for more information.
 
  An alternative to this traffic shaper is the experimental
- Class-Based Queueing (CBQ) scheduling support which you get if you
- say Y to QoS and/or fair queueing above.
+ Class-Based Queuing (CBQ) scheduling support which you get if you
+ say Y to QoS and/or fair queuing above.
 
  To set up and configure shaper devices, you need the shapecfg
  program, available from ftp://shadow.cabi.net/pub/Linux/ in the
diff -r 971dbac4c501 drivers/net/arcnet/Kconfig
--- a/drivers/net/arcnet/KconfigWed Nov 16 16:25:28 2005
+++ b/drivers/net/arcnet/KconfigThu Nov 17 08:24:31 2005
@@ -68,10 +68,10 @@
   packet is stuffed with an extra 4 byte cookie which doesn't
   actually appear on the network. After transmit the driver will send
   back a packet with protocol byte 0 containing the status of the
-  transmition:
+  transmission:
  0=no hardware acknowledge
  1=excessive nak
- 2=transmition accepted by the reciever hardware
+ 2=transmission accepted by the receiver hardware
 
   Received packets are also stuffed with the extra 4 bytes but it will
   be random data.
diff -r 971dbac4c501 drivers/net/irda/Kconfig
--- a/drivers/net/irda/Kconfig  Wed Nov 16 16:25:28 2005
+++ b/drivers/net/irda/Kconfig  Thu Nov 17 08:24:31 2005
@@ -284,7 +284,7 @@
  Say Y here if you want to build support for the USB IrDA FIR Dongle
  device driver.  To compile it as a module, choose M here: the module
  will be called irda-usb.  IrDA-USB support the various IrDA USB
- dongles available and most of their pecularities.  Those dongles
+ dongles available and most of their peculiarities.  Those dongles
  plug in the USB port of your computer, are plug and play, and
  support SIR and FIR (4Mbps) speeds.  On the other hand, those
  dongles tend to be less efficient than a FIR chipset.
@@ -361,7 +361,7