[PATCH net-next v5 1/2] ixgbe: register a mdiobus

2018-12-06 Thread Steve Douthit
Most dsa devices expect a 'struct mii_bus' pointer to talk to switches
via the MII interface.

While this works for dsa devices, it will not work safely with Linux
PHYs in all configurations since the firmware of the ixgbe device may
be polling some PHY addresses in the background.

Signed-off-by: Stephen Douthit 
Reviewed-by: Andrew Lunn 
Reviewed-by: Florian Fainelli 
Tested-by: Andrew Bowers 
---
 drivers/net/ethernet/intel/Kconfig|   1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe.h  |   2 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   5 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 299 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |   2 +
 5 files changed, 309 insertions(+)

diff --git a/drivers/net/ethernet/intel/Kconfig 
b/drivers/net/ethernet/intel/Kconfig
index 59e1bc0f609e..096cf3bc76a0 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -159,6 +159,7 @@ config IXGBE
tristate "Intel(R) 10GbE PCI Express adapters support"
depends on PCI
select MDIO
+   select MDIO_DEVICE
imply PTP_1588_CLOCK
---help---
  This driver supports Intel(R) 10GbE PCI Express family of
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h 
b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 143bdd5ee2a0..08d85e336bd4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -561,6 +562,7 @@ struct ixgbe_adapter {
struct net_device *netdev;
struct bpf_prog *xdp_prog;
struct pci_dev *pdev;
+   struct mii_bus *mii_bus;
 
unsigned long state;
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index b57fa0cfb222..b7907674c565 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -39,6 +39,7 @@
 #include "ixgbe.h"
 #include "ixgbe_common.h"
 #include "ixgbe_dcb_82599.h"
+#include "ixgbe_phy.h"
 #include "ixgbe_sriov.h"
 #include "ixgbe_model.h"
 #include "ixgbe_txrx_common.h"
@@ -11120,6 +11121,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent)
IXGBE_LINK_SPEED_10GB_FULL | IXGBE_LINK_SPEED_1GB_FULL,
true);
 
+   ixgbe_mii_bus_init(hw);
+
return 0;
 
 err_register:
@@ -11170,6 +11173,8 @@ static void ixgbe_remove(struct pci_dev *pdev)
set_bit(__IXGBE_REMOVING, >state);
cancel_work_sync(>service_task);
 
+   if (adapter->mii_bus)
+   mdiobus_unregister(adapter->mii_bus);
 
 #ifdef CONFIG_IXGBE_DCA
