netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support
@ 2018-02-17 20:10 Paul Burton
  2018-02-17 20:10 ` [PATCH v5 01/14] net: pch_gbe: Mark Minnow PHY reset GPIO active low Paul Burton
                   ` (14 more replies)
  0 siblings, 15 replies; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton

The Intel EG20T Platform Controller Hub is used on the MIPS Boston
development board to provide various peripherals including ethernet.
This series fixes some issues with the pch_gbe driver discovered whilst
in use on the Boston board, and implements support for device tree which
we use to provide the PHY reset GPIO.

Applies atop v4.16-rc1.

Hassan Naveed (1):
  net: pch_gbe: Fix TX RX descriptor accesses for big endian systems

Paul Burton (13):
  net: pch_gbe: Mark Minnow PHY reset GPIO active low
  net: pch_gbe: Pull PHY GPIO handling out of Minnow code
  dt-bindings: net: Document Intel pch_gbe binding
  net: pch_gbe: Add device tree support
  net: pch_gbe: Always reset PHY along with MAC
  net: pch_gbe: Allow longer for resets
  net: pch_gbe: Fix handling of TX padding
  net: pch_gbe: Fold pch_gbe_setup_[rt]ctl into pch_gbe_configure_[rt]x
  net: pch_gbe: Use pch_gbe_disable_dma_rx() in pch_gbe_configure_rx()
  net: pch_gbe: Disable TX DMA whilst configuring descriptors
  net: pch_gbe: Ensure DMA is ordered with descriptor writes
  ptp: pch: Allow build on MIPS platforms
  net: pch_gbe: Allow build on MIPS platforms

 Documentation/devicetree/bindings/net/pch_gbe.txt  |  25 ++
 drivers/net/ethernet/oki-semi/pch_gbe/Kconfig      |   2 +-
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    |  27 +-
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 283 ++++++++++++---------
 drivers/ptp/Kconfig                                |   2 +-
 5 files changed, 204 insertions(+), 135 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/pch_gbe.txt

-- 
2.16.1

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v5 01/14] net: pch_gbe: Mark Minnow PHY reset GPIO active low
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
@ 2018-02-17 20:10 ` Paul Burton
  2018-02-17 22:14   ` Andrew Lunn
  2018-02-17 20:10 ` [PATCH v5 02/14] net: pch_gbe: Pull PHY GPIO handling out of Minnow code Paul Burton
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton

The Minnow PHY reset GPIO is set to 0 to enter reset & 1 to leave reset
- that is, it is an active low GPIO. In order to allow for the code to
be made more generic by further patches, indicate to the GPIO subsystem
that the GPIO is active low & invert the values it is set to such that
they reflect logically whether the device is being reset or not.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 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 7cd494611a74..d5c6f2e2d3a2 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
@@ -2688,7 +2688,8 @@ static int pch_gbe_probe(struct pci_dev *pdev,
  */
 static int pch_gbe_minnow_platform_init(struct pci_dev *pdev)
 {
-	unsigned long flags = GPIOF_DIR_OUT | GPIOF_INIT_HIGH | GPIOF_EXPORT;
+	unsigned long flags = GPIOF_DIR_OUT | GPIOF_INIT_LOW |
+		GPIOF_EXPORT | GPIOF_ACTIVE_LOW;
 	unsigned gpio = MINNOW_PHY_RESET_GPIO;
 	int ret;
 
@@ -2700,10 +2701,10 @@ static int pch_gbe_minnow_platform_init(struct pci_dev *pdev)
 		return ret;
 	}
 
-	gpio_set_value(gpio, 0);
-	usleep_range(1250, 1500);
 	gpio_set_value(gpio, 1);
 	usleep_range(1250, 1500);
+	gpio_set_value(gpio, 0);
+	usleep_range(1250, 1500);
 
 	return ret;
 }
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 02/14] net: pch_gbe: Pull PHY GPIO handling out of Minnow code
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
  2018-02-17 20:10 ` [PATCH v5 01/14] net: pch_gbe: Mark Minnow PHY reset GPIO active low Paul Burton
@ 2018-02-17 20:10 ` Paul Burton
  2018-02-17 22:29   ` Andrew Lunn
  2018-02-17 20:10 ` [PATCH v5 03/14] dt-bindings: net: Document Intel pch_gbe binding Paul Burton
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton

The MIPS Boston development board uses the Intel EG20T Platform
Controller Hub, including its gigabit ethernet controller, and requires
that its RTL8211E PHY be reset much like the Minnow platform. Pull the
PHY reset GPIO handling out of Minnow-specific code such that it can be
shared by later patches.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---

Changes in v5:
- Name struct pch_gbe_privdata's platform_init pdata arg, per checkpatch.

Changes in v4: None
Changes in v3:
- Use adapter->pdata as arg to platform_init, to fix bisectability.

Changes in v2: None

 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    |  5 +++-
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 33 +++++++++++++++-------
 2 files changed, 27 insertions(+), 11 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..8ba9ced2d1fd 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -580,15 +580,18 @@ struct pch_gbe_hw_stats {
 
 /**
  * struct pch_gbe_privdata - PCI Device ID driver data
+ * @phy_reset_gpio:		PHY reset GPIO descriptor.
  * @phy_tx_clk_delay:		Bool, configure the PHY TX delay in software
  * @phy_disable_hibernate:	Bool, disable PHY hibernation
  * @platform_init:		Platform initialization callback, called from
  *				probe, prior to PHY initialization.
  */
 struct pch_gbe_privdata {
+	struct gpio_desc *phy_reset_gpio;
 	bool phy_tx_clk_delay;
 	bool phy_disable_hibernate;
-	int (*platform_init)(struct pci_dev *pdev);
+	int (*platform_init)(struct pci_dev *pdev,
+			     struct pch_gbe_privdata *pdata);
 };
 
 /**
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 d5c6f2e2d3a2..712ac2f7bb2c 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
@@ -360,6 +360,16 @@ static void pch_gbe_mac_mar_set(struct pch_gbe_hw *hw, u8 * addr, u32 index)
 	pch_gbe_wait_clr_bit(&hw->reg->ADDR_MASK, PCH_GBE_BUSY);
 }
 
+static void pch_gbe_phy_set_reset(struct pch_gbe_hw *hw, int value)
+{
+	struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
+
+	if (!adapter->pdata || !adapter->pdata->phy_reset_gpio)
+		return;
+
+	gpiod_set_value(adapter->pdata->phy_reset_gpio, value);
+}
+
 /**
  * pch_gbe_mac_reset_hw - Reset hardware
  * @hw:	Pointer to the HW structure
@@ -2592,7 +2602,14 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 	adapter->hw.reg = pcim_iomap_table(pdev)[PCH_GBE_PCI_BAR];
 	adapter->pdata = (struct pch_gbe_privdata *)pci_id->driver_data;
 	if (adapter->pdata && adapter->pdata->platform_init)
-		adapter->pdata->platform_init(pdev);
+		adapter->pdata->platform_init(pdev, adapter->pdata);
+
+	if (adapter->pdata && adapter->pdata->phy_reset_gpio) {
+		pch_gbe_phy_set_reset(&adapter->hw, 1);
+		usleep_range(1250, 1500);
+		pch_gbe_phy_set_reset(&adapter->hw, 0);
+		usleep_range(1250, 1500);
+	}
 
 	adapter->ptp_pdev =
 		pci_get_domain_bus_and_slot(pci_domain_nr(adapter->pdev->bus),
@@ -2686,7 +2703,8 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 /* The AR803X PHY on the MinnowBoard requires a physical pin to be toggled to
  * ensure it is awake for probe and init. Request the line and reset the PHY.
  */
-static int pch_gbe_minnow_platform_init(struct pci_dev *pdev)
+static int pch_gbe_minnow_platform_init(struct pci_dev *pdev,
+					struct pch_gbe_privdata *pdata)
 {
 	unsigned long flags = GPIOF_DIR_OUT | GPIOF_INIT_LOW |
 		GPIOF_EXPORT | GPIOF_ACTIVE_LOW;
@@ -2695,16 +2713,11 @@ static int pch_gbe_minnow_platform_init(struct pci_dev *pdev)
 
 	ret = devm_gpio_request_one(&pdev->dev, gpio, flags,
 				    "minnow_phy_reset");
-	if (ret) {
+	if (!ret)
+		pdata->phy_reset_gpio = gpio_to_desc(gpio);
+	else
 		dev_err(&pdev->dev,
 			"ERR: Can't request PHY reset GPIO line '%d'\n", gpio);
-		return ret;
-	}
-
-	gpio_set_value(gpio, 1);
-	usleep_range(1250, 1500);
-	gpio_set_value(gpio, 0);
-	usleep_range(1250, 1500);
 
 	return ret;
 }
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 03/14] dt-bindings: net: Document Intel pch_gbe binding
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
  2018-02-17 20:10 ` [PATCH v5 01/14] net: pch_gbe: Mark Minnow PHY reset GPIO active low Paul Burton
  2018-02-17 20:10 ` [PATCH v5 02/14] net: pch_gbe: Pull PHY GPIO handling out of Minnow code Paul Burton
@ 2018-02-17 20:10 ` Paul Burton
       [not found]   ` <20180217201037.3006-4-paul.burton-8NJIiSa5LzA@public.gmane.org>
  2018-02-17 20:10 ` [PATCH v5 04/14] net: pch_gbe: Add device tree support Paul Burton
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton, Mark Rutland, Rob Herring, devicetree

Introduce documentation for a device tree binding for the Intel Platform
Controller Hub (PCH) GigaBit Ethernet (GBE) device. Although this is a
PCIe device & thus largely auto-detectable, this binding will be used to
provide the driver with the PHY reset GPIO.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org

---

Changes in v5:
- Use standard gpio & ethernet node names in example.
- Remove bus number from example unit addresses.

Changes in v4: None
Changes in v3:
- New patch.

Changes in v2: None

 Documentation/devicetree/bindings/net/pch_gbe.txt | 25 +++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/pch_gbe.txt

diff --git a/Documentation/devicetree/bindings/net/pch_gbe.txt b/Documentation/devicetree/bindings/net/pch_gbe.txt
new file mode 100644
index 000000000000..cff2687e6e75
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/pch_gbe.txt
@@ -0,0 +1,25 @@
+Intel Platform Controller Hub (PCH) GigaBit Ethernet (GBE)
+
+Required properties:
+- compatible:		Should be the PCI vendor & device ID, eg. "pci8086,8802".
+- reg:			Should be a PCI device number as specified by the PCI bus
+			binding to IEEE Std 1275-1994.
+- phy-reset-gpios:	Should be a GPIO list containing a single GPIO that
+			resets the attached PHY when active.
+
+Example:
+
+	ethernet@0,1 {
+		compatible = "pci8086,8802";
+		reg = <0x00020100 0 0 0 0>;
+		phy-reset-gpios = <&eg20t_gpio 6
+				   GPIO_ACTIVE_LOW>;
+	};
+
+	eg20t_gpio: gpio@0,2 {
+		compatible = "pci8086,8803";
+		reg = <0x00020200 0 0 0 0>;
+
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 04/14] net: pch_gbe: Add device tree support
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
                   ` (2 preceding siblings ...)
  2018-02-17 20:10 ` [PATCH v5 03/14] dt-bindings: net: Document Intel pch_gbe binding Paul Burton
@ 2018-02-17 20:10 ` Paul Burton
  2018-02-17 20:10 ` [PATCH v5 05/14] net: pch_gbe: Always reset PHY along with MAC Paul Burton
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton

Introduce support for retrieving the PHY reset GPIO from device tree,
which will be used on the MIPS Boston development board. This requires
support for probe deferral in order to work correctly, since the order
of device probe is not guaranteed & typically the EG20T GPIO controller
device will be probed after the ethernet MAC.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---

Changes in v5: None
Changes in v4:
- Use ERR_CAST(), thanks kbuild test robot/Fengguang!

Changes in v3: None
Changes in v2:
- Tidy up handling of parsing private data, drop err_out.

 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 31 +++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

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 712ac2f7bb2c..11e8ced4a0f4 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
@@ -23,6 +23,8 @@
 #include <linux/net_tstamp.h>
 #include <linux/ptp_classify.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/of_gpio.h>
 
 #define DRV_VERSION     "1.01"
 const char pch_driver_version[] = DRV_VERSION;
