Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net] net: mvneta: fix the Rx desc DMA address in the Rx path
From: David Miller @ 2018-06-23  1:41 UTC (permalink / raw)
  To: antoine.tenart
  Cc: gregory.clement, mw, netdev, linux-kernel, thomas.petazzoni,
	maxime.chevallier, miquel.raynal, nadavh, stefanc, ymarkman
In-Reply-To: <20180622081539.16584-1-antoine.tenart@bootlin.com>

From: Antoine Tenart <antoine.tenart@bootlin.com>
Date: Fri, 22 Jun 2018 10:15:39 +0200

> When using s/w buffer management, buffers are allocated and DMA mapped.
> When doing so on an arm64 platform, an offset correction is applied on
> the DMA address, before storing it in an Rx descriptor. The issue is
> this DMA address is then used later in the Rx path without removing the
> offset correction. Thus the DMA address is wrong, which can led to
> various issues.
> 
> This patch fixes this by removing the offset correction from the DMA
> address retrieved from the Rx descriptor before using it in the Rx path.
> 
> Fixes: 8d5047cf9ca2 ("net: mvneta: Convert to be 64 bits compatible")
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>

Applied and queued up for -stable, thank you.

^ permalink raw reply

* Re: [PATCH net] net: mscc: make sparse happy
From: David Miller @ 2018-06-23  1:44 UTC (permalink / raw)
  To: antoine.tenart
  Cc: f.fainelli, andrew, netdev, linux-kernel, thomas.petazzoni,
	alexandre.belloni, quentin.schulz, allan.nielsen
In-Reply-To: <20180622095052.26726-1-antoine.tenart@bootlin.com>

From: Antoine Tenart <antoine.tenart@bootlin.com>
Date: Fri, 22 Jun 2018 11:50:52 +0200

> This patch fixes a sparse warning about using an incorrect type in
> argument 2 of ocelot_write_rix(), as an u32 was expected but a __be32
> was given. The conversion to u32 is forced, which is safe as the value
> will be written as-is in the hardware without any modification.
> 
> Fixes: 08d02364b12f ("net: mscc: fix the injection header")
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>

Applied.

^ permalink raw reply

* Re: [PATCH v4] MAINTAINERS: Add file patterns for dsa device tree bindings
From: David Miller @ 2018-06-23  1:45 UTC (permalink / raw)
  To: geert
  Cc: andrew, vivien.didelot, f.fainelli, robh+dt, mark.rutland, netdev,
	devicetree, linux-kernel
In-Reply-To: <20180622100813.29122-1-geert@linux-m68k.org>

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Fri, 22 Jun 2018 12:08:13 +0200

> Submitters of device tree binding documentation may forget to CC
> the subsystem maintainer if this is missing.
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH net 0/2] net: dccp: fixes around rx_tstamp_last_feedback
From: David Miller @ 2018-06-23  1:47 UTC (permalink / raw)
  To: edumazet; +Cc: netdev, gerrit, dccp, eric.dumazet
In-Reply-To: <20180622134415.104266-1-edumazet@google.com>

From: Eric Dumazet <edumazet@google.com>
Date: Fri, 22 Jun 2018 06:44:13 -0700

> This patch series fix some issues with rx_tstamp_last_feedback.
> 
> - Switch to monotonic clock.
> - Avoid potential overflows on fast hosts/networks.

Series applied and queued up for -stable.

Thank you.

^ permalink raw reply

* Re: [PATCH] net: drivers/net: Convert random_ether_addr to eth_random_addr
From: David Miller @ 2018-06-23  1:50 UTC (permalink / raw)
  To: joe
  Cc: derek.chickles, satananda.burla, felix.manlunas, raghu.vatsavayi,
	ulli.kroll, linus.walleij, yisen.zhuang, salil.mehta,
	jeffrey.t.kirsher, bryan.whitehead, UNGLinuxDriver, harish.patil,
	manish.chopra, Dept-GELinuxNICDev, linux-net-drivers, ecree,
	bkenward, grygorii.strashko, w-kwok2, m-karicheri2, jdmason,
	dave.jiang, allenbh, woojung.huh, ath9k-devel, mareklindner, sw,
	a, kvalo, netdev
In-Reply-To: <c558b59181756dfe647bcdeaf659c386701c0948.1529689773.git.joe@perches.com>

From: Joe Perches <joe@perches.com>
Date: Fri, 22 Jun 2018 10:51:00 -0700

> random_ether_addr is a #define for eth_random_addr which is
> generally preferred in kernel code by ~3:1
> 
> Convert the uses of random_ether_addr to enable removing the #define
> 
> Miscellanea:
> 
> o Convert &vfmac[0] to equivalent vfmac and avoid unnecessary line wrap
> 
> Signed-off-by: Joe Perches <joe@perches.com>

I'll apply this to net-next, thanks Joe.

^ permalink raw reply

* Re: [PATCH] ipv6: avoid copy_from_user() via ipv6_renew_options_kern()
From: David Miller @ 2018-06-23  1:57 UTC (permalink / raw)
  To: pmoore; +Cc: netdev, selinux, linux-security-module
In-Reply-To: <152970230022.7734.15824980755229329454.stgit@chester>

From: Paul Moore <pmoore@redhat.com>
Date: Fri, 22 Jun 2018 17:18:20 -0400

> -	const mm_segment_t old_fs = get_fs();
> -
> -	set_fs(KERNEL_DS);
> -	ret_val = ipv6_renew_options(sk, opt, newtype,
> -				     (struct ipv6_opt_hdr __user *)newopt,
> -				     newoptlen);
> -	set_fs(old_fs);

So is it really the case that the traditional construct:

	set_fs(KERNEL_DS);
	... copy_{from,to}_user(...);
	set_fs(old_fs);

is no longer allowed?

Setting fs to KERNEL_DS is supposed to make user copies work on kernel
memory.  Or at least it did for 20+ years :-)

^ permalink raw reply

* Re: [Patch net] net_sched: remove a bogus warning in hfsc
From: David Miller @ 2018-06-23  1:59 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev, pupilla
In-Reply-To: <20180622213316.8080-1-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Fri, 22 Jun 2018 14:33:16 -0700

> In update_vf():
> 
>   cftree_remove(cl);
>   update_cfmin(cl->cl_parent);
> 
> the cl_cfmin of cl->cl_parent is intentionally updated to 0
> when that parent only has one child. And if this parent is
> root qdisc, we could end up, in hfsc_schedule_watchdog(),
> that we can't decide the next schedule time for qdisc watchdog.
> But it seems safe that we can just skip it, as this watchdog is
> not always scheduled anyway.
> 
> Thanks to Marco for testing all the cases, nothing is broken.
> 
> Reported-by: Marco Berizzi <pupilla@libero.it>
> Tested-by: Marco Berizzi <pupilla@libero.it>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Applied, thanks Cong.

^ permalink raw reply

* Re: [PATCH net-next] netns: get more entropy from net_hash_mix()
From: David Miller @ 2018-06-23  2:00 UTC (permalink / raw)
  To: edumazet; +Cc: netdev, eric.dumazet
In-Reply-To: <20180622232747.145594-1-edumazet@google.com>

From: Eric Dumazet <edumazet@google.com>
Date: Fri, 22 Jun 2018 16:27:47 -0700

> struct net are effectively allocated from order-1 pages on x86,
> with one object per slab, meaning that the 13 low order bits
> of their addresses are zero.
> 
> Once shifted by L1_CACHE_SHIFT, this leaves 7 zero-bits,
> meaning that net_hash_mix() does not help spreading
> objects on various hash tables.
> 
> For example, TCP listen table has 32 buckets, meaning that
> all netns use the same bucket for port 80 or port 443.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Maciej Żenczykowski <maze@google.com>

Good catch, applied, thanks Eric.

^ permalink raw reply

* [PATCH 00/14] net: pch_gbe: Cleanups
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton

This series begins the process of cleaning up the pch_gbe network
driver. Whilst my ultimate goal is to add support for using this driver
on the MIPS Boston development board, this series sets that aside in
favor of making some more general cleanups. My hope is that this will
both make the driver a little more maleable & reduce the probability of
me gouging out my eyes.

Applies cleanly atop net-next as of 5424ea27390f ("netns: get more
entropy from net_hash_mix()").

Thanks,
    Paul

Paul Burton (14):
  net: pch_gbe: Remove unused copybreak parameter
  net: pch_gbe: Remove power_{up,down}_phy HAL abstraction
  net: pch_gbe: Remove read_mac_addr HAL abstraction
  net: pch_gbe: Remove sw_reset_phy HAL abstraction
  net: pch_gbe: Remove reset_phy HAL abstraction
  net: pch_gbe: Remove {read,write}_phy_reg HAL abstraction
  net: pch_gbe: Remove init_hw HAL abstraction
  net: pch_gbe: Remove get_bus_info HAL abstraction
  net: pch_gbe: Remove pch_gbe_hal_setup_init_funcs
  net: pch_gbe: Remove PCH_GBE_MAC_IFOP_RGMII define
  net: pch_gbe: Remove dead RINGFREE code
  net: pch_gbe: Use module_pci_driver()
  net: pch_gbe: Inline pch_gbe_mac_mc_addr_list_update
  net: pch_gbe: Clean up pch_gbe_set_multi

 .../net/ethernet/oki-semi/pch_gbe/Makefile    |   2 +-
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe.h   |  40 ---
 .../ethernet/oki-semi/pch_gbe/pch_gbe_api.c   | 262 ------------------
 .../ethernet/oki-semi/pch_gbe/pch_gbe_api.h   |  35 ---
 .../oki-semi/pch_gbe/pch_gbe_ethtool.c        |  19 +-
 .../ethernet/oki-semi/pch_gbe/pch_gbe_main.c  | 193 ++++---------
 .../ethernet/oki-semi/pch_gbe/pch_gbe_phy.c   |   2 +-
 .../ethernet/oki-semi/pch_gbe/pch_gbe_phy.h   |   2 -
 8 files changed, 66 insertions(+), 489 deletions(-)
 delete mode 100644 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
 delete mode 100644 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h

-- 
2.17.1

^ permalink raw reply

* [PATCH 01/14] net: pch_gbe: Remove unused copybreak parameter
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>

The pch_gbe driver includes a 'copybreak' parameter which appears to
have been copied from the e1000e driver but is entirely unused. Remove
the dead code.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c  | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 34a1581eda95..044a7561752c 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -34,7 +34,6 @@ const char pch_driver_version[] = DRV_VERSION;
 #define PCH_GBE_DMA_ALIGN		0
 #define PCH_GBE_DMA_PADDING		2
 #define PCH_GBE_WATCHDOG_PERIOD		(5 * HZ)	/* watchdog time */
-#define PCH_GBE_COPYBREAK_DEFAULT	256
 #define PCH_GBE_PCI_BAR			1
 #define PCH_GBE_RESERVE_MEMORY		0x200000	/* 2MB */
 
@@ -113,8 +112,6 @@ const char pch_driver_version[] = DRV_VERSION;
 
 #define MINNOW_PHY_RESET_GPIO		13
 
-static unsigned int copybreak __read_mostly = PCH_GBE_COPYBREAK_DEFAULT;
-
 static int pch_gbe_mdio_read(struct net_device *netdev, int addr, int reg);
 static void pch_gbe_mdio_write(struct net_device *netdev, int addr, int reg,
 			       int data);
@@ -2784,14 +2781,6 @@ static int __init pch_gbe_init_module(void)
 
 	pr_info("EG20T PCH Gigabit Ethernet Driver - version %s\n",DRV_VERSION);
 	ret = pci_register_driver(&pch_gbe_driver);
-	if (copybreak != PCH_GBE_COPYBREAK_DEFAULT) {
-		if (copybreak == 0) {
-			pr_info("copybreak disabled\n");
-		} else {
-			pr_info("copybreak enabled for packets <= %u bytes\n",
-				copybreak);
-		}
-	}
 	return ret;
 }
 
