[PATCH 0/3] net/phy: Improvements to Cavium Thunder MDIO code.

[PATCH 0/3] net/phy: Improvements to Cavium Thunder MDIO code.

[David Daney]

From: David Daney <david.daney@cavium.com>

The firmware on many Cavium Thunder systems configures the MDIO bus
hardware to be probed as a PCI device.  In order to use the MDIO bus
drivers in this configuration, we must add PCI probing to the driver.

There are two parts to this set of three patches:

 1) Cleanup the PHY probing code in thunder_bgx.c to handle the case
    where there is no PHY attached to a port, as well as being more
    robust in the face of driver loading order by use of
    -EPROBE_DEFER.

 2) Split mdio-octeon.c into two drivers, one with platform probing,
 and the other with PCI probing.  Common code is shared between the
 two.

Tested on several different Thunder and OCTEON systems, also compile
tested on x86_64.

David Daney (3):
  net: thunderx: Cleanup PHY probing code.
  phy: mdio-octeon: Refactor into two files/modules
  phy: mdio-thunder:  Add driver for Cavium Thunder SoC MDIO buses.

 .../devicetree/bindings/net/cavium-mdio.txt        |  61 ++++-
 drivers/net/ethernet/cavium/thunder/thunder_bgx.c  |  31 +--
 drivers/net/phy/Kconfig                            |  22 +-
 drivers/net/phy/Makefile                           |   2 +
 drivers/net/phy/mdio-cavium.c                      | 149 +++++++++++
 drivers/net/phy/mdio-cavium.h                      | 119 +++++++++
 drivers/net/phy/mdio-octeon.c                      | 280 ++-------------------
 drivers/net/phy/mdio-thunder.c                     | 154 ++++++++++++
 8 files changed, 533 insertions(+), 285 deletions(-)
 create mode 100644 drivers/net/phy/mdio-cavium.c
 create mode 100644 drivers/net/phy/mdio-cavium.h
 create mode 100644 drivers/net/phy/mdio-thunder.c

-- 
1.7.11.7

[PATCH 1/3] net: thunderx: Cleanup PHY probing code.

[David Daney]

From: David Daney <david.daney@cavium.com>

Remove the call to force the octeon-mdio driver to be loaded.  Allow
the standard driver loading mechanisms to load the PHY drivers, and
use -EPROBE_DEFER to cause the BGX driver to be probed only after the
PHY drivers are available.

