All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v3 0/2]     Add MDIO driver model support
@ 2018-06-13  4:33 make at marvell.com
  2018-06-13  4:33 ` [U-Boot] [PATCH v3 1/2] dm: mdio: add a uclass for MDIO make at marvell.com
  2018-06-13  4:33 ` [U-Boot] [PATCH v3 2/2] mdio: add marvell MDIO driver make at marvell.com
  0 siblings, 2 replies; 10+ messages in thread
From: make at marvell.com @ 2018-06-13  4:33 UTC (permalink / raw)
  To: u-boot

From: Ken Ma <make@marvell.com>



Changes in v3:
- Move mdio uclass implementation to driver/net folder;
- Replace flat-tree functions with livetree functions and update codes
  and comments to be consistent with driver-model codes style;
- Put struct mii_dev to uclass platdata to avoid the mdio alloc and
  let driver model framework to alloc the memroy automatically,
  meanwhile the mii bus link initialization is added,
- Move marvell mdio driver to driver/net/mdio folder;
- Update codes according to mdio uclass implementation updates.

Changes in v2:
- Fix error printing:
  - Change some debug to pr_err;
  - mii bus has no parent member and it is not a udevice, so dev_err
    is changed to pr_err for mii bus error printings.

Ken Ma (2):
  dm: mdio: add a uclass for MDIO
  mdio: add marvell MDIO driver

 MAINTAINERS                                   |   2 +
 arch/arm/Kconfig                              |   1 +
 doc/device-tree-bindings/net/marvell-mdio.txt |  18 ++
 doc/device-tree-bindings/net/mdio-bus.txt     |  54 ++++++
 drivers/Kconfig                               |   2 +
 drivers/net/Makefile                          |   1 +
 drivers/net/mdio/Kconfig                      |  28 +++
 drivers/net/mdio/Makefile                     |   7 +
 drivers/net/mdio/mdio-uclass.c                | 112 ++++++++++++
 drivers/net/mdio/mvmdio.c                     | 234 ++++++++++++++++++++++++++
 include/dm/uclass-id.h                        |   1 +
 include/net/mdio.h                            |  62 +++++++
 12 files changed, 522 insertions(+)
 create mode 100644 doc/device-tree-bindings/net/marvell-mdio.txt
 create mode 100644 doc/device-tree-bindings/net/mdio-bus.txt
 create mode 100644 drivers/net/mdio/Kconfig
 create mode 100644 drivers/net/mdio/Makefile
 create mode 100644 drivers/net/mdio/mdio-uclass.c
 create mode 100644 drivers/net/mdio/mvmdio.c
 create mode 100644 include/net/mdio.h

-- 
1.9.1

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

* [U-Boot] [PATCH v3 1/2] dm: mdio: add a uclass for MDIO
  2018-06-13  4:33 [U-Boot] [PATCH v3 0/2] Add MDIO driver model support make at marvell.com
@ 2018-06-13  4:33 ` make at marvell.com
  2018-06-14 12:58   ` Simon Glass
                     ` (2 more replies)
  2018-06-13  4:33 ` [U-Boot] [PATCH v3 2/2] mdio: add marvell MDIO driver make at marvell.com
  1 sibling, 3 replies; 10+ messages in thread
From: make at marvell.com @ 2018-06-13  4:33 UTC (permalink / raw)
  To: u-boot

From: Ken Ma <make@marvell.com>

Add a uclass which provides access to MDIO busses and includes
operations required by MDIO.
The implementation is based on the existing mii/phy/mdio data
structures and APIs.
This patch also adds evice tree binding for MDIO bus.

Signed-off-by: Ken Ma <make@marvell.com>
Reviewed-by: sjg at chromium.org, joe.hershberger at ni.com
---

Changes in v3:
- Move mdio uclass implementation to driver/net folder;
- Replace flat-tree functions with livetree functions and update codes
  and comments to be consistent with driver-model codes style;
- Put struct mii_dev to uclass platdata to avoid the mdio alloc and
  let driver model framework to alloc the memroy automatically,
  meanwhile the mii bus link initialization is added,

Changes in v2: None

 MAINTAINERS                               |   1 +
 doc/device-tree-bindings/net/mdio-bus.txt |  54 ++++++++++++++
 drivers/Kconfig                           |   2 +
 drivers/net/Makefile                      |   1 +
 drivers/net/mdio/Kconfig                  |  18 +++++
 drivers/net/mdio/Makefile                 |   6 ++
 drivers/net/mdio/mdio-uclass.c            | 112 ++++++++++++++++++++++++++++++
 include/dm/uclass-id.h                    |   1 +
 include/net/mdio.h                        |  62 +++++++++++++++++
 9 files changed, 257 insertions(+)
 create mode 100644 doc/device-tree-bindings/net/mdio-bus.txt
 create mode 100644 drivers/net/mdio/Kconfig
 create mode 100644 drivers/net/mdio/Makefile
 create mode 100644 drivers/net/mdio/mdio-uclass.c
 create mode 100644 include/net/mdio.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 642c448..9e1fc83 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -335,6 +335,7 @@ M:	Simon Glass <sjg@chromium.org>
 S:	Maintained
 T:	git git://git.denx.de/u-boot-dm.git
 F:	drivers/core/
+F:	drivers/net/mdio/
 F:	include/dm/
 F:	test/dm/
 
diff --git a/doc/device-tree-bindings/net/mdio-bus.txt b/doc/device-tree-bindings/net/mdio-bus.txt
new file mode 100644
index 0000000..68d8b25
--- /dev/null
+++ b/doc/device-tree-bindings/net/mdio-bus.txt
@@ -0,0 +1,54 @@
+MDIO (Management Data Input/Output) busses
+
+MDIO busses can be described with a node for the MDIO master device
+and a set of child nodes for each phy on the bus.
+
+The MDIO node requires the following properties:
+- #address-cells  - number of cells required to define phy address on
+                    the MDIO bus.
+- #size-cells     - should be zero.
+- compatible      - name of MDIO bus controller following generic names
+                    recommended practice.
+- reg	          - address and length of the MDIO register.
+
+Optional property:
+- mdio-name       - MDIO bus name
+
+The child nodes of the MDIO driver are the individual PHY devices
+connected to this MDIO bus. They must have a "reg" property given the
+PHY address on the MDIO bus.
+- reg             - (required) phy address in MDIO bus.
+
+Example for cp110 MDIO node at the SoC level:
+	cp0_mdio: mdio at 12a200 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "marvell,orion-mdio";
+		reg = <0x12a200 0x10>;
+		mdio-name = "cp0-mdio";
+	};
+
+	cp0_xmdio: mdio at 12a600 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "marvell,xmdio";
+		reg = <0x12a600 0x200>;
+		mdio-name = "cp0-xmdio";
+	};
+
+And at the board level, example for armada-8040-mcbin board:
+	&cp0_mdio {
+		ge_phy: ethernet-phy at 0 {
+			reg = <0>;
+		};
+	};
+
+	&cp0_xmdio {
+		phy0: ethernet-phy at 0 {
+			reg = <0>;
+		};
+
+		phy8: ethernet-phy at 8 {
+			reg = <8>;
+		};
+	};
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 9e21b28..0e0982c 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -54,6 +54,8 @@ source "drivers/mtd/Kconfig"
 
 source "drivers/net/Kconfig"
 
+source "drivers/net/mdio/Kconfig"
+
 source "drivers/nvme/Kconfig"
 
 source "drivers/pci/Kconfig"
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 584bfdf..1cda03f 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -70,3 +70,4 @@ obj-$(CONFIG_VSC9953) += vsc9953.o
 obj-$(CONFIG_PIC32_ETH) += pic32_mdio.o pic32_eth.o
 obj-$(CONFIG_DWC_ETH_QOS) += dwc_eth_qos.o
 obj-$(CONFIG_FSL_PFE) += pfe_eth/
+obj-$(CONFIG_MDIO) += mdio/
diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
new file mode 100644
index 0000000..533cc4a
--- /dev/null
+++ b/drivers/net/mdio/Kconfig
@@ -0,0 +1,18 @@
+#
+# MDIO infrastructure and drivers
+#
+
+menu "MDIO Support"
+
+config MDIO
+	bool "Enable MDIO(Management Data Input/Output) drivers"
+	depends on DM
+	help
+	  Enable driver model for MDIO access.
+	  Drivers provide methods to management data
+	  Input/Output.
+	  MDIO uclass provides interfaces to get mdio
+	  udevice or mii bus from its child phy node or
+	  an ethernet udevice which the phy belongs to.
+
+endmenu
diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
new file mode 100644
index 0000000..45ae502
--- /dev/null
+++ b/drivers/net/mdio/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (C) 2018 Marvell International Ltd.
+# Author: Ken Ma<make@marvell.com>
+
+obj-$(CONFIG_MDIO) += mdio-uclass.o
diff --git a/drivers/net/mdio/mdio-uclass.c b/drivers/net/mdio/mdio-uclass.c
new file mode 100644
index 0000000..5a23d21
--- /dev/null
+++ b/drivers/net/mdio/mdio-uclass.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Marvell International Ltd.
+ * Author: Ken Ma<make@marvell.com>
+ */
+
+#include <common.h>
+#include <fdtdec.h>
+#include <errno.h>
+#include <dm.h>
+#include <dm/uclass.h>
+#include <dm/uclass-internal.h>
+#include <miiphy.h>
+#include <net/mdio.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int mdio_mii_bus_get(struct udevice *mdio_dev, struct mii_dev **busp)
+{
+	*busp = (struct mii_dev *)dev_get_uclass_platdata(mdio_dev);
+
+	return 0;
+}
+
+int mdio_device_get_from_phy(ofnode phy_node, struct udevice **devp)
+{
+	ofnode mdio_node;
+
+	mdio_node = ofnode_get_parent(phy_node);
+	return uclass_get_device_by_ofnode(UCLASS_MDIO, mdio_node, devp);
+}
+
+int mdio_mii_bus_get_from_phy(ofnode phy_node, struct mii_dev **busp)
+{
+	struct udevice *mdio_dev;
+	int ret;
+
+	ret = mdio_device_get_from_phy(phy_node, &mdio_dev);
+	if (ret)
+		return ret;
+
+	*busp = (struct mii_dev *)dev_get_uclass_platdata(mdio_dev);
+
+	return 0;
+}
+
+int mdio_device_get_from_eth(struct udevice *eth, struct udevice **devp)
+{
+	struct ofnode_phandle_args phy_args;
+	int ret;
+
+	ret = dev_read_phandle_with_args(eth, "phy", NULL, 0, 0, &phy_args);
+	if (!ret)
+		return mdio_device_get_from_phy(phy_args.node, devp);
+
+	/*
+	 * If there is no phy reference under the ethernet fdt node,
+	 * it is not an error since the ethernet device may do not use
+	 * mode; so in this case, the output mdio device pointer is set
+	 * as NULL.
+	 */
+	*devp = NULL;
+	return 0;
+}
+
+int mdio_mii_bus_get_from_eth(struct udevice *eth, struct mii_dev **busp)
+{
+	struct udevice *mdio_dev;
+	int ret;
+
+	ret = mdio_device_get_from_eth(eth, &mdio_dev);
+	if (ret)
+		return ret;
+
+	if (mdio_dev)
+		*busp = (struct mii_dev *)dev_get_uclass_platdata(mdio_dev);
+	else
+		*busp = NULL;
+
+	return 0;
+}
+
+static int mdio_uclass_pre_probe(struct udevice *dev)
+{
+	struct mii_dev *bus = (struct mii_dev *)dev_get_uclass_platdata(dev);
+	const char *name;
+
+	/* initialize mii_dev struct fields */
+	INIT_LIST_HEAD(&bus->link);
+
+	name = fdt_getprop(gd->fdt_blob, dev_of_offset(dev),
+			   "mdio-name", NULL);
+	if (name)
+		strncpy(bus->name, name, MDIO_NAME_LEN);
+
+	return 0;
+}
+
+static int mdio_uclass_post_probe(struct udevice *dev)
+{
+	struct mii_dev *bus = (struct mii_dev *)dev_get_uclass_platdata(dev);
+
+	return mdio_register(bus);
+}
+
+UCLASS_DRIVER(mdio) = {
+	.id		= UCLASS_MDIO,
+	.name		= "mdio",
+	.pre_probe	= mdio_uclass_pre_probe,
+	.post_probe	= mdio_uclass_post_probe,
+	.per_device_platdata_auto_alloc_size = sizeof(struct mii_dev),
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index d7f9df3..504decd 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -49,6 +49,7 @@ enum uclass_id {
 	UCLASS_LPC,		/* x86 'low pin count' interface */
 	UCLASS_MAILBOX,		/* Mailbox controller */
 	UCLASS_MASS_STORAGE,	/* Mass storage device */
+	UCLASS_MDIO,		/* Management Data Input/Output(MDIO) device */
 	UCLASS_MISC,		/* Miscellaneous device */
 	UCLASS_MMC,		/* SD / MMC card or chip */
 	UCLASS_MOD_EXP,		/* RSA Mod Exp device */
diff --git a/include/net/mdio.h b/include/net/mdio.h
new file mode 100644
index 0000000..5d8d703
--- /dev/null
+++ b/include/net/mdio.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2018 Marvell International Ltd.
+ * Author: Ken Ma<make@marvell.com>
+ */
+
+#ifndef _MDIO_H_
+#define _MDIO_H_
+
+#include <dm.h>	/* Because we dereference struct udevice here */
+#include <phy.h>
+
+/**
+ * mdio_mii_bus_get() - Get mii bus from mdio udevice
+ *
+ * @mdio_dev:	mdio udevice
+ * @busp:	returns mii bus
+ * @returns 0 on success, error code otherwise.
+ */
+int mdio_mii_bus_get(struct udevice *mdio_dev, struct mii_dev **busp);
+
+/**
+ * mdio_device_get_from_phy() - Get the mdio udevice which the phy belongs to
+ *
+ * @phy_node:	phy node offset
+ * @devp:	returns mdio udevice
+ * @returns 0 on success, error code otherwise.
+ */
+int mdio_device_get_from_phy(ofnode phy_node, struct udevice **devp);
+
+/**
+ * mdio_mii_bus_get_from_phy() - Get the mii bus which the phy belongs to
+ *
+ * @phy_node:	phy node offset
+ * @busp:	returns mii bus
+ * @returns 0 on success, error code otherwise.
+ */
+int mdio_mii_bus_get_from_phy(ofnode phy_node, struct mii_dev **busp);
+
+/**
+ * mdio_device_get_from_eth() - When there is a phy reference of "phy = <&...>"
+ *                      under an ethernet udevice fdt node, this function can
+ *                      get the mdio udevice which the phy belongs to
+ *
+ * @dev:	the ethernet udevice which contains the phy reference
+ * @devp:	returns mdio udevice
+ * @returns 0 on success, error code otherwise.
+ */
+int mdio_device_get_from_eth(struct udevice *eth, struct udevice **devp);
+
+/**
+ * mdio_mii_bus_get_from_eth() - When there is a phy reference of
+ *                      "phy = <&...>" under an ethernet udevice fdt node, this
+ *                      function can get the mii bus which the phy belongs to
+ *
+ * @eth:	the ethernet udevice which contains the phy reference
+ * @busp:	returns mii bus
+ * @returns 0 on success, error code otherwise.
+ */
+int mdio_mii_bus_get_from_eth(struct udevice *eth, struct mii_dev **busp);
+
+#endif /* _MDIO_H_ */
-- 
1.9.1

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

* [U-Boot] [PATCH v3 2/2] mdio: add marvell MDIO driver
  2018-06-13  4:33 [U-Boot] [PATCH v3 0/2] Add MDIO driver model support make at marvell.com
  2018-06-13  4:33 ` [U-Boot] [PATCH v3 1/2] dm: mdio: add a uclass for MDIO make at marvell.com
@ 2018-06-13  4:33 ` make at marvell.com
  1 sibling, 0 replies; 10+ messages in thread
From: make at marvell.com @ 2018-06-13  4:33 UTC (permalink / raw)
  To: u-boot

From: Ken Ma <make@marvell.com>

This patch adds a separate driver for the MDIO interface of the
Marvell Ethernet controllers based on driver model. There are two
reasons to have a separate driver rather than including it inside
the MAC driver itself:
  *) The MDIO interface is shared by all Ethernet ports, so a driver
     must guarantee non-concurrent accesses to this MDIO interface. The
     most logical way is to have a separate driver that handles this
     single MDIO interface, used by all Ethernet ports.
  *) The MDIO interface is the same between the existing mv643xx_eth
     driver and the new mvneta/mvpp2 driver. Even though it is for now
     only used by the mvneta/mvpp2 driver, it will in the future be
     used by the mv643xx_eth driver as well.

