Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 0/7] Add clock config and pm support to bcm iProc mdio mux
From: Arun Parameswaran @ 2018-07-26 19:00 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David S. Miller, Florian Fainelli, Rob Herring, Mark Rutland,
	Ray Jui, Scott Branden, Catalin Marinas, Will Deacon, netdev,
	devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <20180726185246.GE10686@lunn.ch>



On 18-07-26 11:52 AM, Andrew Lunn wrote:
> On Thu, Jul 26, 2018 at 11:36:17AM -0700, Arun Parameswaran wrote:
>> Hi,
>> The patchset is based on David Miller's "net" repo.
> 
> Hi Arun
> 
> "net" is intended for bug fixes. This looks like new features, so
> should be against net-next". Please read the netdev FAQ.
> 
>        Andrew
> 
Hi Andrew
The first three patches are 'fixes', and the subsequent patches
(which are new features) depend on the first 3 patches.

Since I had the 'fixes', and the dependency, I used the 'net'
repo.

Please advise if I should still use 'net-next' for all of them.

Thanks
Arun

^ permalink raw reply

* Re: [PATCH net] net/mlx5e: Move mlx5e_priv_flags into en_ethtool.c
From: Saeed Mahameed @ 2018-07-26 18:51 UTC (permalink / raw)
  To: Kamal Heib; +Cc: David S . Miller, Saeed Mahameed, Linux Netdev List
In-Reply-To: <20180715190628.23508-1-kamalheib1@gmail.com>