Reorder the setting of MAC addresses and PHY probing to allow BGX
LMACs with no attached PHY to still be assigned a MAC address.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 31 ++++++++++++-----------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index f8abdff..928c02b 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -978,27 +978,31 @@ static int bgx_init_of_phy(struct bgx *bgx)
 	const char *mac;
 
 	device_for_each_child_node(&bgx->pdev->dev, fwn) {
+		struct phy_device *pd;
 		struct device_node *phy_np;
 		struct device_node *node = to_of_node(fwn);
 
-		/* If it is not an OF node we cannot handle it yet, so
-		 * exit the loop.
-		 */
-		if (!node)
-			break;
-
-		phy_np = of_parse_phandle(node, "phy-handle", 0);
-		if (!phy_np)
-			continue;
-
-		bgx->lmac[lmac].phydev = of_phy_find_device(phy_np);
-
 		mac = of_get_mac_address(node);
 		if (mac)
 			ether_addr_copy(bgx->lmac[lmac].mac, mac);
 
 		SET_NETDEV_DEV(&bgx->lmac[lmac].netdev, &bgx->pdev->dev);
 		bgx->lmac[lmac].lmacid = lmac;
+
+		phy_np = of_parse_phandle(node, "phy-handle", 0);
+		/* If there is no phy or defective firmware presents
+		 * this cortina phy, for which there is no driver
+		 * support, ignore it.
+		 */
+		if (phy_np &&
+		    !of_device_is_compatible(phy_np, "cortina,cs4223-slice")) {
+			/* Wait until the phy drivers are available */
+			pd = of_phy_find_device(phy_np);
+			if (!pd)
+				return -EPROBE_DEFER;
+			bgx->lmac[lmac].phydev = pd;
+		}
+
 		lmac++;
 		if (lmac == MAX_LMAC_PER_BGX) {
 			of_node_put(node);
@@ -1032,9 +1036,6 @@ static int bgx_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct bgx *bgx = NULL;
 	u8 lmac;
 
-	/* Load octeon mdio driver */
-	octeon_mdiobus_force_mod_depencency();
-
 	bgx = devm_kzalloc(dev, sizeof(*bgx), GFP_KERNEL);
 	if (!bgx)
 		return -ENOMEM;
-- 
1.7.11.7

[PATCH 2/3] phy: mdio-octeon: Refactor into two files/modules

[David Daney]

From: David Daney <david.daney@cavium.com>

A follow-on patch uses PCI probing to find the Thunder MDIO hardware.
In preparation for this, split out the common code into a new file
mdio-cavium.c, which will be used by both the existing OCTEON driver,
and the new Thunder PCI based driver.

As part of the refactoring simplify the struct cavium_mdiobus by
removing fields that are only ever used in the probe function and can
just as well be local variables.

Use readq/writeq in preference to readq_relaxed/writeq_relaxed as the
relaxed form was an optimization for an early chip revision, and the
MDIO drivers are not performance bottlenecks that need optimization in
the first place.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/net/phy/Kconfig       |  11 +-
 drivers/net/phy/Makefile      |   1 +
 drivers/net/phy/mdio-cavium.c | 149 ++++++++++++++++++++++
 drivers/net/phy/mdio-cavium.h | 119 ++++++++++++++++++
 drivers/net/phy/mdio-octeon.c | 280 +++---------------------------------------
 5 files changed, 292 insertions(+), 268 deletions(-)
 create mode 100644 drivers/net/phy/mdio-cavium.c
 create mode 100644 drivers/net/phy/mdio-cavium.h

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index f0a7702..40faec9 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -183,15 +183,18 @@ config MDIO_GPIO
 	  To compile this driver as a module, choose M here: the module
 	  will be called mdio-gpio.
 
+config MDIO_CAVIUM
+	tristate
+
 config MDIO_OCTEON
-	tristate "Support for MDIO buses on Octeon and ThunderX SOCs"
+	tristate "Support for MDIO buses on Octeon and some ThunderX SOCs"
 	depends on 64BIT
 	depends on HAS_IOMEM
+	select MDIO_CAVIUM
 	help
-
 	  This module provides a driver for the Octeon and ThunderX MDIO
-	  busses. It is required by the Octeon and ThunderX ethernet device
-	  drivers.
+	  buses. It is required by the Octeon and ThunderX ethernet device
+	  drivers on some systems.
 
 config MDIO_SUN4I
 	tristate "Allwinner sun4i MDIO interface support"
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 680e88f9..041b3d9 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_DP83867_PHY)	+= dp83867.o
 obj-$(CONFIG_STE10XP)		+= ste10Xp.o
 obj-$(CONFIG_MICREL_PHY)	+= micrel.o
 obj-$(CONFIG_MDIO_OCTEON)	+= mdio-octeon.o
+obj-$(CONFIG_MDIO_CAVIUM)	+= mdio-cavium.o
 obj-$(CONFIG_MICREL_KS8995MA)	+= spi_ks8995.o
 obj-$(CONFIG_AT803X_PHY)	+= at803x.o
 obj-$(CONFIG_AMD_PHY)		+= amd.o
diff --git a/drivers/net/phy/mdio-cavium.c b/drivers/net/phy/mdio-cavium.c
new file mode 100644
index 0000000..e796ee1
--- /dev/null
+++ b/drivers/net/phy/mdio-cavium.c
@@ -0,0 +1,149 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (C) 2009-2016 Cavium, Inc.
+ */
+
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/phy.h>
+#include <linux/io.h>
+
+#include "mdio-cavium.h"
+
+static void cavium_mdiobus_set_mode(struct cavium_mdiobus *p,
+				    enum cavium_mdiobus_mode m)
+{
+	union cvmx_smix_clk smi_clk;
+
+	if (m == p->mode)
+		return;
+
+	smi_clk.u64 = oct_mdio_readq(p->register_base + SMI_CLK);
+	smi_clk.s.mode = (m == C45) ? 1 : 0;
+	smi_clk.s.preamble = 1;
+	oct_mdio_writeq(smi_clk.u64, p->register_base + SMI_CLK);
+	p->mode = m;
+}
+
+static int cavium_mdiobus_c45_addr(struct cavium_mdiobus *p,
+				   int phy_id, int regnum)
+{
+	union cvmx_smix_cmd smi_cmd;
+	union cvmx_smix_wr_dat smi_wr;
+	int timeout = 1000;
+
+	cavium_mdiobus_set_mode(p, C45);
+
+	smi_wr.u64 = 0;
+	smi_wr.s.dat = regnum & 0xffff;
+	oct_mdio_writeq(smi_wr.u64, p->register_base + SMI_WR_DAT);
+
+	regnum = (regnum >> 16) & 0x1f;
+
+	smi_cmd.u64 = 0;
+	smi_cmd.s.phy_op = 0; /* MDIO_CLAUSE_45_ADDRESS */
+	smi_cmd.s.phy_adr = phy_id;
+	smi_cmd.s.reg_adr = regnum;
+	oct_mdio_writeq(smi_cmd.u64, p->register_base + SMI_CMD);
+
+	do {
+		/* Wait 1000 clocks so we don't saturate the RSL bus
+		 * doing reads.
+		 */
+		__delay(1000);
+		smi_wr.u64 = oct_mdio_readq(p->register_base + SMI_WR_DAT);
+	} while (smi_wr.s.pending && --timeout);
+
+	if (timeout <= 0)
+		return -EIO;
+	return 0;
+}
+
+int cavium_mdiobus_read(struct mii_bus *bus, int phy_id, int regnum)
+{
+	struct cavium_mdiobus *p = bus->priv;
+	union cvmx_smix_cmd smi_cmd;
+	union cvmx_smix_rd_dat smi_rd;
+	unsigned int op = 1; /* MDIO_CLAUSE_22_READ */
+	int timeout = 1000;
+
+	if (regnum & MII_ADDR_C45) {
+		int r = cavium_mdiobus_c45_addr(p, phy_id, regnum);
+
+		if (r < 0)
+			return r;
+
+		regnum = (regnum >> 16) & 0x1f;
+		op = 3; /* MDIO_CLAUSE_45_READ */
+	} else {
+		cavium_mdiobus_set_mode(p, C22);
+	}
+
+	smi_cmd.u64 = 0;
+	smi_cmd.s.phy_op = op;
+	smi_cmd.s.phy_adr = phy_id;
+	smi_cmd.s.reg_adr = regnum;
+	oct_mdio_writeq(smi_cmd.u64, p->register_base + SMI_CMD);
+
+	do {
+		/* Wait 1000 clocks so we don't saturate the RSL bus
+		 * doing reads.
+		 */
+		__delay(1000);
+		smi_rd.u64 = oct_mdio_readq(p->register_base + SMI_RD_DAT);
+	} while (smi_rd.s.pending && --timeout);
+
+	if (smi_rd.s.val)
+		return smi_rd.s.dat;
+	else
+		return -EIO;
+}
+EXPORT_SYMBOL(cavium_mdiobus_read);
+
+int cavium_mdiobus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val)
+{
+	struct cavium_mdiobus *p = bus->priv;
+	union cvmx_smix_cmd smi_cmd;
+	union cvmx_smix_wr_dat smi_wr;
+	unsigned int op = 0; /* MDIO_CLAUSE_22_WRITE */
+	int timeout = 1000;
+
+	if (regnum & MII_ADDR_C45) {
+		int r = cavium_mdiobus_c45_addr(p, phy_id, regnum);
+
+		if (r < 0)
+			return r;
+
+		regnum = (regnum >> 16) & 0x1f;
+		op = 1; /* MDIO_CLAUSE_45_WRITE */
+	} else {
+		cavium_mdiobus_set_mode(p, C22);
+	}
+
+	smi_wr.u64 = 0;
+	smi_wr.s.dat = val;
+	oct_mdio_writeq(smi_wr.u64, p->register_base + SMI_WR_DAT);
+
+	smi_cmd.u64 = 0;
+	smi_cmd.s.phy_op = op;
+	smi_cmd.s.phy_adr = phy_id;
+	smi_cmd.s.reg_adr = regnum;
+	oct_mdio_writeq(smi_cmd.u64, p->register_base + SMI_CMD);
+
+	do {
+		/* Wait 1000 clocks so we don't saturate the RSL bus
+		 * doing reads.
+		 */
+		__delay(1000);
+		smi_wr.u64 = oct_mdio_readq(p->register_base + SMI_WR_DAT);
+	} while (smi_wr.s.pending && --timeout);
+
+	if (timeout <= 0)
+		return -EIO;
+
+	return 0;
+}
+EXPORT_SYMBOL(cavium_mdiobus_write);
diff --git a/drivers/net/phy/mdio-cavium.h b/drivers/net/phy/mdio-cavium.h
new file mode 100644
index 0000000..4bccd45
--- /dev/null
+++ b/drivers/net/phy/mdio-cavium.h
@@ -0,0 +1,119 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (C) 2009-2016 Cavium, Inc.
+ */
+
+enum cavium_mdiobus_mode {
+	UNINIT = 0,
+	C22,
+	C45
+};
+
+#define SMI_CMD		0x0
+#define SMI_WR_DAT	0x8
+#define SMI_RD_DAT	0x10
+#define SMI_CLK		0x18
+#define SMI_EN		0x20
+
+#ifdef __BIG_ENDIAN_BITFIELD
+#define OCT_MDIO_BITFIELD_FIELD(field, more)	\
+	field;					\
+	more
+
+#else
+#define OCT_MDIO_BITFIELD_FIELD(field, more)	\
+	more					\
+	field;
+
+#endif
+
+union cvmx_smix_clk {
+	u64 u64;
+	struct cvmx_smix_clk_s {
+	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_25_63:39,
+	  OCT_MDIO_BITFIELD_FIELD(u64 mode:1,
+	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_21_23:3,
+	  OCT_MDIO_BITFIELD_FIELD(u64 sample_hi:5,
+	  OCT_MDIO_BITFIELD_FIELD(u64 sample_mode:1,
+	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_14_14:1,
+	  OCT_MDIO_BITFIELD_FIELD(u64 clk_idle:1,
+	  OCT_MDIO_BITFIELD_FIELD(u64 preamble:1,
+	  OCT_MDIO_BITFIELD_FIELD(u64 sample:4,
+	  OCT_MDIO_BITFIELD_FIELD(u64 phase:8,
+	  ;))))))))))
+	} s;
+};
+
+union cvmx_smix_cmd {
+	u64 u64;
+	struct cvmx_smix_cmd_s {
+	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_18_63:46,
+	  OCT_MDIO_BITFIELD_FIELD(u64 phy_op:2,
+	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_13_15:3,
+	  OCT_MDIO_BITFIELD_FIELD(u64 phy_adr:5,
+	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_5_7:3,
+	  OCT_MDIO_BITFIELD_FIELD(u64 reg_adr:5,
+	  ;))))))
+	} s;
+};
+
+union cvmx_smix_en {
+	u64 u64;
+	struct cvmx_smix_en_s {
+	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_1_63:63,
+	  OCT_MDIO_BITFIELD_FIELD(u64 en:1,
+	  ;))
+	} s;
+};
+
+union cvmx_smix_rd_dat {
+	u64 u64;
+	struct cvmx_smix_rd_dat_s {
+	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_18_63:46,
+	  OCT_MDIO_BITFIELD_FIELD(u64 pending:1,
+	  OCT_MDIO_BITFIELD_FIELD(u64 val:1,
+	  OCT_MDIO_BITFIELD_FIELD(u64 dat:16,
+	  ;))))
+	} s;
+};
+
+union cvmx_smix_wr_dat {
+	u64 u64;
+	struct cvmx_smix_wr_dat_s {
+	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_18_63:46,
+	  OCT_MDIO_BITFIELD_FIELD(u64 pending:1,
+	  OCT_MDIO_BITFIELD_FIELD(u64 val:1,
+	  OCT_MDIO_BITFIELD_FIELD(u64 dat:16,
+	  ;))))
+	} s;
+};
+
+struct cavium_mdiobus {
+	struct mii_bus *mii_bus;
+	u64 register_base;
+	enum cavium_mdiobus_mode mode;
+};
+
+#ifdef CONFIG_CAVIUM_OCTEON_SOC
+
+#include <asm/octeon/octeon.h>
+
+static inline void oct_mdio_writeq(u64 val, u64 addr)
+{
+	cvmx_write_csr(addr, val);
+}
+
+static inline u64 oct_mdio_readq(u64 addr)
+{
+	return cvmx_read_csr(addr);
+}
+#else
+#define oct_mdio_writeq(val, addr)	writeq(val, (void *)addr)
+#define oct_mdio_readq(addr)		readq((void *)addr)
+#endif
+
+int cavium_mdiobus_read(struct mii_bus *bus, int phy_id, int regnum);
+int cavium_mdiobus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val);
diff --git a/drivers/net/phy/mdio-octeon.c b/drivers/net/phy/mdio-octeon.c
index 47d4f2f..ab6914f 100644
--- a/drivers/net/phy/mdio-octeon.c
+++ b/drivers/net/phy/mdio-octeon.c
@@ -3,272 +3,26 @@
  * License.  See the file "COPYING" in the main directory of this archive
  * for more details.
  *
