diff mbox series

[net-next,09/16] net: mdio: Add Synopsys DW XPCS management interface support

Message ID 20231205103559.9605-10-fancer.lancer@gmail.com
State New
Headers show
Series net: pcs: xpcs: Add memory-based management iface support | expand

Commit Message

Serge Semin Dec. 5, 2023, 10:35 a.m. UTC
Synopsys DesignWare XPCS IP-core can be synthesized with the device CSRs
being accessible over MCI or APB3 interface instead of the MDIO bus (see
the CSR_INTERFACE HDL parameter). Thus all the PCS registers can be just
memory mapped and be a subject of standard MMIO operations of course
taking into account the way the Clause C45 CSRs mapping is defined. This
commit is about adding a device driver for the DW XPCS Management
Interface platform device and registering it in the framework of the
kernel MDIO subsystem.

DW XPCS platform device is supposed to be described by the respective
compatible string "snps,dw-xpcs-mi", CSRs memory space and optional
peripheral bus clock source. Note depending on the INDIRECT_ACCESS DW XPCS
IP-core synthesize parameter the memory-mapped reg-space can be
represented as either directly or indirectly mapped Clause 45 space. In
the former case the particular address is determined based on the MMD
device and the registers offset (5 + 16 bits all together) within the
device reg-space. In the later case there is only 256 lower address bits
are utilized for the registers mapping. The upper bits are supposed to be
written into the respective viewport CSR in order to reach the entire C45
space.

Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
 drivers/net/mdio/Kconfig        |   8 +
 drivers/net/mdio/Makefile       |   1 +
 drivers/net/mdio/mdio-dw-xpcs.c | 384 ++++++++++++++++++++++++++++++++
 3 files changed, 393 insertions(+)
 create mode 100644 drivers/net/mdio/mdio-dw-xpcs.c

Comments

Maxime Chevallier Dec. 5, 2023, 12:32 p.m. UTC | #1
Hi Serge,

On Tue,  5 Dec 2023 13:35:30 +0300
Serge Semin <fancer.lancer@gmail.com> wrote:

> Synopsys DesignWare XPCS IP-core can be synthesized with the device CSRs
> being accessible over MCI or APB3 interface instead of the MDIO bus (see
> the CSR_INTERFACE HDL parameter). Thus all the PCS registers can be just
> memory mapped and be a subject of standard MMIO operations of course
> taking into account the way the Clause C45 CSRs mapping is defined. This
> commit is about adding a device driver for the DW XPCS Management
> Interface platform device and registering it in the framework of the
> kernel MDIO subsystem.
> 
> DW XPCS platform device is supposed to be described by the respective
> compatible string "snps,dw-xpcs-mi", CSRs memory space and optional
> peripheral bus clock source. Note depending on the INDIRECT_ACCESS DW XPCS
> IP-core synthesize parameter the memory-mapped reg-space can be
> represented as either directly or indirectly mapped Clause 45 space. In
> the former case the particular address is determined based on the MMD
> device and the registers offset (5 + 16 bits all together) within the
> device reg-space. In the later case there is only 256 lower address bits
> are utilized for the registers mapping. The upper bits are supposed to be
> written into the respective viewport CSR in order to reach the entire C45
> space.

Too bad the mdio-regmap driver can't be re-used here, it would deal
with reg width for you, for example. I guess the main reason would be
the direct vs indirect accesses ?

I do have a comment tough :

[...]

> +static inline ptrdiff_t dw_xpcs_mmio_addr_format(int dev, int reg)
> +{
> +	return FIELD_PREP(0x1f0000, dev) | FIELD_PREP(0xffff, reg);
> +}
> +
> +static inline u16 dw_xpcs_mmio_addr_page(ptrdiff_t csr)
> +{
> +	return FIELD_GET(0x1fff00, csr);
> +}
> +
> +static inline ptrdiff_t dw_xpcs_mmio_addr_offset(ptrdiff_t csr)
> +{
> +	return FIELD_GET(0xff, csr);
> +}

You shouldn't use inline in C files, only in headers.

Maxime
Serge Semin Dec. 6, 2023, 4:48 p.m. UTC | #2
Hi Maxime,