This driver supports SMI IEEE for 802.3 Clause 22 and XSMI for IEEE
802.3 Clause 45.

This patch also adds device tree binding for marvell MDIO driver.

Signed-off-by: Ken Ma <make@marvell.com>
Reviewed-by: Chris Packham <judge.packham@gmail.com>
---

Changes in v3:
- Move marvell mdio driver to driver/net/mdio folder;
- Update codes according to mdio uclass implementation updates.

Changes in v2:
- Fix error printing:
  - Change some debug to pr_err;
  - mii bus has no parent member and it is not a udevice, so dev_err
    is changed to pr_err for mii bus error printings.

 MAINTAINERS                                   |   1 +
 arch/arm/Kconfig                              |   1 +
 doc/device-tree-bindings/net/marvell-mdio.txt |  18 ++
 drivers/net/mdio/Kconfig                      |  10 ++
 drivers/net/mdio/Makefile                     |   1 +
 drivers/net/mdio/mvmdio.c                     | 234 ++++++++++++++++++++++++++
 6 files changed, 265 insertions(+)
 create mode 100644 doc/device-tree-bindings/net/marvell-mdio.txt
 create mode 100644 drivers/net/mdio/mvmdio.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9e1fc83..d8584f9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -137,6 +137,7 @@ T:	git git://git.denx.de/u-boot-marvell.git
 F:	arch/arm/mach-kirkwood/
 F:	arch/arm/mach-mvebu/
 F:	drivers/ata/ahci_mvebu.c
+F:	drivers/net/mdio/mvmdio.c
 
 ARM MARVELL PXA
 M:	Marek Vasut <marex@denx.de>
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index dde422b..e52b164 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -432,6 +432,7 @@ config ARCH_MVEBU
 	select DM_SPI
 	select DM_SPI_FLASH
 	select SPI
+	select MDIO
 
 config TARGET_DEVKIT3250
 	bool "Support devkit3250"
diff --git a/doc/device-tree-bindings/net/marvell-mdio.txt b/doc/device-tree-bindings/net/marvell-mdio.txt
new file mode 100644
index 0000000..55db435
--- /dev/null
+++ b/doc/device-tree-bindings/net/marvell-mdio.txt
@@ -0,0 +1,18 @@
+* Marvell MDIO Ethernet Controller interface
+
+The Ethernet controllers of the Marvel Armada 3700 and Armada 7k/8k
+have an identical unit that provides an interface with the MDIO bus.
+This driver handles this MDIO interface.
+
+Mandatory properties:
+SoC specific:
+	- #address-cells: Must be <1>.
+	- #size-cells: Must be <0>.
+	- compatible: Should be "marvell,orion-mdio" (for SMI)
+				"marvell,xmdio"	     (for XSMI)
+	- reg: Base address and size SMI/XMSI bus.
+
+Optional properties:
+	- mdio-name       - MDIO bus name
+
+For example, please refer to "mdio-bus.txt".
diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
index 533cc4a..d1a31e6 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -15,4 +15,14 @@ config MDIO
 	  udevice or mii bus from its child phy node or
 	  an ethernet udevice which the phy belongs to.
 
+config MVMDIO
+	bool "Marvell MDIO interface support"
+	depends on MDIO
+	select PHYLIB
+	help
+	  This driver supports the MDIO interface found in the network
+	  interface units of the Marvell EBU SoCs (Kirkwood, Orion5x,
+	  Dove, Armada 370, Armada XP, Armada 37xx and Armada7K/8K/8KP).
+
+	  This driver is used by the MVPP2 and MVNETA drivers.
 endmenu
diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
index 45ae502..8b2e5a4 100644
--- a/drivers/net/mdio/Makefile
+++ b/drivers/net/mdio/Makefile
@@ -4,3 +4,4 @@
 # Author: Ken Ma<make@marvell.com>
 
 obj-$(CONFIG_MDIO) += mdio-uclass.o
+obj-$(CONFIG_MVMDIO) += mvmdio.o
diff --git a/drivers/net/mdio/mvmdio.c b/drivers/net/mdio/mvmdio.c
new file mode 100644
index 0000000..0fb45ce
--- /dev/null
+++ b/drivers/net/mdio/mvmdio.c
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Marvell International Ltd.
+ * Author: Ken Ma<make@marvell.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+#include <miiphy.h>
+#include <phy.h>
+#include <asm/io.h>
+
+#define MVMDIO_SMI_DATA_SHIFT		0
+#define MVMDIO_SMI_PHY_ADDR_SHIFT	16
+#define MVMDIO_SMI_PHY_REG_SHIFT	21
+#define MVMDIO_SMI_READ_OPERATION	BIT(26)
+#define MVMDIO_SMI_WRITE_OPERATION	0
+#define MVMDIO_SMI_READ_VALID		BIT(27)
+#define MVMDIO_SMI_BUSY			BIT(28)
+
+#define MVMDIO_XSMI_MGNT_REG		0x0
+#define MVMDIO_XSMI_PHYADDR_SHIFT	16
+#define MVMDIO_XSMI_DEVADDR_SHIFT	21
+#define MVMDIO_XSMI_WRITE_OPERATION	(0x5 << 26)
+#define MVMDIO_XSMI_READ_OPERATION	(0x7 << 26)
+#define MVMDIO_XSMI_READ_VALID		BIT(29)
+#define MVMDIO_XSMI_BUSY		BIT(30)
+#define MVMDIO_XSMI_ADDR_REG		0x8
+
+#define MVMDIO_TIMEOUT			10000
+
+struct mvmdio_priv {
+	void *mdio_base;
+};
+
+enum mvmdio_bus_type {
+	BUS_TYPE_SMI,
+	BUS_TYPE_XSMI
+};
+
+/* Wait for the SMI unit to be ready for another operation */
+static int mvmdio_smi_wait_ready(struct mii_dev *bus)
+{
+	u32 timeout = MVMDIO_TIMEOUT;
+	struct mvmdio_priv *priv = bus->priv;
+	u32 smi_reg;
+
+	/* Wait till the SMI is not busy */
+	do {
+		/* Read smi register */
+		smi_reg = readl(priv->mdio_base);
+		if (timeout-- == 0) {
+			pr_err("Error: SMI busy timeout\n");
+			return -ETIME;
+		}
+	} while (smi_reg & MVMDIO_SMI_BUSY);
+
+	return 0;
+}
+
+static int mvmdio_smi_read(struct mii_dev *bus, int addr,
+			   int devad, int reg)
+{
+	struct mvmdio_priv *priv = bus->priv;
+	u32 val;
+	int ret;
+
+	if (devad != MDIO_DEVAD_NONE)
+		return -EOPNOTSUPP;
+
+	ret = mvmdio_smi_wait_ready(bus);
+	if (ret < 0)
+		return ret;
+
+	writel(((addr << MVMDIO_SMI_PHY_ADDR_SHIFT) |
+		(reg << MVMDIO_SMI_PHY_REG_SHIFT)  |
+		MVMDIO_SMI_READ_OPERATION),
+	       priv->mdio_base);
+
+	ret = mvmdio_smi_wait_ready(bus);
+	if (ret < 0)
+		return ret;
+
+	val = readl(priv->mdio_base);
+	if (!(val & MVMDIO_SMI_READ_VALID)) {
+		pr_err("SMI bus read not valid\n");
+		return -ENODEV;
+	}
+
+	return val & GENMASK(15, 0);
+}
+
+static int mvmdio_smi_write(struct mii_dev *bus, int addr, int devad,
+			    int reg, u16 value)
+{
+	struct mvmdio_priv *priv = bus->priv;
+	int ret;
+
+	if (devad != MDIO_DEVAD_NONE)
+		return -EOPNOTSUPP;
+
+	ret = mvmdio_smi_wait_ready(bus);
+	if (ret < 0)
+		return ret;
+
+	writel(((addr << MVMDIO_SMI_PHY_ADDR_SHIFT) |
+		(reg << MVMDIO_SMI_PHY_REG_SHIFT)  |
+		MVMDIO_SMI_WRITE_OPERATION            |
+		(value << MVMDIO_SMI_DATA_SHIFT)),
+	       priv->mdio_base);
+
+	return 0;
+}
+
+static int mvmdio_xsmi_wait_ready(struct mii_dev *bus)
+{
+	u32 timeout = MVMDIO_TIMEOUT;
+	struct mvmdio_priv *priv = bus->priv;
+	u32 xsmi_reg;
+
+	/* Wait till the xSMI is not busy */
+	do {
+		/* Read xSMI register */
+		xsmi_reg = readl(priv->mdio_base);
+		if (timeout-- == 0) {
+			pr_err("xSMI busy time-out\n");
+			return -ETIME;
+		}
+	} while (xsmi_reg & MVMDIO_XSMI_BUSY);
+
+	return 0;
+}
+
+static int mvmdio_xsmi_read(struct mii_dev *bus, int addr,
+			    int devad, int reg)
+{
+	struct mvmdio_priv *priv = bus->priv;
+	int ret;
+
+	if (devad == MDIO_DEVAD_NONE)
+		return -EOPNOTSUPP;
+
+	ret = mvmdio_xsmi_wait_ready(bus);
+	if (ret < 0)
+		return ret;
+
+	writel(reg & GENMASK(15, 0), priv->mdio_base + MVMDIO_XSMI_ADDR_REG);
+	writel(((addr << MVMDIO_XSMI_PHYADDR_SHIFT) |
+		(devad << MVMDIO_XSMI_DEVADDR_SHIFT) |
+		MVMDIO_XSMI_READ_OPERATION),
+	       priv->mdio_base + MVMDIO_XSMI_MGNT_REG);
+
+	ret = mvmdio_xsmi_wait_ready(bus);
+	if (ret < 0)
+		return ret;
+
+	if (!(readl(priv->mdio_base + MVMDIO_XSMI_MGNT_REG) &
+	      MVMDIO_XSMI_READ_VALID)) {
+		pr_err("XSMI bus read not valid\n");
+		return -ENODEV;
+	}
+
+	return readl(priv->mdio_base + MVMDIO_XSMI_MGNT_REG) & GENMASK(15, 0);
+}
+
+static int mvmdio_xsmi_write(struct mii_dev *bus, int addr, int devad,
+			     int reg, u16 value)
+{
+	struct mvmdio_priv *priv = bus->priv;
+	int ret;
+
+	if (devad == MDIO_DEVAD_NONE)
+		return -EOPNOTSUPP;
+
+	ret = mvmdio_xsmi_wait_ready(bus);
+	if (ret < 0)
+		return ret;
+
+	writel(reg & GENMASK(15, 0), priv->mdio_base + MVMDIO_XSMI_ADDR_REG);
+	writel(((addr << MVMDIO_XSMI_PHYADDR_SHIFT) |
+		(devad << MVMDIO_XSMI_DEVADDR_SHIFT) |
+		MVMDIO_XSMI_WRITE_OPERATION | value),
+	       priv->mdio_base + MVMDIO_XSMI_MGNT_REG);
+
+	return 0;
+}
+
+static int mvmdio_probe(struct udevice *dev)
+{
+	struct mii_dev *bus = (struct mii_dev *)dev_get_uclass_platdata(dev);
+	struct mvmdio_priv *priv;
+	enum mvmdio_bus_type type;
+
+	priv = dev_get_priv(dev);
+	priv->mdio_base = (void *)dev_read_addr(dev);
+	bus->priv = priv;
+
+	type = (enum mvmdio_bus_type)dev_get_driver_data(dev);
+	switch (type) {
+	case BUS_TYPE_SMI:
+		bus->read = mvmdio_smi_read;
+		bus->write = mvmdio_smi_write;
+		if (!bus->name)
+			snprintf(bus->name, MDIO_NAME_LEN,
+				 "orion-mdio.%p", priv->mdio_base);
+		break;
+	case BUS_TYPE_XSMI:
+		bus->read = mvmdio_xsmi_read;
+		bus->write = mvmdio_xsmi_write;
+		if (!bus->name)
+			snprintf(bus->name, MDIO_NAME_LEN,
+				 "xmdio.%p", priv->mdio_base);
+		break;
+	}
+
+	return 0;
+}
+
+static const struct udevice_id mvmdio_ids[] = {
+	{ .compatible = "marvell,orion-mdio", .data = BUS_TYPE_SMI },
+	{ .compatible = "marvell,xmdio", .data = BUS_TYPE_XSMI },
+	{ }
+};
+
+U_BOOT_DRIVER(mvmdio) = {
+	.name			= "mvmdio",
+	.id			= UCLASS_MDIO,
+	.of_match		= mvmdio_ids,
+	.probe			= mvmdio_probe,
+	.priv_auto_alloc_size	= sizeof(struct mvmdio_priv),
+};
+
-- 
1.9.1

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