On Sun, Jul 15, 2018 at 12:06 PM, Kamal Heib <kamalheib1@gmail.com> wrote:
> Move the definition of mlx5e_priv_flags into en_ethtool.c because it's
> only used there.
>
> Fixes: 4e59e2888139 ("net/mlx5e: Introduce net device priv flags infrastructure")
> Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en.h         | 7 -------
>  drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 7 +++++++
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index eb9eb7aa953a..84e6a5b42286 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -208,13 +208,6 @@ struct mlx5e_umr_wqe {
>
>  extern const char mlx5e_self_tests[][ETH_GSTRING_LEN];
>
> -static const char mlx5e_priv_flags[][ETH_GSTRING_LEN] = {
> -       "rx_cqe_moder",
> -       "tx_cqe_moder",
> -       "rx_cqe_compress",
> -       "rx_striding_rq",
> -};
> -

Hi Kamal, on a second thought, i would like to drop this change and
keep mlx5e_priv_flags close/local to the below mlx5e_priv_flag.

Please let me know.

>  enum mlx5e_priv_flag {
>         MLX5E_PFLAG_RX_CQE_BASED_MODER = (1 << 0),
>         MLX5E_PFLAG_TX_CQE_BASED_MODER = (1 << 1),
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> index fffe514ba855..2a1c35d82c2e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> @@ -33,6 +33,13 @@
>  #include "en.h"
>  #include "en/port.h"
>
> +static const char mlx5e_priv_flags[][ETH_GSTRING_LEN] = {
> +       "rx_cqe_moder",
> +       "tx_cqe_moder",
> +       "rx_cqe_compress",
> +       "rx_striding_rq",
> +};
> +
>  void mlx5e_ethtool_get_drvinfo(struct mlx5e_priv *priv,
>                                struct ethtool_drvinfo *drvinfo)
>  {
> --
> 2.14.4
>

^ permalink raw reply

* Re: [PATCH bpf] xdp: add NULL pointer check in __xdp_return()
From: Jakub Kicinski @ 2018-07-26 18:47 UTC (permalink / raw)
  To: Björn Töpel
  Cc: Daniel Borkmann, Björn Töpel, Jesper Dangaard Brouer,
	kafai, Taehee Yoo, ast, Netdev, Karlsson, Magnus
In-Reply-To: <CAJ+HfNh3G0cHs2kb+3tBd+L=SEs3L-qiCxUjyG4M60+JLOKJqQ@mail.gmail.com>

On Thu, 26 Jul 2018 14:13:20 +0200, Björn Töpel wrote:
> Den mån 23 juli 2018 kl 21:58 skrev Jakub Kicinski:
> > On Mon, 23 Jul 2018 11:39:36 +0200, Björn Töpel wrote:  
> > > Den fre 20 juli 2018 kl 22:08 skrev Jakub Kicinski:  
> > > > On Fri, 20 Jul 2018 10:18:21 -0700, Martin KaFai Lau wrote:  
> > > > > On Sat, Jul 21, 2018 at 01:04:45AM +0900, Taehee Yoo wrote:  
> > > > > > rhashtable_lookup() can return NULL. so that NULL pointer
> > > > > > check routine should be added.
> > > > > >
> > > > > > Fixes: 02b55e5657c3 ("xdp: add MEM_TYPE_ZERO_COPY")
> > > > > > Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> > > > > > ---
> > > > > >  net/core/xdp.c | 3 ++-
> > > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > > > > > index 9d1f220..1c12bc7 100644
> > > > > > --- a/net/core/xdp.c
> > > > > > +++ b/net/core/xdp.c
> > > > > > @@ -345,7 +345,8 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
> > > > > >             rcu_read_lock();
> > > > > >             /* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
> > > > > >             xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
> > > > > > -           xa->zc_alloc->free(xa->zc_alloc, handle);
> > > > > > +           if (xa)
> > > > > > +                   xa->zc_alloc->free(xa->zc_alloc, handle);  
> > > > > hmm...It is not clear to me the "!xa" case don't have to be handled?  
> > > >
> > > > Actually I have a more fundamental question about this interface I've
> > > > been meaning to ask.
> > > >
> > > > IIUC free() can happen on any CPU at any time, when whatever device,
> > > > socket or CPU this got redirected to completed the TX.  IOW there may
> > > > be multiple producers.  Drivers would need to create spin lock a'la the
> > > > a9744f7ca200 ("xsk: fix potential race in SKB TX completion code") fix?
> > > >  
> > >
> > > Jakub, apologies for the slow response. I'm still in
> > > "holiday/hammock&beer mode", but will be back in a week. :-P  
> >
> > Ah, sorry to interrupt! :)
> >  
> 
> Don't make it a habit! ;-P

:-D

> > > The idea with the xdp_return_* functions are that an xdp_buff and
> > > xdp_frame can have custom allocations schemes. The difference beween
> > > struct xdp_buff and struct xdp_frame is lifetime. The xdp_buff
> > > lifetime is within the napi context, whereas xdp_frame can have a
> > > lifetime longer/outside the napi context. E.g. for a XDP_REDIRECT
> > > scenario an xdp_buff is converted to a xdp_frame. The conversion is
> > > done in include/net/xdp.h:convert_to_xdp_frame.
> > >
> > > Currently, the zero-copy MEM_TYPE_ZERO_COPY memtype can *only* be used
> > > for xdp_buff, meaning that the lifetime is constrained to a napi
> > > context. Further, given an xdp_buff with memtype MEM_TYPE_ZERO_COPY,
> > > doing XDP_REDIRECT to a target that is *not* an AF_XDP socket would
> > > mean converting the xdp_buff to an xdp_frame. The xdp_frame can then
> > > be free'd on any CPU.
> > >
> > > Note that the xsk_rcv* functions is always called from an napi
> > > context, and therefore is using the xdp_return_buff calls.
> > >
> > > To answer your question -- no, this fix is *not* needed, because the
> > > xdp_buff napi constrained, and the xdp_buff will only be free'd on one
> > > CPU.  
> >
> > Oh, thanks, I missed the check in convert_to_xdp_frame(), so the only
> > frames which can come back via the free path are out of the error path
> > in __xsk_rcv_zc()?
> >
> > That path looks a little surprising too, isn't the expectation that if
> > xdp_do_redirect() returns an error the driver retains the ownership of
> > the buffer?
> >
> > static int __xsk_rcv_zc(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
> > {
> >         int err = xskq_produce_batch_desc(xs->rx, (u64)xdp->handle, len);
> >
> >         if (err) {
> >                 xdp_return_buff(xdp);
> >                 xs->rx_dropped++;
> >         }
> >
> >         return err;
> > }
> >
> > This seems to call xdp_return_buff() *and* return an error.
> >  
> 
> Ugh, this is indeed an error. The xdp_return buff should be removed.
> Thanks for spotting this!
> 
> So, yes, the way to get the buffer back (in ZC) to the driver is via
> the error path (recycling) or via the UMEM fill queue.

Okay, I'm gonna test and submit this later today for bpf tree:

diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 72335c2e8108..4e937cd7c17d 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -84,10 +84,8 @@ static int __xsk_rcv_zc(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
 {
        int err = xskq_produce_batch_desc(xs->rx, (u64)xdp->handle, len);
 
-       if (err) {
-               xdp_return_buff(xdp);
+       if (err)
                xs->rx_dropped++;
-       }
 
        return err;
 }

Now the frame return/ZC allocator ->free path is all dead code :S

^ permalink raw reply related

* Re: [PATCH 5/7] net: phy: Add support to configure clock in Broadcom iProc mdio mux
From: Arun Parameswaran @ 2018-07-26 20:00 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David S. Miller, Florian Fainelli, Rob Herring, Mark Rutland,
	Ray Jui, Scott Branden, Catalin Marinas, Will Deacon, netdev,
	devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <20180726192651.GK10686@lunn.ch>

Hi Andrew

On 18-07-26 12:26 PM, Andrew Lunn wrote:
>> +static void mdio_mux_iproc_config_clk(struct iproc_mdiomux_desc *md)
>> +{
>> +	u32 val;
>> +	u32 divisor;
>> +
>> +	if (md->core_clk) {
>> +		divisor = clk_get_rate(md->core_clk) / MDIO_OPERATING_FREQUENCY;
> 
> /**
>  * clk_get_rate - obtain the current clock rate (in Hz) for a clock source.
>  * 		  This is only valid once the clock source has been enabled.
>  * @clk: clock source
>  */
> unsigned long clk_get_rate(struct clk *clk);
> 
> It is generally good practice to call clk_prepare_enable() sometime
> before clk_get_rate() to ensure the clock is ticking, and to show this
> driver is making use of the clock, so it does not get turned off.

Will add 'clk_prepare_enable()' to the probe.

Thanks
Arun

> 
>> +		divisor = divisor / (MDIO_RATE_ADJ_DIVIDENT + 1);
>> +		val = divisor;
>> +		val |= MDIO_RATE_ADJ_DIVIDENT << MDIO_RATE_ADJ_DIVIDENT_SHIFT;
>> +		writel(val, md->base + MDIO_RATE_ADJ_EXT_OFFSET);
>> +		writel(val, md->base + MDIO_RATE_ADJ_INT_OFFSET);
>> +	}
>> +}

^ permalink raw reply

* [PATCH 5/7] net: phy: Add support to configure clock in Broadcom iProc mdio mux
From: Arun Parameswaran @ 2018-07-26 18:36 UTC (permalink / raw)
  To: David S. Miller, Florian Fainelli, Andrew Lunn, Rob Herring,
	Mark Rutland, Ray Jui, Scott Branden, Catalin Marinas,
	Will Deacon
  Cc: netdev, devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list, Arun Parameswaran
In-Reply-To: <1532630184-29450-1-git-send-email-arun.parameswaran@broadcom.com>

Add support to configure the internal rate adjust register based on the
core clock supplied through device tree in the Broadcom iProc mdio mux.

The operating frequency of the mdio mux block is 11MHz. This is derrived
by dividing the clock to the mdio mux with the rate adjust register.

In some SoC's the default values of the rate adjust register do not yield
11MHz. These SoC's are required to specify the clock via the device tree
for proper operation.

Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
---
 drivers/net/phy/mdio-mux-bcm-iproc.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c
index dc65e95..6b400dd 100644
--- a/drivers/net/phy/mdio-mux-bcm-iproc.c
+++ b/drivers/net/phy/mdio-mux-bcm-iproc.c
@@ -13,7 +13,7 @@
  * You should have received a copy of the GNU General Public License
  * version 2 (GPLv2) along with this source code.
  */
-
+#include <linux/clk.h>
 #include <linux/platform_device.h>
 #include <linux/device.h>
 #include <linux/of_mdio.h>
@@ -22,6 +22,10 @@
 #include <linux/mdio-mux.h>
 #include <linux/delay.h>
 
+#define MDIO_RATE_ADJ_EXT_OFFSET	0x000
+#define MDIO_RATE_ADJ_INT_OFFSET	0x004
+#define MDIO_RATE_ADJ_DIVIDENT_SHIFT	16
+
 #define MDIO_PARAM_OFFSET		0x23c
 #define MDIO_PARAM_MIIM_CYCLE		29
 #define MDIO_PARAM_INTERNAL_SEL		25
@@ -44,13 +48,32 @@
 #define BUS_MAX_ADDR			32
 #define EXT_BUS_START_ADDR		16
 
+#define MDIO_OPERATING_FREQUENCY	11000000
+#define MDIO_RATE_ADJ_DIVIDENT		1
+
 struct iproc_mdiomux_desc {
 	void *mux_handle;
 	void __iomem *base;
 	struct device *dev;
 	struct mii_bus *mii_bus;
+	struct clk *core_clk;
 };
 
+static void mdio_mux_iproc_config_clk(struct iproc_mdiomux_desc *md)
+{
+	u32 val;
+	u32 divisor;
+
+	if (md->core_clk) {
+		divisor = clk_get_rate(md->core_clk) / MDIO_OPERATING_FREQUENCY;
+		divisor = divisor / (MDIO_RATE_ADJ_DIVIDENT + 1);
+		val = divisor;
+		val |= MDIO_RATE_ADJ_DIVIDENT << MDIO_RATE_ADJ_DIVIDENT_SHIFT;
+		writel(val, md->base + MDIO_RATE_ADJ_EXT_OFFSET);
+		writel(val, md->base + MDIO_RATE_ADJ_INT_OFFSET);
+	}
+}
+
 static int iproc_mdio_wait_for_idle(void __iomem *base, bool result)
 {
 	unsigned int timeout = 1000; /* loop for 1s */
@@ -175,6 +198,12 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
 		return PTR_ERR(md->base);
 	}
 
+	md->core_clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(md->core_clk)) {
+		dev_info(&pdev->dev, "core_clk not specified\n");
+		md->core_clk = NULL;
+	}
+
 	md->mii_bus = mdiobus_alloc();
 	if (!md->mii_bus) {
 		dev_err(&pdev->dev, "mdiomux bus alloc failed\n");
@@ -206,6 +235,8 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
 		goto out_register;
 	}
 