On Tue, Dec 05, 2023 at 01:32:05PM +0100, Maxime Chevallier wrote:
> Hi Serge,
> 
> On Tue,  5 Dec 2023 13:35:30 +0300
> Serge Semin <fancer.lancer@gmail.com> wrote:
> 
> > Synopsys DesignWare XPCS IP-core can be synthesized with the device CSRs
> > being accessible over MCI or APB3 interface instead of the MDIO bus (see
> > the CSR_INTERFACE HDL parameter). Thus all the PCS registers can be just
> > memory mapped and be a subject of standard MMIO operations of course
> > taking into account the way the Clause C45 CSRs mapping is defined. This
> > commit is about adding a device driver for the DW XPCS Management
> > Interface platform device and registering it in the framework of the
> > kernel MDIO subsystem.
> > 
> > DW XPCS platform device is supposed to be described by the respective
> > compatible string "snps,dw-xpcs-mi", CSRs memory space and optional
> > peripheral bus clock source. Note depending on the INDIRECT_ACCESS DW XPCS
> > IP-core synthesize parameter the memory-mapped reg-space can be
> > represented as either directly or indirectly mapped Clause 45 space. In
> > the former case the particular address is determined based on the MMD
> > device and the registers offset (5 + 16 bits all together) within the
> > device reg-space. In the later case there is only 256 lower address bits
> > are utilized for the registers mapping. The upper bits are supposed to be
> > written into the respective viewport CSR in order to reach the entire C45
> > space.
> 

> Too bad the mdio-regmap driver can't be re-used here, it would deal
> with reg width for you, for example. I guess the main reason would be
> the direct vs indirect accesses ?