* [U-Boot] [PATCH v3 1/2] dm: mdio: add a uclass for MDIO
  2018-06-13  4:33 ` [U-Boot] [PATCH v3 1/2] dm: mdio: add a uclass for MDIO make at marvell.com
@ 2018-06-14 12:58   ` Simon Glass
  2018-06-19 20:58   ` Joe Hershberger
  2018-06-19 21:13   ` [U-Boot] " Joe Hershberger
  2 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2018-06-14 12:58 UTC (permalink / raw)
  To: u-boot

Hi Ken,

On 12 June 2018 at 22:33,  <make@marvell.com> wrote:
> From: Ken Ma <make@marvell.com>
>
> Add a uclass which provides access to MDIO busses and includes
> operations required by MDIO.
> The implementation is based on the existing mii/phy/mdio data
> structures and APIs.
> This patch also adds evice tree binding for MDIO bus.
>
> Signed-off-by: Ken Ma <make@marvell.com>
> Reviewed-by: sjg at chromium.org, joe.hershberger at ni.com
> ---
>
> Changes in v3:
> - Move mdio uclass implementation to driver/net folder;
> - Replace flat-tree functions with livetree functions and update codes
>   and comments to be consistent with driver-model codes style;
> - Put struct mii_dev to uclass platdata to avoid the mdio alloc and
>   let driver model framework to alloc the memroy automatically,
>   meanwhile the mii bus link initialization is added,
>

I this this looks right from a DM perspective but it is missing a
sandbox driver and a simple test of the functions that you define
(presumably put this in test/dm/mdio.c)

Regards,
Simon

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

* [U-Boot] [PATCH v3 1/2] dm: mdio: add a uclass for MDIO
  2018-06-13  4:33 ` [U-Boot] [PATCH v3 1/2] dm: mdio: add a uclass for MDIO make at marvell.com
  2018-06-14 12:58   ` Simon Glass
@ 2018-06-19 20:58   ` Joe Hershberger
  2018-07-05  7:18     ` [U-Boot] [EXT] " Ken Ma
  2018-06-19 21:13   ` [U-Boot] " Joe Hershberger
  2 siblings, 1 reply; 10+ messages in thread
From: Joe Hershberger @ 2018-06-19 20:58 UTC (permalink / raw)
  To: u-boot

On Tue, Jun 12, 2018 at 11:33 PM,  <make@marvell.com> wrote:
> From: Ken Ma <make@marvell.com>
>
> Add a uclass which provides access to MDIO busses and includes
> operations required by MDIO.
> The implementation is based on the existing mii/phy/mdio data
> structures and APIs.
> This patch also adds evice tree binding for MDIO bus.
>
> Signed-off-by: Ken Ma <make@marvell.com>
> Reviewed-by: sjg at chromium.org, joe.hershberger at ni.com
> ---
>
> Changes in v3:
> - Move mdio uclass implementation to driver/net folder;
> - Replace flat-tree functions with livetree functions and update codes
>   and comments to be consistent with driver-model codes style;
> - Put struct mii_dev to uclass platdata to avoid the mdio alloc and
>   let driver model framework to alloc the memroy automatically,
>   meanwhile the mii bus link initialization is added,
>
> Changes in v2: None
>
>  MAINTAINERS                               |   1 +
>  doc/device-tree-bindings/net/mdio-bus.txt |  54 ++++++++++++++
>  drivers/Kconfig                           |   2 +
>  drivers/net/Makefile                      |   1 +
>  drivers/net/mdio/Kconfig                  |  18 +++++
>  drivers/net/mdio/Makefile                 |   6 ++
>  drivers/net/mdio/mdio-uclass.c            | 112 ++++++++++++++++++++++++++++++
>  include/dm/uclass-id.h                    |   1 +
>  include/net/mdio.h                        |  62 +++++++++++++++++
>  9 files changed, 257 insertions(+)
>  create mode 100644 doc/device-tree-bindings/net/mdio-bus.txt
>  create mode 100644 drivers/net/mdio/Kconfig
>  create mode 100644 drivers/net/mdio/Makefile
>  create mode 100644 drivers/net/mdio/mdio-uclass.c
>  create mode 100644 include/net/mdio.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 642c448..9e1fc83 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -335,6 +335,7 @@ M:  Simon Glass <sjg@chromium.org>

I think it makes sense for me to maintain this, unless Simon really
wants it. He's not even Cc'ed, so maybe he doesn't know you are
assigning it to him? Adding him.

>  S:     Maintained
>  T:     git git://git.denx.de/u-boot-dm.git
>  F:     drivers/core/
> +F:     drivers/net/mdio/
>  F:     include/dm/
>  F:     test/dm/
>
> diff --git a/doc/device-tree-bindings/net/mdio-bus.txt b/doc/device-tree-bindings/net/mdio-bus.txt
> new file mode 100644
> index 0000000..68d8b25
> --- /dev/null
> +++ b/doc/device-tree-bindings/net/mdio-bus.txt

Is it reasonable to use Documentation/devicetree/bindings/net/mdio.txt
from Linux?

> @@ -0,0 +1,54 @@
> +MDIO (Management Data Input/Output) busses
> +
> +MDIO busses can be described with a node for the MDIO master device
> +and a set of child nodes for each phy on the bus.
> +
> +The MDIO node requires the following properties:
> +- #address-cells  - number of cells required to define phy address on
> +                    the MDIO bus.
> +- #size-cells     - should be zero.
> +- compatible      - name of MDIO bus controller following generic names
> +                    recommended practice.
> +- reg            - address and length of the MDIO register.
> +
> +Optional property:
> +- mdio-name       - MDIO bus name
> +
> +The child nodes of the MDIO driver are the individual PHY devices
> +connected to this MDIO bus. They must have a "reg" property given the
> +PHY address on the MDIO bus.
> +- reg             - (required) phy address in MDIO bus.
> +
> +Example for cp110 MDIO node at the SoC level:
> +       cp0_mdio: mdio at 12a200 {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               compatible = "marvell,orion-mdio";
> +               reg = <0x12a200 0x10>;
> +               mdio-name = "cp0-mdio";
> +       };
> +
> +       cp0_xmdio: mdio at 12a600 {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               compatible = "marvell,xmdio";
> +               reg = <0x12a600 0x200>;
> +               mdio-name = "cp0-xmdio";
> +       };
> +
> +And at the board level, example for armada-8040-mcbin board:
> +       &cp0_mdio {
> +               ge_phy: ethernet-phy at 0 {
> +                       reg = <0>;
> +               };
> +       };
> +
> +       &cp0_xmdio {
> +               phy0: ethernet-phy at 0 {
> +                       reg = <0>;
> +               };
> +
> +               phy8: ethernet-phy at 8 {
> +                       reg = <8>;
> +               };
> +       };
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 9e21b28..0e0982c 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -54,6 +54,8 @@ source "drivers/mtd/Kconfig"
>
>  source "drivers/net/Kconfig"
>
> +source "drivers/net/mdio/Kconfig"

Seems like this should be in drivers/net/Kconfig.

> +
>  source "drivers/nvme/Kconfig"
>
>  source "drivers/pci/Kconfig"
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 584bfdf..1cda03f 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -70,3 +70,4 @@ obj-$(CONFIG_VSC9953) += vsc9953.o
>  obj-$(CONFIG_PIC32_ETH) += pic32_mdio.o pic32_eth.o
>  obj-$(CONFIG_DWC_ETH_QOS) += dwc_eth_qos.o
>  obj-$(CONFIG_FSL_PFE) += pfe_eth/
> +obj-$(CONFIG_MDIO) += mdio/
> diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
> new file mode 100644
> index 0000000..533cc4a
> --- /dev/null
> +++ b/drivers/net/mdio/Kconfig
> @@ -0,0 +1,18 @@
> +#
> +# MDIO infrastructure and drivers
> +#
> +
> +menu "MDIO Support"
> +
> +config MDIO
> +       bool "Enable MDIO(Management Data Input/Output) drivers"

Put a space before the open paren.

> +       depends on DM

I think this should default to enabled. Maybe that can wait until
there is more adoption of adding DM MDIO support to MAC drivers.

> +       help
> +         Enable driver model for MDIO access.
> +         Drivers provide methods to management data
> +         Input/Output.
> +         MDIO uclass provides interfaces to get mdio
> +         udevice or mii bus from its child phy node or
> +         an ethernet udevice which the phy belongs to.
> +
> +endmenu
> diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
> new file mode 100644
> index 0000000..45ae502
> --- /dev/null
> +++ b/drivers/net/mdio/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright (C) 2018 Marvell International Ltd.
> +# Author: Ken Ma<make@marvell.com>
> +
> +obj-$(CONFIG_MDIO) += mdio-uclass.o
> diff --git a/drivers/net/mdio/mdio-uclass.c b/drivers/net/mdio/mdio-uclass.c
> new file mode 100644
> index 0000000..5a23d21
> --- /dev/null
> +++ b/drivers/net/mdio/mdio-uclass.c
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0+

Don't use C++ style comments.

> +/*
> + * Copyright (C) 2018 Marvell International Ltd.
> + * Author: Ken Ma<make@marvell.com>
> + */
> +
> +#include <common.h>
> +#include <fdtdec.h>
> +#include <errno.h>
> +#include <dm.h>
> +#include <dm/uclass.h>
> +#include <dm/uclass-internal.h>
> +#include <miiphy.h>
> +#include <net/mdio.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +int mdio_mii_bus_get(struct udevice *mdio_dev, struct mii_dev **busp)
> +{
> +       *busp = (struct mii_dev *)dev_get_uclass_platdata(mdio_dev);
> +
> +       return 0;
> +}
> +
> +int mdio_device_get_from_phy(ofnode phy_node, struct udevice **devp)
> +{
> +       ofnode mdio_node;
> +
> +       mdio_node = ofnode_get_parent(phy_node);
> +       return uclass_get_device_by_ofnode(UCLASS_MDIO, mdio_node, devp);
> +}
> +
> +int mdio_mii_bus_get_from_phy(ofnode phy_node, struct mii_dev **busp)
> +{
> +       struct udevice *mdio_dev;
> +       int ret;
> +
> +       ret = mdio_device_get_from_phy(phy_node, &mdio_dev);
> +       if (ret)
> +               return ret;
> +
> +       *busp = (struct mii_dev *)dev_get_uclass_platdata(mdio_dev);
> +
> +       return 0;
> +}
> +
> +int mdio_device_get_from_eth(struct udevice *eth, struct udevice **devp)
> +{
> +       struct ofnode_phandle_args phy_args;
> +       int ret;
> +
> +       ret = dev_read_phandle_with_args(eth, "phy", NULL, 0, 0, &phy_args);
> +       if (!ret)
> +               return mdio_device_get_from_phy(phy_args.node, devp);
> +
> +       /*
> +        * If there is no phy reference under the ethernet fdt node,
> +        * it is not an error since the ethernet device may do not use
> +        * mode; so in this case, the output mdio device pointer is set
> +        * as NULL.
> +        */
> +       *devp = NULL;
> +       return 0;
> +}
> +
> +int mdio_mii_bus_get_from_eth(struct udevice *eth, struct mii_dev **busp)
> +{
> +       struct udevice *mdio_dev;
> +       int ret;
> +
> +       ret = mdio_device_get_from_eth(eth, &mdio_dev);
> +       if (ret)
> +               return ret;
> +
> +       if (mdio_dev)
> +               *busp = (struct mii_dev *)dev_get_uclass_platdata(mdio_dev);
> +       else
> +               *busp = NULL;
> +
> +       return 0;
> +}
> +
> +static int mdio_uclass_pre_probe(struct udevice *dev)
> +{
> +       struct mii_dev *bus = (struct mii_dev *)dev_get_uclass_platdata(dev);
> +       const char *name;
> +
> +       /* initialize mii_dev struct fields */

Include a comment that you are implementing mdio_alloc() setup here.

> +       INIT_LIST_HEAD(&bus->link);
> +
> +       name = fdt_getprop(gd->fdt_blob, dev_of_offset(dev),
> +                          "mdio-name", NULL);
> +       if (name)
> +               strncpy(bus->name, name, MDIO_NAME_LEN);
> +
> +       return 0;
> +}
> +
> +static int mdio_uclass_post_probe(struct udevice *dev)
> +{
> +       struct mii_dev *bus = (struct mii_dev *)dev_get_uclass_platdata(dev);
> +
> +       return mdio_register(bus);
> +}
> +
> +UCLASS_DRIVER(mdio) = {
> +       .id             = UCLASS_MDIO,
> +       .name           = "mdio",
> +       .pre_probe      = mdio_uclass_pre_probe,
> +       .post_probe     = mdio_uclass_post_probe,
> +       .per_device_platdata_auto_alloc_size = sizeof(struct mii_dev),
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index d7f9df3..504decd 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -49,6 +49,7 @@ enum uclass_id {
>         UCLASS_LPC,             /* x86 'low pin count' interface */
>         UCLASS_MAILBOX,         /* Mailbox controller */
>         UCLASS_MASS_STORAGE,    /* Mass storage device */
> +       UCLASS_MDIO,            /* Management Data Input/Output(MDIO) device */

Again, include a space before '('.

>         UCLASS_MISC,            /* Miscellaneous device */
>         UCLASS_MMC,             /* SD / MMC card or chip */
>         UCLASS_MOD_EXP,         /* RSA Mod Exp device */
> diff --git a/include/net/mdio.h b/include/net/mdio.h
> new file mode 100644
> index 0000000..5d8d703
> --- /dev/null
> +++ b/include/net/mdio.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2018 Marvell International Ltd.
> + * Author: Ken Ma<make@marvell.com>
> + */
> +
> +#ifndef _MDIO_H_
> +#define _MDIO_H_
> +
> +#include <dm.h>        /* Because we dereference struct udevice here */

Me thinks that comment is incorrect. Please delete it.

> +#include <phy.h>
> +
> +/**
> + * mdio_mii_bus_get() - Get mii bus from mdio udevice
> + *
> + * @mdio_dev:  mdio udevice
> + * @busp:      returns mii bus
> + * @returns 0 on success, error code otherwise.
> + */
> +int mdio_mii_bus_get(struct udevice *mdio_dev, struct mii_dev **busp);
> +
> +/**
> + * mdio_device_get_from_phy() - Get the mdio udevice which the phy belongs to
> + *
> + * @phy_node:  phy node offset
> + * @devp:      returns mdio udevice
> + * @returns 0 on success, error code otherwise.
> + */
> +int mdio_device_get_from_phy(ofnode phy_node, struct udevice **devp);
> +
> +/**
> + * mdio_mii_bus_get_from_phy() - Get the mii bus which the phy belongs to
> + *
> + * @phy_node:  phy node offset
> + * @busp:      returns mii bus
> + * @returns 0 on success, error code otherwise.
> + */
> +int mdio_mii_bus_get_from_phy(ofnode phy_node, struct mii_dev **busp);
> +
> +/**
> + * mdio_device_get_from_eth() - When there is a phy reference of "phy = <&...>"
> + *                      under an ethernet udevice fdt node, this function can
> + *                      get the mdio udevice which the phy belongs to
> + *
> + * @dev:       the ethernet udevice which contains the phy reference
> + * @devp:      returns mdio udevice
> + * @returns 0 on success, error code otherwise.
> + */
> +int mdio_device_get_from_eth(struct udevice *eth, struct udevice **devp);
> +
> +/**
> + * mdio_mii_bus_get_from_eth() - When there is a phy reference of
> + *                      "phy = <&...>" under an ethernet udevice fdt node, this
> + *                      function can get the mii bus which the phy belongs to
> + *
> + * @eth:       the ethernet udevice which contains the phy reference
> + * @busp:      returns mii bus
> + * @returns 0 on success, error code otherwise.
> + */
> +int mdio_mii_bus_get_from_eth(struct udevice *eth, struct mii_dev **busp);
> +
> +#endif /* _MDIO_H_ */
> --
> 1.9.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

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

* [U-Boot] [PATCH v3 1/2] dm: mdio: add a uclass for MDIO
  2018-06-13  4:33 ` [U-Boot] [PATCH v3 1/2] dm: mdio: add a uclass for MDIO make at marvell.com
  2018-06-14 12:58   ` Simon Glass
  2018-06-19 20:58   ` Joe Hershberger
