Hello,
This series adds support for the MDIO controllers found in the AST2600. In the
AST2500 and earlier the MDIO controller was embedded in the MAC; this has now
been separated out and the register interface rearranged (again).
Please review!
Andrew
Andrew Jeffery (4):
dt-bindings: net: Add aspeed,ast2600-mdio binding
net: phy: Add mdio-aspeed
net: ftgmac100: Add support for DT phy-handle property
net: ftgmac100: Select ASPEED MDIO driver for the AST2600
.../bindings/net/aspeed,ast2600-mdio.yaml | 61 +++++++
drivers/net/ethernet/faraday/Kconfig | 1 +
drivers/net/ethernet/faraday/ftgmac100.c | 37 +++-
drivers/net/phy/Kconfig | 13 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/mdio-aspeed.c | 159 ++++++++++++++++++
6 files changed, 268 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml
create mode 100644 drivers/net/phy/mdio-aspeed.c
--
2.20.1
Ensures we can talk to a PHY via MDIO on the AST2600, as the MDIO
controller is now separate from the MAC.
Signed-off-by: Andrew Jeffery <[email protected]>
---
drivers/net/ethernet/faraday/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/faraday/Kconfig b/drivers/net/ethernet/faraday/Kconfig
index a9b105803fb7..73e4f2648e49 100644
--- a/drivers/net/ethernet/faraday/Kconfig
+++ b/drivers/net/ethernet/faraday/Kconfig
@@ -32,6 +32,7 @@ config FTGMAC100
depends on ARM || NDS32 || COMPILE_TEST
depends on !64BIT || BROKEN
select PHYLIB
+ select MDIO_ASPEED if MACH_ASPEED_G6
---help---
This driver supports the FTGMAC100 Gigabit Ethernet controller
from Faraday. It is used on Faraday A369, Andes AG102 and some
--
2.20.1
The AST2600 design separates the MDIO controllers from the MAC, which is
where they were placed in the AST2400 and AST2500. Further, the register
interface is reworked again, so now we have three possible different
interface implementations, however this driver only supports the
interface provided by the AST2600. The AST2400 and AST2500 will continue
to be supported by the MDIO support embedded in the FTGMAC100 driver.
Signed-off-by: Andrew Jeffery <[email protected]>
---
drivers/net/phy/Kconfig | 13 +++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/mdio-aspeed.c | 159 ++++++++++++++++++++++++++++++++++
3 files changed, 173 insertions(+)
create mode 100644 drivers/net/phy/mdio-aspeed.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 20f14c5fbb7e..206d8650ee7f 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -21,6 +21,19 @@ config MDIO_BUS
if MDIO_BUS
+config MDIO_ASPEED
+ tristate "ASPEED MDIO bus controller"
+ depends on ARCH_ASPEED || COMPILE_TEST
+ depends on OF_MDIO && HAS_IOMEM
+ help
+ This module provides a driver for the independent MDIO bus
+ controllers found in the ASPEED AST2600 SoC. This is a driver for the
+ third revision of the ASPEED MDIO register interface - the first two
+ revisions are the "old" and "new" interfaces found in the AST2400 and
+ AST2500, embedded in the MAC. For legacy reasons, FTGMAC100 driver
+ continues to drive the embedded MDIO controller for the AST2400 and
+ AST2500 SoCs, so say N if AST2600 support is not required.
+
config MDIO_BCM_IPROC
tristate "Broadcom iProc MDIO bus controller"
depends on ARCH_BCM_IPROC || COMPILE_TEST
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 839acb292c38..ba07c27e4208 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -22,6 +22,7 @@ libphy-$(CONFIG_LED_TRIGGER_PHY) += phy_led_triggers.o
obj-$(CONFIG_PHYLINK) += phylink.o
obj-$(CONFIG_PHYLIB) += libphy.o
+obj-$(CONFIG_MDIO_ASPEED) += mdio-aspeed.o
obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o
obj-$(CONFIG_MDIO_BCM_UNIMAC) += mdio-bcm-unimac.o
obj-$(CONFIG_MDIO_BITBANG) += mdio-bitbang.o
diff --git a/drivers/net/phy/mdio-aspeed.c b/drivers/net/phy/mdio-aspeed.c
new file mode 100644
index 000000000000..71496a9ff54a
--- /dev/null
+++ b/drivers/net/phy/mdio-aspeed.c
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Copyright (C) 2019 IBM Corp. */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/mdio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_mdio.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+
+#define DRV_NAME "mdio-aspeed"
+
+#define ASPEED_MDIO_CTRL 0x0
+#define ASPEED_MDIO_CTRL_FIRE BIT(31)
+#define ASPEED_MDIO_CTRL_ST BIT(28)
+#define ASPEED_MDIO_CTRL_ST_C45 0
+#define ASPEED_MDIO_CTRL_ST_C22 1
+#define ASPEED_MDIO_CTRL_OP GENMASK(27, 26)
+#define MDIO_C22_OP_WRITE 0b01
+#define MDIO_C22_OP_READ 0b10
+#define ASPEED_MDIO_CTRL_PHYAD GENMASK(25, 21)
+#define ASPEED_MDIO_CTRL_REGAD GENMASK(20, 16)
+#define ASPEED_MDIO_CTRL_MIIWDATA GENMASK(15, 0)
+
+#define ASPEED_MDIO_DATA 0x4
+#define ASPEED_MDIO_DATA_MDC_THRES GENMASK(31, 24)
+#define ASPEED_MDIO_DATA_MDIO_EDGE BIT(23)
+#define ASPEED_MDIO_DATA_MDIO_LATCH GENMASK(22, 20)
+#define ASPEED_MDIO_DATA_IDLE BIT(16)
+#define ASPEED_MDIO_DATA_MIIRDATA GENMASK(15, 0)
+
+#define ASPEED_MDIO_RETRIES 10
+
+struct aspeed_mdio {
+ void __iomem *base;
+};
+
+static int aspeed_mdio_read(struct mii_bus *bus, int addr, int regnum)
+{
+ struct aspeed_mdio *ctx = bus->priv;
+ u32 ctrl;
+ int i;
+
+ dev_dbg(&bus->dev, "%s: addr: %d, regnum: %d\n", __func__, addr,
+ regnum);
+
+ /* Just clause 22 for the moment */
+ ctrl = ASPEED_MDIO_CTRL_FIRE
+ | FIELD_PREP(ASPEED_MDIO_CTRL_ST, ASPEED_MDIO_CTRL_ST_C22)
+ | FIELD_PREP(ASPEED_MDIO_CTRL_OP, MDIO_C22_OP_READ)
+ | FIELD_PREP(ASPEED_MDIO_CTRL_PHYAD, addr)
+ | FIELD_PREP(ASPEED_MDIO_CTRL_REGAD, regnum);
+
+ iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL);
+
+ for (i = 0; i < ASPEED_MDIO_RETRIES; i++) {
+ u32 data;
+
+ data = ioread32(ctx->base + ASPEED_MDIO_DATA);
+ if (data & ASPEED_MDIO_DATA_IDLE)
+ return FIELD_GET(ASPEED_MDIO_DATA_MIIRDATA, data);
+
+ udelay(100);
+ }
+
+ dev_err(&bus->dev, "MDIO read failed\n");
+ return -EIO;
+}
+
+static int aspeed_mdio_write(struct mii_bus *bus, int addr, int regnum, u16 val)
+{
+ struct aspeed_mdio *ctx = bus->priv;
+ u32 ctrl;
+ int i;
+
+ dev_dbg(&bus->dev, "%s: addr: %d, regnum: %d, val: 0x%x\n",
+ __func__, addr, regnum, val);
+
+ /* Just clause 22 for the moment */
+ ctrl = ASPEED_MDIO_CTRL_FIRE
+ | FIELD_PREP(ASPEED_MDIO_CTRL_ST, ASPEED_MDIO_CTRL_ST_C22)
+ | FIELD_PREP(ASPEED_MDIO_CTRL_OP, MDIO_C22_OP_WRITE)
+ | FIELD_PREP(ASPEED_MDIO_CTRL_PHYAD, addr)
+ | FIELD_PREP(ASPEED_MDIO_CTRL_REGAD, regnum)
+ | FIELD_PREP(ASPEED_MDIO_CTRL_MIIWDATA, val);
+
+ iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL);
+
+ for (i = 0; i < ASPEED_MDIO_RETRIES; i++) {
+ ctrl = ioread32(ctx->base + ASPEED_MDIO_CTRL);
+ if (!(ctrl & ASPEED_MDIO_CTRL_FIRE))
+ return 0;
+
+ udelay(100);
+ }
+
+ dev_err(&bus->dev, "MDIO write failed\n");
+ return -EIO;
+}
+
+static int aspeed_mdio_probe(struct platform_device *pdev)
+{
+ struct aspeed_mdio *ctx;
+ struct mii_bus *bus;
+ int rc;
+
+ bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*ctx));
+ if (!bus)
+ return -ENOMEM;
+
+ ctx = bus->priv;
+ ctx->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(ctx->base))
+ return PTR_ERR(ctx->base);
+
+ bus->name = DRV_NAME;
+ snprintf(bus->id, MII_BUS_ID_SIZE, "%s%d", pdev->name, pdev->id);
+ bus->parent = &pdev->dev;
+ bus->read = aspeed_mdio_read;
+ bus->write = aspeed_mdio_write;
+
+ rc = of_mdiobus_register(bus, pdev->dev.of_node);
+ if (rc) {
+ dev_err(&pdev->dev, "Cannot register MDIO bus!\n");
+ return rc;
+ }
+
+ platform_set_drvdata(pdev, bus);
+
+ return 0;
+}
+
+static int aspeed_mdio_remove(struct platform_device *pdev)
+{
+ mdiobus_unregister(platform_get_drvdata(pdev));
+
+ return 0;
+}
+
+static const struct of_device_id aspeed_mdio_of_match[] = {
+ { .compatible = "aspeed,ast2600-mdio", },
+ { },
+};
+
+static struct platform_driver aspeed_mdio_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .of_match_table = aspeed_mdio_of_match,
+ },
+ .probe = aspeed_mdio_probe,
+ .remove = aspeed_mdio_remove,
+};
+
+module_platform_driver(aspeed_mdio_driver);
+
+MODULE_AUTHOR("Andrew Jeffery <[email protected]>");
+MODULE_LICENSE("GPL");
--
2.20.1
phy-handle is necessary for the AST2600 which separates the MDIO
controllers from the MAC.
I've tried to minimise the intrusion of supporting the AST2600 to the
FTGMAC100 by leaving in place the existing MDIO support for the embedded
MDIO interface. The AST2400 and AST2500 continue to be supported this
way, as it avoids breaking/reworking existing devicetrees.
The AST2600 support by contrast requires the presence of the phy-handle
property in the MAC devicetree node to specify the appropriate PHY to
associate with the MAC. In the event that someone wants to specify the
MDIO bus topology under the MAC node on an AST2400 or AST2500, the
current auto-probe approach is done conditional on the absence of an
"mdio" child node of the MAC.
Signed-off-by: Andrew Jeffery <[email protected]>
---
drivers/net/ethernet/faraday/ftgmac100.c | 37 +++++++++++++++++++++---
1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 030fed65393e..563384b788ab 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -17,6 +17,7 @@
#include <linux/module.h>
#include <linux/netdevice.h>
#include <linux/of.h>
+#include <linux/of_mdio.h>
#include <linux/phy.h>
#include <linux/platform_device.h>
#include <linux/property.h>
@@ -1619,8 +1620,13 @@ static int ftgmac100_setup_mdio(struct net_device *netdev)
if (!priv->mii_bus)
return -EIO;
- if (priv->is_aspeed) {
- /* This driver supports the old MDIO interface */
+ if (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
+ of_device_is_compatible(np, "aspeed,ast2500-mac")) {
+ /* The AST2600 has a separate MDIO controller */
+
+ /* For the AST2400 and AST2500 this driver only supports the
+ * old MDIO interface
+ */
reg = ioread32(priv->base + FTGMAC100_OFFSET_REVR);
reg &= ~FTGMAC100_REVR_NEW_MDIO_INTERFACE;
iowrite32(reg, priv->base + FTGMAC100_OFFSET_REVR);
@@ -1797,7 +1803,8 @@ static int ftgmac100_probe(struct platform_device *pdev)
np = pdev->dev.of_node;
if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
- of_device_is_compatible(np, "aspeed,ast2500-mac"))) {
+ of_device_is_compatible(np, "aspeed,ast2500-mac") ||
+ of_device_is_compatible(np, "aspeed,ast2600-mac"))) {
priv->rxdes0_edorr_mask = BIT(30);
priv->txdes0_edotr_mask = BIT(30);
priv->is_aspeed = true;
@@ -1817,7 +1824,29 @@ static int ftgmac100_probe(struct platform_device *pdev)
priv->ndev = ncsi_register_dev(netdev, ftgmac100_ncsi_handler);
if (!priv->ndev)
goto err_ncsi_dev;
- } else {
+ } else if (np && of_get_property(np, "phy-handle", NULL)) {
+ struct phy_device *phy;
+
+ phy = of_phy_get_and_connect(priv->netdev, np,
+ &ftgmac100_adjust_link);
+ if (!phy) {
+ dev_err(&pdev->dev, "Failed to connect to phy\n");
+ goto err_setup_mdio;
+ }
+
+ /* Indicate that we support PAUSE frames (see comment in
+ * Documentation/networking/phy.txt)
+ */
+ phy_support_asym_pause(phy);
+
+ /* Display what we found */
+ phy_attached_info(phy);
+ } else if (np && !of_get_child_by_name(np, "mdio")) {
+ /* Support legacy ASPEED devicetree descriptions that decribe a
+ * MAC with an embedded MDIO controller but have no "mdio"
+ * child node. Automatically scan the MDIO bus for available
+ * PHYs.
+ */
priv->use_ncsi = false;
err = ftgmac100_setup_mdio(netdev);
if (err)
--
2.20.1
The AST2600 splits out the MDIO bus controller from the MAC into its own
IP block and rearranges the register layout. Add a new binding to
describe the new hardware.
Signed-off-by: Andrew Jeffery <[email protected]>
---
.../bindings/net/aspeed,ast2600-mdio.yaml | 61 +++++++++++++++++++
1 file changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml
diff --git a/Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml b/Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml
new file mode 100644
index 000000000000..fa86f6438473
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/aspeed,ast2600-mdio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ASPEED AST2600 MDIO Controller
+
+maintainers:
+ - Andrew Jeffery <[email protected]>
+
+description: |+
+ The ASPEED AST2600 MDIO controller is the third iteration of ASPEED's MDIO
+ bus register interface, this time also separating out the controller from the
+ MAC.
+
+properties:
+ compatible:
+ const: aspeed,ast2600-mdio
+ reg:
+ maxItems: 1
+ description: The register range of the MDIO controller instance
+ "#address-cells":
+ const: 1
+ "#size-cells":
+ const: 0
+
+patternProperties:
+ "^ethernet-phy@[a-f0-9]$":
+ properties:
+ reg:
+ description:
+ The MDIO bus index of the PHY
+ compatible:
+ enum:
+ - ethernet-phy-ieee802.3-c22
+ - ethernet-phy-ieee802.3-c45
+ required:
+ - reg
+
+required:
+ - compatible
+ - reg
+ - "#address-cells"
+ - "#size-cells"
+
+examples:
+ - |
+ mdio0: mdio@1e650000 {
+ compatible = "aspeed,ast2600-mdio";
+ reg = <0x1e650000 0x8>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ status = "okay";
+
+ ethphy0: ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <0>;
+ };
+ };
--
2.20.1
On Mon, Jul 29, 2019 at 02:09:24PM +0930, Andrew Jeffery wrote:
> The AST2600 design separates the MDIO controllers from the MAC, which is
> where they were placed in the AST2400 and AST2500. Further, the register
> interface is reworked again, so now we have three possible different
> interface implementations, however this driver only supports the
> interface provided by the AST2600. The AST2400 and AST2500 will continue
> to be supported by the MDIO support embedded in the FTGMAC100 driver.
>
> Signed-off-by: Andrew Jeffery <[email protected]>
> ---
> drivers/net/phy/Kconfig | 13 +++
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/mdio-aspeed.c | 159 ++++++++++++++++++++++++++++++++++
> 3 files changed, 173 insertions(+)
> create mode 100644 drivers/net/phy/mdio-aspeed.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 20f14c5fbb7e..206d8650ee7f 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -21,6 +21,19 @@ config MDIO_BUS
>
> if MDIO_BUS
>
> +config MDIO_ASPEED
> + tristate "ASPEED MDIO bus controller"
> + depends on ARCH_ASPEED || COMPILE_TEST
> + depends on OF_MDIO && HAS_IOMEM
> + help
> + This module provides a driver for the independent MDIO bus
> + controllers found in the ASPEED AST2600 SoC. This is a driver for the
> + third revision of the ASPEED MDIO register interface - the first two
> + revisions are the "old" and "new" interfaces found in the AST2400 and
> + AST2500, embedded in the MAC. For legacy reasons, FTGMAC100 driver
> + continues to drive the embedded MDIO controller for the AST2400 and
> + AST2500 SoCs, so say N if AST2600 support is not required.
> +
> config MDIO_BCM_IPROC
> tristate "Broadcom iProc MDIO bus controller"
> depends on ARCH_BCM_IPROC || COMPILE_TEST
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 839acb292c38..ba07c27e4208 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -22,6 +22,7 @@ libphy-$(CONFIG_LED_TRIGGER_PHY) += phy_led_triggers.o
> obj-$(CONFIG_PHYLINK) += phylink.o
> obj-$(CONFIG_PHYLIB) += libphy.o
>
> +obj-$(CONFIG_MDIO_ASPEED) += mdio-aspeed.o
> obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o
> obj-$(CONFIG_MDIO_BCM_UNIMAC) += mdio-bcm-unimac.o
> obj-$(CONFIG_MDIO_BITBANG) += mdio-bitbang.o
> diff --git a/drivers/net/phy/mdio-aspeed.c b/drivers/net/phy/mdio-aspeed.c
> new file mode 100644
> index 000000000000..71496a9ff54a
> --- /dev/null
> +++ b/drivers/net/phy/mdio-aspeed.c
> @@ -0,0 +1,159 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* Copyright (C) 2019 IBM Corp. */
> +
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/mdio.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_mdio.h>
> +#include <linux/phy.h>
> +#include <linux/platform_device.h>
> +
> +#define DRV_NAME "mdio-aspeed"
> +
> +#define ASPEED_MDIO_CTRL 0x0
> +#define ASPEED_MDIO_CTRL_FIRE BIT(31)
> +#define ASPEED_MDIO_CTRL_ST BIT(28)
> +#define ASPEED_MDIO_CTRL_ST_C45 0
> +#define ASPEED_MDIO_CTRL_ST_C22 1
> +#define ASPEED_MDIO_CTRL_OP GENMASK(27, 26)
> +#define MDIO_C22_OP_WRITE 0b01
> +#define MDIO_C22_OP_READ 0b10
> +#define ASPEED_MDIO_CTRL_PHYAD GENMASK(25, 21)
> +#define ASPEED_MDIO_CTRL_REGAD GENMASK(20, 16)
> +#define ASPEED_MDIO_CTRL_MIIWDATA GENMASK(15, 0)
> +
> +#define ASPEED_MDIO_DATA 0x4
> +#define ASPEED_MDIO_DATA_MDC_THRES GENMASK(31, 24)
> +#define ASPEED_MDIO_DATA_MDIO_EDGE BIT(23)
> +#define ASPEED_MDIO_DATA_MDIO_LATCH GENMASK(22, 20)
> +#define ASPEED_MDIO_DATA_IDLE BIT(16)
> +#define ASPEED_MDIO_DATA_MIIRDATA GENMASK(15, 0)
> +
> +#define ASPEED_MDIO_RETRIES 10
> +
> +struct aspeed_mdio {
> + void __iomem *base;
> +};
> +
> +static int aspeed_mdio_read(struct mii_bus *bus, int addr, int regnum)
> +{
> + struct aspeed_mdio *ctx = bus->priv;
> + u32 ctrl;
> + int i;
> +
> + dev_dbg(&bus->dev, "%s: addr: %d, regnum: %d\n", __func__, addr,
> + regnum);
> +
> + /* Just clause 22 for the moment */
> + ctrl = ASPEED_MDIO_CTRL_FIRE
Hi Andrew
In the binding, you say C45 is supported. Here you don't. It would be
nice to be consistent.
> + | FIELD_PREP(ASPEED_MDIO_CTRL_ST, ASPEED_MDIO_CTRL_ST_C22)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_OP, MDIO_C22_OP_READ)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_PHYAD, addr)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_REGAD, regnum);
> +
> + iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL);
> +
> + for (i = 0; i < ASPEED_MDIO_RETRIES; i++) {
> + u32 data;
> +
> + data = ioread32(ctx->base + ASPEED_MDIO_DATA);
> + if (data & ASPEED_MDIO_DATA_IDLE)
> + return FIELD_GET(ASPEED_MDIO_DATA_MIIRDATA, data);
> +
> + udelay(100);
> + }
One of the readx_poll_timeout functions could be used.
> +
> + dev_err(&bus->dev, "MDIO read failed\n");
> + return -EIO;
> +}
> +
> +static int aspeed_mdio_write(struct mii_bus *bus, int addr, int regnum, u16 val)
> +{
> + struct aspeed_mdio *ctx = bus->priv;
> + u32 ctrl;
> + int i;
> +
> + dev_dbg(&bus->dev, "%s: addr: %d, regnum: %d, val: 0x%x\n",
> + __func__, addr, regnum, val);
> +
> + /* Just clause 22 for the moment */
> + ctrl = ASPEED_MDIO_CTRL_FIRE
> + | FIELD_PREP(ASPEED_MDIO_CTRL_ST, ASPEED_MDIO_CTRL_ST_C22)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_OP, MDIO_C22_OP_WRITE)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_PHYAD, addr)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_REGAD, regnum)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_MIIWDATA, val);
> +
> + iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL);
> +
> + for (i = 0; i < ASPEED_MDIO_RETRIES; i++) {
> + ctrl = ioread32(ctx->base + ASPEED_MDIO_CTRL);
> + if (!(ctrl & ASPEED_MDIO_CTRL_FIRE))
> + return 0;
> +
> + udelay(100);
> + }
readx_poll_timeout() here as well.
Otherwise this looks good.
Andrew
On Sun, Jul 28, 2019 at 10:39 PM Andrew Jeffery <[email protected]> wrote:
>
> The AST2600 splits out the MDIO bus controller from the MAC into its own
> IP block and rearranges the register layout. Add a new binding to
> describe the new hardware.
>
> Signed-off-by: Andrew Jeffery <[email protected]>
> ---
> .../bindings/net/aspeed,ast2600-mdio.yaml | 61 +++++++++++++++++++
> 1 file changed, 61 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml b/Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml
> new file mode 100644
> index 000000000000..fa86f6438473
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/aspeed,ast2600-mdio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ASPEED AST2600 MDIO Controller
> +
> +maintainers:
> + - Andrew Jeffery <[email protected]>
> +
> +description: |+
> + The ASPEED AST2600 MDIO controller is the third iteration of ASPEED's MDIO
> + bus register interface, this time also separating out the controller from the
> + MAC.
> +
Should have a:
allOf:
- $ref: "mdio.yaml#"
> +properties:
> + compatible:
> + const: aspeed,ast2600-mdio
> + reg:
> + maxItems: 1
> + description: The register range of the MDIO controller instance
> + "#address-cells":
> + const: 1
> + "#size-cells":
> + const: 0
Then you can drop these 2.
> +
> +patternProperties:
> + "^ethernet-phy@[a-f0-9]$":
> + properties:
> + reg:
> + description:
> + The MDIO bus index of the PHY
And this.
> + compatible:
> + enum:
> + - ethernet-phy-ieee802.3-c22
> + - ethernet-phy-ieee802.3-c45
This isn't specific to ASpeed either and is already covered by
ethernet-phy.yaml.
So that means none of the child node schema is needed here.
> + required:
> + - reg
> +
> +required:
> + - compatible
> + - reg
> + - "#address-cells"
> + - "#size-cells"
> +
> +examples:
> + - |
> + mdio0: mdio@1e650000 {
> + compatible = "aspeed,ast2600-mdio";
> + reg = <0x1e650000 0x8>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + status = "okay";
Don't show status in examples.
> +
> + ethphy0: ethernet-phy@0 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <0>;
> + };
> + };
> --
> 2.20.1
>
On Tue, 30 Jul 2019, at 09:07, Rob Herring wrote:
> On Sun, Jul 28, 2019 at 10:39 PM Andrew Jeffery <[email protected]> wrote:
> >
> > The AST2600 splits out the MDIO bus controller from the MAC into its own
> > IP block and rearranges the register layout. Add a new binding to
> > describe the new hardware.
> >
> > Signed-off-by: Andrew Jeffery <[email protected]>
> > ---
> > .../bindings/net/aspeed,ast2600-mdio.yaml | 61 +++++++++++++++++++
> > 1 file changed, 61 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml b/Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml
> > new file mode 100644
> > index 000000000000..fa86f6438473
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml
> > @@ -0,0 +1,61 @@
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/aspeed,ast2600-mdio.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ASPEED AST2600 MDIO Controller
> > +
> > +maintainers:
> > + - Andrew Jeffery <[email protected]>
> > +
> > +description: |+
> > + The ASPEED AST2600 MDIO controller is the third iteration of ASPEED's MDIO
> > + bus register interface, this time also separating out the controller from the
> > + MAC.
> > +
>
> Should have a:
>
> allOf:
> - $ref: "mdio.yaml#"
Ack.
>
> > +properties:
> > + compatible:
> > + const: aspeed,ast2600-mdio
> > + reg:
> > + maxItems: 1
> > + description: The register range of the MDIO controller instance
>
> > + "#address-cells":
> > + const: 1
> > + "#size-cells":
> > + const: 0
>
> Then you can drop these 2.
Great.
>
> > +
> > +patternProperties:
> > + "^ethernet-phy@[a-f0-9]$":
> > + properties:
> > + reg:
> > + description:
> > + The MDIO bus index of the PHY
>
> And this.
>
> > + compatible:
> > + enum:
> > + - ethernet-phy-ieee802.3-c22
> > + - ethernet-phy-ieee802.3-c45
>
> This isn't specific to ASpeed either and is already covered by
> ethernet-phy.yaml.
>
> So that means none of the child node schema is needed here.
Bah, I'd developed the patches on v5.2 while waiting for v5.3-rc1 to come
out.
>
> > + required:
> > + - reg
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - "#address-cells"
> > + - "#size-cells"
> > +
> > +examples:
> > + - |
> > + mdio0: mdio@1e650000 {
> > + compatible = "aspeed,ast2600-mdio";
> > + reg = <0x1e650000 0x8>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + status = "okay";
>
> Don't show status in examples.
Sorry, copy/paste at fault.
Thanks for the feedback.
Andrew
>
> > +
> > + ethphy0: ethernet-phy@0 {
> > + compatible = "ethernet-phy-ieee802.3-c22";
> > + reg = <0>;
> > + };
> > + };
> > --
> > 2.20.1
> >
>
On Mon, 29 Jul 2019, at 23:03, Andrew Lunn wrote:
> On Mon, Jul 29, 2019 at 02:09:24PM +0930, Andrew Jeffery wrote:
> > The AST2600 design separates the MDIO controllers from the MAC, which is
> > where they were placed in the AST2400 and AST2500. Further, the register
> > interface is reworked again, so now we have three possible different
> > interface implementations, however this driver only supports the
> > interface provided by the AST2600. The AST2400 and AST2500 will continue
> > to be supported by the MDIO support embedded in the FTGMAC100 driver.
> >
> > Signed-off-by: Andrew Jeffery <[email protected]>
> > ---
> > drivers/net/phy/Kconfig | 13 +++
> > drivers/net/phy/Makefile | 1 +
> > drivers/net/phy/mdio-aspeed.c | 159 ++++++++++++++++++++++++++++++++++
> > 3 files changed, 173 insertions(+)
> > create mode 100644 drivers/net/phy/mdio-aspeed.c
> >
> > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> > index 20f14c5fbb7e..206d8650ee7f 100644
> > --- a/drivers/net/phy/Kconfig
> > +++ b/drivers/net/phy/Kconfig
> > @@ -21,6 +21,19 @@ config MDIO_BUS
> >
> > if MDIO_BUS
> >
> > +config MDIO_ASPEED
> > + tristate "ASPEED MDIO bus controller"
> > + depends on ARCH_ASPEED || COMPILE_TEST
> > + depends on OF_MDIO && HAS_IOMEM
> > + help
> > + This module provides a driver for the independent MDIO bus
> > + controllers found in the ASPEED AST2600 SoC. This is a driver for the
> > + third revision of the ASPEED MDIO register interface - the first two
> > + revisions are the "old" and "new" interfaces found in the AST2400 and
> > + AST2500, embedded in the MAC. For legacy reasons, FTGMAC100 driver
> > + continues to drive the embedded MDIO controller for the AST2400 and
> > + AST2500 SoCs, so say N if AST2600 support is not required.
> > +
> > config MDIO_BCM_IPROC
> > tristate "Broadcom iProc MDIO bus controller"
> > depends on ARCH_BCM_IPROC || COMPILE_TEST
> > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> > index 839acb292c38..ba07c27e4208 100644
> > --- a/drivers/net/phy/Makefile
> > +++ b/drivers/net/phy/Makefile
> > @@ -22,6 +22,7 @@ libphy-$(CONFIG_LED_TRIGGER_PHY) += phy_led_triggers.o
> > obj-$(CONFIG_PHYLINK) += phylink.o
> > obj-$(CONFIG_PHYLIB) += libphy.o
> >
> > +obj-$(CONFIG_MDIO_ASPEED) += mdio-aspeed.o
> > obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o
> > obj-$(CONFIG_MDIO_BCM_UNIMAC) += mdio-bcm-unimac.o
> > obj-$(CONFIG_MDIO_BITBANG) += mdio-bitbang.o
> > diff --git a/drivers/net/phy/mdio-aspeed.c b/drivers/net/phy/mdio-aspeed.c
> > new file mode 100644
> > index 000000000000..71496a9ff54a
> > --- /dev/null
> > +++ b/drivers/net/phy/mdio-aspeed.c
> > @@ -0,0 +1,159 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/* Copyright (C) 2019 IBM Corp. */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/delay.h>
> > +#include <linux/mdio.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_mdio.h>
> > +#include <linux/phy.h>
> > +#include <linux/platform_device.h>
> > +
> > +#define DRV_NAME "mdio-aspeed"
> > +
> > +#define ASPEED_MDIO_CTRL 0x0
> > +#define ASPEED_MDIO_CTRL_FIRE BIT(31)
> > +#define ASPEED_MDIO_CTRL_ST BIT(28)
> > +#define ASPEED_MDIO_CTRL_ST_C45 0
> > +#define ASPEED_MDIO_CTRL_ST_C22 1
> > +#define ASPEED_MDIO_CTRL_OP GENMASK(27, 26)
> > +#define MDIO_C22_OP_WRITE 0b01
> > +#define MDIO_C22_OP_READ 0b10
> > +#define ASPEED_MDIO_CTRL_PHYAD GENMASK(25, 21)
> > +#define ASPEED_MDIO_CTRL_REGAD GENMASK(20, 16)
> > +#define ASPEED_MDIO_CTRL_MIIWDATA GENMASK(15, 0)
> > +
> > +#define ASPEED_MDIO_DATA 0x4
> > +#define ASPEED_MDIO_DATA_MDC_THRES GENMASK(31, 24)
> > +#define ASPEED_MDIO_DATA_MDIO_EDGE BIT(23)
> > +#define ASPEED_MDIO_DATA_MDIO_LATCH GENMASK(22, 20)
> > +#define ASPEED_MDIO_DATA_IDLE BIT(16)
> > +#define ASPEED_MDIO_DATA_MIIRDATA GENMASK(15, 0)
> > +
> > +#define ASPEED_MDIO_RETRIES 10
> > +
> > +struct aspeed_mdio {
> > + void __iomem *base;
> > +};
> > +
> > +static int aspeed_mdio_read(struct mii_bus *bus, int addr, int regnum)
> > +{
> > + struct aspeed_mdio *ctx = bus->priv;
> > + u32 ctrl;
> > + int i;
> > +
> > + dev_dbg(&bus->dev, "%s: addr: %d, regnum: %d\n", __func__, addr,
> > + regnum);
> > +
> > + /* Just clause 22 for the moment */
> > + ctrl = ASPEED_MDIO_CTRL_FIRE
>
> Hi Andrew
>
> In the binding, you say C45 is supported. Here you don't. It would be
> nice to be consistent.
Right - but the bindings describe the hardware, and the hardware is capable.
Just that the driver as it stands can't drive it that way.
Having said that I do need to do more digging to understand how to cater to
C45 PHYs wrt the read/write calls.
>
>
> > + | FIELD_PREP(ASPEED_MDIO_CTRL_ST, ASPEED_MDIO_CTRL_ST_C22)
> > + | FIELD_PREP(ASPEED_MDIO_CTRL_OP, MDIO_C22_OP_READ)
> > + | FIELD_PREP(ASPEED_MDIO_CTRL_PHYAD, addr)
> > + | FIELD_PREP(ASPEED_MDIO_CTRL_REGAD, regnum);
> > +
> > + iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL);
> > +
> > + for (i = 0; i < ASPEED_MDIO_RETRIES; i++) {
> > + u32 data;
> > +
> > + data = ioread32(ctx->base + ASPEED_MDIO_DATA);
> > + if (data & ASPEED_MDIO_DATA_IDLE)
> > + return FIELD_GET(ASPEED_MDIO_DATA_MIIRDATA, data);
> > +
> > + udelay(100);
> > + }
>
> One of the readx_poll_timeout functions could be used.
Thanks, I'll take a look.
>
> > +
> > + dev_err(&bus->dev, "MDIO read failed\n");
> > + return -EIO;
> > +}
> > +
> > +static int aspeed_mdio_write(struct mii_bus *bus, int addr, int regnum, u16 val)
> > +{
> > + struct aspeed_mdio *ctx = bus->priv;
> > + u32 ctrl;
> > + int i;
> > +
> > + dev_dbg(&bus->dev, "%s: addr: %d, regnum: %d, val: 0x%x\n",
> > + __func__, addr, regnum, val);
> > +
> > + /* Just clause 22 for the moment */
> > + ctrl = ASPEED_MDIO_CTRL_FIRE
> > + | FIELD_PREP(ASPEED_MDIO_CTRL_ST, ASPEED_MDIO_CTRL_ST_C22)
> > + | FIELD_PREP(ASPEED_MDIO_CTRL_OP, MDIO_C22_OP_WRITE)
> > + | FIELD_PREP(ASPEED_MDIO_CTRL_PHYAD, addr)
> > + | FIELD_PREP(ASPEED_MDIO_CTRL_REGAD, regnum)
> > + | FIELD_PREP(ASPEED_MDIO_CTRL_MIIWDATA, val);
> > +
> > + iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL);
> > +
> > + for (i = 0; i < ASPEED_MDIO_RETRIES; i++) {
> > + ctrl = ioread32(ctx->base + ASPEED_MDIO_CTRL);
> > + if (!(ctrl & ASPEED_MDIO_CTRL_FIRE))
> > + return 0;
> > +
> > + udelay(100);
> > + }
>
> readx_poll_timeout() here as well.
>
> Otherwise this looks good.
Thanks for the review!
Andrew