@@ -2556,13 +2558,40 @@ static void pch_gbe_remove(struct pci_dev *pdev)
 	free_netdev(netdev);
 }
 
+static struct pch_gbe_privdata *
+pch_gbe_get_priv(struct pci_dev *pdev, const struct pci_device_id *pci_id)
+{
+	struct pch_gbe_privdata *pdata;
+	struct gpio_desc *gpio;
+
+	if (!IS_ENABLED(CONFIG_OF))
+		return (struct pch_gbe_privdata *)pci_id->driver_data;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	gpio = devm_gpiod_get(&pdev->dev, "phy-reset", GPIOD_ASIS);
+	if (!IS_ERR(gpio))
+		pdata->phy_reset_gpio = gpio;
+	else if (PTR_ERR(gpio) != -ENOENT)
+		return ERR_CAST(gpio);
+
+	return pdata;
+}
+
 static int pch_gbe_probe(struct pci_dev *pdev,
 			  const struct pci_device_id *pci_id)
 {
 	struct net_device *netdev;
 	struct pch_gbe_adapter *adapter;
+	struct pch_gbe_privdata *pdata;
 	int ret;
 
+	pdata = pch_gbe_get_priv(pdev, pci_id);
+	if (IS_ERR(pdata))
+		return PTR_ERR(pdata);
+
 	ret = pcim_enable_device(pdev);
 	if (ret)
 		return ret;
@@ -2600,7 +2629,7 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 	adapter->pdev = pdev;
 	adapter->hw.back = adapter;
 	adapter->hw.reg = pcim_iomap_table(pdev)[PCH_GBE_PCI_BAR];
-	adapter->pdata = (struct pch_gbe_privdata *)pci_id->driver_data;
+	adapter->pdata = pdata;
 	if (adapter->pdata && adapter->pdata->platform_init)
 		adapter->pdata->platform_init(pdev, adapter->pdata);
 
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 05/14] net: pch_gbe: Always reset PHY along with MAC
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
                   ` (3 preceding siblings ...)
  2018-02-17 20:10 ` [PATCH v5 04/14] net: pch_gbe: Add device tree support Paul Burton
@ 2018-02-17 20:10 ` Paul Burton
  2018-02-17 20:10 ` [PATCH v5 06/14] net: pch_gbe: Allow longer for resets Paul Burton
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton

On the MIPS Boston development board, the EG20T MAC does not report
receiving the RX clock from the (RGMII) RTL8211E PHY unless the PHY is
reset at the same time as the MAC. Since the pch_gbe driver resets the
MAC a number of times - twice during probe, and when taking down the
network interface - we need to reset the PHY at all the same times. Do
that from pch_gbe_mac_reset_hw which is used to reset the MAC in all
cases.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 3 +++
 1 file changed, 3 insertions(+)

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 11e8ced4a0f4..90e795d5cc1c 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
@@ -380,10 +380,13 @@ 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);
+	pch_gbe_phy_set_reset(hw, 1);
 	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_phy_set_reset(hw, 0);
+	usleep_range(1250, 1500);
 	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);
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 06/14] net: pch_gbe: Allow longer for resets
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
                   ` (4 preceding siblings ...)
  2018-02-17 20:10 ` [PATCH v5 05/14] net: pch_gbe: Always reset PHY along with MAC Paul Burton
@ 2018-02-17 20:10 ` Paul Burton
  2018-02-18 15:29   ` kbuild test robot
  2018-02-17 20:10 ` [PATCH v5 07/14] net: pch_gbe: Fix handling of TX padding Paul Burton
                   ` (8 subsequent siblings)
  14 siblings, 1 reply; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton

Resets of the EG20T MAC on the MIPS Boston development board take longer
than the 1000 loops that pch_gbe_wait_clr_bit was performing. Rather
than simply increasing the number of loops, switch to using
readl_poll_timeout_atomic() from linux/iopoll.h in order to provide some
independence from the speed of the CPU.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org

---

Changes in v5:
- Bump up the timeout based on feedback from Marcin.

Changes in v4: None
Changes in v3:
- Switch to using readl_poll_timeout_atomic().

Changes in v2: None

 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 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 90e795d5cc1c..77f7fbd98e8f 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
@@ -24,6 +24,7 @@
 #include <linux/ptp_classify.h>
 #include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
+#include <linux/iopoll.h>
 #include <linux/of_gpio.h>
 
 #define DRV_VERSION     "1.01"
@@ -318,13 +319,11 @@ s32 pch_gbe_mac_read_mac_addr(struct pch_gbe_hw *hw)
  */
 static void pch_gbe_wait_clr_bit(void *reg, u32 bit)
 {
+	int err;
 	u32 tmp;
 
-	/* wait busy */
-	tmp = 1000;
-	while ((ioread32(reg) & bit) && --tmp)
-		cpu_relax();
-	if (!tmp)
+	err = readl_poll_timeout_atomic(reg, tmp, !(tmp & bit), 10, 25000);
+	if (err)
 		pr_err("Error: busy bit is not cleared\n");
 }
 
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 07/14] net: pch_gbe: Fix handling of TX padding
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
                   ` (5 preceding siblings ...)
  2018-02-17 20:10 ` [PATCH v5 06/14] net: pch_gbe: Allow longer for resets Paul Burton
@ 2018-02-17 20:10 ` Paul Burton
  2018-02-19 14:01   ` David Laight
  2018-02-17 20:10 ` [PATCH v5 08/14] net: pch_gbe: Fold pch_gbe_setup_[rt]ctl into pch_gbe_configure_[rt]x Paul Burton
                   ` (7 subsequent siblings)
  14 siblings, 1 reply; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton

The ethernet controller found in the Intel EG20T Platform Controller
Hub requires that we place 2 bytes of padding between the ethernet
header & the packet payload. Our pch_gbe driver handles this by copying
packets to be transmitted to a temporary struct skb with the padding
bytes inserted, however it sets the length of this temporary skb to
equal that of the original, without the 2 padding bytes, and then uses
this length as that of the memory to map for DMA.

This is problematic on systems that don't have cache-coherent DMA, since
if the length of the original buffer is either a multiple of the
system's cache line size or one less than such a multiple then the size
we provide to dma_map_single() will not cover the last byte or two of
the data when rounded up to a cache line boundary. This may result in us
transmitting corrupt data in the last one or two bytes of the packet,
depending upon its length.

Fix this by setting the length of tmp_skb to include the 2 padding
bytes, which is actually the length of the data it holds. This is then
assigned to buffer_info->length & provided to dma_map_single() which
will operate on all of the data as desired. The EG20T datasheet
specifies that the padding bytes should not be included in the length
stored in the TX descriptor, so we switch that to use the length of the
original skb.

Whilst modifying this code we switch to using PCH_GBE_DMA_PADDING rather
than the magic number 2 to specify the size of the padding, making it
clearer what the code is doing, and fix a typo in the comment indicating
that padding is inserted.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 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 77f7fbd98e8f..60e91c0fc98b 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
@@ -1223,13 +1223,12 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 	buffer_info = &tx_ring->buffer_info[ring_num];
 	tmp_skb = buffer_info->skb;
 
-	/* [Header:14][payload] ---> [Header:14][paddong:2][payload]    */
+	/* [Header:14][payload] ---> [Header:14][padding:2][payload] */
 	memcpy(tmp_skb->data, skb->data, ETH_HLEN);
-	tmp_skb->data[ETH_HLEN] = 0x00;
-	tmp_skb->data[ETH_HLEN + 1] = 0x00;
-	tmp_skb->len = skb->len;
-	memcpy(&tmp_skb->data[ETH_HLEN + 2], &skb->data[ETH_HLEN],
-	       (skb->len - ETH_HLEN));
+	memset(&tmp_skb->data[ETH_HLEN], 0, PCH_GBE_DMA_PADDING);
+	memcpy(&tmp_skb->data[ETH_HLEN + PCH_GBE_DMA_PADDING],
+	       &skb->data[ETH_HLEN], skb->len - ETH_HLEN);
+	tmp_skb->len = skb->len + PCH_GBE_DMA_PADDING;
 	/*-- Set Buffer information --*/
 	buffer_info->length = tmp_skb->len;
 	buffer_info->dma = dma_map_single(&adapter->pdev->dev, tmp_skb->data,
@@ -1248,8 +1247,8 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 	/*-- Set Tx descriptor --*/
 	tx_desc = PCH_GBE_TX_DESC(*tx_ring, ring_num);
 	tx_desc->buffer_addr = (buffer_info->dma);
-	tx_desc->length = (tmp_skb->len);
-	tx_desc->tx_words_eob = ((tmp_skb->len + 3));
+	tx_desc->length = (skb->len);
+	tx_desc->tx_words_eob = ((skb->len + 3));
 	tx_desc->tx_frame_ctrl = (frame_ctrl);
 	tx_desc->gbec_status = (DSC_INIT16);
 
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 08/14] net: pch_gbe: Fold pch_gbe_setup_[rt]ctl into pch_gbe_configure_[rt]x
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
                   ` (6 preceding siblings ...)
  2018-02-17 20:10 ` [PATCH v5 07/14] net: pch_gbe: Fix handling of TX padding Paul Burton
@ 2018-02-17 20:10 ` Paul Burton
  2018-02-17 20:10 ` [PATCH v5 09/14] net: pch_gbe: Use pch_gbe_disable_dma_rx() in pch_gbe_configure_rx() Paul Burton
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton

The pch_gbe driver splits configuration of the receive path between
pch_gbe_setup_rctl() & pch_gbe_configure_rx(), which are always called
together and in that order. The split between the two functions seems
somewhat arbitrary, as both are configuring registers for the receive
path. Fold pch_gbe_setup_rctl() into pch_gbe_configure_rx() such that
callers only need to call one function to configure the receive path
registers.

Similarly configuration of transmit path registers is split between
pch_gbe_setup_tctl() & pch_gbe_configure_tx(), and we fold the former
into the latter in the same way.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org

---

Changes in v5:
- New patch.

Changes in v4: None
Changes in v3: None
Changes in v2: None

 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 52 ++++++----------------
 1 file changed, 13 insertions(+), 39 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 60e91c0fc98b..2d6980603ee4 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
@@ -831,39 +831,26 @@ static void pch_gbe_irq_enable(struct pch_gbe_adapter *adapter)
 		   ioread32(&hw->reg->INT_EN));
 }
 
-
-
 /**
- * pch_gbe_setup_tctl - configure the Transmit control registers
+ * pch_gbe_configure_tx - Configure Transmit Unit after Reset
  * @adapter:  Board private structure
  */