@ 2018-06-19 21:13   ` Joe Hershberger
  2 siblings, 0 replies; 10+ messages in thread
From: Joe Hershberger @ 2018-06-19 21:13 UTC (permalink / raw)
  To: u-boot

On Tue, Jun 12, 2018 at 11:33 PM,  <make@marvell.com> wrote:
> From: Ken Ma <make@marvell.com>
>
> Add a uclass which provides access to MDIO busses and includes
> operations required by MDIO.
> The implementation is based on the existing mii/phy/mdio data
> structures and APIs.
> This patch also adds evice tree binding for MDIO bus.
>
> Signed-off-by: Ken Ma <make@marvell.com>
> Reviewed-by: sjg at chromium.org, joe.hershberger at ni.com

A "Reviewed-by" indicates review + acceptance where the person listed
is not a maintainer of said code. [1] It is not simply that it was
commented on by the person. You should only include it in new versions
of patches after someone has replied to a patch and included their
"Reviewed-by" tag. It should be reproduced exactly as it was sent.
There is no need to send a new version solely to include the tag,
though. Patchwork will already take care of that for the current
version.

Also, the "Reviewed-by" should each be on their own line.

[1] http://www.denx.de/wiki/U-Boot/Patches

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

* [U-Boot] [EXT] Re: [PATCH v3 1/2] dm: mdio: add a uclass for MDIO
  2018-06-19 20:58   ` Joe Hershberger
@ 2018-07-05  7:18     ` Ken Ma
  2018-07-09 20:52       ` Joe Hershberger
  0 siblings, 1 reply; 10+ messages in thread
From: Ken Ma @ 2018-07-05  7:18 UTC (permalink / raw)
  To: u-boot

Hi Joe

Please see my comments inline, thanks a lot for your kind and careful review!

Yours,
Ken

-----Original Message-----
From: Joe Hershberger [mailto:joe.hershberger at ni.com] 
Sent: 2018年6月20日 4:59
To: Ken Ma <make@marvell.com>; Simon Glass <sjg@chromium.org>
Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>; Michal Simek <michal.simek@xilinx.com>; Alexander Graf <agraf@suse.de>; Joe Hershberger <joe.hershberger@ni.com>; Heinrich Schuchardt <xypron.glpk@gmx.de>; Stefan Roese <sr@denx.de>; Chris Packham <judge.packham@gmail.com>
Subject: [EXT] Re: [U-Boot] [PATCH v3 1/2] dm: mdio: add a uclass for MDIO

External Email

----------------------------------------------------------------------
On Tue, Jun 12, 2018 at 11:33 PM,  <make@marvell.com> wrote:
> From: Ken Ma <make@marvell.com>
>
> Add a uclass which provides access to MDIO busses and includes 
> operations required by MDIO.
> The implementation is based on the existing mii/phy/mdio data 
> structures and APIs.
> This patch also adds evice tree binding for MDIO bus.
>
> Signed-off-by: Ken Ma <make@marvell.com>
> Reviewed-by: sjg at chromium.org, joe.hershberger at ni.com
> ---
>
> Changes in v3:
> - Move mdio uclass implementation to driver/net folder;
> - Replace flat-tree functions with livetree functions and update codes
>   and comments to be consistent with driver-model codes style;
> - Put struct mii_dev to uclass platdata to avoid the mdio alloc and
>   let driver model framework to alloc the memroy automatically,
>   meanwhile the mii bus link initialization is added,
>
> Changes in v2: None
>
>  MAINTAINERS                               |   1 +
>  doc/device-tree-bindings/net/mdio-bus.txt |  54 ++++++++++++++
>  drivers/Kconfig                           |   2 +
>  drivers/net/Makefile                      |   1 +
>  drivers/net/mdio/Kconfig                  |  18 +++++
>  drivers/net/mdio/Makefile                 |   6 ++
>  drivers/net/mdio/mdio-uclass.c            | 112 ++++++++++++++++++++++++++++++
>  include/dm/uclass-id.h                    |   1 +
>  include/net/mdio.h                        |  62 +++++++++++++++++
>  9 files changed, 257 insertions(+)
>  create mode 100644 doc/device-tree-bindings/net/mdio-bus.txt
>  create mode 100644 drivers/net/mdio/Kconfig  create mode 100644 
> drivers/net/mdio/Makefile  create mode 100644 
> drivers/net/mdio/mdio-uclass.c  create mode 100644 include/net/mdio.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS index 642c448..9e1fc83 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -335,6 +335,7 @@ M:  Simon Glass <sjg@chromium.org>

I think it makes sense for me to maintain this, unless Simon really wants it. He's not even Cc'ed, so maybe he doesn't know you are assigning it to him? Adding him.
[Ken] Thank you for your remind, at first I do not put mdio under driver/net and I could not find a person to mainline it, so I add Simon as maintainer. I will update the maintainer to you. Thanks a lot!

>  S:     Maintained
>  T:     git git://git.denx.de/u-boot-dm.git
>  F:     drivers/core/
> +F:     drivers/net/mdio/
>  F:     include/dm/
>  F:     test/dm/
>
> diff --git a/doc/device-tree-bindings/net/mdio-bus.txt 
> b/doc/device-tree-bindings/net/mdio-bus.txt
> new file mode 100644
> index 0000000..68d8b25
> --- /dev/null
> +++ b/doc/device-tree-bindings/net/mdio-bus.txt

Is it reasonable to use Documentation/devicetree/bindings/net/mdio.txt
from Linux?
[Ken] Linux struct mii_bus implements reset_delay_us and reset_gpiod as optional properties while u-boot struct mii_bus has no such fields at all.
Once uboot struct mii_bus contains reset_delay_us and reset_gpiod, we can add the related part in mdio_uclass.c and mdio_bus.txt.
BTW, I add optional property of mdio name to the txt.
And u-boot mdio is based on uclass and actually it's a bus, so I follow spi_bus.txt(in uboot) naming and name it to mdio_bus.txt.

> @@ -0,0 +1,54 @@
> +MDIO (Management Data Input/Output) busses
> +
> +MDIO busses can be described with a node for the MDIO master device 
> +and a set of child nodes for each phy on the bus.
> +
> +The MDIO node requires the following properties:
> +- #address-cells  - number of cells required to define phy address on
> +                    the MDIO bus.
> +- #size-cells     - should be zero.
> +- compatible      - name of MDIO bus controller following generic names
> +                    recommended practice.
> +- reg            - address and length of the MDIO register.
> +
> +Optional property:
> +- mdio-name       - MDIO bus name
> +
> +The child nodes of the MDIO driver are the individual PHY devices 
> +connected to this MDIO bus. They must have a "reg" property given the 
> +PHY address on the MDIO bus.
> +- reg             - (required) phy address in MDIO bus.
> +
> +Example for cp110 MDIO node at the SoC level:
> +       cp0_mdio: mdio at 12a200 {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               compatible = "marvell,orion-mdio";
> +               reg = <0x12a200 0x10>;
> +               mdio-name = "cp0-mdio";
> +       };
> +
> +       cp0_xmdio: mdio at 12a600 {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               compatible = "marvell,xmdio";
> +               reg = <0x12a600 0x200>;
> +               mdio-name = "cp0-xmdio";
> +       };
> +
> +And at the board level, example for armada-8040-mcbin board:
> +       &cp0_mdio {
> +               ge_phy: ethernet-phy at 0 {
> +                       reg = <0>;
> +               };
> +       };
> +
> +       &cp0_xmdio {
> +               phy0: ethernet-phy at 0 {
> +                       reg = <0>;
> +               };
> +
> +               phy8: ethernet-phy at 8 {
> +                       reg = <8>;
> +               };
> +       };
> diff --git a/drivers/Kconfig b/drivers/Kconfig index 9e21b28..0e0982c 
> 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -54,6 +54,8 @@ source "drivers/mtd/Kconfig"
>
>  source "drivers/net/Kconfig"
>
> +source "drivers/net/mdio/Kconfig"

Seems like this should be in drivers/net/Kconfig.
[Ken] Actually in kernel, MDIO is implemented in driver/net/phy, and then "driver/net/kocnifg" sources "drivers/net/phy/Kconfig".
Since we implement mdio as an UCLASS, I think it's better to provide a single folder with name of "mdio" but not "phy", meanwhile "driver/net/Kconfig" can "drivers/net/mdio/Kconfig".

