2019-07-29 10:04:59

by Claudiu Manoil

[permalink] [raw]
Subject: [PATCH net-next v3 0/4] enetc: Add mdio bus driver for the PCIe MDIO endpoint

First patch fixes a sparse issue and cleans up accessors to avoid
casting to __iomem.
Second patch just registers the PCIe endpoint device containing
the MDIO registers as a standalone MDIO bus driver, to allow
an alternative way to control the MDIO bus. The same code used
by the ENETC ports (eth controllers) to manage MDIO via local
registers applies and is reused.

Bindings are provided for the new MDIO node, similarly to ENETC
port nodes bindings.

Last patch enables the ENETC port 1 and its RGMII PHY on the
LS1028A QDS board, where the MDIO muxing configuration relies
on the MDIO support provided in the first patch.

Changes since v0:
v1 - fixed mdio bus allocation
v2 - cleaned up accessors to avoid casting
v3 - fixed spelling (mostly commit message)

Claudiu Manoil (4):
enetc: Clean up local mdio bus allocation
enetc: Add mdio bus driver for the PCIe MDIO endpoint
dt-bindings: net: fsl: enetc: Add bindings for the central MDIO PCIe
endpoint
arm64: dts: fsl: ls1028a: Enable eth port1 on the ls1028a QDS board

.../devicetree/bindings/net/fsl-enetc.txt | 42 +++-
.../boot/dts/freescale/fsl-ls1028a-qds.dts | 40 ++++
.../arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 6 +
.../net/ethernet/freescale/enetc/enetc_mdio.c | 190 +++++++++++++-----
.../net/ethernet/freescale/enetc/enetc_pf.c | 5 +-
5 files changed, 232 insertions(+), 51 deletions(-)

--
2.17.1


2019-07-29 10:05:42

by Claudiu Manoil

[permalink] [raw]
Subject: [PATCH net-next v3 3/4] dt-bindings: net: fsl: enetc: Add bindings for the central MDIO PCIe endpoint

The on-chip PCIe root complex that integrates the ENETC ethernet
controllers also integrates a PCIe endpoint for the MDIO controller
providing for centralized control of the ENETC mdio bus.
Add bindings for this "central" MDIO Integrated PCIe Endpoint.

Signed-off-by: Claudiu Manoil <[email protected]>
---
v1 - none
v2 - none
v3 - fixed spelling (commit message mostly)

.../devicetree/bindings/net/fsl-enetc.txt | 42 +++++++++++++++++--
1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/fsl-enetc.txt b/Documentation/devicetree/bindings/net/fsl-enetc.txt
index 25fc687419db..b7034ccbc1bd 100644
--- a/Documentation/devicetree/bindings/net/fsl-enetc.txt
+++ b/Documentation/devicetree/bindings/net/fsl-enetc.txt
@@ -11,7 +11,9 @@ Required properties:
to parent node bindings.
- compatible : Should be "fsl,enetc".

-1) The ENETC external port is connected to a MDIO configurable phy:
+1. The ENETC external port is connected to a MDIO configurable phy
+
+1.1. Using the local ENETC Port MDIO interface

In this case, the ENETC node should include a "mdio" sub-node
that in turn should contain the "ethernet-phy" node describing the
@@ -47,8 +49,42 @@ Example:
};
};

-2) The ENETC port is an internal port or has a fixed-link external
-connection:
+1.2. Using the central MDIO PCIe endpoint device
+
+In this case, the mdio node should be defined as another PCIe
+endpoint node, at the same level with the ENETC port nodes.
+
+Required properties:
+
+- reg : Specifies PCIe Device Number and Function
+ Number of the ENETC endpoint device, according
+ to parent node bindings.
+- compatible : Should be "fsl,enetc-mdio".
+
+The remaining required mdio bus properties are standard, their bindings
+already defined in Documentation/devicetree/bindings/net/mdio.txt.
+
+Example:
+
+ ethernet@0,0 {
+ compatible = "fsl,enetc";
+ reg = <0x000000 0 0 0 0>;
+ phy-handle = <&sgmii_phy0>;
+ phy-connection-type = "sgmii";
+ };
+
+ mdio@0,3 {
+ compatible = "fsl,enetc-mdio";
+ reg = <0x000300 0 0 0 0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ sgmii_phy0: ethernet-phy@2 {
+ reg = <0x2>;
+ };
+ };
+
+2. The ENETC port is an internal port or has a fixed-link external
+connection

In this case, the ENETC port node defines a fixed link connection,
as specified by Documentation/devicetree/bindings/net/fixed-link.txt.
--
2.17.1

2019-07-29 10:06:01

by Claudiu Manoil

[permalink] [raw]
Subject: [PATCH net-next v3 4/4] arm64: dts: fsl: ls1028a: Enable eth port1 on the ls1028a QDS board

LS1028a has one Ethernet management interface. On the QDS board, the
MDIO signals are multiplexed to either on-board AR8035 PHY device or
to 4 PCIe slots allowing for SGMII cards.
To enable the Ethernet ENETC Port 1, which can only be connected to a
RGMII PHY, the multiplexer needs to be configured to route the MDIO to
the AR8035 PHY. The MDIO/MDC routing is controlled by bits 7:4 of FPGA
board config register 0x54, and value 0 selects the on-board RGMII PHY.
The FPGA board config registers are accessible on the i2c bus, at address
0x66.

The PF3 MDIO PCIe integrated endpoint device allows for centralized access
to the MDIO bus. Add the corresponding devicetree node and set it to be
the MDIO bus parent.

Signed-off-by: Alex Marginean <[email protected]>
Signed-off-by: Claudiu Manoil <[email protected]>
---
v1 - none
v2 - none
v3 - none

.../boot/dts/freescale/fsl-ls1028a-qds.dts | 40 +++++++++++++++++++
.../arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 6 +++
2 files changed, 46 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
index de6ef39f3118..663c4b728c07 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
@@ -85,6 +85,26 @@
system-clock-frequency = <25000000>;
};
};
+
+ mdio-mux {
+ compatible = "mdio-mux-multiplexer";
+ mux-controls = <&mux 0>;
+ mdio-parent-bus = <&enetc_mdio_pf3>;
+ #address-cells=<1>;
+ #size-cells = <0>;
+
+ /* on-board RGMII PHY */
+ mdio@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ qds_phy1: ethernet-phy@5 {
+ /* Atheros 8035 */
+ reg = <5>;
+ };
+ };
+ };
};

&duart0 {
@@ -164,6 +184,26 @@
};
};
};
+
+ fpga@66 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "fsl,ls1028aqds-fpga", "fsl,fpga-qixis-i2c",
+ "simple-mfd";
+ reg = <0x66>;
+
+ mux: mux-controller {
+ compatible = "reg-mux";
+ #mux-control-cells = <1>;
+ mux-reg-masks = <0x54 0xf0>; /* 0: reg 0x54, bits 7:4 */
+ };
+ };
+
+};
+
+&enetc_port1 {
+ phy-handle = <&qds_phy1>;
+ phy-connection-type = "rgmii-id";
};