-static void pch_gbe_setup_tctl(struct pch_gbe_adapter *adapter)
+static void pch_gbe_configure_tx(struct pch_gbe_adapter *adapter)
 {
 	struct pch_gbe_hw *hw = &adapter->hw;
-	u32 tx_mode, tcpip;
+	u32 tdba, tdlen, dctrl, tx_mode, tcpip;
 
 	tx_mode = PCH_GBE_TM_LONG_PKT |
 		PCH_GBE_TM_ST_AND_FD |
 		PCH_GBE_TM_SHORT_PKT |
 		PCH_GBE_TM_TH_TX_STRT_8 |
-		PCH_GBE_TM_TH_ALM_EMP_4 | PCH_GBE_TM_TH_ALM_FULL_8;
-
+		PCH_GBE_TM_TH_ALM_EMP_4 |
+		PCH_GBE_TM_TH_ALM_FULL_8;
 	iowrite32(tx_mode, &hw->reg->TX_MODE);
 
 	tcpip = ioread32(&hw->reg->TCPIP_ACC);
 	tcpip |= PCH_GBE_TX_TCPIPACC_EN;
 	iowrite32(tcpip, &hw->reg->TCPIP_ACC);
-	return;
-}
-
-/**
- * pch_gbe_configure_tx - Configure Transmit Unit after Reset
- * @adapter:  Board private structure
- */
-static void pch_gbe_configure_tx(struct pch_gbe_adapter *adapter)
-{
-	struct pch_gbe_hw *hw = &adapter->hw;
-	u32 tdba, tdlen, dctrl;
 
 	netdev_dbg(adapter->netdev, "dma addr = 0x%08llx  size = 0x%08x\n",
 		   (unsigned long long)adapter->tx_ring->dma,
@@ -883,35 +870,25 @@ static void pch_gbe_configure_tx(struct pch_gbe_adapter *adapter)
 }
 
 /**
- * pch_gbe_setup_rctl - Configure the receive control registers
+ * pch_gbe_configure_rx - Configure Receive Unit after Reset
  * @adapter:  Board private structure
  */
-static void pch_gbe_setup_rctl(struct pch_gbe_adapter *adapter)
+static void pch_gbe_configure_rx(struct pch_gbe_adapter *adapter)
 {
 	struct pch_gbe_hw *hw = &adapter->hw;
-	u32 rx_mode, tcpip;
-
-	rx_mode = PCH_GBE_ADD_FIL_EN | PCH_GBE_MLT_FIL_EN |
-	PCH_GBE_RH_ALM_EMP_4 | PCH_GBE_RH_ALM_FULL_4 | PCH_GBE_RH_RD_TRG_8;
+	u32 rdba, rdlen, rxdma, rx_mode, tcpip;
 
+	rx_mode = PCH_GBE_ADD_FIL_EN |
+		  PCH_GBE_MLT_FIL_EN |
+		  PCH_GBE_RH_ALM_EMP_4 |
+		  PCH_GBE_RH_ALM_FULL_4 |
+		  PCH_GBE_RH_RD_TRG_8;
 	iowrite32(rx_mode, &hw->reg->RX_MODE);
 
 	tcpip = ioread32(&hw->reg->TCPIP_ACC);
-
 	tcpip |= PCH_GBE_RX_TCPIPACC_OFF;
 	tcpip &= ~PCH_GBE_RX_TCPIPACC_EN;
 	iowrite32(tcpip, &hw->reg->TCPIP_ACC);
-	return;
-}
-
-/**
- * pch_gbe_configure_rx - Configure Receive Unit after Reset
- * @adapter:  Board private structure
- */
-static void pch_gbe_configure_rx(struct pch_gbe_adapter *adapter)
-{
-	struct pch_gbe_hw *hw = &adapter->hw;
-	u32 rdba, rdlen, rxdma;
 
 	netdev_dbg(adapter->netdev, "dma adr = 0x%08llx  size = 0x%08x\n",
 		   (unsigned long long)adapter->rx_ring->dma,
@@ -1954,9 +1931,7 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
 	/* hardware has been reset, we need to reload some things */
 	pch_gbe_set_multi(netdev);
 
-	pch_gbe_setup_tctl(adapter);
 	pch_gbe_configure_tx(adapter);
-	pch_gbe_setup_rctl(adapter);
 	pch_gbe_configure_rx(adapter);
 
 	err = pch_gbe_request_irq(adapter);
@@ -2486,7 +2461,6 @@ static int __pch_gbe_suspend(struct pci_dev *pdev)
 		pch_gbe_down(adapter);
 	if (wufc) {
 		pch_gbe_set_multi(netdev);
-		pch_gbe_setup_rctl(adapter);
 		pch_gbe_configure_rx(adapter);
 		pch_gbe_set_rgmii_ctrl(adapter, hw->mac.link_speed,
 					hw->mac.link_duplex);
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 09/14] net: pch_gbe: Use pch_gbe_disable_dma_rx() in pch_gbe_configure_rx()
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
                   ` (7 preceding siblings ...)
  2018-02-17 20:10 ` [PATCH v5 08/14] net: pch_gbe: Fold pch_gbe_setup_[rt]ctl into pch_gbe_configure_[rt]x Paul Burton
@ 2018-02-17 20:10 ` Paul Burton
  2018-02-17 20:10 ` [PATCH v5 10/14] net: pch_gbe: Disable TX DMA whilst configuring descriptors Paul Burton
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton

The pch_gbe_configure_rx() function open-codes the equivalent of
pch_gbe_disable_dma_rx(). Remove the duplication by moving
pch_gbe_disable_dma_rx(), and pch_gbe_enable_dma_rx() for consistency,
to be defined earlier than pch_gbe_configure_rx() and have
pch_gbe_configure_rx() call pch_gbe_disable_dma_rx() rather than
duplicate its functionality.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org

---

Changes in v5:
- New patch.

Changes in v4: None
Changes in v3: None
Changes in v2: None

 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 48 ++++++++++------------
 1 file changed, 22 insertions(+), 26 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 2d6980603ee4..b6cc4a34ed89 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
@@ -831,6 +831,26 @@ static void pch_gbe_irq_enable(struct pch_gbe_adapter *adapter)
 		   ioread32(&hw->reg->INT_EN));
 }
 
+static void pch_gbe_disable_dma_rx(struct pch_gbe_hw *hw)
+{
+	u32 rxdma;
+
+	/* Disable Receive DMA */
+	rxdma = ioread32(&hw->reg->DMA_CTRL);
+	rxdma &= ~PCH_GBE_RX_DMA_EN;
+	iowrite32(rxdma, &hw->reg->DMA_CTRL);
+}
+
+static void pch_gbe_enable_dma_rx(struct pch_gbe_hw *hw)
+{
+	u32 rxdma;
+
+	/* Enables Receive DMA */
+	rxdma = ioread32(&hw->reg->DMA_CTRL);
+	rxdma |= PCH_GBE_RX_DMA_EN;
+	iowrite32(rxdma, &hw->reg->DMA_CTRL);
+}
+
 /**
  * pch_gbe_configure_tx - Configure Transmit Unit after Reset
  * @adapter:  Board private structure
@@ -876,7 +896,7 @@ static void pch_gbe_configure_tx(struct pch_gbe_adapter *adapter)
 static void pch_gbe_configure_rx(struct pch_gbe_adapter *adapter)
 {
 	struct pch_gbe_hw *hw = &adapter->hw;
-	u32 rdba, rdlen, rxdma, rx_mode, tcpip;
+	u32 rdba, rdlen, rx_mode, tcpip;
 
 	rx_mode = PCH_GBE_ADD_FIL_EN |
 		  PCH_GBE_MLT_FIL_EN |
@@ -897,11 +917,7 @@ static void pch_gbe_configure_rx(struct pch_gbe_adapter *adapter)
 	pch_gbe_mac_force_mac_fc(hw);
 
 	pch_gbe_disable_mac_rx(hw);
-
-	/* Disables Receive DMA */
-	rxdma = ioread32(&hw->reg->DMA_CTRL);
-	rxdma &= ~PCH_GBE_RX_DMA_EN;
-	iowrite32(rxdma, &hw->reg->DMA_CTRL);
+	pch_gbe_disable_dma_rx(hw);
 
 	netdev_dbg(adapter->netdev,
 		   "MAC_RX_EN reg = 0x%08x  DMA_CTRL reg = 0x%08x\n",
@@ -1290,26 +1306,6 @@ void pch_gbe_update_stats(struct pch_gbe_adapter *adapter)
 	spin_unlock_irqrestore(&adapter->stats_lock, flags);
 }
 
-static void pch_gbe_disable_dma_rx(struct pch_gbe_hw *hw)
-{
-	u32 rxdma;
-
-	/* Disable Receive DMA */
-	rxdma = ioread32(&hw->reg->DMA_CTRL);
-	rxdma &= ~PCH_GBE_RX_DMA_EN;
-	iowrite32(rxdma, &hw->reg->DMA_CTRL);
-}
-
-static void pch_gbe_enable_dma_rx(struct pch_gbe_hw *hw)
-{
-	u32 rxdma;
-
-	/* Enables Receive DMA */
-	rxdma = ioread32(&hw->reg->DMA_CTRL);
-	rxdma |= PCH_GBE_RX_DMA_EN;
-	iowrite32(rxdma, &hw->reg->DMA_CTRL);
-}
-
 /**
  * pch_gbe_intr - Interrupt Handler
  * @irq:   Interrupt number
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 10/14] net: pch_gbe: Disable TX DMA whilst configuring descriptors
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
                   ` (8 preceding siblings ...)
  2018-02-17 20:10 ` [PATCH v5 09/14] net: pch_gbe: Use pch_gbe_disable_dma_rx() in pch_gbe_configure_rx() Paul Burton
@ 2018-02-17 20:10 ` Paul Burton
  2018-02-17 20:10 ` [PATCH v5 11/14] net: pch_gbe: Ensure DMA is ordered with descriptor writes Paul Burton
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton

The pch_gbe driver enables TX DMA the first time we call
pch_gbe_configure_tx() and never disables it again, even if we
reconfigure the device & modify the transmit descriptor ring. This seems
unsafe, since the device may continue accessing descriptors whilst they
are in an unpredictable & possibly invalid state - especially on systems
where the CPUs writes to the descriptors is not coherent with DMA.

In the RX path pch_gbe_configure_rx() disables DMA before configuring
the descriptor pointers & before we set up the descriptors, then
pch_gbe_up() calls pch_gbe_enable_dma_rx() to enable DMA again after the
descriptors have been configured. Here we copy that same scheme for the
TX path - pch_gbe_configure_tx() calls pch_gbe_disable_dma_tx() to
disable DMA, and then after the descriptors have been configured
pch_gbe_up() calls pch_gbe_enable_dma_tx() to enable DMA. This should
ensure that the device doesn't begin reading descriptors before we have
configured them.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org

---

Changes in v5:
- New patch.

Changes in v4: None
Changes in v3: None
Changes in v2: None

 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 29 +++++++++++++++++-----
 1 file changed, 23 insertions(+), 6 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 b6cc4a34ed89..4354842b9b7e 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
@@ -851,6 +851,24 @@ static void pch_gbe_enable_dma_rx(struct pch_gbe_hw *hw)
 	iowrite32(rxdma, &hw->reg->DMA_CTRL);
 }
 
+static void pch_gbe_disable_dma_tx(struct pch_gbe_hw *hw)
+{
+	u32 rxdma;
+
+	rxdma = ioread32(&hw->reg->DMA_CTRL);
+	rxdma &= ~PCH_GBE_TX_DMA_EN;
+	iowrite32(rxdma, &hw->reg->DMA_CTRL);
+}
+
+static void pch_gbe_enable_dma_tx(struct pch_gbe_hw *hw)
+{
+	u32 rxdma;
+
+	rxdma = ioread32(&hw->reg->DMA_CTRL);
+	rxdma |= PCH_GBE_TX_DMA_EN;
+	iowrite32(rxdma, &hw->reg->DMA_CTRL);
+}
+
 /**
  * pch_gbe_configure_tx - Configure Transmit Unit after Reset
  * @adapter:  Board private structure
@@ -858,7 +876,7 @@ static void pch_gbe_enable_dma_rx(struct pch_gbe_hw *hw)
 static void pch_gbe_configure_tx(struct pch_gbe_adapter *adapter)
 {
 	struct pch_gbe_hw *hw = &adapter->hw;
-	u32 tdba, tdlen, dctrl, tx_mode, tcpip;
+	u32 tdba, tdlen, tx_mode, tcpip;
 
 	tx_mode = PCH_GBE_TM_LONG_PKT |
 		PCH_GBE_TM_ST_AND_FD |
@@ -876,17 +894,14 @@ static void pch_gbe_configure_tx(struct pch_gbe_adapter *adapter)
 		   (unsigned long long)adapter->tx_ring->dma,
 		   adapter->tx_ring->size);
 
+	pch_gbe_disable_dma_tx(hw);
+
 	/* Setup the HW Tx Head and Tail descriptor pointers */
 	tdba = adapter->tx_ring->dma;
 	tdlen = adapter->tx_ring->size - 0x10;
 	iowrite32(tdba, &hw->reg->TX_DSC_BASE);
 	iowrite32(tdlen, &hw->reg->TX_DSC_SIZE);
 	iowrite32(tdba, &hw->reg->TX_DSC_SW_P);
-
-	/* Enables Transmission DMA */
-	dctrl = ioread32(&hw->reg->DMA_CTRL);
-	dctrl |= PCH_GBE_TX_DMA_EN;
-	iowrite32(dctrl, &hw->reg->DMA_CTRL);
 }
 
 /**
@@ -1945,6 +1960,8 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
 	pch_gbe_alloc_tx_buffers(adapter, tx_ring);
 	pch_gbe_alloc_rx_buffers(adapter, rx_ring, rx_ring->count);
 	adapter->tx_queue_len = netdev->tx_queue_len;
+
+	pch_gbe_enable_dma_tx(&adapter->hw);
 	pch_gbe_enable_dma_rx(&adapter->hw);
 	pch_gbe_enable_mac_rx(&adapter->hw);
 
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 11/14] net: pch_gbe: Ensure DMA is ordered with descriptor writes
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
                   ` (9 preceding siblings ...)
  2018-02-17 20:10 ` [PATCH v5 10/14] net: pch_gbe: Disable TX DMA whilst configuring descriptors Paul Burton
@ 2018-02-17 20:10 ` Paul Burton
  2018-02-17 20:10 ` [PATCH v5 12/14] net: pch_gbe: Fix TX RX descriptor accesses for big endian systems Paul Burton
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton

On weakly ordered systems writes to the RX or TX descriptors may be
reordered with the write to the DMA control register that enables DMA.
If this happens then the device may see descriptors in an intermediate
& invalid state, leading to incorrect behaviour. Add barriers to ensure
that DMA is enabled only after all writes to the descriptors.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org

---

Changes in v5:
- New patch.

Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 6 ++++++
 1 file changed, 6 insertions(+)

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 4354842b9b7e..8e3ad7dcef0b 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
@@ -1260,6 +1260,9 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 	tx_desc->tx_frame_ctrl = (frame_ctrl);
 	tx_desc->gbec_status = (DSC_INIT16);
 
+	/* Ensure writes to descriptors complete before DMA begins */
+	mmiowb();
+
 	if (unlikely(++ring_num == tx_ring->count))
 		ring_num = 0;
 