> +
>  source "drivers/nvme/Kconfig"
>
>  source "drivers/pci/Kconfig"
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 
> 584bfdf..1cda03f 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -70,3 +70,4 @@ obj-$(CONFIG_VSC9953) += vsc9953.o
>  obj-$(CONFIG_PIC32_ETH) += pic32_mdio.o pic32_eth.o
>  obj-$(CONFIG_DWC_ETH_QOS) += dwc_eth_qos.o
>  obj-$(CONFIG_FSL_PFE) += pfe_eth/
> +obj-$(CONFIG_MDIO) += mdio/
> diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig new 
> file mode 100644 index 0000000..533cc4a
> --- /dev/null
> +++ b/drivers/net/mdio/Kconfig
> @@ -0,0 +1,18 @@
> +#
> +# MDIO infrastructure and drivers
> +#
> +
> +menu "MDIO Support"
> +
> +config MDIO
> +       bool "Enable MDIO(Management Data Input/Output) drivers"

Put a space before the open paren.
[Ken] Thanks a lot for your kind remind!

> +       depends on DM

I think this should default to enabled. Maybe that can wait until there is more adoption of adding DM MDIO support to MAC drivers.

> +       help
> +         Enable driver model for MDIO access.
> +         Drivers provide methods to management data
> +         Input/Output.
> +         MDIO uclass provides interfaces to get mdio
> +         udevice or mii bus from its child phy node or
> +         an ethernet udevice which the phy belongs to.
> +
> +endmenu
> diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile new 
> file mode 100644 index 0000000..45ae502
> --- /dev/null
> +++ b/drivers/net/mdio/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright (C) 2018 Marvell International Ltd.
> +# Author: Ken Ma<make@marvell.com>
> +
> +obj-$(CONFIG_MDIO) += mdio-uclass.o
> diff --git a/drivers/net/mdio/mdio-uclass.c 
> b/drivers/net/mdio/mdio-uclass.c new file mode 100644 index 
> 0000000..5a23d21
> --- /dev/null
> +++ b/drivers/net/mdio/mdio-uclass.c
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0+

Don't use C++ style comments.
[Ken] This is the latest GPL license Naming Conventions for C files, please refer to Licenses/README.
	  C source:      // SPDX-License-Identifier: <SPDX License Expression>
      C header:     /* SPDX-License-Identifier: <SPDX License Expression> */
      ASM:    /* SPDX-License-Identifier: <SPDX License Expression> */
      scripts:         # SPDX-License-Identifier: <SPDX License Expression>
      .rst:      .. SPDX-License-Identifier: <SPDX License Expression>
      .dts{i}:  // SPDX-License-Identifier: <SPDX License Expression>

