* Re: [PATCH v1 RFC 3/7] Rename some functions with ksz9477 prefix
From: Florian Fainelli @ 2017-10-09 19:56 UTC (permalink / raw)
To: Tristram.Ha, Andrew Lunn, Pavel Machek, Ruediger Schmitt
Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
netdev, linux-kernel
In-Reply-To: <1507321985-15097-4-git-send-email-Tristram.Ha@microchip.com>
On 10/06/2017 01:33 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Rename some functions with ksz9477 prefix to separate chip specific code
> from common code.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v1 RFC 2/7] Clean up code according to patch check suggestions
From: Florian Fainelli @ 2017-10-09 19:54 UTC (permalink / raw)
To: Tristram.Ha, Andrew Lunn, Pavel Machek, Ruediger Schmitt
Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
netdev, linux-kernel
In-Reply-To: <1507321985-15097-3-git-send-email-Tristram.Ha@microchip.com>
On 10/06/2017 01:33 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Clean up code according to patch check suggestions.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v1 RFC 6/7] Add MIB counter reading support
From: Pavel Machek @ 2017-10-09 19:54 UTC (permalink / raw)
To: Tristram.Ha
Cc: Andrew Lunn, Florian Fainelli, Ruediger Schmitt, muvarov,
nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver, netdev,
linux-kernel
In-Reply-To: <1507321985-15097-7-git-send-email-Tristram.Ha@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 1998 bytes --]
Hi!
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Add MIB counter reading support.
> Rename ksz_9477_reg.h to ksz9477_reg.h for consistency as the product
> name is always KSZ####.
> Header file ksz_priv.h no longer contains any chip specific data.
Nothing obviously wrong here, but if you are doing another iteration,
it would be nice to separate "Rename ksz_9477_reg.h to ksz9477_reg.h"
from the code changes.
Best regards,
> + timeout = 1000;
> + do {
> + ksz_pread32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
> + &data);
> + usleep_range(1, 10);
> + if (!(data & MIB_COUNTER_READ))
> + break;
> + } while (timeout-- > 0);
1000 iterations, 1usec each, so 1msec, but you allow up to 10 msec. Interesting.
> + /* failed to read MIB. get out of loop */
> + if (!timeout) {
> + dev_dbg(dev->dev, "Failed to get MIB\n");
> + return;
> + }
Are you sure this works? AFAICT "timeout" will underflow, so !timeout
will not trigger.
> -static void ksz_get_ethtool_stats(struct dsa_switch *ds, int port,
> - uint64_t *buf)
> -{
> - struct ksz_device *dev = ds->priv;
> - int i;
> - u32 data;
> - int timeout;
> -
> - mutex_lock(&dev->stats_mutex);
> -
> - for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
> - data = MIB_COUNTER_READ;
> - data |= ((mib_names[i].index & 0xFF) << MIB_COUNTER_INDEX_S);
> - ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data);
> -
> - timeout = 1000;
> - do {
> - ksz_pread32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
> - &data);
> - usleep_range(1, 10);
> - if (!(data & MIB_COUNTER_READ))
> - break;
> - } while (timeout-- > 0);
> -
> - /* failed to read MIB. get out of loop */
> - if (!timeout) {
> - dev_dbg(dev->dev, "Failed to get MIB\n");
> - break;
> - }
Hmm. Bug was there already...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v1 RFC 5/7] Break KSZ9477 DSA driver into two files
From: Pavel Machek @ 2017-10-09 19:54 UTC (permalink / raw)
To: Tristram.Ha
Cc: Andrew Lunn, Florian Fainelli, Ruediger Schmitt, muvarov,
nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver, netdev,
linux-kernel
In-Reply-To: <1507321985-15097-6-git-send-email-Tristram.Ha@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 634 bytes --]
On Fri 2017-10-06 13:33:03, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Break KSZ9477 DSA driver into two files in preparation to add more KSZ
> switch drivers.
> Add common functions in ksz_common.h so that other KSZ switch drivers
> can access code in ksz_common.c.
> Add ksz_spi.h for common functions used by KSZ switch SPI drivers.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Pavel Machek <pavel@ucw.cz>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v1 RFC 4/7] Rename ksz_spi.c to ksz9477_spi.c
From: Pavel Machek @ 2017-10-09 19:54 UTC (permalink / raw)
To: Tristram.Ha
Cc: Andrew Lunn, Florian Fainelli, Ruediger Schmitt, muvarov,
nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver, netdev,
linux-kernel
In-Reply-To: <1507321985-15097-5-git-send-email-Tristram.Ha@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 550 bytes --]
On Fri 2017-10-06 13:33:02, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Rename ksz_spi.c to ksz9477_spi.c and update Kconfig in preparation to add
> more KSZ switch drivers.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
This will ask people question they already answered, but I guess
that's ok.
Reviewed-by: Pavel Machek <pavel@ucw.cz>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v1 RFC 3/7] Rename some functions with ksz9477 prefix
From: Pavel Machek @ 2017-10-09 19:54 UTC (permalink / raw)
To: Tristram.Ha
Cc: Andrew Lunn, Florian Fainelli, Ruediger Schmitt, muvarov,
nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver, netdev,
linux-kernel
In-Reply-To: <1507321985-15097-4-git-send-email-Tristram.Ha@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 461 bytes --]
On Fri 2017-10-06 13:33:01, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Rename some functions with ksz9477 prefix to separate chip specific code
> from common code.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Pavel Machek <pavel@ucw.cz>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v1 RFC 2/7] Clean up code according to patch check suggestions
From: Pavel Machek @ 2017-10-09 19:54 UTC (permalink / raw)
To: Tristram.Ha
Cc: Andrew Lunn, Florian Fainelli, Ruediger Schmitt, muvarov,
nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver, netdev,
linux-kernel
In-Reply-To: <1507321985-15097-3-git-send-email-Tristram.Ha@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 421 bytes --]
On Fri 2017-10-06 13:33:00, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Clean up code according to patch check suggestions.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Pavel Machek <pavel@ucw.cz>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v1 RFC 1/7] Replace license with GPL
From: Pavel Machek @ 2017-10-09 19:54 UTC (permalink / raw)
To: Tristram.Ha
Cc: Andrew Lunn, Florian Fainelli, Ruediger Schmitt, muvarov,
nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver, netdev,
linux-kernel
In-Reply-To: <1507321985-15097-2-git-send-email-Tristram.Ha@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 390 bytes --]
On Fri 2017-10-06 13:32:59, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Replace license with GPL.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: linux-next: manual merge of the drivers-x86 tree with the net-next tree
From: Mika Westerberg @ 2017-10-09 19:43 UTC (permalink / raw)
To: Mark Brown
Cc: Darren Hart, Mario Limonciello, Yehezkel Bernat, Andy Shevchenko,
Amir Levy, Michael Jamet, David S. Miller, netdev,
Linux-Next Mailing List, Linux Kernel Mailing List
In-Reply-To: <20171009175633.dm7hjmgoxusspue5@sirena.co.uk>
On Mon, Oct 09, 2017 at 06:56:33PM +0100, Mark Brown wrote:
> +Networking over Thunderbolt cable
> +---------------------------------
> +Thunderbolt technology allows software communication across two hosts
> +connected by a Thunderbolt cable.
> +
> +It is possible to tunnel any kind of traffic over Thunderbolt link but
> +currently we only support Apple ThunderboltIP protocol.
> +
> +If the other host is running Windows or macOS only thing you need to
> +do is to connect Thunderbolt cable between the two hosts, the
> +``thunderbolt-net`` is loaded automatically. If the other host is also
> +Linux you should load ``thunderbolt-net`` manually on one host (it does
> +not matter which one)::
> +
> + # modprobe thunderbolt-net
> +
> +This triggers module load on the other host automatically. If the driver
> +is built-in to the kernel image, there is no need to do anything.
> +
> +The driver will create one virtual ethernet interface per Thunderbolt
> +port which are named like ``thunderbolt0`` and so on. From this point
> +you can either use standard userspace tools like ``ifconfig`` to
> +configure the interface or let your GUI to handle it automatically.
> ++
> + Forcing power
> + -------------
> + Many OEMs include a method that can be used to force the power of a
> + thunderbolt controller to an "On" state even if nothing is connected.
> + If supported by your machine this will be exposed by the WMI bus with
> + a sysfs attribute called "force_power".
> +
> + For example the intel-wmi-thunderbolt driver exposes this attribute in:
> + /sys/devices/platform/PNP0C14:00/wmi_bus/wmi_bus-PNP0C14:00/86CCFD48-205E-4A77-9C48-2021CBEDE341/force_power
> +
> + To force the power to on, write 1 to this attribute file.
> + To disable force power, write 0 to this attribute file.
> +
> + Note: it's currently not possible to query the force power state of a platform.
If possible, I would rather move this chapter to be before "Networking
over Thunderbolt cable". Reason is that it then follows NVM flashing
chapter which is typically where you need to force power in the first
place.
^ permalink raw reply
* [PATCH] wimax/i2400m: Remove VLAIS
From: Matthias Kaehlcke @ 2017-10-09 19:41 UTC (permalink / raw)
To: Inaky Perez-Gonzalez
Cc: linux-wimax, netdev, linux-kernel, Behan Webster, Mark Charlebois,
Arnd Bergmann, Guenter Roeck, Matthias Kaehlcke
From: Behan Webster <behanw@converseincode.com>
Convert Variable Length Array in Struct (VLAIS) to valid C by converting
local struct definition to use a flexible array. The structure is only
used to define a cast of a buffer so the size of the struct is not used
to allocate storage.
Signed-off-by: Behan Webster <behanw@converseincode.com>
Signed-off-by: Mark Charebois <charlebm@gmail.com>
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
drivers/net/wimax/i2400m/fw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wimax/i2400m/fw.c b/drivers/net/wimax/i2400m/fw.c
index c9c711dcd0e6..a89b5685e68b 100644
--- a/drivers/net/wimax/i2400m/fw.c
+++ b/drivers/net/wimax/i2400m/fw.c
@@ -652,7 +652,7 @@ static int i2400m_download_chunk(struct i2400m *i2400m, const void *chunk,
struct device *dev = i2400m_dev(i2400m);
struct {
struct i2400m_bootrom_header cmd;
- u8 cmd_payload[chunk_len];
+ u8 cmd_payload[];
} __packed *buf;
struct i2400m_bootrom_header ack;
--
2.14.2.920.gcf0c67979c-goog
^ permalink raw reply related
* Re: [PATCH v1 RFC 1/7] Replace license with GPL
From: Florian Fainelli @ 2017-10-09 19:40 UTC (permalink / raw)
To: Tristram.Ha, David.Laight
Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
netdev, linux-kernel, andrew, pavel, ruediger.schmitt
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD4112EE49@CHN-SV-EXMX02.mchp-main.com>
On 10/09/2017 11:40 AM, Tristram.Ha@microchip.com wrote:
>> From: Tristram.Ha@microchip.com
>>> Sent: 06 October 2017 21:33
>>> Replace license with GPL.
>>
>> Don't you need permission from all the people who have updated
>> the files in order to make this change?
>>
>> David
>
> I am a little confused by your comment. The 4 original KSZ9477 DSA
> driver files were written by Woojung at Microchip Technology Inc.
> There was a complaint the "AS IS" license is not exactly GPL.
>
> It should be submitted formally to net-next instead of a RFC, but it
> is probably pointless to do that when there is no code change.
>
> I am hoping these drastic changes of KSZ9477 driver can be accepted
> so that the patches can be submitted formally to net-next.
What David is saying is that you need to take into account every one who
has been making changes to the Microchip driver, because anytime
someones submits a patch that gets accepted, their "voice" now counts:
Fortunately, the list seems to be fairly limited:
git shortlog -sne drivers/net/dsa/microchip
5 Arkadi Sharshevsky <arkadis@mellanox.com>
1 Woojung Huh <Woojung.Huh@microchip.com>
So seeking contributor approval for a license change should be fairly
simple.
--
Florian
^ permalink raw reply
* Re: [PATCH v4 1/2] net: phy: DP83822 initial driver submission
From: Andrew F. Davis @ 2017-10-09 19:12 UTC (permalink / raw)
To: Dan Murphy, andrew, f.fainelli; +Cc: netdev, Woojung.Huh
In-Reply-To: <20171009165934.12671-1-dmurphy@ti.com>
On 10/09/2017 11:59 AM, Dan Murphy wrote:
> Add support for the TI DP83822 10/100Mbit ethernet phy.
>
> The DP83822 provides flexibility to connect to a MAC through a
> standard MII, RMII or RGMII interface.
>
> In addition the DP83822 needs to be removed from the DP83848 driver
> as the WoL support is added here for this device.
>
> Datasheet:
> http://www.ti.com/product/DP83822I/datasheet
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>> ---
>
> v4 - Squash DP83822 removal patch into this patch -
> https://www.mail-archive.com/netdev@vger.kernel.org/msg192424.html
>
> v3 - Fixed WoL indication bit and removed mutex for suspend/resume -
> https://www.mail-archive.com/netdev@vger.kernel.org/msg191891.html and
> https://www.mail-archive.com/netdev@vger.kernel.org/msg191665.html
>
> v2 - Updated per comments. Removed unnessary parantheis, called genphy_suspend/
> resume routines and then performing WoL changes, reworked sopass storage and reduced
> the number of phy reads, and moved WOL_SECURE_ON -
> https://www.mail-archive.com/netdev@vger.kernel.org/msg191392.html
>
> drivers/net/phy/Kconfig | 5 +
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/dp83822.c | 302 ++++++++++++++++++++++++++++++++++++++++++++++
> drivers/net/phy/dp83848.c | 3 -
> 4 files changed, 308 insertions(+), 3 deletions(-)
> create mode 100644 drivers/net/phy/dp83822.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index cd931cf9dcc2..8e78a482e09e 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -277,6 +277,11 @@ config DAVICOM_PHY
> ---help---
> Currently supports dm9161e and dm9131
>
> +config DP83822_PHY
> + tristate "Texas Instruments DP83822 PHY"
> + ---help---
> + Supports the DP83822 PHY.
> +
> config DP83848_PHY
> tristate "Texas Instruments DP83848 PHY"
> ---help---
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 416df92fbf4f..df3b82ba8550 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -55,6 +55,7 @@ obj-$(CONFIG_CICADA_PHY) += cicada.o
> obj-$(CONFIG_CORTINA_PHY) += cortina.o
> obj-$(CONFIG_DAVICOM_PHY) += davicom.o
> obj-$(CONFIG_DP83640_PHY) += dp83640.o
> +obj-$(CONFIG_DP83822_PHY) += dp83822.o
> obj-$(CONFIG_DP83848_PHY) += dp83848.o
> obj-$(CONFIG_DP83867_PHY) += dp83867.o
> obj-$(CONFIG_FIXED_PHY) += fixed_phy.o
> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> new file mode 100644
> index 000000000000..de196dbc46cd
> --- /dev/null
> +++ b/drivers/net/phy/dp83822.c
> @@ -0,0 +1,302 @@
> +/*
> + * Driver for the Texas Instruments DP83822 PHY
> + *
> + * Copyright (C) 2017 Texas Instruments Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/ethtool.h>
> +#include <linux/etherdevice.h>
> +#include <linux/kernel.h>
> +#include <linux/mii.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy.h>
> +#include <linux/netdevice.h>
> +
> +#define DP83822_PHY_ID 0x2000a240
> +#define DP83822_DEVADDR 0x1f
> +
> +#define MII_DP83822_MISR1 0x12
> +#define MII_DP83822_MISR2 0x13
> +#define MII_DP83822_RESET_CTRL 0x1f
> +
> +#define DP83822_HW_RESET BIT(15)
> +#define DP83822_SW_RESET BIT(14)
> +
> +/* MISR1 bits */
> +#define DP83822_RX_ERR_HF_INT_EN BIT(0)
> +#define DP83822_FALSE_CARRIER_HF_INT_EN BIT(1)
> +#define DP83822_ANEG_COMPLETE_INT_EN BIT(2)
> +#define DP83822_DUP_MODE_CHANGE_INT_EN BIT(3)
> +#define DP83822_SPEED_CHANGED_INT_EN BIT(4)
> +#define DP83822_LINK_STAT_INT_EN BIT(5)
> +#define DP83822_ENERGY_DET_INT_EN BIT(6)
> +#define DP83822_LINK_QUAL_INT_EN BIT(7)
> +
> +/* MISR2 bits */
> +#define DP83822_JABBER_DET_INT_EN BIT(0)
> +#define DP83822_WOL_PKT_INT_EN BIT(1)
> +#define DP83822_SLEEP_MODE_INT_EN BIT(2)
> +#define DP83822_MDI_XOVER_INT_EN BIT(3)
> +#define DP83822_LB_FIFO_INT_EN BIT(4)
> +#define DP83822_PAGE_RX_INT_EN BIT(5)
> +#define DP83822_ANEG_ERR_INT_EN BIT(6)
> +#define DP83822_EEE_ERROR_CHANGE_INT_EN BIT(7)
> +
> +/* INT_STAT1 bits */
> +#define DP83822_WOL_INT_EN BIT(4)
> +#define DP83822_WOL_INT_STAT BIT(12)
> +
> +#define MII_DP83822_RXSOP1 0x04a5
> +#define MII_DP83822_RXSOP2 0x04a6
> +#define MII_DP83822_RXSOP3 0x04a7
> +
> +/* WoL Registers */
> +#define MII_DP83822_WOL_CFG 0x04a0
> +#define MII_DP83822_WOL_STAT 0x04a1
> +#define MII_DP83822_WOL_DA1 0x04a2
> +#define MII_DP83822_WOL_DA2 0x04a3
> +#define MII_DP83822_WOL_DA3 0x04a4
> +
> +/* WoL bits */
> +#define DP83822_WOL_MAGIC_EN BIT(1)
Datasheet seems to indicate MAGIC_EN is bit 0, not 1.
> +#define DP83822_WOL_SECURE_ON BIT(5)
> +#define DP83822_WOL_EN BIT(7)
> +#define DP83822_WOL_INDICATION_SEL BIT(8)
> +#define DP83822_WOL_CLR_INDICATION BIT(11)
> +
> +static int dp83822_ack_interrupt(struct phy_device *phydev)
> +{
> + int err = phy_read(phydev, MII_DP83822_MISR1);
> +
> + if (err < 0)
> + return err;
> +
The above could also be written:
int err;
err = phy_read(phydev, MII_DP83822_MISR1);
if (err < 0)
return err;
This matches the below better and is more clear to me.
> + err = phy_read(phydev, MII_DP83822_MISR2);
> + if (err < 0)
> + return err;
> +
> + return 0;
> +}
> +
> +static int dp83822_set_wol(struct phy_device *phydev,
> + struct ethtool_wolinfo *wol)
> +{
> + struct net_device *ndev = phydev->attached_dev;
> + u16 value;
> + const u8 *mac;
> +
> + if (wol->wolopts & (WAKE_MAGIC | WAKE_MAGICSECURE)) {
> + mac = (const u8 *)ndev->dev_addr;
> +
> + if (!is_valid_ether_addr(mac))
> + return -EINVAL;
> +
> + /* MAC addresses start with byte 5, but stored in mac[0].
> + * 822 PHYs store bytes 4|5, 2|3, 0|1
> + */
> + phy_write_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_WOL_DA1, (mac[1] << 8) | mac[0]);
> + phy_write_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_WOL_DA2, (mac[3] << 8) | mac[2]);
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_DA3,
> + (mac[5] << 8) | mac[4]);
This 'phy_write_mmd' doesn't match the others, 'MII_DP83822_WOL_DAx'
should be on the next line, or all others should be on same.
> +
> + value = phy_read_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_WOL_CFG);
> + if (wol->wolopts & WAKE_MAGIC)
> + value |= DP83822_WOL_MAGIC_EN;
> + else
> + value &= ~DP83822_WOL_MAGIC_EN;
> +
> + if (wol->wolopts & WAKE_MAGICSECURE) {
> + phy_write_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP1,
> + (wol->sopass[1] << 8) | wol->sopass[0]);
> + phy_write_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP2,
> + (wol->sopass[3] << 8) | wol->sopass[2]);
> + phy_write_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP3,
> + (wol->sopass[5] << 8) | wol->sopass[4]);
> + value |= DP83822_WOL_SECURE_ON;
> + } else {
> + value &= ~DP83822_WOL_SECURE_ON;
> + }
> +
> + value |= (DP83822_WOL_EN | DP83822_WOL_INDICATION_SEL |
> + DP83822_WOL_CLR_INDICATION);
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
> + value);
> + } else {
> + value = phy_read_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_WOL_CFG);
> + value &= ~DP83822_WOL_EN;
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
> + value);
> + }
> +
> + return 0;
> +}
> +
> +static void dp83822_get_wol(struct phy_device *phydev,
> + struct ethtool_wolinfo *wol)
> +{
> + int value;
> + u16 sopass_val;
> +
> + wol->supported = (WAKE_MAGIC | WAKE_MAGICSECURE);
> + wol->wolopts = 0;
> +
> + value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
> + if (value & DP83822_WOL_MAGIC_EN)
> + wol->wolopts |= WAKE_MAGIC;
> +
> + if (~value & DP83822_WOL_CLR_INDICATION)
> + wol->wolopts = 0;
I'm not sure I understand the logic here, why do we clear all other
wolopts if this is not set?
> +
> + if (value & DP83822_WOL_SECURE_ON) {
> + wol->wolopts |= WAKE_MAGICSECURE;
> + } else {
> + wol->wolopts &= ~WAKE_MAGICSECURE;
wol->wolopts is set to 0 at the start, and nothing else sets it, why
clear it here?
> + return;
> + }
> +
> + sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RXSOP1);
> + wol->sopass[0] = (sopass_val & 0xff);
> + wol->sopass[1] = (sopass_val >> 8);
> +
> + sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RXSOP2);
> + wol->sopass[2] = (sopass_val & 0xff);
> + wol->sopass[3] = (sopass_val >> 8);
> +
> + sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RXSOP3);
> + wol->sopass[4] = (sopass_val & 0xff);
> + wol->sopass[5] = (sopass_val >> 8);
Why not encase the above password lines in the 'if (value &
DP83822_WOL_SECURE_ON)' block above, then you can drop the whole else block.
> +}
> +
> +static int dp83822_config_intr(struct phy_device *phydev)
> +{
> + int misr_status;
> + int err;
> +
> + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
> + misr_status = phy_read(phydev, MII_DP83822_MISR1);
> + if (misr_status < 0)
> + return misr_status;
> +
> + misr_status |= (DP83822_RX_ERR_HF_INT_EN |
> + DP83822_FALSE_CARRIER_HF_INT_EN |
> + DP83822_ANEG_COMPLETE_INT_EN |
> + DP83822_DUP_MODE_CHANGE_INT_EN |
> + DP83822_SPEED_CHANGED_INT_EN |
> + DP83822_LINK_STAT_INT_EN |
> + DP83822_ENERGY_DET_INT_EN |
> + DP83822_LINK_QUAL_INT_EN);
> +
> + err = phy_write(phydev, MII_DP83822_MISR1, misr_status);
> + if (err < 0)
> + return err;
> +
> + misr_status = phy_read(phydev, MII_DP83822_MISR2);
> + if (misr_status < 0)
> + return misr_status;
> +
> + misr_status |= (DP83822_JABBER_DET_INT_EN |
> + DP83822_WOL_PKT_INT_EN |
> + DP83822_SLEEP_MODE_INT_EN |
> + DP83822_MDI_XOVER_INT_EN |
> + DP83822_LB_FIFO_INT_EN |
> + DP83822_PAGE_RX_INT_EN |
> + DP83822_ANEG_ERR_INT_EN |
> + DP83822_EEE_ERROR_CHANGE_INT_EN);
> +
> + err = phy_write(phydev, MII_DP83822_MISR2, misr_status);
> + } else {
> + err = phy_write(phydev, MII_DP83822_MISR1, 0);
You should only clear the ones you set, I know it is all of them plus
the other registers are read-only, but for clarity you could have a
define with the mask you are using for each register and then ~MASK when
clearing, like the dp83848.c driver.
> + if (err < 0)
> + return err;
> +
> + err = phy_write(phydev, MII_DP83822_MISR1, 0);
> + }
> +
> + return err;
> +}
> +
> +static int dp83822_phy_reset(struct phy_device *phydev)
> +{
> + int err;
> +
> + err = phy_write(phydev, MII_DP83822_RESET_CTRL, DP83822_HW_RESET);
> + if (err < 0)
> + return err;
> +
> + return 0;
> +}
> +
> +static int dp83822_suspend(struct phy_device *phydev)
> +{
> + int value;
> +
> + value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
> +
> + if (!(value & DP83822_WOL_EN))
> + genphy_suspend(phydev);
> +
> + return 0;
> +}
> +
> +static int dp83822_resume(struct phy_device *phydev)
> +{
> + int value;
> +
> + genphy_resume(phydev);
> +
> + value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
> +
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG, value |
> + DP83822_WOL_CLR_INDICATION);
> +
> +
Extra newline.
> + return 0;
> +}
> +
> +static struct phy_driver dp83822_driver[] = {
> + {
> + .phy_id = DP83822_PHY_ID,
> + .phy_id_mask = 0xfffffff0,
> + .name = "TI DP83822",
> + .features = PHY_BASIC_FEATURES,
> + .flags = PHY_HAS_INTERRUPT,
> + .config_init = genphy_config_init,
> + .soft_reset = dp83822_phy_reset,
> + .get_wol = dp83822_get_wol,
> + .set_wol = dp83822_set_wol,
> + .ack_interrupt = dp83822_ack_interrupt,
> + .config_intr = dp83822_config_intr,
> + .config_aneg = genphy_config_aneg,
> + .read_status = genphy_read_status,
> + .suspend = dp83822_suspend,
> + .resume = dp83822_resume,
> + },
Something is not right about the indenting here, tab then space?
> +};
> +module_phy_driver(dp83822_driver);
> +
> +static struct mdio_device_id __maybe_unused dp83822_tbl[] = {
> + { DP83822_PHY_ID, 0xfffffff0 },
> + { },
> +};
> +MODULE_DEVICE_TABLE(mdio, dp83822_tbl);
> +
> +MODULE_DESCRIPTION("Texas Instruments DP83822 PHY driver");
> +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c
> index 3de4fe4dda77..3966d43c5146 100644
> --- a/drivers/net/phy/dp83848.c
> +++ b/drivers/net/phy/dp83848.c
> @@ -20,7 +20,6 @@
> #define TI_DP83620_PHY_ID 0x20005ce0
> #define NS_DP83848C_PHY_ID 0x20005c90
> #define TLK10X_PHY_ID 0x2000a210
> -#define TI_DP83822_PHY_ID 0x2000a240
>
> /* Registers */
> #define DP83848_MICR 0x11 /* MII Interrupt Control Register */
> @@ -80,7 +79,6 @@ static struct mdio_device_id __maybe_unused dp83848_tbl[] = {
> { NS_DP83848C_PHY_ID, 0xfffffff0 },
> { TI_DP83620_PHY_ID, 0xfffffff0 },
> { TLK10X_PHY_ID, 0xfffffff0 },
> - { TI_DP83822_PHY_ID, 0xfffffff0 },
> { }
> };
> MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
> @@ -110,7 +108,6 @@ static struct phy_driver dp83848_driver[] = {
> DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY"),
> DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY"),
> DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY"),
> - DP83848_PHY_DRIVER(TI_DP83822_PHY_ID, "TI DP83822 10/100 Mbps PHY"),
> };
> module_phy_driver(dp83848_driver);
>
>
^ permalink raw reply
* Re: [PATCH net-next 1/1] [net] bonding: Add NUMA notice
From: Patrick Talbert @ 2017-10-09 19:00 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20171007.232036.1361815658002685993.davem@davemloft.net>
On Sat, Oct 7, 2017 at 6:20 PM, David Miller <davem@davemloft.net> wrote:
> From: Patrick Talbert <ptalbert@redhat.com>
> Date: Thu, 5 Oct 2017 16:23:45 -0400
>
>> Network performance can suffer when a load balancing bond uses slave
>> interfaces which are in different NUMA domains.
>>
>> This compares the NUMA domain of a newly enslaved interface against any
>> existing enslaved interfaces and prints a warning if they do not match.
>>
>> Signed-off-by: Patrick Talbert <ptalbert@redhat.com>
>
> This is a bit over the top, and doesn't even handle cases where
> the device has no specific NUMA node (-1).
Hello David,
The motivation was simply to have a notification in place if the
interface to-be-enslaved does not match the existing one(s). We have
seen performance issues with bonding which were eventually tracked
down to this mismatched NUMA domain issue. I thought it was worth
having the *mild* warning because it can have a decent impact yet is
probably not an obvious thing to check for most users.
Though I now see your point. If the bonded interfaces are VLANs, and
their underlying physical interfaces happen to be in different NUMA
domains, then my check will not know as the VLAN interface numa_node
member will be -1 no matter what. That's probably a pretty common
setup but adding the logic to check for it probably isn't worth it.
Patrick
^ permalink raw reply
* [net-next 10/10] ixgbe: fix crash when injecting AER after failed reset
From: Jeff Kirsher @ 2017-10-09 18:40 UTC (permalink / raw)
To: davem; +Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20171009184000.80053-1-jeffrey.t.kirsher@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
In case where AER recovery fails the device is left in a down state.
Consecutive AER error injection can lead to a double IRQ free.
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 5e2686d106db..c6f9da7990c7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -10861,6 +10861,9 @@ static pci_ers_result_t ixgbe_io_error_detected(struct pci_dev *pdev,
if (!test_bit(__IXGBE_SERVICE_INITED, &adapter->state))
return PCI_ERS_RESULT_DISCONNECT;
+ if (!netif_device_present(netdev))
+ return PCI_ERS_RESULT_DISCONNECT;
+
rtnl_lock();
netif_device_detach(netdev);
--
2.14.2
^ permalink raw reply related
* [net-next 09/10] ixgbe: Update adaptive ITR algorithm
From: Jeff Kirsher @ 2017-10-09 18:39 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20171009184000.80053-1-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
The following change is meant to update the adaptive ITR algorithm to
better support the needs of the network. Specifically with this change what
I have done is make it so that our ITR algorithm will try to prevent either
starving a socket buffer for memory in the case of Tx, or overrunning an Rx
socket buffer on receive.
In addition a side effect of the calculations used is that we should
function better with new features such as XDP which can handle small
packets at high rates without needing to lock us into NAPI polling mode.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 7 +
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 11 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 215 +++++++++++++++++++-------
3 files changed, 178 insertions(+), 55 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 008d0085e01f..468c3555a629 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -435,8 +435,15 @@ static inline unsigned int ixgbe_rx_pg_order(struct ixgbe_ring *ring)
}
#define ixgbe_rx_pg_size(_ring) (PAGE_SIZE << ixgbe_rx_pg_order(_ring))
+#define IXGBE_ITR_ADAPTIVE_MIN_INC 2
+#define IXGBE_ITR_ADAPTIVE_MIN_USECS 10
+#define IXGBE_ITR_ADAPTIVE_MAX_USECS 126
+#define IXGBE_ITR_ADAPTIVE_LATENCY 0x80
+#define IXGBE_ITR_ADAPTIVE_BULK 0x00
+
struct ixgbe_ring_container {
struct ixgbe_ring *ring; /* pointer to linked list of rings */
+ unsigned long next_update; /* jiffies value of last update */
unsigned int total_bytes; /* total bytes processed this int */
unsigned int total_packets; /* total packets processed this int */
u16 work_limit; /* total work allowed per interrupt */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index f1bfae0c41d0..8e2a957aca18 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -806,6 +806,7 @@ static void ixgbe_add_ring(struct ixgbe_ring *ring,
ring->next = head->ring;
head->ring = ring;
head->count++;
+ head->next_update = jiffies + 1;
}
/**
@@ -879,8 +880,11 @@ static int ixgbe_alloc_q_vector(struct ixgbe_adapter *adapter,
/* initialize work limits */
q_vector->tx.work_limit = adapter->tx_work_limit;
- /* initialize pointer to rings */
- ring = q_vector->ring;
+ /* Initialize setting for adaptive ITR */
+ q_vector->tx.itr = IXGBE_ITR_ADAPTIVE_MAX_USECS |
+ IXGBE_ITR_ADAPTIVE_LATENCY;
+ q_vector->rx.itr = IXGBE_ITR_ADAPTIVE_MAX_USECS |
+ IXGBE_ITR_ADAPTIVE_LATENCY;
/* intialize ITR */
if (txr_count && !rxr_count) {
@@ -897,6 +901,9 @@ static int ixgbe_alloc_q_vector(struct ixgbe_adapter *adapter,
q_vector->itr = adapter->rx_itr_setting;
}
+ /* initialize pointer to rings */
+ ring = q_vector->ring;
+
while (txr_count) {
/* assign generic ring traits */
ring->dev = &adapter->pdev->dev;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 211074934d5b..5e2686d106db 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2540,50 +2540,174 @@ enum latency_range {
static void ixgbe_update_itr(struct ixgbe_q_vector *q_vector,
struct ixgbe_ring_container *ring_container)
{
- int bytes = ring_container->total_bytes;
- int packets = ring_container->total_packets;
- u32 timepassed_us;
- u64 bytes_perint;
- u8 itr_setting = ring_container->itr;
+ unsigned int itr = IXGBE_ITR_ADAPTIVE_MIN_USECS |
+ IXGBE_ITR_ADAPTIVE_LATENCY;
+ unsigned int avg_wire_size, packets, bytes;
+ unsigned long next_update = jiffies;
- if (packets == 0)
+ /* If we don't have any rings just leave ourselves set for maximum
+ * possible latency so we take ourselves out of the equation.
+ */
+ if (!ring_container->ring)
return;
- /* simple throttlerate management
- * 0-10MB/s lowest (100000 ints/s)
- * 10-20MB/s low (20000 ints/s)
- * 20-1249MB/s bulk (12000 ints/s)
+ /* If we didn't update within up to 1 - 2 jiffies we can assume
+ * that either packets are coming in so slow there hasn't been
+ * any work, or that there is so much work that NAPI is dealing
+ * with interrupt moderation and we don't need to do anything.
*/
- /* what was last interrupt timeslice? */
- timepassed_us = q_vector->itr >> 2;
- if (timepassed_us == 0)
- return;
+ if (time_after(next_update, ring_container->next_update))
+ goto clear_counts;
- bytes_perint = bytes / timepassed_us; /* bytes/usec */
+ packets = ring_container->total_packets;
- switch (itr_setting) {
- case lowest_latency:
- if (bytes_perint > 10)
- itr_setting = low_latency;
- break;
- case low_latency:
- if (bytes_perint > 20)
- itr_setting = bulk_latency;
- else if (bytes_perint <= 10)
- itr_setting = lowest_latency;
+ /* We have no packets to actually measure against. This means
+ * either one of the other queues on this vector is active or
+ * we are a Tx queue doing TSO with too high of an interrupt rate.
+ *
+ * When this occurs just tick up our delay by the minimum value
+ * and hope that this extra delay will prevent us from being called
+ * without any work on our queue.
+ */
+ if (!packets) {
+ itr = (q_vector->itr >> 2) + IXGBE_ITR_ADAPTIVE_MIN_INC;
+ if (itr > IXGBE_ITR_ADAPTIVE_MAX_USECS)
+ itr = IXGBE_ITR_ADAPTIVE_MAX_USECS;
+ itr += ring_container->itr & IXGBE_ITR_ADAPTIVE_LATENCY;
+ goto clear_counts;
+ }
+
+ bytes = ring_container->total_bytes;
+
+ /* If packets are less than 4 or bytes are less than 9000 assume
+ * insufficient data to use bulk rate limiting approach. We are
+ * likely latency driven.
+ */
+ if (packets < 4 && bytes < 9000) {
+ itr = IXGBE_ITR_ADAPTIVE_LATENCY;
+ goto adjust_by_size;
+ }
+
+ /* Between 4 and 48 we can assume that our current interrupt delay
+ * is only slightly too low. As such we should increase it by a small
+ * fixed amount.
+ */
+ if (packets < 48) {
+ itr = (q_vector->itr >> 2) + IXGBE_ITR_ADAPTIVE_MIN_INC;
+ if (itr > IXGBE_ITR_ADAPTIVE_MAX_USECS)
+ itr = IXGBE_ITR_ADAPTIVE_MAX_USECS;
+ goto clear_counts;
+ }
+
+ /* Between 48 and 96 is our "goldilocks" zone where we are working
+ * out "just right". Just report that our current ITR is good for us.
+ */
+ if (packets < 96) {
+ itr = q_vector->itr >> 2;
+ goto clear_counts;
+ }
+
+ /* If packet count is 96 or greater we are likely looking at a slight
+ * overrun of the delay we want. Try halving our delay to see if that
+ * will cut the number of packets in half per interrupt.
+ */
+ if (packets < 256) {
+ itr = q_vector->itr >> 3;
+ if (itr < IXGBE_ITR_ADAPTIVE_MIN_USECS)
+ itr = IXGBE_ITR_ADAPTIVE_MIN_USECS;
+ goto clear_counts;
+ }
+
+ /* The paths below assume we are dealing with a bulk ITR since number
+ * of packets is 256 or greater. We are just going to have to compute
+ * a value and try to bring the count under control, though for smaller
+ * packet sizes there isn't much we can do as NAPI polling will likely
+ * be kicking in sooner rather than later.
+ */
+ itr = IXGBE_ITR_ADAPTIVE_BULK;
+
+adjust_by_size:
+ /* If packet counts are 256 or greater we can assume we have a gross
+ * overestimation of what the rate should be. Instead of trying to fine
+ * tune it just use the formula below to try and dial in an exact value
+ * give the current packet size of the frame.
+ */
+ avg_wire_size = bytes / packets;
+
+ /* The following is a crude approximation of:
+ * wmem_default / (size + overhead) = desired_pkts_per_int
+ * rate / bits_per_byte / (size + ethernet overhead) = pkt_rate
+ * (desired_pkt_rate / pkt_rate) * usecs_per_sec = ITR value
+ *
+ * Assuming wmem_default is 212992 and overhead is 640 bytes per
+ * packet, (256 skb, 64 headroom, 320 shared info), we can reduce the
+ * formula down to
+ *
+ * (170 * (size + 24)) / (size + 640) = ITR
+ *
+ * We first do some math on the packet size and then finally bitshift
+ * by 8 after rounding up. We also have to account for PCIe link speed
+ * difference as ITR scales based on this.
+ */
+ if (avg_wire_size <= 60) {
+ /* Start at 50k ints/sec */
+ avg_wire_size = 5120;
+ } else if (avg_wire_size <= 316) {
+ /* 50K ints/sec to 16K ints/sec */
+ avg_wire_size *= 40;
+ avg_wire_size += 2720;
+ } else if (avg_wire_size <= 1084) {
+ /* 16K ints/sec to 9.2K ints/sec */
+ avg_wire_size *= 15;
+ avg_wire_size += 11452;
+ } else if (avg_wire_size <= 1980) {
+ /* 9.2K ints/sec to 8K ints/sec */
+ avg_wire_size *= 5;
+ avg_wire_size += 22420;
+ } else {
+ /* plateau at a limit of 8K ints/sec */
+ avg_wire_size = 32256;
+ }
+
+ /* If we are in low latency mode half our delay which doubles the rate
+ * to somewhere between 100K to 16K ints/sec
+ */
+ if (itr & IXGBE_ITR_ADAPTIVE_LATENCY)
+ avg_wire_size >>= 1;
+
+ /* Resultant value is 256 times larger than it needs to be. This
+ * gives us room to adjust the value as needed to either increase
+ * or decrease the value based on link speeds of 10G, 2.5G, 1G, etc.
+ *
+ * Use addition as we have already recorded the new latency flag
+ * for the ITR value.
+ */
+ switch (q_vector->adapter->link_speed) {
+ case IXGBE_LINK_SPEED_10GB_FULL:
+ case IXGBE_LINK_SPEED_100_FULL:
+ default:
+ itr += DIV_ROUND_UP(avg_wire_size,
+ IXGBE_ITR_ADAPTIVE_MIN_INC * 256) *
+ IXGBE_ITR_ADAPTIVE_MIN_INC;
break;
- case bulk_latency:
- if (bytes_perint <= 20)
- itr_setting = low_latency;
+ case IXGBE_LINK_SPEED_2_5GB_FULL:
+ case IXGBE_LINK_SPEED_1GB_FULL:
+ case IXGBE_LINK_SPEED_10_FULL:
+ itr += DIV_ROUND_UP(avg_wire_size,
+ IXGBE_ITR_ADAPTIVE_MIN_INC * 64) *
+ IXGBE_ITR_ADAPTIVE_MIN_INC;
break;
}
- /* clear work counters since we have the values we need */
+clear_counts:
+ /* write back value */
+ ring_container->itr = itr;
+
+ /* next update should occur within next jiffy */
+ ring_container->next_update = next_update + 1;
+
ring_container->total_bytes = 0;
ring_container->total_packets = 0;
-
- /* write updated itr to ring container */
- ring_container->itr = itr_setting;
}
/**
@@ -2625,34 +2749,19 @@ void ixgbe_write_eitr(struct ixgbe_q_vector *q_vector)
static void ixgbe_set_itr(struct ixgbe_q_vector *q_vector)
{
- u32 new_itr = q_vector->itr;
- u8 current_itr;
+ u32 new_itr;
ixgbe_update_itr(q_vector, &q_vector->tx);
ixgbe_update_itr(q_vector, &q_vector->rx);
- current_itr = max(q_vector->rx.itr, q_vector->tx.itr);
+ /* use the smallest value of new ITR delay calculations */
+ new_itr = min(q_vector->rx.itr, q_vector->tx.itr);
- switch (current_itr) {
- /* counts and packets in update_itr are dependent on these numbers */
- case lowest_latency:
- new_itr = IXGBE_100K_ITR;
- break;
- case low_latency:
- new_itr = IXGBE_20K_ITR;
- break;
- case bulk_latency:
- new_itr = IXGBE_12K_ITR;
- break;
- default:
- break;
- }
+ /* Clear latency flag if set, shift into correct position */
+ new_itr &= ~IXGBE_ITR_ADAPTIVE_LATENCY;
+ new_itr <<= 2;
if (new_itr != q_vector->itr) {
- /* do an exponential smoothing */
- new_itr = (10 * new_itr * q_vector->itr) /
- ((9 * new_itr) + q_vector->itr);
-
/* save the algorithm value here */
q_vector->itr = new_itr;
--
2.14.2
^ permalink raw reply related
* [net-next 08/10] ixgbe: fix the FWSM.PT check in ixgbe_mng_present()
From: Jeff Kirsher @ 2017-10-09 18:39 UTC (permalink / raw)
To: davem; +Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20171009184000.80053-1-jeffrey.t.kirsher@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
Bits other than FWSM.PT can be set in IXGBE_SWFW_MODE_MASK making the
previous check invalid.
Change the check for MNG present to be only based on FWSM.PT bit.
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
index 041940c4bb2b..4e5c92dea869 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
@@ -4100,8 +4100,8 @@ bool ixgbe_mng_present(struct ixgbe_hw *hw)
return false;
fwsm = IXGBE_READ_REG(hw, IXGBE_FWSM(hw));
- fwsm &= IXGBE_FWSM_MODE_MASK;
- return fwsm == IXGBE_FWSM_FW_MODE_PT;
+
+ return !!(fwsm & IXGBE_FWSM_FW_MODE_PT);
}
/**
--
2.14.2
^ permalink raw reply related
* [net-next 07/10] ixgbe: fix use of uninitialized padding
From: Jeff Kirsher @ 2017-10-09 18:39 UTC (permalink / raw)
To: davem; +Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20171009184000.80053-1-jeffrey.t.kirsher@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
This patch is resolving Coverity hits where padding in a structure could
be used uninitialized.
- Initialize fwd_cmd.pad/2 before ixgbe_calculate_checksum()
- Initialize buffer.pad2/3 before ixgbe_hic_unlocked()
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
index 2c19070d2a0b..041940c4bb2b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
@@ -3800,10 +3800,10 @@ s32 ixgbe_set_fw_drv_ver_generic(struct ixgbe_hw *hw, u8 maj, u8 min,
fw_cmd.ver_build = build;
fw_cmd.ver_sub = sub;
fw_cmd.hdr.checksum = 0;
- fw_cmd.hdr.checksum = ixgbe_calculate_checksum((u8 *)&fw_cmd,
- (FW_CEM_HDR_LEN + fw_cmd.hdr.buf_len));
fw_cmd.pad = 0;
fw_cmd.pad2 = 0;
+ fw_cmd.hdr.checksum = ixgbe_calculate_checksum((u8 *)&fw_cmd,
+ (FW_CEM_HDR_LEN + fw_cmd.hdr.buf_len));
for (i = 0; i <= FW_CEM_MAX_RETRIES; i++) {
ret_val = ixgbe_host_interface_command(hw, &fw_cmd,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
index 8cea53b62e1b..cb7da5f9c4da 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
@@ -900,6 +900,8 @@ static s32 ixgbe_read_ee_hostif_buffer_X550(struct ixgbe_hw *hw,
/* convert offset from words to bytes */
buffer.address = cpu_to_be32((offset + current_word) * 2);
buffer.length = cpu_to_be16(words_to_read * 2);
+ buffer.pad2 = 0;
+ buffer.pad3 = 0;
status = ixgbe_hic_unlocked(hw, (u32 *)&buffer, sizeof(buffer),
IXGBE_HI_COMMAND_TIMEOUT);
--
2.14.2
^ permalink raw reply related
* [net-next 06/10] ixgbe: add counter for times Rx pages gets allocated, not recycled
From: Jeff Kirsher @ 2017-10-09 18:39 UTC (permalink / raw)
To: davem
Cc: Jesper Dangaard Brouer, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20171009184000.80053-1-jeffrey.t.kirsher@intel.com>
From: Jesper Dangaard Brouer <brouer@redhat.com>
The ixgbe driver have page recycle scheme based around the RX-ring
queue, where a RX page is shared between two packets. Based on the
refcnt, the driver can determine if the RX-page is currently only used
by a single packet, if so it can then directly refill/recycle the
RX-slot by with the opposite "side" of the page.
While this is a clever trick, it is hard to determine when this
recycling is successful and when it fails. Adding a counter, which is
available via ethtool --statistics as 'alloc_rx_page'. Which counts
the number of times the recycle fails and the real page allocator is
invoked. When interpreting the stats, do remember that every alloc
will serve two packets.
The counter is collected per rx_ring, but is summed and ethtool
exported as 'alloc_rx_page'. It would be relevant to know what
rx_ring that cannot keep up, but that can be exported later if
someone experience a need for this.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 ++
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++++
3 files changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index dd5578756ae0..008d0085e01f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -275,6 +275,7 @@ struct ixgbe_rx_queue_stats {
u64 rsc_count;
u64 rsc_flush;
u64 non_eop_descs;
+ u64 alloc_rx_page;
u64 alloc_rx_page_failed;
u64 alloc_rx_buff_failed;
u64 csum_err;
@@ -655,6 +656,7 @@ struct ixgbe_adapter {
u64 rsc_total_count;
u64 rsc_total_flush;
u64 non_eop_descs;
+ u32 alloc_rx_page;
u32 alloc_rx_page_failed;
u32 alloc_rx_buff_failed;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 6d89f28cae06..de5704c7dd1b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -104,6 +104,7 @@ static const struct ixgbe_stats ixgbe_gstrings_stats[] = {
{"tx_flow_control_xoff", IXGBE_STAT(stats.lxofftxc)},
{"rx_flow_control_xoff", IXGBE_STAT(stats.lxoffrxc)},
{"rx_csum_offload_errors", IXGBE_STAT(hw_csum_rx_error)},
+ {"alloc_rx_page", IXGBE_STAT(alloc_rx_page)},
{"alloc_rx_page_failed", IXGBE_STAT(alloc_rx_page_failed)},
{"alloc_rx_buff_failed", IXGBE_STAT(alloc_rx_buff_failed)},
{"rx_no_dma_resources", IXGBE_STAT(hw_rx_no_dma_resources)},
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d83cc9d34de3..211074934d5b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1620,6 +1620,7 @@ static bool ixgbe_alloc_mapped_page(struct ixgbe_ring *rx_ring,
bi->page = page;
bi->page_offset = ixgbe_rx_offset(rx_ring);
bi->pagecnt_bias = 1;
+ rx_ring->rx_stats.alloc_rx_page++;
return true;
}
@@ -6794,6 +6795,7 @@ void ixgbe_update_stats(struct ixgbe_adapter *adapter)
u32 i, missed_rx = 0, mpc, bprc, lxon, lxoff, xon_off_tot;
u64 non_eop_descs = 0, restart_queue = 0, tx_busy = 0;
u64 alloc_rx_page_failed = 0, alloc_rx_buff_failed = 0;
+ u64 alloc_rx_page = 0;
u64 bytes = 0, packets = 0, hw_csum_rx_error = 0;
if (test_bit(__IXGBE_DOWN, &adapter->state) ||
@@ -6814,6 +6816,7 @@ void ixgbe_update_stats(struct ixgbe_adapter *adapter)
for (i = 0; i < adapter->num_rx_queues; i++) {
struct ixgbe_ring *rx_ring = adapter->rx_ring[i];
non_eop_descs += rx_ring->rx_stats.non_eop_descs;
+ alloc_rx_page += rx_ring->rx_stats.alloc_rx_page;
alloc_rx_page_failed += rx_ring->rx_stats.alloc_rx_page_failed;
alloc_rx_buff_failed += rx_ring->rx_stats.alloc_rx_buff_failed;
hw_csum_rx_error += rx_ring->rx_stats.csum_err;
@@ -6821,6 +6824,7 @@ void ixgbe_update_stats(struct ixgbe_adapter *adapter)
packets += rx_ring->stats.packets;
}
adapter->non_eop_descs = non_eop_descs;
+ adapter->alloc_rx_page = alloc_rx_page;
adapter->alloc_rx_page_failed = alloc_rx_page_failed;
adapter->alloc_rx_buff_failed = alloc_rx_buff_failed;
adapter->hw_csum_rx_error = hw_csum_rx_error;
--
2.14.2
^ permalink raw reply related
* [net-next 05/10] ixgbe: split Tx/Rx ring clearing for ethtool loopback test
From: Jeff Kirsher @ 2017-10-09 18:39 UTC (permalink / raw)
To: davem; +Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20171009184000.80053-1-jeffrey.t.kirsher@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
Commit: fed21bcee7a5
("ixgbe: Don't bother clearing buffer memory for descriptor rings)
exposed some issues with the logic in the current implementation of
ixgbe_clean_test_rings() that are being addressed in this patch:
- Split the clearing of the Tx and Rx rings in separate loops. Previously
both Tx and Rx rings were cleared in a rx_desc->wb.upper.length based
loop which could lead to issues if for w/e reason packets were received
outside of the frames transmitted for the loopback test.
- Add check for IXGBE_TXD_STAT_DD to avoid clearing the rings if the
transmits have not comlpeted by the time we enter ixgbe_clean_test_rings()
- Exit early on ixgbe_check_lbtest_frame() failure.
This change fixes a crash during ethtool diagnostic (ethtool -t).
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 53 +++++++++++++++---------
1 file changed, 34 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 72c565712a5f..6d89f28cae06 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -1916,8 +1916,6 @@ static u16 ixgbe_clean_test_rings(struct ixgbe_ring *rx_ring,
unsigned int size)
{
union ixgbe_adv_rx_desc *rx_desc;
- struct ixgbe_rx_buffer *rx_buffer;
- struct ixgbe_tx_buffer *tx_buffer;
u16 rx_ntc, tx_ntc, count = 0;
/* initialize next to clean and descriptor values */
@@ -1925,7 +1923,38 @@ static u16 ixgbe_clean_test_rings(struct ixgbe_ring *rx_ring,
tx_ntc = tx_ring->next_to_clean;
rx_desc = IXGBE_RX_DESC(rx_ring, rx_ntc);
+ while (tx_ntc != tx_ring->next_to_use) {
+ union ixgbe_adv_tx_desc *tx_desc;
+ struct ixgbe_tx_buffer *tx_buffer;
+
+ tx_desc = IXGBE_TX_DESC(tx_ring, tx_ntc);
+
+ /* if DD is not set transmit has not completed */
+ if (!(tx_desc->wb.status & cpu_to_le32(IXGBE_TXD_STAT_DD)))
+ return count;
+
+ /* unmap buffer on Tx side */
+ tx_buffer = &tx_ring->tx_buffer_info[tx_ntc];
+
+ /* Free all the Tx ring sk_buffs */
+ dev_kfree_skb_any(tx_buffer->skb);
+
+ /* unmap skb header data */
+ dma_unmap_single(tx_ring->dev,
+ dma_unmap_addr(tx_buffer, dma),
+ dma_unmap_len(tx_buffer, len),
+ DMA_TO_DEVICE);
+ dma_unmap_len_set(tx_buffer, len, 0);
+
+ /* increment Tx next to clean counter */
+ tx_ntc++;
+ if (tx_ntc == tx_ring->count)
+ tx_ntc = 0;
+ }
+
while (rx_desc->wb.upper.length) {
+ struct ixgbe_rx_buffer *rx_buffer;
+
/* check Rx buffer */
rx_buffer = &rx_ring->rx_buffer_info[rx_ntc];
@@ -1938,6 +1967,8 @@ static u16 ixgbe_clean_test_rings(struct ixgbe_ring *rx_ring,
/* verify contents of skb */
if (ixgbe_check_lbtest_frame(rx_buffer, size))
count++;
+ else
+ break;
/* sync Rx buffer for device write */
dma_sync_single_for_device(rx_ring->dev,
@@ -1945,26 +1976,10 @@ static u16 ixgbe_clean_test_rings(struct ixgbe_ring *rx_ring,
ixgbe_rx_bufsz(rx_ring),
DMA_FROM_DEVICE);
- /* unmap buffer on Tx side */
- tx_buffer = &tx_ring->tx_buffer_info[tx_ntc];
-
- /* Free all the Tx ring sk_buffs */
- dev_kfree_skb_any(tx_buffer->skb);
-
- /* unmap skb header data */
- dma_unmap_single(tx_ring->dev,
- dma_unmap_addr(tx_buffer, dma),
- dma_unmap_len(tx_buffer, len),
- DMA_TO_DEVICE);
- dma_unmap_len_set(tx_buffer, len, 0);
-
- /* increment Rx/Tx next to clean counters */
+ /* increment Rx next to clean counter */
rx_ntc++;
if (rx_ntc == rx_ring->count)
rx_ntc = 0;
- tx_ntc++;
- if (tx_ntc == tx_ring->count)
- tx_ntc = 0;
/* fetch next descriptor */
rx_desc = IXGBE_RX_DESC(rx_ring, rx_ntc);
--
2.14.2
^ permalink raw reply related
* [net-next 03/10] ixgbe: restore normal RSS after last macvlan offload is removed
From: Jeff Kirsher @ 2017-10-09 18:39 UTC (permalink / raw)
To: davem; +Cc: Shannon Nelson, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20171009184000.80053-1-jeffrey.t.kirsher@intel.com>
From: Shannon Nelson <shannon.nelson@oracle.com>
Just like when the last VF is removed, we need to restore normal
operations after the last macvlan offload is removed, else we
get stuck in single queue operations.
To test:
ethtool -l eth1 # note the number of queues in use, ~= cpus
ethtool -K eth1 l2-fwd-offload on
ip link add mv1 link eth1 type macvlan mode bridge
ip link set dev mv1 up
ip link del mv1
ethtool -l eth1 # are we back to the same # of queues, or stuck on 1?
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 3942c6208745..d83cc9d34de3 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -9758,6 +9758,17 @@ static void ixgbe_fwd_del(struct net_device *pdev, void *priv)
limit = find_last_bit(&adapter->fwd_bitmask, 32);
adapter->ring_feature[RING_F_VMDQ].limit = limit + 1;
ixgbe_fwd_ring_down(fwd_adapter->netdev, fwd_adapter);
+
+ /* go back to full RSS if we're done with our VMQs */
+ if (adapter->ring_feature[RING_F_VMDQ].limit == 1) {
+ int rss = min_t(int, ixgbe_max_rss_indices(adapter),
+ num_online_cpus());
+
+ adapter->flags &= ~IXGBE_FLAG_VMDQ_ENABLED;
+ adapter->flags &= ~IXGBE_FLAG_SRIOV_ENABLED;
+ adapter->ring_feature[RING_F_RSS].limit = rss;
+ }
+
ixgbe_setup_tc(pdev, netdev_get_num_tc(pdev));
netdev_dbg(pdev, "pool %i:%i queues %i:%i VSI bitmask %lx\n",
fwd_adapter->pool, adapter->num_rx_pools,
--
2.14.2
^ permalink raw reply related
* [net-next 04/10] ixgbe: add error checks when initializing the PHY
From: Jeff Kirsher @ 2017-10-09 18:39 UTC (permalink / raw)
To: davem; +Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20171009184000.80053-1-jeffrey.t.kirsher@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
Ignoring errors when attempting to identify the PHY can lead to a crash.
Specifically in the case of FW controlled PHYs where the PHY read/write
operations are set to NULL.
Removed redundant comment.
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
index 933c5070f1b6..8cea53b62e1b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
@@ -3192,6 +3192,9 @@ static s32 ixgbe_init_phy_ops_X550em(struct ixgbe_hw *hw)
/* Identify the PHY or SFP module */
ret_val = phy->ops.identify(hw);
+ if (ret_val == IXGBE_ERR_SFP_NOT_SUPPORTED ||
+ ret_val == IXGBE_ERR_PHY_ADDR_INVALID)
+ return ret_val;
/* Setup function pointers based on detected hardware */
ixgbe_init_mac_link_ops_X550em(hw);
@@ -3394,9 +3397,10 @@ static s32 ixgbe_reset_hw_X550em(struct ixgbe_hw *hw)
ixgbe_clear_tx_pending(hw);
/* PHY ops must be identified and initialized prior to reset */
-
- /* Identify PHY and related function pointers */
status = hw->phy.ops.init(hw);
+ if (status == IXGBE_ERR_SFP_NOT_SUPPORTED ||
+ status == IXGBE_ERR_PHY_ADDR_INVALID)
+ return status;
/* start the external PHY */
if (hw->phy.type == ixgbe_phy_x550em_ext_t) {
--
2.14.2
^ permalink raw reply related
* [net-next 02/10] ixgbe: declare ixgbe_mac_operations structures as const
From: Jeff Kirsher @ 2017-10-09 18:39 UTC (permalink / raw)
To: davem; +Cc: Bhumika Goyal, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20171009184000.80053-1-jeffrey.t.kirsher@intel.com>
From: Bhumika Goyal <bhumirks@gmail.com>
Declare ixgbe_mac_operations structures as const as they are only stored
in the mac_ops field of ixgbe_info structure. This field is of type
const and therefore ixgbe_mac_operations structure can be made const
too.
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
index 19fbb2f28ea4..933c5070f1b6 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
@@ -3884,7 +3884,7 @@ static const struct ixgbe_mac_operations mac_ops_X550EM_x_fw = {
.write_iosf_sb_reg = ixgbe_write_iosf_sb_reg_x550,
};
-static struct ixgbe_mac_operations mac_ops_x550em_a = {
+static const struct ixgbe_mac_operations mac_ops_x550em_a = {
X550_COMMON_MAC
.led_on = ixgbe_led_on_t_x550em,
.led_off = ixgbe_led_off_t_x550em,
@@ -3905,7 +3905,7 @@ static struct ixgbe_mac_operations mac_ops_x550em_a = {
.write_iosf_sb_reg = ixgbe_write_iosf_sb_reg_x550a,
};
-static struct ixgbe_mac_operations mac_ops_x550em_a_fw = {
+static const struct ixgbe_mac_operations mac_ops_x550em_a_fw = {
X550_COMMON_MAC
.led_on = ixgbe_led_on_generic,
.led_off = ixgbe_led_off_generic,
--
2.14.2
^ permalink raw reply related
* [net-next 01/10] ixgbe: Clear SWFW_SYNC register during init
From: Jeff Kirsher @ 2017-10-09 18:39 UTC (permalink / raw)
To: davem; +Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20171009184000.80053-1-jeffrey.t.kirsher@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
Added clearing of SW resource bits in the SW/FW synchronization
register to ixgbe_init_swfw_sync_X540().
Updated ixgbe_acquire_swfw_sync_X540 SW Manageability host interface
resource bit error case to match the error handling of the other SW
resource bits. Which is to release the SW resource bits if SW times
out while attempting to acquire the resource.
This allows the driver to load in cases where the semaphore bits
could be stuck after a reset or a crash.
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
index 6ea0d6a5fb90..b8c5fd2a2115 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
@@ -619,12 +619,6 @@ s32 ixgbe_acquire_swfw_sync_X540(struct ixgbe_hw *hw, u32 mask)
usleep_range(5000, 10000);
}
- /* Failed to get SW only semaphore */
- if (swmask == IXGBE_GSSR_SW_MNG_SM) {
- hw_dbg(hw, "Failed to get SW only semaphore\n");
- return IXGBE_ERR_SWFW_SYNC;
- }
-
/* If the resource is not released by the FW/HW the SW can assume that
* the FW/HW malfunctions. In that case the SW should set the SW bit(s)
* of the requested resource(s) while ignoring the corresponding FW/HW
@@ -647,7 +641,8 @@ s32 ixgbe_acquire_swfw_sync_X540(struct ixgbe_hw *hw, u32 mask)
*/
if (swfw_sync & swmask) {
u32 rmask = IXGBE_GSSR_EEP_SM | IXGBE_GSSR_PHY0_SM |
- IXGBE_GSSR_PHY1_SM | IXGBE_GSSR_MAC_CSR_SM;
+ IXGBE_GSSR_PHY1_SM | IXGBE_GSSR_MAC_CSR_SM |
+ IXGBE_GSSR_SW_MNG_SM;
if (swi2c_mask)
rmask |= IXGBE_GSSR_I2C_MASK;
@@ -763,6 +758,8 @@ static void ixgbe_release_swfw_sync_semaphore(struct ixgbe_hw *hw)
**/
void ixgbe_init_swfw_sync_X540(struct ixgbe_hw *hw)
{
+ u32 rmask;
+
/* First try to grab the semaphore but we don't need to bother
* looking to see whether we got the lock or not since we do
* the same thing regardless of whether we got the lock or not.
@@ -771,6 +768,14 @@ void ixgbe_init_swfw_sync_X540(struct ixgbe_hw *hw)
*/
ixgbe_get_swfw_sync_semaphore(hw);
ixgbe_release_swfw_sync_semaphore(hw);
+
+ /* Acquire and release all software resources. */
+ rmask = IXGBE_GSSR_EEP_SM | IXGBE_GSSR_PHY0_SM |
+ IXGBE_GSSR_PHY1_SM | IXGBE_GSSR_MAC_CSR_SM |
+ IXGBE_GSSR_SW_MNG_SM | IXGBE_GSSR_I2C_MASK;
+
+ ixgbe_acquire_swfw_sync_X540(hw, rmask);
+ ixgbe_release_swfw_sync_X540(hw, rmask);
}
/**
--
2.14.2
^ permalink raw reply related
* [net-next 00/10][pull request] 10GbE Intel Wired LAN Driver Updates 2017-10-09
From: Jeff Kirsher @ 2017-10-09 18:39 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jogreene
This series contains updates to ixgbe only.
Emil fixes an issue where the semaphore bits could be stuck after a reset
or a crash, by adding the clearing of software resource bits in the
software/firmware synchronization register. Added error checks when we
attempt to identify and initialize the PHY to prevent a crash. Fixed a
few issues in the logic of ixgbe_clean_test_rings() which was exposed by
a previous commit that was causing a crash in ethtool diagnostics.
Bhumika Goyal fixes a couple of instances which were overlooked when we
made ixgbe_mac_operations constant.
Shannon Nelson fixes an issue to restore normal operations after the
last MACVLAN offload is removed, otherwise we get stuck in a single queue
operations.
The infamous Jesper Dangaard Brouer adds a counter which counts the
number of times the recycle fails and the real page allocator is invoked.
Alex updates the adaptive ITR algorithm to better support the needs of the
network. This attempt to make it so that our ITR algorithm will try to
prevent either starving a socket buffer for memory in the case of
transmit, or overrunning an receive socket buffer on receive. We should
function better with new features like XDP which can handle small packets
at high rates without needing to lock us into NAPI polling mode.
The following are changes since commit c49c777f9c87749b73bc888f097f8a4178382449:
qed: Delete redundant check on dcb_app priority
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 10GbE
Alexander Duyck (1):
ixgbe: Update adaptive ITR algorithm
Bhumika Goyal (1):
ixgbe: declare ixgbe_mac_operations structures as const
Emil Tantilov (6):
ixgbe: Clear SWFW_SYNC register during init
ixgbe: add error checks when initializing the PHY
ixgbe: split Tx/Rx ring clearing for ethtool loopback test
ixgbe: fix use of uninitialized padding
ixgbe: fix the FWSM.PT check in ixgbe_mng_present()
ixgbe: fix crash when injecting AER after failed reset
Jesper Dangaard Brouer (1):
ixgbe: add counter for times Rx pages gets allocated, not recycled
Shannon Nelson (1):
ixgbe: restore normal RSS after last macvlan offload is removed
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 9 +
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 8 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 54 ++++--
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 11 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 233 +++++++++++++++++------
drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c | 19 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 14 +-
7 files changed, 259 insertions(+), 89 deletions(-)
--
2.14.2
^ permalink raw reply
* RE: [PATCH v1 RFC 1/7] Replace license with GPL
From: Tristram.Ha @ 2017-10-09 18:40 UTC (permalink / raw)
To: David.Laight
Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
netdev, linux-kernel, andrew, f.fainelli, pavel, ruediger.schmitt
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD008D3F0@AcuExch.aculab.com>
> From: Tristram.Ha@microchip.com
> > Sent: 06 October 2017 21:33
> > Replace license with GPL.
>
> Don't you need permission from all the people who have updated
> the files in order to make this change?
>
> David
I am a little confused by your comment. The 4 original KSZ9477 DSA
driver files were written by Woojung at Microchip Technology Inc.
There was a complaint the "AS IS" license is not exactly GPL.
It should be submitted formally to net-next instead of a RFC, but it
is probably pointless to do that when there is no code change.
I am hoping these drastic changes of KSZ9477 driver can be accepted
so that the patches can be submitted formally to net-next.
^ 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