&sai1 {
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index 7975519b4f56..de71153fda00 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -536,6 +536,12 @@
compatible = "fsl,enetc";
reg = <0x000100 0 0 0 0>;
};
+ enetc_mdio_pf3: mdio@0,3 {
+ compatible = "fsl,enetc-mdio";
+ reg = <0x000300 0 0 0 0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
ethernet@0,4 {
compatible = "fsl,enetc-ptp";
reg = <0x000400 0 0 0 0>;
--
2.17.1

2019-07-29 13:07:59

by Claudiu Manoil

[permalink] [raw]
Subject: [PATCH net-next v3 1/4] enetc: Clean up local mdio bus allocation

What's needed is basically a pointer to the mdio registers.
This is one way to store it inside bus->priv allocated space,
without upsetting sparse.
Reworked accessors to avoid __iomem casting.
Used devm_* variant to further clean up the init error /
remove paths.

Fixes following sparse warning:
warning: incorrect type in assignment (different address spaces)
expected void *priv
got struct enetc_mdio_regs [noderef] <asn:2>*[assigned] regs

Fixes: ebfcb23d62ab ("enetc: Add ENETC PF level external MDIO support")

Signed-off-by: Claudiu Manoil <[email protected]>
---
v1 - added this patch
v2 - reworked accessors as per Andrew Lunn's request
v3 - cleaned up commit message

.../net/ethernet/freescale/enetc/enetc_mdio.c | 94 +++++++++----------
1 file changed, 46 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
index 77b9cd10ba2b..05094601ece8 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
@@ -8,16 +8,22 @@

#include "enetc_pf.h"

-struct enetc_mdio_regs {
- u32 mdio_cfg; /* MDIO configuration and status */
- u32 mdio_ctl; /* MDIO control */
- u32 mdio_data; /* MDIO data */
- u32 mdio_addr; /* MDIO address */
+#define ENETC_MDIO_REG_OFFSET 0x1c00
+#define ENETC_MDIO_CFG 0x0 /* MDIO configuration and status */
+#define ENETC_MDIO_CTL 0x4 /* MDIO control */
+#define ENETC_MDIO_DATA 0x8 /* MDIO data */
+#define ENETC_MDIO_ADDR 0xc /* MDIO address */
+
+#define enetc_mdio_rd(hw, off) \
+ enetc_port_rd(hw, ENETC_##off + ENETC_MDIO_REG_OFFSET)
+#define enetc_mdio_wr(hw, off, val) \
+ enetc_port_wr(hw, ENETC_##off + ENETC_MDIO_REG_OFFSET, val)
+#define enetc_mdio_rd_reg(off) enetc_mdio_rd(hw, off)
+
+struct enetc_mdio_priv {
+ struct enetc_hw *hw;
};

-#define bus_to_enetc_regs(bus) (struct enetc_mdio_regs __iomem *)((bus)->priv)
-
-#define ENETC_MDIO_REG_OFFSET 0x1c00
#define ENETC_MDC_DIV 258

#define MDIO_CFG_CLKDIV(x) ((((x) >> 1) & 0xff) << 8)
@@ -33,18 +39,19 @@ struct enetc_mdio_regs {
#define MDIO_DATA(x) ((x) & 0xffff)

#define TIMEOUT 1000
-static int enetc_mdio_wait_complete(struct enetc_mdio_regs __iomem *regs)
+static int enetc_mdio_wait_complete(struct enetc_hw *hw)
{
u32 val;

- return readx_poll_timeout(enetc_rd_reg, &regs->mdio_cfg, val,
+ return readx_poll_timeout(enetc_mdio_rd_reg, MDIO_CFG, val,
!(val & MDIO_CFG_BSY), 10, 10 * TIMEOUT);
}

static int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
u16 value)
{
- struct enetc_mdio_regs __iomem *regs = bus_to_enetc_regs(bus);
+ struct enetc_mdio_priv *mdio_priv = bus->priv;
+ struct enetc_hw *hw = mdio_priv->hw;
u32 mdio_ctl, mdio_cfg;
u16 dev_addr;
int ret;
@@ -59,29 +66,29 @@ static int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
mdio_cfg &= ~MDIO_CFG_ENC45;
}

- enetc_wr_reg(&regs->mdio_cfg, mdio_cfg);
+ enetc_mdio_wr(hw, MDIO_CFG, mdio_cfg);

- ret = enetc_mdio_wait_complete(regs);
+ ret = enetc_mdio_wait_complete(hw);
if (ret)
return ret;

/* set port and dev addr */
mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
- enetc_wr_reg(&regs->mdio_ctl, mdio_ctl);
+ enetc_mdio_wr(hw, MDIO_CTL, mdio_ctl);

/* set the register address */
if (regnum & MII_ADDR_C45) {
- enetc_wr_reg(&regs->mdio_addr, regnum & 0xffff);
+ enetc_mdio_wr(hw, MDIO_ADDR, regnum & 0xffff);

- ret = enetc_mdio_wait_complete(regs);
+ ret = enetc_mdio_wait_complete(hw);
if (ret)
return ret;
}

/* write the value */
- enetc_wr_reg(&regs->mdio_data, MDIO_DATA(value));
+ enetc_mdio_wr(hw, MDIO_DATA, MDIO_DATA(value));

- ret = enetc_mdio_wait_complete(regs);
+ ret = enetc_mdio_wait_complete(hw);
if (ret)
return ret;

@@ -90,7 +97,8 @@ static int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum,

static int enetc_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
{
- struct enetc_mdio_regs __iomem *regs = bus_to_enetc_regs(bus);
+ struct enetc_mdio_priv *mdio_priv = bus->priv;
+ struct enetc_hw *hw = mdio_priv->hw;
u32 mdio_ctl, mdio_cfg;
u16 dev_addr, value;
int ret;
@@ -104,41 +112,41 @@ static int enetc_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
mdio_cfg &= ~MDIO_CFG_ENC45;
}

- enetc_wr_reg(&regs->mdio_cfg, mdio_cfg);
+ enetc_mdio_wr(hw, MDIO_CFG, mdio_cfg);

- ret = enetc_mdio_wait_complete(regs);
+ ret = enetc_mdio_wait_complete(hw);
if (ret)
return ret;

/* set port and device addr */
mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
- enetc_wr_reg(&regs->mdio_ctl, mdio_ctl);
+ enetc_mdio_wr(hw, MDIO_CTL, mdio_ctl);

/* set the register address */
if (regnum & MII_ADDR_C45) {
- enetc_wr_reg(&regs->mdio_addr, regnum & 0xffff);
+ enetc_mdio_wr(hw, MDIO_ADDR, regnum & 0xffff);

- ret = enetc_mdio_wait_complete(regs);
+ ret = enetc_mdio_wait_complete(hw);
if (ret)
return ret;
}

/* initiate the read */
- enetc_wr_reg(&regs->mdio_ctl, mdio_ctl | MDIO_CTL_READ);
+ enetc_mdio_wr(hw, MDIO_CTL, mdio_ctl | MDIO_CTL_READ);

- ret = enetc_mdio_wait_complete(regs);
+ ret = enetc_mdio_wait_complete(hw);
if (ret)
return ret;

/* return all Fs if nothing was there */
- if (enetc_rd_reg(&regs->mdio_cfg) & MDIO_CFG_RD_ER) {
+ if (enetc_mdio_rd(hw, MDIO_CFG) & MDIO_CFG_RD_ER) {
dev_dbg(&bus->dev,
"Error while reading PHY%d reg at %d.%hhu\n",
phy_id, dev_addr, regnum);
return 0xffff;
}

- value = enetc_rd_reg(&regs->mdio_data) & 0xffff;
+ value = enetc_mdio_rd(hw, MDIO_DATA) & 0xffff;

return value;
}
@@ -146,12 +154,12 @@ static int enetc_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
int enetc_mdio_probe(struct enetc_pf *pf)
{
struct device *dev = &pf->si->pdev->dev;
- struct enetc_mdio_regs __iomem *regs;
+ struct enetc_mdio_priv *mdio_priv;
struct device_node *np;
struct mii_bus *bus;
- int ret;
+ int err;

- bus = mdiobus_alloc_size(sizeof(regs));
+ bus = devm_mdiobus_alloc_size(dev, sizeof(*mdio_priv));
if (!bus)
return -ENOMEM;

@@ -159,41 +167,31 @@ int enetc_mdio_probe(struct enetc_pf *pf)
bus->read = enetc_mdio_read;
bus->write = enetc_mdio_write;
bus->parent = dev;
+ mdio_priv = bus->priv;
+ mdio_priv->hw = &pf->si->hw;
snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));

- /* store the enetc mdio base address for this bus */
- regs = pf->si->hw.port + ENETC_MDIO_REG_OFFSET;
- bus->priv = regs;
-
np = of_get_child_by_name(dev->of_node, "mdio");
if (!np) {
dev_err(dev, "MDIO node missing\n");
- ret = -EINVAL;
- goto err_registration;
+ return -EINVAL;
}

- ret = of_mdiobus_register(bus, np);
- if (ret) {
+ err = of_mdiobus_register(bus, np);
+ if (err) {
of_node_put(np);
dev_err(dev, "cannot register MDIO bus\n");
- goto err_registration;
+ return err;
}

of_node_put(np);
pf->mdio = bus;

return 0;
-
-err_registration:
- mdiobus_free(bus);
-
- return ret;
}

void enetc_mdio_remove(struct enetc_pf *pf)
{
- if (pf->mdio) {
+ if (pf->mdio)
mdiobus_unregister(pf->mdio);
- mdiobus_free(pf->mdio);
- }
}
--
2.17.1

2019-07-29 15:33:07

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v3 1/4] enetc: Clean up local mdio bus allocation

On Mon, Jul 29, 2019 at 01:03:44PM +0300, Claudiu Manoil wrote:
> What's needed is basically a pointer to the mdio registers.
> This is one way to store it inside bus->priv allocated space,
> without upsetting sparse.
> Reworked accessors to avoid __iomem casting.
> Used devm_* variant to further clean up the init error /
> remove paths.
>
> Fixes following sparse warning:
> warning: incorrect type in assignment (different address spaces)
> expected void *priv
> got struct enetc_mdio_regs [noderef] <asn:2>*[assigned] regs
>
> Fixes: ebfcb23d62ab ("enetc: Add ENETC PF level external MDIO support")
>
> Signed-off-by: Claudiu Manoil <[email protected]>


Thanks, much nicer.

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2019-07-29 16:43:50

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v3 3/4] dt-bindings: net: fsl: enetc: Add bindings for the central MDIO PCIe endpoint

On Mon, Jul 29, 2019 at 01:03:46PM +0300, Claudiu Manoil wrote:
> The on-chip PCIe root complex that integrates the ENETC ethernet
> controllers also integrates a PCIe endpoint for the MDIO controller
> providing for centralized control of the ENETC mdio bus.
> Add bindings for this "central" MDIO Integrated PCIe Endpoint.
>
> Signed-off-by: Claudiu Manoil <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2019-07-29 16:44:38

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v3 4/4] arm64: dts: fsl: ls1028a: Enable eth port1 on the ls1028a QDS board

On Mon, Jul 29, 2019 at 01:03:47PM +0300, Claudiu Manoil wrote:
> LS1028a has one Ethernet management interface. On the QDS board, the
> MDIO signals are multiplexed to either on-board AR8035 PHY device or
> to 4 PCIe slots allowing for SGMII cards.
> To enable the Ethernet ENETC Port 1, which can only be connected to a
> RGMII PHY, the multiplexer needs to be configured to route the MDIO to
> the AR8035 PHY. The MDIO/MDC routing is controlled by bits 7:4 of FPGA
> board config register 0x54, and value 0 selects the on-board RGMII PHY.
> The FPGA board config registers are accessible on the i2c bus, at address
> 0x66.
>
> The PF3 MDIO PCIe integrated endpoint device allows for centralized access
> to the MDIO bus. Add the corresponding devicetree node and set it to be
> the MDIO bus parent.
>
> Signed-off-by: Alex Marginean <[email protected]>
> Signed-off-by: Claudiu Manoil <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Andrew