> +/*
> + * Copyright (C) 2018 Marvell International Ltd.
> + * Author: Ken Ma<make@marvell.com>
> + */
> +
> +#include <common.h>
> +#include <fdtdec.h>
> +#include <errno.h>
> +#include <dm.h>
> +#include <dm/uclass.h>
> +#include <dm/uclass-internal.h>
> +#include <miiphy.h>
> +#include <net/mdio.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +int mdio_mii_bus_get(struct udevice *mdio_dev, struct mii_dev **busp) 
> +{
> +       *busp = (struct mii_dev *)dev_get_uclass_platdata(mdio_dev);
> +
> +       return 0;
> +}
> +
> +int mdio_device_get_from_phy(ofnode phy_node, struct udevice **devp) 
> +{
> +       ofnode mdio_node;
> +
> +       mdio_node = ofnode_get_parent(phy_node);
> +       return uclass_get_device_by_ofnode(UCLASS_MDIO, mdio_node, 
> +devp); }
> +
> +int mdio_mii_bus_get_from_phy(ofnode phy_node, struct mii_dev **busp) 
> +{
> +       struct udevice *mdio_dev;
> +       int ret;
> +
> +       ret = mdio_device_get_from_phy(phy_node, &mdio_dev);
> +       if (ret)
> +               return ret;
> +
> +       *busp = (struct mii_dev *)dev_get_uclass_platdata(mdio_dev);
> +
> +       return 0;
> +}
> +
> +int mdio_device_get_from_eth(struct udevice *eth, struct udevice 
> +**devp) {
> +       struct ofnode_phandle_args phy_args;
> +       int ret;
> +
> +       ret = dev_read_phandle_with_args(eth, "phy", NULL, 0, 0, &phy_args);
> +       if (!ret)
> +               return mdio_device_get_from_phy(phy_args.node, devp);
> +
> +       /*
> +        * If there is no phy reference under the ethernet fdt node,
> +        * it is not an error since the ethernet device may do not use
> +        * mode; so in this case, the output mdio device pointer is set
> +        * as NULL.
> +        */
> +       *devp = NULL;
> +       return 0;
> +}
> +
> +int mdio_mii_bus_get_from_eth(struct udevice *eth, struct mii_dev 
> +**busp) {
> +       struct udevice *mdio_dev;
> +       int ret;
> +
> +       ret = mdio_device_get_from_eth(eth, &mdio_dev);
> +       if (ret)
> +               return ret;
> +
> +       if (mdio_dev)
> +               *busp = (struct mii_dev *)dev_get_uclass_platdata(mdio_dev);
> +       else
> +               *busp = NULL;
> +
> +       return 0;
> +}
> +
> +static int mdio_uclass_pre_probe(struct udevice *dev) {
> +       struct mii_dev *bus = (struct mii_dev *)dev_get_uclass_platdata(dev);
> +       const char *name;
> +
> +       /* initialize mii_dev struct fields */

Include a comment that you are implementing mdio_alloc() setup here.
[Ken] Thanks, I will update.

> +       INIT_LIST_HEAD(&bus->link);
> +
> +       name = fdt_getprop(gd->fdt_blob, dev_of_offset(dev),
> +                          "mdio-name", NULL);
> +       if (name)
> +               strncpy(bus->name, name, MDIO_NAME_LEN);
> +
> +       return 0;
> +}
> +
> +static int mdio_uclass_post_probe(struct udevice *dev) {
> +       struct mii_dev *bus = (struct mii_dev 
> +*)dev_get_uclass_platdata(dev);
> +
> +       return mdio_register(bus);
> +}
> +
> +UCLASS_DRIVER(mdio) = {
> +       .id             = UCLASS_MDIO,
> +       .name           = "mdio",
> +       .pre_probe      = mdio_uclass_pre_probe,
> +       .post_probe     = mdio_uclass_post_probe,
> +       .per_device_platdata_auto_alloc_size = sizeof(struct mii_dev), 
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 
> d7f9df3..504decd 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -49,6 +49,7 @@ enum uclass_id {
>         UCLASS_LPC,             /* x86 'low pin count' interface */
>         UCLASS_MAILBOX,         /* Mailbox controller */
>         UCLASS_MASS_STORAGE,    /* Mass storage device */
> +       UCLASS_MDIO,            /* Management Data Input/Output(MDIO) device */

Again, include a space before '('.
[Ken] Thanks a lot for your kind remind!

>         UCLASS_MISC,            /* Miscellaneous device */
>         UCLASS_MMC,             /* SD / MMC card or chip */
>         UCLASS_MOD_EXP,         /* RSA Mod Exp device */
> diff --git a/include/net/mdio.h b/include/net/mdio.h new file mode 
> 100644 index 0000000..5d8d703
> --- /dev/null
> +++ b/include/net/mdio.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2018 Marvell International Ltd.
> + * Author: Ken Ma<make@marvell.com>
> + */
> +
> +#ifndef _MDIO_H_
> +#define _MDIO_H_
> +
> +#include <dm.h>        /* Because we dereference struct udevice here */

Me thinks that comment is incorrect. Please delete it.
[Ken] Thanks, I will update it.
> +#include <phy.h>
> +
> +/**
> + * mdio_mii_bus_get() - Get mii bus from mdio udevice
> + *
> + * @mdio_dev:  mdio udevice
> + * @busp:      returns mii bus
> + * @returns 0 on success, error code otherwise.
> + */
> +int mdio_mii_bus_get(struct udevice *mdio_dev, struct mii_dev 
> +**busp);
> +
> +/**
> + * mdio_device_get_from_phy() - Get the mdio udevice which the phy 
> +belongs to
> + *
> + * @phy_node:  phy node offset
> + * @devp:      returns mdio udevice
> + * @returns 0 on success, error code otherwise.
> + */
> +int mdio_device_get_from_phy(ofnode phy_node, struct udevice **devp);
> +
> +/**
> + * mdio_mii_bus_get_from_phy() - Get the mii bus which the phy 
> +belongs to
> + *
> + * @phy_node:  phy node offset
> + * @busp:      returns mii bus
> + * @returns 0 on success, error code otherwise.
> + */
> +int mdio_mii_bus_get_from_phy(ofnode phy_node, struct mii_dev 
> +**busp);
> +
> +/**
> + * mdio_device_get_from_eth() - When there is a phy reference of "phy = <&...>"
> + *                      under an ethernet udevice fdt node, this function can
> + *                      get the mdio udevice which the phy belongs to
> + *
> + * @dev:       the ethernet udevice which contains the phy reference
> + * @devp:      returns mdio udevice
> + * @returns 0 on success, error code otherwise.
> + */
> +int mdio_device_get_from_eth(struct udevice *eth, struct udevice 
> +**devp);
> +
> +/**
> + * mdio_mii_bus_get_from_eth() - When there is a phy reference of
> + *                      "phy = <&...>" under an ethernet udevice fdt node, this
> + *                      function can get the mii bus which the phy belongs to
> + *
> + * @eth:       the ethernet udevice which contains the phy reference
> + * @busp:      returns mii bus
> + * @returns 0 on success, error code otherwise.
> + */
> +int mdio_mii_bus_get_from_eth(struct udevice *eth, struct mii_dev 
> +**busp);
> +
> +#endif /* _MDIO_H_ */
> --
> 1.9.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

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

* [U-Boot] [EXT] Re: [PATCH v3 1/2] dm: mdio: add a uclass for MDIO
  2018-07-05  7:18     ` [U-Boot] [EXT] " Ken Ma
@ 2018-07-09 20:52       ` Joe Hershberger
  0 siblings, 0 replies; 10+ messages in thread
From: Joe Hershberger @ 2018-07-09 20:52 UTC (permalink / raw)
  To: u-boot

On Thu, Jul 5, 2018 at 2:18 AM, Ken Ma <make@marvell.com> wrote:
> Hi Joe
>
> Please see my comments inline, thanks a lot for your kind and careful review!
>
> Yours,
> Ken
>
> -----Original Message-----
> From: Joe Hershberger [mailto:joe.hershberger at ni.com]
> Sent: 2018年6月20日 4:59
> To: Ken Ma <make@marvell.com>; Simon Glass <sjg@chromium.org>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>; Michal Simek <michal.simek@xilinx.com>; Alexander Graf <agraf@suse.de>; Joe Hershberger <joe.hershberger@ni.com>; Heinrich Schuchardt <xypron.glpk@gmx.de>; Stefan Roese <sr@denx.de>; Chris Packham <judge.packham@gmail.com>
> Subject: [EXT] Re: [U-Boot] [PATCH v3 1/2] dm: mdio: add a uclass for MDIO
>
> External Email
>
> ----------------------------------------------------------------------
> On Tue, Jun 12, 2018 at 11:33 PM,  <make@marvell.com> wrote:
>> From: Ken Ma <make@marvell.com>
>>
>> Add a uclass which provides access to MDIO busses and includes
>> operations required by MDIO.
>> The implementation is based on the existing mii/phy/mdio data
>> structures and APIs.
>> This patch also adds evice tree binding for MDIO bus.
>>
>> Signed-off-by: Ken Ma <make@marvell.com>
>> Reviewed-by: sjg at chromium.org, joe.hershberger at ni.com
>> ---
>>
>> Changes in v3:
>> - Move mdio uclass implementation to driver/net folder;
>> - Replace flat-tree functions with livetree functions and update codes
>>   and comments to be consistent with driver-model codes style;
>> - Put struct mii_dev to uclass platdata to avoid the mdio alloc and
>>   let driver model framework to alloc the memroy automatically,
>>   meanwhile the mii bus link initialization is added,
>>
>> Changes in v2: None
>>
>>  MAINTAINERS                               |   1 +
>>  doc/device-tree-bindings/net/mdio-bus.txt |  54 ++++++++++++++
>>  drivers/Kconfig                           |   2 +
>>  drivers/net/Makefile                      |   1 +
>>  drivers/net/mdio/Kconfig                  |  18 +++++
>>  drivers/net/mdio/Makefile                 |   6 ++
>>  drivers/net/mdio/mdio-uclass.c            | 112 ++++++++++++++++++++++++++++++
>>  include/dm/uclass-id.h                    |   1 +
>>  include/net/mdio.h                        |  62 +++++++++++++++++
>>  9 files changed, 257 insertions(+)
>>  create mode 100644 doc/device-tree-bindings/net/mdio-bus.txt
>>  create mode 100644 drivers/net/mdio/Kconfig  create mode 100644
>> drivers/net/mdio/Makefile  create mode 100644
>> drivers/net/mdio/mdio-uclass.c  create mode 100644 include/net/mdio.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS index 642c448..9e1fc83 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -335,6 +335,7 @@ M:  Simon Glass <sjg@chromium.org>
>
> I think it makes sense for me to maintain this, unless Simon really wants it. He's not even Cc'ed, so maybe he doesn't know you are assigning it to him? Adding him.
> [Ken] Thank you for your remind, at first I do not put mdio under driver/net and I could not find a person to mainline it, so I add Simon as maintainer. I will update the maintainer to you. Thanks a lot!
>
>>  S:     Maintained
>>  T:     git git://git.denx.de/u-boot-dm.git
>>  F:     drivers/core/
>> +F:     drivers/net/mdio/
>>  F:     include/dm/
>>  F:     test/dm/
>>
>> diff --git a/doc/device-tree-bindings/net/mdio-bus.txt
>> b/doc/device-tree-bindings/net/mdio-bus.txt
>> new file mode 100644
>> index 0000000..68d8b25
>> --- /dev/null
>> +++ b/doc/device-tree-bindings/net/mdio-bus.txt
>
> Is it reasonable to use Documentation/devicetree/bindings/net/mdio.txt
> from Linux?
> [Ken] Linux struct mii_bus implements reset_delay_us and reset_gpiod as optional properties while u-boot struct mii_bus has no such fields at all.
> Once uboot struct mii_bus contains reset_delay_us and reset_gpiod, we can add the related part in mdio_uclass.c and mdio_bus.txt.

While we don' support those optional properties, I still think the
bindings need to be shared. Optional need not be supported, so you not
supporting it is no problem. It can still be defined how it should
look if it was supported.

> BTW, I add optional property of mdio name to the txt.

That's fine. I think it's something that would be reasonable to add in
Linux as well.

> And u-boot mdio is based on uclass and actually it's a bus, so I follow spi_bus.txt(in uboot) naming and name it to mdio_bus.txt.

It's still fundamentally a bus in both cases and we are trying to keep
them as shared as possible. I don't see anything compelling so far why
they need to diverge from the outset.  Also, as Michal commented, it
should be in a separate patch before the support is added.

>> @@ -0,0 +1,54 @@
>> +MDIO (Management Data Input/Output) busses
>> +
>> +MDIO busses can be described with a node for the MDIO master device
>> +and a set of child nodes for each phy on the bus.
>> +
>> +The MDIO node requires the following properties:
>> +- #address-cells  - number of cells required to define phy address on
>> +                    the MDIO bus.
>> +- #size-cells     - should be zero.
>> +- compatible      - name of MDIO bus controller following generic names
>> +                    recommended practice.
>> +- reg            - address and length of the MDIO register.
>> +
>> +Optional property:
>> +- mdio-name       - MDIO bus name
>> +
>> +The child nodes of the MDIO driver are the individual PHY devices
>> +connected to this MDIO bus. They must have a "reg" property given the
>> +PHY address on the MDIO bus.
>> +- reg             - (required) phy address in MDIO bus.
>> +
>> +Example for cp110 MDIO node at the SoC level:
>> +       cp0_mdio: mdio at 12a200 {
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +               compatible = "marvell,orion-mdio";
>> +               reg = <0x12a200 0x10>;
>> +               mdio-name = "cp0-mdio";
>> +       };
>> +
>> +       cp0_xmdio: mdio at 12a600 {
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +               compatible = "marvell,xmdio";
>> +               reg = <0x12a600 0x200>;
>> +               mdio-name = "cp0-xmdio";
>> +       };
>> +
>> +And at the board level, example for armada-8040-mcbin board:
>> +       &cp0_mdio {
>> +               ge_phy: ethernet-phy at 0 {
>> +                       reg = <0>;
>> +               };
>> +       };
>> +
>> +       &cp0_xmdio {
>> +               phy0: ethernet-phy at 0 {
>> +                       reg = <0>;
>> +               };
>> +
>> +               phy8: ethernet-phy at 8 {
>> +                       reg = <8>;
>> +               };
>> +       };
>> diff --git a/drivers/Kconfig b/drivers/Kconfig index 9e21b28..0e0982c
>> 100644
>> --- a/drivers/Kconfig
>> +++ b/drivers/Kconfig
>> @@ -54,6 +54,8 @@ source "drivers/mtd/Kconfig"
>>
>>  source "drivers/net/Kconfig"
>>
>> +source "drivers/net/mdio/Kconfig"
>
> Seems like this should be in drivers/net/Kconfig.
> [Ken] Actually in kernel, MDIO is implemented in driver/net/phy, and then "driver/net/kocnifg" sources "drivers/net/phy/Kconfig".
> Since we implement mdio as an UCLASS, I think it's better to provide a single folder with name of "mdio" but not "phy", meanwhile "driver/net/Kconfig" can "drivers/net/mdio/Kconfig".

Yes, that's exactly what I was asking for. Source it in
driver/net/Kconfig, not in driver/Kconfig.

>> +
>>  source "drivers/nvme/Kconfig"
>>
>>  source "drivers/pci/Kconfig"
>> diff --git a/drivers/net/Makefile b/drivers/net/Makefile index
>> 584bfdf..1cda03f 100644
>> --- a/drivers/net/Makefile
>> +++ b/drivers/net/Makefile
>> @@ -70,3 +70,4 @@ obj-$(CONFIG_VSC9953) += vsc9953.o
>>  obj-$(CONFIG_PIC32_ETH) += pic32_mdio.o pic32_eth.o
>>  obj-$(CONFIG_DWC_ETH_QOS) += dwc_eth_qos.o
>>  obj-$(CONFIG_FSL_PFE) += pfe_eth/
>> +obj-$(CONFIG_MDIO) += mdio/
>> diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig new
>> file mode 100644 index 0000000..533cc4a
>> --- /dev/null
>> +++ b/drivers/net/mdio/Kconfig
>> @@ -0,0 +1,18 @@
>> +#
>> +# MDIO infrastructure and drivers
>> +#
>> +
>> +menu "MDIO Support"
>> +
>> +config MDIO
>> +       bool "Enable MDIO(Management Data Input/Output) drivers"
>
> Put a space before the open paren.
> [Ken] Thanks a lot for your kind remind!
>
>> +       depends on DM
>
> I think this should default to enabled. Maybe that can wait until there is more adoption of adding DM MDIO support to MAC drivers.
>
>> +       help
>> +         Enable driver model for MDIO access.
>> +         Drivers provide methods to management data
>> +         Input/Output.
>> +         MDIO uclass provides interfaces to get mdio
>> +         udevice or mii bus from its child phy node or
>> +         an ethernet udevice which the phy belongs to.
>> +
>> +endmenu
>> diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile new
>> file mode 100644 index 0000000..45ae502
>> --- /dev/null
>> +++ b/drivers/net/mdio/Makefile
>> @@ -0,0 +1,6 @@
>> +# SPDX-License-Identifier: GPL-2.0+
>> +#
>> +# Copyright (C) 2018 Marvell International Ltd.
>> +# Author: Ken Ma<make@marvell.com>
>> +
>> +obj-$(CONFIG_MDIO) += mdio-uclass.o
>> diff --git a/drivers/net/mdio/mdio-uclass.c
>> b/drivers/net/mdio/mdio-uclass.c new file mode 100644 index
>> 0000000..5a23d21
>> --- /dev/null
>> +++ b/drivers/net/mdio/mdio-uclass.c
>> @@ -0,0 +1,112 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>
> Don't use C++ style comments.
> [Ken] This is the latest GPL license Naming Conventions for C files, please refer to Licenses/README.
>           C source:      // SPDX-License-Identifier: <SPDX License Expression>
>       C header:     /* SPDX-License-Identifier: <SPDX License Expression> */
>       ASM:    /* SPDX-License-Identifier: <SPDX License Expression> */
>       scripts:         # SPDX-License-Identifier: <SPDX License Expression>
>       .rst:      .. SPDX-License-Identifier: <SPDX License Expression>
>       .dts{i}:  // SPDX-License-Identifier: <SPDX License Expression>

Argh... not sure why that was the decision, but carry on...

>
>> +/*
>> + * Copyright (C) 2018 Marvell International Ltd.
>> + * Author: Ken Ma<make@marvell.com>
>> + */
>> +
>> +#include <common.h>
>> +#include <fdtdec.h>
>> +#include <errno.h>
>> +#include <dm.h>
>> +#include <dm/uclass.h>
>> +#include <dm/uclass-internal.h>
>> +#include <miiphy.h>
>> +#include <net/mdio.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +int mdio_mii_bus_get(struct udevice *mdio_dev, struct mii_dev **busp)
>> +{
>> +       *busp = (struct mii_dev *)dev_get_uclass_platdata(mdio_dev);
>> +
>> +       return 0;
>> +}
>> +
>> +int mdio_device_get_from_phy(ofnode phy_node, struct udevice **devp)
>> +{
>> +       ofnode mdio_node;
>> +
>> +       mdio_node = ofnode_get_parent(phy_node);
>> +       return uclass_get_device_by_ofnode(UCLASS_MDIO, mdio_node,
>> +devp); }
>> +
>> +int mdio_mii_bus_get_from_phy(ofnode phy_node, struct mii_dev **busp)
>> +{
>> +       struct udevice *mdio_dev;
>> +       int ret;
>> +
>> +       ret = mdio_device_get_from_phy(phy_node, &mdio_dev);
>> +       if (ret)
>> +               return ret;
>> +
>> +       *busp = (struct mii_dev *)dev_get_uclass_platdata(mdio_dev);
>> +
>> +       return 0;
>> +}
>> +
>> +int mdio_device_get_from_eth(struct udevice *eth, struct udevice
>> +**devp) {
>> +       struct ofnode_phandle_args phy_args;
>> +       int ret;
>> +
>> +       ret = dev_read_phandle_with_args(eth, "phy", NULL, 0, 0, &phy_args);
>> +       if (!ret)
>> +               return mdio_device_get_from_phy(phy_args.node, devp);
>> +
>> +       /*
>> +        * If there is no phy reference under the ethernet fdt node,
>> +        * it is not an error since the ethernet device may do not use
>> +        * mode; so in this case, the output mdio device pointer is set
>> +        * as NULL.
>> +        */
>> +       *devp = NULL;
>> +       return 0;
>> +}
>> +
>> +int mdio_mii_bus_get_from_eth(struct udevice *eth, struct mii_dev
>> +**busp) {
>> +       struct udevice *mdio_dev;
>> +       int ret;
>> +
>> +       ret = mdio_device_get_from_eth(eth, &mdio_dev);
>> +       if (ret)
>> +               return ret;
>> +
>> +       if (mdio_dev)
>> +               *busp = (struct mii_dev *)dev_get_uclass_platdata(mdio_dev);
>> +       else
>> +               *busp = NULL;
>> +
>> +       return 0;
>> +}
>> +
>> +static int mdio_uclass_pre_probe(struct udevice *dev) {
>> +       struct mii_dev *bus = (struct mii_dev *)dev_get_uclass_platdata(dev);
>> +       const char *name;
>> +
>> +       /* initialize mii_dev struct fields */
>
> Include a comment that you are implementing mdio_alloc() setup here.
> [Ken] Thanks, I will update.
>
>> +       INIT_LIST_HEAD(&bus->link);
>> +
>> +       name = fdt_getprop(gd->fdt_blob, dev_of_offset(dev),
>> +                          "mdio-name", NULL);
>> +       if (name)
>> +               strncpy(bus->name, name, MDIO_NAME_LEN);
>> +
>> +       return 0;
>> +}
>> +
>> +static int mdio_uclass_post_probe(struct udevice *dev) {
>> +       struct mii_dev *bus = (struct mii_dev
>> +*)dev_get_uclass_platdata(dev);
>> +
>> +       return mdio_register(bus);
>> +}
>> +
>> +UCLASS_DRIVER(mdio) = {
>> +       .id             = UCLASS_MDIO,
>> +       .name           = "mdio",
>> +       .pre_probe      = mdio_uclass_pre_probe,
>> +       .post_probe     = mdio_uclass_post_probe,
>> +       .per_device_platdata_auto_alloc_size = sizeof(struct mii_dev),
>> +};
>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index
>> d7f9df3..504decd 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -49,6 +49,7 @@ enum uclass_id {
>>         UCLASS_LPC,             /* x86 'low pin count' interface */
>>         UCLASS_MAILBOX,         /* Mailbox controller */
>>         UCLASS_MASS_STORAGE,    /* Mass storage device */
>> +       UCLASS_MDIO,            /* Management Data Input/Output(MDIO) device */
>
> Again, include a space before '('.
> [Ken] Thanks a lot for your kind remind!
>
>>         UCLASS_MISC,            /* Miscellaneous device */
>>         UCLASS_MMC,             /* SD / MMC card or chip */
>>         UCLASS_MOD_EXP,         /* RSA Mod Exp device */
>> diff --git a/include/net/mdio.h b/include/net/mdio.h new file mode
>> 100644 index 0000000..5d8d703
>> --- /dev/null
>> +++ b/include/net/mdio.h
>> @@ -0,0 +1,62 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * Copyright (C) 2018 Marvell International Ltd.
>> + * Author: Ken Ma<make@marvell.com>
>> + */
>> +
>> +#ifndef _MDIO_H_
>> +#define _MDIO_H_
>> +
>> +#include <dm.h>        /* Because we dereference struct udevice here */
>
> Me thinks that comment is incorrect. Please delete it.
> [Ken] Thanks, I will update it.
>> +#include <phy.h>
>> +
>> +/**
>> + * mdio_mii_bus_get() - Get mii bus from mdio udevice
>> + *
>> + * @mdio_dev:  mdio udevice
>> + * @busp:      returns mii bus
>> + * @returns 0 on success, error code otherwise.
>> + */
>> +int mdio_mii_bus_get(struct udevice *mdio_dev, struct mii_dev
>> +**busp);
>> +
>> +/**
>> + * mdio_device_get_from_phy() - Get the mdio udevice which the phy
>> +belongs to
>> + *
>> + * @phy_node:  phy node offset
>> + * @devp:      returns mdio udevice
>> + * @returns 0 on success, error code otherwise.
>> + */
>> +int mdio_device_get_from_phy(ofnode phy_node, struct udevice **devp);
>> +
>> +/**
>> + * mdio_mii_bus_get_from_phy() - Get the mii bus which the phy
>> +belongs to
>> + *
>> + * @phy_node:  phy node offset
>> + * @busp:      returns mii bus
>> + * @returns 0 on success, error code otherwise.
>> + */
>> +int mdio_mii_bus_get_from_phy(ofnode phy_node, struct mii_dev
>> +**busp);
>> +
>> +/**
>> + * mdio_device_get_from_eth() - When there is a phy reference of "phy = <&...>"
>> + *                      under an ethernet udevice fdt node, this function can
>> + *                      get the mdio udevice which the phy belongs to
>> + *
>> + * @dev:       the ethernet udevice which contains the phy reference
>> + * @devp:      returns mdio udevice
>> + * @returns 0 on success, error code otherwise.
>> + */
>> +int mdio_device_get_from_eth(struct udevice *eth, struct udevice
>> +**devp);
>> +
>> +/**
>> + * mdio_mii_bus_get_from_eth() - When there is a phy reference of
>> + *                      "phy = <&...>" under an ethernet udevice fdt node, this
>> + *                      function can get the mii bus which the phy belongs to
>> + *
>> + * @eth:       the ethernet udevice which contains the phy reference
>> + * @busp:      returns mii bus
>> + * @returns 0 on success, error code otherwise.
>> + */
>> +int mdio_mii_bus_get_from_eth(struct udevice *eth, struct mii_dev
>> +**busp);
>> +
>> +#endif /* _MDIO_H_ */
>> --
>> 1.9.1
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

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

* [U-Boot] [PATCH v3 2/2] mdio: add marvell MDIO driver
  2018-06-13  4:37 ` [U-Boot] [PATCH v3 2/2] mdio: add marvell MDIO driver make at marvell.com
@ 2018-06-13 13:46   ` Stefan Roese
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Roese @ 2018-06-13 13:46 UTC (permalink / raw)
  To: u-boot

On 13.06.2018 06:37, make at marvell.com wrote:
> From: Ken Ma <make@marvell.com>
> 
> This patch adds a separate driver for the MDIO interface of the
> Marvell Ethernet controllers based on driver model. There are two
> reasons to have a separate driver rather than including it inside
> the MAC driver itself:
>    *) The MDIO interface is shared by all Ethernet ports, so a driver
>       must guarantee non-concurrent accesses to this MDIO interface. The
>       most logical way is to have a separate driver that handles this
>       single MDIO interface, used by all Ethernet ports.
>    *) The MDIO interface is the same between the existing mv643xx_eth
>       driver and the new mvneta/mvpp2 driver. Even though it is for now
>       only used by the mvneta/mvpp2 driver, it will in the future be
>       used by the mv643xx_eth driver as well.
> 
> This driver supports SMI IEEE for 802.3 Clause 22 and XSMI for IEEE
> 802.3 Clause 45.
> 
> This patch also adds device tree binding for marvell MDIO driver.
> 
> Signed-off-by: Ken Ma <make@marvell.com>
> Reviewed-by: Chris Packham <judge.packham@gmail.com>
> ---
> 
> Changes in v3:
> - Move marvell mdio driver to driver/net/mdio folder;
> - Update codes according to mdio uclass implementation updates.
> 
> Changes in v2:
> - Fix error printing:
>    - Change some debug to pr_err;
>    - mii bus has no parent member and it is not a udevice, so dev_err
>      is changed to pr_err for mii bus error printings.
> 
>   MAINTAINERS                                   |   1 +
>   arch/arm/Kconfig                              |   1 +
>   doc/device-tree-bindings/net/marvell-mdio.txt |  18 ++
>   drivers/net/mdio/Kconfig                      |  10 ++
>   drivers/net/mdio/Makefile                     |   1 +
>   drivers/net/mdio/mvmdio.c                     | 234 ++++++++++++++++++++++++++
>   6 files changed, 265 insertions(+)
>   create mode 100644 doc/device-tree-bindings/net/marvell-mdio.txt
>   create mode 100644 drivers/net/mdio/mvmdio.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9e1fc83..d8584f9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -137,6 +137,7 @@ T:	git git://git.denx.de/u-boot-marvell.git
>   F:	arch/arm/mach-kirkwood/
>   F:	arch/arm/mach-mvebu/
>   F:	drivers/ata/ahci_mvebu.c
> +F:	drivers/net/mdio/mvmdio.c
>   
>   ARM MARVELL PXA
>   M:	Marek Vasut <marex@denx.de>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index dde422b..e52b164 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -432,6 +432,7 @@ config ARCH_MVEBU
>   	select DM_SPI
>   	select DM_SPI_FLASH
>   	select SPI
> +	select MDIO
>   
>   config TARGET_DEVKIT3250
>   	bool "Support devkit3250"
> diff --git a/doc/device-tree-bindings/net/marvell-mdio.txt b/doc/device-tree-bindings/net/marvell-mdio.txt
> new file mode 100644
> index 0000000..55db435
> --- /dev/null
> +++ b/doc/device-tree-bindings/net/marvell-mdio.txt
> @@ -0,0 +1,18 @@
> +* Marvell MDIO Ethernet Controller interface
> +
> +The Ethernet controllers of the Marvel Armada 3700 and Armada 7k/8k
> +have an identical unit that provides an interface with the MDIO bus.
> +This driver handles this MDIO interface.
> +
> +Mandatory properties:
> +SoC specific:
> +	- #address-cells: Must be <1>.
> +	- #size-cells: Must be <0>.
> +	- compatible: Should be "marvell,orion-mdio" (for SMI)
> +				"marvell,xmdio"	     (for XSMI)
> +	- reg: Base address and size SMI/XMSI bus.
> +
> +Optional properties:
> +	- mdio-name       - MDIO bus name
> +
> +For example, please refer to "mdio-bus.txt".
> diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
> index 533cc4a..d1a31e6 100644
> --- a/drivers/net/mdio/Kconfig
> +++ b/drivers/net/mdio/Kconfig
> @@ -15,4 +15,14 @@ config MDIO
>   	  udevice or mii bus from its child phy node or
>   	  an ethernet udevice which the phy belongs to.
>   
> +config MVMDIO
> +	bool "Marvell MDIO interface support"
> +	depends on MDIO
> +	select PHYLIB
> +	help
> +	  This driver supports the MDIO interface found in the network
> +	  interface units of the Marvell EBU SoCs (Kirkwood, Orion5x,
> +	  Dove, Armada 370, Armada XP, Armada 37xx and Armada7K/8K/8KP).
> +
> +	  This driver is used by the MVPP2 and MVNETA drivers.
>   endmenu
> diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
> index 45ae502..8b2e5a4 100644
> --- a/drivers/net/mdio/Makefile
> +++ b/drivers/net/mdio/Makefile
> @@ -4,3 +4,4 @@
>   # Author: Ken Ma<make@marvell.com>
>   
>   obj-$(CONFIG_MDIO) += mdio-uclass.o
> +obj-$(CONFIG_MVMDIO) += mvmdio.o
> diff --git a/drivers/net/mdio/mvmdio.c b/drivers/net/mdio/mvmdio.c
> new file mode 100644
> index 0000000..0fb45ce
> --- /dev/null
> +++ b/drivers/net/mdio/mvmdio.c
> @@ -0,0 +1,234 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2018 Marvell International Ltd.
> + * Author: Ken Ma<make@marvell.com>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <dm/device-internal.h>
> +#include <dm/lists.h>
> +#include <miiphy.h>
> +#include <phy.h>
> +#include <asm/io.h>
> +
> +#define MVMDIO_SMI_DATA_SHIFT		0
> +#define MVMDIO_SMI_PHY_ADDR_SHIFT	16
> +#define MVMDIO_SMI_PHY_REG_SHIFT	21
> +#define MVMDIO_SMI_READ_OPERATION	BIT(26)
> +#define MVMDIO_SMI_WRITE_OPERATION	0
> +#define MVMDIO_SMI_READ_VALID		BIT(27)
> +#define MVMDIO_SMI_BUSY			BIT(28)
> +
> +#define MVMDIO_XSMI_MGNT_REG		0x0
> +#define MVMDIO_XSMI_PHYADDR_SHIFT	16
> +#define MVMDIO_XSMI_DEVADDR_SHIFT	21
> +#define MVMDIO_XSMI_WRITE_OPERATION	(0x5 << 26)
> +#define MVMDIO_XSMI_READ_OPERATION	(0x7 << 26)
> +#define MVMDIO_XSMI_READ_VALID		BIT(29)
> +#define MVMDIO_XSMI_BUSY		BIT(30)
> +#define MVMDIO_XSMI_ADDR_REG		0x8
> +
> +#define MVMDIO_TIMEOUT			10000
> +
> +struct mvmdio_priv {
> +	void *mdio_base;
> +};
> +
> +enum mvmdio_bus_type {
> +	BUS_TYPE_SMI,
> +	BUS_TYPE_XSMI
> +};
> +
> +/* Wait for the SMI unit to be ready for another operation */
> +static int mvmdio_smi_wait_ready(struct mii_dev *bus)
> +{
> +	u32 timeout = MVMDIO_TIMEOUT;
> +	struct mvmdio_priv *priv = bus->priv;
> +	u32 smi_reg;
> +
> +	/* Wait till the SMI is not busy */
> +	do {
> +		/* Read smi register */
> +		smi_reg = readl(priv->mdio_base);
> +		if (timeout-- == 0) {
> +			pr_err("Error: SMI busy timeout\n");
> +			return -ETIME;
> +		}
> +	} while (smi_reg & MVMDIO_SMI_BUSY);
> +
> +	return 0;
> +}

You could use wait_for_bit_le32() here instead of implementing your
own custom busy wait polling function (include/wait_bit.h). This also
adds more accurate timeout handling (via timer).

Other than this:

Reviewed-by: Stefan Roese <sr@denx.de>

> +
> +static int mvmdio_smi_read(struct mii_dev *bus, int addr,
> +			   int devad, int reg)
> +{
> +	struct mvmdio_priv *priv = bus->priv;
> +	u32 val;
> +	int ret;
> +
> +	if (devad != MDIO_DEVAD_NONE)
> +		return -EOPNOTSUPP;
> +
> +	ret = mvmdio_smi_wait_ready(bus);
> +	if (ret < 0)
> +		return ret;
> +
> +	writel(((addr << MVMDIO_SMI_PHY_ADDR_SHIFT) |
> +		(reg << MVMDIO_SMI_PHY_REG_SHIFT)  |
> +		MVMDIO_SMI_READ_OPERATION),
> +	       priv->mdio_base);
> +
> +	ret = mvmdio_smi_wait_ready(bus);
> +	if (ret < 0)
> +		return ret;
> +
> +	val = readl(priv->mdio_base);
> +	if (!(val & MVMDIO_SMI_READ_VALID)) {
> +		pr_err("SMI bus read not valid\n");
> +		return -ENODEV;
> +	}
> +
> +	return val & GENMASK(15, 0);
> +}
> +
> +static int mvmdio_smi_write(struct mii_dev *bus, int addr, int devad,
> +			    int reg, u16 value)
> +{
> +	struct mvmdio_priv *priv = bus->priv;
> +	int ret;
> +
> +	if (devad != MDIO_DEVAD_NONE)
> +		return -EOPNOTSUPP;
> +
> +	ret = mvmdio_smi_wait_ready(bus);
> +	if (ret < 0)
> +		return ret;
> +
> +	writel(((addr << MVMDIO_SMI_PHY_ADDR_SHIFT) |
> +		(reg << MVMDIO_SMI_PHY_REG_SHIFT)  |
> +		MVMDIO_SMI_WRITE_OPERATION            |
> +		(value << MVMDIO_SMI_DATA_SHIFT)),
> +	       priv->mdio_base);
> +
> +	return 0;
> +}
> +
> +static int mvmdio_xsmi_wait_ready(struct mii_dev *bus)
> +{
> +	u32 timeout = MVMDIO_TIMEOUT;
> +	struct mvmdio_priv *priv = bus->priv;
> +	u32 xsmi_reg;
> +
> +	/* Wait till the xSMI is not busy */
> +	do {
> +		/* Read xSMI register */
> +		xsmi_reg = readl(priv->mdio_base);
> +		if (timeout-- == 0) {
> +			pr_err("xSMI busy time-out\n");
> +			return -ETIME;
> +		}
> +	} while (xsmi_reg & MVMDIO_XSMI_BUSY);
> +
> +	return 0;
> +}

Again, please use the already available wait bit functions.

Other than this:

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

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

* [U-Boot] [PATCH v3 2/2] mdio: add marvell MDIO driver
  2018-06-13  4:37 [U-Boot] [PATCH v3 0/2] Add MDIO driver model support make at marvell.com
@ 2018-06-13  4:37 ` make at marvell.com
  2018-06-13 13:46   ` Stefan Roese
  0 siblings, 1 reply; 10+ messages in thread
From: make at marvell.com @ 2018-06-13  4:37 UTC (permalink / raw)
  To: u-boot

From: Ken Ma <make@marvell.com>

This patch adds a separate driver for the MDIO interface of the
Marvell Ethernet controllers based on driver model. There are two
reasons to have a separate driver rather than including it inside
the MAC driver itself:
  *) The MDIO interface is shared by all Ethernet ports, so a driver
     must guarantee non-concurrent accesses to this MDIO interface. The
     most logical way is to have a separate driver that handles this
     single MDIO interface, used by all Ethernet ports.
  *) The MDIO interface is the same between the existing mv643xx_eth
     driver and the new mvneta/mvpp2 driver. Even though it is for now
     only used by the mvneta/mvpp2 driver, it will in the future be
     used by the mv643xx_eth driver as well.

This driver supports SMI IEEE for 802.3 Clause 22 and XSMI for IEEE
802.3 Clause 45.

This patch also adds device tree binding for marvell MDIO driver.

Signed-off-by: Ken Ma <make@marvell.com>
Reviewed-by: Chris Packham <judge.packham@gmail.com>
---

Changes in v3:
- Move marvell mdio driver to driver/net/mdio folder;
- Update codes according to mdio uclass implementation updates.

Changes in v2:
- Fix error printing:
  - Change some debug to pr_err;
  - mii bus has no parent member and it is not a udevice, so dev_err
    is changed to pr_err for mii bus error printings.

 MAINTAINERS                                   |   1 +
 arch/arm/Kconfig                              |   1 +
 doc/device-tree-bindings/net/marvell-mdio.txt |  18 ++
 drivers/net/mdio/Kconfig                      |  10 ++
 drivers/net/mdio/Makefile                     |   1 +
 drivers/net/mdio/mvmdio.c                     | 234 ++++++++++++++++++++++++++
 6 files changed, 265 insertions(+)
 create mode 100644 doc/device-tree-bindings/net/marvell-mdio.txt
 create mode 100644 drivers/net/mdio/mvmdio.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9e1fc83..d8584f9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -137,6 +137,7 @@ T:	git git://git.denx.de/u-boot-marvell.git
 F:	arch/arm/mach-kirkwood/
 F:	arch/arm/mach-mvebu/
 F:	drivers/ata/ahci_mvebu.c