Right, it's one of the reasons. I could have used the mdio-regmap
here, but that would have meant to implement an additional abstraction
layer: regspace<->regmap<->mdio-regmap<->mii_bus, instead of just
regspace<->mii_bus. This would have also required to patch the
mdio-remap module somehow in order to have the c45 ops supported. It
would have been much easier to do before the commit 99d5fe9c7f3d ("net:
mdio: Remove support for building C45 muxed addresses"). But since
then MDIO reg-address has no longer had muxed dev-address. Of course I
could have got it back to the mdio-regmap driver only, mix the C22/C45
address in the regmap 'addr' argument, then extract the MMD (for C45)
and reg addresses from the regmap IO ops argument and perform the
respective MMIO access. But as you can see it is much hardware and
would cause additional steps for the address translations, than
just directly implement the C22/C45 IO ops and register the MDIO bus
for them. I couldn't find much benefits to follow that road, sorry.

> 
> I do have a comment tough :
> 
> [...]
> 
> > +static inline ptrdiff_t dw_xpcs_mmio_addr_format(int dev, int reg)
> > +{
> > +	return FIELD_PREP(0x1f0000, dev) | FIELD_PREP(0xffff, reg);
> > +}
> > +
> > +static inline u16 dw_xpcs_mmio_addr_page(ptrdiff_t csr)
> > +{
> > +	return FIELD_GET(0x1fff00, csr);
> > +}
> > +
> > +static inline ptrdiff_t dw_xpcs_mmio_addr_offset(ptrdiff_t csr)
> > +{
> > +	return FIELD_GET(0xff, csr);
> > +}
> 

> You shouldn't use inline in C files, only in headers.

Could you please clarify why? I failed to find such requirement in the
coding style doc. Moreover there are multiple examples of using the
static-inline-ers in the C files in the kernel including the net/mdio
subsystem.

-Serge(y)

> 
> Maxime
Andrew Lunn Dec. 6, 2023, 5:01 p.m. UTC | #3
> > You shouldn't use inline in C files, only in headers.
> 
> Could you please clarify why? I failed to find such requirement in the
> coding style doc. Moreover there are multiple examples of using the
> static-inline-ers in the C files in the kernel including the net/mdio
> subsystem.

The compiler does a better job at deciding what to inline than we
humans do. If you can show the compiler is doing it wrong, then we
might accept them. But in general, netdev does not like inline in .C
file. Also, nothing in MDIO is hot path, it spends a lot of time
waiting for a slow bus. So inline is likely to just bloat the code for
no gain.

   Andrew
Serge Semin Dec. 7, 2023, 1:35 p.m. UTC | #4
Hi Andrew

On Wed, Dec 06, 2023 at 06:01:30PM +0100, Andrew Lunn wrote:
> > > You shouldn't use inline in C files, only in headers.
> > 
> > Could you please clarify why? I failed to find such requirement in the
> > coding style doc. Moreover there are multiple examples of using the
> > static-inline-ers in the C files in the kernel including the net/mdio
> > subsystem.
> 

> The compiler does a better job at deciding what to inline than we
> humans do. If you can show the compiler is doing it wrong, then we
> might accept them.

In general I would have agreed with you especially if the methods were
heavier than what they are:
static inline ptrdiff_t dw_xpcs_mmio_addr_format(int dev, int reg)
{               
        return FIELD_PREP(0x1f0000, dev) | FIELD_PREP(0xffff, reg);
}               
        
static inline u16 dw_xpcs_mmio_addr_page(ptrdiff_t csr)
{       
        return FIELD_GET(0x1fff00, csr);
}       

static inline ptrdiff_t dw_xpcs_mmio_addr_offset(ptrdiff_t csr)
{
        return FIELD_GET(0xff, csr);
}

> But in general, netdev does not like inline in .C
> file.

I see. I'll do as you say if you don't change your mind after my
reasoning below.

> Also, nothing in MDIO is hot path, it spends a lot of time
> waiting for a slow bus. So inline is likely to just bloat the code for
> no gain.

I would have been absolutely with you in this matter, if we were talking
about a normal MDIO bus. In this case the devices are accessed over
the system IO-memory. So the bus isn't that slow.

Regarding the compiler knowing better when to inline the code. Here is
what it does with the methods above. If the inline keyword is
specified the compiler will inline all three methods. If the keyword isn't
specified then dw_xpcs_mmio_addr_format() won't be inlined while the rest
two functions will be. So the only part at consideration is the
dw_xpcs_mmio_addr_format() method since the rest of the functions are
inlined anyway.

The dw_xpcs_mmio_addr_format() function body is of the 5 asm
instructions length (on MIPS). Since the function call in this case
requires two jump instructions (to function and back), one instruction
to save the previous return address on stack and two instructions for
the function arguments, the trade-off of having non-inlined function
are those five additional instructions on each call. There are four
dw_xpcs_mmio_addr_format() calls. So here is what we get in both
cases:
inlined:     5 func ins * 4 calls = 20 ins
non-inlined: (5 func + 1 jump) ins + (1 jump + 1 ra + 2 arg) ins * 4 calls  = 22 ins
but seeing the return address needs to be saved anyway in the callers
here is what we finally get:
non-inlined: (5 func + 1 jump) ins + (1 jump + 2 arg) ins * 4 calls  = 18 ins

So unless I am mistaken in some of the aspects if we have the function
non-inlined then we'll save 2 instructions in the object file, but
each call would require additional _4_ instructions to execute (2
jumps and 2 arg creations), which makes the function execution almost
two times longer than it would have been should it was inlined. IMO in
this case saving 2 instructions of the object file isn't worth than
getting rid from four instructions on each call seeing the DW XPCS
MCI/APB3 buses are the memory IO interfaces which don't require any
long-time waits to perform the ops. Thus I'd suggest to keep the
inline keywords specified here.

What is your conclusion?

-Serge(y)

> 
>    Andrew
Russell King (Oracle) Dec. 7, 2023, 2:02 p.m. UTC | #5
On Thu, Dec 07, 2023 at 04:35:47PM +0300, Serge Semin wrote:
> Hi Andrew
> 
> On Wed, Dec 06, 2023 at 06:01:30PM +0100, Andrew Lunn wrote:
> > The compiler does a better job at deciding what to inline than we
> > humans do. If you can show the compiler is doing it wrong, then we
> > might accept them.
> 
> In general I would have agreed with you especially if the methods were
> heavier than what they are:
> static inline ptrdiff_t dw_xpcs_mmio_addr_format(int dev, int reg)
> {               
>         return FIELD_PREP(0x1f0000, dev) | FIELD_PREP(0xffff, reg);
> }               
>         
> static inline u16 dw_xpcs_mmio_addr_page(ptrdiff_t csr)
> {       
>         return FIELD_GET(0x1fff00, csr);
> }       
> 
> static inline ptrdiff_t dw_xpcs_mmio_addr_offset(ptrdiff_t csr)
> {
>         return FIELD_GET(0xff, csr);
> }
> 
> > But in general, netdev does not like inline in .C
> > file.
> 
> I see. I'll do as you say if you don't change your mind after my
> reasoning below.

This isn't Andrew saying it - you seem to have missed the detail that
"netdev". If Andrew doesn't say it, then DaveM, Jakub or Paolo will.

Have you read the "Inline functions" section in
Documentation/process/4.Coding.rst ?

> > Also, nothing in MDIO is hot path, it spends a lot of time
> > waiting for a slow bus. So inline is likely to just bloat the code for
> > no gain.
> 
> I would have been absolutely with you in this matter, if we were talking
> about a normal MDIO bus. In this case the devices are accessed over
> the system IO-memory. So the bus isn't that slow.
> 
> Regarding the compiler knowing better when to inline the code. Here is
> what it does with the methods above. If the inline keyword is
> specified the compiler will inline all three methods. If the keyword isn't
> specified then dw_xpcs_mmio_addr_format() won't be inlined while the rest
> two functions will be. So the only part at consideration is the
> dw_xpcs_mmio_addr_format() method since the rest of the functions are
> inlined anyway.
> 
> The dw_xpcs_mmio_addr_format() function body is of the 5 asm
> instructions length (on MIPS). Since the function call in this case
> requires two jump instructions (to function and back), one instruction
> to save the previous return address on stack and two instructions for
> the function arguments, the trade-off of having non-inlined function
> are those five additional instructions on each call. There are four
> dw_xpcs_mmio_addr_format() calls. So here is what we get in both
> cases:
> inlined:     5 func ins * 4 calls = 20 ins
> non-inlined: (5 func + 1 jump) ins + (1 jump + 1 ra + 2 arg) ins * 4 calls  = 22 ins
> but seeing the return address needs to be saved anyway in the callers
> here is what we finally get:
> non-inlined: (5 func + 1 jump) ins + (1 jump + 2 arg) ins * 4 calls  = 18 ins
> 
> So unless I am mistaken in some of the aspects if we have the function
> non-inlined then we'll save 2 instructions in the object file, but
> each call would require additional _4_ instructions to execute (2
> jumps and 2 arg creations), which makes the function execution almost
> two times longer than it would have been should it was inlined.

Rather than just focusing on instruction count, you also need to
consider things like branch prediction, prefetching and I-cache
usage. Modern CPUs don't execute instruction-by-instruction anymore.

It is entirely possible that the compiler is making better choices
even if it results in more jumps in the code.
Andrew Lunn Dec. 7, 2023, 2:54 p.m. UTC | #6
On Thu, Dec 07, 2023 at 04:35:47PM +0300, Serge Semin wrote:
> Hi Andrew
> 
> On Wed, Dec 06, 2023 at 06:01:30PM +0100, Andrew Lunn wrote:
> > > > You shouldn't use inline in C files, only in headers.
> > > 
> > > Could you please clarify why? I failed to find such requirement in the
> > > coding style doc. Moreover there are multiple examples of using the
> > > static-inline-ers in the C files in the kernel including the net/mdio
> > > subsystem.
> > 
> 
> > The compiler does a better job at deciding what to inline than we
> > humans do. If you can show the compiler is doing it wrong, then we
> > might accept them.
> 
> In general I would have agreed with you especially if the methods were
> heavier than what they are:
> static inline ptrdiff_t dw_xpcs_mmio_addr_format(int dev, int reg)
> {               
>         return FIELD_PREP(0x1f0000, dev) | FIELD_PREP(0xffff, reg);
> }               
>         
> static inline u16 dw_xpcs_mmio_addr_page(ptrdiff_t csr)
> {       
>         return FIELD_GET(0x1fff00, csr);
> }       
> 
> static inline ptrdiff_t dw_xpcs_mmio_addr_offset(ptrdiff_t csr)
> {
>         return FIELD_GET(0xff, csr);
> }
> 
> > But in general, netdev does not like inline in .C
> > file.
> 
> I see. I'll do as you say if you don't change your mind after my
> reasoning below.
> 
> > Also, nothing in MDIO is hot path, it spends a lot of time
> > waiting for a slow bus. So inline is likely to just bloat the code for
> > no gain.
> 
> I would have been absolutely with you in this matter, if we were talking
> about a normal MDIO bus. In this case the devices are accessed over
> the system IO-memory. So the bus isn't that slow.

O.K, so its not slow. But how often is it used? PHYs tend to get
polled once a second if interrupts are not used. But is the PCS also
polled at the same time? Does this optimisation make a noticeable
difference at once per second? Do you have a requirement that the
system boots very very fast, and this optimisation makes a difference
when there is heavier activity on the PCS at boot? Is the saving
noticeable, when auto-neg takes a little over 1 second.

The best way to make your case is show real world requirements and
benchmark results.

	  Andrew
Serge Semin Dec. 8, 2023, 4:07 p.m. UTC | #7
On Thu, Dec 07, 2023 at 03:54:08PM +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2023 at 04:35:47PM +0300, Serge Semin wrote:
> > Hi Andrew
> > 
> > On Wed, Dec 06, 2023 at 06:01:30PM +0100, Andrew Lunn wrote:
> > > > > You shouldn't use inline in C files, only in headers.
> > > > 
> > > > Could you please clarify why? I failed to find such requirement in the
> > > > coding style doc. Moreover there are multiple examples of using the
> > > > static-inline-ers in the C files in the kernel including the net/mdio
> > > > subsystem.
> > > 
> > 
> > > The compiler does a better job at deciding what to inline than we
> > > humans do. If you can show the compiler is doing it wrong, then we
> > > might accept them.
> > 
> > In general I would have agreed with you especially if the methods were
> > heavier than what they are:
> > static inline ptrdiff_t dw_xpcs_mmio_addr_format(int dev, int reg)
> > {               
> >         return FIELD_PREP(0x1f0000, dev) | FIELD_PREP(0xffff, reg);
> > }               
> >         
> > static inline u16 dw_xpcs_mmio_addr_page(ptrdiff_t csr)
> > {       
> >         return FIELD_GET(0x1fff00, csr);
> > }       
> > 
> > static inline ptrdiff_t dw_xpcs_mmio_addr_offset(ptrdiff_t csr)
> > {
> >         return FIELD_GET(0xff, csr);
> > }
> > 
> > > But in general, netdev does not like inline in .C
> > > file.
> > 
> > I see. I'll do as you say if you don't change your mind after my
> > reasoning below.
> > 
> > > Also, nothing in MDIO is hot path, it spends a lot of time
> > > waiting for a slow bus. So inline is likely to just bloat the code for
> > > no gain.
> > 
> > I would have been absolutely with you in this matter, if we were talking
> > about a normal MDIO bus. In this case the devices are accessed over
> > the system IO-memory. So the bus isn't that slow.
> 

> O.K, so its not slow. But how often is it used? PHYs tend to get
> polled once a second if interrupts are not used. But is the PCS also
> polled at the same time? Does this optimisation make a noticeable
> difference at once per second? Do you have a requirement that the
> system boots very very fast, and this optimisation makes a difference
> when there is heavier activity on the PCS at boot? Is the saving
> noticeable, when auto-neg takes a little over 1 second.
> 
> The best way to make your case is show real world requirements and
> benchmark results.

You do know how to well define your point.) Polling is what currently
implemented in the XPCS driver indeed. So in this case such small
optimization won't be even noticeable. Although theoretically the
IO-access could be performed on the fast paths, in the IRQ context,
but it could be relevant only if the DW XPCS device had the IRQ
feature activated on the particular platform and the DW XPCS driver
supported it. But seeing the driver currently doesn't support it and
the DW XPCS could be also found on the DW MAC SMA MDIO bus
(non-memory-mapped case), always handling IRQ in the hardware IRQ
context would be wrong. Splitting up the handlers would be
over-complication for indeed doubtful reason, since inlining those
methods won't gain significant performance even in that case. And of
course I don't have such strict requirements as you say. So I'll drop
the inline keywords then. Thank you very much for having kept
discussing the topic in order to fully clarify it for me, even though
the issue could have seemed unimportant to spend time for.

