Broadcom iProc based SoCs uses MDIO bus for programming PHYs belonging to
different I/O subsystem like USB, SATA, PCIe, ETHERNET etc. Every subsystem
is referred as "Master" When a master is selected, all PHYs belonging to
this subsystem get active on the MDIO bus and responds to MDIO transaction.
In this way one MDIO controller is shared among all masters hence named as
"Shared MDIO controller".
We have two important entities in "Shared MDIO Bus" framework:
1) shared_mdio_master and
2) shared_mdio_driver.
The shared MDIO controller driver is registered as platform driver and it
creates shared_mdio_master instances and adds them to "Shared MDIO Bus"
framework. The "Shared MDIO Bus" framework will try to match-n-probe each
shared_mdio_master instance with all available shared_mdio_driver based on
DT matching. The shared_mdio_driver will acts as phy provider for
respective PHY frameworks and it will access PHY registers only using
"Shared MDIO Bus" framework APIs. All PHY read/write from
shared_mdio_driver will eventually reach shared MDIO controller driver
(who originally created shared_mdio_master instances) and shared MDIO
controller driver will serialized these PHY read/write as required.
This patch set includes Shared MDIO Bus framework, Shared MDIO block driver
and one Shared MDIO driver, ETHERNET PHY provider driver.
shared_mdio.c: This is the bus framework where all masters are registered
via shared MDIO block driver.
iproc_shared_mdio.c: Represent the shared MDIO block driver. The driver
registers all masters to shared mdio bus framework.
mdio-bcm-iproc-shared.c: Master driver which registers ETHERNET phy to
legacy MII framework.
In future there will be more driver for SATA, USB, PCIe masters which will
register phys for their subsystem.
Patch series is developed based on Linux v4.6-rc1 and available at:
repo: https://github.com/Broadcom/arm64-linux.git
branch: shared_mdio_v0
Pramod Kumar (6):
bus: Add shared MDIO bus framework
Documentation: DT binding doc for iProc Shared MDIO Controller.
bus: Add platform driver for iProc shared MDIO Controller
dt: Add Shared MDIO Controller node for NS2
Documentation: Binding doc for ethernet master in NS2
net:phy: Add Ethernet Master for iProc Shared MDIO Controller
.../bindings/bus/brcm,iproc-shared-mdio.txt | 76 ++++++
.../bindings/net/brcm,iproc-mdio-shared.txt | 32 +++
arch/arm64/boot/dts/broadcom/ns2-svk.dts | 15 ++
arch/arm64/boot/dts/broadcom/ns2.dtsi | 8 +
drivers/bus/Kconfig | 22 ++
drivers/bus/Makefile | 2 +
drivers/bus/iproc_shared_mdio.c | 287 +++++++++++++++++++++
drivers/bus/shared_mdio.c | 179 +++++++++++++
drivers/net/phy/Kconfig | 11 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/mdio-bcm-iproc-shared.c | 116 +++++++++
include/linux/shared_mdio.h | 123 +++++++++
12 files changed, 872 insertions(+)
create mode 100644 Documentation/devicetree/bindings/bus/brcm,iproc-shared-mdio.txt
create mode 100644 Documentation/devicetree/bindings/net/brcm,iproc-mdio-shared.txt
create mode 100644 drivers/bus/iproc_shared_mdio.c
create mode 100644 drivers/bus/shared_mdio.c
create mode 100644 drivers/net/phy/mdio-bcm-iproc-shared.c
create mode 100644 include/linux/shared_mdio.h
--
1.9.1
Add DT binding doc for iProc Shared MDIO Controller which
populate all masters to Shared MDIO framework.
Signed-off-by: Pramod Kumar <[email protected]>
Reviewed-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
---
.../bindings/bus/brcm,iproc-shared-mdio.txt | 76 ++++++++++++++++++++++
1 file changed, 76 insertions(+)
create mode 100644 Documentation/devicetree/bindings/bus/brcm,iproc-shared-mdio.txt
diff --git a/Documentation/devicetree/bindings/bus/brcm,iproc-shared-mdio.txt b/Documentation/devicetree/bindings/bus/brcm,iproc-shared-mdio.txt
new file mode 100644
index 0000000..f455f3d
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/brcm,iproc-shared-mdio.txt
@@ -0,0 +1,76 @@
+Broadcom iProc Shared MDIO Controller
+
+Required properties:
+- compatible:
+ "brcm,iproc-shared-mdio" for shared MDIO controller.
+- reg: specifies the base physical address and size of the registers
+- reg-names: should be "mdio"
+- #address-cells: must be 1.
+- #size-cells: must be 0.
+
+optional:
+child nodes: Masters are represented as a child node of shared MDIO controller
+and all the PHYs handled by this master are represented as its child node.
+
+Master nodes properties are defined as-
+
+Required properties:
+- compatible: Used to match driver of this PHY.
+- reg: MDIO master ID.
+- #address-cells: must be 1.
+- #size-cells: must be 0.
+
+optional:
+-brcm,phy-internal: if presents, PHYs are internal. Absence shows phy is
+external.
+-brcm,is-c45: if presents, Controller uses Clause-45 to issue MDIO transaction.
+else Controller uses Clause-22 for transactions.
+
+PHY nodes are represented as the child node of Master. Child nodes properties
+are defined as-
+
+Required properties:
+-reg: phy id of attached PHY.
+
+optional:
+There could be additional properties required to configure the specific phy like
+phy-mode in case of gpphy node below. These should be defined here and used by
+respective drivers.
+
+Example:
+iproc_mdio: iproc_mdio@663f0000 {
+ compatible = "brcm,iproc-shared-mdio";
+ reg = <0x6602023c 0x14>;
+ reg-names = "mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ sata-master@6 {
+ compatible = "brcm,iproc-ns2-sata-phy";
+ reg = <0x6>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ brcm,phy-internal;
+
+ sata_phy0: sata-phy@1 {
+ reg = <0x1>;
+ #phy-cells = <0>;
+ };
+
+ sata_phy1: sata-phy@2 {
+ reg = <0x2>;
+ #phy-cells = <0>;
+ };
+ };
+
+ eth-master@0 {
+ compatible = "brcm,iproc-mdio-master-eth";
+ reg = <0x0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ gphy0: eth-phy@10 {
+ reg = <0x10>;
+ phy-mode = "mii";
+ };
+ };
+};
--
1.9.1
Add platform driver to populate shared MDIO masters on iProc SoC.
Signed-off-by: Pramod Kumar <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Reviewed-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
Reviewed-by: Vikram Prakash <[email protected]>
---
drivers/bus/Kconfig | 11 ++
drivers/bus/Makefile | 1 +
drivers/bus/iproc_shared_mdio.c | 287 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 299 insertions(+)
create mode 100644 drivers/bus/iproc_shared_mdio.c
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 1b51b1a..c3f2cb9 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -118,6 +118,17 @@ config SHARED_MDIO
Respective drivers would be called matching a compatible string
provided in master node.
+config IPROC_SHARED_MDIO
+ tristate "iProc Shared MDIO Driver"
+ depends on OF
+ select SHARED_MDIO
+ default ARCH_BCM_IPROC
+ help
+ Platform driver for shared MDIO controller. Broadcom's iProc based
+ SoCs have many masters which uses one shared bus to accessed its PHY.
+ This platform driver populates all master to Shared bus and provide
+ the MDIO controller specific implementation.
+
config SIMPLE_PM_BUS
bool "Simple Power-Managed Bus Driver"
depends on OF && PM
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index 2b6d6e9..3e364da 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_SIMPLE_PM_BUS) += simple-pm-bus.o
obj-$(CONFIG_UNIPHIER_SYSTEM_BUS) += uniphier-system-bus.o
obj-$(CONFIG_VEXPRESS_CONFIG) += vexpress-config.o
obj-$(CONFIG_SHARED_MDIO) += shared_mdio.o
+obj-$(CONFIG_IPROC_SHARED_MDIO) += iproc_shared_mdio.o
diff --git a/drivers/bus/iproc_shared_mdio.c b/drivers/bus/iproc_shared_mdio.c
new file mode 100644
index 0000000..af232eb
--- /dev/null
+++ b/drivers/bus/iproc_shared_mdio.c
@@ -0,0 +1,287 @@
+/*
+ * Copyright (C) 2016 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Platform driver for Shared MDIO Masters on iProc SoC
+ */
+
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/shared_mdio.h>
+#include <linux/mutex.h>
+#include <linux/delay.h>
+
+struct shared_mdio_master_param {
+ bool is_internal;
+ bool is_c45;
+ unsigned int master_id;
+};
+
+struct iproc_shared_mdio {
+ struct device *dev;
+ struct mutex lock;
+ void __iomem *regs;
+ unsigned int num_masters;
+ struct shared_mdio_master **masters;
+};
+
+#define PHY_INTERNAL 1
+#define CLAUSE_45 1
+
+#define MDIO_PARAM_OFFSET 0x00
+#define MDIO_PARAM_MIIM_CYCLE 29
+#define MDIO_PARAM_INTERNAL_SEL 25
+#define MDIO_PARAM_BUS_ID 22
+#define MDIO_PARAM_C45_SEL 21
+#define MDIO_PARAM_PHY_ID 16
+#define MDIO_PARAM_PHY_DATA 0
+
+#define MDIO_READ_OFFSET 0x04
+#define MDIO_READ_DATA_MASK 0xffff
+#define MDIO_ADDR_OFFSET 0x08
+
+#define MDIO_CTRL_OFFSET 0x0C
+#define MDIO_CTRL_WRITE_OP 0x1
+#define MDIO_CTRL_READ_OP 0x2
+
+#define MDIO_STAT_OFFSET 0x10
+#define MDIO_STAT_DONE 1
+
+static int iproc_mdio_wait_for_idle(void __iomem *base, bool result)
+{
+ u32 val;
+ unsigned int timeout = 1000; /* loop for 1s */
+
+ do {
+ val = readl(base + MDIO_STAT_OFFSET);
+ if ((val & MDIO_STAT_DONE) == result)
+ return 0;
+
+ usleep_range(1000, 2000);
+ } while (timeout--);
+
+ return -ETIMEDOUT;
+}
+
+static inline u32 get_cmd(bool con, u16 mid, bool is_c45, u16 phyid, u16 val)
+{
+ u32 cmd;
+
+ cmd = (con ? 1 : 0) << MDIO_PARAM_INTERNAL_SEL;
+ cmd |= mid << MDIO_PARAM_BUS_ID;
+ cmd |= (is_c45 ? 1 : 0) << MDIO_PARAM_C45_SEL;
+ cmd |= phyid << MDIO_PARAM_PHY_ID;
+ cmd |= val << MDIO_PARAM_PHY_DATA;
+
+ return cmd;
+}
+
+/*
+ * start_miim_ops- Program and start MDIO transaction over mdio bus.
+ * @base: Base address
+ * @cmd: param values that need to be programmed in MDIO PARAM_OFFSET.
+ * @reg: register offset ot be read/written.
+ * @op: Operation that need to be carried out.
+ * MDIO_CTRL_READ_OP: Read transaction.
+ * MDIO_CTRL_WRITE_OP: Write transaction.
+ *
+ * Return value: Sucessfull Read operation returns read reg values and write
+ * operation returns 0. Failure opertation returns negative error code.
+ */
+static int start_miim_ops(void __iomem *base, u32 cmd, u32 reg, u32 op)
+{
+ int ret = 0;
+
+ writel(0, base + MDIO_CTRL_OFFSET);
+ ret = iproc_mdio_wait_for_idle(base, false);
+ if (ret)
+ return ret;
+
+ writel(cmd, base + MDIO_PARAM_OFFSET);
+ writel(reg, base + MDIO_ADDR_OFFSET);
+
+ writel(op, base + MDIO_CTRL_OFFSET);
+
+ ret = iproc_mdio_wait_for_idle(base, true);
+ if (ret)
+ return ret;
+
+ if (op == MDIO_CTRL_READ_OP)
+ ret = readl(base + MDIO_READ_OFFSET) & MDIO_READ_DATA_MASK;
+
+ return ret;
+}
+
+static int iproc_shared_mdio_read(struct shared_mdio_master *master,
+ u16 phyid, u16 reg)
+{
+ int ret;
+ u32 cmd;
+ struct iproc_shared_mdio *imdio = dev_get_drvdata(master->dev.parent);
+ struct shared_mdio_master_param *param = master->priv;
+
+ mutex_lock(&imdio->lock);
+
+ dev_dbg(imdio->dev, "master=%d phy=%d reg=%d\n",
+ param->master_id, phyid, reg);
+
+ cmd = get_cmd(param->is_internal, param->master_id, param->is_c45,
+ phyid, 0x0);
+ ret = start_miim_ops(imdio->regs, cmd, reg, MDIO_CTRL_READ_OP);
+ if (ret < 0)
+ dev_err(imdio->dev, "MDIO read operation failed!!!");
+
+ mutex_unlock(&imdio->lock);
+ return ret;
+}
+
+static int iproc_shared_mdio_write(struct shared_mdio_master *master,
+ u16 phyid, u16 reg, u16 val)
+{
+ u32 cmd;
+ int ret;
+ struct iproc_shared_mdio *imdio = dev_get_drvdata(master->dev.parent);
+ struct shared_mdio_master_param *param = master->priv;
+
+ mutex_lock(&imdio->lock);
+
+ dev_dbg(imdio->dev, "master=%d phy=%d reg=%d val=0x%x\n",
+ param->master_id, phyid, reg, val);
+
+ /* Write val at reg offset */
+ cmd = get_cmd(param->is_internal, param->master_id, param->is_c45,
+ phyid, val);
+ ret = start_miim_ops(imdio->regs, cmd, reg, MDIO_CTRL_WRITE_OP);
+ if (ret < 0)
+ dev_err(imdio->dev, "MDIO Write operation failed!!!");
+
+ mutex_unlock(&imdio->lock);
+ return ret;
+}
+
+static int iproc_shared_mdio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *dn = dev->of_node, *child;
+ struct iproc_shared_mdio *imdio;
+ struct resource *res;
+ int ret = 0, cnt;
+
+ imdio = devm_kzalloc(dev, sizeof(*imdio), GFP_KERNEL);
+ if (!imdio)
+ return -ENOMEM;
+
+ imdio->num_masters = of_get_available_child_count(dn);
+ if (!imdio->num_masters)
+ return -ENODEV;
+
+ imdio->masters = devm_kcalloc(dev, imdio->num_masters,
+ sizeof(**imdio->masters), GFP_KERNEL);
+ if (!imdio->masters)
+ return -ENOMEM;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mdio");
+ imdio->regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(imdio->regs))
+ return PTR_ERR(imdio->regs);
+
+ imdio->dev = dev;
+ mutex_init(&imdio->lock);
+
+ cnt = 0;
+ for_each_available_child_of_node(dn, child) {
+ unsigned int id;
+ struct shared_mdio_master *master;
+ struct shared_mdio_master_param *param;
+
+ if (of_property_read_u32(child, "reg", &id)) {
+ dev_err(dev, "missing reg property in node %s\n",
+ child->name);
+ ret = -EINVAL;
+ goto fail;
+ }
+
+ master = shared_mdio_alloc_master(dev, child);
+ if (IS_ERR(master)) {
+ ret = PTR_ERR(master);
+ goto fail;
+ }
+ imdio->masters[cnt] = master;
+
+ param = devm_kzalloc(dev, sizeof(*param), GFP_KERNEL);
+ if (!param) {
+ ret = -ENOMEM;
+ goto fail;
+ }
+ master->priv = param;
+ param->master_id = id;
+
+ param->is_internal = of_property_read_bool(child,
+ "brcm,phy-internal");
+ param->is_c45 = of_property_read_bool(child, "brcm,is-c45");
+ master->mdio_read = iproc_shared_mdio_read;
+ master->mdio_write = iproc_shared_mdio_write;
+
+ ret = shared_mdio_add_master(master);
+ if (ret)
+ goto fail;
+ cnt++;
+ }
+ platform_set_drvdata(pdev, imdio);
+ dev_info(dev, "registered %d masters(s)\n", cnt);
+ return 0;
+fail:
+ for (cnt = 0; cnt < imdio->num_masters; cnt++) {
+ if (imdio->masters[cnt]) {
+ shared_mdio_remove_master(imdio->masters[cnt]);
+ imdio->masters[cnt] = NULL;
+ }
+ }
+ return ret;
+}
+
+static int iproc_shared_mdio_remove(struct platform_device *pdev)
+{
+ int i;
+ struct iproc_shared_mdio *imdio = platform_get_drvdata(pdev);
+
+ for (i = 0; i < imdio->num_masters; i++) {
+ if (imdio->masters[i]) {
+ shared_mdio_remove_master(imdio->masters[i]);
+ imdio->masters[i] = NULL;
+ }
+ }
+
+ return 0;
+}
+
+static const struct of_device_id iproc_shared_mdio_of_match[] = {
+ { .compatible = "brcm,iproc-shared-mdio", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, iproc_shared_mdio_of_match);
+
+static struct platform_driver iproc_shared_mdio_driver = {
+ .probe = iproc_shared_mdio_probe,
+ .remove = iproc_shared_mdio_remove,
+ .driver = {
+ .name = "iproc-shared-mdio",
+ .of_match_table = iproc_shared_mdio_of_match,
+ },
+};
+
+module_platform_driver(iproc_shared_mdio_driver);
+
+MODULE_DESCRIPTION("iProc Shared MDIO Master Driver");
+MODULE_AUTHOR("Pramod Kumar <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
1.9.1
Add NS2 Shared MDIO Controller DT node having eth phy as child.
This node represents the NS2 AMAC eth phy.
Signed-off-by: Pramod Kumar <[email protected]>
Reviewed-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
---
arch/arm64/boot/dts/broadcom/ns2-svk.dts | 15 +++++++++++++++
arch/arm64/boot/dts/broadcom/ns2.dtsi | 8 ++++++++
2 files changed, 23 insertions(+)
diff --git a/arch/arm64/boot/dts/broadcom/ns2-svk.dts b/arch/arm64/boot/dts/broadcom/ns2-svk.dts
index ce0ab84..2944418 100644
--- a/arch/arm64/boot/dts/broadcom/ns2-svk.dts
+++ b/arch/arm64/boot/dts/broadcom/ns2-svk.dts
@@ -87,3 +87,18 @@
#size-cells = <1>;
};
};
+
+&iproc_shared_mdio {
+ eth-master@0 {
+ compatible = "brcm,iproc-mdio-master-eth";
+ reg = <0x0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "ok";
+
+ gphy0: eth-phy@10 {
+ reg = <0x10>;
+ phy-mode = "mii";
+ };
+ };
+};
diff --git a/arch/arm64/boot/dts/broadcom/ns2.dtsi b/arch/arm64/boot/dts/broadcom/ns2.dtsi
index 6f81c9d..5b98c81 100644
--- a/arch/arm64/boot/dts/broadcom/ns2.dtsi
+++ b/arch/arm64/boot/dts/broadcom/ns2.dtsi
@@ -330,6 +330,14 @@
<0x65260000 0x1000>;
};
+ iproc_shared_mdio: iproc_shared_mdio@6602023c {
+ compatible = "brcm,iproc-shared-mdio";
+ reg = <0x6602023c 0x14>;
+ reg-names = "mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
timer0: timer@66030000 {
compatible = "arm,sp804", "arm,primecell";
reg = <0x66030000 0x1000>;
--
1.9.1
Adding binding doc for ethernet master present in shared
MDIO controller.
Signed-off-by: Pramod Kumar <[email protected]>
Reviewed-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
---
.../bindings/net/brcm,iproc-mdio-shared.txt | 32 ++++++++++++++++++++++
1 file changed, 32 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/brcm,iproc-mdio-shared.txt
diff --git a/Documentation/devicetree/bindings/net/brcm,iproc-mdio-shared.txt b/Documentation/devicetree/bindings/net/brcm,iproc-mdio-shared.txt
new file mode 100644
index 0000000..1ffdd4b
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/brcm,iproc-mdio-shared.txt
@@ -0,0 +1,32 @@
+Broadcom Ethernet master for shared mdio controller
+
+Required properties:
+- compatible: must be "brcm,iproc-mdio-master-eth"
+- reg: master id of Ethernet block
+- address-cells: should be 1
+- size-cells: should be 0
+
+Sub-nodes:
+ Each port's PHY should be represented as a sub-node.
+
+Sub-nodes required properties:
+- reg: the PHY number
+- phy-mode: media type connecting the PHY and MAC.
+
+
+Example:
+ eth-master@0 {
+ compatible = "brcm,iproc-mdio-master-eth";
+ reg = <0x0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ gphy0: eth-phy@10 {
+ reg = <0x10>;
+ phy-mode = "mii";
+ };
+ status = "ok"
+ };
+
+For more info on ethernet phy binding, please,refer to:
+Documentation/devicetree/bindings/net/phy.txt
+Documentation/devicetree/bindings/net/ethernet.txt
--
1.9.1
Add Ethernet master driver which registers the ethernet phy
to standard legacy mdio framework.
Signed-off-by: Pramod Kumar <[email protected]>
Reviewed-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
---
drivers/net/phy/Kconfig | 11 +++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/mdio-bcm-iproc-shared.c | 116 ++++++++++++++++++++++++++++++++
3 files changed, 128 insertions(+)
create mode 100644 drivers/net/phy/mdio-bcm-iproc-shared.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 6dad9a9..35a7d59 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -271,6 +271,17 @@ config MDIO_BCM_IPROC
This module provides a driver for the MDIO busses found in the
Broadcom iProc SoC's.
+config MDIO_BCM_IPROC_SHARED
+ tristate "Ethernet Master for Broadcom iProc Shared MDIO Controller"
+ depends on OF && OF_MDIO && (ARCH_BCM_IPROC || COMPILE_TEST)
+ default ARCH_BCM_IPROC
+ help
+ This module provides a driver for the ethernet master found in
+ shared MDIO controller in the Broadcom iProc SoC's.
+
+ Enable this to support the Shared MDIO controller.
+ if unsure, say N.
+
endif # PHYLIB
config MICREL_KS8995MA
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index fcdbb92..e1c0bc5 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -44,3 +44,4 @@ obj-$(CONFIG_MDIO_MOXART) += mdio-moxart.o
obj-$(CONFIG_MDIO_BCM_UNIMAC) += mdio-bcm-unimac.o
obj-$(CONFIG_MICROCHIP_PHY) += microchip.o
obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o
+obj-$(CONFIG_MDIO_BCM_IPROC_SHARED) += mdio-bcm-iproc-shared.o
diff --git a/drivers/net/phy/mdio-bcm-iproc-shared.c b/drivers/net/phy/mdio-bcm-iproc-shared.c
new file mode 100644
index 0000000..6844107
--- /dev/null
+++ b/drivers/net/phy/mdio-bcm-iproc-shared.c
@@ -0,0 +1,116 @@
+/*
+ * Copyright (C) 2016 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/shared_mdio.h>
+#include <linux/io.h>
+#include <linux/phy.h>
+#include <linux/of_mdio.h>
+
+struct iproc_mdio_priv {
+ struct shared_mdio_master *master;
+ struct mii_bus *mii_bus;
+};
+
+static int iproc_mdio_mii_read(struct mii_bus *bus, int phyadr, int reg)
+{
+ struct iproc_mdio_priv *priv = (struct iproc_mdio_priv *)bus->priv;
+
+ return shared_mdio_read(priv->master, phyadr, reg);
+}
+
+static int iproc_mdio_mii_write(struct mii_bus *bus, int phyadr, int reg,
+ u16 val)
+{
+ struct iproc_mdio_priv *priv = bus->priv;
+
+ return shared_mdio_write(priv->master, phyadr, reg, val);
+}
+
+static int iproc_mdio_probe(struct shared_mdio_master *master)
+{
+ struct device *dev = &master->dev;
+ struct iproc_mdio_priv *priv;
+ struct mii_bus *bus;
+ struct device_node *dn = dev->of_node;
+ int rc = 0;
+
+ if (of_get_child_count(dn) == 0)
+ return -ENODEV;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+ priv->master = master;
+
+ priv->mii_bus = mdiobus_alloc();
+ if (!priv->mii_bus) {
+ dev_err(dev, "mdio bus alloc failed\n");
+ return -ENOMEM;
+ }
+
+ bus = priv->mii_bus;
+ bus->priv = priv;
+ bus->name = "iProc MDIO bus";
+ snprintf(bus->id, MII_BUS_ID_SIZE, "%s-%d", dev_name(&master->dev),
+ master->dev_num);
+ bus->parent = &master->dev;
+ bus->read = iproc_mdio_mii_read;
+ bus->write = iproc_mdio_mii_write;
+
+ rc = of_mdiobus_register(bus, master->dev.of_node);
+ if (rc) {
+ dev_err(dev, "mdio bus registration failed\n");
+ goto mdiobus_err;
+ }
+
+ shared_mdio_set_drvdata(master, priv);
+ dev_info(dev, "iProc MDIO bus registered\n");
+ return 0;
+
+mdiobus_err:
+ mdiobus_free(bus);
+ return rc;
+}
+
+int iproc_mdio_remove(struct shared_mdio_master *master)
+{
+ struct iproc_mdio_priv *priv = shared_mdio_get_drvdata(master);
+
+ mdiobus_unregister(priv->mii_bus);
+ mdiobus_free(priv->mii_bus);
+
+ return 0;
+}
+
+static const struct of_device_id iproc_mdio_of_match[] = {
+ { .compatible = "brcm,iproc-mdio-master-eth" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, iproc_mdio_of_match);
+
+static struct shared_mdio_driver iproc_mdio_driver = {
+ .driver = {
+ .name = "iproc-mdio-master-eth",
+ .of_match_table = iproc_mdio_of_match,
+ },
+ .probe = iproc_mdio_probe,
+ .remove = iproc_mdio_remove,
+};
+module_shared_mdio_driver(iproc_mdio_driver);
+
+MODULE_DESCRIPTION("Broadcom iProc shared mdio master driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Pramod Kumar <[email protected]>");
--
1.9.1
Add a common shared MDIO bus framework for sharing single (or few) MDIO
bus across IO subsystems such as SATA, PCIe, USB, and Ethernet.
The IO specific PHY drivers will register to common shared MDIO
bus as shared MDIO drivers and access the MDIO bus only using
shared MDIO APIs.
Signed-off-by: Pramod Kumar <[email protected]>
Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
Reviewed-by: Vikram Prakash <[email protected]>
---
drivers/bus/Kconfig | 11 +++
drivers/bus/Makefile | 1 +
drivers/bus/shared_mdio.c | 179 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/shared_mdio.h | 123 ++++++++++++++++++++++++++++++
4 files changed, 314 insertions(+)
create mode 100644 drivers/bus/shared_mdio.c
create mode 100644 include/linux/shared_mdio.h
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index d4a3a31..1b51b1a 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -107,6 +107,17 @@ config OMAP_OCP2SCP
OCP2SCP and in OMAP5, both USB PHY and SATA PHY is connected via
OCP2SCP.
+config SHARED_MDIO
+ bool "Shared MDIO bus"
+ depends on OF
+
+ help
+ Shared MDIO bus implementation for SoCs having common MDIO bus
+ for different IO subsystems. All masters should be registered to
+ this bus via a platform driver using shared framework API.
+ Respective drivers would be called matching a compatible string
+ provided in master node.
+
config SIMPLE_PM_BUS
bool "Simple Power-Managed Bus Driver"
depends on OF && PM
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index ccff007..2b6d6e9 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_SUNXI_RSB) += sunxi-rsb.o
obj-$(CONFIG_SIMPLE_PM_BUS) += simple-pm-bus.o
obj-$(CONFIG_UNIPHIER_SYSTEM_BUS) += uniphier-system-bus.o
obj-$(CONFIG_VEXPRESS_CONFIG) += vexpress-config.o
+obj-$(CONFIG_SHARED_MDIO) += shared_mdio.o
diff --git a/drivers/bus/shared_mdio.c b/drivers/bus/shared_mdio.c
new file mode 100644
index 0000000..909af73
--- /dev/null
+++ b/drivers/bus/shared_mdio.c
@@ -0,0 +1,179 @@
+/*
+ * Copyright (C) 2016 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Shared MDIO Bus
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/errno.h>
+#include <linux/idr.h>
+#include <linux/of_device.h>
+#include <linux/stddef.h>
+#include <linux/module.h>
+#include <linux/shared_mdio.h>
+#include <linux/slab.h>
+
+static DEFINE_IDA(shared_mdio_ida);
+
+static void shared_mdio_release_master(struct device *dev)
+{
+ struct shared_mdio_master *master = to_shared_mdio_master(dev);
+
+ ida_simple_remove(&shared_mdio_ida, master->dev_num);
+ kfree(master);
+}
+
+struct shared_mdio_master *shared_mdio_alloc_master(struct device *parent,
+ struct device_node *node)
+{
+ int ret = 0;
+ struct shared_mdio_master *master;
+
+ master = kzalloc(sizeof(*master), GFP_KERNEL);
+ if (!master) {
+ ret = -ENOMEM;
+ goto fail1;
+ }
+
+ master->dev.parent = parent;
+ master->dev.bus = &shared_mdio_bus;
+ master->dev.release = shared_mdio_release_master;
+
+ device_initialize(&master->dev);
+
+ ret = ida_simple_get(&shared_mdio_ida, 0, 0, GFP_KERNEL);
+ if (ret < 0)
+ goto fail2;
+ master->dev_num = ret;
+
+ dev_set_name(&master->dev, "shared-mdio-master%d", master->dev_num);
+
+ of_node_get(node);
+ master->dev.of_node = node;
+
+ return master;
+
+fail2:
+ kfree(master);
+fail1:
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(shared_mdio_alloc_master);
+
+int shared_mdio_add_master(struct shared_mdio_master *master)
+{
+ if (!master)
+ return -EINVAL;
+
+ return device_add(&master->dev);
+}
+EXPORT_SYMBOL(shared_mdio_add_master);
+
+static int shared_mdio_driver_probe(struct device *dev)
+{
+ int rc;
+ struct shared_mdio_master *master = to_shared_mdio_master(dev);
+ struct shared_mdio_driver *drv = to_shared_mdio_driver(dev->driver);
+
+ if (!drv->probe)
+ return -ENODEV;
+
+ rc = drv->probe(master);
+ if (rc)
+ return rc;
+
+ return 0;
+}
+
+static int shared_mdio_driver_remove(struct device *dev)
+{
+ struct shared_mdio_driver *drv = to_shared_mdio_driver(dev->driver);
+
+ if (drv->remove)
+ return drv->remove(to_shared_mdio_master(dev));
+
+ return 0;
+}
+
+static void shared_mdio_driver_shutdown(struct device *dev)
+{
+ struct shared_mdio_driver *drv = to_shared_mdio_driver(dev->driver);
+
+ if (drv->shutdown)
+ drv->shutdown(to_shared_mdio_master(dev));
+}
+
+int __shared_mdio_register_driver(struct shared_mdio_driver *drv,
+ struct module *owner)
+{
+ int rc;
+
+ drv->driver.bus = &shared_mdio_bus;
+ drv->driver.owner = owner;
+ drv->driver.probe = shared_mdio_driver_probe;
+ drv->driver.remove = shared_mdio_driver_remove;
+ drv->driver.shutdown = shared_mdio_driver_shutdown;
+
+ rc = driver_register(&drv->driver);
+ if (rc) {
+ pr_err("driver_register() failed for %s, error: %d\n",
+ drv->driver.name, rc);
+ return rc;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(__shared_mdio_register_driver);
+
+static int shared_mdio_bus_match(struct device *dev, struct device_driver *drv)
+{
+ /* Attempt an OF style match */
+ if (of_driver_match_device(dev, drv))
+ return 1;
+
+ return 0;
+}
+
+struct bus_type shared_mdio_bus = {
+ .name = "shared_mdio",
+ .match = shared_mdio_bus_match,
+};
+EXPORT_SYMBOL(shared_mdio_bus);
+
+static int __init shared_mdio_init(void)
+{
+ int rc;
+
+ rc = bus_register(&shared_mdio_bus);
+ if (rc) {
+ pr_err("Failed to register shared_mdio bus, error: %d\n", rc);
+ return rc;
+ }
+
+ return 0;
+}
+
+static void __exit shared_mdio_exit(void)
+{
+ bus_unregister(&shared_mdio_bus);
+
+ ida_destroy(&shared_mdio_ida);
+}
+
+subsys_initcall(shared_mdio_init);
+module_exit(shared_mdio_exit);
+
+MODULE_DESCRIPTION("Shared MDIO Bus");
+MODULE_AUTHOR("Anup Patel <[email protected]>");
+MODULE_AUTHOR("Pramod Kumar <[email protected]>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/shared_mdio.h b/include/linux/shared_mdio.h
new file mode 100644
index 0000000..db6e442
--- /dev/null
+++ b/include/linux/shared_mdio.h
@@ -0,0 +1,123 @@
+/*
+ * Copyright (C) 2016 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __SHARED_MDIO_H__
+#define __SHARED_MDIO_H__
+
+#include <linux/device.h>
+#include <linux/types.h>
+
+extern struct bus_type shared_mdio_bus;
+
+/*
+ * This structure represets the mdio master that control the MDIO bus
+ * to access the phy attached on it.
+ * @dev Underlying device for mdio master
+ * @dev_num Unique device number of mdio master
+ * @mdio_write Write callback of mdio master
+ * @mdio_read Read callback for mdio master
+ */
+struct shared_mdio_master {
+ struct device dev;
+ int dev_num;
+ void *priv;
+
+ int (*mdio_write)(struct shared_mdio_master *master,
+ u16 phyid, u16 reg, u16 data);
+ int (*mdio_read)(struct shared_mdio_master *master, u16 phyid, u16 reg);
+};
+#define to_shared_mdio_master(d) \
+ container_of(d, struct shared_mdio_master, dev)
+
+struct shared_mdio_driver {
+ int (*probe)(struct shared_mdio_master *master);
+ int (*remove)(struct shared_mdio_master *master);
+ void (*shutdown)(struct shared_mdio_master *master);
+
+ struct device_driver driver;
+};
+#define to_shared_mdio_driver(d) \
+ container_of(d, struct shared_mdio_driver, driver)
+
+struct shared_mdio_master *shared_mdio_alloc_master(struct device *parent,
+ struct device_node *node);
+
+int shared_mdio_add_master(struct shared_mdio_master *master);
+
+static inline void shared_mdio_remove_master(struct shared_mdio_master *master)
+{
+ device_unregister(&master->dev);
+}
+
+int __shared_mdio_register_driver(struct shared_mdio_driver *drv,
+ struct module *owner);
+
+/* use a define to avoid include chaining to get THIS_MODULE & friends */
+#define shared_mdio_register_driver(drv) \
+ __shared_mdio_register_driver(drv, THIS_MODULE)
+
+static inline void shared_mdio_unregister_driver(
+ struct shared_mdio_driver *drv)
+{
+ driver_unregister(&drv->driver);
+}
+
+/**
+ * module_shared_mdio_driver() - Helper macro for registering a shared_mdio
+ * driver
+ * @__shared_mdio_driver: shared_mdio_driver struct
+ *
+ * Helper macro for shared mdio drivers which do not do anything special in
+ * module init/exit. This eliminates a lot of boilerplate. Each module
+ * may only use this macro once, and calling it replaces module_init()
+ * and module_exit().
+ */
+#define module_shared_mdio_driver(__shared_mdio_driver) \
+ module_driver(__shared_mdio_driver, shared_mdio_register_driver, \
+ shared_mdio_unregister_driver)
+
+
+static inline int shared_mdio_write(struct shared_mdio_master *master,
+ u16 phy, u16 reg, u16 data)
+{
+ if (master->mdio_write)
+ return master->mdio_write(master, phy, reg, data);
+
+ return -ENOTSUPP;
+}
+
+static inline int shared_mdio_read(struct shared_mdio_master *master,
+ u16 phy_id, u16 reg)
+{
+ if (master->mdio_read)
+ return master->mdio_read(master, phy_id, reg);
+
+ return -ENOTSUPP;
+}
+
+/*
+ * Use the following functions to manipulate shared_mdio's per-master
+ * driver-specific data.
+ */
+static inline void *shared_mdio_get_drvdata(struct shared_mdio_master *master)
+{
+ return dev_get_drvdata(&master->dev);
+}
+
+static inline void shared_mdio_set_drvdata(struct shared_mdio_master *master,
+ void *data)
+{
+ dev_set_drvdata(&master->dev, data);
+}
+
+#endif
--
1.9.1
On Thu, Apr 21, 2016 at 02:48:39PM +0530, Pramod Kumar wrote:
> Add DT binding doc for iProc Shared MDIO Controller which
> populate all masters to Shared MDIO framework.
>
> Signed-off-by: Pramod Kumar <[email protected]>
> Reviewed-by: Ray Jui <[email protected]>
> Reviewed-by: Scott Branden <[email protected]>
> ---
> .../bindings/bus/brcm,iproc-shared-mdio.txt | 76 ++++++++++++++++++++++
> 1 file changed, 76 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/bus/brcm,iproc-shared-mdio.txt
>
> diff --git a/Documentation/devicetree/bindings/bus/brcm,iproc-shared-mdio.txt b/Documentation/devicetree/bindings/bus/brcm,iproc-shared-mdio.txt
> new file mode 100644
> index 0000000..f455f3d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/brcm,iproc-shared-mdio.txt
> @@ -0,0 +1,76 @@
> +Broadcom iProc Shared MDIO Controller
> +
> +Required properties:
> +- compatible:
> + "brcm,iproc-shared-mdio" for shared MDIO controller.
> +- reg: specifies the base physical address and size of the registers
> +- reg-names: should be "mdio"
> +- #address-cells: must be 1.
> +- #size-cells: must be 0.
> +
> +optional:
> +child nodes: Masters are represented as a child node of shared MDIO controller
> +and all the PHYs handled by this master are represented as its child node.
Seems kind of useless if child nodes are optional.
> +
> +Master nodes properties are defined as-
> +
> +Required properties:
> +- compatible: Used to match driver of this PHY.
> +- reg: MDIO master ID.
> +- #address-cells: must be 1.
> +- #size-cells: must be 0.
> +
> +optional:
> +-brcm,phy-internal: if presents, PHYs are internal. Absence shows phy is
> +external.
> +-brcm,is-c45: if presents, Controller uses Clause-45 to issue MDIO transaction.
> +else Controller uses Clause-22 for transactions.
Isn't this a property of the phy? IIRC, there is a standard property or
compatible string for this.
> +
> +PHY nodes are represented as the child node of Master. Child nodes properties
> +are defined as-
> +
> +Required properties:
> +-reg: phy id of attached PHY.
> +
> +optional:
> +There could be additional properties required to configure the specific phy like
> +phy-mode in case of gpphy node below. These should be defined here and used by
> +respective drivers.
> +
> +Example:
> +iproc_mdio: iproc_mdio@663f0000 {
> + compatible = "brcm,iproc-shared-mdio";
> + reg = <0x6602023c 0x14>;
> + reg-names = "mdio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sata-master@6 {
mdio@6
> + compatible = "brcm,iproc-ns2-sata-phy";
> + reg = <0x6>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + brcm,phy-internal;
> +
> + sata_phy0: sata-phy@1 {
phy@1
> + reg = <0x1>;
> + #phy-cells = <0>;
> + };
> +
> + sata_phy1: sata-phy@2 {
phy@2
> + reg = <0x2>;
> + #phy-cells = <0>;
> + };
> + };
> +
> + eth-master@0 {
mdio@0
> + compatible = "brcm,iproc-mdio-master-eth";
> + reg = <0x0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + gphy0: eth-phy@10 {
phy@10
> + reg = <0x10>;
> + phy-mode = "mii";
> + };
> + };
> +};
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Apr 21, 2016 at 02:48:42PM +0530, Pramod Kumar wrote:
> Adding binding doc for ethernet master present in shared
> MDIO controller.
>
> Signed-off-by: Pramod Kumar <[email protected]>
> Reviewed-by: Ray Jui <[email protected]>
> Reviewed-by: Scott Branden <[email protected]>
> ---
> .../bindings/net/brcm,iproc-mdio-shared.txt | 32 ++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/brcm,iproc-mdio-shared.txt
>
> diff --git a/Documentation/devicetree/bindings/net/brcm,iproc-mdio-shared.txt b/Documentation/devicetree/bindings/net/brcm,iproc-mdio-shared.txt
> new file mode 100644
> index 0000000..1ffdd4b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/brcm,iproc-mdio-shared.txt
> @@ -0,0 +1,32 @@
> +Broadcom Ethernet master for shared mdio controller
> +
> +Required properties:
> +- compatible: must be "brcm,iproc-mdio-master-eth"
> +- reg: master id of Ethernet block
> +- address-cells: should be 1
> +- size-cells: should be 0
> +
> +Sub-nodes:
> + Each port's PHY should be represented as a sub-node.
> +
> +Sub-nodes required properties:
> +- reg: the PHY number
> +- phy-mode: media type connecting the PHY and MAC.
> +
> +
> +Example:
> + eth-master@0 {
Is this a child of something?
Why is this not just an mdio bus underneath the ethernet controller? How
is this accessed?
> + compatible = "brcm,iproc-mdio-master-eth";
> + reg = <0x0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + gphy0: eth-phy@10 {
> + reg = <0x10>;
> + phy-mode = "mii";
> + };
> + status = "ok"
> + };
> +
> +For more info on ethernet phy binding, please,refer to:
> +Documentation/devicetree/bindings/net/phy.txt
> +Documentation/devicetree/bindings/net/ethernet.txt
> --
> 1.9.1
>
From: Pramod Kumar <[email protected]>
Date: Thu, 21 Apr 2016 14:48:38 +0530
> +struct shared_mdio_master *shared_mdio_alloc_master(struct device *parent,
> + struct device_node *node)
> +{
> + int ret = 0;
> + struct shared_mdio_master *master;
Always order local variable declarations in reverse christmas tree
(longest to shortest line) order.
> +static int shared_mdio_driver_probe(struct device *dev)
> +{
> + int rc;
> + struct shared_mdio_master *master = to_shared_mdio_master(dev);
> + struct shared_mdio_driver *drv = to_shared_mdio_driver(dev->driver);
Likewise.
Please audit your entire submission for this issue.
Hi David,
Thanks for providing input over the patch. Will address the comment as
described below.
> -----Original Message-----
> From: David Miller [mailto:[email protected]]
> Sent: 24 April 2016 23:48
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; bcm-kernel-feedback-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH 1/6] bus: Add shared MDIO bus framework
>
> From: Pramod Kumar <[email protected]>
> Date: Thu, 21 Apr 2016 14:48:38 +0530
>
> > +struct shared_mdio_master *shared_mdio_alloc_master(struct device
> *parent,
> > + struct device_node
*node)
> > +{
> > + int ret = 0;
> > + struct shared_mdio_master *master;
>
> Always order local variable declarations in reverse christmas tree
(longest to
> shortest line) order.
>
Sure. Next patch will address this.
> > +static int shared_mdio_driver_probe(struct device *dev) {
> > + int rc;
> > + struct shared_mdio_master *master = to_shared_mdio_master(dev);
> > + struct shared_mdio_driver *drv =
to_shared_mdio_driver(dev->driver);
>
> Likewise.
Sure.
> Please audit your entire submission for this issue.
Sure. I'll audit the entire patch set for above issue.
Regards,
Pramod
Hi Rob,
Thanks for reviewing and providing your valuable comments.
> -----Original Message-----
> From: Rob Herring [mailto:[email protected]]
> Sent: 23 April 2016 01:41
> To: Pramod Kumar
> Cc: Catalin Marinas; Will Deacon; Masahiro Yamada; Chen-Yu Tsai; Mark
> Rutland; [email protected]; Pawel Moll; Arnd Bergmann; Suzuki K
> Poulose; [email protected]; Punit Agrawal;
[email protected];
> BCM Kernel Feedback; [email protected]
> Subject: Re: [PATCH 2/6] Documentation: DT binding doc for iProc Shared
MDIO
> Controller.
>
> On Thu, Apr 21, 2016 at 02:48:39PM +0530, Pramod Kumar wrote:
> > Add DT binding doc for iProc Shared MDIO Controller which populate all
> > masters to Shared MDIO framework.
> >
> > Signed-off-by: Pramod Kumar <[email protected]>
> > Reviewed-by: Ray Jui <[email protected]>
> > Reviewed-by: Scott Branden <[email protected]>
> > ---
> > .../bindings/bus/brcm,iproc-shared-mdio.txt | 76
> ++++++++++++++++++++++
> > 1 file changed, 76 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/bus/brcm,iproc-shared-mdio.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/bus/brcm,iproc-shared-mdio.txt
> > b/Documentation/devicetree/bindings/bus/brcm,iproc-shared-mdio.txt
> > new file mode 100644
> > index 0000000..f455f3d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/bus/brcm,iproc-shared-mdio.txt
> > @@ -0,0 +1,76 @@
> > +Broadcom iProc Shared MDIO Controller
> > +
> > +Required properties:
> > +- compatible:
> > + "brcm,iproc-shared-mdio" for shared MDIO controller.
> > +- reg: specifies the base physical address and size of the registers
> > +- reg-names: should be "mdio"
> > +- #address-cells: must be 1.
> > +- #size-cells: must be 0.
> > +
> > +optional:
> > +child nodes: Masters are represented as a child node of shared MDIO
> > +controller and all the PHYs handled by this master are represented as
its child
> node.
>
> Seems kind of useless if child nodes are optional.
>
I agree. I'll put it under required properties.
> > +
> > +Master nodes properties are defined as-
> > +
> > +Required properties:
> > +- compatible: Used to match driver of this PHY.
> > +- reg: MDIO master ID.
> > +- #address-cells: must be 1.
> > +- #size-cells: must be 0.
> > +
> > +optional:
> > +-brcm,phy-internal: if presents, PHYs are internal. Absence shows phy
> > +is external.
> > +-brcm,is-c45: if presents, Controller uses Clause-45 to issue MDIO
> transaction.
> > +else Controller uses Clause-22 for transactions.
>
> Isn't this a property of the phy? IIRC, there is a standard property or
compatible
> string for this.
>
Shared MDIO controller's master holds all above property to ensure the
proper MDIO transaction over its bus. Hence tried to pinned here.
These properties are standard one for the Ethernet PHY but Broadcom SoCs
MDIO is shared even with other I/O subsystem. Other subsystem does not
defines this property hence has been defined here.
> > +
> > +PHY nodes are represented as the child node of Master. Child nodes
> > +properties are defined as-
> > +
> > +Required properties:
> > +-reg: phy id of attached PHY.
> > +
> > +optional:
> > +There could be additional properties required to configure the
> > +specific phy like phy-mode in case of gpphy node below. These should
> > +be defined here and used by respective drivers.
> > +
> > +Example:
> > +iproc_mdio: iproc_mdio@663f0000 {
> > + compatible = "brcm,iproc-shared-mdio";
> > + reg = <0x6602023c 0x14>;
> > + reg-names = "mdio";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + sata-master@6 {
>
> mdio@6
Do you advise us to rename the node name as mdio@6? If yes, I don't have
any personal preferences here and would do it. I'd thought to give proper
name so that consumer could get what this node is representing.
Please suggest us.
>
> > + compatible = "brcm,iproc-ns2-sata-phy";
> > + reg = <0x6>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + brcm,phy-internal;
> > +
> > + sata_phy0: sata-phy@1 {
>
> phy@1
>
Same as above.
> > + reg = <0x1>;
> > + #phy-cells = <0>;
> > + };
> > +
> > + sata_phy1: sata-phy@2 {
>
> phy@2
>
Same as above.
> > + reg = <0x2>;
> > + #phy-cells = <0>;
> > + };
> > + };
> > +
> > + eth-master@0 {
>
> mdio@0
Same as above.
>
> > + compatible = "brcm,iproc-mdio-master-eth";
> > + reg = <0x0>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + gphy0: eth-phy@10 {
>
> phy@10
>
Same as above.
> > + reg = <0x10>;
> > + phy-mode = "mii";
> > + };
> > + };
> > +};
> > --
> > 1.9.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Regards,
Pramod
Hi Rob,
Thanks for review and providing your valuable comments.
> -----Original Message-----
> From: Rob Herring [mailto:[email protected]]
> Sent: 23 April 2016 01:44
> To: Pramod Kumar
> Cc: Catalin Marinas; Will Deacon; Masahiro Yamada; Chen-Yu Tsai; BCM
Kernel
> Feedback; Pawel Moll; Mark Rutland; Arnd Bergmann; Suzuki K Poulose;
Punit
> Agrawal; [email protected];
[email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH 5/6] Documentation: Binding doc for ethernet master
in
> NS2
>
> On Thu, Apr 21, 2016 at 02:48:42PM +0530, Pramod Kumar wrote:
> > Adding binding doc for ethernet master present in shared MDIO
> > controller.
> >
> > Signed-off-by: Pramod Kumar <[email protected]>
> > Reviewed-by: Ray Jui <[email protected]>
> > Reviewed-by: Scott Branden <[email protected]>
> > ---
> > .../bindings/net/brcm,iproc-mdio-shared.txt | 32
> ++++++++++++++++++++++
> > 1 file changed, 32 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/net/brcm,iproc-mdio-shared.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/net/brcm,iproc-mdio-shared.txt
> > b/Documentation/devicetree/bindings/net/brcm,iproc-mdio-shared.txt
> > new file mode 100644
> > index 0000000..1ffdd4b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/brcm,iproc-mdio-shared.txt
> > @@ -0,0 +1,32 @@
> > +Broadcom Ethernet master for shared mdio controller
> > +
> > +Required properties:
> > +- compatible: must be "brcm,iproc-mdio-master-eth"
> > +- reg: master id of Ethernet block
> > +- address-cells: should be 1
> > +- size-cells: should be 0
> > +
> > +Sub-nodes:
> > + Each port's PHY should be represented as a sub-node.
> > +
> > +Sub-nodes required properties:
> > +- reg: the PHY number
> > +- phy-mode: media type connecting the PHY and MAC.
> > +
> > +
> > +Example:
> > + eth-master@0 {
>
> Is this a child of something?
>
This is an Shared MDIO master node as described in cover letter and is the
child node of iProc_shared_mdio plarform driver.
> Why is this not just an mdio bus underneath the ethernet controller? How
is this
> accessed?
This is the part for Shared MDIO controller which is shared among other
subsystem as well hence defined here. This works as glue layer between
This controller and legacy MDIO framework and register the MDIO bus to
legacy framework.
When any read/write request is issue from Legacy MDIO controller framework
it gets propagated to Shared controller platform driver via this driver
and finally platform driver issue MDIO transaction over bus.
>
> > + compatible = "brcm,iproc-mdio-master-eth";
> > + reg = <0x0>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + gphy0: eth-phy@10 {
> > + reg = <0x10>;
> > + phy-mode = "mii";
> > + };
> > + status = "ok"
> > + };
> > +
> > +For more info on ethernet phy binding, please,refer to:
> > +Documentation/devicetree/bindings/net/phy.txt
> > +Documentation/devicetree/bindings/net/ethernet.txt
> > --
> > 1.9.1
> >
Regards,
Pramod
On Thu, Apr 21, 2016 at 02:48:37PM +0530, Pramod Kumar wrote:
> Broadcom iProc based SoCs uses MDIO bus for programming PHYs belonging to
> different I/O subsystem like USB, SATA, PCIe, ETHERNET etc. Every subsystem
> is referred as "Master" When a master is selected, all PHYs belonging to
> this subsystem get active on the MDIO bus and responds to MDIO transaction.
> In this way one MDIO controller is shared among all masters hence named as
> "Shared MDIO controller".
>
> We have two important entities in "Shared MDIO Bus" framework:
> 1) shared_mdio_master and
> 2) shared_mdio_driver.
>
> The shared MDIO controller driver is registered as platform driver and it
> creates shared_mdio_master instances and adds them to "Shared MDIO Bus"
> framework.
Hi Pramod
What i'm missing is an explanation why the existing MDIO bus framework
cannot be used. I recently made it more generic so that you can hang
any type of device off it, not just PHYs.
Thanks
Andrew
> +Example:
> +iproc_mdio: iproc_mdio@663f0000 {
> + compatible = "brcm,iproc-shared-mdio";
> + reg = <0x6602023c 0x14>;
> + reg-names = "mdio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sata-master@6 {
> + compatible = "brcm,iproc-ns2-sata-phy";
> + reg = <0x6>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + brcm,phy-internal;
> +
> + sata_phy0: sata-phy@1 {
> + reg = <0x1>;
> + #phy-cells = <0>;
> + };
> +
> + sata_phy1: sata-phy@2 {
> + reg = <0x2>;
> + #phy-cells = <0>;
> + };
> + };
> +
> + eth-master@0 {
> + compatible = "brcm,iproc-mdio-master-eth";
> + reg = <0x0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + gphy0: eth-phy@10 {
> + reg = <0x10>;
> + phy-mode = "mii";
> + };
> + };
> +};
So looking at this, you have an MDIO bus, an MDIO mux on top of that,
and then some MDIO devices on the muxed busses. You don't need a whole
new framework for this. You need a new mdio-mux driver, but the
existing MDIO framework should do what you need.
Andrew
Hi Pramod
I took a closer look. I don't see why the current MDIO code should not
be used, rather than adding a new framework.
What you need for your Non Ethernet PHYs is that they are somehow
probed. The current MDIO code will do that, based on the compatible
string. An mdio device gets passed a struct mdio_device * to its probe
function, giving you the bus and address on the bus for the
device. Your PHY driver can then register itself using
devm_of_phy_provider_register(). The user of the PHY then needs to use
devm_phy_get() to get a handle on the phy, and can then use
phy_power_on()/phy_power_off().
There is a very simple example here for an MDIO device driver:
http://thread.gmane.org/gmane.linux.network/393532
The muxing of the MDIO busses looks a little tricky. At the moment you have:
writel(cmd, base + MDIO_PARAM_OFFSET);
which mixes together the muxing parameters and the write value. Can
this register be accessed as two 16 bit registers? If it can be, you
can cleanly separate out the muxing.
Take a look at mdio-mux-gpio.c and mdio-mux-mmioreg.c for examples of
MDIO muxes.
Andrew
Hi Andrew,
Thanks for reviewing. I really appreciate your effort it.
I am already aware of MDIO mux framework but did not see it fit for our
use case due to below limitations:
1. Current MDIO mux framework is Ethernet centric and it is only meant to
mux multiple MII buses using same MDIO controller. This means it is only
meant for MII-compliant PHY devices (i.e. PHY devices having registers
as-per MII specs). This is not the case with Broadcom SATA PHYs, PCIe
PHYs, and USB PHYs even if we are sharing same MDIO controller for
accessing these PHYs.
2. The MDIO mux framework registers each child bus as MII bus. The Linux
Ethernet MDIO framework will scan for all attached PHY devices on given
MII bus and try to read MII PHY_ID register which is not present in all
Broadcom non-ethernet PHYs.
3. Let's say we ignore point1 and point2 above and go ahead and use MDIO
mux framework then we will still have to emulated MII PHY_ID read for
non-ethernet PHYs. Let's say we also emulate MII PHY_ID read in
non-ethernet PHYs then next thing is non-ethernet PHYs don't follow MII
state machine so all other MII PHY register read/write will not work.
4. Apart from these, by using MDIO mux framework we are making our
non-ethernet PHYs dependent on Linux network drivers which is not
acceptable. What if some product line does not need network subsystem at
all?
As you can see from above points, trying to re-use Linux Ethernet MDIO mux
framework for non-Ethernet PHYs is not the right way. The sole reason
being Linux MDIO mux framework is not generic enough to be shared across
IO subsystems. Due to this reason we had to come-up with "Shared MDIO
framework" which is very simple, generic and independent of I/O
subsystems.
I'll add PCIe PHYs driver based on Shared MDIO framework in next patch
revision to get a feel of its need.
Regards,
Pramod
> -----Original Message-----
> From: Andrew Lunn [mailto:[email protected]]
> Sent: 26 April 2016 02:27
> To: Pramod Kumar
> Cc: Rob Herring; Catalin Marinas; Will Deacon; Masahiro Yamada; Chen-Yu
Tsai;
> Mark Rutland; [email protected]; Pawel Moll; Arnd Bergmann;
Suzuki
> K Poulose; [email protected]; Punit Agrawal; linux-
> [email protected]; BCM Kernel Feedback; linux-arm-
> [email protected]; Anup Patel
> Subject: Re: [PATCH 1/6] bus: Add shared MDIO bus framework
>
> Hi Pramod
>
> I took a closer look. I don't see why the current MDIO code should not
be used,
> rather than adding a new framework.
>
> What you need for your Non Ethernet PHYs is that they are somehow
probed.
> The current MDIO code will do that, based on the compatible string. An
mdio
> device gets passed a struct mdio_device * to its probe function, giving
you the
> bus and address on the bus for the device. Your PHY driver can then
register
> itself using devm_of_phy_provider_register(). The user of the PHY then
needs to
> use
> devm_phy_get() to get a handle on the phy, and can then use
> phy_power_on()/phy_power_off().
>
> There is a very simple example here for an MDIO device driver:
>
> http://thread.gmane.org/gmane.linux.network/393532
>
> The muxing of the MDIO busses looks a little tricky. At the moment you
have:
>
> writel(cmd, base + MDIO_PARAM_OFFSET);
>
> which mixes together the muxing parameters and the write value. Can this
> register be accessed as two 16 bit registers? If it can be, you can
cleanly
> separate out the muxing.
>
> Take a look at mdio-mux-gpio.c and mdio-mux-mmioreg.c for examples of
> MDIO muxes.
>
> Andrew
On Tue, Apr 26, 2016 at 02:03:27PM +0530, Pramod Kumar wrote:
> Hi Andrew,
>
> Thanks for reviewing. I really appreciate your effort it.
>
> I am already aware of MDIO mux framework but did not see it fit for our
> use case due to below limitations:
>
> 1. Current MDIO mux framework is Ethernet centric and it is only meant to
> mux multiple MII buses using same MDIO controller. This means it is only
> meant for MII-compliant PHY devices (i.e. PHY devices having registers
> as-per MII specs).
Nope, this is wrong. You can have a mixture of PHYs and other MDIO
devices on the MII bus. The framework does not care. A PHY is a
special case of an MDIO device.
> 2. The MDIO mux framework registers each child bus as MII bus. The Linux
> Ethernet MDIO framework will scan for all attached PHY devices on given
> MII bus and try to read MII PHY_ID register which is not present in all
> Broadcom non-ethernet PHYs.
It only performs a scan if you don't list the devices in the device
tree. If you do list devices, and include the address on the bus, it
never scans.
A good example of this is Ethernet switches. They occupy multiple
addresses on the MDIO bus, and also do not implement the PHY_ID
register at each address. Yet the MDIO layer is happy with this.
> 3. Let's say we ignore point1 and point2 above and go ahead and use MDIO
> mux framework then we will still have to emulated MII PHY_ID read for
> non-ethernet PHYs.
Nope. Not at all. You have an MDIO device on the bus, not an Ethernet
PHY. Hence the device is not liked to the PHY state machine, etc.
The key concept to get here is that there are MDIO devices, and a
subset of MDIO devices are Ethernet PHYs.....
> 4. Apart from these, by using MDIO mux framework we are making our
> non-ethernet PHYs dependent on Linux network drivers which is not
> acceptable. What if some product line does not need network subsystem at
> all?
This is your only valid point. However, does Broadcom have a product
line which does not include networking? Is not Broadcom a network SoC
vendor?
>
> As you can see from above points, trying to re-use Linux Ethernet MDIO mux
> framework for non-Ethernet PHYs is not the right way.
And as i pointed out, all your arguments are wrong, bar one. And i
doubt that one argument is sufficient to duplicate a lot of code which
already exists and does 95% of what you need.
> I'll add PCIe PHYs driver based on Shared MDIO framework in next patch
> revision to get a feel of its need.
Great. Lets then see what is needed to turn it into an MDIO device.
Andrew
From: Andrew Lunn <[email protected]>
Date: Tue, 26 Apr 2016 14:13:35 +0200
> On Tue, Apr 26, 2016 at 02:03:27PM +0530, Pramod Kumar wrote:
>> As you can see from above points, trying to re-use Linux Ethernet MDIO mux
>> framework for non-Ethernet PHYs is not the right way.
>
> And as i pointed out, all your arguments are wrong, bar one. And i
> doubt that one argument is sufficient to duplicate a lot of code which
> already exists and does 95% of what you need.
+1
On 26/04/16 05:13, Andrew Lunn wrote:
>> 4. Apart from these, by using MDIO mux framework we are making our
>> non-ethernet PHYs dependent on Linux network drivers which is not
>> acceptable. What if some product line does not need network subsystem at
>> all?
>
> This is your only valid point. However, does Broadcom have a product
> line which does not include networking? Is not Broadcom a network SoC
> vendor?
But even with that, there is no reason why we could not decouple the
PHYLIB MDIO framework from PHYLIB and make it available as a more
standalone subsystem which can be utilized when you have a mix of MDIO
devices like here.
I am not clear on how common a shared MDIO bus is on other SoCs, but the
other Broadcom SoCs I am familiar with have dedicated MDIO buses
instances per type of PHY (PCIe, BUSB, Ethernet), thus making the split
a ton easier.
--
Florian
> I am not clear on how common a shared MDIO bus is on other SoCs, but the
> other Broadcom SoCs I am familiar with have dedicated MDIO buses
> instances per type of PHY (PCIe, BUSB, Ethernet), thus making the split
> a ton easier.
I don't actually see this shared bus being an issue, once the mux it
implemented. With the mux, you see N MDIO busses, each acting like a
normal MDIO bus. I've used the GPIO variety and had no issues:
http://marc.info/?l=linux-arm-kernel&m=145910090401796&w=2
Andrew
On Tuesday 26 April 2016 10:23:02 Florian Fainelli wrote:
> On 26/04/16 05:13, Andrew Lunn wrote:
> >> 4. Apart from these, by using MDIO mux framework we are making our
> >> non-ethernet PHYs dependent on Linux network drivers which is not
> >> acceptable. What if some product line does not need network subsystem at
> >> all?
> >
> > This is your only valid point. However, does Broadcom have a product
> > line which does not include networking? Is not Broadcom a network SoC
> > vendor?
>
> But even with that, there is no reason why we could not decouple the
> PHYLIB MDIO framework from PHYLIB and make it available as a more
> standalone subsystem which can be utilized when you have a mix of MDIO
> devices like here.
[adding Kishon Vijay Abraham]
We should also consider how this fits in with drivers/phy/, which is
the generic framework for all PHY devices that are not for ethernet.
The most straightforward way that you mention would be to allow
generic PHY devices to be probed on an MIO bus or mux. This should
just work using mdio_module_driver(), as Andrew already explained.
A more complex problem would be having a PHY driver for a device
that can be either an ethernet phy or some other phy. With today's
frameworks that would require two separate drivers, one in drivers/phy
and one in drivers/net/phy/. If that turns out to be a common problem,
we might want to come up with a way to nest one on top of the other,
or merge two two device structures (struct phy_device and struct phy).
> I am not clear on how common a shared MDIO bus is on other SoCs, but the
> other Broadcom SoCs I am familiar with have dedicated MDIO buses
> instances per type of PHY (PCIe, BUSB, Ethernet), thus making the split
> a ton easier.
I think most commonly, the other PHYs are not on MDIO at all, but are
integrated inside of the SoC as an MMIO based device.
Arnd
> A more complex problem would be having a PHY driver for a device
> that can be either an ethernet phy or some other phy.
I doubt that ever happens. You can have up to 32 different devices on
an MDIO bus. Since an Ethernet PHY and a "some other sort of PHY" are
completely different things, why would a hardware engineer place them
on the same address? It is like saying your ATA controller and VGA
controller share the same slot on the PCI bus...
Andrew
On Tuesday 26 April 2016 20:23:35 Andrew Lunn wrote:
> > A more complex problem would be having a PHY driver for a device
> > that can be either an ethernet phy or some other phy.
>
> I doubt that ever happens. You can have up to 32 different devices on
> an MDIO bus. Since an Ethernet PHY and a "some other sort of PHY" are
> completely different things, why would a hardware engineer place them
> on the same address? It is like saying your ATA controller and VGA
> controller share the same slot on the PCI bus...
To clarify: what I meant is a device that is designed as a PHY for
similar hardware (e.g. SATA, USB3 and PCIe) and that has a common
register set and a single driver, but that driver can operate
in multiple modes. You typically have multiple instances of
such hardware, with each instance linked to exactly one host
device, but one driver for all of them.
See Documentation/devicetree/bindings/phy/apm-xgene-phy.txt
and drivers/phy/phy-xgene.c for one such example.
Arnd
On Tue, Apr 26, 2016 at 09:24:34PM +0200, Arnd Bergmann wrote:
> On Tuesday 26 April 2016 20:23:35 Andrew Lunn wrote:
> > > A more complex problem would be having a PHY driver for a device
> > > that can be either an ethernet phy or some other phy.
> >
> > I doubt that ever happens. You can have up to 32 different devices on
> > an MDIO bus. Since an Ethernet PHY and a "some other sort of PHY" are
> > completely different things, why would a hardware engineer place them
> > on the same address? It is like saying your ATA controller and VGA
> > controller share the same slot on the PCI bus...
>
> To clarify: what I meant is a device that is designed as a PHY for
> similar hardware (e.g. SATA, USB3 and PCIe) and that has a common
> register set and a single driver, but that driver can operate
> in multiple modes. You typically have multiple instances of
> such hardware, with each instance linked to exactly one host
> device, but one driver for all of them.
>
> See Documentation/devicetree/bindings/phy/apm-xgene-phy.txt
> and drivers/phy/phy-xgene.c for one such example.
Interesting. Also, that this lists SGMII. I assume this is a phy in
the MAC in order to talk to the Ethernet PHY.
I still don't see it being a big problem if a phy driver implements an
Ethernet PHY. It just needs to call phy_device_create() and
phy_device_register().
Andrew
On Wed, Apr 27, 2016 at 1:11 AM, Andrew Lunn <[email protected]> wrote:
> On Tue, Apr 26, 2016 at 09:24:34PM +0200, Arnd Bergmann wrote:
>> On Tuesday 26 April 2016 20:23:35 Andrew Lunn wrote:
>> > > A more complex problem would be having a PHY driver for a device
>> > > that can be either an ethernet phy or some other phy.
>> >
>> > I doubt that ever happens. You can have up to 32 different devices on
>> > an MDIO bus. Since an Ethernet PHY and a "some other sort of PHY" are
>> > completely different things, why would a hardware engineer place them
>> > on the same address? It is like saying your ATA controller and VGA
>> > controller share the same slot on the PCI bus...
>>
>> To clarify: what I meant is a device that is designed as a PHY for
>> similar hardware (e.g. SATA, USB3 and PCIe) and that has a common
>> register set and a single driver, but that driver can operate
>> in multiple modes. You typically have multiple instances of
>> such hardware, with each instance linked to exactly one host
>> device, but one driver for all of them.
>>
>> See Documentation/devicetree/bindings/phy/apm-xgene-phy.txt
>> and drivers/phy/phy-xgene.c for one such example.
>
> Interesting. Also, that this lists SGMII. I assume this is a phy in
> the MAC in order to talk to the Ethernet PHY.
>
> I still don't see it being a big problem if a phy driver implements an
> Ethernet PHY. It just needs to call phy_device_create() and
> phy_device_register().
>
> Andrew
It is really interesting to see the evolution of MDIO bus:
1. Traditionally, MDIO controller used to be part of each ethernet controller
itself so each ethernet controller used to have it's own 2 wire MDIO bus
2. Next, we saw SoC with multiple ethernet controllers sharing same MDIO
bus. In other words, we saw multiple MDIO bus being muxed over single
MDIO bus with additional bus select lines (I think this is when
drivers/net/phy/mdio-mux.c APIs were implemented but at this point all
PHYs on muxed MDIO bus were ethernet PHYs).
3. Then, we saw SoC with ethernet switch devices also accessible over
shared MDIO bus (or Muxed MDIO bus) along with ethernet PHYs (I guess
this is why we have drivers/net/phy/mdio_device.c which adds
"mdio_device" for non-ethernet-PHY devices on MDIO bus).
4. Now, we have SoC with SATA PHYs, PCIe PHYs, USB PHYs, and Ethernet
PHYs all accessible over same shared MDIO bus (or Muxed MDIO bus). The
SATA PHYs and PCIe PHYs are registered to "Generic PHY framework". For
USB PHYs, we can either register to "Generic PHY framework" or "USB PHY
framework". For Ethernet PHYs, we register MDIO bus instance to "Ethernet
MDIO framework".
The devices on ARM64 SoC has to be within first 4GB and RAM has to start
from first 4GB to be ARM compliant because ARM64 CPUs have 32bit mode and
all devices and RAM should be available to 32bit mode with MMU disabled.
This means that we only have 4GB to fit all devices registers and some
portion of RAM. Some of these non-Ethernet PHYs have tons of registers so
as number of PHYs increase in a SoC they will eat-up lot of first 4GB
address space. Using same MDIO bus for all types of PHYs (SATA, PCIe, USB,
and Ethernet) is actually a good approach because it actually saves lot of
first 4GB address space. In future, more devices will be moved to a shared
MDIO bus which are less frequently accessed.
For Broadcom iProc SoCs, the design choice has already been made to use
shared MDIO bus for all PHYs. In fact, Broadcom iProc NS2 SoC already has
a shared MDIO bus for SATA PHYs, USB PHYs, PCIe PHYs, and Ethernet
PHYs and more Broadcom iProc SoCs are on their way. Of course, there are
few exceptions in iProc SoCs such as SATA PHYs where we also have memory
mapped registers to access PHYs but other PHYs don't have such memory
mapped registers.
Clearly from above, the traditional 2 wire MDIO bus is now a shared MDIO
bus with 2-wire plus additional select lines. Also, now we have SoCs (such
as Broadcom iProc SoCs) which has such shared MDIO bus and I think
in-future we will have SoCs with a shared MDIO bus for variety of devices.
For long term, we really need a clean solution to fit shared MDIO based
PHY drivers in Linux kernel. Also, shared MDIO based PHY drivers should
not be dependent on any particular IO subsystem (such as Linux Ethernet)
because there are lot of use-cases where people want strip down kernel
image by not-compiling IO subsystems which are not required.
IMHO, we have several options in front of us:
1. Use some light-weight framework (such as shared_mdio.c implemented
by this patchset) under drivers/bus
2. Extend "Generic PHY framework" to allow something like shared MDIO
bus (as-per Arnd's suggestion)
3. Move-out "MDIO-mux APIs" from drivers/net/phy to something like
drivers/mdio-mux and make it independent of "Linux Ethernet subsystem".
(... may be more options ...)
Regards,
Anup
On Wednesday 27 April 2016 10:16:40 Anup Patel wrote:
> It is really interesting to see the evolution of MDIO bus:
>
> 1. Traditionally, MDIO controller used to be part of each ethernet controller
> itself so each ethernet controller used to have it's own 2 wire MDIO bus
>
> 2. Next, we saw SoC with multiple ethernet controllers sharing same MDIO
> bus. In other words, we saw multiple MDIO bus being muxed over single
> MDIO bus with additional bus select lines (I think this is when
> drivers/net/phy/mdio-mux.c APIs were implemented but at this point all
> PHYs on muxed MDIO bus were ethernet PHYs).
>
> 3. Then, we saw SoC with ethernet switch devices also accessible over
> shared MDIO bus (or Muxed MDIO bus) along with ethernet PHYs (I guess
> this is why we have drivers/net/phy/mdio_device.c which adds
> "mdio_device" for non-ethernet-PHY devices on MDIO bus).
>
> 4. Now, we have SoC with SATA PHYs, PCIe PHYs, USB PHYs, and Ethernet
> PHYs all accessible over same shared MDIO bus (or Muxed MDIO bus). The
> SATA PHYs and PCIe PHYs are registered to "Generic PHY framework". For
> USB PHYs, we can either register to "Generic PHY framework" or "USB PHY
> framework". For Ethernet PHYs, we register MDIO bus instance to "Ethernet
> MDIO framework".
Thanks for the extra background information.
> The devices on ARM64 SoC has to be within first 4GB and RAM has to start
> from first 4GB to be ARM compliant because ARM64 CPUs have 32bit mode and
> all devices and RAM should be available to 32bit mode with MMU disabled.
> This means that we only have 4GB to fit all devices registers and some
> portion of RAM. Some of these non-Ethernet PHYs have tons of registers so
> as number of PHYs increase in a SoC they will eat-up lot of first 4GB
> address space. Using same MDIO bus for all types of PHYs (SATA, PCIe, USB,
> and Ethernet) is actually a good approach because it actually saves lot of
> first 4GB address space. In future, more devices will be moved to a shared
> MDIO bus which are less frequently accessed.
I think it remains to be seen if anyone other than Broadcom follows
this model. I have no idea if that's likely or not, perhaps everyone
does this, perhaps you are the only ones.
> For Broadcom iProc SoCs, the design choice has already been made to use
> shared MDIO bus for all PHYs. In fact, Broadcom iProc NS2 SoC already has
> a shared MDIO bus for SATA PHYs, USB PHYs, PCIe PHYs, and Ethernet
> PHYs and more Broadcom iProc SoCs are on their way. Of course, there are
> few exceptions in iProc SoCs such as SATA PHYs where we also have memory
> mapped registers to access PHYs but other PHYs don't have such memory
> mapped registers.
>
> Clearly from above, the traditional 2 wire MDIO bus is now a shared MDIO
> bus with 2-wire plus additional select lines. Also, now we have SoCs (such
> as Broadcom iProc SoCs) which has such shared MDIO bus and I think
> in-future we will have SoCs with a shared MDIO bus for variety of devices.
>
> For long term, we really need a clean solution to fit shared MDIO based
> PHY drivers in Linux kernel. Also, shared MDIO based PHY drivers should
> not be dependent on any particular IO subsystem (such as Linux Ethernet)
> because there are lot of use-cases where people want strip down kernel
> image by not-compiling IO subsystems which are not required.
>
> IMHO, we have several options in front of us:
> 1. Use some light-weight framework (such as shared_mdio.c implemented
> by this patchset) under drivers/bus
I think this has been sufficiently NAKed by everyone
> 2. Extend "Generic PHY framework" to allow something like shared MDIO
> bus (as-per Arnd's suggestion)
> 3. Move-out "MDIO-mux APIs" from drivers/net/phy to something like
> drivers/mdio-mux and make it independent of "Linux Ethernet subsystem".
> (... may be more options ...)
while these two really describe the same thing. I think as a first
step, we can reorganize the Kconfig structure to put ethernet PHY
and the MDIO bus as two separate submenus in drivers/phy/Kconfig,
and make the latter independent of CONFIG_NETDEVICES, see patch
below. With that, you should already be able to write a generic
phy driver that registers itself as an MDIO device driver.
We can also debate moving files from drivers/net/phy and
drivers/usb/phy into drivers/phy/{net,mdio,usb}/ as a follow-up,
but the file location is really not all that important here.
Arnd
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index befd67df08e1..c58b60e70ab2 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -356,8 +356,6 @@ config NET_SB1000
If you don't have this card, of course say N.
-source "drivers/net/phy/Kconfig"
-
source "drivers/net/plip/Kconfig"
source "drivers/net/ppp/Kconfig"
diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index 2ffd63463299..2e2491b344d9 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -11,9 +11,6 @@ menuconfig ETHERNET
if ETHERNET
-config MDIO
- tristate
-
config SUNGEM_PHY
tristate
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 6dad9a9c356c..58447866fe64 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -3,8 +3,9 @@
#
menuconfig PHYLIB
- tristate "PHY Device support and infrastructure"
+ tristate "Ethernet PHY Device support and infrastructure"
depends on NETDEVICES
+ select MDIO
help
Ethernet controllers are usually attached to PHY
devices. This option provides infrastructure for
@@ -164,6 +165,16 @@ config FIXED_PHY
PHYs that are not connected to the real MDIO bus.
Currently tested with mpc866ads and mpc8349e-mitx.
+endif # PHYLIB
+
+config MDIO
+ tristate
+ help
+ The MDIO bus is typically used ethernet PHYs, but can also be
+ used by other PHY drivers.
+
+menu "MDIO bus drivers"
+ depends on MDIO
config MDIO_BITBANG
tristate "Support for bitbanged MDIO buses"
@@ -271,7 +282,7 @@ config MDIO_BCM_IPROC
This module provides a driver for the MDIO busses found in the
Broadcom iProc SoC's.
-endif # PHYLIB
+endmenu # MDIO
config MICREL_KS8995MA
tristate "Micrel KS8995MA 5-ports 10/100 managed Ethernet switch"
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index ce29c17359e9..298893744a17 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -2,7 +2,9 @@
# PHY
#
-menu "PHY Subsystem"
+menu "PHY drivers"
+
+menu "Generic PHY subsystem"
config GENERIC_PHY
bool "PHY Core"
@@ -425,3 +427,7 @@ config PHY_CYGNUS_PCIE
source "drivers/phy/tegra/Kconfig"
endmenu
+
+source "drivers/net/phy/Kconfig"
+
+endmenu