* Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device
From: John Fastabend @ 2011-10-19 20:53 UTC (permalink / raw)
To: David Miller
Cc: mchan@broadcom.com, netdev@vger.kernel.org, dmitry@broadcom.com,
eilong@broadcom.com
In-Reply-To: <20111019.164702.1494656842101009040.davem@davemloft.net>
On 10/19/2011 1:47 PM, David Miller wrote:
> From: "Michael Chan" <mchan@broadcom.com>
> Date: Wed, 19 Oct 2011 13:12:52 -0700
>
>>
>> On Wed, 2011-10-19 at 13:06 -0700, David Miller wrote:
>>> From: "Michael Chan" <mchan@broadcom.com>
>>> Date: Thu, 13 Oct 2011 20:38:01 -0700
>>>
>>>> From: Dmitry Kravkov <dmitry@broadcom.com>
>>>>
>>>> For an FCoE or iSCSI boot device, the networking side must stay "up" all
>>>> the time. Otherwise, the FCoE/iSCSI interface driven by bnx2i/bnx2fc
>>>> will be reset and we'll lose the root file system.
>>>>
>>>> If LRO is enabled, scripts that enable IP forwarding or bridging will
>>>> disable LRO and cause the device to be reset. Disabling LRO on these
>>>> boot devices will prevent the reset.
>>>>
>>>> Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
>>>> Signed-off-by: Michael Chan <mchan@broadcom.com>
>>>
>>> You're still going to have bugs after this.
>>>
>>> What if you get a FIFO overflow or other error condition which requires
>>> a chip reset? You'll lose the root filesystem.
>>
>> That would be no different than a scsi driver experiencing fatal errors,
>> wouldn't it?
>
> It's not fatal if you can bring the chip back up after the reset
> because this is networking.
>
> These things are protocols, built on top of networking technology,
> with retransmits, handshakes, and all sorts of features designed
> to provide reliability.
>
> Things like a LRO change ought to be completely transparent.
As a reference point this works fine in both FCoE and iSCSI stacks
today. The device is reset or link is lost for whatever reason
when the link comes back up the stack logs back in, enumerates
the luns and the scsi stack recovers as expected.
Firmware should do the equivalent login, lun enumeration, etc as
needed.
.John
^ permalink raw reply
* Re: [PATCH net-next] tcp: use TCP_INIT_CWND in tcp_fixup_sndbuf()
From: David Miller @ 2011-10-19 20:55 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1318566282.2533.63.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 14 Oct 2011 06:24:42 +0200
> Initial cwnd being 10 (TCP_INIT_CWND) instead of 3, change
> tcp_fixup_sndbuf() to get more than 16384 bytes (sysctl_tcp_wmem[1]) in
> initial sk_sndbuf
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric. Another reason to obliterate all magic constants :-)
> I believe a similar change in tcp_fixup_rcvbuf() is needed too.
And tcp_init_buffer_space() has a similar set of "4 * tp->advmss"
calculations.
And then some "2 * tp->advmss" cases, also having to do with window
clamping.
^ permalink raw reply
* Re: [PATCH] r8169: fix wrong eee setting for rlt8111evl
From: David Miller @ 2011-10-19 20:57 UTC (permalink / raw)
To: hayeswang; +Cc: romieu, netdev, linux-kernel
In-Reply-To: <1318572877-1549-1-git-send-email-hayeswang@realtek.com>
From: Hayes Wang <hayeswang@realtek.com>
Date: Fri, 14 Oct 2011 14:14:37 +0800
> Correct the wrong parameter for setting EEE for RTL8111E-VL.
>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
Francois, want me to apply this directly or will you handle it?
^ permalink raw reply
* Re: [PATCH] net: Move rcu_barrier from rollback_registered_many to netdev_run_todo.
From: David Miller @ 2011-10-19 20:59 UTC (permalink / raw)
To: ebiederm; +Cc: eric.dumazet, netdev
In-Reply-To: <m1r52g7zf0.fsf@fess.ebiederm.org>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Fri, 14 Oct 2011 01:25:23 -0700
>
> This patch moves the rcu_barrier from rollback_registered_many
> (inside the rtnl_lock) into netdev_run_todo (just outside the rtnl_lock).
> This allows us to gain the full benefit of sychronize_net calling
> synchronize_rcu_expedited when the rtnl_lock is held.
>
> The rcu_barrier in rollback_registered_many was originally a synchronize_net
> but was promoted to be a rcu_barrier() when it was found that people were
> unnecessarily hitting the 250ms wait in netdev_wait_allrefs(). Changing
> the rcu_barrier back to a synchronize_net is therefore safe.
>
> Since we only care about waiting for the rcu callbacks before we get
> to netdev_wait_allrefs() it is also safe to move the wait into
> netdev_run_todo.
>
> This was tested by creating and destroying 1000 tap devices and observing
> /proc/lock_stat. /proc/lock_stat reports this change reduces the hold
> times of the rtnl_lock by a factor of 10. There was no observable
> difference in the amount of time it takes to destroy a network device.
>
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Applied to net-next, thanks Eric.
^ permalink raw reply
* Re: [PATCH net-next 1/1] net: validate HWTSTAMP ioctl parameters
From: David Miller @ 2011-10-19 21:01 UTC (permalink / raw)
To: richardcochran; +Cc: netdev
In-Reply-To: <eca6d279cd96da44d9ad26bdda8fc8c2130c03c1.1318584677.git.richard.cochran@omicron.at>
From: Richard Cochran <richardcochran@gmail.com>
Date: Fri, 14 Oct 2011 11:37:48 +0200
> This patch adds a sanity check on the values provided by user space for
> the hardware time stamping configuration. If the values lie outside of
> the absolute limits, then the ioctl request will be denied.
>
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
Thanks a lot for following up on this.
Applied, thanks again Richard.
^ permalink raw reply
* Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device
From: David Miller @ 2011-10-19 21:03 UTC (permalink / raw)
To: john.r.fastabend; +Cc: mchan, netdev, dmitry, eilong
In-Reply-To: <4E9F38D6.6030700@intel.com>
From: John Fastabend <john.r.fastabend@intel.com>
Date: Wed, 19 Oct 2011 13:53:42 -0700
> As a reference point this works fine in both FCoE and iSCSI stacks
> today. The device is reset or link is lost for whatever reason
> when the link comes back up the stack logs back in, enumerates
> the luns and the scsi stack recovers as expected.
>
> Firmware should do the equivalent login, lun enumeration, etc as
> needed.
I rest my case :-)
^ permalink raw reply
* RE: [net-next-2.6 PATCH 0/8 RFC v2] macvlan: MAC Address filtering support for passthru mode
From: Rose, Gregory V @ 2011-10-19 21:06 UTC (permalink / raw)
To: Roopa Prabhu, netdev@vger.kernel.org
Cc: sri@us.ibm.com, dragos.tatulea@gmail.com, arnd@arndb.de,
kvm@vger.kernel.org, mst@redhat.com, davem@davemloft.net,
mchan@broadcom.com, dwang2@cisco.com, shemminger@vyatta.com,
eric.dumazet@gmail.com, kaber@trash.net, benve@cisco.com
In-Reply-To: <20111019062543.7242.3969.stgit@savbu-pc100.cisco.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Roopa Prabhu
> Sent: Tuesday, October 18, 2011 11:26 PM
> To: netdev@vger.kernel.org
> Cc: sri@us.ibm.com; dragos.tatulea@gmail.com; arnd@arndb.de;
> kvm@vger.kernel.org; mst@redhat.com; davem@davemloft.net;
> mchan@broadcom.com; dwang2@cisco.com; shemminger@vyatta.com;
> eric.dumazet@gmail.com; kaber@trash.net; benve@cisco.com
> Subject: [net-next-2.6 PATCH 0/8 RFC v2] macvlan: MAC Address filtering
> support for passthru mode
>
[snip...]
>
>
> Note: The choice of rtnl_link_ops was because I saw the use case for
> this in virtual devices that need to do filtering in sw like macvlan
> and tun. Hw devices usually have filtering in hw with netdev->uc and
> mc lists to indicate active filters. But I can move from rtnl_link_ops
> to netdev_ops if that is the preferred way to go and if there is a
> need to support this interface on all kinds of interfaces.
> Please suggest.
I'm still digesting the rest of the RFC patches but I did want to quickly jump
in and push for adding this support in netdev_ops. I would like to see these
features available in more devices than just macvtap and macvlan. I can conceive
of use cases for multiple HW MAC and VLAN filters for a VF device that isn't
owned by a macvlan/macvtap interface and only has netdev_ops support. In this
case it would be necessary to program the filters directly to the VF device
interface or PF interface (or lowerdev as you refer to it) instead of going
through macvlan/macvtap.
This work dovetails nicely with some work I've been doing and I'd be very interested
in helping move this forward if we could work out the details that would allow support
of the features we (and the community) require.
- Greg Rose
LAN Access Division
Intel Corp.
^ permalink raw reply
* Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
From: David Miller @ 2011-10-19 21:09 UTC (permalink / raw)
To: glommer
Cc: linux-kernel, akpm, lizf, kamezawa.hiroyu, ebiederm, paul,
gthelen, netdev, linux-mm, kirill, avagin, devel
In-Reply-To: <4E983177.5000005@parallels.com>
From: Glauber Costa <glommer@parallels.com>
Date: Fri, 14 Oct 2011 16:56:23 +0400
> Please let me know what do you think of the following patch. It is
> still crude, and applies on top of what I had, for simplicity. I can
> of course rework all the series for the next submission. The main idea
> is:
Thanks, I promise to look at this soon.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] r8169: fix driver shutdown WoL regression.
From: David Miller @ 2011-10-19 21:08 UTC (permalink / raw)
To: ballarin.marc; +Cc: romieu, netdev, hayeswang
In-Reply-To: <20111014141221.5736721ec0dc65de0537ac48@gmx.de>
From: Marc Ballarin <ballarin.marc@gmx.de>
Date: Fri, 14 Oct 2011 14:12:21 +0200
> On Fri, 14 Oct 2011 12:57:45 +0200
> Francois Romieu <romieu@fr.zoreil.com> wrote:
>
>> ...
>> Marc, can you add your Tested-by: to this patch ? It avoids the extra
>> phy writes which were included in the previous revision of the patch.
>> They did not happen before the regression so there is no reason to
>> sneak them in -release. Works ok with RTL_GIGA_MAC_VER_34.
>
> Works fine on RTL_GIGA_MAC_VER_33 as well.
>
> Tested-by: Marc Ballarin <ballarin.marc@gmx.de>
I'll apply this, thanks everyone.
^ permalink raw reply
* Re: [PATCH net-next 1/1] net: validate HWTSTAMP ioctl parameters
From: Ben Hutchings @ 2011-10-19 21:16 UTC (permalink / raw)
To: Richard Cochran; +Cc: netdev, David Miller
In-Reply-To: <eca6d279cd96da44d9ad26bdda8fc8c2130c03c1.1318584677.git.richard.cochran@omicron.at>
On Fri, 2011-10-14 at 11:37 +0200, Richard Cochran wrote:
> This patch adds a sanity check on the values provided by user space for
> the hardware time stamping configuration. If the values lie outside of
> the absolute limits, then the ioctl request will be denied.
[...]
What does this validation buy us? The driver still has to copy the
values into kernel space again, at which point they may have been
changed to be invalid. Depending on how the driver uses them (perhaps
as array indices), it may have to validate them again to avoid a
security vulnerability.
I think that either SIOCHWTSTAMP should be handled through a discrete
device operation (not ndo_ioctl) which receives a pointer to the
validated structure in kernel memory, or a validation function should be
exported to drivers so that they can call it from their ndo_ioctl
implementations after copying the structure into kernel memory.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: BUG: All network processes hang (brcmsmac/wpa_supplicant)
From: Nico Schottelius @ 2011-10-19 21:42 UTC (permalink / raw)
To: Arend van Spriel
Cc: Nico Schottelius, Eric Dumazet, LKML,
linux-wireless@vger.kernel.org, netdev
In-Reply-To: <4E9EFF51.6000104@broadcom.com>
[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]
Arend van Spriel [Wed, Oct 19, 2011 at 06:48:17PM +0200]:
> On 10/19/2011 04:55 PM, Nico Schottelius wrote:
> commit bee709ab1d390afe69e4407bc86bb706c6fb2965
> Merge: ad1c761 1f2c7e9
> Author: Nico Schottelius <nico@kr.ethz.ch>
> Date: Tue Oct 18 00:04:05 2011 +0200
>
> Merge branch 'fix-edp-vdd-power' of ../keithp/linux
>
> As it drops receive packets it may be your problem. Is your AP on 5GHz?
I've to confess I've no clue, but I guess the answer is "some of them":
Here in ETH Zurich (www.ethz.ch) I'm in an environment with a lot of
surrounding APs (probably 7 or more seen with the same ssid).
> From 7d14bd6cbfbf26369c5958e56a468fd8429841d7 Mon Sep 17 00:00:00 2001
> From: Arend van Spriel <arend@broadcom.com>
> Date: Wed, 19 Oct 2011 18:42:45 +0200
> Subject: [PATCH] staging: brcm80211: fix for rate index in receive status
Applied, recompiling, will test tomorrow.
I'll soon (Friday) leave the environment permanently,
thus testing will not be easily possible anymore.
If this issue is 5Ghz related only, can I somehow trigger it without an AP
running in that band / i.e. make the card try to use the 5 Ghz band although
there is no ap nearby?
Cheers,
Nico
--
PGP key: 7ED9 F7D3 6B10 81D7 0EC5 5C09 D7DC C8E4 3187 7DF0
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 07/10] RDMA/cxgb4: DB Drop Recovery for RDMA and LLD queues.
From: Roland Dreier @ 2011-10-19 22:12 UTC (permalink / raw)
To: Vipul Pandya
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, divy-ut6Up61K2wZBDgjK7y7TUQ,
dm-ut6Up61K2wZBDgjK7y7TUQ, kumaras-ut6Up61K2wZBDgjK7y7TUQ,
swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW
In-Reply-To: <1319044264-779-8-git-send-email-vipul-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
This looks like the only drivers/infiniband patch that depends on the
drivers/net changes?
- R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch net-next]alx: Atheros AR8131/AR8151/AR8152/AR8161 Ethernet driver
From: Francois Romieu @ 2011-10-19 22:21 UTC (permalink / raw)
To: cloud.ren; +Cc: davem, Luis.Rodriguez, netdev, linux-kernel
In-Reply-To: <1319009213-20627-1-git-send-email-cloud.ren@atheros.com>
cloud.ren@atheros.com <cloud.ren@atheros.com> :
> diff --git a/drivers/net/ethernet/atheros/alx/alc_cb.c b/drivers/net/ethernet/atheros/alx/alc_cb.c
[...]
> +int alc_read_phy_reg(struct alx_hw *hw, u32 device_type,
> + u16 reg_addr, u16 *phy_data)
> +{
> + bool fast = false;
> + bool ext = false;
> + u16 error;
> + int retval = 0;
> +
> + ALX_MDIO_LOCK(&hw->mdio_lock);
Frowned upon / useless wrapper.
> +
> + if (device_type != ALX_MDIO_NORM_DEV)
> + ext = true;
This method seems to be always called with device_type = ALX_MDIO_NORM_DEV.
> +
> + error = l1c_read_phy(hw, ext, device_type, fast,
> + reg_addr, phy_data);
Imho the driver wants something like l1c_read_phy_{fast/slow}.
> + if (error) {
> + HW_PRINT(ERR, "Error when reading phy reg (%d).", error);
> + retval = ALX_ERR_PHY_READ_REG;
I am a bit sceptical that private error codes are really useful
in a mundane ethernet driver.
[...]
> +int alc_write_phy_reg(struct alx_hw *hw, u32 device_type,
> + u16 reg_addr, u16 phy_data)
> +{
> + bool fast = false;
> + bool ext = false;
> + u16 error;
> + int retval = 0;
> +
> + ALX_MDIO_LOCK(&hw->mdio_lock);
> +
> + if (device_type != ALX_MDIO_NORM_DEV)
> + ext = true;
This method seems to be always called with device_type = ALX_MDIO_NORM_DEV.
> +
> + error = l1c_write_phy(hw, ext, device_type, fast,
> + reg_addr, phy_data);
It fits in a single 80 columns line.
[...]
> +int alc_init_phy(struct alx_hw *hw)
> +{
> + u16 phy_id[2];
> + int retval = 0;
Useless init.
> +
> + HW_PRINT(DEBUG, "ENTER\n");
Old school.
> +
> + /* 1. init mdio spin lock */
> + ALX_MDIO_LOCK_INIT(&hw->mdio_lock);
The comment adds no value.
> +
> + /* 2. read phy id */
> + retval = alc_read_phy_reg(hw, ALX_MDIO_NORM_DEV,
> + L1C_MII_PHYSID1, &phy_id[0]);
Sic.
> + if (retval)
> + return retval;
> + retval = alc_read_phy_reg(hw, ALX_MDIO_NORM_DEV,
> + L1C_MII_PHYSID1, &phy_id[1]);
> + if (retval)
> + return retval;
> +
> + memcpy(&hw->phy_id, phy_id, sizeof(hw->phy_id));
> +
> + hw->autoneg_advertised = (ALX_LINK_SPEED_1GB_FULL |
> + ALX_LINK_SPEED_10_HALF |
> + ALX_LINK_SPEED_10_FULL |
> + ALX_LINK_SPEED_100_HALF |
> + ALX_LINK_SPEED_100_FULL);
Parenthesis abuse.
[...]
> +int alc_ack_phy_intr(struct alx_hw *hw)
> +{
> + int retval = 0;
Useless init.
> + u16 isr = 0;
> +
> + HW_PRINT(DEBUG, "ENTER\n");
> +
> + retval = alc_read_phy_reg(hw, ALX_MDIO_NORM_DEV,
> + L1C_MII_ISR, &isr);
> + if (retval)
> + return retval;
> +
> + return retval;
The style is terrible. What about:
return alc_read_phy_reg(hw, ALX_MDIO_NORM_DEV, L1C_MII_ISR, &isr);
[...]
> +int alc_clear_mac_addr(struct alx_hw *hw)
> +{
> + int retval = 0;
> +
> + HW_PRINT(DEBUG, "ENTER\n");
> +
> + return retval;
> +}
{alc/alf}_clear_mac_addr always returns 0 and it does not seem to do
anything else.
[...]
> +int alc_config_rx(struct alx_hw *hw)
> +{
> + int retval = 0;
> + return retval;
> +}
Useless. {alc/alf}_config_rx always returns 0 and it does not anything else.
[...]
> +int alc_check_nvram(struct alx_hw *hw, bool *exist)
> +{
> + *exist = false;
> + return 0;
> +}
Strictly identical to alf_check_nvram.
[...]
> +int alc_get_ethtool_regs(struct alx_hw *hw, void *buff)
> +{
> + u32 *regs = (u32 *)buff;
Useless cast from void.
> + int retval = 0;
> +
> + MEM_R32(hw, L1C_LNK_CAP, ®s[0]);
> + MEM_R32(hw, L1C_PMCTRL, ®s[1]);
> + MEM_R32(hw, L1C_HALFD, ®s[2]);
> + MEM_R32(hw, L1C_SLD, ®s[3]);
> + MEM_R32(hw, L1C_MASTER, ®s[4]);
> + MEM_R32(hw, L1C_MANU_TIMER, ®s[5]);
> + MEM_R32(hw, L1C_IRQ_MODU_TIMER, ®s[6]);
> + MEM_R32(hw, L1C_PHY_CTRL, ®s[7]);
> + MEM_R32(hw, L1C_LNK_CTRL, ®s[8]);
> + MEM_R32(hw, L1C_MAC_STS, ®s[9]);
> +
> + MEM_R32(hw, L1C_MDIO, ®s[10]);
> + MEM_R32(hw, L1C_SERDES, ®s[11]);
> + MEM_R32(hw, L1C_MAC_CTRL, ®s[12]);
> + MEM_R32(hw, L1C_GAP, ®s[13]);
> + MEM_R32(hw, L1C_STAD0, ®s[14]);
> + MEM_R32(hw, L1C_STAD1, ®s[15]);
> + MEM_R32(hw, L1C_HASH_TBL0, ®s[16]);
> + MEM_R32(hw, L1C_HASH_TBL1, ®s[17]);
> + MEM_R32(hw, L1C_RXQ0, ®s[18]);
> + MEM_R32(hw, L1C_RXQ1, ®s[19]);
> +
> + MEM_R32(hw, L1C_RXQ2, ®s[20]);
> + MEM_R32(hw, L1C_RXQ3, ®s[21]);
> + MEM_R32(hw, L1C_TXQ0, ®s[22]);
> + MEM_R32(hw, L1C_TXQ1, ®s[23]);
> + MEM_R32(hw, L1C_TXQ2, ®s[24]);
> + MEM_R32(hw, L1C_MTU, ®s[25]);
> + MEM_R32(hw, L1C_WOL0, ®s[26]);
> + MEM_R32(hw, L1C_WOL1, ®s[27]);
> + MEM_R32(hw, L1C_WOL2, ®s[28]);
Tabulate and save code ?
[...]
> diff --git a/drivers/net/ethernet/atheros/alx/alc_hw.h b/drivers/net/ethernet/atheros/alx/alc_hw.h
> --- /dev/null
> +++ b/drivers/net/ethernet/atheros/alx/alc_hw.h
[...]
> +#ifdef __cplusplus
> +extern "C" {
> +#endif/*__cplusplus*/
Please remove this stuff before it gets noticed.
[...]
> +#define L1C_PCI_CMD 0x0004 /* 16bit */
> +#define L1C_PCI_CMD_DISINT BIT_10S
> +#define L1C_PCI_CMD_MASTEREN BIT_2S
> +#define L1C_PCI_CMD_MEMEN BIT_1S
> +#define L1C_PCI_CMD_IOEN BIT_0S
Duplicates from <linux/pci_regs.h>
[...]
> +/********************* PHY regs definition ***************************/
> +
> +/* PHY Control Register */
> +#define L1C_MII_BMCR 0x00
> +#define L1C_BMCR_SPEED_SELECT_MSB 0x0040
> +#define L1C_BMCR_COLL_TEST_ENABLE 0x0080
> +#define L1C_BMCR_FULL_DUPLEX 0x0100
> +#define L1C_BMCR_RESTART_AUTO_NEG 0x0200
> +#define L1C_BMCR_ISOLATE 0x0400
> +#define L1C_BMCR_POWER_DOWN 0x0800
> +#define L1C_BMCR_AUTO_NEG_EN 0x1000
> +#define L1C_BMCR_SPEED_SELECT_LSB 0x2000
> +#define L1C_BMCR_LOOPBACK 0x4000
> +#define L1C_BMCR_RESET 0x8000
> +#define L1C_BMCR_SPEED_MASK 0x2040
> +#define L1C_BMCR_SPEED_1000 0x0040
> +#define L1C_BMCR_SPEED_100 0x2000
> +#define L1C_BMCR_SPEED_10 0x0000
Please use existing #define from <linux/mii.h>.
> +
> +/* PHY Status Register */
> +#define L1C_MII_BMSR 0x01
> +#define L1C_BMSR_EXTENDED_CAPS 0x0001
> +#define L1C_BMSR_JABBER_DETECT 0x0002
> +#define L1C_BMSR_LINK_STATUS 0x0004
> +#define L1C_BMSR_AUTONEG_CAPS 0x0008
> +#define L1C_BMSR_REMOTE_FAULT 0x0010
> +#define L1C_BMSR_AUTONEG_COMPLETE 0x0020
> +#define L1C_BMSR_PREAMBLE_SUPPRESS 0x0040
> +#define L1C_BMSR_EXTENDED_STATUS 0x0100
> +#define L1C_BMSR_100T2_HD_CAPS 0x0200
> +#define L1C_BMSR_100T2_FD_CAPS 0x0400
> +#define L1C_BMSR_10T_HD_CAPS 0x0800
> +#define L1C_BMSR_10T_FD_CAPS 0x1000
> +#define L1C_BMSR_100X_HD_CAPS 0x2000
> +#define L1C_BMMII_SR_100X_FD_CAPS 0x4000
> +#define L1C_BMMII_SR_100T4_CAPS 0x8000
Sic.
[...]
> diff --git a/drivers/net/ethernet/atheros/alx/alf_cb.c b/drivers/net/ethernet/atheros/alx/alf_cb.c
> --- /dev/null
> +++ b/drivers/net/ethernet/atheros/alx/alf_cb.c
[...]
> +int alf_get_ethtool_regs(struct alx_hw *hw, void *buff)
> +{
> + u32 *regs = (u32 *)buff;
> + int i;
> + int retval = 0;
> +
> + MEM_R32(hw, L1F_PM_CSR, ®s[0]);
> + MEM_R32(hw, L1F_DEV_CAP, ®s[1]);
> + MEM_R32(hw, L1F_DEV_CTRL, ®s[2]);
> + MEM_R32(hw, L1F_LNK_CAP, ®s[3]);
> + MEM_R32(hw, L1F_LNK_CTRL, ®s[4]);
> + MEM_R32(hw, L1F_UE_SVRT, ®s[5]);
> + MEM_R32(hw, L1F_EFLD, ®s[6]);
> + MEM_R32(hw, L1F_SLD, ®s[7]);
> + MEM_R32(hw, L1F_PPHY_MISC1, ®s[8]);
> + MEM_R32(hw, L1F_PPHY_MISC2, ®s[9]);
> +
> + MEM_R32(hw, L1F_PDLL_TRNS1, ®s[10]);
> + MEM_R32(hw, L1F_TLEXTN_STATS, ®s[11]);
> + MEM_R32(hw, L1F_EFUSE_CTRL, ®s[12]);
> + MEM_R32(hw, L1F_EFUSE_DATA, ®s[13]);
> + MEM_R32(hw, L1F_SPI_OP1, ®s[14]);
> + MEM_R32(hw, L1F_SPI_OP2, ®s[15]);
> + MEM_R32(hw, L1F_SPI_OP3, ®s[16]);
> + MEM_R32(hw, L1F_EF_CTRL, ®s[17]);
> + MEM_R32(hw, L1F_EF_ADDR, ®s[18]);
> + MEM_R32(hw, L1F_EF_DATA, ®s[19]);
> +
> + MEM_R32(hw, L1F_SPI_ID, ®s[20]);
> + MEM_R32(hw, L1F_SPI_CFG_START, ®s[21]);
> + MEM_R32(hw, L1F_PMCTRL, ®s[22]);
> + MEM_R32(hw, L1F_LTSSM_CTRL, ®s[23]);
> + MEM_R32(hw, L1F_MASTER, ®s[24]);
> + MEM_R32(hw, L1F_MANU_TIMER, ®s[25]);
> + MEM_R32(hw, L1F_IRQ_MODU_TIMER, ®s[26]);
> + MEM_R32(hw, L1F_PHY_CTRL, ®s[27]);
> + MEM_R32(hw, L1F_MAC_STS, ®s[28]);
> + MEM_R32(hw, L1F_MDIO, ®s[29]);
> +
> + MEM_R32(hw, L1F_MDIO_EXTN, ®s[30]);
> + MEM_R32(hw, L1F_PHY_STS, ®s[31]);
> + MEM_R32(hw, L1F_BIST0, ®s[32]);
> + MEM_R32(hw, L1F_BIST1, ®s[33]);
> + MEM_R32(hw, L1F_SERDES, ®s[34]);
> + MEM_R32(hw, L1F_LED_CTRL, ®s[35]);
> + MEM_R32(hw, L1F_LED_PATN, ®s[36]);
> + MEM_R32(hw, L1F_LED_PATN2, ®s[37]);
> + MEM_R32(hw, L1F_SYSALV, ®s[38]);
> + MEM_R32(hw, L1F_PCIERR_INST, ®s[39]);
> +
> + MEM_R32(hw, L1F_LPI_DECISN_TIMER, ®s[40]);
> + MEM_R32(hw, L1F_LPI_CTRL, ®s[41]);
> + MEM_R32(hw, L1F_LPI_WAIT, ®s[42]);
> + MEM_R32(hw, L1F_HRTBT_VLAN, ®s[43]);
> + MEM_R32(hw, L1F_HRTBT_CTRL, ®s[44]);
> + MEM_R32(hw, L1F_RXPARSE, ®s[45]);
> + MEM_R32(hw, L1F_MAC_CTRL, ®s[46]);
> + MEM_R32(hw, L1F_GAP, ®s[47]);
> + MEM_R32(hw, L1F_STAD1, ®s[48]);
> + MEM_R32(hw, L1F_LED_CTRL, ®s[49]);
> +
> + MEM_R32(hw, L1F_HASH_TBL0, ®s[50]);
> + MEM_R32(hw, L1F_HASH_TBL1, ®s[51]);
> + MEM_R32(hw, L1F_HALFD, ®s[52]);
> + MEM_R32(hw, L1F_DMA, ®s[53]);
> + MEM_R32(hw, L1F_WOL0, ®s[54]);
> + MEM_R32(hw, L1F_WOL1, ®s[55]);
> + MEM_R32(hw, L1F_WOL2, ®s[56]);
> + MEM_R32(hw, L1F_WRR, ®s[57]);
> + MEM_R32(hw, L1F_HQTPD, ®s[58]);
> + MEM_R32(hw, L1F_CPUMAP1, ®s[59]);
> + MEM_R32(hw, L1F_CPUMAP2, ®s[60]);
> + MEM_R32(hw, L1F_MISC, ®s[61]);
You ought to iterate with a well chosen data structure so as to avoid
this huge code pollution.
[...]
> +int alf_init_hw_callbacks(struct alx_hw *hw)
> +{
> + int retval = 0;
> +
> + /* NIC */
> + hw->cbs.identify_nic = &alf_identify_nic;
> + /* MAC */
> + hw->cbs.reset_mac = &alf_reset_mac;
> + hw->cbs.start_mac = &alf_start_mac;
> + hw->cbs.stop_mac = &alf_stop_mac;
> + hw->cbs.config_mac = &alf_config_mac;
> + hw->cbs.get_mac_addr = &alf_get_mac_addr;
> + hw->cbs.set_mac_addr = &alf_set_mac_addr;
> + hw->cbs.clear_mac_addr = &alf_clear_mac_addr;
> + hw->cbs.set_mc_addr = &alf_set_mc_addr;
> + hw->cbs.clear_mc_addr = &alf_clear_mc_addr;
> +
> + /* PHY */
> + hw->cbs.init_phy = &alf_init_phy;
> + hw->cbs.reset_phy = &alf_reset_phy;
> + hw->cbs.read_phy_reg = &alf_read_phy_reg;
> + hw->cbs.write_phy_reg = &alf_write_phy_reg;
> + hw->cbs.check_phy_link = &alf_check_phy_link;
> + hw->cbs.setup_phy_link = &alf_setup_phy_link;
> + hw->cbs.setup_phy_link_speed = &alf_setup_phy_link_speed;
> +
> + /* Interrupt */
> + hw->cbs.ack_phy_intr = &alf_ack_phy_intr;
> + hw->cbs.enable_legacy_intr = &alf_enable_legacy_intr;
> + hw->cbs.disable_legacy_intr = &alf_disable_legacy_intr;
> + hw->cbs.enable_msix_intr = &alf_enable_msix_intr;
> + hw->cbs.disable_msix_intr = &alf_disable_msix_intr;
> +
> + /* Configure */
> + hw->cbs.config_rx = &alf_config_rx;
> + hw->cbs.config_tx = &alf_config_tx;
> + hw->cbs.config_fc = &alf_config_fc;
> + hw->cbs.config_rss = &alf_config_rss;
> + hw->cbs.config_msix = &alf_config_msix;
> + hw->cbs.config_wol = &alf_config_wol;
> + hw->cbs.config_aspm = &alf_config_aspm;
> + hw->cbs.config_mac_ctrl = &alf_config_mac_ctrl;
> + hw->cbs.config_pow_save = &alf_config_pow_save;
> + hw->cbs.reset_pcie = &alf_reset_pcie;
> +
> + /* NVRam */
> + hw->cbs.check_nvram = &alf_check_nvram;
> +
> + /* Others */
> + hw->cbs.get_ethtool_regs = alf_get_ethtool_regs;
> +
> + retval = alf_set_hw_capabilities(hw);
> +
> + retval = alf_set_hw_infos(hw);
> +
> + /* print hw flags */
> + HW_PRINT(INFO, "HW Flags = 0x%x\n", hw->flags);
> + return retval;
Almost every method in this file ought to be static.
[...]
> diff --git a/drivers/net/ethernet/atheros/alx/alf_hw.c b/drivers/net/ethernet/atheros/alx/alf_hw.c
> --- /dev/null
> +++ b/drivers/net/ethernet/atheros/alx/alf_hw.c
[...]
> +/* disable/enable MAC/RXQ/TXQ
> + * en
> + * true:enable
> + * false:disable
> + * return
> + * 0:success
> + * non-0-fail
> + */
> +u16 l1f_enable_mac(PETHCONTEXT ctx, bool en, u16 en_ctrl)
The name of this method is rather poor.
> +{
> + u32 rxq, txq, mac, val;
> + u16 i;
> +
> + MEM_R32(ctx, L1F_RXQ0, &rxq);
> + MEM_R32(ctx, L1F_TXQ0, &txq);
> + MEM_R32(ctx, L1F_MAC_CTRL, &mac);
> +
> + if (en) { /* enable */
> + MEM_W32(ctx, L1F_RXQ0, rxq | L1F_RXQ0_EN);
> + MEM_W32(ctx, L1F_TXQ0, txq | L1F_TXQ0_EN);
> + if (0 != (en_ctrl & LX_MACSPEED_1000)) {
> + FIELD_SETL(mac, L1F_MAC_CTRL_SPEED,
> + L1F_MAC_CTRL_SPEED_1000);
> + } else {
> + FIELD_SETL(mac, L1F_MAC_CTRL_SPEED,
> + L1F_MAC_CTRL_SPEED_10_100);
> + }
> + if (0 != (en_ctrl & LX_MACDUPLEX_FULL)) {
> + mac |= L1F_MAC_CTRL_FULLD;
> + } else {
> + mac &= ~L1F_MAC_CTRL_FULLD;
> + }
> + /* rx filter */
> + if (0 != (en_ctrl & LX_FLT_PROMISC)) {
> + mac |= L1F_MAC_CTRL_PROMISC_EN;
> + } else {
> + mac &= ~L1F_MAC_CTRL_PROMISC_EN;
> + }
> + if (0 != (en_ctrl & LX_FLT_MULTI_ALL)) {
> + mac |= L1F_MAC_CTRL_MULTIALL_EN;
> + } else {
> + mac &= ~L1F_MAC_CTRL_MULTIALL_EN;
> + }
> + if (0 != (en_ctrl & LX_FLT_BROADCAST)) {
> + mac |= L1F_MAC_CTRL_BRD_EN;
> + } else {
> + mac &= ~L1F_MAC_CTRL_BRD_EN;
> + }
Code duplication. Who cares ?
> + if (0 != (en_ctrl & LX_FLT_DIRECT)) {
> + mac |= L1F_MAC_CTRL_RX_EN;
> + } else { /* disable all recv if direct not enable */
> + mac &= ~L1F_MAC_CTRL_RX_EN;
> + }
> + if (0 != (en_ctrl & LX_FC_TXEN)) {
> + mac |= L1F_MAC_CTRL_TXFC_EN;
> + } else {
> + mac &= ~L1F_MAC_CTRL_TXFC_EN;
> + }
<blink>do it</blink>
> + if (0 != (en_ctrl & LX_FC_RXEN)) {
> + mac |= L1F_MAC_CTRL_RXFC_EN;
> + } else {
> + mac &= ~L1F_MAC_CTRL_RXFC_EN;
> + }
> + if (0 != (en_ctrl & LX_VLAN_STRIP)) {
> + mac |= L1F_MAC_CTRL_VLANSTRIP;
> + } else {
> + mac &= ~L1F_MAC_CTRL_VLANSTRIP;
> + }
Nevermind.
> + if (0 != (en_ctrl & LX_LOOPBACK)) {
> + mac |= (L1F_MAC_CTRL_LPBACK_EN);
> + } else {
> + mac &= ~L1F_MAC_CTRL_LPBACK_EN;
> + }
> + if (0 != (en_ctrl & LX_SINGLE_PAUSE)) {
> + mac |= L1F_MAC_CTRL_SPAUSE_EN;
> + } else {
> + mac &= ~L1F_MAC_CTRL_SPAUSE_EN;
> + }
> + if (0 != (en_ctrl & LX_ADD_FCS)) {
> + mac |= (L1F_MAC_CTRL_PCRCE | L1F_MAC_CTRL_CRCE);
> + } else {
> + mac &= ~(L1F_MAC_CTRL_PCRCE | L1F_MAC_CTRL_CRCE);
> + }
> + MEM_W32(ctx, L1F_MAC_CTRL, mac | L1F_MAC_CTRL_TX_EN);
It may make some sense to move these ~60 loc in a xyz_enable_something
method...
> + } else { /* disable mac */
... and this would be the xyz_disable_something counterpart.
[...]
> +u16 l1f_enable_aspm(PETHCONTEXT ctx, bool l0s_en, bool l1_en, u8 lnk_stat)
> +{
> + u32 pmctrl;
> + u8 rev = (u8)(FIELD_GETX(ctx->pci_revid, L1F_PCI_REVID));
> +
> + lnk_stat = lnk_stat;
> +
> +
> +#if 0 /* let sys bios control the L0S/L1 enable/disable */
Don't #if 0. Just remove it.
[...]
> +u16 l1f_write_phy(PETHCONTEXT ctx, bool ext, u8 dev, bool fast,
> + u16 reg, u16 data)
> +{
> + u32 val;
> + u16 clk_sel, i, ret = 0;
> +
> +#if MAC_TYPE_FPGA == MAC_TYPE
> + MEM_W32(ctx, L1F_MDIO, 0);
> + US_DELAY(ctx, 30);
> + for (i = 0; i < L1F_MDIO_MAX_AC_TO; i++) {
> + MEM_R32(ctx, L1F_MDIO, &val);
> + if (0 == (val & L1F_MDIO_BUSY)) {
> + break;
> + }
> + US_DELAY(ctx, 10);
> + }
I wonder how many times this 'for' loop is duplicated (4x ?).
[...]
> diff --git a/drivers/net/ethernet/atheros/alx/alf_hw.h b/drivers/net/ethernet/atheros/alx/alf_hw.h
> --- /dev/null
> +++ b/drivers/net/ethernet/atheros/alx/alf_hw.h
[...]
> +#ifdef __cplusplus
> +extern "C" {
> +#endif/*__cplusplus*/
Oops.
[...]
> +#define L1F_PCI_CMD 0x0004 /* 16bit */
> +#define L1F_PCI_CMD_DISINT BIT_10S
> +#define L1F_PCI_CMD_MASTEREN BIT_2S
> +#define L1F_PCI_CMD_MEMEN BIT_1S
> +#define L1F_PCI_CMD_IOEN BIT_0S
Duplicates <linux/pci_regs.h>
[...]
> diff --git a/drivers/net/ethernet/atheros/alx/alx_ethtool.c b/drivers/net/ethernet/atheros/alx/alx_ethtool.c
> +++ b/drivers/net/ethernet/atheros/alx/alx_ethtool.c
[...]
> +static struct ethtool_ops alx_ethtool_ops = {
> + .get_settings = alx_get_settings,
> + .set_settings = alx_set_settings,
> + .get_pauseparam = alx_get_pauseparam,
> + .set_pauseparam = alx_set_pauseparam,
> + .get_drvinfo = alx_get_drvinfo,
> + .get_regs_len = alx_get_regs_len,
> + .get_regs = alx_get_regs,
> + .get_wol = alx_get_wol,
> + .set_wol = alx_set_wol,
> + .get_msglevel = alx_get_msglevel,
> + .set_msglevel = alx_set_msglevel,
> + .nway_reset = alx_nway_reset,
> + .get_link = ethtool_op_get_link,
> + .get_eeprom_len = alx_get_eeprom_len,
> + .get_eeprom = alx_get_eeprom,
> + .set_eeprom = alx_set_eeprom,
> + .get_tx_csum = alx_get_tx_csum,
> + .get_sg = ethtool_op_get_sg,
> + .set_sg = ethtool_op_set_sg,
> +#ifdef NETIF_F_TSO
> + .get_tso = ethtool_op_get_tso,
> +#endif
Please switch to ndo_{fix/set}_features.
> diff --git a/drivers/net/ethernet/atheros/alx/alx_hwcom.h b/drivers/net/ethernet/atheros/alx/alx_hwcom.h
> +++ b/drivers/net/ethernet/atheros/alx/alx_hwcom.h
[...]
> +#define LX_SWAP_DW(_x) (\
> + (((_x) << 24) & 0xFF000000UL) |\
> + (((_x) << 8) & 0x00FF0000UL) |\
> + (((_x) >> 8) & 0x0000FF00UL) |\
> + (((_x) >> 24) & 0x000000FFUL))
> +
> +#define LX_SWAP_W(_x) (\
> + (((_x) >> 8) & 0x00FFU) |\
> + (((_x) << 8) & 0xFF00U))
Duplicates swabXY.
[...]
> +/* interop between drivers */
> +#define LX_DRV_TYPE_MASK SHIFT27(0x1FUL)
> +#define LX_DRV_TYPE_SHIFT 27
> +#define LX_DRV_TYPE_UNKNOWN 0
> +#define LX_DRV_TYPE_BIOS 1
> +#define LX_DRV_TYPE_BTROM 2
> +#define LX_DRV_TYPE_PKT 3
> +#define LX_DRV_TYPE_NDS2 4
> +#define LX_DRV_TYPE_UEFI 5
> +#define LX_DRV_TYPE_NDS5 6
> +#define LX_DRV_TYPE_NDS62 7
> +#define LX_DRV_TYPE_NDS63 8
> +#define LX_DRV_TYPE_LNX 9
> +#define LX_DRV_TYPE_ODI16 10
> +#define LX_DRV_TYPE_ODI32 11
> +#define LX_DRV_TYPE_FRBSD 12
> +#define LX_DRV_TYPE_NTBSD 13
> +#define LX_DRV_TYPE_WCE 14
No.
[...]
> diff --git a/drivers/net/ethernet/atheros/alx/alx_main.c b/drivers/net/ethernet/atheros/alx/alx_main.c
> --- /dev/null
> +++ b/drivers/net/ethernet/atheros/alx/alx_main.c
[...]
> +static int alx_validate_mac_addr(u8 *mac_addr)
> +{
> + int retval = 0;
> +
> + if (mac_addr[0] & 0x01) {
> + printk(KERN_DEBUG "MAC address is multicast\n");
> + retval = ALX_ERR_MAC_ADDR;
> + } else if (mac_addr[0] == 0xff && mac_addr[1] == 0xff) {
> + printk(KERN_DEBUG "MAC address is broadcast\n");
> + retval = ALX_ERR_MAC_ADDR;
> + } else if (mac_addr[0] == 0 && mac_addr[1] == 0 &&
> + mac_addr[2] == 0 && mac_addr[3] == 0 &&
> + mac_addr[4] == 0 && mac_addr[5] == 0) {
> + printk(KERN_DEBUG "MAC address is all zeros\n");
> + retval = ALX_ERR_MAC_ADDR;
> + }
> + return retval;
> +}
Please see is_valid_ether_addr, is_broadcast_ether_addr.
[...]
> +static void alx_receive_skb(struct alx_msix_param *msix,
> + struct sk_buff *skb,
> + u32 vlan_tag, bool vlan_flag)
> +{
> + struct alx_adapter *adpt = msix->adpt;
> +
> + if (adpt->vlgrp && vlan_flag) {
This kind of vlan support code is outdated. Please see ndo_{fix/set}_features.
[...]
> +/* alx_alloc_tx_descriptor - allocate Tx Descriptors */
> +static int alx_alloc_tx_descriptor(struct alx_adapter *adpt,
> + struct alx_tx_queue *txque)
> +{
> + struct alx_ring_header *ring_header = &adpt->ring_header;
> + int size;
> +
> + DRV_PRINT(IF, INFO, "tpq.count = %d\n", txque->tpq.count);
> +
> + size = sizeof(struct alx_buffer) * txque->tpq.count;
> + txque->tpq.tpbuff = kzalloc(size, GFP_KERNEL);
> + if (!txque->tpq.tpbuff)
> + goto err_alloc_tpq_buffer;
> + memset(txque->tpq.tpbuff, 0, size);
Useless memset (kZalloc).
> +
> + /* round up to nearest 4K */
> + txque->tpq.size = txque->tpq.count * sizeof(struct alx_tpdesc);
> +
> + txque->tpq.tpdma = ring_header->dma + ring_header->used;
> + txque->tpq.tpdesc = ring_header->desc + ring_header->used;
> + adpt->hw.tpdma[txque->que_idx] = (u64)txque->tpq.tpdma;
> + ring_header->used += ALIGN(txque->tpq.size, 8);
> +
> + txque->tpq.produce_idx = 0;
> + txque->tpq.consume_idx = 0;
> + txque->max_packets = txque->tpq.count;
> + return 0;
> +
> +err_alloc_tpq_buffer:
> + kfree(txque->tpq.tpbuff);
> + txque->tpq.tpbuff = NULL;
txque->tpq.tpbuff is already NULL before the kfree.
[...]
> +static int alx_alloc_rx_descriptor(struct alx_adapter *adpt,
> + struct alx_rx_queue *rxque)
> +{
[...]
> + rxque->rfq.rfbuff = kzalloc(size, GFP_KERNEL);
> + if (!rxque->rfq.rfbuff)
> + goto err_alloc_rfq_buffer;
> + memset(rxque->rfq.rfbuff, 0, size);
kzalloc + useless memset.
[...]
> + if (CHK_RX_FLAG(SW_QUE)) {
> + size = sizeof(struct alx_sw_buffer) * rxque->swq.count;
> + rxque->swq.swbuff = kzalloc(size, GFP_KERNEL);
> + if (!rxque->swq.swbuff)
> + goto err_alloc_swq_buffer;
> + memset(rxque->swq.swbuff, 0, size);
> +
> + rxque->swq.consume_idx = 0;
> + rxque->swq.produce_idx = 0;
> + }
> +
> + rxque->max_packets = rxque->rrq.count / 2;
> + return 0;
> +
> +err_alloc_swq_buffer:
> + kfree(rxque->swq.swbuff);
> + rxque->swq.swbuff = NULL;
rxque->swq.swbuff is already NULL before the kfree.
[...]
> +static int alx_alloc_all_rtx_descriptor(struct alx_adapter *adpt)
> +{
[...]
> + ring_header->desc = pci_alloc_consistent(pdev, ring_header->size,
> + &ring_header->dma);
> +
> + if (!ring_header->desc) {
> + DRV_PRINT(IF, ERR, "pci_alloc_consistend failed\n");
> + retval = -ENOMEM;
> + goto err_alloc_dma;
> + }
> + memset(ring_header->desc, 0, ring_header->size);
- pci_alloc_consistent returns a zeroed area.
- obsolescent. Ought to be dma_alloc_coherent.
[...]
> +#ifdef CONFIG_PM
> +int alx_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> + int retval;
> +
> + retval = alx_shutdown_internal(pdev, state);
> + if (retval)
> + return retval;
> +
> + return 0;
int alx_suspend(struct pci_dev *pdev, pm_message_t state)
{
return alx_shutdown_internal(pdev, state);
}
You may consider using device_driver.pm.
[...]
> +netdev_tx_t alx_start_xmit_frames(struct sk_buff *skb,
> + struct alx_adapter *adpt,
> + struct alx_tx_queue *txque)
> +{
[...]
> + if (!spin_trylock_irqsave(&adpt->tx_lock, flags)) {
> + DRV_PRINT(TX, EMERG, "tx locked!\n");
> + return NETDEV_TX_LOCKED;
> + }
I am not sure that NETDEV_TX_LOCKED is welcome in new drivers.
> +
> + if (!alx_check_num_tpdescs(txque, skb)) {
> + /* no enough descriptor, just stop queue */
> + netif_stop_queue(adpt->netdev);
> + spin_unlock_irqrestore(&adpt->tx_lock, flags);
> + return NETDEV_TX_BUSY;
> + }
The driver has gone a bridge too far : it should disable the tx queue when
it detects that it may not be able to honor the next xmit request.
[...]
> +static int alx_mac_ioctl(struct net_device *netdev,
> + struct ifreq *ifr, int cmd)
> +{
[...]
> + switch (cmd) {
> + case SIOCDEVGMACREG:
SIOCDEVPRIVATE abuse.
[...]
> +int __devinit alx_init(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
> +{
[...]
> + netdev->base_addr = (unsigned long)adpt->hw.hw_addr;
Use of base_addr ought to be avoided. It's old-fashioned.
[...]
> + adpt->bd_number = cards_found;
bd_number is never used. Please remove it as well as cards_found.
[...]
> +static void __devexit alx_remove(struct pci_dev *pdev)
> +{
[...]
> + for (que_idx = 0; que_idx < adpt->num_txques; que_idx++) {
> + kfree(adpt->tx_queue[que_idx]);
> + adpt->tx_queue[que_idx] = NULL;
> + }
> + for (que_idx = 0; que_idx < adpt->num_rxques; que_idx++) {
> + kfree(adpt->rx_queue[que_idx]);
> + adpt->rx_queue[que_idx] = NULL;
> + }
This is duplicating alx_free_all_rtx_queue.
[...]
> diff --git a/drivers/net/ethernet/atheros/alx/alx_sw.h b/drivers/net/ethernet/atheros/alx/alx_sw.h
> --- /dev/null
> +++ b/drivers/net/ethernet/atheros/alx/alx_sw.h
[...]
> +struct alx_hw {
> + struct alx_adapter *adpt;
> + struct alx_hw_callbacks cbs;
> + u8 __iomem *hw_addr; /* inner register address */
> + u16 pci_venid;
> + u16 pci_devid;
> + u16 pci_sub_devid;
> + u16 pci_sub_venid;
> + u8 pci_revid;
Gross duplication of fields available through adpt->pdev.
[...]
> + spinlock_t mdio_lock;
> + unsigned long mdio_flags;
Please leave it on the stack, thanks.
[...]
> +typedef struct alx_hw ETHCONTEXT;
> +typedef ETHCONTEXT * PETHCONTEXT;
Please remove the obfuscating typedefs.
[...]
> +/* Error Codes */
> +#define ALX_SUCCESS 0
> +
> +#define ALX_ERR_MAC -1
Unused.
> +#define ALX_ERR_MAC_INIT -2
> +#define ALX_ERR_MAC_RESET -3
> +#define ALX_ERR_MAC_START -4
> +#define ALX_ERR_MAC_STOP -5
> +#define ALX_ERR_MAC_CONFIGURE -6
> +#define ALX_ERR_MAC_ADDR -7
> +
> +#define ALX_ERR_PHY -20
> +#define ALX_ERR_PHY_INIT -21
Unused.
> +#define ALX_ERR_PHY_RESET -22
> +#define ALX_ERR_PHY_SETUP_LNK -23
> +#define ALX_ERR_PHY_CHECK_LNK -24
> +#define ALX_ERR_PHY_READ_REG -25
> +#define ALX_ERR_PHY_WRITE_REG -26
> +#define ALX_ERR_PHY_RESOLVED -27
> +
> +#define ALX_ERR_PCIE -40
Unused.
> +#define ALX_ERR_PCIE_RESET -41
> +#define ALX_ERR_PWR_SAVING -42
> +#define ALX_ERR_ASPM -43
> +#define ALX_ERR_EEPROM -44
Unused.
> +#define ALX_ERR_DISABLE_DRV -45
> +
> +#define ALX_ERR_FLOW_CONTROL -50
> +#define ALX_ERR_FC_NOT_NEGOTIATED -51
Unused.
> +#define ALX_ERR_FC_NOT_SUPPORTED -52
Unused.
> +
> +#define ALX_ERR_INVALID_ARGUMENT 0x7FFFFFFD
Unused.
> +#define ALX_ERR_NOT_SUPPORTED 0x7FFFFFFE
> +#define ALX_ERR_NOT_IMPLEMENTED 0x7FFFFFFF
Unused.
--
Ueimor
^ permalink raw reply
* Re: [net-next-2.6 PATCH 0/8 RFC v2] macvlan: MAC Address filtering support for passthru mode
From: Roopa Prabhu @ 2011-10-19 22:30 UTC (permalink / raw)
To: Rose, Gregory V, netdev@vger.kernel.org
Cc: sri@us.ibm.com, dragos.tatulea@gmail.com, arnd@arndb.de,
kvm@vger.kernel.org, mst@redhat.com, davem@davemloft.net,
mchan@broadcom.com, dwang2@cisco.com, shemminger@vyatta.com,
eric.dumazet@gmail.com, kaber@trash.net, benve@cisco.com
In-Reply-To: <43F901BD926A4E43B106BF17856F075501A177719C@orsmsx508.amr.corp.intel.com>
On 10/19/11 2:06 PM, "Rose, Gregory V" <gregory.v.rose@intel.com> wrote:
>> -----Original Message-----
>> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
>> On Behalf Of Roopa Prabhu
>> Sent: Tuesday, October 18, 2011 11:26 PM
>> To: netdev@vger.kernel.org
>> Cc: sri@us.ibm.com; dragos.tatulea@gmail.com; arnd@arndb.de;
>> kvm@vger.kernel.org; mst@redhat.com; davem@davemloft.net;
>> mchan@broadcom.com; dwang2@cisco.com; shemminger@vyatta.com;
>> eric.dumazet@gmail.com; kaber@trash.net; benve@cisco.com
>> Subject: [net-next-2.6 PATCH 0/8 RFC v2] macvlan: MAC Address filtering
>> support for passthru mode
>>
>
> [snip...]
>
>>
>>
>> Note: The choice of rtnl_link_ops was because I saw the use case for
>> this in virtual devices that need to do filtering in sw like macvlan
>> and tun. Hw devices usually have filtering in hw with netdev->uc and
>> mc lists to indicate active filters. But I can move from rtnl_link_ops
>> to netdev_ops if that is the preferred way to go and if there is a
>> need to support this interface on all kinds of interfaces.
>> Please suggest.
>
> I'm still digesting the rest of the RFC patches but I did want to quickly jump
> in and push for adding this support in netdev_ops. I would like to see these
> features available in more devices than just macvtap and macvlan. I can
> conceive
> of use cases for multiple HW MAC and VLAN filters for a VF device that isn't
> owned by a macvlan/macvtap interface and only has netdev_ops support. In this
> case it would be necessary to program the filters directly to the VF device
> interface or PF interface (or lowerdev as you refer to it) instead of going
> through macvlan/macvtap.
>
> This work dovetails nicely with some work I've been doing and I'd be very
> interested
> in helping move this forward if we could work out the details that would allow
> support
> of the features we (and the community) require.
Great. Thanks. I will definitely be interested to get this patch working for
any other use case you have.
Moving the ops to netdev should be trivial. You probably want the ops to
work on the VF via the PF, like the existing ndo_set_vf_mac etc.
Yes, lets work out the details and I can move this to netdev->ops. Let me
know.
Thanks,
Roopa
^ permalink raw reply
* Re: [PATCH] r8169: fix wrong eee setting for rlt8111evl
From: Francois Romieu @ 2011-10-19 22:43 UTC (permalink / raw)
To: David Miller; +Cc: hayeswang, netdev, linux-kernel
In-Reply-To: <20111019.165742.1621277627528277376.davem@davemloft.net>
David Miller <davem@davemloft.net> :
> > Correct the wrong parameter for setting EEE for RTL8111E-VL.
> >
> > Signed-off-by: Hayes Wang <hayeswang@realtek.com>
>
> Francois, want me to apply this directly or will you handle it?
Please apply it directly. I must trust Hayes on this one.
--
Ueimor
^ permalink raw reply
* Re: [PATCH] r8169: fix wrong eee setting for rlt8111evl
From: David Miller @ 2011-10-19 22:46 UTC (permalink / raw)
To: romieu; +Cc: hayeswang, netdev, linux-kernel
In-Reply-To: <20111019224312.GA10574@electric-eye.fr.zoreil.com>
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Thu, 20 Oct 2011 00:43:12 +0200
> David Miller <davem@davemloft.net> :
>> > Correct the wrong parameter for setting EEE for RTL8111E-VL.
>> >
>> > Signed-off-by: Hayes Wang <hayeswang@realtek.com>
>>
>> Francois, want me to apply this directly or will you handle it?
>
> Please apply it directly. I must trust Hayes on this one.
Ok, will do, thanks!
^ permalink raw reply
* Re: [PATCH 0/5] Better namespace handling for /sys/class/net/bonding_masters
From: David Miller @ 2011-10-19 22:49 UTC (permalink / raw)
To: ebiederm; +Cc: gregkh, linux-kernel, netdev, htejun, fubar, andy
In-Reply-To: <m18voh158i.fsf@fess.ebiederm.org>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Wed, 19 Oct 2011 06:27:25 -0700
> David Miller <davem@davemloft.net> writes:
>
>> From: ebiederm@xmission.com (Eric W. Biederman)
>> Date: Thu, 13 Oct 2011 00:47:46 -0700
>>
>>> Greg, Dave I'm don't know whose tree to merge this through as this code
>>> is equally device-core and networking. I am hoping that we can get this
>>> improvement merged for 3.2.
>>
>> I'm happy to take this series into my net-next tree.
>>
>> Greg? Any objections?
>
> on the 13th Greg said:
>
>> These all look fine to me, and I have no problem with them going through
>> the network tree as that will most likely be the easiest way due to any
>> potential merge issues.
>>
>> David, feel free to add an:
>> Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
Awesome, thanks for the reminder, I'll apply these right now.
^ permalink raw reply
* Re: [net-next 0/8] stmmac: update to Oct 2011 version (v4)
From: David Miller @ 2011-10-19 22:53 UTC (permalink / raw)
To: peppe.cavallaro; +Cc: netdev, eric.dumazet
In-Reply-To: <1318932085-14927-1-git-send-email-peppe.cavallaro@st.com>
From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
Date: Tue, 18 Oct 2011 12:01:17 +0200
> This patches update the driver adding the chained
> descriptor mode and some new useful fixes.
>
> I've reviewed some patches after the V1/2/3 and added new ones:
All applied, with the updated version of patch #8.
^ permalink raw reply
* Re: [patch net-next]alx: Atheros AR8131/AR8151/AR8152/AR8161 Ethernet driver
From: Joe Perches @ 2011-10-19 22:59 UTC (permalink / raw)
To: Francois Romieu; +Cc: cloud.ren, davem, Luis.Rodriguez, netdev, linux-kernel
In-Reply-To: <20111019222140.GA9937@electric-eye.fr.zoreil.com>
On Thu, 2011-10-20 at 00:21 +0200, Francois Romieu wrote:
> cloud.ren@atheros.com <cloud.ren@atheros.com> :
> > diff --git a/drivers/net/ethernet/atheros/alx/alc_cb.c b/drivers/net/ethernet/atheros/alx/alc_cb.c
> [...]
A bunch of good style comments.
> > + hw->autoneg_advertised = (ALX_LINK_SPEED_1GB_FULL |
> > + ALX_LINK_SPEED_10_HALF |
> > + ALX_LINK_SPEED_10_FULL |
> > + ALX_LINK_SPEED_100_HALF |
> > + ALX_LINK_SPEED_100_FULL);
> Parenthesis abuse.
Maybe. I use parenthesis too, but not the trailing | alignment.
emacs does leading alignment nicely when you use an open paren.
^ permalink raw reply
* Re: [PATCH] fib_rules: fix unresolved_rules counting
From: David Miller @ 2011-10-19 23:18 UTC (permalink / raw)
To: eric.dumazet; +Cc: zheng.z.yan, netdev
In-Reply-To: <1318909651.2571.41.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 18 Oct 2011 05:47:31 +0200
> Le mardi 18 octobre 2011 à 09:20 +0800, Yan, Zheng a écrit :
>> we should decrease ops->unresolved_rules when deleting a unresolved rule.
>>
>> Signed-off-by: Zheng Yan <zheng.z.yan@intel.com>
...
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: net-next [PATCH 1/1] ipv4: compat_ioctl is local to af_inet.c, make it static
From: David Miller @ 2011-10-19 23:24 UTC (permalink / raw)
To: gerrit; +Cc: netdev
In-Reply-To: <20111015192656.GA4066@gerrit.erg.abdn.ac.uk>
From: Gerrit Renker <gerrit@erg.abdn.ac.uk>
Date: Sat, 15 Oct 2011 13:26:56 -0600
> ipv4: compat_ioctl is local to af_inet.c, make it static
>
> Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next-2.6] [IPV6] cleanup: remove unnecessary include.
From: David Miller @ 2011-10-19 23:27 UTC (permalink / raw)
To: wkevils; +Cc: netdev
In-Reply-To: <CAGXs5wWNwy-dWaXMVp62FiaXxiKnDNNE3PX3DtqPVs9VCY+kMA@mail.gmail.com>
From: Kevin Wilson <wkevils@gmail.com>
Date: Sun, 16 Oct 2011 17:21:57 +0200
> This cleanup patch removes unnecessary include from net/ipv6/ip6_fib.c.
>
> Signed-off-by: Kevin Wilson <wkevils@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] NET: asix: fix ethtool -e for AX88178 USB dongle
From: David Miller @ 2011-10-19 23:31 UTC (permalink / raw)
To: grundler; +Cc: netdev, linux-kernel, allan, freddy
In-Reply-To: <1318866666-956-1-git-send-email-grundler@chromium.org>
From: Grant Grundler <grundler@chromium.org>
Date: Mon, 17 Oct 2011 08:51:06 -0700
> "ethtool -e ethX" dumps EEPROM data. Patch sets EEPROM length for device.
> Ethtool works alot better when the kernel believes the length is > 0.
>
> From: Allan Chou <allan@asix.com.tw>
> Signed-off-by: Grant Grundler <grundler@chromium.org>
Applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH] net: change capability used by socket options IP{,V6}_TRANSPARENT
From: David Miller @ 2011-10-19 23:34 UTC (permalink / raw)
To: zenczykowski; +Cc: maze, netdev, bazsi
In-Reply-To: <1318889783-23183-1-git-send-email-zenczykowski@gmail.com>
From: Maciej Żenczykowski <zenczykowski@gmail.com>
Date: Mon, 17 Oct 2011 15:16:23 -0700
> From: Maciej Żenczykowski <maze@google.com>
>
> Up till now the IP{,V6}_TRANSPARENT socket options (which actually set
> the same bit in the socket struct) have required CAP_NET_ADMIN
> privileges to set or clear the option.
>
> - we make clearing the bit not require any privileges.
> - we deprecate using CAP_NET_ADMIN for this purpose.
> - we allow CAP_NET_RAW to set this bit, because raw
> sockets already effectively allow you to emulate socket
> transparency.
> - we print a warning (but allow it) if you try to set the socket
> option with CAP_NET_ADMIN privs, but without CAP_NET_RAW.
>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
Warnings for something that has worked ever since the feature was
added, and in fact was the only way to make use of the feature, is
terrible.
You must support the status quo forever or else you risk breaking
existing setups. So the warning is pointless, you'll never be
able to remove CAP_NET_ADMIN from these code paths, so there is
zero value in warning about it because we'll never change this.
I'm disliking these changes more and more. I refuse to apply this
patch.
^ permalink raw reply
* Re: [patch] filter: use unsigned int to silence static checker warning
From: David Miller @ 2011-10-19 23:36 UTC (permalink / raw)
To: dan.carpenter; +Cc: netdev, eric.dumazet, xiaosuo, kernel-janitors
In-Reply-To: <20111018070420.GR27732@elgon.mountain>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Tue, 18 Oct 2011 10:04:20 +0300
> This is just a cleanup.
>
> My testing version of Smatch warns about this:
> net/core/filter.c +380 check_load_and_stores(6)
> warn: check 'flen' for negative values
>
> flen comes from the user. We try to clamp the values here between 1
> and BPF_MAXINSNS but the clamp doesn't work because it could be
> negative. This is a bug, but it's not exploitable.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied to net-next, thanks Dan.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox