From: Paul Burton <hidden> Date: 2017-06-02 23:41:03
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.12-rc3.
Paul Burton (7):
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: 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 | 4 +-
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 79 +++++++++++++++++-----
4 files changed, 91 insertions(+), 19 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/pch_gbe.txt
--
2.13.0
From: Paul Burton <hidden> Date: 2017-06-02 23:41:52
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 <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>
Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---
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(-)
From: Paul Burton <hidden> Date: 2017-06-02 23:42:14
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 <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>
Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---
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 | 4 ++-
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 33 +++++++++++++++-------
2 files changed, 26 insertions(+), 11 deletions(-)
@@ -2601,7 +2611,14 @@ static int pch_gbe_probe(struct pci_dev *pdev,adapter->hw.reg=pcim_iomap_table(pdev)[PCH_GBE_PCI_BAR];adapter->pdata=(structpch_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_bus_and_slot(adapter->pdev->bus->number,PCI_DEVFN(12,4));
@@ -2694,7 +2711,8 @@ static int pch_gbe_probe(struct pci_dev *pdev,/* The AR803X PHY on the MinnowBoard requires a physical pin to be toggled to*ensureitisawakeforprobeandinit.RequestthelineandresetthePHY.*/-staticintpch_gbe_minnow_platform_init(structpci_dev*pdev)+staticintpch_gbe_minnow_platform_init(structpci_dev*pdev,+structpch_gbe_privdata*pdata){unsignedlongflags=GPIOF_DIR_OUT|GPIOF_INIT_LOW|GPIOF_EXPORT|GPIOF_ACTIVE_LOW;
@@ -2703,16 +2721,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);+elsedev_err(&pdev->dev,"ERR: Can't request PHY reset GPIO line '%d'\n",gpio);-returnret;-}--gpio_set_value(gpio,1);-usleep_range(1250,1500);-gpio_set_value(gpio,0);-usleep_range(1250,1500);returnret;}
From: Paul Burton <hidden> Date: 2017-06-02 23:42:50
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 <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>
Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---
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(-)
From: Paul Burton <hidden> Date: 2017-06-02 23:43:08
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 <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>
Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---
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(+)
@@ -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_RGMIIiowrite32(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);
From: Paul Burton <hidden> Date: 2017-06-02 23:43:26
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 <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>
Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---
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(-)
From: Paul Burton <hidden> Date: 2017-06-02 23:43:44
Allow the pch_gbe driver to be built on MIPS platforms, in preparation
for its use on the MIPS Boston board.
Signed-off-by: Paul Burton <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>
Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---
Changes in v3: None
Changes in v2: None
drivers/net/ethernet/oki-semi/pch_gbe/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:2587:9-16: WARNING: ERR_CAST can be used with gpio
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
From: kbuild test robot <hidden> Date: 2017-06-03 06:36:53
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:2587:9-16: WARNING: ERR_CAST can be used with gpio
Use ERR_CAST inlined function instead of ERR_PTR(PTR_ERR(...))
Generated by: scripts/coccinelle/api/err_cast.cocci
CC: Paul Burton <redacted>
Signed-off-by: Fengguang Wu <redacted>
---
pch_gbe_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Andrew Lunn <andrew@lunn.ch> Date: 2017-06-03 17:52:06
On Fri, Jun 02, 2017 at 04:40:37PM -0700, Paul Burton wrote:
quoted hunk
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 <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>
Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---
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 | 4 ++-
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 33 +++++++++++++++-------
2 files changed, 26 insertions(+), 11 deletions(-)
Hi Paul
Since you are using the gpiod_ API, the core will take notice of the
active low/active high flag when performing this set.
quoted hunk
+}+ /** * pch_gbe_mac_reset_hw - Reset hardware * @hw: Pointer to the HW structure ret = devm_gpio_request_one(&pdev->dev, gpio, flags, "minnow_phy_reset");- if (ret) {+ if (!ret)+ pdata->phy_reset_gpio = gpio_to_desc(gpio);
Here however, you are using the gpio_ API, which ignores the active
high/low flag in device tree. And in your binding patch, you give the
example:
+ phy-reset-gpios = <&eg20t_gpio 6
+ GPIO_ACTIVE_LOW>;
This active low is totally ignored.
I personally would say this is all messed up, and going to result in
problems for somebody with a board which actually needs an
GPIO_ACTIVE_HIGH.
Please use the gpiod_ API through out and respect the flags in the
device tree binding.
Andrew
Hi Paul
Since you are using the gpiod_ API, the core will take notice of the
active low/active high flag when performing this set.
Correct, and as desired.
quoted
ret = devm_gpio_request_one(&pdev->dev, gpio, flags, "minnow_phy_reset");- if (ret) {+ if (!ret)+ pdata->phy_reset_gpio = gpio_to_desc(gpio);
Here however, you are using the gpio_ API, which ignores the active
high/low flag in device tree. And in your binding patch, you give the
example:
+ phy-reset-gpios = <&eg20t_gpio 6
+ GPIO_ACTIVE_LOW>;
This active low is totally ignored.
First of all, this path is for the existing Minnow platform, which doesn't use
the device tree. That is, this code is the non-DT path so looking at what
happens to flags in the device tree here makes no sense.
If you want to examine what happens in the DT case then please look at
pch_gbe_get_priv() which uses devm_gpiod_get() which should honor the flags
provided by the DT.
I personally would say this is all messed up, and going to result in
problems for somebody with a board which actually needs an
GPIO_ACTIVE_HIGH.
It's a path which only applies to the Minnow board, which is always active
low. Before patch 1 of this series that was done without the GPIOF_ACTIVE_LOW
flag by setting GPIO values to reflect the physical GPIO line low/high rather
than the logical active/not-active. After patch 1 this path began using
GPIOF_ACTIVE_LOW such that the rest of the code can use logical active/not-
active values which work with either active low or active high GPIOs. In this
Minnow-specific path GPIOF_ACTIVE_LOW is hardcoded, but again only applies to
the Minnow board which doesn't take the GPIO value from device tree.
Please use the gpiod_ API through out and respect the flags in the
device tree binding.
The gpiod_ API, quite rightly, retrieves GPIOs associated with a device - for
example via the device tree. The Minnow board, which is what the driver
already supports in-tree, does not do this but instead hardcodes a GPIO number
(MINNOW_PHY_RESET_GPIO). I don't own, use or care about the Minnow platform so
that is not something that I can change. In the path that my patch does add,
the path which is used with DT, I already do use the gpiod_ API & respect
flags from the DT.
Thanks,
Paul
From: Paul Burton <hidden> Date: 2017-06-05 17:32:12
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.12-rc4.
Paul Burton (7):
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: 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 | 4 +-
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 79 +++++++++++++++++-----
4 files changed, 91 insertions(+), 19 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/pch_gbe.txt
--
2.13.0
From: Paul Burton <hidden> Date: 2017-06-05 17:32:32
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 <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>
Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---
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(-)
From: Paul Burton <hidden> Date: 2017-06-05 17:32:50
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 <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>
Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---
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 | 4 ++-
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 33 +++++++++++++++-------
2 files changed, 26 insertions(+), 11 deletions(-)
@@ -2601,7 +2611,14 @@ static int pch_gbe_probe(struct pci_dev *pdev,adapter->hw.reg=pcim_iomap_table(pdev)[PCH_GBE_PCI_BAR];adapter->pdata=(structpch_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_bus_and_slot(adapter->pdev->bus->number,PCI_DEVFN(12,4));
@@ -2694,7 +2711,8 @@ static int pch_gbe_probe(struct pci_dev *pdev,/* The AR803X PHY on the MinnowBoard requires a physical pin to be toggled to*ensureitisawakeforprobeandinit.RequestthelineandresetthePHY.*/-staticintpch_gbe_minnow_platform_init(structpci_dev*pdev)+staticintpch_gbe_minnow_platform_init(structpci_dev*pdev,+structpch_gbe_privdata*pdata){unsignedlongflags=GPIOF_DIR_OUT|GPIOF_INIT_LOW|GPIOF_EXPORT|GPIOF_ACTIVE_LOW;
@@ -2703,16 +2721,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);+elsedev_err(&pdev->dev,"ERR: Can't request PHY reset GPIO line '%d'\n",gpio);-returnret;-}--gpio_set_value(gpio,1);-usleep_range(1250,1500);-gpio_set_value(gpio,0);-usleep_range(1250,1500);returnret;}
From: Paul Burton <hidden> Date: 2017-06-05 17:33:20
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 <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>
Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---
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(-)
From: Paul Burton <hidden> Date: 2017-06-05 17:33:44
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 <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>
Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---
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(+)
@@ -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_RGMIIiowrite32(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);
From: Paul Burton <hidden> Date: 2017-06-05 17:34:07
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 <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>
Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---
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(-)
From: Paul Burton <hidden> Date: 2017-06-05 17:34:15
Allow the pch_gbe driver to be built on MIPS platforms, in preparation
for its use on the MIPS Boston board.
Signed-off-by: Paul Burton <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>
Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: linux-mips@linux-mips.org
Cc: netdev@vger.kernel.org
---
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(-)
Hi Paul
Since you are using the gpiod_ API, the core will take notice of the
active low/active high flag when performing this set.
Correct, and as desired.
quoted
quoted
ret = devm_gpio_request_one(&pdev->dev, gpio, flags, "minnow_phy_reset");- if (ret) {+ if (!ret)+ pdata->phy_reset_gpio = gpio_to_desc(gpio);
Here however, you are using the gpio_ API, which ignores the active
high/low flag in device tree. And in your binding patch, you give the
example:
+ phy-reset-gpios = <&eg20t_gpio 6
+ GPIO_ACTIVE_LOW>;
This active low is totally ignored.
First of all, this path is for the existing Minnow platform, which doesn't use
the device tree. That is, this code is the non-DT path so looking at what
happens to flags in the device tree here makes no sense.
Thanks for the explanation. Now it makes sense.
Andrew
It is possible to enable CONFIG_OF on all architectures, including x86
used by Minnow. If somebody was to do this, i think Minnow breaks. What
i think you really want is:
if pci_id->driver_data;
return (struct pch_gbe_privdata *)pci_id->driver_data;
quoted hunk
++ 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;+}
There should not be a need to protect for !CONFIG_OF, and
devm_gpiod_get() knows how to look in ACPI tables, if an intel or
ARM64 platform it using that to list its GPIOs.
Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: 2017-06-05 18:55:18
On Mon, Jun 05, 2017 at 10:31:31AM -0700, 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.
Signed-off-by: Paul Burton <redacted>
From: Marcin Nowakowski <hidden> Date: 2017-06-06 14:10:14
Hi Paul,
On 03.06.2017 01:40, Paul Burton wrote:
quoted hunk
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.
#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, 500);+ if (err) pr_err("Error: busy bit is not cleared\n"); }
This new timeout value appears to be too low - I'm seeing plenty of timeout warnings now and ultimately the device fails to initialise:
[ 7.541876] pch_gbe: EG20T PCH Gigabit Ethernet Driver - version 1.01
[ 7.566451] pch_gbe: Error: busy bit is not cleared
[ 7.572654] pch_gbe: Error: busy bit is not cleared
[ 7.578727] pch_gbe: Error: busy bit is not cleared
[ 7.587814] pch_gbe 0000:02:00.1: Invalid MAC address, interface disabled.
[ 7.595605] pch_gbe 0000:02:00.1: MAC address : 00:00:00:00:00:00
[ 7.606451] pch_gbe: Error: busy bit is not cleared
[ 7.612572] pch_gbe: Error: busy bit is not cleared
[ 7.618618] pch_gbe: Error: busy bit is not cleared
<...>
[ 10.063351] pch_gbe 0000:02:00.1 eth0: Error: Invalid MAC address
[ 10.074713] pch_gbe: Error: busy bit is not cleared
[ 10.081030] pch_gbe: Error: busy bit is not cleared
[ 10.087178] pch_gbe: Error: busy bit is not cleared
[ 10.093328] pch_gbe: Error: busy bit is not cleared
[ 10.100883] pch_gbe 0000:02:00.1 eth0: Error End
[ 10.106272] IP-Config: Failed to open eth0
My tests show that a timeout value as big as 20000 may be required to make it work reliably ...
Marcin