@@ -1961,6 +1964,9 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
 	pch_gbe_alloc_rx_buffers(adapter, rx_ring, rx_ring->count);
 	adapter->tx_queue_len = netdev->tx_queue_len;
 
+	/* Ensure writes to descriptors complete before DMA begins */
+	mmiowb();
+
 	pch_gbe_enable_dma_tx(&adapter->hw);
 	pch_gbe_enable_dma_rx(&adapter->hw);
 	pch_gbe_enable_mac_rx(&adapter->hw);
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 12/14] net: pch_gbe: Fix TX RX descriptor accesses for big endian systems
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
                   ` (10 preceding siblings ...)
  2018-02-17 20:10 ` [PATCH v5 11/14] net: pch_gbe: Ensure DMA is ordered with descriptor writes Paul Burton
@ 2018-02-17 20:10 ` Paul Burton
  2018-02-18 16:01   ` kbuild test robot
  2018-02-17 20:10 ` [PATCH v5 13/14] ptp: pch: Allow build on MIPS platforms Paul Burton
                   ` (2 subsequent siblings)
  14 siblings, 1 reply; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton

From: Hassan Naveed <hassan.naveed@mips.com>

Fix pch_gbe driver for ethernet operations for a big endian CPU.
Values written to and read from transmit and receive descriptors
in the pch_gbe driver are byte swapped from the perspective of a
big endian CPU, since the ethernet controller always operates in
little endian mode. Rectify this by appropriately byte swapping
these descriptor field values in the driver software.

Signed-off-by: Hassan Naveed <hassan.naveed@mips.com>
Signed-off-by: Paul Burton <paul.burton@mips.com>
Reviewed-by: Paul Burton <paul.burton@mips.com>
Reviewed-by: Matt Redfearn <matt.redfearn@mips.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org

---

Changes in v5:
- Newly included in this series.

Changes in v4: None
Changes in v3: None
Changes in v2:
- Use __le{16,32} for field types, checked with sparse.

 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    | 22 ++++----
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 66 ++++++++++++----------
 2 files changed, 46 insertions(+), 42 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 8ba9ced2d1fd..7159e39b4685 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -431,13 +431,13 @@ struct pch_gbe_hw {
  * @reserved2:		Reserved
  */
 struct pch_gbe_rx_desc {
-	u32 buffer_addr;
-	u32 tcp_ip_status;
-	u16 rx_words_eob;
-	u16 gbec_status;
+	__le32 buffer_addr;
+	__le32 tcp_ip_status;
+	__le16 rx_words_eob;
+	__le16 gbec_status;
 	u8 dma_status;
 	u8 reserved1;
-	u16 reserved2;
+	__le16 reserved2;
 };
 
 /**
@@ -452,14 +452,14 @@ struct pch_gbe_rx_desc {
  * @gbec_status:	GMAC Status
  */
 struct pch_gbe_tx_desc {
-	u32 buffer_addr;
-	u16 length;
-	u16 reserved1;
-	u16 tx_words_eob;
-	u16 tx_frame_ctrl;
+	__le32 buffer_addr;
+	__le16 length;
+	__le16 reserved1;
+	__le16 tx_words_eob;
+	__le16 tx_frame_ctrl;
 	u8 dma_status;
 	u8 reserved2;
-	u16 gbec_status;
+	__le16 gbec_status;
 };
 
 
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 8e3ad7dcef0b..a0b8c8f4b4c9 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
@@ -1254,11 +1254,11 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 
 	/*-- Set Tx descriptor --*/
 	tx_desc = PCH_GBE_TX_DESC(*tx_ring, ring_num);
-	tx_desc->buffer_addr = (buffer_info->dma);
-	tx_desc->length = (skb->len);
-	tx_desc->tx_words_eob = ((skb->len + 3));
-	tx_desc->tx_frame_ctrl = (frame_ctrl);
-	tx_desc->gbec_status = (DSC_INIT16);
+	tx_desc->buffer_addr = cpu_to_le32(buffer_info->dma);
+	tx_desc->length = cpu_to_le16(skb->len);
+	tx_desc->tx_words_eob = cpu_to_le16(skb->len + 3);
+	tx_desc->tx_frame_ctrl = cpu_to_le16(frame_ctrl);
+	tx_desc->gbec_status = cpu_to_le16(DSC_INIT16);
 
 	/* Ensure writes to descriptors complete before DMA begins */
 	mmiowb();
@@ -1447,8 +1447,8 @@ pch_gbe_alloc_rx_buffers(struct pch_gbe_adapter *adapter,
 		}
 		buffer_info->mapped = true;
 		rx_desc = PCH_GBE_RX_DESC(*rx_ring, i);
-		rx_desc->buffer_addr = (buffer_info->dma);
-		rx_desc->gbec_status = DSC_INIT16;
+		rx_desc->buffer_addr = cpu_to_le32(buffer_info->dma);
+		rx_desc->gbec_status = cpu_to_le16(DSC_INIT16);
 
 		netdev_dbg(netdev,
 			   "i = %d  buffer_info->dma = 0x08%llx  buffer_info->length = 0x%x\n",
@@ -1520,7 +1520,7 @@ static void pch_gbe_alloc_tx_buffers(struct pch_gbe_adapter *adapter,
 		skb_reserve(skb, PCH_GBE_DMA_ALIGN);
 		buffer_info->skb = skb;
 		tx_desc = PCH_GBE_TX_DESC(*tx_ring, i);
-		tx_desc->gbec_status = (DSC_INIT16);
+		tx_desc->gbec_status = cpu_to_le16(DSC_INIT16);
 	}
 	return;
 }
@@ -1551,11 +1551,12 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 	i = tx_ring->next_to_clean;
 	tx_desc = PCH_GBE_TX_DESC(*tx_ring, i);
 	netdev_dbg(adapter->netdev, "gbec_status:0x%04x  dma_status:0x%04x\n",
-		   tx_desc->gbec_status, tx_desc->dma_status);
+		   le16_to_cpu(tx_desc->gbec_status), tx_desc->dma_status);
 
 	unused = PCH_GBE_DESC_UNUSED(tx_ring);
 	thresh = tx_ring->count - PCH_GBE_TX_WEIGHT;
-	if ((tx_desc->gbec_status == DSC_INIT16) && (unused < thresh))
+	if ((le16_to_cpu(tx_desc->gbec_status) == DSC_INIT16) &&
+	    (unused < thresh))
 	{  /* current marked clean, tx queue filling up, do extra clean */
 		int j, k;
 		if (unused < 8) {  /* tx queue nearly full */
@@ -1570,47 +1571,49 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 		for (j = 0; j < PCH_GBE_TX_WEIGHT; j++)
 		{
 			tx_desc = PCH_GBE_TX_DESC(*tx_ring, k);
-			if (tx_desc->gbec_status != DSC_INIT16) break; /*found*/
+			if (le16_to_cpu(tx_desc->gbec_status) != DSC_INIT16)
+				break; /*found*/
 			if (++k >= tx_ring->count) k = 0;  /*increment, wrap*/
 		}
 		if (j < PCH_GBE_TX_WEIGHT) {
 			netdev_dbg(adapter->netdev,
 				   "clean_tx: unused=%d loops=%d found tx_desc[%x,%x:%x].gbec_status=%04x\n",
 				   unused, j, i, k, tx_ring->next_to_use,
-				   tx_desc->gbec_status);
+				   le16_to_cpu(tx_desc->gbec_status));
 			i = k;  /*found one to clean, usu gbec_status==2000.*/
 		}
 	}
 