@@ -2809,8 +2798,4 @@ MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
 MODULE_DEVICE_TABLE(pci, pch_gbe_pcidev_id);
 
-module_param(copybreak, uint, 0644);
-MODULE_PARM_DESC(copybreak,
-	"Maximum size of packet that is copied to a new buffer on receive");
-
 /* pch_gbe_main.c */
-- 
2.17.1

^ permalink raw reply related

* [PATCH 02/14] net: pch_gbe: Remove power_{up,down}_phy HAL abstraction
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>

For some reason the pch_gbe driver contains a struct pch_gbe_functions
with pointers used by a HAL abstraction layer, even though there is only
one implementation of each function.

This patch removes the power_up_phy & power_down_phy abstractions in
favor of calling pch_phy_power_up & pch_phy_power_down directly.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 .../net/ethernet/oki-semi/pch_gbe/pch_gbe.h   |  4 ----
 .../ethernet/oki-semi/pch_gbe/pch_gbe_api.c   | 22 -------------------
 .../ethernet/oki-semi/pch_gbe/pch_gbe_api.h   |  2 --
 .../ethernet/oki-semi/pch_gbe/pch_gbe_main.c  | 12 +++++-----
 4 files changed, 6 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 697e29dd4bd3..8dc40faef720 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -336,8 +336,6 @@ struct pch_gbe_hw;
  * @write_phy_reg:	for pch_gbe_hal_write_phy_reg
  * @reset_phy:		for pch_gbe_hal_phy_hw_reset
  * @sw_reset_phy:	for pch_gbe_hal_phy_sw_reset
- * @power_up_phy:	for pch_gbe_hal_power_up_phy
- * @power_down_phy:	for pch_gbe_hal_power_down_phy
  * @read_mac_addr:	for pch_gbe_hal_read_mac_addr
  */
 struct pch_gbe_functions {
@@ -347,8 +345,6 @@ struct pch_gbe_functions {
 	s32 (*write_phy_reg) (struct pch_gbe_hw *, u32, u16);
 	void (*reset_phy) (struct pch_gbe_hw *);
 	void (*sw_reset_phy) (struct pch_gbe_hw *);
-	void (*power_up_phy) (struct pch_gbe_hw *hw);
-	void (*power_down_phy) (struct pch_gbe_hw *hw);
 	s32 (*read_mac_addr) (struct pch_gbe_hw *);
 };
 
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
index 51250363566b..d66933b68934 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
@@ -90,8 +90,6 @@ static const struct pch_gbe_functions pch_gbe_ops = {
 	.write_phy_reg     = pch_gbe_phy_write_reg_miic,
 	.reset_phy         = pch_gbe_phy_hw_reset,
 	.sw_reset_phy      = pch_gbe_phy_sw_reset,
-	.power_up_phy      = pch_gbe_phy_power_up,
-	.power_down_phy    = pch_gbe_phy_power_down,
 	.read_mac_addr     = pch_gbe_mac_read_mac_addr
 };
 
@@ -240,23 +238,3 @@ s32 pch_gbe_hal_read_mac_addr(struct pch_gbe_hw *hw)
 	}
 	return hw->func->read_mac_addr(hw);
 }
-
-/**
- * pch_gbe_hal_power_up_phy - Power up PHY
- * @hw:	Pointer to the HW structure
- */
-void pch_gbe_hal_power_up_phy(struct pch_gbe_hw *hw)
-{
-	if (hw->func->power_up_phy)
-		hw->func->power_up_phy(hw);
-}
-
-/**
- * pch_gbe_hal_power_down_phy - Power down PHY
- * @hw:	Pointer to the HW structure
- */
-void pch_gbe_hal_power_down_phy(struct pch_gbe_hw *hw)
-{
-	if (hw->func->power_down_phy)
-		hw->func->power_down_phy(hw);
-}
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
index 91ce07c8306c..be2f202c26c4 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
@@ -29,7 +29,5 @@ s32 pch_gbe_hal_write_phy_reg(struct pch_gbe_hw *hw, u32 offset, u16 data);
 void pch_gbe_hal_phy_hw_reset(struct pch_gbe_hw *hw);
 void pch_gbe_hal_phy_sw_reset(struct pch_gbe_hw *hw);
 s32 pch_gbe_hal_read_mac_addr(struct pch_gbe_hw *hw);
-void pch_gbe_hal_power_up_phy(struct pch_gbe_hw *hw);
-void pch_gbe_hal_power_down_phy(struct pch_gbe_hw *hw);
 
 #endif
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 044a7561752c..13fc828c7fd3 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -2072,7 +2072,7 @@ static int pch_gbe_open(struct net_device *netdev)
 	err = pch_gbe_setup_rx_resources(adapter, adapter->rx_ring);
 	if (err)
 		goto err_setup_rx;
-	pch_gbe_hal_power_up_phy(hw);
+	pch_gbe_phy_power_up(hw);
 	err = pch_gbe_up(adapter);
 	if (err)
 		goto err_up;
@@ -2081,7 +2081,7 @@ static int pch_gbe_open(struct net_device *netdev)
 
 err_up:
 	if (!adapter->wake_up_evt)
-		pch_gbe_hal_power_down_phy(hw);
+		pch_gbe_phy_power_down(hw);
 	pch_gbe_free_rx_resources(adapter, adapter->rx_ring);
 err_setup_rx:
 	pch_gbe_free_tx_resources(adapter, adapter->tx_ring);
@@ -2104,7 +2104,7 @@ static int pch_gbe_stop(struct net_device *netdev)
 
 	pch_gbe_down(adapter);
 	if (!adapter->wake_up_evt)
-		pch_gbe_hal_power_down_phy(hw);
+		pch_gbe_phy_power_down(hw);
 	pch_gbe_free_tx_resources(adapter, adapter->tx_ring);
 	pch_gbe_free_rx_resources(adapter, adapter->rx_ring);
 	return 0;
@@ -2434,7 +2434,7 @@ static pci_ers_result_t pch_gbe_io_slot_reset(struct pci_dev *pdev)
 	}
 	pci_set_master(pdev);
 	pci_enable_wake(pdev, PCI_D0, 0);
-	pch_gbe_hal_power_up_phy(hw);
+	pch_gbe_phy_power_up(hw);
 	pch_gbe_reset(adapter);
 	/* Clear wake up status */
 	pch_gbe_mac_set_wol_event(hw, 0);
@@ -2479,7 +2479,7 @@ static int __pch_gbe_suspend(struct pci_dev *pdev)
 		pch_gbe_mac_set_wol_event(hw, wufc);
 		pci_disable_device(pdev);
 	} else {
-		pch_gbe_hal_power_down_phy(hw);
+		pch_gbe_phy_power_down(hw);
 		pch_gbe_mac_set_wol_event(hw, wufc);
 		pci_disable_device(pdev);
 	}
@@ -2508,7 +2508,7 @@ static int pch_gbe_resume(struct device *device)
 		return err;
 	}
 	pci_set_master(pdev);
-	pch_gbe_hal_power_up_phy(hw);
+	pch_gbe_phy_power_up(hw);
 	pch_gbe_reset(adapter);
 	/* Clear wake on lan control and status */
 	pch_gbe_mac_set_wol_event(hw, 0);
-- 
2.17.1

^ permalink raw reply related

* [PATCH 03/14] net: pch_gbe: Remove read_mac_addr HAL abstraction
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>

For some reason the pch_gbe driver contains a struct pch_gbe_functions
with pointers used by a HAL abstraction layer, even though there is only
one implementation of each function.

This patch removes the read_mac_addr abstraction in favor of calling
pch_gbe_mac_read_mac_addr directly. Since this is defined in the same
translation unit as all of its callers, we can make it static & remove
it from the pch_gbe.h header.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 .../net/ethernet/oki-semi/pch_gbe/pch_gbe.h   |  3 ---
 .../ethernet/oki-semi/pch_gbe/pch_gbe_api.c   | 19 -------------------
 .../ethernet/oki-semi/pch_gbe/pch_gbe_api.h   |  1 -
 .../ethernet/oki-semi/pch_gbe/pch_gbe_main.c  |  4 ++--
 4 files changed, 2 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 8dc40faef720..5dbfcd55efa8 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -336,7 +336,6 @@ struct pch_gbe_hw;
  * @write_phy_reg:	for pch_gbe_hal_write_phy_reg
  * @reset_phy:		for pch_gbe_hal_phy_hw_reset
  * @sw_reset_phy:	for pch_gbe_hal_phy_sw_reset
