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.
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 | 119 +++++++++++++++---
.../net/ethernet/freescale/enetc/enetc_pf.c | 5 +-
5 files changed, 190 insertions(+), 22 deletions(-)
--
2.17.1
Though it works, this is not how it should have been.
What's needed is a pointer to the mdio registers.
Store it properly inside bus->priv allocated space.
Use 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
.../net/ethernet/freescale/enetc/enetc_mdio.c | 31 +++++++------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
index 77b9cd10ba2b..1e3cd21c13ee 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
@@ -15,7 +15,8 @@ struct enetc_mdio_regs {
u32 mdio_addr; /* MDIO address */
};
-#define bus_to_enetc_regs(bus) (struct enetc_mdio_regs __iomem *)((bus)->priv)
+#define bus_to_enetc_regs(bus) (*(struct enetc_mdio_regs __iomem **) \
+ ((bus)->priv))
#define ENETC_MDIO_REG_OFFSET 0x1c00
#define ENETC_MDC_DIV 258
@@ -146,12 +147,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_regs __iomem **regsp;
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(*regsp));
if (!bus)
return -ENOMEM;
@@ -159,41 +160,33 @@ int enetc_mdio_probe(struct enetc_pf *pf)
bus->read = enetc_mdio_read;
bus->write = enetc_mdio_write;
bus->parent = dev;
+ regsp = bus->priv;
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;
+ *regsp = pf->si->hw.port + ENETC_MDIO_REG_OFFSET;
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
The on-chip PCIe root complex that integrates the ENETC ethernet
controllers also integrates a PCIe enpoint for the MDIO controller
provinding for cetralized control of the ENETC mdio bus.
Add bindings for this "central" MDIO Integrated PCIe Endpoit.
Signed-off-by: Claudiu Manoil <[email protected]>
---
v1 - none
.../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..c090f6df7a39 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 enpoint 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
ENETC ports can manage the MDIO bus via local register
interface. However there's also a centralized way
to manage the MDIO bus, via the MDIO PCIe endpoint
device integrated by the same root complex that also
integrates the ENETC ports (eth controllers).
Depending on board design and use case, centralized
access to MDIO may be better than using local ENETC
port registers. For instance, on the LS1028A QDS board
where MDIO muxing is requiered. Also, the LS1028A on-chip
switch doesn't have a local MDIO register interface.
The current patch registers the above PCIe enpoint as a
separate MDIO bus and provides a driver for it by re-using
the code used for local MDIO access. It also allows the
ENETC port PHYs to be managed by this driver if the local
"mdio" node is missing from the ENETC port node.
Signed-off-by: Claudiu Manoil <[email protected]>
---
v1 - fixed mdio bus allocation
- requested only BAR0 region, as it's the only one used by the driver
.../net/ethernet/freescale/enetc/enetc_mdio.c | 90 +++++++++++++++++++
.../net/ethernet/freescale/enetc/enetc_pf.c | 5 +-
2 files changed, 94 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
index 1e3cd21c13ee..378cc8dd27f9 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
@@ -190,3 +190,93 @@ void enetc_mdio_remove(struct enetc_pf *pf)
if (pf->mdio)
mdiobus_unregister(pf->mdio);
}
+
+#define ENETC_MDIO_DEV_ID 0xee01
+#define ENETC_MDIO_DEV_NAME "FSL PCIe IE Central MDIO"
+#define ENETC_MDIO_BUS_NAME ENETC_MDIO_DEV_NAME " Bus"
+#define ENETC_MDIO_DRV_NAME ENETC_MDIO_DEV_NAME " driver"
+#define ENETC_MDIO_DRV_ID "fsl_enetc_mdio"
+
+static int enetc_pci_mdio_probe(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
+{
+ struct enetc_mdio_regs __iomem **regsp;
+ struct device *dev = &pdev->dev;
+ struct mii_bus *bus;
+ int err;
+
+ bus = devm_mdiobus_alloc_size(dev, sizeof(*regsp));
+ if (!bus)
+ return -ENOMEM;
+
+ bus->name = ENETC_MDIO_BUS_NAME;
+ bus->read = enetc_mdio_read;
+ bus->write = enetc_mdio_write;
+ bus->parent = dev;
+ regsp = bus->priv;
+ snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
+
+ pcie_flr(pdev);
+ err = pci_enable_device_mem(pdev);
+ if (err) {
+ dev_err(dev, "device enable failed\n");
+ return err;
+ }
+
+ err = pci_request_region(pdev, 0, ENETC_MDIO_DRV_ID);
+ if (err) {
+ dev_err(dev, "pci_request_region failed\n");
+ goto err_pci_mem_reg;
+ }
+
+ *regsp = pci_iomap_range(pdev, 0, ENETC_MDIO_REG_OFFSET, 0);
+ if (!bus->priv) {
+ err = -ENXIO;
+ dev_err(dev, "iomap failed\n");
+ goto err_ioremap;
+ }
+
+ err = of_mdiobus_register(bus, dev->of_node);
+ if (err)
+ goto err_mdiobus_reg;
+
+ pci_set_drvdata(pdev, bus);
+
+ return 0;
+
+err_mdiobus_reg:
+ iounmap(*regsp);
+err_ioremap:
+ pci_release_mem_regions(pdev);
+err_pci_mem_reg:
+ pci_disable_device(pdev);
+
+ return err;
+}
+
+static void enetc_pci_mdio_remove(struct pci_dev *pdev)
+{
+ struct mii_bus *bus = pci_get_drvdata(pdev);
+
+ mdiobus_unregister(bus);
+ iounmap(bus_to_enetc_regs(bus));
+ pci_release_mem_regions(pdev);
+ pci_disable_device(pdev);
+}
+
+static const struct pci_device_id enetc_pci_mdio_id_table[] = {
+ { PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, ENETC_MDIO_DEV_ID) },
+ { 0, } /* End of table. */
+};
+MODULE_DEVICE_TABLE(pci, enetc_mdio_id_table);
+
+static struct pci_driver enetc_pci_mdio_driver = {
+ .name = ENETC_MDIO_DRV_ID,
+ .id_table = enetc_pci_mdio_id_table,
+ .probe = enetc_pci_mdio_probe,
+ .remove = enetc_pci_mdio_remove,
+};
+module_pci_driver(enetc_pci_mdio_driver);
+
+MODULE_DESCRIPTION(ENETC_MDIO_DRV_NAME);
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 258b3cb38a6f..7d6513ff8507 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -750,6 +750,7 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
{
struct enetc_pf *pf = enetc_si_priv(priv->si);
struct device_node *np = priv->dev->of_node;
+ struct device_node *mdio_np;
int err;
if (!np) {
@@ -773,7 +774,9 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
priv->phy_node = of_node_get(np);
}
- if (!of_phy_is_fixed_link(np)) {
+ mdio_np = of_get_child_by_name(np, "mdio");
+ if (mdio_np) {
+ of_node_put(mdio_np);
err = enetc_mdio_probe(pf);
if (err) {
of_node_put(priv->phy_node);
--
2.17.1
>-----Original Message-----
>From: Andrew Lunn <[email protected]>
>Sent: Wednesday, July 24, 2019 6:18 PM
>To: Claudiu Manoil <[email protected]>
>Cc: David S . Miller <[email protected]>; Rob Herring
><[email protected]>; Leo Li <[email protected]>; Alexandru Marginean
><[email protected]>; [email protected];
>[email protected]; [email protected]; linux-
>[email protected]
>Subject: Re: [PATCH net-next v1 1/4] enetc: Clean up local mdio bus allocation
>
>On Wed, Jul 24, 2019 at 05:41:38PM +0300, Claudiu Manoil wrote:
>> Though it works, this is not how it should have been.
>> What's needed is a pointer to the mdio registers.
>> Store it properly inside bus->priv allocated space.
>> Use 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
>>
>> .../net/ethernet/freescale/enetc/enetc_mdio.c | 31 +++++++------------
>> 1 file changed, 12 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
>b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
>> index 77b9cd10ba2b..1e3cd21c13ee 100644
>> --- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
>> +++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
>> @@ -15,7 +15,8 @@ struct enetc_mdio_regs {
>> u32 mdio_addr; /* MDIO address */
>> };
>>
>> -#define bus_to_enetc_regs(bus) (struct enetc_mdio_regs __iomem
>*)((bus)->priv)
>> +#define bus_to_enetc_regs(bus) (*(struct enetc_mdio_regs __iomem
>**) \
>> + ((bus)->priv))
>>
>> #define ENETC_MDIO_REG_OFFSET 0x1c00
>> #define ENETC_MDC_DIV 258
>> @@ -146,12 +147,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_regs __iomem **regsp;
>> 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(*regsp));
>> if (!bus)
>> return -ENOMEM;
>>
>> @@ -159,41 +160,33 @@ int enetc_mdio_probe(struct enetc_pf *pf)
>> bus->read = enetc_mdio_read;
>> bus->write = enetc_mdio_write;
>> bus->parent = dev;
>> + regsp = bus->priv;
>> 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;
>> + *regsp = pf->si->hw.port + ENETC_MDIO_REG_OFFSET;
>
>This is all very odd and different to every other driver.
>
>If i get the code write, there are 4 registers, each u32 in size,
>starting at pf->si->hw.port + ENETC_MDIO_REG_OFFSET?
>
>There are macros like enetc_port_wr() and enetc_global_wr(). It think
>it would be much cleaner to add a macro enet_mdio_wr() which takes
>hw, off, val.
>
>#define enet_mdio_wr(hw, off, val) enet_port_wr(hw, off +
>ENETC_MDIO_REG_OFFSET, val)
>
>struct enetc_mdio_priv {
> struct enetc_hw *hw;
>}
>
> struct enetc_mdio_priv *mdio_priv;
>
> bus = devm_mdiobus_alloc_size(dev, sizeof(*mdio_priv));
>
> mdio_priv = bus->priv;
> mdio_priv->hw = pf->si->hw;
>
>
>static int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
> u16 value)
>{
> struct enetc_mdio_priv *mdio_priv = bus->priv;
>...
> enet_mdio_wr(priv->hw, ENETC_MDIO_CFG, mdio_cfg);
>}
>
>All the horrible casts go away, the driver is structured like every
>other driver, sparse is probably happy, etc.
>
This looks more like a matter cosmetic preferences. I mean, I didn't
notice anything "horrible" in the code so far. I actually find it more
ugly to define a new structure with only one element inside, like:
struct enetc_mdio_priv {
struct enetc_hw *hw;
}
What is this technique called? Looks like a second type definition for
another type.
Anyway, if others already did this in the kernel, what can I do?
> >All the horrible casts go away, the driver is structured like every
> >other driver, sparse is probably happy, etc.
> >
>
> This looks more like a matter cosmetic preferences. I mean, I didn't
> notice anything "horrible" in the code so far.
#define bus_to_enetc_regs(bus) (struct enetc_mdio_regs __iomem *)((bus)->priv)
You should not need a cast here, bus->priv is a void *. But bus->priv
is being abused to hold a __iomem pointer.
enetc_wr_reg(®s->mdio_cfg, mdio_cfg);
This is also rather odd, passing the address of something to an IO
operator? I also don't know the C standard well enough to know if it
is guaranteed that:
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 */
};
actually works. On a 64bit system is the compiler allowed to put in
padding to keep the u32 64 bit aligned?
> I actually find it more
> ugly to define a new structure with only one element inside, like:
> struct enetc_mdio_priv {
> struct enetc_hw *hw;
> }
One advantage of this is that struct enetc_hw correctly has all the
__iomem attributes. All the casts to __iomem go away, and sparse is
happy.
> Anyway, if others already did this in the kernel, what can I do?
Clean it up. Make the code more readable and easy to maintain.
Andrew
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
.../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
On Wed, Jul 24, 2019 at 05:41:38PM +0300, Claudiu Manoil wrote:
> Though it works, this is not how it should have been.
> What's needed is a pointer to the mdio registers.
> Store it properly inside bus->priv allocated space.
> Use 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
>
> .../net/ethernet/freescale/enetc/enetc_mdio.c | 31 +++++++------------
> 1 file changed, 12 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
> index 77b9cd10ba2b..1e3cd21c13ee 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
> @@ -15,7 +15,8 @@ struct enetc_mdio_regs {
> u32 mdio_addr; /* MDIO address */
> };
>
> -#define bus_to_enetc_regs(bus) (struct enetc_mdio_regs __iomem *)((bus)->priv)
> +#define bus_to_enetc_regs(bus) (*(struct enetc_mdio_regs __iomem **) \
> + ((bus)->priv))
>
> #define ENETC_MDIO_REG_OFFSET 0x1c00
> #define ENETC_MDC_DIV 258
> @@ -146,12 +147,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_regs __iomem **regsp;
> 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(*regsp));
> if (!bus)
> return -ENOMEM;
>
> @@ -159,41 +160,33 @@ int enetc_mdio_probe(struct enetc_pf *pf)
> bus->read = enetc_mdio_read;
> bus->write = enetc_mdio_write;
> bus->parent = dev;
> + regsp = bus->priv;
> 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;
> + *regsp = pf->si->hw.port + ENETC_MDIO_REG_OFFSET;
This is all very odd and different to every other driver.
If i get the code write, there are 4 registers, each u32 in size,
starting at pf->si->hw.port + ENETC_MDIO_REG_OFFSET?
There are macros like enetc_port_wr() and enetc_global_wr(). It think
it would be much cleaner to add a macro enet_mdio_wr() which takes
hw, off, val.
#define enet_mdio_wr(hw, off, val) enet_port_wr(hw, off + ENETC_MDIO_REG_OFFSET, val)
struct enetc_mdio_priv {
struct enetc_hw *hw;
}
struct enetc_mdio_priv *mdio_priv;
bus = devm_mdiobus_alloc_size(dev, sizeof(*mdio_priv));
mdio_priv = bus->priv;
mdio_priv->hw = pf->si->hw;
static int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
u16 value)
{
struct enetc_mdio_priv *mdio_priv = bus->priv;
...
enet_mdio_wr(priv->hw, ENETC_MDIO_CFG, mdio_cfg);
}
All the horrible casts go away, the driver is structured like every
other driver, sparse is probably happy, etc.
Andrew