- * Copyright (C) 2009-2012 Cavium, Inc.
+ * Copyright (C) 2009-2015 Cavium, Inc.
  */
 
 #include <linux/platform_device.h>
 #include <linux/of_address.h>
 #include <linux/of_mdio.h>
-#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/gfp.h>
 #include <linux/phy.h>
 #include <linux/io.h>
 
-#ifdef CONFIG_CAVIUM_OCTEON_SOC
-#include <asm/octeon/octeon.h>
-#endif
-
-#define DRV_VERSION "1.1"
-#define DRV_DESCRIPTION "Cavium Networks Octeon/ThunderX SMI/MDIO driver"
-
-#define SMI_CMD		0x0
-#define SMI_WR_DAT	0x8
-#define SMI_RD_DAT	0x10
-#define SMI_CLK		0x18
-#define SMI_EN		0x20
-
-#ifdef __BIG_ENDIAN_BITFIELD
-#define OCT_MDIO_BITFIELD_FIELD(field, more)	\
-	field;					\
-	more
-
-#else
-#define OCT_MDIO_BITFIELD_FIELD(field, more)	\
-	more					\
-	field;
-
-#endif
-
-union cvmx_smix_clk {
-	u64 u64;
-	struct cvmx_smix_clk_s {
-	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_25_63:39,
-	  OCT_MDIO_BITFIELD_FIELD(u64 mode:1,
-	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_21_23:3,
-	  OCT_MDIO_BITFIELD_FIELD(u64 sample_hi:5,
-	  OCT_MDIO_BITFIELD_FIELD(u64 sample_mode:1,
-	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_14_14:1,
-	  OCT_MDIO_BITFIELD_FIELD(u64 clk_idle:1,
-	  OCT_MDIO_BITFIELD_FIELD(u64 preamble:1,
-	  OCT_MDIO_BITFIELD_FIELD(u64 sample:4,
-	  OCT_MDIO_BITFIELD_FIELD(u64 phase:8,
-	  ;))))))))))
-	} s;
-};
-
-union cvmx_smix_cmd {
-	u64 u64;
-	struct cvmx_smix_cmd_s {
-	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_18_63:46,
-	  OCT_MDIO_BITFIELD_FIELD(u64 phy_op:2,
-	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_13_15:3,
-	  OCT_MDIO_BITFIELD_FIELD(u64 phy_adr:5,
-	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_5_7:3,
-	  OCT_MDIO_BITFIELD_FIELD(u64 reg_adr:5,
-	  ;))))))
-	} s;
-};
-
-union cvmx_smix_en {
-	u64 u64;
-	struct cvmx_smix_en_s {
-	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_1_63:63,
-	  OCT_MDIO_BITFIELD_FIELD(u64 en:1,
-	  ;))
-	} s;
-};
-
-union cvmx_smix_rd_dat {
-	u64 u64;
-	struct cvmx_smix_rd_dat_s {
-	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_18_63:46,
-	  OCT_MDIO_BITFIELD_FIELD(u64 pending:1,
-	  OCT_MDIO_BITFIELD_FIELD(u64 val:1,
-	  OCT_MDIO_BITFIELD_FIELD(u64 dat:16,
-	  ;))))
-	} s;
-};
-
-union cvmx_smix_wr_dat {
-	u64 u64;
-	struct cvmx_smix_wr_dat_s {
-	  OCT_MDIO_BITFIELD_FIELD(u64 reserved_18_63:46,
-	  OCT_MDIO_BITFIELD_FIELD(u64 pending:1,
-	  OCT_MDIO_BITFIELD_FIELD(u64 val:1,
-	  OCT_MDIO_BITFIELD_FIELD(u64 dat:16,
-	  ;))))
-	} s;
-};
-
-enum octeon_mdiobus_mode {
-	UNINIT = 0,
-	C22,
-	C45
-};
-
-struct octeon_mdiobus {
-	struct mii_bus *mii_bus;
-	u64 register_base;
-	resource_size_t mdio_phys;
-	resource_size_t regsize;
-	enum octeon_mdiobus_mode mode;
-};
-
-#ifdef CONFIG_CAVIUM_OCTEON_SOC
-static void oct_mdio_writeq(u64 val, u64 addr)
-{
-	cvmx_write_csr(addr, val);
-}
-
-static u64 oct_mdio_readq(u64 addr)
-{
-	return cvmx_read_csr(addr);
-}
-#else
-#define oct_mdio_writeq(val, addr)	writeq_relaxed(val, (void *)addr)
-#define oct_mdio_readq(addr)		readq_relaxed((void *)addr)
-#endif
-
-static void octeon_mdiobus_set_mode(struct octeon_mdiobus *p,
-				    enum octeon_mdiobus_mode m)
-{
-	union cvmx_smix_clk smi_clk;
-
-	if (m == p->mode)
-		return;
-
-	smi_clk.u64 = oct_mdio_readq(p->register_base + SMI_CLK);
-	smi_clk.s.mode = (m == C45) ? 1 : 0;
-	smi_clk.s.preamble = 1;
-	oct_mdio_writeq(smi_clk.u64, p->register_base + SMI_CLK);
-	p->mode = m;
-}
-
-static int octeon_mdiobus_c45_addr(struct octeon_mdiobus *p,
-				   int phy_id, int regnum)
-{
-	union cvmx_smix_cmd smi_cmd;
-	union cvmx_smix_wr_dat smi_wr;
-	int timeout = 1000;
-
-	octeon_mdiobus_set_mode(p, C45);
-
-	smi_wr.u64 = 0;
-	smi_wr.s.dat = regnum & 0xffff;
-	oct_mdio_writeq(smi_wr.u64, p->register_base + SMI_WR_DAT);
-
-	regnum = (regnum >> 16) & 0x1f;
-
-	smi_cmd.u64 = 0;
-	smi_cmd.s.phy_op = 0; /* MDIO_CLAUSE_45_ADDRESS */
-	smi_cmd.s.phy_adr = phy_id;
-	smi_cmd.s.reg_adr = regnum;
-	oct_mdio_writeq(smi_cmd.u64, p->register_base + SMI_CMD);
-
-	do {
-		/* Wait 1000 clocks so we don't saturate the RSL bus
-		 * doing reads.
-		 */
-		__delay(1000);
-		smi_wr.u64 = oct_mdio_readq(p->register_base + SMI_WR_DAT);
-	} while (smi_wr.s.pending && --timeout);
-
-	if (timeout <= 0)
-		return -EIO;
-	return 0;
-}
-
-static int octeon_mdiobus_read(struct mii_bus *bus, int phy_id, int regnum)
-{
-	struct octeon_mdiobus *p = bus->priv;
-	union cvmx_smix_cmd smi_cmd;
-	union cvmx_smix_rd_dat smi_rd;
-	unsigned int op = 1; /* MDIO_CLAUSE_22_READ */
-	int timeout = 1000;
-
-	if (regnum & MII_ADDR_C45) {
-		int r = octeon_mdiobus_c45_addr(p, phy_id, regnum);
-		if (r < 0)
-			return r;
-
-		regnum = (regnum >> 16) & 0x1f;
-		op = 3; /* MDIO_CLAUSE_45_READ */
-	} else {
-		octeon_mdiobus_set_mode(p, C22);
-	}
-
-
-	smi_cmd.u64 = 0;
-	smi_cmd.s.phy_op = op;
-	smi_cmd.s.phy_adr = phy_id;
-	smi_cmd.s.reg_adr = regnum;
-	oct_mdio_writeq(smi_cmd.u64, p->register_base + SMI_CMD);
-
-	do {
-		/* Wait 1000 clocks so we don't saturate the RSL bus
-		 * doing reads.
-		 */
-		__delay(1000);
-		smi_rd.u64 = oct_mdio_readq(p->register_base + SMI_RD_DAT);
-	} while (smi_rd.s.pending && --timeout);
-
-	if (smi_rd.s.val)
-		return smi_rd.s.dat;
-	else
-		return -EIO;
-}
-
-static int octeon_mdiobus_write(struct mii_bus *bus, int phy_id,
-				int regnum, u16 val)
-{
-	struct octeon_mdiobus *p = bus->priv;
-	union cvmx_smix_cmd smi_cmd;
-	union cvmx_smix_wr_dat smi_wr;
-	unsigned int op = 0; /* MDIO_CLAUSE_22_WRITE */
-	int timeout = 1000;
-
-
-	if (regnum & MII_ADDR_C45) {
-		int r = octeon_mdiobus_c45_addr(p, phy_id, regnum);
-		if (r < 0)
-			return r;
-
-		regnum = (regnum >> 16) & 0x1f;
-		op = 1; /* MDIO_CLAUSE_45_WRITE */
-	} else {
-		octeon_mdiobus_set_mode(p, C22);
-	}
-
-	smi_wr.u64 = 0;
-	smi_wr.s.dat = val;
-	oct_mdio_writeq(smi_wr.u64, p->register_base + SMI_WR_DAT);
-
-	smi_cmd.u64 = 0;
-	smi_cmd.s.phy_op = op;
-	smi_cmd.s.phy_adr = phy_id;
-	smi_cmd.s.reg_adr = regnum;
-	oct_mdio_writeq(smi_cmd.u64, p->register_base + SMI_CMD);
-
-	do {
-		/* Wait 1000 clocks so we don't saturate the RSL bus
-		 * doing reads.
-		 */
-		__delay(1000);
-		smi_wr.u64 = oct_mdio_readq(p->register_base + SMI_WR_DAT);
-	} while (smi_wr.s.pending && --timeout);
-
-	if (timeout <= 0)
-		return -EIO;
-
-	return 0;
-}
+#include "mdio-cavium.h"
 
 static int octeon_mdiobus_probe(struct platform_device *pdev)
 {
-	struct octeon_mdiobus *bus;
+	struct cavium_mdiobus *bus;
 	struct mii_bus *mii_bus;
 	struct resource *res_mem;
+	resource_size_t mdio_phys;
+	resource_size_t regsize;
 	union cvmx_smix_en smi_en;
 	int err = -ENOENT;
 
@@ -284,17 +38,17 @@ static int octeon_mdiobus_probe(struct platform_device *pdev)
 
 	bus = mii_bus->priv;
 	bus->mii_bus = mii_bus;
-	bus->mdio_phys = res_mem->start;
-	bus->regsize = resource_size(res_mem);
+	mdio_phys = res_mem->start;
+	regsize = resource_size(res_mem);
 
-	if (!devm_request_mem_region(&pdev->dev, bus->mdio_phys, bus->regsize,
+	if (!devm_request_mem_region(&pdev->dev, mdio_phys, regsize,
 				     res_mem->name)) {
 		dev_err(&pdev->dev, "request_mem_region failed\n");
 		return -ENXIO;
 	}
 
 	bus->register_base =
-		(u64)devm_ioremap(&pdev->dev, bus->mdio_phys, bus->regsize);
+		(u64)devm_ioremap(&pdev->dev, mdio_phys, regsize);
 	if (!bus->register_base) {
 		dev_err(&pdev->dev, "dev_ioremap failed\n");
 		return -ENOMEM;
@@ -304,13 +58,12 @@ static int octeon_mdiobus_probe(struct platform_device *pdev)
 	smi_en.s.en = 1;
 	oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN);
 
-	bus->mii_bus->priv = bus;
-	bus->mii_bus->name = "mdio-octeon";
+	bus->mii_bus->name = KBUILD_MODNAME;
 	snprintf(bus->mii_bus->id, MII_BUS_ID_SIZE, "%llx", bus->register_base);
 	bus->mii_bus->parent = &pdev->dev;
 
-	bus->mii_bus->read = octeon_mdiobus_read;
-	bus->mii_bus->write = octeon_mdiobus_write;
+	bus->mii_bus->read = cavium_mdiobus_read;
+	bus->mii_bus->write = cavium_mdiobus_write;
 
 	platform_set_drvdata(pdev, bus);
 
@@ -318,7 +71,7 @@ static int octeon_mdiobus_probe(struct platform_device *pdev)
 	if (err)
 		goto fail_register;
 
-	dev_info(&pdev->dev, "Version " DRV_VERSION "\n");
+	dev_info(&pdev->dev, "Probed\n");
 
 	return 0;
 fail_register:
@@ -330,7 +83,7 @@ fail_register:
 
 static int octeon_mdiobus_remove(struct platform_device *pdev)
 {
-	struct octeon_mdiobus *bus;
+	struct cavium_mdiobus *bus;
 	union cvmx_smix_en smi_en;
 
 	bus = platform_get_drvdata(pdev);
@@ -352,7 +105,7 @@ MODULE_DEVICE_TABLE(of, octeon_mdiobus_match);
 
 static struct platform_driver octeon_mdiobus_driver = {
 	.driver = {
-		.name		= "mdio-octeon",
+		.name		= KBUILD_MODNAME,
 		.of_match_table = octeon_mdiobus_match,
 	},
 	.probe		= octeon_mdiobus_probe,
@@ -367,7 +120,6 @@ EXPORT_SYMBOL(octeon_mdiobus_force_mod_depencency);
 
 module_platform_driver(octeon_mdiobus_driver);
 
-MODULE_DESCRIPTION(DRV_DESCRIPTION);
-MODULE_VERSION(DRV_VERSION);
+MODULE_DESCRIPTION("Cavium OCTEON MDIO bus driver");
 MODULE_AUTHOR("David Daney");
 MODULE_LICENSE("GPL");
-- 
1.7.11.7

[PATCH 3/3] phy: mdio-thunder: Add driver for Cavium Thunder SoC MDIO buses.

[David Daney]

From: David Daney <david.daney@cavium.com>

The Cavium Thunder SoCs have multiple MIDO buses that are part of a
single PCI device.  To model this in the device tree we call the PCI
parent device a "cavium,thunder-8890-mdio-nexus", it has several
children, one for each MDIO bus.

The MDIO bus hardware is identical to that found in the OCTEON SoCs,
so we use that code for things that are not part of the PCI driver
probe/remove

Signed-off-by: David Daney <david.daney@cavium.com>
---
 .../devicetree/bindings/net/cavium-mdio.txt        |  61 +++++++-
 drivers/net/phy/Kconfig                            |  11 ++
 drivers/net/phy/Makefile                           |   1 +
 drivers/net/phy/mdio-thunder.c                     | 154 +++++++++++++++++++++
 4 files changed, 225 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/phy/mdio-thunder.c

diff --git a/Documentation/devicetree/bindings/net/cavium-mdio.txt b/Documentation/devicetree/bindings/net/cavium-mdio.txt
index 04cb749..020df08 100644
--- a/Documentation/devicetree/bindings/net/cavium-mdio.txt
+++ b/Documentation/devicetree/bindings/net/cavium-mdio.txt
@@ -1,9 +1,12 @@
 * System Management Interface (SMI) / MDIO
 
 Properties:
-- compatible: "cavium,octeon-3860-mdio"
+- compatible: One of:
 
-  Compatibility with all cn3XXX, cn5XXX and cn6XXX SOCs.
+   "cavium,octeon-3860-mdio": Compatibility with all cn3XXX, cn5XXX
+                       and cn6XXX SOCs.
+
+   "cavium,thunder-8890-mdio": Compatibility with all cn8XXX SOCs.
 
 - reg: The base address of the MDIO bus controller register bank.
 
@@ -25,3 +28,57 @@ Example:
 			reg = <0>;
 		};
 	};
+
+
+* System Management Interface (SMI) / MDIO Nexus
+
+  Several mdio buses may be gathered as children of a single PCI
+  device, this PCI device is the nexus of the buses.
+
+Properties:
+
+- compatible: "cavium,thunder-8890-mdio-nexus";
+
+- reg: The PCI device and function numbers of the nexus device.
+
+- #address-cells: Must be <2>.
+
+- #size-cells: Must be <2>.
+
+- ranges: As needed for mapping of the MDIO bus device registers.
+
+- assigned-addresses: As needed for mapping of the MDIO bus device registers.
+
+Example:
+
+        mdio-nexus@1,3 {
+                compatible = "cavium,thunder-8890-mdio-nexus";
+                #address-cells = <2>;
+                #size-cells = <2>;
+                reg = <0x0b00 0 0 0 0>; /* DEVFN = 0x0b (1:3) */
+                assigned-addresses = <0x03000000 0x87e0 0x05000000 0x0 0x800000>;
+                ranges = <0x87e0 0x05000000 0x03000000 0x87e0 0x05000000 0x0 0x800000>;
+
+                mdio0@87e0,05003800 {
+                        compatible = "cavium,thunder-8890-mdio";
+                        #address-cells = <1>;
+                        #size-cells = <0>;
+                        reg = <0x87e0 0x05003800 0x0 0x30>;
+
+                        ethernet-phy@0 {
+                                ...
+                                reg = <0>;
+                        };
+                };
+                mdio0@87e0,05003880 {
+                        compatible = "cavium,thunder-8890-mdio";
+                        #address-cells = <1>;
+                        #size-cells = <0>;
+                        reg = <0x87e0 0x05003880 0x0 0x30>;
+
+                        ethernet-phy@0 {
+                                ...
+                                reg = <0>;
+                        };
+                };
+        };
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 40faec9..075a4cc 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -196,6 +196,17 @@ config MDIO_OCTEON
 	  buses. It is required by the Octeon and ThunderX ethernet device
 	  drivers on some systems.
 
+config MDIO_THUNDER
+	tristate "Support for MDIO buses on on ThunderX SOCs"
+	depends on 64BIT
+	depends on PCI
+	select MDIO_CAVIUM
+	help
+	  This driver supports the MDIO interfaces found on Cavium
+	  ThunderX SoCs when the MDIO bus device appears on as a PCI
+	  device.
+
+
 config MDIO_SUN4I
 	tristate "Allwinner sun4i MDIO interface support"
 	depends on ARCH_SUNXI
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 041b3d9..fcdbb92 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_DP83867_PHY)	+= dp83867.o
 obj-$(CONFIG_STE10XP)		+= ste10Xp.o
 obj-$(CONFIG_MICREL_PHY)	+= micrel.o
 obj-$(CONFIG_MDIO_OCTEON)	+= mdio-octeon.o
+obj-$(CONFIG_MDIO_THUNDER)	+= mdio-thunder.o
 obj-$(CONFIG_MDIO_CAVIUM)	+= mdio-cavium.o
 obj-$(CONFIG_MICREL_KS8995MA)	+= spi_ks8995.o
 obj-$(CONFIG_AT803X_PHY)	+= at803x.o
diff --git a/drivers/net/phy/mdio-thunder.c b/drivers/net/phy/mdio-thunder.c
new file mode 100644
index 0000000..5646169
--- /dev/null
+++ b/drivers/net/phy/mdio-thunder.c
@@ -0,0 +1,154 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (C) 2009-2016 Cavium, Inc.
+ */
+
+#include <linux/of_address.h>
+#include <linux/of_mdio.h>
+#include <linux/module.h>
+#include <linux/gfp.h>
+#include <linux/phy.h>
+#include <linux/io.h>
+#include <linux/acpi.h>
+#include <linux/pci.h>
+
+#include "mdio-cavium.h"
+
+struct thunder_mdiobus_nexus {
+	void __iomem *bar0;
+	struct cavium_mdiobus *buses[4];
+};
+
+static int thunder_mdiobus_pci_probe(struct pci_dev *pdev,
+				     const struct pci_device_id *ent)
+{
+	struct device_node *node;
+	struct fwnode_handle *fwn;
+	struct thunder_mdiobus_nexus *nexus;
+	int err;
+	int i;
+
+	nexus = devm_kzalloc(&pdev->dev, sizeof(*nexus), GFP_KERNEL);
+	if (!nexus)
+		return -ENOMEM;
+
+	pci_set_drvdata(pdev, nexus);
+
+	err = pcim_enable_device(pdev);
+	if (err) {
+		dev_err(&pdev->dev, "Failed to enable PCI device\n");
+		pci_set_drvdata(pdev, NULL);
+		return err;
+	}
+
+	err = pci_request_regions(pdev, KBUILD_MODNAME);
+	if (err) {
+		dev_err(&pdev->dev, "pci_request_regions failed\n");
+		goto err_disable_device;
+	}
+
+	nexus->bar0 = pcim_iomap(pdev, 0, pci_resource_len(pdev, 0));
+	if (!nexus->bar0) {
+		err = -ENOMEM;
+		goto err_release_regions;
+	}
+
+	i = 0;
+	device_for_each_child_node(&pdev->dev, fwn) {
+		struct resource r;
+		struct mii_bus *mii_bus;
+		struct cavium_mdiobus *bus;
+		union cvmx_smix_en smi_en;
+
+		/* If it is not an OF node we cannot handle it yet, so
+		 * exit the loop.
+		 */
+		node = to_of_node(fwn);
+		if (!node)
+			break;
+
+		err = of_address_to_resource(node, 0, &r);
+		if (err) {
+			dev_err(&pdev->dev,
+				"Couldn't translate address for \"%s\"\n",
+				node->name);
+			break;
+		}
+
+		mii_bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*bus));
+		if (!mii_bus)
+			break;
+		bus = mii_bus->priv;
+		bus->mii_bus = mii_bus;
+
+		nexus->buses[i] = bus;
+		i++;
+
+		bus->register_base = (u64)nexus->bar0 +
+			r.start - pci_resource_start(pdev, 0);
+
+		smi_en.u64 = 0;
+		smi_en.s.en = 1;
+		oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN);
+		bus->mii_bus->name = KBUILD_MODNAME;
+		snprintf(bus->mii_bus->id, MII_BUS_ID_SIZE, "%llx", r.start);
+		bus->mii_bus->parent = &pdev->dev;
+		bus->mii_bus->read = cavium_mdiobus_read;
+		bus->mii_bus->write = cavium_mdiobus_write;
+
+		err = of_mdiobus_register(bus->mii_bus, node);
+		if (err)
+			dev_err(&pdev->dev, "of_mdiobus_register failed\n");
+
+		dev_info(&pdev->dev, "Added bus at %llx\n", r.start);
+		if (i >= ARRAY_SIZE(nexus->buses))
+			break;
+	}
+	return 0;
+
+err_release_regions:
+	pci_release_regions(pdev);
+
+err_disable_device:
+	pci_set_drvdata(pdev, NULL);
+	return err;
+}
+
+static void thunder_mdiobus_pci_remove(struct pci_dev *pdev)
+{
+	int i;
+	struct thunder_mdiobus_nexus *nexus = pci_get_drvdata(pdev);
+
+	for (i = 0; i < ARRAY_SIZE(nexus->buses); i++) {
+		struct cavium_mdiobus *bus = nexus->buses[i];
+
+		if (!bus)
+			continue;
+
+		mdiobus_unregister(bus->mii_bus);
+		mdiobus_free(bus->mii_bus);
+		oct_mdio_writeq(0, bus->register_base + SMI_EN);
+	}
+	pci_set_drvdata(pdev, NULL);
+}
+
+static const struct pci_device_id thunder_mdiobus_id_table[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, 0xa02b) },
+	{ 0, } /* End of table. */
+};
+MODULE_DEVICE_TABLE(pci, thunder_mdiobus_id_table);
+
+static struct pci_driver thunder_mdiobus_driver = {
+	.name = KBUILD_MODNAME,
+	.id_table = thunder_mdiobus_id_table,
+	.probe = thunder_mdiobus_pci_probe,
+	.remove = thunder_mdiobus_pci_remove,
+};
+
+module_pci_driver(thunder_mdiobus_driver);
+
+MODULE_DESCRIPTION("Cavium ThunderX MDIO bus driver");
+MODULE_LICENSE("GPL");
-- 
1.7.11.7