-Serge(y)

> 
> 	  Andrew
diff mbox series

Patch

diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
index 4a7a303be2f7..39f7ce8087bf 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -185,6 +185,14 @@  config MDIO_IPQ8064
 	  This driver supports the MDIO interface found in the network
 	  interface units of the IPQ8064 SoC
 
+config MDIO_DW_XPCS
+	tristate "Synopsys DesignWare XPCS MI bus support"
+	depends on HAS_IOMEM
+	select MDIO_DEVRES
+	help
+	  This driver supports the MCI/APB3 Management Interface responsible
+	  for communicating with the Synopsys DesignWare XPCS devices.
+
 config MDIO_REGMAP
 	tristate
 	help
diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
index 1015f0db4531..6389d4c3b862 100644
--- a/drivers/net/mdio/Makefile
+++ b/drivers/net/mdio/Makefile
@@ -10,6 +10,7 @@  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
 obj-$(CONFIG_MDIO_CAVIUM)		+= mdio-cavium.o
+obj-$(CONFIG_MDIO_DW_XPCS)		+= mdio-dw-xpcs.o
 obj-$(CONFIG_MDIO_GPIO)			+= mdio-gpio.o
 obj-$(CONFIG_MDIO_HISI_FEMAC)		+= mdio-hisi-femac.o
 obj-$(CONFIG_MDIO_I2C)			+= mdio-i2c.o