if (adapter->flags & IXGBE_FLAG_DCA_ENABLED) {
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
index 919a7af84b42..cc4907f9ff02 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
@@ -3,6 +3,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include "ixgbe.h"
@@ -658,6 +659,304 @@ s32 ixgbe_write_phy_reg_generic(struct ixgbe_hw *hw, u32 
reg_addr,
return status;
 }
 
+#define IXGBE_HW_READ_REG(addr) IXGBE_READ_REG(hw, addr)
+
+/**
+ *  ixgbe_msca_cmd - Write the command register and poll for completion/timeout
+ *  @hw: pointer to hardware structure
+ *  @cmd: command register value to write
+ **/
+static s32 ixgbe_msca_cmd(struct ixgbe_hw *hw, u32 cmd)
+{
+   IXGBE_WRITE_REG(hw, IXGBE_MSCA, cmd);
+
+   return readx_poll_timeout(IXGBE_HW_READ_REG, IXGBE_MSCA, cmd,
+ !(cmd & IXGBE_MSCA_MDI_COMMAND), 10,
+ 10 * IXGBE_MDIO_COMMAND_TIMEOUT);
+}
+
+/**
+ *  ixgbe_mii_bus_read_generic - Read a clause 22/45 register with gssr flags
+ *  @hw: pointer to hardware structure
+ *  @addr: address
+ *  @regnum: register number
+ *  @gssr: semaphore flags to acquire
+ **/
+static s32 ixgbe_mii_bus_read_generic(struct ixgbe_hw *hw, int addr,
+ int regnum, u32 gssr)
+{
+   u32 hwaddr, cmd;
+   s32 data;
+
+   if (hw->mac.ops.acquire_swfw_sync(hw, gssr))
+   return -EBUSY;
+
+   hwaddr = addr << IXGBE_MSCA_PHY_ADDR_SHIFT;
+   if (regnum & MII_ADDR_C45) {
+   hwaddr |= regnum & GENMASK(21, 0);
+   cmd = hwaddr | IXGBE_MSCA_ADDR_CYCLE | IXGBE_MSCA_MDI_COMMAND;
+   } else {
+   hwaddr |= (regnum & GENMASK(5, 0)) << IXGBE_MSCA_DEV_TYPE_SHIFT;
+   cmd = hwaddr | IXGBE_MSCA_OLD_PROTOCOL |
+   IXGBE_MSCA_READ_AUTOINC | IXGBE_MSCA_MDI_COMMAND;
+   }
+
+   data = ixgbe_msca_cmd(hw, cmd);
+   if (data < 0)
+   goto mii_bus_read_done;
+
+   /* For a clause 45 access the address cycle just completed, we still
+* need to do the read command, otherwise just 

[PATCH net-next v5 2/2] ixgbe: use mii_bus to handle MII related ioctls

2018-12-06 Thread Steve Douthit
Use the mii_bus callbacks to address the entire clause 22/45 address
space.  Enables userspace to poke switch registers instead of a single
PHY address.

The ixgbe firmware may be polling PHYs in a way that is not protected by
the mii_bus lock.  This isn't new behavior, but as Andrew Lunn pointed
out there are more addresses available for conflicts.

Signed-off-by: Stephen Douthit 
Reviewed-by: Andrew Lunn 
Reviewed-by: Florian Fainelli 
Tested-by: Andrew Bowers 
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index b7907674c565..caa64afdedfb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8790,6 +8790,15 @@ ixgbe_mdio_read(struct net_device *netdev, int prtad, 
int devad, u16 addr)
u16 value;
int rc;
 
+   if (adapter->mii_bus) {
+   int regnum = addr;
+
+   if (devad != MDIO_DEVAD_NONE)
+   regnum |= (devad << 16) | MII_ADDR_C45;
+
+   return mdiobus_read(adapter->mii_bus, prtad, regnum);
+   }
+
if (prtad != hw->phy.mdio.prtad)
return -EINVAL;
rc = hw->phy.ops.read_reg(hw, addr, devad, );
@@ -8804,6 +8813,15 @@ static int ixgbe_mdio_write(struct net_device *netdev, 
int prtad, int devad,
struct ixgbe_adapter *adapter = netdev_priv(netdev);
struct ixgbe_hw *hw = >hw;
 
+   if (adapter->mii_bus) {
+   int regnum = addr;
+
+   if (devad != MDIO_DEVAD_NONE)
+   regnum |= (devad << 16) | MII_ADDR_C45;
+
+   return mdiobus_write(adapter->mii_bus, prtad, regnum, value);
+   }
+
if (prtad != hw->phy.mdio.prtad)
return -EINVAL;
return hw->phy.ops.write_reg(hw, addr, devad, value);
-- 
2.17.2



[PATCH net-next v5 0/2] Add mii_bus to ixgbe driver for dsa devs

2018-12-06 Thread Steve Douthit
Changes from v4 -> v5
* select MDIO_DEVICE instead of MII in Kconfig
* Collect Tested-by: tags

Changes from v3 -> v4
* Remove unecessary pointer casts
* Fix copy/paste issues in comments
* Simplify setting of swfw semaphore flags
* Collect Reviewed-by: tags

Changes from v2 -> v3
* Added warnings about interactions between this code and PHY polling
  unit to the commit messages.

Changes from v1 -> v2
[PATCH 1/2] ixgbe: register a mdiobus
* Add intel-wired-...@lists.osuosl.org to CC list, see
* select MII in Kconfig (thanks to the kbuild bot)
* Only call mdiobus_regsiter for single x500em_a device
* Use readx_poll_timeout() in ixgbe_msca_cmd()
* Register different bus->read/write callbacks in ixgbe_mii_bus_init()
  so there's device id check on every access
* Use device pci_name() in bus->id instead of parent bridge's name

[PATCH 2/2] ixgbe: use mii_bus to handle MII related ioctls
* Only use mdiobus_read/write for adapters that registered a mdiobus

Stephen Douthit (2):
  ixgbe: register a mdiobus
  ixgbe: use mii_bus to handle MII related ioctls

 drivers/net/ethernet/intel/Kconfig|   1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe.h  |   2 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  23 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 307 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |   2 +
 5 files changed, 335 insertions(+)

-- 
2.17.2



Re: [PATCH net-next v4 0/2] Add mii_bus to ixgbe driver for dsa devs

2018-12-04 Thread Steve Douthit
On 12/3/18 6:46 PM, Florian Fainelli wrote:
> Yes, we have been discussing that topic with Andrew and have a few ideas
> on how that could be achieved, but not code to use that at the moment.
> One of the idea was to finally allow enslaving the DSA master network
> device, that way you could assign specific ports to a specific CPU/DSA
> master network port within the switch, though that requires setting up a
> bridge. Would that work for your use case?

If bridge here means a port based vlan, then yes that works.  If we're
talking about something that shows up under brctl, then that's less
ideal.

It's probably time to split the thread for any further discussion since
this isn't really relevant to intel-wired-lan anymore.


Re: [PATCH net-next v4 0/2] Add mii_bus to ixgbe driver for dsa devs

2018-12-03 Thread Steve Douthit
> Not directly related to this patch series, are you using the legacy or
> "new" way of passing platform data in order to register the DSA switch?
> Since you mentioned 6390, I would assume this must be the "new" way of
> registering DSA devices with mdio_boardinfo etc. In that case, have you
> found yourself limited by the current dsa_chip_platform_data?

With the new method I had an off-by-one error where the 'enabled_ports'
bitmask and 'port_names' array didn't match up in my first attempt.
That's definitely my fault, but the loop that searches for the "cpu"
string didn't like it and segfaulted.

I've got something of a chicken-and-the-egg problem between where I'm
deferring while waiting for netdevs to show up, and registering the
mdio_board_info that needs those same pointers.  For testing I exported
mdiobus_setup_mdiodev_from_board_info() and mdiobus_create_device() so I
could call the setup function again when the data was actually ready.

It'd be nice to have more than one "cpu" port on a switch and have some
way to associate downstream and "cpu" ports.  Not sure exactly what that
would look like, and it's not something I'm going to try and tackle at
the moment, but it's one for the wish list.


[PATCH net-next v4 0/2] Add mii_bus to ixgbe driver for dsa devs

2018-12-03 Thread Steve Douthit
Changes from v3 -> v4
* Remove unecessary pointer casts
* Fix copy/paste issues in comments
* Simplify setting of swfw semaphore flags
* Collect Reviewed-by: tags

Changes from v2 -> v3
* Added warnings about interactions between this code and PHY polling
  unit to the commit messages.

Changes from v1 -> v2
[PATCH 1/2] ixgbe: register a mdiobus
* Add intel-wired-...@lists.osuosl.org to CC list, see
* select MII in Kconfig (thanks to the kbuild bot)
* Only call mdiobus_regsiter for single x500em_a device
* Use readx_poll_timeout() in ixgbe_msca_cmd()
* Register different bus->read/write callbacks in ixgbe_mii_bus_init()
  so there's device id check on every access
* Use device pci_name() in bus->id instead of parent bridge's name

[PATCH 2/2] ixgbe: use mii_bus to handle MII related ioctls
* Only use mdiobus_read/write for adapters that registered a mdiobus

Stephen Douthit (2):
  ixgbe: register a mdiobus
  ixgbe: use mii_bus to handle MII related ioctls

 drivers/net/ethernet/intel/Kconfig|   1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe.h  |   2 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  23 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 307 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |   2 +
 5 files changed, 335 insertions(+)

-- 
2.17.2



[PATCH net-next v4 1/2] ixgbe: register a mdiobus

2018-12-03 Thread Steve Douthit
Most dsa devices expect a 'struct mii_bus' pointer to talk to switches
via the MII interface.

While this works for dsa devices, it will not work safely with Linux
PHYs in all configurations since the firmware of the ixgbe device may
be polling some PHY addresses in the background.

Signed-off-by: Stephen Douthit 
Reviewed-by: Andrew Lunn 
Reviewed-by: Florian Fainelli 
---
 drivers/net/ethernet/intel/Kconfig|   1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe.h  |   2 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   5 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 299 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |   2 +
 5 files changed, 309 insertions(+)

diff --git a/drivers/net/ethernet/intel/Kconfig 
b/drivers/net/ethernet/intel/Kconfig
index 59e1bc0f609e..79ad2c5860f0 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -159,6 +159,7 @@ config IXGBE
tristate "Intel(R) 10GbE PCI Express adapters support"
depends on PCI
select MDIO
+   select MII
imply PTP_1588_CLOCK
---help---
  This driver supports Intel(R) 10GbE PCI Express family of
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h 
b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 143bdd5ee2a0..08d85e336bd4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -561,6 +562,7 @@ struct ixgbe_adapter {
struct net_device *netdev;
struct bpf_prog *xdp_prog;
struct pci_dev *pdev;
+   struct mii_bus *mii_bus;
 
unsigned long state;
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index b57fa0cfb222..b7907674c565 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -39,6 +39,7 @@
 #include "ixgbe.h"
 #include "ixgbe_common.h"
 #include "ixgbe_dcb_82599.h"
+#include "ixgbe_phy.h"
 #include "ixgbe_sriov.h"
 #include "ixgbe_model.h"
 #include "ixgbe_txrx_common.h"
@@ -11120,6 +11121,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent)
IXGBE_LINK_SPEED_10GB_FULL | IXGBE_LINK_SPEED_1GB_FULL,
true);
 
+   ixgbe_mii_bus_init(hw);
+
return 0;
 
 err_register:
@@ -11170,6 +11173,8 @@ static void ixgbe_remove(struct pci_dev *pdev)
set_bit(__IXGBE_REMOVING, >state);
cancel_work_sync(>service_task);
 
+   if (adapter->mii_bus)
+   mdiobus_unregister(adapter->mii_bus);
 
 #ifdef CONFIG_IXGBE_DCA
if (adapter->flags & IXGBE_FLAG_DCA_ENABLED) {
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
index 919a7af84b42..cc4907f9ff02 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
@@ -3,6 +3,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include "ixgbe.h"
@@ -658,6 +659,304 @@ s32 ixgbe_write_phy_reg_generic(struct ixgbe_hw *hw, u32 
reg_addr,
return status;
 }
 
+#define IXGBE_HW_READ_REG(addr) IXGBE_READ_REG(hw, addr)
+
+/**
+ *  ixgbe_msca_cmd - Write the command register and poll for completion/timeout
+ *  @hw: pointer to hardware structure
+ *  @cmd: command register value to write
+ **/
+static s32 ixgbe_msca_cmd(struct ixgbe_hw *hw, u32 cmd)
+{
+   IXGBE_WRITE_REG(hw, IXGBE_MSCA, cmd);
+
+   return readx_poll_timeout(IXGBE_HW_READ_REG, IXGBE_MSCA, cmd,
+ !(cmd & IXGBE_MSCA_MDI_COMMAND), 10,
+ 10 * IXGBE_MDIO_COMMAND_TIMEOUT);
+}
+
+/**
+ *  ixgbe_mii_bus_read_generic - Read a clause 22/45 register with gssr flags
+ *  @hw: pointer to hardware structure
+ *  @addr: address
+ *  @regnum: register number
+ *  @gssr: semaphore flags to acquire
+ **/
+static s32 ixgbe_mii_bus_read_generic(struct ixgbe_hw *hw, int addr,
+ int regnum, u32 gssr)
+{
+   u32 hwaddr, cmd;
+   s32 data;
+
+   if (hw->mac.ops.acquire_swfw_sync(hw, gssr))
+   return -EBUSY;
+
+   hwaddr = addr << IXGBE_MSCA_PHY_ADDR_SHIFT;
+   if (regnum & MII_ADDR_C45) {
+   hwaddr |= regnum & GENMASK(21, 0);
+   cmd = hwaddr | IXGBE_MSCA_ADDR_CYCLE | IXGBE_MSCA_MDI_COMMAND;
+   } else {
+   hwaddr |= (regnum & GENMASK(5, 0)) << IXGBE_MSCA_DEV_TYPE_SHIFT;
+   cmd = hwaddr | IXGBE_MSCA_OLD_PROTOCOL |
+   IXGBE_MSCA_READ_AUTOINC | IXGBE_MSCA_MDI_COMMAND;
+   }
+
+   data = ixgbe_msca_cmd(hw, cmd);
+   if (data < 0)
+   goto mii_bus_read_done;
+
+   /* For a clause 45 access the address cycle just completed, we still
+* need to do the read command, otherwise just get the data
+*/
+   

[PATCH net-next v4 2/2] ixgbe: use mii_bus to handle MII related ioctls

2018-12-03 Thread Steve Douthit
Use the mii_bus callbacks to address the entire clause 22/45 address
space.  Enables userspace to poke switch registers instead of a single
PHY address.

The ixgbe firmware may be polling PHYs in a way that is not protected by
the mii_bus lock.  This isn't new behavior, but as Andrew Lunn pointed
out there are more addresses available for conflicts.

Signed-off-by: Stephen Douthit 
Reviewed-by: Andrew Lunn 
Reviewed-by: Florian Fainelli 
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index b7907674c565..caa64afdedfb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8790,6 +8790,15 @@ ixgbe_mdio_read(struct net_device *netdev, int prtad, 
int devad, u16 addr)
u16 value;
int rc;
 
+   if (adapter->mii_bus) {
+   int regnum = addr;
+
+   if (devad != MDIO_DEVAD_NONE)
+   regnum |= (devad << 16) | MII_ADDR_C45;
+
+   return mdiobus_read(adapter->mii_bus, prtad, regnum);
+   }
+
if (prtad != hw->phy.mdio.prtad)
return -EINVAL;
rc = hw->phy.ops.read_reg(hw, addr, devad, );
@@ -8804,6 +8813,15 @@ static int ixgbe_mdio_write(struct net_device *netdev, 
int prtad, int devad,
struct ixgbe_adapter *adapter = netdev_priv(netdev);
struct ixgbe_hw *hw = >hw;
 
+   if (adapter->mii_bus) {
+   int regnum = addr;
+
+   if (devad != MDIO_DEVAD_NONE)
+   regnum |= (devad << 16) | MII_ADDR_C45;
+
+   return mdiobus_write(adapter->mii_bus, prtad, regnum, value);
+   }
+
if (prtad != hw->phy.mdio.prtad)
return -EINVAL;
return hw->phy.ops.write_reg(hw, addr, devad, value);
-- 
2.17.2



Re: [PATCH net-next v3 1/2] ixgbe: register a mdiobus

2018-12-03 Thread Steve Douthit
On 12/3/18 2:07 PM, Florian Fainelli wrote:
> On 12/3/18 10:55 AM, Steve Douthit wrote:
>> Most dsa devices expect a 'struct mii_bus' pointer to talk to switches
>> via the MII interface.
>>
>> While this works for dsa devices, it will not work safely with Linux
>> PHYs in all configurations since the firmware of the ixgbe device may
>> be polling some PHY addresses in the background.
>>
>> Signed-off-by: Stephen Douthit 
>> ---
> 
> [snip]
> 
>> +/**
>> + *  ixgbe_mii_bus_write - Write a clause 22/45 register
>> + *  @hw: pointer to hardware structure
>> + *  @addr: address
>> + *  @regnum: register number
>> + *  @regnum: valueto write
> 
> This should be @val to match the function parameters

OK

>> + **/
>> +static s32 ixgbe_mii_bus_write(struct mii_bus *bus, int addr, int regnum,
>> +   u16 val)
>> +{
>> +struct ixgbe_adapter *adapter = (struct ixgbe_adapter *)bus->priv;
> 
> Nitpick: cast is not necessary since this is a void * pointer (for that
> reason).

OK, I'll clean up this and other unnecessary casts.

I forgot Andrew's suggestion to squash the swfw semaphore masks from:

+   u32 gssr = hw->phy.phy_semaphore_mask | IXGBE_GSSR_TOKEN_SM;
+
+   if (hw->bus.lan_id)
+   gssr |= IXGBE_GSSR_PHY1_SM;
+   else
+   gssr |= IXGBE_GSSR_PHY0_SM;

to

+   u32 gssr = hw->phy.phy_semaphore_mask;
+   gssr |= IXGBE_GSSR_TOKEN_SM | IXGBE_GSSR_PHY0_SM;

Is it ok to collect both of your 'Reviewed-by:'s with that additional
change for v4?


[PATCH net-next v3 2/2] ixgbe: use mii_bus to handle MII related ioctls

2018-12-03 Thread Steve Douthit
Use the mii_bus callbacks to address the entire clause 22/45 address
space.  Enables userspace to poke switch registers instead of a single
PHY address.

The ixgbe firmware may be polling PHYs in a way that is not protected by
the mii_bus lock.  This isn't new behavior, but as Andrew Lunn pointed
out there are more addresses available for conflicts.

Signed-off-by: Stephen Douthit 
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 82af3b24d222..fb066c491abe 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8790,6 +8790,15 @@ ixgbe_mdio_read(struct net_device *netdev, int prtad, 
int devad, u16 addr)
u16 value;
int rc;
 
+   if (adapter->mii_bus) {
+   int regnum = addr;
+
+   if (devad != MDIO_DEVAD_NONE)
+   regnum |= (devad << 16) | MII_ADDR_C45;
+
+   return mdiobus_read(adapter->mii_bus, prtad, regnum);
+   }
+
if (prtad != hw->phy.mdio.prtad)
return -EINVAL;
rc = hw->phy.ops.read_reg(hw, addr, devad, );
@@ -8804,6 +8813,15 @@ static int ixgbe_mdio_write(struct net_device *netdev, 
int prtad, int devad,
struct ixgbe_adapter *adapter = netdev_priv(netdev);
struct ixgbe_hw *hw = >hw;
 
+   if (adapter->mii_bus) {
+   int regnum = addr;
+
+   if (devad != MDIO_DEVAD_NONE)
+   regnum |= (devad << 16) | MII_ADDR_C45;
+
+   return mdiobus_write(adapter->mii_bus, prtad, regnum, value);
+   }
+
if (prtad != hw->phy.mdio.prtad)
return -EINVAL;
return hw->phy.ops.write_reg(hw, addr, devad, value);
-- 
2.17.2



[PATCH net-next v3 1/2] ixgbe: register a mdiobus

2018-12-03 Thread Steve Douthit
Most dsa devices expect a 'struct mii_bus' pointer to talk to switches
via the MII interface.

While this works for dsa devices, it will not work safely with Linux
PHYs in all configurations since the firmware of the ixgbe device may
be polling some PHY addresses in the background.

Signed-off-by: Stephen Douthit 
---
 drivers/net/ethernet/intel/Kconfig|   1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe.h  |   2 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   5 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 307 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |   2 +
 5 files changed, 317 insertions(+)

diff --git a/drivers/net/ethernet/intel/Kconfig 
b/drivers/net/ethernet/intel/Kconfig
index 59e1bc0f609e..79ad2c5860f0 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -159,6 +159,7 @@ config IXGBE
tristate "Intel(R) 10GbE PCI Express adapters support"
depends on PCI
select MDIO
+   select MII
imply PTP_1588_CLOCK
---help---
  This driver supports Intel(R) 10GbE PCI Express family of
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h 
b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 143bdd5ee2a0..08d85e336bd4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -561,6 +562,7 @@ struct ixgbe_adapter {
struct net_device *netdev;
struct bpf_prog *xdp_prog;
struct pci_dev *pdev;
+   struct mii_bus *mii_bus;
 
unsigned long state;
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 49a4ea38eb07..82af3b24d222 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -39,6 +39,7 @@
 #include "ixgbe.h"
 #include "ixgbe_common.h"
 #include "ixgbe_dcb_82599.h"
+#include "ixgbe_phy.h"
 #include "ixgbe_sriov.h"
 #include "ixgbe_model.h"
 #include "ixgbe_txrx_common.h"
@@ -11120,6 +11121,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent)
IXGBE_LINK_SPEED_10GB_FULL | IXGBE_LINK_SPEED_1GB_FULL,
true);
 
+   ixgbe_mii_bus_init(hw);
+
return 0;
 
 err_register:
@@ -11170,6 +11173,8 @@ static void ixgbe_remove(struct pci_dev *pdev)
set_bit(__IXGBE_REMOVING, >state);
cancel_work_sync(>service_task);
 
+   if (adapter->mii_bus)
+   mdiobus_unregister(adapter->mii_bus);
 
 #ifdef CONFIG_IXGBE_DCA
if (adapter->flags & IXGBE_FLAG_DCA_ENABLED) {
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
index 919a7af84b42..ccc19edaaf9c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
@@ -3,6 +3,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include "ixgbe.h"
@@ -658,6 +659,312 @@ s32 ixgbe_write_phy_reg_generic(struct ixgbe_hw *hw, u32 
reg_addr,
return status;
 }
 
+#define IXGBE_HW_READ_REG(addr) IXGBE_READ_REG(hw, addr)
+
+/**
+ *  ixgbe_msca_cmd - Write the command register and poll for completion/timeout
+ *  @hw: pointer to hardware structure
+ *  @cmd: command register value to write
+ **/
+static s32 ixgbe_msca_cmd(struct ixgbe_hw *hw, u32 cmd)
+{
+   IXGBE_WRITE_REG(hw, IXGBE_MSCA, cmd);
+
+   return readx_poll_timeout(IXGBE_HW_READ_REG, IXGBE_MSCA, cmd,
+ !(cmd & IXGBE_MSCA_MDI_COMMAND), 10,
+ 10 * IXGBE_MDIO_COMMAND_TIMEOUT);
+}
+
+/**
+ *  ixgbe_mii_bus_read_generic - Read a clause 22/45 register with gssr flags
+ *  @hw: pointer to hardware structure
+ *  @addr: address
+ *  @regnum: register number
+ *  @gssr: semaphore flags to acquire
+ **/
+static s32 ixgbe_mii_bus_read_generic(struct ixgbe_hw *hw, int addr,
+ int regnum, u32 gssr)
+{
+   u32 hwaddr, cmd;
+   s32 data;
+
+   if (hw->mac.ops.acquire_swfw_sync(hw, gssr))
+   return -EBUSY;
+
+   hwaddr = addr << IXGBE_MSCA_PHY_ADDR_SHIFT;
+   if (regnum & MII_ADDR_C45) {
+   hwaddr |= regnum & GENMASK(21, 0);
+   cmd = hwaddr | IXGBE_MSCA_ADDR_CYCLE | IXGBE_MSCA_MDI_COMMAND;
+   } else {
+   hwaddr |= (regnum & GENMASK(5, 0)) << IXGBE_MSCA_DEV_TYPE_SHIFT;
+   cmd = hwaddr | IXGBE_MSCA_OLD_PROTOCOL |
+   IXGBE_MSCA_READ_AUTOINC | IXGBE_MSCA_MDI_COMMAND;
+   }
+
+   data = ixgbe_msca_cmd(hw, cmd);
+   if (data < 0)
+   goto mii_bus_read_done;
+
+   /* For a clause 45 access the address cycle just completed, we still
+* need to do the read command, otherwise just get the data
+*/
+   if (!(regnum & MII_ADDR_C45))
+   goto 

[PATCH net-next v3 0/2] Add mii_bus to ixgbe driver for dsa devs

2018-12-03 Thread Steve Douthit
Changes from v2 -> v3
* Added warnings about interactions between this code and PHY polling
  unit to the commit messages.

Changes from v1 -> v2
[PATCH 1/2] ixgbe: register a mdiobus
* Add intel-wired-...@lists.osuosl.org to CC list, see
* select MII in Kconfig (thanks to the kbuild bot)
* Only call mdiobus_regsiter for single x500em_a device
* Use readx_poll_timeout() in ixgbe_msca_cmd()
* Register different bus->read/write callbacks in ixgbe_mii_bus_init()
  so there's device id check on every access
* Use device pci_name() in bus->id instead of parent bridge's name

[PATCH 2/2] ixgbe: use mii_bus to handle MII related ioctls
* Only use mdiobus_read/write for adapters that registered a mdiobus

Stephen Douthit (2):
  ixgbe: register a mdiobus
  ixgbe: use mii_bus to handle MII related ioctls

 drivers/net/ethernet/intel/Kconfig|   1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe.h  |   2 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  23 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 307 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |   2 +
 5 files changed, 335 insertions(+)

-- 
2.17.2



Re: [PATCH net-next v2 1/2] ixgbe: register a mdiobus

2018-12-03 Thread Steve Douthit
On 12/3/18 1:18 PM, Andrew Lunn wrote:
>> Agreed, but I'd argue it's the same behavior we have today with the
>> existing MII ioctls in this driver.  That's not to say this is good,
>> it's just not any less broken than the current state of things.
> 
> Agreed.
> 
> I actually would be happy with a warning in the commit message that
> this code is not sufficient to make use of Linux PHY drivers, because
> of the hardware polling. You can then leave fixing that to whoever
> needs Linux PHY drivers.

OK, will update for v3.

>> AFAICT the polling hardware only pokes the device address that the
>> driver stores in 'hw->phy.mdio.prtad', so the PHY polling unit would
>> never see the switch, if it's even polling at all.  Some of the MAC
>> configurations will store MDIO_PRTAD_NONE, in which case I wouldn't
>> expect the polling unit to be active.  It's up to the board designer to
>> ensure there's no address conflicts on the bus.
> 
> Well, the 6390 does use address 0-10 for its port registers, and there
> is something which looks like a PHYID in register 3. So for your use
> case of DSA, it would be good to ensure it really is disabled.

You can actually strap the 6390 and friends for a multi-chip mode where
they claim only a single address, instead of one per port, plus a couple
more for global registers.  It vastly slows things down because of the
extra indirection, but it allows the switch to play nicely with other
MDIO devs.

>> Then in the ioctl code, in addition to checking the mii_bus is
>> available, also check that the requested address is one that phy_mask
>> says mii_bus owns, otherwise we fall through to the old code.
> 
> I'm not too bothered with the ioctl. It is there so you can shoot
> yourself in the foot. The hardware polling unit just adds more
> interesting weapons you can use to shoot yourself in the foot.

Hooray for enhanced anti-foot artillery :)


Re: [PATCH net-next v2 1/2] ixgbe: register a mdiobus

2018-12-03 Thread Steve Douthit
On 12/3/18 12:21 PM, Andrew Lunn wrote:
> On Mon, Dec 03, 2018 at 05:02:40PM +0000, Steve Douthit wrote:
>> On 12/3/18 11:54 AM, Andrew Lunn wrote:
>>>> +static s32 ixgbe_x550em_a_mii_bus_read(struct mii_bus *bus, int addr,
>>>> + int regnum)
>>>> +{
>>>> +  struct ixgbe_adapter *adapter = (struct ixgbe_adapter *)bus->priv;
>>>> +  struct ixgbe_hw *hw = >hw;
>>>> +  u32 gssr = hw->phy.phy_semaphore_mask | IXGBE_GSSR_TOKEN_SM;
>>>> +
>>>> +  if (hw->bus.lan_id)
>>>> +  gssr |= IXGBE_GSSR_PHY1_SM;
>>>> +  else
>>>> +  gssr |= IXGBE_GSSR_PHY0_SM;
>>>
>>> Hi Steve
>>>
>>> If you only have one bus, do you still need this? One semaphore is all
>>> you need. And i'm not even sure you need that. The MDIO layer will
>>> perform locking, assuming everything goes through the MDIO layer.
>>
>> AFAIK I still need part of it.  There's a PHY polling unit in the card
>> that we need to sync with independent of the locking in the MDIO layer.
>> I can drop the hw->bus.lan_id check though and unconditionally OR in the
>> IXGBE_GSSR_PHY0_SM since we always register on function 0 now.
> 
> Hi Steve
> 
> In general, this is not enough to make a PHY polling unit safe. At
> least not with C22 devices. They often have a register which can be
> used to select a different page. The software driver can do a write to
> swap the page at any time, e.g. to select the page with the
> temperature sensor. After it releases the semaphore for that write
> changing the page, the hardware could then poll the PHY, and read a
> temperature sensor register instead of the link status, and then bad
> things happen.
> 
> When using Intel's PHY drivers embedded within the Intel MAC driver,
> this is not a problem. They can avoid swapping to different
> pages. However, you are on the path to allow the Linux PHY drivers to
> be used, and some of them will change the page. And with the embedded
> SOC you are working on, i would not be too surprised to see somebody
> build a system using a different PHY which the Intel PHY drivers don't
> support, but does have a Linux PHY driver.

Agreed, but I'd argue it's the same behavior we have today with the
existing MII ioctls in this driver.  That's not to say this is good,
it's just not any less broken than the current state of things.  I don't
know what the scope of the locking is for the software/firmware
semaphore on the firmware side.  Maybe someone from Intel has more
details on the locking that would help find a clever locking solution?

> You also need to watch out for your use case of Marvell
> mv88e6390. Polling that makes no sense. Does the hardware actually
> recognise it is not a PHY?

AFAICT the polling hardware only pokes the device address that the
driver stores in 'hw->phy.mdio.prtad', so the PHY polling unit would
never see the switch, if it's even polling at all.  Some of the MAC
configurations will store MDIO_PRTAD_NONE, in which case I wouldn't
expect the polling unit to be active.  It's up to the board designer to
ensure there's no address conflicts on the bus.

I'm making some assumptions about the inner workings of the hardware
here, so someone from Intel would need to confirm those or set me
straight.

How about if I keep track of which PHY addresses the driver wants for
the x550em_a devices, then clear those bits from the phy_mask instead of
+   bus->phy_mask = GENMASK(31, 0);

Then in the ioctl code, in addition to checking the mii_bus is
available, also check that the requested address is one that phy_mask
says mii_bus owns, otherwise we fall through to the old code.

I think that keeps the dsa and PHY polling as separate as it is today
on the ioctl side, and hides the PHY(s) that the ixgbe driver wants to
manage exclusively from the mii_bus altogether.

Let me know what you think.  It'll make the probing code a bit more
complicated, but I think it addresses your concerns.


Re: [PATCH net-next v2 1/2] ixgbe: register a mdiobus

2018-12-03 Thread Steve Douthit
On 12/3/18 11:54 AM, Andrew Lunn wrote:
>> +static s32 ixgbe_x550em_a_mii_bus_read(struct mii_bus *bus, int addr,
>> +   int regnum)
>> +{
>> +struct ixgbe_adapter *adapter = (struct ixgbe_adapter *)bus->priv;
>> +struct ixgbe_hw *hw = >hw;
>> +u32 gssr = hw->phy.phy_semaphore_mask | IXGBE_GSSR_TOKEN_SM;
>> +
>> +if (hw->bus.lan_id)
>> +gssr |= IXGBE_GSSR_PHY1_SM;
>> +else
>> +gssr |= IXGBE_GSSR_PHY0_SM;
> 
> Hi Steve
> 
> If you only have one bus, do you still need this? One semaphore is all
> you need. And i'm not even sure you need that. The MDIO layer will
> perform locking, assuming everything goes through the MDIO layer.

AFAIK I still need part of it.  There's a PHY polling unit in the card
that we need to sync with independent of the locking in the MDIO layer.
I can drop the hw->bus.lan_id check though and unconditionally OR in the
IXGBE_GSSR_PHY0_SM since we always register on function 0 now.


[PATCH net-next v2 1/2] ixgbe: register a mdiobus

2018-12-03 Thread Steve Douthit
Most dsa devices expect a 'struct mii_bus' pointer to talk to switches
via the MII interface.

Signed-off-by: Stephen Douthit 
---
 drivers/net/ethernet/intel/Kconfig|   1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe.h  |   2 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   5 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 307 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |   2 +
 5 files changed, 317 insertions(+)

diff --git a/drivers/net/ethernet/intel/Kconfig 
b/drivers/net/ethernet/intel/Kconfig
index 59e1bc0f609e..79ad2c5860f0 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -159,6 +159,7 @@ config IXGBE
tristate "Intel(R) 10GbE PCI Express adapters support"
depends on PCI
select MDIO
+   select MII
imply PTP_1588_CLOCK
---help---
  This driver supports Intel(R) 10GbE PCI Express family of
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h 
b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 143bdd5ee2a0..08d85e336bd4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -561,6 +562,7 @@ struct ixgbe_adapter {
struct net_device *netdev;
struct bpf_prog *xdp_prog;
struct pci_dev *pdev;
+   struct mii_bus *mii_bus;
 
unsigned long state;
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 49a4ea38eb07..82af3b24d222 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -39,6 +39,7 @@
 #include "ixgbe.h"
 #include "ixgbe_common.h"
 #include "ixgbe_dcb_82599.h"
+#include "ixgbe_phy.h"
 #include "ixgbe_sriov.h"
 #include "ixgbe_model.h"
 #include "ixgbe_txrx_common.h"
@@ -11120,6 +11121,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent)
IXGBE_LINK_SPEED_10GB_FULL | IXGBE_LINK_SPEED_1GB_FULL,
true);
 
+   ixgbe_mii_bus_init(hw);
+
return 0;
 
 err_register:
@@ -11170,6 +11173,8 @@ static void ixgbe_remove(struct pci_dev *pdev)
set_bit(__IXGBE_REMOVING, >state);
cancel_work_sync(>service_task);
 
+   if (adapter->mii_bus)
+   mdiobus_unregister(adapter->mii_bus);
 
 #ifdef CONFIG_IXGBE_DCA
if (adapter->flags & IXGBE_FLAG_DCA_ENABLED) {
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
index 919a7af84b42..ccc19edaaf9c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
@@ -3,6 +3,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include "ixgbe.h"
@@ -658,6 +659,312 @@ s32 ixgbe_write_phy_reg_generic(struct ixgbe_hw *hw, u32 
reg_addr,
return status;
 }
 
+#define IXGBE_HW_READ_REG(addr) IXGBE_READ_REG(hw, addr)
+
+/**
+ *  ixgbe_msca_cmd - Write the command register and poll for completion/timeout
+ *  @hw: pointer to hardware structure
+ *  @cmd: command register value to write
+ **/
+static s32 ixgbe_msca_cmd(struct ixgbe_hw *hw, u32 cmd)
+{
+   IXGBE_WRITE_REG(hw, IXGBE_MSCA, cmd);
+
+   return readx_poll_timeout(IXGBE_HW_READ_REG, IXGBE_MSCA, cmd,
+ !(cmd & IXGBE_MSCA_MDI_COMMAND), 10,
+ 10 * IXGBE_MDIO_COMMAND_TIMEOUT);
+}
+
+/**
+ *  ixgbe_mii_bus_read_generic - Read a clause 22/45 register with gssr flags
+ *  @hw: pointer to hardware structure
+ *  @addr: address
+ *  @regnum: register number
+ *  @gssr: semaphore flags to acquire
+ **/
+static s32 ixgbe_mii_bus_read_generic(struct ixgbe_hw *hw, int addr,
+ int regnum, u32 gssr)
+{
+   u32 hwaddr, cmd;
+   s32 data;
+
+   if (hw->mac.ops.acquire_swfw_sync(hw, gssr))
+   return -EBUSY;
+
+   hwaddr = addr << IXGBE_MSCA_PHY_ADDR_SHIFT;
+   if (regnum & MII_ADDR_C45) {
+   hwaddr |= regnum & GENMASK(21, 0);
+   cmd = hwaddr | IXGBE_MSCA_ADDR_CYCLE | IXGBE_MSCA_MDI_COMMAND;
+   } else {
+   hwaddr |= (regnum & GENMASK(5, 0)) << IXGBE_MSCA_DEV_TYPE_SHIFT;
+   cmd = hwaddr | IXGBE_MSCA_OLD_PROTOCOL |
+   IXGBE_MSCA_READ_AUTOINC | IXGBE_MSCA_MDI_COMMAND;
+   }
+
+   data = ixgbe_msca_cmd(hw, cmd);
+   if (data < 0)
+   goto mii_bus_read_done;
+
+   /* For a clause 45 access the address cycle just completed, we still
+* need to do the read command, otherwise just get the data
+*/
+   if (!(regnum & MII_ADDR_C45))
+   goto do_mii_bus_read;
+
+   cmd = hwaddr | IXGBE_MSCA_READ | IXGBE_MSCA_MDI_COMMAND;
+   data = ixgbe_msca_cmd(hw, cmd);
+   if (data < 0)
+   goto mii_bus_read_done;
+

[PATCH net-next v2 2/2] ixgbe: use mii_bus to handle MII related ioctls

2018-12-03 Thread Steve Douthit
Use the mii_bus callbacks to address the entire clause 22/45 address
space.  Enables userspace to poke switch registers instead of a single
PHY address.

Signed-off-by: Stephen Douthit 
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 82af3b24d222..fb066c491abe 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8790,6 +8790,15 @@ ixgbe_mdio_read(struct net_device *netdev, int prtad, 
int devad, u16 addr)
u16 value;
int rc;
 
+   if (adapter->mii_bus) {
+   int regnum = addr;
+
+   if (devad != MDIO_DEVAD_NONE)
+   regnum |= (devad << 16) | MII_ADDR_C45;
+
+   return mdiobus_read(adapter->mii_bus, prtad, regnum);
+   }
+
if (prtad != hw->phy.mdio.prtad)
return -EINVAL;
rc = hw->phy.ops.read_reg(hw, addr, devad, );
@@ -8804,6 +8813,15 @@ static int ixgbe_mdio_write(struct net_device *netdev, 
int prtad, int devad,
struct ixgbe_adapter *adapter = netdev_priv(netdev);
struct ixgbe_hw *hw = >hw;
 
+   if (adapter->mii_bus) {
+   int regnum = addr;
+
+   if (devad != MDIO_DEVAD_NONE)
+   regnum |= (devad << 16) | MII_ADDR_C45;
+
+   return mdiobus_write(adapter->mii_bus, prtad, regnum, value);
+   }
+
if (prtad != hw->phy.mdio.prtad)
return -EINVAL;
return hw->phy.ops.write_reg(hw, addr, devad, value);
-- 
2.17.2



[PATCH net-next v2 0/2] Add mii_bus to ixgbe driver for dsa devs

2018-12-03 Thread Steve Douthit
Address review comments from v1.  I missed the intel-wired-lan list on
the first submission, so check out the netdev list archive for the first
rev of the patch.

I think it's still up in the air on how best to register a single bus
that's shared among up to four MACs.  This code works for me, but there
might be a better way to do this.

Same caveats about testing still apply.  My test setup is a Marvell
Peridot switch hanging off of a Intel C3000 SoC.  Clause 45 devices and
other ixgbe devices have not been tested.

Changes from v1 -> v2
[PATCH 1/2] ixgbe: register a mdiobus
* Add intel-wired-...@lists.osuosl.org to CC list, see
* select MII in Kconfig (thanks to the kbuild bot)
* Only call mdiobus_regsiter for single x500em_a device
* Use readx_poll_timeout() in ixgbe_msca_cmd()
* Register different bus->read/write callbacks in ixgbe_mii_bus_init()
  so there's device id check on every access
* Use device pci_name() in bus->id instead of parent bridge's name

[PATCH 2/2] ixgbe: use mii_bus to handle MII related ioctls
* Only use mdiobus_read/write for adapters that registered a mdiobus

Stephen Douthit (2):
  ixgbe: register a mdiobus
  ixgbe: use mii_bus to handle MII related ioctls

 drivers/net/ethernet/intel/Kconfig|   1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe.h  |   2 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  23 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 307 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |   2 +
 5 files changed, 335 insertions(+)

-- 
2.17.2



Re: [PATCH net-next 1/2] ixgbe: register a mdiobus

2018-11-30 Thread Steve Douthit
On 11/30/18 12:43 PM, Florian Fainelli wrote:
> 
> 
> On 11/30/2018 9:34 AM, Steve Douthit wrote:
>> On 11/30/18 11:34 AM, Andrew Lunn wrote:
>>>> Yep, registering multiple interfaces is wrong.  The first board I tested
>>>> against only had a single MAC enabled (they can be disabled/hidden via
>>>> straps) so it just happened to work.
>>>
>>> Hi Steve
>>>
>>> Can you hide any/all via straps, or is 00.0 always guaranteed to
>>> exist?
>>
>> You can hide all the devices, but if function 1 is enabled then function
>> 0 must also be enabled, so not all combinations are valid.
>>
>>>> The Intel C3xxx family of SoCs have up to four ixgbe MACs.  These are
>>>> structured as two devices of two functions each on fixed internal root
>>>> ports.
>>>>
>>>> from lspci:
>>>> 
>>>>   +-16.0-[05]--+-00.0
>>>>   |\-00.1
>>>>   +-17.0-[06]--+-00.0
>>>>   |\-00.1
>>>> 
>>>
>>> Is there any other hardware resource which is shared between the MAC
>>> interfaces? I'm just wondering if the driver has already solved this
>>> once. Is there an EEPROM per interface for the MAC address, or one
>>> shared EEPROM?
>>
>> NVM is shared, it's actually the same SPI flash that holds the BIOS for
>> this SoC.  Access is serialized by the swfw_sync semaphore.  I think the
>> device firmware is automagically handling offset translation.
>>
>> I don't think that helps for this case.
>>
>> There might be a better match for shared resources, but nothing springs
>> to mind.
>>
>>> Ah, how about using the 'cards_found' found variable. It is not
>>> perfect, in that it is not decremented in ixgb_remove(), and i wonder
>>> about race conditions since there does not appear to be any lock when
>>> it is incremented. But if cards_found == 0, register the MDIO bus.
>>
>> 'cards_found' doesn't exist for the ixgbe driver.  I could add it and
>> fix the race/decrement issues you mention, but it'd have to be a device
>> type specific count.  It's still possible there are other non-x550em_a
>> ixgbe devices in external PCIe slots that have different resource
>> sharing setups.
>>
>> It's still a lighter weight solution than poking around the parent bus
>> so I'll add a 'x550em_a_devs_found' counter to v2.
>>
> 
> If the MDIO bus block is hardwired to function 0, would it be acceptable
> to walk the PCI bus hierarchy from any driver's probe routine where PCI
> function != 0 and see if it is claimed/enabled already, and if so, skip
> registering the MDIO bus entirely?

It's not really hardwired to any function at the hardware level AFAICT.
Unless by 'hardwired' you mean we always register it to the same devfn?
Walking the bus to find the lowest numbered devfn was my original
suggestion.  I'm good with a PCI walk or a counter to choose which MAC
registers the MDIO bus.  Let me know what the consensus is.

> There is also possibly a problem if you have a shared MDIO device, and
> you turn off/power off the PCI function that does provide it. How can we
> make sure it remains alive for other functions to use it?

I'm not sure this is an issue.  I wouldn't expect the other MACs to lose
the ability to talk to their PHYs.  Those callbacks weren't changed to
use the registered MDIO bus.


Re: [PATCH net-next 1/2] ixgbe: register a mdiobus

2018-11-30 Thread Steve Douthit
On 11/30/18 11:34 AM, Andrew Lunn wrote:
>> Yep, registering multiple interfaces is wrong.  The first board I tested
>> against only had a single MAC enabled (they can be disabled/hidden via
>> straps) so it just happened to work.
> 
> Hi Steve
> 
> Can you hide any/all via straps, or is 00.0 always guaranteed to
> exist?

You can hide all the devices, but if function 1 is enabled then function
0 must also be enabled, so not all combinations are valid.

>> The Intel C3xxx family of SoCs have up to four ixgbe MACs.  These are
>> structured as two devices of two functions each on fixed internal root
>> ports.
>>
>> from lspci:
>> 
>>  +-16.0-[05]--+-00.0
>>  |\-00.1
>>  +-17.0-[06]--+-00.0
>>  |\-00.1
>> 
> 
> Is there any other hardware resource which is shared between the MAC
> interfaces? I'm just wondering if the driver has already solved this
> once. Is there an EEPROM per interface for the MAC address, or one
> shared EEPROM?

NVM is shared, it's actually the same SPI flash that holds the BIOS for
this SoC.  Access is serialized by the swfw_sync semaphore.  I think the
device firmware is automagically handling offset translation.

I don't think that helps for this case.

There might be a better match for shared resources, but nothing springs
to mind.

> Ah, how about using the 'cards_found' found variable. It is not
> perfect, in that it is not decremented in ixgb_remove(), and i wonder
> about race conditions since there does not appear to be any lock when
> it is incremented. But if cards_found == 0, register the MDIO bus.

'cards_found' doesn't exist for the ixgbe driver.  I could add it and
fix the race/decrement issues you mention, but it'd have to be a device
type specific count.  It's still possible there are other non-x550em_a
ixgbe devices in external PCIe slots that have different resource
sharing setups.

It's still a lighter weight solution than poking around the parent bus
so I'll add a 'x550em_a_devs_found' counter to v2.


Re: [PATCH net-next 1/2] ixgbe: register a mdiobus

2018-11-30 Thread Steve Douthit
On 11/30/18 8:21 AM, Andrew Lunn wrote:
> Hi Steve
> 
> Cool to see another interface being made DSA capable.
> 
>> +/**
>> + *  ixgbe_msca - Write the command register and poll for completion/timeout
>> + *  @hw: pointer to hardware structure
>> + *  @cmd: command register value to write
>> + **/
>> +static s32 ixgbe_msca_cmd(struct ixgbe_hw *hw, u32 cmd)
>> +{
>> +u32 i;
>> +
>> +IXGBE_WRITE_REG(hw, IXGBE_MSCA, cmd);
>> +
>> +for (i = 0; i < IXGBE_MDIO_COMMAND_TIMEOUT; i++) {
>> +udelay(10);
>> +cmd = IXGBE_READ_REG(hw, IXGBE_MSCA);
>> +if (!(cmd & IXGBE_MSCA_MDI_COMMAND))
>> +return 0;
>> +}
>> +
>> +return -ETIMEDOUT;
> 
> Please consider using readx_poll_timeout()

OK

>> +}
>> +
>> +/**
>> + *  ixgbe_msca - Use device_id to figure out if MDIO bus is shared between 
>> MACs.
>> + *  The embedded x550(s) in the C3000 line of SoCs only have a single 
>> mii_bus
>> + *  shared between all MACs, and that introduces some new mask flags that 
>> need
>> + *  to be passed to the *_swfw_sync callbacks.
>> + *  @hw: pointer to hardware structure
>> + **/
>> +static bool ixgbe_is_shared_mii(struct ixgbe_hw *hw)
>> +{
>> +switch (hw->device_id) {
>> +case IXGBE_DEV_ID_X550EM_A_KR:
>> +case IXGBE_DEV_ID_X550EM_A_KR_L:
>> +case IXGBE_DEV_ID_X550EM_A_SFP_N:
>> +case IXGBE_DEV_ID_X550EM_A_SGMII:
>> +case IXGBE_DEV_ID_X550EM_A_SGMII_L:
>> +case IXGBE_DEV_ID_X550EM_A_10G_T:
>> +case IXGBE_DEV_ID_X550EM_A_SFP:
>> +case IXGBE_DEV_ID_X550EM_A_1G_T:
>> +case IXGBE_DEV_ID_X550EM_A_1G_T_L:
>> +return true;
>> +}
>> +
>> +return false;
>> +}
>> +
>> +/**
>> + *  ixgbe_mii_bus_read - Read a clause 22/45 register
>> + *  @hw: pointer to hardware structure
>> + *  @addr: address
>> + *  @regnum: register number
>> + **/
>> +static s32 ixgbe_mii_bus_read(struct mii_bus *bus, int addr, int regnum)
>> +{
>> +struct ixgbe_adapter *adapter = (struct ixgbe_adapter *)bus->priv;
>> +struct ixgbe_hw *hw = >hw;
>> +u32 hwaddr, cmd, gssr = hw->phy.phy_semaphore_mask;
>> +s32 data;
>> +
>> +if (ixgbe_is_shared_mii(hw)) {
>> +gssr |= IXGBE_GSSR_TOKEN_SM;
>> +if (hw->bus.lan_id)
>> +gssr |= IXGBE_GSSR_PHY1_SM;
>> +else
>> +gssr |= IXGBE_GSSR_PHY0_SM;
>> +}
> 
> Could you explain this shared bus a bit more.  If there is only one
> physical MDIO bus, you should only register one bus with Linux. So i
> would not normally expect to see ixgbe_is_shared_mii() sort of
> statements into the read/write functions. That tends to happen at the
> point you are registering the MDIO bus, and when looking for the MACs
> PHY on the bus.

Yep, registering multiple interfaces is wrong.  The first board I tested
against only had a single MAC enabled (they can be disabled/hidden via
straps) so it just happened to work.  Then I moved to a board with all
four interfaces enabled and found this.

The Intel C3xxx family of SoCs have up to four ixgbe MACs.  These are
structured as two devices of two functions each on fixed internal root
ports.

from lspci:

+-16.0-[05]--+-00.0
|\-00.1
+-17.0-[06]--+-00.0
|\-00.1


Each of those MACs exposes the same set of MDIO control registers as if
there are four interface, but there's really only a single bus in the
hardware for these SoCs.

How about I change:
1) ixgbe_is_shared_mii() -> ixgbe_is_x550em_a()
2) ixgbe_mii_bus_(read|write) -> ixgbe_x550em_a_mii_bus_(read|write) and
update the phy_semaphore_mask unconditionally
3) Only have ixgbe_mii_bus_init() register something for the x550em_a
devices for now, and leave handling other platforms for the future.
4) Have ixgbe_mii_bus_init() register only a single mdiobus for func 0
of the device downstream of the 00:16.0 root port, or if that's been
disabled then func 0 downstream of the 00:17.0 root port.  All of the
devices on bus 0 of the SoC are at fixed device/function locations.

Would that be acceptable?


Re: [PATCH net-next 0/2] Add mii_bus to ixgbe driver for dsa devs

2018-11-29 Thread Steve Douthit
On 11/29/18 2:03 PM, Jeff Kirsher wrote:
> On Thu, 2018-11-29 at 18:54 +0000, Steve Douthit wrote:
>> Most dsa devices currently expect to get a pointer to a mii_bus from
>> platform data of device tree information.
>>
>> The first patch exposes a mii_bus for ixgbe devices.
>>
>> Currently the ixgbe driver only allows accesses to the probed PHY
>> address via ioctls. The second patch changes that behavior to allow
>> access to all addresses.  This should be useful for doing switch peeks &
>> pokes from userspace for development, test, or if theres no dsa driver
>> for a particular switch yet.
>>
>> I've tested the clause 22 portion of this code on a board that has an
>> Intel C3xxx series SoC connected to a Marvell Peridot switch.  I don't
>> have any clause 45 devices or other ixgbe devices to test with.
>>
>> Stephen Douthit (2):
>>ixgbe: register a mdiobus
>>ixgbe: use mii_bus to handle MII related ioctls
>>
>>   drivers/net/ethernet/intel/ixgbe/ixgbe.h  |   2 +
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  35 ++--
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 192 ++
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |   2 +
>>   4 files changed, 216 insertions(+), 15 deletions(-)
>>
> 
> I will add these changes to my queue, please remember to send patches
> against the drivers in drivers/net/ethernet/intel/* to
> intel-wired-...@lists.osuosl.org  mailing list as well, so that our
> patchwork project can track the patches.

Sorry about that, I'll add it to the CC list for the next rev.

I started adding another board to the platform driver that needs this
code and found a bug.  The new board has more lanes enabled via soft
straps.  That caused sysfs to complain because I ended up with duplicate
bus IDs from this code:

+   /* Use the position of the device in the PCI hierarchy as the id */
+   snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mdio-%s", ixgbe_driver_name,
+pci_name(pdev->bus->self));

Please drop this series for now.

Thinking about this some more I probably need to ensure that only a
single mii_bus is registered out of all the discovered x550em_a devices.
More generally there's not always going to be a 1:1 mapping between MACs
and MIIs on these devices and the code should handle that.

I think for the next rev I'll only register a single mii_bus for the
lowest numbered bus:device.function x500em_a device, and skip mii_bus
setup entirely for other device IDs.

Let me know if there's another/better way to tackle that.


[PATCH net-next 2/2] ixgbe: use mii_bus to handle MII related ioctls

2018-11-29 Thread Steve Douthit
Use the mii_bus callbacks to address the entire clause 22/45 address
space.  Enables userspace to poke switch registers instead of a single
PHY address.

Signed-off-by: Stephen Douthit 
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 29 +--
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index c2141fe35685..345ecdddc749 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8786,27 +8786,26 @@ static int
 ixgbe_mdio_read(struct net_device *netdev, int prtad, int devad, u16 addr)
 {
struct ixgbe_adapter *adapter = netdev_priv(netdev);
-   struct ixgbe_hw *hw = >hw;
-   u16 value;
-   int rc;
+   struct mii_bus *mii_bus = adapter->mii_bus;
+   int regnum = addr;
 
-   if (prtad != hw->phy.mdio.prtad)
-   return -EINVAL;
-   rc = hw->phy.ops.read_reg(hw, addr, devad, );
-   if (!rc)
-   rc = value;
-   return rc;
+   if (devad != MDIO_DEVAD_NONE)
+   regnum |= (devad << 16) | MII_ADDR_C45;
+
+   return mdiobus_read(mii_bus, prtad, regnum);
 }
 
 static int ixgbe_mdio_write(struct net_device *netdev, int prtad, int devad,
u16 addr, u16 value)
 {
struct ixgbe_adapter *adapter = netdev_priv(netdev);
-   struct ixgbe_hw *hw = >hw;
+   struct mii_bus *mii_bus = adapter->mii_bus;
+   int regnum = addr;
 
-   if (prtad != hw->phy.mdio.prtad)
-   return -EINVAL;
-   return hw->phy.ops.write_reg(hw, addr, devad, value);
+   if (devad != MDIO_DEVAD_NONE)
+   regnum |= (devad << 16) | MII_ADDR_C45;
+
+   return mdiobus_write(mii_bus, prtad, regnum, value);
 }
 
 static int ixgbe_ioctl(struct net_device *netdev, struct ifreq *req, int cmd)
@@ -8819,7 +8818,7 @@ static int ixgbe_ioctl(struct net_device *netdev, struct 
ifreq *req, int cmd)
case SIOCGHWTSTAMP:
return ixgbe_ptp_get_ts_config(adapter, req);
case SIOCGMIIPHY:
-   if (!adapter->hw.phy.ops.read_reg)
+   if (!adapter->mii_bus)
return -EOPNOTSUPP;
/* fall through */
default:
@@ -10796,7 +10795,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent)
/* ixgbe_identify_phy_generic will set prtad and mmds properly */
hw->phy.mdio.prtad = MDIO_PRTAD_NONE;
hw->phy.mdio.mmds = 0;
-   hw->phy.mdio.mode_support = MDIO_SUPPORTS_C45 | MDIO_EMULATE_C22;
+   hw->phy.mdio.mode_support = MDIO_SUPPORTS_C45 | MDIO_SUPPORTS_C22;
hw->phy.mdio.dev = netdev;
hw->phy.mdio.mdio_read = ixgbe_mdio_read;
hw->phy.mdio.mdio_write = ixgbe_mdio_write;
-- 
2.17.2



[PATCH net-next 1/2] ixgbe: register a mdiobus

2018-11-29 Thread Steve Douthit
Most dsa devices expect a 'struct mii_bus' pointer to talk to switches
via the MII interface.

Signed-off-by: Stephen Douthit 
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h  |   2 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   6 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 192 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |   2 +
 4 files changed, 202 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h 
b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 143bdd5ee2a0..08d85e336bd4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -561,6 +562,7 @@ struct ixgbe_adapter {
struct net_device *netdev;
struct bpf_prog *xdp_prog;
struct pci_dev *pdev;
+   struct mii_bus *mii_bus;
 
unsigned long state;
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 49a4ea38eb07..c2141fe35685 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -39,6 +39,7 @@
 #include "ixgbe.h"
 #include "ixgbe_common.h"
 #include "ixgbe_dcb_82599.h"
+#include "ixgbe_phy.h"
 #include "ixgbe_sriov.h"
 #include "ixgbe_model.h"
 #include "ixgbe_txrx_common.h"
@@ -11120,6 +11121,9 @@ static int ixgbe_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent)
IXGBE_LINK_SPEED_10GB_FULL | IXGBE_LINK_SPEED_1GB_FULL,
true);
 
+   if (ixgbe_mii_bus_init(hw))
+   e_err(probe, "failed to create mii_bus\n");
+
return 0;
 
 err_register:
@@ -11170,6 +11174,8 @@ static void ixgbe_remove(struct pci_dev *pdev)
set_bit(__IXGBE_REMOVING, >state);
cancel_work_sync(>service_task);
 
+   if (adapter->mii_bus)
+   mdiobus_unregister(adapter->mii_bus);
 
 #ifdef CONFIG_IXGBE_DCA
if (adapter->flags & IXGBE_FLAG_DCA_ENABLED) {
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
index 919a7af84b42..75db340963ad 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
@@ -658,6 +658,198 @@ s32 ixgbe_write_phy_reg_generic(struct ixgbe_hw *hw, u32 
reg_addr,
return status;
 }
 
+/**
+ *  ixgbe_msca - Write the command register and poll for completion/timeout
+ *  @hw: pointer to hardware structure
+ *  @cmd: command register value to write
+ **/
+static s32 ixgbe_msca_cmd(struct ixgbe_hw *hw, u32 cmd)
+{
+   u32 i;
+
+   IXGBE_WRITE_REG(hw, IXGBE_MSCA, cmd);
+
+   for (i = 0; i < IXGBE_MDIO_COMMAND_TIMEOUT; i++) {
+   udelay(10);
+   cmd = IXGBE_READ_REG(hw, IXGBE_MSCA);
+   if (!(cmd & IXGBE_MSCA_MDI_COMMAND))
+   return 0;
+   }
+
+   return -ETIMEDOUT;
+}
+
+/**
+ *  ixgbe_msca - Use device_id to figure out if MDIO bus is shared between 
MACs.
+ *  The embedded x550(s) in the C3000 line of SoCs only have a single mii_bus
+ *  shared between all MACs, and that introduces some new mask flags that need
+ *  to be passed to the *_swfw_sync callbacks.
+ *  @hw: pointer to hardware structure
+ **/
+static bool ixgbe_is_shared_mii(struct ixgbe_hw *hw)
+{
+   switch (hw->device_id) {
+   case IXGBE_DEV_ID_X550EM_A_KR:
+   case IXGBE_DEV_ID_X550EM_A_KR_L:
+   case IXGBE_DEV_ID_X550EM_A_SFP_N:
+   case IXGBE_DEV_ID_X550EM_A_SGMII:
+   case IXGBE_DEV_ID_X550EM_A_SGMII_L:
+   case IXGBE_DEV_ID_X550EM_A_10G_T:
+   case IXGBE_DEV_ID_X550EM_A_SFP:
+   case IXGBE_DEV_ID_X550EM_A_1G_T:
+   case IXGBE_DEV_ID_X550EM_A_1G_T_L:
+   return true;
+   }
+
+   return false;
+}
+
+/**
+ *  ixgbe_mii_bus_read - Read a clause 22/45 register
+ *  @hw: pointer to hardware structure
+ *  @addr: address
+ *  @regnum: register number
+ **/
+static s32 ixgbe_mii_bus_read(struct mii_bus *bus, int addr, int regnum)
+{
+   struct ixgbe_adapter *adapter = (struct ixgbe_adapter *)bus->priv;
+   struct ixgbe_hw *hw = >hw;
+   u32 hwaddr, cmd, gssr = hw->phy.phy_semaphore_mask;
+   s32 data;
+
+   if (ixgbe_is_shared_mii(hw)) {
+   gssr |= IXGBE_GSSR_TOKEN_SM;
+   if (hw->bus.lan_id)
+   gssr |= IXGBE_GSSR_PHY1_SM;
+   else
+   gssr |= IXGBE_GSSR_PHY0_SM;
+   }
+
+   if (hw->mac.ops.acquire_swfw_sync(hw, gssr))
+   return -EBUSY;
+
+   hwaddr = addr << IXGBE_MSCA_PHY_ADDR_SHIFT;
+   if (regnum & MII_ADDR_C45) {
+   hwaddr |= regnum & GENMASK(21, 0);
+   cmd = hwaddr | IXGBE_MSCA_ADDR_CYCLE | IXGBE_MSCA_MDI_COMMAND;
+   } else {
+   hwaddr |= (regnum & GENMASK(5, 0)) << IXGBE_MSCA_DEV_TYPE_SHIFT;
+   cmd = hwaddr | 

[PATCH net-next 0/2] Add mii_bus to ixgbe driver for dsa devs

2018-11-29 Thread Steve Douthit
Most dsa devices currently expect to get a pointer to a mii_bus from
platform data of device tree information.

The first patch exposes a mii_bus for ixgbe devices.

Currently the ixgbe driver only allows accesses to the probed PHY
address via ioctls. The second patch changes that behavior to allow
access to all addresses.  This should be useful for doing switch peeks &
pokes from userspace for development, test, or if theres no dsa driver
for a particular switch yet.

I've tested the clause 22 portion of this code on a board that has an
Intel C3xxx series SoC connected to a Marvell Peridot switch.  I don't
have any clause 45 devices or other ixgbe devices to test with.

Stephen Douthit (2):
  ixgbe: register a mdiobus
  ixgbe: use mii_bus to handle MII related ioctls

 drivers/net/ethernet/intel/ixgbe/ixgbe.h  |   2 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  35 ++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 192 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |   2 +
 4 files changed, 216 insertions(+), 15 deletions(-)

-- 
2.17.2