Re: [PATCH v2 3/5] drivers: net: phy: Add MDIO driver

[Iyappan Subramanian]

Hi Russell,

On Thu, Jul 7, 2016 at 7:14 AM, Russell King - ARM Linux
 wrote:
> On Thu, Jul 07, 2016 at 04:03:02PM +0200, Andrew Lunn wrote:
>> > Wed, Jul 06, 2016 at 04:44:44PM -0700, Iyappan Subramanian wrote:
>> > Hi Andrew,
>> >
>> > On Tue, Jul 5, 2016 at 6:49 AM, Andrew Lunn  wrote:
>> > > On Mon, Jun 06, 2016 at 10:12:35AM -0700, Iyappan Subramanian wrote:
>> > >> Hi Andrew,
>> > >>
>> > >> Thanks for the review.
>> > >>
>> > >> On Tue, May 31, 2016 at 6:11 PM, Andrew Lunn  wrote:
>> > >> > On Tue, May 31, 2016 at 05:10:38PM -0700, Iyappan Subramanian wrote:
>> > >> >> +static int xgene_mdio_reset(struct xgene_mdio_pdata *pdata)
>> > >> >> +{
>> > >> >> + int ret;
>> > >> >> +
>> > >> >> + if (pdata->mdio_id == XGENE_MDIO_RGMII) {
>> > >> >> + if (pdata->dev->of_node) {
>> > >> >> + clk_prepare_enable(pdata->clk);
>> > >> >> + clk_disable_unprepare(pdata->clk);
>> > >> >> + clk_prepare_enable(pdata->clk);
>> > >> >
>> > >> > Hi Iyappan
>> > >> >
>> > >> > Is that a workaround for a hardware problem? If so, i would suggest
>> > >> > adding a comment, to stop people submitting a patch simplifying it.
>> > >>
>> > >> Hardware expects this clock sequence.  I'll add comment as you 
>> > >> suggested.
>> > >
>> > > What exactly does the hardware require? Is this a workaround for a bug
>> > > in the clock generator? Or a workaround for a bug in the MDIO device?
>> >
>> > Hardware requires a clock pulse.  There is no bug.
>>
>> And how are you guaranteeing a pulse? You enable/disable/enable
>> without any sleeps, so it could all happen within a single clock
>> cycle?
>
> It's still really not good, because it does this:
>
> - prepare + enable
> - disable + unprepare
> - prepare + enable
>
> So every time the reset function is called, the prepare and enable
> counts against the clock get incremented.  That's a bug.  This needs
> a better solution.

I'll fix this by adding disable + unprepare on the port shutdown and
resubmit the patches.

>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

Re: [PATCH v2 3/5] drivers: net: phy: Add MDIO driver

[Iyappan Subramanian]

Hi Andrew,

On Thu, Jul 7, 2016 at 7:03 AM, Andrew Lunn  wrote:
>> Wed, Jul 06, 2016 at 04:44:44PM -0700, Iyappan Subramanian wrote:
>> Hi Andrew,
>>
>> On Tue, Jul 5, 2016 at 6:49 AM, Andrew Lunn  wrote:
>> > On Mon, Jun 06, 2016 at 10:12:35AM -0700, Iyappan Subramanian wrote:
>> >> Hi Andrew,
>> >>
>> >> Thanks for the review.
>> >>
>> >> On Tue, May 31, 2016 at 6:11 PM, Andrew Lunn  wrote:
>> >> > On Tue, May 31, 2016 at 05:10:38PM -0700, Iyappan Subramanian wrote:
>> >> >> +static int xgene_mdio_reset(struct xgene_mdio_pdata *pdata)
>> >> >> +{
>> >> >> + int ret;
>> >> >> +
>> >> >> + if (pdata->mdio_id == XGENE_MDIO_RGMII) {
>> >> >> + if (pdata->dev->of_node) {
>> >> >> + clk_prepare_enable(pdata->clk);
>> >> >> + clk_disable_unprepare(pdata->clk);
>> >> >> + clk_prepare_enable(pdata->clk);
>> >> >
>> >> > Hi Iyappan
>> >> >
>> >> > Is that a workaround for a hardware problem? If so, i would suggest
>> >> > adding a comment, to stop people submitting a patch simplifying it.
>> >>
>> >> Hardware expects this clock sequence.  I'll add comment as you suggested.
>> >
>> > What exactly does the hardware require? Is this a workaround for a bug
>> > in the clock generator? Or a workaround for a bug in the MDIO device?
>>
>> Hardware requires a clock pulse.  There is no bug.
>
> And how are you guaranteeing a pulse? You enable/disable/enable
> without any sleeps, so it could all happen within a single clock
> cycle?

I'll add delay to generate the pulse and resubmit the patch.

>
> Andrew

Re: [PATCH v2 3/5] drivers: net: phy: Add MDIO driver

[Russell King - ARM Linux]

On Thu, Jul 07, 2016 at 04:03:02PM +0200, Andrew Lunn wrote:
> > Wed, Jul 06, 2016 at 04:44:44PM -0700, Iyappan Subramanian wrote:
> > Hi Andrew,
> > 
> > On Tue, Jul 5, 2016 at 6:49 AM, Andrew Lunn  wrote:
> > > On Mon, Jun 06, 2016 at 10:12:35AM -0700, Iyappan Subramanian wrote:
> > >> Hi Andrew,
> > >>
> > >> Thanks for the review.
> > >>
> > >> On Tue, May 31, 2016 at 6:11 PM, Andrew Lunn  wrote:
> > >> > On Tue, May 31, 2016 at 05:10:38PM -0700, Iyappan Subramanian wrote:
> > >> >> +static int xgene_mdio_reset(struct xgene_mdio_pdata *pdata)
> > >> >> +{
> > >> >> + int ret;
> > >> >> +
> > >> >> + if (pdata->mdio_id == XGENE_MDIO_RGMII) {
> > >> >> + if (pdata->dev->of_node) {
> > >> >> + clk_prepare_enable(pdata->clk);
> > >> >> + clk_disable_unprepare(pdata->clk);
> > >> >> + clk_prepare_enable(pdata->clk);
> > >> >
> > >> > Hi Iyappan
> > >> >
> > >> > Is that a workaround for a hardware problem? If so, i would suggest
> > >> > adding a comment, to stop people submitting a patch simplifying it.
> > >>
> > >> Hardware expects this clock sequence.  I'll add comment as you suggested.
> > >
> > > What exactly does the hardware require? Is this a workaround for a bug
> > > in the clock generator? Or a workaround for a bug in the MDIO device?
> > 
> > Hardware requires a clock pulse.  There is no bug.
> 
> And how are you guaranteeing a pulse? You enable/disable/enable
> without any sleeps, so it could all happen within a single clock
> cycle?

It's still really not good, because it does this:

- prepare + enable
- disable + unprepare
- prepare + enable

So every time the reset function is called, the prepare and enable
counts against the clock get incremented.  That's a bug.  This needs
a better solution.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v2 3/5] drivers: net: phy: Add MDIO driver

[Andrew Lunn]

> Wed, Jul 06, 2016 at 04:44:44PM -0700, Iyappan Subramanian wrote:
> Hi Andrew,
> 
> On Tue, Jul 5, 2016 at 6:49 AM, Andrew Lunn  wrote:
> > On Mon, Jun 06, 2016 at 10:12:35AM -0700, Iyappan Subramanian wrote:
> >> Hi Andrew,
> >>
> >> Thanks for the review.
> >>
> >> On Tue, May 31, 2016 at 6:11 PM, Andrew Lunn  wrote:
> >> > On Tue, May 31, 2016 at 05:10:38PM -0700, Iyappan Subramanian wrote:
> >> >> +static int xgene_mdio_reset(struct xgene_mdio_pdata *pdata)
> >> >> +{
> >> >> + int ret;
> >> >> +
> >> >> + if (pdata->mdio_id == XGENE_MDIO_RGMII) {
> >> >> + if (pdata->dev->of_node) {
> >> >> + clk_prepare_enable(pdata->clk);
> >> >> + clk_disable_unprepare(pdata->clk);
> >> >> + clk_prepare_enable(pdata->clk);
> >> >
> >> > Hi Iyappan
> >> >
> >> > Is that a workaround for a hardware problem? If so, i would suggest
> >> > adding a comment, to stop people submitting a patch simplifying it.
> >>
> >> Hardware expects this clock sequence.  I'll add comment as you suggested.
> >
> > What exactly does the hardware require? Is this a workaround for a bug
> > in the clock generator? Or a workaround for a bug in the MDIO device?
> 
> Hardware requires a clock pulse.  There is no bug.

And how are you guaranteeing a pulse? You enable/disable/enable
without any sleeps, so it could all happen within a single clock
cycle?

Andrew

Re: [PATCH v2 3/5] drivers: net: phy: Add MDIO driver

[Iyappan Subramanian]

Hi Andrew,

On Tue, Jul 5, 2016 at 6:49 AM, Andrew Lunn  wrote:
> On Mon, Jun 06, 2016 at 10:12:35AM -0700, Iyappan Subramanian wrote:
>> Hi Andrew,
>>
>> Thanks for the review.
>>
>> On Tue, May 31, 2016 at 6:11 PM, Andrew Lunn  wrote:
>> > On Tue, May 31, 2016 at 05:10:38PM -0700, Iyappan Subramanian wrote:
>> >> +static int xgene_mdio_reset(struct xgene_mdio_pdata *pdata)
>> >> +{
>> >> + int ret;
>> >> +
>> >> + if (pdata->mdio_id == XGENE_MDIO_RGMII) {
>> >> + if (pdata->dev->of_node) {
>> >> + clk_prepare_enable(pdata->clk);
>> >> + clk_disable_unprepare(pdata->clk);
>> >> + clk_prepare_enable(pdata->clk);
>> >
>> > Hi Iyappan
>> >
>> > Is that a workaround for a hardware problem? If so, i would suggest
>> > adding a comment, to stop people submitting a patch simplifying it.
>>
>> Hardware expects this clock sequence.  I'll add comment as you suggested.
>
> What exactly does the hardware require? Is this a workaround for a bug
> in the clock generator? Or a workaround for a bug in the MDIO device?

Hardware requires a clock pulse.  There is no bug.

>
> If it is a clock generator bug, i think the fix should be in the clock
> driver. If this is bug in the MDIO device, do you need a sleep in
> there, so you actually have the clock ticking/not ticking for a time?
> Since these calls can all sleep, you have no idea how long/sort the
> clock will be enabled/disabled.
>
>   Andrew

Re: [PATCH v2 3/5] drivers: net: phy: Add MDIO driver

[Andrew Lunn]

On Mon, Jun 06, 2016 at 10:12:35AM -0700, Iyappan Subramanian wrote:
> Hi Andrew,
> 
> Thanks for the review.
> 
> On Tue, May 31, 2016 at 6:11 PM, Andrew Lunn  wrote:
> > On Tue, May 31, 2016 at 05:10:38PM -0700, Iyappan Subramanian wrote:
> >> +static int xgene_mdio_reset(struct xgene_mdio_pdata *pdata)
> >> +{
> >> + int ret;
> >> +
> >> + if (pdata->mdio_id == XGENE_MDIO_RGMII) {
> >> + if (pdata->dev->of_node) {
> >> + clk_prepare_enable(pdata->clk);
> >> + clk_disable_unprepare(pdata->clk);
> >> + clk_prepare_enable(pdata->clk);
> >
> > Hi Iyappan
> >
> > Is that a workaround for a hardware problem? If so, i would suggest
> > adding a comment, to stop people submitting a patch simplifying it.
> 
> Hardware expects this clock sequence.  I'll add comment as you suggested.

What exactly does the hardware require? Is this a workaround for a bug
in the clock generator? Or a workaround for a bug in the MDIO device?

If it is a clock generator bug, i think the fix should be in the clock
driver. If this is bug in the MDIO device, do you need a sleep in
there, so you actually have the clock ticking/not ticking for a time?
Since these calls can all sleep, you have no idea how long/sort the
clock will be enabled/disabled.

  Andrew

Re: [PATCH v2 3/5] drivers: net: phy: Add MDIO driver

[Iyappan Subramanian]

Hi Andrew,

Thanks for the review.

On Tue, May 31, 2016 at 6:11 PM, Andrew Lunn  wrote:
> On Tue, May 31, 2016 at 05:10:38PM -0700, Iyappan Subramanian wrote:
>> +static int xgene_mdio_reset(struct xgene_mdio_pdata *pdata)
>> +{
>> + int ret;
>> +
>> + if (pdata->mdio_id == XGENE_MDIO_RGMII) {
>> + if (pdata->dev->of_node) {
>> + clk_prepare_enable(pdata->clk);
>> + clk_disable_unprepare(pdata->clk);
>> + clk_prepare_enable(pdata->clk);
>
> Hi Iyappan
>
> Is that a workaround for a hardware problem? If so, i would suggest
> adding a comment, to stop people submitting a patch simplifying it.

Hardware expects this clock sequence.  I'll add comment as you suggested.

>
>
>> +static int xgene_mdio_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = >dev;
>> + struct mii_bus *mdio_bus;
>> + const struct of_device_id *of_id;
>> + struct resource *res;
>> + struct xgene_mdio_pdata *pdata;
>> + void __iomem *csr_addr;
>> + int mdio_id = 0, ret = 0;
>> +
>
>
>> + of_id = of_match_device(xgene_mdio_of_match, >dev);
>> + if (mdio_id == XGENE_MDIO_RGMII) {
>> + mdio_bus->read = xgene_mdio_rgmii_read;
>> + mdio_bus->write = xgene_mdio_rgmii_write;
>> + } else {
>> + mdio_bus->read = xgene_xfi_mdio_read;
>> + mdio_bus->write = xgene_xfi_mdio_write;
>> + }
>
>> +static const struct of_device_id xgene_mdio_of_match[] = {
>> + {
>> + .compatible = "apm,xgene-mdio-rgmii",
>> + .data = (void *)XGENE_MDIO_RGMII
>> + },
>> + {
>> + .compatible = "apm,xgene-mdio-xfi",
>> + .data = (void *)XGENE_MDIO_XFI},
>> + {},
>> +};
>
>
> This all makes me think you should have two separate MDIO drivers, one
> for each compatible string. There is not that much shared code.

I would like to keep the driver consistent with the ethernet driver.
Only the mdio read and write functions are hardware specific, and that
too implemented using function pointers.  Other parts of the code are
shared and much cleaner that way.

>
> Andrew

Re: [PATCH v2 3/5] drivers: net: phy: Add MDIO driver

[Iyappan Subramanian]

Hi Andrew,

Thanks for the review.

On Tue, May 31, 2016 at 6:11 PM, Andrew Lunn  wrote:
> On Tue, May 31, 2016 at 05:10:38PM -0700, Iyappan Subramanian wrote:
>> +static int xgene_mdio_reset(struct xgene_mdio_pdata *pdata)
>> +{
>> + int ret;
>> +
>> + if (pdata->mdio_id == XGENE_MDIO_RGMII) {
>> + if (pdata->dev->of_node) {
>> + clk_prepare_enable(pdata->clk);
>> + clk_disable_unprepare(pdata->clk);
>> + clk_prepare_enable(pdata->clk);
>
> Hi Iyappan
>
> Is that a workaround for a hardware problem? If so, i would suggest
> adding a comment, to stop people submitting a patch simplifying it.

Hardware expects this clock sequence.  I'll add comment as you suggested.

>
>
>> +static int xgene_mdio_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = >dev;
>> + struct mii_bus *mdio_bus;
>> + const struct of_device_id *of_id;
>> + struct resource *res;
>> + struct xgene_mdio_pdata *pdata;
>> + void __iomem *csr_addr;
>> + int mdio_id = 0, ret = 0;
>> +
>
>
>> + of_id = of_match_device(xgene_mdio_of_match, >dev);
>> + if (mdio_id == XGENE_MDIO_RGMII) {
>> + mdio_bus->read = xgene_mdio_rgmii_read;
>> + mdio_bus->write = xgene_mdio_rgmii_write;
>> + } else {
>> + mdio_bus->read = xgene_xfi_mdio_read;
>> + mdio_bus->write = xgene_xfi_mdio_write;
>> + }
>
>> +static const struct of_device_id xgene_mdio_of_match[] = {
>> + {
>> + .compatible = "apm,xgene-mdio-rgmii",
>> + .data = (void *)XGENE_MDIO_RGMII
>> + },
>> + {
>> + .compatible = "apm,xgene-mdio-xfi",
>> + .data = (void *)XGENE_MDIO_XFI},
>> + {},
>> +};
>
>
> This all makes me think you should have two separate MDIO drivers, one
> for each compatible string. There is not that much shared code.

I would like to keep the driver consistent with the ethernet driver.
Only the mdio read and write functions are hardware specific, and that
too implemented using function pointers.  Other parts of the code are
shared and much cleaner that way.

>
> Andrew

Re: [PATCH v2 3/5] drivers: net: phy: Add MDIO driver

[Andrew Lunn]

On Tue, May 31, 2016 at 05:10:38PM -0700, Iyappan Subramanian wrote:
> +static int xgene_mdio_reset(struct xgene_mdio_pdata *pdata)
> +{
> + int ret;
> +
> + if (pdata->mdio_id == XGENE_MDIO_RGMII) {
> + if (pdata->dev->of_node) {
> + clk_prepare_enable(pdata->clk);
> + clk_disable_unprepare(pdata->clk);
> + clk_prepare_enable(pdata->clk);

Hi Iyappan

Is that a workaround for a hardware problem? If so, i would suggest
adding a comment, to stop people submitting a patch simplifying it.


> +static int xgene_mdio_probe(struct platform_device *pdev)
> +{
> + struct device *dev = >dev;
> + struct mii_bus *mdio_bus;
> + const struct of_device_id *of_id;
> + struct resource *res;
> + struct xgene_mdio_pdata *pdata;
> + void __iomem *csr_addr;
> + int mdio_id = 0, ret = 0;
> +


> + of_id = of_match_device(xgene_mdio_of_match, >dev);
> + if (mdio_id == XGENE_MDIO_RGMII) {
> + mdio_bus->read = xgene_mdio_rgmii_read;
> + mdio_bus->write = xgene_mdio_rgmii_write;
> + } else {
> + mdio_bus->read = xgene_xfi_mdio_read;
> + mdio_bus->write = xgene_xfi_mdio_write;
> + }

> +static const struct of_device_id xgene_mdio_of_match[] = {
> + {
> + .compatible = "apm,xgene-mdio-rgmii",
> + .data = (void *)XGENE_MDIO_RGMII
> + },
> + {
> + .compatible = "apm,xgene-mdio-xfi",
> + .data = (void *)XGENE_MDIO_XFI},
> + {},
> +};


This all makes me think you should have two separate MDIO drivers, one
for each compatible string. There is not that much shared code.

Andrew

[PATCH v2 3/5] drivers: net: phy: Add MDIO driver

[Iyappan Subramanian]

Currently, SGMII based 1G rely on the hardware registers for link state
and sometimes it's not reliable.  To get most accurate link state, this
interface has to use the MDIO bus to poll the PHY.

In X-Gene SoC, MDIO bus is shared across RGMII and SGMII based 1G
interfaces, so adding this driver to manage MDIO bus.  This driver
registers the mdio bus and registers the PHYs connected to it.

Signed-off-by: Iyappan Subramanian 
Tested-by: Fushen Chen 
Tested-by: Toan Le 
Tested-by: Matthias Brugger 
---
 drivers/net/ethernet/apm/xgene/Kconfig |   1 +
 drivers/net/phy/Kconfig|   7 +
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/mdio-xgene.c   | 531 +
 drivers/net/phy/mdio-xgene.h   | 140 +
 5 files changed, 680 insertions(+)
 create mode 100644 drivers/net/phy/mdio-xgene.c
 create mode 100644 drivers/net/phy/mdio-xgene.h

diff --git a/drivers/net/ethernet/apm/xgene/Kconfig 
b/drivers/net/ethernet/apm/xgene/Kconfig
index 19e38af..300e3b5 100644
--- a/drivers/net/ethernet/apm/xgene/Kconfig
+++ b/drivers/net/ethernet/apm/xgene/Kconfig
@@ -3,6 +3,7 @@ config NET_XGENE
depends on HAS_DMA
depends on ARCH_XGENE || COMPILE_TEST
select PHYLIB
+   select MDIO_XGENE
help
  This is the Ethernet driver for the on-chip ethernet interface on the
  APM X-Gene SoC.
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 6dad9a9..193f418 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -271,6 +271,13 @@ config MDIO_BCM_IPROC
  This module provides a driver for the MDIO busses found in the
  Broadcom iProc SoC's.
 
+config MDIO_XGENE
+   tristate "APM X-Gene SoC MDIO bus controller"
+   help
+ This module provides a driver for the MDIO busses found in the
+ APM X-Gene SoC's.
+
+
 endif # PHYLIB
 
 config MICREL_KS8995MA
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index fcdbb92..9cbd2af 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_XGENE)   += mdio-xgene.o
diff --git a/drivers/net/phy/mdio-xgene.c b/drivers/net/phy/mdio-xgene.c
new file mode 100644
index 000..bd07fd9
--- /dev/null
+++ b/drivers/net/phy/mdio-xgene.c
@@ -0,0 +1,531 @@
+/* Applied Micro X-Gene SoC MDIO Driver
+ *
+ * Copyright (c) 2016, Applied Micro Circuits Corporation
+ * Author: Iyappan Subramanian 
+ *
+ * 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;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "mdio-xgene.h"
+
+static bool xgene_mdio_status;
+
+static bool xgene_enet_rd_indirect(void __iomem *addr, void __iomem *rd,
+  void __iomem *cmd, void __iomem *cmd_done,
+  u32 rd_addr, u32 *rd_data)
+{
+   u32 done;
+   u8 wait = 10;
+
+   iowrite32(rd_addr, addr);
+   iowrite32(XGENE_ENET_RD_CMD, cmd);
+
+   /* wait for read command to complete */
+   while (!(done = ioread32(cmd_done)) && wait--)
+   udelay(1);
+
+   if (!done)
+   return false;
+
+   *rd_data = ioread32(rd);
+   iowrite32(0, cmd);
+
+   return true;
+}
+
+static void xgene_enet_rd_mcx_mac(struct xgene_mdio_pdata *pdata,
+ u32 rd_addr, u32 *rd_data)
+{
+   void __iomem *addr, *rd, *cmd, *cmd_done;
+
+   addr = pdata->mac_csr_addr + MAC_ADDR_REG_OFFSET;
+   rd = pdata->mac_csr_addr + MAC_READ_REG_OFFSET;
+   cmd = pdata->mac_csr_addr + MAC_COMMAND_REG_OFFSET;
+   cmd_done = pdata->mac_csr_addr + MAC_COMMAND_DONE_REG_OFFSET;
+
+   if (!xgene_enet_rd_indirect(addr, rd, cmd, cmd_done, rd_addr, rd_data))
+   dev_err(pdata->dev, "MCX mac read failed, addr: 0x%04x\n",
+   rd_addr);
+}
+
+static bool xgene_enet_wr_indirect(void __iomem *addr, void __iomem *wr,
+