-	while ((tx_desc->gbec_status & DSC_INIT16) == 0x0000) {
+	while ((cpu_to_le16(tx_desc->gbec_status) & DSC_INIT16) == 0x0000) {
 		netdev_dbg(adapter->netdev, "gbec_status:0x%04x\n",
-			   tx_desc->gbec_status);
+			   le16_to_cpu(tx_desc->gbec_status));
 		buffer_info = &tx_ring->buffer_info[i];
 		skb = buffer_info->skb;
 		cleaned = true;
 
-		if ((tx_desc->gbec_status & PCH_GBE_TXD_GMAC_STAT_ABT)) {
+		if ((le16_to_cpu(tx_desc->gbec_status) &
+			PCH_GBE_TXD_GMAC_STAT_ABT)) {
 			adapter->stats.tx_aborted_errors++;
 			netdev_err(adapter->netdev, "Transfer Abort Error\n");
-		} else if ((tx_desc->gbec_status & PCH_GBE_TXD_GMAC_STAT_CRSER)
-			  ) {
+		} else if ((le16_to_cpu(tx_desc->gbec_status) &
+				PCH_GBE_TXD_GMAC_STAT_CRSER)) {
 			adapter->stats.tx_carrier_errors++;
 			netdev_err(adapter->netdev,
 				   "Transfer Carrier Sense Error\n");
-		} else if ((tx_desc->gbec_status & PCH_GBE_TXD_GMAC_STAT_EXCOL)
-			  ) {
+		} else if ((le16_to_cpu(tx_desc->gbec_status) &
+					PCH_GBE_TXD_GMAC_STAT_EXCOL)) {
 			adapter->stats.tx_aborted_errors++;
 			netdev_err(adapter->netdev,
 				   "Transfer Collision Abort Error\n");
-		} else if ((tx_desc->gbec_status &
+		} else if ((le16_to_cpu(tx_desc->gbec_status) &
 			    (PCH_GBE_TXD_GMAC_STAT_SNGCOL |
 			     PCH_GBE_TXD_GMAC_STAT_MLTCOL))) {
 			adapter->stats.collisions++;
 			adapter->stats.tx_packets++;
 			adapter->stats.tx_bytes += skb->len;
 			netdev_dbg(adapter->netdev, "Transfer Collision\n");
-		} else if ((tx_desc->gbec_status & PCH_GBE_TXD_GMAC_STAT_CMPLT)
-			  ) {
+		} else if ((le16_to_cpu(tx_desc->gbec_status) &
+				PCH_GBE_TXD_GMAC_STAT_CMPLT)) {
 			adapter->stats.tx_packets++;
 			adapter->stats.tx_bytes += skb->len;
 		}
@@ -1626,7 +1629,7 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 				   "trim buffer_info->skb : %d\n", i);
 			skb_trim(buffer_info->skb, 0);
 		}
-		tx_desc->gbec_status = DSC_INIT16;
+		tx_desc->gbec_status = cpu_to_le16(DSC_INIT16);
 		if (unlikely(++i == tx_ring->count))
 			i = 0;
 		tx_desc = PCH_GBE_TX_DESC(*tx_ring, i);
@@ -1692,15 +1695,15 @@ pch_gbe_clean_rx(struct pch_gbe_adapter *adapter,
 	while (*work_done < work_to_do) {
 		/* Check Rx descriptor status */
 		rx_desc = PCH_GBE_RX_DESC(*rx_ring, i);
-		if (rx_desc->gbec_status == DSC_INIT16)
+		if (le16_to_cpu(rx_desc->gbec_status) == DSC_INIT16)
 			break;
 		cleaned = true;
 		cleaned_count++;
 
 		dma_status = rx_desc->dma_status;
-		gbec_status = rx_desc->gbec_status;
-		tcp_ip_status = rx_desc->tcp_ip_status;
-		rx_desc->gbec_status = DSC_INIT16;
+		gbec_status = le16_to_cpu(rx_desc->gbec_status);
+		tcp_ip_status = le32_to_cpu(rx_desc->tcp_ip_status);
+		rx_desc->gbec_status = cpu_to_le16(DSC_INIT16);
 		buffer_info = &rx_ring->buffer_info[i];
 		skb = buffer_info->skb;
 		buffer_info->skb = NULL;
@@ -1729,8 +1732,9 @@ pch_gbe_clean_rx(struct pch_gbe_adapter *adapter,
 		} else {
 			/* get receive length */
 			/* length convert[-3], length includes FCS length */
-			length = (rx_desc->rx_words_eob) - 3 - ETH_FCS_LEN;
-			if (rx_desc->rx_words_eob & 0x02)
+			length = le16_to_cpu(rx_desc->rx_words_eob) - 3 -
+					ETH_FCS_LEN;
+			if (le16_to_cpu(rx_desc->rx_words_eob) & 0x02)
 				length = length - 4;
 			/*
 			 * buffer_info->rx_buffer: [Header:14][payload]
@@ -1810,7 +1814,7 @@ int pch_gbe_setup_tx_resources(struct pch_gbe_adapter *adapter,
 
 	for (desNo = 0; desNo < tx_ring->count; desNo++) {
 		tx_desc = PCH_GBE_TX_DESC(*tx_ring, desNo);
-		tx_desc->gbec_status = DSC_INIT16;
+		tx_desc->gbec_status = cpu_to_le16(DSC_INIT16);
 	}
 	netdev_dbg(adapter->netdev,
 		   "tx_ring->desc = 0x%p  tx_ring->dma = 0x%08llx next_to_clean = 0x%08x  next_to_use = 0x%08x\n",
@@ -1851,7 +1855,7 @@ int pch_gbe_setup_rx_resources(struct pch_gbe_adapter *adapter,
 	rx_ring->next_to_use = 0;
 	for (desNo = 0; desNo < rx_ring->count; desNo++) {
 		rx_desc = PCH_GBE_RX_DESC(*rx_ring, desNo);
-		rx_desc->gbec_status = DSC_INIT16;
+		rx_desc->gbec_status = cpu_to_le16(DSC_INIT16);
 	}
 	netdev_dbg(adapter->netdev,
 		   "rx_ring->desc = 0x%p  rx_ring->dma = 0x%08llx next_to_clean = 0x%08x  next_to_use = 0x%08x\n",
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 13/14] ptp: pch: Allow build on MIPS platforms
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
                   ` (11 preceding siblings ...)
  2018-02-17 20:10 ` [PATCH v5 12/14] net: pch_gbe: Fix TX RX descriptor accesses for big endian systems Paul Burton
@ 2018-02-17 20:10 ` Paul Burton
  2018-02-17 20:10 ` [PATCH v5 14/14] net: pch_gbe: " Paul Burton
  2018-02-18 15:31 ` [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support David Miller
  14 siblings, 0 replies; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton

Allow the ptp_pch driver to be built on MIPS platforms in preparation
for use on the MIPS Boston board.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Acked-by: Richard Cochran <richardcochran@gmail.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org

---

Changes in v5:
- Newly included in this series to satisfy Kconfig.

Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/ptp/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index a21ad10d613c..8618982ab96a 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -90,7 +90,7 @@ config DP83640_PHY
 
 config PTP_1588_CLOCK_PCH
 	tristate "Intel PCH EG20T as PTP clock"
-	depends on X86_32 || COMPILE_TEST
+	depends on X86_32 || MIPS || COMPILE_TEST
 	depends on HAS_IOMEM && NET
 	imply PTP_1588_CLOCK
 	help
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 14/14] net: pch_gbe: Allow build on MIPS platforms
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
                   ` (12 preceding siblings ...)
  2018-02-17 20:10 ` [PATCH v5 13/14] ptp: pch: Allow build on MIPS platforms Paul Burton
@ 2018-02-17 20:10 ` Paul Burton
  2018-02-18 15:31 ` [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support David Miller
  14 siblings, 0 replies; 33+ messages in thread
From: Paul Burton @ 2018-02-17 20:10 UTC (permalink / raw)
  To: netdev
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller, linux-mips,
	Paul Burton

Allow the pch_gbe driver to be built on MIPS platforms, allowing its use
on the MIPS Boston development board.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org

---

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/net/ethernet/oki-semi/pch_gbe/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig b/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
index 5f7a35212796..4d3809ae75e1 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
@@ -4,7 +4,7 @@
 
 config PCH_GBE
 	tristate "OKI SEMICONDUCTOR IOH(ML7223/ML7831) GbE"
-	depends on PCI && (X86_32 || COMPILE_TEST)
+	depends on PCI && (X86_32 || MIPS || COMPILE_TEST)
 	select MII
 	select PTP_1588_CLOCK_PCH
 	select NET_PTP_CLASSIFY
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 01/14] net: pch_gbe: Mark Minnow PHY reset GPIO active low
  2018-02-17 20:10 ` [PATCH v5 01/14] net: pch_gbe: Mark Minnow PHY reset GPIO active low Paul Burton
@ 2018-02-17 22:14   ` Andrew Lunn
  0 siblings, 0 replies; 33+ messages in thread
From: Andrew Lunn @ 2018-02-17 22:14 UTC (permalink / raw)
  To: Paul Burton
  Cc: netdev, Hassan Naveed, Matt Redfearn, David S . Miller,
	linux-mips

> @@ -2700,10 +2701,10 @@ static int pch_gbe_minnow_platform_init(struct pci_dev *pdev)
>  		return ret;
>  	}
>  
> -	gpio_set_value(gpio, 0);
> -	usleep_range(1250, 1500);
>  	gpio_set_value(gpio, 1);
>  	usleep_range(1250, 1500);
> +	gpio_set_value(gpio, 0);
> +	usleep_range(1250, 1500);

Hi Paul

It would be better to rewrite and use the gpiod_ API. The GPIO core
would then handle active low/active high.

      Andrew

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 02/14] net: pch_gbe: Pull PHY GPIO handling out of Minnow code
  2018-02-17 20:10 ` [PATCH v5 02/14] net: pch_gbe: Pull PHY GPIO handling out of Minnow code Paul Burton
@ 2018-02-17 22:29   ` Andrew Lunn
  2018-02-17 22:50     ` Paul Burton
  0 siblings, 1 reply; 33+ messages in thread
From: Andrew Lunn @ 2018-02-17 22:29 UTC (permalink / raw)
  To: Paul Burton
  Cc: netdev, Hassan Naveed, Matt Redfearn, David S . Miller,
	linux-mips

On Sat, Feb 17, 2018 at 12:10:25PM -0800, Paul Burton wrote:
> The MIPS Boston development board uses the Intel EG20T Platform
> Controller Hub, including its gigabit ethernet controller, and requires
> that its RTL8211E PHY be reset much like the Minnow platform. Pull the
> PHY reset GPIO handling out of Minnow-specific code such that it can be
> shared by later patches.

Hi Paul

I'm i right in saying the driver currently supports the Atheros AT8031
PHY? The same phy which is supported in drivers/net/phy/at803x.c?

If so, i think you are doing this all wrong. You would be much better
off throwing away pch_gbe_phy.c and write a proper MDIO driver. You
then get the PHY driver for free, and the MDIO code could will handle
your GPIO for you, in the standardised way.

     Andrew

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 03/14] dt-bindings: net: Document Intel pch_gbe binding
       [not found]   ` <20180217201037.3006-4-paul.burton-8NJIiSa5LzA@public.gmane.org>
@ 2018-02-17 22:32     ` Andrew Lunn
  0 siblings, 0 replies; 33+ messages in thread
From: Andrew Lunn @ 2018-02-17 22:32 UTC (permalink / raw)
  To: Paul Burton
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Hassan Naveed, Matt Redfearn,
	David S . Miller, linux-mips-6z/3iImG2C8G8FEW9MqTrA, Mark Rutland,
	Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA

> @@ -0,0 +1,25 @@
> +Intel Platform Controller Hub (PCH) GigaBit Ethernet (GBE)
> +
> +Required properties:
> +- compatible:		Should be the PCI vendor & device ID, eg. "pci8086,8802".
> +- reg:			Should be a PCI device number as specified by the PCI bus
> +			binding to IEEE Std 1275-1994.
> +- phy-reset-gpios:	Should be a GPIO list containing a single GPIO that
> +			resets the attached PHY when active.
> +

Hi Paul

Please see Documentation/devicetree/bindings/net/phy.txt. In
particular:

reset-gpios: The GPIO phandle and specifier for the PHY reset signal.

You should be conforming to the existing binding.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 02/14] net: pch_gbe: Pull PHY GPIO handling out of Minnow code
  2018-02-17 22:29   ` Andrew Lunn
@ 2018-02-17 22:50     ` Paul Burton
  2018-02-17 23:34       ` Andrew Lunn
  0 siblings, 1 reply; 33+ messages in thread
From: Paul Burton @ 2018-02-17 22:50 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, Hassan Naveed, Matt Redfearn, David S . Miller,
	linux-mips

Hi Andrew,

On Sat, Feb 17, 2018 at 11:29:33PM +0100, Andrew Lunn wrote:
> On Sat, Feb 17, 2018 at 12:10:25PM -0800, Paul Burton wrote:
> > The MIPS Boston development board uses the Intel EG20T Platform
> > Controller Hub, including its gigabit ethernet controller, and requires
> > that its RTL8211E PHY be reset much like the Minnow platform. Pull the
> > PHY reset GPIO handling out of Minnow-specific code such that it can be
> > shared by later patches.
> 
> Hi Paul
> 
> I'm i right in saying the driver currently supports the Atheros AT8031
> PHY? The same phy which is supported in drivers/net/phy/at803x.c?

It looks like the driver does contain some code relating to that PHY,
but it's not the one I'm using with the MIPS Boston board - there we
have a Realtek RTL8211E (as mentioned in the commit message) which is
working fine alongside this pch_gbe driver too.

> If so, i think you are doing this all wrong.

Note that this is a driver which is already in mainline, and I didn't
write it. Claiming that *I* am doing this all wrong is a bit of a
stretch - all this patch does is make small changes to some existing
code, which only tangentially relates to a PHY driver, such that it
ceases to be specific to a single platform.

> You would be much better off throwing away pch_gbe_phy.c and write a
> proper MDIO driver. You then get the PHY driver for free, and the MDIO
> code could will handle your GPIO for you, in the standardised way.

Even if that is true, rewriting the driver's PHY handling would be a
very separate change to the changes this series make which allow this
driver to work on a platform besides the Minnowboard. The *only* thing
this series does relating to the PHY is allow the reset GPIO to be
handled properly - rewriting the existing PHY handling is beyond it's
scope.

Note that I do have various cleanups to the driver beyond this series
which I intend to submit after it is functional for my system[1], so I
am not saying that I don't care about improving the driver. But please,
let's do one thing at a time.

Thanks,
    Paul

[1] https://git.linux-mips.org/cgit/paul/linux.git/log/?h=up417-boston-eth-cleanup

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 02/14] net: pch_gbe: Pull PHY GPIO handling out of Minnow code
  2018-02-17 22:50     ` Paul Burton
@ 2018-02-17 23:34       ` Andrew Lunn
  2018-02-18 15:51         ` Paul Burton
  0 siblings, 1 reply; 33+ messages in thread
From: Andrew Lunn @ 2018-02-17 23:34 UTC (permalink / raw)
  To: Paul Burton
  Cc: netdev, Hassan Naveed, Matt Redfearn, David S . Miller,
	linux-mips

> Note that this is a driver which is already in mainline, and I didn't
> write it. Claiming that *I* am doing this all wrong is a bit of a
> stretch - all this patch does is make small changes to some existing
> code, which only tangentially relates to a PHY driver, such that it
> ceases to be specific to a single platform.

Hi Paul

I would so you are doing it all wrong for the reset GPIO.

> Even if that is true, rewriting the driver's PHY handling would be a
> very separate change to the changes this series make which allow this
> driver to work on a platform besides the Minnowboard. The *only* thing
> this series does relating to the PHY is allow the reset GPIO to be
> handled properly - rewriting the existing PHY handling is beyond it's
> scope.

Well, you are adding a device tree binding, which needs to be
supported forever. This is going to make things messy in the future
when you do such a cleanup that you follow the PHY binding, in that
you have to handle both what you add here, and the official PHY
binding.

I would prefer that for the moment, you drop the PHY binding patches
in this series. That is what i object to the most. Adding an MDIO
driver and using the standard PHY driver for this PHY is all
internal. You can change that anytime. But adding a binding means an
ABI.

	Andrew

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 06/14] net: pch_gbe: Allow longer for resets
  2018-02-17 20:10 ` [PATCH v5 06/14] net: pch_gbe: Allow longer for resets Paul Burton
@ 2018-02-18 15:29   ` kbuild test robot
  0 siblings, 0 replies; 33+ messages in thread
From: kbuild test robot @ 2018-02-18 15:29 UTC (permalink / raw)
  To: Paul Burton
  Cc: kbuild-all, netdev, Hassan Naveed, Matt Redfearn,
	David S . Miller, linux-mips, Paul Burton

Hi Paul,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on v4.16-rc1 next-20180216]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Burton/net-pch_gbe-Fixes-MIPS-support/20180218-213023
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:33: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned short uid_hi @@ got short uid_hi @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:33: expected unsigned short uid_hi
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:33: got restricted __be16 <noident>
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:45: sparse: incorrect type in argument 3 (different base types) @@ expected unsigned int uid_lo @@ got ed int uid_lo @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:45: expected unsigned int uid_lo
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:45: got restricted __be32 <noident>
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:56: sparse: incorrect type in argument 4 (different base types) @@ expected unsigned short seqid @@ got short seqid @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:56: expected unsigned short seqid
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:56: got restricted __be16 <noident>
>> drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:325:15: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile @@ got @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:325:15: expected void const volatile
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:325:15: got void
>> drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:325:15: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile @@ got @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:325:15: expected void const volatile
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:325:15: got void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:354:33: sparse: incorrect type in argument 1 (different address spaces) @@ expected void @@ got unsigned int <avoid @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:354:33: expected void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:354:33: got unsigned int
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:361:33: sparse: incorrect type in argument 1 (different address spaces) @@ expected void @@ got unsigned int <avoid @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:361:33: expected void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:361:33: got unsigned int
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:389:33: sparse: incorrect type in argument 1 (different address spaces) @@ expected void @@ got unsigned int <avoid @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:389:33: expected void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:389:33: got unsigned int
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:430:33: sparse: incorrect type in argument 1 (different address spaces) @@ expected void @@ got unsigned int <avoid @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:430:33: expected void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:430:33: got unsigned int
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:463:49: sparse: incorrect type in argument 1 (different address spaces) @@ expected void @@ got unsigned int <avoid @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:463:49: expected void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:463:49: got unsigned int
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:537:41: sparse: incorrect type in argument 1 (different address spaces) @@ expected void @@ got unsigned int <avoid @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:537:41: expected void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:537:41: got unsigned int

vim +325 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c

   150	
   151	static void
   152	pch_rx_timestamp(struct pch_gbe_adapter *adapter, struct sk_buff *skb)
   153	{
   154		struct skb_shared_hwtstamps *shhwtstamps;
   155		struct pci_dev *pdev;
   156		u64 ns;
   157		u32 hi, lo, val;
   158		u16 uid, seq;
   159	
   160		if (!adapter->hwts_rx_en)
   161			return;
   162	
   163		/* Get ieee1588's dev information */
   164		pdev = adapter->ptp_pdev;
   165	
   166		val = pch_ch_event_read(pdev);
   167	
   168		if (!(val & RX_SNAPSHOT_LOCKED))
   169			return;
   170	
   171		lo = pch_src_uuid_lo_read(pdev);
   172		hi = pch_src_uuid_hi_read(pdev);
   173	
   174		uid = hi & 0xffff;
   175		seq = (hi >> 16) & 0xffff;
   176	
 > 177		if (!pch_ptp_match(skb, htons(uid), htonl(lo), htons(seq)))
   178			goto out;
   179	
   180		ns = pch_rx_snap_read(pdev);
   181	
   182		shhwtstamps = skb_hwtstamps(skb);
   183		memset(shhwtstamps, 0, sizeof(*shhwtstamps));
   184		shhwtstamps->hwtstamp = ns_to_ktime(ns);
   185	out:
   186		pch_ch_event_write(pdev, RX_SNAPSHOT_LOCKED);
   187	}
   188	
   189	static void
   190	pch_tx_timestamp(struct pch_gbe_adapter *adapter, struct sk_buff *skb)
   191	{
   192		struct skb_shared_hwtstamps shhwtstamps;
   193		struct pci_dev *pdev;
   194		struct skb_shared_info *shtx;
   195		u64 ns;
   196		u32 cnt, val;
   197	
   198		shtx = skb_shinfo(skb);
   199		if (likely(!(shtx->tx_flags & SKBTX_HW_TSTAMP && adapter->hwts_tx_en)))
   200			return;
   201	
   202		shtx->tx_flags |= SKBTX_IN_PROGRESS;
   203	
   204		/* Get ieee1588's dev information */
   205		pdev = adapter->ptp_pdev;
   206	
   207		/*
   208		 * This really stinks, but we have to poll for the Tx time stamp.
   209		 */
   210		for (cnt = 0; cnt < 100; cnt++) {
   211			val = pch_ch_event_read(pdev);
   212			if (val & TX_SNAPSHOT_LOCKED)
   213				break;
   214			udelay(1);
   215		}
   216		if (!(val & TX_SNAPSHOT_LOCKED)) {
   217			shtx->tx_flags &= ~SKBTX_IN_PROGRESS;
   218			return;
   219		}
   220	
   221		ns = pch_tx_snap_read(pdev);
   222	
   223		memset(&shhwtstamps, 0, sizeof(shhwtstamps));
   224		shhwtstamps.hwtstamp = ns_to_ktime(ns);
   225		skb_tstamp_tx(skb, &shhwtstamps);
   226	
   227		pch_ch_event_write(pdev, TX_SNAPSHOT_LOCKED);
   228	}
   229	
   230	static int hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
   231	{
   232		struct hwtstamp_config cfg;
   233		struct pch_gbe_adapter *adapter = netdev_priv(netdev);
   234		struct pci_dev *pdev;
   235		u8 station[20];
   236	
   237		if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
   238			return -EFAULT;
   239	
   240		if (cfg.flags) /* reserved for future extensions */
   241			return -EINVAL;
   242	
   243		/* Get ieee1588's dev information */
   244		pdev = adapter->ptp_pdev;
   245	
   246		if (cfg.tx_type != HWTSTAMP_TX_OFF && cfg.tx_type != HWTSTAMP_TX_ON)
   247			return -ERANGE;
   248	
   249		switch (cfg.rx_filter) {
   250		case HWTSTAMP_FILTER_NONE:
   251			adapter->hwts_rx_en = 0;
   252			break;
   253		case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
   254			adapter->hwts_rx_en = 0;
   255			pch_ch_control_write(pdev, SLAVE_MODE | CAP_MODE0);
   256			break;
   257		case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
   258			adapter->hwts_rx_en = 1;
   259			pch_ch_control_write(pdev, MASTER_MODE | CAP_MODE0);
   260			break;
   261		case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
   262			adapter->hwts_rx_en = 1;
   263			pch_ch_control_write(pdev, V2_MODE | CAP_MODE2);
   264			strcpy(station, PTP_L4_MULTICAST_SA);
   265			pch_set_station_address(station, pdev);
   266			break;
   267		case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
   268			adapter->hwts_rx_en = 1;
   269			pch_ch_control_write(pdev, V2_MODE | CAP_MODE2);
   270			strcpy(station, PTP_L2_MULTICAST_SA);
   271			pch_set_station_address(station, pdev);
   272			break;
   273		default:
   274			return -ERANGE;
   275		}
   276	
   277		adapter->hwts_tx_en = cfg.tx_type == HWTSTAMP_TX_ON;
   278	
   279		/* Clear out any old time stamps. */
   280		pch_ch_event_write(pdev, TX_SNAPSHOT_LOCKED | RX_SNAPSHOT_LOCKED);
   281	
   282		return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
   283	}
   284	
   285	static inline void pch_gbe_mac_load_mac_addr(struct pch_gbe_hw *hw)
   286	{
   287		iowrite32(0x01, &hw->reg->MAC_ADDR_LOAD);
   288	}
   289	
   290	/**
   291	 * pch_gbe_mac_read_mac_addr - Read MAC address
   292	 * @hw:	            Pointer to the HW structure
   293	 * Returns:
   294	 *	0:			Successful.
   295	 */
   296	s32 pch_gbe_mac_read_mac_addr(struct pch_gbe_hw *hw)
   297	{
   298		struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
   299		u32  adr1a, adr1b;
   300	
   301		adr1a = ioread32(&hw->reg->mac_adr[0].high);
   302		adr1b = ioread32(&hw->reg->mac_adr[0].low);
   303	
   304		hw->mac.addr[0] = (u8)(adr1a & 0xFF);
   305		hw->mac.addr[1] = (u8)((adr1a >> 8) & 0xFF);
   306		hw->mac.addr[2] = (u8)((adr1a >> 16) & 0xFF);
   307		hw->mac.addr[3] = (u8)((adr1a >> 24) & 0xFF);
   308		hw->mac.addr[4] = (u8)(adr1b & 0xFF);
   309		hw->mac.addr[5] = (u8)((adr1b >> 8) & 0xFF);
   310	
   311		netdev_dbg(adapter->netdev, "hw->mac.addr : %pM\n", hw->mac.addr);
   312		return 0;
   313	}
   314	
   315	/**
   316	 * pch_gbe_wait_clr_bit - Wait to clear a bit
   317	 * @reg:	Pointer of register
   318	 * @busy:	Busy bit
   319	 */
   320	static void pch_gbe_wait_clr_bit(void *reg, u32 bit)
   321	{
   322		int err;
   323		u32 tmp;
   324	
 > 325		err = readl_poll_timeout_atomic(reg, tmp, !(tmp & bit), 10, 25000);
   326		if (err)
   327			pr_err("Error: busy bit is not cleared\n");
   328	}
   329	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support
  2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
                   ` (13 preceding siblings ...)
  2018-02-17 20:10 ` [PATCH v5 14/14] net: pch_gbe: " Paul Burton
@ 2018-02-18 15:31 ` David Miller
  2018-02-18 17:03   ` Paul Burton
  14 siblings, 1 reply; 33+ messages in thread
From: David Miller @ 2018-02-18 15:31 UTC (permalink / raw)
  To: paul.burton; +Cc: netdev, hassan.naveed, matt.redfearn, linux-mips


Nobody is going to see and apply these patches if you don't CC: the
Linux networking development list, netdev@vger.kernel.org

Thank you.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 02/14] net: pch_gbe: Pull PHY GPIO handling out of Minnow code
  2018-02-17 23:34       ` Andrew Lunn
@ 2018-02-18 15:51         ` Paul Burton
  2018-02-18 16:14           ` Andrew Lunn
  0 siblings, 1 reply; 33+ messages in thread
From: Paul Burton @ 2018-02-18 15:51 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, Hassan Naveed, Matt Redfearn, David S . Miller,
	linux-mips

Hi Andrew,

On Sun, Feb 18, 2018 at 12:34:42AM +0100, Andrew Lunn wrote:
> > Even if that is true, rewriting the driver's PHY handling would be a
> > very separate change to the changes this series make which allow this
> > driver to work on a platform besides the Minnowboard. The *only* thing
> > this series does relating to the PHY is allow the reset GPIO to be
> > handled properly - rewriting the existing PHY handling is beyond it's
> > scope.
> 
> Well, you are adding a device tree binding, which needs to be
> supported forever. This is going to make things messy in the future
> when you do such a cleanup that you follow the PHY binding, in that
> you have to handle both what you add here, and the official PHY
> binding.

Thank you - it's useful to know what your concern actually is.

> I would prefer that for the moment, you drop the PHY binding patches
> in this series. That is what i object to the most. Adding an MDIO
> driver and using the standard PHY driver for this PHY is all
> internal. You can change that anytime. But adding a binding means an
> ABI.

The problem is that the device in question doesn't actually work unless
we reset the PHY, so just removing the PHY reset GPIO handling would
break things.

How would you feel if I were to adjust the binding to match the standard
PHY binding, but internally leave the driver's PHY handling as-is for
now? That would:

  1) Allow for the pch_gbe driver to move towards more standard PHY
     handling in the future without DT changes.

  2) Be fairly straightforward to implement in this patchset - the code
     reading the DT would just follow the phandle to the PHY node to
     find the reset GPIO - thereby not holding up the rest of the series.

  3) Still function on our hardware.

Thanks,
    Paul

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 12/14] net: pch_gbe: Fix TX RX descriptor accesses for big endian systems
  2018-02-17 20:10 ` [PATCH v5 12/14] net: pch_gbe: Fix TX RX descriptor accesses for big endian systems Paul Burton
@ 2018-02-18 16:01   ` kbuild test robot
  0 siblings, 0 replies; 33+ messages in thread
From: kbuild test robot @ 2018-02-18 16:01 UTC (permalink / raw)
  To: Paul Burton
  Cc: kbuild-all, netdev, Hassan Naveed, Matt Redfearn,
	David S . Miller, linux-mips, Paul Burton

Hi Hassan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on v4.16-rc1 next-20180216]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Burton/net-pch_gbe-Fixes-MIPS-support/20180218-213023
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:33: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned short uid_hi @@ got short uid_hi @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:33: expected unsigned short uid_hi
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:33: got restricted __be16 <noident>
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:45: sparse: incorrect type in argument 3 (different base types) @@ expected unsigned int uid_lo @@ got ed int uid_lo @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:45: expected unsigned int uid_lo
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:45: got restricted __be32 <noident>
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:56: sparse: incorrect type in argument 4 (different base types) @@ expected unsigned short seqid @@ got short seqid @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:56: expected unsigned short seqid
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:177:56: got restricted __be16 <noident>
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:325:15: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile @@ got @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:325:15: expected void const volatile
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:325:15: got void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:325:15: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile @@ got @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:325:15: expected void const volatile
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:325:15: got void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:354:33: sparse: incorrect type in argument 1 (different address spaces) @@ expected void @@ got unsigned int <avoid @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:354:33: expected void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:354:33: got unsigned int
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:361:33: sparse: incorrect type in argument 1 (different address spaces) @@ expected void @@ got unsigned int <avoid @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:361:33: expected void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:361:33: got unsigned int
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:389:33: sparse: incorrect type in argument 1 (different address spaces) @@ expected void @@ got unsigned int <avoid @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:389:33: expected void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:389:33: got unsigned int
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:430:33: sparse: incorrect type in argument 1 (different address spaces) @@ expected void @@ got unsigned int <avoid @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:430:33: expected void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:430:33: got unsigned int
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:463:49: sparse: incorrect type in argument 1 (different address spaces) @@ expected void @@ got unsigned int <avoid @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:463:49: expected void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:463:49: got unsigned int
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:537:41: sparse: incorrect type in argument 1 (different address spaces) @@ expected void @@ got unsigned int <avoid @@
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:537:41: expected void
   drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:537:41: got unsigned int
>> drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:1587:17: sparse: cast from restricted __le16
>> drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:1587:17: sparse: restricted __le16 degrades to integer

vim +1587 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c

  1527	
  1528	/**
  1529	 * pch_gbe_clean_tx - Reclaim resources after transmit completes
  1530	 * @adapter:   Board private structure
  1531	 * @tx_ring:   Tx descriptor ring
  1532	 * Returns:
  1533	 *	true:  Cleaned the descriptor
  1534	 *	false: Not cleaned the descriptor
  1535	 */
  1536	static bool
  1537	pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
  1538			 struct pch_gbe_tx_ring *tx_ring)
  1539	{
  1540		struct pch_gbe_tx_desc *tx_desc;
  1541		struct pch_gbe_buffer *buffer_info;
  1542		struct sk_buff *skb;
  1543		unsigned int i;
  1544		unsigned int cleaned_count = 0;
  1545		bool cleaned = false;
  1546		int unused, thresh;
  1547	
  1548		netdev_dbg(adapter->netdev, "next_to_clean : %d\n",
  1549			   tx_ring->next_to_clean);
  1550	
  1551		i = tx_ring->next_to_clean;
  1552		tx_desc = PCH_GBE_TX_DESC(*tx_ring, i);
  1553		netdev_dbg(adapter->netdev, "gbec_status:0x%04x  dma_status:0x%04x\n",
  1554			   le16_to_cpu(tx_desc->gbec_status), tx_desc->dma_status);
  1555	
  1556		unused = PCH_GBE_DESC_UNUSED(tx_ring);
  1557		thresh = tx_ring->count - PCH_GBE_TX_WEIGHT;
  1558		if ((le16_to_cpu(tx_desc->gbec_status) == DSC_INIT16) &&
  1559		    (unused < thresh))
  1560		{  /* current marked clean, tx queue filling up, do extra clean */
  1561			int j, k;
  1562			if (unused < 8) {  /* tx queue nearly full */
  1563				netdev_dbg(adapter->netdev,
  1564					   "clean_tx: transmit queue warning (%x,%x) unused=%d\n",
  1565					   tx_ring->next_to_clean, tx_ring->next_to_use,
  1566					   unused);
  1567			}
  1568	
  1569			/* current marked clean, scan for more that need cleaning. */
  1570			k = i;
  1571			for (j = 0; j < PCH_GBE_TX_WEIGHT; j++)
  1572			{
  1573				tx_desc = PCH_GBE_TX_DESC(*tx_ring, k);
  1574				if (le16_to_cpu(tx_desc->gbec_status) != DSC_INIT16)
  1575					break; /*found*/
  1576				if (++k >= tx_ring->count) k = 0;  /*increment, wrap*/
  1577			}
  1578			if (j < PCH_GBE_TX_WEIGHT) {
  1579				netdev_dbg(adapter->netdev,
  1580					   "clean_tx: unused=%d loops=%d found tx_desc[%x,%x:%x].gbec_status=%04x\n",
  1581					   unused, j, i, k, tx_ring->next_to_use,
  1582					   le16_to_cpu(tx_desc->gbec_status));
  1583				i = k;  /*found one to clean, usu gbec_status==2000.*/
  1584			}
  1585		}
  1586	
> 1587		while ((cpu_to_le16(tx_desc->gbec_status) & DSC_INIT16) == 0x0000) {
  1588			netdev_dbg(adapter->netdev, "gbec_status:0x%04x\n",
  1589				   le16_to_cpu(tx_desc->gbec_status));
  1590			buffer_info = &tx_ring->buffer_info[i];
  1591			skb = buffer_info->skb;
  1592			cleaned = true;
  1593	
  1594			if ((le16_to_cpu(tx_desc->gbec_status) &
  1595				PCH_GBE_TXD_GMAC_STAT_ABT)) {
  1596				adapter->stats.tx_aborted_errors++;
  1597				netdev_err(adapter->netdev, "Transfer Abort Error\n");
  1598			} else if ((le16_to_cpu(tx_desc->gbec_status) &
  1599					PCH_GBE_TXD_GMAC_STAT_CRSER)) {
  1600				adapter->stats.tx_carrier_errors++;
  1601				netdev_err(adapter->netdev,
  1602					   "Transfer Carrier Sense Error\n");
  1603			} else if ((le16_to_cpu(tx_desc->gbec_status) &
  1604						PCH_GBE_TXD_GMAC_STAT_EXCOL)) {
  1605				adapter->stats.tx_aborted_errors++;
  1606				netdev_err(adapter->netdev,
  1607					   "Transfer Collision Abort Error\n");
  1608			} else if ((le16_to_cpu(tx_desc->gbec_status) &
  1609				    (PCH_GBE_TXD_GMAC_STAT_SNGCOL |
  1610				     PCH_GBE_TXD_GMAC_STAT_MLTCOL))) {
  1611				adapter->stats.collisions++;
  1612				adapter->stats.tx_packets++;
  1613				adapter->stats.tx_bytes += skb->len;
  1614				netdev_dbg(adapter->netdev, "Transfer Collision\n");
  1615			} else if ((le16_to_cpu(tx_desc->gbec_status) &
  1616					PCH_GBE_TXD_GMAC_STAT_CMPLT)) {
  1617				adapter->stats.tx_packets++;
  1618				adapter->stats.tx_bytes += skb->len;
  1619			}
  1620			if (buffer_info->mapped) {
  1621				netdev_dbg(adapter->netdev,
  1622					   "unmap buffer_info->dma : %d\n", i);
  1623				dma_unmap_single(&adapter->pdev->dev, buffer_info->dma,
  1624						 buffer_info->length, DMA_TO_DEVICE);
  1625				buffer_info->mapped = false;
  1626			}
  1627			if (buffer_info->skb) {
  1628				netdev_dbg(adapter->netdev,
  1629					   "trim buffer_info->skb : %d\n", i);
  1630				skb_trim(buffer_info->skb, 0);
  1631			}
  1632			tx_desc->gbec_status = cpu_to_le16(DSC_INIT16);
  1633			if (unlikely(++i == tx_ring->count))
  1634				i = 0;
  1635			tx_desc = PCH_GBE_TX_DESC(*tx_ring, i);
  1636	
  1637			/* weight of a sort for tx, to avoid endless transmit cleanup */
  1638			if (cleaned_count++ == PCH_GBE_TX_WEIGHT) {
  1639				cleaned = false;
  1640				break;
  1641			}
  1642		}
  1643		netdev_dbg(adapter->netdev,
  1644			   "called pch_gbe_unmap_and_free_tx_resource() %d count\n",
  1645			   cleaned_count);
  1646		if (cleaned_count > 0)  { /*skip this if nothing cleaned*/
  1647			/* Recover from running out of Tx resources in xmit_frame */
  1648			netif_tx_lock(adapter->netdev);
  1649			if (unlikely(cleaned && (netif_queue_stopped(adapter->netdev))))
  1650			{
  1651				netif_wake_queue(adapter->netdev);
  1652				adapter->stats.tx_restart_count++;
  1653				netdev_dbg(adapter->netdev, "Tx wake queue\n");
  1654			}
  1655	
  1656			tx_ring->next_to_clean = i;
  1657	
  1658			netdev_dbg(adapter->netdev, "next_to_clean : %d\n",
  1659				   tx_ring->next_to_clean);
  1660			netif_tx_unlock(adapter->netdev);
  1661		}
  1662		return cleaned;
  1663	}
  1664	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 02/14] net: pch_gbe: Pull PHY GPIO handling out of Minnow code
  2018-02-18 15:51         ` Paul Burton
@ 2018-02-18 16:14           ` Andrew Lunn
  0 siblings, 0 replies; 33+ messages in thread
From: Andrew Lunn @ 2018-02-18 16:14 UTC (permalink / raw)
  To: Paul Burton
  Cc: netdev, Hassan Naveed, Matt Redfearn, David S . Miller,
	linux-mips

> How would you feel if I were to adjust the binding to match the standard
> PHY binding, but internally leave the driver's PHY handling as-is for
> now?

Hi Paul

That is a reasonable compromise.

Thanks

     Andrew

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support
  2018-02-18 15:31 ` [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support David Miller
@ 2018-02-18 17:03   ` Paul Burton
  2018-02-18 17:56     ` Andrew Lunn
  2018-02-19  1:15     ` David Miller
  0 siblings, 2 replies; 33+ messages in thread
From: Paul Burton @ 2018-02-18 17:03 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, hassan.naveed, matt.redfearn, linux-mips

Hi David,

On Sun, Feb 18, 2018 at 10:31:12AM -0500, David Miller wrote:
> Nobody is going to see and apply these patches if you don't CC: the
> Linux networking development list, netdev@vger.kernel.org

You're replying to mail that was "To: netdev@vger.kernel.org" and I see
the whole series in the archives[1] so it definitely reached the list.

I'm not sure I see the problem?

Thanks,
    Paul

[1] https://www.spinics.net/lists/netdev/msg484102.html

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support
  2018-02-18 17:03   ` Paul Burton
@ 2018-02-18 17:56     ` Andrew Lunn
  2018-02-18 22:09       ` Paul Burton
  2018-02-19  1:15     ` David Miller
  1 sibling, 1 reply; 33+ messages in thread
From: Andrew Lunn @ 2018-02-18 17:56 UTC (permalink / raw)
  To: Paul Burton
  Cc: David Miller, netdev, hassan.naveed, matt.redfearn, linux-mips

On Sun, Feb 18, 2018 at 09:03:10AM -0800, Paul Burton wrote:
> Hi David,
> 
> On Sun, Feb 18, 2018 at 10:31:12AM -0500, David Miller wrote:
> > Nobody is going to see and apply these patches if you don't CC: the
> > Linux networking development list, netdev@vger.kernel.org
> 
> You're replying to mail that was "To: netdev@vger.kernel.org" and I see
> the whole series in the archives[1] so it definitely reached the list.
> 
> I'm not sure I see the problem?

Hi Paul

I'm guess that David is wondering about version 1-4 of this patchset?
As far as i can see, they were sent to the mips list, not the netdev
list.

	Andrew

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support
  2018-02-18 17:56     ` Andrew Lunn
@ 2018-02-18 22:09       ` Paul Burton
  0 siblings, 0 replies; 33+ messages in thread
From: Paul Burton @ 2018-02-18 22:09 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David Miller, netdev, hassan.naveed, matt.redfearn, linux-mips

Hi Andrew,

On Sun, Feb 18, 2018 at 06:56:07PM +0100, Andrew Lunn wrote:
> On Sun, Feb 18, 2018 at 09:03:10AM -0800, Paul Burton wrote:
> > Hi David,
> > 
> > On Sun, Feb 18, 2018 at 10:31:12AM -0500, David Miller wrote:
> > > Nobody is going to see and apply these patches if you don't CC: the
> > > Linux networking development list, netdev@vger.kernel.org
> > 
> > You're replying to mail that was "To: netdev@vger.kernel.org" and I see
> > the whole series in the archives[1] so it definitely reached the list.
> > 
> > I'm not sure I see the problem?
> 
> Hi Paul
> 
> I'm guess that David is wondering about version 1-4 of this patchset?
> As far as i can see, they were sent to the mips list, not the netdev
> list.

It has been quite a while since v4, but it and earlier revisions were
submitted to the netdev list too:

v4: https://www.spinics.net/lists/netdev/msg438550.html
v3: https://www.spinics.net/lists/netdev/msg438313.html
v2: https://marc.info/?l=linux-netdev&m=145450117711515&w=2

v1 was part of a larger series, but netdev was also copied on the
relevant patches starting here & the patches following it:

v1: https://marc.info/?l=linux-netdev&m=144890083511222&w=2

Thanks,
    Paul

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support
  2018-02-18 17:03   ` Paul Burton
  2018-02-18 17:56     ` Andrew Lunn
@ 2018-02-19  1:15     ` David Miller
  2018-02-19  2:17       ` Florian Fainelli
  1 sibling, 1 reply; 33+ messages in thread
From: David Miller @ 2018-02-19  1:15 UTC (permalink / raw)
  To: paul.burton; +Cc: netdev, hassan.naveed, matt.redfearn, linux-mips

From: Paul Burton <paul.burton@mips.com>
Date: Sun, 18 Feb 2018 09:03:10 -0800

> Hi David,
> 
> On Sun, Feb 18, 2018 at 10:31:12AM -0500, David Miller wrote:
>> Nobody is going to see and apply these patches if you don't CC: the
>> Linux networking development list, netdev@vger.kernel.org
> 
> You're replying to mail that was "To: netdev@vger.kernel.org" and I see
> the whole series in the archives[1] so it definitely reached the list.
> 
> I'm not sure I see the problem?

Sorry.

The issue is that your patch series didn't make it into patchwork
properly, I wonder what happened since you did send it to netdev.

Hmmm...

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support
  2018-02-19  1:15     ` David Miller
@ 2018-02-19  2:17       ` Florian Fainelli
  0 siblings, 0 replies; 33+ messages in thread
From: Florian Fainelli @ 2018-02-19  2:17 UTC (permalink / raw)
  To: David Miller, paul.burton
  Cc: netdev, hassan.naveed, matt.redfearn, linux-mips



On 02/18/2018 05:15 PM, David Miller wrote:
> From: Paul Burton <paul.burton@mips.com>
> Date: Sun, 18 Feb 2018 09:03:10 -0800
> 
>> Hi David,
>>
>> On Sun, Feb 18, 2018 at 10:31:12AM -0500, David Miller wrote:
>>> Nobody is going to see and apply these patches if you don't CC: the
>>> Linux networking development list, netdev@vger.kernel.org
>>
>> You're replying to mail that was "To: netdev@vger.kernel.org" and I see
>> the whole series in the archives[1] so it definitely reached the list.
>>
>> I'm not sure I see the problem?
> 
> Sorry.
> 
> The issue is that your patch series didn't make it into patchwork
> properly, I wonder what happened since you did send it to netdev.
> 
> Hmmm...

The guys at buildroot seem to have seen a number of their patches not
making it to patchwork, thread starts here:

http://buildroot-busybox.2317881.n4.nabble.com/patchwork-ozlabs-org-down-and-e-mails-not-recorded-td183918.html
-- 
Florian

^ permalink raw reply	[flat|nested] 33+ messages in thread

* RE: [PATCH v5 07/14] net: pch_gbe: Fix handling of TX padding
  2018-02-17 20:10 ` [PATCH v5 07/14] net: pch_gbe: Fix handling of TX padding Paul Burton
@ 2018-02-19 14:01   ` David Laight
  2018-02-19 16:42     ` Paul Burton
  0 siblings, 1 reply; 33+ messages in thread
From: David Laight @ 2018-02-19 14:01 UTC (permalink / raw)
  To: 'Paul Burton', netdev@vger.kernel.org
  Cc: Hassan Naveed, Matt Redfearn, David S . Miller,
	linux-mips@linux-mips.org

From: Paul Burton
> Sent: 17 February 2018 20:11
> 
> The ethernet controller found in the Intel EG20T Platform Controller
> Hub requires that we place 2 bytes of padding between the ethernet
> header & the packet payload. Our pch_gbe driver handles this by copying
> packets to be transmitted to a temporary struct skb with the padding
> bytes inserted
...

Uggg WFT is the driver doing that for?

I'd guess that the two byte pad is there so that a 4 byte aligned
frame is still 4 byte aligned when the 14 byte ethernet header is added.
So instead of copying the entire frame the MAC header should be built
(or rebuilt?) two bytes further from the actual data.

	David

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 07/14] net: pch_gbe: Fix handling of TX padding
  2018-02-19 16:42     ` Paul Burton
@ 2018-02-19 16:41       ` David Miller
  0 siblings, 0 replies; 33+ messages in thread
From: David Miller @ 2018-02-19 16:41 UTC (permalink / raw)
  To: paul.burton
  Cc: David.Laight, netdev, hassan.naveed, matt.redfearn, linux-mips

From: Paul Burton <paul.burton@mips.com>
Date: Mon, 19 Feb 2018 08:42:23 -0800

> So whilst I totally agree that copying around the whole frame is awful,
> it's a separate problem to the length used for DMA mapping being
> incorrect which is what this patch addresses & I'd rather not start
> adding more & more fixes or cleanups into this initial series before the
> driver is even functional on my hardware.

Agreed.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 07/14] net: pch_gbe: Fix handling of TX padding
  2018-02-19 14:01   ` David Laight
@ 2018-02-19 16:42     ` Paul Burton
  2018-02-19 16:41       ` David Miller
  0 siblings, 1 reply; 33+ messages in thread
From: Paul Burton @ 2018-02-19 16:42 UTC (permalink / raw)
  To: David Laight
  Cc: netdev@vger.kernel.org, Hassan Naveed, Matt Redfearn,
	David S . Miller, linux-mips@linux-mips.org

Hi David,

On Mon, Feb 19, 2018 at 02:01:25PM +0000, David Laight wrote:
> From: Paul Burton
> > Sent: 17 February 2018 20:11
> > 
> > The ethernet controller found in the Intel EG20T Platform Controller
> > Hub requires that we place 2 bytes of padding between the ethernet
> > header & the packet payload. Our pch_gbe driver handles this by copying
> > packets to be transmitted to a temporary struct skb with the padding
> > bytes inserted
> ...
> 
> Uggg WFT is the driver doing that for?
> 
> I'd guess that the two byte pad is there so that a 4 byte aligned
> frame is still 4 byte aligned when the 14 byte ethernet header is added.
> So instead of copying the entire frame the MAC header should be built
> (or rebuilt?) two bytes further from the actual data.

I agree - the pch_gbe driver is pretty bad and does a lot of things
wrong. Frankly I'm amazed it's in tree, but it is & one patch series
isn't going to fix all of its shortcomings.

So whilst I totally agree that copying around the whole frame is awful,
it's a separate problem to the length used for DMA mapping being
incorrect which is what this patch addresses & I'd rather not start
adding more & more fixes or cleanups into this initial series before the
driver is even functional on my hardware.

Thanks,
    Paul

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2018-02-19 16:41 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-17 20:10 [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support Paul Burton
2018-02-17 20:10 ` [PATCH v5 01/14] net: pch_gbe: Mark Minnow PHY reset GPIO active low Paul Burton
2018-02-17 22:14   ` Andrew Lunn
2018-02-17 20:10 ` [PATCH v5 02/14] net: pch_gbe: Pull PHY GPIO handling out of Minnow code Paul Burton
2018-02-17 22:29   ` Andrew Lunn
2018-02-17 22:50     ` Paul Burton
2018-02-17 23:34       ` Andrew Lunn
2018-02-18 15:51         ` Paul Burton
2018-02-18 16:14           ` Andrew Lunn
2018-02-17 20:10 ` [PATCH v5 03/14] dt-bindings: net: Document Intel pch_gbe binding Paul Burton
     [not found]   ` <20180217201037.3006-4-paul.burton-8NJIiSa5LzA@public.gmane.org>
2018-02-17 22:32     ` Andrew Lunn
2018-02-17 20:10 ` [PATCH v5 04/14] net: pch_gbe: Add device tree support Paul Burton
2018-02-17 20:10 ` [PATCH v5 05/14] net: pch_gbe: Always reset PHY along with MAC Paul Burton
2018-02-17 20:10 ` [PATCH v5 06/14] net: pch_gbe: Allow longer for resets Paul Burton
2018-02-18 15:29   ` kbuild test robot
2018-02-17 20:10 ` [PATCH v5 07/14] net: pch_gbe: Fix handling of TX padding Paul Burton
2018-02-19 14:01   ` David Laight
2018-02-19 16:42     ` Paul Burton
2018-02-19 16:41       ` David Miller
2018-02-17 20:10 ` [PATCH v5 08/14] net: pch_gbe: Fold pch_gbe_setup_[rt]ctl into pch_gbe_configure_[rt]x Paul Burton
2018-02-17 20:10 ` [PATCH v5 09/14] net: pch_gbe: Use pch_gbe_disable_dma_rx() in pch_gbe_configure_rx() Paul Burton
2018-02-17 20:10 ` [PATCH v5 10/14] net: pch_gbe: Disable TX DMA whilst configuring descriptors Paul Burton
2018-02-17 20:10 ` [PATCH v5 11/14] net: pch_gbe: Ensure DMA is ordered with descriptor writes Paul Burton
2018-02-17 20:10 ` [PATCH v5 12/14] net: pch_gbe: Fix TX RX descriptor accesses for big endian systems Paul Burton
2018-02-18 16:01   ` kbuild test robot
2018-02-17 20:10 ` [PATCH v5 13/14] ptp: pch: Allow build on MIPS platforms Paul Burton
2018-02-17 20:10 ` [PATCH v5 14/14] net: pch_gbe: " Paul Burton
2018-02-18 15:31 ` [PATCH v5 00/14] net: pch_gbe: Fixes & MIPS support David Miller
2018-02-18 17:03   ` Paul Burton
2018-02-18 17:56     ` Andrew Lunn
2018-02-18 22:09       ` Paul Burton
2018-02-19  1:15     ` David Miller
2018-02-19  2:17       ` Florian Fainelli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).