Re: [PATCH 1/3] net: thunderx: Cleanup PHY probing code.

[Andrew Lunn]

> +		phy_np = of_parse_phandle(node, "phy-handle", 0);
> +		/* If there is no phy or defective firmware presents
> +		 * this cortina phy, for which there is no driver
> +		 * support, ignore it.
> +		 */
> +		if (phy_np &&
> +		    !of_device_is_compatible(phy_np, "cortina,cs4223-slice")) {

Hi David

What is a cortina,cs4223-slice, and why does it need to be handled differently?

Thanks
	Andrew

Re: [PATCH 1/3] net: thunderx: Cleanup PHY probing code.

[David Daney]

On 03/11/2016 09:31 AM, Andrew Lunn wrote:
>> +		phy_np = of_parse_phandle(node, "phy-handle", 0);
>> +		/* If there is no phy or defective firmware presents
>> +		 * this cortina phy, for which there is no driver
>> +		 * support, ignore it.
>> +		 */
>> +		if (phy_np &&
>> +		    !of_device_is_compatible(phy_np, "cortina,cs4223-slice")) {
>
> Hi David
>
> What is a cortina,cs4223-slice,

It is 1/4 of:

https://www.inphi.com/products/cs4223.php

> and why does it need to be handled differently?
>

$ ls drivers/net/phy/*cortina*
ls: cannot access drivers/net/phy/*cortina*: No such file or directory

For this configuration of thunder_bgx.c, the use of a Linux PHY driver 
is optional.

The firmware should probably not specify a PHY here, but it does so we 
ignore it so we don't wait around forever for the non-existent driver to 
bind.

Re: [PATCH 1/3] net: thunderx: Cleanup PHY probing code.

[Andrew Lunn]

On Fri, Mar 11, 2016 at 09:41:06AM -0800, David Daney wrote:
> On 03/11/2016 09:31 AM, Andrew Lunn wrote:
> >>+		phy_np = of_parse_phandle(node, "phy-handle", 0);
> >>+		/* If there is no phy or defective firmware presents
> >>+		 * this cortina phy, for which there is no driver
> >>+		 * support, ignore it.
> >>+		 */
> >>+		if (phy_np &&
> >>+		    !of_device_is_compatible(phy_np, "cortina,cs4223-slice")) {
> >
> >Hi David
> >
> >What is a cortina,cs4223-slice,
> 
> It is 1/4 of:
> 
> https://www.inphi.com/products/cs4223.php
> 
> >and why does it need to be handled differently?
> >
> 
> $ ls drivers/net/phy/*cortina*
> ls: cannot access drivers/net/phy/*cortina*: No such file or directory
> 
> For this configuration of thunder_bgx.c, the use of a Linux PHY
> driver is optional.
> 
> The firmware should probably not specify a PHY here, but it does so
> we ignore it so we don't wait around forever for the non-existent
> driver to bind.
 
Hi David

I don't see why it should wait around forever. I have boards with
Marvell PHYs, yet if i don't build the Marvell driver, the Ethernet
driver still loads, because the generic PHY driver is used instead.
Why does this not work here?

Also, Documentation/devicetree/bindings/net/phy.txt says:

 compatible: Compatible list, may contain
  "ethernet-phy-ieee802.3-c22" or "ethernet-phy-ieee802.3-c45" for
  PHYs that implement IEEE802.3 clause 22 or IEEE802.3 clause 45
  specifications. If neither of these are specified, the default is to
  assume clause 22.

  If the phy's identifier is known then the list may contain an entry
  of the form: "ethernet-phy-idAAAA.BBBB" where
     AAAA - The value of the 16 bit Phy Identifier 1 register as
            4 hex digits. This is the chip vendor OUI bits 3:18
     BBBB - The value of the 16 bit Phy Identifier 2 register as
            4 hex digits. This is the chip vendor OUI bits 19:24,
            followed by 10 bits of a vendor specific ID.

  The compatible list should not contain other values than those
  listed here.

So having "cortina,cs4223-slice" in the compatible string goes against
the binding. To make this work, you probably need to extend the
whitelist_phys list in of_mdio.c.

       Andrew

Re: [PATCH 1/3] net: thunderx: Cleanup PHY probing code.

[David Daney]

On 03/11/2016 10:00 AM, Andrew Lunn wrote:
> On Fri, Mar 11, 2016 at 09:41:06AM -0800, David Daney wrote:
>> On 03/11/2016 09:31 AM, Andrew Lunn wrote:
>>>> +		phy_np = of_parse_phandle(node, "phy-handle", 0);
>>>> +		/* If there is no phy or defective firmware presents
>>>> +		 * this cortina phy, for which there is no driver
>>>> +		 * support, ignore it.
>>>> +		 */
>>>> +		if (phy_np &&
>>>> +		    !of_device_is_compatible(phy_np, "cortina,cs4223-slice")) {
>>>
>>> Hi David
>>>
>>> What is a cortina,cs4223-slice,
>>
>> It is 1/4 of:
>>
>> https://www.inphi.com/products/cs4223.php
>>
>>> and why does it need to be handled differently?
>>>
>>
>> $ ls drivers/net/phy/*cortina*
>> ls: cannot access drivers/net/phy/*cortina*: No such file or directory
>>
>> For this configuration of thunder_bgx.c, the use of a Linux PHY
>> driver is optional.
>>
>> The firmware should probably not specify a PHY here, but it does so
>> we ignore it so we don't wait around forever for the non-existent
>> driver to bind.
>
> Hi David
>
> I don't see why it should wait around forever. I have boards with
> Marvell PHYs, yet if i don't build the Marvell driver, the Ethernet
> driver still loads, because the generic PHY driver is used instead.
> Why does this not work here?

As I said before, there is no driver for the device, so 
of_phy_find_device() will always return NULL.

It appears that the architects of the cs4223 were not familiar with the 
various IEEE specifications for PHYs, so unsurprisingly, the device 
cannot be handled by any standards compliant generic drivers.

The Marvell PHY architects, on the other hand, seem to have designed 
their devices with a keen eye for following the standards.  So, also 
unsurprisingly, the Marvell devices work perfectly with the generic drivers.

>
> Also, Documentation/devicetree/bindings/net/phy.txt says:
>
>   compatible: Compatible list, may contain
>    "ethernet-phy-ieee802.3-c22" or "ethernet-phy-ieee802.3-c45" for
>    PHYs that implement IEEE802.3 clause 22 or IEEE802.3 clause 45
>    specifications. If neither of these are specified, the default is to
>    assume clause 22.
>
>    If the phy's identifier is known then the list may contain an entry
>    of the form: "ethernet-phy-idAAAA.BBBB" where
>       AAAA - The value of the 16 bit Phy Identifier 1 register as
>              4 hex digits. This is the chip vendor OUI bits 3:18
>       BBBB - The value of the 16 bit Phy Identifier 2 register as
>              4 hex digits. This is the chip vendor OUI bits 19:24,
>              followed by 10 bits of a vendor specific ID.
>
>    The compatible list should not contain other values than those
>    listed here.
>
> So having "cortina,cs4223-slice" in the compatible string goes against
> the binding.

Yes, you are probably correct.  That is why the comment in the patch 
explains the strategy of ignoring this node that is not complaint with 
the binding document.


> To make this work, you probably need to extend the
> whitelist_phys list in of_mdio.c.

That wouldn't work.  There is no driver for the PHY, putting it on a 
white list doesn't solve the missing driver problem.

Re: [PATCH 1/3] net: thunderx: Cleanup PHY probing code.

[Andrew Lunn]

> >I don't see why it should wait around forever. I have boards with
> >Marvell PHYs, yet if i don't build the Marvell driver, the Ethernet
> >driver still loads, because the generic PHY driver is used instead.
> >Why does this not work here?
> 
> As I said before, there is no driver for the device, so
> of_phy_find_device() will always return NULL.

I'm not yet convinced this is true. I really do expect that the
generic PHY driver will bind to it. It might then go horribly wrong,
because it is not standard compliant, but that is a different issue.

The generic driver should probably have a black list for such devices.
This is a PHY issue, not an MDIO issue, and the problem should be
solved in the PHY layer, not in one MDIO driver.

We should also consider what happens when somebody actually writes a
driver for this PHY. Are you not going to use it?

Before this patchset, you did not special case this compatible
string. So at the very least, you need to split this into a separate
patch, so the maintainers can ACK/NACK it, independent of the other
change it is embedded in.

       Andrew

Re: [PATCH 1/3] net: thunderx: Cleanup PHY probing code.

[David Daney]

On 03/11/2016 11:06 AM, Andrew Lunn wrote:
>>> I don't see why it should wait around forever. I have boards with
>>> Marvell PHYs, yet if i don't build the Marvell driver, the Ethernet
>>> driver still loads, because the generic PHY driver is used instead.
>>> Why does this not work here?
>>
>> As I said before, there is no driver for the device, so
>> of_phy_find_device() will always return NULL.
>
> I'm not yet convinced this is true.

Which part don't you believe?  Is it:

   - there is no driver for the device.

or

   - for PHYs with no driver, of_phy_find_device() will return NULL

> I really do expect that the
> generic PHY driver will bind to it. It might then go horribly wrong,
> because it is not standard compliant, but that is a different issue.
>

At a higher level, the way we handle either of:

   - Lack of a driver.

   - "Horribly wrong" driver

is the same, we cannot use a PHY driver.

> The generic driver should probably have a black list for such devices.
> This is a PHY issue, not an MDIO issue, and the problem should be
> solved in the PHY layer, not in one MDIO driver.

This isn't an MDIO driver patch.  This is more about handling a 
defective device tree in the only driver (a non-MDIO driver) that will 
ever see such a device tree node.

>
> We should also consider what happens when somebody actually writes a
> driver for this PHY.   Are you not going to use it?

Easy to answer: We remove the "&& !of_device_is_compatible(phy_np, 
"cortina,cs4223-slice")" clause from this driver.

>
> Before this patchset, you did not special case this compatible
> string. So at the very least, you need to split this into a separate
> patch, so the maintainers can ACK/NACK it, independent of the other
> change it is embedded in.
>

I can, and will, do that.

Thanks,
David Daney

Re: [PATCH 1/3] net: thunderx: Cleanup PHY probing code.

[Florian Fainelli]

On 11/03/16 11:06, Andrew Lunn wrote:
>>> I don't see why it should wait around forever. I have boards with
>>> Marvell PHYs, yet if i don't build the Marvell driver, the Ethernet
>>> driver still loads, because the generic PHY driver is used instead.
>>> Why does this not work here?
>>
>> As I said before, there is no driver for the device, so
>> of_phy_find_device() will always return NULL.
> 
> I'm not yet convinced this is true. I really do expect that the
> generic PHY driver will bind to it. It might then go horribly wrong,
> because it is not standard compliant, but that is a different issue.

I concur with Andrew here, unless the PHY is guaranteed to return
garbage when get_phy_id() is called, there is a good chance that the
Generic PHY driver will be bound to this PHY device, or this is not
happening for you for some reason?

> 
> The generic driver should probably have a black list for such devices.
> This is a PHY issue, not an MDIO issue, and the problem should be
> solved in the PHY layer, not in one MDIO driver.

I considered the possibility once of disabling the generic PHY driver,
such that systems where the vendor-specific PHY driver is expected to be
used could utilize that. That does not play well with the fixed PHYs
using the generic PHY driver though, anyway, I am digressing.

> 
> We should also consider what happens when somebody actually writes a
> driver for this PHY. Are you not going to use it?
> 
> Before this patchset, you did not special case this compatible
> string. So at the very least, you need to split this into a separate
> patch, so the maintainers can ACK/NACK it, independent of the other
> change it is embedded in.
> 
>        Andrew
> 


-- 
Florian

Re: [PATCH 1/3] net: thunderx: Cleanup PHY probing code.

[David Daney]

On 03/11/2016 11:37 AM, Florian Fainelli wrote:
> On 11/03/16 11:06, Andrew Lunn wrote:
>>>> I don't see why it should wait around forever. I have boards with
>>>> Marvell PHYs, yet if i don't build the Marvell driver, the Ethernet
>>>> driver still loads, because the generic PHY driver is used instead.
>>>> Why does this not work here?
>>>
>>> As I said before, there is no driver for the device, so
>>> of_phy_find_device() will always return NULL.
>>
>> I'm not yet convinced this is true. I really do expect that the
>> generic PHY driver will bind to it. It might then go horribly wrong,
>> because it is not standard compliant, but that is a different issue.
>
> I concur with Andrew here, unless the PHY is guaranteed to return
> garbage when get_phy_id() is called, there is a good chance that the
> Generic PHY driver will be bound to this PHY device, or this is not
> happening for you for some reason?
>

get_phy_id() is working a designed.

For this phy, we have:

   compatible = "cortina,cs4223-slice";

Therefore get_phy_id() is being called with a is_c45 value of false.

get_phy_id() is returning a value of 0, which means that it succeeds, 
but the returned phy_id is 0xffffffff, which causes get_phy_device() to 
not create a phy_device, and no driver can be bound.

I know you are all skeptical, but I really think the best thing to do is 
not try to attach a phy driver when this compatible value is encountered.

It is a defective device tree, and I am attempting to handle it in the 
specific site where it can cause problems.

We are trying to distinguish between these two cases:

  - of_phy_find_device() returns NULL because driver is not yet bound

  - of_phy_find_device() returns NULL because "cortina,cs4223-slice"

I don't think we need to build some sort of frame work to handle things 
like this in a general way.

David Daney

Re: [PATCH 1/3] net: thunderx: Cleanup PHY probing code.

[Andrew Lunn]

> For this phy, we have:
> 
>   compatible = "cortina,cs4223-slice";

That actually means something else is happening, i think.

of_mdiobus_register() looks at the children, and decides if each child
is a phy or an mdio device, by calling of_mdiobus_child_is_phy().
Since this compatible string is not in whitelist_phys[], it will
return false. of_mdiobus_register() will then do a
of_mdiobus_register_device(). This compatible means it is an MDIO
device, not a PHY. So when you later call of_phy_find_device() it is
always going to return NULL, because there is no PHY there, only an
MDIO device.

How usable is the hardware without a PHY driver? Is a better solution
that your write a very minimal PHY driver?

     Andrew

Re: [PATCH 1/3] net: thunderx: Cleanup PHY probing code.

[David Daney]

On 03/11/2016 01:35 PM, Andrew Lunn wrote:
[...]
> How usable is the hardware without a PHY driver?

The hardware has always in the past, still does, and probably always 
will work fine without a PHY driver.  Link up/down are correctly handled.

> Is a better solution
> that your write a very minimal PHY driver?

No.  Nothing would be gained.

All we are trying to do, is allow for loading of 1G PHY drivers via the 
-EPROBE_DEFER mechanism while continuing to allow the 10G and 40G ports 
to function without a PHY driver.

Specifically, we are *not* attempting to solve the problem of 
re-architecting the kernel phy_device infrastructure so that it would be 
possible to write a Cortina PHYs driver.  Nor are we proposing that a 
Cortina PHY driver be written that would fit into the current 
infrastructure.

To this end, I still think the current patch takes the best approach.

Thanks,
David Daney