diff --git a/drivers/net/mdio/mdio-dw-xpcs.c b/drivers/net/mdio/mdio-dw-xpcs.c
new file mode 100644
index 000000000000..c47f0a54d31b
--- /dev/null
+++ b/drivers/net/mdio/mdio-dw-xpcs.c
@@ -0,0 +1,384 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Synopsys DesignWare XPCS Management Interface driver
+ *
+ * Copyright (C) 2023 BAIKAL ELECTRONICS, JSC
+ */
+
+#include <linux/atomic.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mdio.h>
+#include <linux/module.h>
+#include <linux/of_mdio.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/property.h>
+#include <linux/sizes.h>
+
+/* Page select register for the indirect MMIO CSRs access */
+#define DW_VR_CSR_VIEWPORT		0xff
+
+struct dw_xpcs_mi {
+	struct platform_device *pdev;
+	struct mii_bus *bus;
+	bool reg_indir;
+	int reg_width;
+	void __iomem *reg_base;
+	struct clk *pclk;
+};
+
+static inline ptrdiff_t dw_xpcs_mmio_addr_format(int dev, int reg)
+{
+	return FIELD_PREP(0x1f0000, dev) | FIELD_PREP(0xffff, reg);
+}
+
+static inline u16 dw_xpcs_mmio_addr_page(ptrdiff_t csr)
+{
+	return FIELD_GET(0x1fff00, csr);
+}
+
+static inline ptrdiff_t dw_xpcs_mmio_addr_offset(ptrdiff_t csr)
+{
+	return FIELD_GET(0xff, csr);
+}
+
+static int dw_xpcs_mmio_read_reg_indirect(struct dw_xpcs_mi *dxmi,
+					  int dev, int reg)
+{
+	ptrdiff_t csr, ofs;
+	u16 page;
+	int ret;
+
+	csr = dw_xpcs_mmio_addr_format(dev, reg);
+	page = dw_xpcs_mmio_addr_page(csr);
+	ofs = dw_xpcs_mmio_addr_offset(csr);
+
+	ret = pm_runtime_resume_and_get(&dxmi->pdev->dev);
+	if (ret)
+		return ret;
+
+	switch (dxmi->reg_width) {
+	case 4:
+		writel(page, dxmi->reg_base + (DW_VR_CSR_VIEWPORT << 2));
+		ret = readl(dxmi->reg_base + (ofs << 2));
+		break;
+	default:
+		writew(page, dxmi->reg_base + (DW_VR_CSR_VIEWPORT << 1));
+		ret = readw(dxmi->reg_base + (ofs << 1));
+		break;
+	}
+
+	pm_runtime_put(&dxmi->pdev->dev);
+
+	return ret;
+}
+
+static int dw_xpcs_mmio_write_reg_indirect(struct dw_xpcs_mi *dxmi,
+					   int dev, int reg, u16 val)
+{
+	ptrdiff_t csr, ofs;
+	u16 page;
+	int ret;
+
+	csr = dw_xpcs_mmio_addr_format(dev, reg);
+	page = dw_xpcs_mmio_addr_page(csr);
+	ofs = dw_xpcs_mmio_addr_offset(csr);
+
+	ret = pm_runtime_resume_and_get(&dxmi->pdev->dev);
+	if (ret)
+		return ret;
+
+	switch (dxmi->reg_width) {
+	case 4:
+		writel(page, dxmi->reg_base + (DW_VR_CSR_VIEWPORT << 2));
+		writel(val, dxmi->reg_base + (ofs << 2));
+		break;
+	default:
+		writew(page, dxmi->reg_base + (DW_VR_CSR_VIEWPORT << 1));
+		writew(val, dxmi->reg_base + (ofs << 1));
+		break;
+	}
+
+	pm_runtime_put(&dxmi->pdev->dev);
+
+	return 0;
+}
+
+static int dw_xpcs_mmio_read_reg_direct(struct dw_xpcs_mi *dxmi,
+					int dev, int reg)
+{
+	ptrdiff_t csr;
+	int ret;
+
+	csr = dw_xpcs_mmio_addr_format(dev, reg);
+
+	ret = pm_runtime_resume_and_get(&dxmi->pdev->dev);
+	if (ret)
+		return ret;
+
+	switch (dxmi->reg_width) {
+	case 4:
+		ret = readl(dxmi->reg_base + (csr << 2));
+		break;
+	default:
+		ret = readw(dxmi->reg_base + (csr << 1));
+		break;
+	}
+
+	pm_runtime_put(&dxmi->pdev->dev);
+
+	return ret;
+}
+
+static int dw_xpcs_mmio_write_reg_direct(struct dw_xpcs_mi *dxmi,
+					 int dev, int reg, u16 val)
+{
+	ptrdiff_t csr;
+	int ret;
+
+	csr = dw_xpcs_mmio_addr_format(dev, reg);
+
+	ret = pm_runtime_resume_and_get(&dxmi->pdev->dev);
+	if (ret)
+		return ret;
+
+	switch (dxmi->reg_width) {
+	case 4:
+		writel(val, dxmi->reg_base + (csr << 2));
+		break;
+	default:
+		writew(val, dxmi->reg_base + (csr << 1));
+		break;
+	}
+
+	pm_runtime_put(&dxmi->pdev->dev);
+
+	return 0;
+}
+
+static int dw_xpcs_mmio_read_c22(struct mii_bus *bus, int addr, int reg)
+{
+	struct dw_xpcs_mi *dxmi = bus->priv;
+
+	if (addr != 0)
+		return -ENODEV;
+
+	if (dxmi->reg_indir)
+		return dw_xpcs_mmio_read_reg_indirect(dxmi, MDIO_MMD_VEND2, reg);
+	else
+		return dw_xpcs_mmio_read_reg_direct(dxmi, MDIO_MMD_VEND2, reg);
+}
+
+static int dw_xpcs_mmio_write_c22(struct mii_bus *bus, int addr, int reg, u16 val)
+{
+	struct dw_xpcs_mi *dxmi = bus->priv;
+
+	if (addr != 0)
+		return -ENODEV;
+
+	if (dxmi->reg_indir)
+		return dw_xpcs_mmio_write_reg_indirect(dxmi, MDIO_MMD_VEND2, reg, val);
+	else
+		return dw_xpcs_mmio_write_reg_direct(dxmi, MDIO_MMD_VEND2, reg, val);
+}
+
+static int dw_xpcs_mmio_read_c45(struct mii_bus *bus, int addr, int dev, int reg)
+{
+	struct dw_xpcs_mi *dxmi = bus->priv;
+
+	if (addr != 0)
+		return -ENODEV;
+
+	if (dxmi->reg_indir)
+		return dw_xpcs_mmio_read_reg_indirect(dxmi, dev, reg);
+	else
+		return dw_xpcs_mmio_read_reg_direct(dxmi, dev, reg);
+}
+
+static int dw_xpcs_mmio_write_c45(struct mii_bus *bus, int addr, int dev,
+				  int reg, u16 val)
+{
+	struct dw_xpcs_mi *dxmi = bus->priv;
+
+	if (addr != 0)
+		return -ENODEV;
+
+	if (dxmi->reg_indir)
+		return dw_xpcs_mmio_write_reg_indirect(dxmi, dev, reg, val);
+	else
+		return dw_xpcs_mmio_write_reg_direct(dxmi, dev, reg, val);
+}
+
+static struct dw_xpcs_mi *dw_xpcs_mi_create_data(struct platform_device *pdev)
+{
+	struct dw_xpcs_mi *dxmi;
+
+	dxmi = devm_kzalloc(&pdev->dev, sizeof(*dxmi), GFP_KERNEL);
+	if (!dxmi)
+		return ERR_PTR(-ENOMEM);
+
+	dxmi->pdev = pdev;
+
+	dev_set_drvdata(&pdev->dev, dxmi);
+
+	return dxmi;
+}
+
+static int dw_xpcs_mi_init_res(struct dw_xpcs_mi *dxmi)
+{
+	struct device *dev = &dxmi->pdev->dev;
+	struct resource *res;
+
+	if (!device_property_read_u32(dev, "reg-io-width", &dxmi->reg_width)) {
+		if (dxmi->reg_width != 2 && dxmi->reg_width != 4) {
+			dev_err(dev, "Invalid regspace data width\n");
+			return -EINVAL;
+		}
+	} else {
+		dxmi->reg_width = 2;
+	}
+
+	res = platform_get_resource_byname(dxmi->pdev, IORESOURCE_MEM, "direct") ?:
+	      platform_get_resource_byname(dxmi->pdev, IORESOURCE_MEM, "indirect");
+	if (!res) {
+		dev_err(dev, "No regspace found\n");
+		return -EINVAL;
+	}
+
+	if (!strcmp(res->name, "indirect"))
+		dxmi->reg_indir = true;
+
+	if ((dxmi->reg_indir && resource_size(res) < dxmi->reg_width * SZ_256) ||
+	    (!dxmi->reg_indir && resource_size(res) < dxmi->reg_width * SZ_2M)) {
+		dev_err(dev, "Invalid regspace size\n");
+		return -EINVAL;
+	}
+
+	dxmi->reg_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(dxmi->reg_base)) {
+		dev_err(dev, "Failed to map regspace\n");
+		return PTR_ERR(dxmi->reg_base);
+	}
+
+	return 0;
+}
+
+static int dw_xpcs_mi_init_clk(struct dw_xpcs_mi *dxmi)
+{
+	struct device *dev = &dxmi->pdev->dev;
+	int ret;
+
+	dxmi->pclk = devm_clk_get_optional(dev, "pclk");
+        if (IS_ERR(dxmi->pclk))
+		return dev_err_probe(dev, PTR_ERR(dxmi->pclk),
+				     "Failed to get ref clock\n");
+
+	pm_runtime_set_active(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret) {
+		dev_err(dev, "Failed to enable runtime-PM\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int dw_xpcs_mi_init_mdio(struct dw_xpcs_mi *dxmi)
+{
+	struct device *dev = &dxmi->pdev->dev;
+	static atomic_t id = ATOMIC_INIT(-1);
+	int ret;
+
+	dxmi->bus = devm_mdiobus_alloc_size(dev, 0);
+	if (!dxmi->bus)
+		return -ENOMEM;
+
+	dxmi->bus->name = "DW XPCS MI";
+	dxmi->bus->read = dw_xpcs_mmio_read_c22;
+	dxmi->bus->write = dw_xpcs_mmio_write_c22;
+	dxmi->bus->read_c45 = dw_xpcs_mmio_read_c45;
+	dxmi->bus->write_c45 = dw_xpcs_mmio_write_c45;
+	dxmi->bus->phy_mask = ~0;
+	dxmi->bus->parent = dev;
+	dxmi->bus->priv = dxmi;
+
+	snprintf(dxmi->bus->id, MII_BUS_ID_SIZE,
+		 "dwxpcs-%x", atomic_inc_return(&id));
+
+	ret = devm_of_mdiobus_register(dev, dxmi->bus, dev_of_node(dev));
+	if (ret) {
+		dev_err(dev, "Failed to create MDIO bus\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int dw_xpcs_mi_probe(struct platform_device *pdev)
+{
+	struct dw_xpcs_mi *dxmi;
+	int ret;
+
+	dxmi = dw_xpcs_mi_create_data(pdev);
+	if (IS_ERR(dxmi))
+		return PTR_ERR(dxmi);
+
+	ret = dw_xpcs_mi_init_res(dxmi);
+	if (ret)
+		return ret;
+
+	ret = dw_xpcs_mi_init_clk(dxmi);
+	if (ret)
+		return ret;
+
+	ret = dw_xpcs_mi_init_mdio(dxmi);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int __maybe_unused dw_xpcs_mi_pm_runtime_suspend(struct device *dev)
+{
+	struct dw_xpcs_mi *dxmi = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(dxmi->pclk);
+
+	return 0;
+}
+
+static int __maybe_unused dw_xpcs_mi_pm_runtime_resume(struct device *dev)
+{
+	struct dw_xpcs_mi *dxmi = dev_get_drvdata(dev);
+
+	return clk_prepare_enable(dxmi->pclk);
+}
+
+const struct dev_pm_ops dw_xpcs_mi_pm_ops = {
+        SET_RUNTIME_PM_OPS(dw_xpcs_mi_pm_runtime_suspend, dw_xpcs_mi_pm_runtime_resume, NULL)
+};
+
+static const struct of_device_id dw_xpcs_mi_of_ids[] = {
+	{ .compatible = "snps,dw-xpcs-mi" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, dw_xpcs_mi_of_ids);
+
+static struct platform_driver dw_xpcs_mi_driver = {
+	.probe = dw_xpcs_mi_probe,
+	.driver = {
+		.name = "dw-xpcs-mi",
+		.pm = &dw_xpcs_mi_pm_ops,
+		.of_match_table = dw_xpcs_mi_of_ids,
+	},
+};
+
+module_platform_driver(dw_xpcs_mi_driver);
+
+MODULE_DESCRIPTION("Synopsys DesignWare XPCS Management Interface driver");
+MODULE_AUTHOR("Serge Semin <Sergey.Semin@baikalelectronics.ru>");
+MODULE_LICENSE("GPL v2");