- * @read_mac_addr:	for pch_gbe_hal_read_mac_addr
  */
 struct pch_gbe_functions {
 	void (*get_bus_info) (struct pch_gbe_hw *);
@@ -345,7 +344,6 @@ struct pch_gbe_functions {
 	s32 (*write_phy_reg) (struct pch_gbe_hw *, u32, u16);
 	void (*reset_phy) (struct pch_gbe_hw *);
 	void (*sw_reset_phy) (struct pch_gbe_hw *);
-	s32 (*read_mac_addr) (struct pch_gbe_hw *);
 };
 
 /**
@@ -676,7 +674,6 @@ void pch_gbe_set_ethtool_ops(struct net_device *netdev);
 
 /* pch_gbe_mac.c */
 s32 pch_gbe_mac_force_mac_fc(struct pch_gbe_hw *hw);
-s32 pch_gbe_mac_read_mac_addr(struct pch_gbe_hw *hw);
 u16 pch_gbe_mac_ctrl_miim(struct pch_gbe_hw *hw, u32 addr, u32 dir, u32 reg,
 			  u16 data);
 #endif /* _PCH_GBE_H_ */
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
index d66933b68934..3c6e009955ab 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
@@ -90,7 +90,6 @@ static const struct pch_gbe_functions pch_gbe_ops = {
 	.write_phy_reg     = pch_gbe_phy_write_reg_miic,
 	.reset_phy         = pch_gbe_phy_hw_reset,
 	.sw_reset_phy      = pch_gbe_phy_sw_reset,
-	.read_mac_addr     = pch_gbe_mac_read_mac_addr
 };
 
 /**
@@ -220,21 +219,3 @@ void pch_gbe_hal_phy_sw_reset(struct pch_gbe_hw *hw)
 	}
 	hw->func->sw_reset_phy(hw);
 }
-
-/**
- * pch_gbe_hal_read_mac_addr - Reads MAC address
- * @hw:	Pointer to the HW structure
- * Returns:
- *	0:	Successfully
- *	ENOSYS:	Function is not registered
- */
-s32 pch_gbe_hal_read_mac_addr(struct pch_gbe_hw *hw)
-{
-	if (!hw->func->read_mac_addr) {
-		struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
-
-		netdev_err(adapter->netdev, "ERROR: configuration\n");
-		return -ENOSYS;
-	}
-	return hw->func->read_mac_addr(hw);
-}
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
index be2f202c26c4..13fcdfb4a94d 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
@@ -28,6 +28,5 @@ s32 pch_gbe_hal_read_phy_reg(struct pch_gbe_hw *hw, u32 offset, u16 *data);
 s32 pch_gbe_hal_write_phy_reg(struct pch_gbe_hw *hw, u32 offset, u16 data);
 void pch_gbe_hal_phy_hw_reset(struct pch_gbe_hw *hw);
 void pch_gbe_hal_phy_sw_reset(struct pch_gbe_hw *hw);
-s32 pch_gbe_hal_read_mac_addr(struct pch_gbe_hw *hw);
 
 #endif
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 13fc828c7fd3..fc5079fa01e8 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -287,7 +287,7 @@ static inline void pch_gbe_mac_load_mac_addr(struct pch_gbe_hw *hw)
  * Returns:
  *	0:			Successful.
  */
-s32 pch_gbe_mac_read_mac_addr(struct pch_gbe_hw *hw)
+static s32 pch_gbe_mac_read_mac_addr(struct pch_gbe_hw *hw)
 {
 	struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
 	u32  adr1a, adr1b;
@@ -2627,7 +2627,7 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 	pch_gbe_hal_get_bus_info(&adapter->hw);
 
 	/* Read the MAC address. and store to the private data */
-	ret = pch_gbe_hal_read_mac_addr(&adapter->hw);
+	ret = pch_gbe_mac_read_mac_addr(&adapter->hw);
 	if (ret) {
 		dev_err(&pdev->dev, "MAC address Read Error\n");
 		goto err_free_adapter;
-- 
2.17.1

^ permalink raw reply related

* [PATCH 04/14] net: pch_gbe: Remove sw_reset_phy HAL abstraction
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>

For some reason the pch_gbe driver contains a struct pch_gbe_functions
with pointers used by a HAL abstraction layer, even though there is only
one implementation of each function.

This patch removes the sw_reset_phy abstraction, which it turns out is
never even used. Its one implementation, which is already called
directly within the same translation unit, can therefore be made static
and removed from the pch_gbe_phy.h header.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h  |  2 --
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c  | 16 ----------------
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h  |  1 -
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c  |  2 +-
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h  |  1 -
 5 files changed, 1 insertion(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 5dbfcd55efa8..47ee7428c3d3 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -335,7 +335,6 @@ struct pch_gbe_hw;
  * @read_phy_reg:	for pch_gbe_hal_read_phy_reg
  * @write_phy_reg:	for pch_gbe_hal_write_phy_reg
  * @reset_phy:		for pch_gbe_hal_phy_hw_reset
- * @sw_reset_phy:	for pch_gbe_hal_phy_sw_reset
  */
 struct pch_gbe_functions {
 	void (*get_bus_info) (struct pch_gbe_hw *);
@@ -343,7 +342,6 @@ struct pch_gbe_functions {
 	s32 (*read_phy_reg) (struct pch_gbe_hw *, u32, u16 *);
 	s32 (*write_phy_reg) (struct pch_gbe_hw *, u32, u16);
 	void (*reset_phy) (struct pch_gbe_hw *);
-	void (*sw_reset_phy) (struct pch_gbe_hw *);
 };
 
 /**
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
index 3c6e009955ab..e1ecfb076029 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
@@ -89,7 +89,6 @@ static const struct pch_gbe_functions pch_gbe_ops = {
 	.read_phy_reg      = pch_gbe_phy_read_reg_miic,
 	.write_phy_reg     = pch_gbe_phy_write_reg_miic,
 	.reset_phy         = pch_gbe_phy_hw_reset,
-	.sw_reset_phy      = pch_gbe_phy_sw_reset,
 };
 
 /**
@@ -204,18 +203,3 @@ void pch_gbe_hal_phy_hw_reset(struct pch_gbe_hw *hw)
 	}
 	hw->func->reset_phy(hw);
 }
-
-/**
- * pch_gbe_hal_phy_sw_reset - Soft PHY reset
- * @hw:	    Pointer to the HW structure
- */
-void pch_gbe_hal_phy_sw_reset(struct pch_gbe_hw *hw)
-{
-	if (!hw->func->sw_reset_phy) {
-		struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
-
-		netdev_err(adapter->netdev, "ERROR: configuration\n");
-		return;
-	}
-	hw->func->sw_reset_phy(hw);
-}
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
index 13fcdfb4a94d..aa802f670055 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
@@ -27,6 +27,5 @@ s32 pch_gbe_hal_init_hw(struct pch_gbe_hw *hw);
 s32 pch_gbe_hal_read_phy_reg(struct pch_gbe_hw *hw, u32 offset, u16 *data);
 s32 pch_gbe_hal_write_phy_reg(struct pch_gbe_hw *hw, u32 offset, u16 data);
 void pch_gbe_hal_phy_hw_reset(struct pch_gbe_hw *hw);
-void pch_gbe_hal_phy_sw_reset(struct pch_gbe_hw *hw);
 
 #endif
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
index a5cad5ea9436..6b35b573beef 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
@@ -184,7 +184,7 @@ s32 pch_gbe_phy_write_reg_miic(struct pch_gbe_hw *hw, u32 offset, u16 data)
  * pch_gbe_phy_sw_reset - PHY software reset
  * @hw:	            Pointer to the HW structure
  */
-void pch_gbe_phy_sw_reset(struct pch_gbe_hw *hw)
+static void pch_gbe_phy_sw_reset(struct pch_gbe_hw *hw)
 {
 	u16 phy_ctrl;
 
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
index 95ad0151ad02..efb955be8cac 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
@@ -26,7 +26,6 @@
 s32 pch_gbe_phy_get_id(struct pch_gbe_hw *hw);
 s32 pch_gbe_phy_read_reg_miic(struct pch_gbe_hw *hw, u32 offset, u16 *data);
 s32 pch_gbe_phy_write_reg_miic(struct pch_gbe_hw *hw, u32 offset, u16 data);
-void pch_gbe_phy_sw_reset(struct pch_gbe_hw *hw);
 void pch_gbe_phy_hw_reset(struct pch_gbe_hw *hw);
 void pch_gbe_phy_power_up(struct pch_gbe_hw *hw);
 void pch_gbe_phy_power_down(struct pch_gbe_hw *hw);
-- 
2.17.1

^ permalink raw reply related

* [PATCH 05/14] net: pch_gbe: Remove reset_phy HAL abstraction
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>

For some reason the pch_gbe driver contains a struct pch_gbe_functions
with pointers used by a HAL abstraction layer, even though there is only
one implementation of each function.

This patch removes the reset_phy abstraction in favor of calling
pch_gbe_phy_hw_reset directly.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h  |  2 --
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c  | 16 ----------------
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h  |  1 -
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c |  4 ++--
 4 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 47ee7428c3d3..02e8da2b6ad2 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -334,14 +334,12 @@ struct pch_gbe_hw;
  * @init_hw:		for pch_gbe_hal_init_hw
  * @read_phy_reg:	for pch_gbe_hal_read_phy_reg
  * @write_phy_reg:	for pch_gbe_hal_write_phy_reg
- * @reset_phy:		for pch_gbe_hal_phy_hw_reset
  */
 struct pch_gbe_functions {
 	void (*get_bus_info) (struct pch_gbe_hw *);
 	s32 (*init_hw) (struct pch_gbe_hw *);
 	s32 (*read_phy_reg) (struct pch_gbe_hw *, u32, u16 *);
 	s32 (*write_phy_reg) (struct pch_gbe_hw *, u32, u16);
-	void (*reset_phy) (struct pch_gbe_hw *);
 };
 
 /**
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
index e1ecfb076029..6fe09af545e8 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
@@ -88,7 +88,6 @@ static const struct pch_gbe_functions pch_gbe_ops = {
 	.init_hw           = pch_gbe_plat_init_hw,
 	.read_phy_reg      = pch_gbe_phy_read_reg_miic,
 	.write_phy_reg     = pch_gbe_phy_write_reg_miic,
-	.reset_phy         = pch_gbe_phy_hw_reset,
 };
 
 /**
@@ -188,18 +187,3 @@ s32 pch_gbe_hal_write_phy_reg(struct pch_gbe_hw *hw, u32 offset,
 		return 0;
 	return hw->func->write_phy_reg(hw, offset, data);
 }
-
-/**
- * pch_gbe_hal_phy_hw_reset - Hard PHY reset
- * @hw:	    Pointer to the HW structure
- */
-void pch_gbe_hal_phy_hw_reset(struct pch_gbe_hw *hw)
-{
-	if (!hw->func->reset_phy) {
-		struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
-
-		netdev_err(adapter->netdev, "ERROR: configuration\n");
-		return;
-	}
-	hw->func->reset_phy(hw);
-}
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
index aa802f670055..96540f6648b5 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
@@ -26,6 +26,5 @@ void pch_gbe_hal_get_bus_info(struct pch_gbe_hw *hw);
 s32 pch_gbe_hal_init_hw(struct pch_gbe_hw *hw);
 s32 pch_gbe_hal_read_phy_reg(struct pch_gbe_hw *hw, u32 offset, u16 *data);
 s32 pch_gbe_hal_write_phy_reg(struct pch_gbe_hw *hw, u32 offset, u16 data);
-void pch_gbe_hal_phy_hw_reset(struct pch_gbe_hw *hw);
 
 #endif
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index fc5079fa01e8..175d6608bdb9 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -2538,7 +2538,7 @@ static void pch_gbe_remove(struct pci_dev *pdev)
 	cancel_work_sync(&adapter->reset_task);
 	unregister_netdev(netdev);
 
-	pch_gbe_hal_phy_hw_reset(&adapter->hw);
+	pch_gbe_phy_hw_reset(&adapter->hw);
 
 	free_netdev(netdev);
 }
@@ -2674,7 +2674,7 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 	return 0;
 
 err_free_adapter:
-	pch_gbe_hal_phy_hw_reset(&adapter->hw);
+	pch_gbe_phy_hw_reset(&adapter->hw);
 err_free_netdev:
 	free_netdev(netdev);
 	return ret;
-- 
2.17.1

^ permalink raw reply related

* [PATCH 06/14] net: pch_gbe: Remove {read,write}_phy_reg HAL abstraction
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>

For some reason the pch_gbe driver contains a struct pch_gbe_functions
with pointers used by a HAL abstraction layer, even though there is only
one implementation of each function.

This patch removes the read_phy_reg & write_phy_reg abstractions in
favor of calling pch_gbe_phy_read_reg_miic & pch_gbe_phy_write_reg_miic
directly.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 .../net/ethernet/oki-semi/pch_gbe/pch_gbe.h   |  4 ---
 .../ethernet/oki-semi/pch_gbe/pch_gbe_api.c   | 36 -------------------
 .../ethernet/oki-semi/pch_gbe/pch_gbe_api.h   |  2 --
 .../oki-semi/pch_gbe/pch_gbe_ethtool.c        |  4 +--
 4 files changed, 2 insertions(+), 44 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 02e8da2b6ad2..728e876bffc6 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -332,14 +332,10 @@ struct pch_gbe_hw;
  * struct  pch_gbe_functions - HAL APi function pointer
  * @get_bus_info:	for pch_gbe_hal_get_bus_info
  * @init_hw:		for pch_gbe_hal_init_hw
- * @read_phy_reg:	for pch_gbe_hal_read_phy_reg
- * @write_phy_reg:	for pch_gbe_hal_write_phy_reg
  */
 struct pch_gbe_functions {
 	void (*get_bus_info) (struct pch_gbe_hw *);
 	s32 (*init_hw) (struct pch_gbe_hw *);
-	s32 (*read_phy_reg) (struct pch_gbe_hw *, u32, u16 *);
-	s32 (*write_phy_reg) (struct pch_gbe_hw *, u32, u16);
 };
 
 /**
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
index 6fe09af545e8..484be4225352 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
@@ -86,8 +86,6 @@ static s32 pch_gbe_plat_init_hw(struct pch_gbe_hw *hw)
 static const struct pch_gbe_functions pch_gbe_ops = {
 	.get_bus_info      = pch_gbe_plat_get_bus_info,
 	.init_hw           = pch_gbe_plat_init_hw,
-	.read_phy_reg      = pch_gbe_phy_read_reg_miic,
-	.write_phy_reg     = pch_gbe_phy_write_reg_miic,
 };
 
 /**
@@ -153,37 +151,3 @@ s32 pch_gbe_hal_init_hw(struct pch_gbe_hw *hw)
 	}
 	return hw->func->init_hw(hw);
 }
-
-/**
- * pch_gbe_hal_read_phy_reg - Reads PHY register
- * @hw:	    Pointer to the HW structure
- * @offset: The register to read
- * @data:   The buffer to store the 16-bit read.
- * Returns:
- *	0:	Successfully
- *	Negative value:	Failed
- */
-s32 pch_gbe_hal_read_phy_reg(struct pch_gbe_hw *hw, u32 offset,
-					u16 *data)
-{
-	if (!hw->func->read_phy_reg)
-		return 0;
-	return hw->func->read_phy_reg(hw, offset, data);
-}
-
-/**
- * pch_gbe_hal_write_phy_reg - Writes PHY register
- * @hw:	    Pointer to the HW structure
- * @offset: The register to read
- * @data:   The value to write.
- * Returns:
- *	0:	Successfully
- *	Negative value:	Failed
- */
-s32 pch_gbe_hal_write_phy_reg(struct pch_gbe_hw *hw, u32 offset,
-					u16 data)
-{
-	if (!hw->func->write_phy_reg)
-		return 0;
-	return hw->func->write_phy_reg(hw, offset, data);
-}
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
index 96540f6648b5..9cd19605f4ff 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
@@ -24,7 +24,5 @@
 s32 pch_gbe_hal_setup_init_funcs(struct pch_gbe_hw *hw);
 void pch_gbe_hal_get_bus_info(struct pch_gbe_hw *hw);
 s32 pch_gbe_hal_init_hw(struct pch_gbe_hw *hw);
-s32 pch_gbe_hal_read_phy_reg(struct pch_gbe_hw *hw, u32 offset, u16 *data);
-s32 pch_gbe_hal_write_phy_reg(struct pch_gbe_hw *hw, u32 offset, u16 data);
 
 #endif
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c
index 731ce1e419e4..da39d771ad87 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c
@@ -125,7 +125,7 @@ static int pch_gbe_set_link_ksettings(struct net_device *netdev,
 	u32 advertising;
 	int ret;
 
-	pch_gbe_hal_write_phy_reg(hw, MII_BMCR, BMCR_RESET);
+	pch_gbe_phy_write_reg_miic(hw, MII_BMCR, BMCR_RESET);
 
 	memcpy(&copy_ecmd, ecmd, sizeof(*ecmd));
 
@@ -204,7 +204,7 @@ static void pch_gbe_get_regs(struct net_device *netdev,
 		*regs_buff++ = ioread32(&hw->reg->INT_ST + i);
 	/* PHY register */
 	for (i = 0; i < PCH_GBE_PHY_REGS_LEN; i++) {
-		pch_gbe_hal_read_phy_reg(&adapter->hw, i, &tmp);
+		pch_gbe_phy_read_reg_miic(&adapter->hw, i, &tmp);
 		*regs_buff++ = tmp;
 	}
 }
-- 
2.17.1

^ permalink raw reply related

* [PATCH 07/14] net: pch_gbe: Remove init_hw HAL abstraction
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>

For some reason the pch_gbe driver contains a struct pch_gbe_functions
with pointers used by a HAL abstraction layer, even though there is only
one implementation of each function.

This patch removes the init_hw abstraction in favor of inlining its
single implementation (pch_gbe_plat_init_hw) into its single caller
(pch_gbe_reset).

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 .../net/ethernet/oki-semi/pch_gbe/pch_gbe.h   |  2 -
 .../ethernet/oki-semi/pch_gbe/pch_gbe_api.c   | 45 -------------------
 .../ethernet/oki-semi/pch_gbe/pch_gbe_api.h   |  1 -
 .../ethernet/oki-semi/pch_gbe/pch_gbe_main.c  | 19 ++++++--
 4 files changed, 15 insertions(+), 52 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 728e876bffc6..2e824baff9d7 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -331,11 +331,9 @@ struct pch_gbe_hw;
 /**
  * struct  pch_gbe_functions - HAL APi function pointer
  * @get_bus_info:	for pch_gbe_hal_get_bus_info
- * @init_hw:		for pch_gbe_hal_init_hw
  */
 struct pch_gbe_functions {
 	void (*get_bus_info) (struct pch_gbe_hw *);
-	s32 (*init_hw) (struct pch_gbe_hw *);
 };
 
 /**
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
index 484be4225352..03fbd4752d4f 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
@@ -57,35 +57,8 @@ static void pch_gbe_plat_get_bus_info(struct pch_gbe_hw *hw)
 	hw->bus.width = pch_gbe_bus_width_pcie_x1;
 }
 
-/**
- * pch_gbe_plat_init_hw - Initialize hardware
- * @hw:	Pointer to the HW structure
- * Returns:
- *	0:		Successfully
- *	Negative value:	Failed-EBUSY
- */
-static s32 pch_gbe_plat_init_hw(struct pch_gbe_hw *hw)
-{
-	s32 ret_val;
-
-	ret_val = pch_gbe_phy_get_id(hw);
-	if (ret_val) {
-		struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
-
-		netdev_err(adapter->netdev, "pch_gbe_phy_get_id error\n");
-		return ret_val;
-	}
-	pch_gbe_phy_init_setting(hw);
-	/* Setup Mac interface option RGMII */
-#ifdef PCH_GBE_MAC_IFOP_RGMII
-	pch_gbe_phy_set_rgmii(hw);
-#endif
-	return ret_val;
-}
-
 static const struct pch_gbe_functions pch_gbe_ops = {
 	.get_bus_info      = pch_gbe_plat_get_bus_info,
-	.init_hw           = pch_gbe_plat_init_hw,
 };
 
 /**
@@ -133,21 +106,3 @@ void pch_gbe_hal_get_bus_info(struct pch_gbe_hw *hw)
 	}
 	hw->func->get_bus_info(hw);
 }
-
-/**
- * pch_gbe_hal_init_hw - Initialize hardware
- * @hw:	Pointer to the HW structure
- * Returns:
- *	0:	Successfully
- *	ENOSYS:	Function is not registered
- */
-s32 pch_gbe_hal_init_hw(struct pch_gbe_hw *hw)
-{
-	if (!hw->func->init_hw) {
-		struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
-
-		netdev_err(adapter->netdev, "ERROR: configuration\n");
-		return -ENOSYS;
-	}
-	return hw->func->init_hw(hw);
-}
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
index 9cd19605f4ff..56cae9cfb5c5 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
@@ -23,6 +23,5 @@
 
 s32 pch_gbe_hal_setup_init_funcs(struct pch_gbe_hw *hw);
 void pch_gbe_hal_get_bus_info(struct pch_gbe_hw *hw);
-s32 pch_gbe_hal_init_hw(struct pch_gbe_hw *hw);
 
 #endif
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 175d6608bdb9..9297a94df999 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -760,14 +760,25 @@ void pch_gbe_reinit_locked(struct pch_gbe_adapter *adapter)
 void pch_gbe_reset(struct pch_gbe_adapter *adapter)
 {
 	struct net_device *netdev = adapter->netdev;
+	struct pch_gbe_hw *hw = &adapter->hw;
+	s32 ret_val;
 
-	pch_gbe_mac_reset_hw(&adapter->hw);
+	pch_gbe_mac_reset_hw(hw);
 	/* reprogram multicast address register after reset */
 	pch_gbe_set_multi(netdev);
 	/* Setup the receive address. */
-	pch_gbe_mac_init_rx_addrs(&adapter->hw, PCH_GBE_MAR_ENTRIES);
-	if (pch_gbe_hal_init_hw(&adapter->hw))
-		netdev_err(netdev, "Hardware Error\n");
+	pch_gbe_mac_init_rx_addrs(hw, PCH_GBE_MAR_ENTRIES);
+
+	ret_val = pch_gbe_phy_get_id(hw);
+	if (ret_val) {
+		netdev_err(adapter->netdev, "pch_gbe_phy_get_id error\n");
+		return;
+	}
+	pch_gbe_phy_init_setting(hw);
+	/* Setup Mac interface option RGMII */
+#ifdef PCH_GBE_MAC_IFOP_RGMII
+	pch_gbe_phy_set_rgmii(hw);
+#endif
 }
 
 /**
-- 
2.17.1

^ permalink raw reply related

* [PATCH 08/14] net: pch_gbe: Remove get_bus_info HAL abstraction
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>

For some reason the pch_gbe driver contains a struct pch_gbe_functions
with pointers used by a HAL abstraction layer, even though there is only
one implementation of each function.

This patch removes the get_bus_info abstraction. Its single
implementation (pch_gbe_plat_get_bus_info) only sets values within a
struct pch_gbe_bus_info which is never used, so we simply remove the
call to it in pch_gbe_probe & remove struct pch_gbe_bus_info entirely.

Now that struct pch_gbe_functions is empty we remove it entirely too.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 .../net/ethernet/oki-semi/pch_gbe/pch_gbe.h   | 23 --------
 .../ethernet/oki-semi/pch_gbe/pch_gbe_api.c   | 58 -------------------
 .../ethernet/oki-semi/pch_gbe/pch_gbe_api.h   |  1 -
 .../ethernet/oki-semi/pch_gbe/pch_gbe_main.c  |  1 -
 4 files changed, 83 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 2e824baff9d7..44c2f291e766 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -326,16 +326,6 @@ struct pch_gbe_regs {
 #define PCH_GBE_FC_FULL			3
 #define PCH_GBE_FC_DEFAULT		PCH_GBE_FC_FULL
 
-
-struct pch_gbe_hw;
-/**
- * struct  pch_gbe_functions - HAL APi function pointer
- * @get_bus_info:	for pch_gbe_hal_get_bus_info
- */
-struct pch_gbe_functions {
-	void (*get_bus_info) (struct pch_gbe_hw *);
-};
-
 /**
  * struct pch_gbe_mac_info - MAC information
  * @addr[6]:		Store the MAC address
@@ -376,17 +366,6 @@ struct pch_gbe_phy_info {
 	u16 autoneg_advertised;
 };
 
-/*!
- * @ingroup Gigabit Ether driver Layer
- * @struct  pch_gbe_bus_info
- * @brief   Bus information
- */
-struct pch_gbe_bus_info {
-	u8 type;
-	u8 speed;
-	u8 width;
-};
-
 /*!
  * @ingroup Gigabit Ether driver Layer
  * @struct  pch_gbe_hw
@@ -398,10 +377,8 @@ struct pch_gbe_hw {
 	struct pch_gbe_regs  __iomem *reg;
 	spinlock_t miim_lock;
 
-	const struct pch_gbe_functions *func;
 	struct pch_gbe_mac_info mac;
 	struct pch_gbe_phy_info phy;
-	struct pch_gbe_bus_info bus;
 };
 
 /**
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
index 03fbd4752d4f..89c0db27b797 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
@@ -20,47 +20,6 @@
 #include "pch_gbe_phy.h"
 #include "pch_gbe_api.h"
 
-/* bus type values */
-#define pch_gbe_bus_type_unknown	0
-#define pch_gbe_bus_type_pci		1
-#define pch_gbe_bus_type_pcix		2
-#define pch_gbe_bus_type_pci_express	3
-#define pch_gbe_bus_type_reserved	4
-
-/* bus speed values */
-#define pch_gbe_bus_speed_unknown	0
-#define pch_gbe_bus_speed_33		1
-#define pch_gbe_bus_speed_66		2
-#define pch_gbe_bus_speed_100		3
-#define pch_gbe_bus_speed_120		4
-#define pch_gbe_bus_speed_133		5
-#define pch_gbe_bus_speed_2500		6
-#define pch_gbe_bus_speed_reserved	7
-
-/* bus width values */
-#define pch_gbe_bus_width_unknown	0
-#define pch_gbe_bus_width_pcie_x1	1
-#define pch_gbe_bus_width_pcie_x2	2
-#define pch_gbe_bus_width_pcie_x4	4
-#define pch_gbe_bus_width_32		5
-#define pch_gbe_bus_width_64		6
-#define pch_gbe_bus_width_reserved	7
-
-/**
- * pch_gbe_plat_get_bus_info - Obtain bus information for adapter
- * @hw:	Pointer to the HW structure
- */
-static void pch_gbe_plat_get_bus_info(struct pch_gbe_hw *hw)
-{
-	hw->bus.type  = pch_gbe_bus_type_pci_express;
-	hw->bus.speed = pch_gbe_bus_speed_2500;
-	hw->bus.width = pch_gbe_bus_width_pcie_x1;
-}
-
-static const struct pch_gbe_functions pch_gbe_ops = {
-	.get_bus_info      = pch_gbe_plat_get_bus_info,
-};
-
 /**
  * pch_gbe_plat_init_function_pointers - Init func ptrs
  * @hw:	Pointer to the HW structure
@@ -69,8 +28,6 @@ static void pch_gbe_plat_init_function_pointers(struct pch_gbe_hw *hw)
 {
 	/* Set PHY parameter */
 	hw->phy.reset_delay_us     = PCH_GBE_PHY_RESET_DELAY_US;
-	/* Set function pointers */
-	hw->func = &pch_gbe_ops;
 }
 
 /**
@@ -91,18 +48,3 @@ s32 pch_gbe_hal_setup_init_funcs(struct pch_gbe_hw *hw)
 	pch_gbe_plat_init_function_pointers(hw);
 	return 0;
 }
-
-/**
- * pch_gbe_hal_get_bus_info - Obtain bus information for adapter
- * @hw:	Pointer to the HW structure
- */
-void pch_gbe_hal_get_bus_info(struct pch_gbe_hw *hw)
-{
-	if (!hw->func->get_bus_info) {
-		struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
-
-		netdev_err(adapter->netdev, "ERROR: configuration\n");
-		return;
-	}
-	hw->func->get_bus_info(hw);
-}
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
index 56cae9cfb5c5..b3b713a32f38 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
@@ -22,6 +22,5 @@
 #include "pch_gbe_phy.h"
 
 s32 pch_gbe_hal_setup_init_funcs(struct pch_gbe_hw *hw);
-void pch_gbe_hal_get_bus_info(struct pch_gbe_hw *hw);
 
 #endif
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 9297a94df999..246167dbeacd 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -2635,7 +2635,6 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 		dev_err(&pdev->dev, "PHY initialize error\n");
 		goto err_free_adapter;
 	}
-	pch_gbe_hal_get_bus_info(&adapter->hw);
 
 	/* Read the MAC address. and store to the private data */
 	ret = pch_gbe_mac_read_mac_addr(&adapter->hw);
-- 
2.17.1

^ permalink raw reply related

* [PATCH 09/14] net: pch_gbe: Remove pch_gbe_hal_setup_init_funcs
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>

The pch_gbe driver calls a pch_gbe_hal_setup_init_funcs function which
ultimately sets the value of one field in struct pch_gbe_phy_info in a
convoluted way.

This patch removes pch_gbe_hal_setup_init_funcs in favor of inlining it,
and in turn its callee pch_gbe_plat_init_function_pointers, into the
single caller pch_gbe_sw_init.

With this pch_gbe_api.c & pch_gbe_api.h are essentially empty, so they
are removed & inclusions of the latter replaced with pch_gbe_phy.h which
was previously being included via pch_gbe_api.h.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 .../net/ethernet/oki-semi/pch_gbe/Makefile    |  2 +-
 .../ethernet/oki-semi/pch_gbe/pch_gbe_api.c   | 50 -------------------
 .../ethernet/oki-semi/pch_gbe/pch_gbe_api.h   | 26 ----------
 .../oki-semi/pch_gbe/pch_gbe_ethtool.c        |  2 +-
 .../ethernet/oki-semi/pch_gbe/pch_gbe_main.c  |  8 +--
 5 files changed, 4 insertions(+), 84 deletions(-)
 delete mode 100644 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
 delete mode 100644 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/Makefile b/drivers/net/ethernet/oki-semi/pch_gbe/Makefile
index 31288d4ad248..862de0f3bc41 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/Makefile
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/Makefile
@@ -1,4 +1,4 @@
 obj-$(CONFIG_PCH_GBE) += pch_gbe.o
 
 pch_gbe-y := pch_gbe_phy.o pch_gbe_ethtool.o pch_gbe_param.o
-pch_gbe-y += pch_gbe_api.o pch_gbe_main.o
+pch_gbe-y += pch_gbe_main.o
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
deleted file mode 100644
index 89c0db27b797..000000000000
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
+++ /dev/null
@@ -1,50 +0,0 @@
-/*
- * Copyright (C) 1999 - 2010 Intel Corporation.
- * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD.
- *
- * This code was derived from the Intel e1000e Linux driver.
- *
- * 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; 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.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see <http://www.gnu.org/licenses/>.
- */
-#include "pch_gbe.h"
-#include "pch_gbe_phy.h"
-#include "pch_gbe_api.h"
-
-/**
- * pch_gbe_plat_init_function_pointers - Init func ptrs
- * @hw:	Pointer to the HW structure
- */
-static void pch_gbe_plat_init_function_pointers(struct pch_gbe_hw *hw)
-{
-	/* Set PHY parameter */
-	hw->phy.reset_delay_us     = PCH_GBE_PHY_RESET_DELAY_US;
-}
-
-/**
- * pch_gbe_hal_setup_init_funcs - Initializes function pointers
- * @hw:	Pointer to the HW structure
- * Returns:
- *	0:	Successfully
- *	ENOSYS:	Function is not registered
- */
-s32 pch_gbe_hal_setup_init_funcs(struct pch_gbe_hw *hw)
-{
-	if (!hw->reg) {
-		struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
-
-		netdev_err(adapter->netdev, "ERROR: Registers not mapped\n");
-		return -ENOSYS;
-	}
-	pch_gbe_plat_init_function_pointers(hw);
-	return 0;
-}
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
deleted file mode 100644
index b3b713a32f38..000000000000
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/*
- * Copyright (C) 1999 - 2010 Intel Corporation.
- * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD.
- *
- * This code was derived from the Intel e1000e Linux driver.
- *
- * 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; 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.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see <http://www.gnu.org/licenses/>.
- */
-#ifndef _PCH_GBE_API_H_
-#define _PCH_GBE_API_H_
-
-#include "pch_gbe_phy.h"
-
-s32 pch_gbe_hal_setup_init_funcs(struct pch_gbe_hw *hw);
-
-#endif
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c
index da39d771ad87..a7bdb53790ff 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c
@@ -17,7 +17,7 @@
  * along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 #include "pch_gbe.h"
-#include "pch_gbe_api.h"
+#include "pch_gbe_phy.h"
 
 /**
  * pch_gbe_stats - Stats item information
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 246167dbeacd..5846e8cf1750 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -18,7 +18,7 @@
  */
 
 #include "pch_gbe.h"
-#include "pch_gbe_api.h"
+#include "pch_gbe_phy.h"
 #include <linux/module.h>
 #include <linux/net_tstamp.h>
 #include <linux/ptp_classify.h>
@@ -2037,12 +2037,8 @@ static int pch_gbe_sw_init(struct pch_gbe_adapter *adapter)
 	adapter->rx_buffer_len = PCH_GBE_FRAME_SIZE_2048;
 	hw->mac.max_frame_size = netdev->mtu + ETH_HLEN + ETH_FCS_LEN;
 	hw->mac.min_frame_size = ETH_ZLEN + ETH_FCS_LEN;
+	hw->phy.reset_delay_us = PCH_GBE_PHY_RESET_DELAY_US;
 
-	/* Initialize the hardware-specific values */
-	if (pch_gbe_hal_setup_init_funcs(hw)) {
-		netdev_err(netdev, "Hardware Initialization Failure\n");
-		return -EIO;
-	}
 	if (pch_gbe_alloc_queues(adapter)) {
 		netdev_err(netdev, "Unable to allocate memory for queues\n");
 		return -ENOMEM;
-- 
2.17.1

^ permalink raw reply related

* [PATCH 10/14] net: pch_gbe: Remove PCH_GBE_MAC_IFOP_RGMII define
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>

The pch_gbe driver currently presumes that the PHY is connected using
RGMII, and would need further work to support other buses. It includes a
define which is always set that conditionalises some of the
RGMII-specific code regardless. Remove it. If we do ever support
different MII buses then preprocessor defines won't be the best way to
select between them anyway.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 9 ---------
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h  | 1 -
 2 files changed, 10 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 5846e8cf1750..11c42aa42b8a 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -366,9 +366,7 @@ static void pch_gbe_mac_reset_hw(struct pch_gbe_hw *hw)
 	/* Read the MAC address. and store to the private data */
 	pch_gbe_mac_read_mac_addr(hw);
 	iowrite32(PCH_GBE_ALL_RST, &hw->reg->RESET);
-#ifdef PCH_GBE_MAC_IFOP_RGMII
 	iowrite32(PCH_GBE_MODE_GMII_ETHER, &hw->reg->MODE);
-#endif
 	pch_gbe_wait_clr_bit(&hw->reg->RESET, PCH_GBE_ALL_RST);
 	/* Setup the receive addresses */
 	pch_gbe_mac_mar_set(hw, hw->mac.addr, 0);
@@ -776,9 +774,7 @@ void pch_gbe_reset(struct pch_gbe_adapter *adapter)
 	}
 	pch_gbe_phy_init_setting(hw);
 	/* Setup Mac interface option RGMII */
-#ifdef PCH_GBE_MAC_IFOP_RGMII
 	pch_gbe_phy_set_rgmii(hw);
-#endif
 }
 
 /**
@@ -1044,7 +1040,6 @@ static void pch_gbe_set_rgmii_ctrl(struct pch_gbe_adapter *adapter, u16 speed,
 	unsigned long rgmii = 0;
 
 	/* Set the RGMII control. */
-#ifdef PCH_GBE_MAC_IFOP_RGMII
 	switch (speed) {
 	case SPEED_10:
 		rgmii = (PCH_GBE_RGMII_RATE_2_5M |
@@ -1060,10 +1055,6 @@ static void pch_gbe_set_rgmii_ctrl(struct pch_gbe_adapter *adapter, u16 speed,
 		break;
 	}
 	iowrite32(rgmii, &hw->reg->RGMII_CTRL);
-#else	/* GMII */
-	rgmii = 0;
-	iowrite32(rgmii, &hw->reg->RGMII_CTRL);
-#endif
 }
 static void pch_gbe_set_mode(struct pch_gbe_adapter *adapter, u16 speed,
 			      u16 duplex)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
index efb955be8cac..23ac38711619 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
@@ -21,7 +21,6 @@
 
 #define PCH_GBE_PHY_REGS_LEN		32
 #define	PCH_GBE_PHY_RESET_DELAY_US	10
-#define PCH_GBE_MAC_IFOP_RGMII
 
 s32 pch_gbe_phy_get_id(struct pch_gbe_hw *hw);
 s32 pch_gbe_phy_read_reg_miic(struct pch_gbe_hw *hw, u32 offset, u16 *data);
-- 
2.17.1

^ permalink raw reply related

* [PATCH 11/14] net: pch_gbe: Remove dead RINGFREE code
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>

The pch_gbe driver includes some code which appears to be an attempt to
work around a problem with the pch_gbe_free_rx_resources &
pch_gbe_free_tx_resources functions that no longer exists. Remove the
code guarded by the never-defined RINGFREE preprocessor macro.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c
index a7bdb53790ff..adaa0024adfe 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c
@@ -349,25 +349,12 @@ static int pch_gbe_set_ringparam(struct net_device *netdev,
 		err = pch_gbe_setup_tx_resources(adapter, adapter->tx_ring);
 		if (err)
 			goto err_setup_tx;
-		/* save the new, restore the old in order to free it,
-		 * then restore the new back again */
-#ifdef RINGFREE
-		adapter->rx_ring = rx_old;
-		adapter->tx_ring = tx_old;
-		pch_gbe_free_rx_resources(adapter, adapter->rx_ring);
-		pch_gbe_free_tx_resources(adapter, adapter->tx_ring);
-		kfree(tx_old);
-		kfree(rx_old);
-		adapter->rx_ring = rxdr;
-		adapter->tx_ring = txdr;
-#else
 		pch_gbe_free_rx_resources(adapter, rx_old);
 		pch_gbe_free_tx_resources(adapter, tx_old);
 		kfree(tx_old);
 		kfree(rx_old);
 		adapter->rx_ring = rxdr;
 		adapter->tx_ring = txdr;
-#endif
 		err = pch_gbe_up(adapter);
 	}
 	return err;
-- 
2.17.1

^ permalink raw reply related

* [PATCH 12/14] net: pch_gbe: Use module_pci_driver()
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>

Make use of the module_pci_driver() macro to remove some needless
boilerplate code from the pch_gbe driver. This does have the side effect
of removing the print of the driver's version during probe, but this is
pretty useless information anyway - the version has changed only once
whilst the driver has been in mainline, despite many changes being made
to it before and since.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 .../ethernet/oki-semi/pch_gbe/pch_gbe_main.c  | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 11c42aa42b8a..3f2dd36d45ad 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -2770,24 +2770,7 @@ static struct pci_driver pch_gbe_driver = {
 	.shutdown = pch_gbe_shutdown,
 	.err_handler = &pch_gbe_err_handler
 };
-
-
-static int __init pch_gbe_init_module(void)
-{
-	int ret;
-
-	pr_info("EG20T PCH Gigabit Ethernet Driver - version %s\n",DRV_VERSION);
-	ret = pci_register_driver(&pch_gbe_driver);
-	return ret;
-}
-
-static void __exit pch_gbe_exit_module(void)
-{
-	pci_unregister_driver(&pch_gbe_driver);
-}
-
-module_init(pch_gbe_init_module);
-module_exit(pch_gbe_exit_module);
+module_pci_driver(pch_gbe_driver);
 
 MODULE_DESCRIPTION("EG20T PCH Gigabit ethernet Driver");
 MODULE_AUTHOR("LAPIS SEMICONDUCTOR, <tshimizu818@gmail.com>");
-- 
2.17.1

^ permalink raw reply related

* [PATCH 13/14] net: pch_gbe: Inline pch_gbe_mac_mc_addr_list_update
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>

The pch_gbe driver sets up multicast address filters using a convoluted
mechanism by which pch_gbe_set_multi allocates an array to hold
multicast addresses, copies desired addresses into that array, calls a
pch_gbe_mac_mc_addr_list_update function which copies addresses out of
that array into MAC registers, then frees the array.

This patch simplifies this somewhat by inlining
pch_gbe_mac_mc_addr_list_update into pch_gbe_set_multi, and removing the
requirement for the MAC addresses to stored consecutively in a single
array.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 .../ethernet/oki-semi/pch_gbe/pch_gbe_main.c  | 73 +++++--------------
 1 file changed, 19 insertions(+), 54 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 3f2dd36d45ad..dc8c4050fad3 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -411,44 +411,6 @@ static void pch_gbe_mac_init_rx_addrs(struct pch_gbe_hw *hw, u16 mar_count)
 	pch_gbe_wait_clr_bit(&hw->reg->ADDR_MASK, PCH_GBE_BUSY);
 }
 
-
-/**
- * pch_gbe_mac_mc_addr_list_update - Update Multicast addresses
- * @hw:	            Pointer to the HW structure
- * @mc_addr_list:   Array of multicast addresses to program
- * @mc_addr_count:  Number of multicast addresses to program
- * @mar_used_count: The first MAC Address register free to program
- * @mar_total_num:  Total number of supported MAC Address Registers
- */
-static void pch_gbe_mac_mc_addr_list_update(struct pch_gbe_hw *hw,
-					    u8 *mc_addr_list, u32 mc_addr_count,
-					    u32 mar_used_count, u32 mar_total_num)
-{
-	u32 i, adrmask;
-
-	/* Load the first set of multicast addresses into the exact
-	 * filters (RAR).  If there are not enough to fill the RAR
-	 * array, clear the filters.
-	 */
-	for (i = mar_used_count; i < mar_total_num; i++) {
-		if (mc_addr_count) {
-			pch_gbe_mac_mar_set(hw, mc_addr_list, i);
-			mc_addr_count--;
-			mc_addr_list += ETH_ALEN;
-		} else {
-			/* Clear MAC address mask */
-			adrmask = ioread32(&hw->reg->ADDR_MASK);
-			iowrite32((adrmask | (0x0001 << i)),
-					&hw->reg->ADDR_MASK);
-			/* wait busy */
-			pch_gbe_wait_clr_bit(&hw->reg->ADDR_MASK, PCH_GBE_BUSY);
-			/* Clear MAC address */
-			iowrite32(0, &hw->reg->mac_adr[i].high);
-			iowrite32(0, &hw->reg->mac_adr[i].low);
-		}
-	}
-}
-
 /**
  * pch_gbe_mac_force_mac_fc - Force the MAC's flow control settings
  * @hw:	            Pointer to the HW structure
@@ -2143,10 +2105,8 @@ static void pch_gbe_set_multi(struct net_device *netdev)
 	struct pch_gbe_adapter *adapter = netdev_priv(netdev);
 	struct pch_gbe_hw *hw = &adapter->hw;
 	struct netdev_hw_addr *ha;
-	u8 *mta_list;
-	u32 rctl;
-	int i;
-	int mc_count;
+	u32 rctl, adrmask;
+	int mc_count, i;
 
 	netdev_dbg(netdev, "netdev->flags : 0x%08x\n", netdev->flags);
 
@@ -2173,20 +2133,25 @@ static void pch_gbe_set_multi(struct net_device *netdev)
 
 	if (mc_count >= PCH_GBE_MAR_ENTRIES)
 		return;
-	mta_list = kmalloc_array(ETH_ALEN, mc_count, GFP_ATOMIC);
-	if (!mta_list)
-		return;
 
-	/* The shared function expects a packed array of only addresses. */
-	i = 0;
-	netdev_for_each_mc_addr(ha, netdev) {
-		if (i == mc_count)
-			break;
-		memcpy(mta_list + (i++ * ETH_ALEN), &ha->addr, ETH_ALEN);
+	/* Load the first set of multicast addresses into MAC address registers
+	 * for use by hardware filtering.
+	 */
+	i = 1;
+	netdev_for_each_mc_addr(ha, netdev)
+		pch_gbe_mac_mar_set(hw, ha->addr, i++);
+
+	/* If there are spare MAC registers, mask & clear them */
+	for (; i < PCH_GBE_MAR_ENTRIES; i++) {
+		/* Clear MAC address mask */
+		adrmask = ioread32(&hw->reg->ADDR_MASK);
+		iowrite32(adrmask | BIT(i), &hw->reg->ADDR_MASK);
+		/* wait busy */
+		pch_gbe_wait_clr_bit(&hw->reg->ADDR_MASK, PCH_GBE_BUSY);
+		/* Clear MAC address */
+		iowrite32(0, &hw->reg->mac_adr[i].high);
+		iowrite32(0, &hw->reg->mac_adr[i].low);
 	}
-	pch_gbe_mac_mc_addr_list_update(hw, mta_list, i, 1,
-					PCH_GBE_MAR_ENTRIES);
-	kfree(mta_list);
 
 	netdev_dbg(netdev,
 		 "RX_MODE reg(check bit31,30 ADD,MLT) : 0x%08x  netdev->mc_count : 0x%08x\n",
-- 
2.17.1

^ permalink raw reply related

* [PATCH 14/14] net: pch_gbe: Clean up pch_gbe_set_multi
From: Paul Burton @ 2018-06-23  3:17 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>

Refactor pch_gbe_set_multi in order to avoid unnecessary indentation &
make it clearer what the code is doing.

The one behavioral change from this patch is that we'll no longer
configure the MAC address registers for multicast addresses when the
IFF_PROMISC or IFF_ALLMULTI flags are set. In these cases, just as when
we want to monitor more multicast addresses than we have MAC address
registers, we disable multicast filtering so the MAC address registers
are unused.

Signed-off-by: Paul Burton <paul.burton@mips.com>

---

 .../ethernet/oki-semi/pch_gbe/pch_gbe_main.c  | 33 +++++++++----------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index dc8c4050fad3..43c0c10dfeb7 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -2110,28 +2110,27 @@ static void pch_gbe_set_multi(struct net_device *netdev)
 
 	netdev_dbg(netdev, "netdev->flags : 0x%08x\n", netdev->flags);
 
-	/* Check for Promiscuous and All Multicast modes */
+	/* By default enable address & multicast filtering */
 	rctl = ioread32(&hw->reg->RX_MODE);
+	rctl |= PCH_GBE_ADD_FIL_EN | PCH_GBE_MLT_FIL_EN;
+
+	/* Promiscuous mode disables all hardware address filtering */
+	if (netdev->flags & IFF_PROMISC)
+		rctl &= ~(PCH_GBE_ADD_FIL_EN | PCH_GBE_MLT_FIL_EN);
+
+	/* If we want to monitor more multicast addresses than the hardware can
+	 * support then disable hardware multicast filtering.
+	 */
 	mc_count = netdev_mc_count(netdev);
-	if ((netdev->flags & IFF_PROMISC)) {
-		rctl &= ~PCH_GBE_ADD_FIL_EN;
+	if ((netdev->flags & IFF_ALLMULTI) || mc_count >= PCH_GBE_MAR_ENTRIES)
 		rctl &= ~PCH_GBE_MLT_FIL_EN;
-	} else if ((netdev->flags & IFF_ALLMULTI)) {
-		/* all the multicasting receive permissions */
-		rctl |= PCH_GBE_ADD_FIL_EN;
-		rctl &= ~PCH_GBE_MLT_FIL_EN;
-	} else {
-		if (mc_count >= PCH_GBE_MAR_ENTRIES) {
-			/* all the multicasting receive permissions */
-			rctl |= PCH_GBE_ADD_FIL_EN;
-			rctl &= ~PCH_GBE_MLT_FIL_EN;
-		} else {
-			rctl |= (PCH_GBE_ADD_FIL_EN | PCH_GBE_MLT_FIL_EN);
-		}
-	}
+
 	iowrite32(rctl, &hw->reg->RX_MODE);
 
-	if (mc_count >= PCH_GBE_MAR_ENTRIES)
+	/* If we're not using multicast filtering then there's no point
+	 * configuring the unused MAC address registers.
+	 */
+	if (!(rctl & PCH_GBE_MLT_FIL_EN))
 		return;
 
 	/* Load the first set of multicast addresses into MAC address registers
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH net-next 3/4] netdevsim: add ipsec offload testing
From: Jakub Kicinski @ 2018-06-23  4:07 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: davem, netdev, anders.roxell, linux-kselftest
In-Reply-To: <1529713898-9200-4-git-send-email-shannon.nelson@oracle.com>

On Fri, 22 Jun 2018 17:31:37 -0700, Shannon Nelson wrote:
> Implement the IPsec/XFRM offload API for testing.
> 
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>

Thanks for the patch!  Just a number of stylistic nit picks.

> diff --git a/drivers/net/netdevsim/ipsec.c b/drivers/net/netdevsim/ipsec.c
> new file mode 100644
> index 0000000..ad64266
> --- /dev/null
> +++ b/drivers/net/netdevsim/ipsec.c
> @@ -0,0 +1,345 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2018 Oracle and/or its affiliates. All rights reserved. */
> +
> +#include <net/xfrm.h>
> +#include <crypto/aead.h>
> +#include <linux/debugfs.h>
> +#include "netdevsim.h"

Other files in the driver sort headers alphabetically and put an empty
line between global and local headers.

> +#define NSIM_IPSEC_AUTH_BITS	128
> +
> +/**
> + * nsim_ipsec_dbg_read - read for ipsec data
> + * @filp: the opened file
> + * @buffer: where to write the data for the user to read
> + * @count: the size of the user's buffer
> + * @ppos: file position offset
> + **/
> +static ssize_t nsim_dbg_netdev_ops_read(struct file *filp,

Doesn't match the kdoc.  Please run 

./scripts/kernel-doc -none $file

if you want kdoc.  Although IMHO you may as well drop the kdoc, your
code is quite self explanatory and local.

> +					char __user *buffer,
> +					size_t count, loff_t *ppos)
> +{
> +	struct netdevsim *ns = filp->private_data;
> +	struct nsim_ipsec *ipsec = &ns->ipsec;
> +	size_t bufsize;
> +	char *buf, *p;
> +	int len;
> +	int i;
> +
> +	/* don't allow partial reads */
> +	if (*ppos != 0)
> +		return 0;
> +
> +	/* the buffer needed is
> +	 * (num SAs * 3 lines each * ~60 bytes per line) + one more line
> +	 */
> +	bufsize = (ipsec->count * 4 * 60) + 60;
> +	buf = kzalloc(bufsize, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	p = buf;
> +	p += snprintf(p, bufsize - (p - buf),
> +		      "SA count=%u tx=%u\n",
> +		      ipsec->count, ipsec->tx);
> +
> +	for (i = 0; i < NSIM_IPSEC_MAX_SA_COUNT; i++) {
> +		struct nsim_sa *sap = &ipsec->sa[i];
> +
> +		if (!sap->used)
> +			continue;
> +
> +		p += snprintf(p, bufsize - (p - buf),
> +			      "sa[%i] %cx ipaddr=0x%08x %08x %08x %08x\n",
> +			      i, (sap->rx ? 'r' : 't'), sap->ipaddr[0],
> +			      sap->ipaddr[1], sap->ipaddr[2], sap->ipaddr[3]);
> +		p += snprintf(p, bufsize - (p - buf),
> +			      "sa[%i]    spi=0x%08x proto=0x%x salt=0x%08x crypt=%d\n",
> +			      i, be32_to_cpu(sap->xs->id.spi),
> +			      sap->xs->id.proto, sap->salt, sap->crypt);
> +		p += snprintf(p, bufsize - (p - buf),
> +			      "sa[%i]    key=0x%08x %08x %08x %08x\n",
> +			      i, sap->key[0], sap->key[1],
> +			      sap->key[2], sap->key[3]);
> +	}
> +
> +	len = simple_read_from_buffer(buffer, count, ppos, buf, p - buf);

Why not seq_file for this?

> +	kfree(buf);
> +	return len;
> +}
> +
> +static const struct file_operations ipsec_dbg_fops = {
> +	.owner = THIS_MODULE,
> +	.open = simple_open,
> +	.read = nsim_dbg_netdev_ops_read,
> +};
> +
> +/**
> + * nsim_ipsec_find_empty_idx - find the first unused security parameter index
> + * @ipsec: pointer to ipsec struct
> + **/
> +static int nsim_ipsec_find_empty_idx(struct nsim_ipsec *ipsec)
> +{
> +	u32 i;
> +
> +	if (ipsec->count == NSIM_IPSEC_MAX_SA_COUNT)
> +		return -ENOSPC;
> +
> +	/* search sa table */
> +	for (i = 0; i < NSIM_IPSEC_MAX_SA_COUNT; i++) {
> +		if (!ipsec->sa[i].used)
> +			return i;
> +	}
> +
> +	return -ENOSPC;

FWIW I personally find bitmaps and find_first_zero_bit() etc. nice and
concise for a small ID allocator, but no objection to open coding.

> +}
> +
> +/**
> + * nsim_ipsec_parse_proto_keys - find the key and salt based on the protocol
> + * @xs: pointer to xfrm_state struct
> + * @mykey: pointer to key array to populate
> + * @mysalt: pointer to salt value to populate
> + *
> + * This copies the protocol keys and salt to our own data tables.  The
> + * 82599 family only supports the one algorithm.

82599 is a fine chip, it's not netdevsim tho? ;)

> + **/
> +static int nsim_ipsec_parse_proto_keys(struct xfrm_state *xs,
> +				       u32 *mykey, u32 *mysalt)
> +{
> +	struct net_device *dev = xs->xso.dev;
> +	unsigned char *key_data;
> +	char *alg_name = NULL;
> +	const char aes_gcm_name[] = "rfc4106(gcm(aes))";
> +	int key_len;

reverse xmas tree please

> +
> +	if (!xs->aead) {
> +		netdev_err(dev, "Unsupported IPsec algorithm\n");
> +		return -EINVAL;
> +	}
> +
> +	if (xs->aead->alg_icv_len != NSIM_IPSEC_AUTH_BITS) {
> +		netdev_err(dev, "IPsec offload requires %d bit authentication\n",
> +			   NSIM_IPSEC_AUTH_BITS);
> +		return -EINVAL;
> +	}
> +
> +	key_data = &xs->aead->alg_key[0];
> +	key_len = xs->aead->alg_key_len;
> +	alg_name = xs->aead->alg_name;
> +
> +	if (strcmp(alg_name, aes_gcm_name)) {
> +		netdev_err(dev, "Unsupported IPsec algorithm - please use %s\n",
> +			   aes_gcm_name);
> +		return -EINVAL;
> +	}
> +
> +	/* The key bytes come down in a bigendian array of bytes, so
> +	 * we don't need to do any byteswapping.

Why the mention of bigendian?  82599 needs big endian? -.^

> +	 * 160 accounts for 16 byte key and 4 byte salt
> +	 */
> +	if (key_len > 128) {

s/128/NSIM_IPSEC_AUTH_BITS/ ?

> +		*mysalt = ((u32 *)key_data)[4];

Is alignment guaranteed?  There are the unaligned helpers if you need
them..

> +	} else if (key_len == 128) {
> +		*mysalt = 0;
> +	} else {
> +		netdev_err(dev, "IPsec hw offload only supports 128 bit keys with optional 32 bit salt\n");
> +		return -EINVAL;
> +	}
> +	memcpy(mykey, key_data, 16);
> +
> +	return 0;
> +}
> +
> +/**
> + * nsim_ipsec_add_sa - program device with a security association
> + * @xs: pointer to transformer state struct
> + **/
> +static int nsim_ipsec_add_sa(struct xfrm_state *xs)
> +{
> +	struct net_device *dev = xs->xso.dev;
> +	struct netdevsim *ns = netdev_priv(dev);
> +	struct nsim_ipsec *ipsec = &ns->ipsec;

xmas tree again (initialize out of line if you have to)

> +	struct nsim_sa sa;
> +	u16 sa_idx;
> +	int ret;
> +
> +	if (xs->id.proto != IPPROTO_ESP && xs->id.proto != IPPROTO_AH) {
> +		netdev_err(dev, "Unsupported protocol 0x%04x for ipsec offload\n",
> +			   xs->id.proto);
> +		return -EINVAL;
> +	}
> +
> +	if (xs->calg) {
> +		netdev_err(dev, "Compression offload not supported\n");
> +		return -EINVAL;
> +	}
> +
> +	/* find the first unused index */
> +	ret = nsim_ipsec_find_empty_idx(ipsec);
> +	if (ret < 0) {
> +		netdev_err(dev, "No space for SA in Rx table!\n");
> +		return ret;
> +	}
> +	sa_idx = (u16)ret;
> +
> +	memset(&sa, 0, sizeof(sa));
> +	sa.used = true;
> +	sa.xs = xs;
> +
> +	if (sa.xs->id.proto & IPPROTO_ESP)
> +		sa.crypt = xs->ealg || xs->aead;
> +
> +	/* get the key and salt */
> +	ret = nsim_ipsec_parse_proto_keys(xs, sa.key, &sa.salt);
> +	if (ret) {
> +		netdev_err(dev, "Failed to get key data for SA table\n");
> +		return ret;
> +	}
> +
> +	if (xs->xso.flags & XFRM_OFFLOAD_INBOUND) {
> +		sa.rx = true;
> +
> +		if (xs->props.family == AF_INET6)
> +			memcpy(sa.ipaddr, &xs->id.daddr.a6, 16);
> +		else
> +			memcpy(&sa.ipaddr[3], &xs->id.daddr.a4, 4);
> +	}
> +
> +	/* the preparations worked, so save the info */
> +	memcpy(&ipsec->sa[sa_idx], &sa, sizeof(sa));
> +
> +	/* the XFRM stack doesn't like offload_handle == 0,
> +	 * so add a bitflag in case our array index is 0
> +	 */
> +	xs->xso.offload_handle = sa_idx | NSIM_IPSEC_VALID;
> +	ipsec->count++;
> +
> +	return 0;
> +}
> +
> +/**
> + * nsim_ipsec_del_sa - clear out this specific SA
> + * @xs: pointer to transformer state struct
> + **/
> +static void nsim_ipsec_del_sa(struct xfrm_state *xs)
> +{
> +	struct net_device *dev = xs->xso.dev;
> +	struct netdevsim *ns = netdev_priv(dev);
> +	struct nsim_ipsec *ipsec = &ns->ipsec;
> +	u16 sa_idx;
> +
> +	sa_idx = xs->xso.offload_handle & ~NSIM_IPSEC_VALID;
> +	if (!ipsec->sa[sa_idx].used) {
> +		netdev_err(dev, "Invalid SA for delete sa_idx=%d\n", sa_idx);
> +		return;
> +	}
> +
> +	memset(&ipsec->sa[sa_idx], 0, sizeof(struct nsim_sa));
> +	ipsec->count--;
> +}
> +
> +/**
> + * nsim_ipsec_offload_ok - can this packet use the xfrm hw offload
> + * @skb: current data packet
> + * @xs: pointer to transformer state struct
> + **/
> +static bool nsim_ipsec_offload_ok(struct sk_buff *skb, struct xfrm_state *xs)
> +{
> +	struct net_device *dev = xs->xso.dev;
> +	struct netdevsim *ns = netdev_priv(dev);
> +	struct nsim_ipsec *ipsec = &ns->ipsec;
> +
> +	ipsec->ok++;
> +
> +	return true;
> +}
> +
> +static const struct xfrmdev_ops nsim_xfrmdev_ops = {
> +	.xdo_dev_state_add = nsim_ipsec_add_sa,
> +	.xdo_dev_state_delete = nsim_ipsec_del_sa,
> +	.xdo_dev_offload_ok = nsim_ipsec_offload_ok,

Please align the initializers by adding tabs before '='.

> +};
> +
> +/**
> + * nsim_ipsec_tx - check Tx packet for ipsec offload
> + * @ns: pointer to ns structure
> + * @skb: current data packet
> + **/
> +int nsim_ipsec_tx(struct netdevsim *ns, struct sk_buff *skb)
> +{
> +	struct nsim_ipsec *ipsec = &ns->ipsec;
> +	struct xfrm_state *xs;
> +	struct nsim_sa *tsa;
> +	u32 sa_idx;
> +
> +	/* do we even need to check this packet? */
> +	if (!skb->sp)
> +		return 1;
> +
> +	if (unlikely(!skb->sp->len)) {
> +		netdev_err(ns->netdev, "%s: no xfrm state len = %d\n",
> +			   __func__, skb->sp->len);

Hmm..  __func__ started appearing in errors?  Perhaps either always or
never add it?

Also, I know this is not a real device, but please always use rate
limited print functions on the data path.

> +		return 0;
> +	}
> +
> +	xs = xfrm_input_state(skb);
> +	if (unlikely(!xs)) {
> +		netdev_err(ns->netdev, "%s: no xfrm_input_state() xs = %p\n",
> +			   __func__, xs);
> +		return 0;
> +	}
> +
> +	sa_idx = xs->xso.offload_handle & ~NSIM_IPSEC_VALID;
> +	if (unlikely(sa_idx > NSIM_IPSEC_MAX_SA_COUNT)) {
> +		netdev_err(ns->netdev, "%s: bad sa_idx=%d max=%d\n",
> +			   __func__, sa_idx, NSIM_IPSEC_MAX_SA_COUNT);
> +		return 0;
> +	}
> +
> +	tsa = &ipsec->sa[sa_idx];
> +	if (unlikely(!tsa->used)) {
> +		netdev_err(ns->netdev, "%s: unused sa_idx=%d\n",
> +			   __func__, sa_idx);
> +		return 0;
> +	}
> +
> +	if (xs->id.proto != IPPROTO_ESP && xs->id.proto != IPPROTO_AH) {
> +		netdev_err(ns->netdev, "%s: unexpected proto=%d\n",
> +			   __func__, xs->id.proto);
> +		return 0;
> +	}
> +
> +	ipsec->tx++;
> +
> +	return 1;
> +}

Looks like the function should return bool?

> +
> +/**
> + * nsim_ipsec_init - initialize security registers for IPSec operation
> + * @ns: board private structure

"board"?  Yes, the kdoc may be best removed ;)

> + **/
> +void nsim_ipsec_init(struct netdevsim *ns)
> +{
> +	ns->netdev->xfrmdev_ops = &nsim_xfrmdev_ops;
> +
> +#define NSIM_ESP_FEATURES	(NETIF_F_HW_ESP | \
> +				 NETIF_F_HW_ESP_TX_CSUM | \
> +				 NETIF_F_GSO_ESP)
> +
> +	ns->netdev->features |= NSIM_ESP_FEATURES;
> +	ns->netdev->hw_enc_features |= NSIM_ESP_FEATURES;
> +
> +	ns->ipsec.pfile = debugfs_create_file("ipsec", 0400, ns->ddir, ns,
> +					      &ipsec_dbg_fops);
> +}
> +
> +void nsim_ipsec_teardown(struct netdevsim *ns)
> +{
> +	struct nsim_ipsec *ipsec = &ns->ipsec;
> +
> +	if (ipsec->count)
> +		netdev_err(ns->netdev, "%s: tearing down IPsec offload with %d SAs left\n",
> +			   __func__, ipsec->count);
> +	debugfs_remove_recursive(ipsec->pfile);
> +}
> diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
> index ec68f38..6ce8604d 100644
> --- a/drivers/net/netdevsim/netdev.c
> +++ b/drivers/net/netdevsim/netdev.c
> @@ -171,6 +171,8 @@ static int nsim_init(struct net_device *dev)
>  	if (err)
>  		goto err_unreg_dev;
>  
> +	nsim_ipsec_init(ns);
> +
>  	return 0;
>  
>  err_unreg_dev:
> @@ -186,6 +188,7 @@ static void nsim_uninit(struct net_device *dev)
>  {
>  	struct netdevsim *ns = netdev_priv(dev);
>  
> +	nsim_ipsec_teardown(ns);
>  	nsim_devlink_teardown(ns);
>  	debugfs_remove_recursive(ns->ddir);
>  	nsim_bpf_uninit(ns);
> @@ -203,11 +206,15 @@ static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  {
>  	struct netdevsim *ns = netdev_priv(dev);
>  
> +	if (!nsim_ipsec_tx(ns, skb))
> +		goto out;
> +
>  	u64_stats_update_begin(&ns->syncp);
>  	ns->tx_packets++;
>  	ns->tx_bytes += skb->len;
>  	u64_stats_update_end(&ns->syncp);
>  
> +out:
>  	dev_kfree_skb(skb);
>  
>  	return NETDEV_TX_OK;
> diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
> index 3a8581a..1708dee 100644
> --- a/drivers/net/netdevsim/netdevsim.h
> +++ b/drivers/net/netdevsim/netdevsim.h
> @@ -29,6 +29,29 @@ struct bpf_prog;
>  struct dentry;
>  struct nsim_vf_config;
>  
> +#if IS_ENABLED(CONFIG_XFRM_OFFLOAD)
> +#define NSIM_IPSEC_MAX_SA_COUNT		33

33 caught my eye - out of curiosity is it 2^5 + 1 to catch some type of
bug or failure mode?

> +#define NSIM_IPSEC_VALID		BIT(31)
> +
> +struct nsim_sa {
> +	struct xfrm_state *xs;
> +	__be32 ipaddr[4];
> +	u32 key[4];
> +	u32 salt;
> +	bool used;
> +	bool crypt;
> +	bool rx;
> +};
> +
> +struct nsim_ipsec {
> +	struct nsim_sa sa[NSIM_IPSEC_MAX_SA_COUNT];
> +	struct dentry *pfile;
> +	u32 count;
> +	u32 tx;
> +	u32 ok;
> +};
> +#endif

No need to wrap struct definitions in #if/#endif.

>  struct netdevsim {
>  	struct net_device *netdev;
>  
> @@ -67,6 +90,9 @@ struct netdevsim {
>  #if IS_ENABLED(CONFIG_NET_DEVLINK)
>  	struct devlink *devlink;
>  #endif
> +#if IS_ENABLED(CONFIG_XFRM_OFFLOAD)
> +	struct nsim_ipsec ipsec;
> +#endif
>  };
>  
>  extern struct dentry *nsim_ddir;
> @@ -147,6 +173,17 @@ static inline void nsim_devlink_exit(void)
>  }
>  #endif
>  
> +#if IS_ENABLED(CONFIG_XFRM_OFFLOAD)
> +void nsim_ipsec_init(struct netdevsim *ns);
> +void nsim_ipsec_teardown(struct netdevsim *ns);
> +int nsim_ipsec_tx(struct netdevsim *ns, struct sk_buff *skb);
> +#else
> +static inline void nsim_ipsec_init(struct netdevsim *ns) {};
> +static inline void nsim_ipsec_teardown(struct netdevsim *ns) {};
> +static inline int nsim_ipsec_tx(struct netdevsim *ns, struct sk_buff *skb)
> +								{ return 1; };

Please use the same formatting for static inlines as the rest of the
file.  The ';' are also unnecessary.

Other than those formatting nit picks looks good to me :)

^ permalink raw reply

* Re: [bpf PATCH v3 1/4] bpf: sockmap, fix crash when ipv6 sock is added
From: Martin KaFai Lau @ 2018-06-23  7:44 UTC (permalink / raw)
  To: John Fastabend; +Cc: ast, daniel, netdev
In-Reply-To: <20180622152134.24502.31858.stgit@john-Precision-Tower-5810>

On Fri, Jun 22, 2018 at 08:21:34AM -0700, John Fastabend wrote:
> This fixes a crash where we assign tcp_prot to IPv6 sockets instead
> of tcpv6_prot.
> 
> Previously we overwrote the sk->prot field with tcp_prot even in the
> AF_INET6 case. This patch ensures the correct tcp_prot and tcpv6_prot
> are used.
> 
> Tested with 'netserver -6' and 'netperf -H [IPv6]' as well as
> 'netperf -H [IPv4]'. The ESTABLISHED check resolves the previously
> crashing case here.
> 
> Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
> Reported-by: syzbot+5c063698bdbfac19f363@syzkaller.appspotmail.com
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>

> ---
>  kernel/bpf/sockmap.c |   58 +++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 48 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
> index 52a91d8..d7fd17a 100644
> --- a/kernel/bpf/sockmap.c
> +++ b/kernel/bpf/sockmap.c
> @@ -140,6 +140,7 @@ static int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
>  static int bpf_tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size);
>  static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
>  			    int offset, size_t size, int flags);
> +static void bpf_tcp_close(struct sock *sk, long timeout);
>  
>  static inline struct smap_psock *smap_psock_sk(const struct sock *sk)
>  {
> @@ -161,7 +162,42 @@ static bool bpf_tcp_stream_read(const struct sock *sk)
>  	return !empty;
>  }
>  
> -static struct proto tcp_bpf_proto;
> +enum {
> +	SOCKMAP_IPV4,
> +	SOCKMAP_IPV6,
> +	SOCKMAP_NUM_PROTS,
> +};
> +
> +enum {
> +	SOCKMAP_BASE,
> +	SOCKMAP_TX,
> +	SOCKMAP_NUM_CONFIGS,
> +};
> +
> +static struct proto *saved_tcpv6_prot __read_mostly;
> +static DEFINE_SPINLOCK(tcpv6_prot_lock);
> +static struct proto bpf_tcp_prots[SOCKMAP_NUM_PROTS][SOCKMAP_NUM_CONFIGS];
> +static void build_protos(struct proto prot[SOCKMAP_NUM_CONFIGS],
> +			 struct proto *base)
> +{
> +	prot[SOCKMAP_BASE]			= *base;
> +	prot[SOCKMAP_BASE].close		= bpf_tcp_close;
> +	prot[SOCKMAP_BASE].recvmsg		= bpf_tcp_recvmsg;
> +	prot[SOCKMAP_BASE].stream_memory_read	= bpf_tcp_stream_read;
> +
> +	prot[SOCKMAP_TX]			= prot[SOCKMAP_BASE];
> +	prot[SOCKMAP_TX].sendmsg		= bpf_tcp_sendmsg;
> +	prot[SOCKMAP_TX].sendpage		= bpf_tcp_sendpage;
> +}
> +
> +static void update_sk_prot(struct sock *sk, struct smap_psock *psock)
> +{
> +	int family = sk->sk_family == AF_INET6 ? SOCKMAP_IPV6 : SOCKMAP_IPV4;
> +	int conf = psock->bpf_tx_msg ? SOCKMAP_TX : SOCKMAP_BASE;
> +
> +	sk->sk_prot = &bpf_tcp_prots[family][conf];
> +}
> +
>  static int bpf_tcp_init(struct sock *sk)
>  {
>  	struct smap_psock *psock;
> @@ -181,14 +217,17 @@ static int bpf_tcp_init(struct sock *sk)
>  	psock->save_close = sk->sk_prot->close;
>  	psock->sk_proto = sk->sk_prot;
>  
> -	if (psock->bpf_tx_msg) {
> -		tcp_bpf_proto.sendmsg = bpf_tcp_sendmsg;
> -		tcp_bpf_proto.sendpage = bpf_tcp_sendpage;
> -		tcp_bpf_proto.recvmsg = bpf_tcp_recvmsg;
> -		tcp_bpf_proto.stream_memory_read = bpf_tcp_stream_read;
> +	/* Build IPv6 sockmap whenever the address of tcpv6_prot changes */
> +	if (sk->sk_family == AF_INET6 &&
> +	    unlikely(sk->sk_prot != smp_load_acquire(&saved_tcpv6_prot))) {
> +		spin_lock_bh(&tcpv6_prot_lock);
> +		if (likely(sk->sk_prot != saved_tcpv6_prot)) {
> +			build_protos(bpf_tcp_prots[SOCKMAP_IPV6], sk->sk_prot);
> +			smp_store_release(&saved_tcpv6_prot, sk->sk_prot);
> +		}
> +		spin_unlock_bh(&tcpv6_prot_lock);
>  	}
> -
> -	sk->sk_prot = &tcp_bpf_proto;
> +	update_sk_prot(sk, psock);
>  	rcu_read_unlock();
>  	return 0;
>  }
> @@ -1111,8 +1150,7 @@ static void bpf_tcp_msg_add(struct smap_psock *psock,
>  
>  static int bpf_tcp_ulp_register(void)
>  {
> -	tcp_bpf_proto = tcp_prot;
> -	tcp_bpf_proto.close = bpf_tcp_close;
> +	build_protos(bpf_tcp_prots[SOCKMAP_IPV4], &tcp_prot);
>  	/* Once BPF TX ULP is registered it is never unregistered. It
>  	 * will be in the ULP list for the lifetime of the system. Doing
>  	 * duplicate registers is not a problem.
> 

^ 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