+F:	drivers/net/mdio/mvmdio.c
 
 ARM MARVELL PXA
 M:	Marek Vasut <marex@denx.de>
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index dde422b..e52b164 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -432,6 +432,7 @@ config ARCH_MVEBU
 	select DM_SPI
 	select DM_SPI_FLASH
 	select SPI
+	select MDIO
 
 config TARGET_DEVKIT3250
 	bool "Support devkit3250"
diff --git a/doc/device-tree-bindings/net/marvell-mdio.txt b/doc/device-tree-bindings/net/marvell-mdio.txt
new file mode 100644
index 0000000..55db435
--- /dev/null
+++ b/doc/device-tree-bindings/net/marvell-mdio.txt
@@ -0,0 +1,18 @@
+* Marvell MDIO Ethernet Controller interface
+
+The Ethernet controllers of the Marvel Armada 3700 and Armada 7k/8k
+have an identical unit that provides an interface with the MDIO bus.
+This driver handles this MDIO interface.
+
+Mandatory properties:
+SoC specific:
+	- #address-cells: Must be <1>.
+	- #size-cells: Must be <0>.
+	- compatible: Should be "marvell,orion-mdio" (for SMI)
+				"marvell,xmdio"	     (for XSMI)
+	- reg: Base address and size SMI/XMSI bus.
+
+Optional properties:
+	- mdio-name       - MDIO bus name
+
+For example, please refer to "mdio-bus.txt".
diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
index 533cc4a..d1a31e6 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -15,4 +15,14 @@ config MDIO
 	  udevice or mii bus from its child phy node or
 	  an ethernet udevice which the phy belongs to.
 
+config MVMDIO
+	bool "Marvell MDIO interface support"
+	depends on MDIO
+	select PHYLIB
+	help
+	  This driver supports the MDIO interface found in the network
+	  interface units of the Marvell EBU SoCs (Kirkwood, Orion5x,
+	  Dove, Armada 370, Armada XP, Armada 37xx and Armada7K/8K/8KP).
+
+	  This driver is used by the MVPP2 and MVNETA drivers.
 endmenu
diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
index 45ae502..8b2e5a4 100644
--- a/drivers/net/mdio/Makefile
+++ b/drivers/net/mdio/Makefile
@@ -4,3 +4,4 @@
 # Author: Ken Ma<make@marvell.com>
 
 obj-$(CONFIG_MDIO) += mdio-uclass.o
+obj-$(CONFIG_MVMDIO) += mvmdio.o
diff --git a/drivers/net/mdio/mvmdio.c b/drivers/net/mdio/mvmdio.c
new file mode 100644
index 0000000..0fb45ce
--- /dev/null
+++ b/drivers/net/mdio/mvmdio.c
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Marvell International Ltd.
+ * Author: Ken Ma<make@marvell.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+#include <miiphy.h>
+#include <phy.h>
+#include <asm/io.h>
+
+#define MVMDIO_SMI_DATA_SHIFT		0
+#define MVMDIO_SMI_PHY_ADDR_SHIFT	16
+#define MVMDIO_SMI_PHY_REG_SHIFT	21
+#define MVMDIO_SMI_READ_OPERATION	BIT(26)
+#define MVMDIO_SMI_WRITE_OPERATION	0
+#define MVMDIO_SMI_READ_VALID		BIT(27)
+#define MVMDIO_SMI_BUSY			BIT(28)
+
+#define MVMDIO_XSMI_MGNT_REG		0x0
+#define MVMDIO_XSMI_PHYADDR_SHIFT	16
+#define MVMDIO_XSMI_DEVADDR_SHIFT	21
+#define MVMDIO_XSMI_WRITE_OPERATION	(0x5 << 26)
+#define MVMDIO_XSMI_READ_OPERATION	(0x7 << 26)
+#define MVMDIO_XSMI_READ_VALID		BIT(29)
+#define MVMDIO_XSMI_BUSY		BIT(30)
+#define MVMDIO_XSMI_ADDR_REG		0x8
+
+#define MVMDIO_TIMEOUT			10000
+
+struct mvmdio_priv {
+	void *mdio_base;
+};
+
+enum mvmdio_bus_type {
+	BUS_TYPE_SMI,
+	BUS_TYPE_XSMI
+};
+
+/* Wait for the SMI unit to be ready for another operation */
+static int mvmdio_smi_wait_ready(struct mii_dev *bus)
+{
+	u32 timeout = MVMDIO_TIMEOUT;
+	struct mvmdio_priv *priv = bus->priv;
+	u32 smi_reg;
+
+	/* Wait till the SMI is not busy */
+	do {
+		/* Read smi register */
+		smi_reg = readl(priv->mdio_base);
+		if (timeout-- == 0) {
+			pr_err("Error: SMI busy timeout\n");
+			return -ETIME;
+		}
+	} while (smi_reg & MVMDIO_SMI_BUSY);
+
+	return 0;
+}
+
+static int mvmdio_smi_read(struct mii_dev *bus, int addr,
+			   int devad, int reg)
+{
+	struct mvmdio_priv *priv = bus->priv;
+	u32 val;
+	int ret;
+
+	if (devad != MDIO_DEVAD_NONE)
+		return -EOPNOTSUPP;
+
+	ret = mvmdio_smi_wait_ready(bus);
+	if (ret < 0)
+		return ret;
+
+	writel(((addr << MVMDIO_SMI_PHY_ADDR_SHIFT) |
+		(reg << MVMDIO_SMI_PHY_REG_SHIFT)  |
+		MVMDIO_SMI_READ_OPERATION),
+	       priv->mdio_base);
+
+	ret = mvmdio_smi_wait_ready(bus);
+	if (ret < 0)
+		return ret;
+
+	val = readl(priv->mdio_base);
+	if (!(val & MVMDIO_SMI_READ_VALID)) {
+		pr_err("SMI bus read not valid\n");
+		return -ENODEV;
+	}
+
+	return val & GENMASK(15, 0);
+}
+
+static int mvmdio_smi_write(struct mii_dev *bus, int addr, int devad,
+			    int reg, u16 value)
+{
+	struct mvmdio_priv *priv = bus->priv;
+	int ret;
+
+	if (devad != MDIO_DEVAD_NONE)
+		return -EOPNOTSUPP;
+
+	ret = mvmdio_smi_wait_ready(bus);
+	if (ret < 0)
+		return ret;
+
+	writel(((addr << MVMDIO_SMI_PHY_ADDR_SHIFT) |
+		(reg << MVMDIO_SMI_PHY_REG_SHIFT)  |
+		MVMDIO_SMI_WRITE_OPERATION            |
+		(value << MVMDIO_SMI_DATA_SHIFT)),
+	       priv->mdio_base);
+
+	return 0;
+}
+
+static int mvmdio_xsmi_wait_ready(struct mii_dev *bus)
+{
+	u32 timeout = MVMDIO_TIMEOUT;
+	struct mvmdio_priv *priv = bus->priv;
+	u32 xsmi_reg;
+
+	/* Wait till the xSMI is not busy */
+	do {
+		/* Read xSMI register */
+		xsmi_reg = readl(priv->mdio_base);
+		if (timeout-- == 0) {
+			pr_err("xSMI busy time-out\n");
+			return -ETIME;
+		}
+	} while (xsmi_reg & MVMDIO_XSMI_BUSY);
+
+	return 0;
+}
+
+static int mvmdio_xsmi_read(struct mii_dev *bus, int addr,
+			    int devad, int reg)
+{
+	struct mvmdio_priv *priv = bus->priv;
+	int ret;
+
+	if (devad == MDIO_DEVAD_NONE)
+		return -EOPNOTSUPP;
+
+	ret = mvmdio_xsmi_wait_ready(bus);
+	if (ret < 0)
+		return ret;
+
+	writel(reg & GENMASK(15, 0), priv->mdio_base + MVMDIO_XSMI_ADDR_REG);
+	writel(((addr << MVMDIO_XSMI_PHYADDR_SHIFT) |
+		(devad << MVMDIO_XSMI_DEVADDR_SHIFT) |
+		MVMDIO_XSMI_READ_OPERATION),
+	       priv->mdio_base + MVMDIO_XSMI_MGNT_REG);
+
+	ret = mvmdio_xsmi_wait_ready(bus);
+	if (ret < 0)
+		return ret;
+
+	if (!(readl(priv->mdio_base + MVMDIO_XSMI_MGNT_REG) &
+	      MVMDIO_XSMI_READ_VALID)) {
+		pr_err("XSMI bus read not valid\n");
+		return -ENODEV;
+	}
+
+	return readl(priv->mdio_base + MVMDIO_XSMI_MGNT_REG) & GENMASK(15, 0);
+}
+
+static int mvmdio_xsmi_write(struct mii_dev *bus, int addr, int devad,
+			     int reg, u16 value)
+{
+	struct mvmdio_priv *priv = bus->priv;
+	int ret;
+
+	if (devad == MDIO_DEVAD_NONE)
+		return -EOPNOTSUPP;
+
+	ret = mvmdio_xsmi_wait_ready(bus);
+	if (ret < 0)
+		return ret;
+
+	writel(reg & GENMASK(15, 0), priv->mdio_base + MVMDIO_XSMI_ADDR_REG);
+	writel(((addr << MVMDIO_XSMI_PHYADDR_SHIFT) |
+		(devad << MVMDIO_XSMI_DEVADDR_SHIFT) |
+		MVMDIO_XSMI_WRITE_OPERATION | value),
+	       priv->mdio_base + MVMDIO_XSMI_MGNT_REG);
+
+	return 0;
+}
+
+static int mvmdio_probe(struct udevice *dev)
+{
+	struct mii_dev *bus = (struct mii_dev *)dev_get_uclass_platdata(dev);
+	struct mvmdio_priv *priv;
+	enum mvmdio_bus_type type;
+
+	priv = dev_get_priv(dev);
+	priv->mdio_base = (void *)dev_read_addr(dev);
+	bus->priv = priv;
+
+	type = (enum mvmdio_bus_type)dev_get_driver_data(dev);
+	switch (type) {
+	case BUS_TYPE_SMI:
+		bus->read = mvmdio_smi_read;
+		bus->write = mvmdio_smi_write;
+		if (!bus->name)
+			snprintf(bus->name, MDIO_NAME_LEN,
+				 "orion-mdio.%p", priv->mdio_base);
+		break;
+	case BUS_TYPE_XSMI:
+		bus->read = mvmdio_xsmi_read;
+		bus->write = mvmdio_xsmi_write;
+		if (!bus->name)
+			snprintf(bus->name, MDIO_NAME_LEN,
+				 "xmdio.%p", priv->mdio_base);
+		break;
+	}
+
+	return 0;
+}
+
+static const struct udevice_id mvmdio_ids[] = {
+	{ .compatible = "marvell,orion-mdio", .data = BUS_TYPE_SMI },
+	{ .compatible = "marvell,xmdio", .data = BUS_TYPE_XSMI },
+	{ }
+};
+
+U_BOOT_DRIVER(mvmdio) = {
+	.name			= "mvmdio",
+	.id			= UCLASS_MDIO,
+	.of_match		= mvmdio_ids,
+	.probe			= mvmdio_probe,
+	.priv_auto_alloc_size	= sizeof(struct mvmdio_priv),
+};
+
-- 
1.9.1

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

end of thread, other threads:[~2018-07-09 20:52 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-13  4:33 [U-Boot] [PATCH v3 0/2] Add MDIO driver model support make at marvell.com
2018-06-13  4:33 ` [U-Boot] [PATCH v3 1/2] dm: mdio: add a uclass for MDIO make at marvell.com
2018-06-14 12:58   ` Simon Glass
2018-06-19 20:58   ` Joe Hershberger
2018-07-05  7:18     ` [U-Boot] [EXT] " Ken Ma
2018-07-09 20:52       ` Joe Hershberger
2018-06-19 21:13   ` [U-Boot] " Joe Hershberger
2018-06-13  4:33 ` [U-Boot] [PATCH v3 2/2] mdio: add marvell MDIO driver make at marvell.com
2018-06-13  4:37 [U-Boot] [PATCH v3 0/2] Add MDIO driver model support make at marvell.com
2018-06-13  4:37 ` [U-Boot] [PATCH v3 2/2] mdio: add marvell MDIO driver make at marvell.com
2018-06-13 13:46   ` Stefan Roese

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.