+	mdio_mux_iproc_config_clk(md);
+
 	dev_info(md->dev, "iProc mdiomux registered\n");
 	return 0;
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH 3/7] arm64: dts: Fix the base address of the Broadcom iProc mdio mux
From: Arun Parameswaran @ 2018-07-26 18:36 UTC (permalink / raw)
  To: David S. Miller, Florian Fainelli, Andrew Lunn, Rob Herring,
	Mark Rutland, Ray Jui, Scott Branden, Catalin Marinas,
	Will Deacon
  Cc: netdev, devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list, Arun Parameswaran
In-Reply-To: <1532630184-29450-1-git-send-email-arun.parameswaran@broadcom.com>

Modify the base address of the mdio mux driver to point to the start of
the mdio mux block's register address space.

Fixes: fd898f75dac7 ("arm64: dts: Add MDIO multiplexer DT node for Stingray")
Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
---
 arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi    | 4 ++--
 arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi b/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi
index 4057197..1a406a7 100644
--- a/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi
+++ b/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi
@@ -482,9 +482,9 @@
 			status = "disabled";
 		};
 
-		mdio_mux_iproc: mdio-mux@6602023c {
+		mdio_mux_iproc: mdio-mux@66020000 {
 			compatible = "brcm,mdio-mux-iproc";
-			reg = <0x6602023c 0x14>;
+			reg = <0x66020000 0x250>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 
diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
index b203152..a70e8dd 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
+++ b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
@@ -278,9 +278,9 @@
 
 		#include "stingray-pinctrl.dtsi"
 
-		mdio_mux_iproc: mdio-mux@2023c {
+		mdio_mux_iproc: mdio-mux@20000 {
 			compatible = "brcm,mdio-mux-iproc";
-			reg = <0x0002023c 0x14>;
+			reg = <0x00020000 0x250>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH 0/7] Add clock config and pm support to bcm iProc mdio mux
From: Arun Parameswaran @ 2018-07-26 18:36 UTC (permalink / raw)
  To: David S. Miller, Florian Fainelli, Andrew Lunn, Rob Herring,
	Mark Rutland, Ray Jui, Scott Branden, Catalin Marinas,
	Will Deacon
  Cc: netdev, devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list, Arun Parameswaran

Hi,
The patchset is based on David Miller's "net" repo.

The patchset extends the Broadcom iProc mdio mux to add support for
suspend/resume and the ability to configure the internal clock
divider.

The base address of the mdio-mux-bcm-iproc is modified to point to the
start of the mdio block's address space, to be able to access all the
mdio's registers. The missing registers are required to configure the
internal clock divider registers in some of the Broadcom SoC's.

Thanks
Arun

Arun Parameswaran (7):
  dt-bindings: net: Fix Broadcom iProc mdio mux driver base address
  net: phy: Fix the register offsets in Broadcom iProc mdio mux driver
  arm64: dts: Fix the base address of the Broadcom iProc mdio mux
  dt-bindings: net: Add clock handle to Broadcom iProc mdio mux
  net: phy: Add support to configure clock in Broadcom iProc mdio mux
  net: phy: Add pm support to Broadcom iProc mdio mux driver
  net: phy: Add pm support for scan ctrl register to bcm mdio mux

 .../bindings/net/brcm,mdio-mux-iproc.txt           |  7 +-
 arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi   |  4 +-
 .../arm64/boot/dts/broadcom/stingray/stingray.dtsi |  4 +-
 drivers/net/phy/mdio-mux-bcm-iproc.c               | 89 ++++++++++++++++++++--
 4 files changed, 92 insertions(+), 12 deletions(-)

-- 
1.9.1

^ permalink raw reply

* Re: [PATCH 7/7] net: phy: Add pm support for scan ctrl register to bcm mdio mux
From: Andrew Lunn @ 2018-07-26 19:36 UTC (permalink / raw)
  To: Arun Parameswaran
  Cc: David S. Miller, Florian Fainelli, Rob Herring, Mark Rutland,
	Ray Jui, Scott Branden, Catalin Marinas, Will Deacon, netdev,
	devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <170a4731-b091-aef4-2d66-2950114e41fc@broadcom.com>

On Thu, Jul 26, 2018 at 12:33:05PM -0700, Arun Parameswaran wrote:
> 
> 
> On 18-07-26 12:20 PM, Andrew Lunn wrote:
> > On Thu, Jul 26, 2018 at 11:36:24AM -0700, Arun Parameswaran wrote:
> >> Add support for saving and restoring the 'scan control' register
> >> in the Broadcom iProc mdio mux driver.
> > 
> > Hi Arun
> > 
> > I don't see anything setting this register. So why is it necessary to
> > save it over suspend/resume?
> > 
> >      Andrew
> > 
> Hi Andrew
> In the Omega SoC (only), this register was defaulted incorrectly (to use
> a test mode) by the hardware. So we added the setup of the register to
> the early bootloader (ATF).
> 
> We still need to restore this register while resuming from sleep.

Hi Arun

It is better to not rely on the bootloader and have the driver do it.
That makes it easier for people to swap to alternative bootloaders.

     Andrew

^ permalink raw reply

* Re: [PATCH 7/7] net: phy: Add pm support for scan ctrl register to bcm mdio mux
From: Arun Parameswaran @ 2018-07-26 19:33 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David S. Miller, Florian Fainelli, Rob Herring, Mark Rutland,
	Ray Jui, Scott Branden, Catalin Marinas, Will Deacon, netdev,
	devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <20180726192050.GJ10686@lunn.ch>



On 18-07-26 12:20 PM, Andrew Lunn wrote:
> On Thu, Jul 26, 2018 at 11:36:24AM -0700, Arun Parameswaran wrote:
>> Add support for saving and restoring the 'scan control' register
>> in the Broadcom iProc mdio mux driver.
> 
> Hi Arun
> 
> I don't see anything setting this register. So why is it necessary to
> save it over suspend/resume?
> 
>      Andrew
> 
Hi Andrew
In the Omega SoC (only), this register was defaulted incorrectly (to use
a test mode) by the hardware. So we added the setup of the register to
the early bootloader (ATF).

We still need to restore this register while resuming from sleep.

Thanks
Arun

^ permalink raw reply

* Re: [PATCH 2/7] net: phy: Fix the register offsets in Broadcom iProc mdio mux driver
From: Andrew Lunn @ 2018-07-26 19:29 UTC (permalink / raw)
  To: Ray Jui
  Cc: Arun Parameswaran, David S. Miller, Florian Fainelli, Rob Herring,
	Mark Rutland, Ray Jui, Scott Branden, Catalin Marinas,
	Will Deacon, netdev, devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <eeaf777f-e0ea-fb41-5a5e-4ab42a35e10f@broadcom.com>

On Thu, Jul 26, 2018 at 12:25:24PM -0700, Ray Jui wrote:
> 
> 
> On 7/26/2018 12:16 PM, Arun Parameswaran wrote:
> >
> >
> >On 18-07-26 12:06 PM, Andrew Lunn wrote:
> >>On Thu, Jul 26, 2018 at 11:36:19AM -0700, Arun Parameswaran wrote:
> >>>Modify the register offsets in the Broadcom iProc mdio mux to start
> >>>from the top of the register address space.
> >>>
> >>>Earlier the base address specified was from the middle of the block's
> >>>register space. The base address will now point to the start of the
> >>>mdio's address space. The offsets have been fixed to match this.
> >>
> >>Hi Arun
> >>
> >>Did you consider a change something like:
> >That looks good. I will make this change to the patch.
> >
> >Thanks
> >Arun
> 
> To make it backward compatible, then length of the resource also needs to be
> adjusted from 0x14 to 0x250 in the driver?
> 
> Otherwise you will end up accessing areas out of 0x14 defined in old DT that
> is not mapped?

struct resource {
       resource_size_t start;
       resource_size_t end;
       const char *name;
       unsigned long flags;
       unsigned long desc;
       struct resource *parent, *sibling, *child;
};

"end" suggests an address, not a length. But it would be good to look
deeper into the code to be sure.

       Andrew

^ permalink raw reply

* Re: [PATCH 5/7] net: phy: Add support to configure clock in Broadcom iProc mdio mux
From: Andrew Lunn @ 2018-07-26 19:26 UTC (permalink / raw)
  To: Arun Parameswaran
  Cc: David S. Miller, Florian Fainelli, Rob Herring, Mark Rutland,
	Ray Jui, Scott Branden, Catalin Marinas, Will Deacon, netdev,
	devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <1532630184-29450-6-git-send-email-arun.parameswaran@broadcom.com>

> +static void mdio_mux_iproc_config_clk(struct iproc_mdiomux_desc *md)
> +{
> +	u32 val;
> +	u32 divisor;
> +
> +	if (md->core_clk) {
> +		divisor = clk_get_rate(md->core_clk) / MDIO_OPERATING_FREQUENCY;

/**
 * clk_get_rate - obtain the current clock rate (in Hz) for a clock source.
 * 		  This is only valid once the clock source has been enabled.
 * @clk: clock source
 */
unsigned long clk_get_rate(struct clk *clk);

It is generally good practice to call clk_prepare_enable() sometime
before clk_get_rate() to ensure the clock is ticking, and to show this
driver is making use of the clock, so it does not get turned off.

> +		divisor = divisor / (MDIO_RATE_ADJ_DIVIDENT + 1);
> +		val = divisor;
> +		val |= MDIO_RATE_ADJ_DIVIDENT << MDIO_RATE_ADJ_DIVIDENT_SHIFT;
> +		writel(val, md->base + MDIO_RATE_ADJ_EXT_OFFSET);
> +		writel(val, md->base + MDIO_RATE_ADJ_INT_OFFSET);
> +	}
> +}

^ permalink raw reply

* Re: [PATCH 2/7] net: phy: Fix the register offsets in Broadcom iProc mdio mux driver
From: Ray Jui @ 2018-07-26 19:25 UTC (permalink / raw)
  To: Arun Parameswaran, Andrew Lunn
  Cc: David S. Miller, Florian Fainelli, Rob Herring, Mark Rutland,
	Ray Jui, Scott Branden, Catalin Marinas, Will Deacon, netdev,
	devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <6188441f-670d-7644-361b-7a5e8e06c9c2@broadcom.com>



On 7/26/2018 12:16 PM, Arun Parameswaran wrote:
> 
> 
> On 18-07-26 12:06 PM, Andrew Lunn wrote:
>> On Thu, Jul 26, 2018 at 11:36:19AM -0700, Arun Parameswaran wrote:
>>> Modify the register offsets in the Broadcom iProc mdio mux to start
>>> from the top of the register address space.
>>>
>>> Earlier the base address specified was from the middle of the block's
>>> register space. The base address will now point to the start of the
>>> mdio's address space. The offsets have been fixed to match this.
>>
>> Hi Arun
>>
>> Did you consider a change something like:
> That looks good. I will make this change to the patch.
> 
> Thanks
> Arun

To make it backward compatible, then length of the resource also needs 
to be adjusted from 0x14 to 0x250 in the driver?

Otherwise you will end up accessing areas out of 0x14 defined in old DT 
that is not mapped?

Thanks,

Ray

>>
>> diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c
>> index 0831b7142df7..2d53e609498c 100644
>> --- a/drivers/net/phy/mdio-mux-bcm-iproc.c
>> +++ b/drivers/net/phy/mdio-mux-bcm-iproc.c
>> @@ -169,6 +169,12 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
>>          md->dev = &pdev->dev;
>>   
>>          res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +
>> +       if (res->start & 0xfff != 0) {
>> +               dev_info(&pdev->dev, "Please upgrade your device tree blob.\n");
>> +               res->start &= ~0xfff;
>> +       }
>> +
>>          md->base = devm_ioremap_resource(&pdev->dev, res);
>>          if (IS_ERR(md->base)) {
>>                  dev_err(&pdev->dev, "failed to ioremap register\n");
>>
>>
>> 	Andrew
>>

^ permalink raw reply

* Re: [PATCH 7/7] net: phy: Add pm support for scan ctrl register to bcm mdio mux
From: Andrew Lunn @ 2018-07-26 19:20 UTC (permalink / raw)
  To: Arun Parameswaran
  Cc: David S. Miller, Florian Fainelli, Rob Herring, Mark Rutland,
	Ray Jui, Scott Branden, Catalin Marinas, Will Deacon, netdev,
	devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <1532630184-29450-8-git-send-email-arun.parameswaran@broadcom.com>

On Thu, Jul 26, 2018 at 11:36:24AM -0700, Arun Parameswaran wrote:
> Add support for saving and restoring the 'scan control' register
> in the Broadcom iProc mdio mux driver.

Hi Arun

I don't see anything setting this register. So why is it necessary to
save it over suspend/resume?

     Andrew

^ permalink raw reply

* Re: [PATCH 6/7] net: phy: Add pm support to Broadcom iProc mdio mux driver
From: Andrew Lunn @ 2018-07-26 19:16 UTC (permalink / raw)
  To: Arun Parameswaran
  Cc: David S. Miller, Florian Fainelli, Rob Herring, Mark Rutland,
	Ray Jui, Scott Branden, Catalin Marinas, Will Deacon, netdev,
	devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <1532630184-29450-7-git-send-email-arun.parameswaran@broadcom.com>

On Thu, Jul 26, 2018 at 11:36:23AM -0700, Arun Parameswaran wrote:
> Add support for suspend and resume to the Broadcom iProc mdio
> mux driver.

> +#ifdef CONFIG_PM_SLEEP
> +static int mdio_mux_iproc_suspend(struct device *dev)
> +{
> +	int i;
> +	struct iproc_mdiomux_desc *md = dev_get_drvdata(dev);
> +
> +	for (i = 0; i < MDIO_NUM_OF_REGS_TO_RESTORE; i++)
> +		md->restore_regs[i] = readl(md->base +
> +					    restore_reg_offsets[i]);
> +
> +	return 0;
> +}
> +
> +static int mdio_mux_iproc_resume(struct device *dev)
> +{
> +	int i;
> +	struct iproc_mdiomux_desc *md = dev_get_drvdata(dev);
> +
> +	for (i = 0; i < MDIO_NUM_OF_REGS_TO_RESTORE; i++)
> +		writel(md->restore_regs[i],
> +		       md->base + restore_reg_offsets[i]);
> +
> +	return 0;
> +}

Hi Arun

Would it not be simpler to just call mdio_mux_iproc_config_clk() on
resume?

	Andrew

^ permalink raw reply

* (no subject)
From: Fritz Micheal. @ 2018-07-26 16:34 UTC (permalink / raw)




-- 
Do you need a loan at 2% interest rate for any reason ?
Every interested applicant should contact us via the below contact
details.
E-mails: fritzmicloanfirm@financier.com
firtzmicloanfirm@gmail.com

Yours Sincerely
Fritz Micheal.

^ permalink raw reply

* Re: [PATCH 1/7] dt-bindings: net: Fix Broadcom iProc mdio mux driver base address
From: Arun Parameswaran @ 2018-07-26 19:15 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David S. Miller, Florian Fainelli, Rob Herring, Mark Rutland,
	Ray Jui, Scott Branden, Catalin Marinas, Will Deacon, netdev,
	devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <20180726190103.GF10686@lunn.ch>

Hi Andrew

On 18-07-26 12:01 PM, Andrew Lunn wrote:
> On Thu, Jul 26, 2018 at 11:36:18AM -0700, Arun Parameswaran wrote:
>> Modify the base address passed to the Broadcom iProc MDIO mux driver
>> to point to the start of the block's register address space.
>>
>> Fixes: ce8d5dbfd64f ("Add DT binding doc for Broadcom MDIO bus multiplexer")
> 
> Hi Arun
> 
> Fixes generally means something explodes, throws an Opps, userspace
> does the wrong thing. Documentation/process/stable-kernel-rules.rst
> says:
> 
>  - It must fix a real bug that bothers people (not a, "This could be a
>    problem..." type thing).
> 
> Please explain how this is a real problem and what people it bothers.
> 
It is only relevant when we need to modify the registers which are not
currently accessible.

I will remove the 'fixes' and use the 'net-next' repo for the change
set.

> I also don't see any attempt to keep backwards compatibility with
> older device tree blobs. Is it intentional you will break such old
> blobs?
> 
I will modify the patch to keep backward compatibility like you
suggested in the other patch.

Thanks
Arun

> Thanks
> 	Andrew
> 
> 
>> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
>> ---
>>  Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt b/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
>> index dfe287a..dc8aa68 100644
>> --- a/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
>> +++ b/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
>> @@ -18,9 +18,9 @@ at- Documentation/devicetree/bindings/net/mdio-mux.txt
>>  
>>  
>>  for example:
>> -		mdio_mux_iproc: mdio-mux@6602023c {
>> +		mdio_mux_iproc: mdio-mux@66020000 {
>>  			compatible = "brcm,mdio-mux-iproc";
>> -			reg = <0x6602023c 0x14>;
>> +			reg = <0x66020000 0x250>;
>>  			#address-cells = <1>;
>>  			#size-cells = <0>;
>>  
>> -- 
>> 1.9.1
>>

^ permalink raw reply

* Re: [PATCH 5/7] net: phy: Add support to configure clock in Broadcom iProc mdio mux
From: Andrew Lunn @ 2018-07-26 19:13 UTC (permalink / raw)
  To: Arun Parameswaran
  Cc: David S. Miller, Florian Fainelli, Rob Herring, Mark Rutland,
	Ray Jui, Scott Branden, Catalin Marinas, Will Deacon, netdev,
	devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <1532630184-29450-6-git-send-email-arun.parameswaran@broadcom.com>

> @@ -175,6 +198,12 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
>  		return PTR_ERR(md->base);
>  	}
>  
> +	md->core_clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(md->core_clk)) {
> +		dev_info(&pdev->dev, "core_clk not specified\n");
> +		md->core_clk = NULL;
> +	}
> +

In the binding, you say the clock is optional. This is a rather strong
message for something which is optional. I think it would be better to
not output anything.

    Andrew

^ permalink raw reply

* Re: [PATCH 2/7] net: phy: Fix the register offsets in Broadcom iProc mdio mux driver
From: Andrew Lunn @ 2018-07-26 19:06 UTC (permalink / raw)
  To: Arun Parameswaran
  Cc: David S. Miller, Florian Fainelli, Rob Herring, Mark Rutland,
	Ray Jui, Scott Branden, Catalin Marinas, Will Deacon, netdev,
	devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <1532630184-29450-3-git-send-email-arun.parameswaran@broadcom.com>

On Thu, Jul 26, 2018 at 11:36:19AM -0700, Arun Parameswaran wrote:
> Modify the register offsets in the Broadcom iProc mdio mux to start
> from the top of the register address space.
> 
> Earlier the base address specified was from the middle of the block's
> register space. The base address will now point to the start of the
> mdio's address space. The offsets have been fixed to match this.

Hi Arun

Did you consider a change something like:

diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c
index 0831b7142df7..2d53e609498c 100644
--- a/drivers/net/phy/mdio-mux-bcm-iproc.c
+++ b/drivers/net/phy/mdio-mux-bcm-iproc.c
@@ -169,6 +169,12 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
        md->dev = &pdev->dev;
 
        res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+       if (res->start & 0xfff != 0) {
+               dev_info(&pdev->dev, "Please upgrade your device tree blob.\n");
+               res->start &= ~0xfff;
+       }
+
        md->base = devm_ioremap_resource(&pdev->dev, res);
        if (IS_ERR(md->base)) {
                dev_err(&pdev->dev, "failed to ioremap register\n");


	Andrew

^ permalink raw reply related

* Re: [PATCH 1/7] dt-bindings: net: Fix Broadcom iProc mdio mux driver base address
From: Andrew Lunn @ 2018-07-26 19:01 UTC (permalink / raw)
  To: Arun Parameswaran
  Cc: David S. Miller, Florian Fainelli, Rob Herring, Mark Rutland,
	Ray Jui, Scott Branden, Catalin Marinas, Will Deacon, netdev,
	devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <1532630184-29450-2-git-send-email-arun.parameswaran@broadcom.com>

On Thu, Jul 26, 2018 at 11:36:18AM -0700, Arun Parameswaran wrote:
> Modify the base address passed to the Broadcom iProc MDIO mux driver
> to point to the start of the block's register address space.
> 
> Fixes: ce8d5dbfd64f ("Add DT binding doc for Broadcom MDIO bus multiplexer")

Hi Arun

Fixes generally means something explodes, throws an Opps, userspace
does the wrong thing. Documentation/process/stable-kernel-rules.rst
says:

 - It must fix a real bug that bothers people (not a, "This could be a
   problem..." type thing).

Please explain how this is a real problem and what people it bothers.

I also don't see any attempt to keep backwards compatibility with
older device tree blobs. Is it intentional you will break such old
blobs?

Thanks
	Andrew


> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
> ---
>  Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt b/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
> index dfe287a..dc8aa68 100644
> --- a/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
> +++ b/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
> @@ -18,9 +18,9 @@ at- Documentation/devicetree/bindings/net/mdio-mux.txt
>  
>  
>  for example:
> -		mdio_mux_iproc: mdio-mux@6602023c {
> +		mdio_mux_iproc: mdio-mux@66020000 {
>  			compatible = "brcm,mdio-mux-iproc";
> -			reg = <0x6602023c 0x14>;
> +			reg = <0x66020000 0x250>;
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  
> -- 
> 1.9.1
> 

^ permalink raw reply

* [net-next 4/6] igb: Use dma_wmb() instead of wmb() before doorbell writes
From: Jeff Kirsher @ 2018-07-26 17:40 UTC (permalink / raw)
  To: davem; +Cc: Venkatesh Srinivas, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <20180726174050.8923-1-jeffrey.t.kirsher@intel.com>

From: Venkatesh Srinivas <venkateshs@google.com>

igb writes to doorbells to post transmit and receive descriptors;
after writing descriptors to memory but before writing to doorbells,
use dma_wmb() rather than wmb(). wmb() is more heavyweight than
necessary before doorbell writes.

On x86, this avoids SFENCEs before doorbell writes in both the
tx and rx refill paths.

Signed-off-by: Venkatesh Srinivas <venkateshs@google.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index e3a0c02721c9..25720d95d4ea 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6031,7 +6031,7 @@ static int igb_tx_map(struct igb_ring *tx_ring,
 	 * We also need this memory barrier to make certain all of the
 	 * status bits have been updated before next_to_watch is written.
 	 */
-	wmb();
+	dma_wmb();
 
 	/* set next_to_watch value indicating a packet is present */
 	first->next_to_watch = tx_desc;
@@ -8531,7 +8531,7 @@ void igb_alloc_rx_buffers(struct igb_ring *rx_ring, u16 cleaned_count)
 		 * applicable for weak-ordered memory model archs,
 		 * such as IA-64).
 		 */
-		wmb();
+		dma_wmb();
 		writel(i, rx_ring->tail);
 	}
 }
-- 
2.17.1

^ permalink raw reply related

* [net-next 1/6] ixgbe: Do not allow LRO or MTU change with XDP
From: Jeff Kirsher @ 2018-07-26 17:40 UTC (permalink / raw)
  To: davem; +Cc: Tony Nguyen, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180726174050.8923-1-jeffrey.t.kirsher@intel.com>

From: Tony Nguyen <anthony.l.nguyen@intel.com>

XDP does not support jumbo frames or LRO.  These checks are being made
outside the driver when an XDP program is loaded, however, there is
nothing preventing these from changing after an XDP program is loaded.
Add the checks so that while an XDP program is loaded, do not allow MTU
to be changed or LRO to be enabled.

Signed-off-by: Tony Nguyen <anthony.l.nguyen@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 | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 5a6600f7b382..c42256e91997 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6469,6 +6469,11 @@ static int ixgbe_change_mtu(struct net_device *netdev, int new_mtu)
 {
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 
+	if (adapter->xdp_prog) {
+		e_warn(probe, "MTU cannot be changed while XDP program is loaded\n");
+		return -EPERM;
+	}
+
 	/*
 	 * For 82599EB we cannot allow legacy VFs to enable their receive
 	 * paths when MTU greater than 1500 is configured.  So display a
@@ -9407,6 +9412,11 @@ static netdev_features_t ixgbe_fix_features(struct net_device *netdev,
 	if (!(adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE))
 		features &= ~NETIF_F_LRO;
 
+	if (adapter->xdp_prog && (features & NETIF_F_LRO)) {
+		e_dev_err("LRO is not supported with XDP\n");
+		features &= ~NETIF_F_LRO;
+	}
+
 	return features;
 }
 
-- 
2.17.1

^ permalink raw reply related

* [net-next 5/6] ixgbe: Reorder Tx/Rx shutdown to reduce time needed to stop device
From: Jeff Kirsher @ 2018-07-26 17:40 UTC (permalink / raw)
  To: davem; +Cc: Alexander Duyck, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <20180726174050.8923-1-jeffrey.t.kirsher@intel.com>

From: Alexander Duyck <alexander.h.duyck@intel.com>

This change is meant to help reduce the time needed to shutdown the
transmit and receive paths for the device. Specifically what we now do
after this patch is disable the transmit path first at the netdev level,
and then work on disabling the Rx. This way while we are waiting on the Rx
queues to be disabled the Tx queues have an opportunity to drain out.

In addition I have dropped the 10ms timeout that was left in the ixgbe_down
function that seems to have been carried through from back in e1000 as far
as I can tell. We shouldn't need it since we don't actually disable the Tx
until much later and we have additional logic in place for verifying the Tx
queues have been disabled.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Don Buchholz <donald.buchholz@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index c42256e91997..aa4f05c36260 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5814,6 +5814,13 @@ void ixgbe_down(struct ixgbe_adapter *adapter)
 	if (test_and_set_bit(__IXGBE_DOWN, &adapter->state))
 		return; /* do nothing if already down */
 
+	/* Shut off incoming Tx traffic */
+	netif_tx_stop_all_queues(netdev);
+
+	/* call carrier off first to avoid false dev_watchdog timeouts */
+	netif_carrier_off(netdev);
+	netif_tx_disable(netdev);
+
 	/* disable receives */
 	hw->mac.ops.disable_rx(hw);
 
@@ -5822,16 +5829,9 @@ void ixgbe_down(struct ixgbe_adapter *adapter)
 		/* this call also flushes the previous write */
 		ixgbe_disable_rx_queue(adapter, adapter->rx_ring[i]);
 
-	usleep_range(10000, 20000);
-
 	/* synchronize_sched() needed for pending XDP buffers to drain */
 	if (adapter->xdp_ring[0])
 		synchronize_sched();
-	netif_tx_stop_all_queues(netdev);
-
-	/* call carrier off first to avoid false dev_watchdog timeouts */
-	netif_carrier_off(netdev);
-	netif_tx_disable(netdev);
 
 	ixgbe_irq_disable(adapter);
 
-- 
2.17.1

^ permalink raw reply related

* [net-next 3/6] igb: Remove superfluous reset to PHY and page 0 selection
From: Jeff Kirsher @ 2018-07-26 17:40 UTC (permalink / raw)
  To: davem
  Cc: Christian Grönke, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <20180726174050.8923-1-jeffrey.t.kirsher@intel.com>

From: Christian Grönke <c.groenke@infodas.de>

This patch reverts two previous applied patches to fix an issue
that appeared when using SGMII based SFP modules. In the current
state the driver will try to reset the PHY before obtaining the
phy_addr of the SGMII attached PHY. That leads to an error in
e1000_write_phy_reg_sgmii_82575. Causing the initialization to
fail:

    igb: Intel(R) Gigabit Ethernet Network Driver - version 5.4.0-k
    igb: Copyright (c) 2007-2014 Intel Corporation.
    igb: probe of ????:??:??.? failed with error -3

The patches being reverted are:

    commit 182785335447957409282ca745aa5bc3968facee
    Author: Aaron Sierra <asierra@xes-inc.com>
    Date:   Tue Nov 29 10:03:56 2016 -0600

        igb: reset the PHY before reading the PHY ID

    commit 440aeca4b9858248d8f16d724d9fa87a4f65fa33
    Author: Matwey V Kornilov <matwey@sai.msu.ru>
    Date:   Thu Nov 24 13:32:48 2016 +0300

         igb: Explicitly select page 0 at initialization

The first reverted patch directly causes the problem mentioned above.
In case of SGMII the phy_addr is not known at this point and will
only be obtained by 'igb_get_phy_id_82575' further down in the code.
The second removed patch selects forces selection of page 0 in the
PHY. Something that the reset tries to address as well.

As pointed out by Alexander Duzck, the patch below fixes the same
issue but in the proper location:

    commit 4e684f59d760a2c7c716bb60190783546e2d08a1
    Author: Chris J Arges <christopherarges@gmail.com>
    Date:   Wed Nov 2 09:13:42 2016 -0500

        igb: Workaround for igb i210 firmware issue

Reverts: 440aeca4b9858248d8f16d724d9fa87a4f65fa33.
Reverts: 182785335447957409282ca745aa5bc3968facee.

Signed-off-by: Christian Grönke <c.groenke@infodas.de>
Reviewed-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_82575.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c b/drivers/net/ethernet/intel/igb/e1000_82575.c
index b13b42e5a1d9..a795c07d0df7 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.c
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.c
@@ -225,19 +225,7 @@ static s32 igb_init_phy_params_82575(struct e1000_hw *hw)
 	hw->bus.func = (rd32(E1000_STATUS) & E1000_STATUS_FUNC_MASK) >>
 			E1000_STATUS_FUNC_SHIFT;
 
-	/* Make sure the PHY is in a good state. Several people have reported
-	 * firmware leaving the PHY's page select register set to something
-	 * other than the default of zero, which causes the PHY ID read to
-	 * access something other than the intended register.
-	 */
-	ret_val = hw->phy.ops.reset(hw);
-	if (ret_val) {
-		hw_dbg("Error resetting the PHY.\n");
-		goto out;
-	}
-
 	/* Set phy->phy_addr and phy->id. */
-	igb_write_phy_reg_82580(hw, I347AT4_PAGE_SELECT, 0);
 	ret_val = igb_get_phy_id_82575(hw);
 	if (ret_val)
 		return ret_val;
-- 
2.17.1

^ permalink raw reply related

* [net-next 2/6] ixgbe: add ipsec security registers into ethtool register dump
From: Jeff Kirsher @ 2018-07-26 17:40 UTC (permalink / raw)
  To: davem; +Cc: Shannon Nelson, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180726174050.8923-1-jeffrey.t.kirsher@intel.com>

From: Shannon Nelson <shannon.nelson@oracle.com>

Add the ixgbe's security configuration registers into
the register dump.

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_ethtool.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index bd1ba88ec1d5..1d688840cd6c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -511,7 +511,7 @@ static void ixgbe_set_msglevel(struct net_device *netdev, u32 data)
 
 static int ixgbe_get_regs_len(struct net_device *netdev)
 {
-#define IXGBE_REGS_LEN  1139
+#define IXGBE_REGS_LEN  1145
 	return IXGBE_REGS_LEN * sizeof(u32);
 }
 
@@ -874,6 +874,14 @@ static void ixgbe_get_regs(struct net_device *netdev,
 	/* X540 specific DCB registers  */
 	regs_buff[1137] = IXGBE_READ_REG(hw, IXGBE_RTTQCNCR);
 	regs_buff[1138] = IXGBE_READ_REG(hw, IXGBE_RTTQCNTG);
+
+	/* Security config registers */
+	regs_buff[1139] = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+	regs_buff[1140] = IXGBE_READ_REG(hw, IXGBE_SECTXSTAT);
+	regs_buff[1141] = IXGBE_READ_REG(hw, IXGBE_SECTXBUFFAF);
+	regs_buff[1142] = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
+	regs_buff[1143] = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+	regs_buff[1144] = IXGBE_READ_REG(hw, IXGBE_SECRXSTAT);
 }
 
 static int ixgbe_get_eeprom_len(struct net_device *netdev)
-- 
2.17.1

^ permalink raw reply related

* [net-next 0/6][pull request] 10GbE Intel Wired LAN Driver Updates 2018-07-26
From: Jeff Kirsher @ 2018-07-26 17:40 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jogreene

This series contains updates to ixgbe and igb.

Tony fixes ixgbe to add checks to ensure jumbo frames or LRO get enabled
after an XDP program is loaded.

Shannon Nelson adds the missing security configuration registers to the
ixgbe register dump, which will help in debugging.

Christian Grönke fixes an issue in igb that occurs on SGMII based SPF
mdoules, by reverting changes from 2 previous patches.  The issue was
that initialization would fail on the fore mentioned modules because the
driver would try to reset the PHY before obtaining the PHY address of
the SGMII attached PHY.

Venkatesh Srinivas replaces wmb() with dma_wmb() for doorbell writes,
which avoids SFENCEs before the doorbell writes.

Alex cleans up and refactors ixgbe Tx/Rx shutdown to reduce time needed
to stop the device.  The code refactor allows us to take the completion
time into account when disabling queues, so that on some platforms with
higher completion times, would not result in receive queues disabled
messages.

The following are changes since commit dc66fe43b7ebdb53628dcbc1f8f15de3e000aacf:
  rds: send: Fix dead code in rds_sendmsg
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 10GbE

Alexander Duyck (2):
  ixgbe: Reorder Tx/Rx shutdown to reduce time needed to stop device
  ixgbe: Refactor queue disable logic to take completion time into
    account

Christian Grönke (1):
  igb: Remove superfluous reset to PHY and page 0 selection

Shannon Nelson (1):
  ixgbe: add ipsec security registers into ethtool register dump

Tony Nguyen (1):
  ixgbe: Do not allow LRO or MTU change with XDP

Venkatesh Srinivas (1):
  igb: Use dma_wmb() instead of wmb() before doorbell writes

 drivers/net/ethernet/intel/igb/e1000_82575.c  |  12 -
 drivers/net/ethernet/intel/igb/igb_main.c     |   4 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |   3 +-
 .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c  |  42 +--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 300 ++++++++++++++----
 5 files changed, 250 insertions(+), 111 deletions(-)

-- 
2.17.1

^ permalink raw reply


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